Stop backplane reference-run staleness from blocking all PRs (Health/summary) - #2821
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>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…suite
The backplane registry's reference-run evidence has a 7-day freshness window
(stale_after_hours=168). When it lapses, validate_registry() emitted a plain
finding, so test_registry_has_no_tbd_placeholders_and_validates — which runs in
`python ci` (the REQUIRED `summary` check) on every PR — went red, blocking ALL
Workflows PRs until someone manually refreshed the evidence. There is no
scheduled refresh, so this recurs weekly (it tripped 2026-07-24 ~14:29Z at 170.8h).
Freshness is an OPERATIONAL concern, not a structural defect: an aged-but-
well-formed reference run does not make the registry invalid. Separate the two:
- Finding gains a `severity` field ("error" default; "stale" for the freshness
lapse). Add blocking_findings() = structural (error) findings only.
- The general structural test asserts blocking_findings(...) == [], tolerating a
stale finding — so the required suite stops going red for unrelated PRs.
- The CLI (`main`) and the dedicated, NON-required backplane lane (health-78,
path-triggered) still exit 1 on staleness, so the freshness signal is preserved
where it belongs instead of gating the whole fleet.
- Tests: staleness is classified non-blocking; a deliberate-break proves
structural defects (missing sha256) still block. Future-dated/malformed
timestamps remain blocking errors.
Scope: only reference-run staleness is reclassified. Deeper follow-up (auto-
refresh the reference run, or schedule the conformance run) left for later.
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR canonicalizes GitHub Actions references during template-drift comparison, refreshes related allowlist fingerprints, and classifies stale backplane registry findings separately from blocking structural findings. ChangesTemplate drift normalization
Backplane registry finding severity
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Backplane Contract Integrity FailedA change to the Check that:
See |
Workflow source neededPR #2821 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. |
![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: dec8bbfd4c
ℹ️ 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 paired workflows use the same action path but genuinely different refs—for example, root actions/checkout@v7 versus a template still pinned to v6—this replaces both refs with <pinned>, so Health 74 reports them as synchronized. It likewise hides reusable-workflow ref changes such as @main versus an unrelated branch. Since the repository has dedicated action-version synchronization and requires unintentional consumer drift to be aligned, normalize only equivalent floating/pinned versions (or Renovate-only digest changes), not every ref indiscriminately.
AGENTS.md reference: AGENTS.md:L37-L43
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: 3
🤖 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 `@scripts/check_template_drift.py`:
- Around line 101-114: Make _canonicalize_action_refs context-aware so it only
canonicalizes actual YAML uses keys, not matching text inside literal block
scalars such as run: |. Track scalar context or use structured workflow parsing
while preserving existing action-path behavior, and add regression coverage
proving refs inside block-scalar content remain unchanged.
- Around line 88-114: Update _canonicalize_action_refs to preserve and re-emit
the closing quote captured by _USES_REF_RE when the uses value is quoted, while
retaining unquoted normalization behavior. Add a regression test covering a
quoted actions/checkout reference and verify the canonicalized result remains
properly terminated.
In `@tests/test_backplane_registry.py`:
- Around line 58-60: Update the test around validate_registry and
blocking_findings to filter findings for the missing reference_run_sha256 case,
then assert that the filtered result contains the expected finding path and
message. Replace the broad blocking-findings assertion so the test specifically
verifies deletion of reference_run_sha256 is not exempted by the stale
carve-out.
🪄 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: 3a8b3c98-3180-4e62-a50b-e65a00da5f44
📒 Files selected for processing (5)
config/template-drift-allowlist.txtscripts/check_template_drift.pyscripts/validate_backplane_registry.pytests/scripts/test_check_template_drift.pytests/test_backplane_registry.py
| _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 the closing quote when canonicalizing quoted values.
The matcher tolerates quotes, but the returned string drops the closing quote. For example, uses: "actions/checkout@v7" normalizes to an unterminated quoted value. Capture and re-emit the closing quote, then add a quoted-input regression test.
Proposed fix
- (?P<rest>['"]?\s*(?:\#.*)?)$""",
+ (?P<closing_quote>['"]?)\s*(?:\#.*)?$""",
...
- return f"{match.group('prefix')}{match.group('action')}@{_PINNED_REF}"
+ return (
+ f"{match.group('prefix')}{match.group('action')}"
+ f"@{_PINNED_REF}{match.group('closing_quote')}"
+ )As per path instructions, changed Python behavior should have accompanying test coverage.
📝 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.
| _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}" | |
| _USES_REF_RE = re.compile( | |
| r"""^(?P<prefix>\s*(?:-\s*)?uses:\s*['"]?) | |
| (?P<action>[^@'"\s]+) | |
| @(?P<ref>[^\s'"#]+) | |
| (?P<closing_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 | |
| return ( | |
| f"{match.group('prefix')}{match.group('action')}" | |
| f"@{_PINNED_REF}{match.group('closing_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 88 - 114, Update
_canonicalize_action_refs to preserve and re-emit the closing quote captured by
_USES_REF_RE when the uses value is quoted, while retaining unquoted
normalization behavior. Add a regression test covering a quoted actions/checkout
reference and verify the canonicalized result remains properly terminated.
Source: Path instructions
| 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 | 🏗️ Heavy lift
Make uses: canonicalization YAML-context aware.
Because this runs on every physical line, a literal block such as run: | containing uses: config@v1 is canonicalized even though it is script/config content, not an Actions key. A ref change there is genuine drift but will be suppressed. Track block-scalar context or parse the workflow structure, and add a regression test.
As per path instructions, changed Python behavior should have regression coverage for this parsing boundary.
Also applies to: 127-127
🤖 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 101 - 114, Make
_canonicalize_action_refs context-aware so it only canonicalizes actual YAML
uses keys, not matching text inside literal block scalars such as run: |. Track
scalar context or use structured workflow parsing while preserving existing
action-path behavior, and add regression coverage proving refs inside
block-scalar content remain unchanged.
Source: Path instructions
| findings = vbr.validate_registry(registry) | ||
|
|
||
| assert vbr.blocking_findings(findings) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the missing-SHA finding explicitly.
assert vbr.blocking_findings(findings) can pass because of an unrelated blocking finding, so it does not prove that deleting reference_run_sha256 survives the stale carve-out. Match the expected path and message in the filtered list.
As per path instructions, prioritize correctness, error handling, and test coverage.
Proposed assertion
- assert vbr.blocking_findings(findings)
+ assert any(
+ finding.path.endswith(
+ ".reference_run_evidence.reference_run_sha256"
+ )
+ and finding.message == "must be sha256"
+ for finding in vbr.blocking_findings(findings)
+ )📝 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.
| findings = vbr.validate_registry(registry) | |
| assert vbr.blocking_findings(findings) | |
| findings = vbr.validate_registry(registry) | |
| assert any( | |
| finding.path.endswith( | |
| ".reference_run_evidence.reference_run_sha256" | |
| ) | |
| and finding.message == "must be sha256" | |
| for finding in vbr.blocking_findings(findings) | |
| ) |
🤖 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 `@tests/test_backplane_registry.py` around lines 58 - 60, Update the test
around validate_registry and blocking_findings to filter findings for the
missing reference_run_sha256 case, then assert that the filtered result contains
the expected finding path and message. Replace the broad blocking-findings
assertion so the test specifically verifies deletion of reference_run_sha256 is
not exempted by the stale carve-out.
Source: Path instructions
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Automated Status SummaryHead SHA: b7addc2
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
…ekly cadence (#2827) Follow-up to #2821. That PR stopped reference-run staleness from failing the REQUIRED suite; this addresses the underlying blocker: staleness still hard-failed the dedicated health-78 lane (its CLI exited 1), so the only way to clear the red was a manual reference-run refresh — which nothing schedules, so it recurs weekly. The contract docs establish the intended model: sibling freshness contracts (langsmith-observability-contract.md) make the scheduled path WARNING-ONLY with opt-in enforcement (enforce_block). The reference_run_evidence is a human-reviewed baseline (carries verifier issue/PR + disposition comment), so a 7-day wall-clock hard-fail on it is mis-calibrated. Align with the documented pattern: - validate_backplane_registry.py `main()`: exit non-zero only on BLOCKING (structural) findings. Reference-run staleness is emitted as a `::warning::` annotation (on stderr, so it surfaces even in --json mode) and is non-blocking by default. New `--enforce-freshness` opts into failing on staleness (mirrors langsmith's enforce_block). Report gains blocking_ok/blocking_count/stale_count and per-finding severity. Structural defects still fail (test-gated). - health-78: add a weekly `schedule:` (warning-only cadence surfacing) and a `workflow_dispatch` `enforce_freshness` input for a deliberate gate. With main() now warning-only, the lane goes green-with-warning on an aged baseline instead of stuck-red — no manual intervention needed to keep CI green. Tests: staleness warning-only (exit 0) with stale_count surfaced; --enforce-freshness fails; ::warning:: annotation emitted; structural defects still block (deliberate-break). 27 pass; black/ruff/actionlint clean. NOT in scope (needs a real cross-repo run, won't fabricate provenance): auto- REGENERATING the evidence by re-emitting the Pension-Data reference conformance run and writing back real provenance. Speccing that separately. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Why
The backplane registry's reference-run evidence has a 7-day freshness window (
stale_after_hours=168). When it lapses,test_registry_has_no_tbd_placeholders_and_validates— which runs inpython ci, part of the requiredsummarycheck — goes red on every PR, because it assertedvalidate_registry(...) == []. There is no scheduled refresh, so it recurs weekly. It tripped 2026-07-24 ~14:29Z (evidence generated 2026-07-17, now 170.8h > 168h), which is why PRs opened this afternoon (e.g. #2817) can no longer merge — and--admincan't bypass a failing required check (empty bypass_actors).Fix
Freshness is operational, not a structural defect — an aged-but-well-formed reference run doesn't make the registry invalid. So separate the two:
Findinggains aseverityfield ("error"default;"stale"for the freshness lapse). Newblocking_findings()returns structural (error) findings only.blocking_findings(...) == [], tolerating astalefinding → the required suite stops going red for unrelated PRs.main) and the dedicated, non-required backplane lane (health-78, which only triggers on backplane paths) still exit 1 on staleness → the freshness signal is preserved where it belongs, instead of gating the whole fleet.Verification
tests/test_backplane_registry.py: 24 pass, incl. the previously-failing structural test (now green on the live registry) + a deliberate-break proving structural defects (missing sha256) still block. Future-dated/malformed timestamps remain blocking errors.Scope / follow-up
Only reference-run staleness is reclassified. The deeper durable fix — auto-refreshing the reference run (or scheduling the conformance run) so freshness self-heals — is left as a follow-up; the same treatment could apply to
deferred issue expired, another time-based finding that would otherwise block later.Unblocks
summarywas red solely due to this.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
uses:pin changes) as non-drift, while still flagging true workflow logic changes.Documentation
Tests