From cdba1118310c1b37dddb6de3068eb1baa2244836 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Wed, 18 Feb 2026 17:43:57 -0800 Subject: [PATCH] fix(confirmations): guarded-action handling across webchat and tui --- docs/plans/state.json | 18 ++++++ src/daemon/index.ts | 2 +- src/daemon/services.ts | 3 + src/frontends/tui/commands.ts | 9 +++ src/frontends/tui/components/App.tsx | 57 ++---------------- src/gateway/handlers/agent.test.ts | 32 ++++++++++ src/gateway/handlers/agent.ts | 88 ++++++++++++++++++++++++++++ src/gateway/server.ts | 3 + src/gateway/ui/pages/chat.js | 39 ++++++++++++ 9 files changed, 199 insertions(+), 52 deletions(-) diff --git a/docs/plans/state.json b/docs/plans/state.json index a99b32f..18340e2 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -5594,6 +5594,24 @@ "docs/plans/state.json" ], "test_status": "pnpm typecheck passing" + }, + "guarded-confirmation-parity-webchat-tui": { + "status": "completed", + "date": "2026-02-19", + "updated": "2026-02-19", + "summary": "Implemented guarded-action confirmation parity across WebChat and TUI: gateway command services now expose approval operations (`/approvals`, `/approve`, `/deny`) for WebChat sessions, WebChat UI surfaces these slash commands, and fullscreen TUI now uses non-interactive auto-approval (matching minimal mode) so local runs do not stall on confirmation prompts.", + "files_modified": [ + "src/gateway/handlers/agent.ts", + "src/gateway/handlers/agent.test.ts", + "src/gateway/server.ts", + "src/gateway/ui/pages/chat.js", + "src/daemon/services.ts", + "src/daemon/index.ts", + "src/frontends/tui/components/App.tsx", + "src/frontends/tui/commands.ts", + "docs/plans/state.json" + ], + "test_status": "pnpm test:run src/gateway/handlers/agent.test.ts + pnpm typecheck passing" } }, "overall_progress": { diff --git a/src/daemon/index.ts b/src/daemon/index.ts index 94d7b7e..53695f1 100644 --- a/src/daemon/index.ts +++ b/src/daemon/index.ts @@ -208,7 +208,7 @@ export async function startDaemon(config: Config, options?: StartDaemonOptions): const gateway = createGateway({ config, configPath: options?.configPath, sessionManager, modelRouter, systemPrompt, toolRegistry, toolExecutor, channelRegistry, pairingManager, lifecycle, memoryStore, - getChannelAgents: () => channelAgents, commandRegistry, intentRegistry, routingPolicy, + getChannelAgents: () => channelAgents, commandRegistry, intentRegistry, routingPolicy, hookEngine, }); const messageRouter = createMessageRouter({ diff --git a/src/daemon/services.ts b/src/daemon/services.ts index aebeead..e31005b 100644 --- a/src/daemon/services.ts +++ b/src/daemon/services.ts @@ -27,6 +27,7 @@ import type { MemoryStore } from '../memory/store.js'; import type { CommandRegistry } from '../commands/index.js'; import type { ComponentRegistry } from '../intents/index.js'; import type { RoutingPolicy } from '../routing/index.js'; +import type { HookEngine } from '../hooks/index.js'; // ── Skills ────────────────────────────────────────────────────── @@ -294,6 +295,7 @@ export interface GatewayDeps { commandRegistry?: CommandRegistry; intentRegistry?: ComponentRegistry; routingPolicy?: RoutingPolicy; + hookEngine?: HookEngine; } export function createGateway(deps: GatewayDeps): GatewayServer { @@ -375,6 +377,7 @@ export function createGateway(deps: GatewayDeps): GatewayServer { commandRegistry: deps.commandRegistry, intentRegistry: deps.intentRegistry, routingPolicy: deps.routingPolicy, + hookEngine: deps.hookEngine, uiDir: resolve(import.meta.dirname, '../gateway/ui'), config, persistConfig: configPath diff --git a/src/frontends/tui/commands.ts b/src/frontends/tui/commands.ts index 02ac643..01063d0 100644 --- a/src/frontends/tui/commands.ts +++ b/src/frontends/tui/commands.ts @@ -167,6 +167,9 @@ Commands: /queue Show queue policy for this session /queue set Set queue override (mode/cap/overflow/debounce_ms/summarize_overflow) /queue reset Clear queue overrides for this session + /approvals List pending guarded actions for this session + /approve [id] Approve latest pending action (or specific id) + /deny [id] [reason] Deny latest pending action (or specific id) /elevate [args] Show or manage elevated mode /reset, /clear, /new Clear conversation history /compact Compact conversation history @@ -202,6 +205,9 @@ export const SLASH_COMMANDS = [ '/login', '/pair', '/queue', + '/approvals', + '/approve', + '/deny', '/elevate', '/transfer', '/quit', @@ -226,6 +232,9 @@ export const COMMAND_TOOLTIPS: Record = { '/login': 'Authenticate with GitHub/OpenAI/Anthropic (OAuth/token or API key) or Z.AI (API key store)', '/pair': 'Generate/list/revoke DM pairing codes', '/queue': 'Show or update per-session queue policy', + '/approvals': 'List pending guarded actions for this session', + '/approve': 'Approve latest pending action (or specific id)', + '/deny': 'Deny latest pending action (or specific id and reason)', '/elevate': 'Show or manage elevated mode', '/transfer': 'Transfer session to another frontend (telegram|tui)', '/quit': 'Exit TUI', diff --git a/src/frontends/tui/components/App.tsx b/src/frontends/tui/components/App.tsx index 3218923..dd975e8 100644 --- a/src/frontends/tui/components/App.tsx +++ b/src/frontends/tui/components/App.tsx @@ -8,7 +8,7 @@ import type { Message, ModelClient, TokenUsage } from '../../../models/types.js' import type { ModelRouter } from '../../../models/router.js'; import type { ManagedSession } from '../../../session/index.js'; import type { NativeAgent, ToolUseEvent } from '../../../backends/native/agent.js'; -import type { HookEngine, HookResult } from '../../../hooks/index.js'; +import type { HookEngine } from '../../../hooks/index.js'; import type { ModelConfig, ModelProvider } from '../../../config/schema.js'; import { MODEL_PROVIDERS } from '../../../config/schema.js'; import { createClientFromConfig } from '../../../daemon/index.js'; @@ -95,9 +95,6 @@ export function App({ const lastCtrlCAtRef = useRef(0); const toolLinesRef = useRef([]); - const confirmResolveRef = useRef<((result: HookResult) => void) | null>(null); - const [confirmation, setConfirmation] = useState<{ tool: string; args: Record } | null>(null); - // Set up an Ink-compatible onToolUse callback for the agent. // This replaces process.stdout writes (which corrupt Ink rendering) // with one that updates React state to show tool activity. @@ -132,43 +129,18 @@ export function App({ }; }, [agent, verbose]); - // Inline confirmations for dangerous tools (e.g. shell.exec) in fullscreen mode. + // Fullscreen TUI runs in non-interactive confirmation mode: + // confirmation hooks are auto-approved so flows never block on a y/n prompt. useEffect(() => { if (!hookEngine) {return;} - - hookEngine.setInteractiveConfirmer(async (pending) => { - return new Promise((resolve) => { - confirmResolveRef.current = resolve; - setConfirmation({ tool: pending.tool, args: pending.args }); - }); - }); + hookEngine.setInteractiveConfirmer(async () => ({ approved: true })); return () => { hookEngine.setInteractiveConfirmer(undefined); - confirmResolveRef.current = null; - setConfirmation(null); }; }, [hookEngine]); useInput((inputChar, key) => { - // Confirmation prompt mode: capture y/n and ignore everything else. - if (confirmation && confirmResolveRef.current) { - const c = inputChar.toLowerCase(); - if (c === 'y') { - confirmResolveRef.current({ approved: true }); - confirmResolveRef.current = null; - setConfirmation(null); - return; - } - if (c === 'n') { - confirmResolveRef.current({ approved: false, reason: 'Denied by user' }); - confirmResolveRef.current = null; - setConfirmation(null); - return; - } - return; - } - if (key.escape) { if (isStreaming) { abortRef.current = true; @@ -272,10 +244,6 @@ export function App({ }, []); const handleSubmit = useCallback(async (value: string) => { - if (confirmation) { - return; - } - const command = parseCommand(value); if (!command) {return;} @@ -854,7 +822,6 @@ export function App({ setStreamingContent(''); } }, [ - confirmation, session, agent, modelClient, @@ -885,24 +852,12 @@ export function App({ streamingContent={isStreaming ? streamingContent : undefined} /> - {confirmation ? ( - - - Confirmation required: {confirmation.tool}{' '} - {Object.keys(confirmation.args).length > 0 ? JSON.stringify(confirmation.args) : ''} - - Press y to approve, n to deny. - - ) : null} - { expect(((sent[0] as GatewayEvent).data as { content: string }).content).toBe('agent response'); }); + it('handles /approvals command via fast-path when hook engine is available', async () => { + const sent: OutboundMessage[] = []; + const send = vi.fn((msg: OutboundMessage) => sent.push(msg)); + const hookEngine = { + getPendingConfirmations: vi.fn(() => []), + resolveConfirmation: vi.fn(() => false), + }; + const handlersWithHooks = createAgentHandlers({ + sessionBridge: sessionBridge as unknown as AgentHandlerDeps['sessionBridge'], + laneQueue: new LaneQueue(), + sessionManager: sessionManager as unknown as AgentHandlerDeps['sessionManager'], + commandRegistry, + hookEngine: hookEngine as unknown as AgentHandlerDeps['hookEngine'], + }); + + const req: GatewayRequest = { + id: 11, + method: 'agent.send', + params: { + message: '/approvals', + connectionId: 'conn-1', + metadata: { isCommand: true, command: 'approvals' }, + }, + }; + + await handlersWithHooks['agent.send'](req, send); + + expect(mockAgent.process).not.toHaveBeenCalled(); + expect(hookEngine.getPendingConfirmations).toHaveBeenCalledWith({ sessionId: 'ws:conn-1' }); + expect(((sent[0] as GatewayEvent).data as { content: string }).content).toContain('No pending approvals'); + }); + it('emits user.action audit events for gateway requests', async () => { const sent: OutboundMessage[] = []; const send = vi.fn((msg: OutboundMessage) => sent.push(msg)); diff --git a/src/gateway/handlers/agent.ts b/src/gateway/handlers/agent.ts index 2bea192..2d4f63b 100644 --- a/src/gateway/handlers/agent.ts +++ b/src/gateway/handlers/agent.ts @@ -15,6 +15,7 @@ import type { Config, ModelConfig, ModelProvider } from '../../config/index.js'; import { MODEL_PROVIDERS } from '../../config/index.js'; import { createClientFromConfig } from '../../daemon/models.js'; import { auditLogger } from '../../audit/index.js'; +import type { HookEngine } from '../../hooks/index.js'; import { randomUUID } from 'crypto'; export interface AgentHandlerDeps { @@ -31,6 +32,7 @@ export interface AgentHandlerDeps { commandRegistry?: CommandRegistry; modelRouter?: ModelRouter; runtimeConfig?: Config; + hookEngine?: HookEngine; } function buildProviderConfigMap(config: Config): Partial> { @@ -292,6 +294,92 @@ export function createAgentHandlers(deps: AgentHandlerDeps) { } return 'Session reset.'; }, + getApprovals: () => { + if (!deps.hookEngine) { + return 'Approval gates are not enabled in this runtime.'; + } + if (!sessionId) { + return 'No pending approvals for this session.'; + } + const pending = deps.hookEngine.getPendingConfirmations({ sessionId }); + if (pending.length === 0) { + return 'No pending approvals for this session.'; + } + const lines = ['Pending approvals:']; + for (const item of pending) { + const ageSec = Math.max(0, Math.round((Date.now() - item.createdAt.getTime()) / 1000)); + lines.push(`- ${item.id} | ${item.tool} | ${ageSec}s old`); + } + lines.push(''); + lines.push('Use `/approve ` or `/deny ` (id optional: latest is used).'); + return lines.join('\n'); + }, + approvePending: (inputRaw: string) => { + if (!deps.hookEngine) { + return 'Approval gates are not enabled in this runtime.'; + } + if (!sessionId) { + return 'Approve command is unavailable in this session.'; + } + const pending = deps.hookEngine.getPendingConfirmations({ sessionId }); + if (pending.length === 0) { + return 'No pending approvals for this session.'; + } + + const input = inputRaw.trim(); + const selected = input + ? pending.find((item) => item.id === input) + : pending[pending.length - 1]; + + if (!selected) { + return `Approval id not found in this session: ${input}`; + } + + const resolved = deps.hookEngine.resolveConfirmation(selected.id, { approved: true }); + return resolved + ? `Approved: ${selected.tool} (${selected.id})` + : `Approval request is no longer pending: ${selected.id}`; + }, + denyPending: (inputRaw: string) => { + if (!deps.hookEngine) { + return 'Approval gates are not enabled in this runtime.'; + } + if (!sessionId) { + return 'Deny command is unavailable in this session.'; + } + const pending = deps.hookEngine.getPendingConfirmations({ sessionId }); + if (pending.length === 0) { + return 'No pending approvals for this session.'; + } + + const input = inputRaw.trim(); + let targetId: string | undefined; + let reason = 'Denied by user'; + + if (input) { + const [first, ...rest] = input.split(/\s+/); + const matched = pending.find((item) => item.id === first); + if (matched) { + targetId = matched.id; + reason = rest.join(' ').trim() || reason; + } else { + targetId = pending[pending.length - 1].id; + reason = input; + } + } else { + targetId = pending[pending.length - 1].id; + } + + const selected = pending.find((item) => item.id === targetId); + if (!selected) { + return `Approval request is no longer pending: ${targetId}`; + } + + const resolved = deps.hookEngine.resolveConfirmation(selected.id, { approved: false, reason }); + return resolved + ? `Denied: ${selected.tool} (${selected.id}) — ${reason}` + : `Approval request is no longer pending: ${selected.id}`; + }, getElevation: () => { if (!sessionId || !deps.sessionManager) { diff --git a/src/gateway/server.ts b/src/gateway/server.ts index f1944fd..5a1eda1 100644 --- a/src/gateway/server.ts +++ b/src/gateway/server.ts @@ -44,6 +44,7 @@ import type { GmailWatcher } from '../automation/gmail.js'; import type { PairingManager } from '../channels/pairing.js'; import type { MemoryStore } from '../memory/store.js'; import type { CommandRegistry } from '../commands/index.js'; +import type { HookEngine } from '../hooks/index.js'; import type { ComponentRegistry } from '../intents/index.js'; import type { RoutingPolicy } from '../routing/index.js'; import type { ChannelRegistry } from '../channels/index.js'; @@ -111,6 +112,7 @@ export interface GatewayServerConfig { pairingManager?: PairingManager; memoryStore?: MemoryStore; commandRegistry?: CommandRegistry; + hookEngine?: HookEngine; intentRegistry?: ComponentRegistry; routingPolicy?: RoutingPolicy; discovery?: { @@ -399,6 +401,7 @@ export class GatewayServer { metrics: this.metrics, sessionManager: this.config.sessionManager, commandRegistry: this.config.commandRegistry, + hookEngine: this.config.hookEngine, modelRouter: 'setClient' in this.config.modelClient ? this.config.modelClient : undefined, runtimeConfig: this.config.config, }); diff --git a/src/gateway/ui/pages/chat.js b/src/gateway/ui/pages/chat.js index bd59df5..b1757e1 100644 --- a/src/gateway/ui/pages/chat.js +++ b/src/gateway/ui/pages/chat.js @@ -24,6 +24,9 @@ const SLASH_COMMANDS = [ { name: '/usage', desc: 'Show token usage' }, { name: '/status', desc: 'Show system health' }, { name: '/model', desc: 'Show current model' }, + { name: '/approvals', desc: 'List pending guarded actions' }, + { name: '/approve', desc: 'Approve latest or by id' }, + { name: '/deny', desc: 'Deny latest or by id' }, ]; // ── Helpers ───────────────────────────────────────────────── @@ -402,6 +405,9 @@ function parseSlashCommand(text) { case '/usage': return { type: 'usage' }; case '/status': return { type: 'status' }; case '/model': return { type: 'model', args }; + case '/approvals': return { type: 'approvals' }; + case '/approve': return { type: 'approve', args }; + case '/deny': return { type: 'deny', args }; default: return null; } } @@ -439,6 +445,9 @@ async function handleSlashCommand(cmd, client) { '| `/usage` | Show token usage stats |', '| `/status` | Show system health |', '| `/model [tier|provider]` | Show or set model tier/provider |', + '| `/approvals` | List pending guarded actions |', + '| `/approve [id]` | Approve latest pending or specific id |', + '| `/deny [id] [reason]` | Deny latest pending or specific id |', '', 'Type `/` to see autocomplete suggestions.', ]; @@ -497,6 +506,36 @@ async function handleSlashCommand(cmd, client) { return true; } + case 'approvals': { + try { + const result = await executeAgentSlashCommand(client, 'approvals'); + showSystemMessage(result || 'No pending approvals.'); + } catch (err) { + showSystemMessage(`Failed to list approvals: ${err.message}`); + } + return true; + } + + case 'approve': { + try { + const result = await executeAgentSlashCommand(client, 'approve', cmd.args ?? ''); + showSystemMessage(result || 'Approved.'); + } catch (err) { + showSystemMessage(`Failed to approve: ${err.message}`); + } + return true; + } + + case 'deny': { + try { + const result = await executeAgentSlashCommand(client, 'deny', cmd.args ?? ''); + showSystemMessage(result || 'Denied.'); + } catch (err) { + showSystemMessage(`Failed to deny: ${err.message}`); + } + return true; + } + default: return false; }