fix(agents): preserve completed post-tool replies#100655
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 3:07 AM ET / 07:07 UTC. Summary PR surface: Source -11, Tests +122, Docs +1. Total +112 across 5 files. Reproducibility: yes. for the review blockers at source level: current main derives both attempt assistant selectors from the same Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Rework the branch around a production-valid attempt/session capture or producer-level regression, preserve the pre-tool Do we have a high-confidence way to reproduce the issue? Yes for the review blockers at source level: current main derives both attempt assistant selectors from the same Is this the best way to solve the issue? No. The current selector cleanup is only a plausible defensive change; the best fix needs a production-valid attempt/session proof and must preserve the pre-tool Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e66f43e27616. Label changesLabel justifications:
Evidence reviewedPR surface: Source -11, Tests +122, Docs +1. Total +112 across 5 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
4a8a648 to
58c6f0e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aafa998f4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if ( | ||
| (params.payloadCount !== 0 && !toolUseTerminal && !lengthTerminal && !thinkingOnlyTerminal) || | ||
| (params.payloadCount !== 0 && !incompleteTerminalAssistant && !thinkingOnlyTerminal) || |
There was a problem hiding this comment.
Keep pre-tool text from masking empty post-tool stops
When the session assistant is the stale pre-tool toolUse turn but currentAttemptAssistant is a blank/empty stop, buildEmbeddedRunPayloads can still produce a payload from earlier assistantTexts (the pre-tool commentary). This payloadCount !== 0 guard then returns null, so a turn that never produced a post-tool answer is treated as a successful final reply instead of retrying or surfacing the incomplete-turn error; that regresses the #76477 guard for the stale-session shape this change introduces. Please keep the tool-use guard unless the current assistant has visible terminal text, or base this check on terminal-assistant output rather than any attempt payload.
Useful? React with 👍 / 👎.
8f8edee to
a017680
Compare
a017680 to
9fb6ac2
Compare
|
Merged via squash.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fb6ac2633
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Android network recovery:** reconnect Gateway sessions immediately when Android regains a validated network instead of waiting for the current reconnect backoff. (#100347) Thanks @ly85206559. | ||
| - **Android Voice layout:** keep Voice settings controls within their intended width so nested cards do not overflow or clip on constrained screens. (#100491) Thanks @IWhatsskill. | ||
| - **Android camera logging:** remove release-path camera clip diagnostics that exposed temporary file details and added noisy invoke logs. (#99484) Thanks @NianJiuZst. | ||
| - **Agent final replies:** prefer the current attempt's terminal assistant across incomplete-turn classification and success metadata so a stale pre-tool snapshot cannot replace a completed post-tool answer with an error. (#94637) Thanks @LiuwqGit. |
There was a problem hiding this comment.
Remove the normal-PR changelog entry
Root AGENTS.md says CHANGELOG.md is release-only and normal PR/direct fixes should carry release-note context in the PR body, squash message, or commit instead; this maintainer-takeover fix adds an Unreleased entry for #94637 directly. Please move this note out of CHANGELOG.md so release generation remains the single owner of changelog edits.
Useful? React with 👍 / 👎.
* fix(agents): prefer current attempt terminal state Co-authored-by: weiqinl <[email protected]> * test(agents): model current attempt terminal state --------- Co-authored-by: weiqinl <[email protected]>
* fix(agents): prefer current attempt terminal state Co-authored-by: weiqinl <[email protected]> * test(agents): model current attempt terminal state --------- Co-authored-by: weiqinl <[email protected]>
Related: #94637
Closes #80918
What Problem This Solves
Fixes an issue where an agent turn that completed after tool execution could return a stale pre-tool assistant snapshot, replacing the completed post-tool answer with an incomplete-turn error.
Why This Change Was Made
The runner now chooses one canonical terminal assistant for the current attempt, falling back to the model-matched session assistant only when the attempt has none. Incomplete-turn classification, failover, result metadata, payloads, and stop reasons all consume that same value so they cannot disagree.
This is a maintainer takeover of #94637 because the contributor branch does not permit maintainer edits. Contributor credit is preserved with
Co-authored-byand the changelog thanks @LiuwqGit.User Impact
Completed answers after tool calls are delivered as successful final replies instead of being replaced by stale incomplete-tool-use errors.
Evidence
mainafter the prior fork head's unrelated stale-base Control UI build failure.