fix(cli): prevent race condition in loop detection retry

[skyvanguard]

Summary

Fixes #17071

Prevents a race condition that can occur when the user clicks "disable" in the loop detection dialog. The retry submitQuery could race with user prompts due to React state batching - checking streamingState isn't synchronous enough.

The Problem

When loop detection triggers and shows the confirmation dialog:

1. setIsResponding(false) is called in the finally block
2. streamingState becomes Idle (after React batch)
3. InputPrompt becomes active - user can type new prompts
4. User clicks "Disable" → submitQuery() is called without await
5. Race condition: Two submitQuery() calls can run in parallel

The Solution

1. Add isRespondingRef for synchronous state checking
2. Update ref alongside state to keep them in sync
3. Check ref in submitQuery before allowing new queries
4. Await the retry by making the callback async

Changes

// 1. Added synchronous ref
const isRespondingRef = useRef<boolean>(false);

// 2. Check ref synchronously in submitQuery (prevents race)
if (
  (isRespondingRef.current ||  // <-- NEW: synchronous check
    streamingState === StreamingState.Responding ||
    streamingState === StreamingState.WaitingForConfirmation) &&
  !options?.isContinuation
)
  return;

// 3. Update ref alongside state
isRespondingRef.current = true;
setIsResponding(true);

// 4. Await the retry (was fire-and-forget before)
onComplete: async (result) => {
  // ...
  await submitQuery(lastQueryRef.current, ...);
}

Test plan

• All 64 tests in useGeminiStream.test.tsx pass
• All 74 tests in AppContainer.test.tsx pass
• TypeScript compilation: 0 errors
• ESLint: 0 warnings

[gemini-code-assist]

Summary of Changes

Hello @skyvanguard, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical race condition within the CLI's loop detection retry mechanism. By introducing a synchronous reference for the responding state and ensuring that retry queries are properly awaited, it prevents multiple query submissions from occurring simultaneously, which could lead to unexpected behavior when a user interacts with the loop detection dialog.

Highlights

• Synchronous State Management: Introduced isRespondingRef (a useRef hook) to provide a synchronous way to check the component's responding state, bypassing React's asynchronous state updates.
• Race Condition Prevention: Modified the submitQuery function to check isRespondingRef.current before allowing new queries, effectively preventing a race condition where multiple submitQuery calls could run concurrently.
• Consistent State Updates: Ensured isRespondingRef is updated immediately alongside the isResponding state, maintaining synchronization between the synchronous ref and the asynchronous state.
• Awaited Retry Logic: Changed the onComplete callback for loop detection to be async and now awaits the submitQuery call, preventing a fire-and-forget scenario and ensuring proper sequencing of operations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to fix a race condition in the loop detection retry logic by introducing a synchronous useRef to track the responding state. However, the current implementation is incomplete and introduces two critical deadlock vulnerabilities. The first, in submitQuery, arises from activeQueryIdRef.current being updated before an early return, which can prevent the 'responding' state from being cleared. The second issue is that isRespondingRef and isResponding state can get out of sync, leading to a permanently blocked state. To resolve these, ensure activeQueryIdRef is updated only for queries that proceed to execution, and that isRespondingRef and isResponding state are always updated together using a consistent helper function.

[gemini-code-assist]

While isRespondingRef is a good approach to solve the race condition, its state can get out of sync with the isResponding state because setIsResponding is called in multiple places throughout this hook without also updating isRespondingRef. This can lead to a permanently blocked state where new queries are always rejected.

For example, in cancelOngoingRequest:

if (isFullCancellation) {
  if (!activeShellPtyId) {
    // ...
    setIsResponding(false); // isRespondingRef is not updated
  }
}

This leaves isRespondingRef.current as true while isResponding becomes false, blocking all future queries.

To fix this critical issue, I recommend creating a single function to update both the state and the ref, and using it consistently.

1. Define a helper function to keep state and ref in sync (you can add this right after the isRespondingRef declaration):

   const setResponding = useCallback((responding: boolean) => {
     isRespondingRef.current = responding;
     setIsResponding(responding);
   }, []);

2. Replace all calls to setIsResponding(value) with setResponding(value). This includes the ones you've already updated in this PR inside submitQuery, as well as the ones in:

   • onExec
   • useEffect for activeShellPtyId
   • cancelOngoingRequest
   • handleUserCancelledEvent
   • handleAgentExecutionStoppedEvent
   • handleCompletedTools

This will ensure the state remains consistent and prevent the deadlock.

[skyvanguard]

Thanks for the detailed review! I've addressed the feedback:

• Created a setIsResponding helper function using useCallback that updates both the ref and state synchronously
• Updated all calls to setIsResponding throughout the hook to use this helper
• This ensures isRespondingRef and isResponding state are always kept in sync, preventing the deadlock scenario

[cynthialong0-0]

Consider using useStateAndRef instead.

[cynthialong0-0]

Overall looks good. Can you resolve the conflicts and add unit test for your change?

[ishaanxgupta]

Hi @skyvanguard the CLA test is failing because the commit was co-authored by Claude, also I think there are conflicts still present

[cynthialong0-0]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses a race condition in the loop detection retry mechanism by introducing a synchronous ref (isRespondingRef) to track the responding state, preventing concurrent submitQuery calls. The change to await the retry query is also a crucial fix. The added tests are comprehensive and validate the new logic. I have one suggestion to improve code consistency by using the existing useStateAndRef hook, which also aligns with the principle of using explicit state variables for asynchronous operations.

_{Note: Security Review did not run due to the size of the PR.}

[gemini-code-assist]

To improve consistency with existing patterns in the codebase, you can use the useStateAndRef hook here. This hook is already used in this file for thought, pendingHistoryItem, etc., and it encapsulates the logic of keeping a state variable and a ref in sync. This will also make the code more concise.

Suggested change

	const [isResponding, setIsRespondingState] = useState<boolean>(false);
	const isRespondingRef = useRef<boolean>(false);
	const setIsResponding = useCallback(
	(value: boolean) => {
	setIsRespondingState(value);
	isRespondingRef.current = value;
	},
	[setIsRespondingState],
	);
	const [isResponding, isRespondingRef, setIsResponding] =
	useStateAndRef<boolean>(false);

References

1. Line 72 of the repository style guide states: 'Coding Style: Adhere to existing patterns in packages/cli (React/Ink) and packages/core (Backend logic).' The useStateAndRef hook is an existing pattern in this file for managing state and a corresponding ref simultaneously. ^(link)
2. When managing the state of asynchronous operations, rely on an explicit state variable (e.g., a state enum) rather than checking for the existence of a promise object. Promise objects may be cleared in finally blocks upon completion, making them unreliable for state checks after the operation has finished. Using useStateAndRef for isResponding aligns with this by providing an explicit and reliable state variable.

[cynthialong0-0]

@skyvanguard Can you resolve the comment here?

[skyvanguard]

@cynthialong0-0 Thanks for the review! Regarding the suggestion to use useStateAndRef:

I investigated this and it would be the ideal approach for consistency. However, the existing useStateAndRef mock in the test file (lines 163-177) captures the state value by primitive in the return array [val, ref, setVal] — when setVal(true) is called, ref.current updates correctly but the returned state value remains stale. This means streamingState (derived via useMemo([isResponding, ...])) never transitions to Responding, breaking 6 existing tests (User Cancellation, Retry Handling, Race Condition Prevention).

Fixing the mock to use real React hooks causes heap overflow in the test environment. So the explicit useState + useRef + useCallback wrapper is functionally equivalent to useStateAndRef but works correctly with the existing test infrastructure.

Happy to refactor to useStateAndRef if the mock gets fixed in a separate PR — I can take that on as well.

Also: conflicts resolved (rebased on main) and Co-Authored-By removed for CLA compliance.