fix(tui): join local PTY reaper during teardown

[lawrencecchen]

Summary

• retain the successful local PTY child reaper JoinHandle in the surface runtime
• join the reaper and reader against one absolute shutdown deadline
• add a lifecycle test proving teardown consumes the owned reaper handle

The startup-failure cleanup in #11414 and terminal-host guard work in #11425 are separate.

Validation

• rustfmt --edition 2024
• git diff --check
• Rust tests deferred to hosted verification per cmux-tui instructions.

---

Summary by cubic

Local PTY teardown previously left the child reaper unmanaged; it now owns and joins the reaper alongside the reader against one absolute shutdown deadline. If the reaper misses that deadline, shutdown remains bounded while its handle stays owned for later cleanup.

• Resets the reaper completion fence before each spawn so stale completions cannot skip a join.
• Skips self-joins and logs reaper timeout or panic outcomes.
• Adds unit and production PTY coverage for successful joins, timeout retention, and self-join teardown.

^{Written for commit 4019f02. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved terminal shutdown reliability by tracking and joining background terminal processes within the shutdown deadline.
  • Prevented shutdown from waiting indefinitely when a background process does not complete, while retaining it for continued cleanup.
  • Ensured failed terminal startup cleans up associated resources and avoids shutdown deadlocks, including self-join scenarios.
  • Added coverage for normal completion, timeout handling, startup failures, and production shutdown behavior.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
cmux166	Canceled		Sep 3, 2026 12:24pm UTC
cmux41	Canceled		Sep 3, 2026 12:24pm UTC

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: e66d5d6d-d010-4206-b443-c0885f4eb0bc

📥 Commits

Reviewing files that changed from the base of the PR and between 93ec66a and 0bd37b1.

📒 Files selected for processing (1)

• cmux-tui/crates/cmux-tui-core/src/surface.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

The PTY runtime now tracks child reaper threads with completion fences. Terminal shutdown joins the reaper within the shared reader deadline and retains timed-out handles. Tests cover successful joins, timeout retention, self-join avoidance, and production reaper cleanup.

PTY reaper lifecycle

Layer / File(s)	Summary
Reaper tracking and spawn cmux-tui/crates/cmux-tui-core/src/surface.rs	PtyTerminalRuntime stores the reaper join handle and completion fence. Local, hosted, exited-terminal, and test runtimes initialize these fields. The local child reaper uses a tracked thread and completion guard. Spawn failures close the PTY master.
Bounded reaper shutdown and validation cmux-tui/crates/cmux-tui-core/src/surface.rs	finish_terminal_reader joins the reaper within the shared deadline. It avoids self-joins, handles panics, and restores timed-out handles. Test helpers and synchronization tests cover these paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 0bd37

The change makes local PTY shutdown more reliably clean up its child-reaper thread, but overlapping shutdown calls can defer cleanup of a timed-out reaper until a later teardown or object drop. The PR is mergeable with owner awareness of this bounded concurrency risk.

Sequence Diagram(s)

sequenceDiagram
  participant Surface
  participant PtyTerminalRuntime
  participant ReaperThread
  participant ChildProcess
  Surface->>PtyTerminalRuntime: finish terminal reader
  PtyTerminalRuntime->>ReaperThread: join within shared deadline
  ReaperThread->>ChildProcess: wait for child exit
  ReaperThread-->>PtyTerminalRuntime: signal completion
  PtyTerminalRuntime-->>Surface: complete shutdown or retain handle

Loading

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the change, motivation, and validation commands, but it omits the required Demo Video, Review Trigger, and Checklist sections. It also uses “Validation” instead of the templat…	Add the required Demo Video, Review Trigger, and Checklist sections. Rename or include the Testing section, document hosted test verification or its result, and complete all applicable checklist items.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: joining the local PTY reaper during teardown.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	PASS. The PR diff changes only three Rust files: mux.rs, surface.rs, and remote.rs. It introduces no Swift changes, so the Swift 6 actor-isolation failure conditions do not apply.
Cmux Swift Blocking Runtime	✅ Passed	PASS: The pull-request diff contains only three Rust files (mux.rs, surface.rs, and session/remote.rs). It contains no Swift changes and therefore introduces no production Swift blocking or timi…
Cmux Browser Automation Off-Main	✅ Passed	PASS: The PR changes only three Rust TUI files: cmux-tui-core/src/mux.rs, cmux-tui-core/src/surface.rs, and cmux-tui/src/session/remote.rs. It does not change Sources/TerminalController.swift,…
Cmux Expensive Synchronous Load	✅ Passed	PASS: The pull-request diff contains only three Rust files (surface.rs, mux.rs, and remote.rs) and contains no Swift changes. The custom check applies only when production Swift introduces or mo…
Cmux Cache Substitution Correctness	✅ Passed	PASS: The pull request changes only cmux-tui/crates/cmux-tui-core/src/surface.rs, a Rust file. The aggregate PR diff contains no Swift, TypeScript, or JavaScript files. Therefore, the cache-substitu…
Cmux No Hacky Sleeps	✅ Passed	PASS: The pull request changes only cmux-tui/crates/cmux-tui-core/src/surface.rs, a Rust file. The rule explicitly covers TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. Therefor…
Cmux Algorithmic Complexity	✅ Passed	PASS. The described PR changes cmux-tui/crates/cmux-tui-core/src/surface.rs by adding one reaper handle and bounded completion waits. The production paths do not scan scalable collections, sort, fil…
Cmux Swift Concurrency	✅ Passed	PASS: The PR diff contains one changed file, cmux-tui/crates/cmux-tui-core/src/surface.rs, with 187 additions and 16 deletions. The file is Rust, and the diff contains no Swift files or cmux-owned S…
Cmux Swift @Concurrent	✅ Passed	PASS: The PR diff from merge base 950dbf6 to HEAD changes only three Rust files: cmux-tui sources. The Swift-only diff is empty, so the Swift @concurrent check is not applicable.
Cmux Swift Package Boundaries	✅ Passed	PASS: The pull-request diff contains only three Rust files under cmux-tui (mux.rs, surface.rs, and session/remote.rs). The verified diff contains no Swift, SwiftPM, Xcode project, or workspace…

Full details: Description check

Explanation

The description explains the change, motivation, and validation commands, but it omits the required Demo Video, Review Trigger, and Checklist sections. It also uses “Validation” instead of the template’s “Testing” heading and states that Rust tests were deferred.

Full details: Cmux Swift Blocking Runtime

Explanation

PASS: The pull-request diff contains only three Rust files (mux.rs, surface.rs, and session/remote.rs). It contains no Swift changes and therefore introduces no production Swift blocking or timing-based synchronization.

Full details: Cmux Browser Automation Off-Main

Explanation

PASS: The PR changes only three Rust TUI files: cmux-tui-core/src/mux.rs, cmux-tui-core/src/surface.rs, and cmux-tui/src/session/remote.rs. It does not change Sources/TerminalController.swift, the control-socket execution policy, or policy tests. No added or deleted lines match the browser socket automation terms in the rule. The check is therefore not applicable.

Full details: Cmux Expensive Synchronous Load

Explanation

PASS: The pull-request diff contains only three Rust files (surface.rs, mux.rs, and remote.rs) and contains no Swift changes. The custom check applies only when production Swift introduces or moves expensive synchronous agent-history loading, so its failure condition is not applicable.

Full details: Cmux Cache Substitution Correctness

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui-core/src/surface.rs, a Rust file. The aggregate PR diff contains no Swift, TypeScript, or JavaScript files. Therefore, the cache-substitution correctness check does not apply.

Full details: Cmux No Hacky Sleeps

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui-core/src/surface.rs, a Rust file. The rule explicitly covers TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. Therefore this Rust teardown change is out of scope, even though it uses deadline-based waiting.

Full details: Cmux Algorithmic Complexity

Explanation

PASS. The described PR changes cmux-tui/crates/cmux-tui-core/src/surface.rs by adding one reaper handle and bounded completion waits. The production paths do not scan scalable collections, sort, filter, or perform per-target rescans. finish_terminal_reader handles at most one reaper and one reader per surface. The added collection-like code is test scaffolding, which the rule excludes. The existing mux surface iteration is unchanged by this PR.

Full details: Cmux Swift Concurrency

Explanation

PASS: The PR diff contains one changed file, cmux-tui/crates/cmux-tui-core/src/surface.rs, with 187 additions and 16 deletions. The file is Rust, and the diff contains no Swift files or cmux-owned Swift concurrency changes. Therefore, the Swift-specific failure conditions do not apply.

Full details: Cmux Swift Package Boundaries

Explanation

PASS: The pull-request diff contains only three Rust files under cmux-tui (mux.rs, surface.rs, and session/remote.rs). The verified diff contains no Swift, SwiftPM, Xcode project, or workspace changes. Therefore the Swift package-boundary check is not applicable.

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat-local-pty-reaper-join

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.