fix(agent): pair the Codex WS delta boundary with a fresh response id (#194) - #213
Open
justrach wants to merge 1 commit into
Open
fix(agent): pair the Codex WS delta boundary with a fresh response id (#194)#213justrach wants to merge 1 commit into
justrach wants to merge 1 commit into
Conversation
…#194) stepResponses advanced codex_sent_upto (the delta boundary) whenever the WS was live but only replaced codex_prev_id when the response carried a new response.id. parseResponses accepts output items without a terminal id, so a response can lack one — leaving the OLD id paired with an advanced boundary. buildBody then sends previous_response_id + messages[codex_sent_upto..], slicing off history the referenced response never held. Move the two in lockstep: install the new id and advance the boundary together; with no usable id (or a failed dup) reset both to null/0 so the next request re-anchors with the full input (buildBody's prev_id==null path) instead of replaying a stale delta against a response that never saw it. Test: stepResponses drives a codex-WS agent with (a) an id-less response — asserts prev_id is dropped and the boundary resets to 0 (full re-anchor); (b) an id-bearing response — asserts the id installs and the boundary advances in lockstep. Leak-checked. Refs #194 Co-Authored-By: blackfloofie <265516171+blackfloofie@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
stepResponses()advances the Codex WS delta boundary (codex_sent_upto) whenever a WS session is live, but only replacescodex_prev_idwhen the parsed response contains a newresponse.id.parseResponses()accepts output items without a terminal response id, so a response can lack one — leaving the old id paired with an advanced boundary.buildBody()then sendsprevious_response_id: <old id>+messages[codex_sent_upto..], slicing off history the referenced response never held.Fix
Move the id and the boundary in lockstep (
agent_steps.zig):codex_sent_uptotogether.null/0so the next request re-anchors with the full input (buildBody'sprev_id == nullpath), instead of replaying a stale delta.No WS teardown is needed — resetting the watermarks is enough; the next request sends the full input over the live session, exactly like the first request of any WS session.
Test
A unit test drives a codex-WS agent through
stepResponses():codex_prev_idis dropped and the boundary resets to 0 (full re-anchor).Leak-checked (testing allocator).
zig build testgreen,zig fmtclean, andscripts/test-pty-codex-ws.py(WS-fail → SSE fallback, WS-off) still passes.Closes #194