diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index be776afd54a..9fc42359712 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -52778,6 +52778,34 @@ describe('Session', () => { ); }); + it('reports a returned interruption as interrupted and keeps its output', async () => { + const abortController = new AbortController(); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(true); + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO); + const execute = vi.fn().mockImplementation(async () => { + abortController.abort(); + return { + llmContent: 'Below is the output before it was cancelled.\npartial', + returnDisplay: 'partial', + interruptedBeforeCompletion: true, + }; + }); + mockToolRegistry.getTool.mockReturnValue( + mockAllowedTool('cancel_tool', execute), + ); + + const result = await ( + session as unknown as ToolCallInternals + ).runToolCalls(abortController.signal, 'prompt-interrupted-cancel', [ + { id: 'interrupted_cancel_call', name: 'cancel_tool', args: {} }, + ]); + + expect(result.parts[0].functionResponse?.response).toEqual({ + error: + 'User intentionally cancelled this tool call. Stop and await further instructions; do not retry or work around it.\n\nBelow is the output before it was cancelled.\npartial', + }); + }); + it('does not execute after cancellation settles during PreToolUse', async () => { const logToolCallSpy = vi .spyOn(core, 'logToolCall') diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index adc5da80f03..5757cd14e02 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -1043,6 +1043,10 @@ export const LOOP_DETECTED_TURN_ERROR_MESSAGE = const TOOL_EXECUTION_CANCELLED_MESSAGE = 'Tool execution was cancelled.'; const TOOL_POST_EXECUTION_CANCELLED_MESSAGE = 'The tool had already completed; its output was discarded.'; +// Mirrors the scheduler's wording for a producer that reports interruption by +// returning (the shell tool on user cancel) instead of throwing. +const TOOL_CANCELLED_BEFORE_COMPLETION_MESSAGE = + 'User intentionally cancelled this tool call. Stop and await further instructions; do not retry or work around it.'; const USER_PROMPT_BLOCKED_TOOL_RESULT = 'A UserPromptSubmit hook blocked this tool result from reaching the model. Do not rerun the tool to get it.'; @@ -19647,16 +19651,29 @@ export class Session implements SessionContext { } } + const interruptedBeforeCompletion = + toolResult.interruptedBeforeCompletion === true; + const cancelReason = interruptedBeforeCompletion + ? TOOL_CANCELLED_BEFORE_COMPLETION_MESSAGE + : TOOL_POST_EXECUTION_CANCELLED_MESSAGE; if ( activeToolAbortSignal.aborted && !(isExecutionTimeout && parentAbortedAtExecutionSettle) ) { status = 'cancelled'; + // Keep the producer's own partial output as the detail, the way + // the scheduler does, instead of discarding it with the notice. + const cancelDetail = + interruptedBeforeCompletion && + typeof toolResult.llmContent === 'string' && + toolResult.llmContent.trim() + ? `\n\n${toolResult.llmContent}` + : ''; responseParts = convertToFunctionErrorResponse( modelFacingToolName, callId, - TOOL_POST_EXECUTION_CANCELLED_MESSAGE, - TOOL_POST_EXECUTION_CANCELLED_MESSAGE, + `${cancelReason}${cancelDetail}`, + cancelReason, ); } terminalStatus = status; @@ -19666,7 +19683,7 @@ export class Session implements SessionContext { status === 'error' && toolResult.error ? new Error(toolResult.error.message) : status === 'cancelled' - ? new Error(TOOL_POST_EXECUTION_CANCELLED_MESSAGE) + ? new Error(cancelReason) : undefined; if (isTrustedTodoWriteTool && status === 'cancelled') { this.todoStopGuard.suspend(); diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 66f9133e01a..f48d0528f73 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -11225,6 +11225,17 @@ describe('CoreToolScheduler telemetry spans', () => { 'The tool had already completed', 'User intentionally cancelled this tool call. Stop', ], + [ + 'tells the model a returned interruption result never completed', + { + llmContent: + 'Command was cancelled by user before it could complete. There was no output before it was cancelled.', + returnDisplay: 'Command cancelled by user.', + interruptedBeforeCompletion: true, + }, + 'User intentionally cancelled this tool call.', + 'had already completed', + ], ])('%s', async (_title, outcome, notice, wrongNotice) => { const { call } = await runAbortedDuringExecution({}, outcome); @@ -11234,6 +11245,31 @@ describe('CoreToolScheduler telemetry spans', () => { expect(responseText).not.toContain(wrongNotice); }); + // The notice says the call did not finish; the producer's own text says how + // far it got. Replacing the whole payload with the notice loses the partial + // output the model needs to decide what remains. + it('keeps the partial output a cancelled producer reported', async () => { + const { call } = await runAbortedDuringExecution( + {}, + { + llmContent: + 'Command was cancelled by user before it could complete. Below is the output before it was cancelled:\nstart', + returnDisplay: 'Command cancelled by user.', + interruptedBeforeCompletion: true, + }, + ); + + const completedCall = expectSettled(call, 'cancelled', 'cancelled'); + const responseText = JSON.stringify(completedCall.response.responseParts); + expect(responseText).toContain( + 'User intentionally cancelled this tool call.', + ); + expect(responseText).toContain( + 'Below is the output before it was cancelled', + ); + expect(responseText).toContain('start'); + }); + // A post-execution cancellation drops the model-visible output, but the // references to files the tool already spilled to disk must survive — // otherwise nothing points at them and they are orphaned (#8180 review). diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 0e1bfc6875a..b06d68140b7 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -1275,13 +1275,19 @@ const createCancelledResponse = ( // model-visible output must not orphan files already persisted to disk. persistedOutputFiles?: string[], visionBridgeNotice?: string, + // A producer that reported its own interrupted work by returning (the shell + // tool on user cancel) already described what ran before the kill. Keep that + // text: the cancellation notice says the call never finished, the detail says + // how far it got. + detail?: string, ): CoreToolCallResponseInfo => { const cancellationPrefix = getModelFacingToolName(request) === ToolNames.TOOL_CALL && executionStatus === 'not_started' ? DEFERRED_TOOL_CALL_CANCELLATION_PREFIX : ''; - const errorMessage = `${cancellationPrefix}[Operation Cancelled] Reason: ${reason}`; + const errorDetail = detail?.trim() ? `\n\n${detail}` : ''; + const errorMessage = `${cancellationPrefix}[Operation Cancelled] Reason: ${reason}${errorDetail}`; return { callId: request.callId, responseParts: [ @@ -6597,8 +6603,12 @@ export class CoreToolScheduler { } if (aborted) { // PostToolUseFailure Hook - // `execute()` returned a result here, so the tool's work did finish. - let cancelMessage = TOOL_CANCELLED_AFTER_COMPLETION_MESSAGE; + // `execute()` returned a result, but that alone does not mean the work + // finished: producers that signal interruption by returning (the shell + // tool on user cancel) set `interruptedBeforeCompletion`. + let cancelMessage = toolResult.interruptedBeforeCompletion + ? TOOL_CANCELLED_BEFORE_COMPLETION_MESSAGE + : TOOL_CANCELLED_AFTER_COMPLETION_MESSAGE; let failureHookArtifacts: ToolArtifact[] | undefined; if (hooksEnabled && messageBus) { const failureHookResult = await this.withHookSpan( @@ -6642,6 +6652,11 @@ export class CoreToolScheduler { executionStatus, failureHookArtifacts, toolResult.persistedOutputFiles, + undefined, + typeof toolResult.llmContent === 'string' && + toolResult.interruptedBeforeCompletion + ? toolResult.llmContent + : undefined, ); observeProducerOutput(cancelledResponse, true); this.setStatusInternal(callId, 'cancelled', cancelledResponse); diff --git a/packages/core/src/tools/askUserQuestion.test.ts b/packages/core/src/tools/askUserQuestion.test.ts index 07d7eeaac9e..9a2408bd531 100644 --- a/packages/core/src/tools/askUserQuestion.test.ts +++ b/packages/core/src/tools/askUserQuestion.test.ts @@ -59,9 +59,9 @@ describe('AskUserQuestionTool', () => { questions: Question[], outcome: ToolConfirmationOutcome, answers?: Record, + signal: AbortSignal = new AbortController().signal, ) => { const invocation = tool.build({ questions }); - const signal = new AbortController().signal; const confirmation = await invocation.getConfirmationDetails(signal); await confirmation.onConfirm(outcome, answers && { answers }); return invocation.execute(signal); @@ -302,6 +302,22 @@ describe('AskUserQuestionTool', () => { ToolConfirmationOutcome.Cancel, ); expect(result.llmContent).toContain('declined to answer'); + // A decline is a completed decision, not interrupted work. + expect(result.interruptedBeforeCompletion).toBeUndefined(); + }); + + it('marks the unanswered questions as interrupted when the turn is aborted', async () => { + const controller = new AbortController(); + controller.abort(); + + const result = await answerAndRun( + [testQuestion({ multiSelect: false })], + ToolConfirmationOutcome.Cancel, + undefined, + controller.signal, + ); + expect(result.llmContent).toContain('declined to answer'); + expect(result.interruptedBeforeCompletion).toBe(true); }); it('should return formatted answers when user provides them', async () => { diff --git a/packages/core/src/tools/askUserQuestion.ts b/packages/core/src/tools/askUserQuestion.ts index abdb1d5a285..f78d3b31bed 100644 --- a/packages/core/src/tools/askUserQuestion.ts +++ b/packages/core/src/tools/askUserQuestion.ts @@ -250,7 +250,7 @@ class AskUserQuestionToolInvocation extends BaseToolInvocation< return details; } - async execute(_signal: AbortSignal): Promise { + async execute(signal: AbortSignal): Promise { try { // In non-interactive mode without ACP support, we cannot collect user input if (!this.canCollectAnswers()) { @@ -267,6 +267,9 @@ class AskUserQuestionToolInvocation extends BaseToolInvocation< return { llmContent: cancellationMessage, returnDisplay: cancellationMessage, + // Only a cancelled turn leaves the questions unanswered; a plain + // decline is a completed decision, not interrupted work. + ...(signal.aborted ? { interruptedBeforeCompletion: true } : {}), }; } diff --git a/packages/core/src/tools/create-sub-session.test.ts b/packages/core/src/tools/create-sub-session.test.ts index ff8994929ca..d217d926cbe 100644 --- a/packages/core/src/tools/create-sub-session.test.ts +++ b/packages/core/src/tools/create-sub-session.test.ts @@ -186,6 +186,8 @@ describe('CreateSubSessionTool', () => { expect(res.returnDisplay).toBe('Cancelled'); expect(res.llmContent).toMatch(/cancelled/i); + // Returned, not thrown: the scheduler must be told the call did not finish. + expect(res.interruptedBeforeCompletion).toBe(true); // The sub-session is NOT cancelled — it has no abort seam — so the tool // must say so rather than implying the work was undone. expect(res.llmContent).toMatch(/runs independently|not cancelled/i); diff --git a/packages/core/src/tools/create-sub-session.ts b/packages/core/src/tools/create-sub-session.ts index 846058df748..5460c5d0dfe 100644 --- a/packages/core/src/tools/create-sub-session.ts +++ b/packages/core/src/tools/create-sub-session.ts @@ -167,7 +167,13 @@ class CreateSubSessionInvocation extends BaseToolInvocation< 'been created; it runs independently and is not cancelled.' : 'create_sub_session was cancelled before it started. No ' + 'sub-session was created.'; - return { llmContent: message, returnDisplay: 'Cancelled' }; + return { + llmContent: message, + returnDisplay: 'Cancelled', + // The turn was cancelled mid-flight, so the call never reported a + // spawn result; the scheduler must not call it completed. + interruptedBeforeCompletion: true, + }; } // Embed a clickable session link in the display output so the web shell diff --git a/packages/core/src/tools/monitor.test.ts b/packages/core/src/tools/monitor.test.ts index 734f2e408e7..45a7c460a37 100644 --- a/packages/core/src/tools/monitor.test.ts +++ b/packages/core/src/tools/monitor.test.ts @@ -869,6 +869,9 @@ describe('MonitorTool', () => { expect(result.llmContent).toContain( 'Monitor was cancelled before it could start.', ); + // The tool returns on abort rather than throwing, so the scheduler needs + // this to report an interruption instead of a completed call. + expect(result.interruptedBeforeCompletion).toBe(true); expect(result.error).toBeUndefined(); }); diff --git a/packages/core/src/tools/monitor.ts b/packages/core/src/tools/monitor.ts index 295afeb17ce..e431249d0be 100644 --- a/packages/core/src/tools/monitor.ts +++ b/packages/core/src/tools/monitor.ts @@ -327,6 +327,9 @@ class MonitorToolInvocation extends BaseToolInvocation< return { llmContent: 'Monitor was cancelled before it could start.', returnDisplay: 'Monitor cancelled.', + // Returned rather than thrown, so the scheduler needs this to avoid + // reporting a monitor that never spawned as already completed. + interruptedBeforeCompletion: true, }; } diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index e4188b3282e..af50a1135ae 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -2195,6 +2195,33 @@ describe('ShellTool', () => { expectSpawned('npm test'); }); + it('marks a user-cancelled command as interrupted before completion', async () => { + const result = await runFg('npm test', { + output: 'start', + exitCode: null, + signal: 15, + aborted: true, + }); + + // The tool returns (rather than throws), so the scheduler needs this + // signal to avoid reporting a killed command as already completed. + expect(result.interruptedBeforeCompletion).toBe(true); + expect(String(result.llmContent)).toContain( + 'Command was cancelled by user before it could complete.', + ); + // What already ran is part of the interrupted account, not a detail the + // cancellation may drop. + expect(String(result.llmContent)).toContain( + 'Below is the output before it was cancelled:\nstart', + ); + }); + + it('does not mark a completed command as interrupted', async () => { + const result = await runFg('npm test', { output: 'done', exitCode: 0 }); + + expect(result.interruptedBeforeCompletion).toBeUndefined(); + }); + it('preserves shell wrapper environment and flags during foreground execution', async () => { const command = `FOO=bar bash -e -c 'echo "$FOO"; false; echo bad'`; await runFg(command); @@ -4477,6 +4504,8 @@ describe('ShellTool', () => { ['timed out'], ); expect(result.returnDisplay).toMatchObject({ outcome: 'completed' }); + // Neither a timeout nor a refused promote is a returned interruption. + expect(result.interruptedBeforeCompletion).toBeUndefined(); }); it('rethrows + kills child when registry.register throws — no orphan zombie', async () => { @@ -5369,6 +5398,8 @@ describe('ShellTool', () => { message: TIMED_OUT, type: ToolErrorType.EXECUTION_TIMEOUT, }); + // A timeout is reported as a tool error, never as a returned interruption. + expect(result.interruptedBeforeCompletion).toBeUndefined(); }); it('returns a structured timeout when the command produced no output', async () => { diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 24e056ac917..1717bbbd2eb 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -3459,6 +3459,11 @@ export class ShellToolInvocation extends BaseToolInvocation< }, ...(persistedOutputFiles !== undefined ? { persistedOutputFiles } : {}), ...(outputBudgetApplied ? { outputBudgetApplied } : {}), + // A user cancel kills the process group but returns a normal result, so + // tell the scheduler its work did not finish. + ...(result.aborted && !wasTimeout && !wasPromoteRefused + ? { interruptedBeforeCompletion: true } + : {}), ...executionError, }; } diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 170bf6adfe0..e53c9a270aa 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -629,6 +629,13 @@ export interface ToolResult { */ outputBudgetApplied?: boolean; + /** + * Set by a producer that knows its own work was cut short but reports that + * outcome as an ordinary result instead of throwing. The scheduler uses it to + * tell the model the call was interrupted rather than completed. + */ + interruptedBeforeCompletion?: boolean; + /** * Markdown string for user display. * This provides a user-friendly summary or visualization of the result. diff --git a/packages/core/src/tools/workflow/workflow.test.ts b/packages/core/src/tools/workflow/workflow.test.ts index ec635197db5..236e02a3008 100644 --- a/packages/core/src/tools/workflow/workflow.test.ts +++ b/packages/core/src/tools/workflow/workflow.test.ts @@ -71,6 +71,7 @@ const live = () => new AbortController().signal; const CANCELLED_BEFORE_START = { llmContent: 'Workflow was cancelled before it could start.', returnDisplay: 'Workflow cancelled.', + interruptedBeforeCompletion: true, }; const runIdOf = (scriptPath: string) => scriptPath.match(/(wf_[0-9a-f]+)\.js$/)![1]; diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 24807d4add3..d6ffdf5a9aa 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -967,6 +967,9 @@ function startCancelledResult(): WorkflowToolResult { return { llmContent: 'Workflow was cancelled before it could start.', returnDisplay: 'Workflow cancelled.', + // Same returned-interruption shape as the monitor: nothing ran, so the + // scheduler must not tell the model the call completed. + interruptedBeforeCompletion: true, }; }