fix(smoke): enforce recorded sandbox identity before smoke-suite teardown - #913
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| { cwd: root, env: { ...env, [hook]: "1" }, encoding: "utf8", timeout: 240_000 }); | ||
| const crashOptions = { cwd: root, env: { ...env, [hook]: "1" }, encoding: "utf8" as const, timeout: 240_000 }; | ||
| assertSmokeSandboxDown(sandbox, ["down", "--preserve-state"], crashOptions); | ||
| const crashed = spawnSync(tsx, [cliPath, "down", "--preserve-state"], crashOptions); |
|
Verdict: APPROVE WITH NITS at head dc9b38b. I reviewed this without prior context: read issue #884 in its own terms, then the PR body and the full diff, then verified the load-bearing claims against the tree. What I checked:
Nits:
Neither rises to a block: the change solves the issue as stated (the remembered sandbox assumption becomes an enforced one at the destructive call), covers the full derived set rather than the issue's undercount, and is honest about its own boundaries, including the subprocess teardown path it could not execute. |
|
APPROVE WITH NITS at head I read this as a fresh reviewer, without the issue's conclusions in front of me, and traced the guard against the actual resolution code. What holds up:
Nits, none blocking:
What I checked that would have caught a problem: the registry filename and |
|
BLOCK on This change hardens smoke-suite teardown. It does not close #884 as that issue now stands. The later comments on #884 replaced the original helper-shaped request. They locate the defect in one resolver serving both a creating verb and a destroying verb, and failing open:
Concrete sequence that still takes a live stack down after this lands:
The suite work is real and, on its own terms, carefully fail-closed: recorded root / marker / Closing #884 with a harness-only change would treat the demonstrated product failure as solved. Keep the smoke guard. Do not close the issue until Named gap: live subprocess teardown was not executed (prohibited here). CI still does not run |
|
BLOCK at head The target-addressed guard does not parse the command the way the CLI does, so it can approve one mesh while Concrete failure sequence:
I reproduced the mismatch broker-free against the PR implementation and the exact parser: the guard returned, while the parsed CLI space was The proof suite checks only Other requested checks held: bare BLOCK |
|
Follow-up on I blocked because this PR closed #884 while leaving the product-side failure sequence in place. Re-fetched just now:
That is the second half of the condition I named. The product walk ( Keep the smoke-side work. It is fail-closed on recorded identities, covers the call-shape set rather than a filename convention, and Nits, neither of which reopens the block:
Named gap unchanged: live subprocess teardown was not executed. CI still does not run |
|
Follow-up, still Re-fetched the PR just now: body still Refs #884 and does not close it, I independently reproduced the duplicate Node returns That is a real hole in the new guard, and it is the same class as the original issue (a check that can be green while the destructive call aims elsewhere). It is not a current call site: the one Product fail-open ( |
|
Delta re-grade of
That closes both nits from the previous head:
Unrecognized flags fail closed ( Remaining nit, not a block: #884 remains OPEN and this PR still does not close it. Product fail-open is unchanged by the delta. Named gap: this tree is still at |
The prior proof covered only one canonical argv form, leaving parser-equivalent forms outside the claimed guarantee.
|
APPROVE WITH NITS at head Delta re-grade from The parser fix closes my block. I exercised the canonical form, flags before The regression proof is load-bearing now. The committed suite has named cells for flag order, equals syntax, and last-wins duplicates. Restoring first-occurrence target binding while preserving equals support makes the duplicate cell fail on Package boundaries still hold: smoke-kit remains private, source-only, no-emit, and has no Nits and named gaps:
APPROVE WITH NITS |
|
Delta re-grade of
Does the newly accepted set contain a call that acts on a root other than the anchor's? No.
So a newly accepted call is one where The duplicate
#884 remains OPEN. Product fail-open is unchanged by the delta. Named gap: this tree is still at |
Verdict: BLOCK at
|
|
BLOCK at head The newly accepted set contains a call that acts on a root other than the anchor's: an empty parsed space. Concrete failure sequence:
I reproduced this broker-free against the current guard, Node parser, and workspace resolver: the guard accepted
Require the effective space to satisfy the same predicate |
|
Follow-up evidence at the same head I reran the counterexample through the actual registered command surface rather than a copied parser configuration:
A broader matrix through the same registered parser, current guard, and production resolver confirmed canonical order, flag-before-target, equals syntax, and repeated last-wins to a non-empty sandbox space stay pinned. A repeated last foreign space is refused. Missing and unknown flags are refused. Only repeated last-wins to empty produced Observed project checks:
The actual destructive The result is sound for non-empty parsed spaces, but it is not sound for every string. The guard must require the same effective-space predicate as |
Verdict: APPROVE at
|
| argv | parsed | CLI honours | guard |
|---|---|---|---|
missing --space |
undefined |
no, follows current | refuse |
--space "" |
"" |
no, follows current | refuse |
--space= |
"" |
no | refuse |
--space=operator --space= |
"" |
no | refuse |
--space= --space=operator |
"operator" |
yes | accept (sandbox record) |
" ", tab, newline, ZWSP, "0", ".", "..", NUL |
truthy string | yes | accept when that record's root is the sandbox |
Zero cases where the guard accepted a value the CLI would treat as absent. The previous teardown ("" accepted, CLI dropped the flag, current-mesh selected a foreign root) is closed on both --space "" and last-wins --space=.
Whitespace-only is truthy on both sides. That is not a remaining gap: the CLI looks up a mesh actually named " ", it does not fall through to current. If that record is missing, the guard is stricter (cannot establish identity) rather than looser.
Lower bound
The one extra refusal seen was a truthy --space target whose planted record pointed at a foreign checkout. The guard refused on root identity. The CLI would have honoured the name and resolved that foreign root. That is the guard doing its job, not a looser acceptance test.
Mutation fixture
Each of M1, M2, M3, M5, M6, M7 has a unique find string at this sha. pnpm smoke:sandbox-guard loads @cotal-ai/smoke-kit from src/, so those source mutations are the copy the suite runs.
Applied on extracted copies of this sha, not by mutating the parked worktree:
- M1 named cell printed
✗ foreign sandbox root is refused by identity: Missing expected exception. - M2 named cell printed
Missing expected exception: generic guard refuses flag-before-target down web - M3 named cell printed
✗ target guard uses the CLI parser's last space value: Got unwanted exception.(equals-form also went red, because first---spacescanning cannot see--space=value; the named last-wins cell still changed state) - M5 named cell printed
check reaches smoke:sandbox-guard - M6 named cell printed
check reaches smoke:sandbox-guard before its first environment-dependent live suite(M6_FIND_EXACTholds inpackage.json'scheckscript; first live step remainspnpm smoke:spawn-from-anywhere:live) - M7 named cell printed
✗ target guard refuses both empty-space argv formsbecause both child empty-argv cells missed their expected exception. That is the mutated emptiness check, not an earlier unrelated failure.
M4 was not applied to the parked tree. The suite's own scan regex was run against the M4 replacement in dogfood-live.smoke.ts. After the rewrite, spawnSync(..., "down", ...) in the finally block is a raw down spawn whose preceding five lines do not contain assertSmokeSandboxDown, which is exactly the raw down spawn is not immediately guarded row.
M2, M4, M5, and M6 still fail via assert.throws / assert.ok / assert.notEqual messages rather than the new refuses/permits printers, so those labels do not appear on a green run. That is throw-only evidence. It is enough for a named red here. It is weaker instrumentation than the empty-space aggregate, which prints ✓ on pass and ✗ on fail.
What was tried that did not break it
Hex encoding still keeps empty, .., absolute, and NUL names inside the anchored meshes/ directory. Parser agreement still holds, including last-wins. Identity still runs before the record read. The empty-name record space..json is now refused rather than verified.
No teardown path remains in this delta where a parsed space the CLI treats as absent is accepted by the guard.
|
BLOCK (tests and mutation evidence) Head graded: 9d74467 (resolved myself from origin/up/884-sandbox-guard after fetch; cat-file reports commit; a one-character-mutated sha fails cat-file as the negative control). What I re-ran by hand, in a clean tree at that head, each restored byte-exact afterward (diff -q against a copy taken before mutating) with the suite green again before the next one:
I also ran the shipped registry end to end (node scripts/mutation-proof.mjs --config bin/smoke/mutations/sandbox-guard.json) and it reports all 7 KILLED, exit 0, with the same named reds. Why BLOCK: one surviving mutation, in the exact class this campaign has hit before (a guarantee that reads as verified because it is written down, with no check behind it). The guard records and checks a fourth identity: the sandbox root's .cotal ownership marker. The code's own doc comment calls this directory load-bearing, because it is the thing that stops a bare down from walking upward into an operator checkout, and markerHeld participates in the same combined refusal as the root, COTAL_HOME, and XDG_CONFIG_HOME identities. But no named cell and no registry mutation covers it. Concrete failure sequence: replace the line const markerHeld = markerIdentity === "same"; with const markerHeld = true; and run pnpm smoke:sandbox-guard. Exit 0, "sandbox guard smoke: PASS", every cell green. Run the full shipped mutation registry too: still all KILLED, exit 0. The marker identity guarantee is silently disabled and nothing notices, even though the other three identities each have a dedicated refusal cell and the root identity has its own registry mutation. A later refactor that deletes the marker check as dead code would ship green the same way. The ask is small and in the suite's existing style: one cell that removes or replaces the marker directory between anchor and guard call and asserts the refusal by name (the error message already carries marker=missing or marker=foreign), plus one registry mutation (markerHeld hardcoded true) expecting that cell red. Root, COTAL_HOME, and XDG_CONFIG_HOME each have exactly this shape; the marker is the only recorded identity without it. What I checked that would have caught a problem elsewhere: coverage and correctness are proven separately (the static scan for call-site coverage, the refusal cells for semantics, and the check-chain assertions for gating are distinct, and mutations 4, 5, and 6 each redden only their own layer); the guard's parseArgs mirror matches the CLI's declared down options including the -f short and strict mode, so last-wins and equals-form agreement is real rather than asserted; the two semanticDownOnly allowlist entries use "down" only as an event name and never spawn it; the changeset is empty-form and pnpm changeset status reports no packages to bump, which matches the fact that every changed file is private test infrastructure. The live subprocess teardown acceptance path remains a named gap, as the description already states; this block is not about that. |
|
APPROVE at head The empty-space regression is fixed. I drove the real public dispatcher with the registered
The lower-bound framing holds: the guard rejects the consumer-absent string and may remain stricter elsewhere, but it does not accept a parsed value that this consumer drops. I independently ran the committed seven-mutation fixture against an exact archive of this head. All seven were KILLED on their named cells. I also reapplied each mutation separately and compared its transcript with the green baseline:
So the Observed checks on the exact published content:
Named gap: the actual destructive APPROVE |
Follow-up: executed at
|
The guard records four identities and refuses unless all four hold, but only three had a named refusal cell. Hardcoding markerHeld to true left the suite fully green, so the marker guarantee was recorded rather than checked. Adds the missing cell and the registry mutation that must redden it.
|
APPROVE at 8a580b7 (resolved myself from origin/up/884-sandbox-guard after fetch; cat-file reports commit; a one-character-mutated sha fails cat-file as the negative control). The delta from the previously blocked head is exactly one suite cell and one registry entry. I materialized both changed files at this sha, verified the blob hashes, and confirmed baseline green before touching anything. What I re-ran against the new cell, each restored byte-exact afterward with the suite green again:
On whether the cell regex could be satisfied while the marker check is dead: the regex demands a refusal message reading root=same, COTAL_HOME=same, XDG_CONFIG_HOME=same, marker=missing. A dead marker check permits instead of refusing, so no message exists to match; and a refactor that hardcoded the whole message while checking nothing would immediately redden the earlier permit cells that pin correct acceptance. I could not construct a passing shape. On the verdict-string instrument (the message reports markerIdentity but the gate uses markerHeld): I probed the reachable states directly. With the marker actually missing, the message reads marker=missing, which is both what the gate acted on and what the cell pins, so in every state the suite can produce the string is a faithful witness. The divergent shape (gate refusing while the string says same) requires markerHeld to disagree with markerIdentity, which only arises under the exact mutation the registry now kills. My verdict: an acceptable instrument note, not a defect worth filing. Printing the gate value instead would erase the missing/foreign distinction the message currently carries, which would make it less faithful in the real states. If anything, a one-line comment in the guard noting the ternary's intent would be a nice-to-have, not a blocker. The named gaps in the description remain accurately stated (CI does not run this suite; the live subprocess teardown path was graded by reading). Nothing in this delta changes either. |
Verdict: APPROVE WITH NITS at
|
| id | change | result vs marker cell |
|---|---|---|
| U1 | rootMatches = false |
WRONG-RED / early unwanted exception. Cell not printed. |
| U2 | sameDirectory miss reports foreign |
KILLED. Missing marker now prints marker=foreign, regex misses. |
| U3 | markerHeld true for foreign as well as same |
SURVIVED. Finding, not a request to weaken the cell. |
| U4 | drop markerHeld from the AND |
KILLED, missing expected exception. |
| U5 | throw always says marker=held |
KILLED, regex mismatch. |
| U7 | skip sameDirectory for the marker (markerIdentity = "same") |
red, missing expected exception (same class as M8). |
| U9 | marker identity compared against the root directory | red, missing expected exception (root still exists after rmSync(.cotal)). |
U3 is the one the registry still does not cover. I am not adding it to the fixture from this seat.
Verdict: APPROVE WITH NITS at 8a580b759168001e80624aa52d4b702d9f52d405. The new cell is specific to missing-marker with the other three identities pinned, and M8 kills the mutation it names. The remaining gap is foreign vs missing on the marker, which this round's registry still treats as out of universe.
|
Follow-up at 8a580b7: the recording side is covered, indirectly but genuinely, and I verified it by execution rather than by reading. The question was whether the registry attacks only the check while the anchor could record a fiction. A malformed version of that mutation dies in setup and proves nothing, so I built the well-formed shapes first:
Then I ran shape 1 through the shipped runner as a trial registry entry (temporary config in /tmp, not committed anywhere): KILLED, red and named, and all 8 existing entries stay killed alongside it. A mutation tool entry for the recording side is therefore well-formed and cheap if the maintainers want the belt-and-braces entry. My honest view: it is optional, not owed. The reason is direction. Every entry in the registry today breaks the guard's decision logic, and the suite catches each by refusing or permitting wrongly at the guard call. The anchor mutations are a different species: they corrupt the fixture the suite hands itself, and the suite catches them because the guard is strict enough that a poisoned fixture cannot produce a consistent pass. That is real coverage, but it is a property of the whole suite-plus-guard system rather than of a check the registry names. A registry of guard mutations that happens to also survive poisoned anchors is honest; a registry entry that mutates the fixture to prove the fixture was honest is weaker evidence than it looks, because the same entry cannot distinguish a guard that catches fiction from a guard that merely disagrees with it. On the reporting mismatch raised alongside this (the message reports markerIdentity while the gate uses markerHeld): I enumerated the reachable states directly. Marker actually missing: message says marker=missing, gate refuses, cell pins exactly that. Marker recreated (different dev/ino, same path): message says marker=foreign, gate refuses. Marker intact: both agree. The only state where the string could say one thing and the gate do another is a disagreement between markerIdentity and markerHeld, and I verified by probing that this state exists only under the exact hardcode mutation M8 already kills. So the string is a faithful witness of the decision in every state the code can reach unmutated. My ruling stands: an instrument note, not a defect. If someone wants to harden it anyway, gating on and printing the same value, or a one-line comment on the ternary, would do; neither is required for correctness. Nothing here changes my verdict. The previously stated gaps (CI does not run this suite; the live subprocess teardown path was graded by reading) remain the accurate open items. |
|
APPROVE WITH NITS at a4c4cca Resolved the head independently: rev-parse in a detached tree, ls-remote refs/pull/913/head, gh headRefOid, cat-file positive and negative controls all agree on this sha. This solves the smoke-side half of issue 884 in the shape the issue itself demands: enforcement at the destructive call, with coverage derived from call shape rather than a maintained list. I exercised the real guard module directly (17/17 cells): foreign cwd refused, missing or foreign COTAL_HOME and XDG_CONFIG_HOME refused, marker missing or replaced refused, the generic guard refuses down web without the target guard, the target guard requires an explicit non-empty --space and verifies the registry record root, repeated --space is last-wins through the same node parseArgs the CLI uses, and unknown down flags fail closed. The repo-wide static scan replicates clean at this sha with zero unguarded files, and the mutation fixture's nine find strings all match the source verbatim. Verified the guard agrees with the CLI's actual semantics: the declared down flags match, down.ts treats an empty parsed space as absent, findCotalRoot terminates at the suite-owned marker, and the registry record path meshes/space..json matches the workspace layout. The issue's enumeration claims hold against package.json, including the suite reachable only by a non-live-named script and the suite reachable by no script at all. The head commit also fixes a real defect found while wiring: the bind-fence teardown was passing down --server, which is not a declared down flag, so that teardown errored out and leaked the fixture broker. I could not run pnpm smoke:sandbox-guard, pnpm typecheck, or pnpm changeset status in my tree because node_modules is absent, so those are inspected rather than exercised. The guard behavior itself was exercised against the module source with a self-contained driver. Nits, non-blocking. CI never invokes pnpm check, so the ratchet proof fires only for local check runs; the PR discloses this and names the one-step ci.yml remedy. Manifest-form down is guarded on folder identity only, not on the manifest's owned root. The guard's downOptions is a copied mirror of the CLI's declared flags, so drift fails closed but would not be flagged by the static scan. The check chain was reordered to seat the guard before the first live entry; behavior-neutral but broader than the minimum diff. Would ship while on call. |
|
BLOCK The folder-rooted identity guard is strong, but the target-addressed guard can approve a different registry entry from the one the CLI uses. Concrete failure sequence:
The target guard must validate the same selected record the CLI honors, including its requested space, before comparing the root. Exercised: |
|
Security review verdict: APPROVE WITH NITS at a4c4cca. Head was resolved independently (ls-remote, gh, rev-parse, cat-file with positive and negative controls); all instruments agree. The sandbox teardown guard survives the full adversarial lens: last-wins --space, empty space in both argv forms, flag-before-target, equals-form, down web without an explicit space, foreign or replaced .cotal markers, COTAL_HOME and XDG_CONFIG_HOME identity swaps, symlink aliases, registry split-brain (the guard reads the canonical space..json file and loadMeshes ranks that same file as winner), and static-scan bypass attempts (the suite scan fails red on unguarded call sites, raw spawns, and the allowlist is two audited non-CLI entries). Exercised broker-free in the review tree: smoke:sandbox-guard PASS, smoke:core-boundary PASS (smoke-kit private, zero deps, 395 shipped files scanned, 0 imports), plus a parser-agreement reproduction running the guard against the real parseCommandArgs down spec: every CLI-accepted form is guard-accepted, unknown flags throw on both sides. Nit N1 (non-blocking): smoke:sandbox-guard runs in the local check gate but no CI workflow invokes check or smoke:sandbox-guard. Adding it to the ci.yml unit job suite list would close the gap. Nit N2: the guard's downOptions table duplicates the CLI down flag spec; a cross-reference comment or shared constant would keep them in sync. Nit N3: duplicate web positionals dedupe harmlessly on both sides. Named gaps: live broker teardown behavior was not exercised (banned for this review); mutation-proof was reviewed statically, not executed. |
The target-addressed sandbox guard keyed only on the canonical filename and the record root. loadMeshes and findMesh key by the document space field, so a canonical file whose document named another space could pass while a legacy record for the requested space selected a foreign root. Require the canonical document space to match the requested space before comparing roots, and pin that refusal with a named cell and mutation.
|
Revised security verdict (supersedes the earlier comment): APPROVE WITH NITS at 2e9c64c. Correction of record: the previously graded head a4c4cca had a real registry split-brain, and the earlier APPROVE WITH NITS at that sha is retracted. Reproduced against the real guard module: a canonical space.<hex(victim)>.json holding {space:"other", root:S} together with a legacy-named file holding {space:"victim", root:O} made assertSmokeSandboxTargetDown pass, because it keyed the record by filename while findMesh keys by the document space field. A down web teardown for space victim would then resolve the legacy entry and act at the operator root. The new head 2e9c64c (fix(smoke): refuse mismatched target registry space) closes it: the guard now requires the canonical record's space field to equal the requested space, with a new suite cell and mutation M10. Re-verified at that sha: smoke:sandbox-guard PASS, smoke:core-boundary PASS (smoke-kit private, zero deps, 395 shipped files, 0 imports), and the split-brain reproduction now refuses with observed space "other", expected space "victim" while the CLI-side selection loop still resolves victim at the operator root, proving guard and consumer now key records identically. Remaining nit (unchanged): smoke:sandbox-guard runs in the local check gate but no CI workflow invokes check or smoke:sandbox-guard. |
|
APPROVE WITH NITS This supersedes my BLOCK at The updated head requires the canonical registry document's
Observed: Also exercised successfully: Nits:
Issue 884 correctly remains open for the product-side resolver work. No live suite was run. |
|
APPROVE WITH NITS on 2e9c64c Independent review of this change against issue 884. Does it solve the issue? The issue's final form has two halves. The smoke-side ask is to enforce the sandbox assumption at the destructive call so a suite that has lost its sandbox refuses instead of tearing down the operator's mesh. The product-side ask is to remove fail-open from the verbs themselves, up first. This change lands the first half in full and wider than the issue's own file list: 21 suites across five directories, derived by call shape rather than by filename or script name. It deliberately does not close the issue because the product-side half remains open, and the PR body says so plainly. That matches the issue's own final state. Checks I ran on this head, with the tree clean before and after:
Things I inspected by reading rather than running: the guard's parseArgs configuration exactly matches the CLI's declared down flags (file/-f, run, space, dry-run, preserve-state, store-dir; strict; positionals allowed; last-wins through the same node:util parser the dispatcher uses), and the target-guard record path matches the workspace registry layout (spaceKey hex naming, document space field authoritative, realpath root comparison). Every edited suite records its anchor before the first CLI call, every down uses the recorded root as cwd, and no scenario deletes the root or its marker before a down. The 21 edited suites are live and were not run, so the real subprocess teardown path stays a named, unexercised acceptance gap. Nits:
On-call judgment: ship. The change is fail-closed in every direction a sandbox can be lost, agrees with the real parser and registry, is mutation-proven, and leaves zero unguarded down call sites. Do not treat it as closing 884; the product-side half stays open and tracked. |
1 participant
Summary
The issue this refers to is about an assumption that was remembered rather than enforced: live smokes call a destructive
downinside a sandbox, and nothing checked that the sandbox actually held. Every attempt to enumerate the affected set reached for a naming convention and produced a different wrong answer, including the enumeration in the issue itself. Filename matching and script-name matching each miss a different file, and one file is reachable by neither. So this guards the destructive call rather than the convention that is supposed to keep it away, which is what makes a suite added next month guarded on the day it is written by someone who never read the issue..cotalownership marker,COTAL_HOME, andXDG_CONFIG_HOMEbefore any CLI invocationThe issue named nine files, while this change guards 21 current suites across five directories. The set was derived by call shape rather than by filename or script name because every naming-based classifier missed a different path. Filename matching missed the two auth suites, script-name matching missed
up-tls-routes-live.smoke.ts, and_ps-arm2.smoke.tsis reached by nopackage.jsonscript at all.The static scan is deliberately described at its actual boundary: it covers every current file edited here plus a regex over
*.smoke.ts. It can still miss a computed verb, a non-.smoke.tsharness or child, and anything placed on its semantic-literal allowlist. It is a ratchet over today's call shapes, not proof that an unguarded teardown is impossible to write. The generic guard separately refusesdown webso a target-addressed teardown cannot silently use folder-root identity checks.smoke:sandbox-guardis inpackage.json'scheckchain, positioned immediately aftersmoke:core-boundaryand before the first live entry, and the suite asserts both of those facts about itself so a later edit cannot quietly undo either. It is not inserted intobin/smoke/ci-suites.txtbecause that file is frozen by position until PR #880, and its round-robin shard walk would re-shard every later suite.CI does not currently execute this suite, and that is an open item rather than something this change resolves. No workflow invokes
pnpm check: grepping.github/workflows/for it returns nothing, while the same grep forpnpm check:docsbundlematchesci.yml:47, so the absence is real rather than a bad pattern.checkis a local developer aggregate. The proof is correct, is exercised by the mutations below, and is reachable by anyone runningpnpm checkor the script directly, but nothing in CI runs it today. The remedy is one step inci.ymlbesidepnpm typecheck, where a broker-free, network-free, roughly one second safety proof belongs, and it is deliberately not in this change because modifying a workflow file needs a credential scope this change's author does not have.Worth being exact about, because presence in something named like a gate is not evidence that anything runs it, and a proof that never executes is indistinguishable from a passing one in every report. #912 measures the scale of the same problem: 88 of 385 gated suites never execute at main's tip, because a shard stops at its first red.
Verification
pnpm smoke:sandbox-guardpnpm smoke:core-boundarypnpm smoke:gate-inventorypnpm typecheckpnpm buildpnpm changeset statusMutation evidence is a committed fixture rather than a description of edits somebody once made by hand:
bin/smoke/mutations/sandbox-guard.jsonholds ten mutations, each naming the assertion it must redden, each pinned to the broker-freepnpm smoke:sandbox-guard. Run it withnode scripts/mutation-proof.mjs --config bin/smoke/mutations/sandbox-guard.json. All ten report KILLED on their named cell. Reverting the emptiness check reddens exactly the two empty-space cells and the roll-up that names them, so the fixture is targeting the behaviour rather than catching collateral from a broadly failing suite.The eighth mutation exists because reviewing the first seven found a gap in them. The guard records four identities and refuses unless all four hold, but only three had a named cell: hardcoding the ownership marker check to
trueleft the suite fully green and the whole registry still reported every mutation killed. The marker guarantee was recorded rather than checked, which is the same fault this change exists to fix, one level up in the evidence. That check now has a cell, and the mutation that used to pass silently reddens it alone. The ninth mutation closes the narrower replacement gap: a marker created after recording has the expected path but foreign identity, and accepting that foreign marker reddens only the named replacement cell. The tenth mutation closes the remaining target-record split: a canonical filename whose document names another space used to satisfy the guard while loadMeshes selected a legacy record for the requested space at a foreign root, and ignoring that document space field reddens only the named cell.Worth stating as a limit on the method: a full registry pass is evidence about the registry. It says every enumerated mutation is caught, not that the surface is enumerated, so the standing question for a fixture like this one is what no entry in it constructs.
That proof has a boundary worth stating: a killed mutation shows the suite depends on the mutated code, not that a real entry point reaches it.
No live suite was run. The destructive CLI call sites were graded by reading because executing the applicable live suites was prohibited. This leaves the real subprocess teardown acceptance path as a named verification gap rather than implying it was exercised.
Reviewing the guard found three defects in it
Each was a correct fix that left a narrower version of the same gap, and the shape is worth recording because all three lived in the seam between a guard and the parser it was imitating.
The first was position: the guard looked for a subcommand at a fixed argv index while the CLI reads positionals. The second was parser semantics: the guard resolved
--spacewithindexOf, taking the first occurrence, while the CLI's parser is last-wins. That one is the serious member of the set, because a repeated flag could have the guard approve one mesh whiledownacted on another. The third was truthiness:typeof "" === "string"satisfied a type check, butdowntreats an empty space as absent and falls through to the current mesh, so--space ""passed a guard whose whole purpose was to require an explicit target.The fix is to stop imitating the parser and call it. The guard now runs
node:util'sparseArgswith the same configuration the CLI uses, so last-wins and--space=forms are decided by the same code that decides them at the call site. Delegating to the resolver proper is not available here, because smoke-kit carries a zero-dependency rail.The comment on the predicate carries the rule that makes it maintainable, and it is an asymmetry rather than an equality: the guard may refuse more values than the CLI honours, but must never accept one the consumer treats as absent. A future edit can then be checked against a stated direction instead of against a reader's memory of which values matter.
Refs #884, and deliberately does not close it. This lands the smoke-side enforcement: the sandbox assumption becomes a checked identity at the destructive call, so a suite that has lost its sandbox refuses rather than reaching the operator's mesh. The product-side ask in that issue remains open, and it is the larger half: a later comment there retracts caller-side marking as sufficient, because a freshly created root has no
.cotalmarker at the momentupresolves, so the walk climbs to the nearest marked ancestor. Marking protects a test suite, which can create its marker before its first CLI call, and cannot protectup, which is the verb that creates the marker. Removing fail-open from the verbs themselves,upfirst and thendown, is not attempted here.