Skip to content

fix(ws): detect a dead socket that still reads open with an app-level ping - #967

Merged
devswha merged 11 commits into
mainfrom
fix/ws-liveness-probe
Oct 13, 2026
Merged

devswha merged 11 commits into
mainfrom
fix/ws-liveness-probe

Conversation

@devswha

@devswha devswha commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Problem

A WebSocket can still report OPEN after 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:

  • the terminal stops updating while the header still says connected, and no reconnecting banner shows;
  • keys go to rawSend on the dead socket and sendInput() returns true, so PaneTerminal never keeps them as a held draft and they are lost;
  • a composer send waits the full 90 s submit timeout;
  • wake() (visibilitychange / pageshow / online) leaves an OPEN socket alone.

Cause

HerdrSocket treats readyState === OPEN as 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

  • Protocol (shared/protocol.ts): new ClientMessage ping {id}, ServerMessage pong {id} and ServerFeature "ping". The doc comments on both frames are new.
  • Server (server/index.ts): answers ping with pong for any role and lists "ping" in its snapshot. A malformed id gets an invalid_ping error frame, not a pong.
  • Remote relay: no code change. MachineRelay already forwards frames as they are, so the remote PC's own snapshot decides whether the client pings. The contract test proves the pass-through.
  • Demo (site/demo/transport.ts): lists "ping" and answers it the way the server does.
  • Client (src/lib/ws.ts):
    • Tracks lastFrameAt. On a bridge whose last snapshot listed "ping", it probes an OPEN socket 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.
    • Only the pong of 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, so canInput() already reads true for them.
    • No pong within 3 s sends a second ping; no pong to that drops the socket on the client side: socket = null, the disconnect handlers run, submits settle as disconnected, close() is called, and connect() 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.
    • A dropped socket is kept in endedEarly (now a WeakSet, shared with wake()), so a 4008 close that arrives after the drop still stops output and closes the replacement. A socket that is CLOSING when 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's wake()).
    • Hiding the page cancels a probe; showing it starts a fresh one. A deadline that fires more than 2 s late (the page was frozen and the timer ran before the queued pong) pings again instead of dropping.
    • While a probe waits, canInput() refuses, so typing goes to the held draft. submit(), pendingAction() and sendSecret() wait the probe out after their snapshot wait and right before sending; if the socket is dropped they settle as disconnected having sent nothing (the secret is still cleared). The composer no longer reports offline during a routine probe.
    • New explicit ConnectionState (idle | connecting | open | probing | waiting | stopped | closed), exposed as state. connected still reads the socket itself, so the banner does not flash on every tab switch.
  • Held draft (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: echo held and rm -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):
    • an OPEN socket 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 as disconnected, 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;
    • during a probe, sendInput and sendKeys are refused; a buffered pane-status frame and a pong with another id leave it probing; the matching pong restores input; a second wake sends no second ping;
    • a socket that only delivers stale frames during a probe is dropped;
    • submit, pendingAction(steer) and sendSecret started during a probe send nothing until the pong, then each sends once;
    • a submit already waiting on its snapshot when wake() runs, and a secret, settle as disconnected with nothing sent when the probe drops the socket;
    • a 4008 close after a probe drop still stops output and closes the replacement;
    • a deadline that runs out while the page is frozen pings again, and the pre-freeze pong no longer counts;
    • hiding the page cancels the probe (no drop in 60 s), showing it starts a fresh one;
    • a reconnected socket with no snapshot is probed when the previous snapshot listed ping;
    • a probe drop doubles the next deadline, and a pong resets it;
    • plus the earlier idle-ping, no-flag and CLOSING tests.
    • The 10 new or changed tests fail with the previous ws.ts of this branch (checked by restoring it).
  • scripts/terminal-input-regression.ts: after the terminal becomes ready with a held, Enter and b are typed: the draft reads ab and nothing reaches the pane; Escape still reaches the pane with the draft kept; Send types ab once 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 lists ping; ping gets a pong with the same id, also for an observe role; a malformed id gets invalid_ping; ping and pong pass through MachineRelay. 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.ts passed on the first revision; not rerun after the review fixes.
  • No browser test of a real half-open socket: Playwright cannot hold a socket half-open, so the unit tests cover that part.

Notes

Merge-session review

Fixed (each with a regression test that fails without the fix):

  • Hiding the page answered the probe (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 expects probing and refused input while hidden.
  • Replacing a CLOSING socket left its probe unresolved (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....
  • Ctrl+Enter under modifyOtherKeys got past the held-draft guard (PaneTerminal.tsx): a modified Enter is decided on before its rewrite to CSI 27;5;13~ and refused like a chord, with the held-input note. Test: terminal-input-regression turns on modifyOtherKeys and presses Ctrl+Enter beside a held draft.
  • A paste with a tab went to the pty and a multi-line paste was dropped in silence (PaneTerminal.tsx): while a draft is held, a paste the draft cannot take is refused with the held-input note. Test: paste a\tb and x\ny beside 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).
  • The input line said "disconnected" and its non-empty line got past the draft rule (TerminalInput.tsx, PaneTerminal.tsx): onEnter returns boolean | string and the held-input reason is shown; sendTerminalLine refuses beside a held draft too and keeps the text. Test: empty Enter and ls + Enter on the input line beside a held draft show the held-input note and send nothing.
  • src/AGENTS.md rows 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 inputReady only when a message arrives.

Notes for the merge step:

Checks: bun run check fast pass; scripts/ui-regression.ts (terminal input, Ctrl+Enter, held-draft lanes) pass; scripts/sticky-modifiers-regression.ts pass.

🤖 Generated with Claude Code

devswha and others added 3 commits October 13, 2026 21:31
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>
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1d9d24dd-e714-4095-b4e7-9b2cf98e897f
📥 Commits

Reviewing files that changed from the base of the PR and between 54ad5d1 and e365cb5.

📒 Files selected for processing (15)
  • changelog.d/ws-liveness-probe.md
  • scripts/terminal-input-regression.ts
  • server/index.ts
  • server/ping.contract.test.ts
  • shared/protocol.ts
  • site/demo/transport.ts
  • src/AGENTS.md
  • src/components/PaneTerminal.tsx
  • src/components/TerminalInput.tsx
  • src/lib/i18n.ja.ts
  • src/lib/i18n.ko.ts
  • src/lib/i18n.tr.ts
  • src/lib/i18n.zh.ts
  • src/lib/ws.test.ts
  • src/lib/ws.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devswha and others added 2 commits October 13, 2026 22:22
…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>
devswha and others added 3 commits October 13, 2026 23:19
…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>
Base automatically changed from fix/ws-connect-deadline to main October 13, 2026 15:56
devswha and others added 3 commits October 14, 2026 00:58
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
devswha merged commit 8246e19 into main Oct 13, 2026
17 checks passed
@devswha
devswha deleted the fix/ws-liveness-probe branch October 13, 2026 16:26
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant