mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 10:52:17 +03:00
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
This commit is contained in:
1
changelog.d/fixes/12330-combo-delete-lkgp-cleanup.md
Normal file
1
changelog.d/fixes/12330-combo-delete-lkgp-cleanup.md
Normal file
@@ -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))
|
||||
@@ -349,8 +349,20 @@ export async function reorderCombos(comboIds: string[]): Promise<ComboReorderRes
|
||||
|
||||
export async function deleteCombo(id: string) {
|
||||
const db = getDbInstance();
|
||||
const combo = db.prepare("SELECT name FROM combos WHERE id = ?").get(id) as
|
||||
{ name?: string } | undefined;
|
||||
const result = db.prepare("DELETE FROM combos WHERE id = ?").run(id);
|
||||
if (result.changes === 0) return false;
|
||||
|
||||
if (combo?.name) {
|
||||
try {
|
||||
const { deleteLKGPByComboName } = await import("../settings/lkgp");
|
||||
await deleteLKGPByComboName(combo.name);
|
||||
} catch (error) {
|
||||
console.error("Failed to clean up LKGP pins for deleted combo:", error);
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
@@ -840,6 +840,7 @@ export {
|
||||
setLKGP,
|
||||
clearAllLKGP,
|
||||
clearLKGP,
|
||||
deleteLKGPByComboName,
|
||||
deleteLKGPByConnectionIds,
|
||||
} from "./settings/lkgp";
|
||||
|
||||
|
||||
@@ -4,6 +4,14 @@
|
||||
|
||||
import { getDbInstance } from "../core";
|
||||
|
||||
/**
|
||||
* Escape SQLite `LIKE` wildcards so a combo name containing `%` or `_` cannot
|
||||
* widen the prefix match into unrelated combos' pins.
|
||||
*/
|
||||
function escapeLikePattern(value: string): string {
|
||||
return value.replace(/[\\%_]/g, (char) => `\\${char}`);
|
||||
}
|
||||
|
||||
export interface LKGPRecord {
|
||||
provider: string;
|
||||
connectionId?: string;
|
||||
@@ -67,6 +75,40 @@ export async function clearLKGP(comboName: string, modelId: string): Promise<voi
|
||||
invalidateCachedLKGP(key);
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete every persisted LKGP pin belonging to a combo. Pins are keyed by
|
||||
* `${comboName}:${modelId}`, so deleting a combo leaves its pins addressable by
|
||||
* a name that no longer resolves — `clearAllLKGP()` is too broad and
|
||||
* `clearLKGP()` needs a modelId the caller no longer knows. Combo delete paths
|
||||
* call this so the pins die with their combo instead of accumulating as
|
||||
* unreachable rows.
|
||||
*/
|
||||
export async function deleteLKGPByComboName(comboName: string): Promise<number> {
|
||||
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
|
||||
|
||||
152
tests/unit/combo-delete-lkgp-cleanup-12326.test.ts
Normal file
152
tests/unit/combo-delete-lkgp-cleanup-12326.test.ts
Normal file
@@ -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<string> {
|
||||
const combo = await comboRepo.createCombo({
|
||||
name,
|
||||
models: [{ provider: "berry", model: "model-x" }],
|
||||
} as Parameters<typeof comboRepo.createCombo>[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);
|
||||
});
|
||||
Reference in New Issue
Block a user