Skip to content

Publish terminal exit at once, and survive feedback that crosses a close - #48

Open
felixhromadko wants to merge 3 commits into
mainfrom
fix/terminal-exit-publish
Open

felixhromadko wants to merge 3 commits into
mainfrom
fix/terminal-exit-publish

Conversation

@felixhromadko

@felixhromadko felixhromadko commented Oct 6, 2026 •

Copy link
Copy Markdown

A short command run through a YAS PTY took about 25 ms longer than the command itself. The server published the exit only after the view's first display frame, and that frame waits out the 20 ms INITIAL_FRAME_GRACE. This PR publishes the exit as soon as the output is drained. It also fixes two races that the faster exit makes more frequent.

Commits

  1. Publish terminal exit state without waiting for the display frame. cleanup_pty_internal bumps a retained watch after the output drain and exit-status collection. Each Terminal connection subscribes before its initial snapshot, then refreshes and publishes the catalogue when the watch changes. Display frames keep their own schedule.
  2. Accept terminal feedback that crosses a view's removal. A client can ACK a frame, or send input, after the server has removed the view (terminal closed, view closed, or catalogue retired). The server rejected all feedback for unknown views, and the reliable loop then dropped the whole connection (reliable frame handler failed: family=16 kind=34). Removed views now leave a bounded record (256) of their acked and sent sequences. Late feedback is accepted and its input dropped, as Surface already does. Feedback for an unsent frame or an unknown view still ends the session. The spec is updated (docs/design/yas.md, Frame feedback).
  3. core: handle SET_FOCUS for a view that already closed. When a session closes, workspace focus falls back to another one, and that one can be closing too. The fire-and-forget SET_FOCUS then rejects with NotFound, and nothing handles the rejection, which kills a Node embedder.

Commit 1 on its own made the disconnect in commit 2 about 3× more frequent under parallel load (7 → 24 drops in 2,400 commands on a v0.3.0 backport). Without commit 2, it is not safe to merge.

Latency

Local Unix socket on an 8-CPU Linux sandbox, 500 runs plus 5 warmups, medians. Commands run through neo's real BlitShellClient, with the published @yas-run/core 0.3.0.

Command main this PR
BlitShellClient.run("uptime") 27.5 ms 12.5 ms
bash -c uptime 29.8 ms 6.6 ms
bash -lc uptime 30.7 ms 12.7 ms

The exit wait drops from ~25 ms to the command's own run time. On the same box, bash -lc uptime without a PTY takes 11–13 ms.

Stress: output capture and disconnects

20 command shapes, 30 rounds, ~1,200 commands per run. A third of the runs go 8 at a time on one connection. The shapes include no output, no trailing newline, 99/100/101 lines (tail boundary), 3,000 and 5,000 lines, a 20,000-char line, Unicode, stderr, exit 3, SIGKILL, CR overwrite, colors, alt screen, a background child holding the PTY, and output written right before exit. Each result is compared byte for byte with a golden capture.

main this PR
Output mismatches (≈9,600 commands) 0 0
Connections dropped by a late FRAME_ACK (4 runs) 18 0
Unhandled SDK rejections (fork SDK, 2 runs) 4 0

The runs use both SDKs (0.3.0 and this branch's js/core), with and without a second client that opens a view on every terminal, like a browser panel. The fixes also hold on a v0.3.0 backport through neo's path: 0 drops in 2,400 commands.

Display

  • Decoded grid check: 300 terminals each on main and this PR. Both the owner's and a second viewer's decoded grids always show the final line (0 failures). With this PR, the owner sees the exit first. Its grid catches up within ~13 ms (p50), max 23 ms.
  • Playwright e2e: same result as main, 93 passed. uplink.spec fails in both because the uplink fixture is not built here. reconnect.spec is flaky on main as well: in a 3× repeat of reconnect and terminal specs, main failed 1 of 33 and this PR passed 33 of 33.
  • A shell that prints and exits at once still shows its full output in the UI:

Exited pane shows full output

Checks

  • cargo test -p yas-server --lib: 944 passed. This includes the new terminal_exit_update_follows_drained_output_once and terminal_feedback_crossing_close_is_ignored_but_unsent_frames_are_rejected.
  • cargo clippy -p yas-server --all-targets -D warnings and cargo fmt --check are clean.
  • js/core vitest: 1,528 passed. The new SET_FOCUS test fails without the fix. tsc --noEmit is clean.

Not covered: macOS, which has a separate ~95 ms fd-close cost in close_fds_except.

View in Indent
Tag @indent to continue the conversation here.

A PTY's exit reached clients only through the terminal catalogue, which a
connection refreshed after writing a view's final frame or on its 100 ms
poll. The first frame waits out the 20 ms initial grace, so every short
command looked about 20 ms slower than it ran.

cleanup_pty_internal now bumps a retained watch after the output is drained
and the exit status collected. Every Terminal connection subscribes before
its initial snapshot and republishes the catalogue when it changes. Display
frames keep their own schedule.
A client can present and acknowledge a frame, or type into a view, after the
server has already removed that view: the client closed the terminal, closed
the view, or the catalogue retired it. The server rejected feedback for any
unknown view, and the reliable event loop treats that as a connection failure,
so a valid late FRAME_ACK dropped the whole connection with every request in
flight.

Removed terminal views now leave a bounded record of their last acknowledged
and last sent sequence. Late feedback for one is accepted and its input
dropped, as Surface already does for retired views; feedback naming an unsent
frame or an unknown view still ends the session. Publishing exit state ahead
of the final frame makes this race much more frequent, because callers close a
finished terminal while its last frame is still in flight.
Workspace focus falls back to another session whenever one closes, and that
session can close in the same moment. SET_FOCUS then fails with NotFound, and
because the request was fire-and-forget the rejection went unhandled, which
terminates a Node embedder.
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Coverage

Crate Lines Functions Regions
alacritty-driver 85.6% (1081/1263) 89.4% (84/94) 89.3% (1746/1955)
browser 25.5% (309/1212) 30.4% (34/112) 27.7% (605/2183)
cli 32.4% (6661/20544) 33.6% (615/1831) 33.4% (9793/29330)
client 67.0% (4974/7425) 68.1% (627/921) 65.1% (6430/9873)
composite-transport 96.3% (526/546) 98.4% (60/61) 96.2% (884/919)
compositor 54.9% (11406/20763) 66.1% (833/1261) 54.8% (15797/28844)
desktop 78.4% (4460/5691) 71.6% (393/549) 75.1% (6211/8267)
edge 62.9% (798/1268) 50.9% (82/161) 57.7% (1038/1799)
fonts 77.3% (1257/1626) 82.7% (129/156) 78.9% (2424/3071)
fssync 86.0% (1609/1872) 85.8% (182/212) 87.0% (2833/3255)
git 70.4% (4462/6334) 68.5% (337/492) 67.3% (6313/9386)
guest 67.3% (6829/10148) 67.0% (488/728) 66.6% (8935/13416)
lsp 78.7% (3573/4542) 80.8% (336/416) 76.7% (5408/7054)
proxy 63.3% (1899/3000) 56.6% (184/325) 63.5% (2929/4613)
runtime-dir 93.6% (117/125) 100.0% (14/14) 94.8% (218/230)
sd-notify 73.9% (68/92) 100.0% (6/6) 83.2% (109/131)
server 70.8% (84676/119569) 73.3% (6144/8379) 68.8% (113354/164720)
ssh 67.2% (708/1054) 75.9% (85/112) 67.2% (1105/1645)
terminal-model 49.9% (314/629) 62.3% (38/61) 50.3% (505/1004)
uplink 94.2% (582/618) 92.6% (50/54) 93.1% (1062/1141)
webrtc-forwarder 33.6% (1321/3932) 44.9% (146/325) 36.0% (2279/6336)
webserver 80.7% (1490/1846) 81.1% (193/238) 83.2% (2551/3067)
website 35.1% (355/1012) 34.4% (53/154) 35.8% (607/1694)
xtask 0.0% (0/5149) 0.0% (0/131) 0.0% (0/9157)
yas 88.9% (28029/31522) 94.0% (2148/2286) 84.0% (44028/52388)
Total 66.5% (167504/251782) 69.5% (13261/19079) 64.9% (237164/365478)

@felixhromadko
felixhromadko requested a review from pcarrier October 9, 2026 11:09
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.

2 participants