diff --git a/app/api/custom-provider/route.ts b/app/api/custom-provider/route.ts index 5280609..f77ad78 100644 --- a/app/api/custom-provider/route.ts +++ b/app/api/custom-provider/route.ts @@ -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), diff --git a/src/core/settings.ts b/src/core/settings.ts index 18b3e1d..22bf9d9 100644 --- a/src/core/settings.ts +++ b/src/core/settings.ts @@ -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; + /** 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[]; } diff --git a/src/server/custom-provider.ts b/src/server/custom-provider.ts index 4dc3eb6..2d947e6 100644 --- a/src/server/custom-provider.ts +++ b/src/server/custom-provider.ts @@ -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 }); diff --git a/tests/custom-provider-route.test.ts b/tests/custom-provider-route.test.ts new file mode 100644 index 0000000..6315ded --- /dev/null +++ b/tests/custom-provider-route.test.ts @@ -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 { + 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"]); + }); +});