Skip to content

feat: implement issue #1166 — pr-review is missing from the canary ring registry — autocut never advances its channel, so merged fixes sit inert (two incidents in three days) - #1168

Merged
don-petry merged 2 commits into
mainfrom
dev-lead/issue-1166-20260923-0122
Sep 23, 2026

Conversation

@don-petry

@don-petry don-petry commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem

pr-review is missing from the canary ring registry — autocut never advances its channel, so merged fixes sit inert (two incidents in three days)

From the issue: As a maintainer, I want pr-review registered in the canary ring registry, so that a merged fix to the review agent reaches its channel automatically instead of sitting inert on main until someone remembers cut-release.sh.

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

Tests added/updated: tests/canary_rollout.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

Closes #1166

Summary by CodeRabbit

  • New Features

    • Added the pr-review agent to canary rollout and ring management.
    • Added configuration for its rollout path, workflow trigger, and gating rules.
  • Bug Fixes

    • Drift audits now correctly recognize registered reusable workflows with grandfathered filenames.
    • Missing workflow files continue to be reported accurately.
  • Tests

    • Added coverage for workflow detection and pr-review registry configuration.

…ng registry — autocut never advances its channel, so merged fixes sit inert (two incidents in three days)
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cd547ad5-d519-40f5-b262-510bb3258841

📥 Commits

Reviewing files that changed from the base of the PR and between a1980bb and 3311a42.

📒 Files selected for processing (5)
  • scripts/canary-rollout.sh
  • scripts/lib/ring-pins.sh
  • scripts/pinned-version-report.sh
  • standards/canary-rings.json
  • test/scripts/lib/ring-pins.bats

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c028178f-e933-4150-b92d-ffda3010a6be

📥 Commits

Reviewing files that changed from the base of the PR and between b89e15b and a1980bb.

📒 Files selected for processing (5)
  • .github/workflows/canary-rollout.yml
  • scripts/canary-rollout.sh
  • scripts/lib/ring-pins.sh
  • standards/canary-rings.json
  • tests/canary_rollout.bats

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change registers pr-review in canary rollout configuration, adds support for its grandfathered workflow filename during drift checks, and adds tests for registration, ring ordering, present-file detection, and missing-file detection.

Changes

Canary rollout registration and drift handling

Layer / File(s) Summary
Register pr-review for canary rollout
.github/workflows/canary-rollout.yml, scripts/lib/ring-pins.sh, standards/canary-rings.json
Adds pr-review to manual dispatch choices, ring-managed reusable declarations, and the canary-ring registry.
Recognize registered workflow paths
scripts/canary-rollout.sh
Allows drift detection to recognize registered workflow paths such as pr-review.yml, while still reporting genuinely absent files.
Validate registration and drift behavior
tests/canary_rollout.bats
Tests present and missing grandfathered workflow files, registry fields, and host-relative ring ordering.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to a1980

pr-review is registered for canary rollout with bounded, health-gated progression; no merge-blocking issue remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: registering pr-review in the canary ring registry so autocut can advance its channel. It is longer than necessary, but it remains specific and related t…
Linked Issues check ✅ Passed Issue #1166 coding objectives are implemented. The registry adds pr-review with the .github-private host, pr-review.yml reusable, caller workflow name, and next through stable rings. `RING_R…
Out of Scope Changes check ✅ Passed The changes stay within issue #1166 scope. The canary-rollout.sh change and its Bats tests support registration of the legacy-named pr-review.yml reusable. No unrelated source, workflow, registry,…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (2 skipped: 2 …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1168
No changes were committed, but the PR still can't be marked done: required check SonarCloud is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-09-23T02:25:34Z

@don-petry

Copy link
Copy Markdown
Contributor Author

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check SonarCloud is still pending. I'll re-check automatically.
Next attempt after: 2026-09-23T02:25:34Z

@don-petry
don-petry enabled auto-merge (squash) September 23, 2026 01:55
@don-petry

Copy link
Copy Markdown
Contributor Author

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request registers the pr-review agent (a ring-0 self-host reviewer with a grandfathered legacy name pr-review.yml) into the canary rings configuration and updates the drift detection logic to prevent false-flagging grandfathered-named reusables as missing. It also adds corresponding test coverage. Feedback on the changes suggests robust shell scripting and jq improvements in _gh_list_reusables, such as stripping carriage returns, initializing variables, guarding jq execution, avoiding the optional/try operator, and using any for path matching.

Comment thread scripts/canary-rollout.sh Outdated
Comment thread scripts/lib/ring-pins.sh
Comment thread standards/canary-rings.json
@don-petry
don-petry disabled auto-merge September 23, 2026 01:56
coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 23, 2026
@donpetry-bot

donpetry-bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at a1980bb411c734c40b94b723cb9164c791571f48 — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: a1980bb411c734c40b94b723cb9164c791571f48
Review mode: triage-approved (single reviewer)

Summary

Registers pr-review in the canary ring registry (host petry-projects/.github-private, engine pr-review.yml, run_workflow = caller-stub display name), syncs RING_REUSABLES and the canary-rollout.yml dispatch enum, and fixes cmd_drift so a present grandfathered-named registered reusable is not false-flagged missing-file. Core change is correct and well-tested, but two unresolved codeant review threads flag a verified real gap: pinned-version-report.sh cannot see the grandfathered pr-review.yml, so the agent is silently omitted from version/drift reports.

Linked issue analysis

Issue #1166 (register pr-review so autocut advances its channel) is substantively addressed: the registry entry carries host, reusable path, run_workflow display name (with the #747 graceful-empty rationale), and host-relative rings matching dev-lead/ci-failure-analyst; the three agreement points (canary-rings.json agent keys, RING_REUSABLES, dispatch enum) are kept in sync, enforced by new bats tests. AC1 (check for a deliberate exclusion) is answered implicitly via the registry _note documenting the #497 self-host risk as bounded by the soak gate, though the PR body does not explicitly state the history-search finding.

Findings

Blocking — unresolved review threads with a verified finding (gates 4):

  1. codeant-ai (scripts/lib/ring-pins.sh:48 and standards/canary-rings.json:1085, rated Major, both threads unresolved): pinned-version-report.sh only scans caller stubs named .github/workflows/<agent>.yml and matches uses: .../<agent>-reusable.yml@... lines (scripts/pinned-version-report.sh:79-84). pr-review's engine is the grandfathered pr-review.yml and its caller is pr-review-trigger.yml, so pr-review is silently omitted from the pinned-version/drift report. Verified against the code at the PR head — the claim is correct. It is non-regressing (pr-review was never in the report) and does not affect the autocut fix, but it perpetuates the exact 'invisible to automation' class of gap pr-review is missing from the canary ring registry — autocut never advances its channel, so merged fixes sit inert (two incidents in three days) #1166 exists to close. Resolution options: make the report registry-driven for stub/reusable names, or open a follow-up issue and reply/resolve the threads with that rationale.

Non-blocking:
2. gemini-code-assist (scripts/canary-rollout.sh, unresolved): stylistic robustness suggestions. Checked against the code — json is declared local and assigned immediately (no set -u exposure), .[]? follows an explicit type=="array" check, and registered comes from internal jq output (no CRLF risk). No action strictly required; may be resolved as noise.
3. Secret scan: run_secret_scanning MCP tool unavailable in this run; the gitleaks CI check passed. No secrets touched by the diff.
4. The workflow change is a single dispatch-enum entry — no new security surface, no third-party actions.

CI status

All required checks green: CodeQL, SonarCloud (x2), agent-shield/AgentShield, dependency-audit. Also green: Lint, ShellCheck, bats suites, duplicate-decl-gate, Secret scan (gitleaks), Agent Security Scan, Analyze (actions). CodeRabbit is PENDING but non-required. CANCELLED dev-lead dispatch/ci-relay/resume runs are superseded duplicates whose latest runs succeeded or skipped.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@donpetry-bot

donpetry-bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 3311a426c549a495b936e7a71c1990a039305f75 — click to expand prior review.

Review — fix requested (cycle 2/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: a1980bb411c734c40b94b723cb9164c791571f48
Review mode: triage-approved (single reviewer)

Summary

Registers pr-review in the canary ring registry (host petry-projects/.github-private, grandfathered engine pr-review.yml, run_workflow = caller-stub display name), syncs RING_REUSABLES and the canary-rollout.yml dispatch enum, and fixes cmd_drift so a present grandfathered-named registered reusable is not false-flagged missing-file. The code is sound and well-tested, but the prior review's blocking finding (unresolved codeant-ai threads flagging a verified pinned-version-report.sh gap) is unaddressed: the head SHA is unchanged and all three review threads remain unresolved.

Linked issue analysis

Issue #1166 is substantively addressed by the diff itself: registry entry with host, reusable path, run_workflow display name (with the #747 graceful-empty rationale documented in the registry _note), host-relative rings matching dev-lead/ci-failure-analyst, the three agreement points kept in sync, and bats tests guarding both the registration shape and the drift grandfathered-name fix. The #497 self-host concern (AC1) is documented as bounded by the soak gate in the registry _note.

Findings

Blocking — prior review findings not addressed (cycle 1 fix-request, same SHA a1980bb4):

  1. Unresolved review threads (gate 4). The two codeant-ai threads (scripts/lib/ring-pins.sh, standards/canary-rings.json; rated Major) remain unresolved, and the finding is independently re-verified at the PR head: scripts/pinned-version-report.sh builds the stub path as .github/workflows/${agent}.yml (→ pr-review.yml, but the actual caller is pr-review-trigger.yml) and greps for uses: …/${agent}-reusable.yml@ (→ pr-review-reusable.yml, but the engine is the grandfathered pr-review.yml). Both conditions fail, so pr-review is silently omitted from the pinned-version/drift report — the same 'invisible to automation' gap class pr-review is missing from the canary ring registry — autocut never advances its channel, so merged fixes sit inert (two incidents in three days) #1166 exists to close. Resolution as stated in the prior review: make the report registry-driven for stub/reusable names, or open a follow-up issue and reply/resolve the threads with that rationale.
  2. The gemini-code-assist thread (scripts/canary-rollout.sh) also remains unresolved. Previously assessed as stylistic noise (json is local and immediately assigned; .[]? follows an explicit type check; registered comes from internal jq output) — may be resolved without changes, but it still counts against gate 4 while open.

Non-blocking:
3. Secret scan: run_secret_scanning MCP tool unavailable in this run; gitleaks CI check is green. The diff touches no secrets.
4. No new commits since the prior review — nothing new to assess beyond thread state.

CI status

All checks green at a1980bb4: CodeQL, SonarCloud (quality gate passed), agent-shield/AgentShield, Agent Security Scan, Secret scan (gitleaks), Lint, ShellCheck, bats suites, duplicate-decl-gate, Analyze (actions), dependency-audit, CodeRabbit. CANCELLED dev-lead dispatch/ci-relay/resume runs are superseded duplicates.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (partial)

A commit was pushed, but not every requested change was applied. Per requested item:

  • scripts/canary-rollout.sh:2513 — applied
  • scripts/lib/ring-pins.sh:48 — not applied
  • standards/canary-rings.json:1085 — applied

The unaddressed items above still need work.

@sonarqubecloud

Copy link
Copy Markdown

@don-petry
don-petry disabled auto-merge September 23, 2026 02:12
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 1 — Quality Gate passed summary (no specific findings)
```
The PR is clean — all quality checks pass with no actionable issues from the bot.

@don-petry
don-petry enabled auto-merge (squash) September 23, 2026 02:12

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 3311a426c549a495b936e7a71c1990a039305f75
Review mode: triage-approved (single reviewer)

Summary

Registers pr-review in the canary ring registry (host petry-projects/.github-private, grandfathered engine pr-review.yml, run_workflow = caller-stub display name), syncs RING_REUSABLES and the canary-rollout.yml dispatch enum, fixes cmd_drift so a present grandfathered-named registered reusable is not false-flagged missing-file, and — new since the prior review — makes pinned-version-report.sh registry-driven via ring_reusable_file/ring_caller_stub plus a registry caller_stub field, closing the prior cycle's blocking finding.

Linked issue analysis

Issue #1166 is substantively addressed: registry entry with host, reusable path, caller_stub, run_workflow display name (with the #747 graceful-empty rationale documented in the registry _note), host-relative rings (next=$host, ring0=$org_infra, ring1, stable=*) matching dev-lead/ci-failure-analyst, #548-default gate knobs, and the agreement points (registry, RING_REUSABLES, dispatch enum) kept in sync. AC1's #497 self-host concern is documented in the _note as bounded by the soak gate. Bats tests guard the registration shape, the drift fix (both the no-false-flag and still-reports-genuinely-missing directions), and both new ring-pins helpers including fallback behavior.

Findings

Prior review (cycle 2 fix-request at a1980bb4) — all findings resolved:

  1. RESOLVED — pinned-version-report.sh gap: the new commit adds ring_reusable_file / ring_caller_stub (scripts/lib/ring-pins.sh) backed by the registry's reusable/caller_stub fields, and pinned-version-report.sh now derives both the stub path (pr-review-trigger.yml) and the uses: regex (dot-escaped pr-review.yml) from them, with convention fallbacks for all other agents. Independently verified in the head diff; both codeant-ai threads are resolved.
  2. RESOLVED — gemini-code-assist thread: its substantive suggestions were applied in _gh_list_reusables (CR stripping of registered paths, json="" initialization, empty-body treated as enumeration failure, .[] after the explicit array type check); thread resolved/outdated.

New in this cycle: no new issues. The _gh_list_reusables registered-paths branch still requires presence in the host listing, so a genuinely-deleted registered reusable is still reported missing (covered by a dedicated bats test). Note: a dev-lead orchestration comment self-reported ring-pins.sh as 'not applied', but the head diff demonstrably contains the fix — the self-report is stale/incorrect and does not block.

Non-blocking: run_secret_scanning MCP tool unavailable in this run; gitleaks CI check is green and the diff touches no secrets.

CI status

All checks green at 3311a426: CodeQL, SonarCloud (quality gate passed), agent-shield/AgentShield, Agent Security Scan, Secret scan (gitleaks), Lint, ShellCheck, bats suites, duplicate-decl-gate, Analyze (actions), dependency-audit, CodeRabbit. Skipped jobs (dependabot-automerge, ecosystem-specific audits, dev-lead relay/resume) are conditional no-ops.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

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.

pr-review is missing from the canary ring registry — autocut never advances its channel, so merged fixes sit inert (two incidents in three days)

2 participants