fix(db): purge orphan proxy_assignments when deleting provider connections (#9246)

Validated in local merge-train (devbox-vm-06-dev002) @ combined-tip (FAST gates: typecheck/complexity/cognitive/changelog/vitest — only pre-existing audit.test.ts flake). Evidence: /home/diegosouzapw/dev/proxys/OmniRoute/.claude/worktrees/merge-train-20260805-222248-suite.log

changelog.d/fixes/9246-purge-orphan-proxy-assignments.md

@@ -0,0 +1 @@
+- **fix(db):** deleting a provider connection (single, batch, or provider-scoped) now purges its account-scoped `proxy_assignments` rows inside an atomic transaction — no more orphan assignments pointing at deleted connections ([#9246](https://github.com/diegosouzapw/OmniRoute/pull/9246)) — thanks @maxmad64bis

src/lib/db/providers.ts

@@ -12,6 +12,7 @@ import {
 } from "./encryption";
 import { createLazyRowProxy } from "./providers/lazyConnectionView";
 import { invalidateDbCache, getCachedRawProviderConnections } from "./readCache";
+import { reorderConnections } from "./providers/deletion";
 import {
   removeConnectionHealth,
   removeConnectionIndex,
@@ -119,7 +120,7 @@ export async function getProviderConnections(
   filter: JsonRecord = {},
   limit?: number,
   offset?: number,
-  columns?: string[],
+  columns?: string[]
 ) {
   const useCache = !columns?.length && limit === undefined && offset === undefined;
   const raw = useCache
@@ -145,7 +146,7 @@ export async function getRawProviderConnections(
   filter: JsonRecord = {},
   limit?: number,
   offset?: number,
-  columns?: string[],
+  columns?: string[]
 ) {
   const db = getDbInstance() as unknown as DbLike;
   let selectCols = "*";
@@ -177,8 +178,6 @@ export async function getRawProviderConnections(
     params.authType = filter.authType;
   }
 
-
-
   if (conditions.length > 0) {
     sql += " WHERE " + conditions.join(" AND ");
   }
@@ -544,7 +543,7 @@ export async function createProviderConnection(data: JsonRecord) {
   _insertConnectionRow(db, encryptConnectionFields({ ...connection }));
   const providerId = toStringOrNull(data.provider);
   if (providerId) {
-    _reorderConnections(db, providerId);
+    reorderConnections(db, providerId);
   }
   backupDbFile("pre-write");
   invalidateDbCache("connections"); // Bust connections read cache
@@ -771,7 +770,7 @@ export async function updateProviderConnection(id: string, data: JsonRecord) {
       typeof existingRecord.provider === "string"
         ? existingRecord.provider
         : String(existingRecord.provider || "");
-    _reorderConnections(db, providerId);
+    reorderConnections(db, providerId);
   }
 
   return withNullableRateLimitOverrides(
@@ -898,99 +897,6 @@ export async function resetConnectionBackoff(id: string): Promise<void> {
   bumpProxyConfigGeneration();
 }
 
-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);
-  if (!existing) return false;
-
-  db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?").run(id);
-  db.prepare("DELETE FROM provider_connections WHERE id = ?").run(id);
-  removeConnectionHealth(id);
-  removeConnectionIndex(id);
-  bumpProxyConfigGeneration();
-  const existingRecord = toRecord(existing);
-  const providerId =
-    typeof existingRecord.provider === "string"
-      ? existingRecord.provider
-      : String(existingRecord.provider || "");
-  _reorderConnections(db, providerId);
-  backupDbFile("pre-write");
-  invalidateDbCache("connections"); // Bust connections read cache
-  invalidateReasoningRoutingRuleCache();
-  return true;
-}
-
-export async function deleteProviderConnections(ids: string[]): Promise<number> {
-  if (ids.length === 0) return 0;
-  const db = getDbInstance();
-
-  const deletedCount = db.transaction(() => {
-    const placeholders = ids.map(() => "?").join(",");
-    db.prepare(`DELETE FROM quota_snapshots WHERE connection_id IN (${placeholders})`).run(...ids);
-    const result = db
-      .prepare(`DELETE FROM provider_connections WHERE id IN (${placeholders})`)
-      .run(...ids);
-    return result.changes ?? 0;
-  })();
-
-  for (const id of ids) {
-    removeConnectionHealth(id);
-    removeConnectionIndex(id);
-  }
-  backupDbFile("pre-write");
-  invalidateDbCache("connections");
-  invalidateReasoningRoutingRuleCache();
-  return deletedCount;
-}
-
-export async function deleteProviderConnectionsByProvider(providerId: string) {
-  const db = getDbInstance() as unknown as DbLike;
-  const connectionIds = db
-    .prepare("SELECT id FROM provider_connections WHERE provider = ?")
-    .all(providerId)
-    .map((row) => {
-      const record = toRecord(row);
-      return typeof record.id === "string" ? record.id : null;
-    })
-    .filter((id): id is string => id !== null);
-
-  if (connectionIds.length > 0) {
-    const deleteSnapshots = db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?");
-    for (const connectionId of connectionIds) {
-      deleteSnapshots.run(connectionId);
-    }
-  }
-
-  const result = db.prepare("DELETE FROM provider_connections WHERE provider = ?").run(providerId);
-  for (const connectionId of connectionIds) {
-    removeConnectionHealth(connectionId);
-    removeConnectionIndex(connectionId);
-  }
-  backupDbFile("pre-write");
-  invalidateDbCache("connections");
-  invalidateReasoningRoutingRuleCache();
-  return result.changes;
-}
-
-export async function reorderProviderConnections(providerId: string) {
-  const db = getDbInstance() as unknown as DbLike;
-  _reorderConnections(db, providerId);
-}
-
-function _reorderConnections(db: DbLike, providerId: string) {
-  const rows = db
-    .prepare(
-      "SELECT id, priority, updated_at FROM provider_connections WHERE provider = ? ORDER BY priority ASC, updated_at DESC"
-    )
-    .all(providerId);
-
-  const update = db.prepare("UPDATE provider_connections SET priority = ? WHERE id = ?");
-  rows.forEach((row, index) => {
-    const current = toRecord(row);
-    update.run(index + 1, current.id);
-  });
-}
-
 export async function cleanupProviderConnections() {
   return 0;
 }
@@ -1005,10 +911,13 @@ export async function getDistinctGroups(): Promise<string[]> {
   return rows.map((r) => String(r.group ?? "")).filter(Boolean);
 }
 
+export { autoMigrateLegacyEncryptedConnections, getGheCopilotHosts } from "./providers/migrations";
 export {
-  autoMigrateLegacyEncryptedConnections,
-  getGheCopilotHosts,
-} from "./providers/migrations";
+  deleteProviderConnection,
+  deleteProviderConnections,
+  deleteProviderConnectionsByProvider,
+  reorderProviderConnections,
+} from "./providers/deletion";
 
 // ──────────────── Re-exports from leaf modules ────────────────
 

src/lib/db/providers/deletion.ts

@@ -0,0 +1,141 @@
+/**
+ * db/providers/deletion.ts — Provider connection deletion & reordering.
+ *
+ * Extracted from db/providers.ts (god-file shrink): the three provider
+ * connection delete paths plus the connection reorder helper they share.
+ * Each delete path also purges the connection's account-scoped
+ * proxy_assignments (#9232) so orphans never keep routing to removed
+ * connections.
+ */
+
+import { getDbInstance } from "../core";
+import { backupDbFile } from "../backup";
+import {
+  removeConnectionHealth,
+  removeConnectionIndex,
+} from "@omniroute/open-sse/services/apiKeyRotator.ts";
+import { invalidateDbCache } from "../readCache";
+import { invalidateReasoningRoutingRuleCache } from "../reasoningRoutingRules";
+import { bumpProxyConfigGeneration } from "../settings";
+import { toRecord } from "./columns";
+
+interface StatementLike<TRow = unknown> {
+  all: (...params: unknown[]) => TRow[];
+  get: (...params: unknown[]) => TRow | undefined;
+  run: (...params: unknown[]) => { changes?: number };
+}
+
+interface DbLike {
+  prepare: <TRow = unknown>(sql: string) => StatementLike<TRow>;
+  transaction: <T>(fn: () => T) => () => T;
+}
+
+// Purge account-scoped proxy_assignments for the deleted connections (#9232):
+// orphan assignments otherwise keep routing to provider_connections that no
+// longer exist. scope='account' lives in exactly this one place.
+function _deleteAccountProxyAssignments(db: DbLike, ids: string[]) {
+  if (ids.length === 0) return;
+  const placeholders = ids.map(() => "?").join(",");
+  db.prepare(
+    `DELETE FROM proxy_assignments WHERE scope = 'account' AND scope_id IN (${placeholders})`
+  ).run(...ids);
+}
+
+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);
+  if (!existing) return false;
+
+  db.transaction(() => {
+    _deleteAccountProxyAssignments(db, [id]);
+    db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?").run(id);
+    db.prepare("DELETE FROM provider_connections WHERE id = ?").run(id);
+  })();
+  removeConnectionHealth(id);
+  removeConnectionIndex(id);
+  bumpProxyConfigGeneration();
+  const existingRecord = toRecord(existing);
+  const providerId =
+    typeof existingRecord.provider === "string"
+      ? existingRecord.provider
+      : String(existingRecord.provider || "");
+  reorderConnections(db, providerId);
+  backupDbFile("pre-write");
+  invalidateDbCache("connections"); // Bust connections read cache
+  invalidateReasoningRoutingRuleCache();
+  return true;
+}
+
+export async function deleteProviderConnections(ids: string[]): Promise<number> {
+  if (ids.length === 0) return 0;
+  const db = getDbInstance() as unknown as DbLike;
+
+  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;
+  })();
+
+  for (const id of ids) {
+    removeConnectionHealth(id);
+    removeConnectionIndex(id);
+  }
+  backupDbFile("pre-write");
+  invalidateDbCache("connections");
+  invalidateReasoningRoutingRuleCache();
+  return deletedCount;
+}
+
+export async function deleteProviderConnectionsByProvider(providerId: string) {
+  const db = getDbInstance() as unknown as DbLike;
+  const connectionIds = db
+    .prepare("SELECT id FROM provider_connections WHERE provider = ?")
+    .all(providerId)
+    .map((row) => {
+      const record = toRecord(row);
+      return typeof record.id === "string" ? record.id : null;
+    })
+    .filter((id): id is string => id !== null);
+
+  const result = db.transaction(() => {
+    if (connectionIds.length > 0) {
+      const deleteSnapshots = db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?");
+      for (const connectionId of connectionIds) {
+        deleteSnapshots.run(connectionId);
+      }
+      _deleteAccountProxyAssignments(db, connectionIds);
+    }
+    return db.prepare("DELETE FROM provider_connections WHERE provider = ?").run(providerId);
+  })();
+  for (const connectionId of connectionIds) {
+    removeConnectionHealth(connectionId);
+    removeConnectionIndex(connectionId);
+  }
+  backupDbFile("pre-write");
+  invalidateDbCache("connections");
+  invalidateReasoningRoutingRuleCache();
+  return result.changes;
+}
+
+export async function reorderProviderConnections(providerId: string) {
+  const db = getDbInstance() as unknown as DbLike;
+  reorderConnections(db, providerId);
+}
+
+export function reorderConnections(db: DbLike, providerId: string) {
+  const rows = db
+    .prepare(
+      "SELECT id, priority, updated_at FROM provider_connections WHERE provider = ? ORDER BY priority ASC, updated_at DESC"
+    )
+    .all(providerId);
+
+  const update = db.prepare("UPDATE provider_connections SET priority = ? WHERE id = ?");
+  rows.forEach((row, index) => {
+    const current = toRecord(row);
+    update.run(index + 1, current.id);
+  });
+}

tests/unit/9232-purge-proxy-assignments-on-delete.test.ts

@@ -0,0 +1,113 @@
+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-9232-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const TEST_PROVIDER = "__test_provider_9232__";
+
+const core = await import("../../src/lib/db/core.ts");
+const providersDb = await import("../../src/lib/db/providers.ts");
+const proxiesDb = await import("../../src/lib/db/proxies.ts");
+
+async function resetStorage() {
+  core.resetDbInstance();
+  for (let attempt = 0; attempt < 10; attempt++) {
+    try {
+      if (fs.existsSync(TEST_DATA_DIR)) fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+      break;
+    } catch (error: unknown) {
+      const code = (error as { code?: string } | undefined)?.code;
+      if ((code === "EBUSY" || code === "EPERM") && attempt < 9) {
+        await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1)));
+      } else throw error;
+    }
+  }
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(async () => {
+  await resetStorage();
+});
+test.after(async () => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+async function setupConnectionWithAssignment() {
+  const created = await providersDb.createProviderConnection({
+    provider: TEST_PROVIDER,
+    authType: "apikey",
+    name: `Test conn ${Date.now()}-${Math.random().toString(36).slice(2, 7)}`,
+    apiKey: `sk-test-9232-${Math.random().toString(36).slice(2, 9)}`,
+  });
+  assert.ok(created?.id, "connection must be created");
+  const proxy = await proxiesDb.createProxy({
+    name: "Test proxy 9232",
+    type: "http",
+    host: "proxy.local",
+    port: 8080,
+  });
+  assert.ok(proxy?.id, "proxy must be created");
+  const assignment = await proxiesDb.assignProxyToScope("account", created.id, proxy.id);
+  assert.ok(assignment, "assignment must be created");
+  return created as { id: string };
+}
+
+async function getAccountAssignmentsFor(connectionId: string) {
+  const all = await proxiesDb.getProxyAssignments({ scope: "account" });
+  return all.filter((a) => a.scopeId === connectionId);
+}
+
+test("#9232: deleteProviderConnection purges account proxy_assignments for the deleted connection", async () => {
+  const conn = await setupConnectionWithAssignment();
+  const before = await getAccountAssignmentsFor(conn.id);
+  assert.equal(before.length, 1, "sanity: assignment exists before delete");
+
+  const deleted = await providersDb.deleteProviderConnection(conn.id);
+  assert.equal(deleted, true, "connection row must be deleted");
+
+  const after = await getAccountAssignmentsFor(conn.id);
+  assert.equal(after.length, 0, "EXPECTED: proxy_assignments rows purged with the connection");
+});
+
+test("#9232: deleteProviderConnections purges account proxy_assignments for the whole batch", async () => {
+  const connA = await setupConnectionWithAssignment();
+  const connB = await setupConnectionWithAssignment();
+  assert.equal((await getAccountAssignmentsFor(connA.id)).length, 1);
+  assert.equal((await getAccountAssignmentsFor(connB.id)).length, 1);
+
+  const deleted = await providersDb.deleteProviderConnections([connA.id, connB.id]);
+  assert.equal(deleted, 2, "both connections deleted");
+
+  assert.equal((await getAccountAssignmentsFor(connA.id)).length, 0);
+  assert.equal((await getAccountAssignmentsFor(connB.id)).length, 0);
+});
+
+test("#9232: deleteProviderConnectionsByProvider purges account proxy_assignments for all provider connections", async () => {
+  const connA = await setupConnectionWithAssignment();
+  const connB = await setupConnectionWithAssignment();
+  assert.equal((await getAccountAssignmentsFor(connA.id)).length, 1);
+  assert.equal((await getAccountAssignmentsFor(connB.id)).length, 1);
+
+  const deleted = await providersDb.deleteProviderConnectionsByProvider(TEST_PROVIDER);
+  assert.equal(deleted, 2, "both provider connections deleted");
+
+  assert.equal((await getAccountAssignmentsFor(connA.id)).length, 0);
+  assert.equal((await getAccountAssignmentsFor(connB.id)).length, 0);
+});
+
+test("#9232: deleteProviderConnections with an empty batch is a no-op and stays valid SQL", async () => {
+  const deleted = await providersDb.deleteProviderConnections([]);
+  assert.equal(deleted, 0, "empty batch deletes nothing");
+});
+
+test("#9232: deleteProviderConnectionsByProvider with no matching connections purges nothing and returns 0", async () => {
+  const deleted = await providersDb.deleteProviderConnectionsByProvider(
+    "__no_such_provider_9232__"
+  );
+  assert.equal(deleted, 0, "no provider connections matched, nothing to delete");
+});