seed-target-dir-selftest scenario 8 is flaky: it asserts the rotation tore the clone, which is not guaranteed #3

New Issue

2026-08-24T04:45:05Z

claude commented

2026-08-24 04:45:05 +00:00

Surfaced during the review of
#2, and not attributable
to that PR — seed-target-dir-selftest.sh, seed-target-dir.sh and
cache-lib.sh are untouched by it.

Symptom

bash scripts/selftest.sh failed once, at scenario 8:

=== 8: seeding while the base republishes the snapshot underneath it ===
PASS: caught the consumer's clone mid-walk at 10372/48805 entries
PASS: the seed completed
ASSERTION FAILED: the seeded tree is truncated: 48805 entries against the
snapshot's 493 (was 48805 before the rotation)

Not reproducible on demand: 6/6 clean in isolation and 6/6 under saturating
CPU load, on both main and the PR branch, plus clean full-suite runs on both
afterwards.

Diagnosis: the test's premise is racy, not the code under test

Scenario 8 rotates a snapshot out from under a consumer that is mid-clone, and
then asserts the consumer ends up holding the new generation. That only
holds if the rotation actually tears the walk.

When the reader-marker interlock does its job — the publisher defers its
unlink until the consumer's marker clears, or the publish simply lands after
the consumer's final inode check — the consumer legitimately completes a
whole, consistent generation 1. The test calls that outcome "truncated".

So the failing run is the interlock working, reported as a defect. The
assertion encodes "the rotation must have interfered" as though it were
guaranteed, when the design's actual guarantee is the weaker and more useful
one: whatever generation the consumer ends up with is internally consistent.

Why it is worth fixing rather than tolerating

scripts/selftest.sh is this repo's only gate — there is no CI here. A
flaky assertion in the only gate trains everyone to re-run it, which is
exactly how a real failure gets waved through. It also fails in the direction
that looks alarming ("the seeded tree is truncated"), so the cost of the false
alarm is high.

Ask

Make the assertion match the guarantee. Either:

assert internal consistency of whichever generation the consumer ends up
with — entry count equals that generation's, no partial subtree — rather
than requiring it to be the new one; or
force the interleaving deterministically the way
prune-cache-selftest.sh's scenario 12 does (it stubs du so the pass's own
measurement publishes the marker, placing it exactly in the window), so the
tear is guaranteed rather than hoped for and the strong assertion becomes
legitimate.

The second is stronger if the interleaving can be forced; the first is honest
if it cannot.

Acceptance criteria

Scenario 8 passes deterministically — 20 consecutive runs clean,
including under CPU load.
Whichever shape is chosen, the assertion still fails if the interlock is
removed. Demonstrate the red/green rather than asserting it.
The scenario's comment states which property it actually pins.

Surfaced during the review of [#2](https://gitdan.com/daniel/gitdan-actions/pulls/2), and **not attributable to that PR** — `seed-target-dir-selftest.sh`, `seed-target-dir.sh` and `cache-lib.sh` are untouched by it. ## Symptom `bash scripts/selftest.sh` failed once, at scenario 8: ``` === 8: seeding while the base republishes the snapshot underneath it === PASS: caught the consumer's clone mid-walk at 10372/48805 entries PASS: the seed completed ASSERTION FAILED: the seeded tree is truncated: 48805 entries against the snapshot's 493 (was 48805 before the rotation) ``` Not reproducible on demand: 6/6 clean in isolation and 6/6 under saturating CPU load, on both `main` and the PR branch, plus clean full-suite runs on both afterwards. ## Diagnosis: the test's premise is racy, not the code under test Scenario 8 rotates a snapshot out from under a consumer that is mid-clone, and then asserts the consumer ends up holding the **new** generation. That only holds if the rotation actually tears the walk. When the reader-marker interlock does its job — the publisher defers its unlink until the consumer's marker clears, or the publish simply lands after the consumer's final inode check — the consumer legitimately completes a **whole, consistent generation 1**. The test calls that outcome "truncated". So the failing run is the interlock working, reported as a defect. The assertion encodes "the rotation must have interfered" as though it were guaranteed, when the design's actual guarantee is the weaker and more useful one: whatever generation the consumer ends up with is internally consistent. ## Why it is worth fixing rather than tolerating `scripts/selftest.sh` is this repo's **only** gate — there is no CI here. A flaky assertion in the only gate trains everyone to re-run it, which is exactly how a real failure gets waved through. It also fails in the direction that looks alarming ("the seeded tree is truncated"), so the cost of the false alarm is high. ## Ask Make the assertion match the guarantee. Either: - assert **internal consistency** of whichever generation the consumer ends up with — entry count equals *that* generation's, no partial subtree — rather than requiring it to be the new one; or - force the interleaving deterministically the way `prune-cache-selftest.sh`'s scenario 12 does (it stubs `du` so the pass's own measurement publishes the marker, placing it exactly in the window), so the tear is guaranteed rather than hoped for and the strong assertion becomes legitimate. The second is stronger if the interleaving can be forced; the first is honest if it cannot. ## Acceptance criteria - [ ] Scenario 8 passes deterministically — 20 consecutive runs clean, including under CPU load. - [ ] Whichever shape is chosen, the assertion still fails if the interlock is removed. Demonstrate the red/green rather than asserting it. - [ ] The scenario's comment states which property it actually pins.

claude added the bug label 2026-08-24 04:45:05 +00:00

claude-reviewer referenced this issue

2026-08-24 04:51:13 +00:00

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

claude referenced this issue from a commit

2026-08-24 16:14:06 +00:00

test(seed): force scenario 8's interleaving instead of racing for it

claude referenced a pull request that will close this issue

2026-08-24 16:15:56 +00:00

test(seed): force scenario 8's interleaving instead of racing for it #4

claude-reviewer referenced this issue

2026-08-24 16:44:34 +00:00