Skip to content

ci: stop resolving SwiftPM in app-host test shards - #13386

Closed
teamleaderleo wants to merge 1 commit into
mainfrom
ci-6134-drop-consumer-spm-resolve
Closed

teamleaderleo wants to merge 1 commit into
mainfrom
ci-6134-drop-consumer-spm-resolve

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • remove SwiftPM cache restore/sanitize/resolve work from every app-host consumer
  • remove the broad-batch package-resolution retry, so the restored admission product is the only app/test input
  • keep the independent CmuxTerminalCore-Package focused regression explicit
  • add workflow guards requiring every app-host wrapper invocation to use the admission xctestrun with test-without-building

The producer already validates the compiled product against revision/Xcode/architecture. The restore path relocates all three xctestruns and stages the compiled PackageFrameworks before tests start. Consumers now own execution only.

Measured baseline and candidate

Green baseline run 35427062807 repeated SwiftPM cache restore + project package resolution in every app-host shard:

shard repeated setup
1 61s
2 67s
3 61s
4 66s
5 64s
6 58s
total 377s / 6.28 macOS runner-minutes

That work sat on every shard's pre-test path, so it also exposed about one minute of critical-path latency.

Fresh admission receipt from hosted run 35645388943:

  • queue to owned runner: 6.0s
  • source preparation: 72.8s
  • package readiness: 56.0s
  • shared compile: 343.6s
  • Swift warning validation: 0.17s
  • artifact publication: 132.1s
  • required admission runner: 690.6s / 11m30.6s
  • compiled product: 851,712,653 bytes

Hosted consumer proof on that head:

Coverage ownership on the refreshed head is unchanged: 2,782 broad selectors (1,134 suites + 1,648 methods), 60 whole suites owned by strict focused gates, plus the existing one-method renderer-memory focused gate. The sharding guard proves full strict suites are excluded from the broad pack exactly once.

Final current-main full-suite validation: run 35647765920 on head 8fb1bfa85d97448c6bed9d5f570981f4066a6644.

Failure accounting

This change adds no flaky retry. The existing classifier, crash accounting, per-test timeout, shard timeout, strict focused gates, and deterministic selector packing remain intact. A broad-batch failure now stays a failure instead of triggering a project-level SwiftPM resolve and batch rerun.

Related: #6134, #7471, #12180, #13095, #13325.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Warning

Review limit reached

Next included review available in 8 minutes.

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: e1be8206-7da7-4633-807a-c5090526219d

📥 Commits

Reviewing files that changed from the base of the PR and between b235279 and d2ef044.

📒 Files selected for processing (3)
  • .github/workflows/ci-macos.yml
  • tests/test_ci_product_publication.py
  • tests/test_ci_unit_test_spm_retry.sh

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: f63d5e00-4065-4f7d-a15e-865fe4ccab8e

📥 Commits

Reviewing files that changed from the base of the PR and between 7ad603e and b235279.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • tests/test_ci_product_publication.py
  • tests/test_ci_unit_test_spm_retry.sh

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


📝 Walkthrough

Walkthrough

The CI workflow now resolves and builds app-host test products in the admission job. App-host unit-test shards restore those products and run them without building or resolving Swift packages. Tests enforce this separation.

Changes

App-host product flow

Layer / File(s) Summary
Admission and consumer workflow
.github/workflows/ci.yml
The consumer job no longer caches, resolves, retries, or restages Swift packages. Failed batches keep their original status. The bundled Ghostty helper test no longer receives CMUX_SOURCE_PACKAGES_DIR.
Workflow contract validation
tests/test_ci_product_publication.py, tests/test_ci_unit_test_spm_retry.sh
Tests verify admission-artifact restoration and test-without-building execution. They reject package resolution, source-package caching, and project builds in the consumer job. Previous resolver retry simulations were removed.

Priority: ⬇️ Low

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

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant macos_compile_admission
  participant app_host_unit_tests
  participant restore_app_host_test_product
  participant xcodebuild
  macos_compile_admission->>xcodebuild: resolve and build app-host test product
  macos_compile_admission->>app_host_unit_tests: provide admission artifact
  app_host_unit_tests->>restore_app_host_test_product: restore compiled product
  restore_app_host_test_product-->>app_host_unit_tests: provide prebuilt xctestrun
  app_host_unit_tests->>xcodebuild: test-without-building
Loading
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 … 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 pull request changes only CI workflow logic and CI guard tests in three files. The diff removes SwiftPM cache/package-resolution steps and changes app-host tests to use restored .xctestrun…
Cmux Swift Actor Isolation ✅ Passed PASS: The review-scoped diff changes only CI YAML and Python/shell tests. It contains no production Swift changes and introduces no actor-isolation declarations or accesses. The custom check therefore…
Cmux Swift Blocking Runtime ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml, a Python test, and a shell test. The authoritative diff contains no .swift files and adds no production Swift synchronization or timin…
Cmux Browser Automation Off-Main ✅ Passed PASS: The PR changes only CI workflow and SwiftPM/product-publication guard tests. The authoritative diff contains no browser socket commands, WebKit callbacks, AppKit access, worker browser router ch…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml and two CI test files. The authoritative diff contains no .swift files and adds no references to RestorableAgentSessionIndex, `Shared…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml, a Python CI test, and a shell CI test. It introduces no production Swift, TypeScript, or JavaScript changes, and it does not substitute …
Cmux No Hacky Sleeps ✅ Passed PASS. The pull request does not introduce a hacky sleep. It removes the app-host resolver loop and its fixed sleep $((attempt * 5)) from .github/workflows/ci.yml. The workflow YAML is explicitly o…
Cmux Algorithmic Complexity ✅ Passed PASS: The PR changes only CI workflow code and test guards. The workflow removes the SwiftPM retry loop and package-cache scans; it adds no nested collection scan, per-target rescan, sorting/filtering…
Cmux Swift Concurrency ✅ Passed PASS: The reviewed range changes only workflow YAML, Python tests, and a shell test. It changes no cmux-owned Swift source. Added lines contain no background Dispatch queues, Combine app state, comple…
Cmux Swift @Concurrent ✅ Passed PASS: The reviewed diff changes only workflow YAML, a Python test, and a shell test. It adds no Swift source, nonisolated async function, @concurrent annotation, actor isolation, or async helper c…
Cmux Swift Package Boundaries ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml and two CI test files. It introduces no production Swift changes. The Swift package boundary rule applies to Swift changes that keep inde…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The PR changes only CI workflow and test guards. It changes no Package.swift, Xcode project package references, .gitignore, or Package.resolved file. The workflow moves app-host consumers …
Cmux Swift Logging ✅ Passed PASS. The authoritative diff changes only .github/workflows/ci.yml and two test/CI files. It changes no Swift, Objective-C, or runtime source files. The Swift logging rule applies only to logging ad…
Cmux User-Facing Error Privacy ✅ Passed PASS: The PR changes only .github/workflows/ci.yml and CI test files. The added text is limited to CI comments, assertions, and an internal test success message; the workflow changes remove SwiftPM …
Cmux Full Internationalization ✅ Passed PASS: The review-scoped diff changes only CI workflow and test files. It adds no Swift UI text, string-catalog or Info.plist entries, web messages, API copy, markdown, or changelog content. Added comm…
Cmux Swiftui State Layout ✅ Passed PASS — The pull request changes only CI YAML and shell/Python tests. The review-scoped diff contains no SwiftUI or Swift source changes and introduces no ObservableObject, @Published, GeometryReader, …
Cmux Architecture Rethink ✅ Passed PASS: The pull request changes only CI workflow and test-guard files; it changes no Swift source. The diff removes the app-host consumer's SwiftPM cache, resolution, restaging, and retry paths. It add…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only .github/workflows/ci.yml, tests/test_ci_product_publication.py, and tests/test_ci_unit_test_spm_retry.sh. The authoritative diff contains no Swift source or u…
Cmux Source Artifacts ✅ Passed PASS. The authoritative diff changes only .github/workflows/ci.yml, tests/test_ci_product_publication.py, and tests/test_ci_unit_test_spm_retry.sh. These are intentional workflow configuration, …
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS — The review range changes only .github/workflows/ci.yml, tests/test_ci_product_publication.py, and tests/test_ci_unit_test_spm_retry.sh. It contains no changed Swift file and no changed pa…
Title check ✅ Passed The title clearly and concisely describes the main change: app-host test shards no longer resolve SwiftPM dependencies.
Description check ✅ Passed The description is detailed and directly explains the change, rationale, measured impact, validation results, failure accounting, and related issues. It does not reproduce the template's Testing, Demo…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • 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.

@teamleaderleo teamleaderleo added the full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks. label Sep 21, 2026 — with ChatGPT Codex Connector
@github-actions

Copy link
Copy Markdown
Contributor

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

@greptile-apps

greptile-apps Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule issue remains.

Summary

The PR makes compile admission the sole owner of SwiftPM resolution and has app-host shards execute the restored compiled product directly.

  • Removes SwiftPM cache restoration, sanitization, package resolution, framework restaging, and resolution retry work from app-host consumers.
  • Retains the independent CmuxTerminalCore-Package regression.
  • Adds guards requiring app-host wrapper calls to use restored xctestrun manifests with test-without-building.
  • Updates the legacy SwiftPM retry test to enforce the build-once/test-many contract.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Compile admission] -->|Resolve SwiftPM| B[Build app-host test products]
  B --> C[Publish compiled product and xctestruns]
  C --> D[Restore product in each app-host shard]
  D --> E[Stage PackageFrameworks]
  E --> F[Run xctestrun test-without-building]
Loading

Reviews (4) · Last reviewed commit: "test(ci): enforce producer-owned SwiftPM..."

@cursor

cursor Bot commented Sep 21, 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.

Copy link
Copy Markdown
Collaborator Author

Hosted validation update for head 9be07a9, CI run 35589300329:

  • Compile admission passed and produced artifact ID 10634762816.
  • tests-build-and-lag restored the same product and passed.
  • All six app-host jobs restored the same 851,732,790-byte archive successfully. There is no Cache Swift packages, Resolve Swift packages, or project-level resolution step in the candidate shards.
  • On green baseline run 35427062807, the six shards spent 61s, 67s, 61s, 66s, 64s, and 58s respectively in the SwiftPM cache+resolve steps this PR removes: 377 seconds / 6.28 macOS runner-minutes per full run, before counting the tiny sanitize/restage steps. Critical-path exposure is about one minute.
  • Candidate shards transition from product restore/home setup directly into XCTest/focused gates; the semantic selector set and host-launch topology are unchanged.

The full run is red for source/test failures unrelated to this CI-only patch. swift-package-tests fails compiling TerminalSurfaceRendererCallbackTests; the app-host shards reach real tests and fail on assertion/time-limit cases. That is useful failure-accounting evidence: this optimization did not hide those failures or convert them to green.

No performance claim is being made from the candidate's total shard durations because the red suites/timeouts changed execution length. The measured claim is the removed setup work above.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 21, 2026 12:18
auto-merge was automatically disabled September 21, 2026 17:30

Pull request was closed

@teamleaderleo
teamleaderleo force-pushed the ci-6134-drop-consumer-spm-resolve branch from 9be07a9 to 7ad603e Compare September 21, 2026 17:30
@teamleaderleo teamleaderleo reopened this Sep 21, 2026
@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 21, 2026 17:37
auto-merge was automatically disabled September 21, 2026 19:30

Pull request was closed

@teamleaderleo
teamleaderleo force-pushed the ci-6134-drop-consumer-spm-resolve branch from b235279 to edc5d20 Compare September 21, 2026 19:30
@cursor

cursor Bot commented Sep 21, 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.

@teamleaderleo teamleaderleo reopened this Sep 21, 2026
@cursor

cursor Bot commented Sep 21, 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.

Copy link
Copy Markdown
Collaborator Author

@greptileai review

Port the build-once/test-many consumer cleanup onto the current reusable macOS workflow.
@teamleaderleo
teamleaderleo force-pushed the ci-6134-drop-consumer-spm-resolve branch from 3a2fa8e to d2ef044 Compare September 22, 2026 02:41

Copy link
Copy Markdown
Collaborator Author

Superseded by #13427, which now targets main and contains this commit plus the downstream app-host cleanup commits as one landing unit. Keeping the combined PR open gives the final topology one current-main merge ref and one required CI result.

auto-merge was automatically disabled September 22, 2026 03:03

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant