Repository navigation
fix(reviewer): bound the PR-head snapshot cache to open-PR heads - #171
Merged
Merged
Conversation
Cached PR-head snapshots (~118MB each) were never evicted, filling the e2-micro's 19GB disk. Each tick now prunes every heads/<sha> snapshot whose SHA is not the head of an open PR, with the keep-set parsed from the full PR queue rather than the review loop (which can break early on the attempt budget). The runtime-dir suffix is pinned to the uid so cron (no $USER) and interactive runs share one cache instead of growing two.
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.
Root cause
prepare_review_worktree()is a write-only cache: every first review of a head SHA downloads and extracts a ~118MB PR-head tarball into$RUNTIME_STATE_DIR/worktrees/<slug>/heads/<sha>,chmod -R a-ws it, and nothing ever deletes it. Snapshots are only reused when a review of the same head SHA is retried on a later tick; once a PR is re-committed, merged, or closed, its snapshot is dead weight forever. On the deployed e2-micro this grew to 6+GB and filled the 19GB disk.Fix: prune to the live open-PR head set every tick
New
prune_stale_review_worktrees()inlib/worktree.shtakes the set of head SHAs to keep and deletes everyheads/<sha>directory not in it (restoringu+wfirst, since snapshots are createda-w).reviewer.shcalls it once per tick after the review loop, so the cache is bounded by the open-PR working set instead of growing without eviction.Two correctness points:
$PRS(field 3 of every row), not accumulated inside the review loop. The loop canbreakearly when it hits the attempt budget (review_one_pr_status -eq 10), so an in-loop accumulation would omit PRs never reached this tick and wrongly evict their still-valid snapshots. An empty$PRS(no open PRs) yields an empty keep-set and evicts everything, which is correct.ONLY_PR/DRY_RUN/RENDER_PROMPT_ONLY), which can populate$PRSwith a single PR — pruning there would evict every other open PR's cache.Secondary fix: deterministic runtime dir
RUNTIME_OWNERwas${USER:-$(id -u ...)}: cron runs with$USERunset (→ uid), interactive runs have it set (→ username), so the daemon grew two parallel runtime dirs and pruning one leaves the other to rot. The suffix is now pinned to the uid first ($(id -u 2>/dev/null || printf '%s' "${USER:-user}")"), so both entry points resolve the same dir. TheREVIEWER_RUNTIME_STATE` override still wins, unchanged.Tests
New fixture
test_prune_stale_review_worktreesintests/fixtures/worktree-state.shbuilds real snapshots viaprepare_review_worktree(so the trees are genuinelychmod a-w) and covers: keep-set survival, stale-SHA eviction, the restore-write-then-delete path on read-only trees, empty keep-set evicting everything, and missingheads/as a return-0 no-op. Full suite:passed 577 fixture assertions (matches pinned EXPECTED_ASSERTIONS);bash -nclean on all 49 shell files.