Skip to content

Fix iOS screenshots job: make the cmux-ios test targets compile - #12867

Closed
austinywang wants to merge 3 commits into
mainfrom
fix-ios-workqueue-test-import
Closed

austinywang wants to merge 3 commits into
mainfrom
fix-ios-workqueue-test-import

Conversation

@austinywang

@austinywang austinywang commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

The release workflow's "Generate iOS App Store screenshots" job has failed on v0.64.24 (https://github.com/manaflow-ai/cmux/actions/runs/35016899628) and v0.64.25 (https://github.com/manaflow-ai/cmux/actions/runs/35225939389), which marks the whole release run red even though the macOS release publishes. fastlane builds the cmux-ios scheme's tests before capturing, and that build has been broken. xcodebuild stops at the first failing batch, so each fix exposed the next error:

  1. GhosttySurfaceWorkQueueTests.swift (added in 90bdd12) never imported its module: cannot find 'GhosttySurfaceWorkQueue' in scope. Added the @testable import CmuxMobileTerminal its neighbors use.
  2. Same file, once visible to Swift 6 checking: mutation of captured var 'order' in concurrently-executing code at eight sites (run https://github.com/manaflow-ai/cmux/actions/runs/35231375264). The recorded order now lives in an OSAllocatedUnfairLock<[String]>, the primitive this package already uses, captured as a let. Assertions unchanged.
  3. MobileWhatsNewReplayTests.swift (from Require explicit opt-in for iOS pairing #12316): recursive expansion of macro 'require', a #require nested in a #require (run https://github.com/manaflow-ai/cmux/actions/runs/35232344070). Hoisted the inner calls into locals. A tokenizer-based scan of all 881 iOS and shared test files for same-macro nesting found one more site, CmxPairingQRBitmapTests.swift, fixed the same way. The same scan confirmed GhosttySurfaceWorkQueueTests.swift was the only test file importing no module under test.

Root cause of the pile-up: nothing compiles the cmux-ios scheme's test targets at PR time. test-ios.yml is workflow_dispatch only and runs swift test on the host, which cannot build the UIKit-dependent targets, so the release-time screenshots job is the only compiler and its failure has been ignored. A PR-time build-for-testing of that scheme on Packages/iOS/**, Packages/Shared/**, and ios/** changes would close it; not included here.

Verification is a capture-only dispatch of ios-screenshots.yml on this branch (upload=false, en-US only); run links are in the comments. Test-only, no UI change, so no HIG page applies.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Improved test synchronization for surface work queue scheduling checks.
    • Clarified pairing QR compatibility test setup without changing expected behavior.
    • Improved readability of “What’s New” replay test setup while preserving existing assertions.

…ueTests

GhosttySurfaceWorkQueueTests.swift is the only file in CmuxMobileTerminalTests
that never imports CmuxMobileTerminal, so the internal GhosttySurfaceWorkQueue
class is out of scope and the cmux-ios scheme's test build fails with "cannot
find 'GhosttySurfaceWorkQueue' in scope". That build failure is what has made
the release workflow's iOS App Store screenshots job fail since the file was
added in 90bdd12.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor Author

Capture-only verification run on this branch (upload=false, en-US): https://github.com/manaflow-ai/cmux/actions/runs/35231375264

@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 17, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: ea480b65-fd52-42f9-9354-70051311a34a

📥 Commits

Reviewing files that changed from the base of the PR and between 8fad0e5 and 7430412.

📒 Files selected for processing (2)
  • Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/CmxPairingQRBitmapTests.swift
  • Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileWhatsNewReplayTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The tests update work queue synchronization and make intermediate test values explicit in pairing QR and WhatsNew replay tests. Assertions and expected behavior remain unchanged.

Changes

Test maintenance

Layer / File(s) Summary
Update work queue test synchronization
Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/GhosttySurfaceWorkQueueTests.swift
Adds the required imports. Replaces NSLock operations with OSAllocatedUnfairLock<[String]> and withLock closures.
Clarify test setup values
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/CmxPairingQRBitmapTests.swift, Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileWhatsNewReplayTests.swift
Assigns the pairing URL scheme and replay identifiers to local constants before API calls. Assertions remain unchanged.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Suggested reviewers: azooz2003-bit

Merge Risk: ⚪ Minimal · up to 74304

The changes only update test synchronization and setup evaluation without affecting product behavior.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 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 Swift Actor Isolation ✅ Passed PASS. The authoritative PR diff changes only three files under Tests paths. The changes add test imports, hoist test requirements, and replace a test-local mutable array with OSAllocatedUnfairLock…
Cmux Swift Blocking Runtime ✅ Passed PASS. The pull request changes only Swift files under three Tests targets. The only synchronization change replaces existing test-only NSLock access with OSAllocatedUnfairLock for deterministic …
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only three Swift test files: CmxPairingQRBitmapTests.swift, MobileWhatsNewReplayTests.swift, and GhosttySurfaceWorkQueueTests.swift. The patch contains no `browser…
Cmux Expensive Synchronous Load ✅ Passed PASS. The pull request changes only three Swift files under Tests/. The diff adds test imports, test setup, lock-protected test state, and local #require bindings. It does not add or move `Restora…
Cmux Cache Substitution Correctness ✅ Passed PASS. The authoritative diff contains only three test files under Tests/. It contains no production Swift, TypeScript, or JavaScript changes. The cache-substitution check therefore does not apply. T…
Cmux No Hacky Sleeps ✅ Passed PASS. The reviewed range changes only three Swift test files under Tests/. The patch adds module imports, hoists #require values, and replaces test-order locking with OSAllocatedUnfairLock. It i…
Cmux Algorithmic Complexity ✅ Passed PASS. The authoritative pull-request diff changes only three files under test directories. The changes update test setup and synchronization, including fixed-size test loops and assertion reads; no pr…
Cmux Swift Concurrency ✅ Passed PASS: The diff changes only three test files. It does not add background Dispatch queues, Combine state, completion-handler APIs, or fire-and-forget Tasks. The existing GhosttySurfaceWorkQueue.async…
Cmux Swift @Concurrent ✅ Passed PASS. The reviewed range changes only three test files. None of the changed declarations is async, nonisolated, or @concurrent; the three test functions remain synchronous. The `GhosttySurfaceWo…
Cmux Swift Package Boundaries ✅ Passed PASS. The authoritative diff changes only three files, and all three are under Tests/. The changes only adjust test imports, test synchronization, and local test setup. No production Sources/ code…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes only three Swift test files. The authoritative diff contains no Package.swift, Package.resolved, .gitignore, workflow, Xcode project, workspace, dependency, or package-reference c…
Cmux Swift Logging ✅ Passed PASS. The review-scoped diff changes only three files under Tests/. The additions update test setup and concurrency-safe assertions; they add no print, debugPrint, dump, NSLog, Logger, fil…
Cmux User-Facing Error Privacy ✅ Passed PASS. The authoritative diff changes only three files under test targets. The changes update test setup, locking, imports, and local test variables. They do not add or modify production user-facing er…
Cmux Full Internationalization ✅ Passed PASS. The authoritative PR diff changes only three files under Tests/: GhosttySurfaceWorkQueueTests.swift, CmxPairingQRBitmapTests.swift, and MobileWhatsNewReplayTests.swift. The changes add t…
Cmux Swiftui State Layout ✅ Passed PASS. The reviewed range changes only three test files. The diff adds test imports, lock-backed test data, and local variables for #require; it introduces no SwiftUI view, ObservableObject/`@Publi…
Cmux Architecture Rethink ✅ Passed PASS. The diff changes only three test files. The OSAllocatedUnfairLock<[String]> replaces an existing NSLock-protected test recorder and keeps the synchronization test-only. The rule explicitly a…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The authoritative diff changes only three Swift test files. The changes adjust test imports, locking, and #require bindings. They add no NSWindow, NSPanel, NSWindowController, SwiftUI `W…
Cmux Source Artifacts ✅ Passed PASS: The pull request changes only three existing Swift test source files under Packages/.../Tests/.... The diff adds test imports, lock-protected test state, and local requirement values. It adds …
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The authoritative diff changes only three Swift files under Packages/**/Tests/**. The diff contains no Swift file under a production **/Sources/** path, and no production-source #if DEBUG …
Cmux No Ambient Global State ✅ Passed PASS. The authoritative diff changes only three Swift files under Tests directories. No production Swift file changes. The new OSAllocatedUnfairLock is local test state, and the other edits only b…
Title check ✅ Passed The title clearly identifies the main change: fixing compilation of the iOS test targets used by the screenshots job.
Description check ✅ Passed The description explains what changed, why it changed, and how the fixes were verified. It does not include the template's review-trigger block or checklist, but the core Summary and Testing informati…
  • 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

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

With the module imported the file reaches Swift 6 checking, which rejects
mutating the captured `var order` inside the @sendable closures handed to
GhosttySurfaceWorkQueue ("mutation of captured var 'order' in
concurrently-executing code"), NSLock or not. Hold the recorded order in an
OSAllocatedUnfairLock<[String]>, the primitive this package already uses for
shared state, so the closures capture a Sendable `let`. Assertions unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@austinywang austinywang changed the title Fix iOS screenshots job: import CmuxMobileTerminal in GhosttySurfaceWorkQueueTests Fix iOS screenshots job: make GhosttySurfaceWorkQueueTests compile Sep 17, 2026
@austinywang

Copy link
Copy Markdown
Contributor Author

Second capture-only verification run, at 8fad0e57e6 with the Swift 6 recorder fix: https://github.com/manaflow-ai/cmux/actions/runs/35232344070

Swift rejects a macro expanded inside its own arguments: "recursive expansion
of macro 'require(_:_:sourceLocation:)'". MobileWhatsNewReplayTests (from
#12316) is the next failure in the cmux-ios scheme test build once
GhosttySurfaceWorkQueueTests compiles. A tokenizer-based scan of all 881 iOS
and shared test files found one more site of the same shape in
CmxPairingQRBitmapTests. Bind each inner #require to a local first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@austinywang austinywang changed the title Fix iOS screenshots job: make GhosttySurfaceWorkQueueTests compile Fix iOS screenshots job: make the cmux-ios test targets compile Sep 17, 2026
@austinywang

Copy link
Copy Markdown
Contributor Author

Third capture-only verification run, at 7430412d75 with the nested #require hoists: https://github.com/manaflow-ai/cmux/actions/runs/35233574429

@austinywang

Copy link
Copy Markdown
Contributor Author

Verification run 3 is green at 7430412d75: "Capture screenshots" succeeded and the screenshot artifacts were produced. https://github.com/manaflow-ai/cmux/actions/runs/35233574429

@lawrencecchen

Copy link
Copy Markdown
Contributor

Fleet instruction update for head 7430412d757dc1158f3503b07f974dd65c2dae64: this PR is classified ios. No macOS build tag is claimed. The current controller app recipe does not establish iOS/test readiness; that requires the appropriate validated recipe. Use cmux-ci for supported jobs, retain the returned ID and receipt, and wait on the same ID after any timeout. Do not use retired maclease allocation or post credentials. Exact-head tags will be posted only after the applicable build succeeds.

@github-actions

Copy link
Copy Markdown
Contributor

Automatic catch-up: main is green again and this branch needed it.

I tried to catch this branch up with main (26a1e88311e5), but these files need a person:

  • Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/GhosttySurfaceWorkQueueTests.swift: not a generated file; needs a person

Nothing was pushed. Merge main locally, fix those, and push; /catch-up is there again whenever you want it.

Automatic catch-up will not try this head again; a new push or /catch-up does.
Label the pull request no-auto-catch-up to opt out.

Catch-up run

@teamleaderleo

Copy link
Copy Markdown
Collaborator

The screenshot and Swift 6 test fixes shipped on main in #12995, commit 0251a44. Closing as superseded.

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.

3 participants