From 2a556b47c3ffc404e131689d0456217b035f73b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 5 Oct 2026 07:45:05 +0200 Subject: [PATCH] fix(coach): offer only providers that can answer and say when a switch does not take The provider-chain summary marked every entry available, so the Coach's provider select offered entries without a key or sign-in. Choosing one saved it to the front of the chain, the resolver skipped it, and the select jumped back without a word. Availability now comes from the presence probe, which names every chain type whose credential is present. After a switch the picker reads the chain back and says so when the server still answers with another provider. --- messages/de.json | 3 +- messages/en.json | 3 +- messages/es.json | 3 +- messages/fr.json | 3 +- messages/it.json | 3 +- messages/ko.json | 3 +- messages/pl.json | 3 +- .../provider-chain/__tests__/route.test.ts | 41 +++++++++++- src/app/api/insights/provider-chain/route.ts | 20 +++--- .../__tests__/coach-model-picker.test.tsx | 18 ++++++ .../coach-panel/coach-model-picker.tsx | 45 +++++++++++-- .../ai/__tests__/provider-presence.test.ts | 18 ++++++ src/lib/ai/capabilities/resolve.ts | 6 ++ src/lib/ai/provider.ts | 64 +++++++++++-------- 14 files changed, 188 insertions(+), 45 deletions(-) diff --git a/messages/de.json b/messages/de.json index 334ca82d0..e129a0e77 100644 --- a/messages/de.json +++ b/messages/de.json @@ -3137,7 +3137,8 @@ "noProvider": "Kein Anbieter verfügbar", "saving": "Wird gespeichert…", "saveFailed": "Die Änderung ließ sich nicht speichern.", - "allSettings": "Alle KI-Einstellungen" + "allSettings": "Alle KI-Einstellungen", + "providerNotActive": "{provider} konnte nicht übernehmen: Es fehlt eine funktionierende Anmeldung oder ein Schlüssel. Richte das in den KI-Einstellungen ein." }, "answer": { "scrollToBottom": "Zur neuesten Nachricht", diff --git a/messages/en.json b/messages/en.json index 6287e5518..936b669a6 100644 --- a/messages/en.json +++ b/messages/en.json @@ -3137,7 +3137,8 @@ "noProvider": "No provider available", "saving": "Saving…", "saveFailed": "That change could not be saved.", - "allSettings": "All AI settings" + "allSettings": "All AI settings", + "providerNotActive": "{provider} could not take over: it has no working sign-in or key. Set it up in AI settings." }, "answer": { "scrollToBottom": "Jump to the latest message", diff --git a/messages/es.json b/messages/es.json index d276a21c1..79a461607 100644 --- a/messages/es.json +++ b/messages/es.json @@ -3137,7 +3137,8 @@ "noProvider": "Ningún proveedor disponible", "saving": "Guardando…", "saveFailed": "No se pudo guardar el cambio.", - "allSettings": "Todos los ajustes de IA" + "allSettings": "Todos los ajustes de IA", + "providerNotActive": "{provider} no pudo encargarse: no tiene un inicio de sesión ni una clave que funcionen. Configúralo en los ajustes de IA." }, "answer": { "scrollToBottom": "Ir al último mensaje", diff --git a/messages/fr.json b/messages/fr.json index bdc8fe468..cbccefe22 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -3137,7 +3137,8 @@ "noProvider": "Aucun fournisseur disponible", "saving": "Enregistrement…", "saveFailed": "Impossible d’enregistrer cette modification.", - "allSettings": "Tous les paramètres IA" + "allSettings": "Tous les paramètres IA", + "providerNotActive": "{provider} n'a pas pu prendre le relais : aucune connexion ni clé valide. Configurez-le dans les réglages IA." }, "answer": { "scrollToBottom": "Aller au dernier message", diff --git a/messages/it.json b/messages/it.json index f58556352..6e50a69ab 100644 --- a/messages/it.json +++ b/messages/it.json @@ -3137,7 +3137,8 @@ "noProvider": "Nessun fornitore disponibile", "saving": "Salvataggio…", "saveFailed": "Impossibile salvare la modifica.", - "allSettings": "Tutte le impostazioni IA" + "allSettings": "Tutte le impostazioni IA", + "providerNotActive": "{provider} non è subentrato: manca un accesso o una chiave funzionante. Configuralo nelle impostazioni IA." }, "answer": { "scrollToBottom": "Vai all'ultimo messaggio", diff --git a/messages/ko.json b/messages/ko.json index 9fb976c03..4ecc63940 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -3137,7 +3137,8 @@ "noProvider": "사용할 수 있는 제공자가 없어요", "saving": "저장 중…", "saveFailed": "변경 사항을 저장하지 못했어요.", - "allSettings": "모든 AI 설정" + "allSettings": "모든 AI 설정", + "providerNotActive": "{provider}(으)로 전환하지 못했습니다. 작동하는 로그인이나 키가 없습니다. AI 설정에서 설정하세요." }, "answer": { "scrollToBottom": "최신 메시지로 이동", diff --git a/messages/pl.json b/messages/pl.json index 98c255698..b324f39b2 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -3137,7 +3137,8 @@ "noProvider": "Brak dostępnego dostawcy", "saving": "Zapisywanie…", "saveFailed": "Nie udało się zapisać zmiany.", - "allSettings": "Wszystkie ustawienia AI" + "allSettings": "Wszystkie ustawienia AI", + "providerNotActive": "{provider} nie mógł przejąć odpowiedzi: brak działającego logowania lub klucza. Skonfiguruj go w ustawieniach AI." }, "answer": { "scrollToBottom": "Przejdź do najnowszej wiadomości", diff --git a/src/app/api/insights/provider-chain/__tests__/route.test.ts b/src/app/api/insights/provider-chain/__tests__/route.test.ts index efb66afab..e66ca14e8 100644 --- a/src/app/api/insights/provider-chain/__tests__/route.test.ts +++ b/src/app/api/insights/provider-chain/__tests__/route.test.ts @@ -26,6 +26,12 @@ vi.mock("@/lib/api-handler", () => ({ vi.mock("@/lib/ai/provider", () => ({ resolveProviderChain: vi.fn(), + probeProviderChain: vi.fn(async () => ({ + entries: [], + localOcrEnabled: false, + managedBy: null, + availableTypes: ["codex", "openai", "admin-openai"], + })), })); vi.mock("@/lib/ai/provider-runner", () => ({ @@ -46,7 +52,7 @@ vi.mock("@/lib/db", () => ({ })); import { GET } from "../route"; -import { resolveProviderChain } from "@/lib/ai/provider"; +import { probeProviderChain, resolveProviderChain } from "@/lib/ai/provider"; import { getLastWorkingProvider } from "@/lib/ai/provider-runner"; import { prisma } from "@/lib/db"; @@ -176,4 +182,37 @@ describe("GET /api/insights/provider-chain", () => { true, ); }); + + it("marks an entry without a credential as unavailable, enabled or not", async () => { + // The Coach's provider select offers only available entries; an entry + // marked available without a key was saved to the front of the chain + // and never answered. + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + aiProviderChain: [ + { providerType: "codex", priority: 1, enabled: true }, + { providerType: "anthropic", priority: 2, enabled: true }, + { providerType: "openai", priority: 3, enabled: false }, + { providerType: "local", priority: 4, enabled: true }, + ], + } as never); + vi.mocked(probeProviderChain).mockResolvedValueOnce({ + entries: [{ providerType: "codex", vision: false }], + localOcrEnabled: false, + managedBy: "user", + availableTypes: ["codex", "openai"], + }); + vi.mocked(resolveProviderChain).mockResolvedValue([ + { providerType: "codex", instance: { type: "codex" } as never }, + ]); + vi.mocked(getLastWorkingProvider).mockReturnValue(null); + + const res = await (GET as () => Promise)(); + const body = (await res.json()) as Envelope; + expect(body.data?.configuredChain).toEqual([ + { providerType: "codex", enabled: true, available: true }, + { providerType: "anthropic", enabled: true, available: false }, + { providerType: "openai", enabled: false, available: true }, + { providerType: "local", enabled: true, available: false }, + ]); + }); }); diff --git a/src/app/api/insights/provider-chain/route.ts b/src/app/api/insights/provider-chain/route.ts index 477cb8159..8ced0a544 100644 --- a/src/app/api/insights/provider-chain/route.ts +++ b/src/app/api/insights/provider-chain/route.ts @@ -7,7 +7,7 @@ import { sanitiseZodIssues, } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; -import { resolveProviderChain } from "@/lib/ai/provider"; +import { probeProviderChain, resolveProviderChain } from "@/lib/ai/provider"; import { forgetLastWorkingProvider, getLastWorkingProvider, @@ -26,11 +26,11 @@ import { providerChainPutSchema } from "@/lib/validations/ai-provider"; * in-process cache, when one is set. Surfaces "Active: codex * (cached: openai)" so a user can see the runner has rerouted to * a fallback without digging through logs. - * - `configuredChain`: ordered list of `{providerType, available}` - * entries. `available` is always true here since the chain - * resolver only returns instantiable entries; the field is wired - * for the v1.4.17 UX where unconfigured slots should still render - * in the list with a "needs setup" pill. + * - `configuredChain`: ordered list of `{providerType, enabled, + * available}` entries. `available` is the presence probe's own + * answer for that entry's credential (`probeProviderChain`), enabled + * or not: an entry without a key, address or sign-in is listed but + * cannot answer, so a picker must not offer it. * * v1.4.16 phase B2 added the matching `PUT` for the new dropdown-driven * settings UX — the user reorders/toggles entries in the section and @@ -58,7 +58,11 @@ export const GET = apiHandler(async () => { }); const persisted = parseProviderChain(userRow?.aiProviderChain ?? null); - const resolved = await resolveProviderChain(user.id); + const [resolved, presence] = await Promise.all([ + resolveProviderChain(user.id), + probeProviderChain(user.id), + ]); + const availableTypes = new Set(presence.availableTypes ?? []); const cached = getLastWorkingProvider(user.id); annotate({ @@ -77,7 +81,7 @@ export const GET = apiHandler(async () => { configuredChain: persisted.map((entry) => ({ providerType: entry.providerType, enabled: entry.enabled, - available: true, + available: availableTypes.has(entry.providerType), })), }); }); diff --git a/src/components/insights/coach-panel/__tests__/coach-model-picker.test.tsx b/src/components/insights/coach-panel/__tests__/coach-model-picker.test.tsx index 91d922e95..2d4a8af6a 100644 --- a/src/components/insights/coach-panel/__tests__/coach-model-picker.test.tsx +++ b/src/components/insights/coach-panel/__tests__/coach-model-picker.test.tsx @@ -10,6 +10,7 @@ import { modelControlFor, reasoningFieldFor, selectableProviders, + switchDidNotTake, } from "../coach-model-picker"; const CHAIN: ProviderChainData["configuredChain"] = [ @@ -72,6 +73,23 @@ describe("selectableProviders", () => { }); }); +describe("switchDidNotTake", () => { + it("is quiet when the chosen provider leads the chain the server resolves", () => { + expect(switchDidNotTake("openai", { activeProvider: "openai" })).toBe( + false, + ); + }); + + it("speaks up when the server still answers with another provider", () => { + // A provider without a working key was saved to the front of the chain; + // the resolver skipped it and the select would jump back silently. + expect(switchDidNotTake("anthropic", { activeProvider: "openai" })).toBe( + true, + ); + expect(switchDidNotTake("anthropic", { activeProvider: null })).toBe(true); + }); +}); + describe("modelControlFor", () => { it("offers the presets for the provider whose key the person saved", () => { const control = modelControlFor("openai", userProvider()); diff --git a/src/components/insights/coach-panel/coach-model-picker.tsx b/src/components/insights/coach-panel/coach-model-picker.tsx index 079d99c96..da9c631b3 100644 --- a/src/components/insights/coach-panel/coach-model-picker.tsx +++ b/src/components/insights/coach-panel/coach-model-picker.tsx @@ -70,6 +70,18 @@ export function selectableProviders( .map((entry) => entry.providerType); } +/** + * True when a saved switch did not take: the chain the server resolves after + * the save is not led by the provider the person chose. A provider whose key + * stops working between the read and the save lands here. + */ +export function switchDidNotTake( + chosen: ProviderType, + fresh: Pick, +): boolean { + return fresh.activeProvider !== chosen; +} + export type ModelControl = | { kind: "preset"; presets: readonly string[]; value: string | null } | { kind: "gateway"; value: string | null } @@ -139,6 +151,7 @@ export function CoachModelPicker() { }); const [error, setError] = useState(null); + const [notice, setNotice] = useState(null); const onSaved = () => { setError(null); @@ -152,11 +165,26 @@ export function CoachModelPicker() { ); const switchProvider = useMutation({ - mutationFn: (provider: ProviderType) => - apiPut("/api/insights/provider-chain", { + mutationFn: async (provider: ProviderType) => { + setNotice(null); + await apiPut("/api/insights/provider-chain", { chain: chainWithFirst(chainQuery.data?.configuredChain ?? [], provider), - }), - onSuccess: onSaved, + }); + // Read back what the server now resolves, so a switch that did not + // take is said rather than shown as the select jumping back. + return apiGet("/api/insights/provider-chain"); + }, + onSuccess: (fresh, provider) => { + queryClient.setQueryData(queryKeys.insightsProviderChain(), fresh); + onSaved(); + if (switchDidNotTake(provider, fresh)) { + setNotice( + t("insights.coach.frame.providerNotActive", { + provider: t(`settings.ai.providerChain.types.${provider}`), + }), + ); + } + }, onError: onFailed, }); const patchProvider = useMutation({ @@ -294,6 +322,15 @@ export function CoachModelPicker() { {t("insights.coach.frame.saving")}

) : null} + {notice ? ( +

+ {notice} +

+ ) : null} {error ? (

{error} diff --git a/src/lib/ai/__tests__/provider-presence.test.ts b/src/lib/ai/__tests__/provider-presence.test.ts index 502b4f1e6..5662de426 100644 --- a/src/lib/ai/__tests__/provider-presence.test.ts +++ b/src/lib/ai/__tests__/provider-presence.test.ts @@ -90,6 +90,7 @@ describe("probeProviderChain", () => { entries: [], localOcrEnabled: false, managedBy: null, + availableTypes: [], }); expect(prisma.user.findUnique).not.toHaveBeenCalled(); }); @@ -123,6 +124,22 @@ describe("probeProviderChain", () => { expect(presence.managedBy).toBe("user"); }); + it("names every type whose credential is present, enabled in the chain or not", async () => { + load( + row({ + aiProviderChain: [ + { providerType: "codex", enabled: true, priority: 0 }, + { providerType: "openai", enabled: false, priority: 1 }, + { providerType: "anthropic", enabled: true, priority: 2 }, + ], + aiOpenaiKeyEncrypted: "enc", + }), + ); + const presence = await probeProviderChain("u1", OWNER); + // Codex is not signed in and no Anthropic key is saved: neither can answer. + expect(presence.availableTypes).toEqual(["openai"]); + }); + it("marks which entries can read an image", async () => { load( row({ @@ -162,6 +179,7 @@ describe("probeProviderChain", () => { entries: [{ providerType: "admin-openai", vision: true }], localOcrEnabled: false, managedBy: "server", + availableTypes: ["admin-openai"], }); }); diff --git a/src/lib/ai/capabilities/resolve.ts b/src/lib/ai/capabilities/resolve.ts index 5b97aee38..0152a7ac7 100644 --- a/src/lib/ai/capabilities/resolve.ts +++ b/src/lib/ai/capabilities/resolve.ts @@ -60,6 +60,12 @@ export interface ProviderPresence { /** The person's in-browser OCR opt-in, which lets a text-only provider read a document. */ localOcrEnabled: boolean; managedBy: AiProviderManagedBy | null; + /** + * Every chain tag whose credential is present, whether or not the person + * enabled it in their chain: what a chain editor may offer as able to + * answer. Absent reads as none, so a hand-built presence offers nothing. + */ + availableTypes?: readonly string[]; /** * The record's `aiResponseTimeoutSeconds`, read off the same row. Not a * presence fact, but the account payload publishes the effective timeout diff --git a/src/lib/ai/provider.ts b/src/lib/ai/provider.ts index a88e6e5ad..4f28958f1 100644 --- a/src/lib/ai/provider.ts +++ b/src/lib/ai/provider.ts @@ -16,7 +16,11 @@ import { } from "./codex-oauth"; import { isPublicUrl } from "@/lib/validations/notifications"; import { isLocalAiHostAllowed } from "./local-host-allowlist"; -import { parseProviderChain, type ProviderChainType } from "./provider-chain"; +import { + parseProviderChain, + PROVIDER_CHAIN_TYPES, + type ProviderChainType, +} from "./provider-chain"; import type { ProviderChainResolved } from "./provider-runner"; import { providerCredentialPolicy, @@ -917,6 +921,7 @@ const NO_PRESENCE: ProviderPresence = Object.freeze({ entries: [], localOcrEnabled: false, managedBy: null, + availableTypes: [], }); /** @@ -995,6 +1000,7 @@ export function probeProviderChain( // A guardian's in-browser OCR is not the record's opt-in to use. localOcrEnabled: false, managedBy: adminKey ? "server" : null, + availableTypes: adminKey ? ["admin-openai"] : [], responseTimeoutSeconds: row.aiResponseTimeoutSeconds, }; } @@ -1003,36 +1009,43 @@ export function probeProviderChain( row.codexConnectionStatus === "connected" && !!row.codexAccessTokenEncrypted && !!row.codexRefreshTokenEncrypted; + const credentialPresent = (providerType: ProviderChainType): boolean => { + switch (providerType) { + case "codex": + return codexConnected; + case "openai": + return !!row.aiOpenaiKeyEncrypted; + case "anthropic": + return !!row.aiAnthropicKeyEncrypted; + case "local": + return !!row.aiBaseUrl; + case "openai-compatible": + return !!( + row.aiCompatBaseUrl && + (row.aiCompatModel || row.aiModel) + ); + case "admin-openai": + return adminKey; + default: + // `admin-codex` is never resolved from a stored chain entry. + return false; + } + }; + const centralCodex = + row.useCentralCodex && appSettingsCentralCodexConnected(settings); const entries: ProviderEntryPresence[] = []; for (const entry of parseProviderChain(row.aiProviderChain ?? null)) { if (!entry.enabled) continue; - const present = (() => { - switch (entry.providerType) { - case "codex": - return codexConnected; - case "openai": - return !!row.aiOpenaiKeyEncrypted; - case "anthropic": - return !!row.aiAnthropicKeyEncrypted; - case "local": - return !!row.aiBaseUrl; - case "openai-compatible": - return !!( - row.aiCompatBaseUrl && - (row.aiCompatModel || row.aiModel) - ); - case "admin-openai": - return adminKey; - default: - // `admin-codex` is never resolved from a stored chain entry. - return false; - } - })(); - if (present) entries.push(vision(entry.providerType)); + if (credentialPresent(entry.providerType)) { + entries.push(vision(entry.providerType)); + } } - if (row.useCentralCodex && appSettingsCentralCodexConnected(settings)) { + if (centralCodex) { entries.push(vision("admin-codex")); } + const availableTypes: string[] = PROVIDER_CHAIN_TYPES.filter((type) => + type === "admin-codex" ? centralCodex : credentialPresent(type), + ); if (entries.length === 0 && userRowHasProviderCredential(row, adminKey)) { // The legacy single-provider fallback, tagged as the operator's slot // the way every consumer of it tags it (the consent gate included). @@ -1051,6 +1064,7 @@ export function probeProviderChain( entries, localOcrEnabled: row.labsLocalOcrEnabled, managedBy: entries.length > 0 ? managedBy : null, + availableTypes, responseTimeoutSeconds: row.aiResponseTimeoutSeconds, }; },