From 6e20b30ec623bbf6e457a61a8d3cd174290a7829 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Sat, 30 May 2026 08:58:19 +0800 Subject: [PATCH] fix(agent-core): surface user interruptions to the model instead of a neutral abort When the user interrupts running tools or parallel subagents, the tool_result fed back to the model was a neutral `Tool "X" was aborted` or a weak "stopped by the user", so the model treated it as a system fault and speculated about capacity/concurrency limits instead of recognizing a deliberate stop. Carry a UserCancellationError as the AbortSignal reason from the cancel sites (Turn.cancel/abortTurn, SessionSubagentHost.cancelAll) through to the message sites (tool-call settle paths and the AgentTool catches), which now emit an explicit "deliberate user action, not a system error/timeout/capacity limit" message. Aborts propagated from another signal (e.g. a subagent's deadline via waitForCurrentTurn) carry their original reason, so a timeout is not mislabeled as a user interruption. The telemetry outcome classifier matches the new "manually interrupted" phrase to keep counting these as cancelled. --- packages/agent-core/src/agent/turn/index.ts | 29 +++++-- packages/agent-core/src/loop/tool-call.ts | 20 ++++- .../agent-core/src/session/subagent-host.ts | 10 ++- .../src/tools/builtin/collaboration/agent.ts | 16 +++- packages/agent-core/src/utils/abort.ts | 28 +++++++ packages/agent-core/test/agent/turn.test.ts | 8 +- .../agent-core/test/loop/abort.e2e.test.ts | 31 +++++++ .../test/session/subagent-host.test.ts | 82 ++++++++++++++++++- packages/agent-core/test/tools/agent.test.ts | 42 ++++++++++ packages/agent-core/test/utils/abort.test.ts | 23 ++++++ 10 files changed, 266 insertions(+), 23 deletions(-) create mode 100644 packages/agent-core/test/utils/abort.test.ts diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index 068d5626c9..7ed428b8f6 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -32,7 +32,7 @@ import { } from '../../loop/index'; import type { AgentEvent, TurnEndedEvent } from '../../rpc'; import type { TelemetryPropertyValue } from '../../telemetry'; -import { abortable } from '../../utils/abort'; +import { abortable, userCancellationReason } from '../../utils/abort'; import { USER_PROMPT_ORIGIN, type PromptOrigin } from '../context'; import { renderUserPromptHookBlockResult, renderUserPromptHookResult } from '../../session/hooks'; import { canonicalTelemetryArgs, isPlainRecord } from './canonical-args'; @@ -147,13 +147,18 @@ export class TurnFlow { this.activeTurn = 'resuming'; } - cancel(turnId?: number): void { + cancel(turnId?: number, reason?: unknown): void { this.agent.records.logRecord({ type: 'turn.cancel', turnId }); if (turnId !== undefined && turnId !== this.currentId) { return; // Ignore cancel for non-active turn } - this.abortTurn(); - this.agent.subagentHost?.cancelAll(); + // A direct cancel (RPC / replay) is the user pressing stop. When the cancel + // is propagated from an aborting signal (e.g. a subagent's deadline via + // waitForCurrentTurn), carry that original reason instead so a timeout is + // not mislabeled to the model as a deliberate user interruption. + const cancelReason = reason ?? userCancellationReason(); + this.abortTurn(cancelReason); + this.agent.subagentHost?.cancelAll(cancelReason); } get currentId() { @@ -174,7 +179,7 @@ export class TurnFlow { const turnId = this.currentId; const onAbort = (): void => { - this.agent.turn.cancel(turnId); + this.agent.turn.cancel(turnId, signal.reason); }; signal.addEventListener('abort', onAbort, { once: true }); @@ -183,9 +188,13 @@ export class TurnFlow { }); } - private abortTurn() { + private abortTurn(reason: unknown) { if (this.activeTurn !== 'resuming') { - this.activeTurn?.controller.abort(); + // The reason (a user cancellation by default, or the originating signal's + // reason when propagated) travels as signal.reason so tools settling on + // this signal can report a deliberate user interruption distinctly from a + // timeout/system abort. linkAbortSignal forwards it to linked subagents. + this.activeTurn?.controller.abort(reason); } this.activeTurn = null; } @@ -798,7 +807,11 @@ type ToolTelemetryResult = Extract['result'] function telemetryToolOutcome(result: ToolTelemetryResult): 'success' | 'error' | 'cancelled' { if (result.isError !== true) return 'success'; const text = toolResultText(result).toLowerCase(); - return text.includes('aborted') || text.includes('cancelled') ? 'cancelled' : 'error'; + return text.includes('aborted') || + text.includes('cancelled') || + text.includes('manually interrupted') + ? 'cancelled' + : 'error'; } function telemetryToolErrorType(result: ToolTelemetryResult): string { diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index c86a5705a9..2e9956ec0f 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -24,6 +24,7 @@ import { } from '../tools/args-validator'; import { PathSecurityError } from '../tools/policies/path-access'; +import { isUserCancellation } from '../utils/abort'; import { errorMessage, isAbortError } from './errors'; import type { LoopEventDispatcher, LoopToolCallEvent } from './events'; import type { LLM, LLMChatResponse } from './llm'; @@ -46,6 +47,19 @@ const TOOL_OUTPUT_NON_TEXT = 'Tool returned non-text content.'; const validators = new WeakMap(); +/** + * Output for an aborted tool call. When the abort carries a user-cancellation + * reason (the user pressed stop), say so explicitly so the model treats it as a + * deliberate interruption instead of a system fault to theorise about or retry. + * Any other abort keeps the neutral wording. + */ +function abortedToolOutput(toolName: string, signal: AbortSignal): string { + if (isUserCancellation(signal.reason)) { + return `The user manually interrupted "${toolName}" (and anything else running at the same time). This was a deliberate user action, not a system error, timeout, or capacity limit. Do not retry automatically or guess at the cause — wait for the user's next instruction.`; + } + return `Tool "${toolName}" was aborted`; +} + export interface ToolCallStepContext { readonly tools?: readonly ExecutableTool[] | undefined; readonly hooks?: LoopHooks | undefined; @@ -285,7 +299,7 @@ async function prepareToolCall( const displayFields = toolCallDisplayFieldsFromExecution(execution); const settleAborted = (): Promise => - settleError(effectiveArgs, `Tool "${call.toolName}" was aborted`, displayFields); + settleError(effectiveArgs, abortedToolOutput(call.toolName, step.signal), displayFields); if (step.signal.aborted) return settleAborted(); @@ -452,7 +466,7 @@ async function runRunnableToolCall( const { toolCall, toolName } = call; if (signal.aborted) { - return makeErrorToolResult(call, effectiveArgs, `Tool "${toolName}" was aborted`); + return makeErrorToolResult(call, effectiveArgs, abortedToolOutput(toolName, signal)); } let toolResult: ExecutableToolResult; @@ -469,7 +483,7 @@ async function runRunnableToolCall( }); } const output = aborted - ? `Tool "${toolName}" was aborted` + ? abortedToolOutput(toolName, signal) : `Tool "${toolName}" failed: ${errorMessage(error)}`; return makeErrorToolResult(call, effectiveArgs, output); } diff --git a/packages/agent-core/src/session/subagent-host.ts b/packages/agent-core/src/session/subagent-host.ts index 49861e9d2e..458e291e23 100644 --- a/packages/agent-core/src/session/subagent-host.ts +++ b/packages/agent-core/src/session/subagent-host.ts @@ -8,7 +8,7 @@ import { prepareSystemPromptContext, type ResolvedAgentProfile, } from '../profile'; -import { linkAbortSignal } from '../utils/abort'; +import { linkAbortSignal, userCancellationReason } from '../utils/abort'; import { collectGitContext } from './git-context'; import type { Session } from './index'; import SUMMARY_CONTINUATION_PROMPT from './summary-continuation.md'; @@ -167,13 +167,15 @@ export class SessionSubagentHost { }; } - cancelAll(): void { + cancelAll(reason: unknown = userCancellationReason()): void { const foregroundChildren = Array.from(this.activeChildren).filter( ([, child]) => !child.runInBackground, ); for (const [childId, child] of foregroundChildren) { - this.session.agents.get(childId)?.subagentHost?.cancelAll(); - child.controller.abort(); + this.session.agents.get(childId)?.subagentHost?.cancelAll(reason); + // Abort with the cancel reason (a user interruption by default) so the + // subagent's in-flight tools report the cause accurately to the model. + child.controller.abort(reason); } } diff --git a/packages/agent-core/src/tools/builtin/collaboration/agent.ts b/packages/agent-core/src/tools/builtin/collaboration/agent.ts index 943d801fbf..a726217cdf 100644 --- a/packages/agent-core/src/tools/builtin/collaboration/agent.ts +++ b/packages/agent-core/src/tools/builtin/collaboration/agent.ts @@ -25,7 +25,11 @@ import { isAbortError } from '../../../loop/errors'; import type { ExecutableToolContext, ExecutableToolResult, ToolExecution } from '../../../loop/types'; import type { ResolvedAgentProfile } from '../../../profile'; import type { SessionSubagentHost, SubagentHandle } from '../../../session/subagent-host'; -import { createDeadlineAbortSignal, type DeadlineAbortSignal } from '../../../utils/abort'; +import { + createDeadlineAbortSignal, + isUserCancellation, + type DeadlineAbortSignal, +} from '../../../utils/abort'; import type { BackgroundProcessManager } from '../../background/manager'; import { toInputJsonSchema } from '../../support/input-schema'; import { matchesGlobRuleSubject } from '../../support/rule-match'; @@ -303,8 +307,11 @@ export class AgentTool implements BuiltinTool { let message: string; if (foregroundDeadline?.timedOut() === true && args.timeout !== undefined) { message = `Agent timed out after ${args.timeout}s.`; + } else if (isUserCancellation(signal.reason)) { + message = + 'The user manually interrupted this subagent (and any sibling agents launched alongside it). This was a deliberate user action, not a system error, a timeout, or a capacity/concurrency limit. Do not retry automatically or speculate about why it failed — wait for the user\'s next instruction.'; } else if (isAbortError(error)) { - message = 'The subagent was stopped by the user.'; + message = 'The subagent was stopped before it finished.'; } else { message = error instanceof Error ? error.message : String(error); } @@ -321,8 +328,11 @@ export class AgentTool implements BuiltinTool { let message: string; if (foregroundDeadline?.timedOut() === true && args.timeout !== undefined) { message = `Agent timed out after ${args.timeout}s.`; + } else if (isUserCancellation(signal.reason)) { + message = + 'The user manually interrupted this subagent (and any sibling agents launched alongside it). This was a deliberate user action, not a system error, a timeout, or a capacity/concurrency limit. Do not retry automatically or speculate about why it failed — wait for the user\'s next instruction.'; } else if (isAbortError(error)) { - message = 'The subagent was stopped by the user.'; + message = 'The subagent was stopped before it finished.'; } else { message = error instanceof Error ? error.message : String(error); } diff --git a/packages/agent-core/src/utils/abort.ts b/packages/agent-core/src/utils/abort.ts index 61e3f16856..ba5f8770f3 100644 --- a/packages/agent-core/src/utils/abort.ts +++ b/packages/agent-core/src/utils/abort.ts @@ -4,6 +4,34 @@ export function abortError(): Error { return error; } +/** + * Marks an abort the user triggered deliberately (e.g. pressing ESC to + * interrupt the agent), as distinct from a timeout, an internal error, or any + * other programmatic abort. It travels as the AbortSignal's `reason`, so code + * that settles an interrupted operation can tell a user interruption apart from + * a failure and report it to the model accordingly instead of emitting a + * neutral "was aborted" that the model mistakes for a system problem. + * + * `name` stays 'AbortError' so existing `isAbortError()` checks (and + * `AbortSignal.throwIfAborted()`) keep treating it as an abort. + */ +export class UserCancellationError extends Error { + readonly userCancelled = true; + + constructor() { + super('Aborted by the user'); + this.name = 'AbortError'; + } +} + +export function userCancellationReason(): UserCancellationError { + return new UserCancellationError(); +} + +export function isUserCancellation(value: unknown): value is UserCancellationError { + return value instanceof UserCancellationError; +} + export function abortable(promise: Promise, signal: AbortSignal): Promise { signal.throwIfAborted(); return new Promise((resolve, reject) => { diff --git a/packages/agent-core/test/agent/turn.test.ts b/packages/agent-core/test/agent/turn.test.ts index 094e06fcb6..d82e354fe6 100644 --- a/packages/agent-core/test/agent/turn.test.ts +++ b/packages/agent-core/test/agent/turn.test.ts @@ -1261,8 +1261,8 @@ describe('Agent turn flow', () => { [wire] turn.cancel { "turnId": 0, "time": "