From 32f067f5bb625d2f0e9e146e993dfc40a2dc9f1c Mon Sep 17 00:00:00 2001 From: hinotoi-agent Date: Mon, 25 May 2026 19:51:54 +0800 Subject: [PATCH] fix(cli): preserve caller context after approval --- src/cli/dispatch.test.ts | 76 ++++++++++++++++++++++++++++++++++++++-- src/cli/dispatch.ts | 40 ++++++++++++++++++--- 2 files changed, 109 insertions(+), 7 deletions(-) diff --git a/src/cli/dispatch.test.ts b/src/cli/dispatch.test.ts index 20ff18707..6a9c58506 100644 --- a/src/cli/dispatch.test.ts +++ b/src/cli/dispatch.test.ts @@ -2,6 +2,32 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; // --- Mocks --- +const approvalState = vi.hoisted(() => ({ + requestApproval: vi.fn(), + approvalHandler: null as + | null + | ((args: { + session: unknown; + payload: Record; + userId: string; + notify: (text: string) => void; + }) => Promise), + registerApprovalHandler: vi.fn( + ( + action: string, + handler: (args: { + session: unknown; + payload: Record; + userId: string; + notify: (text: string) => void; + }) => Promise, + ) => { + if (action === 'cli_command') approvalState.approvalHandler = handler; + }, + ), + observedContexts: [] as CallerContext[], +})); + vi.mock('../log.js', () => ({ log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, })); @@ -29,8 +55,8 @@ vi.mock('./crud.js', () => ({ })); vi.mock('../modules/approvals/index.js', () => ({ - registerApprovalHandler: vi.fn(), - requestApproval: vi.fn(), + registerApprovalHandler: approvalState.registerApprovalHandler, + requestApproval: approvalState.requestApproval, })); // Register a test command so dispatch has something to find @@ -98,6 +124,18 @@ register({ handler: async (args) => ({ echo: args }), }); +register({ + name: 'approval-context-command', + description: 'approval command that records caller context', + resource: 'groups', + access: 'approval', + parseArgs: (raw) => raw, + handler: async (_args, ctx) => { + approvalState.observedContexts.push(ctx); + return { caller: ctx.caller }; + }, +}); + // Commands that return data shaped like real resources (for post-handler filtering tests) register({ name: 'groups-list-data', @@ -152,6 +190,7 @@ import type { CallerContext } from './frame.js'; beforeEach(() => { vi.clearAllMocks(); + approvalState.observedContexts.length = 0; // Default: the four CLI-whitelisted resources with their real scopeFields. const scopeFields: Record = { groups: 'id', @@ -391,6 +430,39 @@ describe('CLI scope enforcement', () => { expect(mockGetContainerConfig).not.toHaveBeenCalled(); }); + it('approval replay preserves the original agent caller context', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetSession.mockReturnValue({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' }); + mockGetAgentGroup.mockReturnValue({ id: 'g1', name: 'Group One' }); + + const ctx = agentCtx(); + const resp = await dispatch({ id: '1', command: 'approval-context-command', args: {} }, ctx); + + expect(resp.ok).toBe(false); + expect(approvalState.requestApproval).toHaveBeenCalledTimes(1); + + const approval = approvalState.requestApproval.mock.calls[0][0] as { payload: Record }; + expect(approval.payload).toEqual({ + frame: { + id: '1', + command: 'approval-context-command', + args: { agent_group_id: 'g1', group: 'g1', id: 'g1' }, + }, + callerContext: ctx, + }); + + expect(approvalState.approvalHandler).toBeTypeOf('function'); + await approvalState.approvalHandler!({ + session: { id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' }, + payload: approval.payload, + userId: 'telegram:admin', + notify: vi.fn(), + }); + + expect(approvalState.observedContexts).toEqual([ctx]); + expect(approvalState.requestApproval).toHaveBeenCalledTimes(1); + }); + // --- Post-handler filtering --- it('group: groups list filters out other groups', async () => { diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 7878a9b57..c35365272 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -14,7 +14,16 @@ import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './fr import { getResource } from './crud.js'; import { lookup } from './registry.js'; -export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise { +type DispatchOptions = { + /** True when a command is being replayed after approval. */ + approved?: boolean; +}; + +export async function dispatch( + req: RequestFrame, + ctx: CallerContext, + opts: DispatchOptions = {}, +): Promise { let cmd = lookup(req.command); // Fallback: if the full command isn't registered, trim the last @@ -101,7 +110,7 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise { +registerApprovalHandler('cli_command', async ({ payload, notify }) => { const frame = payload.frame as RequestFrame; - const response = await dispatch(frame, { caller: 'host' }); + const callerContext = parseCallerContext(payload.callerContext) ?? { caller: 'host' }; + const response = await dispatch(frame, callerContext, { approved: true }); if (response.ok) { const data = typeof response.data === 'string' ? response.data : JSON.stringify(response.data, null, 2); @@ -190,6 +200,26 @@ registerApprovalHandler('cli_command', async ({ session, payload, userId, notify } }); +function parseCallerContext(value: unknown): CallerContext | undefined { + if (!value || typeof value !== 'object') return undefined; + const record = value as Record; + if (record.caller === 'host') return { caller: 'host' }; + if ( + record.caller === 'agent' && + typeof record.sessionId === 'string' && + typeof record.agentGroupId === 'string' && + typeof record.messagingGroupId === 'string' + ) { + return { + caller: 'agent', + sessionId: record.sessionId, + agentGroupId: record.agentGroupId, + messagingGroupId: record.messagingGroupId, + }; + } + return undefined; +} + function err(id: string, code: ErrorCode, message: string): ResponseFrame { return { id, ok: false, error: { code, message } }; }