From e7a701a5356cc8d5674ac40e2deef2fc8929b680 Mon Sep 17 00:00:00 2001 From: qqqys Date: Tue, 7 Jul 2026 17:35:02 +0800 Subject: [PATCH 1/9] fix(channel): Relay ACP permission requests --- packages/channels/base/src/AcpBridge.test.ts | 256 +++++++++++- packages/channels/base/src/AcpBridge.ts | 81 +++- .../channels/base/src/ChannelAgentBridge.ts | 22 ++ .../channels/base/src/ChannelBase.test.ts | 306 +++++++++++++++ packages/channels/base/src/ChannelBase.ts | 371 ++++++++++++++++++ packages/channels/base/src/index.ts | 2 + .../commands/channel/daemon-worker.test.ts | 51 +++ .../cli/src/commands/channel/daemon-worker.ts | 6 + .../cli/src/commands/channel/runtime.test.ts | 79 +++- packages/cli/src/commands/channel/runtime.ts | 46 +++ packages/cli/src/commands/channel/start.ts | 5 + 11 files changed, 1210 insertions(+), 15 deletions(-) diff --git a/packages/channels/base/src/AcpBridge.test.ts b/packages/channels/base/src/AcpBridge.test.ts index 280f51f22f3..5f0ae7cf289 100644 --- a/packages/channels/base/src/AcpBridge.test.ts +++ b/packages/channels/base/src/AcpBridge.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { RequestPermissionResponse } from '@agentclientprotocol/sdk'; import { ACP_EVENT_LOOP_STALL_RESTART_MS, AcpBridge } from './AcpBridge.js'; import { CHANNEL_LOOP_MCP_SERVER_NAME } from './ChannelLoopTools.js'; import type { ChannelLoopToolHandler } from './ChannelAgentBridge.js'; @@ -44,6 +45,13 @@ const child = vi.hoisted(() => { return { instances: [] as MockChild[], + clients: [] as Array<{ + requestPermission: (params: unknown) => Promise; + }>, + connections: [] as Array<{ + initialize: ReturnType; + cancel: ReturnType; + }>, MockChild, spawn: vi.fn(() => { const instance = new MockChild(); @@ -65,9 +73,16 @@ vi.mock('node:stream', () => ({ vi.mock('@agentclientprotocol/sdk', () => ({ PROTOCOL_VERSION: 1, ndJsonStream: vi.fn(() => ({})), - ClientSideConnection: vi.fn().mockImplementation(() => ({ - initialize: vi.fn().mockResolvedValue(undefined), - })), + ClientSideConnection: vi.fn().mockImplementation((createClient) => { + const client = createClient(); + const connection = { + initialize: vi.fn().mockResolvedValue(undefined), + cancel: vi.fn().mockResolvedValue(undefined), + }; + child.clients.push(client); + child.connections.push(connection); + return connection; + }), })); type TestableAcpBridge = AcpBridge & { @@ -91,6 +106,8 @@ type TestableAcpBridge = AcpBridge & { describe('AcpBridge', () => { beforeEach(() => { child.instances.length = 0; + child.clients.length = 0; + child.connections.length = 0; child.spawn.mockClear(); }); @@ -350,4 +367,237 @@ describe('AcpBridge', () => { expect(proc.kill).not.toHaveBeenCalled(); }); + + it('relays ACP permission requests instead of auto-approving them', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + const permissionRequest = vi.fn(); + const permissionResolved = vi.fn(); + bridge.on('permissionRequest', permissionRequest); + bridge.on('permissionResolved', permissionResolved); + + await bridge.start(); + const request = { + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [ + { optionId: 'proceed_once', name: 'Allow' }, + { optionId: 'cancel', name: 'Deny' }, + ], + }; + + const pending = child.clients[0]!.requestPermission(request); + await Promise.resolve(); + + expect(permissionRequest).toHaveBeenCalledTimes(1); + const event = permissionRequest.mock.calls[0]![0]; + expect(event).toMatchObject({ + sessionId: 'session-1', + request, + }); + expect(event.requestId).toMatch(/^acp-permission-/); + + const response = { outcome: { outcome: 'selected', optionId: 'cancel' } }; + await expect( + ( + bridge as unknown as TestableAcpBridge & { + respondToPermission( + requestId: string, + response: typeof response, + ): Promise; + } + ).respondToPermission(event.requestId, response), + ).resolves.toBe(true); + await expect(pending).resolves.toEqual(response); + expect(permissionResolved).toHaveBeenCalledWith({ + requestId: event.requestId, + outcome: response.outcome, + }); + await expect( + ( + bridge as unknown as TestableAcpBridge & { + respondToPermission( + requestId: string, + response: typeof response, + ): Promise; + } + ).respondToPermission(event.requestId, response), + ).resolves.toBe(false); + }); + + it('allows permission request listeners to respond synchronously', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + const response: RequestPermissionResponse = { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }; + bridge.on('permissionRequest', (event) => { + void bridge.respondToPermission(event.requestId, response); + }); + + await bridge.start(); + const pending = child.clients[0]!.requestPermission({ + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'proceed_once', name: 'Allow' }], + }); + + await expect(pending).resolves.toEqual(response); + }); + + it('falls back to the tool call id for permission requests without a session id', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + const permissionRequest = vi.fn(); + bridge.on('permissionRequest', permissionRequest); + + await bridge.start(); + const pending = child.clients[0]!.requestPermission({ + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'cancel', name: 'Deny' }], + }); + await Promise.resolve(); + + const event = permissionRequest.mock.calls[0]![0]; + expect(event.sessionId).toBe('tool-1'); + await bridge.respondToPermission(event.requestId, { + outcome: { outcome: 'cancelled' }, + }); + await expect(pending).resolves.toEqual({ + outcome: { outcome: 'cancelled' }, + }); + }); + + it('resolves matching pending permissions as cancelled when a session is cancelled', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + const permissionRequest = vi.fn(); + const permissionResolved = vi.fn(); + bridge.on('permissionRequest', permissionRequest); + bridge.on('permissionResolved', permissionResolved); + + await bridge.start(); + const first = child.clients[0]!.requestPermission({ + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'cancel', name: 'Deny' }], + }); + const second = child.clients[0]!.requestPermission({ + sessionId: 'session-2', + toolCall: { + toolCallId: 'tool-2', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'cancel', name: 'Deny' }], + }); + await Promise.resolve(); + + const firstEvent = permissionRequest.mock.calls[0]![0]; + const secondEvent = permissionRequest.mock.calls[1]![0]; + await bridge.cancelSession('session-1'); + + expect(child.connections[0]!.cancel).toHaveBeenCalledWith({ + sessionId: 'session-1', + }); + await expect(first).resolves.toEqual({ + outcome: { outcome: 'cancelled' }, + }); + expect(permissionResolved).toHaveBeenCalledWith({ + requestId: firstEvent.requestId, + outcome: { outcome: 'cancelled' }, + }); + expect(permissionResolved).not.toHaveBeenCalledWith({ + requestId: secondEvent.requestId, + outcome: { outcome: 'cancelled' }, + }); + + const response: RequestPermissionResponse = { + outcome: { outcome: 'selected', optionId: 'cancel' }, + }; + await expect( + bridge.respondToPermission(secondEvent.requestId, response), + ).resolves.toBe(true); + await expect(second).resolves.toEqual(response); + }); + + it('resolves pending permissions as cancelled when the ACP child exits', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + const permissionResolved = vi.fn(); + bridge.on('permissionResolved', permissionResolved); + + await bridge.start(); + const pending = child.clients[0]!.requestPermission({ + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'cancel', name: 'Deny' }], + }); + await Promise.resolve(); + + child.instances[0]!.emit('exit', 1, null); + + await expect(pending).resolves.toEqual({ + outcome: { outcome: 'cancelled' }, + }); + expect(permissionResolved).toHaveBeenCalledWith({ + requestId: 'acp-permission-1', + outcome: { outcome: 'cancelled' }, + }); + }); + + it('resolves pending permissions as cancelled on stop', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + + await bridge.start(); + const pending = child.clients[0]!.requestPermission({ + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'cancel', name: 'Deny' }], + }); + await Promise.resolve(); + + bridge.stop(); + + await expect(pending).resolves.toEqual({ + outcome: { outcome: 'cancelled' }, + }); + }); }); diff --git a/packages/channels/base/src/AcpBridge.ts b/packages/channels/base/src/AcpBridge.ts index 1271c4df1a7..c456453e2a4 100644 --- a/packages/channels/base/src/AcpBridge.ts +++ b/packages/channels/base/src/AcpBridge.ts @@ -77,6 +77,14 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { private readonly channelLoopToolHandlers: ChannelLoopToolHandler[] = []; private channelLoopMcpRegistered = false; private channelLoopMcpRegistration: Promise | null = null; + private permissionCounter = 0; + private readonly pendingPermissions = new Map< + string, + { + sessionId: string; + resolve: (response: RequestPermissionResponse) => void; + } + >(); constructor(options: AcpBridgeOptions) { super(); @@ -109,7 +117,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { this.child.stderr?.on('data', (data: Buffer) => { const msg = data.toString().trim(); if (msg) { - process.stderr.write(`[AcpBridge] ${msg}\n`); + process.stderr.write(`[AcpBridge] ${sanitizeLogText(msg, 4096)}\n`); this.maybeKillOnEventLoopStall(msg); } }); @@ -120,6 +128,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { ); // Do not emit sessionDied here: a full ACP process exit is handled by // channel start crash recovery, which reloads the persisted sessions. + this.resolvePendingPermissions(); this.connection = null; this.child = null; this.emit('disconnected', code, signal); @@ -147,15 +156,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { requestPermission: async ( params: RequestPermissionRequest, - ): Promise => { - // Auto-approve for now; Phase 5 will add interactive approval - const options = Array.isArray(params.options) ? params.options : []; - const optionId = - options.find((o) => o.optionId === 'proceed_once')?.optionId || - options[0]?.optionId || - 'proceed_once'; - return { outcome: { outcome: 'selected', optionId } }; - }, + ): Promise => this.requestPermission(params), extMethod: async ( method: string, @@ -245,10 +246,32 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { async cancelSession(sessionId: string): Promise { const conn = this.ensureConnection(); - await conn.cancel({ sessionId }); + try { + await conn.cancel({ sessionId }); + } finally { + this.resolvePendingPermissions(sessionId); + } + } + + async respondToPermission( + requestId: string, + response: RequestPermissionResponse, + ): Promise { + const pending = this.pendingPermissions.get(requestId); + if (!pending) { + return false; + } + this.pendingPermissions.delete(requestId); + pending.resolve(response); + this.emit('permissionResolved', { + requestId, + outcome: response.outcome, + }); + return true; } stop(): void { + this.resolvePendingPermissions(); if (this.child) { this.child.kill(); this.child = null; @@ -319,6 +342,42 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { return this.connection; } + private requestPermission( + request: RequestPermissionRequest, + ): Promise { + const requestId = `acp-permission-${++this.permissionCounter}`; + const sessionId = + typeof request.sessionId === 'string' && request.sessionId.length > 0 + ? request.sessionId + : request.toolCall.toolCallId; + + return new Promise((resolve) => { + this.pendingPermissions.set(requestId, { sessionId, resolve }); + this.emit('permissionRequest', { + requestId, + sessionId, + request, + }); + }); + } + + private resolvePendingPermissions(sessionId?: string): void { + const response: RequestPermissionResponse = { + outcome: { outcome: 'cancelled' }, + }; + for (const [requestId, pending] of this.pendingPermissions) { + if (sessionId !== undefined && pending.sessionId !== sessionId) { + continue; + } + this.pendingPermissions.delete(requestId); + pending.resolve(response); + this.emit('permissionResolved', { + requestId, + outcome: response.outcome, + }); + } + } + private maybeKillOnEventLoopStall(stderr: string): void { const match = ACP_EVENT_LOOP_STALL_RE.exec(stderr); if (!match) return; diff --git a/packages/channels/base/src/ChannelAgentBridge.ts b/packages/channels/base/src/ChannelAgentBridge.ts index a4da0249d2d..1adb7a90ee4 100644 --- a/packages/channels/base/src/ChannelAgentBridge.ts +++ b/packages/channels/base/src/ChannelAgentBridge.ts @@ -1,3 +1,8 @@ +import type { + RequestPermissionRequest, + RequestPermissionResponse, +} from '@agentclientprotocol/sdk'; + export interface AvailableCommand { name: string; description: string; @@ -47,10 +52,23 @@ export interface SessionDiedEvent { reason?: string; } +export interface PermissionRequestEvent { + requestId: string; + sessionId: string; + request: RequestPermissionRequest; +} + +export interface PermissionResolvedEvent { + requestId: string; + outcome?: RequestPermissionResponse['outcome']; +} + interface ChannelAgentBridgeEventMap { sessionDied: [SessionDiedEvent]; textChunk: [sessionId: string, chunk: string]; toolCall: [ToolCallEvent]; + permissionRequest: [PermissionRequestEvent]; + permissionResolved: [PermissionResolvedEvent]; } export interface BridgeSessionInfo { @@ -78,6 +96,10 @@ export interface ChannelAgentBridge { options?: { imageBase64?: string; imageMimeType?: string }, ): Promise; cancelSession(sessionId: string): Promise; + respondToPermission?( + requestId: string, + response: RequestPermissionResponse, + ): Promise; shellCommand?( sessionId: string, command: string, diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index d68def41151..3cda995f38d 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -23,6 +23,7 @@ class TestChannel extends ChannelBase { proactive: Array<{ chatId: string; text: string }> = []; proactiveSupported = false; proactiveTargetSupported: boolean | undefined; + sendMessageError?: Error; connected = false; toolCalls: Array<{ chatId: string; event: unknown }> = []; taskEvents: ChannelTaskLifecycleEvent[] = []; @@ -43,6 +44,9 @@ class TestChannel extends ChannelBase { this.connected = true; } async sendMessage(chatId: string, text: string) { + if (this.sendMessageError) { + throw this.sendMessageError; + } this.sent.push({ chatId, text }); } disconnect() { @@ -133,6 +137,7 @@ function createBridge(): ChannelAgentBridge { isConnected: true, availableCommands: [], setBridge: vi.fn(), + respondToPermission: vi.fn().mockResolvedValue(true), registerChannelLoopToolHandler: vi.fn((handler: ChannelLoopToolHandler) => { channelLoopToolHandler = handler; }), @@ -228,6 +233,306 @@ describe('ChannelBase', () => { }); }); + describe('permission relay', () => { + function respondToPermissionMock(): ReturnType { + return ( + bridge as unknown as { + respondToPermission: ReturnType; + } + ).respondToPermission; + } + + function emitPermission( + sessionId: string, + requestId: string, + options = [ + { + optionId: 'proceed_always_project', + kind: 'allow_always', + name: 'Always Allow in project', + }, + { optionId: 'proceed_once', kind: 'allow_once', name: 'Allow once' }, + { optionId: 'cancel', kind: 'reject_once', name: 'Deny' }, + ], + ): void { + (bridge as unknown as EventEmitter).emit('permissionRequest', { + requestId, + sessionId, + request: { + toolCall: { + toolCallId: `tool-${requestId}`, + kind: 'shell', + title: `Run ${requestId}`, + rawInput: { command: 'echo secret-token' }, + }, + options, + }, + }); + } + + async function startSession( + ch: TestChannel, + env: Partial = {}, + ): Promise { + await ch.handleInbound(envelope({ text: 'run tests', ...env })); + const results = (bridge.newSession as ReturnType).mock + .results; + return results[results.length - 1]!.value as string; + } + + it('sends permission requests to the owning chat and approves with /approve', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + + expect(ch.sent.at(-1)?.chatId).toBe('chat1'); + expect(ch.sent.at(-1)?.text).toContain('需要授权执行命令'); + expect(ch.sent.at(-1)?.text).toContain('命令：'); + expect(ch.sent.at(-1)?.text).toContain('Run req-1'); + expect(ch.sent.at(-1)?.text).toContain('/approve 本次允许'); + expect(ch.sent.at(-1)?.text).toContain('/approve-always 总是允许'); + expect(ch.sent.at(-1)?.text).toContain('/deny 拒绝'); + expect(ch.sent.at(-1)?.text).not.toContain('Request: req-1'); + expect(ch.sent.at(-1)?.text).not.toContain('proceed_once'); + expect(ch.sent.at(-1)?.text).not.toContain('secret-token'); + + await ch.handleInbound(envelope({ text: '/approve' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }); + expect(ch.sent.at(-1)?.text).toBe('Permission approved.'); + }); + + it('requires an explicit request id when multiple permissions are pending', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + emitPermission(sessionId, 'req-2'); + + await ch.handleInbound(envelope({ text: '/approve' })); + + expect(ch.sent.at(-1)?.text).toContain( + 'Multiple permission requests are pending', + ); + expect(ch.sent.at(-1)?.text).toContain('req-1'); + expect(ch.sent.at(-1)?.text).toContain('req-2'); + expect(ch.sent.at(-1)?.text).toContain('req-1: Run req-1'); + expect(ch.sent.at(-1)?.text).toContain('req-2: Run req-2'); + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(respondToPermissionMock()).toHaveBeenCalledTimes(1); + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }); + }); + + it('does not fall back to another pending request when an explicit id is wrong', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + emitPermission(sessionId, 'req-2'); + + await ch.handleInbound(envelope({ text: '/approve missing-request' })); + + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + }); + + it('does not answer permission requests from another chat', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch, { chatId: 'chat2' }); + emitPermission(sessionId, 'req-chat2'); + + await ch.handleInbound( + envelope({ chatId: 'chat1', text: '/approve req-chat2' }), + ); + + expect(ch.sent.at(-1)?.chatId).toBe('chat1'); + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + }); + + it('gates shared-session permission responses to authorized senders', async () => { + const ch = createChannel({ + allowedUsers: ['boss'], + groupPolicy: 'open', + sessionScope: 'thread', + }); + const sessionId = await startSession(ch, { + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'boss', + threadId: 'thread-1', + }); + emitPermission(sessionId, 'req-1'); + + await ch.handleInbound( + envelope({ + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'rando', + text: '/approve req-1', + threadId: 'thread-1', + }), + ); + + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + expect(ch.sent.at(-1)?.text).toContain('Only authorized members'); + + await ch.handleInbound( + envelope({ + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'boss', + text: '/approve req-1', + threadId: 'thread-1', + }), + ); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }); + }); + + it('uses ACP option kinds for approval and denial', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1', [ + { optionId: 'always', kind: 'allow_always', name: 'Allow always' }, + { optionId: 'once', kind: 'allow_once', name: 'Allow once' }, + { optionId: 'never', kind: 'reject_always', name: 'Deny always' }, + { optionId: 'reject', kind: 'reject_once', name: 'Deny once' }, + ]); + + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'once' }, + }); + + emitPermission(sessionId, 'req-2', [ + { optionId: 'always', kind: 'allow_always', name: 'Allow always' }, + { optionId: 'never', kind: 'reject_always', name: 'Deny always' }, + ]); + + await ch.handleInbound(envelope({ text: '/deny req-2' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-2', { + outcome: { outcome: 'cancelled' }, + }); + }); + + it('supports explicit approve-always for persistent permission grants', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1', [ + { + optionId: 'proceed_always_user', + kind: 'allow_always', + name: 'Always Allow for user', + }, + { + optionId: 'proceed_always_project', + kind: 'allow_always', + name: 'Always Allow in project', + }, + { optionId: 'once', kind: 'allow_once', name: 'Allow once' }, + ]); + + expect(ch.sent.at(-1)?.text).toContain( + '/approve-always 总是允许（当前项目）', + ); + + await ch.handleInbound(envelope({ text: '/approve-always req-1' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'proceed_always_project' }, + }); + }); + + it('falls back to user-scope approve-always when project scope is unavailable', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1', [ + { + optionId: 'proceed_always_user', + kind: 'allow_always', + name: 'Always Allow for user', + }, + { optionId: 'once', kind: 'allow_once', name: 'Allow once' }, + ]); + + expect(ch.sent.at(-1)?.text).toContain( + '/approve-always 总是允许（当前用户）', + ); + + await ch.handleInbound(envelope({ text: '/approve-always req-1' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'proceed_always_user' }, + }); + }); + + it('allows approve-always without a request id when one request is pending', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1', [ + { optionId: 'always', kind: 'allow_always', name: 'Allow always' }, + { optionId: 'once', kind: 'allow_once', name: 'Allow once' }, + ]); + + await ch.handleInbound(envelope({ text: '/approve-always' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'always' }, + }); + }); + + it('clears pending permission requests when the session is cleared', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + + await ch.handleInbound(envelope({ text: '/clear' })); + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + }); + + it('cancels the permission request when the relay message cannot be sent', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + ch.sendMessageError = new Error('send failed'); + + emitPermission(sessionId, 'req-1'); + await vi.waitFor(() => + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'cancelled' }, + }), + ); + ch.sendMessageError = undefined; + + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + }); + }); + describe('group history backfill', () => { it('does not record unmentioned group messages when groupHistoryLimit is absent', async () => { const ch = createChannel({ @@ -767,6 +1072,7 @@ describe('ChannelBase', () => { expect(ch.sent).toHaveLength(1); expect(ch.sent[0]!.text).toContain('/help'); expect(ch.sent[0]!.text).toContain('/clear'); + expect(ch.sent[0]!.text).toContain('/approve-always [request-id]'); expect(ch.sent[0]!.text).not.toContain('/cancel'); expect(bridge.prompt).not.toHaveBeenCalled(); }); diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index 624713756a4..2b5c0203129 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -33,6 +33,8 @@ import type { ChannelAgentBridge, ChannelLoopToolCreateInput, ChannelLoopToolResult, + PermissionRequestEvent, + PermissionResolvedEvent, SessionDiedEvent, ToolCallEvent, } from './ChannelAgentBridge.js'; @@ -92,6 +94,17 @@ export interface ChannelLoopPromptOptions { /** Handler for a slash command. Return true if handled, false to forward to agent. */ type CommandHandler = (envelope: Envelope, args: string) => Promise; +type PendingPermission = { + requestId: string; + sessionId: string; + target: SessionTarget; + request: PermissionRequestEvent['request']; +}; +type PermissionOption = PermissionRequestEvent['request']['options'][number]; +type PendingPermissionLookup = + | { kind: 'found'; pending: PendingPermission } + | { kind: 'none'; explicit: boolean } + | { kind: 'ambiguous'; requestIds: string[] }; type ActivePrompt = { cancelled: boolean; cancelPending?: boolean; @@ -204,6 +217,22 @@ export abstract class ChannelBase { ): void => { this.onSessionDied(event.sessionId); }; + private readonly bridgePermissionRequestListener = ( + event: PermissionRequestEvent, + ): void => { + void this.dispatchPermissionRequest(event).catch((err: unknown) => { + process.stderr.write( + `[${this.name}] permission relay failed for request ${sanitizeLogText(event.requestId, 128)}: ${this.lifecycleError(err)}\n`, + ); + }); + }; + private readonly bridgePermissionResolvedListener = ( + event: PermissionResolvedEvent, + ): void => { + this.dispatchPermissionResolved(event); + }; + private readonly pendingPermissions = new Map(); + private readonly pendingPermissionsByChat = new Map(); private readonly channelLoopToolHandler = { canHandle: (sessionId: string) => this.router.getTarget(sessionId)?.channelName === this.name, @@ -240,6 +269,49 @@ export abstract class ChannelBase { this.onToolCall(chatId, event); } + async dispatchPermissionRequest( + event: PermissionRequestEvent, + ): Promise { + const target = this.router.getTarget(event.sessionId); + if (!target || target.channelName !== this.name) { + return; + } + this.removePendingPermission(event.requestId); + const pending: PendingPermission = { + requestId: event.requestId, + sessionId: event.sessionId, + target, + request: event.request, + }; + this.pendingPermissions.set(event.requestId, pending); + const chatKey = this.permissionChatKey(target); + const requestIds = this.pendingPermissionsByChat.get(chatKey) ?? []; + requestIds.push(event.requestId); + this.pendingPermissionsByChat.set(chatKey, requestIds); + try { + await this.sendMessage( + target.chatId, + this.formatPermissionRequest(pending), + ); + } catch (err) { + this.removePendingPermission(event.requestId); + try { + await this.bridge.respondToPermission?.(event.requestId, { + outcome: { outcome: 'cancelled' }, + }); + } catch (respondErr) { + process.stderr.write( + `[${this.name}] permission cancellation failed for request ${sanitizeLogText(event.requestId, 128)}: ${this.lifecycleError(respondErr)}\n`, + ); + } + throw err; + } + } + + dispatchPermissionResolved(event: PermissionResolvedEvent): void { + this.removePendingPermission(event.requestId); + } + constructor( name: string, config: ChannelConfig, @@ -924,16 +996,21 @@ export abstract class ChannelBase { onSessionDied(sessionId: string): void { this.router.removeSessionId(sessionId); this.instructedSessions.delete(sessionId); + this.removePendingPermissionsForSession(sessionId); } private attachBridgeEvents(bridge: ChannelAgentBridge): void { bridge.on('toolCall', this.bridgeToolCallListener); bridge.on('sessionDied', this.bridgeSessionDiedListener); + bridge.on('permissionRequest', this.bridgePermissionRequestListener); + bridge.on('permissionResolved', this.bridgePermissionResolvedListener); } private detachBridgeEvents(bridge: ChannelAgentBridge): void { bridge.off('toolCall', this.bridgeToolCallListener); bridge.off('sessionDied', this.bridgeSessionDiedListener); + bridge.off('permissionRequest', this.bridgePermissionRequestListener); + bridge.off('permissionResolved', this.bridgePermissionResolvedListener); } /** @@ -1036,6 +1113,284 @@ export abstract class ChannelBase { }); } + private permissionChatKey( + target: Pick, + ) { + return `${target.chatId}\0${target.threadId ?? ''}`; + } + + private pendingPermissionIdsForChatKey(chatKey: string): string[] { + const requestIds = this.pendingPermissionsByChat.get(chatKey); + if (!requestIds) { + return []; + } + const live = requestIds.filter((id) => this.pendingPermissions.has(id)); + if (live.length === 0) { + this.pendingPermissionsByChat.delete(chatKey); + } else if (live.length !== requestIds.length) { + this.pendingPermissionsByChat.set(chatKey, live); + } + return live; + } + + private removePendingPermission(requestId: string): void { + const pending = this.pendingPermissions.get(requestId); + if (!pending) { + return; + } + this.pendingPermissions.delete(requestId); + const chatKey = this.permissionChatKey(pending.target); + const requestIds = this.pendingPermissionsByChat.get(chatKey); + if (!requestIds) { + return; + } + const remaining = requestIds.filter((id) => id !== requestId); + if (remaining.length === 0) { + this.pendingPermissionsByChat.delete(chatKey); + } else { + this.pendingPermissionsByChat.set(chatKey, remaining); + } + } + + private removePendingPermissionsForSession(sessionId: string): void { + const requestIds = Array.from(this.pendingPermissions) + .filter(([, pending]) => pending.sessionId === sessionId) + .map(([requestId]) => requestId); + for (const requestId of requestIds) { + this.removePendingPermission(requestId); + } + } + + private pendingPermissionForEnvelope( + envelope: Envelope, + args: string, + ): PendingPermissionLookup { + const trimmed = args.trim(); + if (trimmed) { + const explicit = this.pendingPermissions.get(trimmed); + if ( + explicit && + explicit.target.chatId === envelope.chatId && + explicit.target.threadId === envelope.threadId + ) { + return { kind: 'found', pending: explicit }; + } + return { kind: 'none', explicit: true }; + } + const requestIds = this.pendingPermissionIdsForChatKey( + this.permissionChatKey(envelope), + ); + if (requestIds.length === 0) { + return { kind: 'none', explicit: false }; + } + if (requestIds.length > 1) { + return { kind: 'ambiguous', requestIds }; + } + const pending = this.pendingPermissions.get(requestIds[0]!); + return pending + ? { kind: 'found', pending } + : { kind: 'none', explicit: false }; + } + + private formatPermissionRequest(pending: PendingPermission): string { + const { toolCall } = pending.request; + const title = sanitizeQuotedText(toolCall.title || 'Tool use', 160); + const alwaysOption = this.approvalAlwaysOption(pending); + const replies = [ + '/approve 本次允许', + ...(alwaysOption ? [`/approve-always ${alwaysOption.label}`] : []), + '/deny 拒绝', + ]; + const lines = [ + '需要授权执行命令', + '', + '命令：', + title, + '', + '可回复：', + ...replies, + ]; + return lines.join('\n'); + } + + private approvalOptionId(pending: PendingPermission): string | undefined { + const options = pending.request.options; + return ( + options.find((option) => option.kind === 'allow_once')?.optionId ?? + options.find( + (option) => + option.optionId === 'proceed_once' && + (option as { kind?: string }).kind === undefined, + )?.optionId + ); + } + + private approvalAlwaysOption( + pending: PendingPermission, + ): { optionId: string; label: string } | undefined { + const options = pending.request.options.filter( + (option) => option.kind === 'allow_always', + ); + const option = + this.findScopedAlwaysOption(options, 'project') ?? + this.findScopedAlwaysOption(options, 'user') ?? + options[0]; + if (!option) { + return undefined; + } + return { + optionId: option.optionId, + label: this.approvalAlwaysLabel(option), + }; + } + + private findScopedAlwaysOption( + options: PermissionOption[], + scope: 'project' | 'user', + ): PermissionOption | undefined { + return options.find( + (option) => + option.optionId === `proceed_always_${scope}` || + option.optionId.endsWith(`_${scope}`), + ); + } + + private approvalAlwaysLabel(option: PermissionOption): string { + if ( + option.optionId === 'proceed_always_project' || + option.optionId.endsWith('_project') + ) { + return '总是允许（当前项目）'; + } + if ( + option.optionId === 'proceed_always_user' || + option.optionId.endsWith('_user') + ) { + return '总是允许（当前用户）'; + } + return '总是允许'; + } + + private denialResponse(pending: PendingPermission): { + outcome: + | { outcome: 'selected'; optionId: string } + | { outcome: 'cancelled' }; + } { + const option = + pending.request.options.find( + (candidate) => candidate.kind === 'reject_once', + ) ?? + pending.request.options.find( + (candidate) => + candidate.optionId === 'cancel' && + (candidate as { kind?: string }).kind === undefined, + ); + if (option) { + return { outcome: { outcome: 'selected', optionId: option.optionId } }; + } + return { outcome: { outcome: 'cancelled' } }; + } + + private async handlePermissionResponseCommand( + envelope: Envelope, + args: string, + decision: 'approve' | 'approve-always' | 'deny', + ): Promise { + if (!this.isAuthorizedForSharedSession(envelope)) { + await this.sendMessage( + envelope.chatId, + 'Only authorized members can answer permission requests in this shared session.', + ); + return true; + } + const lookup = this.pendingPermissionForEnvelope(envelope, args); + if (lookup.kind === 'ambiguous') { + const requestList = lookup.requestIds + .slice(0, 6) + .map((id) => { + const pending = this.pendingPermissions.get(id); + const title = pending + ? `: ${sanitizeQuotedText(pending.request.toolCall.title || 'Tool use', 160)}` + : ''; + return `- ${sanitizeQuotedText(id, 128)}${title}`; + }) + .join('\n'); + await this.sendMessage( + envelope.chatId, + `Multiple permission requests are pending for this chat. Reply with /${decision} .\n${requestList}`, + ); + return true; + } + if (lookup.kind === 'none') { + await this.sendMessage( + envelope.chatId, + lookup.explicit + ? 'No pending permission request with that id for this chat.' + : 'No pending permission request for this chat.', + ); + return true; + } + if (!this.bridge.respondToPermission) { + await this.sendMessage( + envelope.chatId, + 'Permission relay is not available for this session.', + ); + return true; + } + + const { pending } = lookup; + const response = (() => { + if (decision === 'deny') { + return this.denialResponse(pending); + } + const optionId = + decision === 'approve' + ? this.approvalOptionId(pending) + : this.approvalAlwaysOption(pending)?.optionId; + return optionId + ? { outcome: { outcome: 'selected' as const, optionId } } + : undefined; + })(); + if (!response) { + await this.sendMessage( + envelope.chatId, + decision === 'approve-always' + ? 'This permission request has no always-allow option.' + : 'This permission request has no approvable option.', + ); + return true; + } + + let accepted: boolean; + try { + accepted = await this.bridge.respondToPermission( + pending.requestId, + response, + ); + } catch (err) { + process.stderr.write( + `[${this.name}] permission response failed for request ${sanitizeLogText(pending.requestId, 128)}: ${this.lifecycleError(err)}\n`, + ); + await this.sendMessage( + envelope.chatId, + 'Failed to answer the permission request.', + ); + return true; + } + this.removePendingPermission(pending.requestId); + await this.sendMessage( + envelope.chatId, + accepted + ? decision === 'approve' + ? 'Permission approved.' + : decision === 'approve-always' + ? 'Permission approved always.' + : 'Permission denied.' + : 'Permission request is no longer pending.', + ); + return true; + } + /** Register shared slash commands. Called from constructor. */ private registerSharedCommands(): void { const doClear = async (envelope: Envelope): Promise => { @@ -1067,6 +1422,7 @@ export abstract class ChannelBase { id, (this.sessionGenerations.get(id) ?? 0) + 1, ); + this.removePendingPermissionsForSession(id); // Cancel an in-flight turn (and drop its buffered follow-ups) before // purging, so a running prompt can't deliver a stale response into — // or resurrect via collect-drain — the just-cleared session. @@ -1183,6 +1539,15 @@ export abstract class ChannelBase { this.registerCommand('clear', clearHandler); this.registerCommand('reset', clearHandler); this.registerCommand('new', clearHandler); + this.registerCommand('approve', (envelope, args) => + this.handlePermissionResponseCommand(envelope, args, 'approve'), + ); + this.registerCommand('approve-always', (envelope, args) => + this.handlePermissionResponseCommand(envelope, args, 'approve-always'), + ); + this.registerCommand('deny', (envelope, args) => + this.handlePermissionResponseCommand(envelope, args, 'deny'), + ); // Read-only: report the current (possibly group-shared) session and workspace. // For a shared session, gate it to authorized senders like /clear — /who @@ -1367,6 +1732,9 @@ export abstract class ChannelBase { : '/clear — Clear your session (aliases: /reset, /new)', '/who — Show current session & workspace', '/status — Show session info', + '/approve [request-id] — Approve a pending permission request', + '/approve-always [request-id] — Always approve a pending permission request', + '/deny [request-id] — Deny a pending permission request', '/remember-channel — Save memory for this chat', '/channel-memory — Show memory for this chat', '/forget-channel confirm — Clear memory for this chat', @@ -1378,6 +1746,9 @@ export abstract class ChannelBase { 'clear', 'reset', 'new', + 'approve', + 'approve-always', + 'deny', 'who', 'status', 'remember-channel', diff --git a/packages/channels/base/src/index.ts b/packages/channels/base/src/index.ts index 3c8a19e196b..3780539a8ac 100644 --- a/packages/channels/base/src/index.ts +++ b/packages/channels/base/src/index.ts @@ -7,6 +7,8 @@ export type { ChannelLoopToolCreateInput, ChannelLoopToolHandler, ChannelLoopToolResult, + PermissionRequestEvent, + PermissionResolvedEvent, SessionDiedEvent, ToolCallEvent, } from './ChannelAgentBridge.js'; diff --git a/packages/cli/src/commands/channel/daemon-worker.test.ts b/packages/cli/src/commands/channel/daemon-worker.test.ts index 7bf2f92ecc9..ba1c472c5d9 100644 --- a/packages/cli/src/commands/channel/daemon-worker.test.ts +++ b/packages/cli/src/commands/channel/daemon-worker.test.ts @@ -6,6 +6,7 @@ const mockLoadChannelsFromExtensions = vi.hoisted(() => vi.fn()); const mockParseConfiguredChannels = vi.hoisted(() => vi.fn()); const mockCreateChannel = vi.hoisted(() => vi.fn()); const mockRegisterToolCallDispatch = vi.hoisted(() => vi.fn()); +const mockRegisterPermissionRelay = vi.hoisted(() => vi.fn()); const mockRegisterSessionCleanup = vi.hoisted(() => vi.fn()); const mockSessionsPath = vi.hoisted(() => vi.fn(() => '/tmp/sessions.json')); const mockLoadSettings = vi.hoisted(() => @@ -71,6 +72,7 @@ const mockBridgeNewSession = vi.hoisted(() => vi.fn()); const mockBridgeLoadSession = vi.hoisted(() => vi.fn()); const mockBridgePrompt = vi.hoisted(() => vi.fn()); const mockBridgeCancelSession = vi.hoisted(() => vi.fn()); +const mockBridgeRespondToPermission = vi.hoisted(() => vi.fn()); const mockBridgeShellCommand = vi.hoisted(() => vi.fn()); const mockBridgeGetAvailableCommands = vi.hoisted(() => vi.fn(() => [])); const mockDaemonChannelBridge = vi.hoisted(() => @@ -85,6 +87,7 @@ const mockDaemonChannelBridge = vi.hoisted(() => loadSession: mockBridgeLoadSession, prompt: mockBridgePrompt, cancelSession: mockBridgeCancelSession, + respondToPermission: mockBridgeRespondToPermission, shellCommand: mockBridgeShellCommand, start: mockBridgeStart, stop: mockBridgeStop, @@ -128,6 +131,7 @@ vi.mock('./runtime.js', () => ({ loadChannelsConfig: mockLoadChannelsConfig, loadChannelsFromExtensions: mockLoadChannelsFromExtensions, parseConfiguredChannels: mockParseConfiguredChannels, + registerPermissionRelay: mockRegisterPermissionRelay, registerSessionCleanup: mockRegisterSessionCleanup, registerToolCallDispatch: mockRegisterToolCallDispatch, selectFirstModel: mockSelectFirstModel, @@ -402,6 +406,48 @@ describe('createDaemonChannelBridgeFacade', () => { expect(listSessions).toHaveBeenCalled(); }); + it('forwards permission responses when present on bridge', async () => { + const respondToPermission = vi.fn().mockResolvedValue(true); + const bridge = { + availableCommands: [], + on: mockBridgeOn, + off: mockBridgeOff, + newSession: mockBridgeNewSession, + loadSession: mockBridgeLoadSession, + prompt: mockBridgePrompt, + cancelSession: mockBridgeCancelSession, + respondToPermission, + }; + + const facade = createDaemonChannelBridgeFacade(bridge, { + exposeShellCommand: false, + }); + + const response = { outcome: { outcome: 'cancelled' as const } }; + await expect(facade.respondToPermission?.('req-1', response)).resolves.toBe( + true, + ); + expect(respondToPermission).toHaveBeenCalledWith('req-1', response); + }); + + it('omits permission responses when absent on bridge', () => { + const bridge = { + availableCommands: [], + on: mockBridgeOn, + off: mockBridgeOff, + newSession: mockBridgeNewSession, + loadSession: mockBridgeLoadSession, + prompt: mockBridgePrompt, + cancelSession: mockBridgeCancelSession, + }; + + const facade = createDaemonChannelBridgeFacade(bridge, { + exposeShellCommand: false, + }); + + expect('respondToPermission' in facade).toBe(false); + }); + it('omits listSessions when absent on bridge', () => { const bridge = { availableCommands: [], @@ -485,6 +531,11 @@ describe('runChannelDaemonWorker', () => { router: mockSessionRouter.mock.results[0]!.value, }), ); + expect(mockRegisterPermissionRelay).toHaveBeenCalledWith( + bridgeFacade, + mockSessionRouter.mock.results[0]!.value, + expect.any(Map), + ); expect(mockResolveProxyUrl).toHaveBeenCalledWith( undefined, 'http://settings-proxy:8080', diff --git a/packages/cli/src/commands/channel/daemon-worker.ts b/packages/cli/src/commands/channel/daemon-worker.ts index 9c2901de768..026a184285e 100644 --- a/packages/cli/src/commands/channel/daemon-worker.ts +++ b/packages/cli/src/commands/channel/daemon-worker.ts @@ -31,6 +31,7 @@ import { loadChannelsConfig, loadChannelsFromExtensions, parseConfiguredChannels, + registerPermissionRelay, registerSessionCleanup, registerToolCallDispatch, selectFirstModel, @@ -151,6 +152,10 @@ export function createDaemonChannelBridgeFacade( cancelSession: bridge.cancelSession.bind(bridge), }; + if (bridge.respondToPermission) { + facade.respondToPermission = bridge.respondToPermission.bind(bridge); + } + if (bridge.getAvailableCommands) { facade.getAvailableCommands = bridge.getAvailableCommands.bind(bridge); } @@ -343,6 +348,7 @@ export async function runChannelDaemonWorker( ); } registerToolCallDispatch(bridgeFacade, createdRouter, channels); + registerPermissionRelay(bridgeFacade, createdRouter, channels); registerSessionCleanup(bridgeFacade, createdRouter, channels); for (const [name, channel] of channels) { diff --git a/packages/cli/src/commands/channel/runtime.test.ts b/packages/cli/src/commands/channel/runtime.test.ts index 1fc00509069..6a001802e80 100644 --- a/packages/cli/src/commands/channel/runtime.test.ts +++ b/packages/cli/src/commands/channel/runtime.test.ts @@ -1,5 +1,6 @@ +import { EventEmitter } from 'node:events'; import { describe, expect, it, vi } from 'vitest'; -import { parseConfiguredChannels } from './runtime.js'; +import { parseConfiguredChannels, registerPermissionRelay } from './runtime.js'; vi.mock('./channel-registry.js', () => ({ getPlugin: async (type: string) => @@ -42,3 +43,79 @@ describe('parseConfiguredChannels', () => { ]); }); }); + +describe('registerPermissionRelay', () => { + function createBridge() { + const emitter = new EventEmitter(); + return Object.assign(emitter, { + availableCommands: [], + newSession: vi.fn(), + loadSession: vi.fn(), + prompt: vi.fn(), + cancelSession: vi.fn(), + respondToPermission: vi.fn().mockResolvedValue(true), + }); + } + + it('cancels permission requests when no route exists', async () => { + const bridge = createBridge(); + const router = { getTarget: vi.fn() }; + + registerPermissionRelay(bridge, router as never, new Map()); + bridge.emit('permissionRequest', { + requestId: 'req-1', + sessionId: 'missing-session', + request: { + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [], + }, + }); + + await vi.waitFor(() => + expect(bridge.respondToPermission).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'cancelled' }, + }), + ); + }); + + it('cancels permission requests when channel dispatch fails', async () => { + const bridge = createBridge(); + const router = { + getTarget: vi.fn(() => ({ channelName: 'telegram', chatId: 'chat1' })), + }; + const channel = { + dispatchPermissionRequest: vi + .fn() + .mockRejectedValue(new Error('send failed')), + dispatchPermissionResolved: vi.fn(), + }; + + registerPermissionRelay( + bridge, + router as never, + new Map([['telegram', channel as never]]), + ); + bridge.emit('permissionRequest', { + requestId: 'req-1', + sessionId: 'session-1', + request: { + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [], + }, + }); + + await vi.waitFor(() => + expect(bridge.respondToPermission).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'cancelled' }, + }), + ); + }); +}); diff --git a/packages/cli/src/commands/channel/runtime.ts b/packages/cli/src/commands/channel/runtime.ts index 3b85c64c825..837e4980d8a 100644 --- a/packages/cli/src/commands/channel/runtime.ts +++ b/packages/cli/src/commands/channel/runtime.ts @@ -7,6 +7,8 @@ import type { ChannelBase, ChannelBaseOptions, ChannelPlugin, + PermissionRequestEvent, + PermissionResolvedEvent, ToolCallEvent, } from '@qwen-code/channel-base'; import { sanitizeLogText } from '@qwen-code/channel-base'; @@ -164,6 +166,50 @@ export function registerToolCallDispatch( }); } +function cancelPermissionRequest( + bridge: ChannelAgentBridge, + requestId: string, +): void { + void bridge + .respondToPermission?.(requestId, { outcome: { outcome: 'cancelled' } }) + .catch((err: unknown) => { + writeStderrLine( + `[Channel] Permission cancellation failed for ${sanitizeLogText(requestId, 128)}: ${err instanceof Error ? sanitizeLogText(err.message, 512) : sanitizeLogText(String(err), 512)}`, + ); + }); +} + +export function registerPermissionRelay( + bridge: ChannelAgentBridge, + router: SessionRouter, + channels: Map, +): void { + bridge.on('permissionRequest', (event: PermissionRequestEvent) => { + const target = router.getTarget(event.sessionId); + if (!target) { + cancelPermissionRequest(bridge, event.requestId); + return; + } + const channel = channels.get(target.channelName); + if (!channel) { + cancelPermissionRequest(bridge, event.requestId); + return; + } + channel.dispatchPermissionRequest(event).catch((err: unknown) => { + writeStderrLine( + `[Channel] Permission relay failed for ${sanitizeLogText(event.requestId, 128)}: ${err instanceof Error ? sanitizeLogText(err.message, 512) : sanitizeLogText(String(err), 512)}`, + ); + cancelPermissionRequest(bridge, event.requestId); + }); + }); + + bridge.on('permissionResolved', (event: PermissionResolvedEvent) => { + for (const channel of channels.values()) { + channel.dispatchPermissionResolved(event); + } + }); +} + export function registerSessionCleanup( bridge: ChannelAgentBridge, router: SessionRouter, diff --git a/packages/cli/src/commands/channel/start.ts b/packages/cli/src/commands/channel/start.ts index ca6ffd4034e..3dc7a7ed9c2 100644 --- a/packages/cli/src/commands/channel/start.ts +++ b/packages/cli/src/commands/channel/start.ts @@ -32,6 +32,7 @@ import { loadChannelsConfig, loadChannelsFromExtensions, parseConfiguredChannels, + registerPermissionRelay, registerSessionCleanup, registerToolCallDispatch, selectFirstModel, @@ -213,6 +214,7 @@ async function startSingle( }) : undefined; registerToolCallDispatch(bridge, router, channels); + registerPermissionRelay(bridge, router, channels); registerSessionCleanup(bridge, router, channels); try { @@ -266,6 +268,7 @@ async function startSingle( channel.disconnect(); await channel.connect(); registerToolCallDispatch(bridge, router, channels); + registerPermissionRelay(bridge, router, channels); registerSessionCleanup(bridge, router, channels); attachDisconnectHandler(bridge); @@ -374,6 +377,7 @@ async function startAll( ); } registerToolCallDispatch(bridge, router, channels); + registerPermissionRelay(bridge, router, channels); registerSessionCleanup(bridge, router, channels); // Connect all channels @@ -472,6 +476,7 @@ async function startAll( process.exit(1); } registerToolCallDispatch(bridge, router, channels); + registerPermissionRelay(bridge, router, channels); registerSessionCleanup(bridge, router, channels); attachDisconnectHandler(bridge); From fb2e777073c05b68337e34d80c473fba567d5bbc Mon Sep 17 00:00:00 2001 From: qqqys Date: Tue, 7 Jul 2026 21:12:05 +0800 Subject: [PATCH 2/9] fix(channel): harden permission relay cleanup --- packages/channels/base/src/AcpBridge.test.ts | 43 +++++++++++- packages/channels/base/src/AcpBridge.ts | 22 +++++- .../channels/base/src/ChannelBase.test.ts | 67 ++++++++++++++++--- packages/channels/base/src/ChannelBase.ts | 17 ++--- .../cli/src/commands/channel/runtime.test.ts | 22 ++++++ 5 files changed, 153 insertions(+), 18 deletions(-) diff --git a/packages/channels/base/src/AcpBridge.test.ts b/packages/channels/base/src/AcpBridge.test.ts index 5f0ae7cf289..c5ea94fca9d 100644 --- a/packages/channels/base/src/AcpBridge.test.ts +++ b/packages/channels/base/src/AcpBridge.test.ts @@ -1,6 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { RequestPermissionResponse } from '@agentclientprotocol/sdk'; -import { ACP_EVENT_LOOP_STALL_RESTART_MS, AcpBridge } from './AcpBridge.js'; +import { + ACP_EVENT_LOOP_STALL_RESTART_MS, + ACP_PERMISSION_RESPONSE_TIMEOUT_MS, + AcpBridge, +} from './AcpBridge.js'; import { CHANNEL_LOOP_MCP_SERVER_NAME } from './ChannelLoopTools.js'; import type { ChannelLoopToolHandler } from './ChannelAgentBridge.js'; @@ -545,6 +549,43 @@ describe('AcpBridge', () => { await expect(second).resolves.toEqual(response); }); + it('resolves pending permissions as cancelled after the response timeout', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }); + const permissionResolved = vi.fn(); + bridge.on('permissionResolved', permissionResolved); + + await bridge.start(); + + vi.useFakeTimers(); + try { + const pending = child.clients[0]!.requestPermission({ + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [{ optionId: 'cancel', name: 'Deny' }], + }); + await Promise.resolve(); + + await vi.advanceTimersByTimeAsync(ACP_PERMISSION_RESPONSE_TIMEOUT_MS); + + await expect(pending).resolves.toEqual({ + outcome: { outcome: 'cancelled' }, + }); + expect(permissionResolved).toHaveBeenCalledWith({ + requestId: 'acp-permission-1', + outcome: { outcome: 'cancelled' }, + }); + } finally { + vi.useRealTimers(); + } + }); + it('resolves pending permissions as cancelled when the ACP child exits', async () => { const bridge = new AcpBridge({ cliEntryPath: '/tmp/qwen', diff --git a/packages/channels/base/src/AcpBridge.ts b/packages/channels/base/src/AcpBridge.ts index c456453e2a4..ccaeae83ac5 100644 --- a/packages/channels/base/src/AcpBridge.ts +++ b/packages/channels/base/src/AcpBridge.ts @@ -39,6 +39,7 @@ export interface AcpBridgeOptions { } export const ACP_EVENT_LOOP_STALL_RESTART_MS = 5 * 60 * 1000; +export const ACP_PERMISSION_RESPONSE_TIMEOUT_MS = 5 * 60 * 1000; const ACP_EVENT_LOOP_STALL_RE = /^\[perf\] acp agent event loop stall: max=(\d+(?:\.\d+)?)ms/m; @@ -83,6 +84,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { { sessionId: string; resolve: (response: RequestPermissionResponse) => void; + timeout: ReturnType; } >(); @@ -261,6 +263,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { if (!pending) { return false; } + clearTimeout(pending.timeout); this.pendingPermissions.delete(requestId); pending.resolve(response); this.emit('permissionResolved', { @@ -352,7 +355,23 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { : request.toolCall.toolCallId; return new Promise((resolve) => { - this.pendingPermissions.set(requestId, { sessionId, resolve }); + const timeout = setTimeout(() => { + const pending = this.pendingPermissions.get(requestId); + if (!pending) { + return; + } + this.pendingPermissions.delete(requestId); + const response: RequestPermissionResponse = { + outcome: { outcome: 'cancelled' }, + }; + pending.resolve(response); + this.emit('permissionResolved', { + requestId, + outcome: response.outcome, + }); + }, ACP_PERMISSION_RESPONSE_TIMEOUT_MS); + timeout.unref?.(); + this.pendingPermissions.set(requestId, { sessionId, resolve, timeout }); this.emit('permissionRequest', { requestId, sessionId, @@ -369,6 +388,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { if (sessionId !== undefined && pending.sessionId !== sessionId) { continue; } + clearTimeout(pending.timeout); this.pendingPermissions.delete(requestId); pending.resolve(response); this.emit('permissionResolved', { diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index 3cda995f38d..7ef66721324 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -286,12 +286,14 @@ describe('ChannelBase', () => { emitPermission(sessionId, 'req-1'); expect(ch.sent.at(-1)?.chatId).toBe('chat1'); - expect(ch.sent.at(-1)?.text).toContain('需要授权执行命令'); - expect(ch.sent.at(-1)?.text).toContain('命令：'); + expect(ch.sent.at(-1)?.text).toContain( + 'Permission required to run a tool', + ); + expect(ch.sent.at(-1)?.text).toContain('Command:'); expect(ch.sent.at(-1)?.text).toContain('Run req-1'); - expect(ch.sent.at(-1)?.text).toContain('/approve 本次允许'); - expect(ch.sent.at(-1)?.text).toContain('/approve-always 总是允许'); - expect(ch.sent.at(-1)?.text).toContain('/deny 拒绝'); + expect(ch.sent.at(-1)?.text).toContain('/approve allow once'); + expect(ch.sent.at(-1)?.text).toContain('/approve-always always allow'); + expect(ch.sent.at(-1)?.text).toContain('/deny deny'); expect(ch.sent.at(-1)?.text).not.toContain('Request: req-1'); expect(ch.sent.at(-1)?.text).not.toContain('proceed_once'); expect(ch.sent.at(-1)?.text).not.toContain('secret-token'); @@ -421,17 +423,66 @@ describe('ChannelBase', () => { }); emitPermission(sessionId, 'req-2', [ + { optionId: 'reject', kind: 'reject_once', name: 'Deny once' }, + ]); + + await ch.handleInbound(envelope({ text: '/deny req-2' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-2', { + outcome: { outcome: 'selected', optionId: 'reject' }, + }); + + emitPermission(sessionId, 'req-3', [ { optionId: 'always', kind: 'allow_always', name: 'Allow always' }, { optionId: 'never', kind: 'reject_always', name: 'Deny always' }, ]); - await ch.handleInbound(envelope({ text: '/deny req-2' })); + await ch.handleInbound(envelope({ text: '/deny req-3' })); expect(respondToPermissionMock()).toHaveBeenCalledWith('req-2', { + outcome: { outcome: 'selected', optionId: 'reject' }, + }); + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-3', { outcome: { outcome: 'cancelled' }, }); }); + it('reports permission requests that lack requested approval options', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1', [ + { optionId: 'reject', kind: 'reject_once', name: 'Deny once' }, + ]); + + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(ch.sent.at(-1)?.text).toBe( + 'This permission request has no approvable option.', + ); + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + + await ch.handleInbound(envelope({ text: '/approve-always req-1' })); + + expect(ch.sent.at(-1)?.text).toBe( + 'This permission request has no always-allow option.', + ); + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + }); + + it('clears pending permission requests when response dispatch fails', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + respondToPermissionMock().mockRejectedValueOnce(new Error('send failed')); + + await ch.handleInbound(envelope({ text: '/approve req-1' })); + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + }); + it('supports explicit approve-always for persistent permission grants', async () => { const ch = createChannel(); const sessionId = await startSession(ch); @@ -450,7 +501,7 @@ describe('ChannelBase', () => { ]); expect(ch.sent.at(-1)?.text).toContain( - '/approve-always 总是允许（当前项目）', + '/approve-always always allow for this project', ); await ch.handleInbound(envelope({ text: '/approve-always req-1' })); @@ -473,7 +524,7 @@ describe('ChannelBase', () => { ]); expect(ch.sent.at(-1)?.text).toContain( - '/approve-always 总是允许（当前用户）', + '/approve-always always allow for this user', ); await ch.handleInbound(envelope({ text: '/approve-always req-1' })); diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index 2b5c0203129..fa9faacb68f 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -1197,17 +1197,17 @@ export abstract class ChannelBase { const title = sanitizeQuotedText(toolCall.title || 'Tool use', 160); const alwaysOption = this.approvalAlwaysOption(pending); const replies = [ - '/approve 本次允许', + '/approve allow once', ...(alwaysOption ? [`/approve-always ${alwaysOption.label}`] : []), - '/deny 拒绝', + '/deny deny', ]; const lines = [ - '需要授权执行命令', + 'Permission required to run a tool', '', - '命令：', + 'Command:', title, '', - '可回复：', + 'Reply with:', ...replies, ]; return lines.join('\n'); @@ -1260,15 +1260,15 @@ export abstract class ChannelBase { option.optionId === 'proceed_always_project' || option.optionId.endsWith('_project') ) { - return '总是允许（当前项目）'; + return 'always allow for this project'; } if ( option.optionId === 'proceed_always_user' || option.optionId.endsWith('_user') ) { - return '总是允许（当前用户）'; + return 'always allow for this user'; } - return '总是允许'; + return 'always allow'; } private denialResponse(pending: PendingPermission): { @@ -1368,6 +1368,7 @@ export abstract class ChannelBase { response, ); } catch (err) { + this.removePendingPermission(pending.requestId); process.stderr.write( `[${this.name}] permission response failed for request ${sanitizeLogText(pending.requestId, 128)}: ${this.lifecycleError(err)}\n`, ); diff --git a/packages/cli/src/commands/channel/runtime.test.ts b/packages/cli/src/commands/channel/runtime.test.ts index 6a001802e80..cbd9d83caba 100644 --- a/packages/cli/src/commands/channel/runtime.test.ts +++ b/packages/cli/src/commands/channel/runtime.test.ts @@ -118,4 +118,26 @@ describe('registerPermissionRelay', () => { }), ); }); + + it('broadcasts resolved permission requests to channels', () => { + const bridge = createBridge(); + const channel = { + dispatchPermissionResolved: vi.fn(), + }; + + registerPermissionRelay( + bridge, + { getTarget: vi.fn() } as never, + new Map([['telegram', channel as never]]), + ); + bridge.emit('permissionResolved', { + requestId: 'req-1', + outcome: { outcome: 'cancelled' }, + }); + + expect(channel.dispatchPermissionResolved).toHaveBeenCalledWith({ + requestId: 'req-1', + outcome: { outcome: 'cancelled' }, + }); + }); }); From 0fbbda97173e2bd331bd759448c54e03153a79b8 Mon Sep 17 00:00:00 2001 From: qqqys Date: Tue, 7 Jul 2026 22:12:54 +0800 Subject: [PATCH 3/9] fix(channel): scope ACP permission approvals --- .../channels/base/src/ChannelBase.test.ts | 83 +++++++++++++++++++ packages/channels/base/src/ChannelBase.ts | 74 ++++++++++++++--- 2 files changed, 147 insertions(+), 10 deletions(-) diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index 7ef66721324..b9fd4502faa 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -361,6 +361,89 @@ describe('ChannelBase', () => { expect(respondToPermissionMock()).not.toHaveBeenCalled(); }); + it('routes single-scope permission requests to the active chat', async () => { + const ch = createChannel({ sessionScope: 'single' }); + await startSession(ch, { senderId: 'alice', chatId: 'alice-dm' }); + + let resolveBob!: (value: string) => void; + const bobPrompt = new Promise((resolve) => { + resolveBob = resolve; + }); + (bridge.prompt as ReturnType).mockImplementationOnce( + () => bobPrompt, + ); + + const bobTurn = ch.handleInbound( + envelope({ + senderId: 'bob', + senderName: 'Bob', + chatId: 'bob-dm', + text: 'needs permission', + }), + ); + await vi.waitFor(() => expect(bridge.prompt).toHaveBeenCalledTimes(2)); + const sessionId = (bridge.prompt as ReturnType).mock + .calls[1]![0] as string; + + emitPermission(sessionId, 'req-bob'); + + await vi.waitFor(() => + expect(ch.sent.at(-1)?.text).toContain( + 'Permission required to run a tool', + ), + ); + expect(ch.sent.at(-1)?.chatId).toBe('bob-dm'); + + resolveBob('agent response'); + await bobTurn; + }); + + it('does not let another group member approve user-scoped permissions', async () => { + const ch = createChannel({ + groupPolicy: 'open', + sessionScope: 'user', + }); + const sessionId = await startSession(ch, { + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'alice', + threadId: 'thread-1', + }); + emitPermission(sessionId, 'req-alice'); + + await ch.handleInbound( + envelope({ + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'bob', + text: '/approve req-alice', + threadId: 'thread-1', + }), + ); + + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + + await ch.handleInbound( + envelope({ + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'alice', + text: '/approve req-alice', + threadId: 'thread-1', + }), + ); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-alice', { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }); + }); + it('gates shared-session permission responses to authorized senders', async () => { const ch = createChannel({ allowedUsers: ['boss'], diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index fa9faacb68f..996fd5f6d98 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -121,6 +121,8 @@ type ActivePrompt = { /** The originating turn's chat/message, so a clear-time eviction can run this * turn's own onPromptEnd (its finally may settle long after — or never). */ chatId: string; + threadId?: string; + isGroup?: boolean; messageId?: string; senderId?: string; senderName?: string; @@ -272,8 +274,8 @@ export abstract class ChannelBase { async dispatchPermissionRequest( event: PermissionRequestEvent, ): Promise { - const target = this.router.getTarget(event.sessionId); - if (!target || target.channelName !== this.name) { + const target = this.permissionTargetForEvent(event); + if (!target) { return; } this.removePendingPermission(event.requestId); @@ -308,6 +310,33 @@ export abstract class ChannelBase { } } + private permissionTargetForEvent( + event: PermissionRequestEvent, + ): SessionTarget | undefined { + const routeTarget = this.router.getTarget(event.sessionId); + if (!routeTarget || routeTarget.channelName !== this.name) { + return undefined; + } + const active = this.activePrompts.get(event.sessionId); + if (!active) { + return routeTarget; + } + const target: SessionTarget = { + channelName: routeTarget.channelName, + senderId: active.senderId ?? routeTarget.senderId, + chatId: active.chatId, + }; + if (active.threadId !== undefined) { + target.threadId = active.threadId; + } + if (active.isGroup !== undefined) { + target.isGroup = active.isGroup; + } else if (routeTarget.isGroup !== undefined) { + target.isGroup = routeTarget.isGroup; + } + return target; + } + dispatchPermissionResolved(event: PermissionResolvedEvent): void { this.removePendingPermission(event.requestId); } @@ -657,6 +686,8 @@ export abstract class ChannelBase { done, resolve: doneResolve, chatId: job.target.chatId, + threadId: job.target.threadId, + isGroup: job.target.isGroup, messageId: job.id, senderId: job.target.senderId, senderName: job.createdBy, @@ -1170,8 +1201,7 @@ export abstract class ChannelBase { const explicit = this.pendingPermissions.get(trimmed); if ( explicit && - explicit.target.chatId === envelope.chatId && - explicit.target.threadId === envelope.threadId + this.canEnvelopeAnswerPendingPermission(envelope, explicit) ) { return { kind: 'found', pending: explicit }; } @@ -1183,13 +1213,35 @@ export abstract class ChannelBase { if (requestIds.length === 0) { return { kind: 'none', explicit: false }; } - if (requestIds.length > 1) { - return { kind: 'ambiguous', requestIds }; + const matching = requestIds + .map((id) => this.pendingPermissions.get(id)) + .filter( + (pending): pending is PendingPermission => + pending !== undefined && + this.canEnvelopeAnswerPendingPermission(envelope, pending), + ); + if (matching.length === 0) { + return { kind: 'none', explicit: false }; + } + if (matching.length > 1) { + return { + kind: 'ambiguous', + requestIds: matching.map((pending) => pending.requestId), + }; } - const pending = this.pendingPermissions.get(requestIds[0]!); - return pending - ? { kind: 'found', pending } - : { kind: 'none', explicit: false }; + return { kind: 'found', pending: matching[0]! }; + } + + private canEnvelopeAnswerPendingPermission( + envelope: Envelope, + pending: PendingPermission, + ): boolean { + return ( + pending.target.chatId === envelope.chatId && + pending.target.threadId === envelope.threadId && + (this.isSharedSessionTarget(pending.target) || + pending.target.senderId === envelope.senderId) + ); } private formatPermissionRequest(pending: PendingPermission): string { @@ -3114,6 +3166,8 @@ export abstract class ChannelBase { done, resolve: doneResolve, chatId: envelope.chatId, + threadId: envelope.threadId, + isGroup: envelope.isGroup, messageId: envelope.messageId, senderId: envelope.senderId, senderName: envelope.senderName, From ccd791486dc1ceb537745c1d0221310c1ea148c6 Mon Sep 17 00:00:00 2001 From: qqqys Date: Wed, 8 Jul 2026 00:09:23 +0800 Subject: [PATCH 4/9] fix(channel): harden permission cancellation diagnostics --- packages/channels/base/src/AcpBridge.test.ts | 7 ++ packages/channels/base/src/AcpBridge.ts | 3 + .../cli/src/commands/channel/runtime.test.ts | 114 +++++++++++++++--- packages/cli/src/commands/channel/runtime.ts | 11 +- 4 files changed, 117 insertions(+), 18 deletions(-) diff --git a/packages/channels/base/src/AcpBridge.test.ts b/packages/channels/base/src/AcpBridge.test.ts index c5ea94fca9d..278e6fcc303 100644 --- a/packages/channels/base/src/AcpBridge.test.ts +++ b/packages/channels/base/src/AcpBridge.test.ts @@ -555,6 +555,9 @@ describe('AcpBridge', () => { cwd: '/tmp', }); const permissionResolved = vi.fn(); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); bridge.on('permissionResolved', permissionResolved); await bridge.start(); @@ -581,7 +584,11 @@ describe('AcpBridge', () => { requestId: 'acp-permission-1', outcome: { outcome: 'cancelled' }, }); + expect(stderr.mock.calls.join('')).toContain( + `[AcpBridge] permission request acp-permission-1 timed out after ${ACP_PERMISSION_RESPONSE_TIMEOUT_MS}ms (session=session-1)`, + ); } finally { + stderr.mockRestore(); vi.useRealTimers(); } }); diff --git a/packages/channels/base/src/AcpBridge.ts b/packages/channels/base/src/AcpBridge.ts index ccaeae83ac5..a71bcf81078 100644 --- a/packages/channels/base/src/AcpBridge.ts +++ b/packages/channels/base/src/AcpBridge.ts @@ -360,6 +360,9 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { if (!pending) { return; } + process.stderr.write( + `[AcpBridge] permission request ${sanitizeLogText(requestId, 128)} timed out after ${ACP_PERMISSION_RESPONSE_TIMEOUT_MS}ms (session=${sanitizeLogText(pending.sessionId, 128)})\n`, + ); this.pendingPermissions.delete(requestId); const response: RequestPermissionResponse = { outcome: { outcome: 'cancelled' }, diff --git a/packages/cli/src/commands/channel/runtime.test.ts b/packages/cli/src/commands/channel/runtime.test.ts index d3f2f1b5252..271b2abab18 100644 --- a/packages/cli/src/commands/channel/runtime.test.ts +++ b/packages/cli/src/commands/channel/runtime.test.ts @@ -115,28 +115,71 @@ describe('registerPermissionRelay', () => { } it('cancels permission requests when no route exists', async () => { + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); const bridge = createBridge(); const router = { getTarget: vi.fn() }; - registerPermissionRelay(bridge, router as never, new Map()); - bridge.emit('permissionRequest', { - requestId: 'req-1', - sessionId: 'missing-session', - request: { - toolCall: { - toolCallId: 'tool-1', - kind: 'shell', - title: 'Run command', + try { + registerPermissionRelay(bridge, router as never, new Map()); + bridge.emit('permissionRequest', { + requestId: 'req-1', + sessionId: 'missing-session', + request: { + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [], }, - options: [], - }, - }); + }); - await vi.waitFor(() => - expect(bridge.respondToPermission).toHaveBeenCalledWith('req-1', { - outcome: { outcome: 'cancelled' }, - }), - ); + await vi.waitFor(() => + expect(bridge.respondToPermission).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'cancelled' }, + }), + ); + expect(stderr.mock.calls.join('')).toContain( + 'No route for session missing-session; cancelling permission req-1', + ); + } finally { + stderr.mockRestore(); + } + }); + + it('does not crash cancelling permission requests without a responder', () => { + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = createBridge(); + delete (bridge as { respondToPermission?: unknown }).respondToPermission; + const router = { getTarget: vi.fn() }; + + try { + registerPermissionRelay(bridge, router as never, new Map()); + + expect(() => + bridge.emit('permissionRequest', { + requestId: 'req-1', + sessionId: 'missing-session', + request: { + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [], + }, + }), + ).not.toThrow(); + expect(stderr.mock.calls.join('')).toContain( + 'No route for session missing-session; cancelling permission req-1', + ); + } finally { + stderr.mockRestore(); + } }); it('cancels permission requests when channel dispatch fails', async () => { @@ -176,6 +219,43 @@ describe('registerPermissionRelay', () => { ); }); + it('logs before cancelling permission requests with no channel', async () => { + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = createBridge(); + const router = { + getTarget: vi.fn(() => ({ channelName: 'telegram', chatId: 'chat1' })), + }; + + try { + registerPermissionRelay(bridge, router as never, new Map()); + bridge.emit('permissionRequest', { + requestId: 'req-1', + sessionId: 'session-1', + request: { + toolCall: { + toolCallId: 'tool-1', + kind: 'shell', + title: 'Run command', + }, + options: [], + }, + }); + + await vi.waitFor(() => + expect(bridge.respondToPermission).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'cancelled' }, + }), + ); + expect(stderr.mock.calls.join('')).toContain( + 'No channel "telegram" for session session-1; cancelling permission req-1', + ); + } finally { + stderr.mockRestore(); + } + }); + it('broadcasts resolved permission requests to channels', () => { const bridge = createBridge(); const channel = { diff --git a/packages/cli/src/commands/channel/runtime.ts b/packages/cli/src/commands/channel/runtime.ts index 3b50d34c4dc..70a8799fa88 100644 --- a/packages/cli/src/commands/channel/runtime.ts +++ b/packages/cli/src/commands/channel/runtime.ts @@ -170,8 +170,11 @@ function cancelPermissionRequest( bridge: ChannelAgentBridge, requestId: string, ): void { + if (!bridge.respondToPermission) { + return; + } void bridge - .respondToPermission?.(requestId, { outcome: { outcome: 'cancelled' } }) + .respondToPermission(requestId, { outcome: { outcome: 'cancelled' } }) .catch((err: unknown) => { writeStderrLine( `[Channel] Permission cancellation failed for ${sanitizeLogText(requestId, 128)}: ${err instanceof Error ? sanitizeLogText(err.message, 512) : sanitizeLogText(String(err), 512)}`, @@ -187,11 +190,17 @@ export function registerPermissionRelay( bridge.on('permissionRequest', (event: PermissionRequestEvent) => { const target = router.getTarget(event.sessionId); if (!target) { + writeStderrLine( + `[Channel] No route for session ${sanitizeLogText(event.sessionId, 128)}; cancelling permission ${sanitizeLogText(event.requestId, 128)}`, + ); cancelPermissionRequest(bridge, event.requestId); return; } const channel = channels.get(target.channelName); if (!channel) { + writeStderrLine( + `[Channel] No channel "${sanitizeLogText(target.channelName, 64)}" for session ${sanitizeLogText(event.sessionId, 128)}; cancelling permission ${sanitizeLogText(event.requestId, 128)}`, + ); cancelPermissionRequest(bridge, event.requestId); return; } From bc52d8809b6abd1d49d039e30c9a07b399355694 Mon Sep 17 00:00:00 2001 From: qqqys Date: Wed, 8 Jul 2026 01:08:05 +0800 Subject: [PATCH 5/9] test(channel): cover permission lookup edge cases --- .../channels/base/src/ChannelBase.test.ts | 83 +++++++++++++++++++ packages/channels/base/src/ChannelBase.ts | 3 - 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index 1b8eead48d5..39482c202d3 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -429,6 +429,52 @@ describe('ChannelBase', () => { expect(respondToPermissionMock()).not.toHaveBeenCalled(); }); + it('does not answer permission requests from another thread', async () => { + const ch = createChannel({ + groupPolicy: 'open', + sessionScope: 'thread', + }); + const sessionId = await startSession(ch, { + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'alice', + threadId: 'thread-1', + }); + emitPermission(sessionId, 'req-thread-1'); + + await ch.handleInbound( + envelope({ + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'alice', + text: '/approve req-thread-1', + threadId: 'thread-2', + }), + ); + + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + + await ch.handleInbound( + envelope({ + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'alice', + text: '/approve req-thread-1', + threadId: 'thread-1', + }), + ); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-thread-1', { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }); + }); + it('routes single-scope permission requests to the active chat', async () => { const ch = createChannel({ sessionScope: 'single' }); await startSession(ch, { senderId: 'alice', chatId: 'alice-dm' }); @@ -512,6 +558,43 @@ describe('ChannelBase', () => { }); }); + it('matches the current sender before reporting ambiguous permissions', async () => { + const ch = createChannel({ + groupPolicy: 'open', + sessionScope: 'user', + }); + const group = { + chatId: 'group1', + isGroup: true, + isMentioned: true, + threadId: 'thread-1', + }; + const aliceSessionId = await startSession(ch, { + ...group, + senderId: 'alice', + }); + emitPermission(aliceSessionId, 'req-alice'); + const bobSessionId = await startSession(ch, { + ...group, + senderId: 'bob', + }); + emitPermission(bobSessionId, 'req-bob'); + + await ch.handleInbound( + envelope({ + ...group, + senderId: 'alice', + text: '/approve', + }), + ); + + expect(ch.sent.at(-1)?.text).toBe('Permission approved.'); + expect(respondToPermissionMock()).toHaveBeenCalledTimes(1); + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-alice', { + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }); + }); + it('gates shared-session permission responses to authorized senders', async () => { const ch = createChannel({ allowedUsers: ['boss'], diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index 940d1aa8bf6..5364e48d13f 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -1785,9 +1785,6 @@ export abstract class ChannelBase { '/approve [request-id] — Approve a pending permission request', '/approve-always [request-id] — Always approve a pending permission request', '/deny [request-id] — Deny a pending permission request', - '/remember-channel — Save memory for this chat', - '/channel-memory — Show memory for this chat', - '/forget-channel confirm — Clear memory for this chat', ]; // Platform-specific commands (registered by adapters, not shared ones) From ae95e9332273ef4a75565e048fad6aceb058ee47 Mon Sep 17 00:00:00 2001 From: qqqys Date: Wed, 8 Jul 2026 03:12:44 +0800 Subject: [PATCH 6/9] fix(channel): close permission relay stale requests --- packages/channels/base/src/AcpBridge.test.ts | 12 +++-- packages/channels/base/src/AcpBridge.ts | 4 +- .../channels/base/src/ChannelBase.test.ts | 52 +++++++++++++++++++ packages/channels/base/src/ChannelBase.ts | 15 ++++++ 4 files changed, 78 insertions(+), 5 deletions(-) diff --git a/packages/channels/base/src/AcpBridge.test.ts b/packages/channels/base/src/AcpBridge.test.ts index 278e6fcc303..a1953d2439e 100644 --- a/packages/channels/base/src/AcpBridge.test.ts +++ b/packages/channels/base/src/AcpBridge.test.ts @@ -554,10 +554,12 @@ describe('AcpBridge', () => { cliEntryPath: '/tmp/qwen', cwd: '/tmp', }); + const permissionRequest = vi.fn(); const permissionResolved = vi.fn(); const stderr = vi .spyOn(process.stderr, 'write') .mockImplementation(() => true); + bridge.on('permissionRequest', permissionRequest); bridge.on('permissionResolved', permissionResolved); await bridge.start(); @@ -574,6 +576,7 @@ describe('AcpBridge', () => { options: [{ optionId: 'cancel', name: 'Deny' }], }); await Promise.resolve(); + const event = permissionRequest.mock.calls[0]![0]; await vi.advanceTimersByTimeAsync(ACP_PERMISSION_RESPONSE_TIMEOUT_MS); @@ -581,11 +584,11 @@ describe('AcpBridge', () => { outcome: { outcome: 'cancelled' }, }); expect(permissionResolved).toHaveBeenCalledWith({ - requestId: 'acp-permission-1', + requestId: event.requestId, outcome: { outcome: 'cancelled' }, }); expect(stderr.mock.calls.join('')).toContain( - `[AcpBridge] permission request acp-permission-1 timed out after ${ACP_PERMISSION_RESPONSE_TIMEOUT_MS}ms (session=session-1)`, + `[AcpBridge] permission request ${event.requestId} timed out after ${ACP_PERMISSION_RESPONSE_TIMEOUT_MS}ms (session=session-1)`, ); } finally { stderr.mockRestore(); @@ -598,7 +601,9 @@ describe('AcpBridge', () => { cliEntryPath: '/tmp/qwen', cwd: '/tmp', }); + const permissionRequest = vi.fn(); const permissionResolved = vi.fn(); + bridge.on('permissionRequest', permissionRequest); bridge.on('permissionResolved', permissionResolved); await bridge.start(); @@ -612,6 +617,7 @@ describe('AcpBridge', () => { options: [{ optionId: 'cancel', name: 'Deny' }], }); await Promise.resolve(); + const event = permissionRequest.mock.calls[0]![0]; child.instances[0]!.emit('exit', 1, null); @@ -619,7 +625,7 @@ describe('AcpBridge', () => { outcome: { outcome: 'cancelled' }, }); expect(permissionResolved).toHaveBeenCalledWith({ - requestId: 'acp-permission-1', + requestId: event.requestId, outcome: { outcome: 'cancelled' }, }); }); diff --git a/packages/channels/base/src/AcpBridge.ts b/packages/channels/base/src/AcpBridge.ts index a71bcf81078..dfe108fa35d 100644 --- a/packages/channels/base/src/AcpBridge.ts +++ b/packages/channels/base/src/AcpBridge.ts @@ -1,5 +1,6 @@ import { spawn } from 'node:child_process'; import type { ChildProcess } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; import { Readable, Writable } from 'node:stream'; import { EventEmitter } from 'node:events'; import { @@ -78,7 +79,6 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { private readonly channelLoopToolHandlers: ChannelLoopToolHandler[] = []; private channelLoopMcpRegistered = false; private channelLoopMcpRegistration: Promise | null = null; - private permissionCounter = 0; private readonly pendingPermissions = new Map< string, { @@ -348,7 +348,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { private requestPermission( request: RequestPermissionRequest, ): Promise { - const requestId = `acp-permission-${++this.permissionCounter}`; + const requestId = `acp-permission-${randomUUID()}`; const sessionId = typeof request.sessionId === 'string' && request.sessionId.length > 0 ? request.sessionId diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index 39482c202d3..b4538e3afb0 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -374,6 +374,35 @@ describe('ChannelBase', () => { expect(ch.sent.at(-1)?.text).toBe('Permission approved.'); }); + it('cancels bridge permissions when the target no longer belongs to the channel', async () => { + const router = { + getTarget: vi.fn(() => ({ + channelName: 'other-channel', + chatId: 'chat1', + })), + setBridge: vi.fn(), + }; + const ch = createChannel({}, { router } as unknown as ChannelBaseOptions); + + await ch.dispatchPermissionRequest({ + requestId: 'req-stale', + sessionId: 'session-1', + request: { + toolCall: { + toolCallId: 'tool-req-stale', + kind: 'shell', + title: 'Run req-stale', + }, + options: [], + }, + }); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-stale', { + outcome: { outcome: 'cancelled' }, + }); + expect(ch.sent).toEqual([]); + }); + it('requires an explicit request id when multiple permissions are pending', async () => { const ch = createChannel(); const sessionId = await startSession(ch); @@ -816,6 +845,29 @@ describe('ChannelBase', () => { 'No pending permission request with that id for this chat.', ); }); + + it('clears pending permission requests when the bridge is replaced', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + const oldRespondToPermission = respondToPermissionMock(); + const newBridge = createBridge(); + + ch.setBridge(newBridge); + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + expect(oldRespondToPermission).not.toHaveBeenCalled(); + expect( + ( + newBridge as unknown as { + respondToPermission: ReturnType; + } + ).respondToPermission, + ).not.toHaveBeenCalled(); + }); }); describe('group history backfill', () => { diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index 5364e48d13f..29a75764ee7 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -287,6 +287,15 @@ export abstract class ChannelBase { ): Promise { const target = this.permissionTargetForEvent(event); if (!target) { + try { + await this.bridge.respondToPermission?.(event.requestId, { + outcome: { outcome: 'cancelled' }, + }); + } catch (respondErr) { + process.stderr.write( + `[${this.name}] permission cancellation failed for request ${sanitizeLogText(event.requestId, 128)}: ${this.lifecycleError(respondErr)}\n`, + ); + } return; } this.removePendingPermission(event.requestId); @@ -562,6 +571,7 @@ export abstract class ChannelBase { if (this.registerBridgeEvents) { this.detachBridgeEvents(this.bridge); } + this.clearPendingPermissions(); this.router.setBridge(bridge); this.bridge = bridge; if (this.loopController) { @@ -1265,6 +1275,11 @@ export abstract class ChannelBase { } } + private clearPendingPermissions(): void { + this.pendingPermissions.clear(); + this.pendingPermissionsByChat.clear(); + } + private pendingPermissionForEnvelope( envelope: Envelope, args: string, From 1b751f2a66ebe89f4932c0b8e81a4d3431e26dc7 Mon Sep 17 00:00:00 2001 From: qqqys Date: Wed, 8 Jul 2026 05:06:56 +0800 Subject: [PATCH 7/9] fix(channel): tighten approve-always option matching --- .../channels/base/src/ChannelBase.test.ts | 28 +++++++++++++++++++ packages/channels/base/src/ChannelBase.ts | 14 ++-------- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index b4538e3afb0..10d3d079386 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -797,6 +797,34 @@ describe('ChannelBase', () => { }); }); + it('does not infer approve-always scope from noncanonical option ids', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1', [ + { + optionId: 'sandbox_bypass_project', + kind: 'allow_always', + name: 'Always allow sandbox bypass', + }, + { + optionId: 'proceed_always_user', + kind: 'allow_always', + name: 'Always Allow for user', + }, + { optionId: 'once', kind: 'allow_once', name: 'Allow once' }, + ]); + + expect(ch.sent.at(-1)?.text).toContain( + '/approve-always always allow for this user', + ); + + await ch.handleInbound(envelope({ text: '/approve-always req-1' })); + + expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { + outcome: { outcome: 'selected', optionId: 'proceed_always_user' }, + }); + }); + it('allows approve-always without a request id when one request is pending', async () => { const ch = createChannel(); const sessionId = await startSession(ch); diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index 29a75764ee7..0dae8e88132 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -1389,23 +1389,15 @@ export abstract class ChannelBase { scope: 'project' | 'user', ): PermissionOption | undefined { return options.find( - (option) => - option.optionId === `proceed_always_${scope}` || - option.optionId.endsWith(`_${scope}`), + (option) => option.optionId === `proceed_always_${scope}`, ); } private approvalAlwaysLabel(option: PermissionOption): string { - if ( - option.optionId === 'proceed_always_project' || - option.optionId.endsWith('_project') - ) { + if (option.optionId === 'proceed_always_project') { return 'always allow for this project'; } - if ( - option.optionId === 'proceed_always_user' || - option.optionId.endsWith('_user') - ) { + if (option.optionId === 'proceed_always_user') { return 'always allow for this user'; } return 'always allow'; From 608431cfd700ce51dc2985ea1f937548777d318d Mon Sep 17 00:00:00 2001 From: qqqys Date: Wed, 8 Jul 2026 07:07:00 +0800 Subject: [PATCH 8/9] test(channel): cover permission relay cleanup gaps --- .../channels/base/src/ChannelBase.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index 10d3d079386..8da11f09e52 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -854,6 +854,36 @@ describe('ChannelBase', () => { ); }); + it('clears pending permission requests when the session dies', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + + (bridge as unknown as EventEmitter).emit('sessionDied', { + sessionId, + }); + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(respondToPermissionMock()).not.toHaveBeenCalled(); + expect(ch.sent.at(-1)?.text).toBe( + 'No pending permission request with that id for this chat.', + ); + }); + + it('reports when the bridge cannot answer permission requests', async () => { + const ch = createChannel(); + const sessionId = await startSession(ch); + emitPermission(sessionId, 'req-1'); + delete (bridge as unknown as { respondToPermission?: unknown }) + .respondToPermission; + + await ch.handleInbound(envelope({ text: '/approve req-1' })); + + expect(ch.sent.at(-1)?.text).toBe( + 'Permission relay is not available for this session.', + ); + }); + it('cancels the permission request when the relay message cannot be sent', async () => { const ch = createChannel(); const sessionId = await startSession(ch); From ba8082239aaaae73a7b685e1cd4b389ca4421b60 Mon Sep 17 00:00:00 2001 From: qqqys Date: Wed, 8 Jul 2026 09:13:50 +0800 Subject: [PATCH 9/9] fix(channel): deliver threaded permission prompts --- .../channels/base/src/ChannelBase.test.ts | 37 ++++++++++++++++++- packages/channels/base/src/ChannelBase.ts | 14 +++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index 8da11f09e52..bfb67090247 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -21,6 +21,7 @@ import type { ChannelLoop, ChannelLoopInput } from './ChannelLoopStore.js'; class TestChannel extends ChannelBase { sent: Array<{ chatId: string; text: string }> = []; proactive: Array<{ chatId: string; text: string }> = []; + proactiveTargets: SessionTarget[] = []; proactiveSupported = false; proactiveTargetSupported: boolean | undefined; sendMessageError?: Error; @@ -83,10 +84,11 @@ class TestChannel extends ChannelBase { } protected override async pushProactive( - target: { chatId: string }, + target: SessionTarget, text: string, ): Promise { this.proactive.push({ chatId: target.chatId, text }); + this.proactiveTargets.push(target); } enableCancelCommand(): void { @@ -504,6 +506,36 @@ describe('ChannelBase', () => { }); }); + it('delivers threaded permission requests through proactive targets when supported', async () => { + const ch = createChannel({ + groupPolicy: 'open', + sessionScope: 'thread', + }); + ch.proactiveSupported = true; + ch.proactiveTargetSupported = true; + const sessionId = await startSession(ch, { + chatId: 'group1', + isGroup: true, + isMentioned: true, + senderId: 'alice', + threadId: 'thread-1', + }); + + emitPermission(sessionId, 'req-thread-1'); + + expect(ch.proactiveTargets.at(-1)).toMatchObject({ + chatId: 'group1', + senderId: 'alice', + threadId: 'thread-1', + }); + expect(ch.proactive.at(-1)?.text).toContain( + 'Permission required to run a tool', + ); + expect(ch.sent.at(-1)?.text).not.toContain( + 'Permission required to run a tool', + ); + }); + it('routes single-scope permission requests to the active chat', async () => { const ch = createChannel({ sessionScope: 'single' }); await startSession(ch, { senderId: 'alice', chatId: 'alice-dm' }); @@ -694,6 +726,7 @@ describe('ChannelBase', () => { expect(respondToPermissionMock()).toHaveBeenCalledWith('req-2', { outcome: { outcome: 'selected', optionId: 'reject' }, }); + expect(ch.sent.at(-1)?.text).toBe('Permission denied.'); emitPermission(sessionId, 'req-3', [ { optionId: 'always', kind: 'allow_always', name: 'Allow always' }, @@ -708,6 +741,7 @@ describe('ChannelBase', () => { expect(respondToPermissionMock()).toHaveBeenCalledWith('req-3', { outcome: { outcome: 'cancelled' }, }); + expect(ch.sent.at(-1)?.text).toBe('Permission denied.'); }); it('reports permission requests that lack requested approval options', async () => { @@ -772,6 +806,7 @@ describe('ChannelBase', () => { expect(respondToPermissionMock()).toHaveBeenCalledWith('req-1', { outcome: { outcome: 'selected', optionId: 'proceed_always_project' }, }); + expect(ch.sent.at(-1)?.text).toBe('Permission approved always.'); }); it('falls back to user-scope approve-always when project scope is unavailable', async () => { diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index 0dae8e88132..390ac2c1066 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -311,10 +311,16 @@ export abstract class ChannelBase { requestIds.push(event.requestId); this.pendingPermissionsByChat.set(chatKey, requestIds); try { - await this.sendMessage( - target.chatId, - this.formatPermissionRequest(pending), - ); + const text = this.formatPermissionRequest(pending); + if ( + target.threadId !== undefined && + this.supportsProactiveSend() && + this.supportsProactiveTarget(target) + ) { + await this.pushProactive(target, text); + } else { + await this.sendMessage(target.chatId, text); + } } catch (err) { this.removePendingPermission(event.requestId); try {