feat (acp): Gate tool-call label enrichment on ACP client capability - #10644
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: b0d7c10050
ℹ️ 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".
| let client_requests_tool_call_label_enrichment = goose_client_capabilities | ||
| .as_ref() | ||
| .and_then(|goose| goose.tool_call_label_enrichment) | ||
| .unwrap_or(false); |
There was a problem hiding this comment.
When running the bundled ACP frontends, this new default treats the missing toolCallLabelEnrichment bit as an opt-out: the desktop initializer still sends only mcpHostCapabilities, customNotifications, and recipeParameterRequests in ui/desktop/src/acp/acpConnection.ts:145-152, and the text UI sends clientCapabilities: {} in ui/text/src/tui.tsx:795-799 and 1338-1342. As a result handle_tool_request returns before spawning title generation and spawn_ready_chain_summary also returns, so these clients stop generating/persisting the enriched tool titles and chain summaries that previously appeared on replay; either set the capability in the bundled clients or preserve the old behavior for clients that have not been updated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ui/desktop:
Currently we deliberately skipped the llm generated enriched notification, so with this PR, it does not change any ui/desktop behaviour. UI change will be a separate PR to remove the special skip logic after this PR is merged
TUI
We paused TUI dev already
|
Documentation preview deployed: https://pr-10644.goose-pr-previews-poc.pages.dev |
lifeizhou-ap
changed the title
|
Hey @ocervinka! LLM generated tool call title and chained summary enrichment is now opt-in through the ACP client capability and disabled by default, so the environment variable is no longer needed. Thanks again for adding the original safeguard! |
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
LGTM - only slight pause would be if it is surprising to make it opt in vs opt out, if any side effects from consumers of it?
The enrichment was originally introduced for Berd, and I've updated its ACP client to request the capability, so its behavior is unchanged. I made it opt-in because enrichment adds an LLM call for every tool call, which isn't a good default for all clients. Some consumers prefer the original title and previously used GOOSE_DISABLE_TOOL_CALL_SUMMARY. Clients can request toolCallLabelEnrichment during initialization if they want enriched titles. |
* origin/main: (24 commits) upgrade to rmcp 2.0 (#10584) chore: opus5 support (including adaptive thinking) (#10686) fix(desktop): use shell working directory on initial launch (#10655) fix(providers): enable inventory refresh for OpenRouter model picker (#10641) Add configurable GOOSE_DOCS_ROOT for air-gapped docs access (#10294) fix(i18n): fix Korean translation errors, untranslated entries, and terminology consistency (#10667) fix(acp): preserve read lines and reload tool output (#10662) fix(acp): surface tool call output in CLI and Desktop (#10654) fix overlong function names in provider requests (#10659) fix (desktop): preserve ACP tool call update fields (#10653) feat: support latest Gemini models (#10630) Merge commit from fork feat (acp): Gate tool-call label enrichment on ACP client capability (#10644) fix(session): use configured session manager for tool summaries (#10628) refactor (acp): simplify ACP tool-call handling and fix chain summaries (#10599) fix: omit disabled OpenRouter reasoning (#10517) enhance the uniffi API layer (#10427) remove sampling from smoke test (#10639) rebuild canonical model db (#10633) fix(ui): clear stale pending ACP connection after terminal recovery failure (#10552) ... # Conflicts: # ui/desktop/src/i18n/messages/ko.json
2 participants
Summary
Followup PR for #10599
toolCallLabelEnrichmentcapability. Before this PR, ACP always perform LLM tool titles and chain summaries.GOOSE_DISABLE_TOOL_CALL_SUMMARYwas used to turn off llm tool title enrichment.GOOSE_DISABLE_TOOL_CALL_SUMMARYenvironment variable.Testing
Unit test and manual