diff --git a/packages/keiko-model-gateway/src/config.ts b/packages/keiko-model-gateway/src/config.ts index b82f762923..73f1aa6056 100644 --- a/packages/keiko-model-gateway/src/config.ts +++ b/packages/keiko-model-gateway/src/config.ts @@ -575,14 +575,22 @@ function resolveProviderEndpointStyle( function resolveProviderApiVersion( rawValue: unknown, - path: string, + providerPath: string, modelId: string, env: EnvSource, + endpointStyle: ProviderEndpointStyle | undefined, ): string | undefined { + const path = `${providerPath}.apiVersion`; const token = envModelToken(modelId); const perModelName = `KEIKO_MODEL_${token}_API_VERSION`; const perModel = env[perModelName]; - const defaultValue = env.KEIKO_DEFAULT_API_VERSION; + // The GLOBAL default applies only where an api version is meaningful: a provider whose + // resolved style is not the Azure deployment path must not inherit it, or one + // KEIKO_DEFAULT_API_VERSION would make every explicitly openai-compatible provider on the + // same server unparseable through the pairing rule below (review finding on #3042 — + // per-model and file values stay authoritative and still fail the pairing loudly). + const defaultValue = + endpointStyle === "azure-openai-deployment" ? env.KEIKO_DEFAULT_API_VERSION : undefined; const value = resolvePerModelValue(perModel, rawValue, defaultValue); if (value === undefined || value === "") { return undefined; @@ -1273,7 +1281,7 @@ function parseProviderConfig( modelId, env, ); - const apiVersion = resolveProviderApiVersion(raw.apiVersion, `${path}.apiVersion`, modelId, env); + const apiVersion = resolveProviderApiVersion(raw.apiVersion, path, modelId, env, endpointStyle); assertProviderEndpointVersion(endpointStyle, apiVersion, path); const realtimeAuthMode = resolveRealtimeAuthMode( raw.realtimeAuthMode, diff --git a/packages/keiko-server/src/gateway-setup.test.ts b/packages/keiko-server/src/gateway-setup.test.ts index 10236f8f20..8e32a635f1 100644 --- a/packages/keiko-server/src/gateway-setup.test.ts +++ b/packages/keiko-server/src/gateway-setup.test.ts @@ -6122,6 +6122,994 @@ describe("handleGatewaySetup", () => { deps.store.close(); } }); + it("persists an explicitly submitted generic endpoint style over the environment default", async () => { + // Codex finding on #3042: the setup request had no generic endpointStyle field, so an + // uploaded LiteLLM config declaring openai-compatible was rebuilt style-less and a server + // running KEIKO_DEFAULT_ENDPOINT_STYLE=azure-openai-deployment resolved the env default — + // wrong URL shape after a reported upload success. + const uiDir = await tempDir("keiko-gw-ui-generic-style-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-generic-style-"), + env: { + ...VAULT_ENV, + KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment", + KEIKO_DEFAULT_API_VERSION: "2025-04-01-preview", + }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewayModelDiscovery: () => Promise.resolve(["example-chat-model"]), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const result = await handleGatewaySetup( + ctx({ + baseUrl: "https://llm-gateway.example.com/v1", + apiKey: "chat-token", + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(result.status).toBe(200); + const provider = currentGatewayConfig(deps)?.providers.find( + (item) => item.modelId === "example-chat-model", + ); + expect(provider?.endpointStyle).toBe("openai-compatible"); + // The default api version is azure-only: an openai-compatible provider must not carry it, + // which is the half of the override this test exists for (review finding on #3046). + expect(provider?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + it("persists a submitted Azure endpoint style with its api version on generic providers", async () => { + const uiDir = await tempDir("keiko-gw-ui-generic-azure-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-generic-azure-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const result = await handleGatewaySetup( + ctx({ + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + deploymentNames: ["azure-chat"], + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }), + deps, + ); + expect(result.status).toBe(200); + const provider = currentGatewayConfig(deps)?.providers.find( + (item) => item.modelId === "azure-chat", + ); + expect(provider?.endpointStyle).toBe("azure-openai-deployment"); + expect(provider?.apiVersion).toBe("2025-04-01-preview"); + deps.store.close(); + }); + it("moves a shared restored provider onto the new gateway protocol", async () => { + // Review finding on #3046: the restored provider followed the shared connection's URL, token + // and header but kept its own endpointStyle, so one connection ended up carrying two + // protocols and the restored provider kept requesting the obsolete route. + const uiDir = await tempDir("keiko-gw-ui-shared-protocol-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + { + modelId: "scan-ocr", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + capability: durableOcrCapability("scan-ocr"), + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-shared-protocol-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => + Promise.resolve(modelIds.filter((modelId) => modelId !== "scan-ocr")), + }); + + const updated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + apiKey: "rotated-token", + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(updated.status).toBe(200); + const byId = new Map( + (currentGatewayConfig(deps)?.providers ?? []).map((provider) => [provider.modelId, provider]), + ); + expect(byId.get("example-chat")?.endpointStyle).toBe("openai-compatible"); + expect(byId.get("scan-ocr")?.endpointStyle).toBe("openai-compatible"); + expect(byId.get("scan-ocr")?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + + it("leaves a restored provider that spoke its own protocol alone", async () => { + // Review finding on #3046: making a shared provider follow the gateway's protocol went one + // step too far. A provider on the same URL, credential and header that deliberately used a + // DIFFERENT valid protocol had its own overwritten by a rotation, even though the gateway's + // request shape was never verified for it. + const uiDir = await tempDir("keiko-gw-ui-own-protocol-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + { + modelId: "scan-ocr", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "openai-compatible", + capability: durableOcrCapability("scan-ocr"), + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-own-protocol-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => + Promise.resolve(modelIds.filter((modelId) => modelId !== "scan-ocr")), + }); + + const rotated = await handleGatewaySetup( + ctx({ preserveExisting: true, apiKey: "rotated-token" }), + deps, + ); + expect(rotated.status).toBe(200); + const ocr = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "scan-ocr", + ); + // It follows the dead credential, because that is the connection it shared… + expect(ocr?.apiKey).toBe("rotated-token"); + // …but keeps the protocol it chose. + expect(ocr?.endpointStyle).toBe("openai-compatible"); + expect(ocr?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + + it("validates the connection with the protocol the setup will persist", async () => { + // Found while pinning the env-completed tuple (review findings on #3046): the connection + // probe parsed a provider with NO protocol, so on a server that sets only + // KEIKO_DEFAULT_ENDPOINT_STYLE the environment turned every probe into an Azure provider + // with no api version and the canonical pairing rejected EVERY setup request — including one + // that explicitly submits a protocol of its own. + const uiDir = await tempDir("keiko-gw-ui-probe-protocol-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-probe-protocol-"), + env: { ...VAULT_ENV, KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment" }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const result = await handleGatewaySetup( + ctx({ + baseUrl: "https://llm-gateway.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["example-chat"], + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(result.status).toBe(200); + const saved = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "example-chat", + ); + expect(saved?.endpointStyle).toBe("openai-compatible"); + // …and the environment must not complete the tuple it was told not to use. + expect(saved?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + + it("rotates a declared api version whose style comes from the environment", async () => { + // Review finding on #3046, the inverse of the pin below: a file that declares apiVersion + // while KEIKO_DEFAULT_ENDPOINT_STYLE supplies the Azure style is a valid completed tuple, and + // clearing the env half left a version with no style — rejected on the next rotation. + const uiDir = await tempDir("keiko-gw-ui-env-style-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + apiVersion: "2025-03-01-preview", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-env-style-"), + env: { ...VAULT_ENV, KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment" }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const rotated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://llm.example.com/v1", + apiKey: "rotated-token", + deploymentNames: ["example-chat"], + }), + deps, + ); + expect(rotated.status).toBe(200); + const saved = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "example-chat", + ); + expect(saved?.apiVersion).toBe("2025-03-01-preview"); + deps.store.close(); + }); + + it("discards observations when an import switches away from an env-resolved Azure tuple", async () => { + // Review finding on #3046: comparing the DURABLE view against the submitted one read a file + // that declares nothing while KEIKO_DEFAULT_* resolves Azure as "unchanged" when the request + // explicitly switched to openai-compatible — so observations made over the deployment path + // survived onto a different request shape. + const uiDir = await tempDir("keiko-gw-ui-env-switch-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + capability: { kind: "chat", supportsDocumentInput: true }, + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-env-switch-"), + env: { + ...VAULT_ENV, + KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment", + KEIKO_DEFAULT_API_VERSION: "2025-04-01-preview", + }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const switched = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["example-chat"], + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(switched.status).toBe(200); + const capability = currentGatewayConfig(deps)?.capabilities?.find( + (entry) => entry.id === "example-chat", + ); + expect(capability).toMatchObject({ supportsDocumentInput: false }); + deps.store.close(); + }); + + it("keeps observations when an import only spells out the default protocol", async () => { + // Review finding on #3046: a stored provider with no endpointStyle and a same-URL import that + // declares "openai-compatible" are the SAME protocol — the adapter sends the identical + // request shape — but the raw comparison read it as a change and discarded verified + // observations the setup probe never re-establishes. + const uiDir = await tempDir("keiko-gw-ui-default-protocol-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-default-protocol-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + deps.gatewayConfig?.set( + parseGatewayConfig({ + providers: [ + { + modelId: "plain-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + capability: { kind: "chat", supportsDocumentInput: true }, + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + + const imported = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["plain-chat"], + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(imported.status).toBe(200); + const capability = currentGatewayConfig(deps)?.capabilities?.find( + (entry) => entry.id === "plain-chat", + ); + expect(capability).toMatchObject({ supportsDocumentInput: true }); + deps.store.close(); + }); + + it("requires deployments when the environment puts the setup on the Azure path", async () => { + // Review finding on #3046: the guard read the RAW request style, so a fresh setup that omits + // the field while KEIKO_DEFAULT_ENDPOINT_STYLE supplies the deployment path fell through to + // generic /models discovery and failed there instead of naming the missing deployments. + const uiDir = await tempDir("keiko-gw-ui-env-deployments-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-env-deployments-"), + env: { + ...VAULT_ENV, + KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment", + KEIKO_DEFAULT_API_VERSION: "2025-04-01-preview", + }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const result = await handleGatewaySetup( + ctx({ baseUrl: "https://my-azure.openai.azure.com", apiKey: "azure-token" }), + deps, + ); + expect(result.status).toBe(400); + expect(result.body).toMatchObject({ error: { code: "GATEWAY_DEPLOYMENTS_REQUIRED" } }); + deps.store.close(); + }); + + it("leaves a dedicated embedding that spoke its own protocol alone", async () => { + // Review finding on #3046: an embedding sharing the primary's URL, credential and header but + // deliberately using a different valid protocol was classified as sharing, so a same-URL + // protocol update rebuilt it with a request shape nothing probed for it. + const uiDir = await tempDir("keiko-gw-ui-embed-protocol-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + { + modelId: "embed-small", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "openai-compatible", + capability: { kind: "embedding", dimensions: 1536 }, + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-embed-protocol-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => + Promise.resolve(modelIds.filter((modelId) => modelId !== "embed-small")), + }); + + const rotated = await handleGatewaySetup( + ctx({ preserveExisting: true, apiKey: "rotated-token" }), + deps, + ); + expect(rotated.status).toBe(200); + const embedding = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "embed-small", + ); + expect(embedding?.endpointStyle).toBe("openai-compatible"); + expect(embedding?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + + it("rotates a declared Azure endpoint whose required version comes from the environment", async () => { + // Review finding on #3046: dropping every undeclared protocol value from the durable view + // left a file that DECLARES azure-openai-deployment and takes its required version from + // KEIKO_MODEL__API_VERSION carrying azure with no version — and inheritance then + // rejected a routine credential rotation with 400. Only a version the pair does not need is + // dropped. + const uiDir = await tempDir("keiko-gw-ui-env-version-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + endpointStyle: "azure-openai-deployment", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-env-version-"), + env: { ...VAULT_ENV, KEIKO_MODEL_EXAMPLE_CHAT_API_VERSION: "2025-04-01-preview" }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const rotated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://llm.example.com/v1", + apiKey: "rotated-token", + deploymentNames: ["example-chat"], + }), + deps, + ); + expect(rotated.status).toBe(200); + const saved = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "example-chat", + ); + expect(saved?.endpointStyle).toBe("azure-openai-deployment"); + deps.store.close(); + }); + + it("keeps verified observations when a rotation only re-resolves an env protocol", async () => { + // Review finding on #3046: comparing the capability identity against the env-RESOLVED view + // made a plain rotation look like a protocol change whenever KEIKO_DEFAULT_* supplied a + // tuple the file never declared — discarding verified observations for nothing. + const uiDir = await tempDir("keiko-gw-ui-env-capability-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + capability: { kind: "chat", supportsDocumentInput: true }, + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-env-capability-"), + env: { + ...VAULT_ENV, + KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment", + KEIKO_DEFAULT_API_VERSION: "2025-04-01-preview", + }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const rotated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://llm.example.com/v1", + apiKey: "rotated-token", + deploymentNames: ["example-chat"], + }), + deps, + ); + expect(rotated.status).toBe(200); + const capability = currentGatewayConfig(deps)?.capabilities?.find( + (entry) => entry.id === "example-chat", + ); + expect(capability).toMatchObject({ supportsDocumentInput: true }); + deps.store.close(); + }); + + it("never seals an environment-supplied protocol into the stored file", async () => { + // Review finding on #3046: inheritance read the env-RESOLVED view, so a rotation wrote a + // protocol the file never declared into the sealed config — removing the variable afterwards + // no longer restored the file's own behavior. + const uiDir = await tempDir("keiko-gw-ui-env-protocol-"); + writeFileSync( + join(uiDir, "keiko.config.json"), + JSON.stringify({ + providers: [ + { + modelId: "example-chat", + baseUrl: "https://llm.example.com/v1", + apiKey: "chat-token", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + "utf8", + ); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-env-protocol-"), + env: { + ...VAULT_ENV, + KEIKO_DEFAULT_ENDPOINT_STYLE: "azure-openai-deployment", + KEIKO_DEFAULT_API_VERSION: "2025-04-01-preview", + }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + + const rotated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://llm.example.com/v1", + apiKey: "rotated-token", + deploymentNames: ["example-chat"], + }), + deps, + ); + expect(rotated.status).toBe(200); + const saved: unknown = JSON.parse(readFileSync(deps.gatewayConfig?.storagePath ?? "", "utf8")); + const providers = (saved as { providers: readonly Record[] }).providers; + const chat = providers.find((provider) => provider.modelId === "example-chat"); + expect(chat).toBeDefined(); + expect(chat).not.toHaveProperty("endpointStyle"); + expect(chat).not.toHaveProperty("apiVersion"); + deps.store.close(); + }); + + it("does not reuse verified chat observations across a protocol change", async () => { + // Review finding on #3046: capability reuse keyed on modelId and base URL only, so a + // same-URL protocol change kept observations made over the OLD request route. The setup + // probe performs buffered chat and reverifies none of them. + const uiDir = await tempDir("keiko-gw-ui-protocol-capability-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-protocol-capability-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const gatewayConfig = deps.gatewayConfig; + if (gatewayConfig === undefined) throw new Error("expected gateway config store"); + gatewayConfig.set( + parseGatewayConfig({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + capability: { kind: "chat", supportsDocumentInput: true }, + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + + const changed = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + deploymentNames: ["azure-chat"], + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(changed.status).toBe(200); + const capability = currentGatewayConfig(deps)?.capabilities?.find( + (entry) => entry.id === "azure-chat", + ); + expect(capability?.kind).toBe("chat"); + // supportsDocumentInput defaults to false and has no authoritative request list, so it can + // only be true here by REUSING the stored capability — which is what a protocol change must + // stop doing. + expect(capability).toMatchObject({ supportsDocumentInput: false }); + deps.store.close(); + }); + + it("inherits the generic protocol from the primary chat provider, not from position zero", async () => { + // Review finding on #3046: the inheritance compared against providers[0]. Array order is not + // a contract — a valid stored file may list a dedicated voice provider first — so a stored + // Azure chat endpoint lost its protocol on an unchanged-endpoint rotation, and the gateway + // was reprobed and persisted with the wrong shape. + const uiDir = await tempDir("keiko-gw-ui-generic-primary-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-generic-primary-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const gatewayConfig = deps.gatewayConfig; + if (gatewayConfig === undefined) throw new Error("expected gateway config store"); + gatewayConfig.set( + parseGatewayConfig({ + providers: [ + { + modelId: "keiko-stt", + baseUrl: "https://speech.example.com", + apiKey: "voice-token", + capability: { + kind: "voice", + supportsSpeechInput: true, + voiceProviderLocality: "customer-hosted", + }, + }, + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + + const rotated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://resource.example.com", + apiKey: "rotated-token", + deploymentNames: ["azure-chat"], + }), + deps, + ); + expect(rotated.status).toBe(200); + const afterRotation = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "azure-chat", + ); + expect(afterRotation?.endpointStyle).toBe("azure-openai-deployment"); + expect(afterRotation?.apiVersion).toBe("2025-03-01-preview"); + deps.store.close(); + }); + it("requires deployment names when the submitted protocol IS the Azure deployment path", async () => { + // Review finding on #3046: the requirement keyed only off a *.services.ai.azure.com + // hostname. Now that the protocol can be stated explicitly, a classic Azure OpenAI host on + // the deployment path with no deployments fell through to generic /models discovery and + // failed there instead of naming what was missing. + const uiDir = await tempDir("keiko-gw-ui-azure-style-deployments-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-azure-style-deployments-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const result = await handleGatewaySetup( + ctx({ + baseUrl: "https://my-azure.openai.azure.com", + apiKey: "azure-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }), + deps, + ); + expect(result.status).toBe(400); + expect(result.body).toMatchObject({ error: { code: "GATEWAY_DEPLOYMENTS_REQUIRED" } }); + deps.store.close(); + }); + it("inherits a stored generic protocol on the SAME endpoint and drops it on a move", async () => { + // Both branches of the inheritance guard (review finding on #3046): a protocol was declared + // for one endpoint, so it survives a credential rotation there and must NOT follow the + // connection to a different host, where nothing declared it. + const uiDir = await tempDir("keiko-gw-ui-generic-inherit-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-generic-inherit-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const gatewayConfig = deps.gatewayConfig; + if (gatewayConfig === undefined) throw new Error("expected gateway config store"); + gatewayConfig.set( + parseGatewayConfig({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + + // SAME endpoint, protocol omitted: the stored declaration still applies. + const rotated = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://resource.example.com", + apiKey: "rotated-token", + deploymentNames: ["azure-chat"], + }), + deps, + ); + expect(rotated.status).toBe(200); + const afterRotation = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "azure-chat", + ); + expect(afterRotation?.endpointStyle).toBe("azure-openai-deployment"); + expect(afterRotation?.apiVersion).toBe("2025-03-01-preview"); + + // DIFFERENT endpoint, protocol omitted: nothing declared it for the new host. + const moved = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://other-resource.example.com", + apiKey: "moved-token", + deploymentNames: ["azure-chat"], + }), + deps, + ); + expect(moved.status).toBe(200); + const afterMove = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "azure-chat", + ); + expect(afterMove?.baseUrl).toBe("https://other-resource.example.com"); + expect(afterMove?.endpointStyle).toBeUndefined(); + expect(afterMove?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + it("replaces the stored protocol atomically when a new style is submitted", async () => { + // Review finding on #3046: a submitted style with an inherited api version is a MIXED + // protocol the canonical parser refuses (a version requires the Azure style), so switching + // an Azure provider to openai-compatible on the same URL failed the save instead of + // replacing the protocol. The submitted protocol is a unit. + const uiDir = await tempDir("keiko-gw-ui-protocol-atomic-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-protocol-atomic-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const gatewayConfig = deps.gatewayConfig; + if (gatewayConfig === undefined) throw new Error("expected gateway config store"); + gatewayConfig.set( + parseGatewayConfig({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + + const switched = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + deploymentNames: ["azure-chat"], + endpointStyle: "openai-compatible", + }), + deps, + ); + expect(switched.status).toBe(200); + const saved = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "azure-chat", + ); + expect(saved?.endpointStyle).toBe("openai-compatible"); + expect(saved?.apiVersion).toBeUndefined(); + deps.store.close(); + }); + + it("verifies and saves a protocol-only preserve-mode update", async () => { + // Review finding on #3046: with only endpointStyle/apiVersion submitted, the request took + // the settings-only path, which never rebuilds providers — the protocol change was accepted + // and silently dropped. + const uiDir = await tempDir("keiko-gw-ui-protocol-only-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-protocol-only-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const gatewayConfig = deps.gatewayConfig; + if (gatewayConfig === undefined) throw new Error("expected gateway config store"); + gatewayConfig.set( + parseGatewayConfig({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + + const protocolOnly = await handleGatewaySetup( + ctx({ + preserveExisting: true, + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }), + deps, + ); + expect(protocolOnly.status).toBe(200); + const saved = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "azure-chat", + ); + expect(saved?.endpointStyle).toBe("azure-openai-deployment"); + expect(saved?.apiVersion).toBe("2025-04-01-preview"); + // A protocol-only update changes the protocol and nothing else — without this the test would + // pass over a rebuild that dropped the connection (review finding on #3046). + expect(saved?.baseUrl).toBe("https://resource.example.com"); + expect(saved?.apiKey).toBe("azure-token"); + deps.store.close(); + }); + + it("names the pairing when an api version has no Azure endpoint to belong to", async () => { + // Review finding on #3046: an api version with no style (fresh) or over an inherited + // openai-compatible style built a pair the canonical parser refuses. The throw surfaced as an + // opaque 502 "credentials could not be verified", which is a request problem reported as an + // upstream one. Bumping the version of a stored Azure endpoint stays legal — same pair. + const uiDir = await tempDir("keiko-gw-ui-orphan-version-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-orphan-version-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const orphan = await handleGatewaySetup( + ctx({ + baseUrl: "https://llm-gateway.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["example-chat"], + apiVersion: "2025-04-01-preview", + }), + deps, + ); + expect(orphan.status).toBe(400); + expect(orphan.body).toMatchObject({ + error: { code: "GATEWAY_API_VERSION_REQUIRES_AZURE_ENDPOINT" }, + }); + + // The other direction of the same rule: the deployment path needs the version that builds + // its URL, and left unnamed it produced the same misleading 502. + const styleWithoutVersion = await handleGatewaySetup( + ctx({ + baseUrl: "https://llm-gateway.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["example-chat"], + endpointStyle: "azure-openai-deployment", + }), + deps, + ); + expect(styleWithoutVersion.status).toBe(400); + expect(styleWithoutVersion.body).toMatchObject({ + error: { code: "GATEWAY_AZURE_ENDPOINT_REQUIRES_API_VERSION" }, + }); + + // A malformed version is a malformed request, not an upstream failure. + const malformed = await handleGatewaySetup( + ctx({ + baseUrl: "https://llm-gateway.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["example-chat"], + endpointStyle: "azure-openai-deployment", + apiVersion: "not-a-date", + }), + deps, + ); + expect(malformed.status).toBe(400); + expect(malformed.body).toMatchObject({ error: { code: "GATEWAY_API_VERSION_INVALID" } }); + + const gatewayConfig = deps.gatewayConfig; + if (gatewayConfig === undefined) throw new Error("expected gateway config store"); + gatewayConfig.set( + parseGatewayConfig({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }, + ], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }), + true, + ); + // The legal case the blunt "reject apiVersion without endpointStyle" rule would have broken. + const bumped = await handleGatewaySetup( + ctx({ + preserveExisting: true, + baseUrl: "https://resource.example.com", + apiKey: "azure-token", + deploymentNames: ["azure-chat"], + apiVersion: "2025-04-01-preview", + }), + deps, + ); + expect(bumped.status).toBe(200); + const saved = currentGatewayConfig(deps)?.providers.find( + (provider) => provider.modelId === "azure-chat", + ); + expect(saved?.endpointStyle).toBe("azure-openai-deployment"); + expect(saved?.apiVersion).toBe("2025-04-01-preview"); + deps.store.close(); + }); + + it("rejects an unsupported generic endpoint style", async () => { + const uiDir = await tempDir("keiko-gw-ui-generic-style-bad-"); + const deps = buildUiHandlerDeps({ + configPath: undefined, + evidenceDir: await tempDir("keiko-gw-ev-generic-style-bad-"), + env: { ...VAULT_ENV }, + uiDbPath: join(uiDir, "keiko-ui.db"), + gatewaySetupTester: (_config, modelIds) => Promise.resolve(modelIds), + }); + const result = await handleGatewaySetup( + ctx({ + baseUrl: "https://llm-gateway.example.com/v1", + apiKey: "chat-token", + deploymentNames: ["example-chat"], + endpointStyle: "bogus-style", + }), + deps, + ); + expect(result).toMatchObject({ status: 400, body: { error: { code: "BAD_REQUEST" } } }); + deps.store.close(); + }); }); // Issue #144: discovery-normalization seam tests. Synthetic generic IDs only — diff --git a/packages/keiko-server/src/gateway-setup.ts b/packages/keiko-server/src/gateway-setup.ts index 089f8f2ab4..790783aec6 100644 --- a/packages/keiko-server/src/gateway-setup.ts +++ b/packages/keiko-server/src/gateway-setup.ts @@ -305,11 +305,16 @@ interface ProviderRawOptions { readonly maxRetries?: number | undefined; readonly retryBaseDelayMs?: number | undefined; readonly apiKeyHeaderName?: string | undefined; + /** Generic endpoint protocol, persisted VERBATIM — see setupEndpointProtocol (#3042). */ + readonly endpointStyle?: string | undefined; + readonly apiVersion?: string | undefined; readonly imageInputModelIds?: readonly string[] | undefined; readonly responseFormatModelIds?: readonly string[] | undefined; readonly embeddingModelIds?: readonly string[] | undefined; readonly modelMetadata?: Readonly> | undefined; readonly current?: GatewayConfig | undefined; + /** The durable stored view — the protocol of record for capability identity (see #3046). */ + readonly stored?: GatewayConfig | undefined; readonly workflowEligibleModelIds?: readonly string[] | undefined; } @@ -317,13 +322,76 @@ interface ProviderRawOptions { // observation (recordVerifiedCapability → replaceModelCapability) survives a routine re-save, // which submits no preserveExisting flag. The mode gate belongs on the ENDPOINT-MOVE carry-over // below, where this URL match misses and nothing verified the stored value at the new endpoint. +// The protocol the adapter actually speaks: an absent endpoint style IS the OpenAI-compatible +// shape, so the two spellings must compare equal wherever a protocol CHANGE is the question. +function effectiveEndpointStyle(style: string | undefined): string { + return style ?? "openai-compatible"; +} + +function storedProviderForModel( + stored: GatewayConfig | undefined, + modelId: string, +): ModelProviderConfig | undefined { + return stored?.providers.find((candidate) => candidate.modelId === modelId); +} + +// What the adapter will send AFTER the save: the statement if there is one, else what the file +// declares, else what the environment already resolves the provider to. Falling through to the +// resolved provider is what keeps a plain rotation — which states nothing — comparing equal. +function effectiveSubmittedProtocol( + protocol: { + readonly submitted: { + readonly endpointStyle?: string | undefined; + readonly apiVersion?: string | undefined; + }; + readonly durable: ModelProviderConfig | undefined; + }, + provider: ModelProviderConfig, +): { readonly endpointStyle: string | undefined; readonly apiVersion: string | undefined } { + return { + endpointStyle: + protocol.submitted.endpointStyle ?? protocol.durable?.endpointStyle ?? provider.endpointStyle, + apiVersion: + protocol.submitted.apiVersion ?? protocol.durable?.apiVersion ?? provider.apiVersion, + }; +} + function existingCapabilityForSetup( current: GatewayConfig | undefined, modelId: string, baseUrl: string, + protocol: { + readonly submitted: { + readonly endpointStyle?: string | undefined; + readonly apiVersion?: string | undefined; + }; + readonly durable: ModelProviderConfig | undefined; + }, ): ModelCapability | undefined { const provider = current?.providers.find((candidate) => candidate.modelId === modelId); if (provider === undefined || !sameBaseUrlIdentity(provider.baseUrl, baseUrl)) return undefined; + // Both sides are the EFFECTIVE protocol — what the adapter will actually send. The durable + // view alone was not enough: with the file silent and KEIKO_DEFAULT_* resolving `current` to + // Azure, an explicit switch to openai-compatible compared undefined against openai-compatible + // and read as unchanged, keeping observations made over the deployment path (review finding on + // #3046). Completing the submitted side with the same defaults keeps a plain rotation — which + // states nothing — comparing equal. + const submitted = effectiveSubmittedProtocol(protocol, provider); + // The protocol is part of the endpoint's identity: the deployment path and the api version + // change the request route, so streaming, tool calling and image observations made over the + // old one prove nothing about the new one. The setup probe performs buffered chat only and + // reverifies none of them, so a reused capability would advertise unverified behavior (review + // finding on #3046). Unchanged protocol, unchanged identity — a rotation still keeps them. + // An absent style and an explicit "openai-compatible" are the SAME protocol — the adapter sends + // the identical request shape — so a same-URL import that merely spells the default out must + // not discard verified observations (review finding on #3046). + if ( + effectiveEndpointStyle(provider.endpointStyle) !== + effectiveEndpointStyle(submitted.endpointStyle) || + provider.apiVersion !== submitted.apiVersion + ) { + return undefined; + } return current?.capabilities?.find((candidate) => candidate.id === modelId); } @@ -432,7 +500,14 @@ function createDefaultSetupCapability( embeddingModelIds?.includes(modelId) === true ? createDefaultEmbeddingCapability(modelId) : createDefaultChatCapability(modelId); - const existing = existingCapabilityForSetup(options.current, modelId, baseUrl); + const existing = existingCapabilityForSetup(options.current, modelId, baseUrl, { + submitted: options, + // The protocol of record is the DURABLE one, which is what the rebuild persists. Comparing + // against the env-RESOLVED view made a plain rotation look like a protocol change whenever + // KEIKO_DEFAULT_* supplied a tuple the file never declared, discarding verified observations + // for nothing (review finding on #3046). + durable: storedProviderForModel(options.stored, modelId), + }); const discovered = options.modelMetadata?.[modelId]; const capability: ModelCapability = { ...baseCapability, @@ -460,6 +535,17 @@ function createDefaultSetupCapability( ); } +// The generic endpoint protocol persists VERBATIM — absent fields stay absent so the runtime +// default layering is unchanged (#3042). +function genericEndpointProtocolRaw( + options: ProviderRawOptions, +): Pick, string> { + return { + ...(options.endpointStyle === undefined ? {} : { endpointStyle: options.endpointStyle }), + ...(options.apiVersion === undefined ? {} : { apiVersion: options.apiVersion }), + }; +} + function providerRaw( modelId: string, baseUrl: string, @@ -478,6 +564,7 @@ function providerRaw( baseUrl, apiKey, apiKeyHeaderName: options.apiKeyHeaderName ?? DEFAULT_API_KEY_HEADER_NAME, + ...genericEndpointProtocolRaw(options), capability: { ...defaultCapability, // The provided list is authoritative, not additive: a model absent from it loses a stored @@ -821,7 +908,12 @@ function currentDedicatedEmbeddingModelIds(config: GatewayConfig | undefined): r const sharesConnection = sameBaseUrlIdentity(provider.baseUrl, primary.baseUrl) && provider.apiKey === primary.apiKey && - (provider.apiKeyHeaderName ?? DEFAULT_API_KEY_HEADER_NAME) === primaryHeader; + (provider.apiKeyHeaderName ?? DEFAULT_API_KEY_HEADER_NAME) === primaryHeader && + // A provider that deliberately spoke a DIFFERENT protocol over the same connection is + // dedicated for this purpose: rebuilding it with the setup-wide protocol would put an + // unprobed request shape on it (review finding on #3046, the embedding twin of the + // restored-provider rule). + spokeStoredGatewayProtocol(provider, primary); return !sharesConnection; }) .map((provider) => provider.modelId); @@ -1375,12 +1467,20 @@ function validateSetupConnection( apiKey: string, apiKeyHeaderName: string, env: EnvSource, + protocol: { + readonly endpointStyle?: string | undefined; + readonly apiVersion?: string | undefined; + } = {}, ): RouteResult | undefined { const linkLocalError = validateLinkLocalGatewayBaseUrl(baseUrl, env); if (linkLocalError !== undefined) return linkLocalError; try { parseGatewayConfig( - buildRawConfig(baseUrl, apiKey, ["setup-validation"], { apiKeyHeaderName }), + buildRawConfig(baseUrl, apiKey, ["setup-validation"], { + apiKeyHeaderName, + ...(protocol.endpointStyle === undefined ? {} : { endpointStyle: protocol.endpointStyle }), + ...(protocol.apiVersion === undefined ? {} : { apiVersion: protocol.apiVersion }), + }), env, linkLocalGatewayOverrideOptions(env), ); @@ -1613,6 +1713,9 @@ interface SetupRequest { readonly baseUrl: string; readonly apiKey: string; readonly apiKeyHeaderName: string; + /** Generic endpoint protocol — see {@link SetupGatewayCredentials} (#3042). */ + readonly endpointStyle: string | undefined; + readonly apiVersion: string | undefined; readonly timeoutMs: number | undefined; readonly deploymentNames: readonly string[]; readonly imageInputModelIds: readonly string[]; @@ -1669,6 +1772,17 @@ interface SetupGatewayCredentials { readonly baseUrl: string; readonly apiKey: string; readonly apiKeyHeaderName: string; + /** + * The generic endpoint PROTOCOL of the setup-wide connection (#3042): submitted values win + * (an uploaded LiteLLM config declares openai-compatible explicitly and a server-side + * KEIKO_DEFAULT_ENDPOINT_STYLE must not override the file's statement after save); absent + * values inherit from the stored primary only while the connection stays on the same + * endpoint, so a persisted style survives a credential rotation but never travels to a moved + * endpoint it was not declared for. The style/apiVersion pairing is enforced by the canonical + * parser on the validation and candidate configs downstream. + */ + readonly endpointStyle: string | undefined; + readonly apiVersion: string | undefined; } interface SetupVoiceProvider { @@ -1833,11 +1947,14 @@ const VOICE_CONNECTION_MUTATION_FIELDS = [ ] as const; // The endpoint-protocol wire values come from the contract seam — one compiler-checked source -// shared with the model gateway's parser and the UI upload parser (#3037 follow-up). -const VOICE_ENDPOINT_STYLES = PROVIDER_ENDPOINT_STYLES; +// shared with the model gateway's parser and the UI upload parser (#3037 follow-up). One list +// for BOTH sections on purpose: an endpoint protocol is a property of the connection, not of +// voice. The former voice-prefixed name made a reviewer read the generic check added in #3046 +// as a voice-only whitelist, so the shared names carry no section here. +const ENDPOINT_STYLE_VALUES = PROVIDER_ENDPOINT_STYLES; const VOICE_REALTIME_AUTH_MODES = REALTIME_AUTH_MODES; -function parseVoiceEndpointEnum( +function parseEndpointEnum( value: unknown, field: string, allowed: readonly T[], @@ -1866,13 +1983,13 @@ function parseVoiceEndpointEnum( function submittedVoiceEndpointOptions( raw: Record, ): SetupParseResult { - const endpointStyle = parseVoiceEndpointEnum( + const endpointStyle = parseEndpointEnum( raw.voiceEndpointStyle, "voiceEndpointStyle", - VOICE_ENDPOINT_STYLES, + ENDPOINT_STYLE_VALUES, ); if (!endpointStyle.ok) return endpointStyle; - const realtimeAuthMode = parseVoiceEndpointEnum( + const realtimeAuthMode = parseEndpointEnum( raw.voiceRealtimeAuthMode, "voiceRealtimeAuthMode", VOICE_REALTIME_AUTH_MODES, @@ -1956,10 +2073,6 @@ function shouldPreserveExisting( return raw.preserveExisting === true && current !== undefined; } -function firstProvider(current: GatewayConfig | undefined): ModelProviderConfig | undefined { - return current?.providers[0]; -} - function currentSpeechInputProvider( current: GatewayConfig | undefined, ): ModelProviderConfig | undefined { @@ -2069,9 +2182,16 @@ function readSetupGatewayCredentials( raw: Record, env: EnvSource, current: GatewayConfig | undefined, + stored: GatewayConfig | undefined, preserveExisting: boolean, ): SetupGatewayCredentials | RouteResult { - const provider = firstProvider(current); + // The MAIN gateway connection, not position zero: array order is not a contract, and a valid + // stored file may list a dedicated voice provider first. Reading the connection — URL, token, + // header and endpoint protocol — off that provider inherited the voice endpoint's values for + // the generic gateway; for the protocol it dropped a stored Azure declaration on an otherwise + // unchanged rotation (review finding on #3046). Same selection the sharing classification + // already uses (#3037). + const provider = storedPrimaryGatewayProvider(current); if ( inheritedTokenForChangedEndpoint( raw, @@ -2093,11 +2213,137 @@ function readSetupGatewayCredentials( return apiKeyHeaderResult.routeError; } const apiKeyHeaderName = apiKeyHeaderResult.value; - const invalidConnection = validateSetupConnection(baseUrl, apiKey, apiKeyHeaderName, env); + const protocol = durableSetupEndpointProtocol( + raw, + { stored, current }, + baseUrl, + preserveExisting, + ); + if ("status" in protocol) { + return protocol; + } + // The probe carries the protocol the setup will actually persist. Validating a protocol-free + // provider let the environment fill the gap: on a server that sets only + // KEIKO_DEFAULT_ENDPOINT_STYLE, every probe became an Azure provider with no api version and + // the canonical pairing rejected EVERY setup request, whatever the operator submitted + // (found while pinning the env-completed tuple, review findings on #3046). + const invalidConnection = validateSetupConnection( + baseUrl, + apiKey, + apiKeyHeaderName, + env, + protocol, + ); if (invalidConnection !== undefined) { return invalidConnection; } - return { baseUrl, apiKey, apiKeyHeaderName }; + return { baseUrl, apiKey, apiKeyHeaderName, ...protocol }; +} + +// Inheritance reads the DURABLE file, not the env-resolved view: a protocol that only exists +// because KEIKO_DEFAULT_* or a KEIKO_MODEL_* override is set was never declared in the file, and +// a rotation that inherited it would seal the transient value in — removing the override +// afterwards would no longer restore the file's own behavior (review finding on #3046, the same +// disk-vs-runtime rule the sharing classification draws). The connection fields stay on the +// runtime view: they are what the smoke test actually verifies. +function durableSetupEndpointProtocol( + raw: Record, + views: { + readonly stored: GatewayConfig | undefined; + readonly current: GatewayConfig | undefined; + }, + baseUrl: string, + preserveExisting: boolean, +): Pick | RouteResult { + const durable = views.stored ?? views.current; + return setupEndpointProtocol( + raw, + storedPrimaryGatewayProvider(durable), + baseUrl, + preserveExisting, + ); +} + +// See SetupGatewayCredentials: submitted protocol wins, absent inherits from the stored primary +// only on the SAME endpoint (a persisted style survives rotations, never travels to a moved +// endpoint), and everything else stays undefined so the runtime default layering is unchanged. +function setupEndpointProtocol( + raw: Record, + provider: ModelProviderConfig | undefined, + baseUrl: string, + preserveExisting: boolean, +): Pick | RouteResult { + const endpointStyle = parseEndpointEnum( + raw.endpointStyle, + "endpointStyle", + ENDPOINT_STYLE_VALUES, + ); + if (!endpointStyle.ok) return endpointStyle.routeError; + const apiVersion = optionalSetupSecret(raw.apiVersion, "apiVersion"); + if (!apiVersion.ok) return apiVersion.routeError; + // The submitted protocol is ATOMIC: stating a style replaces the whole protocol, so an + // inherited api version can never pair with it — switching an Azure provider to + // openai-compatible on the same URL would otherwise build a mixed protocol the canonical + // parser refuses, failing the save instead of performing it (review finding on #3046). + if (endpointStyle.value !== undefined) { + return pairedEndpointProtocol(endpointStyle.value, apiVersion.value); + } + const inheritable = + preserveExisting && provider !== undefined && sameBaseUrlIdentity(baseUrl, provider.baseUrl); + return pairedEndpointProtocol( + inheritable ? provider.endpointStyle : undefined, + apiVersion.value ?? (inheritable ? provider.apiVersion : undefined), + ); +} + +// The canonical pairing, checked on the EFFECTIVE protocol rather than on the submitted fields: +// an api version belongs to the Azure deployment path alone. Without this the config parser threw +// during verification and the operator got an opaque 502 "credentials could not be verified" for +// what is a request problem — a submitted version with no style at all, or a submitted version +// over an inherited openai-compatible style (review finding on #3046). Bumping the version of an +// endpoint whose stored style IS the deployment path stays legal: that is the same pair. +// The canonical api-version shape, mirroring the model gateway's own parser (ADR-0019 keeps the +// two packages apart, so the rule is mirrored and pinned on both sides rather than imported). +const SETUP_API_VERSION_RE = /^\d{4}-\d{2}-\d{2}(?:-preview)?$/u; + +function pairedEndpointProtocol( + endpointStyle: string | undefined, + apiVersion: string | undefined, +): Pick | RouteResult { + if (apiVersion !== undefined && endpointStyle !== "azure-openai-deployment") { + return { + status: 400, + body: errorBody( + "GATEWAY_API_VERSION_REQUIRES_AZURE_ENDPOINT", + 'apiVersion requires endpointStyle to be "azure-openai-deployment".', + ), + }; + } + // The canonical SHAPE, checked here for the same reason as the pairing: a malformed version + // threw inside the candidate loop and surfaced as an opaque 502 for what is a malformed + // request (review finding on #3046). + if (apiVersion !== undefined && !SETUP_API_VERSION_RE.test(apiVersion)) { + return { + status: 400, + body: errorBody( + "GATEWAY_API_VERSION_INVALID", + "apiVersion must be YYYY-MM-DD or YYYY-MM-DD-preview.", + ), + }; + } + // The other direction of the same canonical rule: the deployment path cannot be requested + // without the version that builds its URL. Left unnamed it threw inside the candidate loop and + // surfaced as the same misleading 502 (review finding on #3046). + if (endpointStyle === "azure-openai-deployment" && apiVersion === undefined) { + return { + status: 400, + body: errorBody( + "GATEWAY_AZURE_ENDPOINT_REQUIRES_API_VERSION", + 'endpointStyle "azure-openai-deployment" requires an apiVersion.', + ), + }; + } + return { endpointStyle, apiVersion }; } function validateVoiceProviderConnection( @@ -3351,6 +3597,10 @@ function setupRequiresGatewayVerification( hasNonBlankStringField(raw, "baseUrl") || hasNonBlankStringField(raw, "apiKey") || hasNonBlankStringField(raw, "apiKeyHeaderName") || + // The endpoint PROTOCOL only reaches the providers through the rebuild; without this the + // settings-only path accepted a protocol change and dropped it (review finding on #3046). + hasNonBlankStringField(raw, "endpointStyle") || + hasNonBlankStringField(raw, "apiVersion") || hasNonEmptyListField(raw, "deploymentNames") || imageListRequiresVerification(raw, modelLists, current) ); @@ -3454,7 +3704,7 @@ function readSetupRequest( return setupObjectBodyRequiredResult(correlationId); } const preserveExisting = shouldPreserveExisting(raw, current); - const credentials = readSetupGatewayCredentials(raw, env, current, preserveExisting); + const credentials = readSetupGatewayCredentials(raw, env, current, stored, preserveExisting); if (isRouteResult(credentials)) { return credentials; } @@ -3531,6 +3781,9 @@ interface SetupVerificationInput { readonly baseUrl: string; readonly apiKey: string; readonly apiKeyHeaderName: string; + /** Generic endpoint protocol — see {@link SetupGatewayCredentials} (#3042). */ + readonly endpointStyle: string | undefined; + readonly apiVersion: string | undefined; readonly timeoutMs: number | undefined; readonly deploymentNames: readonly string[]; readonly imageInputModelIds: readonly string[]; @@ -3612,6 +3865,8 @@ function testedImageInputModelIds( function validationConfigForSetup(input: SetupVerificationInput): GatewayConfig { const validationRawConfig = buildRawConfig(input.baseUrl, input.apiKey, ["setup-validation"], { apiKeyHeaderName: input.apiKeyHeaderName, + endpointStyle: input.endpointStyle, + apiVersion: input.apiVersion, imageInputModelIds: input.imageInputModelIds, timeoutMs: input.timeoutMs, }); @@ -3725,11 +3980,14 @@ function finalRawConfigForSetup( const rawConfig = buildRawConfig(input.baseUrl, input.apiKey, configuredModelIds, { preserveExisting: input.preserveExisting, apiKeyHeaderName: input.apiKeyHeaderName, + endpointStyle: input.endpointStyle, + apiVersion: input.apiVersion, imageInputModelIds, responseFormatModelIds, embeddingModelIds, modelMetadata, current: input.current, + stored: input.stored, workflowEligibleModelIds: input.workflowEligibleModelIds, timeoutMs: input.timeoutMs, }); @@ -3761,10 +4019,24 @@ function finalRawConfigForSetup( baseUrl: input.baseUrl, apiKey: input.apiKey, apiKeyHeaderName: input.apiKeyHeaderName, + endpointStyle: input.endpointStyle, + apiVersion: input.apiVersion, }, ); } +// The gateway connection a restored provider may follow: endpoint, credential, header AND the +// protocol spoken over it. +interface SetupGatewayConnection { + readonly baseUrl: string; + readonly apiKey: string; + readonly apiKeyHeaderName: string; + // The raw submitted value: this record is fed to the config parser, which is what validates + // the protocol — the same path genericEndpointProtocolRaw already takes. + readonly endpointStyle?: string | undefined; + readonly apiVersion?: string | undefined; +} + // A provider that SHARED the stored gateway connection (same endpoint AND same credential) // follows a credential rotation — the old token dies with the rotation, and the token must keep // travelling in the header the rebuilt gateway providers now use, or the restored provider would @@ -3785,10 +4057,43 @@ function sharesStoredGatewayConnection( ); } +// Only a provider that SPOKE the gateway's protocol follows it to a new one. One that +// deliberately used a different valid protocol over the same connection keeps its own: the new +// request shape was never verified for it (review finding on #3046). +function spokeStoredGatewayProtocol( + provider: ModelProviderConfig, + storedPrimary: ModelProviderConfig | undefined, +): boolean { + return ( + storedPrimary !== undefined && + provider.endpointStyle === storedPrimary.endpointStyle && + provider.apiVersion === storedPrimary.apiVersion + ); +} + +// The endpoint PROTOCOL is part of the connection, not a private property of the provider: a +// restored provider that follows the gateway's URL, token and header must speak the same way, or +// one shared connection ends up carrying two protocols and the restored provider keeps requesting +// the obsolete route (review finding on #3046). +function restoredProviderProtocolRaw( + provider: ModelProviderConfig, + gateway: SetupGatewayConnection, + storedPrimary: ModelProviderConfig | undefined, + sharedGatewayConnection: boolean, +): Record { + const follows = sharedGatewayConnection && spokeStoredGatewayProtocol(provider, storedPrimary); + const endpointStyle = follows ? gateway.endpointStyle : provider.endpointStyle; + const apiVersion = follows ? gateway.apiVersion : provider.apiVersion; + return { + ...(endpointStyle === undefined ? {} : { endpointStyle }), + ...(apiVersion === undefined ? {} : { apiVersion }), + }; +} + function storedDedicatedProviderRaw( provider: ModelProviderConfig, capability: ModelCapability, - gateway: { readonly baseUrl: string; readonly apiKey: string; readonly apiKeyHeaderName: string }, + gateway: SetupGatewayConnection, storedPrimary: ModelProviderConfig | undefined, ): Record { // Sharing is judged against the STORED primary connection — a provider that rode the old @@ -3805,8 +4110,7 @@ function storedDedicatedProviderRaw( baseUrl: sharedGatewayConnection ? gateway.baseUrl : provider.baseUrl, apiKey: sharedGatewayConnection ? gateway.apiKey : provider.apiKey, ...(apiKeyHeaderName === undefined ? {} : { apiKeyHeaderName }), - ...(provider.endpointStyle === undefined ? {} : { endpointStyle: provider.endpointStyle }), - ...(provider.apiVersion === undefined ? {} : { apiVersion: provider.apiVersion }), + ...restoredProviderProtocolRaw(provider, gateway, storedPrimary, sharedGatewayConnection), ...(provider.outputTokenParameter === undefined ? {} : { outputTokenParameter: provider.outputTokenParameter }), @@ -3827,7 +4131,7 @@ function applyStoredDedicatedProviders( rawConfig: Record, current: GatewayConfig | undefined, restoredModelIds: readonly string[], - gateway: { readonly baseUrl: string; readonly apiKey: string; readonly apiKeyHeaderName: string }, + gateway: SetupGatewayConnection, ): Record { if (restoredModelIds.length === 0 || current === undefined) return rawConfig; const providers: unknown[] = Array.isArray(rawConfig.providers) ? rawConfig.providers : []; @@ -3885,6 +4189,25 @@ function finalRawConfigForTestedSetup( ); } +// One retry (not zero) so a single transient blip — 429 rate-limit, brief timeout, momentary +// content-filter — does not permanently exclude an otherwise-working model from the setup and +// brand it to the user as incompatible. Still bounded so setup latency stays predictable. The +// probe carries the submitted endpoint protocol so an Azure deployment path (or an explicit +// openai-compatible declaration under an env default) is exercised exactly as it will persist. +function candidateProbeOptions( + input: SetupVerificationInput, + smokeTimeoutMs: number, +): ProviderRawOptions { + return { + apiKeyHeaderName: input.apiKeyHeaderName, + endpointStyle: input.endpointStyle, + apiVersion: input.apiVersion, + timeoutMs: smokeTimeoutMs, + maxRetries: 1, + imageInputModelIds: input.imageInputModelIds, + }; +} + async function verifySetupCandidate(input: SetupVerificationInput): Promise { // Defence-in-depth: never send the credential to a candidate URL that has not passed the same // scheme/credential/loopback validation as the originally submitted base URL. @@ -3899,15 +4222,7 @@ async function verifySetupCandidate(input: SetupVerificationInput): Promise_API_VERSION needs it to parse at all (#3040) — so the +// parsed protocol can be an env value the file never contained. Inheriting THAT on a rotation +// would seal a transient default into the sealed config, and removing the variable afterwards +// would no longer restore the file's own behavior (review finding on #3046). Correcting after +// the parse keeps the #3040 fix intact: nothing is masked, only the values the file does not +// declare are dropped from the durable view. +function withFileDeclaredProtocol( + config: GatewayConfig | undefined, + storagePath: string, +): GatewayConfig | undefined { + if (config === undefined || !existsSync(storagePath)) return config; + const declared = fileDeclaredProviderRecords(storagePath); + if (declared === undefined) return config; + return { + ...config, + providers: config.providers.map((provider) => { + const raw = declared.get(provider.modelId); + if (raw === undefined) return provider; + // The FILE's own value wins over the resolved one: a KEIKO_MODEL__API_VERSION that + // overrides a DECLARED version would otherwise be sealed in by a rotation just as an + // undeclared one would (review finding on #3046). An unrecognised declared style is left + // as parsed — the parser is the authority on what a style may be. + return { ...provider, ...fileDeclaredProtocol(raw, provider) }; + }), + }; +} + +// The protocol the FILE declares, kept COHERENT. Each half falls back to the resolved value when +// the other half is declared and the canonical pairing needs it: a file that declares the Azure +// deployment path and takes its required version from KEIKO_MODEL__API_VERSION is only valid +// with that version, and a file that declares the version while the style arrives through +// KEIKO_DEFAULT_ENDPOINT_STYLE is only valid with that style. Dropping the env half of either +// pair left the durable view incoherent, and inheritance then rejected a routine credential +// rotation with 400 (review findings on #3046 — the same coherence argument that stopped #3040 +// from masking that namespace at parse time). +function fileDeclaredProtocol( + raw: Record, + provider: ModelProviderConfig, +): Pick { + const style = declaredEndpointStyle(raw.endpointStyle, provider.endpointStyle); + const version = typeof raw.apiVersion === "string" ? raw.apiVersion : undefined; + if (style === "azure-openai-deployment" && version === undefined) { + return { endpointStyle: style, apiVersion: provider.apiVersion }; + } + if (version !== undefined && style === undefined) { + return { endpointStyle: provider.endpointStyle, apiVersion: version }; + } + return { endpointStyle: style, apiVersion: version }; +} + +// An unrecognised declared style is left as parsed — the parser is the authority on what a style +// may be. +function declaredEndpointStyle( + raw: unknown, + resolved: ModelProviderConfig["endpointStyle"], +): ModelProviderConfig["endpointStyle"] { + if (typeof raw !== "string") return undefined; + const declared = PROVIDER_ENDPOINT_STYLES.find((style) => style === raw); + return declared ?? resolved; +} + +function fileDeclaredProviderRecords( + storagePath: string, +): ReadonlyMap> | undefined { + try { + const parsed: unknown = JSON.parse(readFileSync(storagePath, "utf8")); + if (!isRecord(parsed) || !Array.isArray(parsed.providers)) return undefined; + return new Map( + parsed.providers.flatMap((entry) => + isRecord(entry) && typeof entry.modelId === "string" + ? ([[entry.modelId, entry]] as const) + : [], + ), + ); + } catch { + // An unreadable or malformed file leaves the durable view exactly as parsed — the same + // fall-back durableStoredGatewayConfig makes for that state. + return undefined; + } +} + function validateWorkflowEligibleModelIds( request: SetupRequest, config: GatewayConfig, @@ -4395,11 +4799,17 @@ async function verifyAndSaveExistingConfigUpdate( function shouldRequireDeploymentNames( request: SetupRequest, baseUrlCandidates: readonly string[], + env: EnvSource, ): boolean { - return ( - request.deploymentNames.length === 0 && - baseUrlCandidates.some((baseUrl) => isAzureFoundryBaseUrl(baseUrl)) - ); + if (request.deploymentNames.length !== 0) return false; + // The deployment path IS the requirement: a classic Azure OpenAI host on that path cannot be + // discovered through generic /models, so without this it failed at discovery instead of naming + // the missing deployments (review finding on #3046). The EFFECTIVE style decides — a request + // that omits the field still lands on the deployment path when KEIKO_DEFAULT_ENDPOINT_STYLE + // says so, and discovery would fail exactly the same way. + const effectiveStyle = request.endpointStyle ?? env.KEIKO_DEFAULT_ENDPOINT_STYLE; + if (effectiveStyle === "azure-openai-deployment") return true; + return baseUrlCandidates.some((baseUrl) => isAzureFoundryBaseUrl(baseUrl)); } async function verifyAndSaveGatewaySetup( @@ -4415,7 +4825,7 @@ async function verifyAndSaveGatewaySetup( return figmaFailure; } const baseUrlCandidates = candidateBaseUrls(request.baseUrl); - if (shouldRequireDeploymentNames(request, baseUrlCandidates)) { + if (shouldRequireDeploymentNames(request, baseUrlCandidates, deps.env)) { return deploymentNamesRequiredResult(); } const errors: string[] = []; diff --git a/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsx b/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsx index 5bbfc72d0e..f35673a90d 100644 --- a/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsx +++ b/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsx @@ -2102,6 +2102,365 @@ describe("GatewaySetupDialog", () => { expect(screen.getByText(/realtime voice model uses its own endpoint/i)).toBeInTheDocument(); }); + it("submits the imported generic endpoint protocol while the form points at the uploaded endpoint", async () => { + // #3042 follow-up: an explicit endpoint style in the file must ride the submit, or a + // server-side KEIKO_DEFAULT_ENDPOINT_STYLE resolves over the file's statement after save. + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File( + [ + JSON.stringify({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ], + }), + ], + "keiko.config.json", + { type: "application/json" }, + ), + ); + await waitFor(() => + expect(screen.getByLabelText(/base url/i)).toHaveValue("https://resource.example.com"), + ); + await userEvent.type(screen.getByLabelText(/api token/i), "example-token"); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + expect(vi.mocked(setupGateway).mock.calls[0]?.[0]).toMatchObject({ + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }); + }); + it("omits the imported generic protocol when the gateway URL is left empty", async () => { + // The other branch of the URL binding (review finding on #3046): a preserve-mode submit + // that states no gateway URL inherits the stored connection, so the uploaded protocol has + // no endpoint to apply to and must not ride along. + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File( + [ + JSON.stringify({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ], + }), + ], + "keiko.config.json", + { type: "application/json" }, + ), + ); + await waitFor(() => + expect(screen.getByLabelText(/base url/i)).toHaveValue("https://resource.example.com"), + ); + await userEvent.clear(screen.getByLabelText(/base url/i)); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + const payload = vi.mocked(setupGateway).mock.calls[0]?.[0] ?? {}; + expect(payload).not.toHaveProperty("endpointStyle"); + expect(payload).not.toHaveProperty("apiVersion"); + }); + + it("keeps a declared generic protocol when the uploaded file states no gateway URL", async () => { + // Review finding on #3046: a file may declare the protocol and leave the URL to the operator + // — a shape this parser accepts. Clearing the protocol because the URL is absent discarded + // an explicit declaration, and the save then probed and persisted the OpenAI-compatible + // shape (or whatever KEIKO_DEFAULT_ENDPOINT_STYLE says) against an Azure endpoint. + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File( + [ + JSON.stringify({ + providers: [ + { + modelId: "azure-chat", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ], + }), + ], + "keiko.config.json", + { type: "application/json" }, + ), + ); + await waitFor(() => + expect(screen.getByLabelText(/deployment names/i)).toHaveValue("azure-chat"), + ); + + await userEvent.type(screen.getByLabelText(/base url/i), "https://resource.example.com"); + await userEvent.type(screen.getByLabelText(/api token/i), "example-token"); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + expect(vi.mocked(setupGateway).mock.calls[0]?.[0]).toMatchObject({ + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }); + }); + + it("keeps the imported protocol when the URL is edited to the server-equivalent form", async () => { + // Review finding on #3046: the server's canonical identity also drops a terminal + // /chat/completions, so trimming an imported URL down to the endpoint the server would + // derive is not an endpoint change — but the narrower client copy read it as one and + // dropped the protocol. + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File( + [ + JSON.stringify({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://gateway.example.com/v1/chat/completions", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ], + }), + ], + "keiko.config.json", + { type: "application/json" }, + ), + ); + await waitFor(() => + expect(screen.getByLabelText(/base url/i)).toHaveValue( + "https://gateway.example.com/v1/chat/completions", + ), + ); + + await userEvent.clear(screen.getByLabelText(/base url/i)); + await userEvent.type(screen.getByLabelText(/base url/i), "https://gateway.example.com/v1"); + await userEvent.type(screen.getByLabelText(/api token/i), "example-token"); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + expect(vi.mocked(setupGateway).mock.calls[0]?.[0]).toMatchObject({ + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }); + }); + + it("drops a previous file's generic protocol when the next upload declares none", async () => { + // Review finding on #3046: applyUploadedGenericEndpoint returned early for a file that + // states no gateway URL, leaving the PREVIOUS file's protocol and its URL binding in state. + // The visible URL field still held the old URL, so the binding matched and the stale + // protocol rode the submit of a file that never mentioned one. A file with no URL states + // no protocol either. + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + const upload = async (providers: readonly Record[]): Promise => { + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File([JSON.stringify({ providers })], "keiko.config.json", { + type: "application/json", + }), + ); + }; + + await upload([ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ]); + await waitFor(() => + expect(screen.getByLabelText(/base url/i)).toHaveValue("https://resource.example.com"), + ); + + // The corrected file lists the deployment but states no connection at all — the URL is + // supplied by the operator, and with it the protocol that belongs to it. + await upload([{ modelId: "plain-chat", capability: { id: "plain-chat", kind: "chat" } }]); + await waitFor(() => + expect(screen.getByLabelText(/deployment names/i)).toHaveValue("plain-chat"), + ); + expect(screen.getByLabelText(/base url/i)).toHaveValue("https://resource.example.com"); + + await userEvent.type(screen.getByLabelText(/api token/i), "example-token"); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + const payload = vi.mocked(setupGateway).mock.calls[0]?.[0] ?? {}; + expect(payload).not.toHaveProperty("endpointStyle"); + expect(payload).not.toHaveProperty("apiVersion"); + }); + + it("keeps the imported generic protocol across a semantics-preserving URL edit", async () => { + // Review finding on #3046: the binding compared raw strings, so adding or removing a + // trailing slash — the same endpoint by the gateway's own sameBaseUrlIdentity rule — threw + // the uploaded protocol away and produced a save without it. + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File( + [ + JSON.stringify({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com/openai", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ], + }), + ], + "keiko.config.json", + { type: "application/json" }, + ), + ); + await waitFor(() => + expect(screen.getByLabelText(/base url/i)).toHaveValue("https://resource.example.com/openai"), + ); + await userEvent.type(screen.getByLabelText(/base url/i), "/"); + await userEvent.type(screen.getByLabelText(/api token/i), "example-token"); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + expect(vi.mocked(setupGateway).mock.calls[0]?.[0]).toMatchObject({ + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }); + }); + + it("drops the imported generic protocol when the user retypes the gateway URL", async () => { + vi.mocked(setupGateway).mockResolvedValueOnce({ + ok: true, + testedModelId: "azure-chat", + testedModelIds: ["azure-chat"], + providerCount: 1, + models: [], + config: { + providers: [], + circuitBreaker: { failureThreshold: 5, cooldownMs: 30_000, halfOpenProbes: 2 }, + }, + }); + render(); + + await userEvent.upload( + await screen.findByLabelText(/load keiko\.config\.json/i), + new File( + [ + JSON.stringify({ + providers: [ + { + modelId: "azure-chat", + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: { id: "azure-chat", kind: "chat" }, + }, + ], + }), + ], + "keiko.config.json", + { type: "application/json" }, + ), + ); + await waitFor(() => + expect(screen.getByLabelText(/base url/i)).toHaveValue("https://resource.example.com"), + ); + await userEvent.clear(screen.getByLabelText(/base url/i)); + await userEvent.type(screen.getByLabelText(/base url/i), "https://other.example.com/v1"); + await userEvent.type(screen.getByLabelText(/api token/i), "example-token"); + await userEvent.click(screen.getByRole("button", { name: /test & save/i })); + + await waitFor(() => expect(setupGateway).toHaveBeenCalledTimes(1)); + const payload = vi.mocked(setupGateway).mock.calls[0]?.[0] ?? {}; + expect(payload).not.toHaveProperty("endpointStyle"); + expect(payload).not.toHaveProperty("apiVersion"); + }); + it("rejects an unreadable configuration upload with a visible alert and no field changes", async () => { render(); diff --git a/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx b/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx index 7384b6a088..654e18ecfa 100644 --- a/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx +++ b/packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx @@ -593,6 +593,10 @@ interface GatewayFormFields { readonly importedVoiceRealtimeAuthMode: string; /** The uploaded endpoint URL the imported protocol is bound to. */ readonly importedVoiceEndpointBaseUrl: string; + /** Generic endpoint protocol imported from a config upload, bound to its gateway URL (#3042). */ + readonly importedEndpointStyle: string; + readonly importedApiVersion: string; + readonly importedEndpointBaseUrl: string; readonly figmaAccessToken: string; readonly preserveExisting: boolean; readonly hasStoredVoiceProvider: boolean; @@ -644,6 +648,66 @@ function importedVoiceEndpointPayload(fields: GatewayFormFields): Partial 0 && value[end - 1] === "/") { + end -= 1; + } + return value.slice(0, end); +} + +const CHAT_COMPLETIONS_SUFFIX = "/chat/completions"; + +// The server's normalizeBaseUrl also drops a terminal /chat/completions before comparing, so +// trimming an imported URL down to the endpoint the server would derive is NOT an endpoint +// change. A narrower client copy read it as one and dropped the imported protocol (review +// finding on #3046). The rule is mirrored rather than imported: keiko-ui may not depend on +// keiko-model-gateway (ADR-0019), and the shape is pinned on both sides. +function normalizedEndpoint(raw: string): string { + const trimmed = stripTrailingSlashes(raw.trim()); + return trimmed.endsWith(CHAT_COMPLETIONS_SUFFIX) + ? stripTrailingSlashes(trimmed.slice(0, -CHAT_COMPLETIONS_SUFFIX.length)) + : trimmed; +} + +function canonicalEndpointIdentity(raw: string): string { + const normalized = normalizedEndpoint(raw); + try { + return stripTrailingSlashes(new URL(normalized).href); + } catch { + return normalized; + } +} + +// The imported GENERIC protocol rides only on a submit that still points at the uploaded gateway +// endpoint — a manually retyped URL must not inherit the file's protocol (#3042), while an edit +// that does not change the endpoint's identity keeps it (#3046). +function importedEndpointPayload(fields: GatewayFormFields): Partial { + const submittedBaseUrl = fields.baseUrl.trim(); + if (submittedBaseUrl === "") return {}; + // An empty binding means the file declared a protocol but left the URL to the operator, so it + // applies to whatever endpoint is entered — the same "unbound" rule the voice twin uses. The + // imported values always come from the CURRENT file, so this can never resurrect an earlier + // upload's protocol (review findings on #3046). + if ( + fields.importedEndpointBaseUrl !== "" && + canonicalEndpointIdentity(submittedBaseUrl) !== + canonicalEndpointIdentity(fields.importedEndpointBaseUrl) + ) { + return {}; + } + return { + ...(fields.importedEndpointStyle === "" ? {} : { endpointStyle: fields.importedEndpointStyle }), + ...(fields.importedApiVersion === "" ? {} : { apiVersion: fields.importedApiVersion }), + }; +} + function buildSetupGatewayPayload( fields: GatewayFormFields, derived: DerivedSubmitFields, @@ -663,6 +727,7 @@ function buildSetupGatewayPayload( ? {} : { embeddingModelIds: fields.importedEmbeddingModelIds }), ...importedVoiceEndpointPayload(fields), + ...importedEndpointPayload(fields), ...(!fields.workflowEligibleModelIdsConfigured ? {} : { workflowEligibleModelIds: derived.parsedWorkflowEligibleModelIds }), @@ -2158,6 +2223,9 @@ export function GatewaySetupDialog({ const [importedVoiceApiVersion, setImportedVoiceApiVersion] = useState(""); const [importedVoiceRealtimeAuthMode, setImportedVoiceRealtimeAuthMode] = useState(""); const [importedVoiceEndpointBaseUrl, setImportedVoiceEndpointBaseUrl] = useState(""); + const [importedEndpointStyle, setImportedEndpointStyle] = useState(""); + const [importedApiVersion, setImportedApiVersion] = useState(""); + const [importedEndpointBaseUrl, setImportedEndpointBaseUrl] = useState(""); const [uploadReadPending, setUploadReadPending] = useState(false); const [workflowEligibleModelIdsConfigured, setWorkflowEligibleModelIdsConfigured] = useState(false); @@ -2448,6 +2516,11 @@ export function GatewaySetupDialog({ setImportedEmbeddingModelIds(fields.embeddingModelIds ?? []); if (fields.figmaAccessToken !== undefined) setFigmaAccessToken(fields.figmaAccessToken); applyUploadedVoiceConfig(fields); + // GENERIC state, so it is applied here rather than inside the voice helper: every sibling in + // there returns early when the file carries no voice section, and one added to the voice + // entry point would silently reintroduce the stale-binding defect this function exists to + // fix (review finding on #3046). + applyUploadedGenericEndpoint(fields); } /** @@ -2534,6 +2607,17 @@ export function GatewaySetupDialog({ // all, its protocol declaration REPLACES the previous upload's (a corrected re-upload without // a style must clear the stale one); a file with no voice section leaves the visible voice // fields — and therefore the protocol that belongs to them — untouched. + // Twin of the voice binding below: the generic protocol is BOUND to the uploaded gateway + // URL and cleared when the file carries none, so a stale binding cannot outlive a corrected + // re-upload (#3042). The generic binding is file-scoped without exception — a file that + // states no gateway URL states no protocol either, and returning early here would leave the + // PREVIOUS file's style bound to a URL the form still holds, riding the next submit unseen. + function applyUploadedGenericEndpoint(fields: GatewayConfigUploadFields): void { + setImportedEndpointStyle(fields.endpointStyle ?? ""); + setImportedApiVersion(fields.apiVersion ?? ""); + setImportedEndpointBaseUrl(fields.baseUrl?.trim() ?? ""); + } + function applyUploadedVoiceEndpoint(fields: GatewayConfigUploadFields): void { if (fields.voiceBaseUrl === undefined) return; setImportedVoiceEndpointStyle(fields.voiceEndpointStyle ?? ""); @@ -2584,6 +2668,9 @@ export function GatewaySetupDialog({ importedVoiceApiVersion, importedVoiceRealtimeAuthMode, importedVoiceEndpointBaseUrl, + importedEndpointStyle, + importedApiVersion, + importedEndpointBaseUrl, voiceSpeechOutputModelId, voiceOutputVoiceId, voiceOutputVoiceIdConfigured, diff --git a/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.test.ts b/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.test.ts index fde8c81a3f..50521a7b96 100644 --- a/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.test.ts +++ b/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.test.ts @@ -309,10 +309,9 @@ describe("parseGatewayConfigUpload", () => { }); it("accepts an explicit openai-compatible endpoint style on a generic provider", () => { - // LiteLLM production audit: LiteLLM-shaped files state the default explicitly — an explicit - // endpointStyle "openai-compatible" on a chat/embedding provider is behaviorally identical - // to the absent default and must not refuse the file. The Azure deployment style, unknown - // values, and a chat/embedding apiVersion keep refusing. + // LiteLLM production audit + #3042 follow-up: KNOWN styles travel through the form now. + // Unknown values keep refusing, and an UNPAIRABLE protocol (an api version without the + // Azure style) is canonical-invalid — the save would refuse it after a reported success. const explicitDefault = JSON.stringify({ providers: [providerFixture({ endpointStyle: "openai-compatible" })], }); @@ -328,7 +327,7 @@ describe("parseGatewayConfigUpload", () => { providerFixture({ endpointStyle: "openai-compatible", apiVersion: "2025-03-01-preview" }), ], }); - expect(parseGatewayConfigUpload(orphanVersion)).toEqual({ outcome: "unsupportedSetting" }); + expect(parseGatewayConfigUpload(orphanVersion)).toEqual({ outcome: "invalid" }); }); it("unifies header names that differ only in case across providers", () => { @@ -1066,15 +1065,15 @@ describe("parseGatewayConfigUpload", () => { }); it("refuses generic provider settings the form cannot represent with an honest outcome", () => { - // Review finding on #3031: endpoint style / API version / output token parameter on a chat - // or embedding provider would be silently rebuilt with defaults — a changed runtime - // configuration behind a success message. Voice providers are exempt: the voice setup route - // regenerates their endpoint details itself. + // Review finding on #3031, narrowed by the #3042 follow-up: the endpoint protocol travels + // now, so only the settings the form still has no field for refuse — and an Azure style + // WITHOUT its api version is canonical-invalid rather than unsupported. Voice providers + // stay exempt: the voice setup route regenerates their endpoint details itself. const azure = JSON.stringify({ providers: [providerFixture({ endpointStyle: "azure-openai-deployment" })], }); - expect(parseGatewayConfigUpload(azure)).toEqual({ outcome: "unsupportedSetting" }); + expect(parseGatewayConfigUpload(azure)).toEqual({ outcome: "invalid" }); // Retry tuning matching what the rebuild writes stays representable; a DIFFERENT value // would be silently rewritten on save and refuses instead (review finding on #3031). const matching = JSON.stringify({ @@ -1299,6 +1298,101 @@ describe("parseGatewayConfigUpload", () => { }); describe("appliedGatewayConfigFieldCount", () => { + it("carries an explicit generic endpoint style into the form fields", () => { + // Codex finding on #3042: the tolerated explicit openai-compatible was dropped, so a server + // running KEIKO_DEFAULT_ENDPOINT_STYLE=azure-openai-deployment resolved the env default + // over the file's statement after a reported upload success. + const fields = fieldsOf( + JSON.stringify({ + providers: [providerFixture({ endpointStyle: "openai-compatible" })], + }), + ); + + expect(fields.endpointStyle).toBe("openai-compatible"); + expect(fields.apiVersion).toBeUndefined(); + }); + it("carries a generic Azure endpoint style with its api version", () => { + const fields = fieldsOf( + JSON.stringify({ + providers: [ + providerFixture({ + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }), + ], + }), + ); + + expect(fields.endpointStyle).toBe("azure-openai-deployment"); + expect(fields.apiVersion).toBe("2025-04-01-preview"); + }); + it("refuses generic providers that disagree on the endpoint protocol", () => { + // One form, one connection, one protocol — a split declaration cannot be represented. + const split = JSON.stringify({ + providers: [ + providerFixture({ modelId: "gpt-a", endpointStyle: "openai-compatible" }), + providerFixture({ + modelId: "gpt-b", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + }), + ], + }); + + expect(parseGatewayConfigUpload(split)).toEqual({ outcome: "invalid" }); + }); + it("refuses generic providers that disagree on the api version alone", () => { + // The uniformity check has two axes; the split-style fixture above never reaches the second + // one. Two Azure providers with DIFFERENT valid versions must refuse too (review finding on + // #3046) — one form holds one connection, and silently picking a version would send half + // the deployments to an API contract they were not configured for. + const splitVersion = JSON.stringify({ + providers: [ + providerFixture({ + modelId: "azure-a", + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-03-01-preview", + }), + providerFixture({ + modelId: "azure-b", + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "2025-04-01-preview", + capability: undefined, + }), + ], + }); + + expect(parseGatewayConfigUpload(splitVersion)).toEqual({ outcome: "invalid" }); + }); + + it("refuses a malformed Azure api version on a generic provider", () => { + // The canonical parser requires YYYY-MM-DD or YYYY-MM-DD-preview; accepting "not-a-date" + // here reports success for a file Test & Save then refuses (review finding on #3046). The + // voice endpoint already mirrors this rule — the generic one must too. + const malformed = JSON.stringify({ + providers: [ + providerFixture({ + baseUrl: "https://resource.example.com", + endpointStyle: "azure-openai-deployment", + apiVersion: "not-a-date", + }), + ], + }); + + expect(parseGatewayConfigUpload(malformed)).toEqual({ outcome: "invalid" }); + }); + + it("still refuses an unknown generic endpoint style", () => { + const unknown = JSON.stringify({ + providers: [providerFixture({ endpointStyle: "bogus-style" })], + }); + + expect(parseGatewayConfigUpload(unknown)).toEqual({ outcome: "unsupportedSetting" }); + }); + it("counts filled scalars, deployments, and every defined flag list — empty included", () => { const fields = fieldsOf(JSON.stringify({ providers: [providerFixture()] })); diff --git a/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.ts b/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.ts index a5ebb08d29..619a05ada0 100644 --- a/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.ts +++ b/packages/keiko-ui/src/app/components/desktop/modals/gatewayConfigParsing.ts @@ -40,7 +40,8 @@ const SUPPORTED_API_KEY_HEADER_NAMES = new Set([ * verbatim through the voice setup fields (#3037), and profiles map to the output voice. * Retry tuning (maxRetries, retryBaseDelayMs) stays tolerated everywhere: the form has no field * for it either, but it does not change which endpoint or protocol the connection speaks. - * `endpointStyle` is checked value-aware instead (see unsupportedGenericEndpointStyle). + * KNOWN `endpointStyle`/`apiVersion` values travel through the form (#3042); unknown styles + * refuse (see unknownGenericEndpointStyle). */ /** * A secret REFERENCE is a credential source the form cannot carry (the token field takes a @@ -52,22 +53,21 @@ const SUPPORTED_API_KEY_HEADER_NAMES = new Set([ const UNSUPPORTED_CREDENTIAL_REFERENCE = "apiKeySecretRef"; const UNSUPPORTED_GENERIC_PROVIDER_SETTINGS = [ - "apiVersion", "outputTokenParameter", "realtimeAuthMode", "voiceProfiles", ] as const; /** - * An explicit `endpointStyle: "openai-compatible"` on a chat/embedding provider is behaviorally - * identical to the absent default — LiteLLM-shaped files state the default explicitly, and - * refusing it would reject exactly the files this feature exists for (LiteLLM production - * audit). The Azure deployment style and unknown values keep refusing: the setup form cannot - * represent a generic provider that speaks a different endpoint shape. + * KNOWN endpoint styles travel through the form into the setup request (#3042 follow-up: the + * explicit openai-compatible of a LiteLLM file must survive a server-side + * KEIKO_DEFAULT_ENDPOINT_STYLE, and Azure files carry their deployment style with the api + * version). Only an UNKNOWN style keeps refusing — the form cannot represent an endpoint shape + * the product does not speak. */ -function unsupportedGenericEndpointStyle(value: unknown): boolean { +function unknownGenericEndpointStyle(value: unknown): boolean { if (value === undefined) return false; - return !(typeof value === "string" && value.trim() === "openai-compatible"); + return !(typeof value === "string" && ENDPOINT_STYLE_VALUES.has(value.trim())); } /** * Top-level blocks the setup form can represent. A file carrying `grounding`, `reranker`, @@ -111,6 +111,12 @@ export interface GatewayConfigUploadFields { readonly baseUrl: string | undefined; readonly apiKey: string | undefined; readonly apiKeyHeaderName: string | undefined; + /** + * The generic connection's endpoint protocol, submitted verbatim so an explicit declaration + * survives a server-side default (#3042). `undefined` when the file does not state it. + */ + readonly endpointStyle: string | undefined; + readonly apiVersion: string | undefined; readonly timeoutMs: string | undefined; readonly deploymentNames: readonly string[]; /** @@ -611,7 +617,7 @@ function carriesUnsupportedGenericSetting( typeof entry.modelId === "string" && ((genericIds.has(entry.modelId.trim()) && (UNSUPPORTED_GENERIC_PROVIDER_SETTINGS.some((setting) => entry[setting] !== undefined) || - unsupportedGenericEndpointStyle(entry.endpointStyle))) || + unknownGenericEndpointStyle(entry.endpointStyle))) || // A secret reference refuses on EVERY provider kind — no form field can carry it. entry[UNSUPPORTED_CREDENTIAL_REFERENCE] !== undefined), ); @@ -877,8 +883,10 @@ interface VoiceEndpointScalars { } // The endpoint-protocol wire values come from the contract seam — the same compiler-checked -// source the server setup route and the model gateway validate against (#3037 follow-up). -const VOICE_ENDPOINT_STYLES = new Set(PROVIDER_ENDPOINT_STYLES); +// source the server setup route and the model gateway validate against (#3037 follow-up). One +// set for BOTH sections on purpose: a provider's endpoint protocol is not voice-specific, and a +// voice-sounding name here made a reviewer read the generic check as a voice whitelist (#3046). +const ENDPOINT_STYLE_VALUES = new Set(PROVIDER_ENDPOINT_STYLES); const VOICE_REALTIME_AUTH_MODES = new Set(REALTIME_AUTH_MODES); /** @@ -893,7 +901,7 @@ const VOICE_REALTIME_AUTH_MODES = new Set(REALTIME_AUTH_MODES); const AZURE_API_VERSION_RE = /^\d{4}-\d{2}-\d{2}(?:-preview)?$/u; function representableVoiceEndpoint(endpoint: VoiceEndpointScalars): boolean { - if (endpoint.endpointStyle !== undefined && !VOICE_ENDPOINT_STYLES.has(endpoint.endpointStyle)) { + if (endpoint.endpointStyle !== undefined && !ENDPOINT_STYLE_VALUES.has(endpoint.endpointStyle)) { return false; } if ( @@ -948,7 +956,10 @@ function timeoutField(timeoutMs: number | undefined): string | undefined { function fieldsFrom( partition: PartitionedProviders, - generic: ConnectionScalars, + generic: ConnectionScalars & { + readonly endpointStyle: string | undefined; + readonly apiVersion: string | undefined; + }, voice: VoiceFields, figmaAccessToken: string | undefined, voiceRetryTuningReset: boolean, @@ -965,6 +976,8 @@ function fieldsFrom( baseUrl: generic.baseUrl, apiKey: generic.apiKey, apiKeyHeaderName: generic.apiKeyHeaderName, + endpointStyle: generic.endpointStyle, + apiVersion: generic.apiVersion, timeoutMs: timeoutField(generic.timeoutMs), deploymentNames: unique(partition.generic.map((provider) => provider.modelId)), imageInputModelIds: speaksAboutFlags @@ -1048,13 +1061,40 @@ function uploadRoot(serialized: string): Record | undefined { return objectRecord(root) ? root : undefined; } +// One form, one connection, one protocol: the generic providers must agree on the endpoint +// style and api version, exactly like the voice section's uniform-endpoint rule (#3042). +function uniformGenericEndpointProtocol( + providers: readonly ParsedProvider[], +): + | { readonly endpointStyle: string | undefined; readonly apiVersion: string | undefined } + | undefined { + const endpointStyle = uniformScalar(providers.map((provider) => provider.endpointStyle)); + const apiVersion = uniformScalar(providers.map((provider) => provider.apiVersion)); + if (!endpointStyle.ok || !apiVersion.ok) return undefined; + // The canonical pairing rule, mirrored: the Azure deployment style REQUIRES an api version, + // and an api version requires the Azure style — carrying an unpairable protocol would report + // upload success for a file the save then refuses (#3042). The version SHAPE is mirrored too + // (API_VERSION_RE), like the voice endpoint already does (review finding on #3046). + if (apiVersion.value !== undefined && !AZURE_API_VERSION_RE.test(apiVersion.value)) { + return undefined; + } + if (endpointStyle.value === "azure-openai-deployment" && apiVersion.value === undefined) { + return undefined; + } + if (endpointStyle.value !== "azure-openai-deployment" && apiVersion.value !== undefined) { + return undefined; + } + return { endpointStyle: endpointStyle.value, apiVersion: apiVersion.value }; +} + function assembledFields( root: Record, partition: PartitionedProviders, ): GatewayConfigUploadResult { const generic = uniformConnectionScalars(partition.generic); + const genericProtocol = uniformGenericEndpointProtocol(partition.generic); const voice = voiceFields(partition); - if (generic === undefined || voice === undefined) { + if (generic === undefined || genericProtocol === undefined || voice === undefined) { return { outcome: "invalid" }; } // A PRESENT but non-object figma block is corrupted input, not an absent connector — the @@ -1066,7 +1106,7 @@ function assembledFields( outcome: "fields", fields: fieldsFrom( partition, - generic, + { ...generic, ...genericProtocol }, voice, figma.value, importedVoiceRetryTuningReset(root.providers, voice), diff --git a/packages/keiko-ui/src/lib/api.ts b/packages/keiko-ui/src/lib/api.ts index 7bef4e2d8b..ad0e537835 100644 --- a/packages/keiko-ui/src/lib/api.ts +++ b/packages/keiko-ui/src/lib/api.ts @@ -615,6 +615,9 @@ export interface GatewaySetupInput { readonly voiceBaseUrl?: string | undefined; readonly voiceApiKey?: string | undefined; readonly voiceApiKeyHeaderName?: string | undefined; + /** Generic endpoint protocol, persisted verbatim on rebuilt providers (#3042). */ + readonly endpointStyle?: string | undefined; + readonly apiVersion?: string | undefined; readonly voiceModelId?: string | undefined; readonly voiceSpeechToTextModelId?: string | undefined; readonly voiceRealtimeModelId?: string | undefined;