Skip to content

fix(ci): validate all test manifests before relocation - #20229

Merged
teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
BlueRaddish:fix/oct12-cmux-b
Oct 19, 2026
Merged

teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
BlueRaddish:fix/oct12-cmux-b

Conversation

@BlueRaddish

@BlueRaddish BlueRaddish commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Restoring an app-host test product rewrites each manifest as it goes. If a later manifest references a missing bundle, the restore fails after earlier manifests have already been relocated. Restore now validates and serializes every manifest before writing any of them, so a later validation failure leaves the downloaded product unchanged.

This protects against validation and serialization failures; it does not make multiple filesystem writes an atomic transaction.

Testing

  • The new regression fails on 4e81fbcf and passes on fdd01f61 with the same command:
    python3 tests/test_app_host_test_products.py TestProductHandoff.test_invalid_later_manifest_does_not_rewrite_earlier_products
  • python3 tests/test_app_host_test_products.py: all 15 tests pass on Linux using an exact source export. The new regression also passes on Windows. Two existing relocation fixtures assume POSIX path separators and fail identically on the original source on Windows.
  • Both commits ran the installed normal pre-commit hook successfully. verify-local.py --affected HEAD^ --list was used to inspect scope selection; it is a preview, not an executed static-check suite.

These checks exercise the portable product handoff code with real plist files. No Xcode build or macOS app execution was performed.

Changelog

none

Proof

CI tooling only; the regression compares all manifest bytes before and after the rejected restore.

Checklist

  • Behavior changes have added or updated tests
  • Reviewed with a subagent before merge (cmux-review), and all bot and human review comments resolved

AI assistance: Codex helped investigate, implement, and verify this change.


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


Summary by cubic

Fixes a bug in CI product restore where a later invalid manifest could leave earlier manifests already relocated. Restore now validates and serializes every manifest before writing any, so a validation failure leaves the downloaded product unchanged. Adds a regression test that fails on the old behavior.

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

View guided diff

Summary by CodeRabbit

  • Bug Fixes
    • Manifest files are now written only after all relocated manifests pass validation. If a manifest has a missing or unscoped test bundle, the restore operation fails without leaving previously processed manifests partially updated.

BlueRaddish and others added 2 commits October 13, 2026 06:41
@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in 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: 0aad1ab1-1f3d-41bb-90e1-63fbccb96964
📥 Commits

Reviewing files that changed from the base of the PR and between 370c51e and fdd01f6.

📒 Files selected for processing (2)
  • scripts/ci/app_host_test_products.py
  • tests/test_app_host_test_products.py

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

📝 Walkthrough

restore now buffers serialized manifest rewrites until it has processed every manifest. A regression test verifies that a later invalid manifest leaves transferred .xctestrun files unchanged.

Changes: Manifest restore

Layer / File(s) Summary
Buffer and validate manifest rewrites
scripts/ci/app_host_test_products.py, tests/test_app_host_test_products.py
restore writes buffered manifests after processing the loop. The regression test checks that an invalid manifest raises an error without changing transferred manifest bytes.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: teamleaderleo

Merge Risk: ⚪ Minimal · up to fdd01

The change defers manifest writes until processing is complete, and the regression test checks that a later invalid manifest does not alter transferred files. The reported tests pass, with no concrete unresolved merge risk indicated.

Pre-merge checks · Passed 24 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (24 passed)
Check name Status Explanation
Description check Passed The description includes the required Summary, Testing, Changelog, Proof, and Checklist sections. It provides clear behavior, test commands, limitations, and scope. The subagent-review checklist item …
Title check Passed The title is concise, specific, and accurately describes validating all test manifests before relocation.
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 The pull request changes only CI test-product manifest validation and its regression test. The diff adds in-memory manifest rewrites before filesystem writes; it does not create or modify Cloud termin…
Cmux Swift Actor Isolation Passed PASS: The pull request changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. The production change is Python, and the added test is Python. No Swift productio…
Cmux Swift Blocking Runtime Passed PASS: The pull request changes only Python files (scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py). The diff introduces no production Swift code or synchronization AP…
Cmux Browser Automation Off-Main Passed PASS: The pull request changes only Python product-restore logic and its Python regression test. It does not modify browser socket automation commands, Sources/TerminalController.swift, `ControlComm…
Cmux Expensive Synchronous Load Passed The pull request changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py, both Python files. The diff contains no production Swift changes and no agent-history, …
Cmux Cache Substitution Correctness Passed PASS: The pull request changes only Python files: scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It contains no production Swift, TypeScript, or JavaScript change, …
Cmux No Hacky Sleeps Passed The pull request changes a Python CI/runtime script and its tests, but it introduces no sleeps, timers, polling, fixed delays, or wall-clock waits. The changed logic only validates and buffers manifes…
Cmux Algorithmic Complexity Passed PASS. The production change adds one dictionary and one linear write pass in scripts/ci/app_host_test_products.py:177-184. Restore remains linear in the manifests and manifest contents; it does not …
Cmux Swift Concurrency Passed PASS: The pull request changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. The diff contains no Swift code and introduces no Dispatch queues, Combine state,…
Cmux Swift @Concurrent Passed PASS. The reviewed diff changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It contains no Swift files or Swift functions, so the @concurrent rules do not…
Cmux Swift Package Boundaries Passed The pull request changes only Python CI logic and Python tests: scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It introduces no production Swift changes or app-targ…
Cmux Swiftpm Lockfiles Passed PASS. The reviewed range changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It contains no SwiftPM package, Package.resolved, Xcode project, .gitignore…
Cmux Swift Logging Passed PASS — the pull request changes only Python CI code and Python tests. It adds no production Swift logging and does not add or materially change print, debugPrint, dump, NSLog, file logging, or…
Cmux User-Facing Error Privacy Passed PASS. The diff only changes write timing in scripts/ci/app_host_test_products.py and adds a regression test. The script runs from GitHub CI workflows and publishes test paths through GITHUB_ENV; n…
Cmux Full Internationalization Passed PASS. The diff changes only the CI product-relocation script and its regression test. It adds manifest validation/serialization and test assertions, with no user-facing Swift, web, metadata, API, mark…
Cmux Swiftui State Layout Passed The pull request changes only Python CI logic and its Python regression test: scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It introduces no SwiftUI code, Observab…
Cmux Architecture Rethink Passed PASS: The pull request changes only Python files: scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It introduces no Swift architecture change and does not introduce a…
Cmux Swift Auxiliary Window Close Shortcuts Passed The pull request changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. The diff contains no Swift or window declarations, so the auxiliary-window close-shortc…
Cmux Source Artifacts Passed The diff changes only two intentional repository files: the CI source script scripts/ci/app_host_test_products.py and its regression test tests/test_app_host_test_products.py. No logs, caches, tem…
Cmux No Test Or Debug Seam In Production Source Passed PASS: The pull request changes only scripts/ci/app_host_test_products.py and tests/test_app_host_test_products.py. It changes no Swift file under a production Sources/ path, so the specified tes…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Probably yours: Fast static checks failed: a static check failed, FAILED feature-flags (1.32s).

CI failed on fdd01f6156 (run 38360694294 attempt 4): 1 code.

Job Verdict Why
Fast static checks code a static check failed
Matched log lines
Fast static checks: FAILED feature-flags (1.32s)

Not re-run automatically: Fast static checks is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

@github-actions

Copy link
Copy Markdown
Contributor

CI fast guards failed on fdd01f6156 (https://github.com/manaflow-ai/cmux/actions/runs/38360694294). It does not block the merge; a red guard merged into main breaks it for every open PR.
The log named no failed step; see the run.

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally. This comment is updated in place on each push.

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BlueRaddish the test with a bad later manifest makes this convincing. Thanks!

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Merging at fdd01f6 (independent review SHIP). The only failing step is lint-feature-flags on an expired reviewBy date in Sources/FeatureFlags.swift ('agent-inbox-quick-view-enabled-release'), which failed on main for every PR until it was extended there. linux-preflight and ci-status fail only as its rollups. This PR doesn't touch that file.

@teamleaderleo
teamleaderleo merged commit 675e750 into manaflow-ai:main Oct 19, 2026
115 of 124 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for fdd01f6156, merged 2026-10-19 05:52:07 UTC

  • Not verified at merge: ci-status (failure), Fast static checks (failure), linux-preflight (failure)
  • Verified: backend migrations applied, CI fast guards, CI timing, GhosttyKit release check, guards (7), plan, web-validation
  • Skipped by policy: apply-production, apply-staging, browser, Claude wrapper regressions, Dogfood build #​${{ github.event.pull_request.number }}, macos, macOS admission gate, remote-daemon, suite-coverage, ui-tests, web, web-build, and 2 more
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 19, 2026
lawrencecchen added a commit that referenced this pull request Oct 19, 2026
…re; drop a stale fast-guard step reference

tests/test_app_host_test_products.py: main's #20229 test breaks a cmux-cli-tests
manifest, which the cmux-next profile does not have; it now breaks the fixture
scheme's only manifest. ci-guards.yml: the golden interactions judge step read
steps.fast-guard, a step this job does not have (base red in
tests/test_ci_guard_workflow_structure.py on feat-cmux-next b9429af).
teamleaderleo pushed a commit to teamleaderleo/cmux that referenced this pull request Oct 19, 2026
…cx-q1lnl)

37 main commits since 5da8121.

Conflict choices:
- modify/delete (36 files: legacy CLI/, Sources/, cmuxTests/, CmuxCloud,
  CmuxControlSocket, CmuxSettings RightSidebarWidth, CmuxTextActions,
  CmuxMobileShell test, Resources/Localizable.xcstrings, tmux wait-for
  fixture and test): kept deleted.
- cmux.xcodeproj/project.pbxproj, tests/test-execution.toml,
  docs/cli-contract.md: feat-cmux-next's side (main's `vm clean` contract
  rows not ported; the legacy Swift CLI is gone).
- scripts/gh-merge-green, tests/test_gh_merge_green.py: main's logic
  (manaflow-ai#21957 merge-freeze issues replace the WINDOW read). feat's manaflow-ai#21309 WINDOW
  freeze metadata is superseded by it; feat's Swift canary check name and the
  god-file path canonicalization stay.
- .github/workflows/ci-guards.yml: added main's review-date flag test and the
  stale agent PR step; dropped test_cloud_upgrade_rollout_retirement.py and the
  main full-suite steps (files absent on feat-cmux-next).
- scripts/ci/classify_failures.py: main's imports minus ui_tests_dispatch
  (module deleted on feat-cmux-next).
- scripts/ci/app_host_test_products.py: main's validate-then-write (manaflow-ai#20229),
  without the numeric-locale alias block feat-cmux-next removed.
- tests/test_ci_production_secrets_protected_env.py: feat-cmux-next's
  nightly track environment rule.
- skills/cmux-cloud-vm/references/commands.md: feat-cmux-next's side (no
  `vm clean` on the cmux-next CLI). skills/cmux-testing/SKILL.md: feat's rows
  plus main's web test verdict row.
- Run API: main's range has no Run API or staging dogfood/journey change;
  feat's manaflow-ai#21337 Run API stays as is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants