Skip to content

fix: pin nightly_detect_failure checkout to the tested SHA#214

Closed
kim-em wants to merge 1 commit into
nightly-testingfrom
fix/nightly-tag-race-condition
Closed

fix: pin nightly_detect_failure checkout to the tested SHA#214
kim-em wants to merge 1 commit into
nightly-testingfrom
fix/nightly-tag-race-condition

Conversation

@kim-em

@kim-em kim-em commented May 2, 2026

Copy link
Copy Markdown

This PR fixes a race condition in nightly_detect_failure.yml's handle_success job. The job was checking out ref: nightly-testing after CI completed, so if a new commit landed on nightly-testing between the start of CI and the firing of the workflow_run event, the nightly-testing-YYYY-MM-DD tag (and the nightly-testing-green / nightly-testing-daily branch updates) would point to an untested commit. Downstream consumers — daily.yml, nanoda, leanchecker, executable jobs — then test that untested commit, producing spurious failures attributed to nanoda/leanchecker rather than to the broken tag. See the discussion on Zulip, where @semorrison and @Kha diagnosed this.

The fix pins the checkout to ${{ github.event.workflow_run.head_sha }}, the SHA whose CI just succeeded, so the tag is always anchored to a tested commit. Side effect worth noting: nightly-testing-daily and nightly-testing-green will now track the most recent successfully-tested commit rather than the tip of nightly-testing at the moment a CI run finishes — which is what they were already documented to do.

Also fix the handle_failure Zulip message, which used ${{ github.sha }} — in a workflow_run event this is the SHA of the workflow's branch (the default branch), not the SHA of the failing run, so the linked commit was misleading whenever a new commit had landed during CI.

🤖 Prepared with Claude Code

This PR fixes a race condition in the `handle_success` job: the workflow
checked out `ref: nightly-testing` after CI completed, so if a new commit
landed on `nightly-testing` between the start of CI and the firing of the
`workflow_run` event, the `nightly-testing-YYYY-MM-DD` tag (and the
`nightly-testing-green` / `nightly-testing-daily` branch updates) would
point to an untested commit. Downstream consumers — `daily.yml`, nanoda,
leanchecker, executable jobs — then test that untested commit, producing
spurious failures attributed to nanoda/leanchecker rather than to the
broken tag.

Pin the checkout to `${{ github.event.workflow_run.head_sha }}`, the SHA
whose CI just succeeded, so the tag is always anchored to a tested commit.

Also fix the `handle_failure` Zulip message, which used `${{ github.sha }}`
— in a `workflow_run` event this is the SHA of the workflow's branch, not
the SHA of the failing run, so the linked commit was misleading whenever a
new commit had landed during CI.

🤖 Prepared with Claude Code
@kim-em

kim-em commented May 2, 2026

Copy link
Copy Markdown
Author

Closing — workflow changes belong on mathlib4 master, not on the nightly-testing repo. Reopening there.

@kim-em kim-em closed this May 2, 2026
@kim-em kim-em deleted the fix/nightly-tag-race-condition branch May 2, 2026 12:35
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