Merge pull request 'test(seed): force scenario 8's interleaving instead of racing for it' (#4) from fix/seed-selftest-flake into main

README.md

@@ -361,17 +361,52 @@ bash scripts/selftest.sh --fast   # fixture-only suites, no compiler
 | suite | covers |
 |---|---|
 | `hardlink-clone-selftest.sh` | that a build in a clone cannot mutate its source — with a control proving a raw `cp -al` does. Needs a real compiler. |
-| `seed-target-dir-selftest.sh` | seed-source preference, lock-file stripping, two jobs racing on one cache key, **and a seed racing a publisher's rotation of the source it is reading** — the race that actually truncates a tree |
+| `seed-target-dir-selftest.sh` | seed-source preference, lock-file stripping, two jobs racing on one cache key, **and a seed whose source is rotated — or silently loses a subtree — underneath its clone**: the two ways a hardlink clone tears |
 | `publish-snapshot-selftest.sh` | the atomic swap, that a live consumer survives a republish, and the publisher's side of the rotation race: deferred reclamation under a live reader, and its sweep once the reader is gone |
 | `prune-cache-selftest.sh` | liveness, protection, locking, eviction order, self-clear, **and that a cache a job claims *inside* the check-to-unlink window survives it** — against a real scratch `origin` |
 | `restore-mtimes-selftest.sh` | the merge hazard and the watermark that closes it, including the two-jobs-one-namespace case. Needs a real compiler. |
 
 Every suite runs the actual script, not a reimplementation of its logic, and
 every fix scenario is paired with a control that reproduces the bug — a
-scenario that passes either way proves nothing. The concurrency scenarios race
+scenario that passes either way proves nothing.
-real processes rather than mocking the interleaving, and gate the interfering
+
-step on *observed* progress of the step it interferes with, so the window is
+The concurrency scenarios take one of three shapes, and none of them races
-hit deterministically instead of on a fast machine's coin flip.
+for the interleaving its assertion depends on.
+
+**A genuine race whose asserted invariant holds under any interleaving.**
+`seed-target-dir-selftest.sh` scenario 7 starts two real seeds on one cache
+key and asserts only what must be true whichever of them wins the rename.
+
+**A `PATH` stub on a command the code under test calls at a known point**,
+which places the interference inside the window rather than hoping it lands
+there. `prune-cache-selftest.sh` scenario 12 stubs `du`, so the pass's own
+measurement publishes a reader marker strictly between its check and its
+unlink; `seed-target-dir-selftest.sh` scenarios 8a and 8b stub `cp`, so the
+consumer's own clone is what rotates the snapshot underneath it, or what
+loses a subtree of its own source, strictly inside the identity window. 8a
+does start a second real process — the actual `publish-snapshot.sh` — but the
+stub is what fixes where its swap lands; the concurrency is incidental to the
+determinism. Every stub asserts that it fired, because a scenario whose
+interference silently did not happen passes for the wrong reason.
+
+**A synthetic stand-in for the other side, where that artefact *is* the
+contract.** `publish-snapshot-selftest.sh` scenarios 6 to 8 hold a
+`.reading-*` marker instead of running a slow consumer: the marker is the
+whole agreement between reader and publisher, so holding one is being a
+reader, and racing a real one would make the suite's runtime the thing under
+test.
+
+Gating the interfering step on *observed progress* of the step it interferes
+with was an earlier answer here, and it is not one: seeing that a walk has
+started says nothing about where it will be when the interference lands, so
+the assertion downstream held only some of the time (issue #3). No scenario
+does it any more.
+
+Where more than one guard could catch a fault, a scenario should assert
+*which* one did — otherwise deleting the guard under test leaves the suite
+green because a sibling fires in its place. Scenarios 8a and 8b of the seed
+suite do; scenario 9 of the same suite does not yet, which is why a mutation
+survives it (issue #5).
 
 The action YAML holds no logic beyond wiring; everything testable lives in
 `scripts/`. A composite action needs `shell: bash` on every `run:` step, and

scripts/publish-snapshot-selftest.sh

@@ -41,7 +41,7 @@
 #      that marker must still defer.
 #
 # The consumer's half of the same race — a seed catching a rotation mid-clone
-# — is in seed-target-dir-selftest.sh scenario 8.
+# — is in seed-target-dir-selftest.sh scenarios 8a and 8b.
 set -euo pipefail
 script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 . "$script_dir/cache-lib.sh"

scripts/seed-target-dir-selftest.sh

@@ -29,14 +29,24 @@
 #      at no point is a partially-populated directory visible under the final
 #      name. This is the property that replaces "the runner only has one job
 #      slot" with an actual guarantee.
-#   8. SEED VS PUBLISH ROTATION — the race scenario 7 does NOT cover, and the
+#  8a. SEED VS PUBLISH ROTATION — the race scenario 7 does NOT cover, and the
 #      one that actually mattered: a consumer hardlink-cloning a snapshot
-#      while the publisher of that snapshot rotates it and unlinks the
+#      while the publisher of that snapshot rotates it. Two seeds racing on a
-#      generation being read. Two seeds racing on a DESTINATION is a different
+#      DESTINATION is a different race from a seed racing a publisher on its
-#      race from a seed racing a publisher on its SOURCE, and only the second
+#      SOURCE, and only the second one can truncate a tree. What it pins is
-#      one can truncate a tree. Against the unguarded version this scenario
+#      that the consumer notices its source was REPLACED and re-clones,
-#      reproduces a silent partial clone reported as success — 20,328 of
+#      ending up with the whole generation now published — not that the
-#      48,805 entries, `seed: cloned in 1s`, exit 0, seeded-from=base-snapshot.
+#      rotation interfered with the copy, which is not guaranteed and is what
+#      made the racing version of this scenario flaky (issue #3).
+#  8b. AND A SILENTLY TRUNCATED WALK — the same tear seen through the other
+#      check: a subtree unlinked out of the parent's listing before `cp -al`
+#      reads it is never visited, the copy exits 0 and the source's identity
+#      never changes. Against the unguarded version this is a partial tree
+#      renamed into place and reported as a success — 20,328 of 48,805
+#      entries, `seed: cloned in 1s`, exit 0, seeded-from=base-snapshot.
+#      Both scenarios force their interleaving rather than racing for it, and
+#      each asserts WHICH check caught the tear, so neither stays green if
+#      the check it exercises is removed.
 #   9. AN UNREADABLE SOURCE FAILS LOUDLY — the clone reports a distinct status
 #      instead of renaming whatever it managed to produce into place, and the
 #      seed SCRIPT turns that status into a failed job rather than a silent
@@ -75,9 +85,17 @@ make_tree() {
 
 seed() { bash "$script_dir/seed-target-dir.sh" "$@" > "$scratch/log" 2>&1 || { tail -40 "$scratch/log"; fail "seed-target-dir.sh exited non-zero"; }; }
 
-# Always succeeds and always prints a number: an absent directory is "0
+# The same script, with the stub directory scenarios 8a and 8b write into
-# entries so far", which is the normal state at the top of the progress poll
+# ahead of the real coreutils on PATH. Kept separate so no other scenario can
-# below, not an error worth aborting the suite over.
+# pick a stub up by accident, and so the caller keeps the exit status instead
+# of aborting on it. Both halves live here rather than in the first scenario
+# that needs them, so either scenario can be run, moved or mutated alone.
+mkdir -p "$scratch/bin"
+real_cp=$(command -v cp)
+seed_with_stub() { PATH="$scratch/bin:$PATH" bash "$script_dir/seed-target-dir.sh" "$@"; }
+
+# Always succeeds and always prints a number: a directory that does not exist
+# is 0 entries, not an error worth aborting the suite over.
 tree_entries() {
   local n
   n=$(find "$1" -mindepth 1 2>/dev/null | wc -l) || n=0
@@ -85,12 +103,56 @@ tree_entries() {
   return 0
 }
 
-# A tree wide enough that a hardlink clone of it takes long enough to be
+# Waits for a file a concurrently running script will create. The publisher in
-# caught mid-walk. The race under test is a real interleaving, not a mocked
+# scenario 8a is started from inside the consumer's process tree rather than
-# one, so the fixture has to be big enough for the window to exist: a
+# by this script, so its completion cannot be waited on as a job.
-# four-file tree clones in microseconds and no scheduling could ever land
+wait_for_file() {
-# inside it. Built by cloning one small template directory N times, which is N
+  local path="$1" what="$2" deadline
-# forks rather than N*M file creations.
+  deadline=$(( $(date +%s) + 60 ))
+  until [ -e "$path" ]; do
+    [ "$(date +%s)" -lt "$deadline" ] || fail "$what"
+    sleep 0.1
+  done
+}
+
+# Reads the clone's own torn-read report back and asserts WHICH of its three
+# checks fired: the copy's exit status, the entry count, or the source's
+# identity. Scenarios 8a and 8b each force exactly one of the three, and a
+# scenario that only asserted "some tear was reported" would stay green if
+# the check it exercises were deleted and one of the others happened to fire
+# in its place.
+#
+#   assert_tear <log> <cp-status> <same|short> <same|differs> <message>
+assert_tear() {
+  local log="$1" want_rc="$2" want_count="$3" want_inode="$4" msg="$5" line
+  local re='cp rc=([0-9]+), ([0-9]+)/([0-9]+) entries, source inode ([0-9]+) -> ([0-9]+)'
+  # `|| line=""` rather than a bare assignment: no match makes grep exit 1,
+  # which under `set -e` would abort the suite with no message at all — the
+  # exact case this assertion exists to report.
+  line=$(grep -o 'was torn ([^)]*)' "$log" | head -1) || line=""
+  [ -n "$line" ] || { tail -20 "$log"; fail "no torn read was reported at all ($msg)"; }
+  [[ $line =~ $re ]] || fail "unrecognised torn-read report: ${line}"
+  [ "${BASH_REMATCH[1]}" = "$want_rc" ] || fail "expected cp to exit ${want_rc}: ${line}"
+  case "$want_count" in
+    same)  [ "${BASH_REMATCH[2]}" -eq "${BASH_REMATCH[3]}" ] || fail "expected a whole staging tree: ${line}" ;;
+    short) [ "${BASH_REMATCH[2]}" -lt "${BASH_REMATCH[3]}" ] || fail "expected a short staging tree: ${line}" ;;
+    *) fail "assert_tear: bad entry-count expectation '${want_count}'" ;;
+  esac
+  case "$want_inode" in
+    same)    [ "${BASH_REMATCH[4]}" = "${BASH_REMATCH[5]}" ] || fail "expected the source's identity to hold: ${line}" ;;
+    differs) [ "${BASH_REMATCH[4]}" != "${BASH_REMATCH[5]}" ] || fail "expected the source's identity to change: ${line}" ;;
+    *) fail "assert_tear: bad identity expectation '${want_inode}'" ;;
+  esac
+  ok "$msg (${line})"
+}
+
+# A tree of many sibling subtrees, built by cloning one small template
+# directory N times — N forks rather than N*M file creations. The shape is
+# what scenarios 8a and 8b need: two generations of DIFFERENT entry count, so
+# an assertion can tell which one a consumer ended up with, and enough
+# sibling subtrees that removing one moves the count. Size no longer has to
+# buy a race window — both scenarios force their interleaving — so it stays
+# small enough to be free.
 make_wide_tree() {
   local d="$1" marker="$2" ndirs="$3" i
   mkdir -p "$d/debug/deps/.tmpl" "$d/debug/.fingerprint/x"
@@ -170,62 +232,147 @@ fi
 ok "neither racing job fell through to a cold start"
 
 echo
-echo "=== 8: seeding while the base republishes the snapshot underneath it ==="
+echo "=== 8a: a rotation landing inside the clone's identity window ==="
+# The race scenario 7 does NOT cover: a seed racing a publisher on its SOURCE
+# rather than two seeds racing on a DESTINATION. What this pins is that a
+# consumer whose source is REPLACED WHOLESALE mid-clone notices the
+# substitution and re-clones, ending up holding the whole generation now
+# published — NOT that the rotation necessarily interfered with the copy,
+# which is not guaranteed and is what made the racing version of this
+# scenario flaky (issue #3).
+#
+# The interleaving is forced rather than hoped for, the way
+# prune-cache-selftest.sh's scenario 12 forces a marker into the
+# check-to-unlink window: the consumer's own `cp` performs the rotation, so
+# it lands strictly after the entry count and inode that open the identity
+# window and strictly before the inode that closes it. The publisher is the
+# real publish-snapshot.sh running concurrently; only WHEN it runs is
+# arranged.
 ROT=$(cache_key feat/rotate)
 rm -rf "$root/snapshot-$BASE_KEY" "$root/target-$BASE_KEY"
-# Generation 1 is wide (the consumer will still be walking it when the swap
+# The two generations must differ in entry count, or "the seeded tree matches
-# happens); the generation replacing it is small, so the publisher's own
+# the published snapshot" would hold for whichever one the consumer ended up
-# staging clone does not itself outlast the consumer's.
+# with and the fixture could not distinguish the outcomes at all.
-make_wide_tree "$root/snapshot-$BASE_KEY" gen1 800
+make_wide_tree "$root/snapshot-$BASE_KEY" gen1 12
-make_wide_tree "$root/target-$BASE_KEY" gen2 8
+make_wide_tree "$root/target-$BASE_KEY" gen2 3
 gen1_entries=$(tree_entries "$root/snapshot-$BASE_KEY")
+gen1_inode=$(stat -c '%i' "$root/snapshot-$BASE_KEY")
+[ "$gen1_entries" -ne "$(tree_entries "$root/target-$BASE_KEY")" ] \
+  || fail "both generations have ${gen1_entries} entries — this fixture cannot tell them apart"
 
-( bash "$script_dir/seed-target-dir.sh" "$ROT" "$BASE_KEY" "$root" jobRot > "$scratch/logRot" 2>&1; echo $? > "$scratch/rcRot" ) &
+cat > "$scratch/bin/cp" <<EOF
-seed_pid=$!
+#!/usr/bin/env bash
+# Fires once, and only on the consumer's own top-level hardlink clone —
+# identified by its destination, which is that seed's private staging path.
+# Every other cp in the process tree (the unshare copies, and the publisher's
+# own staging clone) falls through to the real one.
+if [ "\${@: -1}" = "$root/.stage-jobRot" ] && [ ! -e "$scratch/rotated" ]; then
+  : > "$scratch/rotated"
+  rc=0; "$real_cp" "\$@" || rc=\$?
+  # Concurrently: the publisher's post-swap drain wait is against THIS
+  # consumer's marker, which is held until the identity read that follows
+  # this cp returns, so running the publish inline would deadlock the two
+  # sides against each other.
+  ( bash "$script_dir/publish-snapshot.sh" "$BASE_KEY" "$root" pubRot > "$scratch/logPub" 2>&1
+    echo \$? > "$scratch/rcPub" ) &
+  # Hand control back only once the swap is on disk, so the identity read
+  # immediately after this cp is guaranteed to resolve to the new generation.
+  deadline=\$(( \$(date +%s) + 60 ))
+  while [ "\$(stat -c '%i' "$root/snapshot-$BASE_KEY" 2>/dev/null)" = "$gen1_inode" ]; do
+    [ "\$(date +%s)" -lt "\$deadline" ] || { echo "stub cp: the publisher never swapped the snapshot" >&2; exit 90; }
+    sleep 0.05
+  done
+  exit \$rc
+fi
+exec "$real_cp" "\$@"
+EOF
+chmod +x "$scratch/bin/cp"
 
-# Rotate only once the clone is demonstrably mid-walk. Gating on observed
+rcRot=0
-# progress rather than on a sleep is what makes the interleaving reproducible
+seed_with_stub "$ROT" "$BASE_KEY" "$root" jobRot > "$scratch/logRot" 2>&1 || rcRot=$?
-# instead of a coin flip that passes on a fast machine for the wrong reason.
+[ -e "$scratch/rotated" ] \
-threshold=$(( gen1_entries / 5 ))
+  || fail "the stubbed cp never fired: no rotation was placed in the window, so this scenario proves nothing"
-progress=0
+ok "the rotation was placed inside the consumer's identity window"
-deadline=$(( $(date +%s) + 60 ))
+wait_for_file "$scratch/rcPub" "the publisher never finished"
-while :; do
+[ "$(cat "$scratch/rcPub")" = "0" ] || { tail -40 "$scratch/logPub"; fail "publish-snapshot.sh exited non-zero"; }
-  progress=$(tree_entries "$root/.stage-jobRot")
+[ "$rcRot" = "0" ] || { tail -40 "$scratch/logRot"; fail "the seed exited non-zero"; }
-  if [ "$progress" -ge "$threshold" ]; then break; fi
+ok "the seed completed"
-  if ! kill -0 "$seed_pid" 2>/dev/null; then
-    fail "the seed finished before its clone could be caught mid-walk (fixture too small for this machine?)"
-  fi
-  if [ "$(date +%s)" -ge "$deadline" ]; then
-    fail "the staging clone never reached ${threshold} of ${gen1_entries} entries"
-  fi
-done
-ok "caught the consumer's clone mid-walk at ${progress}/${gen1_entries} entries"
 
-bash "$script_dir/publish-snapshot.sh" "$BASE_KEY" "$root" pubRot > "$scratch/logPub" 2>&1 \
+# WHICH check caught the rotation is the point of this scenario, so it is
-  || { tail -40 "$scratch/logPub"; fail "publish-snapshot.sh exited non-zero"; }
+# asserted rather than assumed. The copy succeeded and the staging tree holds
-wait "$seed_pid"
+# every entry the source had when the walk began, so neither cp's exit status
+# nor the entry count is a witness here — the source's identity changing
+# under the walk is the only one. Reading the report back is what keeps this
+# scenario sensitive to losing that single comparison.
+assert_tear "$scratch/logRot" 0 same differs "the substitution was caught by the source's identity alone"
 
 rot_dir="$root/target-$ROT"
-[ "$(cat "$scratch/rcRot")" = "0" ] || { tail -40 "$scratch/logRot"; fail "the seed exited non-zero"; }
-ok "the seed completed"
-# THE assertion. Before the guard, this is where it failed: the seed reported
-# `cloned in 1s` and exit 0 while target-<rot> held less than half the entries
-# of the snapshot it claimed to have cloned. Comparing against the snapshot as
-# it stands NOW is the right bar either way — a clone that raced the rotation
-# must end up holding one complete generation, and a consumer caught mid-walk
-# re-reads, so that generation is the new one.
 snap_entries=$(tree_entries "$root/snapshot-$BASE_KEY")
 rot_entries=$(tree_entries "$rot_dir")
 [ "$rot_entries" -eq "$snap_entries" ] \
-  || fail "the seeded tree is truncated: ${rot_entries} entries against the snapshot's ${snap_entries} (was ${gen1_entries} before the rotation)"
+  || fail "the seeded tree is not the generation now published: ${rot_entries} entries against the snapshot's ${snap_entries} (generation 1 had ${gen1_entries})"
-ok "the seeded tree is complete (${rot_entries} entries, no silent truncation)"
+ok "the seed re-cloned and holds the whole published generation (${rot_entries} entries)"
 assert_content "$rot_dir/debug/.fingerprint/x/dep-lib-x" gen2 "the seeded tree holds one whole generation, not a splice of two"
-grep -q 'was torn' "$scratch/logRot" || fail "the rotation was not detected as a torn read"
+leftovers=$(find "$root" -maxdepth 1 \( -name '.stage-*' -o -name '.reading-*' -o -name '.publish-*' \) -print)
-ok "the torn read was detected and reported, not swallowed"
+[ -z "$leftovers" ] || fail "scratch left behind: ${leftovers}"
-[ -z "$(find "$root" -maxdepth 1 \( -name '.stage-*' -o -name '.reading-*' -o -name '.publish-*' \) -print -quit)" ] \
-  || fail "scratch left behind: $(find "$root" -maxdepth 1 \( -name '.stage-*' -o -name '.reading-*' -o -name '.publish-*' \) -print)"
 ok "no staging, reader-marker or deferred-generation scratch left behind"
+rm -f "$scratch/bin/cp"
 
+echo
+echo "=== 8b: a subtree unlinked out from under the walk, silently ==="
+# The other way a clone tears, and the one with nothing to report: a subtree
+# that leaves the parent's listing before `cp -al` reads it is simply never
+# visited. The copy exits 0 and the source's identity never changes, so the
+# entry count taken before the walk is the only witness there is — this is
+# the mode that used to publish a partial tree and call it a success.
+#
+# Modelled by renaming the subtree out and back around the consumer's own cp:
+# out before the walk starts (the only way to be missed without an error),
+# back before the retry, because the real thing that removes entries — a
+# publish rotating a generation away — has a whole generation at the path by
+# the time the retry looks.
+TRUNC=$(cache_key feat/truncate)
+TRUNC_BASE=$(cache_key release/1)
+make_wide_tree "$root/snapshot-$TRUNC_BASE" trunkgen 6
+victim="$root/snapshot-$TRUNC_BASE/debug/deps/d3"
+trunc_entries=$(tree_entries "$root/snapshot-$TRUNC_BASE")
+[ "$(tree_entries "$victim")" -gt 0 ] \
+  || fail "the subtree this scenario removes is empty — its removal would not change the entry count"
+
+cat > "$scratch/bin/cp" <<EOF
+#!/usr/bin/env bash
+if [ "\${@: -1}" = "$root/.stage-jobTrunc" ] && [ ! -e "$scratch/unlinked" ]; then
+  : > "$scratch/unlinked"
+  mv "$victim" "$scratch/held"
+  rc=0; "$real_cp" "\$@" || rc=\$?
+  mv "$scratch/held" "$victim"
+  exit \$rc
+fi
+exec "$real_cp" "\$@"
+EOF
+chmod +x "$scratch/bin/cp"
+
+rcTrunc=0
+seed_with_stub "$TRUNC" "$TRUNC_BASE" "$root" jobTrunc > "$scratch/logTrunc" 2>&1 || rcTrunc=$?
+[ -e "$scratch/unlinked" ] \
+  || fail "the stubbed cp never fired: nothing was unlinked mid-walk, so this scenario proves nothing"
+ok "a subtree was taken out of the source's listing before the walk read it"
+[ "$rcTrunc" = "0" ] || { tail -40 "$scratch/logTrunc"; fail "the seed exited non-zero"; }
+ok "the seed completed"
+assert_tear "$scratch/logTrunc" 0 short same "the silent truncation was caught by the entry count alone"
+
+trunc_dir="$root/target-$TRUNC"
+[ "$(tree_entries "$trunc_dir")" -eq "$trunc_entries" ] \
+  || fail "the seeded tree is short: $(tree_entries "$trunc_dir") entries against the source's ${trunc_entries}"
+ok "the seed re-cloned and holds every entry the source has (${trunc_entries})"
+[ -e "$victim" ] || fail "the fixture did not put the subtree back"
+[ -e "$trunc_dir/debug/deps/d3/f0" ] \
+  || fail "the subtree missed by the first walk is absent from the seeded tree"
+ok "the subtree the first walk never saw is present in the seeded tree"
+leftovers=$(find "$root" -maxdepth 1 \( -name '.stage-*' -o -name '.reading-*' \) -print)
+[ -z "$leftovers" ] || fail "scratch left behind: ${leftovers}"
+ok "no staging or reader-marker scratch left behind"
+rm -f "$scratch/bin/cp"
 echo
 echo "=== 9: a source that cannot be read fails loudly ==="
 rc=0