mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
🛑 fix: Forward Run Abort Signal to Foreground Tool Calls
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.
This commit is contained in:
parent
d5b2a8580e
commit
48fded564a
2 changed files with 102 additions and 1 deletions
|
|
@ -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<string, unknown>) => {
|
||||
capturedSignals.push(config.signal as AbortSignal | undefined);
|
||||
await new Promise<void>((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<ToolExecuteResult[]>((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<string, unknown>) =>
|
||||
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<ToolExecuteResult[]>((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[] = [];
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>);
|
||||
|
||||
/* 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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue