feat: implement issue #743 — Canary promotions fail at tag-move step — _gh_move_tag swallows the API error (persistent since 2026-07-15)

[don-petry]

Closes #743

Implemented by dev-lead agent. Please review.

[chatgpt-codex-connector]

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

[coderabbitai]

Warning

Review limit reached

@don-petry, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 36cd88b9-06a1-4f0e-823c-37667f395284

📥 Commits

Reviewing files that changed from the base of the PR and between 85c4703 and c21a7ff.

📒 Files selected for processing (2)

• scripts/canary-rollout.sh
• tests/canary_rollout.bats

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev-lead/issue-743-20260715-1439

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

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

No changes were needed for this PR.

[Copilot]

Pull request overview

This PR fixes the Canary Rollout promotion pipeline by making _gh_move_tag surface the underlying gh api failure details instead of suppressing them, and by ensuring the POST (create-ref) fallback is only attempted when the PATCH failed due to a genuine 404 (missing ref). This aligns with issue #743’s diagnostic goal: unmask the real GitHub API rejection so the next promotion-due run is actionable.

Changes:

• Update scripts/canary-rollout.sh _gh_move_tag to capture and emit the gh api error output as a single-line ::error:: message, rather than discarding it.
• Restrict the POST create-ref fallback path to only 404/not-found PATCH failures to avoid masking non-404 rejections.
• Add Bats regression tests covering: successful PATCH, non-404 PATCH rejection (no POST), 404 fallback to POST, and POST failure surfacing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
scripts/canary-rollout.sh	Stop swallowing gh api errors in _gh_move_tag, emit actionable failure details, and only POST-create on genuine 404.
tests/canary_rollout.bats	Add regression coverage ensuring _gh_move_tag surfaces errors and uses POST fallback only for missing refs.

[gemini-code-assist]

Code Review

This pull request improves error handling in the _gh_move_tag function within scripts/canary-rollout.sh by capturing and surfacing GitHub API errors instead of swallowing them. It ensures that the fallback to creating a tag via POST is only triggered when the PATCH request returns a genuine 404 ("not found") error, preventing other API failures (such as ruleset rejections) from being masked. Comprehensive unit tests have been added in tests/canary_rollout.bats to verify these behaviors. There are no review comments, so I have no feedback to provide.

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

Addressed 0 threads:
  (no open threads)
Test verification: PASS — 215/215 bats tests passed (tests/canary_rollout.bats)
  Tests 100–103 specifically cover the _gh_move_tag fix from this PR (issue #743):
    ok 100 _gh_move_tag: surfaces the API rejection and does NOT fall back to POST on a non-404 PATCH failure
    ok 101 _gh_move_tag: a successful PATCH moves the tag and never falls back to POST
    ok 102 _gh_move_tag: falls back to POST (create) when the ref is genuinely absent (404)
    ok 103 _gh_move_tag: surfaces the create error when the 404 POST fallback also fails
Files changed: none
```
The PR is clean: no open review threads, no CI failures, no CHANGES_REQUESTED reviews, and all tests pass. Both bot reviewers left only positive `COMMENTED` reviews summarizing the fix.

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Diagnostic fix for issue #743: _gh_move_tag in scripts/canary-rollout.sh no longer swallows the gh api error — the PATCH response is captured and surfaced as a single-line ::error::, and the POST create-ref fallback now only fires on a genuine 404/'not found' so non-404 rejections (e.g. ruleset/bypass) are no longer masked by a subsequent 422. Four new bats regression tests cover all paths (PATCH ok, non-404 reject without POST fallback, 404-to-POST success, 404-to-POST failure). Implementation is correct (proper rc capture via the assignment-&&-return pattern, local vars, newline-flattened annotation output) and matches the issue's acceptance criteria.

Linked issue analysis

Issue #743 (canary promotions fail at tag-move; error suppressed) asked for (1) a diagnostic change surfacing the real API rejection with a regression test, and (2) a fix for whatever that reveals. This PR fully delivers part 1 and satisfies acceptance criterion 1 (no silent 2>/dev/null on the operative call; bats coverage of the failure path). Criterion 2 is met in its alternate form: the next promotion-due run will clearly report the real API reason. Part 2 is inherently follow-up work once the underlying rejection is visible — if the next promotion-due run still fails, expect a follow-up issue with the now-visible error. No changes to immutable release tags or the refs/tags ruleset (criterion 3).

Findings

No blocking findings. Notes: (1) The 404 detection is a substring heuristic on the error text ('not found'/'http 404'); a false-positive match would merely trigger the POST fallback whose own error is then surfaced — nothing gets masked. (2) Secret scan: the run_secret_scanning MCP tool was not available in this session; gitleaks CI check passed and the diff contains no credential-like content. (3) Both bot reviews (Copilot, Gemini) were comment-only with zero findings; no unresolved review threads; dev-lead fix-reviews pass reported 215/215 bats tests green.

CI status

All checks green: ShellCheck ✓, Lint ✓, Canary-rollout Tests (lint + bats) ✓, CodeQL ✓, SonarCloud Quality Gate passed (0 issues, 0 hotspots), Secret scan (gitleaks) ✓, Agent Security Scan ✓, AgentShield ✓, npm audit ✓, CodeRabbit status SUCCESS. Remaining SKIPPED checks are ecosystem-conditional audits not applicable to this change. mergeable=MERGEABLE; mergeStateStatus=BLOCKED pending review only.

---

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

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Diagnostic fix for issue #743: _gh_move_tag in scripts/canary-rollout.sh no longer swallows gh api errors. The PATCH response is captured and surfaced as a single-line ::error:: annotation, and the POST create-ref fallback now fires only on a genuine 404/'not found' so non-404 rejections (ruleset/bypass/permission) are no longer masked by a subsequent 422 'Reference already exists'. Four new bats regression tests cover all paths (PATCH ok, non-404 reject without POST fallback, 404-to-POST success, 404-to-POST failure). Implementation is correct: proper rc capture via the assignment-&&-return pattern, local vars, lowercase substring match, newline-flattened annotation output.

Linked issue analysis

Issue #743 (canary promotions fail at tag-move; error suppressed) asked for (1) a diagnostic change surfacing the real API rejection with regression-test coverage, and (2) a fix for whatever the visible error reveals. This PR fully delivers part 1 and satisfies acceptance criterion 1 (no silent 2>/dev/null on the operative call; bats coverage of the failure path emitting ::error::). Criterion 2 is met in its alternate form: the next promotion-due run will clearly report the real API rejection. Part 2 is inherently follow-up work once the underlying rejection is visible. Criterion 3 holds: no changes to immutable release tags or the refs/tags ruleset.

Findings

No blocking findings. Notes: (1) The 404 detection is a substring heuristic on the error text ('not found'/'http 404'); a false-positive match would merely trigger the POST fallback whose own error is then surfaced — nothing gets masked, and a POST on an existing ref cannot succeed silently. (2) Secret scan: the run_secret_scanning MCP tool was not available in this session; the gitleaks CI check passed and the diff contains no credential-like content. (3) Nit (non-blocking): the ::error:: lines are emitted to stderr; the Actions runner scans both streams, and even if annotation rendering varied the raw error text still lands in the log, which satisfies the diagnostic goal. (4) Both bot reviews (Copilot, Gemini) were comment-only with zero findings; no unresolved review threads; dev-lead fix-reviews pass reported 215/215 bats tests green, including tests 100–103 covering this fix. Triage assessment confirmed — nothing missed.

CI status

All checks green: ShellCheck ✓, Lint ✓, Canary-rollout Tests (lint + bats) ✓, CodeQL ✓, SonarCloud Quality Gate passed (0 issues, 0 hotspots), Secret scan (gitleaks) ✓, Agent Security Scan ✓, AgentShield ✓, npm audit ✓, CodeRabbit SUCCESS. Remaining SKIPPED checks are ecosystem-conditional audits not applicable to this change.

---

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