fix(core): enforce terminal user turn invariant and normalize request contents - #29612

Code Review

This pull request ensures that the final request contents sent to the Gemini API satisfy the API invariant of ending with a valid user turn. It normalizes contents by appending a user turn (such as "Please continue." or missing function responses) if the history ends with a model turn or an empty user turn. Additionally, it updates stripToolCallIdPrefixes to preserve functionResponse parts and filter out empty turns, filters out tool responses from the rewind viewer interactions, and adds comprehensive test coverage. The reviewer suggested using 'generic_tool' as a trimmed fallback for missing tool names in the generated function responses to maintain consistency.

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

Code Review

This pull request introduces normalization logic in GeminiChat to ensure that the final contents dispatched to the Gemini API always end with a valid user turn, satisfying the API invariant. It handles cases where the history ends with a model turn by appending a user turn with either synthesized tool responses or a 'Please continue.' prompt. Additionally, it updates stripToolCallIdPrefixes to preserve functionResponse parts and filter out empty turns, refines the interactions filtering in RewindViewer to exclude tool responses, and adds comprehensive unit tests to verify these behaviors. I have no feedback to provide as there are no review comments.

Thanks for working on this! I left several inline comments regarding edge cases around multi-turn history state, stripToolCallIdPrefixes role alternation, scrubPart in historyHardening.ts, and RewindViewer turn stats.

Code Review

This pull request introduces robust history hardening and terminal user turn invariant enforcement to ensure API request contents sent to Gemini always end with a valid user turn. It handles closing unanswered tool calls and responses, filters out synthetic tool responses from rewind points in the CLI, and preserves nested parts within function responses during history scrubbing. The reviewer feedback highlights three high-severity issues: two instances where in-memory history mutations are not persisted to the chat recording service (causing memory-disk desynchronization), and one instance of duplicated logic in RewindViewer.tsx that should reuse the newly introduced isToolResponseMessage helper.

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

Code Review

This pull request hardens history management and API request normalization to ensure compliance with Gemini API invariants, specifically enforcing that the final payload ends with a valid user turn. It introduces mechanisms to close unanswered tool calls and tool responses before recording new user messages, filters out synthetic tool response messages from rewind points and turn statistics calculations, and preserves nested parts within function responses during history scrubbing and prefix stripping. Comprehensive unit tests have been added across the CLI and core packages to validate these history integrity and rollback behaviors. I have no additional feedback to provide as no review comments were submitted.

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

Code Review Summary

This PR targets HTTP 400 Bad Request (INVALID_ARGUMENT: "Requests ending with a model turn are not supported") errors after /rewind, stream aborts, or tool cancellation rollbacks. While preserving functionResponse.parts in scrubPart / stripToolCallIdPrefixes and checking lastTurn.content.role !== 'user' during context-management deduplication are good fixes, there are several blocking correctness bugs, state synchronization issues, and test gaps that should be addressed before merging:

1. Stateful mutation inside the retryable apiCall closure ([PITFALL-007], [OOP-CVA-002], [PR-HYGIENE-002]): In packages/core/src/core/geminiChat.ts, the egress normalization block mutates this.agentHistory and this.chatRecordingService inside apiCall (which is invoked repeatedly by retryWithBackoff and streamWithRetries). This appends duplicate synthetic turns to persistent history and disk on every retry attempt, pollutes this.agentHistory when apiHistoryOverride or BeforeModel modifiedContents is used, pushes prefix-stripped tool call IDs into this.agentHistory (breaking ID pairing with unstripped model functionCall turns), and leaves lastContentsToUse stale for AfterModel hooks.
2. Overly broad isToolResponseMessage predicate (Correctness): In packages/cli/src/ui/utils/rewindFileOps.ts, using parts.some(...) instead of parts.every(...) classifies any user message containing both a synthesized functionResponse (via closeUnansweredToolCallsTurn or steering hints) and authored user text as a synthetic tool response—hiding real user prompts from /rewind (RewindViewer.tsx) and misattributing per-turn file edit stats (calculateTurnStats).
3. Production vs. test mismatch in useGeminiStream.ts rollback anchor ([PITFALL-003], Correctness): historyLengthAfterUserPromptRef.current = geminiClient.getHistory().length runs before geminiClient.sendMessageStream adds the user prompt to geminiClient. In production (unlike the unit test, which pre-seeds client.setHistory before calling submitQuery), cancelling a continuation tool call rolls geminiClient back to the previous turn's trailing model response and erases already-completed tool rounds from the current turn.
4. Missing unit test coverage ([TEST-002]): packages/cli/src/ui/utils/rewindFileOps.test.ts was not updated with unit tests for isToolResponseMessage or multi-step tool turns in calculateTurnStats, and the new hadEmptyTrailingModelTurn branch in closeUnansweredToolResponseTurn has no test coverage.

Good PR Checkmarks Scorecard

Code Review

This pull request introduces robust history repair and normalization mechanisms to maintain the Gemini API's invariant that request contents must end with a valid user turn. It filters out synthetic tool responses from rewind points, rolls back unanswered trailing model function calls for cancelled batches, and ensures proper pairing and alternation of turns. Feedback on the changes suggests adding a defensive initialization check in GeminiClient.discardTrailingUnansweredToolCallTurn to prevent potential unhandled exceptions if the chat session is not yet initialized.

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

Code Review

This pull request introduces robust history repair and normalization mechanisms to ensure that outbound payloads to the Gemini API always end with a valid user turn carrying content, maintaining API invariants. It adds a utility function isToolResponseMessage to identify synthetic tool responses, allowing the CLI to exclude them from rewind points and turn statistics. Additionally, it replaces the previous history rollback logic with a precise discardTrailingUnansweredToolCallTurn method, which discards only unanswered model function calls while preserving completed tool rounds and the originating user prompt. Extensive unit tests have been added across the CLI and core packages to validate these history hardening, turn normalization, and rewind behaviors. There are no review comments to address.

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