Skip to content

Add weekly GitHub Actions cache cleanup - #325

Merged
3 commits merged into
mainfrom
dev/actions-cache-cleanup
May 12, 2026
Merged

Add weekly GitHub Actions cache cleanup#325
3 commits merged into
mainfrom
dev/actions-cache-cleanup

Conversation

@ANcpLua

@ANcpLua ANcpLua commented May 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a weekly GitHub Actions workflow that deletes stale refs/pull/*/merge cache entries
  • keep refs/heads/main caches untouched
  • support manual dry-run and retention-window inputs

Verification

  • ruby -e 'require "yaml"; YAML.load_file(ARGV.fetch(0)); puts "yaml ok"' .github/workflows/actions-cache-cleanup.yml
  • local dry run of the embedded workflow script with DRY_RUN=true
  • actionlint .github/workflows/actions-cache-cleanup.yml
  • git diff --check

View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

Copilot AI review requested due to automatic review settings May 11, 2026 22:49
@github-actions

Copy link
Copy Markdown

@coderabbitai autofix

@claude

claude Bot commented May 11, 2026

Copy link
Copy Markdown

Claude finished @ANcpLua's task in 1m 24s —— View job


Review — Add weekly GitHub Actions cache cleanup

  • Read .github/workflows/actions-cache-cleanup.yml
  • Cross-check against existing workflows + CLAUDE.md conventions
  • Identify defects / improvements
  • Post review feedback

Overall this is a clean, well-scoped workflow — minimized permissions (actions: write, contents: read), concurrency group with cancel-in-progress: false (correct for a deletion job), set -euo pipefail, stdlib-only Python so no pip install step, and the iterate-then-delete order means the pagination cursor doesn't shift while you walk it. Two small things worth tightening, plus one description/code mismatch.

Note on capabilities: I can't push directly to .github/workflows/* from this bot (GitHub App permissions don't allow workflow modifications — see the FAQ), so this is a comment-only review.


1. Filter is broader than the PR description claims — refs/pull/*/merge vs refs/pull/*

The PR body says "deletes stale refs/pull/*/merge cache entries", but the code at .github/workflows/actions-cache-cleanup.yml:122 filters on the prefix only:

if not ref.startswith("refs/pull/"):
    continue

That also matches refs/pull/<n>/head (and any other refs/pull/<n>/* ref GitHub may surface). In practice Actions usually stores against /merge, but the code and description should agree. Either tighten the filter to ref.endswith("/merge") (or check ref.startswith("refs/pull/") and ref.endswith("/merge")), or relax the PR description to "PR-ref caches". Tightening is the safer pick if the intent really is "leave non-merge PR caches alone."

2. last_accessed_at missing/empty crashes the whole run

At .github/workflows/actions-cache-cleanup.yml:121-125:

last_accessed = str(cache.get("last_accessed_at", ""))
...
accessed_at = datetime.fromisoformat(last_accessed.replace("Z", "+00:00"))

If last_accessed_at is ever missing or empty (defensive .get(..., "") already hints you don't fully trust the field), datetime.fromisoformat("+00:00") raises ValueError and the whole cleanup aborts mid-pass — every subsequent cache for that run goes untouched. A tiny guard keeps the job idempotent:

if not last_accessed:
    continue
try:
    accessed_at = datetime.fromisoformat(last_accessed.replace("Z", "+00:00"))
except ValueError:
    print(f"::warning::cache id {cache.get('id')} has unparseable last_accessed_at={last_accessed!r}; skipping")
    continue

3. Manual-dispatch dry_run default

Minor UX nit: the workflow_dispatch default for dry_run is "false". Most "delete stuff" workflows default the manual path to dry-run so a curious maintainer poking the button doesn't immediately delete production state; the scheduled cron still runs real with an explicit override. Up to you — the explicit choice enum already makes the button safe-by-acknowledgement.

4. Nothing to fix on pagination

if len(page_items) < 100: break is fine — GitHub's pagination contract guarantees a full page until the last one for this endpoint. The Link-header pattern would be slightly more correct but adds no real value here.


If you'd like me to draft the two-line patch as a diff snippet you can apply locally for #1 and #2, just say the word — I just can't push it from this bot.
· Branch dev/actions-cache-cleanup

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Risk: Medium — scheduled workflow with actions: write can delete more caches than intended if the ref filter is too broad.

This PR adds a scheduled + manually-triggerable GitHub Actions workflow to prune stale Actions caches associated with pull request refs, aiming to keep cache storage usage under control without affecting main caches.

Changes:

  • Add a weekly scheduled workflow plus workflow_dispatch inputs for retention window (minimum_age_days) and dry_run.
  • Implement an embedded Python script that enumerates Actions caches via the GitHub API, filters by ref + last-accessed time, and optionally deletes them.

Blockers

  • The current ref filter matches all refs/pull/* caches, but the PR description/header comment state it should delete only refs/pull/*/merge caches. This can lead to unintended deletions.

Important issues

  • The script can crash if last_accessed_at is missing/invalid because datetime.fromisoformat(...) will raise, aborting the whole cleanup run.
  • Workflow permissions include contents: read even though the job doesn’t read repository contents; tightening permissions would reduce blast radius.

Minor / optional

  • None noted.

Comment on lines +120 to +123
ref = str(cache.get("ref", ""))
last_accessed = str(cache.get("last_accessed_at", ""))
if not ref.startswith("refs/pull/"):
continue
if not ref.startswith("refs/pull/"):
continue

accessed_at = datetime.fromisoformat(last_accessed.replace("Z", "+00:00"))

permissions:
actions: write
contents: read
@ANcpLua
ANcpLua marked this pull request as ready for review May 12, 2026 02:26
@github-actions

Copy link
Copy Markdown

@coderabbitai autofix

@claude

claude Bot commented May 12, 2026

Copy link
Copy Markdown

Claude finished @ANcpLua's task in 1m 42s —— View job


Review — Add weekly GitHub Actions cache cleanup

  • Read .github/workflows/actions-cache-cleanup.yml
  • Cross-check earlier review comments against current file
  • Identify mechanically-fixable defects
  • Attempt agentic fix
  • Post review feedback

I drafted the fix and tried to push it to this branch, but the remote rejected it: this GitHub App doesn't carry the workflows permission, so any .github/workflows/* write is server-side blocked (the previous bot run noted the same; see the FAQ). Comment-only review with a ready-to-apply patch below.


The three findings the earlier reviewers raised (filter scope, last_accessed_at parse crash, unused contents: read) all still apply at HEAD. None of them are stylistic — each one is a real correctness or least-privilege bug. I'd land all three together.

1. Filter doesn't match what the header comment + PR body promise — .github/workflows/actions-cache-cleanup.yml:122

File header at line 4 says "Deletes stale PR merge-ref caches"; PR body says "refs/pull//merge"*. The actual predicate is ref.startswith("refs/pull/"), which also matches refs/pull/<n>/head. Tighten to endswith("/merge") so head-ref caches are preserved.

2. Missing/malformed last_accessed_at aborts the run — .github/workflows/actions-cache-cleanup.yml:121,125

last_accessed = str(cache.get("last_accessed_at", "")) already hedges for missing data, but the very next line calls datetime.fromisoformat(last_accessed.replace("Z", "+00:00")). Empty string → fromisoformat("+00:00")ValueError → entire pass dies, every cache enumerated after the bad one survives. Cheap to guard.

3. contents: read is unused — .github/workflows/actions-cache-cleanup.yml:30

No actions/checkout, no API call that reads repo contents. Drop it; actions: write is the only scope the cleanup actually needs.

Not blocking, but worth a thought

  • workflow_dispatch dry_run default (.github/workflows/actions-cache-cleanup.yml:22): defaulting to "false" means a maintainer who pokes the manual button immediately deletes prod state. The cron path explicitly sets false, so flipping the dispatch default to "true" only changes the human-driven case — typically the right safety bias. Optional.
  • Pagination at :114: if len(page_items) < 100: break is fine for this endpoint; Link-header walking would be marginally more correct but adds no real value.

Patch (paste into git apply from repo root)

--- a/.github/workflows/actions-cache-cleanup.yml
+++ b/.github/workflows/actions-cache-cleanup.yml
@@ -27,7 +27,6 @@ on:

 permissions:
   actions: write
-  contents: read

 concurrency:
   group: actions-cache-cleanup
@@ -119,10 +118,22 @@ jobs:
           for cache in caches:
               ref = str(cache.get("ref", ""))
               last_accessed = str(cache.get("last_accessed_at", ""))
-              if not ref.startswith("refs/pull/"):
+              if not (ref.startswith("refs/pull/") and ref.endswith("/merge")):
                   continue

-              accessed_at = datetime.fromisoformat(last_accessed.replace("Z", "+00:00"))
+              if not last_accessed:
+                  print(
+                      f"::warning::cache id {cache.get('id')} has no last_accessed_at; skipping"
+                  )
+                  continue
+              try:
+                  accessed_at = datetime.fromisoformat(last_accessed.replace("Z", "+00:00"))
+              except ValueError:
+                  print(
+                      f"::warning::cache id {cache.get('id')} has unparseable "
+                      f"last_accessed_at={last_accessed!r}; skipping"
+                  )
+                  continue
               if accessed_at <= cutoff:
                   targets.append(cache)

· Branch dev/actions-cache-cleanup

ANcpLua added a commit that referenced this pull request May 12, 2026
The destructive tier runs on a runner without actions/checkout, so the
working dir has no .git. gh pr {review,merge,view} without --repo tries
to derive the repository from the local git context and dies with
"fatal: not a git repository". Add --repo "$GITHUB_REPOSITORY" to every
gh call in the workflow so they bind to the calling repository via env
instead of git-context detection.

Observed on the first destructive run against PR #325 — both attempts
failed at the admin-merge step with that exact error.
Three findings from the copilot-pull-request-reviewer pass on PR #325,
all valid:

1. Filter mismatch (blocker, line 122): the previous predicate
   `ref.startswith("refs/pull/")` matched both refs/pull/<N>/head and
   refs/pull/<N>/merge, but the PR description and the workflow header
   comment say only the merge-test refs should be deleted. The head
   refs back the active build for the open PR's head SHA and are still
   useful while the PR is open. Tighten to require the `/merge`
   suffix; head caches are now preserved.

2. fromisoformat fragility (important, line 125): if last_accessed_at
   is missing/empty or malformed, datetime.fromisoformat raises and
   aborts the whole cleanup run. Skip those entries with a clear
   ::warning:: instead so one bad cache record cannot break the job.
   Also handle the empty-string case explicitly (str(None) -> "None"
   but cache.get(..., "") -> "" is the actual path).

3. Least-privilege permissions (important, line 30): the job only
   talks to the actions/caches API; it does not check out the repo or
   read repository contents. Drop `contents: read` so the workflow
   token grants only `actions: write`.
Copilot AI review requested due to automatic review settings May 12, 2026 02:30

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved by destructive auto-merge (admin tier)

@ghost
ghost merged commit 7298c40 into main May 12, 2026
13 checks passed
@github-actions

Copy link
Copy Markdown

@coderabbitai autofix

@ghost
ghost deleted the dev/actions-cache-cleanup branch May 12, 2026 02:30
@claude

claude Bot commented May 12, 2026

Copy link
Copy Markdown

Claude finished @ANcpLua's task in 52s —— View job


Review — Add weekly GitHub Actions cache cleanup

  • Read .github/workflows/actions-cache-cleanup.yml at HEAD
  • Re-verify earlier findings against the merged file
  • Post final review

All three substantive findings from the earlier review rounds (Copilot + my prior pass) already landed in commit c5e629a and shipped with the merged PR. The file at HEAD is clean against every concern raised on this thread:

# Earlier finding Status at HEAD
1 Filter was refs/pull/* — would also match …/head caches ✅ Fixed at .github/workflows/actions-cache-cleanup.yml:124ref.startswith("refs/pull/") and ref.endswith("/merge"); comment at L121-123 explains why head caches are preserved
2 datetime.fromisoformat on missing/empty last_accessed_at would crash the whole pass ✅ Fixed at .github/workflows/actions-cache-cleanup.yml:127-137 — empty-string skip + ValueError skip, both emit ::warning:: and continue
3 contents: read was unused (least-privilege) ✅ Fixed at .github/workflows/actions-cache-cleanup.yml:28-29 — only actions: write remains

Nothing else worth flagging:

  • set -euo pipefail, stdlib-only Python, casefold() on DRY_RUN, AOT-irrelevant for a runner script — all good.
  • concurrency.cancel-in-progress: false is correct for a deletion job — never cancel a delete mid-flight.
  • if len(page_items) < 100: break pagination is fine for this endpoint (Link-header walking would be marginally more correct but adds no value).
  • The remaining "optional" nit — workflow_dispatch.dry_run defaulting to "false" — is a judgment call the author already heard and chose to leave. The choice enum makes the manual button a deliberate two-step, so this isn't a defect.

Why no commit on this branch

PR is already merged, and this GitHub App can't write to .github/workflows/* anyway (workflow-modification scope is server-side blocked — see the FAQ). Neither matters here, because there's nothing left to fix. ✅ Approving.
· Branch dev/actions-cache-cleanup

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved by destructive auto-merge (admin tier)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

minimum_age_days:
description: "Delete pull-ref caches last accessed at least this many days ago"
required: false
default: "3"
Comment on lines +19 to +26
dry_run:
description: "Report matching caches without deleting them"
required: false
default: "false"
type: choice
options:
- "false"
- "true"
jobs:
cleanup:
name: Delete stale pull-request caches
runs-on: ubuntu-latest
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants