Repository navigation
feat: implement issue #1052 — Codify the off-peak scheduling standard and offset this repo's 11 minute-0 crons - #1053
Conversation
… and offset this repo's 11 minute-0 crons
🤖 CodeAnt AI — Review Status
|
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (19)
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 |
codeant-ai
Bot
added
the
size:L
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1053 |
|
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. |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
![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 establishes an organization-wide standard for scheduled workflow timing, ensuring all GitHub Actions cron schedules are offset from the top of the hour to prevent execution delays. It introduces helper utilities in scripts/lib/cron-timing.sh to deterministically calculate offset minutes, a CI check script scripts/check-cron-timing.sh to enforce compliance, and comprehensive BATS tests. The review feedback suggests enhancing the BATS tests by asserting specific exit codes rather than generic non-zero statuses, enabling pipefail in the pipeline test, and aligning the cron parsing logic for better consistency.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 8ddcb744eddf9ba7e2165a11ed73440b90edb011
Review mode: triage-approved (single reviewer)
Summary
Codifies the off-peak scheduled-workflow timing standard in standards/ci-standards.md, offsets all 11 minute-0 crons in this repo (minute field only, frequency untouched), and adds a CI check (scripts/check-cron-timing.sh + scripts/lib/cron-timing.sh) wired into ci.yml's Lint job with bats coverage. Independently verified that every chosen minute matches the deterministic recipe (cksum(basename) % 59 + 1) and that no two workflows share a cron expression afterward. Triage's low-risk assessment is confirmed; risk is MEDIUM only because the change adds new CI-enforcement logic and a new workflow.
Linked issue analysis
Closes #1052. AC #1 met: new "Scheduled Workflow Timing" section cites the .github-private origin (epic #722, story #726) and is written for downstream deferral. AC #2 met: all 11 crons offset, minute field only, both historical collisions (0 */4 and 0 8 * * 1 pairs) de-collided — verified no duplicate expressions remain. AC #3 met: check fails minute-0 crons with a pointer to the standard and a suggested replacement minute; wired into the Lint job. AC #4 met: two bats suites cover the lib and the check; ShellCheck CI is green. AC #5 substantively met: the downstream fan-out exclusion (18 crons across 9 consumer repos, tracked in .github-private#726) is stated explicitly in the standard section and the check script header, though not in the PR description body itself (minor placement nit, non-blocking).
Findings
No blocking findings.
- Verified: all 11 replacement minutes exactly match cron_offset_minute's hash recipe; determinism, range (1–59), and de-collision are test-covered.
- Noted (non-blocking, echoes the open CodeAnt thread): ci-standards.md claims "Distinct filenames hash to distinct minutes" — with 59 buckets this is not guaranteed in general. In practice a collision is caught by the duplicate-cron bats test that runs against the real workflow tree in CI. Suggest softening the wording in a follow-up.
- Noted (non-blocking): detection matches only quoted cron expressions and a bare "0" minute field; an unquoted cron or a list form like "0,30 * * * *" would slip through. Acceptable for this repo's conventions.
- Unresolved review threads are all third-party bot auto-comments (4 low-priority gemini test-precision nitpicks, 1 CodeAnt wording suggestion addressed above); no human reviewer input is pending.
- New cron-timing-tests.yml workflow is hygienic: SHA-pinned checkout, persist-credentials: false, contents: read, timeout set. No Actions security smells; scripts contain no injection risk.
- MCP secret scanning was unavailable in this run; the gitleaks CI check passed (no secrets in the diff — content is crons, shell, docs).
CI status
All substantive checks green at 8ddcb74: CI Lint (including the new cron-timing step), ShellCheck, Agent Security Scan, Secret scan (gitleaks), CodeQL, SonarCloud quality gate (0 new issues), duplicate-decl-gate, and every bats suite including the new Cron-timing Tests. The CANCELLED entries are the Dev-Lead agent's own dispatch/ci-relay orchestration runs superseded by concurrency — not code checks. Dependency-audit jobs skipped for absent ecosystems. mergeStateStatus BLOCKED solely on the pending review this verdict satisfies.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
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
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 8164d7517a9b7c6eae2e195d5649db8620a4ab1e
Review mode: triage-approved (single reviewer)
Summary
Implements issue #1052: codifies the off-peak scheduled-workflow timing standard in standards/ci-standards.md, offsets all 11 minute-0 crons (minute field only, all distinct, frequency preserved), adds a CI enforcement script (scripts/check-cron-timing.sh + pure helper lib) wired into the ci.yml Lint job, and covers it with bats tests plus a dedicated SHA-pinned, least-privilege test workflow. All CI green, all review threads resolved, prior review's concern addressed.
Linked issue analysis
Closes #1052. All five acceptance criteria are substantively addressed: (1) the Scheduled Workflow Timing section is added to standards/ci-standards.md, cites its .github-private origin, and is written for downstream deference; (2) all 11 minute-0 crons are offset changing only the minute field, and the two colliding pairs (0 */4 and 0 8 * * 1) are de-collided — verified by a repo-wide no-duplicate-cron bats test; (3) check-cron-timing.sh fails minute-0 crons with a pointer to the standard and is wired into the existing ci.yml Lint job; (4) bats tests follow repo conventions and ShellCheck CI is green; (5) downstream fan-out is explicitly out of scope, tracked in .github-private#726.
Findings
No blocking findings.
- Non-blocking (docs): header comments in scripts/check-cron-timing.sh / test headers and the ci-standards.md CI-enforcement paragraph claim the check is wired into
.dev-lead/scripts/dev-lead-lint.sh, but that script (hosted in .github-private) currently only runs shellcheck, agent-profile, and CODEOWNERS checks. The real enforcement (ci.yml Lint job) is present and CI-verified; consider correcting or completing the dev-lead-lint claim in a follow-up. - Security posture of the new cron-timing-tests.yml workflow is good: SHA-pinned checkout, persist-credentials: false, permissions: contents: read, timeout, concurrency group.
- cron_offset_minute yields 1..59 (never 0); hash-collision possibility is honestly documented and backstopped by the duplicate-cron test.
- Since the prior (dismissed) approval at 8ddcb74, the single new commit addressed the CodeAnt collision-guarantee review comment; all 6 review threads are resolved.
- Secret-scanning MCP tool not available in this run; the gitleaks CI check passed.
CI status
All required checks green: Lint (incl. new cron-timing step), ShellCheck, bats suites, Validate cron timing, CodeQL, Agent Security Scan, AgentShield, Secret scan (gitleaks), SonarCloud quality gate, dependency-audit. No failures; remaining entries skipped by design.
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: #1053 |
|
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: f66454a696a9b1f7235e413bdc26e3247f62106e
Review mode: triage-approved (single reviewer)
Summary
Confirms the triage-approved assessment. PR #1053 codifies the off-peak scheduled-workflow timing standard in standards/ci-standards.md, offsets all 11 minute-0 crons in this repo (minute field only), de-collides the two colliding cron pairs, and adds a deterministic-offset helper lib, CI enforcement script, and bats coverage. Delta since the prior approved cascade review (8164d75) is only a merge of main; PR content is unchanged. All CI green, all review threads resolved.
Linked issue analysis
Closes #1052. All acceptance criteria are substantively addressed: (1) standards/ci-standards.md gains a canonical Scheduled Workflow Timing section citing its .github-private origin and written for downstream deferral; (2) every scheduled workflow in this repo is offset off minute 0 with no two sharing an identical cron (verified by a dedicated bats test against the real workflow tree, including the 0 8 * * 1 and 0 */4 collisions); (3) scripts/check-cron-timing.sh fails minute-0 crons with a pointer to the standard and a suggested replacement minute, wired into ci.yml's Lint job and unit-tested.
Findings
No blocking findings. Notes: (a) new cron-timing-tests.yml workflow is well-hardened — contents: read only, persist-credentials: false, SHA-pinned checkout, timeout, concurrency group; (b) cron_offset_minute hash collisions are possible by design and explicitly backstopped by the no-duplicate-crons test; (c) all frequency/hour fields are preserved — only minute fields changed, per the standard's own scoping rule; (d) prior bot/AI review threads (codeant-ai, gemini-code-assist) are all resolved; (e) secret-scanning MCP tool unavailable in this run — gitleaks CI check passed (SUCCESS), noted per protocol.
CI status
All checks green at f66454a: Lint, ShellCheck, bats suites, Validate cron timing, CodeQL, Agent Security Scan, Secret scan (gitleaks), SonarCloud, agent-shield, dependency-audit. The CANCELLED dev-lead/dispatch and ci-relay entries are superseded agent-relay runs with a later SUCCESS; no required check failed.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
2 participants
User description
Closes #1052
Implemented by dev-lead agent. Please review.
CodeAnt-AI Description
Offset scheduled workflows and enforce off-peak cron timing
What Changed
Impact
✅ Fewer scheduled workflow delays at :00✅ Smoother repository workload across each hour✅ Immediate feedback for non-compliant workflow schedules💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.