mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-03 05:45:04 +03:00
fix(db): cascade-delete orphaned model aliases when a provider is removed (#4348)
* fix(db): cascade-delete orphaned model aliases when a provider is removed (port from 9router#1409) Deleting a custom provider removed its connections and node but left the imported model-alias rows (key=<alias>, value="<providerId>/<model>") behind, so re-importing the same provider was blocked by stale "already exists" aliases. Add a deleteModelAliasesForProvider(providerId) DB helper that drops every alias whose stored value begins with "<providerId>/", and call it from the provider-node DELETE handler so a fresh import is unblocked. Reported-by: nguyenvanhuy0612 (https://github.com/decolua/9router/issues/1409) Co-authored-by: nguyenvanhuy0612 <57367674+nguyenvanhuy0612@users.noreply.github.com> * chore(quality): rebaseline models.ts file-size to 1221 (#1409 + sibling #1294 growth) --------- Co-authored-by: nguyenvanhuy0612 <57367674+nguyenvanhuy0612@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
94f474b943
commit
2dc38c1618
@@ -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=<alias>`, `value="<providerId>/<model>"`). 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 `<providerId>/` (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)
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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=<alias>, value="<providerId>/<model>")
|
||||
// so re-importing the same provider isn't blocked by stale "already exists" aliases.
|
||||
await deleteModelAliasesForProvider(id);
|
||||
|
||||
return NextResponse.json({ success: true });
|
||||
} catch (error) {
|
||||
|
||||
@@ -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 = <alias>`, `value = "<providerId>/<model>"`
|
||||
* (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 `<providerId>/`, so a
|
||||
* fresh import is unblocked.
|
||||
*
|
||||
* Only string values starting with the exact `"<providerId>/"` prefix match, so unrelated
|
||||
* providers and user-facing settings aliases (whose value is the bare alias, not a
|
||||
* `<providerId>/<model>` string) are left untouched.
|
||||
*
|
||||
* @returns the list of alias keys that were removed.
|
||||
*/
|
||||
export async function deleteModelAliasesForProvider(providerId: string): Promise<string[]> {
|
||||
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) {
|
||||
|
||||
@@ -43,6 +43,7 @@ export {
|
||||
getModelAliases,
|
||||
setModelAlias,
|
||||
deleteModelAlias,
|
||||
deleteModelAliasesForProvider,
|
||||
|
||||
// MITM Alias
|
||||
getMitmAlias,
|
||||
|
||||
@@ -15,6 +15,7 @@ export {
|
||||
getModelAliases,
|
||||
setModelAlias,
|
||||
deleteModelAlias,
|
||||
deleteModelAliasesForProvider,
|
||||
getMitmAlias,
|
||||
setMitmAliasAll,
|
||||
getApiKeys,
|
||||
|
||||
95
tests/unit/db-model-aliases-cascade.test.ts
Normal file
95
tests/unit/db-model-aliases-cascade.test.ts
Normal file
@@ -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=<alias>, value="<providerId>/<model>".
|
||||
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");
|
||||
});
|
||||
Reference in New Issue
Block a user