fix(terminal): drop stale terminal_output chunks after a reconnect race - #437
Open
RonenMars wants to merge 1 commit into
Open
fix(terminal): drop stale terminal_output chunks after a reconnect race#437RonenMars wants to merge 1 commit into
RonenMars wants to merge 1 commit into
Conversation
Guard against a frame from a superseded WS connection landing after a reconnect reset, using the streamer's new per-session seq field.
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.
Summary
Investigated a Terminal-tab bug where displayed conversation lines appeared shuffled.
useTerminalStream.tstrusts raw WebSocket arrival order to feed its client-side VT100 emulator (virtual-terminal.ts) — normally reliable, but a reconnect can leave a staleterminal_outputframe from the superseded connection in flight, and it can land after the client has already reset state for a freshterminal_replay, landing content out of order.The companion streamer PR (threadbase-streamer#299) adds a per-session monotonic
seqtoterminal_output/terminal_replay. This PR consumes it here: alastSeqRefguard drops anyterminal_outputchunk whoseseqisn't greater than the last accepted one, andterminal_replay.seqbaselines it on (re)subscribe.seq(older version) never trips the guard —msg.seq != nullgates both checks.Textwrap, working as intended).Base branch note
Branched from
fix/diff-viewer-full-width-lines-integration(currently the most recent upstream branch) since basing offmaindirectly produced Podfile.lock rebase conflicts from unrelated iOS build-number bump commits already on that branch. This PR's own diff touches onlyhooks/useTerminalStream.ts,services/ws-client.ts, and its new test file — no overlap with the base branch's changes.Test plan
npx tsc --noEmit— cleannpx eslint hooks/useTerminalStream.ts services/ws-client.ts __tests__/unit/hooks/useTerminalStream.seq.test.tsx— clean__tests__/unit/hooks/useTerminalStream.seq.test.tsx: accepts increasing seq, drops a stale/lower seq, baselines fromterminal_replay.seq, and never rejects when a streamer omitsseqentirelyuseTerminalStream.userMessages.test.tsxanduseTerminalStream.watchdog.test.tsxstill pass (14/14 total across the three suites)