Repository navigation
fix(agents): never answer a new prompt with the previous turn's answer - #189
Conversation
A subagent's stored answer was cleared only when a new turn reported its start. Between a submitted prompt and that start (a slow or lost start hook), get_agent_result, and wait_for_agent once it ended "quiet", returned the previous turn's answer as if it answered the new prompt. Submitting a prompt now clears the stored answer, where the prompt's turn mark is set. The runtime gateway dropped every non-start event with a new turn id once a turn id was active. Each hook runs on its own connection and can time out, so when a turn's start hook was lost the whole turn, its Stop and final answer included, was ignored and the card kept the previous turn's status. The gateway now tracks whether a turn is open: after an idle, an unseen turn id opens the next turn; during an open turn, or for an id already seen, the event is still dropped as out of order.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: ed44aae1a5
ℹ️ 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 (lease.activeTurnId && (lease.turnOpen || lease.seenTurnIds.has(message.turnId))) return null; | ||
| lease.activeTurnId = message.turnId; |
There was a problem hiding this comment.
When an older turn's start hook was lost, rejecting one of its events here does not remember its ID. The sequence UserPromptSubmit(B) → PostToolUse(A) → Stop(B) → Stop(A) therefore drops A's tool event while B runs, but accepts A's late Stop as a new turn once B is idle. If both Stops carry results, the recordAnswer call in src/main/index.ts replaces B's correct answer with A's stale one, so get_agent_result returns the previous turn's answer again. Preserve the conflicting-ID rejection across the idle transition rather than treating that ID as unseen.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in a777f7c. An event of another turn that arrives while a turn is open is now remembered as seen before it is dropped, so that turn's late Stop after the running turn goes idle stays stale instead of opening a new turn. Test: "RuntimeGateway: an earlier turn rejected while the next one ran cannot replace it after that turn ends" in tests/agent-runtime-gateway.test.mjs (UserPromptSubmit(B), PostToolUse(A), Stop(B), Stop(A); only B's events are delivered). It failed before the fix with A's Stop delivered.
| if (prompt.submit) markSubmittedPrompt(session, session.turnStarts ?? 0); | ||
| if (!this.inputChecked(id, data + (prompt.submit ? submitKey! : ""), "automation")) { | ||
| prompt.receipt.stage = "not-written"; |
There was a problem hiding this comment.
If an OpenCode submit's PTY write fails, this call has already deleted the saved answer even though the following failure path reports a not-written receipt. Consequently, a failed send_to_agent erases the last completed answer from get_agent_result, despite no new prompt being delivered. The staged path likewise clears the answer before beforeSubmit can veto submission or the Enter write can fail. Clear the answer only after a successful submit write, retaining the pre-write turn mark, or restore it on these failure paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in a777f7c. The prompt's turn mark is still set before the write, but the stored answer is now cleared only after the submit was written. A failed write (not-written receipt), a failed Enter write, or a beforeSubmit veto in the staged path keeps the last answer. Tests: "a prompt that was not written keeps the previous turn's answer" in tests/opencode-result.test.mjs, and "a staged prompt clears the previous answer only once its Enter is written" in tests/prompt-delivery.test.mjs (veto, Enter write failure, success). Both failed before the fix.
…swer (review) An event of another turn that arrived while a turn ran was dropped but not remembered, so that turn's late Stop, arriving after the running turn went idle, opened a new turn and its result replaced the correct answer. Such a turn id is now remembered as seen and stays stale. The stored answer was cleared before the prompt's submit was written. A failed write, a failed Enter write, or a controller veto before Enter delivered nothing yet erased the last answer. The answer is now cleared only after the submit is written; the prompt's turn mark is still set before the write.
Split out of #160 as a small, independent PR on 1.8.5 (it touches only what 1.8.5 does not already cover). It does not depend on the other parts.
1.8.5 already makes
wait_for_agentreturnidleonly after a turn that started after the latest prompt (turnProgress,promptTurnMark), and clears the stored answer when a turn starts. Two gaps remain around that.Problem 1: the previous answer answers the new prompt
The stored answer (Codex Stop hook, OpenCode plugin) is cleared only when the next turn reports its start. Between
send_to_agentand that start, the old answer is still returned:Before:
get_agent_resultreturnsanswer: "part A done".get_agent_resultreturnsanswer: "part A done", andwait_for_agent, which endsquietwhen no turn start follows, returns the same answer. The orchestrator takes part A's answer as part B's result.After: submitting a prompt clears the stored answer. Until part B's turn reports its answer, both calls return no
answer(the screen output is still there), then they return part B's answer.Problem 2: a turn whose start hook was lost is ignored completely
RuntimeGatewaydropped every non-start event with a new turn id once a turn id was active. Each hook runs on its own connection with its own timeout, so if a turn'sUserPromptSubmitis lost, all of that turn's later events (PostToolUse, Stop, and the final answer the Stop carries) were dropped as "out of order". The card kept the previous turn's status, and the answer was never recorded.After: the gateway tracks whether a turn is open. After an idle, an unseen turn id opens the next turn. During an open turn, or for an id already seen (a late event of an earlier turn), the event is still dropped, exactly as before.
Change
TerminalManager: the three places that setpromptTurnMarkfor a submitted prompt still set it before the write; the stored answer is deleted (clearSubmittedPromptAnswer) only after the submit was written. A failed write, a failed Enter write or a controller veto before Enter keeps the last answer, since nothing new was submitted.RuntimeGateway: aturnOpenflag on the lease; non-start events with an unseen id are accepted only between turns. An id rejected because another turn was running is remembered as seen, so its late Stop after that turn ends stays stale.Two files in
src/, about 40 changed lines.Why this and not less
Clearing the answer where the prompt's turn mark is already set reuses the maintainer's own notion of "the prompt's turn" and touches no reader:
answer(),waitForandresultstay as they are. Filtering inanswer()by turn count instead would hide a correct answer when the CLI handles a queued prompt inside the same busy period (OpenCode does that), so the simpler "clear on submit" is also the safer one. The gateway fix keeps the existing stale-event protection (seen ids, late starts of older turns) and only stops treating "start never arrived" as "stale".What changes for existing workflows
Validation
tests/opencode-result.test.mjs: after part A's answer,send_to_agentpart B;get_agent_resultmust have no answer andwait_for_agentmust endquietwithout one; part B's answer is then returned. On f3eee29 it fails (actual: { text: 'part A done' }, expected: undefined).tests/agent-runtime-gateway.test.mjs: with turn 2'sUserPromptSubmitmissing, its PostToolUse and Stop (and a Stop-only turn 3) must be delivered, while an event of another turn during turn 1 and late events of turn 1 stay dropped. On f3eee29 it fails (actual: 'idle', expected: 'working').tests/agent-runtime-gateway.test.mjs:UserPromptSubmit(B),PostToolUse(A),Stop(B),Stop(A)must deliver only B's events. Without remembering the rejected id, A's late Stop was delivered as a new turn.tests/opencode-result.test.mjsandtests/prompt-delivery.test.mjs: a prompt whose write fails (OpenCode), whose Enter write fails or whose submit is vetoed (Codex staged path) keeps the previous answer; a submitted one clears it. Before the change all three variants lost the answer.npm run build(typecheck),npm run audit:secrets, the eager renderer bundle check and the full test suite pass. Full suite: 2656 tests, 2650 pass, 0 fail, 6 skipped. The renderer is untouched (eager bundle unchanged against the other branches;renderer-bundle-splittingpasses).