From ee0073dc4a77836c3a1a7436ddc3f504320df84b Mon Sep 17 00:00:00 2001 From: Aman <1402357+Zartharas@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:54:59 -0600 Subject: [PATCH] fix(db): invalidate stale LKGP pins on connection delete (#9936) --- .../fixes/8887-lkgp-connection-delete.md | 1 + src/lib/db/providers/deletion.ts | 15 ++ src/lib/db/readCache.ts | 8 + src/lib/db/settings/lkgp.ts | 50 ++++++ ...r-connection-invalidates-lkgp-8887.test.ts | 169 ++++++++++++++++++ 5 files changed, 243 insertions(+) create mode 100644 changelog.d/fixes/8887-lkgp-connection-delete.md create mode 100644 tests/unit/delete-provider-connection-invalidates-lkgp-8887.test.ts diff --git a/changelog.d/fixes/8887-lkgp-connection-delete.md b/changelog.d/fixes/8887-lkgp-connection-delete.md new file mode 100644 index 0000000000..f00dbb655e --- /dev/null +++ b/changelog.d/fixes/8887-lkgp-connection-delete.md @@ -0,0 +1 @@ +- fix(db): invalidate stale LKGP pins when provider connections are deleted (#8887) diff --git a/src/lib/db/providers/deletion.ts b/src/lib/db/providers/deletion.ts index 44624fb8b9..5ae9cf9ea6 100644 --- a/src/lib/db/providers/deletion.ts +++ b/src/lib/db/providers/deletion.ts @@ -11,6 +11,7 @@ import { getDbInstance } from "../core"; import { backupDbFile } from "../backup"; import { cleanupComboConnectionRefs } from "../combos"; +import { deleteLKGPByConnectionIds } from "../settings/lkgp"; import { removeConnectionHealth, removeConnectionIndex, @@ -65,6 +66,17 @@ async function _cleanupDeletedComboConnectionRefs(connectionIds: string | string } } +async function _cleanupDeletedLKGPConnectionRefs(connectionIds: string | string[]): Promise { + const ids = Array.isArray(connectionIds) ? connectionIds : [connectionIds]; + if (ids.length === 0) return; + + try { + await deleteLKGPByConnectionIds(ids); + } catch (error) { + console.error("Failed to clean up LKGP 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); @@ -77,6 +89,7 @@ export async function deleteProviderConnection(id: string) { })(); await _cleanupDeletedComboConnectionRefs(id); + await _cleanupDeletedLKGPConnectionRefs(id); removeConnectionHealth(id); removeConnectionIndex(id); @@ -114,6 +127,7 @@ export async function deleteProviderConnections(ids: string[]): Promise })(); await _cleanupDeletedComboConnectionRefs(existingIds); + await _cleanupDeletedLKGPConnectionRefs(existingIds); for (const id of ids) { removeConnectionHealth(id); @@ -150,6 +164,7 @@ export async function deleteProviderConnectionsByProvider(providerId: string) { })(); await _cleanupDeletedComboConnectionRefs(connectionIds); + await _cleanupDeletedLKGPConnectionRefs(connectionIds); for (const connectionId of connectionIds) { removeConnectionHealth(connectionId); diff --git a/src/lib/db/readCache.ts b/src/lib/db/readCache.ts index 79fff8c2d2..8c0f5b73ce 100644 --- a/src/lib/db/readCache.ts +++ b/src/lib/db/readCache.ts @@ -210,6 +210,14 @@ export async function setCachedLKGP( lkgpCache.invalidate(`lkgp:${comboName}:${modelId}`); } +/** + * Invalidate one persisted LKGP pin by its `${comboName}:${modelId}` storage key, + * or every cached LKGP pin when no key is provided. + */ +export function invalidateCachedLKGP(pinKey?: string): void { + lkgpCache.invalidate(pinKey ? `lkgp:${pinKey}` : undefined); +} + // ──────────────── Combo Cache Invalidation Signal ──────────────── // // The nested-combo expansion caches live in request handlers diff --git a/src/lib/db/settings/lkgp.ts b/src/lib/db/settings/lkgp.ts index b5b990f641..df090f4004 100644 --- a/src/lib/db/settings/lkgp.ts +++ b/src/lib/db/settings/lkgp.ts @@ -47,3 +47,53 @@ export function clearAllLKGP(): void { const db = getDbInstance(); db.prepare("DELETE FROM key_value WHERE namespace = 'lkgp'").run(); } + +/** + * Delete persisted LKGP pins whose connectionId references a removed provider + * connection. Provider-level pins and legacy/unparseable values are preserved. + */ +export async function deleteLKGPByConnectionIds(connectionIds: string[]): Promise { + if (connectionIds.length === 0) return 0; + + const deletedConnectionIds = new Set(connectionIds.filter(Boolean)); + if (deletedConnectionIds.size === 0) return 0; + + const db = getDbInstance(); + const rows = db + .prepare("SELECT key, value FROM key_value WHERE namespace = 'lkgp'") + .all() as Array<{ key?: string; value?: string }>; + + const staleKeys: string[] = []; + + for (const row of rows) { + if (!row?.key || !row.value) continue; + + let parsed: unknown; + try { + parsed = JSON.parse(row.value); + } catch { + continue; + } + + if (typeof parsed !== "object" || parsed === null) continue; + + const connectionId = (parsed as LKGPRecord).connectionId; + if (typeof connectionId === "string" && deletedConnectionIds.has(connectionId)) { + staleKeys.push(row.key); + } + } + + if (staleKeys.length === 0) return 0; + + const deleteStatement = db.prepare("DELETE FROM key_value WHERE namespace = 'lkgp' AND key = ?"); + for (const key of staleKeys) { + deleteStatement.run(key); + } + + const { invalidateCachedLKGP } = await import("../readCache"); + for (const key of staleKeys) { + invalidateCachedLKGP(key); + } + + return staleKeys.length; +} diff --git a/tests/unit/delete-provider-connection-invalidates-lkgp-8887.test.ts b/tests/unit/delete-provider-connection-invalidates-lkgp-8887.test.ts new file mode 100644 index 0000000000..83226544fe --- /dev/null +++ b/tests/unit/delete-provider-connection-invalidates-lkgp-8887.test.ts @@ -0,0 +1,169 @@ +/** + * Issue #8887 — deleting a provider connection must invalidate LKGP pins that + * reference it without disturbing surviving, provider-level, or legacy pins. + */ + +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-lkgp-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 lkgpDb = await import("../../src/lib/db/settings/lkgp.ts"); +const readCache = await import("../../src/lib/db/readCache.ts"); + +async function resetStorage() { + 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; +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("#8887: single delete removes only the matching LKGP pin", async () => { + const doomedId = await createConnection("berry", "single-doomed"); + const survivorId = await createConnection("berry", "single-survivor"); + + await lkgpDb.setLKGP("single-doomed", "model-x", "berry", doomedId); + await lkgpDb.setLKGP("single-survivor", "model-y", "berry", survivorId); + + assert.equal(await providersDb.deleteProviderConnection(doomedId), true); + + assert.equal(await lkgpDb.getLKGP("single-doomed", "model-x"), null); + assert.deepEqual(await lkgpDb.getLKGP("single-survivor", "model-y"), { + provider: "berry", + connectionId: survivorId, + }); +}); + +test("#8887: single delete invalidates a warmed LKGP read-cache entry", async () => { + const doomedId = await createConnection("berry", "cached-doomed"); + + await lkgpDb.setLKGP("cached-doomed", "model-x", "berry", doomedId); + + assert.deepEqual(await readCache.getCachedLKGP("cached-doomed", "model-x"), { + provider: "berry", + connectionId: doomedId, + }); + + assert.equal(await providersDb.deleteProviderConnection(doomedId), true); + + assert.equal( + await readCache.getCachedLKGP("cached-doomed", "model-x"), + null, + "deleted LKGP pins must not survive in the 5s read cache" + ); +}); + +test("#8887: bulk delete removes every matching LKGP pin", async () => { + const doomedA = await createConnection("berry", "bulk-doomed-a"); + const doomedB = await createConnection("berry", "bulk-doomed-b"); + const survivorId = await createConnection("berry", "bulk-survivor"); + + await lkgpDb.setLKGP("bulk-a", "model-x", "berry", doomedA); + await lkgpDb.setLKGP("bulk-b", "model-y", "berry", doomedB); + await lkgpDb.setLKGP("bulk-survivor", "model-z", "berry", survivorId); + + assert.equal(await providersDb.deleteProviderConnections([doomedA, doomedB]), 2); + + assert.equal(await lkgpDb.getLKGP("bulk-a", "model-x"), null); + assert.equal(await lkgpDb.getLKGP("bulk-b", "model-y"), null); + assert.deepEqual(await lkgpDb.getLKGP("bulk-survivor", "model-z"), { + provider: "berry", + connectionId: survivorId, + }); +}); + +test("#8887: provider-wide delete removes that provider's LKGP pins only", async () => { + const berryId = await createConnection("berry", "provider-doomed"); + const cherryId = await createConnection("cherry", "provider-survivor"); + + await lkgpDb.setLKGP("provider-doomed", "model-x", "berry", berryId); + await lkgpDb.setLKGP("provider-survivor", "model-y", "cherry", cherryId); + + assert.equal(await providersDb.deleteProviderConnectionsByProvider("berry"), 1); + + assert.equal(await lkgpDb.getLKGP("provider-doomed", "model-x"), null); + assert.deepEqual(await lkgpDb.getLKGP("provider-survivor", "model-y"), { + provider: "cherry", + connectionId: cherryId, + }); +}); + +test("#8887: provider-level LKGP pins without connectionId are preserved", async () => { + const doomedId = await createConnection("berry", "provider-level"); + + await lkgpDb.setLKGP("provider-level", "model-x", "berry"); + + assert.equal(await providersDb.deleteProviderConnection(doomedId), true); + assert.deepEqual(await lkgpDb.getLKGP("provider-level", "model-x"), { provider: "berry" }); +}); + +test("#8887: legacy LKGP values are preserved during connection cleanup", async () => { + const doomedId = await createConnection("berry", "legacy"); + const db = core.getDbInstance(); + + db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('lkgp', ?, ?)").run( + "legacy:model-x", + "berry" + ); + + assert.equal(await providersDb.deleteProviderConnection(doomedId), true); + assert.deepEqual(await lkgpDb.getLKGP("legacy", "model-x"), { provider: "berry" }); +}); + +test("#8887: deleting an unpinned connection does not disturb unrelated LKGP state", async () => { + const doomedId = await createConnection("berry", "unpinned"); + const survivorId = await createConnection("cherry", "unrelated"); + + await lkgpDb.setLKGP("unrelated", "model-y", "cherry", survivorId); + + assert.equal(await providersDb.deleteProviderConnection(doomedId), true); + assert.deepEqual(await lkgpDb.getLKGP("unrelated", "model-y"), { + provider: "cherry", + connectionId: survivorId, + }); +});