fix(mcp): resolve hub tool auto-approve to underlying server#13493
Merged
fix(mcp): resolve hub tool auto-approve to underlying server#13493
Conversation
When auto-approving hub tools (invoke/exec), the tool's serverId is 'hub' which doesn't match any real MCP server. This fix resolves the underlying server via resolveHubTool and updates disabledAutoApproveTools on the correct server. Also ensures the tool action is confirmed even when no server is found, instead of silently returning. Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: suyao <[email protected]>
eeee0717
reviewed
Mar 16, 2026
src/renderer/src/pages/home/Messages/Tools/hooks/useMcpToolApproval.ts
Outdated
Show resolved
Hide resolved
![cherry-ai-bot[bot]](https://avatars.githubusercontent.com/in/3013849?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Thanks for tackling the hub auto-approve path and for adding thorough branch coverage around the resolution/fallback behavior. I found one remaining blocking issue: the persistence path now resolves hub tools to the underlying server, but the UI-side isAutoApproved calculation in useMcpToolApproval still only checks tool.serverId directly. For hub invoke / exec tools, that means the execution layer can auto-approve correctly while the hook still thinks the tool is not auto-approved and may render the wrong waiting state. Please align the hook's auto-approved detection with the same hub-resolution logic used in the execution and persistence paths before merge.
src/renderer/src/pages/home/Messages/Tools/hooks/useMcpToolApproval.ts
Outdated
Show resolved
Hide resolved
The isAutoApproved UI state was not resolving hub invoke/exec tools to their underlying server, causing the approval state machine to be inconsistent between the execution layer and rendering. Extract a shared resolveHubToolServer helper and add a useEffect to asynchronously resolve hub tools for the UI auto-approve state, matching the same logic used in the execution layer. Also simplify the autoApprove callback to use the shared helper, eliminating duplicated hub resolution logic. Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: suyao <[email protected]>
When multiple tool calls arrive in a single turn, all check auto-approve simultaneously and block on requestToolConfirmation. Clicking auto-approve on one tool only confirmed that single tool. Now the execution layer registers tool-id-to-name mappings and calls confirmSameNameTools after confirmation, matching the legacy middleware behavior. For hub invoke/exec tools, the underlying tool name is used as the mapping key so only tools targeting the same underlying server are batch-confirmed. Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: suyao <[email protected]>
Contributor
There was a problem hiding this comment.
The previous blocking issue (UI isAutoApproved not resolving hub tools) has been addressed. The new commit adds batch-confirm for same-name tools, and the comprehensive test coverage confirms the hub resolution logic works for both UI state and persistence. No remaining blocking issues. Approving.
eeee0717
approved these changes
Mar 16, 2026
resolveHubToolName is synchronous and returns null when toolNameMapping has been cleared (e.g. after invalidateCache). Unlike callMcpTool which lazily refreshes the mapping, the ResolveHubTool IPC handler did not. This caused both the execution layer and the UI auto-approve callback to silently fail hub tool resolution, falling back to updating the hub server's disabledAutoApproveTools — which isToolAutoApproved ignores for hub servers (invoke/exec are hardcoded as not auto-approved). Add resolveHubToolNameAsync that mirrors callMcpTool's lazy refresh pattern, and use it in the IPC handler. Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: suyao <[email protected]>
Replace dynamic import with static import in the IPC handler. Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: suyao <[email protected]>
…ndler Reverts changes to ipc.ts to avoid code owner review. The resolveHubToolNameAsync helper remains in mcp-bridge.ts for future use. Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: suyao <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Before this PR:
When a user clicked "Auto-approve" on a hub tool (
invoke/exec), theautoApprovefunction looked for a server withid === 'hub', found none, and silently returned without confirming the tool or persisting the auto-approve preference.After this PR:
The
autoApprovefunction resolves hub tools to their underlying server viaresolveHubTool, then correctly updatesdisabledAutoApproveToolson the real server. If resolution fails or no server is found, the tool action is still confirmed (instead of silently dropped).fixes: #13481
Why we need it and why it was done in this way
Hub tools use a virtual
serverIdof'hub'which doesn't correspond to any real MCP server entry. The fix resolves the underlying server and tool name through the existingwindow.api.mcp.resolveHubToolIPC call, which is already used elsewhere in the codebase (e.g.,aiCore/utils/mcp.ts).The following tradeoffs were made:
disabledAutoApproveToolsupdate logic in the hub resolution branch for clarity. This keeps the hub path self-contained and avoids restructuring the existing flow.The following alternatives were considered:
Breaking changes
None.
Special notes for your reviewer
autoApprovecallback is nowasync(it already was viauseCallback, this just addsawaitfor the IPC call).toolResponsewas added to theuseCallbackdependency array since it's now referenced inside the callback.Checklist
/gh-pr-review,gh pr diff, or GitHub UI) before requesting review from othersRelease note