fix: show approval card for builtin and provider tools#13154
Merged
DeJeune merged 1 commit intoCherryHQ:mainfrom Mar 3, 2026
Merged
Conversation
- Fixed useToolApproval hook to include builtin and provider tool types - Previously only MCP tools showed approval cards, now builtin and provider tools also display the approval UI correctly
DeJeune
approved these changes
Mar 3, 2026
GeorgeDong32
approved these changes
Mar 3, 2026
Collaborator
GeorgeDong32
left a comment
There was a problem hiding this comment.
Review Summary
✅ Approve
Code Quality Assessment
- Correctness: 9/10 - Fix correctly addresses the root cause
- Readability: 7/10 - Logic is correct but variable name is now misleading
- Maintainability: 8/10 - Minimal changes, easy to backport
Security Review
- ✅ Input validation with optional chaining
- ✅ Type-safe comparison with
- ✅ No logical bypass vulnerabilities
Suggestions (Non-blocking)
- Consider renaming to or similar for clarity
- Future: Add unit tests for tool type routing logic
The fix properly extends tool approval flow to include and tools, resolving issue #13021. CI checks pass.
EurFelux
pushed a commit
that referenced
this pull request
Mar 3, 2026
Fixed an issue where builtin and provider tools would hang in streaming state without showing the approval card to users. Fixes #13021 <img width="1339" height="406" alt="изображение" src="https://github.com/user-attachments/assets/b1cc4ca3-bc5b-4042-95b7-75732187c82f" /> Before this PR: Builtin tools (like builtin_web_search, builtin_knowledge_search) and provider-executed tools would not show the approval card to users. The assistant would hang in streaming state because the UI was looking for permissions in the wrong place. After this PR: All tool types (MCP, builtin, and provider) now correctly display the approval card when user confirmation is required. Users can approve or deny tool executions, and the streaming state properly completes after the tool finishes. Why we need it and why it was done in this way The root cause was in the useToolApproval hook which only treated MCP tools as requiring the approval UI: // Before: Only MCP tools used the correct approval flow const isMcpTool = ... && tool?.type === 'mcp' When a builtin or provider tool was called, the hook switched to useAgentToolApproval which looks for permissions in Redux store. However, these tools use a different permission system (requestToolConfirmation from userConfirmation.ts), so the Redux store was empty and no approval card was shown. The fix extends the condition to include all tool types: // After: All tool types use the correct approval flow const isMcpTool = ... || tool?.type === 'builtin' || tool?.type === 'provider' ### Release note ```release-note fix: show approval card for builtin and provider tools ``` Co-authored-by: Alexandr Prokopenko <[email protected]>
DeJeune
added a commit
that referenced
this pull request
Mar 4, 2026
### What this PR does This is a release PR for **Cherry Studio v1.7.23**. **Changes included:** - Bump version from 1.7.22 to 1.7.23 - Update release notes with user-facing bug fixes ### Release Notes <!--LANG:en--> Cherry Studio 1.7.23 - Bug Fixes 🐛 Bug Fixes - [Selection] Fix app crash on Windows when closing action window - [MiniApp] Fix settings state synchronization and region filter consistency - [Plugin Browser] Make detail modal text selectable and improve search experience - [Tools] Fix approval card not showing for builtin and provider tools ### Included Commits - fix(Selection): prevent Windows crash when closing transparent action window (#13177) - fix(renderer): synchronize miniapp settings state and respect region filter (#13166) - fix: improve plugin browser UX with three small fixes (#13153) - fix: show approval card for builtin and provider tools (#13154) - fix: support esc to close modal (#13159) - fix: open external editor in new window instead of reusing existing one (#13160) - fix: render directory Select options with optionRender (#13152) - refactor: replace static pnpm patch with postinstall script for claude-agent-sdk (#13139) - feat: add dev-only message data inspection button (#13142) - docs: add review workflow to CLAUDE.md (#13145) - chore(deps): upgrade @uiw/codemirror packages to 4.25.7 (#13149) - fix(ci): skip CI on PR body/title edits, only re-run on base branch changes (#13150) ### Review Checklist - [ ] Review generated release notes in `electron-builder.yml` - [ ] Verify version bump in `package.json` - [ ] CI passes - [ ] Merge to trigger release build ### Release note ```release-note Cherry Studio 1.7.23 - Bug Fixes 🐛 Bug Fixes - [Selection] Fix app crash on Windows when closing action window - [MiniApp] Fix settings state synchronization and region filter consistency - [Plugin Browser] Make detail modal text selectable and improve search experience - [Tools] Fix approval card not showing for builtin and provider tools ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: suyao <[email protected]> Co-authored-by: Claude Opus 4.6 <[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
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.
Fixed an issue where builtin and provider tools would hang in streaming state without showing the approval card to users.
Fixes #13021
Before this PR:
Builtin tools (like builtin_web_search, builtin_knowledge_search) and provider-executed tools would not show the approval card to users. The assistant would hang in streaming state because the UI was looking for permissions in the wrong place.
After this PR:
All tool types (MCP, builtin, and provider) now correctly display the approval card when user confirmation is required. Users can approve or deny tool executions, and the streaming state properly completes after the tool finishes.
Why we need it and why it was done in this way
The root cause was in the useToolApproval hook which only treated MCP tools as requiring the approval UI:
// Before: Only MCP tools used the correct approval flow
const isMcpTool = ... && tool?.type === 'mcp'
When a builtin or provider tool was called, the hook switched to useAgentToolApproval which looks for permissions in Redux store. However, these tools use a different permission system (requestToolConfirmation from userConfirmation.ts), so the Redux store was empty and no approval card was shown.
The fix extends the condition to include all tool types:
// After: All tool types use the correct approval flow
const isMcpTool = ... || tool?.type === 'builtin' || tool?.type === 'provider'