feat: implement issue #696 — F4: canary-rollout engine operates on v<major>-<tier> channels (fallback-safe) [#657] - #697
Conversation
…major>-<tier> channels (fallback-safe) [#657]
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 38 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 (3)
📝 WalkthroughWalkthroughThe canary rollout engine now resolves major-scoped ChangesMajor-scoped canary channels
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 — waiting on PR blockers (intent: review-changes)PR: #697 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Phase F4 of the major-scoped channels epic (#657), enabling the canary rollout engine to resolve and promote major-scoped channel tags (v-) while remaining backward-compatible with the legacy bare-tier fleet. It introduces several helper functions to parse semver majors and resolve tags, updates evaluation, promotion, and autocut commands to use these resolved tags, and adds comprehensive test coverage. The review feedback suggests caching the results of _agent_current_major and _channel_tag_commit using associative arrays to avoid redundant GitHub API requests during execution.
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
|
@donpetry-bot I'm on it — starting a fresh review now. Results will appear in a few minutes. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #697 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
|
@donpetry-bot I'm on it — starting a fresh review now. Results will appear in a few minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/canary-rollout.sh`:
- Around line 158-199: Memoize the result of _agent_current_major per agent for
the lifetime of the script invocation, and have _resolved_channel reuse that
cached value instead of calling _host_release_versions through
_agent_current_major on every lookup. Ensure all channel resolutions, including
evaluate and promote paths, share the same per-agent cache while preserving the
existing fallback behavior.
- Around line 152-156: Move the pure `_looks_like_oid` predicate from the
orchestrator script into `scripts/lib/canary-rollout.sh` alongside
`major_component` and `channel_tag`, and update callers to use the sourced
helper. Add direct bats coverage for valid 7–64-character lowercase hexadecimal
IDs and invalid values, preserving the predicate’s existing behavior.
- Around line 1499-1509: Update _autocut_agent to retain both the resolved
next-channel tag and commit from the existing channel_commit call near line
1488, rather than discarding the tag. For non-major bumps, reuse that captured
tag instead of calling _resolved_channel_tag "$agent" next again; keep the
major-bump channel_tag behavior unchanged.
In `@scripts/lib/canary-rollout.sh`:
- Around line 122-127: Update major_component to replace the [[ ... ]] && printf
... || true chain with an explicit if/then/else structure, preserving the
existing strict version match, major-number output, and empty output for
non-matching input while avoiding ShellCheck SC2015.
🪄 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: 420e0b93-597a-4f68-a566-9a586ee6a9c0
📒 Files selected for processing (3)
scripts/canary-rollout.shscripts/lib/canary-rollout.shtests/canary_rollout.bats
Superseded by automated re-review at
|
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Superseded by automated re-review at
|
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 85bbb0d502a91a276c7b6bf98564a7ab02ec8408
Review mode: triage-approved (single reviewer)
Summary
PR #697 implements epic #657 Phase F4: major-scoped v- canary channels with fall-back-safe resolution (prefer the v-scoped tag when it exists, else the legacy bare tier), covering channel resolution, evaluate, promote, and autocut, plus ~200 lines of new bats coverage including the transition-safety byte-identical guard. All prior review findings (CodeRabbit/Gemini memoization, _looks_like_oid relocation to lib, SC2015 idiom, autocut double-resolution) were addressed in commits f04b73b and 85bbb0d; CodeRabbit dismissed its changes-requested review and all 6 review threads are resolved. CI is fully green (ShellCheck, bats, CodeQL, SonarCloud, gitleaks, Agent Security Scan). Risk MEDIUM (non-trivial rollout-engine logic; no auth/secrets/DB/security surface).
Linked issue analysis
Closes #696 (F4 of epic #657). All five requirements are substantively addressed: (1) channel resolution prefers /v- with fallback to bare / via _resolved_channel, guarded by _looks_like_oid so absent/sentinel responses fall back; (2) autocut of a MAJOR bump seeds a fresh v-next line while minor/patch advance the current major's v-next, falling back to bare next on today's fleet; (3) promote advances the resolved tag within a major line (v2 promotion never touches v1-*, tested); (4) evaluate reports the major line (e.g. 'candidate (v2-next)'); (5) current major derived from the highest vX.Y.Z release tag — no new registry field. Pure helpers (major_component, channel_tag, _looks_like_oid) live in scripts/lib/canary-rollout.sh as required. Tests cover every scenario the issue lists, including the byte-identical bare-tier transition guard.
Findings
- All 4 CodeRabbit actionable comments and both Gemini comments are resolved: memoization added (_AGENT_MAJOR_CACHE, _CHANNEL_COMMIT_CACHE), _looks_like_oid moved to lib with direct bats coverage, major_component rewritten as an explicit conditional, and autocut's duplicate next-resolution removed. CodeRabbit dismissed its CHANGES_REQUESTED review at head.
- INFO (carried forward, non-blocking): the per-invocation caches are never invalidated after a tag move; safe today since nothing re-reads a channel commit after mutating it in the same run, but a future read-after-write would see a stale value.
- INFO: fall-back safety is sound — _resolved_channel only prefers the v-form when the probe returns a real 7-64 hex oid; no injection surface (agent/tier come from the internal rings registry, major_component emits digits only).
- INFO: run_secret_scanning MCP tool not available in this environment; relied on the green gitleaks CI check. No secrets in the diff.
CI status
All required checks green: ShellCheck, Lint and bats, CodeQL, SonarCloud (quality gate passed), gitleaks secret scan, Agent Security Scan, npm audit, agent-shield. The two CANCELLED dev-lead dispatch/ci-relay entries are the dev-lead agent's own superseded orchestration runs, not required checks; their later runs succeeded or were intentionally skipped ([skip ci-relay]).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 85bbb0d502a91a276c7b6bf98564a7ab02ec8408
Review mode: triage-approved (single reviewer)
Summary
Implements epic #657 Phase F4: the canary-rollout engine can now resolve, promote, and autocut major-scoped v- channel tags while remaining fallback-safe on today's bare-tier fleet. Resolution prefers /v- when it exists (verified via _looks_like_oid), else the legacy bare /; a major-bump autocut seeds a fresh v-next line; promote advances the resolved tag within its major line. Pure helpers (major_component, channel_tag, _looks_like_oid) live in scripts/lib/canary-rollout.sh, keeping the lib pure per #696. 200+ lines of new bats coverage including the transition-safety guard (bare-tier fixture behaves byte-identically to pre-F4).
Linked issue analysis
Closes #696 (epic #657, Phase F4). All four change areas from the issue are implemented: (1) fallback-safe channel resolution in channel_commit/_resolved_channel; (2) _autocut_agent seeds v-next on a major bump and advances the current major's v-next (with bare fallback) on minor/patch; (3) cmd_promote advances the resolved frontier tag within a major line — a v2 promotion never touches v1-* tags (test-asserted); (4) evaluate reports the major line (e.g. 'candidate (v2-next)'). All five test scenarios mandated by the issue are present. The critical transition constraint (must not break the running bare-tier fleet before F5) is guarded by dedicated byte-identical fallback tests. Registry derivation approach used (major derived from highest vX.Y.Z release tag, no schema churn) — exactly as the issue preferred.
Findings
No blocking findings.
- Secret scan: run_secret_scanning MCP tool unavailable in this environment; gitleaks CI check is green and manual diff inspection found no credentials (test fixtures use placeholder SHAs only).
- Prior bot reviews fully addressed in commits f04b73b/85bbb0d5: memoization added (_AGENT_MAJOR_CACHE, _CHANNEL_COMMIT_CACHE) per gemini/CodeRabbit performance comments; _looks_like_oid moved to the pure lib; major_component avoids the SC2015 A && B || C idiom. CodeRabbit's CHANGES_REQUESTED review was dismissed after fixes; all 6 review threads are resolved.
- Triage cleared this as low-risk; I classify it MEDIUM (non-trivial logic in the production tag-mutation engine), which still satisfies the auto-approve gate. The fallback design plus transition-safety tests mitigate the rollout risk.
- Minor (non-blocking): _resolved_channel is invoked separately by _resolved_channel_tag in cmd_promote, but the memoization caches make repeated resolution cheap and consistent within a run.
CI status
All substantive checks green: ShellCheck, Lint and bats, CodeQL, Secret scan (gitleaks), Agent Security Scan, AgentShield, SonarCloud, npm audit, Analyze (actions). Cancelled/skipped entries (dev-lead dispatch/ci-relay, ecosystem-specific audits) are agent relay jobs superseded by later successful runs or N/A ecosystems — not failures.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
2 participants
Closes #696
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
New Features
Bug Fixes
Tests