Make Health 74 template-drift check pin-tolerant (stop the recurrence) - #2817
Conversation
…ng it Health 74 Template Drift went red on main after every Renovate action-pin bump (#2799, and #2795 queued next), forcing a manual allowlist re-baseline each time — the 4th such re-baseline just landed (#2815). Root cause: normalize_text only stabilized line endings/whitespace, so a `uses: <action>@<sha> # vX` bump changed the fingerprint and staled the allowlist, even though consumer templates SHA-pin by contract while root floats major tags (an intentional, permanent divergence). Fix at the source: canonicalize GitHub Actions `uses:` refs in normalize_text — `uses: <action>@<ref>` (+ trailing `# vX` comment) collapses to `<action>@<pinned>`. The action PATH is preserved, so swapping to a different action is still drift and genuine logic changes still register; only the mutable pin ref is ignored. This makes action-pin bumps and the pinned-vs-floating divergence invisible to the check, so the allowlist stops going stale on dependency bumps. - scripts/check_template_drift.py: add _canonicalize_action_refs; apply in normalize_text. - tests: 6 new cases — SHA bump not drift, pinned==floating, different action still drift, reusable-workflow ref bump not drift / path change is, and a deliberate-break proving real logic drift still fails despite canonicalization. - config/template-drift-allowlist.txt: regenerated all 15 fingerprints under the new normalization (reasons preserved; 2026-07-24b note added). Verified: a simulated checkout SHA bump leaves the checker green (exit 0) with NO re-baseline; a real logic change still fails (exit 1). 25 drift tests pass. Workflows-repo-internal check only (consumers never run it) — no fleet blast radius. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Workflow source neededPR #2817 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe template drift checker now canonicalizes GitHub Actions references before comparison, ignoring ref and pin changes while detecting action path and workflow logic changes. Tests cover these cases, and the allowlist fingerprints are re-baselined. ChangesTemplate drift canonicalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 890e84be74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match = _USES_REF_RE.match(line) | ||
| if not match: | ||
| return line | ||
| return f"{match.group('prefix')}{match.group('action')}@{_PINNED_REF}" |
There was a problem hiding this comment.
When the compared workflows call the same reusable workflow path but with different refs, e.g. stranske/Workflows/.github/workflows/reusable-10-ci-python.yml@main in the template and @v1 or a SHA in root, this normalization collapses both lines to the same <pinned> value and Health 74 reports no drift. That masks exactly the consumer-versioning drift this check should catch, since the repo contract says consumer templates currently reference reusable workflows with @main and new @v1 guidance requires coordinated policy/docs changes. Please restrict this canonicalization to third-party action SHA pins (or otherwise preserve reusable-workflow refs).
AGENTS.md reference: AGENTS.md:L29-L32
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/template-drift-allowlist.txt`:
- Around line 7-12: Update the re-baseline comment in
config/template-drift-allowlist.txt to state that fingerprints can also become
stale when consumer-template logic changes or action paths are substituted, in
addition to genuine root-workflow logic changes. Preserve the existing
explanation about action-pin canonicalization and dependency bumps.
In `@scripts/check_template_drift.py`:
- Around line 84-114: Update _canonicalize_action_refs and _USES_REF_RE so
quoted uses values retain their closing quote during canonicalization, while
unquoted values remain unchanged; ensure the output preserves matching quote
delimiters around the canonicalized action reference. Add a regression test
covering a quoted actions/checkout ref and its expected canonical form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8168fffb-902c-4fa8-8e5c-79ca288516d4
📒 Files selected for processing (3)
config/template-drift-allowlist.txtscripts/check_template_drift.pytests/scripts/test_check_template_drift.py
| # 2026-07-24b re-baseline: fingerprints regenerated after check_template_drift.py | ||
| # gained action-pin canonicalization (uses:<action>@<ref> collapses to @<pinned>). | ||
| # From here, Renovate action-pin bumps and the intentional pinned-vs-floating | ||
| # divergence no longer change these fingerprints, so this allowlist stops going | ||
| # stale on dependency bumps; only a genuine logic change to a root workflow will. | ||
| # |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify what still invalidates the allowlist.
The checker hashes both main_text and template_text, and preserves action paths. A consumer-template logic change or action-path substitution can also stale these fingerprints; it is not limited to genuine root-workflow logic changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/template-drift-allowlist.txt` around lines 7 - 12, Update the
re-baseline comment in config/template-drift-allowlist.txt to state that
fingerprints can also become stale when consumer-template logic changes or
action paths are substituted, in addition to genuine root-workflow logic
changes. Preserve the existing explanation about action-pin canonicalization and
dependency bumps.
| # Matches a GitHub Actions `uses:` line and captures the action path (owner/repo, | ||
| # owner/repo/subpath, or a reusable-workflow path) separately from its `@<ref>` | ||
| # pin and any trailing `# vX.Y.Z` comment. Optional surrounding quotes on the | ||
| # value are tolerated. Local `./path` actions have no `@ref` and never match. | ||
| _USES_REF_RE = re.compile( | ||
| r"""^(?P<prefix>\s*(?:-\s*)?uses:\s*['"]?) | ||
| (?P<action>[^@'"\s]+) | ||
| @(?P<ref>[^\s'"#]+) | ||
| (?P<rest>['"]?\s*(?:\#.*)?)$""", | ||
| re.VERBOSE, | ||
| ) | ||
|
|
||
| # Canonical placeholder that a pinned/floating action ref collapses to, so that a | ||
| # Renovate SHA bump or a pinned-vs-floating difference is not seen as drift. | ||
| _PINNED_REF = "<pinned>" | ||
|
|
||
|
|
||
| def _canonicalize_action_refs(line: str) -> str: | ||
| """Collapse a `uses: <action>@<ref>` pin to `<action>@<pinned>`. | ||
|
|
||
| The action PATH is preserved (so swapping to a different action is still | ||
| drift); only the mutable `@<ref>` + trailing version comment is canonicalized. | ||
| This makes action-pin bumps (Renovate) and the intentional pinned-vs-floating | ||
| divergence between root and consumer templates invisible to the drift check, | ||
| while genuine logic changes still register. | ||
| """ | ||
|
|
||
| match = _USES_REF_RE.match(line) | ||
| if not match: | ||
| return line | ||
| return f"{match.group('prefix')}{match.group('action')}@{_PINNED_REF}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve closing quotes during canonicalization.
The opening quote is consumed by prefix, but the closing quote is discarded with rest. Thus uses: "actions/checkout@v7" normalizes to a malformed quoted line and differs from the equivalent unquoted form, causing false drift. Preserve matching quote delimiters and add a quoted-value regression test.
🐛 Proposed fix
- r"""^(?P<prefix>\s*(?:-\s*)?uses:\s*['"]?)
+ r"""^(?P<prefix>\s*(?:-\s*)?uses:\s*)
+ (?P<quote>['"]?)
(?P<action>[^@'"\s]+)
@(?P<ref>[^\s'"#]+)
- (?P<rest>['"]?\s*(?:\#.*)?)$""",
+ (?P=quote)\s*(?:\#.*)?$""",
@@
- return f"{match.group('prefix')}{match.group('action')}@{_PINNED_REF}"
+ quote = match.group("quote")
+ return (
+ f"{match.group('prefix')}{quote}{match.group('action')}"
+ f"@{_PINNED_REF}{quote}"
+ )As per path instructions, prioritize correctness and test coverage for Python changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Matches a GitHub Actions `uses:` line and captures the action path (owner/repo, | |
| # owner/repo/subpath, or a reusable-workflow path) separately from its `@<ref>` | |
| # pin and any trailing `# vX.Y.Z` comment. Optional surrounding quotes on the | |
| # value are tolerated. Local `./path` actions have no `@ref` and never match. | |
| _USES_REF_RE = re.compile( | |
| r"""^(?P<prefix>\s*(?:-\s*)?uses:\s*['"]?) | |
| (?P<action>[^@'"\s]+) | |
| @(?P<ref>[^\s'"#]+) | |
| (?P<rest>['"]?\s*(?:\#.*)?)$""", | |
| re.VERBOSE, | |
| ) | |
| # Canonical placeholder that a pinned/floating action ref collapses to, so that a | |
| # Renovate SHA bump or a pinned-vs-floating difference is not seen as drift. | |
| _PINNED_REF = "<pinned>" | |
| def _canonicalize_action_refs(line: str) -> str: | |
| """Collapse a `uses: <action>@<ref>` pin to `<action>@<pinned>`. | |
| The action PATH is preserved (so swapping to a different action is still | |
| drift); only the mutable `@<ref>` + trailing version comment is canonicalized. | |
| This makes action-pin bumps (Renovate) and the intentional pinned-vs-floating | |
| divergence between root and consumer templates invisible to the drift check, | |
| while genuine logic changes still register. | |
| """ | |
| match = _USES_REF_RE.match(line) | |
| if not match: | |
| return line | |
| return f"{match.group('prefix')}{match.group('action')}@{_PINNED_REF}" | |
| # Matches a GitHub Actions `uses:` line and captures the action path (owner/repo, | |
| # owner/repo/subpath, or a reusable-workflow path) separately from its `@<ref>` | |
| # pin and any trailing `# vX.Y.Z` comment. Optional surrounding quotes on the | |
| # value are tolerated. Local `./path` actions have no `@ref` and never match. | |
| _USES_REF_RE = re.compile( | |
| r"""^(?P<prefix>\s*(?:-\s*)?uses:\s*) | |
| (?P<quote>['"]?) | |
| (?P<action>[^@'"\s]+) | |
| @(?P<ref>[^\s'"#]+) | |
| (?P=quote)\s*(?:\#.*)?$""", | |
| re.VERBOSE, | |
| ) | |
| # Canonical placeholder that a pinned/floating action ref collapses to, so that a | |
| # Renovate SHA bump or a pinned-vs-floating difference is not seen as drift. | |
| _PINNED_REF = "<pinned>" | |
| def _canonicalize_action_refs(line: str) -> str: | |
| """Collapse a `uses: <action>@<ref>` pin to `<action>@<pinned>`. | |
| The action PATH is preserved (so swapping to a different action is still | |
| drift); only the mutable `@<ref>` + trailing version comment is canonicalized. | |
| This makes action-pin bumps (Renovate) and the intentional pinned-vs-floating | |
| divergence between root and consumer templates invisible to the drift check, | |
| while genuine logic changes still register. | |
| """ | |
| match = _USES_REF_RE.match(line) | |
| if not match: | |
| return line | |
| quote = match.group("quote") | |
| return ( | |
| f"{match.group('prefix')}{quote}{match.group('action')}" | |
| f"@{_PINNED_REF}{quote}" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check_template_drift.py` around lines 84 - 114, Update
_canonicalize_action_refs and _USES_REF_RE so quoted uses values retain their
closing quote during canonicalization, while unquoted values remain unchanged;
ensure the output preserves matching quote delimiters around the canonicalized
action reference. Add a regression test covering a quoted actions/checkout ref
and its expected canonical form.
Source: Path instructions
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Automated Status SummaryHead SHA: bf2a202
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
1 participant
Why
Health 74 Template Drift went red on
mainafter every Renovate action-pin bump (#2799; #2795 is queued next), forcing a manual allowlist re-baseline each time — the 4th such re-baseline just landed in #2815. This PR fixes the root cause so it stops recurring, which is the systemic follow-up flagged in #2815.Root cause
normalize_textonly stabilized line endings + trailing whitespace. Consumer templates SHA-pin third-party actions by contract (docs/HISTORY.md/ #1925) while root floats major tags — an intentional, permanent divergence. So everyuses: <action>@<sha> # vXbump changed the normalized content → changed the fingerprint → staled the allowlist.Fix (chosen over a Renovate-flow auto-commit hook)
Canonicalize
uses:refs innormalize_text:uses: <action>@<ref>(+ trailing# vXcomment) collapses to<action>@<pinned>. The action path is preserved, so:I chose this over auto-refreshing the allowlist in the Renovate flow because it needs zero new automation, fixes all pin-bump sources (not just Renovate), and makes the check measure what it's actually for (logical drift, not cosmetic pin state). Blast radius is the Workflows repo only — this check compares root vs template and never runs in consumer repos.
Changes
scripts/check_template_drift.py: add_canonicalize_action_refs, apply innormalize_text.tests/scripts/test_check_template_drift.py: 6 new cases incl. a deliberate-break proving real logic drift still fails despite canonicalization.config/template-drift-allowlist.txt: regenerated all 15 fingerprints under the new normalization (reasons preserved; dated note added).Verification
tests/scripts+tests/workflows).Supersedes
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Chores