From d48bf743f2cec4221ee51f1f51c92b96f07c84b8 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 12 Jul 2026 20:06:15 -0700 Subject: [PATCH] fix(approval): scope smart deny owner overrides to one operation Co-authored-by: Sergei Ivanov --- acp_adapter/permissions.py | 25 ++- .../hooks/use-message-stream/gateway-event.ts | 4 +- .../assistant-ui/tool/approval.test.tsx | 28 ++- .../components/assistant-ui/tool/approval.tsx | 14 +- apps/desktop/src/lib/chat-messages.ts | 1 + apps/desktop/src/store/prompts.ts | 2 + cli.py | 22 ++- gateway/platforms/api_server.py | 11 +- gateway/platforms/qqbot/adapter.py | 10 +- gateway/platforms/qqbot/keyboards.py | 54 +++--- gateway/platforms/whatsapp_cloud.py | 4 + gateway/run.py | 43 ++++- locales/af.yaml | 4 + locales/de.yaml | 4 + locales/en.yaml | 4 + locales/es.yaml | 4 + locales/fr.yaml | 4 + locales/ga.yaml | 4 + locales/hu.yaml | 4 + locales/it.yaml | 4 + locales/ja.yaml | 4 + locales/ko.yaml | 4 + locales/pt.yaml | 4 + locales/ru.yaml | 4 + locales/tr.yaml | 4 + locales/uk.yaml | 4 + locales/zh-hant.yaml | 4 + locales/zh.yaml | 4 + plugins/platforms/discord/adapter.py | 13 ++ plugins/platforms/feishu/adapter.py | 18 +- plugins/platforms/matrix/adapter.py | 15 +- plugins/platforms/slack/adapter.py | 66 +++---- plugins/platforms/teams/adapter.py | 61 +++---- plugins/platforms/telegram/adapter.py | 27 +-- tests/acp/test_permissions.py | 27 +++ tests/cli/test_cli_approval_ui.py | 32 ++++ tests/gateway/test_api_server_runs.py | 19 ++ .../gateway/test_approval_prompt_redaction.py | 60 +++++++ tests/gateway/test_slack_approval_buttons.py | 18 ++ .../gateway/test_telegram_approval_buttons.py | 43 +++++ tests/gateway/test_tui_approval_redaction.py | 22 +++ tests/tools/test_approval.py | 106 +++++++++++ .../test_execute_code_approval_cluster.py | 168 +++++++++++++++++- tools/approval.py | 163 +++++++++++------ tools/terminal_tool.py | 2 + tui_gateway/server.py | 7 + ui-tui/src/__tests__/approvalAction.test.ts | 21 ++- .../createGatewayEventHandler.test.ts | 17 ++ ui-tui/src/app/createGatewayEventHandler.ts | 8 +- ui-tui/src/components/prompts.tsx | 15 +- ui-tui/src/gatewayTypes.ts | 2 +- ui-tui/src/types.ts | 2 + 52 files changed, 1011 insertions(+), 203 deletions(-) diff --git a/acp_adapter/permissions.py b/acp_adapter/permissions.py index 29bd101edd99..5f29a96725cc 100644 --- a/acp_adapter/permissions.py +++ b/acp_adapter/permissions.py @@ -38,19 +38,22 @@ def _permission_option_supports_kind(kind: str) -> bool: return True -def _build_permission_options(*, allow_permanent: bool) -> list[PermissionOption]: +def _build_permission_options( + *, allow_permanent: bool, smart_denied: bool = False, +) -> list[PermissionOption]: """Return ACP options that match Hermes approval semantics.""" - options = [ - PermissionOption(option_id="allow_once", kind="allow_once", name="Allow once"), - PermissionOption( + options = [PermissionOption( + option_id="allow_once", kind="allow_once", name="Allow once", + )] + if not smart_denied: + options.append(PermissionOption( option_id="allow_session", # ACP has no session-scoped kind, so use the closest persistent # hint while keeping Hermes semantics in the option id. kind="allow_always", name="Allow for session", - ), - ] - if allow_permanent: + )) + if allow_permanent and not smart_denied: options.append( PermissionOption( option_id="allow_always", @@ -59,7 +62,7 @@ def _build_permission_options(*, allow_permanent: bool) -> list[PermissionOption ), ) options.append(PermissionOption(option_id="deny", kind="reject_once", name="Deny")) - if _permission_option_supports_kind("reject_always"): + if not smart_denied and _permission_option_supports_kind("reject_always"): options.append( PermissionOption( option_id="deny_always", @@ -129,11 +132,15 @@ def make_approval_callback( description: str, *, allow_permanent: bool = True, + smart_denied: bool = False, **_: object, ) -> str: from agent.async_utils import safe_schedule_threadsafe - options = _build_permission_options(allow_permanent=allow_permanent) + options = _build_permission_options( + allow_permanent=allow_permanent, + smart_denied=smart_denied, + ) tool_call = _build_permission_tool_call(command, description) coro = request_permission_fn( diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts index c4efb85d5b41..189ac0c88d6a 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts @@ -489,9 +489,11 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { setApprovalRequest({ // false only when a tirith warning forbids it; backend omits the field otherwise. allowPermanent: payload?.allow_permanent !== false, + choices: Array.isArray(payload?.choices) ? payload.choices.filter(choice => typeof choice === 'string') : undefined, command, description, - sessionId: sessionId ?? null + sessionId: sessionId ?? null, + smartDenied: payload?.smart_denied === true }) if (sessionId) { 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 2955fafe8d6f..910a436f81f2 100644 --- a/apps/desktop/src/components/assistant-ui/tool/approval.test.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/approval.test.tsx @@ -30,9 +30,13 @@ function part(toolName: string): ToolPart { return { toolName, type: `tool-${toolName}` } as unknown as ToolPart } -function setRequest(command = 'rm -rf /tmp/x', allowPermanent?: boolean) { +function setRequest( + command = 'rm -rf /tmp/x', + allowPermanent?: boolean, + extra: { choices?: string[]; smartDenied?: boolean } = {} +) { $activeSessionId.set('sess-1') - setApprovalRequest({ allowPermanent, command, description: 'dangerous command', sessionId: 'sess-1' }) + setApprovalRequest({ allowPermanent, command, description: 'dangerous command', sessionId: 'sess-1', ...extra }) } function mockGateway() { @@ -131,6 +135,26 @@ describe('PendingToolApproval', () => { expect(screen.queryByRole('menuitem', { name: /Always allow/ })).toBeNull() }) + it('renders only Once and Deny for a Smart DENY owner override', () => { + setRequest('rm -rf /tmp/x', true, { smartDenied: true }) + render() + + expect(screen.getByRole('button', { name: /Run/ })).toBeTruthy() + expect(screen.getByRole('button', { name: /Reject/ })).toBeTruthy() + expect(screen.queryByRole('button', { name: /More approval options/ })).toBeNull() + expect(screen.queryByText(/Allow this session/)).toBeNull() + expect(screen.queryByText(/Always allow/)).toBeNull() + }) + + it('renders only choices explicitly supplied by the gateway event', () => { + setRequest('rm -rf /tmp/x', true, { choices: ['once', 'deny'] }) + render() + + expect(screen.getByRole('button', { name: /Run/ })).toBeTruthy() + expect(screen.getByRole('button', { name: /Reject/ })).toBeTruthy() + expect(screen.queryByRole('button', { name: /More approval options/ })).toBeNull() + }) + it('renders a floating fallback when no pending tool row is mounted', () => { setRequest('rm /tmp/hermes_approval_test.txt') const { container } = render() diff --git a/apps/desktop/src/components/assistant-ui/tool/approval.tsx b/apps/desktop/src/components/assistant-ui/tool/approval.tsx index 3416c22727b4..9d0d69b06a1d 100644 --- a/apps/desktop/src/components/assistant-ui/tool/approval.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/approval.tsx @@ -110,6 +110,10 @@ const ApprovalBar: FC<{ request: ApprovalRequest; surface: 'floating' | 'inline' 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 choices = request.choices ?? (request.smartDenied ? ['once', 'deny'] : undefined) + const allowSession = choices ? choices.includes('session') : true + const allowAlways = choices ? choices.includes('always') : allowPermanent + const hasMoreOptions = allowSession || allowAlways const hasCommand = request.command.trim().length > 0 const respond = useCallback( @@ -183,8 +187,8 @@ const ApprovalBar: FC<{ request: ApprovalRequest; surface: 'floating' | 'inline' {submitting === 'once' ? : copy.run} {submitting !== 'once' && {isMac ? '⌘⏎' : 'Ctrl⏎'}} - - + {hasMoreOptions && } + {hasMoreOptions &&