Keep CLI socket-discovery tests off the host's real cmux

[teamleaderleo]

Summary

Two tests in CMUXCLIErrorOutputRegressionTests expect the CLI to find no live socket: testAmbientTaggedCLIListsEveryDeadSocketCandidateOnFailure and testLaunchCapableCommandsReachTheirDispatchPathWithoutLiveImplicitSocket. On runner cmuxs-mac-mini-3 they failed because the CLI reached the runner's own cmux, which runs as the same user:

cmux: default socket /tmp/cmux-debug-cli-ambient-all-dead-….sock is unavailable; using /Users/cmux/.local/state/cmux/cmux-501.sock.

Failing runs:

• Reload the Ghostty config when its files change, and show config errors #14859: run 36262203469, job 108461945002
• the Checkpoint terminal scrollback so a crash no longer loses it #14852 and Show opt-in model, context %, and estimated cost next to agent status in the sidebar #14855 runs

Root cause

The tests moved the home directory only through CFFIXED_USER_HOME. That does move the resolver's state directory, because homeDirectoryForCurrentUser honors it. But implicit discovery also reads the machine-wide /tmp/*-last-socket-path marker mirrors and tries the legacy /tmp/cmux.sock and /tmp/cmux-<uid>.sock aliases. Every cmux running as that user shares those paths.

writeStableSocketMarker worked around the stable /tmp marker back when discovery stopped at the first readable marker. Discovery now walks every marker, so the real app's /tmp/cmux-last-socket-path was followed to its live socket. The same leak lets these tests send ping, restore and fork to a developer's running cmux.

Fix

• Tests: BundledCLITestSupport.hermeticCLIEnvironment(home:) sets HOME, CFFIXED_USER_HOME, XDG_* and TMPDIR inside the temp home. It drops every inherited CMUX* and XDG_* variable and sets CMUX_TEST_ISOLATED_SOCKET_DISCOVERY=1. All 20 CFFIXED_USER_HOME call sites in the suite now use it.
• CLI: in Debug builds only, CLISocketPathResolver honors that key. It skips marker files outside the state directory and drops the legacy /tmp aliases. The variant's own default socket and state-directory markers still count. Release builds compile the check out, and neither production discovery order nor socket locations change.
• New tests:
  • a guard that the helper never exposes the real home or socket pins
  • a spawned-CLI regression: a per-tag /tmp marker names a live socket, and the CLI must not use it
  • a resolver test comparing the isolated and ambient candidate lists

Commits

• ed883fc9ba0: failing regression. The new CLI test fails, because the CLI follows the /tmp marker, and the resolver test does not compile yet.
• d52024cbadc: fix
• a test tidy-up commit

Verification

• verify-local --affected mf/main --swift-changed mf/main: swift-syntax, test-wiring and feature-flags passed.
• No local xcodebuild; the machine is under heavy load. App-host execution of the suite is left to CI.
• Suites in other files that also set CFFIXED_USER_HOME are not migrated here. Most of them pin CMUX_SOCKET_PATH.

🤖 Generated with Claude Code

---

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

---

Summary by cubic

Fixes socket-discovery tests reaching the host's real cmux by isolating implicit discovery to the test's state directory in Debug builds.

Bug Fixes

• CLISocketPathResolver honors CMUX_TEST_ISOLATED_SOCKET_DISCOVERY=1 in Debug only, skipping /tmp marker files and legacy aliases outside the state directory.
• Release builds compile the check out; production discovery order and socket locations are unchanged.

Refactors

• Added BundledCLITestSupport.hermeticCLIEnvironment(home:) and migrated all 20 CFFIXED_USER_HOME call sites in the suite to it.
• Added tests covering the helper's isolation guarantees, a spawned-CLI regression for /tmp markers naming a live socket, and a resolver test comparing ambient vs isolated candidate lists.

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

Summary by CodeRabbit

• Tests
  • CLI test runs now use isolated temporary environments, reducing interference from machine-wide settings and socket markers.
  • Added regression coverage to verify socket discovery stays within the test environment while retaining local candidates.

[github-actions]

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

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 34c35351-c24d-4d96-9376-62fa2ab7b67a

📥 Commits

Reviewing files that changed from the base of the PR and between b1daa44 and f859835.

📒 Files selected for processing (3)

• CLI/CLISocketPathResolver.swift
• cmuxTests/BundledCLILinkageTests.swift
• cmuxTests/CMUXCLIErrorOutputRegressionTests.swift

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

---

📝 Walkthrough

Walkthrough

The CLI socket resolver adds debug-only confinement for implicit discovery candidates. CLI subprocess tests now use a hermetic environment and include coverage for environment paths and socket discovery.

Changes

CLI socket discovery

Layer / File(s)	Summary
Resolver discovery confinement CLI/CLISocketPathResolver.swift	A debug-only setting enables filtering of requested, fallback, and marker-file candidates outside stateDirectory. Release builds disable the setting.
Hermetic CLI environment and regression coverage cmuxTests/BundledCLILinkageTests.swift, cmuxTests/CMUXCLIErrorOutputRegressionTests.swift	Test support creates a private environment for CLI subprocesses. Tests use the helper and check environment confinement, machine-wide marker exclusion, and retention of test-local candidates.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f8598

The CLI tests exclude machine-wide socket markers and legacy aliases while retaining their intended test-local candidates. No issue identified here needs to block merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f8598

The change appears to reduce the chance that tests contact a running cmux on the host, without changing release behavior. Isolation is deliberately limited rather than a guarantee that every reachable socket is test-owned, and the tests were not observed running in this review.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected boundary is local CLI-to-same-user Unix-socket discovery, principally for Debug CLI subprocesses using temporary homes. The new setting does not increase release-build discovery reachability.

Security Findings and Attack Paths

• inferred — Previously, an implicit CLI command using a temporary home could still follow a shared /tmp marker to a live same-user cmux. The added regression models that path and asserts that isolated discovery does not send its ping to the simulated outside listener; this is test evidence, not an observed execution result.

Trust Boundaries and Controls

• observed — The resolver requires an owned socket that accepts connections and applies ownership, file-type, link-count, and size checks before reading marker contents. Those checks do not constrain an accepted marker's destination to the temporary state directory.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: preventing CLI socket-discovery tests from connecting to the host's real cmux instance.
Description check	✅ Passed	The description explains the failure, root cause, implementation, added tests, verification results, and remaining CI coverage. It does not include the template checklist or a demo attachment, but the…
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 Cloud Persistent Session And Early Input	✅ Passed	PASS. The reviewed diff changes CLI socket discovery and test environment setup in three Swift files. It does not change Cloud terminal creation, persistent cmux-tui transport, manual renderers, PTY r…
Cmux Swift Actor Isolation	✅ Passed	The only production change is in CLI/CLISocketPathResolver.swift. It adds a stored Bool, a static environment-key constant, and synchronous path-filtering helpers to an existing value-type resolve…
Cmux Swift Blocking Runtime	✅ Passed	PASS. The production change in CLI/CLISocketPathResolver.swift only adds a Debug-gated flag and path filtering. It adds no semaphore, blocking wait, sleep, delayed dispatch, polling loop, main-queue…
Cmux Browser Automation Off-Main	✅ Passed	PASS: The pull request changes only CLI socket discovery and CLI test environment files. The authoritative diff contains no browser.* commands, WebKit/AppKit automation routing, socket-worker policy c…
Cmux Expensive Synchronous Load	✅ Passed	PASS. The only production Swift change is in CLI/CLISocketPathResolver.swift; it adds a Debug-only environment flag and filters socket-discovery paths by state directory. The diff adds no `Restorabl…
Cmux Cache Substitution Correctness	✅ Passed	PASS: The production diff only adds Debug-only filtering for CLI socket discovery in CLI/CLISocketPathResolver.swift. It does not replace a fresh authoritative read with a cache or opportunistic val…
Cmux No Hacky Sleeps	✅ Passed	The pull request changes only three Swift files. It introduces no TypeScript, JavaScript, shell, or build/runtime-script changes. The existing Swift socket poll remains unchanged, so this check has no…
Cmux Algorithmic Complexity	✅ Passed	PASS. The only production change is in CLI/CLISocketPathResolver.swift. It filters the fixed socket-discovery inputs and standardizes each path once. SocketPathMarkerFiles.paths returns at most tw…
Cmux Swift Concurrency	✅ Passed	The pull request introduces no legacy async pattern covered by the check. The added Swift lines contain no Dispatch, Combine, Task, async/await, completion-handler, or fire-and-forget usage. The only …
Cmux Swift @Concurrent	✅ Passed	The PR adds no changed async, await, nonisolated, @concurrent, or actor-isolation declarations. The new hermeticCLIEnvironment helper is synchronous. The existing async theme test and its ex…
Cmux Swift Package Boundaries	✅ Passed	PASS. The production diff adds a small Debug-only test-isolation branch to the existing CLI/CLISocketPathResolver, plus path filtering. It does not introduce or materially expand a reusable domain f…
Cmux Swiftpm Lockfiles	✅ Passed	PASS. The PR changes only CLI/CLISocketPathResolver.swift and two test files. It does not change Package.swift, Package.resolved, .gitignore, workflows, or Xcode package references. The SwiftP…
Cmux Swift Logging	✅ Passed	PASS. The only production Swift change is CLI/CLISocketPathResolver.swift, which adds environment-gated path filtering and no logging calls or logger declarations. The changed test code writes socke…
Cmux User-Facing Error Privacy	✅ Passed	PASS. The only production-file change adds Debug-only socket-discovery filtering and the internal CMUX_TEST_ISOLATED_SOCKET_DISCOVERY key. It adds no user-facing error, alert, command-output, or rec…
Cmux Full Internationalization	✅ Passed	PASS: The production diff only adds Debug-only socket-discovery isolation and a configuration key. It adds no user-facing text. Existing CLI diagnostics remain localized and their catalog entries are …
Cmux Swiftui State Layout	✅ Passed	PASS. The pull request changes only CLI socket resolution and CLI test support. The authoritative diff contains no SwiftUI imports, ObservableObject, @Published, @Observable, GeometryReader, lazy/list…
Cmux Architecture Rethink	✅ Passed	PASS. The diff adds a local CLISocketPathResolver invariant: when the Debug-only test key is set, implicit candidates must remain under stateDirectory; explicit paths remain unchanged. The test he…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS. The PR changes only CLI socket discovery and CLI test environment support in three Swift files. The scoped diff adds no NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup, or clo…
Cmux Source Artifacts	✅ Passed	All three changed paths are tracked Swift source or test files. The diff adds resolver logic, test-environment support, and regression tests. No changed path or added source line introduces a local/ge…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS: The only changed Swift file outside test targets is CLI/CLISocketPathResolver.swift, which is not under a **/Sources/** path. The custom check therefore does not apply. The other changed Swi…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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]

Merge receipt for f859835461: every check was green at merge (16 verified; 14 skipped by policy). Full suite runs on main after merge.