fix(coding-agent): preserve late advisor notes after terminal answers#5186
Conversation
Prevented late interrupting advisor findings from waking the primary after a terminal text answer when no queued work remains. Added regression coverage for the advisor-confirmation path so duplicate primary turns are caught. Fixes #4840
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 708eafaf8d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| #hasTerminalTextAnswerWithoutQueuedWork(): boolean { | ||
| if (this.agent.hasQueuedMessages() || this.#pendingNextTurnMessages.length > 0) return false; | ||
| return isTerminalTextAssistantAnswer(this.agent.state.messages.at(-1)); |
There was a problem hiding this comment.
When two late interrupting notes arrive after one completed primary answer (for example, multiple configured advisors each report a concern), the first preserve path appends an advisor custom message to agent.state.messages. This check then looks only at the new tail, no longer sees the terminal assistant answer, and routes the next note through steer with triggerTurn: true, recreating the duplicate primary turn this change is intended to avoid. Consider looking back past preserved advisor cards or caching the terminal-answer state until a real primary/user turn changes it.
Useful? React with 👍 / 👎.
| if (part.type === "thinking") continue; | ||
| return false; |
There was a problem hiding this comment.
Terminal Anthropic answers can include non-actionable metadata blocks such as redactedThinking (extended thinking) or fallback alongside final text; the provider code persists those content types. This predicate currently ignores only plain thinking, so such an otherwise-terminal text answer returns false and a late concern/blocker still wakes a duplicate primary turn. These metadata-only blocks should be handled like thinking rather than making the answer non-terminal.
Useful? React with 👍 / 👎.
2 participants
Repro
A focused regression in
packages/coding-agent/test/agent-session-advisor-suppression.test.tsdrives a primary turn to a terminal one-line answer, then runs an advisoradvise({ severity: "concern" })after the primary is idle. Before the fix,bun run gen:tool-views && bun test packages/coding-agent/test/agent-session-advisor-suppression.test.tsfailed because the late advisor finding scheduled another primary/advisor cycle instead of remaining a card.Cause
AgentSession#routeAdviceinpackages/coding-agent/src/session/agent-session.tsrouted idle non-suppressed advisorconcern/blockernotes throughsendCustomMessage(..., { deliverAs: "steer", triggerTurn: true }).resolveAdvisorDeliveryChannelinpackages/coding-agent/src/advisor/advise-tool.tshad no terminal-tail predicate, so a late advisor finding after an assistantstopanswer looked like actionable idle work and woke the primary even when no queued messages existed.Fix
AgentSessionand passed it into advisor delivery routing.resolveAdvisorDeliveryChannelto preserve late interrupting advisor notes as visible advisor cards when the primary already ended cleanly and no queued work remains.packages/coding-agent.Verification
bun test packages/coding-agent/src/advisor/__tests__/advisor.test.ts packages/coding-agent/test/agent-session-advisor-suppression.test.ts→ 79 pass, 0 fail.bun check→ passed. Pre-publish gate ingh_push_branchpassed and pushed708eafaf8d31. Fixes #4840.