From 9d5ab8ccbf5382b016dcdd334ae71640b585940e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= Date: Wed, 2 Sep 2026 00:17:11 +0800 Subject: [PATCH] fix(combos): clear LKGP pins when a combo is deleted deleteCombo() removed the combos row but left the combo's LKGP pins in key_value. Pins are keyed by `${comboName}:${modelId}`, so once the combo is gone the rows are unreachable: the combo 404s, clearLKGP() needs a modelId the caller no longer has, and clearAllLKGP() is far too broad. Provider-connection deletion already cleans up its pins via deleteLKGPByConnectionIds() (#8887). This applies the same discipline to combo deletion with deleteLKGPByComboName(), which also invalidates the read cache so a stale pin cannot be served for the rest of the TTL. Closes #12326 --- .../fixes/12330-combo-delete-lkgp-cleanup.md | 1 + .../db/repositories/sqliteComboRepository.ts | 12 ++ src/lib/db/settings.ts | 1 + src/lib/db/settings/lkgp.ts | 42 +++++ .../combo-delete-lkgp-cleanup-12326.test.ts | 152 ++++++++++++++++++ 5 files changed, 208 insertions(+) create mode 100644 changelog.d/fixes/12330-combo-delete-lkgp-cleanup.md create mode 100644 tests/unit/combo-delete-lkgp-cleanup-12326.test.ts diff --git a/changelog.d/fixes/12330-combo-delete-lkgp-cleanup.md b/changelog.d/fixes/12330-combo-delete-lkgp-cleanup.md new file mode 100644 index 0000000000..212ba0a6a1 --- /dev/null +++ b/changelog.d/fixes/12330-combo-delete-lkgp-cleanup.md @@ -0,0 +1 @@ +- **fix(combos):** deleting a combo now clears its persisted LKGP pins instead of leaving unreachable `key_value` rows behind ([#12330](https://github.com/diegosouzapw/OmniRoute/pull/12330)) diff --git a/src/lib/db/repositories/sqliteComboRepository.ts b/src/lib/db/repositories/sqliteComboRepository.ts index 4263f51034..c0ec91237e 100644 --- a/src/lib/db/repositories/sqliteComboRepository.ts +++ b/src/lib/db/repositories/sqliteComboRepository.ts @@ -349,8 +349,20 @@ export async function reorderCombos(comboIds: string[]): Promise `\\${char}`); +} + export interface LKGPRecord { provider: string; connectionId?: string; @@ -67,6 +75,40 @@ export async function clearLKGP(comboName: string, modelId: string): Promise { + if (!comboName) return 0; + + const db = getDbInstance(); + const prefix = `${comboName}:`; + const rows = db + .prepare("SELECT key FROM key_value WHERE namespace = 'lkgp' AND key LIKE ? ESCAPE '\\'") + .all(`${escapeLikePattern(prefix)}%`) as Array<{ key?: string }>; + + const staleKeys = rows.map((row) => row?.key).filter((key): key is string => Boolean(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; +} + /** * Delete persisted LKGP pins whose connectionId references a removed provider * connection (#8887). A pin persisted by `setLKGP()` carries the connection it diff --git a/tests/unit/combo-delete-lkgp-cleanup-12326.test.ts b/tests/unit/combo-delete-lkgp-cleanup-12326.test.ts new file mode 100644 index 0000000000..a35920535d --- /dev/null +++ b/tests/unit/combo-delete-lkgp-cleanup-12326.test.ts @@ -0,0 +1,152 @@ +/** + * Issue #12326 — deleting a combo must remove the LKGP pins keyed by its name + * without disturbing surviving combos' 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-12326-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const comboRepo = await import("../../src/lib/db/repositories/sqliteComboRepository.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, maxRetries: 5, retryDelay: 100 }); + 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 createCombo(name: string): Promise { + const combo = await comboRepo.createCombo({ + name, + models: [{ provider: "berry", model: "model-x" }], + } as Parameters[0]); + + assert.equal(typeof combo.id, "string", "combo fixture must return an id"); + return combo.id as string; +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +test("#12326: deleting a combo removes its LKGP pins", async () => { + const doomedId = await createCombo("doomed-combo"); + await createCombo("survivor-combo"); + + await lkgpDb.setLKGP("doomed-combo", "model-x", "berry", "conn-1"); + await lkgpDb.setLKGP("doomed-combo", "model-y", "berry", "conn-2"); + await lkgpDb.setLKGP("survivor-combo", "model-x", "berry", "conn-3"); + + assert.equal(await comboRepo.deleteCombo(doomedId), true); + + assert.equal(await lkgpDb.getLKGP("doomed-combo", "model-x"), null); + assert.equal(await lkgpDb.getLKGP("doomed-combo", "model-y"), null); + assert.deepEqual(await lkgpDb.getLKGP("survivor-combo", "model-x"), { + provider: "berry", + connectionId: "conn-3", + }); +}); + +test("#12326: deleting a combo invalidates warmed LKGP read-cache entries", async () => { + const doomedId = await createCombo("cached-combo"); + + await lkgpDb.setLKGP("cached-combo", "model-x", "berry", "conn-1"); + + assert.deepEqual(await readCache.getCachedLKGP("cached-combo", "model-x"), { + provider: "berry", + connectionId: "conn-1", + }); + + assert.equal(await comboRepo.deleteCombo(doomedId), true); + + assert.equal( + await readCache.getCachedLKGP("cached-combo", "model-x"), + null, + "deleted combos' LKGP pins must not survive in the read cache" + ); +}); + +test("#12326: a combo whose name prefixes another keeps the sibling's pins", async () => { + const doomedId = await createCombo("prod"); + await createCombo("prod-canary"); + + await lkgpDb.setLKGP("prod", "model-x", "berry", "conn-1"); + await lkgpDb.setLKGP("prod-canary", "model-x", "berry", "conn-2"); + + assert.equal(await comboRepo.deleteCombo(doomedId), true); + + assert.equal(await lkgpDb.getLKGP("prod", "model-x"), null); + assert.deepEqual( + await lkgpDb.getLKGP("prod-canary", "model-x"), + { provider: "berry", connectionId: "conn-2" }, + "the ':' delimiter must keep a prefix-sharing sibling's pins intact" + ); +}); + +test("#12326: LIKE wildcards in a combo name do not widen the cleanup", async () => { + const doomedId = await createCombo("temp_a"); + await createCombo("tempXa"); + + await lkgpDb.setLKGP("temp_a", "model-x", "berry", "conn-1"); + await lkgpDb.setLKGP("tempXa", "model-x", "berry", "conn-2"); + + assert.equal(await comboRepo.deleteCombo(doomedId), true); + + assert.equal(await lkgpDb.getLKGP("temp_a", "model-x"), null); + assert.deepEqual( + await lkgpDb.getLKGP("tempXa", "model-x"), + { provider: "berry", connectionId: "conn-2" }, + "'_' must be escaped so it cannot match an arbitrary character" + ); +}); + +test("#12326: deleting an unknown combo id leaves LKGP state untouched", async () => { + await createCombo("untouched-combo"); + await lkgpDb.setLKGP("untouched-combo", "model-x", "berry", "conn-1"); + + assert.equal(await comboRepo.deleteCombo("00000000-0000-0000-0000-000000000000"), false); + + assert.deepEqual(await lkgpDb.getLKGP("untouched-combo", "model-x"), { + provider: "berry", + connectionId: "conn-1", + }); +}); + +test("#12326: deleting a combo without pins succeeds", async () => { + const doomedId = await createCombo("no-pins-combo"); + + assert.equal(await comboRepo.deleteCombo(doomedId), true); + assert.equal(await lkgpDb.getLKGP("no-pins-combo", "model-x"), null); +});