From 48fded564a30ae80ff5e2a7300a2b17b672a596e Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 2 Sep 2026 11:47:24 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=91=20fix:=20Forward=20Run=20Abort=20S?= =?UTF-8?q?ignal=20to=20Foreground=20Tool=20Calls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stopping a generation left in-flight tool calls running on the far side. For MCP over streamable-http the external server kept burning CPU until the tool finished or timed out, with no `notifications/cancelled` ever sent. The MCP layer was never the problem: `MCPManager.callTool` already spreads `options.signal` into `client.request`, and the SDK's `Protocol.request` sends `notifications/cancelled` the moment that signal aborts. The signal simply never arrived. Agent tools do not execute inside LangGraph's ToolNode — `eventDrivenMode` dispatches `on_tool_execute` and `createToolExecuteHandler` invokes them. The agents SDK puts the run's abort signal on the batch request (`ToolExecuteBatchRequest.signal`, documented as one handlers should forward), but the handler destructured every field except `signal` and built its invoke config without it. `config.signal` was therefore `undefined` in every foreground tool, so `createMCPTool` derived no signal and the SDK registered no abort listener. Forward it. This restores cancellation for all signal-aware foreground tools, not just MCP. The detached background invoke keeps its own controller, since that work deliberately outlives the turn. An aborted call now rejects promptly and resolves an error result rather than hanging, so the abort is logged at debug instead of as a tool error — a user pressing Stop should not spray the error log. --- packages/api/src/agents/handlers.spec.ts | 77 ++++++++++++++++++++++++ packages/api/src/agents/handlers.ts | 26 +++++++- 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/packages/api/src/agents/handlers.spec.ts b/packages/api/src/agents/handlers.spec.ts index d39e6b261b..fbdcf02fcd 100644 --- a/packages/api/src/agents/handlers.spec.ts +++ b/packages/api/src/agents/handlers.spec.ts @@ -309,6 +309,83 @@ describe('createToolExecuteHandler', () => { }); }); + describe('run cancellation', () => { + it('forwards the batch abort signal into foreground tool invocations', async () => { + const capturedSignals: (AbortSignal | undefined)[] = []; + const controller = new AbortController(); + const tool = { + name: 'slow_tool', + invoke: jest.fn(async (_args: unknown, config: Record) => { + capturedSignals.push(config.signal as AbortSignal | undefined); + await new Promise((resolve, reject) => { + const signal = config.signal as AbortSignal | undefined; + if (signal == null) { + setTimeout(resolve, 50); + return; + } + signal.addEventListener('abort', () => reject(signal.reason), { once: true }); + }); + return { content: 'never' }; + }), + }; + const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({ + loadedTools: [tool] as never[], + })); + const handler = createToolExecuteHandler({ loadTools }); + + const results = await new Promise((resolve, reject) => { + const request: ToolExecuteBatchRequest = { + toolCalls: [{ id: 'call-1', name: 'slow_tool', args: {} }] as ToolCallRequest[], + signal: controller.signal, + resolve, + reject, + }; + handler.handle('on_tool_execute', request); + setTimeout(() => controller.abort(new Error('Operation aborted by user')), 10); + }); + + expect(capturedSignals[0]).toBe(controller.signal); + expect(results).toHaveLength(1); + expect(results[0].status).toBe('error'); + }); + + it('logs a cancelled tool call as debug rather than a tool error', async () => { + const errorSpy = jest.spyOn(logger, 'error').mockReturnValue(logger); + const controller = new AbortController(); + const tool = { + name: 'slow_tool', + invoke: jest.fn( + (_args: unknown, config: Record) => + new Promise((_resolve, reject) => { + const signal = config.signal as AbortSignal; + signal.addEventListener('abort', () => reject(signal.reason), { once: true }); + }), + ), + }; + const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({ + loadedTools: [tool] as never[], + })); + const handler = createToolExecuteHandler({ loadTools }); + + await new Promise((resolve, reject) => { + const request: ToolExecuteBatchRequest = { + toolCalls: [{ id: 'call-1', name: 'slow_tool', args: {} }] as ToolCallRequest[], + signal: controller.signal, + resolve, + reject, + }; + handler.handle('on_tool_execute', request); + setTimeout(() => controller.abort(new Error('Operation aborted by user')), 10); + }); + + expect( + errorSpy.mock.calls.filter(([message]) => + String(message).includes('[ON_TOOL_EXECUTE] Tool slow_tool error'), + ), + ).toHaveLength(0); + }); + }); + describe('tool argument normalization', () => { it('parses JSON-string args for object-schema tools before invocation', async () => { const capturedArgs: unknown[] = []; diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index c06620124d..958f887a90 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -4411,7 +4411,15 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand return { handle: async (_event: string, data: ToolExecuteBatchRequest) => { - const { toolCalls, agentId, configurable, metadata, resolve, reject } = data; + const { + toolCalls, + agentId, + configurable, + metadata, + signal: runSignal, + resolve, + reject, + } = data; const callerCapabilityProjection = resolveCallerCapabilityProjectionSnapshot( ( data as ToolExecuteBatchRequest & { @@ -5884,6 +5892,13 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand toolCall: toolCallConfig, configurable: mergedConfigurable, metadata, + /** The run's cancellation signal. Without it a foreground + * tool call keeps running after Stop: an MCP call never + * sends `notifications/cancelled`, and every other + * signal-aware tool keeps burning quota on a turn the + * user already abandoned. Detached background calls + * intentionally use their own controller instead. */ + ...(runSignal != null && { signal: runSignal }), } as Record); /* Only sandbox-bound calls carry a runtime session hint, so @@ -5981,6 +5996,15 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand return errorResult(tc, modelBoundContentFilterErrorMessage(toolError.body)); } const { message, logContext } = getSafeToolError(toolError); + /** A user Stop rejects every in-flight call at once. That is + * the abort working, not a fault, so it stays out of the + * error log; the message still reaches the model turn. */ + if (runSignal?.aborted === true) { + logger.debug(`[ON_TOOL_EXECUTE] Tool ${tc.name} cancelled by run abort`, { + name: logContext.name, + }); + return errorResult(tc, message); + } const req = mergedConfigurable?.req as ServerRequest | undefined; const filteredError = filteredToolOutputResult(tc, req, { errorMessage: message,