fix(acp): return JSON-RPC errors from dispatch handlers instead of killing the connection

[matt2e]

Problem

The ACP SDK treats an Err escaping a cx.spawn()ed task as fatal: the task actor sits in the same try_join! as the connection's incoming/outgoing actors, so the whole connection tears down and clients surface "ACP connection closed". Three handler paths in crates/goose/src/acp/server/dispatch.rs used ? to propagate errors out of their spawned tasks:

• set_config_option — build_config_update() runs after every successful provider/model/mode/thinking-effort change. If it errored (missing session, unbuildable provider, provider absent from inventory), the connection dropped right after a settings switch. Now responds with the error and logs a warning.
• set_config_option — a value that is not a value ID killed the connection instead of returning invalid_params. Now responds with invalid_params.
• close_session — on_close_session() errors were wired to tear down the connection (latent; its only fallible call currently always returns Ok). Now responds with the error, matching the list_sessions pattern.

How we got here

These three sites were outliers, and the history explains why they survived: the meaning of ? in these positions changed twice without the lines themselves ever changing.

• feat(acp): add session/set_config and stabilize list, delete and close #7984 (Mar 2026) introduced the handlers with these ?s as inline handler bodies. Under the sacp SDK pinned at the time, any error escaping a handler closure was already connection-fatal — but that was true of every handler, SDK-wide.
• fix: handle acp requests concurrently #8781 (Apr 2026) wrapped the handler bodies in cx.spawn so slow handlers don't serialize the dispatch loop. The ?s were carried verbatim into the spawned tasks. (The same diff converted fork_session to the safe respond_with_result.)
• refactor: switch to official new rust-sdk for ACP #9062 (May 2026) switched to the official agent-client-protocol SDK, whose incoming actor reports non-spawned handler errors back to the client as JSON-RPC errors (report_handler_error). That silently de-fanged every inline ? in the file — but not these three, which had been moved inside cx.spawn a week earlier. Spawned-task errors still propagate through task_actor and kill the connection.

By the pre-fix state, dispatch.rs had 14 call sites correctly using respond_with_error/respond_with_result versus these 3 bare ?s — including five correct match arms inside the same set_config_option handler, directly above the buggy build_config_update(...).await?. This change brings the last three in line with the file's convention.

I audited the remaining dispatch.rs handlers (the only HandleDispatchFrom/cx.spawn sites in the repo): all other spawned tasks already route handler errors through the responder, and errors from the non-spawned handler path are reported back by the SDK's incoming actor, so they are not fatal. The remaining ?s on responder.respond*/send_notification only fail when the transport is already gone.

One known trade-off: when build_config_update() fails, the underlying config change has already been applied server-side, so the client receives an error for a change that actually took effect and may render a stale value. That's strictly better than tearing down the connection, and the warn! log makes it diagnosable.

Testing

• New regression test: a set_config_option with a boolean (non-value-ID) value yields an invalid_params error response, and a subsequent request on the same connection still succeeds. Verified the test hangs on the unfixed code (the request never gets a response once the connection tears down) and passes with the fix.
• cargo test -p goose --test acp_server_test (48 passed), cargo check -p goose, cargo clippy --all-targets.

🤖 Generated with Claude Code