From ca7cb9ac90b8cec69ed54dc53e4d29f11018d253 Mon Sep 17 00:00:00 2001 From: weishu Date: Tue, 31 Mar 2026 18:36:31 +0800 Subject: [PATCH] auto approve title tool for codex --- .../agent/backends/acp/AcpSdkBackend.test.ts | 40 ++++ cli/src/agent/backends/acp/AcpSdkBackend.ts | 16 +- cli/src/agent/permissionAdapter.test.ts | 196 ++++++++++++++++++ cli/src/agent/permissionAdapter.ts | 67 +++++- cli/src/codex/utils/permissionHandler.test.ts | 17 ++ .../permission/BasePermissionHandler.ts | 75 ++++--- 6 files changed, 371 insertions(+), 40 deletions(-) create mode 100644 cli/src/agent/permissionAdapter.test.ts diff --git a/cli/src/agent/backends/acp/AcpSdkBackend.test.ts b/cli/src/agent/backends/acp/AcpSdkBackend.test.ts index 5bc86812..e4c8bb5e 100644 --- a/cli/src/agent/backends/acp/AcpSdkBackend.test.ts +++ b/cli/src/agent/backends/acp/AcpSdkBackend.test.ts @@ -30,6 +30,46 @@ afterEach(() => { }); describe('AcpSdkBackend', () => { + it('allows the permission handler to resolve requests immediately', async () => { + const backend = new AcpSdkBackend({ command: 'opencode' }); + let capturedRequestId: string | null = null; + + backend.onPermissionRequest((request) => { + capturedRequestId = request.id; + void backend.respondToPermission(request.sessionId, request, { + outcome: 'selected', + optionId: 'allow-once' + }); + }); + + const backendInternal = backend as unknown as { + handlePermissionRequest: (params: unknown, requestId: string | number | null) => Promise; + }; + + await expect(backendInternal.handlePermissionRequest({ + sessionId: 'session-1', + toolCall: { + toolCallId: 'tool-approve', + title: 'hapi_change_title', + rawInput: { title: 'Rename chat' } + }, + options: [ + { + optionId: 'allow-once', + name: 'Allow once', + kind: 'allow_once' + } + ] + }, null)).resolves.toEqual({ + outcome: { + outcome: 'selected', + optionId: 'allow-once' + } + }); + + expect(capturedRequestId).toBe('tool-approve'); + }); + it('emits turn_complete after trailing tool updates from the same turn', async () => { backendStatics.UPDATE_QUIET_PERIOD_MS = 8; backendStatics.UPDATE_DRAIN_TIMEOUT_MS = 200; diff --git a/cli/src/agent/backends/acp/AcpSdkBackend.ts b/cli/src/agent/backends/acp/AcpSdkBackend.ts index aa797e96..13a3e054 100644 --- a/cli/src/agent/backends/acp/AcpSdkBackend.ts +++ b/cli/src/agent/backends/acp/AcpSdkBackend.ts @@ -331,16 +331,24 @@ export class AcpSdkBackend implements AgentBackend { options }; + const responsePromise = new Promise((resolve) => { + this.pendingPermissions.set(toolCallId, { resolve }); + }); + if (this.permissionHandler) { - this.permissionHandler(request); + try { + this.permissionHandler(request); + } catch (error) { + this.pendingPermissions.delete(toolCallId); + throw error; + } } else { logger.debug('[ACP] No permission handler registered; cancelling request'); + this.pendingPermissions.delete(toolCallId); return { outcome: { outcome: 'cancelled' } }; } - return await new Promise((resolve) => { - this.pendingPermissions.set(toolCallId, { resolve }); - }); + return await responsePromise; } private notifyResponseComplete(): void { diff --git a/cli/src/agent/permissionAdapter.test.ts b/cli/src/agent/permissionAdapter.test.ts new file mode 100644 index 00000000..53a547c6 --- /dev/null +++ b/cli/src/agent/permissionAdapter.test.ts @@ -0,0 +1,196 @@ +import { describe, expect, it } from 'vitest'; +import type { ApiSessionClient } from '@/api/apiSession'; +import type { AgentBackend, PermissionRequest, PermissionResponse } from './types'; +import { PermissionAdapter } from './permissionAdapter'; + +type FakeAgentState = { + requests: Record; + completedRequests: Record; +}; + +type Harness = ReturnType; + +function createHarness() { + let agentState: FakeAgentState = { + requests: {}, + completedRequests: {} + }; + + const rpcHandlers = new Map Promise | unknown>(); + let permissionHandler: ((request: PermissionRequest) => void) | null = null; + const respondCalls: Array<{ + sessionId: string; + request: PermissionRequest; + response: PermissionResponse; + }> = []; + + const session = { + rpcHandlerManager: { + registerHandler(method: string, handler: (params: unknown) => Promise | unknown) { + rpcHandlers.set(method, handler); + } + }, + updateAgentState(handler: (state: FakeAgentState) => FakeAgentState) { + agentState = handler(agentState); + } + } as unknown as ApiSessionClient; + + const backend: AgentBackend = { + async initialize() {}, + async newSession() { + return 'agent-session'; + }, + async prompt() {}, + async cancelPrompt() {}, + async respondToPermission(sessionId, request, response) { + respondCalls.push({ sessionId, request, response }); + }, + onPermissionRequest(handler) { + permissionHandler = handler; + }, + async disconnect() {} + }; + + new PermissionAdapter(session, backend); + + return { + rpcHandlers, + respondCalls, + getAgentState: () => agentState, + emitPermissionRequest(request: PermissionRequest) { + if (!permissionHandler) { + throw new Error('Permission handler was not registered'); + } + permissionHandler(request); + } + }; +} + +async function flushAsyncWork(): Promise { + await Promise.resolve(); + await Promise.resolve(); +} + +function buildRequest(overrides?: Partial): PermissionRequest { + return { + id: 'perm-1', + sessionId: 'session-1', + toolCallId: 'perm-1', + title: 'Read', + rawInput: { path: 'README.md' }, + options: [ + { + optionId: 'allow-once', + name: 'Allow once', + kind: 'allow_once' + }, + { + optionId: 'allow-always', + name: 'Allow always', + kind: 'allow_always' + } + ], + ...overrides + }; +} + +describe('PermissionAdapter', () => { + it('auto-approves change_title permissions without queueing them', async () => { + const harness = createHarness(); + + harness.emitPermissionRequest(buildRequest({ + id: 'perm-title', + toolCallId: 'perm-title', + title: 'hapi_change_title', + rawInput: { title: 'Rename chat' } + })); + + await flushAsyncWork(); + + expect(harness.respondCalls).toEqual([ + { + sessionId: 'session-1', + request: expect.objectContaining({ + id: 'perm-title', + title: 'hapi_change_title' + }), + response: { outcome: 'selected', optionId: 'allow-once' } + } + ]); + expect(harness.getAgentState().requests).toEqual({}); + expect(harness.getAgentState().completedRequests).toMatchObject({ + 'perm-title': { + tool: 'hapi_change_title', + status: 'approved', + decision: 'approved' + } + }); + }); + + it('auto-approves change_title aliases detected from the tool call id', async () => { + const harness = createHarness(); + + harness.emitPermissionRequest(buildRequest({ + id: 'mcp__hapi__change_title-1', + toolCallId: 'mcp__hapi__change_title-1', + title: undefined, + rawInput: { title: 'Rename chat' } + })); + + await flushAsyncWork(); + + expect(harness.respondCalls).toHaveLength(1); + expect(harness.getAgentState().requests).toEqual({}); + expect(harness.getAgentState().completedRequests).toMatchObject({ + 'mcp__hapi__change_title-1': { + status: 'approved', + decision: 'approved' + } + }); + }); + + it('keeps non-title permissions pending until the hub responds', async () => { + const harness = createHarness(); + + harness.emitPermissionRequest(buildRequest({ + id: 'perm-read', + toolCallId: 'perm-read', + title: 'Read' + })); + + expect(harness.respondCalls).toEqual([]); + expect(harness.getAgentState().requests).toMatchObject({ + 'perm-read': { + tool: 'Read' + } + }); + + const permissionRpc = harness.rpcHandlers.get('permission'); + expect(permissionRpc).toBeTypeOf('function'); + + await permissionRpc?.({ + id: 'perm-read', + approved: true, + decision: 'approved' + }); + + expect(harness.respondCalls).toEqual([ + { + sessionId: 'session-1', + request: expect.objectContaining({ + id: 'perm-read', + title: 'Read' + }), + response: { outcome: 'selected', optionId: 'allow-once' } + } + ]); + expect(harness.getAgentState().requests).toEqual({}); + expect(harness.getAgentState().completedRequests).toMatchObject({ + 'perm-read': { + tool: 'Read', + status: 'approved', + decision: 'approved' + } + }); + }); +}); diff --git a/cli/src/agent/permissionAdapter.ts b/cli/src/agent/permissionAdapter.ts index 9c62dda3..5df8029b 100644 --- a/cli/src/agent/permissionAdapter.ts +++ b/cli/src/agent/permissionAdapter.ts @@ -3,6 +3,10 @@ import type { AgentState } from '@/api/types'; import type { ApiSessionClient } from '@/api/apiSession'; import { logger } from '@/ui/logger'; import { deriveToolName } from '@/agent/utils'; +import { + resolveToolAutoApprovalDecision, + type AutoApprovalDecision +} from '@/modules/common/permission/BasePermissionHandler'; interface PermissionResponseMessage { id: string; @@ -17,11 +21,18 @@ function deriveToolInput(request: PermissionRequest): unknown { return request.rawOutput; } -function pickOptionId(request: PermissionRequest, preferredKinds: string[]): string | null { +function pickOptionId( + request: PermissionRequest, + preferredKinds: string[], + options?: { fallbackToFirst?: boolean } +): string | null { for (const kind of preferredKinds) { const match = request.options.find((option) => option.kind === kind); if (match) return match.optionId; } + if (options?.fallbackToFirst === false) { + return null; + } return request.options.length > 0 ? request.options[0].optionId : null; } @@ -42,14 +53,20 @@ export class PermissionAdapter { } private handlePermissionRequest(request: PermissionRequest): void { - this.pendingRequests.set(request.id, request); - const toolName = deriveToolName({ title: request.title, kind: request.kind, rawInput: request.rawInput }); const input = deriveToolInput(request); + const autoDecision = resolveToolAutoApprovalDecision(undefined, toolName, request.toolCallId); + + if (autoDecision) { + void this.autoApproveRequest(request, toolName, input, autoDecision); + return; + } + + this.pendingRequests.set(request.id, request); this.session.updateAgentState((currentState) => ({ ...currentState, @@ -66,6 +83,50 @@ export class PermissionAdapter { logger.debug(`[ACP] Permission request queued: ${toolName} (${request.id})`); } + private async autoApproveRequest( + request: PermissionRequest, + toolName: string, + input: unknown, + decision: AutoApprovalDecision + ): Promise { + const optionId = pickOptionId( + request, + decision === 'approved_for_session' + ? ['allow_always', 'allow_once'] + : ['allow_once', 'allow_always'], + { fallbackToFirst: false } + ); + + const outcome: PermissionResponse = optionId + ? { outcome: 'selected', optionId } + : { outcome: 'cancelled' }; + + await this.backend.respondToPermission(request.sessionId, request, outcome); + + const timestamp = Date.now(); + const status = outcome.outcome === 'selected' ? 'approved' : 'canceled'; + + this.session.updateAgentState((currentState) => ({ + ...currentState, + completedRequests: { + ...currentState.completedRequests, + [request.id]: { + tool: toolName, + arguments: input, + createdAt: timestamp, + completedAt: timestamp, + status, + decision: outcome.outcome === 'selected' ? decision : 'abort' + } + } + } satisfies AgentState)); + + logger.debug( + `[ACP] Auto-${outcome.outcome === 'selected' ? 'approved' : 'cancelled'} ` + + `${toolName} (${request.id}) with decision=${decision}` + ); + } + private async handlePermissionResponse(response: PermissionResponseMessage): Promise { const pending = this.pendingRequests.get(response.id); if (!pending) { diff --git a/cli/src/codex/utils/permissionHandler.test.ts b/cli/src/codex/utils/permissionHandler.test.ts index 49c28014..f51ec3f2 100644 --- a/cli/src/codex/utils/permissionHandler.test.ts +++ b/cli/src/codex/utils/permissionHandler.test.ts @@ -35,6 +35,23 @@ function createHarness(mode: 'default' | 'read-only' | 'safe-yolo' | 'yolo') { } describe('CodexPermissionHandler', () => { + it('auto-approves change_title tools in default mode', async () => { + const { handler, getAgentState } = createHarness('default'); + + await expect(handler.handleToolCall('perm-1', 'mcp__hapi__change_title', { title: 'Rename' })).resolves.toEqual({ + decision: 'approved' + }); + + expect(getAgentState().requests).toEqual({}); + expect(getAgentState().completedRequests).toMatchObject({ + 'perm-1': { + tool: 'mcp__hapi__change_title', + status: 'approved', + decision: 'approved' + } + }); + }); + it('auto-approves yolo requests for the session', async () => { const { handler, getAgentState } = createHarness('yolo'); diff --git a/cli/src/modules/common/permission/BasePermissionHandler.ts b/cli/src/modules/common/permission/BasePermissionHandler.ts index eca40ef3..d4c9df58 100644 --- a/cli/src/modules/common/permission/BasePermissionHandler.ts +++ b/cli/src/modules/common/permission/BasePermissionHandler.ts @@ -10,7 +10,7 @@ type RpcHandlerManagerLike = { export type AutoApprovalDecision = 'approved' | 'approved_for_session'; -type AutoApprovalRuleSet = { +export type AutoApprovalRuleSet = { alwaysToolNameHints?: string[]; alwaysToolIdHints?: string[]; writeToolNameHints?: string[]; @@ -28,6 +28,46 @@ const AUTO_APPROVE_TOOL_NAME_HINTS = [ const AUTO_APPROVE_TOOL_ID_HINTS = ['change_title', 'save_memory']; const AUTO_APPROVE_WRITE_TOOL_HINTS = ['write', 'edit', 'create', 'delete', 'patch', 'fs-edit']; +export function resolveToolAutoApprovalDecision( + mode: PermissionMode | undefined, + toolName: string, + toolCallId: string, + ruleOverrides?: AutoApprovalRuleSet +): AutoApprovalDecision | null { + const rules = { + alwaysToolNameHints: ruleOverrides?.alwaysToolNameHints ?? AUTO_APPROVE_TOOL_NAME_HINTS, + alwaysToolIdHints: ruleOverrides?.alwaysToolIdHints ?? AUTO_APPROVE_TOOL_ID_HINTS, + writeToolNameHints: ruleOverrides?.writeToolNameHints ?? AUTO_APPROVE_WRITE_TOOL_HINTS + }; + + const lowerTool = toolName.toLowerCase(); + const lowerId = toolCallId.toLowerCase(); + const decisionForMode: AutoApprovalDecision = mode === 'yolo' ? 'approved_for_session' : 'approved'; + + if (rules.alwaysToolNameHints.some((name) => lowerTool.includes(name))) { + return decisionForMode; + } + + if (rules.alwaysToolIdHints.some((name) => lowerId.includes(name))) { + return decisionForMode; + } + + if (mode === 'yolo') { + return 'approved_for_session'; + } + + if (mode === 'safe-yolo') { + return 'approved'; + } + + if (mode === 'read-only') { + const isWriteTool = rules.writeToolNameHints.some((name) => lowerTool.includes(name)); + return isWriteTool ? null : 'approved'; + } + + return null; +} + export type PermissionHandlerClient = { rpcHandlerManager: RpcHandlerManagerLike; updateAgentState: (handler: (state: AgentState) => AgentState) => void; @@ -83,38 +123,7 @@ export abstract class BasePermissionHandler lowerTool.includes(name))) { - return decisionForMode; - } - - if (rules.alwaysToolIdHints.some((name) => lowerId.includes(name))) { - return decisionForMode; - } - - if (mode === 'yolo') { - return 'approved_for_session'; - } - - if (mode === 'safe-yolo') { - return 'approved'; - } - - if (mode === 'read-only') { - const isWriteTool = rules.writeToolNameHints.some((name) => lowerTool.includes(name)); - return isWriteTool ? null : 'approved'; - } - - return null; + return resolveToolAutoApprovalDecision(mode, toolName, toolCallId, ruleOverrides); } protected addPendingRequest(