diff --git a/CHANGELOG.md b/CHANGELOG.md index 33b9f7da80..30123fc466 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ _In development — bullets added per PR; finalized at release._ - **fix(dashboard): a disabled connection's last error is now visible** — the provider card's error badge counts a disabled connection (`isActive === false`) that has an error (its effective status is still error/expired/unavailable), but the connection row hid the `lastError` text for disabled rows — so the operator saw the error count without being able to see what failed. The row now shows the error text whenever there is one, regardless of the active toggle. (thanks @ntdung6868) - **fix(providers): the "Test Connection One-by-One" OAuth probe can no longer hang the queue forever** — the OAuth connection-test path called bare `fetch(url, { method, headers })` with no `AbortController`/signal/timeout, so when a provider's probe endpoint accepted the socket but never responded, the awaited fetch never settled and the one-by-one test queue stalled indefinitely (the API-key path was already bounded via `validateProviderApiKey`'s `timeoutMs`). Both the initial probe and the post-refresh retry are now bounded with `AbortSignal.timeout(30s)` — matching the API-key path's 30s budget — and a timed-out probe resolves as a failure with a clear `Test timed out after 30s` message in the same shape as every other test error. (thanks @ntdung6868) - **fix(providers): a deactivated account is labeled distinctly from a revoked token** — a Codex connection whose OAuth refresh is fully healthy but whose ChatGPT account has been deactivated by the provider gets a `401` from the upstream API. The connection test labeled that the same as a bad credential (`Token invalid or revoked` → `upstream_auth_error`), so the operator couldn't tell a deactivated account from a revoked token. The test now reads the `401`/`403` body and, when it indicates account deactivation, classifies it as `account_deactivated` — which the dashboard already renders as "Account Deactivated". A plain auth `401` is unchanged. (thanks @ntdung6868) +- **fix(db): cascade-delete orphaned model aliases when a provider is removed** — deleting a custom provider removed its connections and node but left behind the imported model-alias rows (stored as `key=`, `value="/"`). Those stale aliases then blocked re-importing the same provider — the import dedup treated them as "already exists", so no new models appeared. A new `deleteModelAliasesForProvider(providerId)` DB helper drops every alias whose stored value begins with `/` (leaving other providers and user-defined settings aliases untouched), and the provider-node DELETE handler now calls it after removing the connections and node, so a fresh import is unblocked. (thanks @nguyenvanhuy0612) --- diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index e4ea9772e5..f9b676b6ed 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -76,6 +76,7 @@ "_rebaseline_2026_06_20_1330_ai_sdk_image": "Re-baseline #1330 (accept AI SDK-style {type:image, image:data-URL string} parts): openai-to-kiro.ts 798->807 (+9, new image-part branch mirroring the existing image_url handling), crossing the 800 cap. Freeze at 807. Cohesive translator branch; the other two translators (claude 776, gemini 630) stay under cap.", "_rebaseline_2026_06_20_1447_disabled_conn_error": "Re-baseline #1447 (show a disabled connection's last error): ConnectionRow.tsx 941->942 (+1), a single import line for the extracted shouldShowConnectionLastError helper. Minimal, not extractable.", "_rebaseline_2026_06_20_1449_1444_test_route": "Re-baseline providers test route.ts 842->887: combined growth of sibling fixes #1449 (bound OAuth connection-test probe with a timeout) + #1444 (label a deactivated account distinctly from a revoked token), both at the same connection-test chokepoint. Cohesive route handler; not extractable without hiding the test flow.", + "_rebaseline_2026_06_20_1409_1294_models": "Re-baseline src/lib/db/models.ts 1184->1221: combined growth of sibling fixes #1409 (cascade-delete orphaned model aliases when a provider is removed) + #1294 (persist max_input_tokens/max_output_tokens on custom models), both adding CRUD at the existing models domain module. Cohesive db module; not extractable.", "cap": 800, "frozen": { "open-sse/translator/request/openai-to-kiro.ts": 807, @@ -157,7 +158,7 @@ "src/lib/db/apiKeys.ts": 1662, "src/lib/db/core.ts": 1820, "src/lib/db/migrationRunner.ts": 1125, - "src/lib/db/models.ts": 1184, + "src/lib/db/models.ts": 1221, "src/lib/db/providers.ts": 1050, "src/lib/db/proxies.ts": 1048, "src/lib/db/settings.ts": 1149, diff --git a/src/app/api/provider-nodes/[id]/route.ts b/src/app/api/provider-nodes/[id]/route.ts index feba366e20..9ba272fa5c 100644 --- a/src/app/api/provider-nodes/[id]/route.ts +++ b/src/app/api/provider-nodes/[id]/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { + deleteModelAliasesForProvider, deleteProviderConnectionsByProvider, deleteProviderNode, getProviderConnections, @@ -150,6 +151,9 @@ export async function DELETE(_request: Request, { params }: { params: Promise<{ await deleteProviderConnectionsByProvider(id); await deleteProviderNode(id); + // #1409: drop orphaned model-alias rows (key=, value="/") + // so re-importing the same provider isn't blocked by stale "already exists" aliases. + await deleteModelAliasesForProvider(id); return NextResponse.json({ success: true }); } catch (error) { diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index cc39a8cc86..a9a7ddd603 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -297,6 +297,34 @@ export async function deleteModelAlias(alias: string) { backupDbFile("pre-write"); } +/** + * Cascade-delete every model-alias row that resolves to the given provider. + * + * Managed/imported aliases are stored as `key = `, `value = "/"` + * (e.g. `setModelAlias("x-fast", "providerX/fast-model")`). When a custom provider is + * removed, its connections and node are deleted but these alias rows are left behind, + * which then block re-importing the same provider ("already exists" / no new models) — see + * #1409. This removes every alias whose stored value begins with `/`, so a + * fresh import is unblocked. + * + * Only string values starting with the exact `"/"` prefix match, so unrelated + * providers and user-facing settings aliases (whose value is the bare alias, not a + * `/` string) are left untouched. + * + * @returns the list of alias keys that were removed. + */ +export async function deleteModelAliasesForProvider(providerId: string): Promise { + const prefix = `${providerId}/`; + const aliases = await getModelAliases(); + const removed: string[] = []; + for (const [alias, value] of Object.entries(aliases)) { + if (typeof value !== "string" || !value.startsWith(prefix)) continue; + await deleteModelAlias(alias); + removed.push(alias); + } + return removed; +} + // ──────────────── MITM Alias ──────────────── export async function getMitmAlias(toolName?: string) { diff --git a/src/lib/localDb.ts b/src/lib/localDb.ts index 7a6911a71b..eee2234a10 100755 --- a/src/lib/localDb.ts +++ b/src/lib/localDb.ts @@ -43,6 +43,7 @@ export { getModelAliases, setModelAlias, deleteModelAlias, + deleteModelAliasesForProvider, // MITM Alias getMitmAlias, diff --git a/src/models/index.ts b/src/models/index.ts index 88da17a5be..929981be86 100755 --- a/src/models/index.ts +++ b/src/models/index.ts @@ -15,6 +15,7 @@ export { getModelAliases, setModelAlias, deleteModelAlias, + deleteModelAliasesForProvider, getMitmAlias, setMitmAliasAll, getApiKeys, diff --git a/tests/unit/db-model-aliases-cascade.test.ts b/tests/unit/db-model-aliases-cascade.test.ts new file mode 100644 index 0000000000..2d1a76ce30 --- /dev/null +++ b/tests/unit/db-model-aliases-cascade.test.ts @@ -0,0 +1,95 @@ +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-db-aliases-cascade-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const models = await import("../../src/lib/db/models.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: any) { + if ((error?.code === "EBUSY" || error?.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 }); +}); + +test("deleteModelAliasesForProvider removes only the target provider's aliases", async () => { + // Managed/imported aliases are stored as key=, value="/". + await models.setModelAlias("x-fast", "providerX/fast-model"); + await models.setModelAlias("x-smart", "providerX/smart-model"); + await models.setModelAlias("y-mini", "providerY/mini-model"); + + const removed = await models.deleteModelAliasesForProvider("providerX"); + + assert.deepEqual(removed.sort(), ["x-fast", "x-smart"]); + + const after = await models.getModelAliases(); + // providerX aliases are gone… + assert.equal(after["x-fast"], undefined); + assert.equal(after["x-smart"], undefined); + // …and providerY's alias remains untouched. + assert.equal(after["y-mini"], "providerY/mini-model"); +}); + +test("deleteModelAliasesForProvider does not match providers sharing a name prefix", async () => { + // "providerX" must not cascade-delete "providerXL"'s aliases (no partial-prefix match). + await models.setModelAlias("x-fast", "providerX/fast-model"); + await models.setModelAlias("xl-fast", "providerXL/fast-model"); + + const removed = await models.deleteModelAliasesForProvider("providerX"); + + assert.deepEqual(removed, ["x-fast"]); + + const after = await models.getModelAliases(); + assert.equal(after["x-fast"], undefined); + assert.equal(after["xl-fast"], "providerXL/fast-model"); +}); + +test("after cascade delete, re-adding the same provider alias succeeds (re-import unblocked)", async () => { + await models.setModelAlias("x-fast", "providerX/fast-model"); + + await models.deleteModelAliasesForProvider("providerX"); + + // Re-import: the alias key/value can be set again with no stale row blocking it. + await models.setModelAlias("x-fast", "providerX/fast-model"); + + const after = await models.getModelAliases(); + assert.equal(after["x-fast"], "providerX/fast-model"); +}); + +test("deleteModelAliasesForProvider returns an empty list when there is nothing to remove", async () => { + await models.setModelAlias("y-mini", "providerY/mini-model"); + + const removed = await models.deleteModelAliasesForProvider("providerX"); + + assert.deepEqual(removed, []); + const after = await models.getModelAliases(); + assert.equal(after["y-mini"], "providerY/mini-model"); +});