Skip to content

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

Closed
#1168
Closed
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
#1168
Labels
bugBug reportsdev-leadFor dev-lead agent pickup

Description

@don-petry
don-petry
opened on Sep 23, 2026

Story

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.

The gap

standards/canary-rings.json registers 16 agents. pr-review is not one of them — pr-review-mention and pr-auto-review are, but not the agent whose scripts actually perform reviews (scripts/review-one-pr.sh, scripts/post-pr-review.sh, scripts/engine.sh, reusable .github/workflows/pr-review.yml in petry-projects/.github-private).

Consequence: canary-rollout.yml's 4-hourly autocut — "the FRONT END that closes the last manual seam" (#1069), gated by CANARY_AUTO_CUT, currently true — never cuts pr-review. Its channel only moves when a human runs cut-release.sh.

What that cost, twice in three days

fix merged to main live on pr-review/v1-next gap
combsmith/.github-private#1795 (non-required checks deadlocking every merge) 2026-09-19 13:37 only after a manual cut to v1.10.0 every PR in .github-private stayed deadlocked meanwhile
combsmith/.github-private#1875 (approval read-back, #1874) 2026-09-21 13:22 only after a manual cut to v1.11.0 phantom approvals continued at 13:40 and 13:42, stranding two PRs

In both cases the fix was correct and merged; the repo kept exhibiting the fixed bug because the agent runs from the channel tag, not main.

Not a host problem — check before assuming otherwise

Hosting is not the reason it is missing: the registry already supports agents hosted in .github-private — dev-lead (petry-projects/.github-private, .github/workflows/dev-lead-reusable.yml) and ci-failure-analyst (same host) are both registered. So pr-review living there is no barrier.

But check for a deliberate exclusion before adding it. pr-review is the ring-0 self-host (#497): it reviews the repo that hosts it, and .github-private pins pr-review/v1-next precisely to dogfood the next channel. Auto-advancing next means a bad merge reaches the repo's own review automation immediately, with no earlier tier to catch it. That may have been a conscious choice. If it was, this issue's outcome is to document the exclusion rather than close the gap silently.

Acceptance Criteria

  1. Search the history (docs(standards): add Advanced Security (GHAS) org standard #495, canary autocut watches only the reusable blob, so script-only changes never ship; plus _gh_move_tag cannot create a missing channel ref #1019, apply-repo-settings can never reach the fleet: stub not deployable, ring tags missing, canary deadlocked on a vendored script #1045, 💡 Canary Pipeline Self-Health Observability & Reliability Standard #1069, enable pull request limits #497) for a deliberate decision to exclude pr-review. State the finding in the PR. If the exclusion was intentional, stop and document it — in standards/canary-rings.json or the rollout ADR — and close this issue as documentation.
  2. If no such decision exists: register pr-review in standards/canary-rings.json with its host (petry-projects/.github-private), reusable path, run_workflow, and ring membership consistent with how it is consumed today (.github-private is the next-tier self-host).
  3. Keep the three agreement points in sync — RING_REUSABLES in scripts/lib/ring-pins.sh, the agent enum in canary-rollout.yml, and the registry — since tests/canary_rollout.bats asserts the choice list equals .agents | keys and will fail otherwise.
  4. Prove it with a dry run, not by inspection: canary-rollout.yml action=autocut, agent=pr-review in dry-run mode shows the version it would cut and the channel it would advance, and the computed refs resolve. Paste the output in the PR.
  5. Confirm no regression for the 16 already-registered agents: an evaluate-all (read-only) before and after shows the same tiers.
  6. Note in the PR whether CANARY_AUTO_PROMOTE (also true) means pr-review will now promote past next automatically, and whether that is wanted for the self-host — if not, say how it is bounded.

Dev Notes

References

combsmith/.github-private#1795, #1874/#1875, #1882; #1069 (autocut), #1019 (registry agreement), #497 (self-hosting circular dependency), standards/canary-rings.json, scripts/lib/ring-pins.sh, .github/workflows/canary-rollout.yml.

Activity

  1. don-petry
    added
    bugBug reports
    dev-leadFor dev-lead agent pickup
    on Sep 23, 2026

  2. don-petry commented on Sep 23, 2026

    @don-petry
    don-petry
    on Sep 23, 2026
    ContributorAuthor

    Dev-Lead Implementation Plan

    Issue: #1166 — pr-review is missing from the canary ring registry — autocut never advances its channel

    History finding (AC #1)

    Searched the in-repo record (both rollout ADRs, ci-standards.md, #495/#497/#1019/#1045/#1069). No deliberate decision to exclude pr-review was found. The opposite: both ADRs explicitly name pr-review as a versioned, self-hosting agent slated to adopt the ring model (docs/initiatives/canary-rollout-adr.md §Scope + Decision 1; docs/initiatives/agent-canary-rings-adr.md — "#499/#616/#688"). The #497 self-host/circular-dependency concern is addressed by the ring design ("production self-review duty stays on stable… breaking the circular dependency"), not used to exclude the agent. pr-review.yml evaded the drift audit only because that scan matches *-reusable.yml and the engine is the grandfathered pr-review.yml (#1127). Conclusion: this is an oversight, not an intentional exclusion → proceed to register (AC #2).

    Scope

    Register the pr-review agent in the canary ring registry and keep the three agreement points in sync, so 4-hourly autocut cuts/advances its channel automatically.

    Implementation Checklist

    • standards/canary-rings.json — add .agents["pr-review"] (host petry-projects/.github-private, reusable .github/workflows/pr-review.yml, run_workflow, host-relative rings next→ring0→ring1→stable, standard Define the release soak standard (ring-promotion gate): time floor + traffic-scaled sample + health #548 gate — mirroring the .github-private-hosted siblings dev-lead/ci-failure-analyst)
    • scripts/lib/ring-pins.sh — add pr-review to RING_REUSABLES (keep alphabetical)
    • .github/workflows/canary-rollout.yml — add pr-review to the dispatch agent enum (keep alphabetical)

    Tests to Write

    • New assertion in tests/canary_rollout.bats: pr-review is registered with the expected host/reusable and its ring tiers resolve (next=.github-private, ring0=.github)
    • Existing agreement gates must stay green: tests/canary_rollout.bats "the agent choice list agrees with the registry" and test/scripts/lib/ring-pins.bats "RING_REUSABLES equals the canary-rings.json agent registry"

    Verification (AC #4/#5/#6)

    • Dry-run: canary-rollout.sh autocut --dry-run scoped to pr-review (+ resolve computed refs) — paste output.
    • evaluate-all (read-only) before/after — confirm the 16 existing agents' tiers are unchanged.
    • Note the CANARY_AUTO_PROMOTE=true consequence for the self-host and how promotion past next is bounded by the soak/health gate.

    Test Command

    bats tests/canary_rollout.bats test/scripts/lib/ring-pins.bats and bash .dev-lead/scripts/dev-lead-lint.sh

  3. don-petry commented on Sep 23, 2026

    @don-petry
    don-petry
    on Sep 23, 2026
    ContributorAuthor

    Dev-Lead: Progress (in progress — not yet durable)

    Provisional. Nothing is pushed yet; the automation posts the authoritative
    completion record (with PR number + head SHA) after the push. Treat the notes
    below as intent, not delivered results.

    History finding (AC #1) — intent

    Searched the in-repo record (both rollout ADRs, ci-standards.md, and the referenced issues). No deliberate decision to exclude pr-review was found. Both ADRs name it as a versioned self-hosting agent slated for the ring model; the #497 self-host/circular-dependency concern is addressed by the ring design (production duty stays on outer tiers, gated by soak health), not used to exclude it. pr-review.yml evaded the drift audit only because that scan matched *-reusable.yml and the engine keeps its grandfathered name (#1127). → Treated as an oversight; proceeded to register.

    Plan execution (intended, in this workspace)

    • Added .agents["pr-review"] to standards/canary-rings.json (host petry-projects/.github-private, reusable .github/workflows/pr-review.yml, host-relative rings next→ring0→ring1→stable, standard Define the release soak standard (ring-promotion gate): time floor + traffic-scaled sample + health #548 gate — mirroring the .github-private-hosted siblings).
    • Added pr-review to RING_REUSABLES (scripts/lib/ring-pins.sh) and to the dispatch agent enum (.github/workflows/canary-rollout.yml) — the three agreement points (AC Add coding standards & principles section to AGENTS.md #3).
    • run_workflow = PR Review Agent — Trigger (the caller stub's display name), NOT a *.yml filename: on outer tiers that lack the workflow, a display name yields the graceful "could not find any workflows named …" → [] ([Fleet Monitor] petry-projects/.github — .github/workflows/canary-rollout.yml #747), whereas a filename 404s and _run_json mis-triages it as a transient failure that fails the gate closed (caught via a local dry evaluate).
    • Fixed a false-positive that registration would otherwise introduce: drift's missing-file check listed host reusables by *-reusable.yml suffix only, so the grandfathered pr-review.yml was reported as a "stale registry entry" every cycle. _gh_list_reusables now also admits a present registered path; a genuinely-deleted one is still reported. (+2 regression tests.)

    Local check output (provisional — NOT the pushed run)

    Dry-run autocut for pr-review (the AC #4 evidence) — read-only:

    ::notice::autocut pr-review: detected bump=minor (feat commit)
autocut pr-review: reusable changed on petry-projects/.github-private (main 4fd4eacd35ba) vs next 781b63c8fce8 — cutting v1.12.0 (bump=minor), moving pr-review/v1-next.
[DRY-RUN] would: cut pr-review/v1.12.0 at 4fd4eacd35ba ... + move pr-review/v1-next

    This is the gap itself: a merged fix (main 4fd4eac) has been sitting inert vs the next candidate (781b63c). Ring members resolve: next=.github-private, ring0=.github, ring1=TalkTerm/bmad-bgreat-suite, stable=*.

    evaluate pr-review (read-only) → next->ring0 BLOCKED (dwell met, sample 455, cum_fail=13 triaged PRE_EXISTING), i.e. under CANARY_AUTO_PROMOTE=true it will not silently advance past next — the soak gate holds it pending human --allow-pre-existing. That bounds the self-host risk (AC #6). Existing agents evaluated unchanged; the registry diff is purely additive (16→17, the 16 existing entries byte-identical).

    Files touched (this workspace, not yet pushed)

    • standards/canary-rings.json, scripts/lib/ring-pins.sh, .github/workflows/canary-rollout.yml, scripts/canary-rollout.sh (drift listing fix), tests/canary_rollout.bats (registration + drift regression tests)

    Notes

  4. don-petry commented on Sep 23, 2026

    @don-petry
    don-petry
    on Sep 23, 2026
    ContributorAuthor

    Dev-Lead: Implementation Complete — PR #1168

    The implementation for issue #1166 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.

  5. don-petry
    added a commit that references this issue on Sep 23, 2026

    feat: implement issue #1166 — pr-review is missing from the canary ri…

    21bdd36
  6. don-petry
    closed this as completedin #1168on Sep 23, 2026
  7. don-petry
    added 2 commits that reference this issue on Oct 3, 2026

    fix: remove duplicate pr-review entry in canary-rings.json

    0157747

    feat: implement issue #1106 — pr-review is not a registered canary-ri…

    fd430b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions