diff --git a/changelog.d/fixes/9719-combo-connection-pins.md b/changelog.d/fixes/9719-combo-connection-pins.md new file mode 100644 index 0000000000..79c3973ee0 --- /dev/null +++ b/changelog.d/fixes/9719-combo-connection-pins.md @@ -0,0 +1 @@ +- fix(db): clear stale combo connection pins when provider connections are deleted (#9719) diff --git a/src/app/api/providers/[id]/route.ts b/src/app/api/providers/[id]/route.ts index 460c3d1c8f..c40f15448c 100644 --- a/src/app/api/providers/[id]/route.ts +++ b/src/app/api/providers/[id]/route.ts @@ -25,7 +25,6 @@ import { import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { isApiKeyRevealEnabled, maskStoredApiKey } from "@/lib/apiKeyExposure"; import { cleanupProviderModelsAfterConnectionDelete } from "@/lib/db/models"; -import { cleanupComboConnectionRefs } from "@/lib/db/combos"; import { refreshConnectionRateLimits, enableRateLimitProtection, @@ -367,13 +366,6 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i console.error(`Failed to clean up models for deleted ${connection.provider} connection:`, e); } - // Remove stale connectionId references from combo route steps. - try { - await cleanupComboConnectionRefs(id); - } catch (e) { - console.error("Failed to clean up combo route refs for deleted connection:", e); - } - // Auto sync to Cloud if enabled await syncToCloudIfEnabled(); diff --git a/src/lib/db/combos.ts b/src/lib/db/combos.ts index 6cdebbd85b..7c050084cd 100644 --- a/src/lib/db/combos.ts +++ b/src/lib/db/combos.ts @@ -96,39 +96,60 @@ export function setActiveCombo(name: string, db = getDbInstance()): void { * Called after a provider connection is removed so combo routes don't carry * stale references. */ -export async function cleanupComboConnectionRefs(connectionId: string) { +export async function cleanupComboConnectionRefs(connectionIds: string | string[]) { + const deletedConnectionIds = new Set( + (Array.isArray(connectionIds) ? connectionIds : [connectionIds]).filter(Boolean) + ); + + if (deletedConnectionIds.size === 0) return 0; + const combos = await getCombos(); let touched = 0; + for (const combo of combos) { if (!Array.isArray(combo.models)) continue; + let changed = false; + const models = (combo.models as unknown as Record[]).map((step) => { let out = step; - if (out.connectionId === connectionId) { + + if (typeof out.connectionId === "string" && deletedConnectionIds.has(out.connectionId)) { const { connectionId: _, ...rest } = out; out = rest; changed = true; } + if (Array.isArray(out.allowedConnectionIds)) { const filtered = out.allowedConnectionIds.filter( - (id: string) => id !== connectionId + (id) => typeof id !== "string" || !deletedConnectionIds.has(id) ); + if (filtered.length !== out.allowedConnectionIds.length) { - out = { ...out, allowedConnectionIds: filtered }; + out = { + ...out, + allowedConnectionIds: filtered, + }; changed = true; } } + return out; }); + if (changed && typeof combo.id === "string") { try { const { id, ...rest } = combo; - await updateCombo(combo.id, { ...rest, models }); + await updateCombo(combo.id, { + ...rest, + models, + }); touched++; } catch { // One combo failing should not block cleanup of the rest. } } } + return touched; } diff --git a/src/lib/db/providers/deletion.ts b/src/lib/db/providers/deletion.ts index 38848e30df..44624fb8b9 100644 --- a/src/lib/db/providers/deletion.ts +++ b/src/lib/db/providers/deletion.ts @@ -10,6 +10,7 @@ import { getDbInstance } from "../core"; import { backupDbFile } from "../backup"; +import { cleanupComboConnectionRefs } from "../combos"; import { removeConnectionHealth, removeConnectionIndex, @@ -41,6 +42,29 @@ function _deleteAccountProxyAssignments(db: DbLike, ids: string[]) { ).run(...ids); } +function _selectExistingConnectionIds(db: DbLike, ids: string[]): string[] { + if (ids.length === 0) return []; + + const placeholders = ids.map(() => "?").join(","); + + return db + .prepare(`SELECT id FROM provider_connections WHERE id IN (${placeholders})`) + .all(...ids) + .map((row) => { + const record = toRecord(row); + return typeof record.id === "string" ? record.id : null; + }) + .filter((id): id is string => id !== null); +} + +async function _cleanupDeletedComboConnectionRefs(connectionIds: string | string[]): Promise { + try { + await cleanupComboConnectionRefs(connectionIds); + } catch (error) { + console.error("Failed to clean up combo route refs for deleted connections:", error); + } +} + export async function deleteProviderConnection(id: string) { const db = getDbInstance() as unknown as DbLike; const existing = db.prepare("SELECT provider FROM provider_connections WHERE id = ?").get(id); @@ -51,6 +75,9 @@ export async function deleteProviderConnection(id: string) { db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?").run(id); db.prepare("DELETE FROM provider_connections WHERE id = ?").run(id); })(); + + await _cleanupDeletedComboConnectionRefs(id); + removeConnectionHealth(id); removeConnectionIndex(id); bumpProxyConfigGeneration(); @@ -68,25 +95,35 @@ export async function deleteProviderConnection(id: string) { export async function deleteProviderConnections(ids: string[]): Promise { if (ids.length === 0) return 0; + const db = getDbInstance() as unknown as DbLike; + const existingIds = _selectExistingConnectionIds(db, ids); const deletedCount = db.transaction(() => { const placeholders = ids.map(() => "?").join(","); + db.prepare(`DELETE FROM quota_snapshots WHERE connection_id IN (${placeholders})`).run(...ids); + _deleteAccountProxyAssignments(db, ids); + const result = db .prepare(`DELETE FROM provider_connections WHERE id IN (${placeholders})`) .run(...ids); + return result.changes ?? 0; })(); + await _cleanupDeletedComboConnectionRefs(existingIds); + for (const id of ids) { removeConnectionHealth(id); removeConnectionIndex(id); } + backupDbFile("pre-write"); invalidateDbCache("connections"); invalidateReasoningRoutingRuleCache(); + return deletedCount; } @@ -111,6 +148,9 @@ export async function deleteProviderConnectionsByProvider(providerId: string) { } return db.prepare("DELETE FROM provider_connections WHERE provider = ?").run(providerId); })(); + + await _cleanupDeletedComboConnectionRefs(connectionIds); + for (const connectionId of connectionIds) { removeConnectionHealth(connectionId); removeConnectionIndex(connectionId); diff --git a/tests/unit/delete-provider-connection-clears-combo-pins-8887.test.ts b/tests/unit/delete-provider-connection-clears-combo-pins-8887.test.ts new file mode 100644 index 0000000000..b38a81c8d0 --- /dev/null +++ b/tests/unit/delete-provider-connection-clears-combo-pins-8887.test.ts @@ -0,0 +1,221 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-combo-pins-8887-")); + +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const combosDb = await import("../../src/lib/db/combos.ts"); + +type JsonRecord = Record; + +async function resetStorage(): Promise { + core.resetDbInstance(); + + for (let attempt = 0; attempt < 10; attempt++) { + try { + fs.rmSync(TEST_DATA_DIR, { + recursive: true, + force: true, + }); + break; + } catch (error: unknown) { + const code = + error && typeof error === "object" && "code" in error + ? String((error as { code?: unknown }).code) + : ""; + + if ((code === "EBUSY" || code === "EPERM") && attempt < 9) { + await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1))); + continue; + } + + throw error; + } + } + + fs.mkdirSync(TEST_DATA_DIR, { + recursive: true, + }); +} + +async function createConnection(provider: string, name: string): Promise { + const connection = await providersDb.createProviderConnection({ + provider, + authType: "apikey", + name, + apiKey: `test-key-${name}`, + }); + + assert.equal(typeof connection.id, "string", "provider fixture must return a connection id"); + + return connection.id as string; +} + +async function createPinnedCombo(name: string, models: JsonRecord[]): Promise { + await combosDb.createCombo({ + name, + strategy: "priority", + models, + }); +} + +async function readModels(name: string): Promise { + const combo = await combosDb.getComboByName(name); + + assert.ok(combo, `combo ${name} must still exist`); + assert.ok(Array.isArray(combo.models), `combo ${name} must retain a models array`); + + return combo.models as JsonRecord[]; +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(() => { + core.resetDbInstance(); + + fs.rmSync(TEST_DATA_DIR, { + recursive: true, + force: true, + }); +}); + +test("#8887: single connection delete clears only matching combo pins", async () => { + const doomedId = await createConnection("openai", "single-doomed"); + const survivorId = await createConnection("openai", "single-survivor"); + + await createPinnedCombo("single-delete-8887", [ + { + provider: "openai", + model: "gpt-5.6-sol", + connectionId: doomedId, + allowedConnectionIds: [doomedId, survivorId], + }, + { + provider: "openai", + model: "gpt-5.6-sol", + connectionId: survivorId, + }, + ]); + + assert.equal(await providersDb.deleteProviderConnection(doomedId), true); + + const models = await readModels("single-delete-8887"); + + assert.equal( + "connectionId" in models[0], + false, + "single delete must remove the deleted direct connection pin" + ); + assert.deepEqual( + models[0].allowedConnectionIds, + [survivorId], + "single delete must remove the deleted id from allowedConnectionIds" + ); + assert.equal( + models[1].connectionId, + survivorId, + "single delete must preserve surviving connection pins" + ); +}); + +test("#8887: bulk connection delete clears every matching combo pin", async () => { + const doomedA = await createConnection("anthropic", "bulk-doomed-a"); + const doomedB = await createConnection("anthropic", "bulk-doomed-b"); + const survivorId = await createConnection("anthropic", "bulk-survivor"); + + await createPinnedCombo("bulk-delete-8887", [ + { + provider: "anthropic", + model: "claude-sonnet-5", + connectionId: doomedA, + allowedConnectionIds: [doomedA, survivorId], + }, + { + provider: "anthropic", + model: "claude-sonnet-5", + connectionId: doomedB, + allowedConnectionIds: [doomedB, survivorId], + }, + { + provider: "anthropic", + model: "claude-sonnet-5", + connectionId: survivorId, + }, + ]); + + assert.equal(await providersDb.deleteProviderConnections([doomedA, doomedB]), 2); + + const models = await readModels("bulk-delete-8887"); + + assert.equal( + "connectionId" in models[0], + false, + "bulk delete must remove the first deleted direct pin" + ); + assert.equal( + "connectionId" in models[1], + false, + "bulk delete must remove the second deleted direct pin" + ); + assert.deepEqual(models[0].allowedConnectionIds, [survivorId]); + assert.deepEqual(models[1].allowedConnectionIds, [survivorId]); + assert.equal(models[2].connectionId, survivorId); +}); + +test("#8887: provider-wide delete clears that provider's combo pins only", async () => { + const doomedA = await createConnection("nvidia", "provider-doomed-a"); + const doomedB = await createConnection("nvidia", "provider-doomed-b"); + const otherProviderId = await createConnection("cerebras", "provider-survivor"); + + await createPinnedCombo("provider-delete-8887", [ + { + provider: "nvidia", + model: "z-ai/glm-5.2", + connectionId: doomedA, + allowedConnectionIds: [doomedA, doomedB, otherProviderId], + }, + { + provider: "nvidia", + model: "deepseek-ai/deepseek-v4-pro", + connectionId: doomedB, + }, + { + provider: "cerebras", + model: "zai-glm-4.7", + connectionId: otherProviderId, + }, + ]); + + assert.equal(await providersDb.deleteProviderConnectionsByProvider("nvidia"), 2); + + const models = await readModels("provider-delete-8887"); + + assert.equal( + "connectionId" in models[0], + false, + "provider delete must remove its first deleted direct pin" + ); + assert.equal( + "connectionId" in models[1], + false, + "provider delete must remove its second deleted direct pin" + ); + assert.deepEqual( + models[0].allowedConnectionIds, + [otherProviderId], + "provider delete must preserve ids belonging to other providers" + ); + assert.equal( + models[2].connectionId, + otherProviderId, + "provider delete must preserve another provider's direct pin" + ); +});