[dormancygrace]

The serial terminal (GET /api/vm/terminal?port=/dev/tty…) ran picocom in a PTY, and no build shipped picocom: docs/usb-composition.md said a custom static build was bundled, but neither platform/build.sh nor packages.list built or packaged it, so every serial terminal ended with "Unable to start terminal session." (found while testing #76). As decided, the server now bridges the port itself instead of packaging picocom.

Stacked on #76 (its branch is the base, merged in at 9b0c91f): the per-device lease in serial_lease.go there is shared by the console probe and the terminal session, so either keeps the other out in both start orders.

The bridge (serial_session.go)

• serialSettingsFromQuery validates what terminal_command.go validated before: the port pattern, baud (the full Bnnn list), parity (none/even/odd, also n/e/o), flow control (none/hard/soft, also n/h/s/x), data bits 5–8, stop bits 1–2. ttyGS ports stay nominal 9600 8N1 without flow control, as before.
• runSerialSession claims the device's lease, opens it (O_RDWR|O_NOCTTY|O_NONBLOCK|O_CLOEXEC), flock(LOCK_EX|LOCK_NB) and TIOCEXCL against other programs, applies raw termios (cfmakeraw equivalent plus the speed via CBAUD/Ispeed/Ospeed, parity, size, stop bits, CRTSCTS or IXON|IXOFF, VMIN/VTIME 0), flushes, and copies bytes both ways:
  • device → browser: a poll/read loop; every data write to the WebSocket, notices included, goes through one writer with a mutex (gorilla allows one data writer at a time; the pings are control frames and stay outside);
  • browser → device: the WebSocket reader puts messages on a bounded queue (32 messages of at most 64 KiB) and never waits on the tty itself; a writer goroutine drains it with POLLOUT waits in 200 ms slices that end with the session. No write timeout, so a slow line still gets its bytes. The ping goroutine is the watch on the connection's life: a ping to a browser that has gone fails and ends the session, so a line that does not take data (flow control asserted, nothing at the other end) cannot keep the device claimed after the browser closed;
  • the descriptor is closed after every goroutine has returned, and the lease released after that, so a new session cannot open the device while an old goroutine still polls a reused descriptor number.
• Resize JSON (binary) is dropped; the browser's OSC 777 size reply is input, as with picocom. Ping/pong and read limits as the shell terminal.
• Ends with a message in the terminal: the port held by another session ("in use by another session (a terminal or the console test)"), by another program (flock), a missing or unplugged device (EIO/ENXIO/ENODEV, or EOF on the tty).
• No PTY, no child process, no terminal-program key sequences.
• The shell terminal (/bin/sh -l in a PTY) is unchanged.

Removed

scripts/build-picocom.sh, its requirements in the legacy scripts/package.sh and scripts/build-in-container.sh, the row in scripts/README.md, and the docs paragraph about the bundled build. The docs now describe the bridge.

Checked

• go vet, go test -tags teststub ./..., and the package under -race. Tests with a pty as the device: bytes both ways including ANSI and all 256 byte values; resize JSON dropped and the OSC 777 reply delivered; the line settings reach the device (stop bits, speed, raw; the pty driver forces CS8 without parity, so those two are covered by the termios unit test); a 15 KiB paste arrives in order; a second session is refused and the first keeps working; the console probe reports busy while a session is open and not after; a terminal is refused while the probe holds the device and admitted afterwards; a disconnect during traffic in both directions with a blocked 64 KiB browser write (20 runs under -race, this is what panicked before); the browser closing during a blocked write releases the device within the ping interval; the device going away ends the session with a notice; a missing device; the query validation and the termios mapping.
• On the stand (USB serial enabled, before the review changes): a terminal on /dev/ttyGS0 opens and stays (before this change: "Unable to start terminal session"), POST /api/vm/serial-console/test reports busy while it is open and silent after it closes, a second terminal on the same port gets the "in use" notice, and an invalid query is a 400.

Not covered

• A real UART (ttyUSB, ttyS) at its speed: the stand has none attached; the speed and framing settings are checked on a pty and by the termios test.
• The review changes (one writer, the queue, the shared lease) are verified on ptys; the stand run predates them.

🤖 Generated with Claude Code

[dormancygrace]

Review of 56d63f83cb3718a965d0a6ea54208403cea0d608. The existing Go serial/terminal tests pass, but three additional failure paths reproduce locally:

1. [P1] Serialize all WebSocket data writes, including disconnect notices. server/service/vm/serial_session.go:342–345 calls serialNotice from the input loop while the serial-reader goroutine can be in WriteMessage or its own serialNotice (lines 308–324). Gorilla requires a single concurrent data writer. Closing the PTY while a large browser write is blocked produced an actual panic: concurrent write to websocket connection, with the stack through serialNotice:240 and runSerialSession:344. The recovered HTTP-handler panic already aborts cleanup; a panic in the independently spawned reader goroutine would not be covered by HTTP recovery. Route data and notices through one writer, or protect all data writes and write-deadline changes with the same mutex. Add a regression for disconnect during simultaneous traffic and run it with -race.

2. [P2] Make blocked serial writes cancellable. writeAllSerial (lines 221–234) retries EAGAIN/POLLOUT forever with no cancellation or total deadline. It runs inside the WebSocket reader, so a closed browser cannot be observed while output is blocked. Reproduced using the existing serialTestDevice/serialTestSession: keep the PTY master open without reading, send one allowed 64 KiB text message, wait for the tty output queue to fill, then close the WebSocket. The session stays alive and serialPortOpen(device) remains true. This is also relevant to hardware/software flow control. Arrange independent connection-lifetime cancellation and make the write wait honor it; a short arbitrary write timeout alone would break legitimate slow UART transfers. Verify that closing the browser releases the tty even when its peer never drains data.

3. [P2] The probe/terminal exclusion works only in one start order. Adding serialPortOpen to the probe's busy check does not reserve anything. Start probeSerialConsole(device, 500*time.Millisecond), wait until it makes the PTY raw, then call openSerial. It succeeds while the probe is active; when the probe finishes, its deferred restoration puts the active terminal back into canonical mode. Both effects reproduced. flock/TIOCEXCL do not exclude an already-open probe that never acquired the same lease. Share an atomic per-device lease between probes and terminal sessions, held until restoration/close, and test both start orders. This extends the overlapping-probe issue noted on usb: a console helper for headless Linux hosts (script, helper disk, console test) #76.

Reproduction output:

browser closed, but blocked serial write has not ended; port claimed=true
panic: concurrent write to websocket connection
terminal opened the port while probe already held it
probe completion restored canonical mode over the active terminal's raw mode

This PR is stacked on #76, not main. Rebase it on the corrected parent before final integration. No physical-device access or deployment was used for this review.

[dormancygrace]

All three in 486513e, on top of the corrected #76 merged in as 7607a45 (merged rather than rebased, so the branch keeps its history; the base..head diff is the same).

1. One WebSocket writer — every data write and its deadline, notices included, goes through wsWriter with a mutex; the pings are control frames and stay outside. Regression: TestSerialSessionDisconnectDuringTraffic (a 64 KiB browser write the pty does not take, the device side chattering, then the master closed: both ends send their notices at once); 20 runs under -race, clean.

2. Cancellable writes — the WebSocket reader no longer waits on the tty: messages go through a bounded queue (32 × 64 KiB) to a writer goroutine, and writeAllSerial waits for POLLOUT in 200 ms slices that end with the session. The ping goroutine is the connection-lifetime watch: a ping to a browser that has gone fails and ends the session. No write timeout, so a slow line still gets its bytes. The fd is closed after every goroutine has returned, and the lease released after that. Regression: TestSerialSessionReleasesTheDeviceWhenTheBrowserGoesDuringABlockedWrite (your reproduction: master open and not reading, one 64 KiB message, browser closed → session ends and serialPortOpen is false).

3. Shared lease — serial_lease.go from usb: a console helper for headless Linux hosts (script, helper disk, console test) #76 (3d61c10) is the one registry; the probe holds it for its whole run including the termios restoration, the session claims the same. Regression: TestSerialSessionRefusedWhileTheProbeHoldsTheDevice (probe first → the terminal gets the "in use" notice, admitted after the probe ends); the terminal-first order is the existing TestSerialSessionIsExclusiveAndSeenByTheProbe, now with the generic notice.

go vet, the whole package under -race, go build ./.... PTYs only; the devices were not touched, so the stand run in the description predates these changes.

[dormancygrace]

Re-review complete on 934b3bfc24f26dc86ccbe35f26ae640e3e0fb3a4. The previous findings are fixed. The complete vm package passes under -race, including blocked-write cancellation, disconnect during traffic, and probe/terminal exclusion in both orders. All WebSocket data writes now share the writer mutex. Updated the branch with the corrected #76/current-main integration. No device changes. Waiting for CI on the integrated head before merging, as authorized.