feat: implement issue #360 — [Fleet Monitor] petry-projects/ContentTwin — .github/workflows/ci.yml - #362
Conversation
…in — .github/workflows/ci.yml
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe CI workflow adds job timeouts and retry loops for external tool installation. A new Bats suite parses the workflow and verifies triggers, permissions, concurrency, timeout bounds, and retry behavior. ChangesCI workflow hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 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. |
![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 a new BATS test suite (scripts/tests/ci-workflow.bats) to validate the GitHub Actions CI workflow configuration, ensuring triggers, permissions, concurrency, timeouts, and retry loops are correctly defined. The reviewer's feedback focuses on hardening the inline Python scripts used in these tests. Specifically, they recommend using safe navigation and fallbacks (such as yaml.safe_load(...) or {}) and avoiding chained .get() calls to prevent potential TypeError or KeyError exceptions when parsing YAML elements that might be missing or null.
There was a problem hiding this comment.
Pull request overview
This PR addresses Fleet Monitor issue #360 by hardening the repository’s CI workflow to reduce transient failures and prevent long-running hangs, helping keep .github/workflows/ci.yml reliability within the monitored failure-rate threshold.
Changes:
- Added per-job
timeout-minutesvalues across CI jobs to prevent 6-hour default timeouts from inflating failure metrics. - Wrapped shfmt download (curl) and bats installation (apt-get) in bounded retry loops to mitigate transient network/mirror/CDN failures.
- Added a Bats test suite that validates the CI workflow’s YAML integrity and asserts the presence of the new resiliency controls (timeouts + retries).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scripts/tests/ci-workflow.bats |
Adds regression tests that assert CI workflow structure and the new timeout/retry resilience requirements. |
.github/workflows/ci.yml |
Implements timeouts and bounded retries to reduce CI flakiness and prevent hung jobs. |
CI Failure: SonarCloud Code AnalysisStep: Quality Gate check — SonarCloud flagged Suggested fix: Add |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CI Failure: SonarCloud Code AnalysisStep: Quality Gate — Security Rating on New Code The Quality Gate failed because new code got a Security Rating of C (required A). SonarCloud flagged Suggested fix: Add |
Use Python context managers (with statements) for all file operations in the BATS tests to properly close file handles and address SonarCloud's C Security Rating failure (unclosed file handle vulnerability). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Fixed in scripts/tests/ci-workflow.bats: replaced unclosed file handles with proper Python context managers (with statements) to address the C Security Rating failure. |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
CI Failure: SonarCloud Code AnalysisStep: Quality Gate analysis No GitHub Actions run backs this check (it is the external SonarCloud Quality Gate), so this is inferred from the diff. The PR only touches .github/workflows/ci.yml and adds a new scripts/tests/ci-workflow.bats file. The two new retry loops (shfmt curl download, bats apt-get install) share near-identical "for attempt in 1 2 3; do ... sleep ...; done" structure, and the new bats file repeats the same python3 -c "import yaml; ..." snippet across roughly 7 test cases -- a classic pattern SonarCloud flags as code duplication, which can fail the "Duplicated Lines on New Code" quality gate condition. It's also possible the gate failed on "Coverage on New Code," since the new bats tests and workflow YAML aren't measured by a coverage tool SonarCloud recognizes. Suggested fix: Extract the shared retry logic into a single reusable script (e.g. scripts/retry.sh attempts delay -- cmd...) and factor the repeated python3 -c "import yaml..." snippet in the bats tests into one helper function, then re-run the analysis to confirm the duplication metric drops below threshold. |
Dev-Lead — review-changes (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: LOW
Reviewed commit: feb41ee48c525858e0ec3d59220ff4260e786a99
Review mode: triage-approved (single reviewer)
Summary
CI-hardening PR for Fleet Monitor issue #360: adds per-job timeout-minutes (all within org cap 1-59), bounded 3-attempt retry loops for the shfmt curl download and bats apt-get install, curl --proto '=https' hardening, and a bats regression suite guarding all of it. All CI green, all 7 review threads resolved, triage low-risk assessment confirmed.
Linked issue analysis
Issue #360 flagged ci.yml at an 11.1% failure rate (2/18 runs) over the monitored window. The PR substantively addresses both identified transient-failure modes: (1) unretried network installs (shfmt via curl, bats via apt-get) now retry 3x with linear backoff, and (2) jobs without timeout-minutes that could run to GitHub's 6h default and count as timed_out failures now declare bounded timeouts (15/15/20/30 min). The new bats suite prevents regression of both fixes and of existing trigger/permission/concurrency config.
Findings
No blocking findings.
- Security posture preserved: job-level permissions unchanged (contents: read), actions/checkout remains SHA-pinned, no new actions or dependencies introduced.
- The SonarCloud finding (HTTPS not enforced on redirects) was fixed with curl --proto '=https' and is guarded by two new test assertions; the thread is resolved.
- All 6 gemini-code-assist suggestions (safe YAML navigation in the inline Python test helpers) were applied and their threads resolved.
- Retry loop logic is correct: bounded at 3 attempts, backs off (5s/10s/15s), exits non-zero after exhaustion so genuine outages still fail the job.
- Non-blocking note: the shfmt binary is downloaded without checksum verification. This is pre-existing behavior (version-pinned URL) and the PR improves it with --proto '=https'; consider adding a sha256 check in a follow-up.
- Secret-scanning MCP tool unavailable in this environment; the gitleaks CI check passed on this head SHA.
- Branch is BEHIND main but MERGEABLE; no conflict.
CI status
All required checks green on feb41ee: Lint, Format, Test, Secret scan (gitleaks), CodeQL, Analyze (actions), SonarCloud, agent-shield, dependency-audit (applicable ecosystems skipped as expected).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Closes #360
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
CI Improvements
Tests