[joelmoss]

Continues #154 Phase 1, after #181.

Multiple clients on one session, and who owns the size

The design doc's end state is several clients attached to one session at once — a pane on the Mac and a pane elsewhere on the same shell. A second attach now joins rather than replaces, and both see the same output.

That settles an open thread from #181 differently from how it was proposed: there is no superseded client to notify, because nothing is superseded.

What arbitrates instead is a single size owner. The session follows one client's window; other clients' resizes are recorded but not applied until one of them acts, which claims ownership. Only an attached client advertising a nonzero size may own — a relay forked into a pipe reports 0x0, and letting that own would resize every other viewer to nothing. When the owner detaches the size is unowned; with exactly one client left the pty snaps to its size immediately.

The load-bearing part is deciding what "acts" means, because a terminal answers on the same channel the user types on. vcs/crates/wr-agent/src/input.rs is a parser, not a substring check, and it carries state between calls so a report split across two frames cannot be misread as a keystroke. Excluded from claiming: focus reports, CPR, DSR, DA1/2/3, XTVERSION/XTGETTCAP, OSC colour and clipboard answers, kitty keyboard flags reports (CSI ? n u — one character from a kitty key event), and wheel scroll. A keystroke or a click claims. Same failure family as the replay buffer's query stripping, which the design doc says to recognise before rediscovering it as a bug.

Three gaps against the Swift daemon

Found by working through what SessionDaemonEndToEndTests covers that the agent does not. All three are behaviours the daemon has and the replacement was about to ship without.

A setsid'd grandchild survived being killed. terminate() signalled the pty's child alone. Hanging up a pty reaches its foreground process group, but a child that calls setsid() has left both that group and the shell's session — so nothing reached it, and quitting the app left it running forever. This was a review finding fixed in the daemon (SessionPTY.descendantProcessIDs), and losing it in the port is a regression. The walk is ported: parent-pid links, the one relation setsid cannot escape, via proc_listallpids + PROC_PIDTBSDINFO on macOS and /proc/<pid>/stat on Linux.

The snapshot is taken before anything is signalled, for the reason the daemon's own comment gives: once the shell is hung up its children start exiting, and a descendant already gone from the table is one no later sweep can find. kill/kill_all now do this with the store lock released, and kill_all sweeps once for every session rather than looping — a scan plus up to half a second of waiting per session, under the lock, would have stalled every list and attach on quit.

Exited carried the raw waitpid status, not an exit code. exit 7 arrived as 1792. Both ends already agree on the shell convention — SessionDaemon.exitCode produces it, SessionAttachClient returns it as its own status — so the same frame meant a different number depending on which backend served the session. wr-agent attach also discarded it and always exited 0; it now propagates it, so a command run through an attach is indistinguishable from running it directly.

An oversized session list would have panicked. Frame::encode asserts rather than truncating, so a Sessions reply past the 1 MiB cap killed the connection thread and left the client waiting for a frame that never came. It replies Failure now, which the app already renders.

Pty::write_all shipped unused

Correcting the record on this thread, where I reported the partial-input bug as fixed in 5b1f6166. It was not. write_all was added and unit-tested, but the edit to its call site was overwritten by a later rewrite of serve.rs in the same session, so what landed still called pty.write and dropped the tail of a large paste. The test passed because it drives write_all directly and never goes through the Input path.

Fixed here for real: input routes through Session::write_input, which classifies the bytes for the size-owner policy and then calls pty.write_all. There is no longer a call site that can be missed.

Tests

AgentControlPlaneTests drives AgentControlClient against a real wr-agent serve. The agent's own integration tests are cheaper and cover more, so this covers only what they structurally cannot: the greeting, the envelope and the descriptor list are each encoded in Rust and decoded by hand in Swift, and a disagreement about a field width or a byte order would pass every test on either side alone while presenting to a user as a permanently empty sidebar. Verified by reversing the id bytes in encode_descriptor_list — the Swift suite goes red.

Sessions are created the way the app creates them, by running wr-agent attach with the configuration in its environment. The app never sends an Attach frame — it hands libghostty an attach command to run as the pane's shell — so attach, resize, input and exit stay the Rust suite's to cover.

SessionDaemonEndToEndTests is deliberately kept. The daemon still ships during the drain and shares WorkroomSessionProtocol with the agent, so that is live coverage; both pairs go together in the deletion PR.

kill_reaches_a_setsid_grandchild uses a real perl setsid() grandchild and observes its pid from outside the session — verified to fail, surviving the full 5s wait, with the descendant walk disabled.

A failing integration test leaked its agent, forever. Each test ends with let _ = agent.kill(), which an unwinding panic skips straight past, and a leaked agent never reaches its idle timeout because it is still holding the session's shell. Development runs had left 88 agents and their shells alive on my machine. Spawned is a Drop wrapper, so the kill happens during the unwind; a full run now leaks none.

Verification

• 2595 app tests pass, 0 failed, 1 skipped (read from the .xcresult, not the log tail)
• 111 lib + 11 attach + 8 transport Rust tests, default features
• cargo clippy --all-targets -- -D warnings clean; cargo fmt and swift-format --strict clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01UdxRcKCgPJQxxnTe4aaKK2

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; the remaining bare-Escape size-ownership edge case is non-blocking.

Summary

• Broadcasts session output to every attached client while arbitrating PTY geometry through one active size owner.
• Distinguishes user actions from terminal protocol replies and bounds retained CSI state.
• Terminates detached descendants and guards delayed signaling against PID reuse.
• Propagates shell exit codes through Exited frames and wr-agent attach.
• Adds Rust integration coverage and Swift-to-Rust wire-format tests with automatic process cleanup.

Diagram

sequenceDiagram
    participant C1 as Client 1
    participant A as wr-agent
    participant P as Session PTY
    participant C2 as Client 2

    C1->>A: Attach(size 80x24)
    A->>P: Apply 80x24
    C2->>A: Attach(size 120x40)
    Note over A: Client 1 remains size owner
    C2->>A: Resize 200x50
    Note over A: Record only
    C2->>A: User input
    A->>P: Apply 200x50 and write input
    P-->>A: Output
    A-->>C1: Output
    A-->>C2: Output

Loading

_{Reviews (2) · Last reviewed commit: "fix: address the review findings on #182"}