Create cleanup_cron_job.yml - #376
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA weekly GitHub Actions workflow is added to list completed runs, skip recent or open-PR-related runs, and delete eligible successful or cancelled runs. ChangesCleanup Cron Job
Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions
participant gh as gh CLI
participant GitHubAPI as GitHub API
participant PullRequests as Pull Requests
GitHubActions->>gh: authenticate and compute cutoff
gh->>GitHubAPI: list completed workflow runs
GitHubAPI-->>gh: paginated run results
gh->>PullRequests: fetch state for linked PRs
PullRequests-->>gh: open/closed state
gh->>GitHubAPI: delete eligible workflow runs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.github/workflows/cleanup_cron_job.yml:
- Around line 1-10: The Cleanup Successful Workflow Runs workflow is missing an
explicit least-privilege permissions declaration. Add a permissions block at the
workflow or cleanup job level for the cleanup job in the cleanup_cron_job
workflow, setting actions: write and contents: read so the job can delete
workflow runs without inheriting broader default token access.
- Around line 7-10: The cleanup cron workflow currently allows overlapping runs,
which can cause concurrent jobs to race on shared run IDs. Add a top-level
concurrency block to the workflow used by the cleanup job, using github.workflow
as the group and keeping cancel-in-progress disabled, so only one scheduled
cleanup execution runs at a time.
- Around line 7-10: The cleanup job definition lacks a descriptive display name,
so the Actions UI only shows the job key. Update the job configuration for the
cleanup job by adding a name on the same job block that contains runs-on and
steps, using a clear label like Cleanup successful runs, so the job is easier to
identify in workflow runs.
- Around line 18-22: The cleanup job currently limits gh run list to 500
results, which can miss older successful runs in repositories with high
activity. Update the RUNS collection logic in the cleanup workflow to either
paginate through all pages or raise the query to iterate beyond the current cap,
using the existing gh run list command and its JSON/filter pipeline. Keep the
rest of the cleanup flow intact, but ensure the run selection logic can see
every successful run before deletion.
- Around line 54-58: The cleanup loop in the workflow can hit GitHub API
secondary rate limits when deleting many runs back-to-back. Update the deletion
logic around the run-processing loop that calls gh run delete to add throttling
or batching, such as a small sleep between deletions or a retry/backoff
strategy, so the cleanup stays within API limits while still processing all
eligible runs.
- Around line 18-23: The shell block in the cleanup_cron_job workflow is missing
strict error handling, so failures in the gh run list/delete flow can be ignored
and leave RUNS empty. Add set -euo pipefail immediately after the run: | header
for this script so the job exits on command failures, unset variables, and
pipeline errors, keeping the existing RUNS assignment and loop behavior under
fail-fast semantics.
- Around line 30-52: The PR-state check in the cleanup cron job is failing open
because `gh pr list`/`gh pr view` errors are being swallowed, which can delete
runs when the PR state is unknown. Update the guard in the cleanup script so
that the `PR_NUMBER`/`STATE` lookup treats any command failure or empty/unknown
result as “open” and sets `DELETE_RUN=false` in that case. Use the existing `gh
pr list`, `gh pr view`, and `STATE` normalization logic to locate the fix, and
make the comparison fail closed whenever the PR status cannot be determined.
🪄 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 Plus
Run ID: 33dc3864-cf4d-4d4e-ac06-5e9489cb2c6b
📒 Files selected for processing (1)
.github/workflows/cleanup_cron_job.yml
mmusgrov
left a comment
mmusgrov
left a comment
There was a problem hiding this comment.
The coderabbitai review comments look like they add value.
Definitely! I'm addressing all of them as well |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/cleanup_cron_job.yml:
- Around line 33-41: The cleanup workflow is using an unsupported gh run list
--page option and it also stops based on the filtered RUNS payload instead of
the raw page contents. Update the loop in cleanup_cron_job.yml around the gh run
list call to remove --page and use the available pagination mechanism, then
change the break condition to inspect the unfiltered page response so pagination
continues until all pages are scanned. Reference the RUNS assignment and the jq
length check in the cleanup loop when making the fix.
- Around line 7-9: The workflow permissions block is missing pull request read
access, which causes the PR-state guard in the cleanup job to skip PR-associated
runs. Update the permissions in the cleanup cron job workflow to include
pull-requests: read alongside the existing actions: write and contents: read so
the gh pr list and gh pr view checks in the cleanup logic can work correctly.
- Around line 57-78: The PR lookup in the cleanup workflow is using
headBranch/gh pr list, which can resolve the wrong PR when branch names are
reused or forked. Update the cleanup logic to read the workflow run’s
pull_requests association and use those PR numbers directly, then fetch each PR
state with gh pr view; keep the existing branch/run loop but replace the
ambiguous PR_NUMBER resolution in the cleanup job.
🪄 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 Plus
Run ID: 80b95bb0-e576-4096-9d8f-f8abcea6a45f
📒 Files selected for processing (1)
.github/workflows/cleanup_cron_job.yml
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/cleanup_cron_job.yml (1)
65-65: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not swallow delete failures.
Line 65 makes the step succeed even when
gh run deletefails for auth, rate-limit, or API errors, bypassing the strict mode added above. Let it fail, or add bounded retries that still fail the job after exhaustion.Proposed fix
- gh run delete "$RUN_ID" --repo "$REPO" || echo "Failed to delete run $RUN_ID" >&2 + gh run delete "$RUN_ID" --repo "$REPO"🤖 Prompt for 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. In @.github/workflows/cleanup_cron_job.yml at line 65, The cleanup step currently masks failures in the gh run delete call by falling back to a successful echo, which bypasses the workflow’s strict error handling. Update the cleanup logic in the gh run delete path to fail the job when deletion fails, or wrap it in bounded retries that still exit non-zero after exhaustion; keep the behavior aligned with the surrounding shell strict mode and the deletion loop that processes RUN_ID values.
🤖 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 @.github/workflows/cleanup_cron_job.yml:
- Around line 32-34: The cleanup workflow query in the gh api pipeline is
relying on the server-side status=completed filter, which can miss older runs in
large repos. Update the actions/runs lookup in the cleanup_cron_job workflow to
remove that query parameter and instead filter for .status == "completed"
locally in the jq chain, keeping the existing success/cancelled and cutoff-date
logic intact.
- Around line 40-43: The cleanup job is gating run handling only on the EVENT
being pull_request, which misses other PR-associated runs even when .prs is
present. Update the logic around the PR_NUMBERS lookup to use the existing .prs
association as the primary check, and only fall back to the event name for
fail-closed behavior when no PR number is associated. Keep the fix localized to
the event/PR handling block in the workflow so it covers pull_request_target and
similar PR-linked runs.
---
Duplicate comments:
In @.github/workflows/cleanup_cron_job.yml:
- Line 65: The cleanup step currently masks failures in the gh run delete call
by falling back to a successful echo, which bypasses the workflow’s strict error
handling. Update the cleanup logic in the gh run delete path to fail the job
when deletion fails, or wrap it in bounded retries that still exit non-zero
after exhaustion; keep the behavior aligned with the surrounding shell strict
mode and the deletion loop that processes RUN_ID values.
🪄 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 Plus
Run ID: 475af09f-cf70-439b-94bb-3c569cab9743
📒 Files selected for processing (1)
.github/workflows/cleanup_cron_job.yml
| if [[ "$EVENT" == "pull_request" ]]; then | ||
| PR_NUMBERS=$(echo "$run" | jq -r '.prs[]') | ||
|
|
||
| for PR_NUMBER in $PR_NUMBERS; do |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow around the referenced lines.
file=".github/workflows/cleanup_cron_job.yml"
wc -l "$file"
sed -n '1,220p' "$file"
# Find where EVENT and prs are populated/used in this workflow.
rg -n '\bEVENT\b|\.prs\b|pull_request|pull_request_target|gh pr view|DELETE_RUN' "$file"Repository: jbosstm/lra
Length of output: 3116
🌐 Web query:
GitHub Actions workflow runs API pull_requests field pull_request_target event docs
💡 Result:
In the GitHub Actions REST API, the workflow_runs object includes a pull_requests field, which is defined as an array of minimal pull request objects associated with that specific workflow run [1][2]. However, technical documentation and community reports indicate that this field can be unreliable or empty when a workflow is triggered by the pull_request_target event [3]. While the API schema formally requires the pull_requests array [1][2], users have reported that for runs triggered by pull_request_target, the field often returns an empty array even when the run is clearly associated with an open pull request in the GitHub UI [3]. Regarding the pull_request_target event specifically: 1. Nature of the Event: The pull_request_target event is designed to run workflows in the context of the base repository's default branch, rather than the pull request's head branch [4]. This allows the workflow to securely access repository secrets and write permissions because it executes trusted code from the base branch, even when triggered by a pull request from a fork [4][5]. 2. Context Access: Within the workflow environment, you can access information about the triggering pull request using the github context (e.g., ${{ github.event.pull_request.number }}) [6]. Because the github context mirrors the webhook payload for the event, it is the primary and recommended method for accessing pull request details during the workflow execution, rather than relying on the workflow_runs REST API endpoint [6][7]. 3. Security Warning: Because pull_request_target runs code from the base branch, it is considered safe for secrets. However, if you attempt to check out the pull request's head code (e.g., using actions/checkout with a PR ref), you may expose the workflow to malicious code from a fork [4]. GitHub has implemented protections in newer versions of actions/checkout to prevent unsafe checkouts by default when using this event [4]. If you require the pull request details via the API for external tooling, it is generally more reliable to use the Pulls API (e.g., GET /repos/{owner}/{repo}/pulls/{pull_number}) using the pull request number obtained from the workflow's event payload, rather than relying on the pull_requests field within the workflow_runs API response [5].
Citations:
- 1: https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28
- 2: https://docs.github.com/rest/actions/workflow-runs
- 3: [Schema Inaccuracy] Pull request field missing from GitHub Actions REST API github/rest-api-description#5070
- 4: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
- 5: https://dev.to/suzukishunsuke/secure-github-actions-by-pullrequesttarget-641
- 6: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 7: https://stackoverflow.com/questions/76465553/how-can-i-use-the-github-rest-api-to-get-to-the-detail-of-the-event-that-trigger
Use PR association instead of gating on pull_request only. .prs is already available here, but if [[ "$EVENT" == "pull_request" ]] skips other PR-associated runs such as pull_request_target, so an open PR can still lose its run entry. Check any non-empty .prs, and only fall back to the event name to fail closed when a PR-like run has no associated PR number.
🤖 Prompt for 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.
In @.github/workflows/cleanup_cron_job.yml around lines 40 - 43, The cleanup job
is gating run handling only on the EVENT being pull_request, which misses other
PR-associated runs even when .prs is present. Update the logic around the
PR_NUMBERS lookup to use the existing .prs association as the primary check, and
only fall back to the event name for fail-closed behavior when no PR number is
associated. Keep the fix localized to the event/PR handling block in the
workflow so it covers pull_request_target and similar PR-linked runs.
3 participants
NO_TESTS