Teardown matches the socket spelling the engine carries, and verifies after the cascade (refs #344, refs #345)

[vyskocilm]

refs #344, refs #345

Teardown's verification leg matched a string no process on the machine carries,
so it could not fail. Fixed, plus the four regression tests that hold it, plus
the measurement showing the fifth cannot exist.

redteam round: OWED, deferred by maintainer ruling, NOT waived. See the
last section — it names the round's target so whoever runs it does not have to
re-derive it.

The defect

Engine.paths() returned the HOST socket /tmp/snug-<uid>-<pid>/sock/podman-<pid>.sock.
Since Tier C the engine is exec'd with an argv written in its DERIVED view —
Spec builds "unix://" + guestSock — so its host-visible cmdline names
/snug/engine/sock/podman-<pid>.sock. engine.go says the two names must not be
tidied into one; reap.go was never told.

So waitQuiet returned nil on its first poll of every run, signalOwned was
never reached, the SIGKILL escalation never happened, and the "this sandbox's
containers did not die with it" warning was unreachable. #167's lesson —
host-side code reading engine-namespaced pids — unlearned for paths.

paths() now returns the recorded GUEST spelling, and Spec is the one place
that records it: a second derivation of the host→guest mapping in reap.go
would agree with Spec until the day it did not. It returns nil rather than
falling back to e.sock, because that spelling appears in no command line on the
machine, so a fallback is indistinguishable from not sweeping while looking like
a sweep. Emptiness has its own failure mode, so stopLocked reconciles it
against the lifeline.

The position

Stop split into Detach (drop the keepalive, at payload exit) and Stop
(verify, reap, remove the run directory, from the caller's own deferred
cleanup). Also fixes a latent bug: step 4 removed the run directory, holding the
live engine's socket and generated conf, while the engine was still serving it.

Be precise about what the split buys, because the loose version is false and
this PR measured it. Real podman 5.8.4 bundle, wall clock payload-exit to
snug-exit:

fixed wiring    15ms
shipped wiring  15ms, 16ms, 15ms

No difference. The engine is Pdeathsig'd to P1, the stage, not to P0
(internal/stage/enginefork.go:156), and MainServe returns the moment
runOneSandbox has sent the "exited" event, whereupon exitOnStageError
calls os.Exit(0). P1 waits for nothing P0 does. On a clean run the shipped
wiring costs nothing.

What the position actually buys is a GUARANTEE where there was a RACE:

• Stage.Wait() returns on recvEvent — when the event BYTES ARRIVE — and does
  NOT wait for P1 to exit. At payload exit, P0 and a still-exiting P1 race on
  separate cores. P1 wins 7/7 on an idle laptop and nothing MAKES it win.
• Stage.Close() ends with cmd.Process.Wait(), which BLOCKS until P1 is
  reaped, and the kernel delivers a child's pdeathsig in
  forget_original_parent before that wait wakes. After it, the engine's
  SIGKILL is already queued.

A race it usually won, versus a barrier it cannot lose.

The 15.3s was measured on a decoy

The round that found #344 recorded "15.3s of added teardown with a decoy
standing in for the engine". A decoy started by a harness is not forked by the
stage and carries no Pdeathsig, so it CANNOT die with P1: it survives,
waitQuiet burns the whole budget, and 15.3s is exactly right for that
process. What it models is an engine outside the cascade's reach — the rare
case the sweep exists for — never an ordinary engine on a clean run.

General form, which is the transferable part: a substitute in a round models
the subject only in the dimensions it shares with it, and the round must say
which dimensions those are. This decoy shared the cmdline (the dimension under
test) and not the parentage (the dimension that decides whether it dies).

quietBudget is REWRITTEN, not renumbered: 2s, covering scheduling and a walk of
/proc for a SIGKILL already delivered, with the relation to idleTimeout
deleted rather than kept as a coincidence. killBudget (3s) names the second
wait, which was a bare literal.

The accident sentence, inverted

The old text accepted one risk: the only way this reaps something else is a
foreign command line containing the host socket path. With the mark fixed the
string is /snug/engine/sock/podman-<pid>.sock — a string the PAYLOAD can
author, since it can put candidate pids on its own argv and payload processes
are visible in the host /proc. What stops that is not the matcher but WHEN the
sweep runs: after the stage is reaped, the sandbox's pid namespace has collapsed
and no payload process is alive to match. Stated in the file, because moving the
sweep back re-opens it.

Tests

Four of five specified, plus a documented impossibility for the fifth.

test	what it holds
TestTeardownMatchesTheArgvTheEngineIsStartedWith	every element of paths() appears in the argv Spec produced; fails on the code that shipped
TestStopEscalatesToSIGKILLWhenTheEngineOutlivesTheCascade	drives stopLocked step 3 for the first time in this repo's history
TestPathsIsEmptyUntilSpecHasRun	paths() empty before Spec, non-empty after (the positive control)
TestStopLockedReconcilesADialledLifelineWithNoMark	both arms of the reconciliation warning — a warning that always fires is as useless as one that never does
TestSweepDoesNotMatchTheHostSocketSpelling	a live decoy naming the HOST spelling is not swept, with a mandatory guest-spelling decoy that must be, at both cmdlineNamesPath and ownedPIDs
test/guard/enginereapordering_test.go (3 tests)	the three-package wiring: container.go wires Detach not Stop, main.go defers cleanup before sandbox.Run, exec.go defers st.Close
TestDescribeSanitisesACommandLine	landed in #362

A mutation that trips a test's own guard has graded the guard, not the assertion

Stated as a method, because it generalises to every negative test in this repo
and it is not obvious: a mutation is only evidence about the assertion it
actually reaches.

The host-spelling negative needed two. Reverting paths() to e.sock — the
pre-#344 spelling, the obvious mutation — does fail the test, but at the VACUITY
GUARD: host and guest collapse to the same string and the guard fires first.
That grades the guard. It says the test refuses to run vacuously, and nothing
whatsoever about whether the negative assertion works. Reported as sufficient, it
would have looked like proof.

paths() returning {e.guestSock, e.sock} keeps the two spellings distinct, so
the guard passes and the mutation reaches the assertion — which then fails at
both cmdlineNamesPath and ownedPIDs, while the guest-spelling positive
control still holds. That is the one that grades the test.

Before believing a mutation, check WHICH assertion it tripped.

Every one mutation-proved. The escalation test's decoy blocks on a shell read
nothing closes, so it outlives quietBudget by construction rather than by
timing; SIGKILL→SIGCONT fails it and prints the never-before-rendered "did
not die with it" warning. The ordering guard proves each regex matches today's
source before trusting it to prove absence, and mustFindOne fails on zero AND
on more than one, with distinct messages.

The ordering guard's third test is the weakest and says so in its first
sentence. opts.OnPayloadExit() is called synchronously inside runStaged's
guard.wait callback, so it always precedes that function's own deferred
st.Close() whichever line comes first textually. It pins a written convention,
not a runtime ordering.

The fifth test cannot exist, and that is the deliverable

test/integration/enginereapteardown_test.go was written as the fifth slot and
the mandatory mutation (onPayloadExit: eng.Detach → eng.Stop) DOES NOT FAIL
IT — measurement above. No threshold was tuned to make it pass. On a clean run
no black-box wall-clock test can distinguish the fixed wiring from the shipped
bug's.

Kept as a narrower real-engine ratchet: clean teardown stays well under
quietBudget, positive-controlled by a real /v1.41/version 200 in the same run
being timed. Its threshold is quietBudget/2, computed by parsing
internal/engine/engine.go's own const quietBudget at test time rather than
copied, because that value already went stale once. The comment names the trap:
raising it toward 3s is not a relaxation, it is a disablement.

The property doing the clearing is named, so a later change re-opens the
question instead of inheriting a stale pass (CLAUDE.md's rule, and #313's round
cleared a route on a property #276 then deleted). The slot is unfillable because,
and only because, the engine is Pdeathsig'd to P1 rather than P0 AND
MainServe waits on nothing P0 does. Change either and the wall-clock test
becomes both writable and necessary.

Also here

SNUG_REAP_SOCK deleted. reaperScript has read only $SNUG_REAP_DIR since
#167 deleted the reaper's host-side podman stop; the variable survived that
deletion unread — a path handed to a process designed to outlive snug, for no
reason anyone can state. The sentence survives as a RULE: every run-specific path
travels in the environment, never argv, because two /proc sweeps match on path
strings and a path in argv makes snug's own cleanup match its own sweep. The
test's negative named sock alone, so deleting the parameter would have turned
it green by deletion — it now asserts the SET (run directory and socket
path).

#345's surviving second copy (refs #345). orphansweep.go's doc comment is
correct since #362 — the number IS recyclable while the fd is held, measured —
and twelve lines below its closing warning, inside the function that warning
governs, sat the original false claim: "the number cannot be recycled while
pidfd is held, so the checks below and the kill all name the same task." #270's
shape: a rule fixed at the site the reporter quoted while a pre-existing copy
survives. No behavioural consequence today — the kill goes through the pin and
fails closed with ESRCH.

Swept the SET, not the site, before fixing. No third copy. reap.go:195 and
teardown.go:444 both state it correctly, so the header's claim about its two
neighbours is true — verified rather than inherited. Nuance worth carrying: they
state the same GUARANTEE, not the same WORDS, so a fixed-string sweep would
report two sites and miss the third, and a sweep that finds nothing looks
identical to one that proves absence.

Five stale inheritances in one branch, and what caught each

All the same shape — prose or a spec copying state that lived somewhere else,
each true where it was written and false where it was read. Recorded as a base
rate, not a confession, so the catches are counted alongside the defects:
excluding the ones caught in-lane would quietly imply this mechanism only
produces defects that ship.

#	the stale statement	caught by
1	"nothing verified against a real podman" — true of /usr/bin/podman (the distrobox shim P1 refuses by name), false of the pinned 5.8.4 bundle the suite drives	checking a caveat before repeating it
2	the review's under 3s — derived from quietBudget = 15s, which this same commit series rewrote to 2s, so the threshold passed on the bug	reading the constant the number came from
3	the 15.3s — a decoy's cost read as an ordinary engine's	measuring it
4	orphansweep.go's inline pin claim — corrected at the header in #362, left standing twelve lines below, inside the function that header governs	verifying the tree after a merge
5	a new test's doc comment calling itself "#344's fifth and last regression test" — the fifth is the one that cannot exist	review, in-lane, before it shipped

The distribution is the interesting part. One was caught by a mechanical
guard (TestEveryDesignDocACommentCitesExists, on a separate citation defect in
the same lane). One by checking the tree after a merge. Three only because
someone refused to repeat a number without checking it. That is an argument for
the habit, not an apology for the branch — and it is the same measured base rate
CLAUDE.md already records for @git-ro binding ~/.gitconfig for a full
milestone before a mechanical sweep, not a redteam round, caught it.

Engines used

fakepodman via $SNUG_PODMAN for the matcher measurements behind 80ca6b2.
The pinned static podman 5.8.4 bundle for the integration timing test and its
positive control. Per-claim, not per-branch: /usr/bin/podman here is distrobox's
distrobox-host-exec shim, which preflight P1 refuses by name.

Gate

make gate green on every commit. Integration green with a real engine.

The owed redteam round

#344 owes a redteam round. It was deferred by maintainer ruling, not
waived. Definition-of-done rule 3 applies in full: this is engine/stage and it
changes what gets SIGKILLed and when. The round is scheduled against MERGED code,
in its own lane.

The reasoning is worth stating rather than referencing: a round is worth running
against the code that SHIPS, and a lane in flight is not that code yet. Running
one mid-lane spends it on a diff that is still moving and produces the one thing
a round record must never be — a note saying a surface was attacked when what was
attacked was a draft.

The round's target, written here so it does not have to be re-derived:

1. The matcher. Decoys naming the GUEST spelling
   /snug/engine/sock/podman-<pid>.sock against decoys naming the HOST spelling
   /tmp/snug-<uid>-<pid>/sock/podman-<pid>.sock. The guest string is one a
   PAYLOAD can author on its own argv, which the host string was not.
2. The SIGKILL escalation in stopLocked step 3 — unreachable before this
   branch, so it has one unit test and no adversarial exposure at all.
3. Whether the ORDERING that makes the guest sock non-forgeable actually
   holds. The sweep runs only after the stage is reaped and the sandbox's pid
   namespace has collapsed, so no payload process should be alive to match. That
   is the claim; it has not been attacked.

Engines, per-claim and never per-branch. Everything behind 80ca6b2 was
measured against test/integration/testdata/fakepodman via $SNUG_PODMAN. The
new integration work drives the pinned static podman 5.8.4 bundle at
~/.local/opt/podman-static. /usr/bin/podman on this host is distrobox's
distrobox-host-exec shim, which preflight P1 refuses by name.