Skip to content

build: skip unchanged diff sidecar builds - #13212

Merged
teamleaderleo merged 6 commits into
mainfrom
perf/upstream-diff-sidecar-sweep
Sep 20, 2026
Merged

teamleaderleo merged 6 commits into
mainfrom
perf/upstream-diff-sidecar-sweep

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Reviewer summary

Skips diff sidecar builds when the inputs are unchanged and keeps the normal rebuild path when they are not.

Checks

Remove the unconditional alwaysOutOfDate override from the Diff Sidecar phase. Its Rust manifest, lockfile, toolchain file, production sources, and build/verification scripts are already declared as inputs, and the generated sidecar path is declared as an output. Unrelated Swift edits can therefore avoid rerunning the release Cargo build.

This is based directly on current upstream main; it does not copy the fork-only branch and does not claim an end-to-end speedup (see the measurement above).

Build-setting contract

The phase declares an architecture/deployment-target keyed stamp and the script writes it only after the verified destination artifact is complete. Each executed Xcode phase retires stamps for other architecture/deployment configurations before producing the shared bundled binary, so only the stamp corresponding to that shared artifact survives. The Xcode phase explicitly pins CMUX_DIFF_SIDECAR_ARCHS and CMUX_DIFF_SIDECAR_MIN_MACOS to the corresponding Xcode build settings before invoking the script. Direct script callers retain their override support.

CMUX_DIFF_SIDECAR_MIN_MACOS and signing overrides remain script-level controls; changing them in a reused DerivedData path should be treated as invalidating the output and merits a follow-up keyed contract if those overrides become a supported Xcode configuration.

Testing

  • bash -n scripts/build-diff-sidecar.sh
  • ./scripts/check-pbxproj.sh
  • git diff --check
  • Reviewed the script's full production input list and output verification path.
  • The three-commit reviewed result has been replayed onto the latest upstream main. Skip/rebuild behaviour was validated locally (see the note at the top) on the same build files as the current head; the current head differs from the measured commit only by a rebase over Add Copy Debug Information to iOS Settings #13007 (iOS-only). CI macOS validation remains pending behind the repository's fork-workflow approval gate.

Summary by CodeRabbit

  • Build Improvements
    • Improved Diff Sidecar build tracking across supported architectures and macOS deployment targets.
    • Reduced unnecessary rebuilds while ensuring updates occur when the built component changes.
    • Improved consistency when switching build configurations.
    • Enhanced build reliability by cleaning up temporary tracking data after completed or interrupted builds.

Replaces #12972 (same commits, head branch moved into the org so it gets the build cache and can be kept current with main).

🤖 Generated with Claude Code


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

Removes the unconditional Diff Sidecar rebuild so unrelated app edits no longer rerun the release Cargo build when the Rust inputs are unchanged.

Build-setting contract

  • The Xcode phase pins CMUX_DIFF_SIDECAR_ARCHS and CMUX_DIFF_SIDECAR_MIN_MACOS to the matching build settings and declares a keyed stamp output so Xcode can skip the phase when inputs are unchanged.
  • The script writes the stamp only after the verified destination artifact is complete, and each executed phase retires stale stamps for other architecture and deployment-target configurations.
  • Direct script callers retain override support because the stamp is optional; changing CMUX_DIFF_SIDECAR_MIN_MACOS or signing overrides in a reused DerivedData path invalidates the output.

Written for commit fcb2436. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 4 commits September 19, 2026 09:19
The phase already declares its Rust source inputs and generated binary output, so Xcode can avoid rerunning the release sidecar build for unrelated app edits.

Prior-art: teamleaderleo/Glaeda and teamleaderleo/Tact
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 6 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: c1e11060-c7f0-4a67-b39d-8d3eb9a50c0d

📥 Commits

Reviewing files that changed from the base of the PR and between 692f2c0 and fcb2436.

📒 Files selected for processing (2)
  • cmux.xcodeproj/project.pbxproj
  • scripts/build-diff-sidecar.sh

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.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable regression remains in the incremental sidecar build contract.

Summary

The PR makes the Diff Sidecar build phase incremental while preserving verification before marking an output current.

  • Removes the unconditional alwaysOutOfDate setting.
  • Declares an architecture- and deployment-target-specific stamp output.
  • Removes stale configuration stamps before rebuilding the shared sidecar.
  • Writes the stamp atomically only after the destination binary has been built, verified, installed, and optionally signed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Xcode[Xcode dependency analysis] --> Check{Inputs or keyed stamp changed?}
    Check -->|No| Skip[Skip sidecar build]
    Check -->|Yes| Clean[Remove stale sidecar stamps]
    Clean --> Build[Build requested Rust architectures]
    Build --> Verify[Verify slices, deployment target, and artifact]
    Verify --> Install[Install and optionally sign bundled binary]
    Install --> Stamp[Atomically write keyed stamp]
Loading

Reviews (3) · Last reviewed commit: "Merge main to repair missing CmuxFoundat..."

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Repaired the failed compile admission by integrating main at 692f2c0, including the CmuxFoundation import fix from #13236. The failed logs reported missing AgentPIDProcessIdentity in the process helpers; this was a shared base failure. Current head: fcb2436.

Shell syntax and pbxproj parsing/normalization pass locally; the branch retains the reviewed sidecar stamp/input/output contract. The remaining PR diff preserves the original optimization scope. Fresh hosted static/web checks pass; native compile admission is running. No checks bypassed.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 20, 2026 18:05
@teamleaderleo
teamleaderleo merged commit b093335 into main Sep 20, 2026
39 of 40 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 20, 2026
e77a6b1 feat: add current-work reads and Find Work (manaflow-ai#13269)
6386ba5 Cache Ghostty CLI helper builds across local invocations (manaflow-ai#13206)
8f6c0ea ci: measure compiled test artifact transfer cost (manaflow-ai#13172)
55092b9 docs: add a concise guide for public CMUX writing (manaflow-ai#13257)
9e7d3be fix(web): preserve locale preference during prefetch (manaflow-ai#13255)
b093335 build: skip unchanged diff sidecar builds (manaflow-ai#13212)
b79d77d perf: skip unchanged bundled resource builds (manaflow-ai#13209)
4c19fcb feat: expose stable surface and workspace IDs in catalog reads (manaflow-ai#13247)
95fdfd7 ci: add safe stale run janitor (manaflow-ai#13143)
0bcf003 docs: make the contributor verification ladder explicit (manaflow-ai#13242)
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