mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-14 11:12:17 +03:00
* fix(db): clear combo pins when connections are deleted * docs: add changelog entry for #9719 --------- Co-authored-by: Zartharas <1402357+Zartharas@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
6f3738b009
commit
ed7a68e1a9
1
changelog.d/fixes/9719-combo-connection-pins.md
Normal file
1
changelog.d/fixes/9719-combo-connection-pins.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(db): clear stale combo connection pins when provider connections are deleted (#9719)
|
||||
@@ -25,7 +25,6 @@ import {
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { isApiKeyRevealEnabled, maskStoredApiKey } from "@/lib/apiKeyExposure";
|
||||
import { cleanupProviderModelsAfterConnectionDelete } from "@/lib/db/models";
|
||||
import { cleanupComboConnectionRefs } from "@/lib/db/combos";
|
||||
import {
|
||||
refreshConnectionRateLimits,
|
||||
enableRateLimitProtection,
|
||||
@@ -367,13 +366,6 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i
|
||||
console.error(`Failed to clean up models for deleted ${connection.provider} connection:`, e);
|
||||
}
|
||||
|
||||
// Remove stale connectionId references from combo route steps.
|
||||
try {
|
||||
await cleanupComboConnectionRefs(id);
|
||||
} catch (e) {
|
||||
console.error("Failed to clean up combo route refs for deleted connection:", e);
|
||||
}
|
||||
|
||||
// Auto sync to Cloud if enabled
|
||||
await syncToCloudIfEnabled();
|
||||
|
||||
|
||||
@@ -96,39 +96,60 @@ export function setActiveCombo(name: string, db = getDbInstance()): void {
|
||||
* Called after a provider connection is removed so combo routes don't carry
|
||||
* stale references.
|
||||
*/
|
||||
export async function cleanupComboConnectionRefs(connectionId: string) {
|
||||
export async function cleanupComboConnectionRefs(connectionIds: string | string[]) {
|
||||
const deletedConnectionIds = new Set(
|
||||
(Array.isArray(connectionIds) ? connectionIds : [connectionIds]).filter(Boolean)
|
||||
);
|
||||
|
||||
if (deletedConnectionIds.size === 0) return 0;
|
||||
|
||||
const combos = await getCombos();
|
||||
let touched = 0;
|
||||
|
||||
for (const combo of combos) {
|
||||
if (!Array.isArray(combo.models)) continue;
|
||||
|
||||
let changed = false;
|
||||
|
||||
const models = (combo.models as unknown as Record<string, unknown>[]).map((step) => {
|
||||
let out = step;
|
||||
if (out.connectionId === connectionId) {
|
||||
|
||||
if (typeof out.connectionId === "string" && deletedConnectionIds.has(out.connectionId)) {
|
||||
const { connectionId: _, ...rest } = out;
|
||||
out = rest;
|
||||
changed = true;
|
||||
}
|
||||
|
||||
if (Array.isArray(out.allowedConnectionIds)) {
|
||||
const filtered = out.allowedConnectionIds.filter(
|
||||
(id: string) => id !== connectionId
|
||||
(id) => typeof id !== "string" || !deletedConnectionIds.has(id)
|
||||
);
|
||||
|
||||
if (filtered.length !== out.allowedConnectionIds.length) {
|
||||
out = { ...out, allowedConnectionIds: filtered };
|
||||
out = {
|
||||
...out,
|
||||
allowedConnectionIds: filtered,
|
||||
};
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
|
||||
return out;
|
||||
});
|
||||
|
||||
if (changed && typeof combo.id === "string") {
|
||||
try {
|
||||
const { id, ...rest } = combo;
|
||||
await updateCombo(combo.id, { ...rest, models });
|
||||
await updateCombo(combo.id, {
|
||||
...rest,
|
||||
models,
|
||||
});
|
||||
touched++;
|
||||
} catch {
|
||||
// One combo failing should not block cleanup of the rest.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return touched;
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
|
||||
import { getDbInstance } from "../core";
|
||||
import { backupDbFile } from "../backup";
|
||||
import { cleanupComboConnectionRefs } from "../combos";
|
||||
import {
|
||||
removeConnectionHealth,
|
||||
removeConnectionIndex,
|
||||
@@ -41,6 +42,29 @@ function _deleteAccountProxyAssignments(db: DbLike, ids: string[]) {
|
||||
).run(...ids);
|
||||
}
|
||||
|
||||
function _selectExistingConnectionIds(db: DbLike, ids: string[]): string[] {
|
||||
if (ids.length === 0) return [];
|
||||
|
||||
const placeholders = ids.map(() => "?").join(",");
|
||||
|
||||
return db
|
||||
.prepare(`SELECT id FROM provider_connections WHERE id IN (${placeholders})`)
|
||||
.all(...ids)
|
||||
.map((row) => {
|
||||
const record = toRecord(row);
|
||||
return typeof record.id === "string" ? record.id : null;
|
||||
})
|
||||
.filter((id): id is string => id !== null);
|
||||
}
|
||||
|
||||
async function _cleanupDeletedComboConnectionRefs(connectionIds: string | string[]): Promise<void> {
|
||||
try {
|
||||
await cleanupComboConnectionRefs(connectionIds);
|
||||
} catch (error) {
|
||||
console.error("Failed to clean up combo route refs for deleted connections:", error);
|
||||
}
|
||||
}
|
||||
|
||||
export async function deleteProviderConnection(id: string) {
|
||||
const db = getDbInstance() as unknown as DbLike;
|
||||
const existing = db.prepare("SELECT provider FROM provider_connections WHERE id = ?").get(id);
|
||||
@@ -51,6 +75,9 @@ export async function deleteProviderConnection(id: string) {
|
||||
db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?").run(id);
|
||||
db.prepare("DELETE FROM provider_connections WHERE id = ?").run(id);
|
||||
})();
|
||||
|
||||
await _cleanupDeletedComboConnectionRefs(id);
|
||||
|
||||
removeConnectionHealth(id);
|
||||
removeConnectionIndex(id);
|
||||
bumpProxyConfigGeneration();
|
||||
@@ -68,25 +95,35 @@ export async function deleteProviderConnection(id: string) {
|
||||
|
||||
export async function deleteProviderConnections(ids: string[]): Promise<number> {
|
||||
if (ids.length === 0) return 0;
|
||||
|
||||
const db = getDbInstance() as unknown as DbLike;
|
||||
const existingIds = _selectExistingConnectionIds(db, ids);
|
||||
|
||||
const deletedCount = db.transaction(() => {
|
||||
const placeholders = ids.map(() => "?").join(",");
|
||||
|
||||
db.prepare(`DELETE FROM quota_snapshots WHERE connection_id IN (${placeholders})`).run(...ids);
|
||||
|
||||
_deleteAccountProxyAssignments(db, ids);
|
||||
|
||||
const result = db
|
||||
.prepare(`DELETE FROM provider_connections WHERE id IN (${placeholders})`)
|
||||
.run(...ids);
|
||||
|
||||
return result.changes ?? 0;
|
||||
})();
|
||||
|
||||
await _cleanupDeletedComboConnectionRefs(existingIds);
|
||||
|
||||
for (const id of ids) {
|
||||
removeConnectionHealth(id);
|
||||
removeConnectionIndex(id);
|
||||
}
|
||||
|
||||
backupDbFile("pre-write");
|
||||
invalidateDbCache("connections");
|
||||
invalidateReasoningRoutingRuleCache();
|
||||
|
||||
return deletedCount;
|
||||
}
|
||||
|
||||
@@ -111,6 +148,9 @@ export async function deleteProviderConnectionsByProvider(providerId: string) {
|
||||
}
|
||||
return db.prepare("DELETE FROM provider_connections WHERE provider = ?").run(providerId);
|
||||
})();
|
||||
|
||||
await _cleanupDeletedComboConnectionRefs(connectionIds);
|
||||
|
||||
for (const connectionId of connectionIds) {
|
||||
removeConnectionHealth(connectionId);
|
||||
removeConnectionIndex(connectionId);
|
||||
|
||||
@@ -0,0 +1,221 @@
|
||||
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-pins-8887-"));
|
||||
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const providersDb = await import("../../src/lib/db/providers.ts");
|
||||
const combosDb = await import("../../src/lib/db/combos.ts");
|
||||
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
|
||||
async function resetStorage(): Promise<void> {
|
||||
core.resetDbInstance();
|
||||
|
||||
for (let attempt = 0; attempt < 10; attempt++) {
|
||||
try {
|
||||
fs.rmSync(TEST_DATA_DIR, {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
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 createConnection(provider: string, name: string): Promise<string> {
|
||||
const connection = await providersDb.createProviderConnection({
|
||||
provider,
|
||||
authType: "apikey",
|
||||
name,
|
||||
apiKey: `test-key-${name}`,
|
||||
});
|
||||
|
||||
assert.equal(typeof connection.id, "string", "provider fixture must return a connection id");
|
||||
|
||||
return connection.id as string;
|
||||
}
|
||||
|
||||
async function createPinnedCombo(name: string, models: JsonRecord[]): Promise<void> {
|
||||
await combosDb.createCombo({
|
||||
name,
|
||||
strategy: "priority",
|
||||
models,
|
||||
});
|
||||
}
|
||||
|
||||
async function readModels(name: string): Promise<JsonRecord[]> {
|
||||
const combo = await combosDb.getComboByName(name);
|
||||
|
||||
assert.ok(combo, `combo ${name} must still exist`);
|
||||
assert.ok(Array.isArray(combo.models), `combo ${name} must retain a models array`);
|
||||
|
||||
return combo.models as JsonRecord[];
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
await resetStorage();
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
|
||||
fs.rmSync(TEST_DATA_DIR, {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
});
|
||||
|
||||
test("#8887: single connection delete clears only matching combo pins", async () => {
|
||||
const doomedId = await createConnection("openai", "single-doomed");
|
||||
const survivorId = await createConnection("openai", "single-survivor");
|
||||
|
||||
await createPinnedCombo("single-delete-8887", [
|
||||
{
|
||||
provider: "openai",
|
||||
model: "gpt-5.6-sol",
|
||||
connectionId: doomedId,
|
||||
allowedConnectionIds: [doomedId, survivorId],
|
||||
},
|
||||
{
|
||||
provider: "openai",
|
||||
model: "gpt-5.6-sol",
|
||||
connectionId: survivorId,
|
||||
},
|
||||
]);
|
||||
|
||||
assert.equal(await providersDb.deleteProviderConnection(doomedId), true);
|
||||
|
||||
const models = await readModels("single-delete-8887");
|
||||
|
||||
assert.equal(
|
||||
"connectionId" in models[0],
|
||||
false,
|
||||
"single delete must remove the deleted direct connection pin"
|
||||
);
|
||||
assert.deepEqual(
|
||||
models[0].allowedConnectionIds,
|
||||
[survivorId],
|
||||
"single delete must remove the deleted id from allowedConnectionIds"
|
||||
);
|
||||
assert.equal(
|
||||
models[1].connectionId,
|
||||
survivorId,
|
||||
"single delete must preserve surviving connection pins"
|
||||
);
|
||||
});
|
||||
|
||||
test("#8887: bulk connection delete clears every matching combo pin", async () => {
|
||||
const doomedA = await createConnection("anthropic", "bulk-doomed-a");
|
||||
const doomedB = await createConnection("anthropic", "bulk-doomed-b");
|
||||
const survivorId = await createConnection("anthropic", "bulk-survivor");
|
||||
|
||||
await createPinnedCombo("bulk-delete-8887", [
|
||||
{
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-5",
|
||||
connectionId: doomedA,
|
||||
allowedConnectionIds: [doomedA, survivorId],
|
||||
},
|
||||
{
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-5",
|
||||
connectionId: doomedB,
|
||||
allowedConnectionIds: [doomedB, survivorId],
|
||||
},
|
||||
{
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-5",
|
||||
connectionId: survivorId,
|
||||
},
|
||||
]);
|
||||
|
||||
assert.equal(await providersDb.deleteProviderConnections([doomedA, doomedB]), 2);
|
||||
|
||||
const models = await readModels("bulk-delete-8887");
|
||||
|
||||
assert.equal(
|
||||
"connectionId" in models[0],
|
||||
false,
|
||||
"bulk delete must remove the first deleted direct pin"
|
||||
);
|
||||
assert.equal(
|
||||
"connectionId" in models[1],
|
||||
false,
|
||||
"bulk delete must remove the second deleted direct pin"
|
||||
);
|
||||
assert.deepEqual(models[0].allowedConnectionIds, [survivorId]);
|
||||
assert.deepEqual(models[1].allowedConnectionIds, [survivorId]);
|
||||
assert.equal(models[2].connectionId, survivorId);
|
||||
});
|
||||
|
||||
test("#8887: provider-wide delete clears that provider's combo pins only", async () => {
|
||||
const doomedA = await createConnection("nvidia", "provider-doomed-a");
|
||||
const doomedB = await createConnection("nvidia", "provider-doomed-b");
|
||||
const otherProviderId = await createConnection("cerebras", "provider-survivor");
|
||||
|
||||
await createPinnedCombo("provider-delete-8887", [
|
||||
{
|
||||
provider: "nvidia",
|
||||
model: "z-ai/glm-5.2",
|
||||
connectionId: doomedA,
|
||||
allowedConnectionIds: [doomedA, doomedB, otherProviderId],
|
||||
},
|
||||
{
|
||||
provider: "nvidia",
|
||||
model: "deepseek-ai/deepseek-v4-pro",
|
||||
connectionId: doomedB,
|
||||
},
|
||||
{
|
||||
provider: "cerebras",
|
||||
model: "zai-glm-4.7",
|
||||
connectionId: otherProviderId,
|
||||
},
|
||||
]);
|
||||
|
||||
assert.equal(await providersDb.deleteProviderConnectionsByProvider("nvidia"), 2);
|
||||
|
||||
const models = await readModels("provider-delete-8887");
|
||||
|
||||
assert.equal(
|
||||
"connectionId" in models[0],
|
||||
false,
|
||||
"provider delete must remove its first deleted direct pin"
|
||||
);
|
||||
assert.equal(
|
||||
"connectionId" in models[1],
|
||||
false,
|
||||
"provider delete must remove its second deleted direct pin"
|
||||
);
|
||||
assert.deepEqual(
|
||||
models[0].allowedConnectionIds,
|
||||
[otherProviderId],
|
||||
"provider delete must preserve ids belonging to other providers"
|
||||
);
|
||||
assert.equal(
|
||||
models[2].connectionId,
|
||||
otherProviderId,
|
||||
"provider delete must preserve another provider's direct pin"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user