diff --git a/apps/app/src/react-app/domains/connections/provider-auth/local-provider-engine-state.ts b/apps/app/src/react-app/domains/connections/provider-auth/local-provider-engine-state.ts index 4db0855ef1..9e20f39274 100644 --- a/apps/app/src/react-app/domains/connections/provider-auth/local-provider-engine-state.ts +++ b/apps/app/src/react-app/domains/connections/provider-auth/local-provider-engine-state.ts @@ -1,36 +1,65 @@ import { useMemo } from "react"; -import { useQuery } from "@tanstack/react-query"; +import { keepPreviousData, useQuery } from "@tanstack/react-query"; import type { EngineV2PreviewStatus, OpenworkServerClient } from "@/app/lib/openwork-server"; import { isCloudManagedProviderKey } from "./cloud-provider-config"; /** - * How a provider on this device stands on the engine chat runs on, when it - * cannot be used there. A provider without one is ready. + * How a provider on this device stands on the engine chat runs on. + * - `ready`: the engine has it; it is in the model picker. * - `managed_by_organization`: the organization allows only its own providers. * - `not_on_engine`: OpenCode v2 cannot use it yet (a sign-in, or no adapter). */ -export type LocalProviderEngineState = "managed_by_organization" | "not_on_engine"; +export type LocalProviderEngineState = "ready" | "managed_by_organization" | "not_on_engine"; -export type LocalProviderEngineStatus = Pick; +export type LocalProviderEngineStatus = Pick; /** - * Read from the server's skipped list, never guessed: a provider OpenCode v2 - * skipped is never "Ready to use". Only while chat runs on v2 and the server - * explains its skips (the `localProviderMirror` rollout); otherwise nothing - * changes. Organization providers keep their own rows and states. + * What OpenCode v2 reports for the providers on this device: the ones it + * mirrored are ready, the ones it skipped carry their reason. Only while chat + * runs on v2 and the server explains its skips (the `localProviderMirror` + * rollout); otherwise there is no engine view and nothing changes. + * Organization providers keep their own rows and states. */ export function localProviderEngineStates(status: LocalProviderEngineStatus | null | undefined): ReadonlyMap { if (!status?.enabled || !status.chatRouting || !status.skippedProviders) return new Map(); - return new Map(status.skippedProviders.flatMap((provider): Array<[string, LocalProviderEngineState]> => - isCloudManagedProviderKey(provider.id) - ? [] - : [[provider.id, provider.reason === "policy" ? "managed_by_organization" : "not_on_engine"]])); + const states = new Map(); + for (const id of status.mirroredProviderIds) { + if (!isCloudManagedProviderKey(id)) states.set(id, "ready"); + } + for (const provider of status.skippedProviders) { + if (!isCloudManagedProviderKey(provider.id)) states.set(provider.id, provider.reason === "policy" ? "managed_by_organization" : "not_on_engine"); + } + return states; +} + +export type DeviceProvider = { id: string; name: string; source?: "env" | "api" | "config" | "custom" }; + +/** + * "On this device" while chat runs on v2: the providers OpenCode v1 lists for + * this folder, reconciled with what the engine reports. A provider the + * engine mirrored or skipped is always listed, even when v1's list does not + * have it yet, so a provider that works in the picker is never missing and a + * skipped one shows its reason. Hidden (disconnected) providers stay hidden. + */ +export function deviceProviders(input: { + connected: readonly DeviceProvider[]; + engine: ReadonlyMap; + hidden: ReadonlySet; + nameFor: (id: string) => string; +}): DeviceProvider[] { + const listed = new Set(input.connected.map((provider) => provider.id)); + return [ + ...input.connected, + ...[...input.engine.keys()].flatMap((id) => + listed.has(id) || input.hidden.has(id.trim().toLowerCase()) ? [] : [{ id, name: input.nameFor(id) }]), + ]; } /** - * The engine states for Settings › AI Providers. `refreshKey` changes when the - * connected providers do; the server finishes mirroring a saved key before - * the save returns, so the next read already explains it. + * The engine's view for Settings › AI Providers. `refreshKey` changes whenever + * the provider list is read again; the server finishes mirroring a saved key + * before the save returns, so that read already explains it. The last answer + * stays on screen while the next one loads. */ export function useLocalProviderEngineStates(client: OpenworkServerClient | null, refreshKey: string): ReadonlyMap { const status = useQuery({ @@ -39,6 +68,7 @@ export function useLocalProviderEngineStates(client: OpenworkServerClient | null enabled: Boolean(client), staleTime: 30_000, retry: false, + placeholderData: keepPreviousData, }); return useMemo(() => localProviderEngineStates(status.data), [status.data]); } diff --git a/apps/app/src/react-app/shell/settings-route.tsx b/apps/app/src/react-app/shell/settings-route.tsx index 159571c7e9..0367d03058 100644 --- a/apps/app/src/react-app/shell/settings-route.tsx +++ b/apps/app/src/react-app/shell/settings-route.tsx @@ -85,7 +85,7 @@ import ProviderAuthModal, { PROVIDER_LABELS } from "@/react-app/domains/connecti import { keylessProviderIds } from "@/react-app/domains/connections/provider-auth/provider-policy"; import ConnectionsModals from "@/react-app/domains/connections/modals"; import { AiSettingsView } from "@/react-app/domains/settings/pages/ai-view"; -import { useLocalProviderEngineStates } from "@/react-app/domains/connections/provider-auth/local-provider-engine-state"; +import { deviceProviders, useLocalProviderEngineStates } from "@/react-app/domains/connections/provider-auth/local-provider-engine-state"; // Side-effect imports: register extension config components into the registry. import { OllamaConfig } from "@/react-app/domains/settings/ollama-config"; import "@/react-app/domains/settings/browser-extension-config"; @@ -1805,12 +1805,16 @@ function SettingsRouteContent(props: SettingsSurfaceProps = {}) { const disabledProviderIdSet = new Set( disabledProviders.map((id) => id.trim().toLowerCase()).filter(Boolean), ); + // Read the engine's view again whenever the provider list is read again, + // including when v1's list did not change. const localProviderStates = useLocalProviderEngineStates( selectedWorkspaceEndpoint?.client ?? openworkClient, - providerConnectedIds.join("\n"), + `${providerAuthSnapshot.providerLoadState.status}\n${providerConnectedIds.join("\n")}`, ); - const connectedProviders = [ - ...providers.flatMap((provider) => + // While chat runs on v2, v1's list can lag the engine (it caches per folder): + // a provider the engine mirrored or skipped stays listed with its state. + const connectedProviders = deviceProviders({ + connected: providers.flatMap((provider) => providerConnectedIdSet.has(provider.id) && !disabledProviderIdSet.has(provider.id.trim().toLowerCase()) ? [{ @@ -1820,14 +1824,10 @@ function SettingsRouteContent(props: SettingsSurfaceProps = {}) { }] : [], ), - // v1 leaves a provider the organization does not allow out of its list. - // On v2 it stays visible with the reason instead of disappearing. - ...[...localProviderStates.keys()].flatMap((id) => - providerConnectedIdSet.has(id) || disabledProviderIdSet.has(id.trim().toLowerCase()) - ? [] - : [{ id, name: providers.find((provider) => provider.id === id)?.name ?? PROVIDER_LABELS[id.toLowerCase()] ?? id }], - ), - ]; + engine: localProviderStates, + hidden: disabledProviderIdSet, + nameFor: (id) => providers.find((provider) => provider.id === id)?.name ?? PROVIDER_LABELS[id.toLowerCase()] ?? id, + }); // Disconnect hides built-in/config providers through disabled_providers, and // the engine then omits them everywhere. List them so they can come back. const hiddenProviders = [...new Set(disabledProviders.map((id) => id.trim()).filter(Boolean))].flatMap((id) => diff --git a/apps/app/tests/local-provider-engine-state.test.tsx b/apps/app/tests/local-provider-engine-state.test.tsx index e2fa068207..241498ad3d 100644 --- a/apps/app/tests/local-provider-engine-state.test.tsx +++ b/apps/app/tests/local-provider-engine-state.test.tsx @@ -2,12 +2,13 @@ import { afterEach, expect, mock, spyOn, test } from "bun:test"; import { renderToStaticMarkup } from "react-dom/server"; import { createOpenworkServerClient } from "../src/app/lib/openwork-server"; -import { localProviderEngineStates } from "../src/react-app/domains/connections/provider-auth/local-provider-engine-state"; +import { deviceProviders, localProviderEngineStates } from "../src/react-app/domains/connections/provider-auth/local-provider-engine-state"; import { LocalProviderStatus } from "../src/react-app/domains/settings/pages/provider-rows"; const status = { enabled: true, chatRouting: true, + mirroredProviderIds: ["anthropic", "lpr_team", "ipr_gateway"], skippedProviders: [ { id: "openrouter", reason: "policy" as const }, { id: "openai", reason: "oauth" as const }, @@ -20,30 +21,50 @@ afterEach(() => { mock.restore(); }); -test("a provider OpenCode v2 skipped is never ready, and the reason decides its state", () => { +test("a provider OpenCode v2 mirrored is ready, a skipped one is never ready and its reason decides its state", () => { const states = localProviderEngineStates(status); expect([...states]).toEqual([ + ["anthropic", "ready"], ["openrouter", "managed_by_organization"], ["openai", "not_on_engine"], ["google", "not_on_engine"], ]); expect(states.has("lpr_team")).toBe(false); + expect(states.has("ipr_gateway")).toBe(false); +}); + +test("On this device lists every provider the engine reports, even when v1's cached list has not caught up", () => { + const nameFor = (id: string) => ({ anthropic: "Anthropic", google: "Google" }[id] ?? id); + // v1 still lists only the first key; the second one is mirrored and in the picker. + const connected = [{ id: "openrouter", name: "OpenRouter", source: "api" as const }, { id: "lpr_team", name: "Team models" }]; + const engine = localProviderEngineStates({ enabled: true, chatRouting: true, mirroredProviderIds: ["openrouter", "anthropic", "lpr_team"], skippedProviders: [{ id: "google", reason: "unsupported" }, { id: "openai", reason: "oauth" }] }); + + expect(deviceProviders({ connected, engine, hidden: new Set(["openai"]), nameFor })).toEqual([ + { id: "openrouter", name: "OpenRouter", source: "api" }, + { id: "lpr_team", name: "Team models" }, + { id: "anthropic", name: "Anthropic" }, + { id: "google", name: "Google" }, + ]); + // Without the engine's view (v1 chat, an older server, the rollout off) the list is v1's, unchanged. + expect(deviceProviders({ connected, engine: localProviderEngineStates(null), hidden: new Set(), nameFor })).toEqual(connected); }); test("nothing changes on v1, while v2 is not chat's engine, or when the server does not explain its skips", () => { expect(localProviderEngineStates({ ...status, chatRouting: false }).size).toBe(0); expect(localProviderEngineStates({ ...status, enabled: false }).size).toBe(0); - expect(localProviderEngineStates({ enabled: true, chatRouting: true }).size).toBe(0); + expect(localProviderEngineStates({ enabled: true, chatRouting: true, mirroredProviderIds: ["anthropic"] }).size).toBe(0); expect(localProviderEngineStates(null).size).toBe(0); }); test("rows read Ready to use, the organization's lock, or not available on the new engine", () => { const ready = renderToStaticMarkup(); + const mirrored = renderToStaticMarkup(); const managed = renderToStaticMarkup(); const unnamed = renderToStaticMarkup(); const unavailable = renderToStaticMarkup(); expect(ready).toContain("Ready to use"); + expect(mirrored).toContain("Ready to use"); expect(managed).toContain("Acme manages AI providers"); expect(managed).toContain("lucide-lock"); expect(managed).not.toContain("text-red"); diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 1c3a230c88..d4ad3bff4d 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -1005,10 +1005,15 @@ export async function startServer( await engineV2Preview.refreshProviders(); // With the localProviderMirror rollout (the mirror then explains // its skips), also rebuild v1's provider state for this folder: - // AI Providers lists providers from v1, which otherwise keeps the - // old list until a restart. Never while v1 is running work. + // AI Providers lists providers from v1, which caches them per + // folder and otherwise keeps the old list until a restart. + // Disposed in place, not through the engine pool: the pool skips a + // folder already reloaded onto the same config file, and saved + // credentials are not part of that file, so every save after the + // first was skipped. Never spawns an engine, and never while v1 is + // running work. if (engineV2Preview.status().skippedProviders && !await engineHasActiveSessions(config, workspace)) { - await reloadOpencodeEngine(config, workspace, engineMcpServerState, { reason: "local_provider_auth", awaitPostRefreshSync: false }) + await reloadOpencodeEngineInPlace(config, workspace, engineMcpServerState, { awaitPostRefreshSync: false }) .catch((error) => logger.log("warn", "Could not refresh OpenCode v1 providers after a provider change.", { error: error instanceof Error ? error.message : String(error), })); diff --git a/docs/features/local-provider-mirror/README.md b/docs/features/local-provider-mirror/README.md index 62a98b43dc..001e71031e 100644 --- a/docs/features/local-provider-mirror/README.md +++ b/docs/features/local-provider-mirror/README.md @@ -24,8 +24,12 @@ With the feature on, for the signed-in member: adds `skippedProviders: [{ id, reason }]`, with `reason` one of `policy`, `oauth`, `unsupported`, `metadata` or `credentials`. - After a key is saved or a provider sign-in finishes, the server mirrors it - into v2 and rebuilds v1's provider list for that folder (never while v1 runs - work), so AI Providers and the picker agree right away. + into v2 and disposes v1's instance for that folder in place (never while v1 + runs work, never through the engine pool, which skips a folder whose config + file did not change), so v1's provider list is fresh on the next read. +- AI Providers lists every provider on this device that the engine mirrored + (Ready to use) or skipped (with its reason), even when v1's list has not + caught up, so a provider that works in the picker is never missing. - AI Providers reads the skipped list: a sign-in v2 cannot use yet (ChatGPT, for example) reads "Not available on the new engine yet", and a provider the organization does not allow keeps its row with a lock and "Acme manages AI diff --git a/evals/proofs/v2-local-provider-mirror.proof.test.ts b/evals/proofs/v2-local-provider-mirror.proof.test.ts index 47dc965732..809a8fccdc 100644 --- a/evals/proofs/v2-local-provider-mirror.proof.test.ts +++ b/evals/proofs/v2-local-provider-mirror.proof.test.ts @@ -9,8 +9,9 @@ const test = spec.world(v2LocalProviderMirror, { const WITHIN = 60_000; -test("an owner's OpenRouter key reaches the new engine's model picker, and AI Providers stays honest about sign-ins and Acme's provider rule", async ({ world, user, probe, step, evidence }) => { - const openRouterModels = async () => (await probe.dom('[data-testid="composer-model-picker"] [data-model-key^="openrouter:"]')).elements.length; +test("an owner's OpenRouter and Anthropic keys reach the new engine's model picker and AI Providers, which stays honest about sign-ins and Acme's provider rule", async ({ world, user, probe, step, evidence }) => { + const pickerModels = async (providerId: string) => (await probe.dom(`[data-testid="composer-model-picker"] [data-model-key^="${providerId}:"]`)).elements.length; + const openRouterModels = () => pickerModels("openrouter"); const row = async (providerId: string) => (await probe.dom(`[data-testid="local-provider-${providerId}"]`)).elements.map(element => element.text).join(" "); /** Open the chat's model picker and search it; opening it also asks the desktop to refresh providers. */ @@ -23,10 +24,10 @@ test("an owner's OpenRouter key reaches the new engine's model picker, and AI Pr } /** Reopen the picker until it agrees with `until`, as a person would after a change. */ - async function pickerUntil(label: string, until: (count: number) => boolean) { + async function pickerUntil(label: string, until: (count: number) => boolean, provider = { id: "openrouter", name: "OpenRouter" }) { return probe.eventually(async () => { - await searchPicker("OpenRouter"); - const count = await openRouterModels(); + await searchPicker(provider.name); + const count = await pickerModels(provider.id); if (!until(count)) await user.press("Escape"); return count; }, { within: 120_000, label, until }); @@ -34,16 +35,18 @@ test("an owner's OpenRouter key reaches the new engine's model picker, and AI Pr await user.resizeViewport({ width: 1440, height: 900, deviceScaleFactor: 1 }); - async function connectOpenRouter() { + /** Settings › AI Providers › Connect a provider, paste a key, Save key: the path a person takes. */ + async function connectProvider(name: string, key: string) { await user.navigate(world.settingsUrl); await user.see({ role: "button", label: "Connect a provider" }, { timeoutMs: WITHIN }); await user.click({ role: "button", label: "Connect a provider" }); - await user.type({ label: "Search providers" }, "OpenRouter"); - await user.click({ role: "button", label: /^OpenRouter/ }); - await user.type({ label: "API key" }, world.openRouterKey); + await user.type({ label: "Search providers" }, name); + await user.click({ role: "button", label: new RegExp(`^${name}`) }); + await user.type({ label: "API key" }, key); await user.click({ role: "button", label: "Save key" }); await user.notSee({ role: "button", label: "Save key" }, { timeoutMs: WITHIN }); } + const connectOpenRouter = () => connectProvider("OpenRouter", world.openRouterKey); const engineStatus = async () => { const status = await probe.desktopApi("/experimental/engine-v2-preview/status"); const body = typeof status.body === "object" && status.body !== null ? status.body : {}; @@ -85,6 +88,28 @@ test("an owner's OpenRouter key reaches the new engine's model picker, and AI Pr await user.press("Escape"); }); + await step("after: a second key, Anthropic, is listed in AI Providers next to OpenRouter right after it is saved", async () => { + await connectProvider("Anthropic", world.anthropicKey); + await user.see({ testId: "local-provider-openrouter" }, { timeoutMs: WITHIN }); + // As a person does: no reload, no restart. A short wait covers rendering, not a later refresh. + const anthropic = await probe.eventually(() => row("anthropic"), { within: 20_000, label: "the Anthropic row right after saving its key", until: value => value.includes("Ready to use") }); + const openRouter = await row("openrouter"); + const status = await engineStatus(); + expect(status.mirrored).toContain('"anthropic"'); + expect(anthropic).toContain("Ready to use"); + expect(openRouter).toContain("Ready to use"); + evidence.recordAssertionEvidence("Both providers added on this computer are listed as ready", `Anthropic: ${anthropic || "missing"} | OpenRouter: ${openRouter || "missing"} | engine mirrored ${status.mirrored}`, anthropic.includes("Ready to use") && openRouter.includes("Ready to use")); + await user.screenshot(); + }); + + await step("after: Anthropic's models are in the picker too", async () => { + const count = await pickerUntil("Anthropic models in the picker", value => value > 0, { id: "anthropic", name: "Anthropic" }); + expect(count).toBeGreaterThan(0); + evidence.recordAssertionEvidence("The second key lists its models on the new engine", `${count} Anthropic models match "Anthropic"`, count > 0); + await user.screenshot(); + await user.press("Escape"); + }); + await step("after: a ChatGPT sign-in the new engine cannot use yet says so instead of Ready to use", async () => { await world.signInWithChatGpt(); await user.navigate(world.settingsUrl); diff --git a/evals/worlds/v2-local-provider-mirror.ts b/evals/worlds/v2-local-provider-mirror.ts index 22bacc6d5c..f6609d46e5 100644 --- a/evals/worlds/v2-local-provider-mirror.ts +++ b/evals/worlds/v2-local-provider-mirror.ts @@ -85,6 +85,7 @@ export async function v2LocalProviderMirror(seed: Seed) { app, den, workspace, session, setMirror, allowPersonalProviders, signInWithChatGpt, organizationName: "Acme", openRouterKey: "sk-or-v1-synthetic-not-a-real-key", + anthropicKey: "sk-ant-synthetic-not-a-real-key", settingsUrl: `${app.webUrl}/workspace/${encodeURIComponent(workspace.workspaceId)}/settings/ai`, sessionUrl: `${app.webUrl}/workspace/${encodeURIComponent(workspace.workspaceId)}/session/${encodeURIComponent(session.sessionId)}`, };