fix(wr-agent): replay the busy state to a reattaching client (#359)

[joelmoss]

Fixes #359

Summary

The busy indicator now survives a reattach. A program reports busy with OSC 9;4 once, at the start of its work, so a pane reattaching mid-turn (an app relaunch, local or remote) always showed idle. wr-agent's shadow terminal now registers GHOSTTY_TERMINAL_OPT_PROGRESS_REPORT, keeps the last report, and the replay re-emits it after the screen and before the parser continuation. The app's existing 9;4 path then restores the spinner and its safety timer; no Swift change.

• Explicit idle. When no report is held, the replay sends a REMOVE (ESC ] 9;4;0 ST), so a client that kept its view across a reconnect drops a busy state the program has since cleared. A fresh client treats it as a no-op.
• Never on disk. record() (the --screens record) carries no 9;4: a record outlives its program, so a restored pane is painted idle.
• A killed program doesn't pin the spinner. A program killed or crashed mid-turn never sends its REMOVE, and the shadow can't see the shell's next prompt (OSC 133 D) the way the app does. So the shadow notes the pty's foreground process group when a report arrives, and a live attach or a hand-off freeze replays idle while another group owns the pty. The report is kept, so ^Z then fg is busy again to the next client.
• Soundness. The callback's userdata is a Box::into_raw slot freed in Drop after ghostty_terminal_free, so a move of ShadowTerminal never aliases the pointer the terminal holds.

Known limits, documented in code and docs/designs/remote-workrooms.md:

• The owner is sampled after the read, so a reporter that exits within the same read chunk is recorded as the shell and its report stays until a REMOVE (ponytail: comment in shadow.rs).
• The hand-off table carries no owner, so a session handed off while its reporter is stopped (^Z) comes back idle until the reporter's next report.
• A hand-off from a pre-fix agent replays one REMOVE for a turn still running (the old shadow never tracked the report).
• App-side, not changed here: the app counts ERROR (2) and PAUSE (4) as working, so a replayed ERROR keeps a reattaching pane spinning until the safety timer.

Test Coverage

Tests: 29 → 29 test files (new tests are in-source #[cfg(test)] modules and an existing integration file)
Coverage: 83% value-weighted at the gate (15/18 paths); the later review cycles added owner-tracking tests on top.
Test value: 11 tests written, 0 rejected by the authoring gate, 0 existing tests extended, 3 paths weakly covered (two unreachable FFI guards, rejected as needing a seam; clone_via_snapshot's untracked slot).

New tests, each shown to fail when the code it guards is broken:

• a_reattaching_client_is_told_the_session_is_busy (integration, real agent + socket): red before the fix.
• a_reattaching_client_is_not_told_a_cleared_report: red when REMOVE stops clearing, or when idle sends no REMOVE.
• only_the_replay_carries_the_progress_report (table, incl. percent 0 and the explicit REMOVE): red when the report moves into record() or idle sends nothing.
• the_progress_report_does_not_split_the_continuation: red when the report follows the continuation.
• a_restored_pane_is_never_told_the_session_was_busy (real flush → Screens::render): red when the writer uses replay().
• an_adopted_session_replays_the_progress_report_it_was_frozen_with: hand-off round trip.
• a_report_from_a_program_that_has_gone_is_not_replayed, an_adopted_report_is_dropped_once_its_sender_goes, a_hand_off_after_the_reporter_has_gone_is_frozen_idle (real ptys, set -m, file-gated reporter): red when the foreground check, adopt's owner note, or the live freeze is removed.
• the_progress_report_follows_the_program_that_sent_it, a_new_report_belongs_to_its_own_sender (closure-driven, no pty): red when the owner comparison inverts or the owner is noted only once.

Pre-Landing Review

Three review cycles (the skill's cap) with testing, maintainability, security, performance, simplification and Red Team specialists, then the adversarial pass. All findings were INFORMATIONAL; none critical.

• Cycle 1: Red Team found a crashed agent would replay stale busy, and the replay could never say idle. Fixed (owner tracking; explicit REMOVE). Security found Box aliasing under Stacked Borrows. Fixed. Plus stale docs and a fixed-sleep test.
• Cycle 2: the owner drop mutated state on a read path and lost the report across ^Z/fg. Fixed (read-only replay_for). Folded write + owner note into write_from_pty; new closure-driven and adopt tests.
• Cycle 3: attach path not directly tested (now one shared live_replay helper), a test that could not fail (replaced), hand-off docs. Continued past the cap on the maintainer's explicit approval, on tests + mutation checks rather than a fourth full review.
• Adversarial (Claude): the hand-off freezing the full replay would hand a dead reporter's report to the shell. Fixed (freeze live_replay) with a mutation-verified test. Other items are the documented limits above.
• Advisory, skipped: the one-line has test helper is repeated across three test modules.

Outside review (Codex): structured review completed, gate PASS (two P2s, both the documented owner-sampling and hand-off-owner limits). Codex adversarial challenge: unavailable (timed out after 9 minutes; missing coverage, not a pass).

Review binding note: the last fixes landed after the final full review pass, under the approval above.

Exploratory QA

Not run as a separate pass: this is an agent-internal change with no UI. Behaviour was exercised through the real agent binary over a socket and through real ptys in the tests above. Reset behaviour was probed directly: a full reset (ESC c) clears the stored report; alt-screen switches and DECSTR keep it, as the app's live Ghostty does.

Design Review

No frontend files changed — design review skipped.

Eval Results

No prompt-related files changed — evals skipped.

Scope Drift

Scope Check: CLEAN against #359 (all three Fix items and both Tests items). Added beyond the issue, on the maintainer's choice: the explicit idle REMOVE and the stale-owner drop.

Plan Completion

Plan completion audit: not run (no plan is bound to this branch and no docs/designs/ file matches). Fix: add "Plan: " to the PR body, or run /autoplan.

TODOS

No TODO items completed in this PR.

Documentation

Status: updated. One authored doc was corrected and extended to match #359 (wr-agent replays the last OSC 9;4 progress report, or an explicit REMOVE when idle, to reattaching clients).

Scope: audited the 6 unstaged wr-agent files (handoff.rs, screens.rs, session.rs, shadow.rs, terminal.rs, tests/attach_survives_detach.rs) against base 5d5f033 (no committed, staged or untracked content), plus the discovered docs (README, CONTRIBUTING, AGENTS, macapp/AGENTS, docs/agents/, docs/designs/, docs/repository-notes.md).

Documentation health:

• docs/designs/remote-workrooms.md: updated. Hand-off screen item now says the outgoing agent sends the replay a reattaching client would get (Shadow::replay_for), so the busy state crosses a hand-off. New paragraph under Terminal-state durability describes the busy state on reattach: re-emitted report or explicit REMOVE, report before the parser continuation, idle once the sending process group no longer owns the pty (live attach and hand-off freeze), report kept for ^Z/fg, the sample-after-read limit and the two integration tests. The Stop-and-reboot screen restoration #232 record bullet now says record() also omits the progress report and a restored pane is painted idle.
• docs/designs/oq8-cross-machine-reattach.md: current. The 'Live display state' row is still accurate for disk persistence.
• docs/repository-notes.md, CONTRIBUTING.md, README.md, AGENTS.md, macapp/AGENTS.md: current.
• Other designs and agent docs: current, no relevant mentions.

Coverage debt: reference coverage for the new behaviour lives only in the design doc, which is appropriate for an agent-internal behaviour with no new flags, wire fields or user-facing surface. No how-to or tutorial gaps. Minor wording debt: Success Criteria and the Distribution Plan still describe the on-disk record as replay()'s VT bytes; the #232 bullet now holds the precise definition.

Diagram drift: none; the audited docs contain no diagrams naming the changed entities.

Test plan

• WR_AGENT_HAS_TERMINAL_STATE=1 cargo test --manifest-path vcs/Cargo.toml -p wr-agent --features terminal-state: 514 passed, 0 failed (6 ignored, pre-existing)
• cargo test --manifest-path vcs/Cargo.toml -p wr-agent: 479 passed, 0 failed
• cargo clippy -p wr-agent --all-targets -- -D warnings, with and without --features terminal-state: clean
• cargo fmt -p wr-agent -- --check: clean
• Not run here: a live app relaunch mid-turn against a real Claude Code session (the integration test drives the real agent binary instead)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-07T07:49:16.562405Z	42fa384	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[Copilot]

Copilot review overview

🟡 Changes recommended

The core reattachment integration test finishes its reporting command before detaching, so it does not exercise the stated mid-turn regression.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Test lacks a live mid-turn reattach scenario · New
• Correct test contract for OSC 9;4 removal · New
• Update comment to reference the live replay path · New

What changed in this PR

Replays OSC 9;4 progress state when clients reattach to live wr-agent sessions.

Changes:

• Tracks and replays progress reports with foreground-process ownership.
• Excludes progress state from persisted screens.
• Adds reattach, hand-off, ownership, and restoration tests and documentation.

File	Description
vcs/​crates/​wr-agent/​src/​terminal.rs	Captures and replays progress reports.
vcs/​crates/​wr-agent/​src/​shadow.rs	Tracks report ownership by process group.
vcs/​crates/​wr-agent/​src/​session.rs	Applies live replay during attach and hand-off.
vcs/​crates/​wr-agent/​src/​screens.rs	Keeps persisted screens idle.
vcs/​crates/​wr-agent/​src/​handoff.rs	Updates hand-off documentation.
vcs/​crates/​wr-agent/​tests/​attach_survives_detach.rs	Adds reattachment integration coverage.
docs/​designs/​remote-workrooms.md	Documents progress-state durability.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[joelmoss]

🤖 Left by Claude Code: Fixed in 42fa384. The reporter is now a foreground /bin/sh child that is still running (sleep 30) when the client reattaches, as in the issue repro, so the replay is busy because its live sender owns the pty. Confirmed it goes red when the owner check in Shadow::replay_for is inverted.

[joelmoss]

🤖 Left by Claude Code: Fixed in 42fa384. The doc and value card now say the restored repaint carries no busy report (a REMOVE at most); the assertion already checks only for ESC]9;4;3.

[joelmoss]

🤖 Left by Claude Code: Fixed in 42fa384. The doc and value card now name the live replay (live_replay -> Shadow::replay_for) that frozen() captures.