speed up agent switching with provider caching and spinning-up indicator#8612
speed up agent switching with provider caching and spinning-up indicator#8612
Conversation
Cache external ACP providers (Claude Code, Codex) in memory so switching back to a recently-used agent reuses the warm subprocess, persist model lists to disk for instant picker fill on cold starts, pre-warm the last-used provider on server boot, and surface a dedicated "Spinning up …" state in the chat UI bracketed around the prepare call. Also de-dupes concurrent prepareSession calls so a background prepare from the agent picker and a foreground one from sendMessage join the same in-flight promise instead of racing duplicate newSession round-trips. Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbb4f53d63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let cache = provider_cache.lock().await; | ||
| cache.get(&provider_name).map(Arc::clone) | ||
| }; | ||
| match cached { | ||
| Some(p) => p, |
There was a problem hiding this comment.
This cache hit path returns the same Arc<dyn Provider> for every session that selects the same provider name, which is unsafe for ACP providers because AcpProvider owns a single ACP session created at connect time and stream() always uses that stored session id. In practice, opening a second chat on the same ACP provider can route prompts/cancels through the first chat’s ACP session, causing cross-session context/tool-state leakage; cache entries need to be scoped by session (or cache only startup artifacts, not the full provider instance).
Useful? React with 👍 / 👎.
| prepared.set(key, entry); | ||
| prepared.set(sessionId, entry); | ||
| gooseToLocal.set(gooseSessionId, sessionId); | ||
|
|
||
| await acpApi.setProvider(gooseSessionId, providerId); |
There was a problem hiding this comment.
The new ordering marks the session as prepared before acpApi.setProvider(...) completes, so if setProvider fails (e.g., backend still spinning up or transient ACP error), the failed session remains cached as prepared with the requested provider id. Subsequent prepareSession calls then skip the provider update path and return this stale entry, which can leave later sends bound to the wrong/uninitialized backend session instead of retrying provider setup.
Useful? React with 👍 / 👎.
1 participant
Category: improvement
User Impact: Switching to an external AI agent like Claude Code or Codex feels much faster — the model picker fills instantly from cache, the previously-used agent is pre-warmed at server boot, and a clear "Spinning up [Agent]…" indicator appears in chat whenever the backend isn't ready yet.
Problem: Switching to an ACP-based agent took ~25 seconds with almost no feedback. The model picker hung on "Loading…" the entire time, and if you sent a message during that window the chat showed "Responding…" — implying the model was already working — even though the backend was still booting the Node subprocess and doing handshake/entitlements calls.
Solution: Three layers. Backend caches the long-lived
AcpProvider(the Node subprocess + handshake) in memory keyed by provider name, persists model lists to disk so the picker has something to render on cold starts, and pre-warms the last-used provider in a background task at server boot. Frontend adds aspinning_upchat state, computes initial loading state up front so it doesn't flash "Thinking…" before transitioning, and de-duplicates concurrent prepare calls so a background prepare from the agent picker and a foreground prepare from sendMessage join the same in-flight promise instead of racing duplicatenewSessionround-trips.File changes
crates/goose-acp/src/server.rs
Added in-memory
provider_cache(Arc<Mutex<HashMap<String, Arc<dyn Provider>>>>) and amodel_cachefield onGooseAcpAgent. Newget_or_create_providerhelper checks the cache before constructing a provider.set_session_config_optionnow sends a cachedConfigOptionUpdatenotification immediately so the picker fills before the real backend confirms.update_providerwrites fresh model options to the disk cache after a successful create.crates/goose-acp/src/model_cache.rs (new)
Disk-persistence layer for model lists. Stores per-provider
SessionConfigOptionsnapshots withcached_at/last_used_attimestamps. Atomic writes via.tmp+fs::rename. Used to (a) seed the picker on cold starts and (b) decide which provider to pre-warm.crates/goose-acp/src/server_factory.rs
Spawns a
tokiotask on server boot that looks up the last-used provider from the model cache and primes the in-memory provider cache by callingget_or_create_provider. Best-effort — failures are silently ignored since this is a warm-up.crates/goose-acp/src/lib.rs / Cargo.toml
Exports the new
model_cachemodule and addschronowithserdefeatures for the timestamps.crates/goose/src/acp/provider.rs
Minor: removed verbose tracing-target debug logs that were used during this investigation.
ui/goose2/src/shared/api/acpSessionTracker.ts
Added an
inFlight: Map<string, Promise<string>>so concurrentprepareSessioncalls join the same promise. ExposesisPrepareInFlight()/isSessionPrepared()souseChatcan decide when to show the spinning-up indicator.ui/goose2/src/shared/api/acp.ts
Re-exports the two new readiness checks as
acpIsPrepareInFlight/acpIsSessionPrepared.ui/goose2/src/features/chat/hooks/useChat.ts
Computes
needsPrepare/prepareInFlightup front and sets the initial chat state tospinning_up(instead ofthinking) when the backend session isn't ready. Always awaitsacpPrepareSessionif a background prepare is in flight, sosendMessagedoesn't fall through to "Responding…" while the backend is actually still booting.ui/goose2/src/features/chat/ui/LoadingGoose.tsx
Adds
spinning_uptoLoadingChatStateand aproviderNameprop. Renders "Spinning up [Agent]…" via a new translation key, falling back to a generic message when no provider name is provided.ui/goose2/src/features/chat/ui/ChatView.tsx
Threads the selected provider's display label into
LoadingGooseand includesspinning_upin the indicator visibility condition.ui/goose2/src/features/chat/lib/sessionActivity.ts
Includes
spinning_upinisSessionRunningso the sidebar / activity affordances treat it like other in-progress states.ui/goose2/src/shared/i18n/locales/en/chat.json
Adds
loading.spinningUpandloading.spinningUpFallbackstrings.ui/goose2/src/shared/types/chat.ts
Adds
spinning_upto theChatStateunion.ui/goose2/src/shared/api/acpApi.ts + acpNotificationHandler.ts
Removed
[models-debug]console instrumentation that was only added to investigate the original issue.ui/goose2/src/features/chat/hooks/tests/*.ts + LoadingGoose.test.tsx
New test for the
spinning_up→streamingtransition during prepare. Updated mocks to include the two new exports from@/shared/api/acp. New rendering tests for thespinning_upstate with and without a provider name.Reproduction steps
cargo build -p goose-clithenGOOSE_BIN="$PWD/target/debug/goose" just goose2 dev.Notes for reviewers
AGENTS.md, backend changes needcargo fmt,cargo clippy --all-targets -- -D warnings, andjust generate-openapi. I have not run these in this environment — please run locally before merging.