Skip to content

fix(cloud): run the link client in its own process group - #22017

Merged
teamleaderleo merged 3 commits into
mainfrom
fix/cloud-link-process-group
Oct 19, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
fix/cloud-link-process-group

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

cmux DEV crashed twice on 2026-10-18 (build 108, 03:58 and 20:46) with EXC_BREAKPOINT on the cmux-next main-thread watchdog thread: the debug-only 10 s hang assertion in MainThreadWatchdog.checkHang. Both reports have procRole: Background. In the 20:46 report the main thread is idle in mach_msg inside -[NSApplication run].

The assertion only looked at wall-clock time since the last run-loop heartbeat. In a backgrounded, throttled app the main thread can sit runnable but barely scheduled, so the heartbeat stays stale for 10 s while the thread does almost no work. By the time the crash report is captured, the thread has gone back to sleep. Its message says "busy", but it never checked CPU.

The fix: the assertion now also needs the main thread to have used at least half the threshold in CPU time during the stall. The watchdog thread reads that with thread_info on the main thread's port, using the reader BusyWatchdog already had. If the CPU reading fails, wall-clock time decides, as before. The hang report path does not change: logger.fault and onHang/Sentry still fire at 2 s. A main thread blocked on a lock or a synchronous IPC uses no CPU either, so it is now reported but no longer asserted.

Testing

  • MainThreadWatchdogPolicyTests adds three tests: the assertion policy for idle/throttled stalls vs busy stalls, the nil-CPU fallback, and reading main-thread CPU from another thread while the main thread spins. The tests were committed first in bfcf792. Red locally: the test target does not compile against the base. Green: swift build --target CmuxNextControlTests on cosmic-gray. The full local swift test cannot link because the base has an unrelated red (MarkdownViewerWritableTests.Host does not conform to FilePageHosting), so CI runs the tests.

Changelog

none

🤖 Generated with Claude Code

teamleaderleo and others added 2 commits October 18, 2026 23:40
Refs #11008

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cancelling or timing out a Cloud link signalled only the cmux-tui client.
A helper that ignores SIGTERM and inherited the link pipes survived, kept
stdout open and left the attempt wedged. Spawn the client with
POSIX_SPAWN_SETPGROUP, signal the whole group, and kill what is left of
the group once the leader exits: at once on cancel or disconnect, after
the bounded stderr drain when the client exits on its own.

Fixes #11008

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Oct 19, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 53919658-9535-4d57-a590-ee3ced69e5e7
📥 Commits

Reviewing files that changed from the base of the PR and between 86d6b5e and 2f49c49.

📒 Files selected for processing: 3
  • Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudLinkProcess.swift
  • Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink.swift
  • Packages/macOS/CmuxCloud/Tests/CmuxCloudTests/CloudMachineLinkProcessGroupTests.swift
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

SIGTERM goes to the client only, so cmux-tui still closes its ssh child
with EOF. SSH links no longer sweep their group: OpenSSH's ProxyCommand
child of a persisted ControlMaster stays in the group that started it,
and killing it would drop every connection sharing the master. The
leftover sweep runs beside the actor hop, and a missing or unusable
client keeps the Process-style error text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Probably yours: CMUXCLITestAssertions.swift:45, CMUXCLIErrorOutputRegressionTests.swift fails here but not on main's latest full suite.

CI failed on 2f49c49d27 (run 39215077752 attempt 1): 1 code.

Job Verdict Why
macos / app-host unit tests (changed suites) code a test failed

Failures:

  • new in this PR CMUXCLITestAssertions.swift:45 testBareRelativeDirectoryPathOpenBypassesProtectedSocketForExternalCLI(): Expectation failed: try lhs() == rhs() (job)
  • new in this PR CMUXCLIErrorOutputRegressionTests.swift CMUXCLIErrorOutputRegressionTests.testBareRelativeDirectoryPathOpenBypassesProtectedSocketForExterna...: failed (job)
Matched log lines
macos / app-host unit tests (changed suites): ✘ Test testBareRelativeDirectoryPathOpenBypassesProtectedSocketForExternalCLI() recorded an issue at CMUXCLITestAssertions.swift:45:13: Expectation failed: try lhs() == rhs()

Not re-run automatically: macos / app-host unit tests (changed suites) is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

CI fast guards failed on 2f49c49d27 (https://github.com/manaflow-ai/cmux/actions/runs/39215077752). It does not block the merge; a red guard merged into main breaks it for every open PR.
The log named no failed step; see the run.

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally.

Merge receipt for 2f49c49d27, merged 2026-10-19 19:48:01 UTC

  • Not verified at merge: app-host unit tests (failure), ci-status (failure), macOS status (failure)
  • Verified: backend migrations applied, macOS compile admission, CI fast guards, CI timing, detect-ios-changes, Fast static checks, GhosttyKit release check, guards (18), ios-tests, late-placement, linux-preflight, macOS admission gate, and 6 more
  • Skipped by policy: admission-placement, apply-production, apply-staging, browser, Claude wrapper regressions, CLI product tests, CLI regressions (${{ matrix.shard }}/3), close lapsed freezes, Dogfood build #​${{ github.event.pull_request.number }}, ios-simulator, ios-simulator-build, mobile-core-package, and 10 more
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions

Copy link
Copy Markdown
Contributor

Dogfood tours of 2f49c49d

cloud-machine-author-tour at 2f49c49d: not run

skipped: CI built this head on a runner pool whose products the UI test Macs cannot load, and media never compiles one; gh workflow run pr-media.yml -f pr=&lt;n&gt; -f allow_compile=true does

Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

The red macos / app-host unit tests (changed suites) comes from CMUXCLIErrorOutputRegressionTests.testBareRelativeDirectoryPathOpenBypassesProtectedSocketForExternalCLI (CLI code this PR doesn't touch). The same test fails on main: run 39232003699, job 118001773763. macOS status and ci-status are red only because they roll up that job. A correctness review of 2f49c49 found no blockers.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

merge-override: Only red is CMUXCLIErrorOutputRegressionTests.testBareRelativeDirectoryPathOpenBypassesProtectedSocketForExternalCLI (CLI, untouched here). The same test fails on main run 39232003699, job 118001773763 (sharded app-host unit tests 3/7). macOS status and ci-status only roll it up. CmuxCloud swift-package-tests pass; review found no blockers.

@teamleaderleo
teamleaderleo merged commit 0656224 into main Oct 19, 2026
@teamleaderleo
teamleaderleo deleted the fix/cloud-link-process-group branch October 19, 2026 19:48
@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant