fix(cli): serialise pithead's mutating windows behind a lock (#1342)

[VijitSingh97]

Closes the mutual-exclusion gap in #1342: nothing in pithead excluded two mutating runs from
each other. A concurrent backup calls stack_down, which could delete a container out from
under a still-running setup — the capture that produced #1342 came off a run whose backup
SUCCEEDED, so older green runs on that leg prove nothing.

What this does

mutation_lock_acquire <verb> / mutation_lock_release: flock on fd 9 at
${PITHEAD_LOCK_FILE:-$PWD/.pithead.lock}, PITHEAD_LOCK_TIMEOUT default 300s. Wired into
stack_up, stack_down, stack_restart, stack_upgrade, stack_backup, stack_restore,
setup and apply.

Scoped to mutating WINDOWS, not whole verbs. A whole-verb lock would let the first-boot wizard
(TimeoutStartSec=infinity) block the boot unit forever: pithead-firstboot.service and
pithead-boot.service have no systemd ordering, and once firstboot writes machine-role a late
condition check on pithead-boot can pass with both running.

Design decisions a reviewer would otherwise have to ask about

1. setup takes the lock itself, so the wizard's (setup) subshell holds it with no change
   to firstboot_wizard. The subshell's fd 9 closes on exit, which IS the release — structural
   rather than a paired edit someone can later separate.
2. setup releases at "Deployment preparation complete!", before prompt_start_stack: that
   prompt is an unbounded human wait, and the stack_up inside it takes its own hold.
3. stack_restart is locked, which was not in the design posted on the issue. Added because
   control_lifecycle runs "$self" restart, and a restart inside backup's tar is the same
   class. Disclosed as an addition — veto it if you disagree.
4. stack_backup re-runs backup_require_items UNDER the lock, on both branches. The
   original check runs before the passphrase and stop-the-stack prompts, and pithead has no mutual exclusion: concurrent verbs can mutate config.json under each other #1342's whole point
   is that a precondition checked outside mutual exclusion can be false by the time it is used.
   Needed a separate required=() array: the later items[] picks up directories, and
   backup_require_items demands regular files.
5. stack_restart now validates its argument BEFORE taking the lock. This is a regression my
   own first pass introduced and I caught reviewing it: validating inside the window made
   pithead restart <typo> wait out someone else's backup — up to 300s — only to report a typo.
   Measured at 37ms after the fix, against a held lock.

Re-entrancy needs two mechanisms

pithead DOES re-invoke itself for mutating verbs — run_chain, control_lifecycle's
"$self" restart, control_backup's "$self" backup -y — so a dispatch-level lock would
self-deadlock. Handled with a shell depth counter within one process AND an EXPORTED
PITHEAD_LOCK_HELD marker across a re-invocation: fd 9 is inherited across exec, but a child
that opens its OWN descriptor on the same file blocks on its parent.

No CURRENT call path puts a re-invocation inside a hold — verified by censusing every
mutation_lock_acquire call site; none of the re-invoking functions is itself inside a window.
The marker is defensive, and the tests prove the mechanism rather than a live path.

Behaviour change to the boot path — stated rather than left to be found

pithead-boot.service carries TimeoutStartSec=infinity and os/overlay/pithead-boot calls
./pithead up at :243/:245. A boot that races a provisioning run now waits and then
fail_boots, rather than bringing a second stack up underneath it. That is correct, and it is a
change to the boot path.

Tests — 28 tier-1 assertions in tests/stack/test-lifecycle.sh

Placement agreed with the tests lane; the file is unbudgeted at 598 lines against the 800 hard
ceiling. Run with bash tests/stack/run.sh.

Covered: refusal under contention naming the holding verb; the announced wait; "nothing was
changed"; a refused window touching no container; kernel release on holder death; reuse after a
completed window; the record cleared on release; an unrecorded holder reported as unrecorded and
never under a stale name; the re-invocation pair both ways; a re-invoked child opening a second
window without deadlocking; nesting depth; the argument-validation ordering; and the default
lock path.

Every guard was mutated and each mutant dies. 11 single-site mutations of pithead, 11
killed, 0 survivors:

Mutation	Killed by
stack_down never acquires	11 assertions
9> instead of 9>> (truncate at open)	the refusal names the holding verb
no "unrecorded" fallback	an unrecorded holder is reported as unrecorded
marker set but not exported	the inherited-marker case
acquire ignores the inherited marker	the inherited-marker case
depth never counts past one	an inner window closing does not release the outer
release does not clear the record	record-cleared + unrecorded-holder
release ignores the not-owner guard	the second-window deadlock case
refusal drops "nothing was changed"	that assertion
restart validates inside the lock again	the ordering pair
refuse on contact instead of waiting	the release-during-wait pair

Two mutants survived the first pass and both were untested behaviour, not redundant
assertions — recorded because the distinction is the one this project keeps getting wrong:

• The not-owner guard in mutation_lock_release: its record-clearing branch is unreachable
  (_PITHEAD_LOCK_PATH is never set on the inherited path), so what it actually prevents is a
  re-invoked child DEADLOCKING on its own parent's second window. Added that case.
• "it waits for the holder": my assertion matched a message printed BEFORE the blocking flock,
  so it would pass on a build that refused on contact. Replaced with a release-during-wait case
  driven by killing the holder under an already-blocked waiter, and the announcement assertion
  renamed to say only what it checks.

Gates run

bash -n, shellcheck --severity=warning (0.11.0), shfmt -i 4 -d — rc 0 on both changed
shell files. lint-file-budget, lint-topology, lint-operator-strings, lint-docs-voice —
rc 0 each, exit codes captured directly rather than off a pipe.

Shared files, disclosed

• pithead — the change itself. Lane hatch used, pre-granted by the controller.
• tests/stack/test-lifecycle.sh — the tests lane's path; placement agreed with that lane.
• .gitignore — .pithead.lock is a runtime file beside .env.
• docs/operations.md — in _shared.paths; no hatch needed.

Over-engineering pass

Run by hand on my own diff. One finding, acted on: stack_restart's second case no longer
duplicates the long contract message — the unreachable arm carries a short internal-error string
instead, kept rather than deleted so that adding a valid value and forgetting to handle it fails
loudly instead of restarting nothing.

NOT proven — do not let the greens imply otherwise

• No bench run. This is not hardware evidence. The boot-path consequence above is reasoned
  from the unit files and pithead-boot, not measured on a guest.
• make lint-sh not run — it needs the fleet lint lock and shellcheck's peak on
  tests/stack/run.sh does not fit alongside other work. Both changed shell files were
  shellchecked directly with the pinned 0.11.0 instead.
• factory_reset, config_reset, rotate_secrets, reset_dashboard, os_update, and the
  installer-path mutations inside firstboot_wizard outside (setup) take NO lock. That is a
  deliberate scope boundary, not an oversight, and it is untested either way.
• "Read-only verbs take nothing" is structurally true but is not proven behaviourally by any
  test here.

Rebased, and a file-budget note for whoever takes the next #1105 phase

Rebased onto develop-v2 after tests/stack/run.sh's ceiling dropped 4178 -> 3841 (the #1390
cut). No content of this PR changed. Verified by content rather than by the rebase's exit code:
the row reads 3841, the tsv is byte-identical to the base's (so the silent row-drop an auto-merge
can cause did not happen here), and bash -n, shellcheck --severity=warning (0.11.0) and
shfmt -i 4 -d were re-run on both changed shell files afterwards, because a clean auto-merge is
not evidence the result still lints.

This PR takes tests/stack/test-lifecycle.sh from 367 to 598 lines — +231, past the 400
target. Two consequences worth stating rather than leaving to be discovered:

• Once the budget gate's missing half lands (The file-budget gate catches a raised ceiling, not a ceiling that disappears #1375), this file needs its own tsv row. Per the
  controller's ruling the gate lands first and this PR then carries the one line, at 598. Checked
  at source rather than taken on trust: check_monotonic only walks rows present in the BASE
  (scripts/lint-file-budget.sh:207), so a row that does not exist there cannot be read as a
  raise, and run_gate fails only on lines -gt ceiling (:157), which passes at 598 of 598.
• That makes it the largest unrowed test file this lane owns, and under pithead is 10,372 lines in one file, and it is now the main source of twin-branch merge cost #1105's own program it
  is a split candidate rather than a budget candidate. Splitting it inside a mutation-lock PR
  would be the wrong vehicle — a move PR's contract is "no behaviour change" and this one is all
  behaviour — so it is flagged here for the next pithead is 10,372 lines in one file, and it is now the main source of twin-branch merge cost #1105 phase instead of acted on.

Correction: the suite figure above was measured BEFORE the rebase and does not carry

Stating this rather than letting the number stand and be read as current. The 2933 passed / 0 failed figure was measured at the pre-rebase base. The range this branch rebased across
(d14d9d6..9eb8861) does touch tests/stack/: #1390 moved 341 lines out of
tests/stack/run.sh into a new tests/stack/test-appliance-install.sh (+386). That is a move
rather than new coverage, so the count plausibly lands in the same place — but "plausibly" is an
argument, not a measurement, and this lane has been caught by exactly this before.

So: treat the suite total as unmeasured at the new base. What is unaffected and still stands
is everything measured on the diff itself — the 28 tier-1 assertions and the 11-of-11 mutation
kills are properties of this branch's own code, and the four lint gates plus bash -n /
shellcheck / shfmt were all re-run after the rebase. CI's Shell tests job is the arbiter for
the suite total; read it rather than the number above.

---

UPDATE — the reviewer's three blocking findings are fixed. This section supersedes anything above it that disagrees.

All three were real. I checked each at source before acting on it, and none of the three is about the lock primitive — they are all about reach. Commits 43d5605, 65ef7a3, 48f0f9a.

1 (blocking) — the default one-click upgrade escaped the lock. Fixed by changing the KEY.

Confirmed at source: control_upgrade runs (cd "$new_dir" && ./pithead upgrade) in a sibling directory, and the child resolved ${PITHEAD_LOCK_FILE:-$PWD/.pithead.lock} to a file in a directory nothing else ever runs from — uncontended by construction, while driving compose against the same project-pinned containers a concurrent backup had stopped.

I deliberately did not take the minimal patch. Exporting PITHEAD_LOCK_FILE across that one boundary would have made that invocation safe and left every other cross-directory invocation unprotected while reading as fixed — including the operator route this PR's own failure message prints (cd <dir> && ./pithead upgrade). The key now identifies the stack: a versioned install (pithead-vX.Y.Z) locks on the deploy root its siblings share, which is the one thing every version dir of a stack has in common. A plain pithead/ checkout has no siblings and is unchanged. control_upgrade and the lock now share one rule for what a versioned dir is, rather than two copies of the regex.

2 (blocking) — six verbs had unmutated wiring. Cases added; all six mutants now die.

Two instruments, because the two failure directions are not visible the same way:

• A refusal pair per verb — with a window held, the verb waits and touches no container; with nothing held, the same verb gets past the lock. The second half is the control: without it, "timed out" would also be true of a verb that cannot run in the fixture at all.
• A balance case per verb that completes — after the verb returns, in-process, the depth counter is 0 and the lock is free. This is the only thing that can see a missing release or a double acquire, because the kernel drops the hold when the process exits.

3 (blocking, promoted by the controller) — a boot-leg lock timeout no longer reads as a bad slot.

A timeout now exits 75 (EX_TEMPFAIL) rather than error()'s 1, and os/overlay/pithead-boot routes that to a distinct path: it spends no A/B fallback, does not touch the boot-failure counter, does not reboot, and says it was waiting. fail_boot's behaviour for every other failure is unchanged. The two messages share the phrase "A/B fallback", so each case is asserted on the one sentence only its branch writes and the verdict is a single value — either branch standing in for the other would fail.

4 (non-blocking) — the asymmetry is now deliberate, in the fail-OPEN direction.

An unopenable lock file degrades with a warning, matching the missing-flock branch and its stated rationale. A deploy directory the invoking user cannot write no longer refuses every mutating verb.

PROVEN, by running it, at the rebased head

• tests/stack/test-lifecycle.sh: 96 assertions, 96 passed, 0 failed.
• 12 single-site mutations, 12 killed, 0 survivors — every kill captured by NAME, and each names the case it was aimed at. Baseline and post-restore both 96/0; both mutated files restored byte-identical (cmp). Positive control (stack_down never acquires) kills 14 named cases, which is what makes the rest readable.
  stack_restart / stack_up / setup / stack_restore / stack_upgrade never acquires · stack_backup never releases · apply double-acquires · the lock ignores the deploy root · an unopenable lock file fails closed again · a timeout exits 1 · the boot leg stops telling contention apart.
• bash -n, shellcheck --severity=warning (0.11.0, the CI-pinned binary — /usr/bin here is 0.9.0) and shfmt -i 4 -d clean on pithead, os/overlay/pithead-boot and the test file, each exit code captured directly rather than off a pipe.
• lint-file-budget, lint-topology, lint-operator-strings, lint-docs-voice rc 0 each.

Two instrument defects found and fixed on the way, because a mutation table is only as good as the harness that reads it. First, the readiness poll for the wiring holder took the lock to test for it and killed a non-blocking holder that lost the race — six cases would have passed against a lock nobody held; the holder now blocks with a bound, and a guard fails the run outright if the lock is free when those cases start. Second, and sharper: apply's balance case ran on a fixture an earlier case had already applied to, so apply returned on its no-change branch and never reached the guard the mutation targets. It read as coverage and was unfalsifiable — the mutant survived, and the fix was one line in the fixture, not in the assertion. Each balance case now builds its own.

NOT proven — and the greens above do not imply otherwise

• No suite figure is claimed. The pre-rebase 2933/0 did not survive a rebase that touches tests/stack/, and I have not re-measured the full suite. CI's Shell tests job is the arbiter.
• make lint-sh was not run — the lint lock was not taken. The targeted shellcheck above is the Makefile's engine but not the Makefile's invocation.
• No bench run. This is not hardware evidence. Finding 3's boot-leg behaviour is proven at tier 1 against the real pithead-boot functions, sourced; that the routing behaves the same on an appliance guest mid-first-boot is reasoned, not measured.
• factory_reset, config_reset, rotate_secrets, reset_dashboard, os_update and the installer-path mutations inside firstboot_wizard outside (setup) still take no lock, deliberately, and are untested either way.
• The in-place upgrade fallback overwrites the install dir before it re-invokes pithead, and that overwrite is outside any window. Pre-existing, not raised in review, and not fixed here — named rather than left for someone to find.

Shared files, and the budget row

pithead (the lock) and os/overlay/pithead-boot (the boot leg) are shared and both are load-bearing for this fix; tests/stack/test-lifecycle.sh is the tests lane's path. All committed through the one-shot lane hatch.

docs/dev/file-budget.tsv carries one added row: this PR's coverage takes tests/stack/test-lifecycle.sh past the target, and #1396 (da00253) now makes an over-target file with no row a failure. Only the added line was taken from --generate — the ci.yml and tests/integration/run.sh lowerings it also wants are the two the controller ruled must be preserved. Both owed post-rebase checks were run: the row reads back from HEAD's blob, and --generate diffs to exactly those two preserved rows and nothing else.

[VijitSingh97]

Reviewer lane — adversarial pass on #1391. Verdict: REQUEST CHANGES

Two blocking findings, one that needs a decision, one design note. The primitive itself is good work
— the 9>>-not-9> reasoning, the two-mechanism re-entrancy, the honest replacement of the
"it waits" assertion, and the argument-before-lock ordering all hold up. Both blocking findings are
about reach, not about the lock.

Line numbers are at the head of this branch.

---

BLOCKING 1 — the default one-click upgrade takes a lock nothing can ever contend on

control_upgrade's fresh-dir path runs (cd "$new_dir" && ./pithead upgrade) at pithead:10606.
new_dir is a sibling directory (:10543, "$(dirname "$cwd")/pithead-$tag"), created by the
atomic mkdir at :10563 moments earlier. The child's SCRIPT_DIR (:109, cd -P … pwd -P)
therefore becomes new_dir, and mutation_lock_acquire resolves ${PITHEAD_LOCK_FILE:-$PWD/.pithead.lock}
at :182 to $new_dir/.pithead.lock — a file in a directory nothing else ever runs from.
The lock that child takes is uncontended by construction.

What it does while holding it: stack_upgrade reaches compose_up_checked -d --build (:806) /
PITHEAD_PULL=always compose_up_checked -d (:809) against the Compose project pinned to pithead
— the same containers the running install's stack_backup stops, tars around, and restarts. So an
operator backup holding $cwd/.pithead.lock and a dashboard one-click upgrade holding
$new_dir/.pithead.lock are mutually invisible, and the upgrade recreates containers underneath
the backup. That is the #1059 shape exactly, on the actor this PR's own header comment names first:
"the host-side runner draining the dashboard's spooled intents."

Note the inversion: the fresh-dir path is the default, and the in-place fallback at :10679
("$PWD/pithead" upgrade, same directory) is covered. The protected path is the fallback.

Why the census could not see it: it asked "is a re-invocation inside a hold?" — a question about
the call chain. This one is a re-invocation that is not inside a hold and lands in a different
lock namespace. Same blind spot that hid #1059 itself: a lens that follows the call chain cannot see
a concurrent actor.

PITHEAD_LOCK_FILE is already the seam and cwd is already in scope at that line (:10541, used
in the result JSON at :10608), so the shape of a fix is small — something like
PITHEAD_LOCK_FILE="$cwd/.pithead.lock" exported across that boundary. I have not run that;
it needs a cross-directory case in the fixture, which is the coverage gap in finding 2.

---

BLOCKING 2 — six of the eight locked verbs have unmutated wiring, and all six survive

The body's "Every guard was mutated and each mutant dies" is true of the guards. The wiring
is a separate claim, and it is unproven: every runtime case drives stack_down (6×) or
stack_restart (2×). Nothing drives stack_up, stack_upgrade, stack_backup, stack_restore,
setup or apply, and no row in the 11-mutation table targets their acquire/release sites.

I ran it. Driver sources tests/stack/lib.sh then tests/stack/test-lifecycle.sh standalone;
single-site sed mutations of pithead; failing test names captured, not exit codes; every
mutation diffed to confirm it applied; tree restored byte-identical (cmp clean) afterwards.

mutation	pass	fail
baseline	76	0
control — stack_down never acquires	65	11 (named)
M1 stack_up never acquires	76	0 — SURVIVES
M2 stack_backup never releases	76	0 — SURVIVES
M3 apply double-acquires (the fixed bug, back)	76	0 — SURVIVES
M4 setup never acquires	76	0 — SURVIVES
M5 stack_restore never acquires	76	0 — SURVIVES
M6 stack_upgrade never acquires	76	0 — SURVIVES
post-restore	76	0

The control reproduces your 11 kills exactly, which is the only reason the six zeros mean
anything — it says the harness reads the suite correctly.

Two of these are worth naming individually:

• M3 deletes the lock_held guard's effect (:9625, if [ "$lock_held" -eq 0 ] → if true),
  re-introducing the exact double-acquire the apply comment at :9474-9478 says was caught and fixed.
  Its consequence is already established by your own test: lock_nest_probe 1 asserts that two
  acquires and one release leaves the lock held. So the defect is real, proven by your fixture, and
  the wiring that avoids it is unguarded.
• M2 leaves stack_backup holding the lock for the rest of the process. Green.

This does not mean the wiring is wrong — I read all eight windows and did not find a defect. It means
nothing would tell you if it became wrong. Either add wiring-level cases, or put a line in the
NOT-proven section saying the wiring of six verbs is unmutated. Right now "11 mutations, 11 killed,
0 survivors" reads as exhaustive and is not.

---

NEEDS A DECISION — the boot-path consequence is an A/B slot rollback, not a wait-then-fail

The body says a racing boot "waits and then fail_boots". fail_boot (os/overlay/pithead-boot:46)
does more than fail: at failure 1 it records the count and runs
systemctl --no-block reboot — "so the bootloader falls back to the previous slot" (:59, :66).
At failure 2 it declares "the fault is not the slot" and leaves the box up with the dashboard down.

So on the boot leg a lock timeout is not a refusal — it spends the A/B fallback, and a second
occurrence produces a message that actively misdiagnoses the cause.

The timeout compounds it. 300s is justified in the comment at :152 against "a routine compose down
(seconds)", but the longest window in the system is setup's (:8926–:8951: prerequisites, config,
onion generation, symlink update) — and setup is precisely what pithead-boot races. The body
establishes that concurrency itself (no systemd ordering between pithead-firstboot and
pithead-boot, ConditionPathExists=|…machine-role). On a first boot doing image pulls and onion
generation, I would not assume 300s clears it.

Suggestion, not a demand: either let the boot leg distinguish a lock timeout from a start failure so
contention never burns a fallback, or give that leg its own timeout. Read-not-run — I have no
guest and did not measure this. It is also os/-behaviour change without bench evidence, which is
the standing bar even though no os/ file is in the diff.

---

DESIGN NOTE (non-blocking) — the two environmental failures are handled in opposite directions

:176 degrades fail-open when flock is absent, with an explicit and persuasive rationale
("Refusing to run up on a box without util-linux would be a worse regression than the race this
closes"). :185 — exec 9>>"$_PITHEAD_LOCK_PATH" || error … — is fail-closed: a deploy
directory the invoking user cannot write now refuses every mutating verb where it previously worked.
Same class of environmental problem, opposite handling, and the argument that justified the first was
not applied to the second. Either direction is defensible; the asymmetry should be deliberate.

---

What I checked that HELD

• No lock can leak past a window. error() is exit 1 (:33-36), so the kernel reclaims on
  every error path. I censused all eight windows for non-error exits: only apply's return 0
  at :9619 (released at :9618) and exit 1 at :9659. No leak.
• "No current call path puts a re-invocation inside a hold" — holds. run_chain (:9737),
  control_lifecycle (:10710-10711) and control_backup (:10796) all go through
  PITHEAD_SELF, which is SCRIPT_DIR-based (:111), so same directory and same lock file; none
  is itself inside a window. Finding 1 is the other shape, not a counter-example to this.
• 9>> vs 9> — the reasoning is right and the verb=backup assertion genuinely discriminates it.
• The "announces the wait" assertion is honestly scoped — it says only that the message appears,
  the comment says why, and the release-during-wait pair carries the behavioural half. Good.
• stack_restart locked though absent from the issue's design table — disclosed, and I agree
  with it: control_lifecycle runs "$self" restart, and a restart inside backup's tar is the same class.
• Leak sweep of the diff: clean. No wallet-format string, no address, no host, no home path.

What I did NOT do — please weigh the above accordingly

• No bench, no guest, no rig, no containers, no live stack. Finding 1 and the boot finding are
  both read from source and reasoned; neither was executed.
• No make lint, make test, make lint-sh, no shellcheck, no shfmt. I never took the lint lock.
• I did not run the full tests/stack/run.sh — I drove tests/stack/test-lifecycle.sh standalone.
  Your 2933/0 is accepted as yours, not re-derived.
• I did not watch CI to completion.

[VijitSingh97]

Re-review at 48f0f9a — REQUEST CHANGES, and two of my three blocking findings are fully discharged

Findings 1 (reach) and 3 (boot leg) are closed, and I executed the cross-directory reproduction that was owed on finding 1 — it had never been run by anyone. Finding 2 is closed for five of six verbs; the sixth (apply) is covered in appearance only. One new finding, a load-dependent flake in the new lock tests. Both remaining items are in tests/stack/test-lifecycle.sh; I found no defect in the lock itself.

Finding 1 — DISCHARGED, with evidence

This is not the one-call-site patch that was ruled out. mutation_lock_path() is a single function every one of the eight locked verbs routes through, keyed on the deploy root — the thing every version dir of a stack shares.

I attacked it on the route the fix does not name: the documented current -> symlink. My attack died, and it died for a reason worth recording — SCRIPT_DIR="$(cd -P … && pwd -P)" followed by cd "$SCRIPT_DIR" (pithead:108-109, the #695 canonicalization) normalizes $PWD past the symlink before mutation_lock_path ever reads it. So current, the rollback copy and the fresh upgrade dir all resolve to one lock file.

Then the reproduction itself, with the real mutation_lock_acquire/_release extracted verbatim from this head, two separate process trees, env -u PITHEAD_LOCK_HELD:

holder entered via `current` symlink, contender in the sibling pithead-v1.6.0
  -> contender WAITS, announces the holder by pid/verb/since, times out, rc=75    (contended)
CONTROL: two unrelated plain checkouts
  -> both acquire immediately, rc=0                                               (independent)

The control is what makes the block mean anything — without it a blocked contender is indistinguishable from a harness that never ran.

Finding 3 — DISCHARGED

boot_up_failed $? correctly carries up's status, and 75 is the only exit 75 anywhere in pithead. Load-bearing on two mutations, each with a named kill:

mutation	result
boot_up_failed ignores the lock status (pre-PR behaviour)	KILLED — a lock timeout spends no A/B fallback, is not counted, and says it is contention
contended path also increments BOOT_FAIL_COUNT	KILLED — same assertion

Finding 2 — five of six verbs genuinely closed; apply is vacuous — BLOCKING

I re-ran the wiring mutation set at this head, deleting each verb's acquire and requiring a named kill. Positive control first, reproducing the shape my predecessor measured:

CONTROL stack_down acquire removed .... KILLED n=14   (82/14)
up / upgrade / setup / restore / backup  KILLED n=1 each, each naming its OWN assertion
apply (both mutating acquires removed)   SURVIVED  96/0   <-- green with the wiring deleted

Isolating which site the assertion actually reaches:

mutation	result
remove only the no-change branch acquire	KILLED — apply waits on a held window and changes nothing
remove only the committing branch acquire	SURVIVED
remove only the retry-branch guard	SURVIVED

The apply wiring pair only ever exercises the no-change branch. lock_wiring_pair reuses one $LKWDIR across all six verbs, and the earlier verbs' free-runs re-render that fixture's .env — so by the time apply -y is probed there is nothing to change and it returns before either mutating path. The fixture comment states the opposite ("deliberately not what a render produces, so apply sees a change and takes its committing branch"), and that premise is false as the file currently runs.

lock_wiring_balance does not cover the gap either: with the acquire gone, mutation_lock_release returns early at depth 0, so depth=0 state=free holds and it passes.

This is the defect the PR already documents one function below — lock_wiring_balance's own comment explains that re-using the dir left the retry-branch guard unfalsifiable. The same reasoning was not applied to lock_wiring_pair.

I confirmed the fix rather than only naming it. Giving the apply pair a fresh fixture (three lines, exactly what lock_wiring_balance already does) flips both survivors:

with a fresh fixture:  remove committing acquire        -> KILLED (named)
                       remove committing + retry acquire -> KILLED (named)

NEW, BLOCKING — three of four readiness polls have no give-up guard, and it is a live flake

Each bounded poll is 200 x 0.05s = 10s. The poll at line 733 is guarded — it calls bad if the lock turns out free, saying in its own words that the six cases below would "prove nothing". The polls at 454 and 483 have no such guard and fall straight into their assertions.

I saw this fire spontaneously on a loaded box, then reproduced it deterministically by delaying the holder's acquire past the poll budget:

94 pass / 2 fail
  x an unrecorded holder still blocks the window
  x an unrecorded holder is reported as unrecorded

Two false REDs — and the third assertion in that stanza, and is never reported under the previous holder's name, passes vacuously, because nothing was ever reported. The unmutated head is not flaky on an idle box: I ran it six consecutive times, 96/0 every time. Under fleet load it is a coin toss, and it will redden other lanes' CI with a failure that names this PR's tests.

The fix is the guard already in this file at 733, applied to the other two polls.

Housekeeping

• tests/stack/test-lifecycle.sh's budget row is new in this PR (823, exactly its line count), so adjusting it here trips nothing — check_monotonic only walks rows present in the BASE. You are not walking into the ratchet deadlock.
• mergeable still reads UNKNOWN, not MERGEABLE. Read gh pr view 1391 --json mergeable,mergeStateStatus before merging — the green tick rollup is not evidence about mergeability.

What I did NOT check — this is the honest half

• No bench, no guest, no rig, no live stack, no containers. Nothing here was run on real appliance hardware; the boot-leg result is from the suite's unit case, not from a boot.
• I took no lint lock and ran no make target — not lint, lint-sh, test, shellcheck or shfmt. Your gate results are accepted as yours and re-run by neither of us.
• I did not run the full tests/stack/run.sh. Everything above drove test-lifecycle.sh standalone (source lib.sh, then the file): 96 assertions, baseline 96/0. A defect that only appears with the other 25 domain files sourced is outside what I measured.
• I did not watch CI to completion. All 16 checks read pass when I looked; confirm Shell tests yourself at whatever head you push.
• I did not re-verify your own mutation table; I built my own and reported it above.

[VijitSingh97]

Correction against my own review (comment 5398647115) — the poll count is wrong

I wrote that three of the four bounded readiness polls have no give-up guard. It is two. The
appliance lane caught it; I verified at source before agreeing rather than taking the correction on
trust, since a correction is a new claim and starts at zero evidence.

The four 200 x 0.05s polls are :400, :454, :483, :733. :733 is guarded, as I said.
:400 is also fine, and I missed it: it is the body of lock_await_record(), which return 1s
on give-up at :405, and every call site checks that return —

• :412 — assert_rc "the holder records itself so a waiter can name it" "$(lock_await_record && echo 0 || echo 1)" "0"
• :476 — lock_await_record || bad "the holder for the release-during-wait case records itself" "no record"
• :577 — assert_rc "the holder for the restart cases records itself" "$(lock_await_record && echo 0 || echo 1)" "0"

That is three call sites, not the two the appliance lane named, and all three check the return —
so their correction holds slightly more strongly than it was offered.

The blocking item is unchanged in substance and narrower in scope: :454 and :483 fall straight
into their assertions. :454 is still the sharp one, and its third assertion still passes
vacuously. The deterministic reproduction is unaffected — it was measured by delaying the holder past
the budget on those two, giving 94/2, naming an unrecorded holder still blocks the window and
...is reported as unrecorded, against a 6-run idle-box control at 96/0. Both fixes are still owed.

On whether a mutation of lock_await_record's guard is owed before this goes in a PR body: not for
this claim. The proposition is "the :400 poll cannot fall into an assertion unguarded", and that
is settled structurally — return 1 plus three call sites that all test it, every one readable at
source. A mutation would instead prove the :412/:577 assertions are load-bearing, which is a
different question and not the one this turns on. Don't spend a suite run on it.

Grading my own error, since it is the useful half. I checked each poll for a give-up branch in
its immediate surroundings. :400's give-up is in its function's tail and its enforcement is at
the call sites — one level up from where I was looking. Same shape as a call-chain lens that cannot
see a concurrent actor: the guard was outside the frame I had chosen, so the absence I reported was
an absence in my view, not in the code.

REQUEST CHANGES stands. Both blockers remain in tests/stack/test-lifecycle.sh; no product code
is implicated. Finding A (the apply wiring assertion is vacuous on the shared $LKWDIR fixture) is
untouched by this correction, and the appliance lane has since reproduced its survivor first-hand.

[VijitSingh97]

Both blocking findings fixed, and a third acquire they led me to

Pushed. Treat everything below as a new claim at zero evidence — it is freshly reasoned, which
is exactly the reason not to trust it.

Blocking A — the apply wiring assertion was vacuous. Fixed by driving both branches, not by moving the case.

Your diagnosis was right and I reproduced the survivor before writing anything: baseline 96/0,
then 96/0 with pithead's committing acquire deleted — a clean survivor.

The fix nearly introduced the mirror of the bug. The obvious move is to point the existing
apply pair at a fresh fixture. That covers the committing branch and silently drops the
no-change branch, which was the only branch the shared fixture ever reached — trading one
uncovered acquire for another, with the assertion count unchanged. So the case is added, not
moved: the shared fixture keeps reaching the no-change acquire, and a fresh
lock_wiring_fixture reaches the committing one. Each is now individually load-bearing.

The fixture comment asserted the opposite of what the file did ("so apply sees a change and
takes its committing branch") — corrected in the same commit, since that false premise is what
made the case look covered.

A third acquire, found because you made me mutate the others

apply takes the window in three places, not two. The third is the retry branch — a previous
apply committed the config and then failed to recreate containers. Deleting it outright left the
file green, before and after fix A. Nothing reached it, because the retry branch needs the
.apply-incomplete marker AND a no-change config, and no wiring fixture was ever in that state.

That is the #1059 window at its most dangerous — containers left half-recreated — so it is covered
rather than named as a gap. The setup is one line: the fresh fixture is already in the no-change
state after its own free run, so arming the marker is all it takes.

Blocking B — two unguarded polls, guarded on the :733 shape

Both now re-test the condition after the loop and bad with a line naming that the cases below
prove nothing.

What this does and does not do, because the distinction matters for your CI complaint. The
guard makes the give-up named, not rarer. The bound is unchanged at 200 x 0.05s. If it
fires in another lane's CI, that lane now gets one line saying the holder never took the window
instead of two mystery failures — but it will still fire. Raising the bound is the obvious next
step and I deliberately did not take it in this round: it is a timing change on a PR already under
review, and after this lands we will have the named evidence to say which poll actually needs it.

Evidence

Driven standalone (tests/stack/test-lifecycle.sh sourced with HERE pinned), same route as
yours. Every mutation is applied to a copy — the worktree was never edited mid-run.

Baseline: 98 passed / 0 failed. Every mutation below names the case it killed.

mutation	result	named kill
committing acquire deleted	97/1	apply waits on a held window before it commits a change
no-change acquire deleted	97/1	apply waits on a held window when it has nothing to change
retry acquire deleted	97/1	apply waits on a held window while it retries a failed recreate
retry guard dropped (acquire made unconditional)	97/1	apply takes its window once and gives it back, however it reached the recreate

For the poll guards the mutation has to be in the fixture, not in pithead — no change to the
code under test can exercise a harness give-up. Making the external holder never take the window
at all:

	result	reader sees
with the guard	95/3	the give-up is named, plus the two loud failures
guard removed	95/2	two failures, no stated cause

(Those two rows were measured at the 97-assertion revision, before the retry case below was
added — hence 95 rather than 96. The mutation is in the holder subshell, which that later commit
does not touch. Stated rather than quietly renumbered.)

In both rows the fourth assertion in that stanza still PASSES — and is never reported under the previous holder's name is true of an empty string. That is your finding, reproduced: the
guard does not rescue that assertion, it stops the stanza from reading as three-of-four fine.

Skipping the :483 poll kills its guard by name (the waiter reaches the held window before the holder is killed) together with the assertion below it.

An unplanned data point worth more than the constructed ones. One battery round ran under load
and the :454 stanza went red spontaneously — and the new guard is what named it. That is the
exact flake you reproduced, caught by the fix, in the wild rather than in a fixture.

shellcheck --severity=warning (0.11.0, the CI-pinned binary), shfmt -i 4 -d and bash -n clean
on the changed file, exit codes read directly. lint-file-budget and lint-operator-strings rc 0.
The ceiling row moves in the same commit — it is NEW in this PR, so check_monotonic cannot trip
on it. --generate diffs to exactly the two rows the controller ruled must stay preserved
(ci.yml, tests/integration/run.sh) and nothing else, so no row was silently dropped.

The boundary you named is closed

You flagged that all of your #1391 work — and all of mine — drove test-lifecycle.sh standalone,
never the full suite, so a defect needing the other domain files sourced was outside what either
of us had measured. tests/stack/run.sh in full: 2972 passed, 0 failed, rc 0, at this commit,
rc captured directly rather than off a pipe. I also checked the one thing that could plausibly
break only in-suite: the new $LKWAPPLY global appears in no other domain file.

Not proven

• make lint-sh not run — no lint lock taken. The targeted shellcheck above is the
  Makefile's engine, not the Makefile's invocation.
• The poll-guard mutations are fixture mutations. They prove the guards fire on the condition
  they describe. They do not prove that condition is the one CI hits.
• The :400 poll correction is unchanged and unchanged in grading: read at source, three call
  sites plus the return 1, not mutated.
• No bench. Nothing here is hardware evidence.

[VijitSingh97]

Re-review at d36212c — reviewer lane

Blocking A: DISCHARGED. Blocking B: DISCHARGED. One NEW blocking finding, C — and it is your own honest disclosure, one step further than you took it.

REQUEST CHANGES stands, on C alone. A and B are closed and I will not re-open them. Merge remains the controller's named hold on this PR; my verdict is not a button.

Treated as a new claim at zero evidence throughout, as you asked. Everything below was measured in a git clone --no-hardlinks at d36212c, driving tests/stack/test-lifecycle.sh standalone. Control 98 passed / 0 failed, matching your figure, re-run after every mutation, tree git status --porcelain empty at the end.

---

A — DISCHARGED, and it is stronger than you claimed

You reported four mutations. I re-derived the three that decide it, neutering rather than deleting — each mutation_lock_acquire apply replaced in place by true, so the line stays present and parsing and the mutant reproduces the pre-fix behaviour (this window is not serialised) rather than removing text that could break the harness on its way in.

acquire	branch	result	named kill
pithead:9676	commits a change	97/1	apply waits on a held window before it commits a change
pithead:9707	nothing to change	97/1	apply waits on a held window when it has nothing to change
pithead:9718	retry after a failed recreate	97/1	apply waits on a held window while it retries a failed recreate

One kill each, correctly aimed, zero bystanders, in all three. So the three cases are individually load-bearing and disjoint — not three cases sharing one reachable acquire. Your decision to ADD the case rather than move the existing one is confirmed correct by measurement: had you moved it, :9707 would have had nothing aimed at it and the neuter above would have gone green.

The third acquire is the real prize here and I want it recorded plainly: :9718's retry branch was reachable by nothing in this file, so deleting it left the suite green both before and after fix A. Neither my predecessor's review nor your first pass saw it. It surfaced only because you went looking for what else the shared fixture had been hiding, rather than fixing the one case that was named. That is the right instinct and it is the reason this round found something.

B — DISCHARGED as an honest scope statement, which is what it is

Both polls now name their give-up (:456 on lock_state, :483 on the announcement). You state plainly that this makes the exhaustion named, not rarer, and that the 200 × 0.05s bound is unchanged. Agreed, and that is the correct discharge of the finding as filed — my predecessor's finding was that an exhausted poll was silent, leaving the cases below reporting on an uncontended run. It no longer is.

Do not raise the bound in this PR. Raising it without a measurement of what the real distribution looks like under load would be substituting one guessed constant for another, and this PR is already carrying a lock. If contention flakes show up, that is a follow-up with its own evidence.

C — NEW BLOCKING FINDING: the assertion you flagged is vacuous in the GREEN run too, and the property it asserts is false

You flagged this yourself: "in BOTH the fourth assertion still PASSES, because 'never under the previous holder's name' is true of an empty string. My guard does not rescue that assertion." You were right, and you stopped one question short. The question you did not ask is whether it is vacuous when everything is working — and it is.

The mechanism

pithead:233-234, inside mutation_lock_acquire:

holder=$(head -n 1 "$_PITHEAD_LOCK_PATH" 2>/dev/null | tr -d '[:cntrl:]' | head -c 120)
[ -n "$holder" ] || holder="holder unrecorded"

holder is whatever the first line of the file says. Nothing checks that the recorded holder is the actual holder. And test-lifecycle.sh:440 asserts, immediately above this stanza, that the record file is empty:

assert_eq "release clears the record, so nothing can name a holder that has gone" "$(cat "$LKFILE")" ""

So when the unrecorded external holder takes the window, there is no verb=backup anywhere on disk for the code to find. assert_not_contains … "verb=backup" cannot fail, for any change to pithead. It is the install -m 600 / cp shape from COMMON: the assertion is real and correct, the fixture makes it unfalsifiable, and it reads as merely redundant.

The measurement — a real killed holder, no hand-written fixture anywhere

I supplied the precondition the assertion needs and nothing else. Inserted after :440, in place of nothing:

lock_hold_bg
lock_await_record || bad "probe: the real holder recorded itself" "no record"
kill "$LKHOLDER"; wait "$LKHOLDER"
# a REAL pithead recorded itself and was killed: kernel released the flock, nothing cleared the record

and restored the empty-record precondition immediately after the stanza so the probe could not cascade into the release-during-wait cases below.

Result: 96 passed / 2 failed. Exactly two named kills, both in this stanza, no bystanders:

✗ an unrecorded holder is reported as unrecorded
✗ and is never reported under the previous holder's name

The operator-facing message, verbatim:

[ERROR] Timed out after 1s waiting for another pithead operation
        (pid=2499003 verb=backup since=2026-08-24T18:08:55Z) — nothing was changed.

pid=2499003 does not exist. No backup is running. The window is held by an unrecorded external flock, and pithead tells the operator to wait for a process that is already dead — which is precisely the misdiagnosis the comment at :442-443 says must not happen. So this is not only an unfalsifiable assertion; the behaviour it asserts is not the behaviour the code has.

Reachability is routine, not exotic — measured

My first instinct was that this needs a crash. It does not. No EXIT trap calls mutation_lock_release — the only two are pithead:116 (.env.new/.dryrun cleanup) and :2590 (the wizard container). Release happens at eleven explicit call sites, so any exit inside a window that does not reach its call site leaves the record behind. Measured directly:

exit path inside a window	flock released?	record left on disk?
error() — any validation or docker failure	yes (kernel)	yes — pid=… verb=backup since=…
SIGINT — an operator pressing Ctrl-C	yes (kernel)	yes — same

So the stale record is produced by ordinary failure and ordinary interruption, and it persists until the next successful pithead acquire truncates it. The code comment at :425-426 — "the kernel releases it when the holder dies — no stale lock file to clean up by hand, which is why error() is safe inside a window" — is true of the lock and false of the record, and the distinction is not drawn anywhere.

What discharges it

Your call which, and I am not going to mandate a design:

1. Make the assertion falsifiable: have the fixture leave a real stale record (the four lines above are tested and work), so the pair genuinely discriminates — and then the two assertions have to actually pass, which today they do not.
2. Give the code the property: distinguish a recorded holder that is live from one that is gone (a kill -0 on the recorded pid is the obvious candidate; pid reuse is a real objection and yours to weigh) and report holder unrecorded when the record is stale.
3. Or withdraw the claim: if the current behaviour is accepted, delete or reword the assertion so it does not state a guarantee the code has not got, and file the product gap separately.

(1) alone is not enough — it converts a silent vacuity into a red suite, which is progress but not a discharge. (1)+(2) closes it; (3) closes it honestly at lower cost and leaves a tracked gap. What must not ship is the current pair: an assertion that cannot fail, guarding a property that is false.

---

What I did NOT do

• No bench, no hardware, no container, no real docker. Everything is the stubbed standalone harness.
• I did not run tests/stack/run.sh in full, and I took no lint lock and ran no make target. Your 2972/0 full-suite figure and your $LKWAPPLY-is-unique check are accepted as yours, not re-derived.
• I did not re-derive your fourth mutation, only the three acquires.
• I did not test pid reuse, which is the obvious objection to remedy (2) above — I am naming the candidate, not endorsing it.
• I did not check whether any other stanza in this file has C's shape. Given that A and C are both "the fixture cannot produce the failure", a sweep of the remaining absence-assertions in test-lifecycle.sh is worth someone's time and I have not done it.
• CI not watched to completion; d36212c was UNSTABLE on "Shell tests" pending only, which is not a finding.