cargo-cache: eviction reader-check is check-then-delete, kept safe by policy rather than structure #1

New Issue

2026-08-24T00:56:43Z

claude commented

2026-08-24 00:56:43 +00:00

Moved from daniel/gitdan#14, filed
2026-08-23 before the shared cache action had a tracker of its own.

It belongs here: gitdan owns the outer ring of the two-ring cache
scheme — scripts/ci-cache-reclaim.sh, the host-level arbiter that runs
from a systemd timer on gitdan-ci, sees every *-ci-target volume at once,
and fires at 12% free. This repo owns the inner ring — the per-repo,
in-workflow prune that touches only its own volume and fires at 10% free
(scripts/prune-cache.sh:78, MIN_FREE_PCT="${4:-10}"). This issue is
entirely about the inner ring, and both fixes it proposes land in this repo:
the structural one in scripts/prune-cache.sh, the tripwire-test one in
scripts/prune-cache-selftest.sh.

Still live as of 3b2dec6. The window is present at both eviction sites
— scripts/prune-cache.sh:175 and :208 each do is_locked "$dir" && continue, then usage_gb / echo / summary_line, then rm -rf "$dir"
at :179 and :212. Note the timeline: 3b2dec6 "cover the stale-reader-
marker sweep and its bound" landed ~20 minutes before this was filed, so
that test did not address it.

Original body follows verbatim.

A tripwire, not a live bug. prune-cache.sh's reader-marker check is
check-then-delete:

is_locked "$dir" && continue
dir_gb=$(usage_gb ...)      # window
echo ...                    # window
summary_line ...            # window
rm -rf "$dir"

A consumer can acquire its reader marker after the check and before the delete.

Unreachable today, verified by tracing every source that is ever
hardlink-cloned from:

a protected branch's own snapshot — is_protected excludes it from every
eviction pass before is_locked is reached; or
an out-of-$ROOT seed-fallback-dir — prune-cache.sh's globs never match it.

So the gap is dead code today. The reason it is dead is policy, not
structure: it stays dead only while snapshots belong to protected refs and
protected refs are never eviction candidates. Change either — add a
non-protected publisher ref, or let the prune pass consider snapshots — and this
becomes live, silently, with nothing failing to warn you.

Filed because that is precisely the class this project keeps getting bitten by
(see zemyna #914, and the modular-by-default rationale: a property maintained
by vigilance rather than by construction fails at scale). The code says so in
its own comment — "this guard is redundant by policy… it is here so that stops
being the reason it is safe" — which is honest, but a comment is not a tripwire.

What would close this

Either:

Structural: close the window (re-check the marker immediately before
rm -rf, the same way hardlink_clone_into re-derives its safety checks
right before acting rather than trusting how it got there); or
A real tripwire: a test asserting that snapshots of non-protected refs are
never eviction candidates, so changing the policy fails a test instead of
quietly arming the window.

The reviewer's recommendation was to leave it as a documented residual rather
than add locking complexity to an unreachable path — hence this issue rather
than a change. Not urgent; it should not be silently forgotten either.

> **Moved from [`daniel/gitdan#14`](https://gitdan.com/daniel/gitdan/issues/14)**, filed > 2026-08-23 before the shared cache action had a tracker of its own. > > It belongs here: `gitdan` owns the **outer ring** of the two-ring cache > scheme — `scripts/ci-cache-reclaim.sh`, the host-level arbiter that runs > from a systemd timer on gitdan-ci, sees every `*-ci-target` volume at once, > and fires at 12% free. This repo owns the **inner ring** — the per-repo, > in-workflow prune that touches only its own volume and fires at 10% free > (`scripts/prune-cache.sh:78`, `MIN_FREE_PCT="${4:-10}"`). This issue is > entirely about the inner ring, and both fixes it proposes land in this repo: > the structural one in `scripts/prune-cache.sh`, the tripwire-test one in > `scripts/prune-cache-selftest.sh`. > > **Still live as of `3b2dec6`.** The window is present at both eviction sites > — `scripts/prune-cache.sh:175` and `:208` each do `is_locked "$dir" && > continue`, then `usage_gb` / `echo` / `summary_line`, then `rm -rf "$dir"` > at `:179` and `:212`. Note the timeline: `3b2dec6` *"cover the stale-reader- > marker sweep and its bound"* landed ~20 minutes **before** this was filed, so > that test did not address it. > > Original body follows verbatim. --- A tripwire, not a live bug. `prune-cache.sh`'s reader-marker check is **check-then-delete**: ``` is_locked "$dir" && continue dir_gb=$(usage_gb ...) # window echo ... # window summary_line ... # window rm -rf "$dir" ``` A consumer can acquire its reader marker after the check and before the delete. **Unreachable today**, verified by tracing every source that is ever hardlink-cloned from: 1. a protected branch's own snapshot — `is_protected` excludes it from every eviction pass **before** `is_locked` is reached; or 2. an out-of-`$ROOT` `seed-fallback-dir` — `prune-cache.sh`'s globs never match it. So the gap is dead code today. The reason it is dead is **policy, not structure**: it stays dead only while snapshots belong to protected refs and protected refs are never eviction candidates. Change either — add a non-protected publisher ref, or let the prune pass consider snapshots — and this becomes live, silently, with nothing failing to warn you. Filed because that is precisely the class this project keeps getting bitten by (see zemyna #914, and the `modular-by-default` rationale: a property maintained by vigilance rather than by construction fails at scale). The code says so in its own comment — *"this guard is redundant by policy… it is here so that stops being the reason it is safe"* — which is honest, but a comment is not a tripwire. ## What would close this Either: - **Structural**: close the window (re-check the marker immediately before `rm -rf`, the same way `hardlink_clone_into` re-derives its safety checks right before acting rather than trusting how it got there); or - **A real tripwire**: a test asserting that snapshots of non-protected refs are never eviction candidates, so changing the policy fails a test instead of quietly arming the window. The reviewer's recommendation was to leave it as a documented residual rather than add locking complexity to an unreachable path — hence this issue rather than a change. Not urgent; it should not be silently forgotten either.

claude added the enhancement label 2026-08-24 00:56:43 +00:00

claude referenced a pull request that will close this issue

2026-08-24 04:11:49 +00:00

fix(prune-cache): close the reader-marker check-then-delete window #2

claude-reviewer referenced this issue

2026-08-24 04:22:30 +00:00

fix(prune-cache): close the reader-marker check-then-delete window #2

claude-reviewer referenced this issue

2026-08-24 04:43:40 +00:00

fix(prune-cache): close the reader-marker check-then-delete window #2

claude closed this issue

2026-08-24 14:35:53 +00:00