ci: surface cache budget consumers and cap the E2E await deadline - #599
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 49 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThe cache-warming workflow now removes redundant cache duplicates and reports largest entries. E2E scenario deadlines default to 25 seconds, with tests validating the value and a minimum safety threshold. ChangesCache maintenance
Scenario deadline defaults
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
ytallo
added
the
no-ticket
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/cache-warm.yml:
- Around line 84-98: The duplicate-cache selection in “Delete superseded
duplicates of the same cache key” currently groups only by key; retain and emit
each row’s ref and version from actions_caches, sort by key, ref, and version
with newest last_accessed_at first, and update the awk grouping to keep only the
newest row per (key, ref, version) while collecting older IDs and sizes for
deletion.
- Around line 259-265: Update the cache-report pipeline in the workflow to
remove the head -15 stage and limit output to the first 15 records within the
existing awk formatter, preserving the current sorting and table formatting
while avoiding SIGPIPE failures under pipefail.
- Around line 93-99: Update the cache-listing flow that populates rows so gh api
--paginate output is written to a temporary file first and its exit status is
checked before any processing or deletion. Only run sort/awk and mapfile after a
successful listing, and clean up the temporary file on success and failure.
🪄 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: CHILL
Plan: Pro Plus
Run ID: c1fefcb8-dc1e-4bd3-9415-9bed8f33159e
📒 Files selected for processing (3)
.github/workflows/cache-warm.ymlharness/tests/e2e/src/types/scenario.rsharness/tests/e2e/src/types/scenario/compiled.rs
Comment on lines
+84
to
+98
| # A key can hold several entries, one per ref that saved it. Only the | ||
| # newest is ever restored, so the rest are dead weight that still counts | ||
| # against the repository budget — and evicting real entries under LRU. | ||
| - name: Delete superseded duplicates of the same cache key | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| run: | | ||
| set -euo pipefail | ||
| mapfile -t rows < <( | ||
| gh api --method GET "repos/$REPOSITORY/actions/caches" \ | ||
| -F per_page=100 --paginate \ | ||
| --jq '.actions_caches[] | [.key, .last_accessed_at, .id, .size_in_bytes] | @tsv' \ | ||
| | sort -t"$(printf '\t')" -k1,1 -k2,2r \ | ||
| | awk -F"$(printf '\t')" '$1 == prev { print $3 "\t" $4 } { prev = $1 }' |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate workflow:"
git ls-files | grep -F '.github/workflows/cache-warm.yml' || true
echo
echo "Relevant workflow excerpt:"
if [ -f .github/workflows/cache-warm.yml ]; then
sed -n '1,140p' .github/workflows/cache-warm.yml | cat -n
fi
echo
echo "Search for cache API cache fields in repo docs/scripts:"
rg -n "actions_caches|size_in_bytes|last_accessed_at|cacheVersion|version|ref" .github README.md 2>/dev/null || trueRepository: iii-hq/workers
Length of output: 40519
🌐 Web query:
GitHub Actions REST API list cache entries actions_caches response fields id key ref version last_accessed_at size_in_bytes
💡 Result:
The GitHub Actions REST API endpoint to list cache entries for a repository is GET /repos/{owner}/{repo}/actions/caches [1][2]. When a request to this endpoint is successful (HTTP 200), the response body contains a total_count integer and an actions_caches array containing objects with the following fields [1][2]: id: The integer ID of the cache [1][2]. ref: The Git reference (branch or pull request) associated with the cache [1][2]. key: The cache key or prefix [1][2]. version: The version string for the cache [1][2]. last_accessed_at: A date-time string indicating when the cache was last accessed [1][2]. created_at: A date-time string indicating when the cache was created [1][2]. size_in_bytes: The size of the cache in bytes, represented as an integer [1][2].
Citations:
- 1: https://docs.github.com/en/rest/actions/cache?apiVersion=2022-11-28
- 2: https://docs.github.com/en/enterprise-server@3.20/rest/actions/cache?apiVersion=2022-11-28
Group duplicate-candidate rows by ref and version too.
actions_caches entries are scoped by key, version, and ref. This step drops .ref and .version, so it can delete valid caches from another branch/PR or cache version. Keep the newest row per (key, ref, version) before collecting IDs to delete.
🤖 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/cache-warm.yml around lines 84 - 98, The duplicate-cache
selection in “Delete superseded duplicates of the same cache key” currently
groups only by key; retain and emit each row’s ref and version from
actions_caches, sort by key, ref, and version with newest last_accessed_at
first, and update the awk grouping to keep only the newest row per (key, ref,
version) while collecting older IDs and sizes for deletion.
Comment on lines
+93
to
+99
| mapfile -t rows < <( | ||
| gh api --method GET "repos/$REPOSITORY/actions/caches" \ | ||
| -F per_page=100 --paginate \ | ||
| --jq '.actions_caches[] | [.key, .last_accessed_at, .id, .size_in_bytes] | @tsv' \ | ||
| | sort -t"$(printf '\t')" -k1,1 -k2,2r \ | ||
| | awk -F"$(printf '\t')" '$1 == prev { print $3 "\t" $4 } { prev = $1 }' | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the workflow around the reported lines and surrounding shell options.
if [ -f .github/workflows/cache-warm.yml ]; then
echo "## file exists"
wc -l .github/workflows/cache-warm.yml
echo "## lines 1-140"
sed -n '1,140p' ./github/workflows/cache-warm.yml | cat -n
else
echo "file missing"
git ls-files .github/workflows || true
fi
echo "## occurrences of set -euo pipefail / mapfile / gh api caches"
rg -n "set -euo pipefail|mapfile|gh api --method GET \"repos/\$REPOSITORY/actions/caches\"|--paginate|cache" .github/workflows/cache-warm.yml .github/workflows 2>/dev/null || trueRepository: iii-hq/workers
Length of output: 296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## workflow snippet"
sed -n '1,160p' .github/workflows/cache-warm.yml | cat -n
echo "## occurrences"
rg -n "set -euo pipefail|mapfile|gh api --method GET|actions/caches|--paginate|rm -rf|xargs|delete" .github/workflows/cache-warm.yml
echo "## bash behavior for process substitution with set -e and non-zero commands"
bash -c 'set -euo pipefail; x=1; true; y=($(false)); echo "$?"' || echo "status=$?"
echo "## bash behavior for process substitution exit status in same shell under set -u (no read)”
bash -c 'set -euo pipefail; while IFS= read -r _ < <(false); do true; done; echo final' || echo "status=$?"
echo "## bash behavior with read capturing from command substitution before assignment"
bash -c 'set -euo pipefail; mapfile -t rows < <(false); echo ${`#rows`[@]}' || echo "status=$?"Repository: iii-hq/workers
Length of output: 8384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## bash behavior tests\n'
python3 - <<'PY'
import tempfile, subprocess, os
cases = [
("mapfile-subset", 'set -euo pipefail; mapfile -t rows < <(false); printf "len=%d rows=[%s]\\n" "${`#rows`[@]}" "$*"'),
("read-loop-subset", 'set -euo pipefail; count=1; while IFS= read -r _ < <(false); do :; done; printf "count=%d\\n" "$count"'),
]
for name, script in cases:
f = tempfile.NamedTemporaryFile(mode='w', delete_on_close=False, suffix='.sh')
f.write("#!/bin/sh\n" + script + "\n")
f.close()
os.chmod(f.name, 0o755)
r = subprocess.run(["/bin/sh", f.name], capture_output=True, text=True)
print(f"## {name}")
print("stdout=", r.stdout.strip())
print("stderr=", repr(r.stderr.strip()))
print("returncode=", r.returncode)
PY
printf '\n## workflow lines 230-270\n'
sed -n '230,270p' .github/workflows/cache-warm.yml | cat -n
printf '\n## all mapfile process substitutions in workflow\n'
rg -n "mapfile -t .*< <\\(" .github/workflows/cache-warm.ymlRepository: iii-hq/workers
Length of output: 333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## bash behavior tests\n'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
for case in \
'mapfile-subset set -euo pipefail; mapfile -t rows < <(false); printf "len=%d\n" "${`#rows`[@]}"' \
'read-loop-subset set -euo pipefail; count=1; while IFS= read -r _ < <(false); do :; done; printf "count=%d\n" "$count"' \
'command-subset set -euo pipefail; x=($(false)); printf "len=%d\n" "${`#x`[@]}"' \
'mapfile-subset-silent set -euo pipefail; mapfile -t rows < <({ false >&2; printf "bad\n"; }); printf "len=%d\n" "${`#rows`[@]}"' \
; do
name=${case%% *}
cmd=${case#* }
printf '%s\n' "$cmd" > "$tmpdir/test.sh"
chmod +x "$tmpdir/test.sh"
printf '%s\n' "## $name"
bash "$tmpdir/test.sh" || printf 'RETURNED_NONZERO\n'
wait
done
printf '\n## workflow lines 230-270\n'
sed -n '230,270p' .github/workflows/cache-warm.yml | cat -n
printf '\n## all mapfile process substitutions in workflow\n'
rg -n "mapfile -t .*< <\\(" .github/workflows/cache-warm.ymlRepository: iii-hq/workers
Length of output: 2884
Propagate cache-listing failures before deleting anything.
mapfile -t rows < <(...) still returns success even when the process substitution exits non-zero, so a failed or interrupted gh api --paginate can leave an empty or partial rows array and proceed to delete nothing. Capture the listing to a temp file first, then mapfile only after the listing command exits successfully.
🤖 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/cache-warm.yml around lines 93 - 99, Update the
cache-listing flow that populates rows so gh api --paginate output is written to
a temporary file first and its exit status is checked before any processing or
deletion. Only run sort/awk and mapfile after a successful listing, and clean up
the temporary file on success and failure.
The repository sits at 10.42 GiB against a 10 GiB hard budget, so the `Report cache budget` job fails and GitHub evicts by last-access. The report gave a total but no way to see what was consuming the space, which made the eviction look arbitrary. List the 15 largest entries with their key and ref. On current data that immediately shows 4.01 GiB — 38% of the budget — is Windows release-build caches from `_rust-binary.yml`, written on tag pushes and read only by the next release of the same worker, while the pinned-engine cache that every PR run depends on gets evicted and costs 308s to rebuild each time. Whether to keep those is a judgement call about release build times, so this only makes the trade-off visible. The `ref` column is what tells a reader whether an entry is shared (refs/heads/main) or scoped to one PR. Row capping happens inside the awk pass: piping through `head` would close sort's stdout early and SIGPIPE it under `pipefail`.
`scenario_ms` covers only the await phase — stack boot is budgeted separately by `readiness_ms` — and the slowest passing scenario observed in CI takes ~4s. At 60s, a run with two wedged scenarios spent 122s of its 257s scenario step waiting on deadlines rather than doing work. 25s keeps roughly 6x headroom over the slowest observed pass while capping what a wedged scenario costs. The default-deadline test now also asserts the headroom, so a future reduction cannot quietly approach the real runtime.
ytallo
changed the title
ytallo
force-pushed
the
fix/ci-cache-prune-and-scenario-deadline
branch
from
July 24, 2026 20:37
031aa38 to
0a65d0f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
harness-integrationjob takes 10m31s. Measured breakdown of that run:1. Cache budget visibility (
cache-warm.yml)The 308s is a cache miss on every PR run — there is no
integration-engine-*entry in the repository at all. The caching architecture is correct (PRs run withsave-cache: false;cache-warm.ymlsaves withtrue), and the warm job does succeed. The entry simply doesn't survive: the repository sits at 10.42 GiB against a 10 GiB hard budget, so GitHub evicts by last access and the engine cache loses.The budget report gave a total with no way to see what was consuming the space, which made the eviction look arbitrary. It now lists the 15 largest entries with their key and ref.
On current data that immediately shows 4.01 GiB — 38% of the budget — is Windows release-build caches from
_rust-binary.yml, written on tag pushes byrelease.ymland read only by the next release of the same worker. Every PR pays for that by having the engine cache evicted instead.Pruning those is a judgement call about release build times, so this PR does not make it — it only stops it being invisible. Reclaiming that 4 GiB, or raising the cache limit (configurable since Nov 2025, billed above 10 GiB), is what actually removes the 308s.
2. E2E await deadline (
scenario_ms)scenario_mswas 60s. It covers only the await phase — stack boot is budgeted separately byreadiness_ms, left unchanged — and the slowest passing scenario observed in CI takes 3.9s. In the run measured above, two wedged scenarios burned 122s of the 257s scenario step purely waiting on deadlines.Lowered to 25s: roughly 6x headroom over the slowest observed pass, and a wedged scenario now costs 25s instead of 60s. The default-deadline test additionally asserts that headroom, so a future reduction can't quietly creep toward real runtime.
On pruning
The first version of this PR deleted entries sharing the same
key, on the assumption that only the newest is ever restored. CodeRabbit pointed out that entries are scoped by(key, ref, version), and it was right. Checked against live data:refs/pull/{592,594,597,598}/merge;(key, ref, version)— GitHub enforces uniqueness on that tuple, so the corrected grouping would be dead code.So the step was removed rather than regrouped. Caches from closed PRs are already handled by the existing
prune-closed-prsjob. The honest conclusion: there is no safe automatic pruning to add here — the real waste is the 4 GiB of Windows release caches, which is a human decision.Expected effect
This PR saves 35s per wedged scenario on red runs, and unblocks the budget check once the space question is settled. The large win (308s) depends on the decision in §1.
Validation
cargo test --manifest-path harness/Cargo.toml -p harness-integration— 84 passed, 0 failed.cache-warm.ymlparses; the editedrun:block passesbash -n.pipefailenabled, exit 0 — row capping lives inside theawkpass precisely becauseheadwould close sort's stdout early and SIGPIPE it underpipefail.No write or DELETE path against the GitHub API remains in this PR.
CI-only, so no Linear ticket; labelled
no-ticketperpr-linear-check.yml.