fix(ai): defer Codex model discovery until a Codex session starts - #1145
Conversation
Opening any plan, annotate, or code review builds the shared AI runtime, and the runtime called every provider's fetchModels() while constructing itself. For the Codex provider that starts a throwaway `codex app-server` process, so a review launched Codex even when the user never opened Ask AI. On macOS with a quarantined Homebrew Codex payload this surfaces as a Gatekeeper confirmation dialog in front of the review the user actually asked for. Codex discovery now runs on explicit activation instead. The provider is still registered and still advertised through /api/ai/capabilities using its static fallback model metadata, so nothing about discovery is user-visible until a session is created for it. createBestEffortOnce() memoizes the discovery call so it runs at most once per runtime and a failure leaves the static fallback in place rather than blocking session creation. /api/ai/session gained a beforeProviderSession hook, invoked for the resolved provider id before the session is created. /api/ai/capabilities deliberately does not invoke it: the editor probes capabilities automatically on load, so activating a provider there would reintroduce the same eager launch through a different path. Because discovery can replace the provider's model list, the session handler compares the requested model against the pre-activation default. A caller that sent no model, or sent the pre-activation default, gets the post-activation default; an explicitly chosen model is always honored. Without this a first Codex session would pin the static fallback model that discovery just replaced. Both runtimes are changed the same way, and the other providers keep their existing eager discovery, which beforeCapabilities still awaits. Tests cover the regression with a fake Codex executable rather than a real one: runtime construction and a capabilities probe must not invoke discovery, the first Codex session must, the second must not, and a failing discovery must still create a session on the fallback metadata.
Follow-up to the deferred Codex discovery change, addressing the review findings on backnotprop#1145 while keeping the deferral intact: constructing the runtime and probing /api/ai/capabilities still never spawns `codex app-server`. - /api/ai/capabilities now accepts ?activate=<providerId>: it runs the same createBestEffortOnce initializer the session path uses (no second discovery path) and responds with the refreshed capabilities payload. A plain capabilities probe still activates nothing. Both runtimes get this through the shared endpoint (packages/ai is vendored into the Pi server by vendor.sh). - The apps activate the selected provider on explicit user gestures -- opening the Ask AI surface or switching the provider picker -- via the new useAIProviderActivation hook (single-flight per provider id), then merge the refreshed models and reasoning efforts into state so the model picker and per-model reasoning-effort selector populate past the static fallback. (review finding 1) - A resolver-derived model is no longer persisted: useAIProviderConfig and AISettingsTab write the per-provider model preference only on an explicit user pick, so a saved Codex model the pre-activation fallback list doesn't include survives instead of being clobbered by the fallback id. The session request still falls back; the cookie doesn't. (review finding 2) - The session handler resolves the requested model by membership in the post-activation model list instead of comparing against the pre-activation default, so sessions after the first can no longer pin a stale fallback id that discovery already replaced. (review finding 3) Tests: activation endpoint behavior (shared endpoints plus both runtimes against a hermetic fake codex on PATH), effectiveModel membership resolution, and saved-preference no-clobber (DOM tests for useAIProviderConfig persistence). Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
|
Reviewed this in depth. The deferral itself is exactly right and we verified the no-spawn boundary holds. It did leave the client stuck on the static fallback model list though: the picker froze, the reasoning effort control disappeared, and the fallback id could overwrite a saved model preference. We pushed a fix-up commit to your branch (d0058f9) rather than bounce it back: an explicit activate call on the capabilities endpoint (runs your same createBestEffortOnce initializer, only on user gesture), no-clobber semantics for saved preferences, and a membership check for the session model. Your commits are untouched and your tests all still pass. Take a look, and if it reads right to you this is ready to merge. |
|
Thanks for the fix-up addition. Totally cool to merge, go for it. |
Summary
Constructing the shared AI runtime called every provider's
fetchModels(), and the Codex provider's implementation spawnscodex app-server. Opening any plan, annotate, or code review therefore executed Codex even when the user never opened Ask AI. Codex discovery now runs on explicit activation instead.The eager launch dates from #971, which moved
codex-sdkontocodex app-server. Before that the provider had nofetchModels(), so runtime construction started no process.Solves #1144, which is quite annoying to receive the popup about "codex" on every plannotator interaction.
Why
The spawned process is short lived and cleaned up, so usually nothing is visible. It becomes visible when starting the binary itself prompts: on macOS with a quarantined Homebrew payload, a Gatekeeper confirmation dialog in front of the review the user asked for. Invisible or not, it is a subprocess for a provider nobody selected, started by opening a review. The editor's automatic
/api/ai/capabilitiesprobe had the same effect, since it awaited the same discovery.Changes
createBestEffortOnce()inpackages/ai/endpoints.ts, memoizing an initializer so it runs at most once per runtime and a failure is swallowed, leaving the provider's static fallback metadata in place instead of blocking session creation.beforeProviderSession?(providerId)toAIEndpointDeps, invoked by/api/ai/sessionfor the resolved provider before the session is created./api/ai/capabilitiesdeliberately does not invoke it: the editor probes capabilities automatically on load, so activating there would reintroduce the same eager launch through another path.fetchModels()during construction. Every other provider keeps its existing eager discovery, whichbeforeCapabilitiesstill awaits.Codex stays listed by
/api/ai/capabilitiesthroughout, on its fallback model metadata until the first Codex session.Validation
bun run typecheckacross core, shared, AI, server, UI, strict consumer, and Pi: passed.packages/ai/ai.test.ts: 108 passed.packages/server/ai-runtime.test.tsandapps/pi-extension/server/ai-runtime.test.ts: 1 passed each. Each spawns a child process with a fakecodexonPATHand asserts{ hasCodex: true, afterCapabilities: false, sessionStatus: 200, afterSession: true }, so Codex is still advertised, not executed by the capabilities probe, and executed by the first Codex session.mainwith its ownbun install, so no local branch or build artifact is involved. A fakecodexrecords its invocation, and the real Homebrew binary (codex-cli 0.144.6) was confirmed separately with a 60ms process poll catchingcodex app-serverduring runtime construction. macOS, Bun 1.3.14:codexinvoked duringcreateAIRuntime()/api/ai/capabilitiescodex-sdksessionmaina54b46bbapp-server)Not covered: real Codex binaries, the Gatekeeper dialog itself, reasoning-effort metadata refresh in the UI after activation, and startup-latency measurement.
Real e2e testing
I am using plannotator on a daily basis. I use a custom local-only integration branch including this branch/PR and also #1143, to fully review/check/test it in real scenarios. So far so good. Please, feel free to ask me about further testing, tweaks or refactors (or modify directly the branch yourself, it's good!). Happy to help, as you know me!