Add Cache Reaper workflow to manage stale and orphaned caches#100
Add Cache Reaper workflow to manage stale and orphaned caches#100kpj2006 wants to merge 8 commits intoAOSSIE-Org:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new GitHub Actions workflow that weekly (or manually) enumerates Actions caches, identifies stale, zero-byte, or closed-PR caches, logs candidates with reasons, and optionally deletes them while reporting counts and freed MB. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cache-reaper.yml:
- Around line 13-15: Add an explicit type for the stale_days input in the GitHub
Actions workflow: update the input named stale_days (under inputs: stale_days)
to include "type: string" alongside the existing description and default so the
intent matches dry_run's style and clarifies that the value is a string.
- Around line 58-65: Wrap the call to github.rest.actions.deleteActionsCacheById
in a try/catch so a single deletion failure won't abort the whole loop: inside
the loop around deleteActionsCacheById (referencing DRY_RUN, cache.id, deleted,
freed) try the delete and only increment deleted and add to freed on success; on
catch log/record the error along with cache.id (and optionally track a failures
counter) and continue to the next cache so processing resumes for remaining
entries.
- Around line 51-55: The current closedPR detection only tests the ref string
(closedPR) and can match open PRs; instead extract the PR number from cache.ref
(matching refs/pull/(\d+)/merge), call the GitHub API (e.g.,
octokit.rest.pulls.get or github.pulls.get) to fetch that PR and check its
state, and set closedPR = true only if the returned PR.state is 'closed' or if
merged_at is non-null; update the logic that computes reason to rely on this
API-backed closedPR value and handle missing/failed API calls gracefully (treat
as not-closed or log and skip deletion).
- Around line 36-45: Wrap the loop that calls
github.rest.actions.getActionsCacheList in a try/catch to handle API failures:
catch errors from getActionsCacheList (including rate limits/network issues) and
log a clear message including the error and relevant context (owner/repo/page)
via the workflow runner (or rethrow a more descriptive error), and ensure the
catch can break or retry as appropriate; update references to
getActionsCacheList, data.actions_caches, page and all inside the try block so
errors are caught and handled consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62311a42-9e74-4242-a0fb-72aae7e1edb8
📒 Files selected for processing (1)
.github/workflows/cache-reaper.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/cache-reaper.yml (1)
49-56:⚠️ Potential issue | 🔴 Critical
closedPRcurrently matches open PR caches too.
cache.ref?.match(/refs\/pull\/\d+\/merge/)only proves the cache belongs to a PR ref. On Line 54, that makes open PR caches deletion candidates as well, which can break active CI runs. Query the PR state and only treat closed or merged PRs as orphaned.Proposed fix
- const closedPR = cache.ref?.match(/refs\/pull\/\d+\/merge/); + let closedPR = false; + const prMatch = cache.ref?.match(/refs\/pull\/(\d+)\/merge/); + if (prMatch) { + try { + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: Number(prMatch[1]), + }); + closedPR = pr.state === 'closed' || pr.merged_at !== null; + } catch (error) { + console.warn(`Skipping PR-backed cache ${cache.key}: ${error.message}`); + } + } if (!stale && !zeroByte && !closedPR) continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cache-reaper.yml around lines 49 - 56, The current closedPR check (closedPR = cache.ref?.match(/refs\/pull\/\d+\/merge/)) only detects PR refs and can mark active/open PR caches for deletion; update the logic so you extract the PR number from cache.ref (from the refs/pull/<num>/merge pattern) and call the GitHub API to fetch that pull request's state, then set closedPR to true only when the PR is closed or merged (e.g., pr.state === 'closed' or pr.merged === true); update the deletion reason computation that uses closedPR so only truly closed/merged PRs are treated as orphaned while leaving open PR caches intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cache-reaper.yml:
- Around line 30-33: The STALE_DAYS parsing is unsafe: validate the raw input
string before using parseInt and computing cutoff to avoid 0, negative, or
malformed values; read the raw input (the same template '${{
github.event.inputs.stale_days }}'), ensure it matches a positive-integer
pattern (e.g. /^\d+$/) and that parsed value is >= 1, and if validation fails
fail-fast (throw/exit the workflow with a clear error); only then assign
STALE_DAYS via parseInt and compute cutoff (cutoff.setDate(cutoff.getDate() -
STALE_DAYS)); reference symbols: STALE_DAYS, DRY_RUN, cutoff, parseInt.
---
Duplicate comments:
In @.github/workflows/cache-reaper.yml:
- Around line 49-56: The current closedPR check (closedPR =
cache.ref?.match(/refs\/pull\/\d+\/merge/)) only detects PR refs and can mark
active/open PR caches for deletion; update the logic so you extract the PR
number from cache.ref (from the refs/pull/<num>/merge pattern) and call the
GitHub API to fetch that pull request's state, then set closedPR to true only
when the PR is closed or merged (e.g., pr.state === 'closed' or pr.merged ===
true); update the deletion reason computation that uses closedPR so only truly
closed/merged PRs are treated as orphaned while leaving open PR caches intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30b8724a-8775-49d2-9310-2aef2215f5e2
📒 Files selected for processing (1)
.github/workflows/cache-reaper.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/cache-reaper.yml (1)
63-65:⚠️ Potential issue | 🔴 CriticalDon't treat every PR cache as orphaned.
Line 63 only checks whether the ref looks like
refs/pull/<n>/merge; it never verifies that the PR is actually closed. As written, the scheduled run can delete caches for active PRs and disrupt ongoing CI. Resolve the PR state before marking the cache deletable, or drop this criterion entirely.Suggested fix
for (const cache of all) { const stale = new Date(cache.last_accessed_at) < cutoff; const zeroByte = (cache.size_in_bytes || 0) === 0; - const closedPR = cache.ref?.match(/refs\/pull\/\d+\/merge/); + let closedPR = false; + const prMatch = cache.ref?.match(/^refs\/pull\/(\d+)\/merge$/); + if (prMatch) { + try { + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: Number(prMatch[1]), + }); + closedPR = pr.state === 'closed'; + } catch (err) { + console.log(`⚠️ Failed to resolve PR ${prMatch[1]} for cache ${cache.key}: ${err.message}`); + } + } if (!stale && !zeroByte && !closedPR) continue;#!/bin/bash # Verify that PR-originated caches are matched by ref only, with no PR-state lookup. sed -n '60,68p' .github/workflows/cache-reaper.yml printf '\n---\n' rg -n 'refs\\/pull|github\\.rest\\.pulls\\.get|octokit\\.rest\\.pulls\\.get' .github/workflows/cache-reaper.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cache-reaper.yml around lines 63 - 65, The current check using closedPR = cache.ref?.match(/refs\/pull\/\d+\/merge/) only tests ref pattern and can mark active PR caches as deletable; update the logic so that when cache.ref matches refs/pull/<n>/merge you extract the pull number and call the GitHub API (e.g., octokit.rest.pulls.get with owner, repo, and pull_number) to verify the PR's state === 'closed' before treating the cache as orphaned, or alternatively remove the ref-only criterion entirely; adjust the conditional that uses stale, zeroByte, and closedPR to rely on the verified closed state (or the removed criterion) so active PR caches are not deleted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/cache-reaper.yml:
- Around line 63-65: The current check using closedPR =
cache.ref?.match(/refs\/pull\/\d+\/merge/) only tests ref pattern and can mark
active PR caches as deletable; update the logic so that when cache.ref matches
refs/pull/<n>/merge you extract the pull number and call the GitHub API (e.g.,
octokit.rest.pulls.get with owner, repo, and pull_number) to verify the PR's
state === 'closed' before treating the cache as orphaned, or alternatively
remove the ref-only criterion entirely; adjust the conditional that uses stale,
zeroByte, and closedPR to rely on the verified closed state (or the removed
criterion) so active PR caches are not deleted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f51821d-aeb5-4490-824a-ca14ee1e5fcb
📒 Files selected for processing (1)
.github/workflows/cache-reaper.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Hello 👋 This PR has had no activity for more than 2 weeks. If you are still working on it, please push an update or leave a comment. Ping a maintainer if you believe it is ready for review or merge! This PR will be automatically closed in 7 days if there is no further activity. |
Addressed Issues:
Fixes #(issue number)
Screenshots/Recordings:
Additional Notes:
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit