Skip to content

fix(desktop): render description on inline approval card for plugin approvals - #62411

Closed
ethernet8023 wants to merge 1 commit into
NousResearch:mainfrom
ethernet8023:fix/desktop-approval-description-62402
Closed

ethernet8023 wants to merge 1 commit into
NousResearch:mainfrom
ethernet8023:fix/desktop-approval-description-62402

Conversation

@ethernet8023

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes the desktop inline approval card showing a useless placeholder (<terminal> (plugin approval rule)) instead of the actual approval context when a plugin pre_tool_call hook escalates a tool call to the human-approval gate.

When a plugin returns {"action": "approve"}, the backend (tools/approval.py:request_tool_approval) sets the approval's command field to a synthetic label and puts the real info in description. The inline ApprovalBar only rendered command (in the "Command" expander) and never showed description, so users approved without seeing what they were approving.

Related Issue

Fixes #62402

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • apps/desktop/src/components/assistant-ui/tool/approval.tsx:
    • Render description as a context <p> line on the inline ApprovalBar (the floating fallback bar already did this)
    • Detect synthetic plugin commands (starting with <) via isSyntheticCommand and set hasCommand = false for those, hiding the "Command" toggle and the <pre> block in the "Always allow" dialog
    • Use hasCommand consistently in the "Always allow" dialog instead of raw request.command.trim()

How to Test

  1. Add a pre_tool_call plugin rule that returns {"action": "approve"} for a command-bearing tool (e.g. terminal)
  2. In the desktop app, trigger a tool call that matches the rule
  3. On the inline approval card, the plugin's reason text (from description) is now visible as a context line
  4. The "Command" toggle is hidden (the synthetic <terminal> (plugin approval rule) label is no longer shown)
  5. For regular dangerous-command approvals (non-synthetic command), the description is also shown as context, and the "Command" toggle still works as before

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run tests and all tests pass (vitest run --environment jsdom — 18/18 passed)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: NixOS (Linux)

Documentation & Housekeeping

  • N/A — no config keys, docs, or tool schemas changed

…pprovals

Plugin `pre_tool_call` hooks that return `{"action": "approve"}` set the
approval's `command` to a synthetic label (`<terminal> (plugin approval
rule)`) while the real info lives in `description`. The inline approval
card only rendered `command`, so users approved without seeing what runs.

- Render `description` as a context line on the inline `ApprovalBar` (same
  as the floating fallback bar already does)
- Detect synthetic plugin commands (starting with `<`) and hide the
  Command expander and Always-allow dialog `<pre>` block for those cases
- Use `hasCommand` consistently in the Always-allow dialog instead of
  raw `request.command.trim()`

Fixes NousResearch#62402
@ethernet8023
ethernet8023 force-pushed the fix/desktop-approval-description-62402 branch from c1bae03 to 3314a9e Compare July 11, 2026 02:20
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists labels Jul 11, 2026

@kshitijk4poor kshitijk4poor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the focused fix — the diagnosis is correct: plugin pre_tool_call approvals put the human-readable context in description, and the inline desktop card needs to show it.

I found two changes needed before this is ready:

  1. ApprovalBar is shared by the inline card and the floating fallback. The fallback already renders request.description in its header, so the new unconditional description paragraph makes the fallback show the same reason twice. Please restrict the new paragraph to surface === 'inline', or consolidate the rendering so there is a single source of description text.

  2. The synthetic-command check is too broad. request.command.trim().startsWith('<') also matches valid shell commands that begin with input redirection, such as < /dev/null command. That would hide the real command from both the Command disclosure and the “Always allow” confirmation. Please identify the specific backend-generated plugin label (<tool> (plugin approval rule)) rather than every <-prefixed command, or add an explicit field to the approval payload.

One integration note: this overlaps with #62092 in the same two files. That PR also preserves multiline descriptions and bounds long content, so please rebase or coordinate the two changes to avoid a conflicting/duplicated implementation.

The focused desktop test suite (12/12), TypeScript build, eslint, Prettier, and CI otherwise passed.

@kshitijk4poor
kshitijk4poor self-requested a review July 12, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop approval card shows a placeholder instead of the command for hook approvals

3 participants