Review nits on #2, both cosmetic.
The sentence added last round left its paragraph at 127 characters in a file
that otherwise wraps comments at 78-79 — the rewrap after the insert simply
did not happen. `awk 'length($0)>80'` over both files now reports one comment
line, the pre-existing 93-character usage string at :4.
Scenario 14's header called its fixture "the only shape that can tell the two
timestamps apart". The property it needs is old mtime with fresh ctime; an
old directory renamed a moment ago is the production instance of that, not
the only construction of it. "Production's shape" was already carrying the
argument.
No behaviour change and no assertion change.
Review finding on #2. Swapping the settle window's `stat -c %Z` for `%Y` —
the exact substitution the comment beside it calls the wrong signal — left
the suite green at 37/37. The scenario built its aside with `mkdir`, so the
fixture's mtime was also ~now and the two timestamps agreed; a fixture whose
clocks agree cannot tell them apart.
Every real aside is the opposite shape: a cache last written days ago,
renamed a moment ago. Under `%Y` the window would never fire for one, the
sweeper would silently return to reclaiming asides another pass is still
deciding about, and nothing would say so — the mechanism guarded by a comment
again, which is what the previous finding was about.
So the fixture is now built the way the pass builds one: an old directory
`mv`d into the aside name. `%Z` -> `%Y` now fails scenario 14, as does
deleting the guard outright.
Also sharpens why the PID alternative was rejected: the `$$` in an aside's
name was that pass's PID inside its own job container, so testing it from
another one is not unreliable, it is meaningless.
No change to prune-cache.sh's behaviour; the assertions are untouched.
Review findings on #2. The first is the one that mattered: the sweeper this
PR added had the shape the PR exists to remove. Pass A renames a candidate
aside; pass B's sweep sees an aside with no readers and reclaims it; A then
finds a reader and restores. `rm -rf` traverses fd-relative, so the rename
does not stop it and A can republish a half-emptied tree under a live cache
name. `capacity: 1` bounds it today, which is exactly the kind of reason
this PR was written to stop relying on.
The unlink itself was never the problem — the ordering proof covers it under
any interleaving, since the aside name only exists after the evicting pass's
rename. What was missing is that an aside with no readers is indistinguishable
from one a pass has just created and not yet decided about. The sweeper now
leaves an aside alone until it has settled (EVICTION_ASIDE_SETTLE_SECONDS,
default 60), which separates the two without having to identify the pass that
created it — a PID is meaningless across the job containers these passes run
in, and recycles. Read from ctime, not mtime: rename(2) updates the first and
leaves the second at whenever the cache was last written, which is the signal
list_by_lru wants and the wrong one here.
That is a bound, not a construction, and both the code comment and the README
now say which of the two properties is which instead of asserting the broader
one.
Also from the review: a pass that declined every dead cache it found no
longer signs off with "no dead-branch caches found", and evict_dir no longer
promises a later reclamation of an aside that is already gone.
Scenario 14 covers the settle window against the script's own default, with
nothing faked — the directory really was set aside a moment ago. Scenario 4
gains the summary assertion. 31 -> 37 assertions; each new gate verified red
by defeating it alone in a scratch copy.
The Versioning section named the moving-major-tag model but not the release
step that model implies. This is the first change where that gap has a
consequence: `v1` and `origin/main` are the same commit today, so merging
the eviction fix is the first thing that makes them diverge — at which point
the fix is on `main` and every consumer is still fetching the old scripts.
Records what was previously only implicit: that merging ships nothing, that
re-pointing `v1` is a deliberate post-merge action because it changes what
another repository's CI runs next, the exact commands and the check that it
took, what a consumer is promised by pinning `@v1` and what forces a `v2`,
and who is actually downstream. Also states why the moving pointer is the
right model here rather than immutable release tags, since "safer in
general" is the obvious objection and it deserves an answer.
Docs only; no script or action definition is touched.
Both eviction sites checked for a consumer's `.reading-` marker and then,
seconds later — `usage_gb` runs `du -sk` over a multi-GB tree between the
two — unlinked the directory. A consumer that started a clone inside that
gap had its source removed mid-walk, which `cp -al` does not report: a
subtree unlinked before its parent is listed is silently omitted.
Unreachable today, and only by policy: snapshots belong to protected refs
and protected refs never reach the marker check. `cargo-cache` and
`cargo-cache-publish` take that ref list as two independent inputs, so a
workflow listing a publisher in one and not the other arms this with no
code change at all.
Closed structurally, with publish-snapshot.sh's rotation rather than a new
mechanism: the candidate is renamed aside and only then re-examined, so the
scan the unlink rests on happens strictly after the rename. A consumer that
resolved the directory published its marker before that scan and cannot be
missed; one arriving after cannot resolve the path and starts cold, the same
degrade the publisher's swap window already produces. Renaming disturbs no
clone in flight — no entry is unlinked and the inode is unchanged — so a
declined eviction costs a deferred eviction and nothing else. A reprieved
cache is put back under its own name; one whose name a concurrent seed has
retaken is left aside and swept by a later pass once its readers drain,
since nothing else globs a dotted name.
prune-cache-selftest gains three scenarios (21 -> 31 assertions). Scenario
12 is the one that bites: the `du` the pass runs on its candidate publishes
the marker, placing it strictly after the check and strictly before the
unlink. Against check-then-delete, 1-11 pass and 12 fails; breaking only
the second look and leaving the rename fails it too.
claude
