-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat(code-mode): use server names for MCP extensions #6284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances MCP extension handling in goose by enabling code mode in ACP and improving extension naming for better LLM interaction.
Key Changes
- ACP code mode support: Adds
--with-builtinCLI argument allowing ACP clients to enable built-in extensions like code_execution and developer - Server-based extension naming: Extensions without configured names now use MCP server-declared names instead of random identifiers, with collision detection
- Search filtering: Excludes
extensionmanagerfrom code_execution search results to reduce LLM confusion about internal tools
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/src/agents/extension_manager.rs | Adds resolve_extension_name() function to use server-declared names with collision handling via random suffixes |
| crates/goose/src/agents/code_execution_extension.rs | Filters out extensionmanager from module search results |
| crates/goose-cli/src/session/mod.rs | Removes generate_extension_name() - extensions now use empty name strings to trigger server name resolution |
| crates/goose-cli/src/commands/acp.rs | Adds add_builtins() helper and --with-builtin parameter support for ACP mode |
| crates/goose-cli/src/cli.rs | Adds --with-builtin CLI argument with comma-separated builtin names |
| crates/goose/tests/acp_integration_test.rs | Adds test_acp_with_builtin_and_mcp integration test verifying code_execution works with MCP servers, updates test server to declare explicit name |
| crates/goose/tests/test_data/openai_*.txt | Updates test fixtures with new model format and adds builtin extension interaction test data |
| } | ||
| } | ||
|
|
||
| #[test_case("kiwi", Some("kiwi-mcp-server"), None, "^kiwi$" ; "ACP session prefers explicit name")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: all of these are real server names scraped from the public MCP impls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
51de2f1 to
451c7c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
edbb8e6 to
c84aafb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
ee2e3f2 to
e32f7f6
Compare
e32f7f6 to
ebd1f3b
Compare
| .env("GOOSE_MODE", "approve") | ||
| .env("OPENAI_HOST", mock_server.uri()) | ||
| .env("OPENAI_API_KEY", "test-key") | ||
| .env("GOOSE_PATH_ROOT", data_root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this to isolate the DBs between tests, but allow us to intentionally share them. This will be very important in subsequent change for session load, resume and list capabilities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
|
I'm not sure why - but the GUI isn't working with this - never get a response, no matter what I do (I just flicked to main and tried again) - if I turn off code mode it is fine. |
|
@michaelneale good check and I promise to use the gui also from now on or at least until all the things load extensions the same way. What happened is that on main we load the same extension multiple times. My change was overzealous wrt collisions and it added a suffix if an extension already exists. This is wrong even if loading the same extension multiple times is inefficient. I will fix the code to only do collision avoidance on anonymous extensions. There is a real chance they will collide as all the paths are like /mcp and sometimes even the server name is fake 'StatelessApp'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
|
xcap needed an update to be compatible with core-foundation 0.10.0 → 0.10.1 I'm running a manual flow to capture windows, my screen etc to make sure everything is keen |
653b2e1 to
b8ece62
Compare
|
noticed xcap update was attempted in #4344 and the main thing was the CI packages. I used the same literal packages from xcap themselves, so 🤞 it will pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
b8ece62 to
ed23d02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
4452d23 to
3605eac
Compare
| shellexpand = "3.1.0" | ||
| indoc = "2.0.5" | ||
| xcap = "0.0.14" | ||
| xcap = "=0.4.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the revlock version that allows everything else to pass. Movement past this in #6301
|
re-ran all steps manually, choosing xcap 0.4.0 which is the latest version that needs no new platform deps on linux. |
|
thanks for all the support @michaelneale! |
|
fyi opened this nashaofu/xcap#250 and once it is merged and when we can upgrade will be a lot easier to detect subtle permissions problem on screen capture that need the user to overcome. |
* main: fix: adding more open models (#6300) docs: add goose for vs code extension (#6262) feat(code-mode): use server names for MCP extensions (#6284) docs: agent skills compatibility note (#6299) docs: clarify GOOSE_TERMINAL requires ~/.zshenv for zsh users (#6297) feat: add OpenAI Codex CLI provider (#6263) docs: fix Resources menu (#6292) Remove Advent of AI announcement banner (#6291) Add blog post: How We Use goose to Maintain goose (#6289)
Signed-off-by: Adrian Cole <[email protected]> Signed-off-by: James Loope <[email protected]>


Summary
Makes code mode usable in ACP and improves extension naming for better LLM accuracy.
Code mode in ACP: Adds
--with-builtinarg so ACP clients can enable code_execution and developer extensions.Better extension names: When no name is configured (e.g., CLI
--with-streamable-http-extension), extensions now use server-declared names instead of anonymous identifiers. User-configured names from goose config or ACP sessions are still preferred.Before (CLI arg, no configured name):
After:
─── search-flight | kiwi-mcp-server ──────────────────────────LLMs can now match extension names to server info in their prompts. Collision suffix added when duplicate names exist.
Type of Change
AI Assistance
Testing
New integration test:
test_acp_with_builtin_and_mcp- verifies code_execution and MCP servers work together in ACP mode.Manual:
Related Issues
Closes #6188