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.
This commit is contained in:
Brooklyn Nicholson 2026-06-11 18:42:59 -05:00
parent 81436e143e
commit 55a18e6860
7 changed files with 10 additions and 23 deletions

View file

@ -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',

View file

@ -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(

View file

@ -68,9 +68,7 @@ function keyedPromptStore<T extends KeyedPrompt>(): PromptStore<T> {
// 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

View file

@ -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(

View file

@ -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({

View file

@ -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(

View file

@ -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