fix(ws): a 426 latches SSE immediately — the server answered authoritatively (#427) - #443
Merged
Conversation
…ry (#427) ws.zig has always returned error.UpgradeRequired for a non-101 handshake whose status is 426, with a doc comment saying the caller should fall back to SSE — but no caller consumed it. postLive treated it as a generic transport failure, so the server's authoritative "I will not upgrade this endpoint" still cost one full rebuild plus a fresh dial (which 426s again) before ws_failures_before_fallback finally set ws_off. postLive now folds `declined` into the ladder's fallback term: a 426 latches ws_off immediately and THIS attempt falls through to the SSE path, with no error.CodexWsReanchor asking request() to rebuild first. openai/codex does the same thing (WebsocketStreamOutcome::FallbackToHttp). The trace note names the reason instead of claiming a second failure that never happened. Everything else is unchanged: a non-426 handshake failure keeps its free retry and only latches on the second, and the delta-body guard still routes through CodexWsReanchor so a previous_response_id is never replayed on SSE. Tests: agent_ws_mock grows refuseUpgrade, which serves the whole ladder over one loopback port — `refusals` refused handshakes with a caller-chosen status line, then the SSE POST graff falls back to — plus releaseAccept, so a failed assertion reports rather than parking fut.await on a blocked accept. The two new tests in agent_ws_fallback_test.zig drive postLive end to end: a 426 with exactly one dial and the turn served over SSE, and a 500 that still spends the free retry first. Both assert the sink saw no transport_aborted — choosing a transport is a routing decision, not a cut stream. 994 -> 996 tests. agent_ws.zig was at 598/600, so the read loop's `budget` if/else is folded to one line to pay for the new branch; no behaviour rides on it. 599 lines. Co-Authored-By: Codegraff <blackfloofie@codegraff.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.
Fixes #427, the codex-parity gap found in the deepwiki comparison:
ws.zighas always raisederror.UpgradeRequireddistinctly on a 426 handshake, but no caller consumed it — so a server that declared it won't upgrade cost one full rebuild + redial before the two-failure latch.The fix
One term folded into the ladder's existing decision:
declined or wsShouldFallback(...). A 426 latchesws_offand serves this attempt over the persistent SSE path — noCodexWsReanchor, no rebuild, no second dial. The structurally-unreachable delta-body case still routes through the existing guard so aprevious_response_idcan never reach SSE. Trace notes the decision in the file's own ternary idiom;transport_aborteddeliberately untouched (a routing decision is not a stream abort). Cap paid honestly: one syntactic fold,agent_ws.zigat 599/600.Evidence
agent_ws_mock.refuseUpgradeserves the whole ladder on one loopback port (N refused handshakes with a chosen status, then the SSE POST graff falls back to). Newagent_ws_fallback_test.zigdrives the realpostLiveend-to-end: (a) one 426 → 1 dial,ws_off, SSE completion, correct trace note; (b) a 500 handshake keeps today's retry-then-latch exactly; (c) zerotransport_abortedevents in either case.declinedterm → 995 pass / 1 fail, exactly test (a).acceptisn't cancellable, so failed assertions used to hang the mock —releaseAccept(throwaway dial, the http: a send-failed request re-pools its dead connection — one WriteFailed poisons every later non-streaming request (compaction, [title], subagents) #177 trick) + LIFO-ordered guards make reverts fail clean instead of parking forever. Worth copying for future multi-connection mocks.