Repository navigation
fix(ws): detect a dead socket that still reads open with an app-level ping - #967
Merged
Merged
Conversation
A WebSocket that stays in CONNECTING (a phone waking on a dead network, a tunnel that drops the handshake) had no deadline, so no new attempt ever started. wake() also returned when no reconnect timer was set, which is exactly the state of a page that froze before its socket's close event ran. Close and retry an attempt still CONNECTING after 8 s, ending it on the client side without waiting for a close event. wake() now ends a dead socket whose close event is pending and connects at once; an open or connecting socket is left alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hakes connect - wake() leaves a CLOSING socket to its close event, so an output-stalled (4008) close still stops output and a submit-result that arrives during the closing handshake is still delivered. - A CLOSED socket wake() ends early is remembered: if its late close event says 4008, the replacement connection is closed and output stops. - The connect deadline doubles with each attempt given up (8 s, 16 s, capped at 30 s) and starts over once an attempt opens, so a remote PC whose upgrade waits up to 15 s for the machine relay still connects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n it does not answer
A socket can read OPEN while its TCP path is gone (Wi-Fi to cellular, a NAT entry expired while
the phone slept). No close event comes for minutes, so the terminal froze while the header said
connected, and typed keys went into the dead socket instead of the held draft.
- protocol: ClientMessage `ping {id}`, ServerMessage `pong {id}`, ServerFeature "ping".
- server: answers ping with pong for any role, lists "ping" in its snapshot; MachineRelay
already forwards both unchanged, so a remote PC's own snapshot decides.
- demo transport: lists "ping" and answers it.
- HerdrSocket: tracks lastFrameAt; on a bridge listing "ping" it probes an OPEN socket on
wake (visible, pageshow, online) and after 20 s without a frame while visible. Any frame ends
the probe; none within 3 s drops the socket (disconnect handlers, submits settle) and connects
at once with retries reset. While a probe waits, canInput() and submit() refuse, so typing goes
to the held draft. New ConnectionState (`state`) for the next change to build on.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ing in a held draft
Review follow-ups on the liveness probe:
- a held draft now takes all later typing until Send or Discard, so a probe that
ends mid-command can no longer split it ("echo " held, "rm -rf build\r" run);
chords are refused with a note and the input line's Enter is refused
- only the pong of a ping in flight ends a probe; buffered frames and stray pongs
no longer re-enable input on a dead socket
- submit, pendingAction and sendSecret wait out a probe (before sending, after the
snapshot wait) instead of refusing or skipping it
- a socket dropped by a probe is kept in endedEarly (now a WeakSet), so a late
4008 still stops output
- hiding the page cancels a probe; a deadline that fires late after a freeze pings
again instead of dropping
- the ping feature is remembered across reconnects, so a socket whose snapshot
never comes is probed too
- a second ping before a drop, and each probe drop doubles the per-ping deadline
(cap 12 s) until a pong
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Text, pastes, Backspace and line breaks still join the held draft, but Ctrl+C, Escape, Tab and arrows go to the program as before, so the Stop button and an interrupt keep working while input is held. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 13, 2026
…replaced closing socket Hiding the page ended the probe, and a submit, pending action or secret waiting on it then went out on a socket no pong had vouched for. The probe now pauses: input stays refused, waiters keep waiting, and the page coming back pings again. connect() could replace a CLOSING socket without ending it, leaving its probe deadline over the new socket's input and a submit waiting on that probe unsettled forever. It now ends the old connection first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ide a held draft - Ctrl+Enter under modifyOtherKeys was rewritten to CSI 27;5;13~ before the held-draft check and, with no line break in it, reached the program and ran its partial line. A modified Enter is now decided on before the rewrite and refused like a chord. - A paste the draft cannot take (a tab) went to the pty ahead of the held start, and a multi-line one was dropped in silence. Both are now refused with the held-input note. - The input line's empty Enter said "disconnected" on a connected terminal, and a non-empty line was typed with Enter beside the held draft. onEnter now returns the reason and both are refused with the held-input note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d, never dropped in silence Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
…input (#956) ## Problem In direct typing, input the terminal cannot take yet (socket down, or the pane not input-ready after a phone resume) is held for review in the "Input held until the terminal is ready" banner. The held text did not match what was typed: - `lss`, Backspace, ` -la` was held as `lss -la`: the deleted character came back. - `ls`, Enter, `pwd` was held as `lspwd`. - A multi-line paste, raw (`a\rb`) or bracketed (`\x1b[200~a\nb\x1b[201~`), vanished whole with no notice. ## Cause `applyToDraft` (src/lib/draft.ts) kept a chunk only if it held no C0/C1 control at all and otherwise returned the draft unchanged with no flag. DEL, BS, CR and LF were all treated like arrows and xterm's automatic reports. ## Fix - A leading run of DEL / BS in a chunk deletes that many code points from the held text (`Array.from`, so an emoji or Hangul syllable is never split). Text after the deletions in the same chunk is folded in as usual. - A chunk (or the inside of a bracketed paste) with CR/LF keeps its text, with a single space where each run of line breaks was, and sets a new `leftOut` flag. The held text never contains a line break, so Send still never presses Enter. - The banner shows a new notice when `leftOut` is set: "An Enter or line break was left out: Send never presses Enter." (ko/ja/zh, and tr since that locale lists the neighbouring strings). `draftIsEmpty` counts the flag, and `restoreDraft` reads it back (anything but `true` reads as false, so older records restore unchanged). - An Enter or a run of line breaks with no text held changes nothing (no empty notice, nothing stored). - A tab inside a bracketed paste is held as a space (a typed Tab stays out: it is completion). A paste, or a multi-line chunk, that still has other controls sets `truncated` as well, so the notice says text was left out instead of implying it was kept. - Once `truncated` is set, Backspace no longer edits the held text: it may be meant for the chunk that was dropped. - Other typed controls (arrows, Tab, Ctrl keys, CPR/focus/mouse reports) stay silent, as before. ## Tests - `src/lib/draft.test.ts`: `ab` + `\x7f` -> `a`; `\b` likewise; empty + `\x7f` -> unchanged; surrogate/Hangul deletion; `\x7f\x7f\x7fthe` replaces; `ls`,`\r`,`pwd` -> `ls pwd` + `leftOut`; `a\rb` and `\x1b[200~a\nb\x1b[201~` -> `a b` + `leftOut`; restore of the flag; bare Enter / empty paste -> `EMPTY_DRAFT`; bracketed `a\tb` -> `a b`; raw `a\tb\nc` and bracketed `a\x1bb` -> `truncated`; Backspace after a loss keeps the held text. - `scripts/terminal-input-regression.ts` (via `ui-regression.ts`): in the not-ready state, a bare Enter shows no left-out notice, `b` + Backspace removes the `b`, Enter after text shows the notice, and after reload both the held text (`a `) and the notice are back; Send sends `a `. - Ran: `bun run typecheck` (pass), `HERDR_TEST_MODE=unit bun test ./src/lib/draft.test.ts` (23 pass; 5 fail on the first commit's draft.ts), `bun run test:unit` (3076 pass, 0 fail), `bun run build` + `bun run check run bun scripts/ui-regression.ts` (all PASS). ## Notes - Design choice: the space where a line break was is a character the user never typed. It keeps `ls` Enter `pwd` readable as two words instead of `lspwd`, and the notice says a break was left out. The alternative (keep `\n` in the draft and have Send deliver it as a bracketed paste) was not taken: it changes what Send writes and depends on the program's paste mode. - A lone Enter with nothing held shows nothing, as before. The flag is for Enter/line breaks only, not the "special keys dropped" count that was removed earlier (#561). - Known limits, left out on purpose (the banner shows the held text before Send): Backspace right after a break deletes the space that stands for it (fixing it needs new stored state); arrows that move the cursor, Ctrl+U, Ctrl+W, Alt+Backspace and Cmd+Backspace are not followed; a chunk with DEL after text is dropped whole; a raw single-line chunk with a control is still dropped silently, since it cannot be told from typing. The changelog names only the Backspace key. - Overlap: #951 reworks the held-input banner into an activity row and reads `draft.truncated` there. Whichever merges second needs to carry the `leftOut` notice into the new row. No open PR touches `src/lib/draft.ts`. - Source: mobile-terminal stability audit finding input-3. ## Merge-session review Fixed in ca92b54: - Medium: a typed ESC CR (physical Shift+Enter or Alt+Enter) went through the line-break branch and set both `truncated` and `leftOut`, so the draft showed the "too long" notice and later Backspace stopped editing it. A typed chunk containing ESC is now treated as a key sequence again: neither held nor told. Tests: `\x1b\r` / `\x1b\n` -> `EMPTY_DRAFT`; `ls`, `\x1b\r`, `\x7f`, `x` -> `lx` with no flags (both failed at f9b902c). - Low: Backspace that erased all held text kept `leftOut`, leaving an empty banner with only the line-break notice (and saving it). Erasing everything held now returns the empty draft. Test: `ls`, `\r`, three `\x7f` -> `EMPTY_DRAFT` (failed at f9b902c). - Low: the truncated notice blamed length even when a paste with control characters set it. The string is now "Some input could not be held and was left out." in en/ko/ja/zh/tr; `src/lib/i18n.test.ts` checks every locale has it. The changelog fragment was updated to match. Deferred: - Coordination with #968 (stacked on #967): on restack after this PR merges, #968's `eraseFromDraft` expectations need `leftOut: false`, its Backspace erase should reuse this PR's ERASE path instead of a second implementation, and `heldInputAction` must check line breaks and ESC CR before its `applyToDraft(...) !== EMPTY_DRAFT` "hold" test if blocking stays the contract (update src/AGENTS.md to match). Its existing `"line1\nline2"` / `"\x1b\r"` -> `block` tests catch a naive merge. Not a defect in this PR alone. - #951 overlap: the truncated notice's English key changed, so whichever of #951 and this PR merges second must use the new key. Checks: `bun run check fast` (typecheck, build, 3078 unit pass), `CI=1 bun run check run --build bun scripts/terminal-input-regression.ts` (PASS). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a space, after #956 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
#967 landed with its own review fixes, so this keeps main's version of the probe and held-draft code and drops the parallel #967 fixes this branch carried. Only this PR's own change remains: re-attach a pane whose output stopped while the page was put away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
…ped output (#968) ## Problem A phone put away while its pane stays attached comes back to "Terminal output stopped because this device could not keep up." and a dead terminal for the whole PC. The pane stays attached in two cases: a message queued from this tab (`queueWaits`), or a mirrored pane (`fixedGridRef`). Its frozen page sends no `pty-ack`, so the server's 2 s stall check closes it with 4008. Unlocking the phone does not recover it. The only ways out are the banner's Reconnect link, which reloads the whole app, or picking another pane. This is stability-audit findings connection-2, history-2 and moshi-2. ## Cause - On a 4008 close, `HerdrSocket` sets its stopped flag and schedules no reconnect. `wake()` refuses while the output is stopped (intended: no automatic reconnect loop). - Only `attach()` clears that flag. `PaneTerminal.back()` calls `resume()`, which attaches, only for a released pane. A pane that kept its attach only gets `refit()`, which sends a resize into a closed socket. - A frozen page often reads the server's close only after it resumes, after `visibilitychange` and `back()` have already run. Also, the 2 s stall clock can still run out while the page catches up on the ACKs it held back. So checking visibility when the close arrives is not enough. ## Fix - `src/lib/ws.ts`: - New getters `outputStopped` and `stoppedWhileHidden`. The private field is renamed to `stopped`. - A 4008 counts as the hidden page's in three cases: it came while the page was hidden; it came while the page had been hidden and not yet shown; or it came within `SHOWN_GRACE_MS` (5 s) of the page being shown again. - A page that connects while hidden counts as put away until it is first shown. - The `attach()` that clears the stop also clears the mark and uses up the grace period. A second stall on the new connection while the page is in use is therefore final. - `wake()` still refuses a stopped output, as `src/AGENTS.md` requires. - `src/components/PaneTerminal.tsx`: - New `reattach()`. It clears `stalledRef`, the notice and `ended`, runs the `resume()` path (stream-order `\x1bc` reset, unwatch if watching, `socket.attach`), and restores `disableStdin`. The attach reconnects, and the connection's open replays every attach remembered for that PC. - It runs once when the page is back in use. It is called from `back()` when the close was read while the page was hidden. It is called from the `output_stalled` handler, in a microtask, when the close is read after the page was shown. - A 4008 while the page stays in use keeps today's behaviour: notice, no automatic reconnect. - The banner's Reconnect `<a href="?machine=&pane=">` (a full page reload) is now a `<button>` that runs the same in-place `reattach()`. The label is unchanged, so no i18n entries were added. - Docs: `docs/terminal-flow-control.md` step 5 and the QA paragraph, and the 4008 rule in `src/AGENTS.md`. - Changelog fragment `changelog.d/recover-hidden-output-stall.md`. ## Tests - `src/lib/ws.test.ts`: four new cases. Each one fails without the fix, checked by stashing `ws.ts`. - A 4008 while hidden is marked. Showing the page reconnects nothing by itself. An attach after showing clears `outputStopped`, reconnects, and replays both remembered attaches. - A 4008 read 1 s after the page is shown again is marked. After the re-attach, a second 4008 while in use is final (`stoppedWhileHidden` false). - A 4008 long after the page came back is final. A later `wake()` and a hide/show do not reconnect, even 60 s later. - A page that connected while hidden counts as put away until it is first shown. - `scripts/output-browser-qa.ts`, real herdr, xterm and Chromium, run with `bun run check run`: **passed**. - The existing visible-stall flow now clicks the Reconnect **button** and still asserts no automatic reconnect loop. - A new scenario fakes `visibilityState`/`hasFocus`, puts the page away with ACKs held and the pane still attached (desktop, release-when-away off), and waits for the 4008. It then shows the page and asserts that the pane attaches again with no click, the notice goes away, and exactly one reconnect happened. - With the two recovery branches disabled, the new scenario times out at "shown page attaches again by itself". - It covers only the order where the close is read while the page is hidden. The other order is covered by the unit tests. This script is not part of CI's lanes. - `bun run typecheck`: passed. `bun run test:unit`: 3091 pass, 0 fail, 8 skip; this ran before the small follow-up commit, after which `ws.test.ts` and typecheck were run again and passed. `check full`/`concurrent` were not run. ## Notes - Stacked on #967: it builds on #967's `ConnectionState`/`endedEarly` and the wake/probe code in `ws.ts`. Retarget to main after #967 merges. - #951 adds its own `?machine=&pane=` Reconnect link in a composer notice. It is a different element, so there is no conflict, but that link still reloads the page. A follow-up could send it through the same re-attach. - #960 (stall clock driven by ACK progress, server side) and issue #781 (stop hidden clients from reaching 4008 at all) are the prevention half. This PR is client recovery only and does not touch the server. The queued message that the server's `stopSlowClient` closes is still #686/#874. - The banner also shows for `attach_conflict`, so the button re-attaches in place for that code too. The server no longer sends it; only a test mentions it. - The re-attach redraws the pane in view from a reset screen. Other panes of the same PC come back through the replayed attach and their snapshot, as they do after a pane pick today. ## Review follow-up (cc86059, 0303ac0) These fix the probe and held-draft code this branch carries from #967 (#967 itself was not updated). Fix: 1. (high) Hiding the page during a probe released waiting sends on an unverified socket. `endProbe()` now records whether the probe was answered; `probeSettled()` lets a waiting submit, pending action or secret go only after a pong. A probe cancelled by hiding (or ended by the connection) settles them as `disconnected`, and the secret is cleared on that path. The secret still lives in the closure while it waits for the pong; that is inherent to waiting, and it is never sent unanswered. 2. (high) Ctrl+Enter under modifyOtherKeys (`ESC[27;5;13~`) passed the held-draft guard and committed the partial line. New pure `heldInputAction()` in `src/lib/draft.ts` decides by meaning: Enter itself (captured before the remap), any encoded Enter (`CSI 27;m;13~`, `CSI 13;m u`), any line break and chords are blocked with the existing "Not sent: send or discard the held input first." note. The input line (`sendTerminalLine`) and the composer (`submitComposerMessage`, new `held_input` code in `submitNote`) also refuse while a draft is held. 3. (medium) The pong returned before handlers ran, so a view that stored `inputReady=false` during the probe stayed disabled. The probe's pong now goes on to handlers after the state is updated, and PaneTerminal's 1 s poll also re-reads `canInput()` (covers the probe start, which has no frame). 4. (medium) Remembered ping support could drop an older bridge after a reconnect. A snapshot that leaves out `ping` now ends a probe in flight as answered (trusted as before) without counting a drop. A separate deadline for a snapshot that never comes was left out: on such a socket the probe's own two pings still decide. 5. (medium) Backspace now edits the held draft (`eraseFromDraft`, whole code points). A paste the draft cannot keep whole (line breaks) is refused with the same note instead of being lost silently. Line breaks are not held: Send types the draft with `sendInput`, so a held newline would run lines. 6. (low, not changed) Typing during a probe goes to the held draft. Buffering keys and flushing them on the pong would queue input typed into an unverified socket, against the never-queue rule in `src/AGENTS.md`; holding only after the first ping times out would send on a possibly dead socket for the first deadline, which defeats the probe. Left as a known UX trade-off for the maintainer to decide. Tests: - `src/lib/ws.test.ts`: three new cases (hidden page refuses waiting submit/pending action/secret, then a fresh probe's pong sends a new submit; the pong reaches handlers with `canInput()` true; a snapshot without `ping` ends a remembered probe and the connection is kept past 30 s). All three fail with `ws.ts` stashed. - `src/lib/draft.test.ts`: `heldInputAction` table (encoded Ctrl+Enter, CR/LF/Alt+Enter, Backspace, single- and multi-line pastes, pass-through controls) and `eraseFromDraft`. - `scripts/terminal-input-regression.ts`: with a held draft, the pane sends `ESC[>4;2m`, Ctrl+Enter shows the note and sends nothing, Backspace edits the draft. - `bun run typecheck` passed; `bun run test:unit` 3099 pass, 0 fail, 8 skip; `bun run build` then `bun run check run bun scripts/ui-regression.ts` passed (all PASS, exit 0). `check full`/`concurrent` not run. ## Merge-session review (d0b66cc..HEAD) Fixed: 1. (low) A probe the hidden page cancelled while a submit, pending action or secret still waited for the snapshot let the frame go out unverified (`probeSettled()` only counted a probe whose timer was still running). Requests now read a probe generation (`probeMark()`) before the snapshot wait; a probe in flight then, or started during the wait, counts even if it already ended, and only its pong lets the request go. Test: `src/lib/ws.test.ts` "refuses a submit, a pending action and a secret whose probe the hidden page cancelled while they waited for the snapshot" (fails without the fix). 2. (low) `held_input` was missing from `submitNotTyped()`, so Send now on a local held queue row refused for a held terminal draft turned the row "uncertain" (Copy only). Added, with a case in `src/lib/compose.test.ts`. 3. (medium, merge coordination with #956) `heldInputAction()` now blocks line breaks, encoded Enter and pasted tabs before it asks `applyToDraft()`, so its answer no longer changes when #956 makes `applyToDraft()` fold breaks into a space. `eraseFromDraft()` now follows #956's Backspace rule: it leaves a truncated draft alone and returns `EMPTY_DRAFT` when erased to nothing. Checked with a throwaway no-commit merge of #956 into this head: `src/lib/draft.test.ts` passes (30/30). Policy kept: Enter beside a held draft is refused with "Not sent: send or discard the held input first." (this PR), not folded into a space with the left-out notice (#956). #956's left-out path still applies to the chunk that starts a draft. For the restack onto main once #956 is merged: - `scripts/terminal-input-regression.ts` conflicts textually. Take this PR's post-ready block. In #956's not-ready step, the Enter pressed with "a" held is now refused: expect the "Not sent" note and draft text "a" (no left-out notice), and adjust the reload/Send expectations from "a " to "a" accordingly. - Keep one Backspace path: replace `setDraft(eraseFromDraft)` in PaneTerminal with `setDraft((prev) => applyToDraft(prev, input))` (#956's ERASE) and drop `eraseFromDraft` and its tests. Keeping `eraseFromDraft` instead fails typecheck after #956 (`InputDraft` gains a required `leftOut`, so its test fixtures need `leftOut: false`); the throwaway merge ran `bun test` only. Deferred: - Item 6 of the review follow-up above (typing during a probe goes to the held draft) is still a maintainer decision. Checks: `bun run check fast` passed (typecheck, build, unit 3101 pass / 0 fail / 8 skip). `CI=1 bun run check run --build bun scripts/ui-regression.ts` passed (terminal input, held input and Safari replay steps included). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
Conflicts: - src/components/TerminalInput.tsx: send() keeps reading the draft store and takes main's onEnter string result (#967 held-input reason). - scripts/terminal-input-regression.ts: the PR's boolean socket refusal is renamed refuseConnections/currentSocket beside main's refuse() submit-result callback and toPage; the PR block reuses main's note locator. - docs/terminal-input.md: main's key-bar paragraph (#962) plus the Send-during-composition sentences. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
## Summary Four review fixes from cc86059 and 66599c7 never reached main when #967 merged. I checked each claim against origin/main (a338f8b) and found no equivalent handling for any of them. All four are restored here on main's current probe design (`pauseProbe`/`probePaused`). The original `endProbe(answered)`/`probeAnswered` scheme is not brought back: main handles the hidden-page case differently on purpose, by keeping submits waiting (ws.test.ts "keeps a submit and a secret waiting when the page hides during a probe"). | Dropped fix | Gap on main | Restored as | |---|---|---| | Composer send refused beside a held terminal draft (cc86059) | `submitComposerMessage` never checked `draftHeldForRef`. The message box, quick sends and a queued "Send now" typed text and pressed Enter after a partial line that had already reached the shell. | `submitComposerMessage` answers `held_input`. `submitNote` gives the held-input note. | | `held_input` counts as nothing typed (66599c7) | `submitNotTyped` did not list it, so a refused queued row would be marked uncertain. | Added to `submitNotTyped`. A refused "Send now" now stays held. | | Probe pong goes on to handlers, and the poll re-reads `canInput()` (cc86059) | `ws.ts` returned early on every pong. If a frame arrived during the probe, the view stored `inputReady=false`. On a quiet pane, the "Waiting for terminal input…" banner and the disabled key bar then stayed until some later frame arrived. | The pong reaches the handlers after `endProbe()`. PaneTerminal's 1 s poll calls `setInputReady(socket.canInput(pane))`. | | A snapshot without `ping` ends a probe (cc86059) | A probe sent before the snapshot, on ping support remembered from the last connection, dropped a healthy older bridge after 3 s + 3 s. If the page hid before the snapshot, the paused probe was never cleared, because `probe()` returns early on `!pingKnown`. `canInput()` then stayed false for the whole connection. | The snapshot handler calls `endProbe()` when `!pingKnown && this.probing`. This covers both the running and the paused case. | ## Tests - `ws.test.ts`: three tests from cc86059, adapted to main. They cover the pong reaching the handlers, the snapshot without ping ending a running probe while the connection and a waiting submit carry on, and the paused-probe variant. I ran all three against main's unfixed `ws.ts` and they failed. - `compose.test.ts`: checks the `held_input` entries for `submitNotTyped` and `submitNote`. - `bun run check fast`: typecheck, build and lint pass. `server/portal.test.ts` failed once in the parallel unit run. It passes on its own, and this PR does not touch that file. - Browser runs with `CI=1 bun run check run`: `ui-regression.ts` (held draft, composer and terminal input checks), `ui-contexts-regression.ts`, `terminal-input-regression.ts`, `held-rows-demo-regression.ts` and `pending-pane-switch-regression.ts` all pass. Docs: I updated `src/AGENTS.md` (WS client and Held terminal draft rows) and extended `changelog.d/ws-liveness-probe.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Review Fixes from the review, in a8752f8: - **Medium: Send now on a server-owned queued row skipped the held-input guard.** `actOnPending` sent `pending-action` straight away, and the server pastes the text and presses Enter for non-Claude agents, so a partial line left in the pty would run with the message. The guard now runs in `actOnPending` for every Send now, before `begin`. It refuses with `held_input`, does not mark the row uncertain, and leaves Send now (once the draft is settled) and Discard working. The changelog fragment and the `src/AGENTS.md` row are now accurate as written. - **Low: no test covered the composer guard or the poll's `canInput` re-read.** `scripts/pending-pane-switch-regression.ts` now holds a terminal draft and checks three things: Send now on a server-owned row sends no `pending-action` and shows the held note, a message-box send sends no `submit` and keeps its text, and Send now delivers exactly once after Discard. `scripts/terminal-input-regression.ts` holds the probe's pings on a quiet pane. It checks that "Waiting for terminal input…" shows within about a second and clears after the pong. I confirmed each test fails without its fix. - **Low: a refused Send now on a row showed the raw English reason.** `PendingMessages` now sends `held_input` through `submitNote` along with `input_draft`. A new unit test, `src/components/PendingMessages.test.tsx`, renders it in Korean. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 14, 2026
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.
Problem
A WebSocket can still report
OPENafter its TCP path is gone, for example when a phone moves from Wi-Fi to cellular or wakes on a changed network. No close event arrives for tens of seconds to minutes. In that time:rawSendon the dead socket andsendInput()returns true, so PaneTerminal never keeps them as a held draft and they are lost;wake()(visibilitychange / pageshow / online) leaves anOPENsocket alone.Cause
HerdrSockettreatsreadyState === OPENas proof that the connection is alive. The protocol has no app-level ping. Bun's own pings and idle timeout only tell the server about a dead peer, and the browser never sees them.Fix
shared/protocol.ts): newClientMessageping {id},ServerMessagepong {id}andServerFeature"ping". The doc comments on both frames are new.server/index.ts): answerspingwithpongfor any role and lists"ping"in its snapshot. A malformed id gets aninvalid_pingerror frame, not a pong.MachineRelayalready forwards frames as they are, so the remote PC's own snapshot decides whether the client pings. The contract test proves the pass-through.site/demo/transport.ts): lists"ping"and answers it the way the server does.src/lib/ws.ts):lastFrameAt. On a bridge whose last snapshot listed"ping", it probes anOPENsocket on wake (visible, pageshow, online) and after 20 s without a frame while the page is visible (checked every 5 s). The flag is remembered across reconnects, so a socket whose snapshot never arrives (a relay path that dies right after the upgrade, a failed snapshot) is probed too.pongof a ping in flight ends a probe. Any frame still postpones the idle probe, but a frame the browser buffered before a sleep, or a pong with another id, does not prove the path is alive. The probe ends before handlers run, socanInput()already reads true for them.socket = null, the disconnect handlers run, submits settle asdisconnected,close()is called, andconnect()runs at once with retries reset. The open handler then replays the role and attaches. Each probe that ends in a drop doubles the per-ping deadline (cap 12 s) until a pong comes back, so a slow relayed link is not dropped over and over.endedEarly(now aWeakSet, shared withwake()), so a 4008 close that arrives after the drop still stops output and closes the replacement. A socket that isCLOSINGwhen the deadline hits is left to its close event (same rule as fix(ws): give up a stuck connect attempt and reconnect at once on wake #954'swake()).canInput()refuses, so typing goes to the held draft.submit(),pendingAction()andsendSecret()wait the probe out after their snapshot wait and right before sending; if the socket is dropped they settle asdisconnectedhaving sent nothing (the secret is still cleared). The composer no longer reports offline during a routine probe.ConnectionState(idle | connecting | open | probing | waiting | stopped | closed), exposed asstate.connectedstill reads the socket itself, so the banner does not flash on every tab switch.src/components/PaneTerminal.tsx): once input is held for a pane, all later typing joins the draft until Send or Discard, also after the terminal is ready again. Before this, keys refused during a probe (or a disconnect) were held while the next keys after the pong went straight to the pty:echoheld andrm -rf build⏎run. Text, pastes, Backspace and anything with a line break join the draft; Enter is left out of it, a sticky-modifier chord is refused with a note (new string, translated in ko/ja/zh/tr), and the input line's Enter is refused while a draft is held. Other control keys (Ctrl+C, Escape from the Stop button, Tab, arrows) still reach the program, so an interrupt never waits on the draft.src/AGENTS.md: the WS client row records the probe rules, and a new row records the held-draft rule.Tests
src/lib/ws.test.ts(fake socket and fake timers):OPENsocket with no pong gets a second ping at 3 s and is dropped at 6 s. The disconnect handler runs once, a pending submit settles asdisconnected, a new socket starts with no backoff and its open replays role and attach at the saved geometry, and the late close of the old socket changes nothing;sendInputandsendKeysare refused; a bufferedpane-statusframe and a pong with another id leave it probing; the matching pong restores input; a second wake sends no second ping;submit,pendingAction(steer)andsendSecretstarted during a probe send nothing until the pong, then each sends once;wake()runs, and a secret, settle asdisconnectedwith nothing sent when the probe drops the socket;ping;CLOSINGtests.ws.tsof this branch (checked by restoring it).scripts/terminal-input-regression.ts: after the terminal becomes ready withaheld, Enter andbare typed: the draft readsaband nothing reaches the pane; Escape still reaches the pane with the draft kept; Send typesabonce with no Enter. It fails without the PaneTerminal change (checked by restoring it and rebuilding).server/ping.contract.test.ts(integration, real herdr): the snapshot listsping; ping gets a pong with the same id, also for an observe role; a malformed id getsinvalid_ping; ping and pong pass throughMachineRelay. Not rerun for the review fixes (no server change).bun run typecheck: pass.HERDR_TEST_MODE=unit bun test ./src/lib/ws.test.ts: 49 pass.bun run test:unit: 3088 pass, 0 fail, 8 skip (includes the test: enforce AGENTS.md conventions with repo-wide guard tests #949 convention guards).bun run check run --build bun scripts/terminal-input-regression.ts: pass.scripts/ui-contexts-regression.tspassed on the first revision; not rerun after the review fixes.Notes
wake()rules andSOCKET_CLOSINGhandling inws.ts.shared/protocol.tsandserver/index.tsare inputs to the remote bundle, so the release must raiseREMOTE_BUNDLE_VERSION. The release guard from ci: fail a release that forgets the remote bundle version or a telemetry key #948 (scripts/release-guards.ts) refuses to tag otherwise. It is not bumped here. Until a remote PC updates, its snapshot does not listping, so it is never probed and behaves as before.disconnectedon a probe drop rather than waiting for the close event: a half-open socket may not close for minutes, and the server never types a message more than 45 s after it arrived.inputReadydrives the "Waiting for terminal input…" banner and the key bar, so a sub-second probe on a healthy link would flash both on every wake. The held-draft banner already shows the case the user can act on, and the composer no longer reports offline. The remaining gap: PaneTerminal recomputesinputReadyonly when a message arrives.pingflag survives a reconnect: if a PC is downgraded to a bridge withoutping, the first probe after the reconnect costs one drop before the new snapshot clears the flag.resume()in PaneTerminal still resets the screen before the attach answers (render-scroll-1's second half).applyToDraftBackspace and line breaks) and the same lines ofscripts/terminal-input-regression.ts: a textual conflict, and the test expectation changes with it: after fix(terminal): apply Backspace and tell left-out line breaks in held input #956 an Enter in the draft becomes a space plus a notice, so the held and sent text becomea binstead ofab. Never an Enter either way.submit()guard area inws.ts, plusprotocol.tsand the demo transport.server/index.ts's message switch. These are textual merges with no conflict in meaning.server/machine-relay.ts. Nothing changed there.Merge-session review
Fixed (each with a regression test that fails without the fix):
ws.ts): a submit, pending action or secret waiting on a probe was sent on an unchecked socket once the page hid. A hidden page now pauses the probe (probePaused): input stays refused, waiters keep waiting, and showing the page pings again. Test:keeps a submit and a secret waiting when the page hides during a probe...; the existing hide test now expectsprobingand refused input while hidden.ws.ts):connect()now ends a non-null previous socket (CLOSING, or CLOSED before its close event) first, so its probe and waiting submits settle, the disconnect handlers run, and a late 4008 from it still stops output (endedEarly). Test:settles a submit waiting on a probe when a CLOSING socket is replaced....PaneTerminal.tsx): a modified Enter is decided on before its rewrite toCSI 27;5;13~and refused like a chord, with the held-input note. Test:terminal-input-regressionturns on modifyOtherKeys and presses Ctrl+Enter beside a held draft.PaneTerminal.tsx): while a draft is held, a paste the draft cannot take is refused with the held-input note. Test: pastea\tbandx\nybeside a held draft; nothing reaches the pane, and the multi-line paste is told or held, never dropped in silence (written so it still holds once fix(terminal): apply Backspace and tell left-out line breaks in held input #956 folds line breaks into the draft).TerminalInput.tsx,PaneTerminal.tsx):onEnterreturnsboolean | stringand the held-input reason is shown;sendTerminalLinerefuses beside a held draft too and keeps the text. Test: empty Enter andls+ Enter on the input line beside a held draft show the held-input note and send nothing.src/AGENTS.mdrows and the changelog fragment updated to match.Deferred (unchanged from the builder's notes, including its review finding 8): the items under "Left out on purpose" above, such as PaneTerminal recomputing
inputReadyonly when a message arrives.Notes for the merge step:
terminal-input-regressionSend step now checks the draft's own text, so it holds whether fix(terminal): apply Backspace and tell left-out line breaks in held input #956 showsabora b.Checks:
bun run check fastpass;scripts/ui-regression.ts(terminal input, Ctrl+Enter, held-draft lanes) pass;scripts/sticky-modifiers-regression.tspass.🤖 Generated with Claude Code