Skip to content

Commit d80eeb5

Browse files
Automakerclaude
andcommitted
fix(cli): chat-stream cancel race left spinner stuck
Two leaks in processGeminiStreamEvents that #145 / #152 didn't cover: 1. case ServerGeminiEventType.UserCancelled fell through with a plain `break`, which only exits the switch — the for-await kept iterating and any toolCallRequests already collected this iteration would still get scheduled at the post-loop scheduleToolCalls call. 2. The post-loop scheduleToolCalls fired unconditionally. If abort landed in the same tick as a chunk that carried finish_reason=tool_calls (model emitted tool calls while user was hitting Esc), the scheduler added the tools in 'validating' state with an already-aborted signal. The per-tool aborted check at coreToolScheduler.ts:861 eventually marks them 'cancelled', but the React state briefly flips streamingState back to Responding — sticking the spinner until the 3s forceCancelStaleToolCalls grace window catches up. Fix: UserCancelled returns early; scheduleToolCalls is gated on !signal.aborted. Two new tests in the Cancellation describe block cover both arms (UserCancelled-then-toolCallRequest, and toolCallRequest-arriving-after-abort). 51/51 useGeminiStream tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 85b038e commit d80eeb5

2 files changed

Lines changed: 110 additions & 2 deletions

File tree

packages/cli/src/ui/hooks/useGeminiStream.test.tsx

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,99 @@ describe('useGeminiStream', () => {
10571057
// Nothing should happen because the state is not `Responding`
10581058
expect(abortSpy).not.toHaveBeenCalled();
10591059
});
1060+
1061+
it('should not schedule tool calls collected before a UserCancelled event', async () => {
1062+
// Repro for chat-stream cancel race: model emits a ToolCallRequest in
1063+
// one chunk, then UserCancelled in the next (e.g. user pressed Esc
1064+
// mid-stream and turn.ts converted the abort into a UserCancelled
1065+
// event). The for-await previously ran past the cancel and scheduled
1066+
// the queued tool call after the fact, leaving streamingState stuck.
1067+
mockSendMessageStream.mockReturnValue(
1068+
(async function* () {
1069+
yield {
1070+
type: ServerGeminiEventType.ToolCallRequest,
1071+
value: {
1072+
callId: 'call-after-cancel',
1073+
name: 'shell',
1074+
args: { command: 'echo hi' },
1075+
},
1076+
};
1077+
yield { type: ServerGeminiEventType.UserCancelled };
1078+
})(),
1079+
);
1080+
1081+
const { result } = renderTestHook();
1082+
1083+
await act(async () => {
1084+
await result.current.submitQuery('test query');
1085+
});
1086+
1087+
await waitFor(() => {
1088+
expect(mockAddItem).toHaveBeenCalledWith(
1089+
expect.objectContaining({
1090+
type: 'info',
1091+
text: 'User cancelled the request.',
1092+
}),
1093+
expect.any(Number),
1094+
);
1095+
});
1096+
1097+
expect(mockScheduleToolCalls).not.toHaveBeenCalled();
1098+
expect(result.current.streamingState).toBe(StreamingState.Idle);
1099+
});
1100+
1101+
it('should not schedule tool calls when the abort signal is set before the stream ends', async () => {
1102+
// Second arm of the same race: abort fires while the stream is
1103+
// still iterating, but the underlying SDK delivers the buffered
1104+
// ToolCallRequest chunk before honoring the abort. Stream then
1105+
// ends naturally (no UserCancelled event). Without the
1106+
// !signal.aborted guard at scheduleToolCalls, the post-loop
1107+
// schedule fires with an aborted signal and the React state
1108+
// briefly flips toolCalls into 'validating' — sticking the spinner.
1109+
let yieldToolCall: () => void;
1110+
const toolCallGate = new Promise<void>((r) => {
1111+
yieldToolCall = r;
1112+
});
1113+
1114+
mockSendMessageStream.mockReturnValue(
1115+
(async function* () {
1116+
yield { type: ServerGeminiEventType.Content, value: 'partial' };
1117+
await toolCallGate;
1118+
yield {
1119+
type: ServerGeminiEventType.ToolCallRequest,
1120+
value: {
1121+
callId: 'call-mid-cancel',
1122+
name: 'shell',
1123+
args: { command: 'ls' },
1124+
},
1125+
};
1126+
})(),
1127+
);
1128+
1129+
const { result } = renderTestHook();
1130+
1131+
await act(async () => {
1132+
result.current.submitQuery('test query');
1133+
});
1134+
1135+
await waitFor(() => {
1136+
expect(mockSendMessageStream).toHaveBeenCalledTimes(1);
1137+
});
1138+
1139+
// User presses Esc before the buffered tool-call chunk gets delivered.
1140+
act(() => {
1141+
result.current.cancelOngoingRequest();
1142+
});
1143+
1144+
// SDK now delivers the chunk that was already in flight, then ends.
1145+
await act(async () => {
1146+
yieldToolCall!();
1147+
await new Promise((r) => setTimeout(r, 10));
1148+
});
1149+
1150+
expect(mockScheduleToolCalls).not.toHaveBeenCalled();
1151+
expect(result.current.streamingState).toBe(StreamingState.Idle);
1152+
});
10601153
});
10611154

10621155
describe('Slash Command Handling', () => {

packages/cli/src/ui/hooks/useGeminiStream.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,13 @@ export const useGeminiStream = (
12201220
case ServerGeminiEventType.UserCancelled:
12211221
flushBufferedStreamEvents();
12221222
handleUserCancelledEvent(userMessageTimestamp);
1223-
break;
1223+
// Stop processing immediately. A plain `break` only exits the
1224+
// switch — the for-await would continue to the next chunk and
1225+
// any toolCallRequests already accumulated this iteration would
1226+
// still get scheduled at line 1302 below. That left the spinner
1227+
// stuck during chat-stream cancels (chat-only freeze, distinct
1228+
// from the tool-execution cancel covered by #145 / #152).
1229+
return StreamProcessingStatus.UserCancelled;
12241230
case ServerGeminiEventType.Error:
12251231
flushBufferedStreamEvents();
12261232
handleErrorEvent(event.value, userMessageTimestamp);
@@ -1299,7 +1305,16 @@ export const useGeminiStream = (
12991305
discardBufferedStreamEvents();
13001306
flushBufferedStreamEventsRef.current.delete(flushBufferedStreamEvents);
13011307
}
1302-
if (toolCallRequests.length > 0) {
1308+
// Skip scheduling if the user already cancelled. The for-await may
1309+
// have collected toolCallRequests from a chunk that arrived in the
1310+
// same tick as the abort (e.g. model emits finish_reason=tool_calls
1311+
// right as the user presses Esc). Scheduling them post-cancel adds
1312+
// 'validating' tools to the React state — which flips streamingState
1313+
// back to Responding before the per-tool aborted check at
1314+
// coreToolScheduler.ts:861 can mark them 'cancelled'. The 3s
1315+
// forceCancelStaleToolCalls rescue eventually clears it, but the
1316+
// user sees a stuck spinner until then.
1317+
if (toolCallRequests.length > 0 && !signal.aborted) {
13031318
scheduleToolCalls(toolCallRequests, signal);
13041319
}
13051320
return StreamProcessingStatus.Completed;

0 commit comments

Comments
 (0)