diff --git a/apps/pi-extension/server/ai-runtime.test.ts b/apps/pi-extension/server/ai-runtime.test.ts new file mode 100644 index 000000000..5060ccdf8 --- /dev/null +++ b/apps/pi-extension/server/ai-runtime.test.ts @@ -0,0 +1,152 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { + chmodSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("createPiAIRuntime Codex discovery", () => { + test("capabilities does not execute Codex and session activation does", async () => { + if (process.platform === "win32") return; + + const dir = mkdtempSync(join(tmpdir(), "plannotator-pi-lazy-codex-")); + tempDirs.push(dir); + const marker = join(dir, "codex-ran"); + const codex = join(dir, "codex"); + writeFileSync(codex, `#!/bin/sh\necho ran > '${marker}'\nexit 1\n`); + chmodSync(codex, 0o755); + + const runner = join(dir, "runner.ts"); + const runtimeUrl = pathToFileURL(join(import.meta.dir, "ai-runtime.ts")).href; + writeFileSync(runner, ` + import { existsSync } from "node:fs"; + import { createPiAIRuntime } from ${JSON.stringify(runtimeUrl)}; + const runtime = await createPiAIRuntime({ cwd: ${JSON.stringify(dir)} }); + if (!runtime) throw new Error("Pi AI runtime unavailable"); + const capabilities = await runtime.endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities"), + ); + const data = await capabilities.json(); + const afterCapabilities = existsSync(${JSON.stringify(marker)}); + const session = await runtime.endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex-sdk", + }), + }), + ); + console.log(JSON.stringify({ + hasCodex: data.providers.some((provider) => provider.id === "codex-sdk"), + afterCapabilities, + sessionStatus: session.status, + afterSession: existsSync(${JSON.stringify(marker)}), + })); + runtime.dispose(); + `); + + const proc = Bun.spawn([process.execPath, runner], { + cwd: join(import.meta.dir, ".."), + env: { ...process.env, PATH: `${dir}:/usr/bin:/bin` }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + expect(exitCode, stderr).toBe(0); + expect(JSON.parse(stdout.trim())).toEqual({ + hasCodex: true, + afterCapabilities: false, + sessionStatus: 200, + afterSession: true, + }); + }, 15_000); + + test("capabilities?activate= runs discovery once and shares it with the session path", async () => { + if (process.platform === "win32") return; + + const dir = mkdtempSync(join(tmpdir(), "plannotator-pi-activate-codex-")); + tempDirs.push(dir); + const marker = join(dir, "codex-ran"); + const codex = join(dir, "codex"); + writeFileSync(codex, `#!/bin/sh\necho ran >> '${marker}'\nexit 1\n`); + chmodSync(codex, 0o755); + + const runner = join(dir, "runner.ts"); + const runtimeUrl = pathToFileURL(join(import.meta.dir, "ai-runtime.ts")).href; + writeFileSync(runner, ` + import { existsSync, readFileSync } from "node:fs"; + import { createPiAIRuntime } from ${JSON.stringify(runtimeUrl)}; + const runs = () => existsSync(${JSON.stringify(marker)}) + ? readFileSync(${JSON.stringify(marker)}, "utf8").trim().split("\\n").length + : 0; + const runtime = await createPiAIRuntime({ cwd: ${JSON.stringify(dir)} }); + if (!runtime) throw new Error("Pi AI runtime unavailable"); + const probe = await runtime.endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities"), + ); + const runsAfterProbe = runs(); + const activate = await runtime.endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities?activate=codex-sdk"), + ); + const runsAfterActivate = runs(); + const session = await runtime.endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex-sdk", + }), + }), + ); + console.log(JSON.stringify({ + probeStatus: probe.status, + runsAfterProbe, + activateStatus: activate.status, + runsAfterActivate, + sessionStatus: session.status, + runsAfterSession: runs(), + })); + runtime.dispose(); + `); + + const proc = Bun.spawn([process.execPath, runner], { + cwd: join(import.meta.dir, ".."), + env: { ...process.env, PATH: `${dir}:/usr/bin:/bin` }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + expect(exitCode, stderr).toBe(0); + expect(JSON.parse(stdout.trim())).toEqual({ + probeStatus: 200, + runsAfterProbe: 0, + activateStatus: 200, + runsAfterActivate: 1, + sessionStatus: 200, + runsAfterSession: 1, + }); + }, 15_000); +}); diff --git a/apps/pi-extension/server/ai-runtime.ts b/apps/pi-extension/server/ai-runtime.ts index 07ed3e644..cf6c4ea64 100644 --- a/apps/pi-extension/server/ai-runtime.ts +++ b/apps/pi-extension/server/ai-runtime.ts @@ -36,6 +36,7 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}): const registry = new ai.ProviderRegistry(); const sessionManager = new ai.SessionManager(); const modelDiscovery: Promise[] = []; + const providerInitializers = new Map Promise>(); try { await import("../generated/ai/providers/claude-agent-sdk.ts"); @@ -59,10 +60,13 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}): cwd, ...(codexPath ? { codexExecutablePath: codexPath } : {}), }); - registry.register(provider); + const providerId = registry.register(provider); if (provider && "fetchModels" in provider) { - modelDiscovery.push( - (provider as { fetchModels: () => Promise }).fetchModels().catch(() => {}), + providerInitializers.set( + providerId, + ai.createBestEffortOnce( + () => (provider as { fetchModels: () => Promise }).fetchModels(), + ), ); } } @@ -121,6 +125,9 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}): beforeCapabilities: async () => { await Promise.allSettled(modelDiscovery); }, + beforeProviderSession: async (providerId) => { + await providerInitializers.get(providerId)?.(); + }, }), dispose: () => { sessionManager.disposeAll(); diff --git a/packages/ai/ai.test.ts b/packages/ai/ai.test.ts index 8cf3e453a..e315b139f 100644 --- a/packages/ai/ai.test.ts +++ b/packages/ai/ai.test.ts @@ -6,12 +6,17 @@ import { registerProviderFactory, createProvider, } from "./provider.ts"; -import { AI_ENDPOINT_PATHS, createAIEndpoints } from "./endpoints.ts"; +import { + AI_ENDPOINT_PATHS, + createAIEndpoints, + createBestEffortOnce, +} from "./endpoints.ts"; import type { AIProvider, AISession, AIMessage, AIContext, + CreateSessionOptions, } from "./types.ts"; import { buildWindowsCommandScriptSpawnCommand, @@ -78,6 +83,40 @@ function mockProvider(name = "mock"): AIProvider { }; } +describe("createBestEffortOnce", () => { + test("coalesces concurrent calls and caches completion", async () => { + let calls = 0; + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const initialize = createBestEffortOnce(async () => { + calls++; + await gate; + }); + + const first = initialize(); + const second = initialize(); + expect(calls).toBe(1); + release(); + await Promise.all([first, second]); + await initialize(); + expect(calls).toBe(1); + }); + + test("swallows initialization failure and does not retry", async () => { + let calls = 0; + const initialize = createBestEffortOnce(async () => { + calls++; + throw new Error("discovery failed"); + }); + + await expect(initialize()).resolves.toBeUndefined(); + await expect(initialize()).resolves.toBeUndefined(); + expect(calls).toBe(1); + }); +}); + // --------------------------------------------------------------------------- // Command path helpers // --------------------------------------------------------------------------- @@ -577,6 +616,80 @@ describe("AI endpoints", () => { expect(data.providers[0].capabilities.fork).toBe(true); }); + test("capabilities does not activate providers", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + let activated = false; + reg.register(mockProvider("codex-sdk"), "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async () => { + activated = true; + }, + }); + + const res = await endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities"), + ); + + expect(res.status).toBe(200); + expect(activated).toBe(false); + }); + + test("capabilities?activate= runs the provider initializer and reports refreshed models", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + const activations: string[] = []; + const provider = { + ...mockProvider("codex-sdk"), + models: [{ id: "static-model", label: "Static", default: true }], + }; + reg.register(provider, "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async (providerId) => { + activations.push(providerId); + provider.models = [ + { id: "discovered-model", label: "Discovered", default: true }, + ]; + }, + }); + + const res = await endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities?activate=codex"), + ); + const data = await res.json(); + + expect(res.status).toBe(200); + expect(activations).toEqual(["codex"]); + expect(data.providers[0].models).toEqual([ + { id: "discovered-model", label: "Discovered", default: true }, + ]); + }); + + test("capabilities?activate= ignores unknown provider ids", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + let activated = false; + reg.register(mockProvider("codex-sdk"), "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async () => { + activated = true; + }, + }); + + const res = await endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities?activate=missing"), + ); + + expect(res.status).toBe(200); + expect(activated).toBe(false); + }); + test("capabilities waits for pending provider discovery", async () => { const reg = new ProviderRegistry(); const sm = new SessionManager(); @@ -682,6 +795,192 @@ describe("AI endpoints", () => { expect(createRes.status).toBe(200); }); + test("session creation activates only the resolved provider before createSession", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + const events: string[] = []; + reg.register(mockProvider("claude-agent-sdk"), "claude"); + reg.register({ + ...mockProvider("codex-sdk"), + async createSession() { + events.push("create"); + return mockSession(`session-${++sessionCounter}`, null); + }, + }, "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async (providerId) => { + events.push(`activate:${providerId}`); + }, + }); + + const res = await endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex", + }), + }), + ); + + expect(res.status).toBe(200); + expect(events).toEqual(["activate:codex", "create"]); + }); + + test("session creation replaces the static default with the discovered default", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + let createdModel: string | undefined; + const provider = { + ...mockProvider("codex-sdk"), + models: [{ id: "static-model", label: "Static", default: true }], + async createSession(options: CreateSessionOptions) { + createdModel = options.model; + return mockSession(`session-${++sessionCounter}`, null); + }, + }; + reg.register(provider, "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async () => { + provider.models = [ + { id: "discovered-model", label: "Discovered", default: true }, + ]; + }, + }); + + const res = await endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex", + model: "static-model", + }), + }), + ); + + expect(res.status).toBe(200); + expect(createdModel).toBe("discovered-model"); + }); + + test("session creation honors a requested model the provider still offers", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + let createdModel: string | undefined; + const provider = { + ...mockProvider("codex-sdk"), + models: [{ id: "static-model", label: "Static", default: true }], + async createSession(options: CreateSessionOptions) { + createdModel = options.model; + return mockSession(`session-${++sessionCounter}`, null); + }, + }; + reg.register(provider, "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async () => { + provider.models = [ + { id: "discovered-default", label: "Default", default: true }, + { id: "discovered-alt", label: "Alt" }, + ]; + }, + }); + + const res = await endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex", + model: "discovered-alt", + }), + }), + ); + + expect(res.status).toBe(200); + expect(createdModel).toBe("discovered-alt"); + }); + + test("session creation snaps an unlisted model to the discovered default on every session", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + const createdModels: Array = []; + const provider = { + ...mockProvider("codex-sdk"), + models: [{ id: "static-model", label: "Static", default: true }], + async createSession(options: CreateSessionOptions) { + createdModels.push(options.model); + return mockSession(`session-${++sessionCounter}`, null); + }, + }; + reg.register(provider, "codex"); + const endpoints = createAIEndpoints({ + registry: reg, + sessionManager: sm, + beforeProviderSession: async () => { + provider.models = [ + { id: "discovered-model", label: "Discovered", default: true }, + ]; + }, + }); + + // Two sessions with the stale pre-discovery fallback id: the second one + // runs after activation already happened, and must not pin the stale id + // just because it no longer matches the pre-activation default. + for (let i = 0; i < 2; i++) { + const res = await endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex", + model: "static-model", + }), + }), + ); + expect(res.status).toBe(200); + } + + expect(createdModels).toEqual(["discovered-model", "discovered-model"]); + }); + + test("session creation passes the requested model through when the provider reports no models", async () => { + const reg = new ProviderRegistry(); + const sm = new SessionManager(); + let createdModel: string | undefined; + reg.register({ + ...mockProvider("mock"), + async createSession(options: CreateSessionOptions) { + createdModel = options.model; + return mockSession(`session-${++sessionCounter}`, null); + }, + }); + + const endpoints = createAIEndpoints({ registry: reg, sessionManager: sm }); + const res = await endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + model: "anything-goes", + }), + }), + ); + + expect(res.status).toBe(200); + expect(createdModel).toBe("anything-goes"); + }); + test("session creation clamps client-supplied cost controls", async () => { const { reg, endpoints } = setup(); let seenOptions: { maxTurns?: number; maxBudgetUsd?: number } | null = null; diff --git a/packages/ai/endpoints.ts b/packages/ai/endpoints.ts index 557452db8..e944047a1 100644 --- a/packages/ai/endpoints.ts +++ b/packages/ai/endpoints.ts @@ -83,11 +83,23 @@ export interface AIEndpointDeps { getCwd?: () => string; /** Optional hook to finish lazy provider capability loading before reporting capabilities. */ beforeCapabilities?: () => Promise | void; + /** Optional hook to finish provider-specific lazy initialization before creating a session. */ + beforeProviderSession?: (providerId: string) => Promise | void; } const MAX_CLIENT_MAX_TURNS = 99; const MAX_CLIENT_BUDGET_USD = 5; +export function createBestEffortOnce( + initialize: () => Promise, +): () => Promise { + let result: Promise | null = null; + return () => { + result ??= initialize().catch(() => {}); + return result; + }; +} + function clampPositiveInteger(value: unknown, max: number): number | undefined { if (typeof value !== "number" || !Number.isFinite(value)) return undefined; return Math.max(1, Math.min(max, Math.floor(value))); @@ -113,11 +125,28 @@ function clampPositiveNumber(value: unknown, max: number): number | undefined { * ``` */ export function createAIEndpoints(deps: AIEndpointDeps) { - const { registry, sessionManager, getCwd, beforeCapabilities } = deps; + const { + registry, + sessionManager, + getCwd, + beforeCapabilities, + beforeProviderSession, + } = deps; return { - "/api/ai/capabilities": async (_req: Request) => { + "/api/ai/capabilities": async (req: Request) => { await beforeCapabilities?.(); + // Explicit provider activation (?activate=): run the same + // deferred initializer the session path uses, then report the refreshed + // metadata — so the client's model picker can move past a provider's + // static fallback without creating a session. A plain capabilities + // probe must never activate anything: the editor calls it automatically + // on load, and activating there would reintroduce the eager launch this + // deferral exists to prevent. + const activateId = new URL(req.url).searchParams.get("activate"); + if (activateId && registry.get(activateId)) { + await beforeProviderSession?.(activateId); + } const defaultEntry = registry.getDefault(); const providerDetails = registry.list().map(id => { const p = registry.get(id)!; @@ -151,9 +180,10 @@ export function createAIEndpoints(deps: AIEndpointDeps) { } // Resolve provider: by ID, or default - const provider = providerId - ? registry.get(providerId) - : registry.getDefault()?.provider; + const providerEntry = providerId + ? { id: providerId, provider: registry.get(providerId) } + : registry.getDefault(); + const provider = providerEntry?.provider; if (!provider) { return Response.json( @@ -163,12 +193,23 @@ export function createAIEndpoints(deps: AIEndpointDeps) { } try { + await beforeProviderSession?.(providerEntry.id); + // Resolve the model against the post-activation list: a requested + // model the (possibly refreshed) provider still offers is honored, + // anything else — including a stale pre-discovery fallback id — snaps + // to the provider's current default. Providers that report no models + // pass the request through verbatim. + const models = provider.models ?? []; + const effectiveModel = + model && models.some((candidate) => candidate.id === model) + ? model + : models.find((candidate) => candidate.default)?.id ?? models[0]?.id ?? model; const boundedMaxTurns = clampPositiveInteger(maxTurns, MAX_CLIENT_MAX_TURNS); const boundedMaxBudgetUsd = clampPositiveNumber(maxBudgetUsd, MAX_CLIENT_BUDGET_USD); const options: CreateSessionOptions = { context, cwd: getCwd?.(), - model, + model: effectiveModel, ...(boundedMaxTurns !== undefined && { maxTurns: boundedMaxTurns }), ...(boundedMaxBudgetUsd !== undefined && { maxBudgetUsd: boundedMaxBudgetUsd }), reasoningEffort, diff --git a/packages/ai/index.ts b/packages/ai/index.ts index a2e172888..18e37b2d3 100644 --- a/packages/ai/index.ts +++ b/packages/ai/index.ts @@ -95,7 +95,11 @@ export { SessionManager } from "./session-manager.ts"; export type { SessionEntry, SessionManagerOptions } from "./session-manager.ts"; // HTTP endpoints -export { createAIEndpoints, isAIEndpointPath } from "./endpoints.ts"; +export { + createAIEndpoints, + createBestEffortOnce, + isAIEndpointPath, +} from "./endpoints.ts"; export type { AIEndpoints, AIEndpointDeps, diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 073374ebe..4d8092113 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -39,6 +39,7 @@ import { getAgentSwitchSettings, getEffectiveAgentName } from '@plannotator/ui/u import { getPlanSaveSettings } from '@plannotator/ui/utils/planSave'; import { type AIProviderOption } from '@plannotator/ui/utils/aiProvider'; import { useAIProviderConfig } from '@plannotator/ui/hooks/useAIProviderConfig'; +import { useAIProviderActivation } from '@plannotator/ui/hooks/useAIProviderActivation'; import { markPlanAIAnnouncementSeen, needsPlanAIAnnouncement } from '@plannotator/ui/utils/planAIAnnouncement'; import { markLookAndFeelAnnouncementSeen, needsLookAndFeelAnnouncement } from '@plannotator/ui/utils/lookAndFeelAnnouncement'; import { markVimModeAnnouncementSeen, needsVimModeAnnouncement } from '@plannotator/ui/utils/vimModeAnnouncement'; @@ -476,6 +477,16 @@ const App: React.FC = () => { available: aiAvailable, origin, }); + // Explicit provider activation: runs deferred (Codex) model discovery on a + // user gesture and merges the refreshed metadata, so the model picker and + // reasoning-effort control populate past the static fallback. Never called + // on load — that would reintroduce the eager `codex app-server` spawn. + const activateAIProvider = useAIProviderActivation({ + onCapabilities: (providers, defaultProvider) => { + setAiProviders(providers); + setAiDefaultProvider(defaultProvider); + }, + }); const [showPlanAIAnnouncement, setShowPlanAIAnnouncement] = useState(needsPlanAIAnnouncement); const [showLookAndFeelAnnouncement, setShowLookAndFeelAnnouncement] = useState(needsLookAndFeelAnnouncement); const [showVimModeAnnouncement, setShowVimModeAnnouncement] = useState(needsVimModeAnnouncement); @@ -3421,9 +3432,20 @@ const App: React.FC = () => { // per-model reasoning effort); the app only composes the session reset (the // hook can't own it — see the cycle note in useAIProviderConfig). const handleAIConfigChange = useCallback((config: { providerId?: string | null; model?: string | null; reasoningEffort?: string | null }) => { + // Switching the picker to a provider is an explicit gesture — activate it + // so its deferred model discovery (Codex) refreshes the advertised list. + if (config.providerId) activateAIProvider(config.providerId); applyConfigChange(config); resetAISession(); - }, [applyConfigChange, resetAISession]); + }, [activateAIProvider, applyConfigChange, resetAISession]); + + // Opening the Ask AI surface with a provider selected is the other explicit + // gesture that should surface the provider's real model list. + const aiSurfaceOpen = isPanelOpen && rightSidebarTab === 'ai'; + useEffect(() => { + if (!aiAvailable || !aiSurfaceOpen) return; + activateAIProvider(aiConfig.providerId); + }, [aiAvailable, aiSurfaceOpen, aiConfig.providerId, activateAIProvider]); const openAIChat = useCallback(() => { if (wideModeType !== null) { diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index e17c25b42..fbcb41805 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -19,6 +19,7 @@ import { configStore, useConfigValue, setReviewPanelView } from '@plannotator/ui import { loadDiffFont } from '@plannotator/ui/utils/diffFonts'; import { getAgentSwitchSettings, getEffectiveAgentName } from '@plannotator/ui/utils/agentSwitch'; import { useAIProviderConfig } from '@plannotator/ui/hooks/useAIProviderConfig'; +import { useAIProviderActivation } from '@plannotator/ui/hooks/useAIProviderActivation'; import { LookAndFeelAnnouncementDialog } from '@plannotator/ui/components/LookAndFeelAnnouncementDialog'; import { markLookAndFeelAnnouncementSeen, @@ -609,6 +610,16 @@ const ReviewApp: React.FC = () => { available: aiAvailable, origin, }); + // Explicit provider activation: runs deferred (Codex) model discovery on a + // user gesture and merges the refreshed metadata, so the model picker and + // reasoning-effort control populate past the static fallback. Never called + // on load — that would reintroduce the eager `codex app-server` spawn. + const activateAIProvider = useAIProviderActivation({ + onCapabilities: (providers, defaultProvider) => { + setAiProviders(providers); + setAiDefaultProvider(defaultProvider); + }, + }); // The 0.20.0 release / look-and-feel announcement also runs in code review. // Seen-state is a shared cookie (host-scoped), so dismissing it in either app // suppresses it in the other — it appears once across both. @@ -730,9 +741,20 @@ const ReviewApp: React.FC = () => { // app only composes the session reset (the hook can't own it — see the cycle // note in useAIProviderConfig). const handleAIConfigChange = useCallback((config: { providerId?: string | null; model?: string | null; reasoningEffort?: string | null }) => { + // Switching the picker to a provider is an explicit gesture — activate it + // so its deferred model discovery (Codex) refreshes the advertised list. + if (config.providerId) activateAIProvider(config.providerId); applyConfigChange(config); resetAISession(); - }, [applyConfigChange, resetAISession]); + }, [activateAIProvider, applyConfigChange, resetAISession]); + + // Opening the Ask AI sidebar tab with a provider selected is the other + // explicit gesture that should surface the provider's real model list. + const aiSurfaceOpen = reviewSidebar.isOpen && reviewSidebar.activeTab === 'ai'; + useEffect(() => { + if (!aiAvailable || !aiSurfaceOpen) return; + activateAIProvider(aiConfig.providerId); + }, [aiAvailable, aiSurfaceOpen, aiConfig.providerId, activateAIProvider]); // File-aware Ask AI: the all-files surface resolves the owning file itself // (its toolbar selection lives in a file the single-file panel may never diff --git a/packages/server/ai-runtime.test.ts b/packages/server/ai-runtime.test.ts new file mode 100644 index 000000000..9c87c1085 --- /dev/null +++ b/packages/server/ai-runtime.test.ts @@ -0,0 +1,150 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { + chmodSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("createAIRuntime Codex discovery", () => { + test("capabilities does not execute Codex and session activation does", async () => { + if (process.platform === "win32") return; + + const dir = mkdtempSync(join(tmpdir(), "plannotator-lazy-codex-")); + tempDirs.push(dir); + const marker = join(dir, "codex-ran"); + const codex = join(dir, "codex"); + writeFileSync(codex, `#!/bin/sh\necho ran > '${marker}'\nexit 1\n`); + chmodSync(codex, 0o755); + + const runner = join(dir, "runner.ts"); + const runtimeUrl = pathToFileURL(join(import.meta.dir, "ai-runtime.ts")).href; + writeFileSync(runner, ` + import { existsSync } from "node:fs"; + import { createAIRuntime } from ${JSON.stringify(runtimeUrl)}; + const runtime = await createAIRuntime({ cwd: ${JSON.stringify(dir)} }); + const capabilities = await runtime.endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities"), + ); + const data = await capabilities.json(); + const afterCapabilities = existsSync(${JSON.stringify(marker)}); + const session = await runtime.endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex-sdk", + }), + }), + ); + console.log(JSON.stringify({ + hasCodex: data.providers.some((provider) => provider.id === "codex-sdk"), + afterCapabilities, + sessionStatus: session.status, + afterSession: existsSync(${JSON.stringify(marker)}), + })); + runtime.dispose(); + `); + + const proc = Bun.spawn([process.execPath, runner], { + cwd: import.meta.dir, + env: { ...process.env, PATH: `${dir}:/usr/bin:/bin` }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + expect(exitCode, stderr).toBe(0); + expect(JSON.parse(stdout.trim())).toEqual({ + hasCodex: true, + afterCapabilities: false, + sessionStatus: 200, + afterSession: true, + }); + }, 15_000); + + test("capabilities?activate= runs discovery once and shares it with the session path", async () => { + if (process.platform === "win32") return; + + const dir = mkdtempSync(join(tmpdir(), "plannotator-activate-codex-")); + tempDirs.push(dir); + const marker = join(dir, "codex-ran"); + const codex = join(dir, "codex"); + writeFileSync(codex, `#!/bin/sh\necho ran >> '${marker}'\nexit 1\n`); + chmodSync(codex, 0o755); + + const runner = join(dir, "runner.ts"); + const runtimeUrl = pathToFileURL(join(import.meta.dir, "ai-runtime.ts")).href; + writeFileSync(runner, ` + import { existsSync, readFileSync } from "node:fs"; + import { createAIRuntime } from ${JSON.stringify(runtimeUrl)}; + const runs = () => existsSync(${JSON.stringify(marker)}) + ? readFileSync(${JSON.stringify(marker)}, "utf8").trim().split("\\n").length + : 0; + const runtime = await createAIRuntime({ cwd: ${JSON.stringify(dir)} }); + const probe = await runtime.endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities"), + ); + const runsAfterProbe = runs(); + const activate = await runtime.endpoints["/api/ai/capabilities"]( + new Request("http://localhost/api/ai/capabilities?activate=codex-sdk"), + ); + const runsAfterActivate = runs(); + const session = await runtime.endpoints["/api/ai/session"]( + new Request("http://localhost/api/ai/session", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + context: { mode: "plan-review", plan: { plan: "# Test" } }, + providerId: "codex-sdk", + }), + }), + ); + console.log(JSON.stringify({ + probeStatus: probe.status, + runsAfterProbe, + activateStatus: activate.status, + runsAfterActivate, + sessionStatus: session.status, + runsAfterSession: runs(), + })); + runtime.dispose(); + `); + + const proc = Bun.spawn([process.execPath, runner], { + cwd: import.meta.dir, + env: { ...process.env, PATH: `${dir}:/usr/bin:/bin` }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + expect(exitCode, stderr).toBe(0); + expect(JSON.parse(stdout.trim())).toEqual({ + probeStatus: 200, + runsAfterProbe: 0, + activateStatus: 200, + runsAfterActivate: 1, + sessionStatus: 200, + runsAfterSession: 1, + }); + }, 15_000); +}); diff --git a/packages/server/ai-runtime.ts b/packages/server/ai-runtime.ts index a94203696..7432a6f70 100644 --- a/packages/server/ai-runtime.ts +++ b/packages/server/ai-runtime.ts @@ -1,5 +1,6 @@ import { createAIEndpoints, + createBestEffortOnce, createProvider, ProviderRegistry, SessionManager, @@ -25,6 +26,7 @@ export async function createAIRuntime(options: CreateAIRuntimeOptions = {}): Pro const registry = new ProviderRegistry(); const sessionManager = new SessionManager(); const modelDiscovery: Promise[] = []; + const providerInitializers = new Map Promise>(); try { await import("@plannotator/ai/providers/claude-agent-sdk"); @@ -48,10 +50,13 @@ export async function createAIRuntime(options: CreateAIRuntimeOptions = {}): Pro cwd, ...(codexPath ? { codexExecutablePath: codexPath } : {}), }); - registry.register(provider); + const providerId = registry.register(provider); if ("fetchModels" in provider) { - modelDiscovery.push( - (provider as { fetchModels: () => Promise }).fetchModels().catch(() => {}), + providerInitializers.set( + providerId, + createBestEffortOnce( + () => (provider as { fetchModels: () => Promise }).fetchModels(), + ), ); } } @@ -102,6 +107,9 @@ export async function createAIRuntime(options: CreateAIRuntimeOptions = {}): Pro beforeCapabilities: async () => { await Promise.allSettled(modelDiscovery); }, + beforeProviderSession: async (providerId) => { + await providerInitializers.get(providerId)?.(); + }, }); return { diff --git a/packages/ui/aiProvider.test.ts b/packages/ui/aiProvider.test.ts index 4ab293619..d0e39ab86 100644 --- a/packages/ui/aiProvider.test.ts +++ b/packages/ui/aiProvider.test.ts @@ -88,6 +88,20 @@ describe('AI provider origin defaults', () => { expect(next.preferredModels['codex-local']).toBe('codex-alt'); }); + it('leaves the saved preferred model untouched when no model is passed', () => { + // A provider switch (or any non-model change) persists without a model: + // a resolver-derived fallback must never overwrite a saved preference + // that the currently-advertised model list happens not to include (e.g. + // before deferred Codex discovery has run). + const next = applyAIProviderSelection( + settings({ preferredModels: { 'codex-local': 'gpt-5.3-codex' } }), + { providerId: 'codex-local', model: null, origin: 'codex' }, + ); + + expect(next.preferredModels['codex-local']).toBe('gpt-5.3-codex'); + expect(next.providerByOrigin.codex).toBe('codex-local'); + }); + it('stores explicit choices as the global fallback for origins without a dedicated provider', () => { const next = applyAIProviderSelection( settings({ providerId: 'claude-local' }), diff --git a/packages/ui/aiProviderConfigPersistence.test.tsx b/packages/ui/aiProviderConfigPersistence.test.tsx new file mode 100644 index 000000000..1608db7b0 --- /dev/null +++ b/packages/ui/aiProviderConfigPersistence.test.tsx @@ -0,0 +1,170 @@ +/** + * Persistence tests for the shared AI provider/model selection hook. + * + * Guards the "saved preference no-clobber" invariant: with Codex model + * discovery deferred until activation, capabilities can advertise only the + * provider's static fallback model. The hook then resolves the *session* + * model to that fallback, but must never write the resolver-derived fallback + * back into the saved per-provider preference — only an explicit user pick + * may change the cookie. + * + * Requires DOM_TESTS=1 (happy-dom preload). Run: + * DOM_TESTS=1 bun test aiProviderConfigPersistence + */ +import { describe, test, expect, beforeEach, afterAll } from 'bun:test'; +import React, { useEffect } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { act } from 'react'; +import { useAIProviderConfig } from './hooks/useAIProviderConfig'; +import { + getAIProviderSettings, + saveAIProviderSettings, + type AIProviderOption, +} from './utils/aiProvider'; +import { setStorageBackend, resetStorageBackend, type StorageBackend } from './utils/storage'; + +const hasDom = typeof document !== 'undefined'; + +// In-memory storage so tests don't depend on happy-dom cookie semantics. +const memory = new Map(); +const memoryBackend: StorageBackend = { + getItem: (key) => memory.get(key) ?? null, + setItem: (key, value) => void memory.set(key, value), + removeItem: (key) => void memory.delete(key), +}; + +type HookResult = ReturnType; + +function Harness(props: { + providers: AIProviderOption[]; + onRender: (result: HookResult) => void; +}) { + const result = useAIProviderConfig({ + providers: props.providers, + defaultProvider: 'codex-local', + available: true, + origin: null, + }); + useEffect(() => { + props.onRender(result); + }); + return null; +} + +// Capabilities before deferred discovery: only the static fallback model. +const fallbackOnlyProviders: AIProviderOption[] = [ + { + id: 'codex-local', + name: 'codex-sdk', + models: [{ id: 'gpt-5.6-sol', label: 'GPT Fallback', default: true }], + }, +]; + +let root: Root | null = null; +let container: HTMLElement | null = null; + +async function mountHarness(providers: AIProviderOption[]): Promise<() => HookResult> { + let latest: HookResult | null = null; + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( { latest = r; }} />); + }); + return () => { + if (!latest) throw new Error('Harness did not render'); + return latest; + }; +} + +async function unmountHarness() { + if (root) { + await act(async () => root!.unmount()); + root = null; + } + container?.remove(); + container = null; +} + +beforeEach(async () => { + await unmountHarness(); + memory.clear(); + if (hasDom) setStorageBackend(memoryBackend); +}); + +afterAll(async () => { + await unmountHarness(); + resetStorageBackend(); +}); + +describe('useAIProviderConfig persistence', () => { + test.skipIf(!hasDom)('a resolver-derived fallback model is not persisted over the saved preference', async () => { + // The user's real Codex model preference, saved from a prior session — + // not present in the pre-activation fallback list. + saveAIProviderSettings({ + providerId: 'codex-local', + preferredModels: { 'codex-local': 'gpt-5.3-codex' }, + providerByOrigin: {}, + }); + + const getResult = await mountHarness(fallbackOnlyProviders); + + // Session-facing state falls back to the advertised model... + expect(getResult().aiConfig.providerId).toBe('codex-local'); + expect(getResult().aiConfig.model).toBe('gpt-5.6-sol'); + + // ...and a non-model change (reasoning effort) persists without touching + // the saved model preference. + await act(async () => { + getResult().applyConfigChange({ reasoningEffort: 'high' }); + }); + + expect(getAIProviderSettings().preferredModels['codex-local']).toBe('gpt-5.3-codex'); + }); + + test.skipIf(!hasDom)('a provider switch does not overwrite the saved model preference either', async () => { + saveAIProviderSettings({ + providerId: 'codex-local', + preferredModels: { 'codex-local': 'gpt-5.3-codex' }, + providerByOrigin: {}, + }); + + const providers: AIProviderOption[] = [ + ...fallbackOnlyProviders, + { + id: 'claude-local', + name: 'claude-agent-sdk', + models: [{ id: 'claude-default', label: 'Claude Default', default: true }], + }, + ]; + const getResult = await mountHarness(providers); + + // Switch away and back: both are provider-only gestures. + await act(async () => { + getResult().applyConfigChange({ providerId: 'claude-local' }); + }); + await act(async () => { + getResult().applyConfigChange({ providerId: 'codex-local' }); + }); + + const persisted = getAIProviderSettings(); + expect(persisted.preferredModels['codex-local']).toBe('gpt-5.3-codex'); + expect(persisted.providerId).toBe('codex-local'); + }); + + test.skipIf(!hasDom)('an explicit model pick is persisted', async () => { + saveAIProviderSettings({ + providerId: 'codex-local', + preferredModels: { 'codex-local': 'gpt-5.3-codex' }, + providerByOrigin: {}, + }); + + const getResult = await mountHarness(fallbackOnlyProviders); + + await act(async () => { + getResult().applyConfigChange({ model: 'gpt-5.6-sol' }); + }); + + expect(getAIProviderSettings().preferredModels['codex-local']).toBe('gpt-5.6-sol'); + }); +}); diff --git a/packages/ui/components/AISettingsTab.tsx b/packages/ui/components/AISettingsTab.tsx index 5010471a5..f5270cfbf 100644 --- a/packages/ui/components/AISettingsTab.tsx +++ b/packages/ui/components/AISettingsTab.tsx @@ -2,7 +2,6 @@ import type React from 'react'; import { getProviderMeta } from './ProviderIcons'; import { getAIProviderSettings, - resolveAIModelForProvider, resolveAIProviderSelection, saveAIProviderSelection, savePreferredModel, @@ -37,9 +36,11 @@ export const AISettingsTab: React.FC = ({ const handleSelectProvider = (providerId: string) => { onProviderChange(providerId); - const provider = providers.find(p => p.id === providerId) ?? null; - const model = resolveAIModelForProvider(provider, getAIProviderSettings().preferredModels); - saveAIProviderSelection({ providerId, model, origin }); + // Persist only the provider choice. Writing the resolved model here would + // clobber a saved preference the currently-advertised model list doesn't + // include (e.g. before deferred Codex discovery has run) with the static + // fallback id — the saved model only changes when the user picks one. + saveAIProviderSelection({ providerId, model: null, origin }); }; const handleModelChange = (providerId: string, modelId: string) => { diff --git a/packages/ui/hooks/useAIProviderActivation.ts b/packages/ui/hooks/useAIProviderActivation.ts new file mode 100644 index 000000000..97ee81290 --- /dev/null +++ b/packages/ui/hooks/useAIProviderActivation.ts @@ -0,0 +1,47 @@ +import { useCallback, useRef } from 'react'; +import type { AIProviderOption } from '../utils/aiProvider'; + +/** + * Explicit AI provider activation for lazily-initialized providers. + * + * Codex model discovery is deferred until a session starts (it spawns a + * `codex app-server` process — see #1144), so /api/ai/capabilities advertises + * static fallback metadata until then. This hook lets the apps run that same + * deferred initializer on an explicit user gesture — opening the Ask AI + * surface, or switching the provider picker — via + * `GET /api/ai/capabilities?activate=`, then merge the refreshed + * provider metadata (real model list + reasoning efforts) back into app state. + * + * Single-flight per provider id: repeated gestures don't re-fetch (activation + * is idempotent server-side anyway), but a failed fetch un-latches so a later + * gesture can retry. Providers without deferred initialization are a server-side + * no-op, so callers don't need to know which providers are lazy. + */ +export interface ActivatedAIProvider extends AIProviderOption { + capabilities: Record; +} + +export function useAIProviderActivation(options: { + onCapabilities: (providers: ActivatedAIProvider[], defaultProvider: string | null) => void; +}) { + const activatedRef = useRef>(new Set()); + const onCapabilitiesRef = useRef(options.onCapabilities); + onCapabilitiesRef.current = options.onCapabilities; + + return useCallback((providerId: string | null | undefined) => { + if (!providerId || activatedRef.current.has(providerId)) return; + activatedRef.current.add(providerId); + fetch(`/api/ai/capabilities?activate=${encodeURIComponent(providerId)}`) + .then(res => (res.ok ? res.json() : null)) + .then(data => { + if (!data?.available) { + activatedRef.current.delete(providerId); + return; + } + onCapabilitiesRef.current(data.providers ?? [], data.defaultProvider ?? null); + }) + .catch(() => { + activatedRef.current.delete(providerId); + }); + }, []); +} diff --git a/packages/ui/hooks/useAIProviderConfig.ts b/packages/ui/hooks/useAIProviderConfig.ts index 4ad3243d4..addf8c9f2 100644 --- a/packages/ui/hooks/useAIProviderConfig.ts +++ b/packages/ui/hooks/useAIProviderConfig.ts @@ -99,9 +99,13 @@ export function useAIProviderConfig({ } const reasoningEffort = model ? (reasoningEffortByModel[model] ?? null) : null; const next = { ...prev, providerId, model, reasoningEffort, reasoningEffortByModel }; + // Only an explicit model pick is persisted. A resolver-derived model + // (e.g. the static fallback a lazily-discovered provider advertises + // before activation) must never overwrite the user's saved preference + // — the session request may fall back, the cookie may not. saveAIProviderSelection({ providerId: next.providerId, - model: next.model, + model: config.model !== undefined ? next.model : null, origin, settings: saved, });