feat: implement issue #941 — [Fleet Monitor] petry-projects/.github-private — .github/workflows/test-deletion-guard.yml - #942
Conversation
…rivate — .github/workflows/test-deletion-guard.yml
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 48 minutes and 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughFleet Monitor high-failure reporting now uses a shared helper that excludes configured gate workflows from rate-based tracking, keeps gate ChangesFleet Monitor gate-workflow exclusion
Sequence Diagram(s)sequenceDiagram
participant fleet_monitor_sh as "scripts/fleet_monitor.sh"
participant filter_high_failure as "filter_high_failure"
participant jq as "jq"
fleet_monitor_sh->>filter_high_failure: filter_high_failure "$metrics_file"
filter_high_failure->>jq: parse TSV and build tracked JSON
jq-->>filter_high_failure: filtered rows
filter_high_failure-->>fleet_monitor_sh: JSON output
fleet_monitor_sh->>fleet_monitor_sh: write fleet_high_failure.json
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 — waiting on PR blockers (intent: review-changes)PR: #942 |
|
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. |
![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 refactors the high-failure workflow filtering logic by extracting it into a reusable filter_high_failure function in scripts/fleet_report.sh and introducing FLEET_GATE_WORKFLOWS to exclude gate workflows from high-failure tracking. It also adds corresponding documentation in AGENTS.md and comprehensive unit tests in tests/fleet_report.bats. The review feedback suggests validating the positional parameter in filter_high_failure to prevent potential crashes under strict shell options, and adding error handling and cleanup for temporary files created with mktemp in the test helper.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/fleet_report.sh`:
- Around line 14-15: Clarify the behavior of FLEET_GATE_WORKFLOWS in
fleet_report.sh: the current default-expansion makes the env var a full-list
override, so setting it replaces the built-in test-deletion-guard.yml and
holdout-guard.yml entries. Update the nearby comment and/or variable handling in
fleet_report.sh so it explicitly documents that permanent gates must be added to
the default list, or introduce a separate additive env var instead of relying on
FLEET_GATE_WORKFLOWS alone.
In `@tests/fleet_report.bats`:
- Around line 311-312: Update the Bats cases around filter_high_failure so they
assert the helper succeeds before inspecting $output; each run of
filter_high_failure should be followed by a check that $status is 0, then the
existing jq length assertion. Apply this to every affected case in
tests/fleet_report.bats so the tests fail if filter_high_failure returns a
non-zero exit even when it emits JSON-like output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b31b3123-39bb-4e61-8a1a-855e8a6c261c
📒 Files selected for processing (4)
AGENTS.mdscripts/fleet_monitor.shscripts/fleet_report.shtests/fleet_report.bats
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: a52f0ca2d2e090538450605b9b5d464a43ebc78b
Cascade: triage → deep (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5)
Summary
PR #941 extracts the inline high-failure jq filter from fleet_monitor.sh into a tested filter_high_failure() in fleet_report.sh (sourced at fleet_monitor.sh:24) and adds a documented gate-workflow exclusion (FLEET_GATE_WORKFLOWS, default test-deletion-guard.yml + holdout-guard.yml) so intentional guard failures stop creating false-positive Fleet Monitor trackers; ERROR rows for gates still surface. No security surface (CI/monitoring shell only; jq uses --arg so no injection; no auth/secrets/DB). All CI checks green and downstream impact is (none). The triage signals (gemini mktemp error-handling + BATS cleanup robustness) and all four advisory comments are already fixed in the current head: f="${1:-}" with a guard returning 1, mktemp || exit in _mk_metrics, full-list-override comment clarified, and [ status -eq 0 ] added to every new test; CodeRabbit marked its threads addressed and dismissed its changes-requested.
Findings
- MINOR: Test helper temp files leak on assertion failure: _mk_metrics creates a mktemp file but rm -f "$m" runs only after assertions, so a failing @test aborts before cleanup, leaving a file in /tmp. Test-only, error-path-only, negligible in CI; a BATS teardown() or trap-based cleanup would close it. Not blocking. (
tests/fleet_report.bats:289) - INFO: run_secret_scanning MCP tool not available in this environment; relied on the green gitleaks CI check. No credential-like content in the diff (shell filter logic + tests + docs).
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5). Reply if you need a human review.
2 participants
Closes #941
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
New Features
Bug Fixes