fix(cli): interactive terminal is dead — the server aborts every WebSocket upgrade task at handshake

[zackees]

Context

The interactive terminal shipped in #241 is non-functional. Every /terminal/ws connection completes its HTTP handshake and then closes immediately, with no frames and no WebSocket close frame, so the browser dialog goes straight from Connected — interactive shell to Disconnected. Restart opens a new shell....

This is not a partial failure or an edge case — no terminal session can be established at all, for any user, in any host. It reproduces against main with no local modifications.

The regression arrived with the kernal-api migration (d5afb46, #242). The terminal landed in 22ea046 against direct axum + tokio-tungstenite, and docs/interactive-terminal.md records a passing Chromium/WebKit browser validation on 2026-09-12 — before the migration moved the upgrade onto the kernal_api::http_server facade. Nothing has exercised a live session since.

Evidence

Raw WebSocket probe against an unmodified build. Server started via fastled --internal-serve-dir-headless <dir>; a Python client connects with the correct Origin/Host:

WS OPEN
WS ERROR: ConnectionClosedError no close frame received or sent

Zero frames received. The handshake succeeds; the connection then dies.

Not caused by the caller. Forcing the PTY spawn to fail with SHELL=/nonexistent/shell produces the same result, which is the diagnostic detail: terminal.rs sends {"error":"Terminal failed"} on a spawn failure, and even that frame never arrives. The application callback is not running at all.

Not the working tree. git stash of all local changes, rebuild, same failure. The browser fixture fails identically on pristine main.

Blast radius. /terminal/ws is the only WebSocket route, so the terminal is the sole victim — but it is fully broken rather than degraded.

Root cause

kernal-api v0.1.7 (6ca1c16), src/http_server.rs:

520:  .with_upgrades();                                    // serve_connection -> with_upgrades
522:  let mut upgrade_tasks = tokio::task::JoinSet::new();
524:  let outcome = tokio::time::timeout(limits.connection_timeout, async {
526:      loop {
527:          tokio::select! {
528:              result = &mut connection => break result,
529:              Some(task) = upgrades_rx.recv() => { upgrade_tasks.spawn(task); }
...
539:  upgrade_tasks.abort_all();

With with_upgrades(), hyper's connection future resolves as soon as the 101 Switching Protocols response has been written — the socket is handed off, and driving the connection is over. Line 528 therefore breaks the loop the instant an upgrade succeeds, and line 539 aborts every task in the JoinSet unconditionally. Any task not yet drained from upgrades_rx is dropped with the receiver instead.

The upgrade callback is cancelled microseconds after the handshake, on every connection. abort_all() reads as intended cleanup for the timeout/error paths, but it fires on the success path too.

Why the upstream test suite misses it. websocket.rs:336 — server_emits_a_real_switching_protocols_response — passes |_socket| async {} as the callback and asserts only the 101 response bytes. A callback that does nothing observable cannot detect a callback that never runs. The only other upgrade test (upgrade_rejects_non_websocket_before_claiming_transport) covers the rejection path.

Proposal

1. Fix upstream in kernal-api. On a successful upgrade the connection future resolving is the expected transition, not a termination condition: it must not tear down the upgrade tasks. The correct shape is to drain the JoinSet to completion (or keep serving until all upgrade tasks finish) rather than abort_all() on the normal path. Aborting is only right for the timeout and error arms.
2. Pin the fixed tag in this repo (Cargo.toml:16).
3. Add a regression test here that would have caught it, because the upstream test cannot: a Rust integration test that connects a real WebSocket client to a running server::start_server and asserts a server→client frame is received. Asserting the 101 is not sufficient — that is the exact blind spot that let this ship.
4. Correct the record in docs/interactive-terminal.md: the 2026-09-12 validation predates the migration and is currently misleading as evidence that the terminal works.

Acceptance criteria

1. RED first. A new Rust test opens a real WebSocket to the server and asserts a frame arrives from the PTY (e.g. the shell's prompt, or a marker from an injected command). Written against current main, this must fail — reproducing the silent close — before the fix.
2. After the fix, that test passes and the browser fixture passes: FASTLED_TERMINAL_BINARY=... FASTLED_ESBUILD=... uv run --with playwright pytest tests/frontend/test_terminal.py -v -s (Chromium and WebKit).
3. A terminal session survives more than the handshake in a real browser: the dialog reaches Connected — interactive shell, a typed command produces output, and the session stays up until explicitly closed.
4. soldr cargo test --workspace, bash lint, and bash test pass.
5. The failure mode is no longer silent: if the upgrade callback cannot run, the client sees an error rather than a connection that closes with no close frame. A dead terminal must not look like a normal disconnect.
6. Safari remains a required target: no JSPI flags, WebAssembly.Suspending, or WebAssembly.promising.

Decisions

• Priority P0, not P2. The feat(terminal) feature from feat: add interactive PTY terminal with launch cwd and clud access #241 does not function; this is a total feature outage, not a rough edge.
• Filed here rather than in zackees/kernal-api, because this repo owns the user-visible regression, the reproduce commands, and the validation record. The code fix belongs upstream and this issue should track it.
• The upstream test is not the fix. Passing |_socket| async {} and asserting 101 would go green without the terminal working, so the acceptance criteria are anchored on an observed frame from a real PTY.
• No local vendor/patch of kernal-api as the primary fix — this repo's convention is native, fail-loud behavior and no shims. If an interim unblock is needed before an upstream release, that should be an explicit, separately-decided stopgap rather than the default path. Raised in Open questions.
• The docs/interactive-terminal.md validation record is left in place but must be annotated rather than deleted — it is accurate for the commit it names, and misleading only by omission of the migration that followed.

Open questions

• Interim unblock. v0.1.7 is the newest tag (git ls-remote --tags confirms), so there is no fixed version to pin today. Do we wait on an upstream release, or take a temporary patch/pin (a [patch] override or a fork) so the terminal is not dead for the duration? I have not assumed either.
• Scope of the fix. Whether the upstream change is a narrow reorder (don't abort_all() on the success path) or a restructure of the connection-task lifecycle. This determines whether it is a point release or a larger change.
• Other consumers. http_server's WebSocket facade is generic; any other service on kernal-api with a WebSocket route has the same defect. Worth fixing upstream regardless of who reports it next.

Related issues

• feat(terminal): launch an agent in the browser terminal and preserve its session #254 — feat(terminal): launch an agent in the browser terminal and preserve its session (open). Both of its phases assume a working transport; this blocks them.
• feat: add xterm.js terminal to CLI web app #240 — feat: add xterm.js terminal to CLI web app (closed). The feature this breaks.
• zackees/kernal-api#219 — feat(terminal): own PTY-backed WebSocket sessions for rebased app servers (closed). Introduced the facade path that carries the defect.

[zackees]

Fixed upstream and verified here.

Root cause confirmed as described. kernal-api's serving loop treated hyper's connection future resolving as the end of the connection. With with_upgrades() that future resolves as soon as the 101 is written, so upgrade_tasks.abort_all() cancelled every upgrade task microseconds after a successful handshake.

Upstream fix: zackees/kernal-api PR #289, released as v0.1.8. The connection budget now bounds the handshake only; pending upgrade tasks are drained and joined to completion, and an upgraded connection's lifetime belongs to the handler that received it. Two tests cover what the old coverage could not — the existing test's callback was |_socket| async {}, so a callback that never ran looked identical to one that did.

Here: this repo now pins v0.1.10 (Cargo.toml:16, crates/fastled-cli/Cargo.toml:33).

Verified on the real transport, not just the unit tests. A direct WebSocket probe against the built CLI now receives PTY frames:

WS OPEN
FRAME: b'\x1b[?2004h'
FRAME: b'\r\n\x1b[1;36m...served project\x1b[0m via ...\r\n'

and test_terminal_240_interactive_browser passes on Chromium, including the 120 KB paste, resize, copy, hide/reopen, and restart.

Two further defects surfaced once the transport worked; both are separate:

• The shell-exit control frame was malformed JSON ({\"exit\":0} — a raw string spells \" literally), so every shell exit raised an uncaught JSON.parse exception in the page. Fixed here with a Rust test on the frame.
• fix(terminal): a write parked on a full PTY input queue holds its slot until the foreground program exits #256 — a write parked on a full PTY input queue holds its slot until the foreground program exits.

CLOSING: the reported failure no longer reproduces and the terminal is functional.