Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/cli/src/nonInteractiveCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,13 @@ export async function runNonInteractive(
}

if (toolCallRequests.length > 0) {
// Re-check the abort signal before scheduling any tool calls.
// A SIGINT can fire after the stream loop has already buffered
// a tool-call event; without this guard the side effect would
// execute even though the user has cancelled.
if (abortController.signal.aborted) {
handleCancellationError(config);
}
textOutput.ensureTrailingNewline();
const completedToolCalls = await scheduler.schedule(
toolCallRequests,
Expand Down
48 changes: 48 additions & 0 deletions packages/core/src/core/turn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,54 @@ describe('Turn', () => {
expect(turn.getDebugResponses().length).toBe(1);
});

it('should not yield tool_call_request when signal is aborted before a function-call chunk is materialized', async () => {
// Regression test for #28091: a SIGINT can fire after the top-of-loop
// abort check but before we materialize tool calls from a chunk that
// arrived after cancellation. Without the extra guard the side effect
// runs after the user cancelled.
const abortController = new AbortController();
const mockResponseStream = (async function* () {
yield {
type: StreamEventType.CHUNK,
value: {
candidates: [{ content: { parts: [{ text: 'First part' }] } }],
} as GenerateContentResponse,
};
abortController.abort();
yield {
type: StreamEventType.CHUNK,
value: {
functionCalls: [
{
id: 'late-call',
name: 'late_after_cancel',
args: {},
isClientInitiated: false,
},
],
} as unknown as GenerateContentResponse,
};
})();
mockSendMessageStream.mockResolvedValue(mockResponseStream);

const events = [];
const reqParts: Part[] = [{ text: 'Test late tool call' }];
for await (const event of turn.run(
{ model: 'gemini' },
reqParts,
abortController.signal,
)) {
events.push(event);
}

expect(events).toEqual([
{ type: GeminiEventType.Content, value: 'First part' },
{ type: GeminiEventType.UserCancelled },
]);
// No tool call should have been queued for the scheduler.
expect(turn.pendingToolCalls).toEqual([]);
});

it('should yield InvalidStream event if sendMessageStream throws InvalidStreamError', async () => {
const error = new InvalidStreamError(
'Test invalid stream',
Expand Down
12 changes: 11 additions & 1 deletion packages/core/src/core/turn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,18 @@ export class Turn {
yield { type: GeminiEventType.Content, value: text, traceId };
}

// Handle function calls (requesting tool execution)
// Handle function calls (requesting tool execution).
//
// Re-check the abort signal here: a SIGINT can fire between the
// top-of-loop check above and the moment we materialize tool calls
// from a chunk that arrived after cancellation. Without this guard
// a delayed function-call chunk is yielded to the scheduler and
// the side effect runs after the user cancelled.
const functionCalls = resp.functionCalls ?? [];
if (functionCalls.length > 0 && signal?.aborted) {
yield { type: GeminiEventType.UserCancelled };
return;
}
for (const fnCall of functionCalls) {
const event = this.handlePendingFunctionCall(fnCall, traceId);
if (event) {
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/scheduler/scheduler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,21 @@ describe('Scheduler (Orchestrator)', () => {
expect(mockStateManager.dequeue).not.toHaveBeenCalled(); // Loop broke
});

it('should not enqueue or validate tool calls when scheduled with an already-aborted signal (regression #28091)', async () => {
// If a delayed tool-call chunk reaches the scheduler after the user
// cancelled, we must not invoke the tool registry / validators or
// enqueue the call — the late side effect would run before the queue
// processor's own abort check kicked in.
abortController.abort();

await scheduler.schedule(req1, signal);

expect(mockStateManager.enqueue).not.toHaveBeenCalled();
expect(mockStateManager.cancelAllQueued).toHaveBeenCalledWith(
'Operation cancelled',
);
});

it('cancelAll() should cancel active call and clear queue', () => {
const activeCall: ValidatingToolCall = {
status: CoreToolCallStatus.Validating,
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/scheduler/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,18 @@ export class Scheduler {
this.isProcessing = true;
this.isCancelling = false;
this.state.clearBatch();

// Guard against late-arriving requests scheduled after cancellation:
// if the caller's signal is already aborted, short-circuit instead of
// validating + enqueuing tool calls that would then execute their
// local side effects before the queue processor sees the abort.
// Match the cancellation pattern used by `_processNextItem`.
if (signal.aborted) {
this.state.cancelAllQueued('Operation cancelled');
this.isProcessing = false;
return this.state.completedBatch;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Returning early when signal.aborted is true bypasses the finally block of _startBatch. This prevents this._processNextInRequestQueue() from being called, which will cause any subsequent or concurrent tool call batches queued in this.requestQueue to hang indefinitely and never resolve or reject. To prevent this promise/queue leak, we must ensure this._processNextInRequestQueue() is called before returning early.

Suggested change
if (signal.aborted) {
this.state.cancelAllQueued('Operation cancelled');
this.isProcessing = false;
return this.state.completedBatch;
}
if (signal.aborted) {
this.state.cancelAllQueued('Operation cancelled');
this.isProcessing = false;
this._processNextInRequestQueue();
return this.state.completedBatch;
}
References
  1. Asynchronous operations that can be cancelled by the user should accept and propagate an AbortSignal to ensure cancellability and prevent dangling processes or network requests.


const currentApprovalMode = this.config.getApprovalMode();

// Sort requests to ensure Topic changes happen before actions in the same batch.
Expand Down