Skip to content

build: stabilize project resources for incremental builds - #12976

Open
teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/project-pif-incrementality
Open

teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:fix/project-pif-incrementality

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Two malformed resource entries in project.pbxproj make Xcode produce a different project description on every load, so it re-plans every build. Both are repaired, and both are needed (fixing only the first still gave 5 descriptions in 5 loads):

  • Resources/bin/open belonged to no group, so Xcode invented a Recovered References group with a new id each load. It is now in the Resources group.
  • The English cloud-agent-skill.md was not under en.lproj/ like its Japanese variant. It moves there; the coverage script and tests follow.

Behavior change: on main Xcode drops that variant group and ships the English file unlocalized, so the Japanese file is never selected. With this change, Japanese-preferring users get ja.lproj/cloud-agent-skill.md. The literal resourceURL/cloud-agent-skill.md fallback in CloudAgentSkillLauncher.skillMarkdown no longer matches anything.

Testing

Parent commit 9c2ba78be4 against this PR, separate worktrees and DerivedData, builds alternated, ./scripts/reload.sh --tag <tag> --prod-auth. M4 Pro, Xcode 26.3, on a machine other agents were also building on. The edit is one comment line in one Swift file.

project descriptions, repeated loads no-op build one-file edit build descriptions written
parent commit 3 in 3 35 s, 34 s 53 s, 55 s one per build
this PR 1 in 7 26 s, 27 s 50 s, 51 s 0

Commands for the project-description check are in the first comment.

Not explained: after the PR worktree sat idle for ~25 minutes, Xcode wrote a new build description anyway, twice, with the project description unchanged. The old and new descriptions differ only in the order of compiled <lang>.lproj/Localizable.strings entries. After the first gap the next one-file edit recompiled the whole cmux target (608 s); after the second it did not (52 s). The parent commit re-plans on every build, so it cannot show this either way. Something besides the project file invalidates the build description, and this PR does not address it.

Passed: tests/test_cloud_vm_skill_coverage.py, tests.test_normalize_pbxproj, ./scripts/check-pbxproj.sh. cmuxTests has not been run against the relocated resource, and no app test job ran on this PR's CI.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes two malformed Xcode project resource entries so incremental builds reuse a stable build description instead of synthesizing project references during project loading. No-op builds in the malformed state dropped from about 97–101s to 72–75s.

  • Attaches the orphaned Resources/bin/open file reference to the Resources group.
  • Moves the English cloud-agent-skill.md into Resources/en.lproj/ and updates the project file path plus the direct coverage/test references.

Written for commit 129fac9. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Documentation

    • Updated references to the bundled Cloud Agent skill documentation to use its English localization path.
    • Ensured the documentation is correctly included and discoverable within the app bundle.
    • Content remains unchanged; only its bundled location was updated.
  • Tests

    • Updated validation messages and coverage checks to reflect the localized documentation location.
    • Confirmed the bundled documentation remains available at its updated path.

@github-actions

github-actions Bot commented Sep 19, 2026

Copy link
Copy Markdown

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

@coderabbitai

coderabbitai Bot commented Sep 19, 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: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 3b07921c-8e2f-4f12-b0dc-d116bcbaa22c

📥 Commits

Reviewing files that changed from the base of the PR and between 7f4e0d1 and 129fac9.

📒 Files selected for processing (5)
  • Resources/en.lproj/cloud-agent-skill.md
  • Sources/Cloud/CloudAgentSkillLauncher.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/CloudAgentSkillLauncherTests.swift
  • tests/test_cloud_vm_skill_coverage.py
💤 Files with no reviewable changes (1)
  • Resources/en.lproj/cloud-agent-skill.md

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


📝 Walkthrough

Walkthrough

The bundled skill markdown moved to Resources/en.lproj/cloud-agent-skill.md. The Xcode project, launcher documentation, test message, and coverage check now use that path. The project group also includes an open file reference.

Changes

Skill resource localization

Layer / File(s) Summary
Project resource wiring
cmux.xcodeproj/project.pbxproj
The English resource reference now uses en.lproj/cloud-agent-skill.md. An open file reference was added to the group.
Reference and validation alignment
Sources/Cloud/CloudAgentSkillLauncher.swift, cmuxTests/CloudAgentSkillLauncherTests.swift, tests/test_cloud_vm_skill_coverage.py
Documentation, the test failure message, and the coverage path now use the localized skill resource.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: lawrencecchen, austinywang

Merge Risk: ⚪ Minimal · up to 129fa

The localized resource wiring preserves English fallback and intentionally serves Japanese content to Japanese users, so no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 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 scoped diff changes only a Markdown resource rename, Xcode resource metadata, a documentation comment, a test message, and a coverage path. No Cloud terminal creation, cmux-tui transport, re…
Cmux Swift Actor Isolation ✅ Passed PASS. The only production Swift diff changes a documentation comment in Sources/Cloud/CloudAgentSkillLauncher.swift; it adds no declaration, isolation annotation, protocol, model, Sendable reference…
Cmux Swift Blocking Runtime ✅ Passed PASS: The pull-request diff does not introduce or expand blocking or timing-based synchronization. The only production Swift change updates a documentation path in CloudAgentSkillLauncher.swift; the…
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only the Cloud skill resource path, Xcode resource-group membership, and related test/coverage references. No changed file contains a browser.* command, WebKit wait, s…
Cmux Expensive Synchronous Load ✅ Passed PASS. The authoritative diff changes only a documentation string in production Swift, a test message, project resource references, and resource paths. It adds or moves no `RestorableAgentSessionIndex.…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request does not substitute a cached value for a fresh authoritative read. The only production Swift change is a documentation path update; skillMarkdown and installSkillFile implem…
Cmux No Hacky Sleeps ✅ Passed PASS. The pull request changes a Swift documentation path, Xcode project resource references, a resource rename, a Swift test message, and a Python test path constant. The authoritative diff adds no s…
Cmux Algorithmic Complexity ✅ Passed PASS: The PR introduces no algorithmic complexity issue. The only production Swift change updates a documentation path; CloudAgentSkillLauncher executable logic is unchanged. The other changes updat…
Cmux Swift Concurrency ✅ Passed PASS. The authoritative diff changes only a Swift documentation path and a test failure message. It adds no Dispatch, Combine, completion-handler, or fire-and-forget Task pattern. The existing async/a…
Cmux Swift @Concurrent ✅ Passed PASS. The authoritative diff changes only a documentation comment in Sources/Cloud/CloudAgentSkillLauncher.swift and a test diagnostic string in the Swift files. The existing openAgent(_:) async i…
Cmux Swift Package Boundaries ✅ Passed PASS: The authoritative diff changes only one production Swift line, updating a documentation path in Sources/Cloud/CloudAgentSkillLauncher.swift. The Swift implementation and API are unchanged. The…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes cmux.xcodeproj/project.pbxproj only for the localized markdown path and the Resources/bin/open group entry. It does not change any SwiftPM package reference, Package.swift, …
Cmux Swift Logging ✅ Passed PASS: The reviewed diff changes only a resource path, a test failure message, the Xcode project resource reference, and a Python coverage path. No changed Swift line adds or materially changes print
Cmux User-Facing Error Privacy ✅ Passed PASS: The pull request does not add or materially change a user-facing error, alert, command output, API error body, or recovery message. The production changes only update a documentation path and Xc…
Cmux Full Internationalization ✅ Passed The PR introduces no user-facing Swift text. The only Swift change is a documentation comment, and the changed test message is covered by the test exemption. The Markdown file is a byte-identical rena…
Cmux Swiftui State Layout ✅ Passed PASS: The PR changes only a resource rename/path, an Xcode project group entry, and related documentation/test paths. The only changed Swift files contain no SwiftUI views or state/layout constructs, …
Cmux Architecture Rethink ✅ Passed PASS: The pull request does not introduce a Swift architectural-rethink violation. The only Swift change updates a documentation path; CloudAgentSkillLauncher behavior and ownership remain unchanged…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The authoritative diff changes one Swift documentation path in Sources/Cloud/CloudAgentSkillLauncher.swift and one test message. It introduces or materially changes no NSWindow, NSPanel, `…
Cmux Source Artifacts ✅ Passed PASS. The pull request changes only durable source, test, script, project configuration, and a localized product resource. The resource is a 100% rename to Resources/en.lproj/cloud-agent-skill.md; n…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The authoritative diff changes only one Swift file under Sources/Cloud, and its sole code change is a documentation path update. It adds no #if DEBUG or test-build guard, debug/test seam mem…
Title check ✅ Passed The title clearly identifies the main change: stabilizing Xcode project resources to improve incremental build behavior.
Description check ✅ Passed The description provides a detailed summary, explains the motivation, documents testing and measured results, and records known limitations. The Demo Video, Review Trigger, and Checklist sections are …
Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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 force-pushed the fix/project-pif-incrementality branch from 7f4e0d1 to c270c5f Compare September 19, 2026 01:02
@teamleaderleo

teamleaderleo commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

Reproducing the project-description check

Results are in the PR description. These are the commands.

Setup: a clean worktree with vendor/bonsplit and ghostty populated and a prebuilt GhosttyKit.xcframework present; without the xcframework, project loading fails with local binary target 'GhosttyKit' … does not contain a binary artifact.

Counting distinct project descriptions (~10 s per load after the first):

for i in 1 2 3; do
  xcodebuild -showBuildSettings -project cmux.xcodeproj -scheme cmux -configuration Debug \
    -destination 'platform=macOS' -derivedDataPath /tmp/pif-dd >/dev/null
done
ls /tmp/pif-dd/Build/Intermediates.noindex/XCBuildData/PIFCache/project | grep '^PROJECT@'

One PROJECT@… entry means every load produced the same description. main at e91468b846 gave three entries for three loads with the same content hash and a different mod value each time.

Diffing two dumps directly:

xcodebuild -project cmux.xcodeproj -dumpPIF /tmp/pif-1.json -clonedSourcePackagesDirPath /tmp/sp -skipPackagePluginValidation
xcodebuild -project cmux.xcodeproj -dumpPIF /tmp/pif-2.json -clonedSourcePackagesDirPath /tmp/sp -skipPackagePluginValidation
shasum /tmp/pif-1.json /tmp/pif-2.json

On main the dumps differ at two leaf ids, and every other difference is a signature derived from them: the synthesized Recovered References group at /groupTree/children[12] in the project PIF, and /buildPhases[2]/buildFiles[3]/guid in the cmux target PIF. With this PR the dumps are byte-identical.

@teamleaderleo
teamleaderleo force-pushed the fix/project-pif-incrementality branch from c270c5f to c59d039 Compare September 19, 2026 01:05
@cursor

cursor Bot commented Sep 19, 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 force-pushed the fix/project-pif-incrementality branch from c59d039 to 129fac9 Compare September 19, 2026 01:05
@cursor

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

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