-
Notifications
You must be signed in to change notification settings - Fork 914
fix(agent-core): surface user interruptions to the model instead of a neutral abort #236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<ExecutableTool, ToolArgsValidator>(); | ||
|
|
||
| /** | ||
| * 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.`; | ||
|
Comment on lines
+57
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For tools that observe neither the abort signal nor settle within the 2s grace window after the user presses stop, Useful? React with 👍 / 👎.
Comment on lines
+57
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Some tools handle an aborted signal by returning their own error result instead of throwing (for example Grep returns Useful? React with 👍 / 👎. |
||
| } | ||
| 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<PreparedToolCallTask> => | ||
| 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); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the user presses stop while a tool hook is pending (for example an approval/authorization hook waiting on input), the hook catch paths still return neutral messages such as
Tool "X" was aborted during authorizeToolExecution hookinstead of going through this new user-cancellation wording. SinceTurnFlow.cancel()now marks direct cancels withUserCancellationError, that user-initiated path can still persist a neutral tool result for the model to see on the next turn, which undercuts the fix for common cancellation points before the tool actually starts executing.Useful? React with 👍 / 👎.