diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 4b5229321..c9ecf3c37 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -346,6 +346,71 @@ describe('Session', () => { ); }); + it('allows info confirmation tools in plan mode', async () => { + const executeSpy = vi.fn().mockResolvedValue({ + llmContent: 'ok', + returnDisplay: 'ok', + }); + const onConfirmSpy = vi.fn().mockResolvedValue(undefined); + const invocation = { + params: { + url: 'https://example.com/docs', + prompt: 'Summarize the docs', + }, + getDefaultPermission: vi.fn().mockResolvedValue('ask'), + getConfirmationDetails: vi.fn().mockResolvedValue({ + type: 'info', + title: 'Confirm Web Fetch', + prompt: 'Allow fetching docs?', + urls: ['https://example.com/docs'], + onConfirm: onConfirmSpy, + }), + getDescription: vi.fn().mockReturnValue('Fetch docs'), + toolLocations: vi.fn().mockReturnValue([]), + execute: executeSpy, + }; + const tool = { + name: 'web_fetch', + kind: core.Kind.Fetch, + build: vi.fn().mockReturnValue(invocation), + }; + + mockToolRegistry.getTool.mockReturnValue(tool); + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.PLAN); + mockConfig.getPermissionManager = vi.fn().mockReturnValue(null); + mockChat.sendMessageStream = vi.fn().mockResolvedValue( + (async function* () { + yield { + type: core.StreamEventType.CHUNK, + value: { + functionCalls: [ + { + id: 'call-info-plan', + name: 'web_fetch', + args: { + url: 'https://example.com/docs', + prompt: 'Summarize the docs', + }, + }, + ], + }, + }; + })(), + ); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'research the docs first' }], + }); + + expect(mockClient.requestPermission).toHaveBeenCalled(); + expect(onConfirmSpy).toHaveBeenCalledWith( + core.ToolConfirmationOutcome.ProceedOnce, + { answers: undefined }, + ); + expect(executeSpy).toHaveBeenCalled(); + }); + it('returns permission error for disabled tools (L1 isToolEnabled check)', async () => { const executeSpy = vi.fn(); const invocation = { diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index fd009dddf..acea4ebb2 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -674,22 +674,6 @@ export class Session implements SessionContext { const approvalMode = this.config.getApprovalMode(); const isPlanMode = approvalMode === ApprovalMode.PLAN; - // PLAN mode: block non-read-only tools - if ( - isPlanMode && - !isExitPlanModeTool && - !isAskUserQuestionTool && - needsConfirmation - ) { - return earlyErrorResponse( - new Error( - `Plan mode is active. The tool "${fc.name}" cannot be executed because it modifies the system. ` + - 'Please use the exit_plan_mode tool to present your plan and exit plan mode before making changes.', - ), - fc.name, - ); - } - if (finalPermission === 'deny') { return earlyErrorResponse( new Error( @@ -702,14 +686,30 @@ export class Session implements SessionContext { } let didRequestPermission = false; + let confirmationDetails: ToolCallConfirmationDetails | undefined; if (needsConfirmation) { - const confirmationDetails = + confirmationDetails = await invocation.getConfirmationDetails(abortSignal); // Centralised rule injection (for display and persistence) injectPermissionRulesIfMissing(confirmationDetails, pmCtx); + if ( + isPlanMode && + !isExitPlanModeTool && + !isAskUserQuestionTool && + confirmationDetails.type !== 'info' + ) { + return earlyErrorResponse( + new Error( + `Plan mode is active. The tool "${fc.name}" cannot be executed because it modifies the system. ` + + 'Please use the exit_plan_mode tool to present your plan and exit plan mode before making changes.', + ), + fc.name, + ); + } + const messageBus = this.config.getMessageBus?.(); const hooksEnabled = this.config.getEnableHooks?.() ?? false; let hookHandled = false; diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 83cf69033..1799ed56e 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -2480,6 +2480,70 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => { } }); + it('should allow info confirmation tools in plan mode after approval', async () => { + const onConfirmSpy = vi.fn().mockResolvedValue(undefined); + const infoTool = new MockTool({ + name: 'web_fetch', + getDefaultPermission: async () => 'ask', + getConfirmationDetails: async () => ({ + type: 'info' as const, + title: 'Confirm Web Fetch', + prompt: 'Fetch https://example.com/docs', + urls: ['https://example.com/docs'], + onConfirm: onConfirmSpy, + }), + execute: async () => ({ + llmContent: 'Fetched docs', + returnDisplay: 'Fetched docs', + }), + }); + const onAllToolCallsComplete = vi.fn(); + const onToolCallsUpdate = vi.fn(); + const scheduler = createPlanModeScheduler( + infoTool, + onAllToolCallsComplete, + onToolCallsUpdate, + ); + + const abortController = new AbortController(); + const request = { + callId: '1', + name: 'web_fetch', + args: { + url: 'https://example.com/docs', + prompt: 'Summarize the API docs', + }, + isClientInitiated: false, + prompt_id: 'prompt-plan-info', + }; + + await scheduler.schedule([request], abortController.signal); + + const awaitingCall = (await waitForStatus( + onToolCallsUpdate, + 'awaiting_approval', + )) as WaitingToolCall; + + expect(awaitingCall.confirmationDetails.type).toBe('info'); + + await awaitingCall.confirmationDetails.onConfirm( + ToolConfirmationOutcome.ProceedOnce, + ); + + await vi.waitFor(() => { + expect(onAllToolCallsComplete).toHaveBeenCalled(); + }); + + expect(onConfirmSpy).toHaveBeenCalledWith( + ToolConfirmationOutcome.ProceedOnce, + undefined, + ); + + const completedCalls = onAllToolCallsComplete.mock + .calls[0][0] as ToolCall[]; + expect(completedCalls[0].status).toBe('success'); + }); + it('should handle user cancellation of ask_user_question in plan mode', async () => { const mockTool = createAskUserQuestionMockTool(); const onAllToolCallsComplete = vi.fn(); diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 959c86e99..6af1107f9 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -903,6 +903,7 @@ export class CoreToolScheduler { // it must bypass both YOLO auto-approve and plan-mode blocking. const isAskUserQuestionTool = reqInfo.name === ToolNames.ASK_USER_QUESTION; + let confirmationDetails: ToolCallConfirmationDetails | undefined; if (approvalMode === ApprovalMode.YOLO && !isAskUserQuestionTool) { this.setToolCallOutcome( @@ -910,30 +911,33 @@ export class CoreToolScheduler { ToolConfirmationOutcome.ProceedAlways, ); this.setStatusInternal(reqInfo.callId, 'scheduled'); - } else if ( - isPlanMode && - !isExitPlanModeTool && - !isAskUserQuestionTool - ) { - this.setStatusInternal(reqInfo.callId, 'error', { - callId: reqInfo.callId, - responseParts: convertToFunctionResponse( - reqInfo.name, - reqInfo.callId, - getPlanModeSystemReminder(), - ), - resultDisplay: 'Plan mode blocked a non-read-only tool call.', - error: undefined, - errorType: undefined, - }); } else { - // Get confirmation details from the tool - const confirmationDetails = + confirmationDetails = await invocation.getConfirmationDetails(signal); // ── Centralised rule injection ────────────────────────────────── injectPermissionRulesIfMissing(confirmationDetails, pmCtx); + if ( + isPlanMode && + !isExitPlanModeTool && + !isAskUserQuestionTool && + confirmationDetails.type !== 'info' + ) { + this.setStatusInternal(reqInfo.callId, 'error', { + callId: reqInfo.callId, + responseParts: convertToFunctionResponse( + reqInfo.name, + reqInfo.callId, + getPlanModeSystemReminder(), + ), + resultDisplay: 'Plan mode blocked a non-read-only tool call.', + error: undefined, + errorType: undefined, + }); + continue; + } + // AUTO_EDIT mode: auto-approve edit-like and info tools if ( approvalMode === ApprovalMode.AUTO_EDIT && @@ -990,14 +994,14 @@ export class CoreToolScheduler { if (resolution.status === 'accepted') { this.handleConfirmationResponse( reqInfo.callId, - confirmationDetails.onConfirm, + confirmationDetails!.onConfirm, ToolConfirmationOutcome.ProceedOnce, signal, ); } else { this.handleConfirmationResponse( reqInfo.callId, - confirmationDetails.onConfirm, + confirmationDetails!.onConfirm, ToolConfirmationOutcome.Cancel, signal, );