Repository navigation
feat: implement issue #861 — [#850 Phase 1b] Make compliance audit + deploy compliance v<M>-stable-aware (flag bare tags) - #863
Conversation
…deploy compliance v<M>-stable-aware (flag bare tags)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
PR Summary by QodoEnforce major-scoped v- pins for RING + dev-lead stubs
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Pull request overview
Updates the org compliance audit and the standard-workflow deployment drift logic to enforce major-scoped v-form pins (v<M>-<tier>) for ring-managed stubs and dev-lead stubs, and adds/updates bats coverage to lock in the new policy for issue #861.
Changes:
- Compliance audit: require v-form pins for RING centralized stubs and dev-lead stubs, and tighten dev-lead
agent_refsync to match the fullv<M>-<tier>channel. - Deploy drift check: recognize v-form refs in templates, and treat bare-tier stubs as compliant only when the agent has no releases.
- Tests: expand bats coverage for the new drift / acceptance cases across deploy + compliance audit checks.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
scripts/compliance-audit.sh |
Enforces v-form pins for RING stubs and dev-lead stubs; updates drift messaging accordingly. |
scripts/deploy-standard-workflows.sh |
Extends drift detection to allow v-form refs and makes bare-tier acceptance release-aware. |
test/scripts/compliance-audit/centralized-stub-pins.bats |
Adds integration-style coverage to ensure bare tier pins are flagged and v-form pins are accepted for RING stubs. |
test/scripts/compliance-audit/dev-lead-stub-channel-sync.bats |
Updates mirror helpers + tests to extract/match full v<M>-<tier> channels and reject bare pins. |
test/scripts/deploy-standard-workflows/emit-vform.bats |
Adds deploy drift-check coverage for “released agent => bare is drift” and “no release => bare is compliant”. |
Comments suppressed due to low confidence (1)
scripts/deploy-standard-workflows.sh:332
is_already_compliantalways callsring_host_current_major(agh apirequest) before checking whether the existing stub is already pinned to a tier-correctv<M>-<tier>ref. This adds an avoidable network call for every already-compliant v-form stub, which can slow large sweeps and increase risk of GitHub API rate limiting.
Consider short-circuiting by first extracting existing_ref and returning early when ring_vform_tier_aligned passes; only call ring_host_current_major when you still need to decide whether a bare tier stub is allowed (no-release case).
local host major
host="$(cut -d/ -f1-2 <<< "$prefix")"
major="$(ring_host_current_major "$host" "$base")"
if [[ -z "$major" ]]; then
local ref
while IFS= read -r ref; do
[[ -n "$ref" ]] && grep -qF "${prefix}@${ref}" <<< "$existing_content" && return 0
done < <(ring_accepted_refs "$base" "$repo")
fi
# A stub already pinned to the repo's tier-correct `v<M>-<tier>` form (any
# major) is compliant regardless of release state. A WRONG-tier v-form — or a
# bare stub once the agent has a release — is not accepted here and stays drift.
local existing_ref
existing_ref=$(grep -oE "@${base}/[^[:space:]\"']+" <<< "$existing_content" | head -1)
existing_ref="${existing_ref#@}"
if [[ -n "$existing_ref" ]] && ring_vform_tier_aligned "$existing_ref" "$base" "$repo"; then
return 0
fi
return 1
Code Review by Qodo
Context used✅ Compliance rules (platform):
87 rules 1. Audit/deploy pin mismatch
|
| # bare-tier pins the fleet still carries; the wrong tier stays drift. | ||
| if stub_pin_acceptable "$decoded" "$reusable" "$canonical" "$legacy" \ | ||
| || { [ "$is_ring" = 1 ] && ring_major_form_acceptable "$decoded" "$reusable" "$chan" "$repo"; }; then | ||
| # Compliance rule (major-scoped-channels epic #657 F5, #861): |
There was a problem hiding this comment.
1. Audit/deploy pin mismatch 🐞 Bug ≡ Correctness
check_centralized_workflow_stubs() now accepts only major-scoped v<M>-<tier> pins for RING stubs, so a bare <agent>/<tier> pin is always flagged as drift. deploy-standard-workflows.sh still treats bare-tier pins as compliant when ring_host_current_major() finds no releases, so the deploy sweep may never remediate the audit finding for pre-release agents.
Agent Prompt
### Issue description
`scripts/compliance-audit.sh` now *unconditionally* flags bare `<agent>/<tier>` pins for RING reusables by removing the `stub_pin_acceptable` path for RING entries and requiring `ring_major_form_acceptable`. However, `scripts/deploy-standard-workflows.sh` explicitly keeps bare-tier stubs compliant when the reusable has **no release** (no current major), meaning the deploy sweep will not converge the fleet with the audit.
### Issue Context
- The shared ring model (`scripts/lib/ring-pins.sh`) defines `ring_host_current_major()` which returns empty when there are no release tags, and callers should fall back to bare-tier pins.
- Deploy logic already implements and tests this no-release grace.
### Fix Focus Areas
- scripts/compliance-audit.sh[1566-1576]
### Implementation guidance
Update `check_centralized_workflow_stubs()` so RING pin compliance is **release-aware**, matching deploy behavior:
1. Determine the current major for the reusable (via `ring_host_current_major` or an equivalent gh_api-based helper, ideally cached per agent for performance).
2. If there is **no current major** (empty), allow bare-tier pins (e.g., via `stub_pin_acceptable` using the computed bare canonical + legacy) the same way deploy considers them compliant.
3. If there **is** a current major, require the `v<M>-<tier>` form (current behavior).
Add/adjust integration tests to cover the no-release audit behavior (mirroring `emit-vform.bats`’s `unset GH_RELEASE_REFS` scenario) so audit+deploy cannot regress into disagreement again.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 884db6bdda6c95171b21f9b73c910a8a93af64e2
Review mode: triage-approved (single reviewer)
Summary
Makes the compliance audit and deploy drift-check major-scoped-channel aware (#657 F5): RING workflow stubs and the dev-lead stub must now pin v<M>-<tier> channels; a bare <agent>/<tier> pin is drift. The deploy-side bare-tier grace is now major-aware — bare pins stay compliant only while the agent has no release. 187+/84- across 2 scripts and 3 bats files.
Linked issue analysis
Closes #861 ([#850 Phase 1b]). All three scope items are substantively addressed: (1) check_centralized_workflow_stubs now requires ring_major_form_acceptable for RING reusables and flags bare tier pins, with the finding message naming the expected v<M>-<tier> form; (2) check_dev_lead_stub pin regex + uses_channel extraction require dev-lead/v[0-9]+-(stable|next|ring[0-9]+) and agent_ref must match the full v-form (major split-brain caught); (3) is_already_compliant in deploy-standard-workflows.sh accepts v-form template refs into the ring branch and grants bare-tier grace only when ring_host_current_major is empty. Audit stays major-agnostic on tier as required. Guardrails respected: no standards/workflows/*.yml edits, no scripts/lib/ring-pins.sh changes, no consumer-repo edits. All helpers the new code calls (ring_tier_for_repo, ring_host_current_major, ring_accepted_refs, ring_vform_tier_aligned) verified present at head. Dependency ordering satisfied — #860 (Part 1 template re-pins) is closed, so a live audit sees v-form templates.
Findings
No blocking findings.
- Logic verified against head: bare pin on a released agent falls through
is_already_compliantto thering_vform_tier_alignedcheck (fails → drift), matching the new emit-vform.bats cases; the no-release path preserves the existingring_accepted_refsgrace (dry-run.bats no-release case preserved as required). - Tests were strengthened, not weakened: new F5 integration tests exercise the whole check via mocked
gh_api, replacing the now-obsolete bare-tier transition test; dev-lead mirror helpers updated to the v-form policy including a bare-pin-yields-empty case and a major split-brain case. - Secret scanning MCP tool was not available in this run — gitleaks CI check is green; no secrets in the diff (shell logic + tests only).
- Minor (non-blocking): the stale NOTE at the top of
check_centralized_workflow_stubsstill says dev-lead is 'pinned to the dev-lead/stable channel' — could be updated to v-form wording in a follow-up.
CI status
All checks green: ShellCheck ✓, Lint ✓, Lint and bats ✓ (×2), CodeQL ✓, SonarCloud Quality Gate ✓, Secret scan (gitleaks) ✓, agent-shield ✓, Agent Security Scan ✓, npm audit ✓, CodeRabbit ✓. Remaining entries are expected SKIPPED (ecosystem-specific audits, dependabot). Bot review comments are quota/no-op noise only; dev-lead fix-reviews pass reported no-changes. No unresolved human review threads.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
| if { [ "$is_ring" = 1 ] && ring_major_form_acceptable "$decoded" "$reusable" "$chan" "$repo"; } \ | ||
| || { [ "$is_ring" != 1 ] && stub_pin_acceptable "$decoded" "$reusable" "$canonical" "$legacy"; }; then | ||
| continue | ||
| fi |
There was a problem hiding this comment.
Suggestion: The new ring-only acceptance path unconditionally rejects bare <base>/<tier> pins, but deploy logic still treats bare pins as compliant when an agent has no release major yet. This creates an audit/deploy contract split where audit reports unavoidable drift that deploy will not fix. Align this check with deploy by allowing bare-tier ring pins when no current major exists for that reusable host. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Compliance audit flags unfixable drift for pre-release ring stubs.
- ⚠️ Deploy sweep leaves some pre-release stubs unrepinned.
- ⚠️ Engineers see recurring non-actionable ci-workflows findings.Steps of Reproduction ✅
1. Run the org-wide audit script `scripts/compliance-audit.sh` (entrypoint at lines 1–40)
so that it iterates repositories and invokes `check_centralized_workflow_stubs "$repo"`
from the main loop at `scripts/compliance-audit.sh:2854-2860`.
2. In some repo `R`, ensure a centralized ring-managed stub (e.g.
`.github/workflows/auto-rebase.yml`) is pinned to a bare-tier channel such as `uses:
petry-projects/.github/.github/workflows/auto-rebase-reusable.yml@auto-rebase/stable`,
which matches the channel model wired through `centralized[]` and `ring_canonical_ref` in
`check_centralized_workflow_stubs` at `scripts/compliance-audit.sh:1490-1521`.
3. When `check_centralized_workflow_stubs` runs for `R`, it sets `is_ring=1` and
`chan="${reusable%-reusable}"`, then loads the stub into `decoded` (lines 59–65) and
evaluates the new condition at `scripts/compliance-audit.sh:1567-1577`: the RING path
calls `ring_major_form_acceptable "$decoded" "$reusable" "$chan" "$repo"`, which only
accepts `uses:` refs in major-scoped `<base>/v<M>-<tier>` form (implementation at
`scripts/compliance-audit.sh:1470-1483`), so a bare `auto-rebase/stable` pin is rejected
and the `if` falls through, producing a `non-stub-*.yml` drift finding even though the
stub matches the bare-tier ring channel.
4. Deploy the same stub via `scripts/deploy-standard-workflows.sh`: `is_already_compliant`
at `scripts/deploy-standard-workflows.sh:287-323` computes
`major="$(ring_host_current_major "$host" "$base")"` and, when the agent has no current
major (`major` empty), executes the `if [[ -z "$major" ]]` block at lines 55–62 to treat
any bare-tier ref from `ring_accepted_refs "$base" "$repo"` (defined in
`scripts/lib/ring-pins.sh:46-63`) as compliant, so the deploy sweep leaves the bare-tier
stub unchanged while the audit continues to flag it as non-compliant.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/compliance-audit.sh
**Line:** 1575:1578
**Comment:**
*Api Mismatch: The new ring-only acceptance path unconditionally rejects bare `<base>/<tier>` pins, but deploy logic still treats bare pins as compliant when an agent has no release major yet. This creates an audit/deploy contract split where audit reports unavoidable drift that deploy will not fix. Align this check with deploy by allowing bare-tier ring pins when no current major exists for that reusable host.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if ! printf '%s\n' "$decoded" | grep -qE "^[[:space:]]*uses:[[:space:]]*petry-projects/\\.github-private/\\.github/workflows/dev-lead-reusable\\.yml@dev-lead/v[0-9]+-(stable|next|ring[0-9]+)([[:space:]]|$)"; then | ||
| add_finding "$repo" "ci-workflows" "dev-lead-stub-pin" "error" \ | ||
| "The \`dev-lead.yml\` caller stub must pin a \`dev-lead\` channel tag — \`petry-projects/.github-private/.github/workflows/dev-lead-reusable.yml@dev-lead/<channel>\` where <channel> is \`stable\` (default), \`next\`, or \`ring<N>\`. Re-sync from \`standards/workflows/dev-lead.yml\`." \ | ||
| "The \`dev-lead.yml\` caller stub must pin a major-scoped \`dev-lead\` channel tag — \`petry-projects/.github-private/.github/workflows/dev-lead-reusable.yml@dev-lead/v<M>-<channel>\` where <channel> is \`stable\` (default), \`next\`, or \`ring<N>\` (a bare \`dev-lead/<channel>\` tier pin is drift). Re-sync from \`standards/workflows/dev-lead.yml\`." \ | ||
| "standards/ci-standards.md#dev-lead-agent" |
There was a problem hiding this comment.
Suggestion: This check now requires dev-lead/v<M>-<channel> unconditionally, but deployment still emits/accepts bare dev-lead/<channel> when no release major exists. That makes dev-lead.yml impossible to satisfy in no-release states and creates recurring compliance failures. Gate the strict v-form requirement on detected current major (or mirror deploy’s no-major fallback). [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Dev-lead stubs can fail compliance before first release.
- ⚠️ Deploy and audit disagree on dev-lead pin format.
- ⚠️ Teams face persistent dev-lead ci-workflows findings pre-release.Steps of Reproduction ✅
1. Run `scripts/compliance-audit.sh` so that the main audit loop at
`scripts/compliance-audit.sh:2835-2860` evaluates each repository and calls
`check_dev_lead_stub "$repo"` for non-meta repos (definition at
`scripts/compliance-audit.sh:1615-1780`).
2. In a non-meta repo that has adopted dev-lead, ensure `.github/workflows/dev-lead.yml`
is pinned to the legacy bare-tier channel that deployment emits when no release major
exists, e.g. `uses:
petry-projects/.github-private/.github/workflows/dev-lead-reusable.yml@dev-lead/stable`;
this is what `emit_ref_for` in `scripts/deploy-standard-workflows.sh:103-115` produces
when `ring_host_current_major` returns an empty major for dev-lead.
3. `check_dev_lead_stub` fetches and decodes the stub (lines 124–128) and then evaluates
the new pin rule at `scripts/compliance-audit.sh:136-142`: the `grep -qE` condition only
accepts `uses:` lines of the form `@dev-lead/v[0-9]+-(stable|next|ring[0-9]+)`, so any
bare `dev-lead/<channel>` pin is rejected and `add_finding` is invoked, yielding a
`ci-workflows/dev-lead-stub-pin` error that cannot be resolved while deployment continues
to emit bare-tier pins in no-major states.
4. If a maintainer attempts to satisfy the audit by pinning `@dev-lead/vM-stable` before
any `dev-lead/vM.*` tags exist on the host (`petry-projects/.github-private`), the next
run of `scripts/deploy-standard-workflows.sh` treats that v-form as drift:
`is_already_compliant` (lines 287–323) calls `ring_host_current_major`, sees `major`
empty, and only accepts bare-tier refs from `ring_accepted_refs`, so the deploy sweep
rewrites the stub back to `dev-lead/<channel>`, causing an oscillation where dev-lead.yml
cannot simultaneously pass both deploy and audit in a no-release-major state.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/compliance-audit.sh
**Line:** 1638:1641
**Comment:**
*Api Mismatch: This check now requires `dev-lead/v<M>-<channel>` unconditionally, but deployment still emits/accepts bare `dev-lead/<channel>` when no release major exists. That makes `dev-lead.yml` impossible to satisfy in no-release states and creates recurring compliance failures. Gate the strict v-form requirement on detected current major (or mirror deploy’s no-major fallback).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| host="$(cut -d/ -f1-2 <<< "$prefix")" | ||
| major="$(ring_host_current_major "$host" "$base")" | ||
| if [[ -z "$major" ]]; then | ||
| local ref | ||
| while IFS= read -r ref; do | ||
| [[ -n "$ref" ]] && grep -qF "${prefix}@${ref}" <<< "$existing_content" && return 0 | ||
| done < <(ring_accepted_refs "$base" "$repo") |
There was a problem hiding this comment.
Suggestion: The bare-tier grace is keyed only on major being empty, but major is also empty when ring_host_current_major fails to query tags (it returns empty on API failure). That means transient GH/API errors can incorrectly mark bare stubs as compliant and skip required re-pins. Differentiate “no releases found” from “lookup failed” (e.g., by status code/explicit sentinel) before allowing this grace path. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Major-scoped migration can silently skip some stubs.
- ⚠️ Bare-tier pins may persist after releases due to API errors.
- ⚠️ Drift detection during deploy becomes sensitive to transient failures.Steps of Reproduction ✅
1. Run `scripts/deploy-standard-workflows.sh` to perform a standards deployment sweep; for
each centralized workflow, the deploy logic calls `is_already_compliant
"$existing_content" "$template" "$repo"` defined at
`scripts/deploy-standard-workflows.sh:287-323` to decide whether the stub in a target repo
needs re-pinning.
2. For a ring-managed reusable whose template `uses:` line points at a channel ref
(matched by the `[[ "$ref_after" =~ ^${base}/(v[0-9]+-)?(stable|next|ring[0-9]+)$ ]]`
condition at `scripts/deploy-standard-workflows.sh:42-50`), control enters the ring
branch, and `host="$(cut -d/ -f1-2 <<< "$prefix")"` and `major="$(ring_host_current_major
"$host" "$base")"` are computed at lines 55–57.
3. `ring_host_current_major` is implemented in `scripts/lib/ring-pins.sh:14-29` and calls
`gh api "repos/${host}/git/matching-refs/tags/${base}/v" --jq '.[]?.ref'`; on any
GitHub/API failure (network, auth, or rate limiting) the `gh api` command exits non-zero,
the `|| { ...; return 0; }` handler runs, it prints a warning and returns success with no
output, so `major` is set to the empty string exactly as in the legitimate “no tags exist
yet” case.
4. When `major` is empty due to this lookup failure, the `if [[ -z "$major" ]]` block at
`scripts/deploy-standard-workflows.sh:58-62` executes the bare-tier grace path: it
iterates over `ring_accepted_refs "$base" "$repo"` (defined in
`scripts/lib/ring-pins.sh:46-63`) and returns 0 as soon as it finds a matching bare-tier
ref in `existing_content`, marking the stub “already compliant” and skipping the intended
re-pin to a `v<M>-<tier>` channel even though the agent actually has published releases
and should be migrated.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/deploy-standard-workflows.sh
**Line:** 315:321
**Comment:**
*Incorrect Condition Logic: The bare-tier grace is keyed only on `major` being empty, but `major` is also empty when `ring_host_current_major` fails to query tags (it returns empty on API failure). That means transient GH/API errors can incorrectly mark bare stubs as compliant and skip required re-pins. Differentiate “no releases found” from “lookup failed” (e.g., by status code/explicit sentinel) before allowing this grace path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix…deploy compliance v<M>-stable-aware (flag bare tags) (#863) Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>



User description
Closes #861
Implemented by dev-lead agent. Please review.
CodeAnt-AI Description
Require major-scoped workflow pins for ring and dev-lead stubs
What Changed
v<M>-<tier>pin; bare tier pins like@.../stableare treated as drift once a release exists.v<M>-<tier>pins continue to pass, and wrong-tier pins are still flagged.dev-lead/v<M>-<tier>for both the workflow pin andagent_ref, so the two references stay on the same major and tier.Impact
✅ Fewer false-compliant workflow stubs✅ Earlier detection of outdated bare tier pins✅ Fewer split-channel dev-lead checkouts💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.