Skip to content

test: keep CmuxTerminal pasteboard tests off the cooperative pool - #15006

Merged
teamleaderleo merged 1 commit into
mainfrom
fix/cmuxterminal-pasteboard-test-isolation
Sep 27, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
fix/cmuxterminal-pasteboard-test-isolation

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

swift test --package-path Packages/macOS/CmuxTerminal could deadlock for good. In run 36314061362 it sat silent for 53.6 minutes until the job timeout. More often it froze for about 6 s, which is what fails remoteOutputDoesNotBlockTheMainActorOnNativeParser (its 5 s deadline expires while everything is frozen, as on #14876).

Every NSPasteboard call is a synchronous XPC request to the pasteboard server. The pasteboard suites ran in parallel on Swift Testing's cooperative pool, which has one thread per CPU. When every pool thread is waiting on the pasteboard server at once, the replies never arrive and the whole test process stops. A stack sample of the hung helper showed exactly that: 15 threads in _CFPBXPCSendMessageWithReplySync under CFPasteboardCreate / CFPasteboardBeginGeneration, and nothing else running. A standalone program that starts N concurrent tasks, each creating a named pasteboard, reproduces it on a 14-core mini: 13 tasks finish in 13 ms, 14 or more deadlock every time. The 6 s freezes are the same thing with one pool thread held by a test's 5 s C-stub wait, which frees the pool when the stub times out.

The six pasteboard suites now run on the main actor, where the app's pasteboard callers also run, so they no longer hold pool threads. Test code only.

Testing

On cmux7s (14 cores), swift test --package-path Packages/macOS/CmuxTerminal:

  • Before: a permanent hang on the first run. Repeated with --skip-build, about 1 run in 4 froze for 5.9 s and failed remoteOutputDoesNotBlockTheMainActorOnNativeParser. A variant that doubled the pasteboard calls per test without moving the suites hung 3 of 4 runs.
  • After: 15/15 sequential runs and 20/20 runs 5 at a time pass, all 326 tests in about 1.2 s, no test slower than 4 s.
  • PR CI: macos / swift-package-tests ran CmuxTerminal on a fleet mini and passed.

#14997 makes a hang like this fail after 120 s with the stuck tests named, instead of after 60 minutes.

Changelog

none

🤖 Generated with Claude Code

@cursor

cursor Bot commented Sep 27, 2026

Copy link
Copy Markdown

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.

@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

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

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 27e563d7-f2bb-43ad-9303-2587be47f921

📥 Commits

Reviewing files that changed from the base of the PR and between 7d17d13 and 12ff43d.

📒 Files selected for processing (4)
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/PasteboardRejectedHTMLFallbackTests.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceReviewRegressionTests.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardTemporaryImageAdoptionTests.swift

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: 0aa10d05-61b3-4b51-bfdf-0d453109024d

📥 Commits

Reviewing files that changed from the base of the PR and between 1921636 and 7d17d13.

📒 Files selected for processing (5)
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/IsolatedPasteboards.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/PasteboardRejectedHTMLFallbackTests.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceReviewRegressionTests.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift
  • Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardTemporaryImageAdoptionTests.swift

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


📝 Walkthrough

Walkthrough

Pasteboard service tests now use uniquely named standard and selection pasteboards instead of the host pasteboards. The test suites also add main-actor isolation where specified.

Changes

Pasteboard Test Isolation

Layer / File(s) Summary
Create isolated pasteboard helper
Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/IsolatedPasteboards.swift
Adds a helper to create uniquely named pasteboards, build a service with optional directory and file-manager arguments, and release both pasteboards.
Use isolated pasteboards in test suites
Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/PasteboardRejectedHTMLFallbackTests.swift, Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceReviewRegressionTests.swift, Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift, Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardTemporaryImageAdoptionTests.swift
Tests create services through the helper and defer pasteboard release. Specified suites are marked @MainActor.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 7d17d

The tests now use isolated pasteboards, and no supported change-related failure remains. Mergeability risk is minimal.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 7d17d

The tests now use separate pasteboards instead of the host clipboard, with cleanup tied to each test. No production behavior change or new security concern was identified, though security coverage is not complete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The observed change is confined to pasteboard tests and reduces their incidental interaction with the host clipboard; production callers retain their existing constructor path.

Trust Boundaries and Controls

  • observed — The test helper selects both pasteboard dependencies explicitly rather than falling back to either host pasteboard in the convenience initializer.

Resilience and Maintainability Implications

  • inferred — Deferred release in inspected callers preserves test ownership through normal completion and throwing exits; platform pasteboard-server failure behavior was not established.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
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 authoritative PR diff changes only CmuxTerminal pasteboard test helpers and test suites. It adds isolated pasteboards, main-actor annotations, and test service construction; it does not chan…
Cmux Swift Actor Isolation ✅ Passed The pull request changes only files under Packages/macOS/CmuxTerminal/Tests. The actor-isolation rule explicitly allows test changes. No production models, service protocols, Sendable reference type…
Cmux Swift Blocking Runtime ✅ Passed PASS. The authoritative diff changes only files under Packages/macOS/CmuxTerminal/Tests/..., which belong to the CmuxTerminalTests test target. The patch adds isolated pasteboard test scaffolding …
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only CmuxTerminal pasteboard test helpers and tests. The authoritative diff contains no browser socket automation commands, WebKit/AppKit worker-lane routing, or browser…
Cmux Expensive Synchronous Load ✅ Passed PASS: The authoritative diff changes only Swift test files under Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services. It adds test-only IsolatedPasteboards and updates test suites to use …
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only test files under Packages/macOS/CmuxTerminal/Tests/.... The diff adds and uses IsolatedPasteboards in test code, with no production Swift, TypeScript, or JavaSc…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR changes only Swift test files under Packages/macOS/CmuxTerminal/Tests. It adds isolated pasteboard test scaffolding and main-actor annotations. It does not change TypeScript, JavaScript…
Cmux Algorithmic Complexity ✅ Passed The review-scoped diff changes only Swift test files under Packages/macOS/CmuxTerminal/Tests/...; it does not change production Swift, TypeScript, JavaScript, shell, or runtime code. The added `Isol…
Cmux Swift Concurrency ✅ Passed PASS. The authoritative diff changes only Tests/ Swift files. Added code uses @MainActor, synchronous pasteboard helpers, and defer cleanup. It introduces no DispatchQueue, DispatchGroup, Co…
Cmux Swift @Concurrent ✅ Passed The diff adds @MainActor to pasteboard test suites and a synchronous IsolatedPasteboards helper. It adds no @concurrent or new nonisolated async declarations. The existing async regression tes…
Cmux Swift Package Boundaries ✅ Passed PASS: The authoritative diff changes only five Swift files under Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests. The new IsolatedPasteboards helper and all call-site changes are test code. No…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes only CmuxTerminal test source files under Packages/macOS/CmuxTerminal/Tests/.... It does not change Package.swift, Package.resolved, .gitignore, workflows, Xcode project f…
Cmux Swift Logging ✅ Passed PASS. The diff changes only CmuxTerminal test sources. It adds no print, debugPrint, dump, NSLog, Logger, or ad hoc file/stdout logging. The added code creates isolated pasteboards and updat…
Cmux User-Facing Error Privacy ✅ Passed The reviewed diff changes only test files under Packages/macOS/CmuxTerminal/Tests/.... It adds test-only IsolatedPasteboards and updates pasteboard test suites with @MainActor; it does not alter…
Cmux Full Internationalization ✅ Passed The authoritative diff contains only files under Packages/macOS/CmuxTerminal/Tests/.... The changes add a test-only pasteboard helper, @MainActor annotations, and test setup updates. No production…
Cmux Swiftui State Layout ✅ Passed PASS — The PR changes only CmuxTerminal test helpers and pasteboard test suites. The diff adds AppKit/Foundation pasteboard handling and @MainActor test annotations, with no SwiftUI import, view body,…
Cmux Architecture Rethink ✅ Passed PASS: The diff changes only CmuxTerminal test files. It adds main-actor isolation and per-test pasteboard injection to prevent synchronous pasteboard XPC calls from exhausting Swift Testing’s cooperat…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The authoritative diff changes only CmuxTerminal test files under Tests/CmuxTerminalTests/Services. It adds IsolatedPasteboards and updates pasteboard test setup; it adds no NSWindow, `NSP…
Cmux Source Artifacts ✅ Passed All five changed paths are intentional Swift test source files under Packages/macOS/CmuxTerminal/Tests/.... The added IsolatedPasteboards.swift is a hand-written test helper, and the other changes…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The pull request changes only Swift files under Packages/macOS/CmuxTerminal/Tests/.... The authoritative diff contains no changed Swift file under a production Sources/ path, so it introduce…
Title check ✅ Passed The title clearly identifies the main change: moving CmuxTerminal pasteboard tests off Swift Testing’s cooperative pool to prevent hangs and freezes.
Description check ✅ Passed The description is complete and relevant. It explains the deadlock cause, the implementation, detailed test results, and the internal-only changelog entry. The demo video is not applicable to this tes…
✨ 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

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

CmuxTerminal's package tests could deadlock for good (53.6 minutes until the
job timeout in run 36314061362) or freeze for about 6 s, which failed
remoteOutputDoesNotBlockTheMainActorOnNativeParser.

Every NSPasteboard call is a synchronous request to the pasteboard server.
The pasteboard suites ran in parallel on Swift Testing's cooperative pool,
one thread per CPU. When every pool thread is waiting on the pasteboard
server at once, the replies never arrive and the process stops. A standalone
program with 14 concurrent tasks that each create a named pasteboard
deadlocks on a 14-core mini; 13 tasks finish.

The pasteboard suites now run on the main actor, like the app's pasteboard
callers, so they no longer hold pool threads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo force-pushed the fix/cmuxterminal-pasteboard-test-isolation branch from 7d17d13 to 12ff43d Compare September 27, 2026 13:50
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@teamleaderleo
teamleaderleo merged commit 68d3936 into main Sep 27, 2026
56 of 57 checks passed
@teamleaderleo
teamleaderleo deleted the fix/cmuxterminal-pasteboard-test-isolation branch September 27, 2026 14:12
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 12ff43d08e: every check was green at merge (20 verified; 17 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 27, 2026
648d5c1 Add a Paste Last Screenshot action with an unbound shortcut (manaflow-ai#14955)
ff61677 ci: avoid partial blobs in catch-up merges (manaflow-ai#15023)
4d0d112 ci: retry transient catch-up GraphQL failures (manaflow-ai#15021)
212e808 ci: attribution scores a lone suspect and reports app-host crashes apart (manaflow-ai#14952)
4cabdf4 test: settle the window before measuring the unread sidebar-row invalidation (manaflow-ai#14568)
12ec99b Add a release-media capture tool for changelog screenshots and clips (manaflow-ai#15010)
ee2cda0 Backfill Unreleased changelog and draft next release cards (manaflow-ai#14999)
be4adf8 Show a brief notice when Cmd+V fails on an oversized image or a timeout (manaflow-ai#14953)
23d22d7 ci: an owned pool the run starts on now beats an earlier one it queues on (manaflow-ai#14993)
05d0190 ci: catch-up posts once per head, says less, and merges inserted declarations (manaflow-ai#15018)
4ee4b21 ci: fail stalled Swift package tests instead of waiting out the job timeout (manaflow-ai#14997)
9ce512a merge-main: run local guards only when asked (manaflow-ai#15016)
d60108a ci: clear test-e2e's fixed DerivedData with clear-dirs.sh (manaflow-ai#14994)
1d7895e ci: run the shell and CLI no-socket lanes in parallel (manaflow-ai#14990)
6e7d25f Honor macOS Differentiate Without Color, Increase Contrast and Reduce Transparency (manaflow-ai#14991)
966b355 Stop interrupting focused work: sidebar jumps, Computer Use focus steal, quit dialog on logout (manaflow-ai#14961)
e1f1cb2 Strip control characters from feedback attachment filenames (manaflow-ai#14783)
0758c9f test: find the onboarding window the test presented, not a leftover (manaflow-ai#15015)
b35c540 fix(spm): resolve GhosttyKit/GhosttyRuntimeTestStubs target name collisions (manaflow-ai#10569)
ef33bed Map .purs artifacts to the Haskell highlight.js grammar (manaflow-ai#14202)
e2a167a Highlight Elixir and Erlang files in the file editor (manaflow-ai#13732)
972c449 fix: wrap Linux browser download card label (manaflow-ai#11157)
f563884 Add Aside to browser data import detection (manaflow-ai#13379)
091d0ea Add cmux send --paste and hint at it for large multi-line sends (manaflow-ai#14937)
3ffcdbb test(ios): keep folder-tap stat tests off the real 2 s deadline (manaflow-ai#15017)
68d3936 test: keep CmuxTerminal pasteboard tests off the cooperative pool (manaflow-ai#15006)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant