Preserve live remote PTYs when relay lease disappears

[austinywang]

Summary

• separate passive daemon retirement from explicit authenticated persistent-slot shutdown
• route both automatic exit reasons through one activity snapshot and require zero stdio connections plus zero live PTY sessions
• preserve detached PTYs across relay lease loss until they exit or are explicitly closed
• write timestamped automatic-exit reasons and activity counts to daemon.log
• update the remote-daemon lifecycle documentation to state the ownership boundary

Root cause

The relay .slot file and activeConnections describe transport ownership, but the lease-gone branch used those transport facts as authority to return from the daemon and run hub.closeAll(). That bypassed the live-session guard already used by the empty-idle branch and terminated persistent PTYs during a reconnect gap.

Authenticated daemon.shutdown remains intentionally destructive for final workspace cleanup. Passive lease observation now only makes an otherwise empty daemon eligible to retire.

Test-first proof

The branch preserves red-before-green history:

1. 2dd592e8ee changes only tests. On the old production code, TestPersistentDaemonPreservesActivePTYAfterObservedSlotLeaseDisappears failed with persistent daemon retired while its detached PTY was still active; the empty-idle test also exposed the missing lifecycle log.
2. 06e27e445b implements the shared automatic-retirement gate and lifecycle diagnostics. The regression reconnects to the same detached PTY after lease removal, then proves the daemon retires only after pty.close leaves the hub empty.

Verification

• focused lifecycle tests: 2/2 passed
• focused race lifecycle suite, including explicit destructive shutdown: 3/3 passed
• go test ./... -count=1 -timeout 10m
• go vet ./...
• strict test-determinism gate: 0 findings
• Linux/arm64 daemon binary cross-build: valid aarch64 ELF
• Linux/arm64 daemon test binary cross-compile: valid aarch64 ELF
• gofmt and git diff --check

go test -race ./... was attempted twice. Each run hit a different pre-existing short-deadline integration timeout under race load; both the affected shutdown sibling and every lifecycle test in this PR pass together under focused race execution.

Scope

No Swift files or warning/file-length budget files change. Explicit workspace shutdown behavior is unchanged.

Closes #9706

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Note

Medium Risk
Changes persistent SSH PTY daemon lifecycle and retirement timing on the remote host; behavior is well-tested but affects reconnect gaps and workspace teardown boundaries.

Overview
Fixes premature persistent daemon shutdown when the relay .slot lease disappears while a detached PTY is still running.

Passive retirement now uses one gate: automatic exit only when stdio connections and live PTY sessions (including sessions mid-teardown) are both zero. Lease loss alone no longer tears down live detached sessions; they survive until exit, explicit pty.close, or the empty idle timeout. Authenticated daemon.shutdown for workspace cleanup is unchanged.

The PTY hub counts sessions through teardown completion (sessionTeardownCount / WaitGroup) so retirement cannot race idle reaping. Automatic exits log RFC3339Nano timestamps, reason (slot_lease_removed or empty_idle_timeout), and activity counts to daemon.log. Docs describe passive lease policy and lifecycle diagnostics.

^{Reviewed by Cursor Bugbot for commit e539849. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Preserves live remote PTYs when a relay slot lease disappears and retires the daemon only when it’s truly empty (no stdio and no live or tearing‑down PTY sessions). Adds timestamped exit logs with reason and activity counts, and tightens idle/lease checks and docs on passive lease policy. Addresses #9706.

• Bug Fixes

  • Detached PTYs survive ~/.cmux/relay/<port>.slot lease loss; retirement requires zero stdio connections and zero PTY sessions, including sessions mid‑teardown.
  • Daemon waits for PTY teardown to finish; closeAll blocks until processes and PTY files close, and the idle reaper counts a session as active until teardown completes. Authenticated daemon.shutdown stays destructive.

• Refactors

  • Single automatic‑exit gate for lease loss and empty idle timeout; logs RFC3339Nano exit time, reason (slot_lease_removed or empty_idle_timeout), and activity counts to daemon.log.
  • Defers session scans while connections are active; README/spec clarify passive retirement and that detached PTYs outlive lease loss until they exit or are closed.

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

Summary by CodeRabbit

• Bug Fixes

  • Persistent remote daemons now preserve active detached PTY sessions when a relay-slot lease disappears.
  • Daemons shut down only after connections and PTY sessions are fully inactive, or after an empty idle timeout.
  • Clean workspace teardown now safely retires eligible disconnected daemons.

• Documentation

  • Improved guidance on daemon lifecycle, shutdown behavior, and crash diagnostics.

• Diagnostics

  • Shutdown logs now clearly identify the exit reason and report active connection and session counts.

[coderabbitai]

📝 Walkthrough

Walkthrough

The persistent daemon now tracks connections and PTY sessions together when evaluating shutdown. It preserves detached PTYs after lease removal, retires only when activity is empty, logs exit reasons, and adds lifecycle tests for lease loss and idle timeout.

Changes

Persistent daemon lifecycle

Layer / File(s)	Summary
Activity tracking and exit classification daemon/remote/cmd/cmuxd-remote/persistent_lifecycle.go	The daemon tracks active connections and sessions, classifies automatic exit reasons, and writes structured exit logs.
Polling and shutdown integration daemon/remote/cmd/cmuxd-remote/main.go, daemon/remote/README.md, docs/remote-daemon-spec.md	The accept loop evaluates lease state with activity, delays shutdown while sessions remain, performs lease cleanup, and documents passive retirement.
Lifecycle regression validation daemon/remote/cmd/cmuxd-remote/main_test.go, daemon/remote/cmd/cmuxd-remote/persistent_lifecycle_test.go	Tests verify idle-timeout logging and preserve detached PTYs until reconnection and explicit closure after lease removal.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PersistentDaemon
  participant PTYSession
  participant SlotLease
  Client->>PersistentDaemon: Disconnect stdio
  PersistentDaemon->>PTYSession: Check active sessions
  PersistentDaemon->>SlotLease: Observe lease removal
  SlotLease-->>PersistentDaemon: Lease absent
  PersistentDaemon-->>Client: Preserve detached PTY
  Client->>PersistentDaemon: Reconnect and reattach
  Client->>PTYSession: Close PTY
  PersistentDaemon-->>PersistentDaemon: Log lease-removal exit

Loading

Possibly related PRs

• manaflow-ai/cmux#9019: Both modify persistent-daemon lifecycle handling in main.go, with this PR focusing on shutdown activity tracking.
• manaflow-ai/cmux#9111: Both modify persistent PTY lifecycle handling, while this PR focuses on lease loss and idle expiration.

Suggested reviewers: lawrencecchen

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Cmux Algorithmic Complexity	❌ Error	main.go:1218 scans wsPTYHub.sessions via activeSessionCount on every poll; the old code short-circuited this O(S) scan while activeConnections was nonzero.	Guard activeSessionCount behind zero active connections, or maintain an O(1) active-session counter in wsPTYHub before building the snapshot.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue #9706 by guarding automatic retirement with zero connections and sessions, preserving detached PTYs, and adding exit diagnostics.
Out of Scope Changes check	✅ Passed	The code, tests, documentation, and diagnostics changes are directly related to the linked issue and stated lifecycle objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation	✅ Passed	The PR changes only Go and Markdown files; the parent-to-HEAD diff contains no Swift paths or actor-isolation declarations.
Cmux Swift Blocking Runtime	✅ Passed	The full PR changes only Go and Markdown files; it introduces no Swift production changes or Swift blocking-runtime primitives.
Cmux Browser Automation Off-Main	✅ Passed	The PR changes only remote-daemon Go, tests, and documentation; it does not modify the rule's Swift browser automation targets or add browser socket commands.
Cmux Expensive Synchronous Load	✅ Passed	The full PR diff contains only four Go files and two Markdown files, with no Swift paths or Swift expensive-load symbols; the Swift-specific check is not applicable.
Cmux Cache Substitution Correctness	✅ Passed	Not applicable: the PR diff contains only Go and Markdown changes, with no production Swift, TypeScript, or JavaScript changes.
Cmux No Hacky Sleeps	✅ Passed	The full PR changes only Go and Markdown files; it introduces no TypeScript, JavaScript, shell, or covered build/runtime script delay.
Cmux Swift Concurrency	✅ Passed	The commit changes only two Go and two Markdown files; no Swift files or Swift concurrency patterns are introduced or expanded.
Cmux Swift @Concurrent	✅ Passed	The complete PR range changes only Go, Markdown, and test files; it contains no Swift changes or Swift concurrency annotations to assess.
Cmux Swift Package Boundaries	✅ Passed	The PR diff changes only Go and Markdown files; it contains no production Swift or Package.swift changes, so the Swift package-boundary check is not applicable.
Cmux Swiftpm Lockfiles	✅ Passed	The PR changes only Go, Go test, and documentation files; it changes no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project paths.
Cmux Swift Logging	✅ Passed	The complete PR diff changes only Go tests/source and Markdown; it contains no Swift/Objective-C files, so the Swift logging rule is not applicable.
Cmux User-Facing Error Privacy	✅ Passed	The only new production text is a sanitized daemon.log line with timestamp, exit reason, and activity counts; no vendor, credential, token, session ID, raw payload, or other forbidden data is exposed.
Cmux Full Internationalization	✅ Passed	The PR changes only Go daemon lifecycle code/tests and operational daemon documentation; no Swift, catalog, web, or locale files change, and the new daemon.log line is operational diagnostics.
Cmux Swiftui State Layout	✅ Passed	The commit changes only Go, Markdown, and documentation files; no Swift or SwiftUI code is added or modified, so the state-layout check is not applicable.
Cmux Architecture Rethink	✅ Passed	The PR range changes only Go tests/implementation and Markdown; it contains no Swift source or project-wiring edits, so the Swift architectural rethink rule is not applicable.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	The full PR diff against origin/main changes only Go and Markdown files; it introduces no Swift auxiliary windows or close-shortcut code.
Cmux Source Artifacts	✅ Passed	All six changed paths are Go source/tests or Markdown docs; the PR adds no paths, binary diffs, artifact directories, logs, caches, or build output.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The PR changes only Go and Markdown files across both commits; no Swift file under a production Sources/ path is changed, so this check is not applicable.
Cmux No Ambient Global State	✅ Passed	The two-commit PR range changes only Go, Markdown, and documentation files; the Swift diff is empty, so this Swift-only check is not applicable.
Title check	✅ Passed	The title clearly summarizes the primary change: preserving live remote PTYs after relay lease loss.
Description check	✅ Passed	The description thoroughly explains the change, root cause, testing, scope, and issue linkage, but omits the template checklist and demo video.

✨ 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 issue-9706-lease-gone-live-pty

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@daemon/remote/cmd/cmuxd-remote/main_test.go`:
- Around line 2068-2073: Update the automatic-exit log assertions in
daemon/remote/cmd/cmuxd-remote/main_test.go lines 2068-2073 and
daemon/remote/cmd/cmuxd-remote/persistent_lifecycle_test.go lines 508-512 to
extract the time= field and parse it with time.RFC3339Nano, while preserving
validation of the existing reason and activity counts.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a36c1257-8b9c-4ffe-b7c0-6175b28799af

📥 Commits

Reviewing files that changed from the base of the PR and between 29d809d and 06e27e4.

📒 Files selected for processing (6)

• daemon/remote/README.md
• daemon/remote/cmd/cmuxd-remote/main.go
• daemon/remote/cmd/cmuxd-remote/main_test.go
• daemon/remote/cmd/cmuxd-remote/persistent_lifecycle.go
• daemon/remote/cmd/cmuxd-remote/persistent_lifecycle_test.go
• docs/remote-daemon-spec.md