eb7878b82282eb89837e2fa7baedba4e93327d5f
15
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 claude
|
eb7878b822
|
docs(readme): correct the scenario census and the #5 citation
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. |
||
|
claude
|
bd60b430e0
|
test(seed): pin the two unguarded terms of the torn-clone condition
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 |
||
|
claude
|
0ee21bfbc8
|
docs(readme): census the three concurrency-scenario shapes honestly
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. |
||
|
claude
|
5551e994da
|
docs(readme): describe how the concurrency scenarios actually work now
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. |
||
|
claude
|
dc0fef71f8
|
test(seed): force scenario 8's interleaving instead of racing for it
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
|
||
|
claude
|
b0d63c807b
|
style(prune-cache): rewrap the settle-window comment; drop an overclaim
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. |
||
|
claude
|
dcd73dd82d
|
test(prune-cache): build scenario 14's aside the way the pass builds one
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. |
||
|
claude
|
65f0782233
|
fix(prune-cache): stop the aside sweeper depending on timing it cannot see
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. |
||
|
claude
|
c6a3fa6d97
|
docs(readme): say who moves the v1 tag, when, and what it promises
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. |
||
|
claude
|
88ab063b64
|
fix(prune-cache): close the reader-marker check-then-delete window
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. |
||
 claudeandClaude Opus 5
|
3b2dec6a50
|
test(cargo-cache): cover the stale-reader-marker sweep and its bound
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
|
||
|
claudeandClaude Opus 5
|
719475831b
|
fix(cargo-cache): scope the safety claim to what the code actually prevents
Follow-up to |
||
|
claudeandClaude Opus 5
|
f57e2a6013
|
fix(cargo-cache): close the seed-vs-republish race the design claimed to close
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 |
||
|
claudeandClaude Opus 5
|
248af3061e
|
feat(cargo-cache): hardlink-clone a per-ref Cargo cache from a published snapshot
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
|
8503883138
|
feat: cargo-cache composite action skeleton (phase-0 resolution probe) |