Skip to content

fix(nightly): the resolved build commit carries the tip's scripts/ci/nightly-steps - #31121

Merged
lawrencecchen merged 3 commits into
feat-cmux-nextfrom
fix/nightly-same-paths-steps
Nov 1, 2026
Merged

lawrencecchen merged 3 commits into
feat-cmux-nextfrom
fix/nightly-same-paths-steps

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up from the #31013 review (lead decision, 15:3xZ). Since #30912, the nightly runs step scripts from scripts/ci/nightly-steps/ in the resolved build commit's checkout, under the tip's workflow. CMUX_TUI_TREE_SAME_PATHS only made the build commit carry the tip's nightly.yml and Chief lockfile. So a published commit behind the tip, whose step scripts differ from the tip's, could run older copies of steps that the tip's workflow names.

Change

  • resolve-nightly-cmux-tui-client adds scripts/ci/nightly-steps to CMUX_TUI_TREE_SAME_PATHS.
  • pin-cmux-tui.sh compares git rev-parse <rev>:<path>. For a directory this is its tree object, so a candidate that differs from the tip in any step script is skipped. The message is "scripts/ci/nightly-steps differs from the tip", the same as for nightly.yml today.

Cost: when the tip changed a step script after the newest published commit, the nightly now takes a newer published commit, or builds the tip while its tree publishes (resolve-tree-pending.sh). That is the same as after a nightly.yml change today. In the last 14 days on feat, 8 of 41 step-script commits did not also change nightly.yml (nightly.yml changed in 239 commits). So this adds a skip rarely.

Out of scope, tracked in #31127: other scripts that the tip's workflow runs from the build commit's checkout (scripts/ci/*.py, scripts/cmux-next/*.sh, 90 steps in 19 jobs). The lead decided not to widen this list for them, because each path makes skips more frequent. #31127 proposes the root-cause fix: steps that run tip-workflow logic take their scripts from the workflow commit's checkout.

Red and green (test first, then fix)

The new case in scripts/cmux-next/tests/pin-cmux-tui-resolve-newest.test.sh runs nightly.yml's own CMUX_TUI_TREE_SAME_PATHS. It publishes a commit with an older step script, then a tip with a newer one. The resolver must refuse the older commit with a clear message, and must accept it once the tip's tree is published.

  • Red at 349227ff466 (test only): FAIL: nightly must not resolve a published revision with an older nightly step script.
  • Green at 24a7029fc3b (fix) and at this head: PASS.
  • cmux-next.yml already runs this test file. No CI job, step or gate is added.

Observed / Not run

Risk tier: high (risk_tier.py: nightly.yml is on the release path).

Changelog

none

…step scripts differ from the tip's (red)

Since #30912 the nightly runs step scripts from scripts/ci/nightly-steps/ in the
resolved commit's checkout, under the tip's workflow. A published commit behind the
tip whose step scripts differ would run older copies of steps the tip's workflow
names. The case runs nightly.yml's own CMUX_TUI_TREE_SAME_PATHS policy.

Red here: bash scripts/cmux-next/tests/pin-cmux-tui-resolve-newest.test.sh
  FAIL: nightly must not resolve a published revision with an older nightly step script
…nightly-steps

resolve-nightly-cmux-tui-client's CMUX_TUI_TREE_SAME_PATHS gains scripts/ci/nightly-steps.
pin-cmux-tui.sh compares `git rev-parse <rev>:<path>`, which for a directory is its
tree, so a candidate whose step scripts differ in any file from the tip's is skipped
with "scripts/ci/nightly-steps differs from the tip", as for nightly.yml and the Chief
lockfile. Follow-up from the #31013 review.

Cost: a nightly no longer builds a published commit behind the tip when the tip changed
a step script after it; it takes a newer published commit, or builds the tip while its
tree publishes (resolve-tree-pending.sh), as for a nightly.yml change today.

Green here: bash scripts/cmux-next/tests/pin-cmux-tui-resolve-newest.test.sh   PASS
@lawrencecchen lawrencecchen added the risk:high Risk tier (docs/ci/risk-tiers.md) label Nov 1, 2026
…-same-paths-steps

# Conflicts:
#	.github/workflows/nightly.yml
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 9fed58b: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr31121 -f ref=fix/nightly-same-paths-steps -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 9fed58b

What the reviewer confirmed:

  • For a directory, the resolver compares the tree object, so equal means byte-identical contents. A missing directory gives missing on both sides.
  • In the resolver's shallow checkout (fetch-depth 64, at most 50 commits searched), a directory path behaves the same as a file path. The list has no quoting or word-splitting problem.
  • Only nightly.yml:2725 sets the variable in nightly.yml. cmux-next-media.yml keeps its own value, and no test pins the old value.
  • Red at 349227ff466, green at this head. With the directory removed from the list, the test fails again. The test reads nightly.yml's actual value.
  • Cost: a tip that changes only step scripts still resolves, because its tree key equals the published key. The tree-pending path builds the tip. When no commit is found, the error says how to start a publisher. No commit pattern makes every nightly fail.
  • The merge adds only the word, the comment line and the test.

Non-blocking, and present before this PR: other scripts the tip's workflow calls from the build checkout (scripts/ci/*.py, scripts/cmux-next/*.sh) are not covered by the list yet.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 1, 2026
@lawrencecchen
lawrencecchen merged commit 302f848 into feat-cmux-next Nov 1, 2026
189 of 196 checks passed
@lawrencecchen
lawrencecchen deleted the fix/nightly-same-paths-steps branch November 1, 2026 16:29

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 9fed58b6 Deployed Nov 1, 2026 by lawrencecchen via cmux-tui tree preflight #96825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:high Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant