Harness driver / PTY attach (drive & view) correctness bugs — consolidated audit

[willwashburn]

relay agent drive and relay agent watch (= agent attach --mode view) misbehave. A code audit across packages/cli (attach clients), packages/harness-driver, and crates/relay-pty + crates/broker found the following verified defects. Every claim below was checked against the actual code paths.

Structural root cause: snapshot ↔ live stream cannot be synchronized

worker_stream events carry no sequence number and are excluded from the replay buffer (crates/broker/src/listen_api.rs:2520), so an attaching client cannot correlate its HTTP snapshot with a stream position:

• Lost output: everything emitted between the snapshot response and the WS subscription is unrecoverable. A gap that cuts a repaint or escape sequence leaves the screen garbled until the next full redraw. (packages/cli/src/cli/lib/attach.ts:61-66 claims the window is "≤10ms" — not true for drive.)
• Duplicated output: the snapshot grid is advanced by the PTY reader before chunks are queued for broadcast (crates/relay-pty/src/pty.rs:328-332), and the 16ms coalescing buffer (crates/broker/src/pty_worker.rs:708-719) is not flushed at snapshot time — bytes can appear in the snapshot and again on the stream.
• Drive widens the gap and injects a repaint into it (packages/cli/src/cli/lib/attach-drive.ts:740-766): order is snapshot → GET /pending → resize agent PTY (SIGWINCH → immediate TUI repaint) → only then WS connect (:619). The user sees a stale snapshot at the old geometry until the agent next outputs. Predictive echo is seeded from the stale snapshot bytes, so early predictions render at wrong positions.
• Watch loses output forever on lag: one shared 512-slot broadcast channel for all events of all workers (crates/broker/src/runtime/init.rs:383); on RecvError::Lagged only durable events are backfilled; clients discard replay_gap frames without re-snapshotting (attach-view.ts:189-202, attach-drive.ts:337-361).

Fix direction: per-worker monotonic offset on worker_stream; include offset in snapshot response; clients subscribe first, snapshot second, drop buffered chunks ≤ snapshot offset; move drive's resize after subscription; re-snapshot on replay_gap.

Drive-specific bugs

1. Pending counter corrupted by full event replay — drive opens /ws with no sinceSeq (attach-drive.ts:453); broker replays up to 1000 historical durable events (listen_api.rs:320-328, 2394-2424; replay_buffer.rs:14). Drive seeds from GET /pending (attach-drive.ts:749) then re-counts replayed delivery_queued/agent_pending_drained (:677-684). Agent with 2 pending / 20 lifetime messages shows pending=22.
2. Status line splices escape sequences and clobbers DECSC — paintStatus runs after every chunk (attach-drive.ts:669-674) emitting ESC 7 … ESC 8 (:415-427). Arbitrary chunk boundaries mean it can splice mid-CSI and overwrite the terminal's single saved-cursor register while the agent has a DECSC pending. Same pattern in attach-passthrough.ts:236-240,481-489.
3. Multi-byte UTF-8 stdin corruption — each stdin chunk decoded independently via Buffer.toString('utf-8') (attach-drive.ts:531, attach-passthrough.ts:353); split characters in large pastes become U+FFFD. Output direction already solves this (crates/relay-pty/src/utf8_stream.rs); the input WS also accepts binary frames (listen_api.rs:1664-1667).
4. Ctrl+C during attach setup strands the worker in manual_flush — mode flipped at attach-drive.ts:722 but SIGINT/SIGTERM handlers installed only inside runDriveSessionLoop (:647-650), after three awaited HTTP round-trips. Passthrough has the inverted twin (worker left in auto_inject, attach-passthrough.ts:249-303).
5. manual_flush doesn't stop worker-side automation — injections already forwarded sit in pending_worker_injections and fire body+\r mid-drive (pty_worker.rs:318,961-1055); try_auto_enter can submit the human's half-typed input (wrap.rs:349-379); prompt auto-responders race the human's keystrokes (pty_worker.rs:741-748).
6. Input acks mean "queued", not "written" — SendInput replies OK once the frame is enqueued to the worker (runtime/api.rs:992-1008; listen_api.rs:1554-1570); actual PTY write failure (pty_worker.rs:535-548) is only a fire-and-forget worker_error. Client rollback path never runs; keystrokes silently lost while reported delivered.
7. No settled-guard on WS message handler — output and reverse-video status repaints keep spraying onto the shell prompt after detach (attach-drive.ts:659-688, attach-passthrough.ts:475-494, predictive-echo.ts:305-309).
8. Delivery-mode save/restore get-then-set race between concurrent sessions; failed initial read force-restores auto_inject, cancelling an explicit hold (attach.ts:216-242).
9. Minor: status-line ANSI written to non-TTY stdout (attach-drive.ts:482-492); stdout backpressure ignored in all three modes.

Rust PTY layer

10. Blocking write_all inside the tokio select loop with a real deadlock cycle — relay-pty/src/pty.rs:364-382 blocks on a 128-slot std channel + ack, called without spawn_blocking from the worker's single task on every drive keystroke and injection (pty_worker.rs:537,977-1024, wrap.rs:374). Child floods output while not reading stdin → kernel PTY buffer fills → drainer wedges → queue fills → select loop blocks → pty_rx (cap 256) fills → reader blocks in blocking_send (pty.rs:332) → permanent hang; all attach clients freeze; watchdog dead.
11. Sleeps inside select arms (100–120ms: pty_worker.rs:970,986,994,1023; wrap.rs:338) stall streaming and keystroke processing — visible drive/watch jank.
12. Snapshot omits terminal modes — snapshot.rs:148-203 re-emits cells/SGR/cursor position only; drops alt-screen, DECTCEM, DECCKM, bracketed paste, mouse reporting, wrap mode, scroll region, saved cursor. After attach, arrow keys send wrong sequences, cursor visibility wrong.
13. Resize is last-writer-wins with no ownership (runtime/api.rs:1035-1083); view never resizes and paints CUP coordinates that garble on smaller terminals.
14. Stateless-per-chunk strip_ansi / suggestion detectors mis-parse sequences split across 4KB reads (ansi.rs:18-69; wrap.rs:381-387; terminal.rs:140-144) — affects injection readiness/timing.
15. No-PID watchdog can declare a silently-thinking child dead after ~30s (pty.rs:491-606).

harness-driver package

16. Stale WS close handler clobbers reconnects — transport.ts:532-539 mutates instance state without checking the closing socket is current; disconnect→reconnect leaves two live sockets feeding the same listeners (every event duplicated), a leaked socket, and a spurious third connection.
17. Events-WS message handler uses data.toString() instead of the existing rawDataToString (transport.ts:507-529 vs :381-389) — fragmented/binary frames silently swallowed. Reconnect loop has no backoff cap (:536-538).
18. Headless workers lose newlines — runtime/headless.rs:246-280 strips terminators via next_line() and emits stripped lines as chunk; node tail concatenates all output onto one line.
19. Latent protocol drift: typed BrokerEvent::Delivery* variants serialize agent w/o event_id (crates/broker/src/protocol.rs:419-434), mismatching protocol.ts:283-362. Dead code today; first use would be silently dropped by clients.
20. Minor: unbounded subscribeWorkerStream queue (client.ts:716-741); input_serializers map never pruned (listen_api.rs:1602-1616); predictive echo suppresses first prediction at prompts ending in a space (predictive-echo.ts:263-266).

Verified clean (ruled out)

utf8_stream.rs output decoding; queue.rs/inject.rs retry & priority logic; write-queue FIFO ordering vs injection \r; EOF/exit ordering in pty_worker.rs:908-946; PtyInputStream FIFO ack correlation; predictive-echo server-chunk ordering; durable-event replay dedup/TOCTOU in handle_dashboard_ws; enum/wire-string spellings between protocol.ts and emitted Rust JSON.

Suggested fix priority

1. Stream cursor + subscribe-first attach (fixes lost/dup output for both modes) + drive resize-after-subscribe + WS sinceSeq cutoff (fixes pending counter)
2. Blocking write_all → async ack; remove sleeps from select arms
3. Signal handlers before mode flip; UTF-8-safe stdin forwarding (binary frames)
4. Snapshot terminal modes
5. Transport stale-close-handler guard; headless newline preservation; protocol type alignment

[willwashburn]

Fixes for all items are on branch claude/harness-pty-abstraction-bugs-o2alrk:

• 6a4db09 — per-worker stream offsets, offset-bearing snapshots, subscribe-first attach clients, drive resize-after-subscribe, sinceSeq cutoff + pending-counter fix (structural root cause, drive item 1)
• 4a77520 — non-blocking PTY writes (submit_write + oneshot ack, removes the deadlock cycle), deadline-driven injection pacing (items 10, 11)
• 4ff6854 — PTY input acked only after confirmed write with failure propagation + timeout; set_interactive_hold frame pauses injections/auto-enter/auto-responders during drive (items 5, 6)
• e2792f3 — signal handlers before delivery-mode flip, streaming UTF-8 stdin decode, post-teardown output guards, ANSI-boundary-held + coalesced status repaints, non-TTY guard, detach-safe mode restore, bounded stdout backpressure (items 2, 3, 4, 7, 8, 9)
• 1c08f61 — snapshots capture/replay terminal modes (alt-screen, DECTCEM, DECCKM, bracketed paste, mouse, autowrap) + conservative detach terminal reset (item 12; scroll region reset-to-full documented in-code)
• f9da582 — transport stale-close-handler guard, rawDataToString on events path, reconnect backoff, headless newline preservation, Delivery* protocol alignment, bounded stream queues, input_serializers pruning (items 16–20)

Not addressed (lower priority / by design, still open): resize ownership across multiple attach clients (13), stateless per-chunk strip_ansi/suggestion detection (14), no-PID watchdog heuristic (15).

Verification on the combined branch: cargo test -p relay-pty 182 passed; cargo test -p agent-relay-broker --lib 679 passed; clippy clean; vitest harness-driver + cli 648 passed; tsc --noEmit clean.

---

Generated by Claude Code