feat: implement issue #1242 — canary-rollout: move the pair/ring verdict (per-pair state, tag-gap hold) into the pure gate core

[don-petry]

Problem

canary-rollout: move the pair/ring verdict (per-pair state, tag-gap hold) into the pure gate core

From the issue: AGENTS.md (decision-making reusables, L124–125) says decision logic belongs in the pure, side-effect-free core scripts/lib/canary-rollout.sh, with the orchestrator scripts/canary-rollout.sh feeding it facts gathered from gh.

Risk

Low — changes automation shell logic under scripts/, covered by shellcheck (--severity=warning) and the bats suite.

Test plan

Tests added/updated: tests/canary_rollout.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's Lint (shellcheck) and bats checks show pass/fail; watch subsequent dev-lead / pr-review runs for behavioral regressions.

Closes #1242

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 03974d58-7e7c-4326-9cc3-abb5297c77e0

📥 Commits

Reviewing files that changed from the base of the PR and between 36444b1 and f6b8090.

📒 Files selected for processing (3)

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

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[don-petry]

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1243
No changes were committed, but the PR still can't be marked done: required check SonarCloud is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-10-03T17:17:46Z

[don-petry]

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check SonarCloud is still pending. I'll re-check automatically.
Next attempt after: 2026-10-03T17:17:46Z

[don-petry]

No description provided.

[codeant-ai]

CodeAnt Nitpicks

2 code suggestions

1. Every tag lookup is marked known, so an API outage returning empty commits is treated as absent tags; all pairs can become ON_CANDIDATE and falsely report COMPLETE.

Incorrect condition logic · scripts/canary-rollout.sh:1322

---

2. The data-gap reconstruction also marks failed tag lookups as known; an outage-affected pair with both empty commits is omitted instead of being tracked BLOCKED.

Incorrect condition logic · scripts/canary-rollout.sh:1915

[gemini-code-assist]

Code Review

This pull request introduces a new pair_verdict function to handle transition states between adjacent rings in the canary rollout process, improving robustness against tag-lookup errors and unresolvable sources. It also adds comprehensive BATS tests to verify these verdicts. The review feedback suggests a best practice for the new BATS tests: explicitly asserting [ "$status" -eq 0 ] immediately after executing commands with run to ensure failures are caught explicitly.

[donpetry-bot]

Superseded by automated re-review at 70459e74dd337f999f80dac852508bfb15fe67a2 — 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: 70459e74dd337f999f80dac852508bfb15fe67a2
Review mode: triage-approved (single reviewer)

Summary

Pure refactor of the canary-rollout release gate: adds a side-effect-free pair_verdict to scripts/lib/canary-rollout.sh with a full decision-matrix bats suite, and switches _frontier_state / _frontier_state_resilient to use it. I checked that behavior is unchanged. The code looks correct, but there is still an unresolved review thread and one CI check (cubic) is still running, so this can't be auto-approved.

Linked issue analysis

Closes #1242. Status of each acceptance criterion:

• Pure verdict function in lib (no gh, temp-file or env access): met. pair_verdict is pure and its precedence is explicit (HOLD_UNKNOWN > ON_CANDIDATE > UNRESOLVABLE_SOURCE > PENDING).
• Decision-matrix tests: met. The tests cover the full {resolved, absent, errored} × {src, dst} matrix, precedence, the empty-vs-- sentinel, fail-closed handling of unknown flags, the all-absent legacy COMPLETE and the total-outage case. None of them depend on gh.
• Existing end-to-end tests pass unchanged: met. No existing test is modified and the bats CI check is green.
• The "_ring_commits gathers whether the lookup errored" part is only partly done. feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240 (canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1225) is still OPEN, so the orchestrator cannot yet distinguish an errored lookup. Both call sites hard-code unknown=0 and say so in a comment. This means the HOLD_UNKNOWN branch in _frontier_state is unreachable for now, and the tag-gap hold still has to be wired in at both call sites when feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240 lands. The issue itself says to do this work after feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240, so the PR is ahead of that order. This is acceptable, but whoever lands feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240 must pass the real unknown flags at both call sites.

Findings

Blocking (gate):

1. Unresolved review thread (codeant-ai, scripts/canary-rollout.sh:1917). In the data-gap path, an empty source commit makes _reusable_differs return 0, which leads to classify_failure returning PRE_EXISTING instead of REGRESSION. I checked this: the behavior is pre-existing. The old code made the same _reusable_differs "$agent" "$cand" "$prior" call with an empty cand, so this PR introduces no regression, and the pair is still tracked as BLOCKED either way. The thread still needs a disposition: either fix it here (e.g. treat an empty cand as differs=1 / REGRESSION to fail closed) or reply and resolve it with a follow-up issue.

Non-blocking:
2. The CodeAnt nitpicks ("every tag lookup is marked known" at L1322/L1915) describe the deferred #1225 wiring explained above. They are pre-existing behavior, not a regression, and the in-code comment acknowledges them.
3. The HOLD_UNKNOWN branch in _frontier_state emits triage - on a BLOCKED line, while the format doc says triage is REGRESSION|PRE_EXISTING|SUSPECT when BLOCKED. The branch is unreachable today, but it should be reconciled when #1240 wires in the unknown flags.

Behavior-preservation check: channel_commit returns either a SHA or an empty string, never a literal -. Mapping empty to - therefore keeps the old dstc != cand equality exactly (old: both empty → equal; new: both - → ON_CANDIDATE). The orchestrator acts the same way on PENDING and UNRESOLVABLE_SOURCE (_pair_state), so its output is unchanged. No secrets and no workflow changes.

CI status

Lint, ShellCheck, bats, Lint and bats, CodeQL, SonarCloud (Quality Gate passed), gitleaks, AgentShield, Agent Security Scan and duplicate-decl-gate are all green. The dev-lead dispatch/ci-relay/resume runs show some CANCELLED entries, but each has a successful or skipped duplicate run (superseded runs). cubic · AI code reviewer is still IN_PROGRESS.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). 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]

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

Agent reasoning

Bot: sonarqubecloud[bot]
Status: "4 New issues" reported without specific file/line details
Action taken: Code review for known SonarQube hotspot patterns — none identified
Quality Gate: PASSED (non-blocking)
Result: Unable to address issues due to lack of specificity in bot comment
```

[donpetry-bot]

Superseded by automated re-review at 70459e74dd337f999f80dac852508bfb15fe67a2 — 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: 70459e74dd337f999f80dac852508bfb15fe67a2
Review mode: triage-approved (single reviewer)

Summary

This re-review covers the same commit (70459e7) as the cycle-1 fix-requested review, and no new commits have been pushed. The change still looks correct and keeps existing behaviour: a pure pair_verdict is added to scripts/lib/canary-rollout.sh, and _frontier_state / _frontier_state_resilient now use it. It can't be auto-approved yet: 5 review threads are unresolved and one CI check (cubic) is still in progress.

Linked issue analysis

Closes #1242. The pure core function and the gh-free decision-matrix tests are in place, and the existing end-to-end tests pass unchanged. Wiring "lookup errored" through from _ring_commits is deferred to #1240 / #1225, which is still open. Both call sites hard-code unknown=0 and say so in a comment. That means the HOLD_UNKNOWN branch can't be reached yet, and whoever lands #1240 must pass the real unknown flags at both call sites. This order is acceptable.

Findings

Blocking (gate 4: unresolved review threads):

1. codeant-ai scripts/canary-rollout.sh:1917: in the data-gap path, an empty source commit makes _reusable_differs return 0, so the failure is classified as PRE_EXISTING instead of REGRESSION. This behaviour is pre-existing: the old code made the same call, and the pair is still tracked as BLOCKED. But the thread has no reply. Either fail closed here (treat an empty cand as differs=1) or reply and resolve it with a follow-up issue.
2. gemini-code-assist tests/canary_rollout.bats:891, 895, 900, 904 (4 threads, low priority): add [ "$status" -eq 0 ] after each run _pv_rings .... A trivial test-hardening change. Apply it or resolve the threads.

The dev-lead fix-bot-comment run handled only the SonarCloud summary comment (no changes). None of the threads above got a reply or a disposition, and no commit followed the cycle-1 review.

Non-blocking (carried forward):

• The CodeAnt nitpicks at L1322 / L1915 ("every tag lookup is marked known") are the deferred canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1225 wiring. They are not a regression in this PR.
• The HOLD_UNKNOWN branch prints triage - on a BLOCKED line, but the format doc says BLOCKED lines carry REGRESSION|PRE_EXISTING|SUSPECT. The branch can't be reached today. Fix this when feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240 lands.

Behaviour-preservation check: ${x:--} maps an empty commit to -, so dstc != cand still behaves exactly as before. _pair_state handles PENDING and UNRESOLVABLE_SOURCE the same way. No secrets, workflow, or auth changes. The MCP secret-scanning tool isn't available in this environment; gitleaks is green.

CI status

Green: Lint, ShellCheck, bats, Lint and bats, CodeQL, SonarCloud (Quality Gate passed), gitleaks, AgentShield, Agent Security Scan, duplicate-decl-gate. The CANCELLED dev-lead dispatch/ci-relay/resume entries were superseded by successful or skipped re-runs. cubic · AI code reviewer is still IN_PROGRESS.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). 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.

[cubic-dev-ai]

All reported issues were addressed across 3 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[don-petry]

Dev-Lead — fix-reviews (partial)

A commit was pushed, but not every requested change was applied. Per requested item:

• scripts/canary-rollout.sh:1917 — applied
• tests/canary_rollout.bats:891 — applied
• tests/canary_rollout.bats:895 — applied
• tests/canary_rollout.bats:900 — applied
• tests/canary_rollout.bats:904 — applied
• scripts/canary-rollout.sh:1322 — not applied
• tests/canary_rollout.bats:890 — applied

The unaddressed items above still need work.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: fix-reviews)

PR: #1243
No changes were committed, but the PR still can't be marked done: required check SonarCloud is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-10-03T17:25:32Z

[don-petry]

No description provided.

[donpetry-bot]

Superseded by automated re-review at 074cfcf8836625756135fa1e902de684c9934b0b — 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: 70459e74dd337f999f80dac852508bfb15fe67a2
Cascade: triage → deep+duck (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: gemini-3.8-flash [sonnet 5.5] → audit: opus 5.5 [opus 4.8, opus 4.7])

Summary

Both reviewers rate the PR MEDIUM risk and escalate, so they agree fully. They converge on two points: the HOLD_UNKNOWN fail-closed path is dead because both call sites pass unknown=0 (the #1225 gap, with #1240 unmerged), and the empty-source differs=1 change in _frontier_state_resilient is an untested behavior change that goes beyond a pure refactor. The two reviewers labelled these findings with different categories, so they were not merged under the exact file+line+category rule. The rubber duck alone added the sentinel-normalization placement, the missing orchestrator-wiring tests, and the unconfirmed CI state (dev-lead dispatch CANCELLED).

Cross-engine agreement

full

Findings

• minor (scripts/canary-rollout.sh:1322): _frontier_state and _frontier_state_resilient call pair_verdict with unknown=0 for both rings every time, so the HOLD_UNKNOWN / fail-closed branch never runs. A total tag-lookup outage still reads as ON_CANDIDATE and COMPLETE. This matches base behavior and is the open canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1225 gap; PR feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240 is not merged and canary-rollout: move the pair/ring verdict (per-pair state, tag-gap hold) into the pure gate core #1242 asked for it to land first. The cubic-dev-ai P1 and codeant threads are unresolved. Either wait for feat: implement issue #1225 — canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1240 or reply to and resolve the threads, saying the gap is tracked in canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1225.
• major (scripts/canary-rollout.sh:1322): Both orchestrator call sites pass unknown=0 for both rings, so HOLD_UNKNOWN is never produced and the new *) BLOCKED branch in _frontier_state is dead code. A tag-lookup API outage still reads as ABSENT, giving ON_CANDIDATE and COMPLETE, which is the fail-open the core documents against. This is a pre-existing limitation, but the PR should link the follow-up (canary-rollout: tag resolution cannot tell an API error from an absent tag, so a total lookup outage reads as "fully rolled out" #1225) or propagate the error state.
• minor (scripts/canary-rollout.sh:1919): Commit 24c2733 was pushed after this review was dispatched for 70459e7. In _frontier_state_resilient an empty source commit now forces differs=1, so classify_failure returns REGRESSION instead of PRE_EXISTING. Failing closed is the right direction, but canary-rollout: move the pair/ring verdict (per-pair state, tag-gap hold) into the pure gate core #1242 requires a pure refactor with no behavior change, and no bats test covers this classification. Add a test, or move the change to its own issue or PR.
• minor (scripts/canary-rollout.sh:1919): In _frontier_state_resilient, an empty src commit now forces differs=1, so classify_failure yields REGRESSION instead of PRE_EXISTING. This is arguably safer, but it is an unrelated semantic change in a 'move logic to core' refactor. It has no test, the PR description does not mention it, and the differs decision stays in the orchestrator instead of the pure core.
• minor (scripts/lib/canary-rollout.sh:458): pair_verdict uses '-' as the absent sentinel. The orchestrator wrapper repeats the empty-to-'-' normalization at each call site, so the pure core is the better place to enforce it.
• minor (tests/canary_rollout.bats:811): The tests cover pair_verdict and a test-only _pv_rings fold, but not the orchestrator wiring in _frontier_state or _frontier_state_resilient (BLOCKED line emission, the differs=1 change). Some single-assertion tests also skip an explicit run and $status check.
• info (tests/canary_rollout.bats:890): The P3 bot finding (missing [ "$status" -eq 0 ] after run) is fixed in 24c2733, which adds the assertion to all four _pv_rings run calls. The cubic thread is not resolved yet. Bats could not be run in the review sandbox; CI bats passed at 70459e7.
• info: dev-lead / dispatch shows CANCELLED. The other check conclusions came back empty in statusCheckRollup, so it could not be confirmed that the required bats and shellcheck checks are green.

---

Reviewed by the PR-review cascade (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: gemini-3.8-flash [sonnet 5.5] → audit: opus 5.5 [opus 4.8, opus 4.7]). 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.

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[don-petry]

Dev-Lead — fix-reviews (partial)

A commit was pushed, but not every requested change was applied. Per requested item:

• scripts/canary-rollout.sh:1322 — not applied
• scripts/canary-rollout.sh:1919 — applied

The unaddressed items above still need work.

[donpetry-bot]

Automated review — escalated to human

The automated review cascade escalated this PR to a human reviewer at review cycle 3/3 (risk: MEDIUM, reviewed commit 24c2733a185bb2ac5d075e4c0ac606f3b856eadd).

Why: the cascade could neither approve the PR nor auto-request fixes, so it requested human review via CODEOWNERS and set the needs-human-review label. A human should review the PR, or remove the needs-human-review label to re-engage the automated cascade.

Reviewer summary: Both reviewers rate the PR MEDIUM risk and escalate, so they agree fully. They converge on two points. The review-fix change in _frontier_state_resilient (empty source now gives differs=1, so REGRESSION instead of PRE_EXISTING) is an untested behavior change in a refactor scoped as 'no behavior change'. Both pair_verdict call sites also hardcode unknown=0, which leaves the HOLD_UNKNOWN tag-gap path dead until #1225/#1240 land. The rubber duck alone added that no test exercises _frontier_state or _frontier_state_resilient through the new verdict, so a wiring regression would go unnoticed, and noted the harmless '-' sentinel conflation.

This note is updated in place on re-escalation; it is not re-posted.

[don-petry]

dev-lead is withholding action on this item.

It is labeled needs-human-review (flagged for human review — this label is applied by automation as well as by people, so an item can become held without anyone noticing), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the needs-human-review label.

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[don-petry]

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

[don-petry]

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

[don-petry]

Merged main (#1238, #1240) into this branch (074cfcf, fast-forward, no force). Two real conflicts, both in the pair-verdict code: this PR passed unknown=0 to pair_verdict because #1225 had not landed; main now supplies per-ring tag-lookup errors. Resolution: pair_verdict receives the real unknown flags, and HOLD_UNKNOWN keeps main's behaviour exactly (tag-gap warning, _CANARY_TAG_GAP_FLAG, BLOCKED line with the same fields). The sync-issues path keeps main's tag-gap row, then uses pair_verdict for the pending check. No existing test was changed or removed (the test diff against main is additions only); the full suite passes (415/415) and shellcheck --severity=warning is clean on both scripts.

@donpetry-bot please review head 074cfcf.

---

Generated by Claude Code

[donpetry-bot]

@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes.

[don-petry]

CI note: SonarCloud failed on 074cfcf before any analysis ran — the scanner could not fetch the pull request from SonarCloud (Something went wrong while trying to get the pullrequest with key '1243', after a ~34s timeout, exit code 3). That is a SonarCloud-side API failure, not this diff (the SonarCloud Code Analysis result and the other checks are independent). I re-ran the failed job once; a second failure would be treated as real.

---

Generated by Claude Code

[donpetry-bot]

Superseded by automated re-review at f6b8090c14020b97b8f3b3ee4b4c5819d2f6f12c — 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: 074cfcf8836625756135fa1e902de684c9934b0b
Cascade: triage → deep+duck (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: gemini-3.8-flash [sonnet 5.5] → audit: opus 5.5 [opus 4.8, opus 4.7])

Summary

Both reviewers rate risk MEDIUM, but the deep reviewer escalates on gates (an unresolved cubic P2 thread, an out-of-scope behavior change, and missing test coverage) while the rubber duck approves, so agreement is partial. Both flagged the same behavior change in _frontier_state_resilient: an empty source commit now yields triage '-' instead of a classify_failure result. They filed it under different categories, so the entries are not merged. Both also noted that the new branch has no orchestrator-level test. The findings unique to the rubber duck are that the bats tests don't exercise the _frontier_state case arms and that two '-' / empty sentinel definitions could drift. The deep reviewer alone noted that _frontier_state_resilient duplicates the HOLD_UNKNOWN check by hand.

Cross-engine agreement

partial

Findings

• minor: The issue says this is a pure refactor with no behavior change, but one data-gap case changes. In _frontier_state_resilient, an unresolvable (empty) source commit used to get triage PRE_EXISTING and now gets '-'. No needs-human escalation is lost, because sync-issues ranks PRE_EXISTING and '-' the same. The change is out of the issue's stated scope and should be noted in the PR body or the issue.
• minor: In _frontier_state_resilient, an empty src commit now yields triage '-' instead of classify_failure(differs...). This is not a pure refactor and is described only in a code comment. Downstream consumers of the triage column may treat '-' differently. The pair stays BLOCKED, so it fails closed.
• minor: No bats test covers the new 'empty src commit + run-history data gap → BLOCKED with triage "-" and datagap=1' branch. The cubic P2 thread on this line is still unresolved. Add a test that stubs ring0's tag as absent while ring1 resolves, plus a fetch failure, and assert the '-' triage row.
• minor: Tests cover pair_verdict and a test-only fold helper (_pv_rings), not the orchestrator case arms in _frontier_state or the resilient path. The wiring, such as the catch-all arm treating unrecognised verdicts as HOLD, is untested.
• info: AC2 is only partly met. _frontier_state_resilient still checks prev_unknown/ch_unknown '= 1' by hand before calling pair_verdict, so the HOLD_UNKNOWN decision is duplicated there instead of coming from the pure core. Both paths fail closed. A 'case' on the verdict, as in _frontier_state, would leave one source of truth.
• info: pair_verdict treats a literal '-' commit and an empty commit as the same ABSENT value. This is fine given that commits are SHAs, but both callers must keep normalizing with ${x:--}. The resilient caller tests [ -z "$cand" ] separately, so the two sentinel definitions could drift.

---

Reviewed by the PR-review cascade (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: gemini-3.8-flash [sonnet 5.5] → audit: opus 5.5 [opus 4.8, opus 4.7]). 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]

@donpetry-bot please re-review head f6b8090. It addresses the cycle-1 findings: the empty-source - triage behaviour change is removed (the data-gap block is byte-identical to main again), a new test pins that an empty source commit stays tracked BLOCKED with triage PRE_EXISTING through _frontier_state_resilient, and pair_verdict uses [[ ]] (Sonar S7688). The triage policy question is tracked as #1244 item 14. Full suite passes (416/416), shellcheck is clean.

---

Generated by Claude Code

[donpetry-bot]

@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes.

[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

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Refactor of the canary-rollout release gate. It adds a pure pair_verdict to scripts/lib/canary-rollout.sh with gh-free decision-matrix bats tests, and _frontier_state / _frontier_state_resilient now act on its result. The new head f6b8090 fixes the cycle-1 finding: the empty-source - triage branch is gone, so the data-gap path is identical to main apart from the call to pair_verdict, and a new orchestrator test pins that behaviour. Behaviour is unchanged, CI is green and no threads are open.

Linked issue analysis

Closes #1242.

• Pure verdict in lib (no gh, temp-file or env access): met. Precedence is explicit: HOLD_UNKNOWN > ON_CANDIDATE > UNRESOLVABLE_SOURCE > PENDING.
• Decision-matrix tests: met. The tests cover the full {resolved, absent, errored} × {src, dst} matrix, precedence, empty vs -, fail-closed handling of unknown flags, all-absent COMPLETE and total-outage HOLD.
• Existing end-to-end tests unchanged: met. No existing test was modified (the only bats changes are additions) and bats CI is green.
• No behaviour change: met as of f6b8090. The cycle-1 empty-src triage change was reverted and is now pinned by a new _frontier_state_resilient test that expects PRE_EXISTING.

Findings

Prior (cycle-1, 074cfcf) findings:

• Empty src commit got triage - instead of classify_failure (not a pure refactor): resolved. Compared with origin/main, the only change in the data-gap block is the pair_verdict condition.
• No test for the empty-src data-gap branch: resolved. The new test asserts ring0->ring1 BLOCKED … PRE_EXISTING.
• Unresolved cubic P2 thread: resolved. No unresolved review threads remain.
• Orchestrator case arms not directly unit-tested: carried forward as non-blocking. The * arm fails closed (it holds BLOCKED and logs the tag gap), so a missing or unknown verdict cannot promote.
• _frontier_state_resilient still checks unknown = 1 by hand before pair_verdict: carried forward as non-blocking (info). Both checks fail closed. Folding the check into a case on the verdict later would leave one source of truth.

Equivalence check: _ring_commits emits a 0/1 unknown flag, and empty commits are normalised to - at both call sites. So pair_verdict reproduces the old unknown = 1 hold and the dstc != cand pending rule exactly: both empty gives ON_CANDIDATE, and PENDING and UNRESOLVABLE_SOURCE both go to _pair_state. The only difference is that an unexpected non-0/1 flag now fails closed, which is stricter. shellcheck --severity=warning is clean locally. No secrets, workflows or dependencies are touched.

New issues: none.

CI status

All substantive checks are green: Lint, ShellCheck, bats, Lint and bats, CodeQL (actions/python), SonarCloud (Quality Gate passed), gitleaks, AgentShield, Agent Security Scan, duplicate-decl-gate, dependency-audit, cubic and CodeRabbit. The CANCELLED dev-lead dispatch, resume and ci-relay entries are superseded automation runs, each with a successful or skipped duplicate, not code checks.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.