feat(acp): add session/set_mode handler#7801
Conversation
b0fcc99 to
5f2d83f
Compare
cde57ee to
be357d0
Compare
be357d0 to
3b7416a
Compare
|
this PR is too big, so I will split it into two.
|
3b7416a to
d24f7ee
Compare
d24f7ee to
cde9cd9
Compare
|
fyi I rescoped this PR to the ACP prep part of mode support. I have a local branch ready to add session-scope mode (and persist) that will fix #7603 This is a pre-req as doing both in the same change was too blurry, especially as a lot of the session persistence logic was plumbing to server and the Goose UI (not affecting ACP in really any way). |
| false | ||
| } | ||
|
|
||
| async fn update_mode(&self, _session_id: &str, _mode: GooseMode) -> Result<(), ProviderError> { |
There was a problem hiding this comment.
so this is a callback which agentic providers can implement to sync their mode for a given session. It is basically the same way permissions integration in ACP works.
| } | ||
| } | ||
|
|
||
| async fn run_with_child( |
There was a problem hiding this comment.
this is mechanically moving around because things were getting hairy in the older layout, especially as we add things like mode and later other things.
|
will rebase after #7843 which will drift it |
|
side note: I notice the whole permissions integration is hairy, but there's a lot of code moving around with high frequency. I'd like to let that settle before refactoring, I think with some message types we can avoid some of the duplication mode/permission logic that is handled both in ACP internally and also somewhat similarly in the agent loops. |
|
#7854 is the mode mutablility/persistence thing split off from this. it keeps things less coupled.. when both of these are in, connecting acp to updatable mode will be less to review |
Add session/set_mode handler that validates the mode string and session existence, rejecting mode changes until per-session persistence lands (#7603). Zed mode selector populates from session/new and session/load responses but previously mode changes were silently ignored. Extract ExpectedSessionId trait so server and provider tests share common_tests. Both paths now exercise identical error cases. Signed-off-by: Adrian Cole <adrian@tetrate.io>
bbc3d9d to
637041a
Compare
* origin/main: fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843)
* main: (191 commits) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) ...
* main: (65 commits) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) ...
…oken-retry * origin/main: (21 commits) Remove java/.ai-usage-marker directory (#7925) test(acp): add terminal delegation fixtures and fix shell singleton (#7923) fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892) feat: persist GooseMode per-session via session DB (#7854) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) ...
* origin/main: (72 commits) No Check do Check (#7942) Log 500 errors and also show error for direct download (#7936) fix: retry on authentication failure with credential refresh (#7812) Remove java/.ai-usage-marker directory (#7925) test(acp): add terminal delegation fixtures and fix shell singleton (#7923) fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892) feat: persist GooseMode per-session via session DB (#7854) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) ...
Summary
The ACP server advertises available modes in
session/newandsession/loadresponses but has nosession/set_modehandler. Zed's mode selector populates from these responses but mode changes are silently ignored. There are no tests for mode validation or error paths, and the server and provider test fixtures diverge: the server fixture uses raw ACP session IDs while the provider fixture uses goose session IDs, preventing shared test coverage.This adds mode validation and aligns the test fixtures.
session/set_modevalidates the mode string and session existence, and rejects mode changes since per-session persistence is not yet implemented (#7603). TheExpectedSessionIdtrait lets server and provider tests share the same test functions. Both paths exercise identical error cases.Type of Change
AI Assistance
Testing
Zed (ACP)
Kill goose and clear session/log state:
Build the release binary:
Add to
~/.config/zed/settings.json:Mode selection (expected rejection):
Open Zed's agent panel. The mode selector (bottom bar, next to model selector) shows "auto" as the default.
Related Issues
Prerequisite for #7603
Screenshots
Zed supports mode selection, so I used that.
Before
Goose has no mode selection, just model:
After
The selector briefly shows "approve" then reverts to "auto" -- Zed optimistically updates the UI, then silently reverts when the server returns an error. The error exists until modes are session scoped in #7603