Skip to content

fix: show approval card for builtin and provider tools#13154

Merged
DeJeune merged 1 commit intoCherryHQ:mainfrom
al-prk:fix/tool-approval-for-builtin-and-provider-tools
Mar 3, 2026
Merged

fix: show approval card for builtin and provider tools#13154
DeJeune merged 1 commit intoCherryHQ:mainfrom
al-prk:fix/tool-approval-for-builtin-and-provider-tools

Conversation

@al-prk
Copy link
Copy Markdown
Contributor

@al-prk al-prk commented Mar 2, 2026

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'

- 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
Copy link
Copy Markdown
Collaborator

@GeorgeDong32 GeorgeDong32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

  1. Consider renaming to or similar for clarity
  2. 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.

@DeJeune DeJeune merged commit 1fb2583 into CherryHQ:main Mar 3, 2026
21 checks passed
@kangfenmao kangfenmao mentioned this pull request Mar 3, 2026
4 tasks
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tool calls hang when user approval is required (without Auto Approve)

3 participants