fix: never lose custom endpoint models or pins on a failed probe
A connect that fell back to typed model ids replaced the stored model list
with the typed ids alone, then pruned the user pins against it. A transient
/models failure (503, timeout) therefore deleted every discovered model, its
detected metadata, and its pins, and answered 200.
- DiscoverResult carries the endpoint's HTTP status. A 401/403 fails the
connect even when ids are typed: a rejected key is not a missing model list.
- A failed probe keeps the stored snapshot for the SAME base URL, so the
prune cannot delete pins for a probe that failed. A new base URL starts
clean, so an older endpoint's models never follow the URL.
- Comment corrections: the route contract and the CustomProviderSettings
fields described the behaviour from before the manual-ids change.
- tests/custom-provider-route.test.ts covers the four branches: transient
failure, rejected key, new endpoint with no model list, working probe.
tsc and eslint clean. Suite: 2554 tests, 2 failures, both also failing on
6959482 (render-sidebar date drift, subscription-auth install artifact).
This commit is contained in:
@@ -1,11 +1,17 @@
|
||||
/**
|
||||
* The custom endpoint's connect surface (one provider in the Keys list).
|
||||
* GET → non-secret config { baseUrl, models, overrides } + keyPresent (masked).
|
||||
* POST → connect: validate + normalize the base URL, probe /models (fail
|
||||
* fast), store the key, persist { baseUrl, models }, re-sync the
|
||||
* provider into piModels(). Discovery lives here because connecting
|
||||
* and discovering are one user action. Stored user pins survive a
|
||||
* reconnect, pruned to the ids the endpoint still advertises.
|
||||
* GET → non-secret config { baseUrl, models, overrides, manualModels } +
|
||||
* keyPresent (masked).
|
||||
* POST → connect: validate + normalize the base URL, probe /models, store
|
||||
* the key, persist { baseUrl, models, overrides, manualModels },
|
||||
* re-sync the provider into piModels(). Discovery lives here because
|
||||
* connecting and discovering are one user action. A failed probe is
|
||||
* fatal UNLESS the user typed model IDs: an endpoint that serves chat
|
||||
* but no model list is configured from those ids alone. A failed
|
||||
* probe for the endpoint already stored keeps its last snapshot, so
|
||||
* a blip cannot prune the stored pins. A 401/403 always fails: a
|
||||
* rejected key is not a missing model list. Stored user pins survive
|
||||
* a reconnect, pruned to the ids the connect ends up offering.
|
||||
* PATCH → store per-model pins only. No probe: pins must be editable with
|
||||
* the local server switched off (the case they exist for).
|
||||
* DELETE → disconnect: drop the stored config + the stored key and unregister
|
||||
@@ -48,11 +54,25 @@ export async function POST(request: Request) {
|
||||
const key = keyProvided ? (body!.key as string).trim() : undefined;
|
||||
|
||||
const manualModels = parseModelIds(body?.manualModels);
|
||||
const record = readSettingsRecord() ?? { version: 1 };
|
||||
const found = await discoverModels(base, key ? key : undefined);
|
||||
// A rejected key is not a missing model list: never let typed ids turn an
|
||||
// auth failure into a stored "connected" endpoint.
|
||||
if (!found.ok && (found.status === 401 || found.status === 403)) {
|
||||
return Response.json({ error: found.error }, { status: 502 });
|
||||
}
|
||||
if (!found.ok && !manualModels.length) {
|
||||
return Response.json({ error: `${found.error}. If this endpoint has no /models list, add model IDs manually.` }, { status: 502 });
|
||||
}
|
||||
const models = mergeManualModels(found.ok ? found.models : [], manualModels);
|
||||
// A failed probe says nothing about what the endpoint offers: keep the last
|
||||
// snapshot of the SAME endpoint, so a transient failure cannot shrink the
|
||||
// model list and prune its pins. A different base URL starts clean.
|
||||
const discovered = found.ok
|
||||
? found.models
|
||||
: record.customProvider?.baseUrl === base
|
||||
? (record.customProvider?.models ?? [])
|
||||
: [];
|
||||
const models = mergeManualModels(discovered, manualModels);
|
||||
|
||||
if (keyProvided) {
|
||||
if (key) {
|
||||
@@ -63,9 +83,8 @@ export async function POST(request: Request) {
|
||||
}
|
||||
}
|
||||
|
||||
const record = readSettingsRecord() ?? { version: 1 };
|
||||
// User pins are never rewritten by discovery — only pruned to what the
|
||||
// endpoint still advertises (the single prune law, core/settings.ts).
|
||||
// User pins are never rewritten by discovery — only pruned to the ids this
|
||||
// connect offers (discovered ∪ typed).
|
||||
const overrides = sanitizeCustomModelOverrides(
|
||||
record.customProvider?.overrides,
|
||||
models.map((m) => m.id),
|
||||
|
||||
@@ -254,11 +254,16 @@ export interface CustomModelOverride {
|
||||
export interface CustomProviderSettings {
|
||||
/** Normalized base URL (see normalizeBaseUrl). Empty/absent = not configured. */
|
||||
baseUrl?: string;
|
||||
/** Models discovered from the endpoint (refreshed on connect). */
|
||||
/** Models this endpoint offers: advertised ids merged with the typed
|
||||
* manual ones (rewritten on connect; a failed probe keeps the same
|
||||
* endpoint's last snapshot). */
|
||||
models?: CustomModel[];
|
||||
/** User pins keyed by model id — pure user intent, never rewritten by
|
||||
* discovery, pruned to the endpoint's live ids on connect. */
|
||||
* discovery, pruned on connect to the ids that connect offers. */
|
||||
overrides?: Record<string, CustomModelOverride>;
|
||||
/** Model ids the user typed by hand, for an endpoint that publishes no
|
||||
* usable /models list. Kept apart from the snapshot so a reconnect can
|
||||
* re-merge them after discovery refreshes. */
|
||||
manualModels?: string[];
|
||||
}
|
||||
|
||||
|
||||
@@ -101,8 +101,12 @@ export function syncCustomProvider(cp: CustomProviderSettings | undefined): void
|
||||
);
|
||||
}
|
||||
|
||||
/** Outcome of a model-discovery probe. */
|
||||
export type DiscoverResult = { ok: true; models: CustomModel[] } | { ok: false; error: string };
|
||||
/** Outcome of a model-discovery probe. `status` is the endpoint's HTTP status
|
||||
* when it answered at all, so callers can tell a rejected key from a missing
|
||||
* model list. */
|
||||
export type DiscoverResult =
|
||||
| { ok: true; models: CustomModel[] }
|
||||
| { ok: false; error: string; status?: number };
|
||||
|
||||
/** Probe GET {baseUrl}/models for model ids. Scheme validated (http/https
|
||||
* only); keyless endpoints answer without a bearer. Non-2xx or unparseable
|
||||
@@ -118,7 +122,7 @@ export async function discoverModels(baseUrl: string, apiKey?: string): Promise<
|
||||
headers: key ? { authorization: `Bearer ${key}` } : {},
|
||||
signal: controller.signal,
|
||||
});
|
||||
if (!res.ok) return { ok: false, error: `Endpoint returned ${res.status} ${res.statusText}` };
|
||||
if (!res.ok) return { ok: false, error: `Endpoint returned ${res.status} ${res.statusText}`, status: res.status };
|
||||
const body = (await res.json().catch(() => null)) as unknown;
|
||||
const list = extractModels(body);
|
||||
if (list === null) return { ok: false, error: "Endpoint response was not a model list" };
|
||||
@@ -131,6 +135,9 @@ export async function discoverModels(baseUrl: string, apiKey?: string): Promise<
|
||||
}
|
||||
}
|
||||
|
||||
/** The connect's model list: what the endpoint advertises, plus every typed id
|
||||
* it does not advertise (at default limits, so the pins can correct them).
|
||||
* Discovered metadata always wins over a typed id of the same name. */
|
||||
export function mergeManualModels(discovered: readonly CustomModel[], manualIds: readonly string[]): CustomModel[] {
|
||||
const byId = new Map(discovered.map((m) => [m.id, m]));
|
||||
for (const id of manualIds) if (!byId.has(id)) byId.set(id, { id });
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
/**
|
||||
* POST /api/custom-provider — the connect policy around manually typed model
|
||||
* ids. Two rules earn a route test (the pure pieces live in
|
||||
* custom-endpoint.test.ts):
|
||||
* 1. Typed ids make a FAILED probe survivable, but never a rejected key.
|
||||
* 2. A failed probe never shrinks the snapshot it already stored: that prune
|
||||
* would take the model metadata and the user's pins with it.
|
||||
*/
|
||||
import assert from "node:assert/strict";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { after, before, describe, it } from "node:test";
|
||||
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "lh-custom-route-"));
|
||||
process.env.LH_DATA_DIR = dir;
|
||||
process.env.LH_AUTH_FILE = path.join(dir, "auth.json");
|
||||
const SETTINGS_FILE = path.join(dir, "settings.json");
|
||||
|
||||
const realFetch = globalThis.fetch;
|
||||
after(() => {
|
||||
globalThis.fetch = realFetch;
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
delete process.env.LH_DATA_DIR;
|
||||
delete process.env.LH_AUTH_FILE;
|
||||
});
|
||||
|
||||
type Route = typeof import("../app/api/custom-provider/route.ts");
|
||||
let route: Route;
|
||||
before(async () => {
|
||||
route = await import("../app/api/custom-provider/route.ts");
|
||||
});
|
||||
|
||||
const BASE = "http://h/v1";
|
||||
/** A connected endpoint with two discovered models, a pin on each, one typed id. */
|
||||
const CONNECTED = {
|
||||
version: 1,
|
||||
customProvider: {
|
||||
baseUrl: BASE,
|
||||
models: [{ id: "disc-1", context: 131072 }, { id: "disc-2" }],
|
||||
overrides: { "disc-1": { context: 200000 }, "disc-2": { maxTokens: 8192 } },
|
||||
manualModels: ["m1"],
|
||||
},
|
||||
};
|
||||
|
||||
function seed(record: unknown): void {
|
||||
fs.writeFileSync(SETTINGS_FILE, JSON.stringify(record, null, "\t"), "utf-8");
|
||||
}
|
||||
|
||||
function stored(): { customProvider?: { models?: { id: string }[]; overrides?: unknown; manualModels?: string[] } } {
|
||||
return JSON.parse(fs.readFileSync(SETTINGS_FILE, "utf-8"));
|
||||
}
|
||||
|
||||
function answerWith(status: number, statusText: string, body?: unknown): void {
|
||||
globalThis.fetch = (async () => (body === undefined ? new Response("nope", { status, statusText }) : Response.json(body))) as typeof fetch;
|
||||
}
|
||||
|
||||
function post(payload: unknown): Promise<Response> {
|
||||
return route.POST(
|
||||
new Request("http://localhost/api/custom-provider", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(payload),
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
describe("POST /api/custom-provider — typed ids and a failed probe", () => {
|
||||
it("a transient failure keeps the stored models, their metadata, and their pins", async () => {
|
||||
seed(CONNECTED);
|
||||
answerWith(503, "Service Unavailable");
|
||||
|
||||
const res = await post({ baseUrl: BASE, manualModels: "m1" });
|
||||
assert.equal(res.status, 200);
|
||||
|
||||
const cp = stored().customProvider;
|
||||
assert.deepEqual(cp?.models?.map((m) => m.id), ["disc-1", "disc-2", "m1"]);
|
||||
assert.deepEqual(cp?.models?.[0], { id: "disc-1", context: 131072 }, "detected metadata survives");
|
||||
assert.deepEqual(cp?.overrides, CONNECTED.customProvider.overrides, "the prune must not fire on a failed probe");
|
||||
assert.deepEqual(cp?.manualModels, ["m1"]);
|
||||
});
|
||||
|
||||
it("a rejected key fails the connect even when ids are typed", async () => {
|
||||
seed(CONNECTED);
|
||||
answerWith(401, "Unauthorized");
|
||||
const before = stored();
|
||||
|
||||
const res = await post({ baseUrl: BASE, key: "sk-abcdefghijk", manualModels: "m1" });
|
||||
assert.equal(res.status, 502);
|
||||
assert.deepEqual(stored(), before, "nothing is stored on an auth failure");
|
||||
});
|
||||
|
||||
it("a new endpoint with no model list starts from the typed ids alone", async () => {
|
||||
seed(CONNECTED);
|
||||
answerWith(404, "Not Found");
|
||||
|
||||
const res = await post({ baseUrl: "http://other/v1", manualModels: "y, x" });
|
||||
assert.equal(res.status, 200);
|
||||
|
||||
const cp = stored().customProvider;
|
||||
assert.deepEqual(cp?.models?.map((m) => m.id), ["x", "y"], "the previous endpoint's models do not follow the URL");
|
||||
assert.equal(cp?.overrides, undefined, "no pin survives a list the old pins do not belong to");
|
||||
assert.deepEqual(cp?.manualModels, ["y", "x"], "the typed order is kept as typed");
|
||||
});
|
||||
|
||||
it("no model list and no typed ids still refuses, with the hint", async () => {
|
||||
answerWith(404, "Not Found");
|
||||
|
||||
const res = await post({ baseUrl: BASE });
|
||||
assert.equal(res.status, 502);
|
||||
const body = (await res.json()) as { error: string };
|
||||
assert.match(body.error, /404/);
|
||||
assert.match(body.error, /add model IDs manually/);
|
||||
});
|
||||
|
||||
it("a working probe merges the typed ids and keeps discovered metadata", async () => {
|
||||
seed(CONNECTED);
|
||||
answerWith(200, "OK", { data: [{ id: "disc-1", context_length: 65536 }] });
|
||||
|
||||
const res = await post({ baseUrl: BASE, manualModels: "disc-1, m2" });
|
||||
assert.equal(res.status, 200);
|
||||
|
||||
const cp = stored().customProvider;
|
||||
assert.deepEqual(cp?.models?.map((m) => m.id), ["disc-1", "m2"]);
|
||||
assert.deepEqual(cp?.models?.[0], { id: "disc-1", context: 65536 }, "discovery is not replaced by a bare typed id");
|
||||
assert.deepEqual(cp?.overrides, { "disc-1": { context: 200000 } }, "pins follow the ids this connect offers");
|
||||
assert.deepEqual(cp?.manualModels, ["disc-1", "m2"]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user