fix(combos): prioritize SQLite row id over inner JSON id and notify delete errors (#12213)

When a combo is duplicated or imported, its inner data JSON blob may
retain a stale id from the template. withRowId previously kept the inner
string id instead of prioritizing the database primary key (row.id),
causing GET /api/combos to return mismatched ids and breaking subsequent
DELETE / PUT operations with 404.

Also add an error notification branch to handleDelete in the combos page
so failed delete requests surface actionable feedback instead of failing
silently.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
This commit is contained in:
Bob.Hou
2026-08-31 23:46:54 -04:00
committed by GitHub
parent d0529c0365
commit 2bd3023e09
3 changed files with 71 additions and 1 deletions

View File

@@ -911,6 +911,9 @@ export default function CombosPage() {
if (res.ok) {
setCombos(combos.filter((c) => c.id !== id));
notify.success(t("comboDeleted"));
} else {
const err = await res.json().catch(() => null);
notify.error(err?.error?.message || err?.error || t("errorDeleting"));
}
} catch (error) {
notify.error(t("errorDeleting"));

View File

@@ -41,10 +41,15 @@ function getComboId(value: unknown): string | null {
return typeof row.id === "string" && row.id.trim().length > 0 ? row.id : null;
}
/**
* Enforces the SQLite row's primary key id on the parsed JSON record.
* The database row.id column is always authoritative over any stale id
* persisted inside the data JSON blob (e.g. from duplication or import).
*/
function withRowId(payload: string, row: JsonRecord): JsonRecord {
const parsed = withSortOrder(payload, getSortOrder(row));
const comboId = getComboId(row);
if (comboId && typeof parsed.id !== "string") {
if (comboId) {
parsed.id = comboId;
}
return parsed;

View File

@@ -0,0 +1,62 @@
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-id-test-"));
process.env.DATA_DIR = TEST_DATA_DIR;
const core = await import("../../src/lib/db/core.ts");
const { getCombos, getComboById, getComboByName, deleteCombo } = await import(
"../../src/lib/db/repositories/sqliteComboRepository.ts"
);
test.after(() => {
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
});
test("getCombos and getComboById prioritize SQLite table primary key over inner data JSON id", async () => {
const db = core.getDbInstance();
const rowId = "2dafe555-77d1-4e42-b795-1e5b99e2b649";
const staleInnerId = "da8b4aad-52bc-423c-b9f5-74e2654bbd00";
// Simulate a combo duplicated/imported where data JSON carries the template's stale id
const dataPayload = JSON.stringify({
id: staleInnerId,
name: "test-mismatched-combo",
description: "Combo with mismatched inner JSON id",
models: [{ id: "step-1", model: "openai/gpt-4o" }],
strategy: "priority",
});
db.prepare(
`INSERT INTO combos (id, name, data, sort_order, created_at, updated_at)
VALUES (?, ?, ?, 1, datetime('now'), datetime('now'))`
).run(rowId, "test-mismatched-combo", dataPayload);
// 1. getCombos must return the authoritative table primary key
const list = await getCombos();
const found = list.find((c) => c.name === "test-mismatched-combo");
assert.ok(found, "combo should be returned by getCombos");
assert.equal(
found.id,
rowId,
"getCombos must return the database row id so frontend operations target the real primary key"
);
// 2. getComboById querying by the rowId must return the combo with matching id
const byId = await getComboById(rowId);
assert.ok(byId, "combo should be found by primary key rowId");
assert.equal(byId.id, rowId, "getComboById must normalize id to the table primary key");
// 3. getComboByName must also normalize id to the table primary key
const byName = await getComboByName("test-mismatched-combo");
assert.ok(byName, "combo should be found by name");
assert.equal(byName.id, rowId, "getComboByName must normalize id to the table primary key");
// 4. deleteCombo using the id returned by getCombos must succeed
const deleted = await deleteCombo(found.id as string);
assert.equal(deleted, true, "deleteCombo with the id from getCombos must delete the row");
});