From 81436e143ebb1230dcdef398b46304d29ee26bfc Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 11 Jun 2026 18:23:59 -0500 Subject: [PATCH 1/2] fix(approval): carry allow_permanent to TUI + desktop approval prompts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a tirith content-security warning is present the approval backend forces allow_permanent=False and silently downgrades an "always" choice to session scope (the persistence loop in check_all_command_guards only honors "always" → permanent when no tirith finding exists). But the gateway notify payload that drives the TUI and the Electron desktop app never carried that flag, so both surfaces always rendered "Always allow" — offering a permanent allow the backend would quietly refuse to persist. Plumb allow_permanent end-to-end: - tools/approval.py: include `allow_permanent: not has_tirith` in the gateway approval_data the notify callback emits as `approval.request`. - ui-tui: thread `allowPermanent` through the event handler, gateway types, and ApprovalReq; ApprovalPrompt drops the "always" option (and renumbers the quick-pick keys) when it's false. - apps/desktop: thread `allow_permanent` through the gateway payload type, the per-session approval store, and the inline ApprovalBar, which now hides the "Always allow…" dropdown item when permanent allow is disallowed — reusing the existing DropdownMenu / confirm-Dialog UI. The desktop/TUI render path for approvals already landed in #38578 (the root cause of approvals not surfacing in the GUI); this completes the salvage of #37856 by carrying allow_permanent across both surfaces. #37856's original thread-local _block() approach is dropped: desktop/TUI approvals resolve via approval.respond → resolve_gateway_approval (the per-session queue), not the _block()/request_id correlation, so a worker-thread callback waiting on _block would never be released by the real UI. Tests: gateway notify payload carries allow_permanent (True without tirith, False with a tirith warning); ui-tui approvalAction reduced option set + event-handler allowPermanent propagation; desktop store round-trip + the ApprovalBar showing/hiding "Always allow". Supersedes #37856 Closes #37812 Co-authored-by: LeonSGP43 --- .../app/session/hooks/use-message-stream.ts | 3 + .../assistant-ui/tool-approval.test.tsx | 45 +++++++++++++- .../components/assistant-ui/tool-approval.tsx | 26 +++++--- apps/desktop/src/lib/chat-messages.ts | 2 + apps/desktop/src/store/prompts.test.ts | 6 ++ apps/desktop/src/store/prompts.ts | 4 ++ tests/tools/test_command_guards.py | 62 +++++++++++++++++++ tools/approval.py | 7 +++ ui-tui/src/__tests__/approvalAction.test.ts | 11 ++++ .../createGatewayEventHandler.test.ts | 23 +++++++ ui-tui/src/app/createGatewayEventHandler.ts | 7 ++- ui-tui/src/components/prompts.tsx | 37 +++++++---- ui-tui/src/gatewayTypes.ts | 6 +- ui-tui/src/types.ts | 3 + 14 files changed, 214 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-message-stream.ts b/apps/desktop/src/app/session/hooks/use-message-stream.ts index 86244c84386..9f978f2f305 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream.ts @@ -933,6 +933,9 @@ export function useMessageStream({ // raise it and wait — the sidebar flags "needs input" and the inline bar // surfaces once the user focuses that chat. setApprovalRequest({ + // Only an explicit false (tirith content-security warning) drops the + // permanent-allow option; the 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 diff --git a/apps/desktop/src/components/assistant-ui/tool-approval.test.tsx b/apps/desktop/src/components/assistant-ui/tool-approval.test.tsx index fb6c71f6b30..0d13371afee 100644 --- a/apps/desktop/src/components/assistant-ui/tool-approval.test.tsx +++ b/apps/desktop/src/components/assistant-ui/tool-approval.test.tsx @@ -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' @@ -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() { @@ -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() + }) }) diff --git a/apps/desktop/src/components/assistant-ui/tool-approval.tsx b/apps/desktop/src/components/assistant-ui/tool-approval.tsx index 068573131ae..f897d63f756 100644 --- a/apps/desktop/src/components/assistant-ui/tool-approval.tsx +++ b/apps/desktop/src/components/assistant-ui/tool-approval.tsx @@ -61,6 +61,10 @@ 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 + // The backend drops the permanent-allow path when a tirith content-security + // warning is present (it would silently degrade "always" → session scope), so + // don't offer the option. Only an explicit false hides it. + const allowPermanent = request.allowPermanent !== false const respond = useCallback( async (choice: ApprovalChoice) => { @@ -144,16 +148,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} diff --git a/apps/desktop/src/lib/chat-messages.ts b/apps/desktop/src/lib/chat-messages.ts index 5e3a725f303..e569f2582f2 100644 --- a/apps/desktop/src/lib/chat-messages.ts +++ b/apps/desktop/src/lib/chat-messages.ts @@ -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 diff --git a/apps/desktop/src/store/prompts.test.ts b/apps/desktop/src/store/prompts.test.ts index 7e454639ace..d6ddeabf197 100644 --- a/apps/desktop/src/store/prompts.test.ts +++ b/apps/desktop/src/store/prompts.test.ts @@ -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', () => { diff --git a/apps/desktop/src/store/prompts.ts b/apps/desktop/src/store/prompts.ts index 76780a3b6dc..314575298b9 100644 --- a/apps/desktop/src/store/prompts.ts +++ b/apps/desktop/src/store/prompts.ts @@ -68,6 +68,10 @@ 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 { + // Only an explicit false hides "Always allow" — set when the backend will not + // honor a permanent allow (a tirith content-security warning is present), so + // the bar doesn't offer a choice that silently degrades to session scope. + allowPermanent?: boolean command: string description: string } diff --git a/tests/tools/test_command_guards.py b/tests/tools/test_command_guards.py index b9be6837971..86ad13d3eb1 100644 --- a/tests/tools/test_command_guards.py +++ b/tests/tools/test_command_guards.py @@ -285,3 +285,65 @@ class TestProgrammingErrorsPropagateFromWrapper: 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 = set_current_session_key(session_key) + 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 diff --git a/tools/approval.py b/tools/approval.py index 2fba7e1101b..95e0c3b0511 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1428,6 +1428,13 @@ 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 (see prompt_dangerous_approval + # below). When a tirith content-security warning is present the + # backend silently downgrades an "always" choice to session scope + # (see the persistence loop after the decision), so the UI must not + # offer a permanent allow it can't honor. Without this field the + # TUI/desktop always render "Always allow". + "allow_permanent": not has_tirith, } decision = _await_gateway_decision( session_key, notify_cb, approval_data, surface="gateway" diff --git a/ui-tui/src/__tests__/approvalAction.test.ts b/ui-tui/src/__tests__/approvalAction.test.ts index 851b5093448..662fb71b960 100644 --- a/ui-tui/src/__tests__/approvalAction.test.ts +++ b/ui-tui/src/__tests__/approvalAction.test.ts @@ -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' }) + }) }) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 121b9011c7f..aa75e0df5f4 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -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)) diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 7636eb6946f..475e452466e 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -728,8 +728,13 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return case 'approval.request': { const description = String(ev.payload.description ?? 'dangerous command') + // Backend omits the field for the common case; only an explicit false + // (tirith warning present) 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 diff --git a/ui-tui/src/components/prompts.tsx b/ui-tui/src/components/prompts.tsx index 3dfd31be869..22b472f2995 100644 --- a/ui-tui/src/components/prompts.tsx +++ b/ui-tui/src/components/prompts.tsx @@ -7,10 +7,16 @@ 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 +// When the backend disallows a permanent allow (tirith content-security +// warning present) the "always" option is dropped — picking it would only +// be silently downgraded to session scope, so don't offer it. +const APPROVAL_OPTS_NO_ALWAYS = ['once', 'session', 'deny'] as const 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 @@ -18,10 +24,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 @@ -34,26 +37,31 @@ type ApprovalAction = * 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 } } @@ -62,9 +70,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) @@ -99,7 +108,7 @@ export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) { - {OPTS.map((o, i) => ( + {opts.map((o, i) => ( {sel === i ? '▸ ' : ' '} @@ -108,7 +117,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 + ) } diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index 531f08feec8..f2829e1ded4 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -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' } diff --git a/ui-tui/src/types.ts b/ui-tui/src/types.ts index cf89c73ae3c..02deb363d22 100644 --- a/ui-tui/src/types.ts +++ b/ui-tui/src/types.ts @@ -90,6 +90,9 @@ export interface DelegationStatus { } export interface ApprovalReq { + // When false the backend will not honor a permanent allow (e.g. a tirith + // content-security warning is present), so the prompt hides "Always allow". + allowPermanent?: boolean command: string description: string } From 55a18e68600d2fd39ac50e76ef98c7ef6d9e2153 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 11 Jun 2026 18:42:59 -0500 Subject: [PATCH 2/2] chore(approval): tighten allow_permanent comments + DRY the no-always opt set Collapse the verbose multi-line rationale comments across the TUI/desktop/ backend approval surfaces into single-line "why" notes, and derive APPROVAL_OPTS_NO_ALWAYS from APPROVAL_OPTS instead of re-listing it. No behavior change. --- apps/desktop/src/app/session/hooks/use-message-stream.ts | 3 +-- .../desktop/src/components/assistant-ui/tool-approval.tsx | 4 +--- apps/desktop/src/store/prompts.ts | 4 +--- tools/approval.py | 8 ++------ ui-tui/src/app/createGatewayEventHandler.ts | 3 +-- ui-tui/src/components/prompts.tsx | 8 +++----- ui-tui/src/types.ts | 3 +-- 7 files changed, 10 insertions(+), 23 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-message-stream.ts b/apps/desktop/src/app/session/hooks/use-message-stream.ts index 9f978f2f305..442435956ac 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream.ts @@ -933,8 +933,7 @@ export function useMessageStream({ // raise it and wait — the sidebar flags "needs input" and the inline bar // surfaces once the user focuses that chat. setApprovalRequest({ - // Only an explicit false (tirith content-security warning) drops the - // permanent-allow option; the backend omits the field otherwise. + // 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', diff --git a/apps/desktop/src/components/assistant-ui/tool-approval.tsx b/apps/desktop/src/components/assistant-ui/tool-approval.tsx index f897d63f756..6a3dc6c0d9c 100644 --- a/apps/desktop/src/components/assistant-ui/tool-approval.tsx +++ b/apps/desktop/src/components/assistant-ui/tool-approval.tsx @@ -61,9 +61,7 @@ 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 - // The backend drops the permanent-allow path when a tirith content-security - // warning is present (it would silently degrade "always" → session scope), so - // don't offer the option. Only an explicit false hides it. + // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow". const allowPermanent = request.allowPermanent !== false const respond = useCallback( diff --git a/apps/desktop/src/store/prompts.ts b/apps/desktop/src/store/prompts.ts index 314575298b9..a514556d102 100644 --- a/apps/desktop/src/store/prompts.ts +++ b/apps/desktop/src/store/prompts.ts @@ -68,9 +68,7 @@ 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 { - // Only an explicit false hides "Always allow" — set when the backend will not - // honor a permanent allow (a tirith content-security warning is present), so - // the bar doesn't offer a choice that silently degrades to session scope. + // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow". allowPermanent?: boolean command: string description: string diff --git a/tools/approval.py b/tools/approval.py index 95e0c3b0511..3baf1fd10d5 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1428,12 +1428,8 @@ 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 (see prompt_dangerous_approval - # below). When a tirith content-security warning is present the - # backend silently downgrades an "always" choice to session scope - # (see the persistence loop after the decision), so the UI must not - # offer a permanent allow it can't honor. Without this field the - # TUI/desktop always render "Always allow". + # 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( diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 475e452466e..d6023b507bc 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -728,8 +728,7 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return case 'approval.request': { const description = String(ev.payload.description ?? 'dangerous command') - // Backend omits the field for the common case; only an explicit false - // (tirith warning present) drops the permanent-allow option. + // Only an explicit false (tirith warning) drops the permanent-allow option. const allowPermanent = ev.payload.allow_permanent !== false patchOverlayState({ diff --git a/ui-tui/src/components/prompts.tsx b/ui-tui/src/components/prompts.tsx index 22b472f2995..c88ef3bfe22 100644 --- a/ui-tui/src/components/prompts.tsx +++ b/ui-tui/src/components/prompts.tsx @@ -8,10 +8,8 @@ import type { ApprovalReq, ClarifyReq, ConfirmReq } from '../types.js' import { TextInput } from './textInput.js' const APPROVAL_OPTS = ['once', 'session', 'always', 'deny'] as const -// When the backend disallows a permanent allow (tirith content-security -// warning present) the "always" option is dropped — picking it would only -// be silently downgraded to session scope, so don't offer it. -const APPROVAL_OPTS_NO_ALWAYS = ['once', 'session', '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 @@ -34,7 +32,7 @@ type ApprovalAction = { kind: 'choose'; choice: ApprovalChoice } | { kind: 'move * * 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( diff --git a/ui-tui/src/types.ts b/ui-tui/src/types.ts index 02deb363d22..830e532ce8d 100644 --- a/ui-tui/src/types.ts +++ b/ui-tui/src/types.ts @@ -90,8 +90,7 @@ export interface DelegationStatus { } export interface ApprovalReq { - // When false the backend will not honor a permanent allow (e.g. a tirith - // content-security warning is present), so the prompt hides "Always allow". + // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow". allowPermanent?: boolean command: string description: string