Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
33 changes: 33 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,39 @@ 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('should not leak queued batches when scheduled with an already-aborted signal (regression #28091)', async () => {
// The aborted-signal short-circuit must still drain the request
// queue via the `finally` block — otherwise a follow-up batch
// queued behind it would never resolve and hang the caller.
abortController.abort();

// First batch is rejected by the abort guard.
await scheduler.schedule(req1, signal);

// A second batch scheduled with a fresh, non-aborted signal must
// still make progress (i.e. the scheduler must not be stuck in the
// "busy" state after the early-return path).
const fresh = new AbortController();
await expect(
scheduler.schedule(req2, fresh.signal),
).resolves.toBeDefined();
});

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

// Sort requests to ensure Topic changes happen before actions in the same batch.
const sortedRequests = [...requests].sort((a, b) => {
if (a.name === UPDATE_TOPIC_TOOL_NAME) return -1;
if (b.name === UPDATE_TOPIC_TOOL_NAME) return 1;
return 0;
});

try {
// 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`.
// Inside the `try` so the `finally` still drains the request queue —
// otherwise concurrent batches waiting in `requestQueue` would hang.
if (signal.aborted) {
this.state.cancelAllQueued('Operation cancelled');
return this.state.completedBatch;
}

const currentApprovalMode = this.config.getApprovalMode();

// Sort requests to ensure Topic changes happen before actions in the same batch.
const sortedRequests = [...requests].sort((a, b) => {
if (a.name === UPDATE_TOPIC_TOOL_NAME) return -1;
if (b.name === UPDATE_TOPIC_TOOL_NAME) return 1;
return 0;
});

const toolRegistry = this.context.toolRegistry;
const newCalls: ToolCall[] = sortedRequests.map((request) => {
const enrichedRequest: ToolCallRequestInfo = {
Expand Down