Repository navigation
Report and stop stale cmux-tui servers - #8769
lawrencecchen wants to merge 623 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change introduces shared release identity metadata, local server status and stop commands, protocol shutdown handling, peer-process validation, and shutdown propagation through TUI and headless execution. Build workflows stamp and verify distribution identities. ChangesRelease identity and compatibility metadata
Shutdown protocol and propagation
Local server lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ServerLifecycle
participant ControlServer
participant Mux
CLI->>ServerLifecycle: inspect identify
ServerLifecycle->>ControlServer: send identify
ControlServer-->>ServerLifecycle: release identity and capabilities
CLI->>ServerLifecycle: stop
ServerLifecycle->>ControlServer: send shutdown
ControlServer->>Mux: request_shutdown and close surfaces
Mux-->>ControlServer: shutdown state
ControlServer-->>ServerLifecycle: acknowledgement and disconnect
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds
Confidence Score: 5/5This PR is safe to merge. The shutdown path is well-coordinated, all user-facing errors are localized, and the surface-close issue flagged in the previous review is addressed by the new atomic drain design. The core shutdown flow is heavily tested: confirmed ACK writes, coordinator-locked surface drain, retry backoff in ServerShutdownCleanup, and a watchdog that force-exits after cleanup. Error messages from close_all_surfaces_for_shutdown are consistently converted to the stable shutdown_cleanup_incomplete code before leaving the server. The legacy stop path quarantines the socket atomically before spawning the helper, verifies PID ownership, and fences the process tree before any SIGKILL. Both English and Japanese localization strings are complete. Build.rs guarantees a non-null identity for every build. No correctness regressions found. Files Needing Attention: No files require special attention. The server_lifecycle.rs and legacy_process.rs additions are large but well-structured and covered by the CLI integration tests. Important Files Changed
Reviews (62): Last reviewed commit: "fix(tui): transfer legacy socket ownersh..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/cmux-tui-build-package.yml:
- Line 110: Update the package job steps using CMUX_TUI_DISTRIBUTION_VERSION to
avoid interpolating inputs.version directly into Bash source. Define the release
version through each step’s env mapping, then consume the environment variable
in the affected commands at the referenced occurrences.
In `@cmux-tui/crates/cmux-tui/src/cli.rs`:
- Around line 699-708: Update the `stop` failure branch and the `connect`
failure branch in the CLI command match to use anyhow’s alternate error
formatter, preserving and printing the complete cause chain instead of only the
outermost context.
🪄 Autofix (Beta)
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: 223e5034-e4b2-4da3-8ed9-c169afaa73c7
⛔ Files ignored due to path filters (1)
cmux-tui/dist/RELEASING-TUI.mdis excluded by!**/dist/**
📒 Files selected for processing (16)
.github/workflows/cmux-tui-build-package.ymlcmux-tui/crates/cmux-tui-core/src/lib.rscmux-tui/crates/cmux-tui-core/src/mux.rscmux-tui/crates/cmux-tui-core/src/release.rscmux-tui/crates/cmux-tui-core/src/server.rscmux-tui/crates/cmux-tui/src/app.rscmux-tui/crates/cmux-tui/src/cli.rscmux-tui/crates/cmux-tui/src/localization.rscmux-tui/crates/cmux-tui/src/main.rscmux-tui/crates/cmux-tui/src/server_lifecycle.rscmux-tui/crates/cmux-tui/src/session/mod.rscmux-tui/crates/cmux-tui/src/session/remote.rscmux-tui/crates/cmux-tui/tests/cli.rscmux-tui/docs/getting-started.mdcmux-tui/docs/protocol.mdcmux-tui/spec/commands.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@cmux-tui/crates/cmux-tui/src/app.rs`:
- Line 4172: Update App::shutdown_requested() to include
self.session.daemon_shutdown_requested() in its combined shutdown check, making
it the single source of truth for all shutdown signals. Then remove the
redundant !self.session.daemon_shutdown_requested() condition from the
event-loop while condition and any corresponding duplicate condition in the
related shutdown path around the additional referenced code.
In `@cmux-tui/crates/cmux-tui/src/localization.rs`:
- Around line 23-46: Extend ServerMessages and both ENGLISH and JAPANESE
catalogs with entries for not_a_session, too_many_surfaces,
close_surface_failed, and attach_initial_size_unsupported. In
cmux-tui/crates/cmux-tui/src/server_lifecycle.rs lines 20-51, replace the raw
session-validation error with the catalog entry; in lines 157-187, replace both
raw surface-close errors; and in cmux-tui/crates/cmux-tui/src/cli.rs lines
612-619, replace the raw initial-attach sizing error. Use the localized entries
consistently with neighboring ServerMessages errors.
In `@cmux-tui/crates/cmux-tui/src/server_lifecycle.rs`:
- Around line 251-287: Enforce RESPONSE_TIMEOUT on every iteration of
read_matching_response and wait_for_disconnect, not only when read_line returns
WouldBlock or TimedOut. Add an unconditional deadline check at the top of each
loop and return the existing timeout failure once expired, including replacing
wait_for_disconnect’s shutdown_timed_out-only guard. Preserve processing of
unrelated events and notifications while the deadline has not elapsed.
🪄 Autofix (Beta)
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: ee046a8b-2207-4b09-8ba5-59c92c1a625e
📒 Files selected for processing (15)
.github/workflows/cmux-tui-build-package.ymlcmux-tui/crates/cmux-tui-core/src/lib.rscmux-tui/crates/cmux-tui-core/src/mux.rscmux-tui/crates/cmux-tui-core/src/server.rscmux-tui/crates/cmux-tui/src/app.rscmux-tui/crates/cmux-tui/src/cli.rscmux-tui/crates/cmux-tui/src/localization.rscmux-tui/crates/cmux-tui/src/machine_provider_runtime.rscmux-tui/crates/cmux-tui/src/main.rscmux-tui/crates/cmux-tui/src/server_lifecycle.rscmux-tui/crates/cmux-tui/src/session/mod.rscmux-tui/crates/cmux-tui/src/session/remote.rscmux-tui/crates/cmux-tui/tests/cli.rscmux-tui/docs/protocol.mdcmux-tui/spec/commands.md
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
There was a problem hiding this comment.
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 `@cmux-tui/crates/cmux-tui-core/src/release.rs`:
- Around line 61-66: Treat empty environment-variable values as absent before
fallback selection: update stamped_build_commit() in
cmux-tui/crates/cmux-tui-core/src/release.rs:61-66 and the corresponding
stamped_ghostty_commit() logic in cmux-tui/crates/cmux-tui-core/build.rs:38-43
to filter each option_env! result before chaining .or calls. Add coverage
verifying empty overrides fall through to the next non-empty commit stamp.
🪄 Autofix (Beta)
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: 4a85482b-ac3e-4730-b982-826205fded2b
⛔ Files ignored due to path filters (1)
cmux-tui/dist/RELEASING-TUI.mdis excluded by!**/dist/**
📒 Files selected for processing (9)
cmux-tui/crates/cmux-tui-core/build.rscmux-tui/crates/cmux-tui-core/src/platform.rscmux-tui/crates/cmux-tui-core/src/release.rscmux-tui/crates/cmux-tui/src/cli.rscmux-tui/crates/cmux-tui/src/localization.rscmux-tui/crates/cmux-tui/src/server_lifecycle.rscmux-tui/crates/cmux-tui/tests/cli.rscmux-tui/docs/protocol.mdcmux-tui/spec/commands.md
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
2 similar comments
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
2 similar comments
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
3 similar comments
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
# Conflicts: # cmux-tui/bindings/ERGONOMICS.md # cmux-tui/bindings/RELEASING.md # cmux-tui/bindings/cpp/.cmux-sdk-manifest.json # cmux-tui/bindings/cpp/include/cmux/raw/generated/models.hpp # cmux-tui/bindings/go/raw/.cmux-sdk-manifest.json # cmux-tui/bindings/go/raw/README.md # cmux-tui/bindings/go/raw/generated_commands.go # cmux-tui/bindings/go/raw/generated_events.go # cmux-tui/bindings/go/raw/generated_metadata.go # cmux-tui/bindings/go/raw/generated_presence_test.go # cmux-tui/bindings/go/raw/generated_types.go # cmux-tui/bindings/java/src/com/cmux/raw/.cmux-sdk-manifest.json # cmux-tui/bindings/java/src/com/cmux/raw/IdentifyResult.java # cmux-tui/bindings/java/src/com/cmux/raw/Protocol.java # cmux-tui/bindings/python/cmux/raw/_generated/.cmux-sdk-manifest.json # cmux-tui/bindings/python/cmux/raw/_generated/_schema.py # cmux-tui/bindings/python/cmux/raw/_generated/metadata.py # cmux-tui/bindings/python/cmux/raw/_generated/models.py # cmux-tui/bindings/python/tests/test_protocol.py # cmux-tui/bindings/rust/src/generated/.cmux-sdk-manifest.json # cmux-tui/bindings/rust/src/generated/commands.rs # cmux-tui/bindings/rust/src/generated/events.rs # cmux-tui/bindings/rust/src/generated/metadata.rs # cmux-tui/bindings/rust/src/generated/mod.rs # cmux-tui/bindings/rust/src/generated/types.rs # cmux-tui/bindings/typescript/src/raw/generated/.cmux-sdk-manifest.json # cmux-tui/bindings/typescript/src/raw/generated/commands.ts # cmux-tui/bindings/typescript/src/raw/generated/events.ts # cmux-tui/bindings/typescript/src/raw/generated/index.ts # cmux-tui/bindings/typescript/src/raw/generated/metadata.ts # cmux-tui/bindings/typescript/src/raw/generated/types.ts # cmux-tui/bindings/zig/src/raw/generated/.cmux-sdk-manifest.json # cmux-tui/bindings/zig/src/raw/generated/presence_test.zig # cmux-tui/bindings/zig/src/raw/generated/protocol.zig # cmux-tui/crates/cmux-tui-core/src/lib.rs # cmux-tui/crates/cmux-tui-core/src/mux.rs # cmux-tui/crates/cmux-tui-core/src/server.rs # cmux-tui/crates/cmux-tui-core/src/surface.rs # cmux-tui/crates/cmux-tui-core/src/terminal_host_runtime.rs # cmux-tui/crates/cmux-tui-core/src/workspace_registry.rs # cmux-tui/crates/cmux-tui/src/app.rs # cmux-tui/crates/cmux-tui/src/cli.rs # cmux-tui/crates/cmux-tui/src/cli/wire.rs # cmux-tui/crates/cmux-tui/src/machine_provider_client.rs # cmux-tui/crates/cmux-tui/src/main.rs # cmux-tui/crates/cmux-tui/tests/cli.rs # cmux-tui/docs/protocol.md # cmux-tui/spec/commands.md # cmux-tui/spec/sdk-schema.json
…at-tui-server-upgrade
# Conflicts: # Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRestoreSpawnSchedulerTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
# Conflicts: # Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownCoordinator.swift # Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swift
…at-tui-server-upgrade
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Summary
Tests
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Stops stale local
cmux-tuiservers, upgrades them in place, and hardens the macOS terminal lifecycle so stuck closes never block new terminals. Adds restartable local shutdown with health reporting, pane-visible capacity errors, and deterministic cleanup so stale state can’t block new sessions.New Features
shutdown;identifyreturnsshutdown_cleanup; regenerated SDKs (102 commands, 46 events); preservesstream_endwhen a delivery queue is full.cmux-tui-processto wrap atomic TCP/Unix bind/accept/connect and guard process/PTY creation; fences remote socket creation/inheritance; bounds WebSocket server and browser-transport shutdown; targets terminal-host and workspace cleanup exactly with receipts kept across retries; reaps provider peers;cmux-ptycan remove env keys; publishes machine identity atomically.VERSION; sets and verifiesMACOSX_DEPLOYMENT_TARGET; validates TUI package launchers;cmux-tui.ymlsupportsuse_blacksmith_macosand pinned runners.Migration
cmux-tui server status, thencmux-tui server stop, then start the new binary; stopping exits pane processes./procand permission forpidfd_send_signal(5.1–5.2 use exact/proc/<pid>handles). Publishedcmux-tuipackages require macOS 15+.Written for commit b9ad9df. Summary will update on new commits.
Summary by CodeRabbit
cmux-tui server statusandcmux-tui server stop, plus a local-only shutdown command.identify/pingnow exchange release identity (version, build stamps) and protocol for compatibility checks.attach-surface --colsnow requires server support.Note
High Risk
Changes native pointer ownership, teardown concurrency, and close-path admission across create/deinit/hibernate—core macOS terminal stability—with a large new surface area despite heavy tests.
Overview
macOS terminal runtime now treats every Ghostty surface as a bounded ownership reservation from
ghostty_surface_newuntilghostty_surface_freecompletes.TerminalSurfaceRuntimeTeardownCoordinatoris reworked: teardown runs on gated utility-priority threads instead ofDispatchQueue, submissions go through an ingress drain, and two close workers with 5s watchdogs can fence new creation when both slots are busy or fully stalled.When admission is saturated, creation is deferred or queued in a bounded overflow FIFO (per-surface weak refs, batched MainActor rescans) rather than scanning the full registry. If both close workers exceed the watchdog, deferred surfaces get a visible pane error (“too many sessions still closing”) while keeping FIFO order for retries after a late free returns.
TerminalSurfacewires this through create/teardown/deinit/hibernate paths, returns awaitable teardown tickets, and panes implement show/clear runtime creation failure onTerminalSurfacePaneHosting.CI / packaging stamps builds via
VERSION, setsMACOSX_DEPLOYMENT_TARGETfrommacos-deployment-target.txtwith post-build verification, runsverify_artifact_identity.pyon native, npm, and PyPI smoke paths, adds TUI package launcher validation, and letscmux-tui.ymlpick macOS runners viause_blacksmith_macos.Tests add extensive
TerminalSurfaceRestoreSpawnSchedulerTestscoverage for admission recovery, overflow ordering, stalled-close failure batches, and lifecycle cancellation.Reviewed by Cursor Bugbot for commit 44ae65d. Bugbot is set up for automated code reviews on this repo. Configure here.