Skip to content

feat: implement issue #1046 — canary gate: a v-scoped channel tag shadows the release tag in _gh_candidate_cut_date, so any cross-repo agent adopting v<M>-<tier> channels is permanently BLOCKED (indeterminate) — blocks #1592 - #1051

Merged
don-petry merged 5 commits into
mainfrom
dev-lead/issue-1046-20260901-0128
Sep 1, 2026

Conversation

@don-petry

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

Copy link
Copy Markdown
Contributor

User description

Closes #1046

Implemented by dev-lead agent. Please review.


CodeAnt-AI Description

Prevent v-scoped channel tags from being mistaken for release tags during canary rollout checks

What Changed

  • Canary cut-date detection now considers only strict vMAJOR.MINOR.PATCH release tags
  • Lightweight channel tags such as v139-next no longer override release tags or produce an empty cut date
  • Same-repository checks use the annotated release tag’s date and avoid substituting a commit date
  • Added coverage for local and cross-repository tags, including cases with and without a matching release tag

Impact

✅ Canary gates no longer remain indeterminate because of channel tags
✅ Accurate release cut dates for rollout health checks
✅ Reliable local and cross-repository rollout detection

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

…dows the release tag in _gh_candidate_cut_date, so any cross-repo agent adopting v<M>-<tier> channels is permanently BLOCKED (indeterminate) — blocks #1592
@don-petry
don-petry requested a review from a team as a code owner September 1, 2026 01:44
@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

@codeant-ai

codeant-ai Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR 9cf2f06 Sep 01, 2026 · 01:44 01:47

@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.

@codeant-ai

codeant-ai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Important

Approval pending

CodeRabbit has no unresolved comments, but it could not review the latest commit because the review limit was reached. Follow the review guidance in this comment to continue.


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.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Sep 1, 2026

@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 addresses an issue where v-scoped channel tags shadow actual release tags during candidate cut date resolution, causing the rollout gate to get stuck at BLOCKED. The fix introduces a strict release tag suffix filter (_is_release_tag_suffix) and updates both the local and GitHub API resolution paths, accompanied by comprehensive test coverage. The review feedback suggests simplifying nested double quotes in parameter expansions and improving BATS assertions by checking for exact non-zero exit statuses to prevent false positives from execution errors.

Comment thread tests/canary_rollout.bats Outdated
Comment thread tests/canary_rollout.bats Outdated
Comment thread scripts/canary-rollout.sh
Comment thread scripts/canary-rollout.sh
Comment thread tests/canary_rollout.bats
@codeant-ai

codeant-ai Bot commented Sep 1, 2026

Copy link
Copy Markdown

CodeAnt Nitpicks

1 code suggestion

1. The shared stub returns an annotated release for every for-each-ref call, so tests cannot detect incorrect ref filtering or missing-release behavior in existing orchestrator scenarios.

Incomplete implementation · tests/canary_rollout.bats:1247

@don-petry
don-petry enabled auto-merge (squash) September 1, 2026 01:47
@donpetry-bot

donpetry-bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 9cf2f0688fadd4f2177afc94de0e4be90a4bcd13 — 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: 9cf2f0688fadd4f2177afc94de0e4be90a4bcd13
Review mode: triage-approved (single reviewer)

Summary

The change correctly fixes #1046: _is_release_tag_suffix (scripts/lib/canary-rollout.sh) filters refs to strict vMAJOR.MINOR.PATCH before they can match in both _gh_candidate_cut_date (cross-repo API path) and candidate_cut_date (same-repo for-each-ref path), and the local path now only trusts an annotated tag's dereferenced commit + tagger date instead of silently substituting a lightweight tag's commit date. Test coverage matches the acceptance criteria, including the exact live shadow ordering (v139-next sorting before v139.4.0 at the same commit). The implementation itself looks sound — escalating only because three review threads remain unresolved (decision gate: no unresolved threads).

Linked issue analysis

Closes #1046. All acceptance criteria are substantively addressed: (1) API path filters to immutable release tags only; (2) same filter on the local for-each-ref path; (3) local path prefers the annotated tagger date and skips lightweight release-named tags; (4) regression test uses the real shape — lightweight dev-lead/v139-next alongside annotated dev-lead/v139.4.0 at the same commit, with the channel tag sorting first; (5) both cross-repo and same-repo paths are covered. The final exit criterion (re-seeding dev-lead/v139-* yields a resolvable cut) is a live-verification step outside this PR.

Findings

Unresolved review threads (blocking auto-approve):

  1. gemini-code-assist (medium, tests/canary_rollout.bats:3551 and :3559) — suggests [ "$status" -eq 1 ] instead of [ "$status" -ne 0 ]. Note: -ne 0 matches the existing convention in this test file (e.g. the _looks_like_oid tests), so refuting-and-resolving is a reasonable disposition.
  2. codeant-ai (Major, tests/canary_rollout.bats:3576) — test stubs always return both refs in one response, so an API-pagination omission of the release ref would go undetected. Underlying point is real but pre-existing: gh api repos/$repo/git/matching-refs/... without --paginate only reads the first page, and channel tags now consume page slots. Worth a follow-up issue rather than blocking this fix.

Non-blocking notes:

  • CodeAnt nitpick on the shared stub (tests/canary_rollout.bats:1247) returning an annotated release for every for-each-ref call — minor test-fidelity point.
  • Nested quotes in ${ref#refs/tags/"$agent"/} were challenged by gemini and correctly refuted by the owner (glob-metacharacter safety); those two threads are resolved.
  • MCP secret scanning tool unavailable in this run; gitleaks CI check passed.

Resolution path: resolve or refute the three open threads (and optionally file the pagination follow-up), then this PR meets all gates.

CI status

All substantive checks green: Lint, ShellCheck, Canary-rollout bats, Compliance tests, CodeQL, SonarCloud (quality gate passed, 0 issues), gitleaks, Agent Security Scan, duplicate-decl-gate, AgentShield. Several dev-lead / dispatch / ci-relay runs show CANCELLED — these are concurrency-cancelled duplicate agent triggers (the primary run completed SUCCESS) and are not CI failures. mergeStateStatus=BLOCKED reflects the pending required review (org-leads), not failing checks.


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 1, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 576d04a3633e214ba76a62645832e4cd4a1f953b — 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: 9cf2f0688fadd4f2177afc94de0e4be90a4bcd13
Review mode: triage-approved (single reviewer)

Summary

Re-review at the same head SHA (9cf2f06) previously reviewed in cycle 1. The implementation correctly fixes #1046: _is_release_tag_suffix (scripts/lib/canary-rollout.sh) restricts candidate-cut-date resolution to strict vMAJOR.MINOR.PATCH release tags in both the cross-repo API path (_gh_candidate_cut_date) and the same-repo for-each-ref path (candidate_cut_date), and the local path now only trusts an annotated tag's dereferenced commit + tagger date (lightweight release-named tags are skipped). Test coverage reproduces the exact live shadow ordering (v139-next sorting before v139.4.0 at the same commit) for both paths. No new commits and no thread activity since the prior fix-request — the three review threads that blocked auto-approval remain unresolved, so the decision is unchanged: escalate.

Linked issue analysis

Closes #1046 (v-scoped channel tag shadows the release tag, wedging the canary gate at BLOCKED indeterminate; blocks .github-private#1592). Substantively addressed: (1) the API path filters matching-refs results to immutable release suffixes before matching; (2) the same filter is applied on the local for-each-ref path; (3) the local path uses only the annotated tag's tagger date and never substitutes a lightweight tag's commit date; (4) regression tests use the real shape — lightweight dev-lead/v139-next alongside annotated dev-lead/v139.4.0 at the same commit, channel tag sorting first; (5) both cross-repo and same-repo paths covered, including the no-release-found → empty case. The final exit criterion (live re-seed of dev-lead/v139-*) is a verification step outside this PR.

Findings

Unresolved review threads (blocking auto-approve — unchanged since the prior review):

  1. gemini-code-assist (medium, tests/canary_rollout.bats:3551 and :3559) — suggests [ "$status" -eq 1 ] instead of [ "$status" -ne 0 ]. -ne 0 matches the existing convention in this test file, so refuting-and-resolving remains a reasonable disposition.
  2. codeant-ai (Major, tests/canary_rollout.bats:3576) — stubs return both refs in one response, so a release ref dropped by API pagination would go undetected. The underlying pagination gap is real but pre-existing (gh api matching-refs without --paginate); a follow-up issue is the right vehicle, not this PR.

No thread has replies or resolution since the cycle-1 fix-request, and the head SHA is unchanged — the requested action (resolve or refute the three threads) has not yet happened.

Non-blocking notes:

  • CodeAnt nitpick on the shared for-each-ref stub (tests/canary_rollout.bats:1247) — minor test-fidelity point.
  • gemini's nested-quoting suggestion on ${ref#refs/tags/"$agent"/} was refuted and both threads are resolved.
  • MCP secret-scanning tool unavailable in this run; gitleaks CI check passed.

CI status

All substantive checks green: Lint, ShellCheck, bats, CodeQL, SonarCloud (quality gate passed), gitleaks, Agent Security Scan, AgentShield, duplicate-decl-gate, dependency-audit. dev-lead / dispatch and ci-relay CANCELLED entries are concurrency-cancelled duplicate agent triggers (primary runs SUCCESS), not failures. mergeStateStatus=BLOCKED reflects the pending required org-leads review, not failing checks.


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
don-petry disabled auto-merge September 1, 2026 02:02
@donpetry-bot

Copy link
Copy Markdown
Contributor

CI checks on this PR are still running. Once they complete, re-mention @donpetry-bot to trigger a fresh review.

Posted by the donpetry-bot PR-review cascade.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) September 1, 2026 02:11
@don-petry
don-petry disabled auto-merge September 1, 2026 02:13
@don-petry
don-petry enabled auto-merge (squash) September 1, 2026 02:14
@don-petry
don-petry disabled auto-merge September 1, 2026 02:15
@don-petry

Copy link
Copy Markdown
Contributor Author

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

Agent reasoning
Issues addressed: 0
- Quality Gate already passing with 0 new issues — no fixes needed
- Non-blocking suggestions from gemini-code-assist (simplify quotes, improve assertions) — not required for merge
Files changed: None
Skipped (informational): 0
```
**No actionable issues to fix.** The PR is passing all quality gates and CI checks. The suggestion review from gemini-code-assist is informational and not blocking.

@don-petry
don-petry enabled auto-merge (squash) September 1, 2026 02:16
@donpetry-bot

donpetry-bot commented Sep 1, 2026 •

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

Review — fix requested (cycle 3/3)

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

Findings to fix

Automated review — NEEDS HUMAN REVIEW

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

Summary

Re-review at new head 576d04a (prior review was at 9cf2f06). All findings from the prior fix-request are now resolved in code: (1) the two gemini test-assertion threads — the rejection tests now pin [ "$status" -eq 1 ] — fixed and both threads marked resolved; (2) the CodeAnt pagination gap — --paginate was added to the gh api matching-refs call in _gh_candidate_cut_date plus a new pagination-guard regression test, and the shared for-each-ref stub now emits a realistic annotated-release line. The #1046 fix itself remains sound: _is_release_tag_suffix filters both the cross-repo API path and the same-repo for-each-ref path to strict vMAJOR.MINOR.PATCH release tags, and the local path only trusts an annotated tag's dereferenced commit + tagger date. Verified the implementation at the head SHA directly. One decision gate still fails: the CodeAnt pagination thread (tests/canary_rollout.bats:3576) has a substantive owner reply describing the fix — which I verified landed — but the thread is not marked resolved in GitHub. Everything else is green; escalating on that single formality per the no-unresolved-threads gate. Remaining action: mark that one thread resolved (no code change needed).

Linked issue analysis

Closes #1046 (v-scoped channel tag shadows the release tag in _gh_candidate_cut_date, wedging the canary gate at BLOCKED indeterminate; blocks .github-private#1592). Substantively addressed at this head: (1) cross-repo API path filters matching-refs results to immutable release suffixes before matching, now with --paginate so a release ref pushed to page 2 by channel refs is not dropped; (2) same filter on the local for-each-ref path; (3) local path uses only the annotated tag's tagger date, skipping lightweight release-named tags, with the commit-date fallback unchanged; (4) regression tests reproduce the exact live shadow ordering (lightweight v139-next sorting before annotated v139.4.0 at the same commit) for both paths, plus the only-channel-tag → empty case and a pagination guard. The final exit criterion (live re-seed of dev-lead/v139-*) is a verification step outside this PR.

Findings

Prior findings — all resolved by the new commits:

  1. gemini (tests/canary_rollout.bats, 2 threads): -ne 0 → -eq 1 assertions — fixed at head, both threads resolved.
  2. gemini (scripts/canary-rollout.sh:368/:405, 2 threads): nested-quote simplification — correctly refuted by owner (glob-metacharacter safety in ${var#pattern}), both threads resolved.
  3. codeant-ai (Major, tests/canary_rollout.bats:3576): pagination omission — fixed: --paginate added in _gh_candidate_cut_date and a pagination-guard test added. Owner replied with the fix details; I verified both landed at head. However, the thread is still not marked resolved (isResolved=false) — this is the sole failing decision gate.

New issues: none found in the delta (9cf2f06...576d04a: the review-changes commit plus a clean merge of main, which brought in the unrelated already-merged #1048).

Non-blocking notes:

  • CodeAnt's shared-stub nitpick (bats:1247/:1340) also addressed — stub now emits an annotated-release for-each-ref line.
  • MCP secret-scanning tool unavailable in this run; gitleaks CI check passed.

Single remaining action: mark the codeant-ai thread at tests/canary_rollout.bats:3576 as resolved. No code changes required — on the next cycle this PR meets all gates.

CI status

All checks green at head 576d04a: Lint, ShellCheck, bats (both jobs), CodeQL, SonarCloud (quality gate passed), Secret scan (gitleaks), Agent Security Scan, AgentShield, duplicate-decl-gate, dependency-audit (npm audit pass; other ecosystems skipped), CodeRabbit. dependabot-automerge and ci-relay SKIPPED as expected. mergeStateStatus=BLOCKED reflects the pending required review, not failing checks.


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
don-petry disabled auto-merge September 1, 2026 02:18
@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) September 1, 2026 02:26
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-01T03:27:58Z.

@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: 087878d2786eca83c1abdbb2434d96dbd46a4fa6
Review mode: triage-approved (single reviewer)

Summary

Re-review at head 087878d (prior fix-request was at 576d04a, cycle 3/3). The sole failing gate from the prior review — the unresolved codeant-ai pagination thread at tests/canary_rollout.bats:3576 — is now marked resolved; all 5 review threads on the PR are resolved. The delta since the prior reviewed SHA is exclusively two merges of main (bringing in the already-merged, unrelated #1050 and #1053); the PR's own diff (scripts/canary-rollout.sh, scripts/lib/canary-rollout.sh, tests/canary_rollout.bats) is byte-identical to what was verified last cycle. The #1046 fix is sound: _is_release_tag_suffix filters both the cross-repo API path (with --paginate) and the same-repo for-each-ref path to strict vMAJOR.MINOR.PATCH release tags, the local path only trusts an annotated tag's dereferenced commit + tagger date, and regression tests cover the shadow ordering, the only-channel-tag case, and pagination. All decision gates pass — approving.

Linked issue analysis

Closes #1046 (v-scoped channel tag such as dev-lead/v139-next shadows the release tag in candidate cut-date resolution, wedging the canary gate at BLOCKED indeterminate). Substantively addressed: (1) cross-repo API path filters matching-refs results to immutable release suffixes before matching, with --paginate so a release ref pushed past page 1 by channel refs is not dropped; (2) the same strict filter on the local for-each-ref path; (3) the local path uses only the annotated tag's tagger date, skipping lightweight release-named tags, with the commit-date fallback unchanged; (4) regression tests reproduce the exact live shadow ordering for both paths. The live re-seed of dev-lead/v139-* remains a post-merge verification step outside this PR, as noted last cycle.

Findings

Prior findings — all resolved:

  1. gemini test-assertion threads (2): fixed (-eq 1 assertions), threads resolved.
  2. gemini nested-quote suggestions (2): correctly refuted by owner, threads resolved.
  3. codeant-ai pagination gap (Major, tests/canary_rollout.bats:3576): fix (--paginate + pagination-guard test) verified last cycle; the thread — the single failing gate at 576d04a — is now marked resolved.

New issues: none. The delta 576d04a...087878d contains only two clean merges of main (unrelated, already-merged #1050 and #1053); no changes to this PR's files.

Non-blocking notes:

  • MCP secret-scanning tool unavailable in this run; the gitleaks CI check passed at head.
  • A prior sweep run withheld approval because advisory bots (CodeRabbit/Codex/Qodo) were rate-limited; substantive advisory coverage exists from CodeAnt and Gemini across 3 review cycles, and CodeRabbit reports no unresolved comments.

CI status

All validation checks green at head 087878d: Lint, ShellCheck, bats (both jobs), CodeQL, Analyze (actions), SonarCloud (Quality Gate passed), Secret scan (gitleaks), Agent Security Scan, AgentShield, duplicate-decl-gate, SonarCloud Code Analysis, dependency-audit (npm audit pass; other ecosystems skipped), pr-auto-review, CodeRabbit. The only non-green entries are dev-lead / dispatch and dev-lead / ci-relay runs CANCELLED — concurrency-superseded agent-automation dispatches, not CI validation. mergeStateStatus=BLOCKED reflects the pending required review, not failing checks.


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

@don-petry
don-petry merged commit 02825fb into main Sep 1, 2026
27 of 37 checks passed
@don-petry
don-petry deleted the dev-lead/issue-1046-20260901-0128 branch September 1, 2026 02:32
don-petry added a commit that referenced this pull request Sep 1, 2026
Resolves the conflict in tests/canary_rollout.bats by keeping BOTH test
blocks: #1046's `_is_release_tag_suffix` suite (landed on main via #1051)
and this PR's `_is_evicted_run` suite. Both branches appended to the same
region of the file; neither change touches the other's code.

Hand-resolved because dev-lead's rebase dispatch was itself cancelled by a
concurrency eviction (run 33462968616) — the exact defect this PR fixes.

Verified on the merged tree:
  - _is_evicted_run declared 1x, _is_release_tag_suffix declared 1x
  - 9 + 3 tests present, no duplicate @test names, no conflict markers
  - duplicate-decl-gate (live on main since #1033) passes
  - bash -n clean; no repeated statement blocks
don-petry added a commit that referenced this pull request Sep 1, 2026
Addresses the review nit on tests/canary_rollout.bats. A generic `-ne 0`
lets a script error (syntax error, command-not-found) pass as a expected
failure; `-eq 1` pins the contract.

Verified every non-eviction path returns exactly 1 before tightening:
cancelled+3, success+0, failure+0, empty count, non-numeric count,
missing arg, and no args all exit 1.

Matches the convention already used by the _is_release_tag_suffix suite
that landed on main via #1051.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

2 participants