fix(serve): re-attaching to a session delivered its transcript out of order#31
Merged
Conversation
… order
A client reconnecting to a `serve --listen` session was shown output before the context it belonged to.
Two independent defects, both on the attach path, both reproduced by tests written before the fix.
**1. The backlog raced the live stream.** `attach` registered the connection's sink into the `OutFanout`
(live frames flowing at once) while catch-up was left to a *client-initiated* `get_messages {since}`
issued afterwards. Frames carry no sequence number, so delivery order is the only order a client has:
whatever arrived first was rendered first. Re-attaching to a run in flight — which `serve_ws`'s own docs
call "the entire point" — the client got that run's live `event` frames immediately and its history,
maybe, later.
**2. Mid-run, the history was unobtainable at all.** `get_messages` was absent from the busy loop's
accepted-command list, so for the whole duration of a run it was rejected as busy. The reconnecting
client was told to come back later, having already been shown that run's live output.
**3. Committed history alone was never going to be enough** (the part I initially shrugged off). The turn
in flight hasn't committed, so it is nowhere in the history — a client attaching mid-turn would pick it
up from the middle: a `tool_end` for a `tool_start` it never received, an assistant message missing its
opening.
Catch-up is now **seeded server-side at attach, and is base plus frames**: a one-shot `catchup` frame
carrying the transcript as of the current run's start, immediately followed by a replay of every frame
that run has emitted so far, and only then does the sink go live. The two reconstruct exactly what a
client that never dropped has seen.
The base deliberately does *not* advance at mid-run checkpoints. If it did, a message committed mid-turn
would land in the catch-up *and* again in the replayed frames, and the client would render it twice.
Ordering is structural, not lucky. The history snapshot and the in-flight-turn recording both live inside
`OutFanout`, under the very lock `broadcast` takes, so seed → replay → register is one critical section.
Two locks would leave a window where a message commits after the snapshot is read but before the sink is
registered — broadcast to the old sink set, absent from the catch-up, and dropped from that client's view
entirely.
`get_messages` is now answerable mid-run from the same snapshot (the run holds `&mut session`, the same
borrow problem `LiveStats` already solves for `get_state`), so a client can reconcile rather than wait.
Costs are small and bounded. `Session::messages` is already an `Arc`, so publishing the base is a
refcount bump and an `Arc::ptr_eq` guard means an idle session pays one pointer compare per command. The
replay recording is capped at 4 MiB per session (`TURN_REPLAY_MAX_BYTES`) — one tool-heavy turn can emit
megabytes and the daemon multiplexes many sessions — and past the cap it is dropped and the catch-up
reports `turn_truncated: true` rather than handing over a turn with a silent hole in it.
Protocol note: `catchup` is a new unsolicited frame on the WebSocket/UDS transports. Fresh sessions with
no history and no run in flight get none. stdio is untouched (one connection for process life, no
re-attach).
Tests: `crates/agent/tests/serve_reattach_catchup.rs` — four, each re-attaching to a run genuinely in
flight (a real `bash sleep`, not a race against a fast local round trip). All four failed before this
change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
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.
The bug
A client reconnecting to a
serve --listensession was shown output before the context it belonged to. Three defects on the attach path — each reproduced by a test written before the fix, all of which failed.1. The backlog raced the live stream.
attachregistered the connection's sink into theOutFanout— live frames flowing at once — while catch-up was left to a client-initiatedget_messages {since}issued afterwards. Frames carry no sequence number, so delivery order is the only order a client has. Re-attaching to a run in flight (whichserve_ws's own docs call "the entire point"), the client got that run's live output immediately and its history, maybe, later.2. Mid-run the history was unobtainable at all.
get_messageswas absent from the busy loop's accepted-command list, so for a run's entire duration it was rejected as busy — the client told to come back later, having already been shown that run's live output:3. Committed history alone was never going to be enough. The turn in flight hasn't committed, so it's nowhere in the history. A client attaching mid-turn picked it up from the middle — in the test, a
tool_endfor abashcall whosetool_startit never received.The fix
Catch-up is now seeded server-side at attach, and is base plus frames: a one-shot
catchupframe carrying the transcript as of the current run's start, immediately followed by a replay of every frame that run has emitted so far, and only then does the sink go live. Together they reconstruct exactly what a client that never dropped has seen.The base deliberately does not advance at mid-run checkpoints. If it did, a message committed mid-turn would land in the catch-up and again in the replayed frames — rendered twice.
Ordering is structural, not lucky. The history snapshot and the in-flight-turn recording both live inside
OutFanout, under the very lockbroadcasttakes, so seed → replay → register is one critical section. Two locks would leave a window where a message commits after the snapshot is read but before the sink is registered: broadcast to the old sink set, absent from the catch-up, and dropped from that client's view entirely.get_messagesis now answerable mid-run from the same snapshot — the run holds&mut session, the same borrow problemLiveStatsalready solves forget_state— so a client can reconcile rather than wait.Cost
Small and bounded, by design:
Session::messagesis already anArc, so publishing the base is a refcount bump. AnArc::ptr_eqguard means an idle session pays one pointer compare per command.TURN_REPLAY_MAX_BYTES). One tool-heavy turn can emit megabytes and the daemon multiplexes many sessions, so an uncapped recorder would be a per-session leak for the life of a run. Past the cap it's dropped and the catch-up reportsturn_truncated: true— rather than handing the client a turn with a silent hole in it.broadcast's zero-copy single-sink fast path is preserved; the recording clone is the one allocation added, and only while a turn is in flight.Protocol
catchupis a new unsolicited frame on the WebSocket/UDS transports:{"type":"catchup","data":{"messages":[...],"leaf_id":"..."},"turn_in_flight":true,"turn_truncated":false}datais byte-for-byteget_messages' payload, so clients parse one shape either way. Fresh sessions with no history and no run in flight get none. stdio is untouched (one connection for process life, no re-attach). Clients that ignore unknown frame types are unaffected — nothing in the suite broke — but a client should learn this frame to actually benefit.Tests
crates/agent/tests/serve_reattach_catchup.rs— four tests, each re-attaching to a run genuinely in flight (a realbash sleep, so it's not a race against a fast local round trip):reattach_midrun_can_fetch_missed_history—get_messagesmid-run must not be rejected as busyreattach_delivers_history_before_live_frames— the backlog arrives before any liveeventreattach_replays_the_in_flight_turn_not_just_committed_history— notool_endwithout itstool_startreattach_catchup_frame_has_the_get_messages_payload_shape— pins the wire shape, so the others can't pass on an accidental matchAll four failed before this change. Full agent suite green (59 test binaries, 0 failures), workspace clippy clean.
serve.rs's frame catalogue andARCHITECTURE.mdboth documented the oldget_messages {since}catch-up flow and are updated.🤖 Generated with Claude Code