feat(#613): relocate canary-rollout engine + bootstrap into .github (workflow disabled)#624
Conversation
…workflow disabled) PR-B of epic #613: move the release-promotion / repo-bootstrap tooling into petry-projects/.github so all org tooling + standards share one canonical home (the #596 principle, extended). .github already owns the standards (standards/rulesets/, lib/ring-pins.sh, the canonical apply-repo-settings.sh); this brings the orchestrators alongside them. Relocated from .github-private (byte-identical): - scripts/canary-rollout.sh, scripts/lib/canary-rollout.sh (+ tests/canary_rollout.bats) - standards/canary-rings.json (ring registry — now co-located with lib/ring-pins.sh) - docs/{bootstrap/new-repo-validation,release/versioning,initiatives/canary-rollout-adr}.md - .github/workflows/canary-rollout.yml — SHIPS DISABLED (schedule commented out, workflow_dispatch only) so this PR lands with zero operational effect. The live cutover (re-enable schedule here + delete the copy from .github-private in the same window, to avoid a double-run race on protected channel tags) is PR-C. Reconcile fixes required by the move (bootstrap now sits next to .github's canonical 346L apply-repo-settings.sh, not .github-private's 129L subset): - bootstrap-new-repo.sh: bridge dry-run onto DRY_RUN (the var the canonical settings script reads), keeping DEV_LEAD_DRY_RUN for backward-compat. Without this a real `DRY_RUN=true` bootstrap would issue LIVE settings PATCHes — a latent bug the relocation exposes. - test_bootstrap_new_repo.bats: the real-sub-script tests now exercise the canonical settings script, which requires GH_TOKEN and fetches current state even under dry-run. Provide a dummy token (gh is stubbed) and return a non-"{}" repo object from the gh read-stub (canonical treats "{}" as a failed fetch). Assertion wording updated to the canonical script's dry-run messages. Auth prerequisite (already applied, org-admin): petry-projects/.github added to the selected-repos list for the RELEASE_MANAGER_APP_KEY secret + RELEASE_MANAGER_APP_CLIENT_ID variable, so the workflow can mint the release-manager App token once enabled. The App (id 4193127) is installed org-wide, so no per-repo install gap. Validation: canary_rollout.bats 88/88, test_bootstrap_new_repo.bats 13/13, shellcheck clean, canary-rings.json valid, workflow YAML parses (dispatch-only). Refs #613 #596 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds a ring-staged canary rollout system, including rollout decision logic, orchestration, workflow wiring, ring registry data, bootstrap automation, supporting documentation, and Bats tests for rollout and onboarding flows. ChangesCanary rollout system
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive, ring-staged, health-gated canary rollout system for agent releases, along with repository bootstrapping automation, detailed documentation, and extensive test suites. The code review highlights several critical correctness and portability issues: a silent command failure bug in _run_json that could lead to premature promotions, macOS compatibility issues with base64 flags, a redundant CLI check, a style guide violation regarding bare return statements, and opportunities to improve BATS test assertions.
There was a problem hiding this comment.
Pull request overview
Relocates the canary-rollout engine and new-repo bootstrap orchestration tooling into petry-projects/.github as the canonical org-standards home, with the canary-rollout workflow shipping dispatch-only (scheduled trigger commented out) to keep the change operationally inert until the follow-up cutover.
Changes:
- Adds the canary-rollout orchestrator + pure decision core and the ring registry (
standards/canary-rings.json) to this repo. - Adds/updates bootstrap orchestration (
scripts/bootstrap-new-repo.sh) and its Bats coverage, including DRY_RUN bridging. - Ports supporting documentation and the (currently schedule-disabled)
Canary Rolloutworkflow.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/canary-rollout.yml |
Adds the canary-rollout workflow in dispatch-only mode (schedule commented out) and wires GitHub App token minting. |
docs/bootstrap/new-repo-validation.md |
Documents the bootstrap flow and recorded DRY_RUN walkthrough. |
docs/initiatives/canary-rollout-adr.md |
Adds ADR describing the canary rollout model and operations. |
docs/release/versioning.md |
Adds agent versioning/channel-tag documentation (including cross-repo notes). |
scripts/bootstrap-new-repo.sh |
Adds DRY_RUN-aware repo bootstrap orchestrator and cross-repo PR helpers. |
scripts/canary-rollout.sh |
Adds the canary-rollout engine (autocut/evaluate/promote/rollback/sync-issues/drift). |
scripts/lib/canary-rollout.sh |
Adds the pure decision core used by the orchestrator (unit-testable functions). |
standards/canary-rings.json |
Adds the ring registry (agents, ring members, gate knobs). |
tests/test_bootstrap_new_repo.bats |
Adds Bats coverage for bootstrap orchestration (dry-run, sequencing, ring behavior, e2e dry-run). |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.github/workflows/canary-rollout.yml:
- Line 110: The `::error::` message in the canary rollout workflow is too long
and fails linting; shorten the string emitted by the shell step in the canary
rollout job so it stays under the line-length limit. Keep the message focused on
the refusal to fall back to `github.token`, and rely on the nearby comment for
the app token requirement details.
- Around line 114-121: The checkout step currently persists the App token in git
credentials even though later tag moves use gh api with GH_TOKEN, so update the
actions/checkout step to disable credential persistence. Keep the existing
Checkout (with tags) setup and App token usage, but add persist-credentials:
false so the workflow does not leave an unnecessary credential behind for later
steps.
In `@docs/bootstrap/new-repo-validation.md`:
- Line 45: The markdown file has fenced code blocks opened without a language
specifier, triggering MD040 in CI. Update both bare fences in the affected
examples to use a language tag (for example, text) while keeping the existing
content unchanged. Use the fenced block openings in the bootstrap instructions
section as the unique markers to locate and fix both occurrences.
In `@docs/initiatives/canary-rollout-adr.md`:
- Line 3: The markdownlint MD013 failure is coming from several overly long
prose lines in the ADR content, so reflow the affected paragraphs at natural
clause boundaries while preserving meaning and formatting. Update the text
around the “Status” line and the nearby narrative about the implemented model,
superseding the rings sketch, and the rollout caveat so each line stays under
the 200-character limit. Keep the same wording and structure, just wrap the
content in the document sections that mention initiative `#495`, issues `#499`–#502
and `#868`, and the supersession note.
In `@scripts/bootstrap-new-repo.sh`:
- Around line 211-214: The duplicate check in the bootstrap flow is too broad
because it searches the entire ring-pins content instead of the target ring
entry, which can wrongly mark a repo as already synced. Update the
repo-existence check in the bootstrap script’s ring registration logic so it is
scoped to the target ring’s array entry (the `ring=(` line) before checking for
the repo, and keep the existing success path and logging behavior in sync with
that narrower match.
In `@scripts/lib/canary-rollout.sh`:
- Around line 56-66: The `robust_sample_target` header comment is stale and
still documents the 4th argument as daily counts while the function signature
now uses `cap_multiple` (`robust_sample_target` with `cap_multiple="${4:-3}";
shift 4`). Update the docstring to name the 4th positional parameter as
`cap_multiple`, describe it as the spike-cap multiplier instead of a hardcoded
3×, and clarify that daily counts begin after the first four args so callers
don’t misread the interface.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab680e63-8818-4d79-a437-b5f4d95c0292
📒 Files selected for processing (10)
.github/workflows/canary-rollout.ymldocs/bootstrap/new-repo-validation.mddocs/initiatives/canary-rollout-adr.mddocs/release/versioning.mdscripts/bootstrap-new-repo.shscripts/canary-rollout.shscripts/lib/canary-rollout.shstandards/canary-rings.jsontests/canary_rollout.batstests/test_bootstrap_new_repo.bats
Correctness / behavior:
- canary-rollout.sh _run_json: fail CLOSED on a genuine gh failure (return 1 +
::error:: instead of silent []) — a swallowed error read as "zero failures" could
green-light a bad promotion. Empty-but-successful still returns [] (Gemini critical).
- canary-rollout.sh _reusable_differs: host-aware — for a cross-repo agent (host !=
THIS_REPO, e.g. dev-lead once the engine runs from .github) the reusable blob isn't in
the local checkout, so resolve it on the host via gh api (_gh_blob_sha) and fail CLOSED
(unresolvable compare → differs=1, disabling the benign-failure allowlist). +3 unit
tests (Copilot). this-repo agents keep the local git rev-parse path (existing tests green).
- bootstrap-new-repo.sh ring-pins dup check: scope to the target ring's array line so a
repo registered in a DIFFERENT ring no longer false-positives "in sync" (CodeRabbit).
Portability:
- bootstrap-new-repo.sh base64: encode via `base64 | tr -d '\n'` (GNU wraps, BSD has no
-w0); decode via `base64 -d || base64 -D` at all three sites (Gemini/Copilot).
Hardening / hygiene:
- canary-rollout.yml: persist-credentials: false on checkout (tag moves use gh api, not
git push, so the git credential is unused — CodeRabbit/zizmor).
- lib/canary-rollout.sh _semver_gt: explicit `return $?` (Gemini); robust_sample_target
docstring documents the cap_multiple arg (CodeRabbit).
- bootstrap-new-repo.sh: drop the redundant local gh-CLI check (main() checks) (Gemini).
- canary_rollout.bats: `run grep -c` idiom instead of `$(... || true)` (Gemini).
Docs (relocated ADR/versioning canonicalized in .github — refreshed to live behavior):
- canary-rollout.yml header + ADR §4: tag moves are `gh api` ref updates on the host, not
`git tag -f` + push; mover credential is the release-manager GitHub App token, not a PAT.
- ADR §3 gate: replace the old 7-day-window / failure-rate model with the #548 graduated
dwell+sample floors and cumulative zero-failures-since-cut; gate-state table matches impl.
- ADR cadence: timer runs autocut → promote-all (arm-gated by CANARY_AUTO_PROMOTE) /
evaluate-all → sync-issues, not "evaluate-only, never promotes on timer".
- versioning.md: the six #482 reusables are now registered (cross-repo gh api moves, #870/#1054);
only feature-ideation remains absent (Copilot).
Validation: canary_rollout.bats 91/91 (+3), test_bootstrap_new_repo.bats 13/13,
shellcheck no new findings (22 pre-existing info-only), markdownlint 0, yamllint clean.
Refs #613 #596
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Advisory findings addressed (commit 6491f3f)Thanks Gemini / Copilot / CodeRabbit. All findings triaged and fixed — summary: Correctness / behavior
Portability
Hardening / hygiene
Docs (the relocated ADR/versioning are being canonicalized here, so refreshed to live behavior)
The 3 CodeRabbit "Major" lint findings (MD013/MD040/line-length) were already resolved by an earlier lint commit — CI Lint is green. Validation: Note: the workflow still ships disabled ( |
|
don-petry
left a comment
There was a problem hiding this comment.
All 26 checks green, threads resolved — auto-review readiness re-check.
PR-C2 of the epic #613 cutover. Re-enables the 4-hourly `schedule:` on the relocated canary-rollout workflow (shipped disabled in #624), making .github the live host for autocut → promote-all → sync-issues. MUST land AFTER petry-projects/.github-private#1128 (which deletes the engine + workflow there, stopping that repo's schedule). Both CANARY_AUTO_CUT and CANARY_AUTO_PROMOTE are 'true', so the two schedules must never overlap — this is sequenced strictly after the .github-private removal merges. Live-validated already: workflow_dispatch evaluate run 28990172900 in .github minted the release-manager App token, resolved cross-repo dev-lead channel tags on .github-private, and computed the #548 gate — confirming the engine runs correctly from its new home. Refs #613 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ive) (#632) docs(#631): remove duplicate versioning.md; point canary ADR to .github-private canonical Epic #613 (PR #624) copied docs/release/versioning.md into .github, creating a second copy that has since diverged from the .github-private original. .github's own standards already name .github-private as authoritative (ci-standards.md §"authoritative process lives in that repo's docs/release/"; agent-canary-rings-adr.md links there), and the doc documents cut-release.sh which lives in .github-private. - Remove docs/release/versioning.md (the redundant #624 copy). - Repoint canary-rollout-adr.md's two links to the .github-private canonical URL. No content lost — canonical copy stays in .github-private. Closes #631. Refs #613 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>



What
PR-B of epic #613 — relocate the release-promotion / repo-bootstrap tooling into
petry-projects/.githubso all org tooling + standards share one canonical home (the #596 principle, extended)..githubalready owns the standards (standards/rulesets/,lib/ring-pins.sh, the canonicalapply-repo-settings.sh); this brings the orchestrators alongside them.Ships operationally inert
The workflow ships disabled —
schedule:is commented out, leavingworkflow_dispatchonly — so this PR lands with zero operational effect. The live cutover is a follow-up (PR-C): re-enableschedulehere and delete the copy from.github-privatein the same window, so the two repos never both runscheduleand race to move the same protected channel tags.Files
Relocated byte-identical from
.github-private:scripts/canary-rollout.sh,scripts/lib/canary-rollout.sh(+tests/canary_rollout.bats)standards/canary-rings.json— ring registry, now co-located withlib/ring-pins.sh(read via a repo-relative path, so this removes a cross-repo hop)docs/bootstrap/new-repo-validation.md,docs/release/versioning.md,docs/initiatives/canary-rollout-adr.mdChanged (3):
.github/workflows/canary-rollout.ymlschedule:commented outscripts/bootstrap-new-repo.shDRY_RUN(keepDEV_LEAD_DRY_RUN).githubsettings script readsDRY_RUN; without this a realDRY_RUN=truebootstrap would issue LIVE settings PATCHes — a latent bug the move exposestests/test_bootstrap_new_repo.batsGH_TOKEN; non-{}gh read-stub; assertion wording{}as a failed fetchAuth prerequisite (already applied — org admin)
petry-projects/.githubadded to the selected-repos list for theRELEASE_MANAGER_APP_KEYsecret +RELEASE_MANAGER_APP_CLIENT_IDvariable, so the workflow can mint the release-manager App token once enabled. The App (id4193127) is installed org-wide → no per-repo install gap.Validation
tests/canary_rollout.bats— 88/88tests/test_bootstrap_new_repo.bats— 13/13shellcheckclean;canary-rings.jsonvalid JSON; workflow YAML parses (on:=[workflow_dispatch]only)Follow-up (not this PR)
schedulehere + delete engine/workflow/duplicateapply-rulesets.sh/apply-repo-settings.shfrom.github-private; de-dupelib/push-protection.sh.Refs #613 #596
🤖 Generated with Claude Code
Summary by CodeRabbit