From f34a9742105d3abf01e9cb060b60e793db2ed628 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Wed, 18 Feb 2026 10:35:42 -0800 Subject: [PATCH] feat: add session-scoped workflow approval gate commands --- README.md | 6 ++ docs/plans/state.json | 24 +++++++- src/commands/builtin/index.test.ts | 46 ++++++++++++++- src/commands/builtin/index.ts | 51 ++++++++++++++++ src/commands/index.ts | 3 + src/commands/types.ts | 3 + src/daemon/index.ts | 2 +- src/daemon/routing.test.ts | 94 ++++++++++++++++++++++++++++++ src/daemon/routing.ts | 82 ++++++++++++++++++++++++++ src/hooks/engine.test.ts | 28 +++++++++ src/hooks/engine.ts | 28 ++++++++- src/hooks/types.ts | 3 + src/tools/executor.ts | 5 ++ 13 files changed, 369 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index fd1db2a..b56637c 100644 --- a/README.md +++ b/README.md @@ -471,6 +471,9 @@ Notes: | `/local` | Switch to local model | | `/cloud` | Switch to cloud model | | `/model` | Show model info and options | +| `/approvals` | List pending approval gates for current session | +| `/approve [id]` | Approve latest (or specific) pending gate | +| `/deny [id] [reason]` | Deny latest (or specific) pending gate | ## Web UI Dashboard @@ -518,6 +521,9 @@ pnpm tui:fs | `/pair` | Generate/list/revoke DM pairing codes | | `/fullscreen` | Switch to fullscreen mode | | `/transfer ` | Transfer session to another frontend (`telegram` or `tui`) | +| `/approvals` | List pending approval gates for current session | +| `/approve [id]` | Approve latest (or specific) pending gate | +| `/deny [id] [reason]` | Deny latest (or specific) pending gate | | `/quit` | Exit | #### Runtime Model Switching diff --git a/docs/plans/state.json b/docs/plans/state.json index 74225fd..717fdd2 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -5283,6 +5283,28 @@ "docs/plans/state.json" ], "test_status": "pnpm test:run src/models/rotating.test.ts src/daemon/clientFactory.test.ts src/config/schema.test.ts + pnpm typecheck passing" + }, + "workflow-approval-gates-tier-b2": { + "status": "completed", + "date": "2026-02-18", + "updated": "2026-02-18", + "summary": "Implemented cross-channel workflow approval gates with session-scoped pending confirmations and explicit `/approvals`, `/approve`, `/deny` commands. HookEngine now stores request context (session/channel/sender), ToolExecutor propagates context for confirm hooks, and router command fast-path can list/resolve approvals for the active session.", + "files_modified": [ + "src/hooks/types.ts", + "src/hooks/engine.ts", + "src/hooks/engine.test.ts", + "src/tools/executor.ts", + "src/commands/types.ts", + "src/commands/builtin/index.ts", + "src/commands/builtin/index.test.ts", + "src/commands/index.ts", + "src/daemon/routing.ts", + "src/daemon/routing.test.ts", + "src/daemon/index.ts", + "README.md", + "docs/plans/state.json" + ], + "test_status": "pnpm test:run src/hooks/engine.test.ts src/commands/builtin/index.test.ts src/daemon/routing.test.ts src/tools/executor.test.ts + pnpm typecheck passing" } }, "overall_progress": { @@ -5306,7 +5328,7 @@ "gmail_auth_cli": "flynn gmail-auth command implemented with OAuth2 flow, doctor check, config routed to Telegram", "native_audio_support": "completed — smart routing for native audio (Gemini/OpenAI/GitHub) vs Whisper transcription fallback", "remaining_phases_completion": "Phase 1: 3/3 (100%) — context levels, command registry, memory structure. Phase 2: 3/3 (100%) — component registry, confidence routing, history index. Phase 3: 2/2 (100%) — adaptive memory/compaction, truthfulness/autonomy hardening", - "next_up": "Implement Tier B2 workflow approval gates (await-approval pattern across channels)" + "next_up": "Implement Tier B4 skill discovery index (registry-backed search/install flow)" }, "soul_md_and_cron_create": { "date": "2026-02-11", diff --git a/src/commands/builtin/index.test.ts b/src/commands/builtin/index.test.ts index 549519d..f7bc9d4 100644 --- a/src/commands/builtin/index.test.ts +++ b/src/commands/builtin/index.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi } from 'vitest'; -import { createContextCommand, createElevateCommand, createModelCommand, createQueueCommand, createResearchCommand, createTransferCommand } from './index.js'; +import { createApproveCommand, createApprovalsCommand, createContextCommand, createDenyCommand, createElevateCommand, createModelCommand, createQueueCommand, createResearchCommand, createTransferCommand } from './index.js'; describe('builtin /model command', () => { it('passes through the full argument string', async () => { @@ -197,3 +197,47 @@ describe('builtin /transfer command', () => { expect(result).toEqual({ handled: true, text: 'Transfer command is not available in this session.' }); }); }); + +describe('builtin approval commands', () => { + it('calls getApprovals for /approvals', async () => { + const cmd = createApprovalsCommand(); + const getApprovals = vi.fn(() => '1 pending'); + const result = await cmd.execute([], { + channel: 'test', + senderId: 'user', + sessionId: 's1', + rawInput: '/approvals', + services: { getApprovals }, + }); + expect(getApprovals).toHaveBeenCalledOnce(); + expect(result).toEqual({ handled: true, text: '1 pending' }); + }); + + it('passes raw input to approvePending for /approve', async () => { + const cmd = createApproveCommand(); + const approvePending = vi.fn(() => 'approved'); + const result = await cmd.execute(['abc-123'], { + channel: 'test', + senderId: 'user', + sessionId: 's1', + rawInput: '/approve abc-123', + services: { approvePending }, + }); + expect(approvePending).toHaveBeenCalledWith('abc-123'); + expect(result).toEqual({ handled: true, text: 'approved' }); + }); + + it('passes raw input to denyPending for /deny', async () => { + const cmd = createDenyCommand(); + const denyPending = vi.fn(() => 'denied'); + const result = await cmd.execute(['abc-123', 'too', 'risky'], { + channel: 'test', + senderId: 'user', + sessionId: 's1', + rawInput: '/deny abc-123 too risky', + services: { denyPending }, + }); + expect(denyPending).toHaveBeenCalledWith('abc-123 too risky'); + expect(result).toEqual({ handled: true, text: 'denied' }); + }); +}); diff --git a/src/commands/builtin/index.ts b/src/commands/builtin/index.ts index 7ed8b3d..aafae90 100644 --- a/src/commands/builtin/index.ts +++ b/src/commands/builtin/index.ts @@ -236,6 +236,54 @@ export function createTransferCommand(): CommandDefinition { }; } +export function createApprovalsCommand(): CommandDefinition { + return { + name: 'approvals', + description: 'Show pending approval gates for this session', + execute: async (_args, ctx) => { + if (!ctx.services?.getApprovals) { + return notAvailable('Approvals command'); + } + return { + handled: true, + text: await ctx.services.getApprovals(), + }; + }, + }; +} + +export function createApproveCommand(): CommandDefinition { + return { + name: 'approve', + description: 'Approve a pending gate (latest by default, or by id)', + execute: async (args, ctx) => { + if (!ctx.services?.approvePending) { + return notAvailable('Approve command'); + } + return { + handled: true, + text: await ctx.services.approvePending(args.join(' ').trim()), + }; + }, + }; +} + +export function createDenyCommand(): CommandDefinition { + return { + name: 'deny', + description: 'Deny a pending gate (latest by default, optional id and reason)', + execute: async (args, ctx) => { + if (!ctx.services?.denyPending) { + return notAvailable('Deny command'); + } + return { + handled: true, + text: await ctx.services.denyPending(args.join(' ').trim()), + }; + }, + }; +} + export function registerBuiltinCommands(registry: CommandRegistry): void { registry.register(createHelpCommand(registry)); registry.register(createStatusCommand()); @@ -248,4 +296,7 @@ export function registerBuiltinCommands(registry: CommandRegistry): void { registry.register(createElevateCommand()); registry.register(createQueueCommand()); registry.register(createTransferCommand()); + registry.register(createApprovalsCommand()); + registry.register(createApproveCommand()); + registry.register(createDenyCommand()); } diff --git a/src/commands/index.ts b/src/commands/index.ts index d0ad650..cc467bb 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -10,5 +10,8 @@ export { createResetCommand, createQueueCommand, createTransferCommand, + createApprovalsCommand, + createApproveCommand, + createDenyCommand, registerBuiltinCommands, } from './builtin/index.js'; diff --git a/src/commands/types.ts b/src/commands/types.ts index 43fdfb4..3c36dab 100644 --- a/src/commands/types.ts +++ b/src/commands/types.ts @@ -35,4 +35,7 @@ export interface CommandServices { setQueue?: (input: string) => Promise | string; resetQueue?: () => Promise | string; transferSession?: (target: string) => Promise | string; + getApprovals?: () => Promise | string; + approvePending?: (input: string) => Promise | string; + denyPending?: (input: string) => Promise | string; } diff --git a/src/daemon/index.ts b/src/daemon/index.ts index 3c51f90..74d447d 100644 --- a/src/daemon/index.ts +++ b/src/daemon/index.ts @@ -213,7 +213,7 @@ export async function startDaemon(config: Config, options?: StartDaemonOptions): const messageRouter = createMessageRouter({ sessionManager, modelRouter, systemPrompt, toolRegistry, toolExecutor, - config, memoryStore, agentConfigRegistry, agentRouter, sandboxManager, commandRegistry, intentRegistry, routingPolicy, skillRegistry, + config, memoryStore, agentConfigRegistry, agentRouter, sandboxManager, commandRegistry, hookEngine, intentRegistry, routingPolicy, skillRegistry, ...createConfiguredExternalBackends(config), }); channelRegistry.setMessageHandler(messageRouter.handler); diff --git a/src/daemon/routing.test.ts b/src/daemon/routing.test.ts index a64a034..4c0f71a 100644 --- a/src/daemon/routing.test.ts +++ b/src/daemon/routing.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { AgentRouter } from '../agents/router.js'; import { AgentConfigRegistry } from '../agents/registry.js'; +import { HookEngine } from '../hooks/index.js'; import type { ModelTier } from '../models/router.js'; import { createMessageRouter } from './routing.js'; import { AgentOrchestrator } from '../backends/index.js'; @@ -817,6 +818,99 @@ describe('daemon command fast-path integration', () => { const outbound = reply.mock.calls[0]?.[0] as OutboundMessage | undefined; expect(outbound?.text).toContain('Backend: codex'); }); + + it('lists and resolves pending approvals for the current session via /approvals + /approve', async () => { + const processSpy = vi.spyOn(AgentOrchestrator.prototype, 'process'); + const session = { + id: 'telegram:approvals-user', + addMessage: vi.fn(), + getHistory: vi.fn(() => []), + clear: vi.fn(), + replaceHistory: vi.fn(), + getConfig: vi.fn(() => undefined), + setConfig: vi.fn(), + deleteConfig: vi.fn(), + }; + + const hookEngine = new HookEngine({ confirm: [], log: [], silent: [] }); + const pendingPromise = hookEngine.requestConfirmation( + 'shell.exec', + { command: 'rm -rf /tmp/example' }, + { sessionId: session.id, channel: 'telegram', sender: 'approvals-user' }, + ); + const pending = hookEngine.getPendingConfirmations({ sessionId: session.id }); + const pendingId = pending[0]?.id; + if (!pendingId) { + throw new Error('Expected pending approval id'); + } + + const commandRegistry = new CommandRegistry(); + registerBuiltinCommands(commandRegistry); + + const router = createMessageRouter({ + sessionManager: { + getSession: vi.fn(() => session), + } as unknown as MessageRouterDeps['sessionManager'], + modelRouter: { + getAvailableTiers: () => ['fast', 'default', 'complex', 'local'], + getAllLabels: () => ({ fast: 'fast', default: 'default', complex: 'complex', local: 'local' }), + getLabel: (tier: string) => tier, + } as unknown as MessageRouterDeps['modelRouter'], + systemPrompt: 'test prompt', + toolRegistry: { + clone() { return this; }, + register: vi.fn(), + } as unknown as MessageRouterDeps['toolRegistry'], + toolExecutor: {} as unknown as MessageRouterDeps['toolExecutor'], + config: { + agents: { + primary_tier: 'default', + delegation: { + compaction: 'fast', + memory_extraction: 'fast', + classification: 'fast', + tool_summarisation: 'fast', + complex_reasoning: 'complex', + }, + max_delegation_depth: 3, + max_iterations: 10, + }, + compaction: { enabled: false }, + models: { default: { provider: 'anthropic', model: 'claude' } }, + } as unknown as MessageRouterDeps['config'], + commandRegistry, + hookEngine, + }); + + const reply = vi.fn(async (_message: OutboundMessage) => {}); + + await router.handler({ + id: 'approvals-1', + channel: 'telegram', + senderId: 'approvals-user', + text: '/approvals', + timestamp: Date.now(), + metadata: { isCommand: true, command: 'approvals' }, + } as MessageRouterInput, reply); + + const listReply = reply.mock.calls[0]?.[0] as OutboundMessage | undefined; + expect(String(listReply?.text)).toContain('Pending approvals:'); + expect(String(listReply?.text)).toContain(pendingId); + + await router.handler({ + id: 'approvals-2', + channel: 'telegram', + senderId: 'approvals-user', + text: `/approve ${pendingId}`, + timestamp: Date.now(), + metadata: { isCommand: true, command: 'approve', commandArgs: pendingId }, + } as MessageRouterInput, reply); + + const approveReply = reply.mock.calls[1]?.[0] as OutboundMessage | undefined; + expect(String(approveReply?.text)).toContain('Approved: shell.exec'); + await expect(pendingPromise).resolves.toEqual({ approved: true }); + expect(processSpy).not.toHaveBeenCalled(); + }); }); describe('daemon external backend integration', () => { diff --git a/src/daemon/routing.ts b/src/daemon/routing.ts index ee4a53e..89a13be 100644 --- a/src/daemon/routing.ts +++ b/src/daemon/routing.ts @@ -20,6 +20,7 @@ import { AgentConfigRegistry, AgentRouter } from '../agents/index.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'; import { createClientFromConfig } from './models.js'; import { matchReactionPrompt } from '../automation/reactions.js'; import type { SkillRegistry } from '../skills/index.js'; @@ -116,6 +117,7 @@ export function createMessageRouter(deps: { agentRouter?: AgentRouter; sandboxManager?: SandboxManager; commandRegistry?: CommandRegistry; + hookEngine?: HookEngine; intentRegistry?: ComponentRegistry; routingPolicy?: RoutingPolicy; skillRegistry?: SkillRegistry; @@ -936,6 +938,86 @@ export function createMessageRouter(deps: { deps.sessionManager.transferSession(msg.channel, msg.senderId, toFrontend, toUserId); return `Session transferred to ${destinationLabel}`; }, + + getApprovals: () => { + if (!deps.hookEngine) { + return 'Approval gates are not enabled in this runtime.'; + } + const pending = deps.hookEngine.getPendingConfirmations({ sessionId: session.id }); + 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.'; + } + const pending = deps.hookEngine.getPendingConfirmations({ sessionId: session.id }); + 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.'; + } + const pending = deps.hookEngine.getPendingConfirmations({ sessionId: session.id }); + 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}`; + }, }, }); diff --git a/src/hooks/engine.test.ts b/src/hooks/engine.test.ts index d11fdc5..4c8d2dc 100644 --- a/src/hooks/engine.test.ts +++ b/src/hooks/engine.test.ts @@ -87,4 +87,32 @@ describe('HookEngine', () => { args: { cmd: 'ls' }, })); }); + + it('stores request context and supports filtered pending lookups', async () => { + const engine = new HookEngine({ confirm: ['shell.*'], log: [], silent: [] }); + const p1 = engine.requestConfirmation( + 'shell.exec', + { cmd: 'ls' }, + { sessionId: 'telegram:1', channel: 'telegram', sender: '1' }, + ); + const p2 = engine.requestConfirmation( + 'shell.exec', + { cmd: 'pwd' }, + { sessionId: 'discord:2', channel: 'discord', sender: '2' }, + ); + + const telegramPending = engine.getPendingConfirmations({ sessionId: 'telegram:1' }); + expect(telegramPending).toHaveLength(1); + expect(telegramPending[0].channel).toBe('telegram'); + expect(telegramPending[0].sender).toBe('1'); + + const discordPending = engine.getPendingConfirmations({ channel: 'discord' }); + expect(discordPending).toHaveLength(1); + expect(discordPending[0].sessionId).toBe('discord:2'); + + engine.resolveConfirmation(telegramPending[0].id, { approved: true }); + engine.resolveConfirmation(discordPending[0].id, { approved: false, reason: 'Denied' }); + await expect(p1).resolves.toEqual({ approved: true }); + await expect(p2).resolves.toEqual({ approved: false, reason: 'Denied' }); + }); }); diff --git a/src/hooks/engine.ts b/src/hooks/engine.ts index 197ed68..a3c3c55 100644 --- a/src/hooks/engine.ts +++ b/src/hooks/engine.ts @@ -44,7 +44,11 @@ export class HookEngine { return 'silent'; } - async requestConfirmation(tool: string, args: Record): Promise { + async requestConfirmation( + tool: string, + args: Record, + context?: { sessionId?: string; channel?: string; sender?: string }, + ): Promise { const id = randomUUID(); if (this.interactiveConfirmer) { @@ -56,6 +60,9 @@ export class HookEngine { id, tool, args, + sessionId: context?.sessionId, + channel: context?.channel, + sender: context?.sender, resolve, createdAt: new Date(), }; @@ -74,8 +81,23 @@ export class HookEngine { return true; } - getPendingConfirmations(): PendingConfirmation[] { - return Array.from(this.pendingConfirmations.values()); + getPendingConfirmations(filter?: { sessionId?: string; channel?: string; sender?: string }): PendingConfirmation[] { + const pending = Array.from(this.pendingConfirmations.values()); + if (!filter) { + return pending; + } + return pending.filter((item) => { + if (filter.sessionId && item.sessionId !== filter.sessionId) { + return false; + } + if (filter.channel && item.channel !== filter.channel) { + return false; + } + if (filter.sender && item.sender !== filter.sender) { + return false; + } + return true; + }); } clearExpiredConfirmations(maxAgeMs: number = 5 * 60 * 1000): number { diff --git a/src/hooks/types.ts b/src/hooks/types.ts index e33777e..c13aa78 100644 --- a/src/hooks/types.ts +++ b/src/hooks/types.ts @@ -9,6 +9,9 @@ export interface PendingConfirmation { id: string; tool: string; args: Record; + sessionId?: string; + channel?: string; + sender?: string; resolve: (result: HookResult) => void; createdAt: Date; } diff --git a/src/tools/executor.ts b/src/tools/executor.ts index eb61221..9d0b140 100644 --- a/src/tools/executor.ts +++ b/src/tools/executor.ts @@ -221,6 +221,11 @@ export class ToolExecutor { const hookResult = await this.hooks.requestConfirmation( toolName, args as Record, + { + sessionId: context?.sessionId, + channel: context?.channel, + sender: context?.sender, + }, ); auditLogger?.toolApproval({