ci(adr): resolve the ADR checker from the effective tree, not the PR head - #1508
Merged
Merged
Conversation
The PR arm checks out the PR HEAD by design (the merge ref is stale), but GitHub takes the WORKFLOW from the merge ref. So a PR branched before #1481 gets the workflow file and not scripts/verify-adr-numbering.js, and the step dies with MODULE_NOT_FOUND before printing a single ADR line — red, with nothing an author can act on. Hit #1504 first; it would have hit most open PRs on their next event. Resolve the checker the same way the ADRs are resolved: the PR's version if it changed the script, main's otherwise. Verified against #1504's head (script absent): green, 29 ADRs. Same head with a synthetic second ADR-025 added: red, naming both files. Against main (script present): resolves from HEAD, green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ADR numbers are uniqueis red on #1504 with no ADR output in the log. That is my bug, not #1504's.The PR arm checks out the PR head on purpose —
refs/pull/N/mergeis recomputed lazily and can be badly stale. But GitHub takes the workflow file from the merge ref. So any PR branched before #1481 landed (12:27Z today) runs this workflow against a tree that has noscripts/verify-adr-numbering.js, and the step dies withMODULE_NOT_FOUNDbefore printing a single ADR line. #1504 was first; nearly every open PR would have hit it on its next event.Fix: resolve the checker the same way the ADRs are already resolved — the PR's version if the PR changed the script,
main's otherwise.Verified, all three from a real checkout at the affected head:
9bb229df(script absent)✓ 29 ADRs, 29 distinct numbersADR-025mainhead (script present)The mutation matters: without it, "green on #1504" is equally consistent with a guard that now checks nothing.
Unrelated but re-measured while testing: #1295 no longer adds a second ADR-025. It was rebased at 13:52Z and now only modifies ADR-025/027. The collision I filed against it is resolved.
🤖 Generated with Claude Code