feat (acp): exposed available tools in acp schema - #10097
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89e9fb0c62
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { | ||
| ...extension, | ||
| description: extension.description ?? '', | ||
| available_tools: availableToolsOrUndefined(extension.available_tools), |
There was a problem hiding this comment.
When a configured extension has a non-empty allowlist, this line carries it into extensionsList, but the recipe picker rebuilds each option in RecipeExtensionSelector.toRecipeExtension without available_tools. In the recipe modal, selecting such an extension then saves it without the field; the backend treats an omitted allowlist as all tools, so the recipe silently widens tool access. Please carry available_tools through the selector's copied recipe extension.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c886b0d8f5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| display_name: config.display_name, | ||
| timeout: config.timeout, | ||
| bundled: config.bundled, | ||
| available_tools: availableToolsOrUndefined(config.available_tools), |
There was a problem hiding this comment.
For configured extensions with a non-empty allowlist, this conversion now carries available_tools into extensionsList, but the settings edit path rebuilds configs from form data in createExtensionConfig (which has no available_tools) before handleUpdateExtension calls addExtension. If a user opens an allowlisted extension in Settings and saves any change, this field is omitted on the write request here and the backend treats omission as all tools, widening access; please carry the original allowlist through updates.
Useful? React with 👍 / 👎.
c886b0d to
f034a74
Compare
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
makes sense and default available if none I also agree with (least surprise)
* main: (26 commits) Fix MCP app sandbox bridge lifecycle (#10064) fix(bedrock): send inference config (max_tokens, temperature) on Converse (#9889) feat(providers): support OpenRouter request parameters (#9276) Migrate local inference model management to ACP (#10124) (attempt to) fix disk space errors in linux release builds (#10024) feat: add --edit session flag to edit conversation before forking (#9799) feat: add iFlytek Spark and Astron MaaS providers (#9837) fix(desktop): dedupe Nostr session deep link imports (#9918) [codex] Add SessionStart hook parity outside CLI (#9970) feat(providers): add Fireworks AI declarative provider (#9990) fix(providers): don't retry deterministically-permanent 400s (thinking-block immutability) (#10005) fix(deps): downgrade pkcs8 to v0.10 to match sec1/pkcs1 v0.7 (#10119) chore(deps): bump actions/cache from 5.0.2 to 6.0.0 (#10051) Make OpenAI Responses API store param configurable (#10040) remove unsupported model (#10121) chore(release): bump version to 1.40.0 (minor) (#10099) move ollama provider into goose-providers (#9986) UI acp migratoin: Decouple desktop UI types from generated OpenAPI types (#10109) fix(otel): use async reqwest client so OTLP export works in `goose serve` mode (#10100) feat (acp): exposed available tools in acp schema (#10097) ...
* main: (42 commits) Fix MCP app sandbox bridge lifecycle (#10064) fix(bedrock): send inference config (max_tokens, temperature) on Converse (#9889) feat(providers): support OpenRouter request parameters (#9276) Migrate local inference model management to ACP (#10124) (attempt to) fix disk space errors in linux release builds (#10024) feat: add --edit session flag to edit conversation before forking (#9799) feat: add iFlytek Spark and Astron MaaS providers (#9837) fix(desktop): dedupe Nostr session deep link imports (#9918) [codex] Add SessionStart hook parity outside CLI (#9970) feat(providers): add Fireworks AI declarative provider (#9990) fix(providers): don't retry deterministically-permanent 400s (thinking-block immutability) (#10005) fix(deps): downgrade pkcs8 to v0.10 to match sec1/pkcs1 v0.7 (#10119) chore(deps): bump actions/cache from 5.0.2 to 6.0.0 (#10051) Make OpenAI Responses API store param configurable (#10040) remove unsupported model (#10121) chore(release): bump version to 1.40.0 (minor) (#10099) move ollama provider into goose-providers (#9986) UI acp migratoin: Decouple desktop UI types from generated OpenAPI types (#10109) fix(otel): use async reqwest client so OTLP export works in `goose serve` mode (#10100) feat (acp): exposed available tools in acp schema (#10097) ...
2 participants
Summary
When we work on the Acp server, we deliberately hide the available tools attribute on extension and recipe extension schema. However users may already have existing config with this field. Discussion here
This PR exposes the available tools in acp schema to serve the existing extension config. For acp client if available tools is not defined or an empty list, it means all the tools are enabled for the extension.
Testing
Unit test and Manual