feat: implement issue #605 — Shadow-mode dual-run for safe agent canary validation - #1252
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
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: 29 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 (8)
✨ 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 — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-07-15T03:59:43Z. |
There was a problem hiding this comment.
Pull request overview
Implements the shadow-mode dual-run classification + signal contract for issue #605, enabling safe canary validation by comparing a silent next-channel agent run against the stable run on the same PR and emitting a machine-readable shadow_dual_run signal for the health-gated promotion workflow (#501).
Changes:
- Add a pure Bash classification library (
scripts/lib/shadow-compare.sh) plus a wrapper (scripts/shadow-run.sh) that emits JSON signal + step-summary Markdown. - Register
shadow_dual_runas a required gate signal fordev-leadinrelease/registry.ymland document the contract + integration status. - Add Bats coverage for the new library and wire the new test into the Lint workflow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_shadow_compare.bats |
New unit tests covering normalization, classification, blocking rules, JSON signal output, and report rendering. |
tests/test_release_registry.bats |
Asserts release/registry.yml now requires shadow_dual_run for the dev-lead gate. |
scripts/shadow-run.sh |
New wrapper script to classify stable vs shadow outputs and emit JSON signal + step summary without posting to PRs. |
scripts/lib/shadow-compare.sh |
New pure library implementing classification, blocking decision, JSON signal generation, and report rendering. |
release/registry.yml |
Declares shadow_dual_run as a required dev-lead gate signal and documents the signals field. |
docs/release/shadow-mode.md |
New documentation describing shadow-mode behavior, statuses, and signal contract. |
docs/release/runbook.md |
Updates the release runbook to reference shadow-mode as a required input to #501. |
.github/workflows/lint.yml |
Adds the new Bats test to the workflow’s test list. |
Superseded by automated re-review at
|
… tonumber Addresses the Copilot review threads on #1252: - sc_conclusion_present now treats a 'skipped' conclusion as absent: a gated-off (conditional/paths-filtered) shadow lane never ran, so stable=success + shadow=skipped classifies as NO_SHADOW (inconclusive), not a false BLOCKING REGRESSION. - sc_signal_json coerces run ids via (tonumber? // null) so a non-numeric value (e.g. the literal 'null' from gh --jq) yields null instead of making jq exit non-zero — which under set -euo pipefail would break shadow-run.sh's 'always exit 0, never disrupt the PR' contract. Adds regression tests for both (skipped->NO_SHADOW; non-numeric run id -> null, output still valid JSON). All 28 bats pass; shellcheck clean.
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Superseded by automated re-review at
|
|
@donpetry-bot re-review please — the carried-forward finding #4 is resolved. The PR no longer auto-closes #605: the body's All 3 code findings remain resolved, all Copilot threads resolved, CI green. |
|
@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: bde49276f999440f39d8b14af6829891419667cd
Review mode: triage-approved (single reviewer)
Summary
Delivers the comparison + signal half of shadow-mode dual-run (#605): a pure bash classification library (scripts/lib/shadow-compare.sh), a signal-emitting wrapper (scripts/shadow-run.sh), registry gate wiring (shadow_dual_run as a required dev-lead gate signal), docs, and 27 bats tests. Since the prior review (67fb3f1) the only changes are merges of main; all three prior review findings (jq tonumber crash on non-numeric run ids, 'skipped' conclusion misclassified as present, missing 'skipped' test coverage) are fixed in the current head and their threads resolved.
Linked issue analysis
Issue #605 (shadow-mode dual-run, prerequisite for #587, part of #495) is substantively addressed for the declared scope: this PR lands the comparison + promotion-signal contract, while the dispatch half (running the shadow lane and suppressing its PR output) is explicitly tracked in #1269. The PR correctly avoids a closing reference so #605 stays open until both halves land.
Findings
No blocking findings.
- Triage assessment confirmed: additive change, pure logic, conservative gate (only REGRESSION blocks promotion; DIVERGED is advisory).
- scripts/shadow-run.sh writes only fixed-vocabulary values (status tokens, true/false) to GITHUB_ENV — no injection surface from lane outputs.
- .github/workflows/lint.yml change is a one-line bats test registration; no permissions or trigger changes.
- All 3 prior review threads resolved and verified fixed in the diff (tonumber? // null coercion; skipped listed as absent-run sentinel; NO_SHADOW skipped test added).
- run_secret_scanning MCP tool unavailable in this environment; gitleaks CI check passed (SUCCESS).
CI status
All validation checks green: shellcheck, ShellCheck, bats, unit-tests, Lint, actionlint, CodeQL (actions + python), SonarCloud, gitleaks secret scan, agent-shield, guard, holdout-guard, and all structure/permission validators SUCCESS. Conditional dependency-audit jobs SKIPPED (no matching ecosystems). Two CANCELLED entries (dev-lead / dispatch, dev-lead / ci-relay) are agent automation lanes superseded by later pushes, not validation checks.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.



Part of #605 (comparison + signal half only). This PR delivers only half of #605 — the dispatch half (run the shadow lane + suppress its PR output) is tracked in #1269. Issue #605 must remain open until both halves land, so it does not prematurely satisfy #587's prerequisite.
Implemented by dev-lead agent. Please review.