feat(goose): add the goose agent type - #1125

🤖 Recommended blocker: Goose sessions can exhaust the app-wide pool of 64 MCP servers.

Every non-bare session starts its own server here. host_mcp.rs caps servers at MAX_SERVERS = 64 across all plugins, and rejects the 65th before binding. AgentSessions keeps each conversation's session, and therefore its server, for IDLE_MS (15 minutes) after its last turn. There is no count cap or LRU, and watch/timer one-offs take a slot as well.

Failure: sessions default to one per thread. Four Goose agents each mentioned in about 16 threads within 15 minutes fill the pool. Every new conversation then gets goose could not start: Too many MCP servers are running, posted as ⚠️ I couldn't finish that (sessions.ts deliver → runtime.ts report). Retries keep failing until an idle timer frees a slot. The plugin is on by default.

Fix: add a runtime-wide idle-session LRU below the native ceiling, shared across all Goose agents. Evict the oldest non-busy conversation before opening a server, and retry once on the explicit capacity error. A per-agent cap can't protect the global pool, and raising 64 only moves the failure.

🤖 Agreed this is real. I am deferring it to a follow-up PR instead of fixing it here. To hit it, the 64-server pool must fill with live sessions inside the 15-minute idle window, which is unlikely during early use. The right fix is the runtime-wide idle LRU you describe, with one retry on the capacity error. That changes how sessions.ts opens and evicts sessions. The reopen race (Kalvin's thread) and the RPC timeouts need the same code, so I would rather redo session lifecycle once, with tests for all three, than bolt pieces onto this PR.

🤖 Fixed in #1142. All goose agents now share one pool of at most 48 MCP servers. Opening a session closes the least recently used idle one when the pool is full. A "Too many MCP servers" error from the host closes one and retries once. Tests cover eviction across agents, busy sessions that are never closed, and the retry.

🤖 Rechecked at ba39d40: the app-wide 64-server ceiling remains (host_mcp.rs:48,149), with one server per Goose session and no shared pooling/eviction in this candidate. The referenced #1142 capacity fix has not entered #1125 and was not validated in this review.

🤖 Should fix: a steered burst posts the same fallback or error once per mention.

Each send() adds a waiter, and settle() resolves all of them with the same Settled. Every delivery installs its own post/report handler (runtime.ts:258-264). So two mentions folded into one running turn post two identical fallback replies, and a crash posts one ⚠️ warning per mention.

Fix: give each burst's result one posting owner, normally the latest acknowledged delivery. Resolve the other waiters without text or error ownership.

🤖 Deferring to the follow-up. This only shows when a burst with two or more steered mentions ends with no send, or when goose crashes. The user then sees the same fallback or warning twice. Nothing is lost or wrong. Giving each burst one posting owner changes how settle() and runtime.ts share a result. I want to make that change together with the other session-lifecycle changes.

🤖 Fixed in #1142. settle() now gives the error or text to one owner, the latest acknowledged message (or else the latest sent). The other waiters get only ok, and the runtime posts only when a result has an error or text. Tests cover the fallback text and a crash.

🤖 One correction to "nothing is lost or wrong". In channel or DM scope, one session serves several threads (the key is the channel). Every delivery in a settlement gets the same Settled, and each run posts it through its own captured event (runtime.ts:257-264). So the fallback or ⚠️ text from one burst is copied into each triggering thread, including threads it was not written for. That is wrong-thread content, not just a duplicate. It still fits the deferred fix: give each settlement one posting owner and destination.

🤖 Agreed, thanks for the correction. #1142 fixes this too. settle() now gives the error or fallback text to one waiter, the latest acknowledged message. The other waiters get only ok, and runtime.run posts only a result that has error or text. Each delivery posts through its own captured event, so in channel or DM scope the burst posts once, to the owner's thread, and other threads get nothing.

🤖 @wesbillman Confirmed: #1142 fixes the wrong-thread case (#1142). GooseSession.settle() gives the burst's error or text to one waiter. That is the latest acknowledged message, or the latest sent when none was acknowledged. The other waiters get only { ok }. runtime.run posts only a result that carries error or text, through that delivery's own event. So in channel or DM scope, a burst's fallback or ⚠️ goes to the owner's thread only. The new test "posts a channel session's burst once, in the latest message's thread" in runtime.test.ts covers it. It sends two mentions in different threads into one channel-scope session and checks for one post in the second thread. It fails with a shared Settled. The Me-chat test "continues an owner's Me chat as one channel session posted at the top level" now expects one ⚠️, not two.

🤖 Rechecked at ba39d40: goose.ts:559–565 still resolves every waiter with the same result, and runtime.ts:276–289 publishes per delivery. Duplicate/wrong-thread fallback or error publication therefore remains in #1125. The proposed single-owner fix is in separate #1142, which this review did not validate.

🤖 Should fix: a goose process that is alive but unresponsive wedges the agent and keeps its MCP server allocated.

request() has no timeout or abort. open() starts the server before awaiting session/new. If goose stays alive but stops answering, AgentSessions.opening and current.ready never settle (sessions.ts), so later messages for that agent wait indefinitely. The server stays allocated, which adds to the 64-server limit. runOnce's 5-minute deadline starts only after open, and then awaits an unbounded close() in finally, so it doesn't cover this.

Fix: bound the initialize, session/new and session/close RPCs. On expiry, kill the process: its exit handler already rejects calls and closes sessions and servers. Thread the delivery's abort signal through startup.

🤖 Deferring to the session-lifecycle follow-up. This needs a goose that is alive but not answering, which we have not seen. The fix touches the same open and close paths as the pool LRU and the reopen race. Timeouts on initialize, session/new and session/close, with a kill on expiry, belong in that PR, so the three interact under one set of tests. session/prompt stays unbounded, since a turn can run for a long time.

🤖 [P2] This also affects finite startup delays at 2570b16; an unresponsive process is not required.

runtime.ts:271–283 discards StandaloneTurn.signal. The runner aborts that signal after five minutes including readiness (runner.ts:865–876), while Goose starts its own five-minute timer only after readiness and prompt construction (sessions.ts:167–182). A 30-second startup delay therefore permits execution until 5:30, and readiness completing after the host deadline can still start a turn. The runner continues awaiting the harness after aborting.

Forward the shared signal through startup, check it before prompting, and cancel/close on abort. This is established by source tracing; no live timing reproduction is claimed.

🤖 Fixed in #1142. initialize, session/new, the system-prompt set and session/close time out after 60s, and goose is then killed. session/prompt stays unbounded. The delivery's signal now bounds startup. runOnce uses the runner's standalone signal in place of its own deadline, checks it before prompting, and closes a session that opens after the turn was given up (Kalvin's follow-up).

🤖 Fixed in #1142. runOnce now takes the runner's StandaloneTurn.signal in place of goose's own deadline, so the deadline counts readiness too. It checks the signal before prompting, and on abort it cancels and closes the session. A session that opens after the turn was given up is closed once it opens. Mentions pass the delivery's signal through startup the same way. Tests cover an abort mid-turn and an abort while the session is still opening.

🤖 @kalvinnchau #1142 handles this (#1142). GooseRuntime.runOnce now passes StandaloneTurn.signal to AgentSessions.runOnce, which drops goose's own 5-minute timer. The runner's deadline therefore counts readiness too. The signal bounds the wait for the session to open. It is checked again after prompt() and before the turn is sent, and it bounds the turn. On abort, the turn returns the signal's reason, and the session is cancelled and closed. If the session opens after the turn was given up, it is closed as soon as it opens and prompt() is never called. Mentions pass the delivery's signal through startup the same way. Tests in sessions.test.ts: "stops a one-off turn when its signal aborts" and "gives up a one-off turn whose session is still opening, and closes it once open".

🤖 Rechecked at ba39d40: runtime.ts:299–309 still discards StandaloneTurn.signal, so the shared deadline does not bound startup and work can begin after it expires. Forward the signal through startup and cancel/close on abort. The #1142 implementation described above is absent from this candidate; this review did not validate #1142.

🤖 Should fix: Activity matches an agent's sessions by display name.

recent() keeps rows whose title starts with ${name} · . Names aren't unique and can change. Two Goose agents named "Helper" in the default workspace see each other's sessions, and renaming an agent hides its own recent sessions. Opening a row then reads that session id without checking that the session belongs to this agent's pubkey.

Fix: filter by the agent pubkey recorded in the session's first prompt (as startedFor already does for find), not by title.

🤖 Deferring to a follow-up. Activity is a read-only view of the owner's own goose sessions on their own machine, so no data crosses users. The mix-up only happens between the same owner's agents that share a name and a workspace, or after a rename. Filtering by the pubkey in the first prompt, as startedFor does, is the right fix. I will add it in the follow-up.

🤖 Fixed in #1142. recent() now takes the agent's pubkey. It lists the sessions that mention the pubkey, and keeps those whose first prompt names it in Agent:, the same check startedFor makes. The test covers a renamed agent and another agent with the same name.

🤖 Optional: useRead drops the shown value on error.

The loading branch keeps prior.value, but the error branch replaces it. If the provider re-read after "Save to goose" fails, the Provider select disables and the key fields and models disappear. Keep value: prior.value in the error state too, as the type already allows.

🤖 Leaving this one for the follow-up, since it is optional. A failed re-read clears the fields until the user picks Try again, and that brings everything back. Keeping prior.value in the error state is a one-line change, and I will fold it into the follow-up.

🤖 Fixed in #1142. The error state now keeps prior.value. A new test makes the re-read after Save fail. The Provider select stays enabled and shows the chosen provider, next to the error and Try again.

🤖 [P2] Reserve the reopening slot before rebuilding context

After idle expiry, a settings change, or a process exit, two close mentions can both await fresh() before either registers opening[key]. Both then open sessions, and adopt() overwrites the first entry without closing it. The orphan's idle timer cannot close it because it only acts on the currently mapped session, leaving its Goose session and MCP server allocated until process exit. Dispatcher serialization does not prevent this: runtime.run() returns while delivery is unresolved (runtime.ts:246-266).

A preserved scripted ACP probe from #1110 opened two replacement sessions for one conversation and tracked only the second; the session implementation and adapter are byte-identical at this head. This was not rerun at the current head. This is distinct from normal 15-minute idle retention exhausting the server pool.

Reserve a single opening promise before any fresh() await, with context rebuilding and opening inside it. Add a concurrent-reopen test with gated context rebuilding that asserts one replacement session and complete cleanup.

🤖 Thanks. Deferring to the session-lifecycle follow-up. The orphan is one goose session and its MCP server, and it lasts until the goose process exits. The token change in 2570b16 means the orphan's token admits nothing outside goose. Reserving the opening promise before fresh() is the right fix. It lands best next to the pool LRU, which changes the same opening path. That PR will add the gated concurrent-reopen test you describe.

🤖 Fixed in #1142. deliver now reserves the opening promise before any await, and fresh() runs inside it. The new gated test holds fresh(), sends two messages, and checks for one replacement session. It also checks that both sessions and both servers are closed after the idle timeout. The test fails on the old order.

🤖 Reproduced again at ba39d40: concurrent reopening creates three sessions overall, leaves session-2 and its MCP server unclosed, and tracks only session-3. The reservation still follows the fresh() await (sessions.ts:126–128). #1142 is a separate proposed fix, not a fix present in this candidate.

🤖 [P2] Keep processes alive while sessions are opening

Saving a model, provider or workspace change while goose.open() is pending calls retire(), but this only counts live and once users. The opening session has not been adopted into either, so its process is killed. Delivery fails with goose could not start: goose is not running and is not retried; the fingerprint retry at lines 244–248 only runs after a successful open.

Reproduced at 2570b16 using the PR's fakes: warm the process, hold serve while opening a second conversation, change the environment, call reconfigure(), then release it. The process is killed and delivery fails. Count in-flight opens as process users, decrement in a finally block, and retry retirement after decrement. Add this gated-open regression test.

🤖 Fixed in #1142 (e9bde62). open() now counts its session as a user of the process until the caller records it, decrements in a finally, and calls retire() again after. The new test holds serve while a second conversation opens, saves new settings, then releases it. The delivery now succeeds in the old process, and the process stops once nothing runs in it. The test fails at 2570b16.

🤖 Rechecked at ba39d40: this still reproduces in #1125. Holding serve during an open, changing settings, calling reconfigure(), then releasing returns goose could not start: goose is not running with the process killed. The opening session is still omitted from retirement's users (sessions.ts:287–297). The referenced fix is in separate #1142; it is not part of this candidate and was not validated in this review.

🤖 Should fix: settings changed A→B→A leak the old goose process.

retire() returns early for any process whose fingerprint matches the current settings, not only the actual current process. To reproduce:

1. Keep a session busy in process A1.
2. Change the model, so process B starts.
3. Change it back, so process() starts a new A2: current was B, so the fingerprints differ.
4. Let A1's session finish and idle out.

drop() then calls retire(A1), which returns because A1's fingerprint equals processFingerprint(). A1 has no users and is not current, but it stays alive until app exit or the next settings change. This differs from Kalvin's retire-while-opening thread: A1 here was a real adopted user.

Fix: keep only the process that this.current resolves to, by identity, and kill any non-current process with no users.

🤖 Fixed in #1142 (e9bde62). retire() now keeps only the process that this.current resolves to, by identity, and only while its settings are current. Any other process with no running or opening sessions is killed. The new test runs A→B→A with a busy session in A1, lets it idle out, and checks that A1 stops and A2 keeps running. The test fails at 2570b16.

🤖 [P1] Construct the claim guard lazily

then_some(Claim { … }) eagerly constructs its argument even for Join::Inside. The unused guard immediately drops and Claim::drop (lines 455–459) resets the session to UNCLAIMED before forwarding. Thus initialize → notifications/initialized succeeds, but the next request with the valid session ID gets 404. A holder of the persisted bearer can initialize again without triggering duplicate-client shutdown; discovery/ping during CLAIMING can also reset ownership.

An isolated Rust reproduction confirmed the reset and successful reclaim; this was not native HTTP or Goose E2E. Use .then(|| Claim { … }). Add native tests for several consecutive authenticated requests and discovery/ping during held initialization. The existing tests stop after one admitted exchange, and the Node substitute does not exercise this destructor.

🤖 @kalvinnchau Good catch, confirmed. The unused guard reset every request's claim. 5ff7d67 builds the guard only for the claiming initialize, and its Drop now releases only CLAIMING, so a stray guard can never undo a finished claim. Two native tests cover what you described: keeps_the_session_across_requests and holds_the_claim_while_initialize_waits. Both failed before the fix.