feat: implement issue #725 — [Phase 2] Add scheduled-run reliability analysis to the fleet health daily check - #1217
Conversation
…analysis to the fleet health daily check
|
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: 4 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 (3)
✨ 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 introduces schedule-reliability tracking for GitHub Actions workflows across a fleet, adding cron parsing, expected tick calculation, and reliability classification (STABLE, DEGRADED, STALLED) along with comprehensive tests. The feedback focuses on performance and portability improvements: consolidating YAML parsing and cron tick counting into a single Python invocation to reduce process spawning overhead, replacing here-docs with inline Python commands, piping stdin directly to Python to avoid environment variable size limits, and using the standard -jf flag for BSD/macOS date compatibility.
There was a problem hiding this comment.
Pull request overview
Adds scheduled-run reliability analysis to the Actions Fleet Monitor so scheduled workflows can be flagged when their cron ticks are expected but schedule-event runs are missing, helping detect “green but never ran” stalls across the org fleet.
Changes:
- Extend per-workflow run telemetry to capture
eventand compute schedule-only actual run counts. - Add cron extraction + expected-tick counting + reliability classification + a new “Schedule Reliability” report section.
- Add Bats coverage for cron tick expansion, cron extraction, reliability classification, and report rendering.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/fleet_monitor.sh | Captures run event, computes per-workflow schedule reliability metrics, and appends a new Schedule Reliability section to reports. |
| scripts/fleet_report.sh | Adds cron tick counting, cron extraction, schedule reliability classification, and report rendering helpers. |
| tests/fleet_report.bats | Adds tests covering cron tick expansion, cron extraction, reliability classification, and section rendering/sorting. |
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 — rate-limited (intent: review-changes)PR: #1217 |
|
Note @don-petry I received your request but all AI engines are currently rate-limited. I'll retry automatically once the rate limit clears. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 391b210d3b7e572a62bf10cf737dc39cdd614180
Review mode: triage-approved (single reviewer)
Summary
Adds scheduled-run reliability analysis to the Actions Fleet Monitor (issue #725): captures the run event field, parses schedule.cron from workflow YAML (single python3 -c invocation reading stdin), counts expected ticks over the lookback window with Vixie-cron DOM/DOW semantics, classifies STABLE/DEGRADED/STALLED with a tolerance for best-effort scheduling, and renders a severity-sorted Schedule Reliability section in both the step summary and issue-body report. 224 lines of new bats coverage. Triage assessment confirmed; no issues found that would block merge.
Linked issue analysis
Issue #725 is substantively addressed — all 6 acceptance criteria are met: (1) expected ticks vs. actual schedule-event runs with hit-rate and missed-tick metrics; (2) run event captured in the per-workflow query so schedule runs are isolated from other triggers; (3) cron parsed from workflow file content (API does not expose it), multi-cron entries summed, DOM/DOW Vixie rule implemented and documented; (4) Schedule Reliability section added to both GITHUB_STEP_SUMMARY and the issue-body report, severity-sorted consistent with high-failure tracking; (5) SCHEDULE_TOLERANCE_TICKS absorbs late/boundary ticks and the 1000-result cap plus default-branch caveat are documented in the report footer; (6) bats coverage added to tests/fleet_report.bats (already in the lint bats list) for tick expansion, cron extraction, classification, and rendering.
Findings
No blocking findings.
- Security: no concerns. Fetched workflow content is treated as data only (piped to python3 stdin, never eval'd or interpolated into code); embedded Python is single-quoted with parameters passed via env vars; no workflow-file or permission changes; gitleaks, CodeQL, AgentShield, and the Agent Security Scan are all green. The run_secret_scanning MCP tool was not available in this environment (noted; gitleaks CI check passed).
- Prior bot feedback: verified as addressed at head. The 3 gemini-code-assist threads that remain unresolved on GitHub (consolidate parse+count into one Python invocation; drop here-docs for python3 -c; pipe content via stdin instead of env) are all implemented in the head code — workflow_expected_ticks is a single python3 -c invocation reading stdin, and zero heredocs remain in fleet_report.sh. The threads were simply never marked resolved. All 4 Copilot findings (invalid grep in test, 'local' in a pipeline subshell, PyYAML dependency, stale docstring) are resolved/outdated. CodeRabbit approved at the head SHA.
- Minor (non-blocking): the cron parse/tick-count logic is duplicated between count_cron_ticks (kept for unit-testability) and workflow_expected_ticks — a future refactor could share it. classify_schedule_reliability's expected<=0 branch prints without a trailing newline, but fleet_monitor.sh only calls it when expected_ticks > 0, so the read never hits that path.
CI status
All validation checks green: Lint (shellcheck, bats, template-drift, validate-agent-profiles, validate-personas, gh-aw-compile), CI (ShellCheck, Lint, Secret scan/gitleaks, Agent Security Scan, Compile agentic workflows), Tests (unit-tests), CodeQL (actions + python), SonarCloud (quality gate passed, 0 new issues, 0 hotspots), AgentShield, Holdout Guard, Test-Deletion Guard, CodeRabbit. The two CANCELLED entries (dev-lead / dispatch, dev-lead / ci-relay) are the Dev-Lead Agent's own orchestration jobs superseded by the final merge commit — not code validators. Merge state is BEHIND main (mergeable), which the merge queue/update will handle.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 54e3b0b32f6eb2e692b2305e0151b25d51a64b5b
Review mode: triage-approved (single reviewer)
Summary
Adds a Schedule Reliability section to the Actions Fleet Monitor (issue #725): captures each run's event field, parses schedule.cron from workflow YAML (stdlib-only regex, no PyYAML), counts expected ticks over the lookback window in a single python3 -c invocation, classifies STABLE/DEGRADED/STALLED with a tolerance so late-but-present runs are not scored as misses, and renders the section in both the step summary and issue-body report. 20 new bats tests cover tick expansion, cron extraction, classification, and rendering. All CI green; triage assessment confirmed.
Linked issue analysis
Issue #725 is substantively addressed — all 6 acceptance criteria met: (1) expected ticks vs schedule-event runs with hit-rate and missed count; (2) run .event captured in the per-workflow query, isolating schedule from dispatch/push runs; (3) cron parsed from workflow file content (contents API, raw), multi-cron entries summed, default-branch-only caveat documented in the report note; (4) Schedule Reliability section added to both GITHUB_STEP_SUMMARY and the issue-body report, sorted STALLED > DEGRADED > STABLE consistent with high-failure styling; (5) SCHEDULE_TOLERANCE_TICKS (default 1) absorbs late/boundary ticks and the 1000-results-per-window cap is noted; (6) bats coverage added in tests/fleet_report.bats, which is already in the lint.yml bats allow-list. The existing 12-field metrics TSV contract is untouched — schedule metrics use a separate parallel temp file, per the issue's dev notes.
Findings
No blocking findings.
- Secret scan: run_secret_scanning MCP tool not available in this environment (noted, not a failure); the gitleaks CI check passed and the diff introduces no credentials.
- Security: workflow content fetched read-only via the GitHub API is piped to python3 stdin and parsed with a regex — no eval, no injection path; cron/start/end are passed via env vars, not interpolated into code.
- set -e safety verified: helper error paths print 0 and exit 0; the read call site is guarded by expected_ticks > 0 so classify_schedule_reliability always emits a newline-terminated row; workflow-file fetch failures warn and skip (ERROR-sentinel tolerance pattern).
- Review threads: 3 gemini-code-assist threads show unresolved in the UI, but each has a 'Done' reply from the author and I verified the fixes in the current diff (workflow_expected_ticks is a single python3 -c invocation; here-docs replaced with python3 -c; workflow content piped via stdin). All 4 Copilot findings (invalid grep -c invocation, local-in-pipeline-subshell, PyYAML runtime dependency, stale docstring) are resolved/outdated. No unanswered human-reviewer questions.
- Minor (non-blocking): expected ticks are computed from the workflow file at HEAD over the full window, so a newly added scheduled workflow could briefly over-report missed ticks; tolerance and threshold mitigate, and the report is informational.
CI status
All substantive checks green at 54e3b0b: Lint, ShellCheck, shellcheck, bats, unit-tests, CodeQL (actions + python), Agent Security Scan, agent-shield, Secret scan (gitleaks), SonarCloud (quality gate passed, 0 new issues / 0 hotspots), gh-aw-compile, template-drift, validate-agent-profiles, validate-personas, holdout-guard, Test-Deletion Guard, dependency-audit, CodeRabbit (approved). The dev-lead / dispatch, dev-lead / ci-relay, and prior review / review entries marked failed are cancelled superseded orchestration runs, not code-quality failures; the pending review / review check is this review itself.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Superseded by automated re-review at 54e3b0b.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 54e3b0b32f6eb2e692b2305e0151b25d51a64b5b
Review mode: triage-approved (single reviewer)
Summary
Adds scheduled-run reliability analysis (#725) to the fleet health daily check: a pure-Python 5-field cron tick counter, cron extraction from workflow YAML (regex, no PyYAML dependency), a tolerance-aware STABLE/DEGRADED/STALLED classifier, and a sorted Schedule Reliability report section, with 20 new bats tests covering tick counting, extraction, classification, and rendering. Triage cleared this as low-risk; confirmation review verifies the assessment holds and all reviewer findings are addressed.
Linked issue analysis
Issue #725 acceptance criteria are substantively addressed: (1) expected ticks vs. actual schedule-event runs plus hit-rate and missed-tick metrics are computed per scheduled workflow; (2) the runs query now captures the run event field, and only event==schedule runs are counted; (3) cron expressions are parsed from workflow file content (the API does not expose cron), multi-cron workflows are summed, and the default-branch-only caveat is documented in the report footnote; (4) both the Step Summary and issue-body report gain the section, sorted STALLED > DEGRADED > STABLE consistent with high-failure tracking; (5) SCHEDULE_TOLERANCE_TICKS (default 1) absorbs best-effort delay so a late-but-present run is not scored as a miss.
Findings
No blocking findings.
- Unresolved bot threads verified fixed in code: 3 gemini-code-assist threads remain unresolved in the UI, but each has a substantive reply and the fix is present at the reviewed SHA — workflow_expected_ticks is a single python3 -c invocation (O(1) spawn per workflow), count_cron_ticks passes params via env vars instead of a here-doc, and extract_crons reads stdin directly with no env-var intermediary. All copilot-reviewer threads (grep invocation, local-in-subshell, PyYAML dependency) are resolved and outdated.
- Security: static python3 -c/awk programs with untrusted content passed only via stdin/env — no shell interpolation of workflow content; numeric inputs are sanitized (case-glob guard on expected_ticks, awk numeric coercion). Read-only gh api calls. No new secrets, auth, or workflow-permission changes.
- Correctness spot-checks: half-open window avoids end-boundary double counting; Vixie-cron DOM/DOW either-match rule implemented; DOW 7→0 normalized; hit rate capped at 100%; malformed crons tolerated as 0 (non-fatal), consistent with existing ERROR-sentinel tolerance.
- Minor (non-blocking): count_cron_ticks/extract_crons duplicate logic now embedded in workflow_expected_ticks — retained as test-visible units; regex cron extraction could theoretically match a commented-out cron line, an accepted documented trade-off.
- Secret-scanning MCP tool not available in this environment; gitleaks CI check is green.
CI status
All substantive checks green: shellcheck, ShellCheck, bats, unit-tests, Lint, CodeQL, Analyze (actions/python), Agent Security Scan, agent-shield, Secret scan (gitleaks), SonarCloud quality gate, guard, holdout-guard, template-drift, gh-aw-compile, validate-agent-profiles, validate-personas, Compile agentic workflows. Pending entries are the review/dev-lead agent workflows themselves; cancelled entries are superseded duplicate runs. CodeRabbit approved.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Superseded by automated re-review at 54e3b0b.
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 5b0f0cf3622b3b4b039273a99201f9cb207e116f
Review mode: triage-approved (single reviewer)
Summary
Adds scheduled-run reliability analysis (#725, epic #722) to the daily fleet monitor: captures the run event field, parses schedule.cron from workflow YAML (stdlib-only Python, no PyYAML), expands crons to expected ticks over the lookback window, classifies STALLED/DEGRADED/STABLE with a tolerance, and renders a sorted Schedule Reliability section in both the step summary and issue-body report. 22 new bats tests cover tick expansion, YAML extraction, classification, and rendering.
Linked issue analysis
Closes #725. All six acceptance criteria are substantively met: (1) expected ticks, actual schedule runs, hit rate, and missed count computed per scheduled workflow; (2) runs query now captures .event so schedule runs are isolated; (3) cron parsed from the workflow file (API does not expose it), multi-cron workflows summed, Vixie DOM/DOW either-match rule implemented, default-branch caveat documented; (4) section added to both GITHUB_STEP_SUMMARY and the issue-body report with severity-sorted rows consistent with the high-failure style; (5) SCHEDULE_TOLERANCE_TICKS (default 1) absorbs late/boundary ticks and the 1000-results cap is noted in the report; (6) bats coverage added to tests/fleet_report.bats (already in the lint.yml bats list, so no workflow change needed).
Findings
No blocking findings. All four prior bot-review findings were fixed and confirmed at this SHA: BSD-compatible date -jf flag, grep -cv test fix, removal of local inside the pipeline while-subshell (which would have killed the report under set -e), and elimination of the PyYAML dependency in favor of a stdlib regex parser (the production runner does not install PyYAML). The parallel schedule-metrics TSV preserves the existing 12-field metrics contract used by the high-failure JSON export. Per-workflow file fetch adds one bounded API call per workflow with non-fatal warn-and-skip on failure, consistent with the ERROR-sentinel pattern. expected_ticks is sanitized against non-numeric output before arithmetic. Minor non-blocking note: count_cron_ticks and workflow_expected_ticks duplicate the cron parse/match logic (~80 lines) — acceptable here since each must be a self-contained python3 -c invocation, but a shared fixture could be considered if a third caller appears. The run_secret_scanning MCP tool was unavailable in this environment; the gitleaks CI check passed.
CI status
All required checks green: shellcheck, ShellCheck, Lint, bats, unit-tests, CodeQL (actions + python), gitleaks secret scan, SonarCloud, Agent Security Scan, agent-shield, holdout-guard, template-drift, guard, validate-agent-profiles, validate-personas, gh-aw-compile. Cancelled dev-lead/dispatch and review/review entries are superseded automation runs, not failures. CodeRabbit approved; donpetry-bot approved.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 5b0f0cf3622b3b4b039273a99201f9cb207e116f
Review mode: triage-approved (single reviewer)
Summary
Adds a Schedule Reliability section to the daily fleet health check: per-workflow expected cron ticks (parsed from workflow YAML via stdlib-only regex, expanded by a pure-Python 5-field cron counter) vs. actual schedule-triggered runs, classified as STABLE / DEGRADED / STALLED with a tolerance for late ticks. 3 files, +578/−3, with 19 new bats tests covering tick expansion, classification, and report rendering.
Linked issue analysis
Closes #725 — all six acceptance criteria are substantively addressed: (1) expected-vs-actual + hit-rate/missed metrics computed per scheduled workflow; (2) the run event field is captured and schedule-triggered runs isolated; (3) crons are parsed from the workflow file (contents API, since the workflows API does not expose cron), multi-cron workflows are summed, and Vixie-cron DOM/DOW OR-semantics are implemented and documented; (4) the report section is rendered in both GITHUB_STEP_SUMMARY and the issue-body report, sorted STALLED-first consistent with high-failure tracking; (5) SCHEDULE_TOLERANCE_TICKS absorbs late/boundary ticks and the 1000-result cap + default-branch caveats are documented in the report footer; (6) bats coverage added to tests/fleet_report.bats (already wired into CI — the bats check ran and passed).
Findings
No blocking findings.
- All 8 prior review threads (gemini-code-assist, copilot-pull-request-reviewer) are resolved, and the fixes are verified in the current diff: single-invocation
workflow_expected_ticks(no O(N) python spawns),python3 -cwith env-var parameters instead of here-docs, stdin piping instead of a WF_CONTENT env intermediary, PyYAML dependency removed (stdlib regex),local iconremoved from the pipeline-subshell loop, the invalidgrep -c cron -vtest assertion fixed, and BSDdate -jfportability. - Verified
wf_filecarries the full.pathfrom the workflows API, so therepos/{repo}/contents/{wf_file}fetch is correct; read failures are non-fatal (warn + skip). - Verified
label_to_iconalready maps DEGRADED (🟠); this PR adds STALLED (🔴) and STABLE (✅) — no unmapped labels. - Delta since the previously reviewed SHA (54e3b0b) is only merges from
main(#1197, #1204); no changes to this PR's files. - Security: read-only
gh apicalls; untrusted YAML content is passed to Python via stdin (never interpolated into code); numeric inputs are sanitized (case … *[!0-9]*) before arithmetic. No secrets in the diff. Therun_secret_scanningMCP tool is not available in this environment; the gitleaks CI check passed. - Minor (non-blocking): one extra contents-API call per workflow per daily run — acceptable at current fleet size, with a graceful skip on failure. The minute-stepping cron counter is O(window-minutes) per cron (~10k iterations for 7 days) — negligible in-process.
CI status
All required checks green at 5b0f0cf: shellcheck/ShellCheck, bats, unit-tests, Lint, CodeQL (actions + python), SonarCloud quality gate, gitleaks secret scan, agent-shield, holdout-guard, template-drift, gh-aw-compile, validate-agent-profiles/personas — SUCCESS. CANCELLED entries are superseded dev-lead dispatch/ci-relay and prior review-workflow runs; the pending review / review check is this review itself. Dependency-audit jobs SKIPPED (no matching ecosystems changed).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Closes #725
Implemented by dev-lead agent. Please review.