feat(acp): add session/set_config and stabilize list, delete and close#7984
feat(acp): add session/set_config and stabilize list, delete and close#7984codefromthecrypt wants to merge 5 commits intomainfrom
Conversation
8908db2 to
a769e96
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a769e96489
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
draft as while the bot comment was a red herring, in investigating it I found a different bug nearby where we aren't implementing model change, just it appears we are. I am going to fix that and re-open for review when done |
3b4f484 to
34e59df
Compare
|
This was quite possibly the most tedious manual Q/A I've ever done in goose. hope it helps! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34e59df074
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Adrian Cole <adrian@tetrate.io>
34e59df to
1cd9c46
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cd9c4616d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Adrian Cole <adrian@tetrate.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5c93809a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
I have spent 3 dauys on this and do not and won't spend unlimited time feeding copilot where if ever more than 50% accurate still wastes similar time and many rabbit holes. the last I did is the last revision |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecf5badfe1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Adrian Cole <adrian@tetrate.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a77cb4f17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
codex is getting a bit much I think... over confident and not always helpful |
|
as usual, the last several comments of codex are either garbage or highly speculative. The only one not completely invalid is assuming agents (which it sometimes it conflates with clients), randomly change the model and we might not know about it, and wants even more complicated code to address, that too. I'll look into it, but yes... this bot has been a cure worse than the disease. |
|
yes at the moment there is no open source agent that sends unsolicited model changes, even though mode sometimes is (escape planning mode) but that's also different than our permission mode (e.g. smart approve) this is ready for a human review (I've had enough of codex and don't want to lose more time with it again on this PR) |
Summary
This change stabilizes ACP session related functionality that is re-used across multiple ACP agents. This does so by adding the new unified
session/set_config_optionand formalizing existing support likesession/list,session/deleteandsession/closewith integration tests. To unlock this, it updates to the latest sacp lib as the prior revlocked us into old schema types.ACP stabilized
session/set_config_optionas a unified config method, supersedingsession/set_modeandsession/set_model.Instead of separate
modesandmodelsfields insession/new, agents return a singleconfig_optionsarray that clients use to populate their UI and send changes back.Here are some notable clients/IDEs that support deviations of this config:
set_config_optionwhen agents advertiseconfig_options, falls back otherwiseset_config_optionsession/set_modeThis updates
goose acpto acceptset_config_optionand fall back toset_mode/set_modelto support the above, taking care not to duplicate code.Goose is also a client when using ACP providers (e.g. claude-acp, codex-acp). The provider now handles
ConfigOptionUpdatenotifications to sync local state, supporting the same patterns in the other direction.Here are some notable agents that support deviations of this config:
config_optionsand falls back toset_modeset_config_optionsession/set_modeandsession/set_modelWe couldn't add this before due to revlock in sacp 10, not without manually writing JSON.
So this bumps to sacp 11 to get
SetSessionConfigOptionRequest.It also adds
if_requestrouting and minor call-site updates.Note:
session/set_config_optionalso addsthought_level, but we returninvalid_paramsuntil we figure out how to normalize thinking.Session lifecycle methods (
session/list,session/close) now use typedif_requestdispatch via sacp's unstable feature flags.session/deleteis a standard ACP method (RFD #395) not yet in the schema crate, so it remains in custom method dispatch untilDeleteSessionRequestis added.Goose as a client now sends
session/closeon shutdown to ACP agents that advertisesessionCapabilities.close. This lets agents free resources:The
#[custom_method]macro no longer forces a_goose/prefix on all methods. Standard ACP methods likesession/deleteuse their standard names so clients don't need goose-specific knowledge. Goose-only methods (_goose/tools,_goose/config/extensions, etc.) keep their prefix.All ACP providers now use a single sentinel
"current"instead of per-provider default model strings ("default","gpt-5.4","auto-gemini-3"). At connect time, goose resolves it to the agent's actualcurrent_model_idfromsession/new. This fixes a clash where claude-acp's old default"default"could overwrite the user's real model choice, and eliminates per-provider constants that drift as agents change their model names.Notable bugs fixed along the way:
fs_errType of Change
AI Assistance
Testing
Integration tests
ACP provider updates:
Scoped provider tests (includes claude-code to prove sentinel check doesn't break non-ACP agentic providers):
Zed
Kill goose and clear state:
Add to
~/.config/zed/settings.json:Mode selection:
Model selection:
Model selection — set_config_option forwarding (claude-acp):
Replace the
envin~/.config/zed/settings.jsonwith:Restart Zed:
Model selection — set_model fallback (gemini-cli):
Replace the
envin~/.config/zed/settings.jsonwith:Restart Zed:
Session list:
acpx (
set_config_optiononly client)acpx uses
session/set_config_optionexclusively (nosession/set_modeorsession/set_modelfallback).Add to
~/.acpx/config.json:{ "agents": { "goose": { "command": "env RUST_LOG=debug,sacp=trace /path/to/goose/target/release/goose acp --with-builtin developer" } } }Kill goose and clear state:
Mode selection (approve mode denies tool calls):
Model selection:
Goose CLI
Setup providers:
$ source bin/activate-hermit $ npm install -g @zed-industries/claude-agent-acp @zed-industries/codex-acp @google/gemini-cli $ just release-binaryEach test should be run with this cleanup first:
session/close on shutdown (claude-acp):
sessionCapabilities.close)session/closewas sentGoose UI
We need to use Goose UI for mid-session ACP provider verifications.
ACP provider updates:
Each test should be run with this cleanup first:
set_config_option — mode (claude-acp):
GOOSE_PROVIDER=claude-acp GOOSE_MODEL=default RUST_LOG=debug,sacp=trace just run-uiset_config_optionto claude-acp:set_mode fallback — mode (gemini-cli):
GOOSE_PROVIDER=gemini-acp GOOSE_MODEL=gemini-2.5-flash RUST_LOG=debug,sacp=trace just run-uisession/set_mode(notset_config_option):Related Issues
Uses ACP #411