feat(acp): add read tool and delegate filesystem I/O to ACP clients#7668
feat(acp): add read tool and delegate filesystem I/O to ACP clients#7668codefromthecrypt merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90c3dabc8f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5073bc5a42
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f598194348
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff9fb9d226
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f1fd9312c
ℹ️ 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".
| pub async fn run_fs_read_text_file_true<C: Connection>() { | ||
| let temp_dir = tempfile::tempdir().unwrap(); | ||
| let config_yaml = format!( | ||
| "GOOSE_MODEL: {TEST_MODEL}\nGOOSE_PROVIDER: openai\nextensions:\n developer:\n enabled: true\n type: platform\n name: developer\n description: Developer\n display_name: Developer\n bundled: true\n available_tools: []\n" |
There was a problem hiding this comment.
this satisfies copilot which rightly pointed out you can add developer explicitly in the config yaml, without splitting this into two tests in a file that will already get long. Other paths use the arg syntax.
Note: in the experimental acp-server we hard-code developer as always here, but this is technically possible to set in goose acp
4f1fd93 to
fe0bae2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe0bae23e8
ℹ️ 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".
fe0bae2 to
62a00ee
Compare
|
rebased as there wasa a lot of drift.. still need to adjust for feedback but no time left today |
When the ACP client declares FileSystemCapability (read_text_file and/or write_text_file), the server wraps the developer extension with AcpTools that routes read/write/edit tool calls through the client instead of using local disk. This works regardless of whether developer is registered via CLI builtins or config.yaml. Signed-off-by: Adrian Cole <adrian@tetrate.io>
62a00ee to
0ecef10
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ecef1085d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23654aaa7d
ℹ️ 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".
| pub(crate) fn get_tools() -> Vec<Tool> { | ||
| vec![ | ||
| Tool::new( |
There was a problem hiding this comment.
Advertise the new
read tool in developer tool list
call_tool now handles "read", but DeveloperClient::get_tools() still only returns write/edit/shell/tree, so the model never sees read in normal (non-ACP) sessions and in ACP sessions without fs_read delegation. That makes the headline feature effectively unreachable unless callers hard-code an undiscoverable tool name, which breaks the expected parity between CLI and ACP tool availability.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes codex that's the point!
This could be a case where a comment is useful -- if the model is confused, a human might be too.
jamadeo
left a comment
There was a problem hiding this comment.
I'd say merge sooner rather than later and we can always follow up on having the ACP tools themselves emit edit locations etc.
| pub(crate) fn get_tools() -> Vec<Tool> { | ||
| vec![ | ||
| Tool::new( |
There was a problem hiding this comment.
Yes codex that's the point!
This could be a case where a comment is useful -- if the model is confused, a human might be too.
* main: (270 commits) test(acp): align provider and server test parity (#7822) fix(acp): register MCP extensions when resuming a session (#7806) fix(goose): load .gitignore in prompt_manager for hint file filtering (#7795) fix: remap max_completion_tokens to max_tokens for OpenAI-compatible providers (#7765) fix(openai): preserve Responses API tool call/output linkage (#7759) chore(deps): bump @hono/node-server from 1.19.9 to 1.19.11 in /evals/open-model-gym/mcp-harness (#7687) fix: return ContextLengthExceeded when prompt exceeds effective KV cache size (#7815) feat: MCP Roots support (#7790) fix(google): use `includeThoughts/part.thought` for thinking handling (#7593) refactor: simplify tokenizer initialization — remove unnecessary Result wrapper (#7744) Fix model selector showing wrong model in tabs (#7784) Stop collecting goosed stderr after startup (#7814) fix: avoid word splitting by space for windows shell commands (#7781) (#7810) Simplify and make it not break on linux (#7813) Add preferred microphone selection (#7805) Remove dependency on posthog-rs (#7811) feat: load hints in nested subdirs (#7772) feat(acp): add read tool and delegate filesystem I/O to ACP clients (#7668) Support secret interpolation in streamable HTTP extension URLs (#7782) More logging for command injection classifier model training (#7779) ...
Summary
ACP file system methods (
fs/read_text_fileandfs/write_text_file) let agents delegate file I/O to the editor instead of hitting disk directly. Reads can return unsaved editor state, and writes let the editor track modifications (Zed shows a native diff). The client advertises support viaclientCapabilities.fsduring initialize.When ACP fs capabilities are present, an
AcpToolsdecorator wrapsDeveloperClientand intercepts the three filesystem tools (read,write,edit), delegating I/O to the client. All other tools (shell, tree, etc.) pass through to the inner client untouched. Non-ACP sessions use the developer extension directly withfs-errfor local I/O.AcpToolsinjects thereadtool only when the client advertisesfs.readTextFile. Non-ACP sessions have noreadtool — the model uses shell for reads, matching the existing developer extension design. This follows the "tool not offered" pattern used by claude-agent-acp, fast-agent, and mistral-vibe.File tools now self-report their locations via
CallToolResult.metawith atool_locationskey, replacing post-hoc argument extraction for read/write/edit. Shell tool location extraction remains as a fallback.The ACP-wrapped developer extension now preserves its instructions in system prompts (previously dropped by
add_clientpassingNoneforserver_info).Type of Change
AI Assistance
Testing
Base Case (no ACP):
No ACP client, so
DeveloperClienthandles I/O directly viafs-err. Output containstest-write-content-67890.Zed
Clear logs and session/thread state:
Add to
~/.config/zed/settings.json:Read delegation:
Make a file called test.txt and save it empty. Then add "marry had a little dog" to its buffer without saving.
Prompt in Zed:
Verify the ACP interactions in Goose. Zed has no telemetry or logs about ACP:
Write delegation + diff view:
Prompt in Zed:
Verify the ACP interactions in Goose. Zed has no telemetry or logs about ACP:
Related Issues
Fixes #7451
Replaces #6940
Uses design from #7388
RFD about symmetric file caps (only both true/both false in practice, but no guidance): agentclientprotocol/agent-client-protocol#662
Screenshots