Skip to content

ci(seed): keep the trusted seed on the Mac before the R2 upload - #15411

Merged
teamleaderleo merged 2 commits into
mainfrom
ci/seed-keep-before-save
Sep 28, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
ci/seed-keep-before-save

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

The owned minis now take their j14 seeds from the LAN seed archive, which copies each kept seed off the trusted seed Mac within a minute of the keep. The keep ran after Save seed, so every LAN seed first waited for the R2 upload: p50 160 s, p90 203 s over 120 seed jobs on 2026-09-28.

This moves Keep the seed on this Mac ahead of Save seed. Keep only clones the DerivedData tree (seed_derived_data.py keep -> stash), and nothing between the two steps writes to it, so the kept copy is still exactly the seed the upload tars. The product steps still come after both.

One behaviour change: if the R2 upload fails, the Mac still keeps the seed. That seed is valid, and the LAN path can serve it. seed_decide.py still reads Save seed for R2 coverage, so decide is unchanged.

Measured context is on cmuxterm-hq#658. In short, seed age at use is set by production latency (commit to LAN archive, p50 19 to 33 min), not by transfer. This cuts about 2.7 min from that latency.

Tests: tests/test_seed_derived_data.py now pins keep before save and save before the product steps. test_seed_derived_data and test_seed_decide pass (86 tests).

🤖 Generated with Claude Code


Summary by cubic

Moves the trusted seed Mac's keep step ahead of the R2 upload so the LAN archive can serve seeds to the owned minis about 160 s sooner.

  • Keep only clones the DerivedData tree, so the kept copy is exactly what the upload tars.
  • If the R2 upload fails, the Mac still keeps a valid seed the LAN path can serve; seed_decide.py still reads Save seed for R2 coverage.
  • Tests now pin keep before save and save before the product steps.

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

Review in cubic

Summary by CodeRabbit

  • Build Improvements
    • Eligible build seed data is now retained on the Mac before being uploaded. Packaging continues after the seed is saved.
  • Tests
    • Updated workflow checks to verify the seed is retained before saving and that packaging follows the save.

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

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 3 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: 9cd42f66-0591-4b44-b108-54cdf16b03ca

📥 Commits

Reviewing files that changed from the base of the PR and between debe41c and d4ea62e.

📒 Files selected for processing (2)
  • .github/workflows/seed-derived-data.yml
  • tests/test_seed_derived_data.py

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: 203a42da-a2fc-4cc2-876f-8e5aa967e15f

📥 Commits

Reviewing files that changed from the base of the PR and between c9b235a and debe41c.

📒 Files selected for processing (2)
  • .github/workflows/seed-derived-data.yml
  • tests/test_seed_derived_data.py

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


📝 Walkthrough

Walkthrough

The workflow now retains the local seed before saving it to R2. The ordering test checks that seed adoption precedes retention, and that saving precedes package staging.

Changes

Seed Retention

Layer / File(s) Summary
Local seed retention ordering
.github/workflows/seed-derived-data.yml, tests/test_seed_derived_data.py
The workflow runs the conditional local retention step before saving the seed. The test checks that adoption precedes retention, retention precedes saving, and saving precedes package staging.

Priority: ➖ Normal

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

Change: Other

Merge Risk: ⚪ Minimal · up to debe4

The seed is kept locally before upload, with no confirmed issue requiring a fix before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to debe4

The seed is retained earlier on the trusted Mac, including when the remote upload fails. The review found no demonstrated new route for untrusted code to publish a seed, but could not verify the external archive or deployed runner configuration.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly exposed failure state is a seed retained on an eligible Mac before R2 publication, for possible use through the existing local/LAN path; it is not a new public workflow endpoint. The external archive’s behavior was not verified.

Trust Boundaries and Controls

  • inferred — No new attacker-controlled path through the changed test was found. The workflow’s existing gates remain, but their exclusion of a non-main dispatch from local retention depends in part on the deployed trusted-pool name not also being configured as a general pool; those values were unavailable.

Resilience and Maintainability Implications

  • observed — Keep is best-effort. The helper uses a temporary incoming path, retains an existing complete key on repetition, and prunes old temporary paths on a later promotion. The ordering test does not exercise interruption or concurrent workflow runs.

Hardening Proposals

  • proposed — If local retention must be main-push-only independently of pool configuration, add an explicit event and ref condition to local-cache setup rather than relying solely on the matrix and configured pool names.
🚥 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 1 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 seed-cache workflow ordering and its test. The diff does not create or modify Cloud terminal sessions, cmux-tui clients, transports, PTY readiness, manual renderers…
Cmux Swift Actor Isolation ✅ Passed The reviewed diff changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. It contains no Swift changes and introduces no actor-isolation behavior. The custom chec…
Cmux Swift Blocking Runtime ✅ Passed PASS: The reviewed diff changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. It contains no Swift changes and introduces no Swift blocking or timing-based sync…
Cmux Browser Automation Off-Main ✅ Passed PASS: The diff changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. It reorders seed retention and updates ordering assertions. It does not modify browser sock…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. The diff contains no Swift changes and no agent-history or synchronous Swift load. …
Cmux Cache Substitution Correctness ✅ Passed PASS: The PR changes only a GitHub Actions workflow and Python tests; it changes no production Swift, TypeScript, or JavaScript source. The workflow moves seed_derived_data.py keep before `Save seed…
Cmux No Hacky Sleeps ✅ Passed PASS: The PR changes only a GitHub Actions workflow and a test. The runtime rule explicitly excludes workflow YAML, and the added workflow step contains no fixed sleep, timer, polling, or wall-clock w…
Cmux Algorithmic Complexity ✅ Passed The pull request only moves the existing seed_derived_data.py keep workflow step before Save seed and updates a test. scripts/ci/seed_derived_data.py is unchanged. The diff adds no scalable-coll…
Cmux Swift Concurrency ✅ Passed The pull request changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. The reviewed diff contains no Swift code, so it does not introduce or expand any Swift co…
Cmux Swift @Concurrent ✅ Passed PASS: The reviewed diff changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. It contains no Swift files, Swift declarations, or Swift call-site changes. Theref…
Cmux Swift Package Boundaries ✅ Passed The authoritative PR diff changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. It contains no Swift or SwiftPM production changes. The Swift package-boundaries…
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. The workflow diff only moves the existing Keep the seed on this Mac step before Save seed; it a…
Cmux Swift Logging ✅ Passed The pull request changes only a GitHub Actions workflow and a Python test. It changes no Swift files and adds or materially changes no Swift logging. The Swift logging check is therefore not applicabl…
Cmux User-Facing Error Privacy ✅ Passed PASS: The diff changes only an internal GitHub Actions seed workflow and its test. The new step and comments are CI/operator-facing, and the seed_derived_data.py keep error output is unchanged from …
Cmux Full Internationalization ✅ Passed PASS: The PR changes only a GitHub Actions workflow and a Python test. The added workflow step name and comments are CI-operational text, not user-facing Swift, web UI, API, metadata, rendered markdow…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. The diff contains CI YAML and Python test ordering changes, with no Swift, SwiftUI,…
Cmux Architecture Rethink ✅ Passed PASS. The PR changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py; it contains no Swift changes. The workflow change reorders a CI seed-retention step before th…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. The authoritative diff contains no Swift or window-related files, so the Swift auxiliary-window clo…
Cmux Source Artifacts ✅ Passed The pull request changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. These are intentional workflow configuration and test source files. The diff adds no loca…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only .github/workflows/seed-derived-data.yml and tests/test_seed_derived_data.py. It changes no Swift file under a production Sources/ path, so it cannot introduce a tes…
Title check ✅ Passed The title clearly identifies the primary change: moving the trusted seed keep step before the R2 upload.
Description check ✅ Passed The description explains the problem, resulting behavior, important failure-mode change, measured impact, and tests executed. It omits explicit Changelog and Checklist sections, but the core informati…
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 1 files. (1 skipped: 1 unsupported.)

✨ 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.

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

CI failure attribution

CI passes on d4ea62e854 (run 36456176443 attempt 1).

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.

The LAN seed archive reads kept seeds from the trusted seed Mac within a
minute of the keep. Keep ran after "Save seed", so every LAN seed waited on
the R2 upload first (p50 160 s on 2026-09-28). Keep only clones the tree, so
the kept copy is still exactly the seed the upload tars.

cmuxterm-hq#658.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@teamleaderleo
teamleaderleo force-pushed the ci/seed-keep-before-save branch from debe41c to 1ea30d7 Compare September 28, 2026 17:02
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Reviewed the exact two-file change. Moving the APFS clone before the R2 upload preserves the same bounded seed tree, keeps product-mutating steps after both operations, and the workflow regression test pins the intended order. All required workflow guards are green; no blocking issue found.

— Mochi

@teamleaderleo
teamleaderleo merged commit 7f08715 into main Sep 28, 2026
53 checks passed
@teamleaderleo
teamleaderleo deleted the ci/seed-keep-before-save branch September 28, 2026 17:40
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for d4ea62e854: every check was green at merge (11 verified; 13 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 28, 2026
762c3ed Recover a Cloud machine graph stuck on an equal-cursor conflict (manaflow-ai#15328)
524ebff ci: replay the fuzz regressions on sidebar, split and window changes (manaflow-ai#15412)
818d475 Let a user's Cloud open dial even right after a background link failure (manaflow-ai#15291)
97491a7 Let the Cloud toolbar name the machine-list failure it has (manaflow-ai#15236)
0abac32 PR media: adopt CI's build only, start when CI completes, run for every app PR (manaflow-ai#15418)
7f08715 ci(seed): keep the trusted seed on the Mac before the R2 upload (manaflow-ai#15411)
5663c13 Finish the destroy work where a Cloud machine is first found gone (manaflow-ai#15359)

# Conflicts:
#	.github/workflows/ci.yml
#	.github/workflows/pr-media.yml
#	.github/workflows/seed-derived-data.yml
#	.github/workflows/test-e2e.yml
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