Adds scenario 11, which races a real hardlink_clone_into against a real,
concurrent publish-snapshot.sh republish of the exact snapshot being
cloned, and asserts the publisher's own log reports entering its
reader-drain wait. Deleting reader_lock_acquire left every suite green
at the identical 129 assertions (issue #10): the marker it writes is
what the publisher's wait_for_readers checks, and with it gone the
publisher sees zero readers on its first check and proceeds straight to
reclaiming the rotated generation, silently.
Kept as its own scenario rather than folded into 8a: 8a already pins
exactly one property (the identity check) for exactly one mutant, and
the suite's one-scenario-one-mutant diagonal across 8a to 8d and 10 is
deliberate.
The previous wording said the unshare pass aborts the clone "before the copy's
own exit status is ever consulted", which describes neither version. Unmutated,
the status is consulted immediately after the copy and fires first, so the
unshare pass is never reached; mutated, there is no check left to consult at
either point. As written a reader could take it for a claim that
unshare_mutable_paths runs before the torn-clone condition inside
hardlink_clone_into, which is the kind of ordering this file is otherwise
careful to state exactly (see the reader-marker ordering proof it sits under).
Names the mutation instead: deleting the exit-status check does not change the
outcome, because the unshare pass aborts the clone in its place. The mechanism
is unchanged and still holds — `cp -al` over a mode-000 source creates the
destination preserving mode 000 before failing, and `find | xargs` over that
returns 1 under pipefail, which _unshare_files propagates.
Review of #9 found the methodology section falsified by that PR and owned by
nobody — the scoping that fenced it off was wrong, #8 never touches these
paragraphs.
Three fixes:
* The PATH-stub paragraph named 8a and 8b as the seed suite's stubs. It is now
four scenarios on two commands: 8a/8b/8c stub `cp` at the clone, 10 stubs it
one level down at the per-file unshare, and 8d stubs `stat` — a mechanism
the paragraph did not mention at all, and the only way to make an identity
that could not be READ the sole witness.
* The assert-which-guard-fired paragraph cited issue #5 as a live example of a
surviving mutation. #5 is the issue this PR closes, so a reader following
that citation landed on "removing it leaves every suite green", which is no
longer true. Scenario 9's own sentence stands — the matrix confirms it
survives every mutant — so it now says WHY it survives (an unreadable source
leaves the staging dir at mode 000, and the unshare pass aborts the clone
before the copy's exit status is consulted) instead of citing a closed
issue.
* Added the mutual-masking hazard the sweep turned up, since it is the general
lesson rather than a fact about two particular terms: two guards that can
each catch the same fault make each other unnecessary, so no fixture built
around that fault pins either one.
Also names scenario 8d for what it is in its own comment — a regression guard
on a defensive term, not a reproduction of a reachable state. Every route to
the state it constructs is closed off (a rotation hands the witness to 8a, a
genuinely absent source hands it to 8c), which is the reason it is worth
pinning rather than a reason to doubt it.
hardlink_clone_into's torn-clone detection is a four-term condition, and a
mutation sweep found two of the four unpinned: removing either
`[ "$cp_rc" -eq 0 ]` (issue #5) or `[ "$i_before" != missing ]` left all five
suites green. They were unpinned for the same reason — they mask each other.
A source that vanishes mid-clone reads as `missing` at both ends AND fails
`cp -al`, so with both terms present either one catches it and neither is
individually necessary.
Isolating them needs a state each term alone can see:
8c cp reports failure over a tree that is in fact whole. Neither inference
sees anything — 0 entries short, one unchanged inode — so the exit
status is the only witness. Forced with the PATH stub 8a/8b already
use, on the consumer's own top-level clone.
8d both identity reads fail while the copy succeeds. `_dir_inode` folds
every stat failure into the string `missing`, so two failed reads
compare equal TO EACH OTHER; without the sentinel term the tree is
published on the strength of two errors. Stubs `stat` narrowly — only
the `%i` reads of this clone's own source — because taking the source
away would fail `cp -al` too and pin 8c's property over again.
The sweep also found `_unshare_files`'s xargs status unpinned, which is the
guard that stops a staging tree whose dep-info files still point at the
SOURCE's inodes from being renamed into place — not a tear, so all four
clone checks pass it, and exactly the silent cross-branch stale-reuse the
scheme exists to prevent. Scenario 10 pins it by refusing the dep-info
unshares and asserting the clone discards rather than publishes.
assert_tear gains an `unreadable` identity expectation, and its `same` case
now demands a READ identity rather than two equal strings — `missing` equals
`missing`, which is the exact confusion 8d exists to pin.
Each of the five pinning scenarios was run in isolation against each
mutation; the result is a clean diagonal, so every scenario fails only for
its own term.
Closes#5
Four corrections from the review of #8, all in the files this PR already
touches:
- A local signal on the line that creates .publish-new-. The other four shapes
each got a note at their producing line, which is the whole premise of #7 —
someone renaming TMP_DST reads its own comment block and would never see the
contract note 45 lines up at OLD.
- '.publish-old- is milder' understated it. Milder is true; bounded is not. A
key whose branch is merged, deleted or renamed is never published again, so
its rotated generation stays until something outside this repo takes it —
which is the case gitdan#30 itself makes, two lines away.
- The drift check now says the prefix constants ci-cache-reclaim.sh DECLARES,
not the ones it enumerates. Those are different numbers: collect_entries()
globs .stage- and .evicting- only, because .reading- is read and never
swept. Only the declared reading makes the five-against-five count work, and
a countability check that needs a coin flip to count is not one.
- publish-snapshot.sh's own header had two stale names ten lines above the
stale pointer this PR fixes: step 1 staged at .stage-<tag> (that is
hardlink_clone_into's inner path; the staged snapshot is .publish-new-<tag>)
and step 2 named .publish-old-<tag> without the key. Pre-existing and
outside both ACs, but #6's thesis is that a plausible-looking wrong name is
the worst kind, and these are in the file the PR is about.
Comments and docs only. With comments and blank lines stripped, all three
scripts hash identically to origin/main.
The contract block asserted that neither side reclaims .publish-new- 'today'.
True when written and about to stop being true: gitdan#30 tracks adding both
.publish-* prefixes to the arbiter's enumeration, and a sibling track is
landing it this round. A comment that dates itself against a merge in flight
is worse than no comment.
Rewords all four sites (cache-lib.sh, publish-snapshot.sh, and README's table
row and prose) to reference gitdan#30 and keep the mechanism that made the
shape worth catching — .publish-new- is tagged per job per run exactly as
.stage- is — rather than the arbiter's momentary contents. The rule itself is
unchanged; it is the durable part, and it is what found this.
Adds the counting check while there: the five names here and the prefixes
ci-cache-reclaim.sh enumerates are meant to be the same length, so a mismatch
is the cheapest signal that one side gained a shape without telling the other.
daniel/gitdan's host-level arbiter (scripts/ci-cache-reclaim.sh) reclaims the
dot-prefixed trees this repo's scripts strand inside the cache volumes it
scans, and reads this repo's reader markers to decide whether one is still
live. That arrangement was documented only on the consuming side: a
contributor here could add or rename a dot-prefixed shape with no local signal
that anything outside the repo depended on the spelling, and the arbiter
enumerates by explicit prefix — deliberately, so it never sees a .reading-*
marker as a candidate — which makes an unannounced shape invisible to it
rather than conservatively handled.
Adds the producing side's half at the sites someone changing a name will
actually be looking at, pointing at gitdan's LEFTOVER NAMING CONTRACT block as
canonical rather than restating it:
- cache-lib.sh gains a header block naming every shape this repo creates under
a cache root, its producing function, and how each strands; plus the rule
that adding a shape obliges the same matching prefix over there as renaming
one does.
- Site notes at .stage-'s and .reading-'s producing lines, and at
.evicting-'s in prune-cache.sh.
- publish-snapshot.sh's .publish-old- / .publish-new- pair is documented as
the shapes that are NOT in the arbiter's list today, with .publish-new-
called out as the one that strands exactly as .stage- does and that neither
side reclaims.
- The staleness direction: CACHE_READ_STALE_SECONDS and STALE_LOCK_SECONDS are
mirrored there and the mirrors must be >= ours, because raising ours alone
makes the arbiter delete a tree under an in-flight clone (its minimum-age
guard does not back-stop that case). Lowering ours is safe in any order.
- README gains a short section a newcomer meets before adding a scratch
directory under a cache root, cross-linked from the cache-layout block.
Comments and docs only; no behaviour change.
Closes#7
publish-snapshot.sh's header described the silent-truncation failure mode as
the one "this script's own selftest (scenario 8)" reproduces. Wrong twice:
the scenario that reproduces a truncated clone is in
seed-target-dir-selftest.sh, and since #4 split the old scenario 8 into 8a and
8b it is 8b. publish-snapshot-selftest.sh does have a scenario 8 — the
abandoned-marker case — so the pointer landed on a real scenario with a
plausible number that tests something else.
Names the suite as well as the number, and says what the local scenario 8
actually is so the collision cannot re-form.
Closes#6
Three corrections to the methodology paragraph, all of the same kind: it
stated as fact things that hold for some scenarios and not others.
"Run the real scripts as real concurrent processes" is true of seed scenario
7 and half-true of 8a; 8b, prune 12 and publish 6-8 spawn nothing. A reader
who stopped at that topic sentence would take away "spawn real concurrent
processes", which is the instinct that produced #3. The paragraph now leads
with the three shapes actually in use and says which scenarios take each:
a genuine race whose invariant holds under any interleaving; a PATH stub that
places the interference inside the window; and a synthetic stand-in for the
other side where the artefact is itself the contract.
That third shape — publish-snapshot-selftest.sh's held .reading-* marker —
was unmentioned, so the README implied that suite stubs something it does
not. Its own defence (the marker IS the contract between the two sides, and
racing a real slow consumer would make the suite's runtime the thing under
test) is a good reason and now sits beside the other two.
"Where more than one guard could catch a fault, the scenario asserts which
one did" was written as description when it is a target: seed scenario 9 does
not, which is exactly why #5's mutation survives it. Stated as the rule plus
its one live exception — a rule asserted as fact with a known counterexample
is the same defect as the sentence this paragraph replaced.
README's Development section stated the repo's methodology for writing
concurrency scenarios as "gate the interfering step on observed progress of
the step it interferes with, so the window is hit deterministically". That
described the progress poll scenario 8 used, which this branch removes — and
the property it claims is precisely what issue #3 records as false: observing
that a walk has started says nothing about where it will be when the
interference lands.
Left standing it would tell the next contributor to build the next scenario
the way this one had to be rewritten. Replaced with what the suites do: stub,
on PATH, a command the code under test calls at a known point, so placement is
a fact rather than a scheduling outcome; assert the stub fired; and assert
which guard caught the fault where more than one could.
Also: the seed suite's table row now names both tear modes, and
publish-snapshot-selftest.sh's cross-reference points at 8a and 8b rather than
a scenario 8 that no longer exists. (publish-snapshot.sh's similar mislabel
predates this branch and is left alone.)
The stub directory and the real-cp lookup move up next to seed_with_stub, so
8b no longer depends on setup buried in 8a's block and either scenario can be
run or mutated alone.
Scenario 8 started a real publisher once a consumer's clone was observed
past a fraction of the tree, then asserted the consumer ended up holding
the NEW generation. That premise is racy: when the publish lands after the
consumer's last identity read, the consumer legitimately completes a whole,
consistent generation 1 — and the assertion called that "the seeded tree is
truncated". The one gate this repo has was failing intermittently in the
most alarming direction available.
Replaced with two scenarios that place the interference deterministically,
the way prune-cache-selftest.sh scenario 12 places a marker inside the
check-to-unlink window: the consumer's own `cp` is stubbed, so whatever the
stub does happens strictly after hardlink_clone_into read the source's
entry count and inode and strictly before it reads that inode again.
8a the stub runs the real copy, then the real publish-snapshot.sh
concurrently, and returns only once the swap is on disk. The copy
succeeds and the staging tree is whole, so the source's identity is
the only witness.
8b the stub renames a subtree out of the source's listing before the
walk starts and back before the retry. The copy exits 0 and the
source's identity never changes, so the entry count is the only
witness — the silent truncation this whole guard exists for.
Each asserts WHICH of the clone's checks reported the tear (assert_tear),
so neither stays green if the check it exercises is deleted and another
happens to fire in its place.
Closes#3
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.
The staleness path decides whether a publisher may reclaim disk, so getting it
wrong means an abandoned marker pins a snapshot generation forever — the exact
outcome the bound exists to prevent. It was previously covered only by
analogy to prune-cache.sh's .ci-lock-* staleness, which is not the bar.
New publish-snapshot-selftest.sh scenario 8 asserts both directions against
the SAME backdated marker, which is what separates "honours the bound" from
"ignores anything that looks old":
* under CACHE_READ_STALE_SECONDS=86400 a three-hour-old marker is left alone
and still defers reclamation, exactly as a live reader does;
* under the 7200s default the same marker is swept, reported as swept, and the
generation it was pinning — plus the one deferred by the first half — is
reclaimed.
Backdated with `touch -d`, not slept for; the suite stays fast.
Red-proven by mutation rather than against the pre-fix scripts, since the
whole mechanism is new there and "it does not exist yet" proves nothing about
the threshold logic. Mutating live_reader_count's bound test to `true` (never
sweep) fails scenario 8:
ASSERTION FAILED: the stale marker was not reported as swept
and to `false` (sweep everything, bound ignored) fails scenario 6 instead,
which is the right blast radius — ignoring the bound means unlinking under a
LIVE reader:
ASSERTION FAILED: the previous generation was unlinked while a reader
still held it
Also documents the entry-count check's measured cost in the README: on ext4
with a warm cache over 78,554 entries, 44 ms per metadata walk against 3,126 ms
for the `cp -al` it guards — about 2.8%. Not a perf-claiming change; the number
is there so the next reader does not have to wonder.
The reviewer's other nit — scenarios 8/9 of the seed suite exercising
publish-snapshot.sh's interlock — was already covered by the cross-reference
in this file's header, so no move.
Verification: `bash scripts/selftest.sh` — 5 suites, exit 0, 88 assertions
(82 before this commit, 63 at baseline). shellcheck: no new findings.
Refs: daniel/gitdan#11, zemyna#911
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L
Follow-up to f57e2a6, addressing the reviewer's sharpest question: is the race
"closed by construction", or merely detected and retried? The honest answer is
"both, on different paths", and the README said only the first half.
* README now states three claims separately instead of collapsing them:
a publisher rotating a snapshot cannot tear a clone of it (by construction —
the marker ordering prevents the unlink, and the consumer's verification is
a redundant second check on that path); every OTHER way the source can
change mid-clone is detected, not prevented (the eviction pass's reader
check is check-then-delete, and a seed-fallback-dir has no interlock at all
— there, verification plus a bounded retry and a loud failure is the whole
guard); and disk reclamation is bounded rather than immediate. Overclaiming
this property once was the finding; overclaiming it twice would be worse.
* publish-snapshot-selftest.sh now covers the PUBLISHER's half of the race,
where a reader of the swap belongs. Scenario 3 only ever covered a consumer
that had already FINISHED cloning — safe for free, since its own hardlinks
keep the inodes alive. New scenario 6 covers a reader still in flight past
the grace period (the generation is left on disk, the deferral is warned
about, and an earlier consumer is still unaffected); scenario 7 covers the
sweep, so "we defer instead of forcing" cannot quietly become a disk leak.
Red-proven against 248af306's scripts:
ASSERTION FAILED: the previous generation was unlinked while a reader
still held it
The consumer's half stays in seed-target-dir-selftest.sh scenario 8, which
still red-proves at 16693 of 48805 entries against the same scripts.
* seed-target-dir-selftest.sh now asserts what happens when the retries are
EXHAUSTED, not just what hardlink_clone_into returns: an unreadable source
makes the seed script exit non-zero, name the reason, leave no target dir,
and — the one that matters — not fall through to its cold-start branch. A
corrupt-cache bug degrading into an invisible 4x-slower CI job is the
failure mode worth pinning down. Skipped when running as root, where mode
bits deny nothing.
* usage_kb: a directory we cannot read measured as the empty string, which was
then spliced into usage_gb's awk program and made it a syntax error at the
exact moment something was already going wrong. Now measures 0.
Verification: `bash scripts/selftest.sh` — 5 suites, exit 0, 82 assertions
(was 75 after f57e2a6, 63 before). shellcheck over scripts/: no new findings.
Measured the cost the reviewer asked about, on ext4, warm cache, 78,554
entries: `cp -al` 3126 ms against 44 ms for one `find | wc -l`. Two counts per
attempt is ~2.8% on top of the clone. Not measured on the CI runner's volume.
Refs: daniel/gitdan#11, zemyna#911
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L
The shared action's justification over zemyna's and emowheel's schemes was
that hardlink-cloning from a published snapshot closes gitdan #911 "by
construction, not by the single job slot". Review disproved that. This makes
the claim true, and corrects the README where it could only be bounded.
Finding 1 (verdict-level) — silent partial clone
------------------------------------------------
`hardlink_clone_into` ran `cp -al` with no exit-status check, and both call
sites invoked it as a condition, which suppresses `set -e` for the whole call.
A publisher's `rm -rf` of the generation it rotated away therefore unlinked
entries beneath an in-flight consumer walk, and the truncated tree was renamed
into place and reported as success.
Both layers are fixed:
* The consumer verifies its own clone. Every attempt checks `cp -al`'s status
explicitly, the source directory's inode before and after (a wholesale
replacement mid-walk splices two generations), and the entry count — the
only signal for a subtree unlinked before its parent was listed, since
`cp -al` reports no error for one it never saw. Any failure discards the
staging tree and retries; exhausting the attempts returns a distinct status
2 and fails the job rather than seeding a partial cache. `unshare_subtree` /
`_unshare_files` now propagate failure too — a swallowed unshare leaves the
clone aliasing its source, the exact corruption that step exists to prevent.
* The publisher does not unlink under a reader. A consumer publishes a
`.reading-<snapshot>-<tag>` marker before it resolves the snapshot path; the
publisher scans for markers after its first rename. A consumer holding the
old generation therefore published its marker before that scan and cannot be
missed; one arriving after the scan necessarily resolves to the new
generation. The publisher waits for readers to drain and, on timeout,
DEFERS reclamation rather than forcing it — the old generation is left as
`.publish-old-<key>-<tag>` and swept by a later publish.
So correctness is closed by construction; disk reclamation is bounded, not
immediate. The residual is capped at one deferred generation per publisher
ref, and the README now says exactly that instead of the disproved claim.
Finding 2 — restore-mtimes.sh ran with no errexit
-------------------------------------------------
`set -euo pipefail` was glued to the end of a comment (`# soundness.set -euo
pipefail`), so it was entirely commented out: a partial failure of the
`git log | awk` pipeline would have produced wrong mtimes across the whole
restore instead of failing loudly. Moved to its own line. Audited every other
script for the same defect — this was the only instance. Independent
confirmation: shellcheck's two SC2164 warnings on this file's `cd "$repo_root"`
disappear now that errexit is actually in effect.
Finding 3 — lock-acquire window
-------------------------------
A just-seeded directory was unlocked until a later action step, so a
concurrent job's prune pass could evict it. `seed-target-dir.sh` now takes an
optional lock-id and writes the lock marker on every path out of the script,
including into the staging tree before its rename, so the directory carries a
lock the instant it appears under its final name. The action's acquire step
stays (it is idempotent and stamps the LRU marker).
Also hardened `prune-cache.sh` to treat a directory with live reader markers
as locked. Today no reachable configuration prunes a snapshot — only protected
refs publish them and protected refs are excluded from every pass — so this is
redundant by policy; it is here so that stops being the reason it is safe.
Verification
------------
New selftest scenario 8 races a real seed against a real publish rotation,
gating the rotation on the seed's *observed* clone progress so the window is
hit deterministically rather than on a fast machine's coin flip. Red-proven
against the unguarded scripts, three consecutive runs:
ASSERTION FAILED: the seeded tree is truncated: 15443 entries against the
snapshot's 493 (was 48805 before the rotation) (15443 / 16986 / 16498)
Green after the fix, six consecutive runs, catching the clone mid-walk at
~10.5k of 48805 entries each time. Scenario 9 covers deferred reclamation and
its later sweep; scenario 10 covers an unreadable source failing loudly.
`bash scripts/selftest.sh`: 5 suites, exit 0, 75 assertions (was 63).
shellcheck over `scripts/`: no new findings, two SC2164 warnings resolved.
Docs: README's republish-safety paragraph replaced with what the code now
guarantees, including the bounded disk residual stated explicitly; new
`read-grace-seconds` / `reader-stale-seconds` inputs documented in the
`cargo-cache-publish` table; the selftest table names the new race.
Refs: daniel/gitdan#11, zemyna#911
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L
Replaces the phase-0 resolution probe with the real actions, merging the two
independent per-branch Cargo cache implementations on this forge into the
design neither of them had.
## The merge
- zemyna seeds a PR branch by `cp -al` hardlink clone (near-free: cost scales
with inode count, not bytes) from the base branch's LIVE target dir — a
torn read waiting for a second job slot (its own #911).
- emowheel seeds from a PUBLISHED IMMUTABLE SNAPSHOT (no race by
construction) but with `cp -a`, duplicating ~35 GB per branch.
This ships hardlink-clone FROM a published snapshot: zemyna's cost profile,
emowheel's soundness, and #911 closed structurally rather than by the runner
happening to have one execution slot.
## The bug both implementations have
A build inside a `cp -al` clone DOES mutate the directory it was cloned from.
Cargo replaces real artifacts, but writes its metadata — and build scripts
write their OUT_DIR — with a plain truncating write, straight through the
shared inode. Measured set: `.fingerprint/<unit>/dep-<target>` (under
CARGO_UNSTABLE_CHECKSUM_FRESHNESS), `build/<pkg>/{output,root-output,out/**}`,
`deps/*.d` and `<profile>/*.d`.
The checksum-freshness case is a wrong answer, not a slow build: a PR clone
rewrites the base's dep-info to describe the PR's sources while the base's
cache still holds the artifact built from the base's; once the PR merges, the
base's next run finds the checksums match, reports `Fresh`, and links a binary
built from the pre-merge code. Reproduced end to end.
Fix: hardlink the artifacts (the GB), real-copy the metadata (the MB) — about
3.7% of a 6.9 GB Bevy target dir, against 100% for a full copy.
## Contents
- `cargo-cache/action.yml` — consume: resolve keys, seed from the base's
snapshot via staging + one atomic rename, strip Cargo lock files, unshare
the mutable paths, restore mtimes from git history, lock, prune.
- `cargo-cache-publish/action.yml` — publish: record the build watermark,
atomically republish the snapshot on a protected branch, release the lock
(`mode: release-lock` for the `if: always()` step).
- `scripts/` — all logic, so it is testable standalone; the YAML is wiring.
- `scripts/*selftest.sh` + `selftest.sh` — five suites, 63 assertions, every
fix paired with a control that reproduces the bug. All green locally.
Eviction merges emowheel's liveness pass (dead branches pruned
unconditionally, not gated on disk pressure) with LRU-under-pressure, but
inverts the order within the pressure pass: `target-*` before `snapshot-*`,
because a snapshot is hardlinked to everything cloned from it, so evicting one
frees almost no real bytes while costing every future PR its warm start.
restore-mtimes.sh is ported from emowheel (the watermark variant, which closes
the merge hazard zemyna's copy still has) with its provenance de-projectised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L
claude
