feat: implement issue #1150 — [Phase 2] Cross-repo remediation PR driver (DRY_RUN-gated, idempotent) - #1270
Conversation
…ver (DRY_RUN-gated, idempotent)
|
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: 30 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 (2)
✨ 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 |
![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 the Phase 2 network driver in scripts/fleet_stub_remediate.sh to automate the remediation of drifted workflow stubs by creating branches, updating files, and opening pull requests. It also adds a comprehensive suite of BATS tests. The review feedback focuses on enhancing the robustness of the Bash script and tests under set -e by safely handling command substitutions and jq parsing, utilizing $BATS_TEST_TMPDIR for temporary test files, and asserting exact exit codes for negative grep checks.
There was a problem hiding this comment.
Pull request overview
Implements the Phase 2 cross-repo remediation “PR driver” for fleet workflow stubs, extending the existing pure remediation-plan builder with a DRY_RUN-gated network main that can open one grouped PR per consumer repo and verify byte-identity post-write.
Changes:
- Add a Phase-2 network remediation driver to
scripts/fleet_stub_remediate.sh(branch creation, contents PUT, SHA verify, one-PR-per-repo, scope fail-closed gating). - Add Bats coverage for driver behaviors (default DRY_RUN,
--dry-run, idempotent skip, grouping, SHA mismatch, and live-scope guard).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/fleet_stub_remediate.sh | Adds the Phase-2 network driver (DRY_RUN + repo-scope gating, branch/PUT/PR flow, byte-identity verification). |
| tests/fleet_stub_remediate.bats | Adds a gh shim harness + tests that exercise the new network driver behaviors without network access. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #1270 |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: d7a52b0622b3fe550d76d85b59347d94697147a2
Review mode: triage-approved (single reviewer)
Summary
Implements the Phase-2 cross-repo remediation PR driver (#1150) in scripts/fleet_stub_remediate.sh with bats coverage. Although the raw diff spans 9 files (+1211), 7 of them are byte-identical to current main (stale merge-base — the branch is BEHIND); the effective delta is only scripts/fleet_stub_remediate.sh and tests/fleet_stub_remediate.bats. The driver is DRY_RUN-by-default, fail-closed (live writes refused without an explicit repo scope), idempotent, opens exactly one proposal PR per repo against the consumer default branch, and verifies post-PUT blob-SHA byte-identity before opening any PR. All 9 prior bot-review threads are resolved with matching fixes in the review-changes commits.
Linked issue analysis
Linked issue #1150 ([Phase 2] Cross-repo remediation PR driver) — all 7 acceptance criteria are substantively addressed:
- Per-repo branch + contents-PUT + one PR —
_remediate_repogroups plan entries by repo, creates one branch, PUTs each drifted stub, opens one PR (verified by the "two stubs → ONE branch, two PUTs, ONE PR" test). - DRY_RUN-aware, default true —
DRY_RUN="${DRY_RUN:-true}"; dry-run logs intended mutations and the test asserts zero write API calls. - Idempotent —
gh pr list --headskip plus branch-reuse fallback on the refs POST, with a dedicated test. - Proposal PR, never direct push — PR opened against the consumer default branch with a deterministic title/body citing the canonical source (tested).
- Post-PUT byte-identity verify — written blob SHA compared to the canonical SHA; mismatch fails the repo before any PR is opened (tested via WRITTEN_SHA override).
- bats + shellcheck — a gh shim on PATH logs the write-call sequence;
batsandshellcheckCI checks are green. - Fail-closed blast-radius bound — DRY_RUN=false with no repo scope emits
::warning::and forces dry-run (tested).
Findings
No blocking findings. Notes for the maintainer (non-blocking):
- Stale merge-base / branch BEHIND:
.github/workflows/spec-drift.yml,scripts/caller_stub_freeze.sh, and 5 test/fixture files appear in the diff but are byte-identical to current main (blob SHAs verified per file). They merge as no-ops; updating the branch would shrink the diff to the true 2-file delta. - Cross-day idempotency nuance: the remediation branch is date-scoped (
fleet-stub-remediate/YYYY-MM-DD), so thegh pr list --headcheck only suppresses re-runs on the same day; an open remediation PR from a prior day would not prevent a new PR the next day. The one-PR-per-repo-per-run bound (AC #1) still holds, and the Phase-3 pilot scope / REMEDIATE_MAX_REPOS ceiling will further bound this. - Prior review threads all resolved (gemini-code-assist ×8, copilot ×1):
set -ecrash guards (|| trueon the_canonical_bytespipeline), jq optional chaining for null-safe.sha/.numberreads, BATS_TEST_TMPDIR-scoped mktemp, exact grep exit-status assertions, and the trailing-newline-preserving temp-file read (IFS= read -r -d '') that protects the blob-SHA byte-identity check for YAML files. - Secret scan: the
run_secret_scanningMCP tool was not available in this run; the gitleaks CI check passed and no credential-like content appears in the delta.
CI status
All checks green at the reviewed commit: shellcheck, bats, unit, CodeQL (actions + python), gitleaks secret scan, agent-shield, SonarCloud, caller-stub-freeze, spec-drift-workflow, and all validate-* guards SUCCESS. The CANCELLED dev-lead / dispatch / ci-relay entries are superseded agent-automation runs (latest dispatch SUCCESS; relay commits are [skip ci-relay]). Merge state is BEHIND main — an update/rebase may be required by branch protection before merge.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1270 |
|
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. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 6a22d5fddcaf78fbdd36450a5543575f8f315b09
Review mode: triage-approved (single reviewer)
Summary
Adds the Phase-2 cross-repo remediation network driver to scripts/fleet_stub_remediate.sh (+342/-11, 2 files). The driver consumes the Phase-1 plan, and per consumer repo creates one branch, PUTs canonical stub bytes, verifies blob-SHA byte-identity, and opens exactly one proposal PR. DRY_RUN defaults to true, and live mode fail-closes to dry-run when no explicit repo scope is supplied. 8 new bats tests exercise the driver via a deterministic gh shim.
Linked issue analysis
Closes #1150. All 7 acceptance criteria are substantively met: (1) branch + contents-PUT + one PR per repo via run_remediation/_remediate_repo; (2) DRY_RUN=true default with zero write calls (test-asserted); (3) idempotent skip via gh pr list --head and branch-reuse fallback; (4) PR opened against the consumer default branch citing the canonical source; (5) post-PUT blob SHA verified against the canonical SHA, failing the repo before opening a misleading PR; (6) bats tests with a gh shim plus green shellcheck; (7) fail-closed blast-radius bound — DRY_RUN=false with no REMEDIATE_REPOS scope downgrades to dry-run with a ::warning::.
Findings
No blocking findings.
- The run_secret_scanning MCP tool was not available in this environment; the gitleaks CI check passed and no credential-like content appears in the diff.
- Minor (non-blocking): the remediation branch name is date-stamped (fleet-stub-remediate/YYYY-MM-DD), so the idempotency check (gh pr list --head) will not detect an open remediation PR from a previous day's branch — a cross-day duplicate PR is theoretically possible. Worth considering in Phase 3.
- Minor (non-blocking): gh pr create stderr is suppressed (2>/dev/null), losing error detail on failure; the ::error:: fallback still fails the repo correctly.
- All 9 prior review threads (gemini-code-assist, copilot-pull-request-reviewer) are resolved with matching fixes in the diff: set -e crash guards, jq null-safe filters, trailing-newline preservation via temp file + read -d '', BATS_TEST_TMPDIR-scoped mktemp, and exact grep exit-status assertions.
CI status
All required checks green: shellcheck, bats, unit-tests, Lint, CodeQL (actions + python), agent-shield, Agent Security Scan, gitleaks secret scan, SonarCloud, actionlint, guard, holdout-guard, caller-stub-freeze, template-drift, validate-* checks. A few CANCELLED/SKIPPED entries are superseded duplicate runs with matching SUCCESS runs; no failures. CodeRabbit and donpetry-bot both approved.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Closes #1150
Implemented by dev-lead agent. Please review.