Skip to content

fix(scripts): split ablation-dist-preflight two readings so a DELETE ablation mutate leg can pass - #19640

Merged
os-warren merged 6 commits into
mainfrom
claude/issue-19348-ablation-preflight-absent-mode
Sep 22, 2026
Merged

os-warren merged 6 commits into
mainfrom
claude/issue-19348-ablation-preflight-absent-mode

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #19348

scripts/ablation-dist-preflight.mjs --absent answered two independent questions with one marker and one exit code. Split, in the order this repo prescribes: delete the construct that permits the error, then make the correct shape the only spelling, and only last consider a check.

The defect is one level down from where the card puts it, and that matters

The card says the tree limb "implements only the restore-leg reading -- it exits 1 whenever the working tree is dirty". Measured, that is not what the code did: classifyTree already derived a mutate leg, and a DELETE ablation whose marker occurs in the source passed. The card's premise holds -- a DELETE ablation's mutate leg could not pass its own pre-flight -- but the mechanism is one level down, and finding it is what makes the card's "second, independent trap" the same defect seen from the other side:

The dist/ reading and the tree reading ask about two different artifacts, and a bundler re-spells literals between them. tsup emits label:"Operator" for the source's label: 'Operator' -- different quotes, no space after the colon. One string cannot be both. So the two readings demanded mutually exclusive spellings, and the only invocation that satisfied both at once was the one that proved nothing.

Before -- reproduction, on origin/main at 34545d6

The card's repro names packages/spec/src/ai/skill.form.ts, which is fenced off for this task (every region of packages/spec/src is held by an open PR). The equivalent, stated exactly and reproducible by anyone: a throwaway workspace whose source spells the literal with single quotes and whose dist/ carries the double-quoted emission, carrying the unmodified script and its imports. A DELETE ablation drops the label entry; both files are then re-read on disk (grep -c: source 1 -> 0, dist 1 -> 0) before any verdict is taken.

run marker passed dist/ reading tree reading exit
A label:"Operator" (what dist emits) GREEN "marker absent from all 3 built files" RED "restore leg: ... 1 path still differs from HEAD" 1
B label: 'Operator' (what the source spells) GREEN vacuously -- that spelling was never in dist green, mutate leg 0

Run A is the card's reading, shape for shape. Run B is the card's second trap, and it is the same coin: the spelling that makes the tree reading agree is the spelling that makes the dist/ reading meaningless.

The sharpest form of the before reading is this pair: a correct DELETE mutate leg (run A) and a genuinely unrestored tree (a restore leg with a leaked build-written baseline) print byte-identical tree verdicts and the same exit code. The instrument could not tell them apart.

The ordered remedy

1. Delete the construct that permits the error. Three constructs, all of them "one thing answering two questions":

  • one marker for two artifacts -> --source-marker=SPELLING names what the SOURCE carries. The tree reading probes with it; the dist/ reading never sees it. Omitted, the tree reading falls back to the positional marker -- correct exactly when the build does not re-spell, which is why every invocation in the three documents that state this script's usage keeps working untouched.
  • one leg value for two questions -> classifyTree grows a third leg. restore is now asserted only on a CLEAN tree; dirt no marker explains is indeterminate. Collapsing "you are restoring" into "I cannot tell" was the defect: the failure of an inference is not a finding, and reported as one it told a correct mutate leg to restore the very mutation being measured. indeterminate stays RED -- refusing is the safe direction -- and the refusal now carries both readings with the exact remedy for each.
  • one exit code for two readings -> 1 dist, 2 usage, 3 tree, 4 both. A driver that trusts the status can now tell "the mutation is not in the artifact" from "the tree is not what you think".

A --leg=mutate flag was considered and rejected: the script cannot check a claim, and a mistaken --leg=mutate at a real restore leg would switch off the leaked-artifact catch -- the one assertion here that has already recovered a measured run. --source-marker demands evidence instead, and the control below shows it cannot be used to disarm that catch.

2. Make the correct invocation the only spelling. Unknown options were discarded in silence -- argv.includes('--absent') plus a startsWith('--') filter -- so a mistyped --absnet ran the OPPOSITE mode and printed a verdict that reads exactly like a real one. Unknown options are now a usage refusal; --source-marker has exactly one spelling (joined with =, because with a space its value lands in the marker position); a blank value is refused; and a marker that itself begins with two dashes is now named rather than dropped. The wiring that chooses the tree reading's spelling is a named export, treeReadingMarker, because inline it was the one part of the split no case could reach.

3. The presence pre-condition -- the ruling.

A dist-domain presence check is NOT added, and should not be. The reason is the clock, not an oversight: --absent runs AFTER the rebuild, so the pre-mutation dist/ no longer exists and nothing readable at that moment witnesses that the marker was ever in the artifact. Every surviving proxy is the SOURCE, whose spelling is -- by this very defect -- the one that differs. Such a check would fire on the correct invocation of the card's own repro (run A above), turning a fixed path back into a red one.

A quote-and-whitespace-normalised near-miss probe was the one candidate that could fire, and it was measured against the card's own incident: on that tree it finds nothing, because the construct really did leave dist/. It would not have caught the attempt the card describes. It buys only the narrow case "marker mis-spelled AND dist not rebuilt", at the price of a new false-red class on non-unique markers -- which this file's header already documents as the scan's known weakness. Rejected on that measurement.

The presence pre-condition IS owed, and rung 1 pays it, in the domain where a witness survives. With --source-marker, a DELETE ablation's mutate leg is green only when a tracked path had that literal at HEAD and lost it -- evidence that the construct existed and left -- while the dist/ reading runs in the emitted spelling. Neither reading is vacuous, which is exactly what the measured first attempt lacked. And because the dist-domain half is a reading rather than a check, every --absent pass now prints what it did not prove and names where that reading is taken: this script in default mode on the PRISTINE build, before the mutation.

After -- same corpus, same commands

run invocation dist/ reading tree reading exit
A emitted marker + --source-marker with the source spelling GREEN, non-vacuous green, mutate leg 0
A emitted marker alone GREEN RED "cannot tell which leg this is", both readings named 3
B source spelling alone GREEN, still states what it did not prove green, mutate leg 0

Both directions of the control, because one direction proves nothing

  • The mutate leg passes. Run A with --source-marker, exit 0, tree reading "mutate leg: 1 path differs from HEAD, of which 1 carries the marker".
  • The restore leg still refuses. Source restored, a build-written baseline left dirty: exit 3, and the refusal names packages/demo/generated/baseline.json.
  • The new flag cannot disarm that refusal. The same unrestored tree with --source-marker passed: still exit 3, still names the baseline, and the message sharpens to "the --source-marker you named ... is not the spelling that moved".
  • Instrument-live control. A marker still present in dist/ (label:"Value", 1 hit, known target inside the scanned radius of 3 text files) is RED on the dist/ reading -- so the scan is reading those files, and the greens above are not a dead instrument.

Red-first: two ablations, both taken from a committed tree, both restored to the HEAD blob

There is no dist/ mediation to prove here -- the subject is a scripts/*.mjs file that node loads from source, with no package exports between it and the runner -- so the pre-flight's own hazard does not apply to its own ablation. On-disk landing was proved both ways for each leg (injected text counted to 1, deleted text counted to 0) before any verdict was read.

  1. classifyTree's third leg removed. Self-test: 15 cases RED, including 6 of the 9 in the new split battery. End to end, a genuinely unrestored tree (a dirty generated/baseline.json) was reported "working tree clean against HEAD" at exit 0. Direction observed, stated as observed: this is a false green, which is more severe than main's false red -- treeVerdict's clean branch is now keyed on the leg, so removing the third leg routes unexplained dirt into the clean return rather than into the restore refusal. Both are wrong; the pins fire on either.
  2. treeReadingMarker reduced to return marker. Self-test: 2 cases RED (exactly the wiring pins), and the card's repro under the corrected invocation reverts to exit 3. This is the discriminating reading for the user-visible fix, and no pure-table case reaches it -- which is why the wiring is a named export.

Restore was proved by state, not by exit code, on every leg: worktree blob equal to the HEAD blob, git diff HEAD empty, whole-tree git status --porcelain empty, and the injected text counted back to 0.

Checks

  • node scripts/ablation-dist-preflight.mjs --self-test -- 64 cases, 5 batteries, all pass. Two new batteries, both on the roster and both floored, and SELF_TEST_BATTERY_FLOOR raised 3 -> 5 so deleting one cannot silence it.
  • The 27 gate families scripts/pm/dispatch-gates.mjs derives for this path: all green, and all 27 were green on the pristine tree before the change too, so no verdict here is inherited.
  • pnpm lint (eslint . --no-inline-config, the whole repo, 117s) exit 0 at this branch's head. Not a narrowed run, so no narrowing to justify. Its positive control: a code-shaped line injected inside a block comment in the changed file makes the same command exit 1 with comment-swallow/no-code-inside-block-comment. Worth recording that the first control chosen -- an unused variable -- did not fire: only 2 rules resolve for a scripts/*.mjs file here (no-restricted-imports and the comment-swallow rule), no-unused-vars is not configured, and parserOptions.project is null, so type-aware linting is off.
  • No typecheck family covers scripts/*.mjs; node --check passes and the file is .mjs, not TypeScript.

Changeset

None, deliberately, and this is a departure from the dispatch word -- flagged rather than taken silently. The repo's criterion is whether a published surface moved. Measured: 70 published workspace packages, zero of which name scripts/ or this file in files[] (positive control: the same predicate fires on dist in @objectstack/spec's files[]). So nothing releases. An empty-frontmatter changeset was written and then removed after reading scripts/check-empty-changeset.mjs, which rejects exactly that shape (#5471: an empty changeset is a real input to changesets/action and can take its "All changesets are empty; not creating PR" branch). The correct spelling here is the skip-changeset label, which is the PM seat's act -- I have added no label.

Acceptance notes

Out of scope for this PR, recorded rather than fixed:

  • scripts/ablation-replace.mjs restores from HEAD, not from the pre-mutation working copy, so running it over an uncommitted edit destroys that edit while printing ok restored. Hit during this task: it printed blob 4f6d6941d531 -> 22713e9104d8 going in and blob after restore 591ef264a023 coming out, compared the latter only against HEAD, and reported success. A one-line comparison of the two blobs it already prints would have named it. The ablation discipline does require committing first, which is why this is a gap rather than a contradiction.
  • scripts/ablation-replace.mjs line 538 prints the follow-up pre-flight command for a DELETE ablation using the source anchor as the marker. That is precisely the spelling that makes the dist/ reading vacuous on any package whose build re-spells literals -- the tool hands the author the wrong half of the pair. Fixing it means teaching it --source-marker, which is its file, not this one.
  • This file's header names .claude/skills/dogfood-verification/SKILL.md as one of "three documents that state this script's invocation"; that file no longer mentions the script. Stale prose, no behaviour attached.
  • The three documents that do state the invocation still show only the default (present) form. Now that the default mode on the pristine build is named as the presence reading for a DELETE ablation, that procedure step is worth writing down where authors read it -- outside this PR's fence.

Generated by Claude Code

…E mutate leg can pass

The dist reading and the tree reading ask about two different artifacts, and a
bundler re-spells literals between them. They shared one marker and one exit
code, so the two demanded mutually exclusive spellings: the emitted spelling
made the dist reading true and the tree reading refuse a correct mutate leg,
while the source spelling made the tree reading agree and the dist reading pass
vacuously.

- `--source-marker=<text>` names the spelling the SOURCE carries; the tree
  reading probes with it and the dist reading never sees it.
- `classifyTree` grows a third leg. `restore` is now asserted only on a CLEAN
  tree; dirt the marker cannot explain is `indeterminate` and RED, with both
  readings and the remedy for each. Collapsing those two was the defect.
- One exit code per question: 1 dist, 2 usage, 3 tree, 4 both.
- Unknown options are a usage refusal; they used to be discarded in silence, so
  a mistyped `--absnet` ran the opposite mode.
- Every `--absent` pass now states what it did not prove.

Claude-Session: https://claude.ai/code/session_01UDXER3sdqfeVYpEWZs5mZx
Co-authored-by: Claude <noreply@anthropic.com>
…lf-test

Two new batteries, both on the roster and both floored:

- `argv: the correct invocation is the only one` -- 11 parse cases and 5
  exit-code cases, including the mistyped `--absnet` that used to be discarded
  in silence and the proof that the tree reading's code is not the dist one.
- `the two-marker split: source spelling vs emitted spelling` -- a real git
  corpus replaying the measured shape end to end: the emitted spelling alone
  cannot see the mutation and is INDETERMINATE rather than a bogus restore
  verdict, `--source-marker` turns the same tree into a green mutate leg, and
  a leaked build artifact after a restore still refuses with the flag passed.

Five pure-table cases and one real-git case move from leg `restore` to leg
`indeterminate`, which is the split itself.

Claude-Session: https://claude.ai/code/session_01UDXER3sdqfeVYpEWZs5mZx
Co-authored-by: Claude <noreply@anthropic.com>
…test can see it

`sourceMarker ?? marker` inline in `run()` is the whole split, and it was the
one part of it no case could reach: an ablation that replaced it with `marker`
left every pure-table case green while the card's repro went straight back to
exit 3. `treeReadingMarker()` is that choice as a named export with three pins,
one of which holds the coalescing nullish -- with `||` an empty source marker
would fall back to the emitted spelling and rebuild the defect.

Claude-Session: https://claude.ai/code/session_01UDXER3sdqfeVYpEWZs5mZx
Co-authored-by: Claude <noreply@anthropic.com>
@os-warren os-warren added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 22, 2026 — with Claude

Copy link
Copy Markdown
Collaborator Author

skip-changeset applied by the seat — route 1, and here is the determination it rests on

Check Changeset failed at head 40b0845dc9758ba38847c63a4c73e9bd0a5bfe49 (check run 106603376021) with "This PR adds no changeset." Read at 2026-09-22T03:27Z by the domain:spec seat, session session_01UDXER3sdqfeVYpEWZs5mZx. Labels are the seat's act, ⛔ never the dev's — the dispatch forbade the dev from writing one.

The gate names three routes and the diff decides which, so this was determined rather than judged:

⛔ Route 0 does not apply. That route is for a PR whose only .changeset rows are CHANGED rather than added — it corrects somebody else's pending release note, the label must ⛔ NOT be applied, and the check stays red pending a written confirmation. This PR has zero .changeset rows of any status, so the route is structurally out.

⚠️ Worth saying explicitly, because card #18375 is live and is precisely about this label being able to suppress that refusal: this is not that case. Ruling batch #158 item 1 ② forbids skip-changeset on a PR that edits an existing changeset. This PR edits none.

✅ Route 1 applies — it releases nothing. The whole diff is one file: scripts/ablation-dist-preflight.mjs, modified, +372/−40. Nothing outside scripts/.

Verified rather than assumed, on origin/main:

reading value
root package.json @objectstack/spec-monorepo, private: true, no files key
where scripts/ lives the repo root, i.e. inside that private package — 399 tracked files under it
control — a package that IS published @objectstack/lint, not private, files: ['dist', 'README.md', 'CHANGELOG.md']

⇒ no published package's files reaches the root scripts/ tree, and the package that contains it publishes nothing. The change cannot appear in any release.

⛔ Route 2 was not taken, and the gate's tie-breaker was not needed. The gate says "if you are unsure between routes 1 and 2, take route 2", because a wrong label is caught by review while a wrong empty changeset is caught by nobody. That tie-breaker governs uncertainty; this is a positive determination from the package boundary, not a coin-flip. ⛔ And an empty-frontmatter changeset is not an option in any case — newly added ones are rejected (#5471), because an all-empty set stalls the release silently and greenly (#4898).

⚠️ This note is the record the label owes. A gate-semantic label written without its reason is exactly the shape #18375 exists to close.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 138/138 CONTRACT_REVIEW_TIER
Head-sha: 40b0845dc9758ba38847c63a4c73e9bd0a5bfe49

Stamp control measured by the adopting seat over the reviewer's transcript: all 138 assistant rows carry the model CONTRACT_REVIEW_TIER names, read from origin/main at 2026-09-22T03:41Z. Total, non-zero, no fallback row. The reviewer returned the line with no control, as instructed. Text below is the reviewer's, adopted.

① Derived judgments

1. Re-spelling premise — TRUE in its load-bearing half, FALSE in the half it prints to users. Measured, not accepted. No node_modules and no built dist/ in that checkout, so esbuild 0.28.2 was installed standalone and a fixture compiled under this repo's actual tsup settings (root tsup.config.ts: format esm+cjs, target es2020, no minify; git grep -w minify origin/main over all tracked files returns exactly 2 hits, both the plugin build CLI flag defaulting false, none in any of the 21 tsup configs).
Source label: 'Operator' emits as label: "Operator" — quotes re-spelled single to double, space after the colon PRESERVED. The no-space form appears only under --minify, which this repo never sets.
So the coupling is real: one marker string cannot satisfy both readings, because the quote characters differ. But the clause "drops the space after a colon", new in this PR at header line 179 and again in the user-facing indeterminate refusal at line 641, is wrong for this repo's build. See flag ③-A.

2. Decoupling — it works. Reproduced end to end, both sides. Two fixture repos built (gitignored dist/, as in the real repo), a DELETE ablation run, every exit captured before any pipe:

  • base, emitted spelling: dist GREEN, tree RED restore leg: ... 1 path still differs from HEAD — exit 1. Tells the author to restore the mutation being measured.
  • base, source spelling: dist GREEN vacuously, tree GREEN mutate leg — exit 0. The only exit-0 invocation is the one that proves nothing. Premise confirmed first-hand.
  • head, emitted spelling alone: dist GREEN with the new caveat, tree INDETERMINATE RED naming the --source-marker remedy — exit 3.
  • head, emitted plus --source-marker at the source spelling: dist exact, tree mutate leg green — exit 0.

classifyTree at head is genuinely three-legged: restore is asserted only when files.length === 0; dirt the marker cannot explain is indeterminate, which treeVerdict refuses. Indeterminate no longer masquerades as restore.

3. Exit-code split — coherent, no caller broken. {0,1,2,3,4} is character-for-character the register the sibling tool already uses: scripts/ablation-replace.mjs line 158 exports { OK: 0, EVIDENCE: 1, USAGE: 2, ANCHOR: 3, RESTORE: 4 }. Usage stays 2 at both base and head.
Caller search, radius = all tracked files at origin/main (includes .claude/**, docs, packages, scripts), exit captured before any pipe: 10 hits, every one prose or a printed suggestion — the usage banner, the header, .claude/agents/os-dev.md:255, packages/qa/dogfood/README.md:54, and ablation-replace.mjs:538 which only prints a suggested command line. Zero programmatic invocations whose status is read; no check:* entry, no workflow, no turbo task. Positive control on the same command and scope fired (13 hits for the sibling script). Known target outside the radius: untracked working-tree files and human agents following the prose procedure. Nothing reads status, so re-scoping 1 breaks no caller.

4. ⭐ Presence pre-condition — the asymmetry is JUSTIFIED, not a hedge. The no-witness argument holds independently: dist/ is gitignored at origin/main (.gitignore lines 8, 59, 82) and tsup runs clean: true, so at --absent time the pre-mutation artifact exists neither in the tree nor at HEAD. A dist-domain presence check would have to probe with the source spelling, and case H2 proves that spelling has zero dist hits on a correct run — the check would fire on exactly the invocation it must pass. So there is no sound instrument to add, and none was added.
The source-domain half is real and bites: absent-mode mutate requires some tracked path to have carried the literal at HEAD and lost it, and case H1 refused precisely because none did. Crucially the gap is declared, not silent — every --absent pass now prints "NOT proved by this line: that the marker was ever IN dist/".
One narrowing: the tree limb is repo-scoped while the dist limb is package-scoped. The two limbs do not agree on scope — consistent with the deliberate whole-tree design, but the presence evidence is weaker than the sentence implies.

5. ⭐ Ablations — claim (1) VERIFIED and exact, and a second ablation found a real gap. Both in scratch copies; the head worktree stayed clean at 40b0845 throughout.
Ablation 1 — remove the third leg. On the measured incident shape: classifyTree returns restore, treeVerdict returns ok=true, message "working tree clean against HEAD", paths empty — the leaked path is not even named — and exitCodeFor yields 0. FALSE GREEN, confirmed, and more severe than main's false red. The leg is load-bearing, and the self-test catches it: 15 failures across 3 batteries, exit 1.
Why the direction inverted: base's treeVerdict re-checked files.length === 0 itself before printing the clean line; head moved that guard into classifyTree and the clean branch now trusts leg === 'restore' alone. See flag ③-B.
Ablation 2 — the reviewer's own, severing the production wiring. Changing run() line 772 to probe the tree with marker instead of treeMarker restores the exact pre-fix defect. Self-test: 64/64 pass, exit 0. The split battery's readWith helper reimplements the fallback inline at line 1083 instead of calling treeReadingMarker, and run() is never invoked by the self-test. The one line that is the fix is uncovered. See flag ③-C.

6. SELF_TEST_BATTERY_FLOOR 3 to 5 — a FLOOR, confirmed two-sided. Not gate weakening. The comparison is a minimum. Proved in both directions rather than read: adding a sixth battery beyond the pinned 5 keeps the self-test green (65 cases, exit 0), so it is not an equality or a ceiling; deleting a roster row reds with two distinct messages, exit 1. Raising it tightens. The 64 cases over 5 batteries do run: 26 + 3 + 7 + 19 + 9 = 64, matching the printed count exactly.

7. Semver — skip-changeset is the right route, verified independently. All 83 package.json files at origin/main enumerated and parsed: 70 published (private not true), every one declaring an explicit files[]. None includes scripts or any entry reaching the repo-root scripts/ directory. The root package.json is private: true. No published package is rooted at the repo root, so no tarball can carry this file.
The repo says so itself: .github/workflows/lint.yml line 3458 — "'this PR edits a CI-internal script' is the textbook skip-changeset case".

8. ⚠️ CI — 31 names, 21 success, 10 skipped, 0 failure; every skip rostered. 35 check runs at this head, reduced to latest run per check NAME by job conclusion, never an aggregate.
Four names carry superseded history. Check Changeset ran failure at 03:23:37Z and skipped at 03:26:48Z; latest is skipped, the documented skip-changeset window. No cancelled lane anywhere, so no aggregator green rests on one.
Each of the 10 judged against scripts/pm/check-expected-skips.mjs on origin/main by importing EXPECTED_SKIPS and testing exact string equality, ⛔ not by grepping the file, because a file grep is contaminated by the self-test prose — the control proved it ("Lint & Repo Gates" appears in the file yet is correctly absent from the roster). Result: all 10 rostered, against an 11-row roster. Four positive controls correctly unrostered. The roster tool itself run on this head's payload: exit 0; its own self-test at origin/main: 99 cases, exit 0.
⚠️ The uninterpolated matrix expression. Dogfood Regression Gate (${{ matrix.shard }}/3) is not a malformed name — it is the single check run GitHub creates for a matrix job skipped before matrix expansion, when no shard value exists to substitute. Contrast Test Core (1/6) through (6/6), which expanded and ran. The roster can cover it and does, deliberately: the row is spelled raw, its reason states "the raw matrix template IS the name a job skipped before matrix expansion reports", and the tool's self-test pins that exactly 2 roster rows spell a raw template. ⇒ not rounded to expected; the roster anticipates it explicitly and its own gate verifies that.
⚠️ The aggregate is not the measurement. Dogfood Regression Gate (bare) reports success 6 seconds after start while its 3 member lanes never expanded — an if: always() roll-up. Its green is not evidence the dogfood suite ran. Same for skips whose verdict is deferred to the merge queue (Build Core, Temporal Conformance, flagged REQUIRED in their rows). At this head those lanes are NOT MEASURED, correctly and expectedly, for a scripts/-only diff.
The lanes that bear on this change did run green: Lint & Repo Gates (18 min), all four Type Check · lanes, TypeScript Type Check, Test Core 6/6 shards, Check Documentation Links, and the four claim/queue guards.

Design point, in the change's favour. Adding an optional flag rather than changing the documented invocation is forced and correct: .claude/** is a GOVERNED_TIER_S surface, so two of the three documents stating this script's invocation cannot be edited by a code PR. The plain invocation those documents print stays valid at head, and the --leg= alternative is rejected in the header for the right reason — the script cannot check a declaration, and a mistaken one would switch off the leaked-artifact catch.

② Semver level

None — no published surface moves. The sole changed file is dev-side agent tooling in no published package's files[] (70 published packages enumerated and parsed; root is private: true). No package version, export, type or runtime behaviour changes. The changeset omission is correct and skip-changeset is the route the repo's own workflow prescribes for a CI-internal script. A changeset here would create a release entry for a release that does not exist.

③ Boundary flags

A. A new user-facing message states a fact that is false for this repo's build. "tsup … drops the space after a colon" (header line 179, and the indeterminate refusal at line 641, which is what an author actually reads at the moment of choosing a spelling). Measured: esbuild emits label: "Operator" with the space unless minifying, and no tsup config in this repo sets minify. An author who follows the clause literally passes a positional marker this build never emits; in --absent that is 0 dist hits, which is a pass — the vacuous green this script exists to stop, reachable through its own remedy text. Partly mitigated: the actionable sentence ("the spelling in the source") is correct, and every absent pass now prints the NOT-proved caveat. Text fix, but it belongs in the instrument's own file.

B. The refactor traded defence in depth for a single point whose failure direction is a false green. Base's treeVerdict independently re-checked files.length === 0 before printing the clean line, so the same ablation there could not have produced a false green. At head the clean-green branch trusts leg === 'restore' alone, and when that inference breaks the output is "working tree clean against HEAD" with an empty dirty-path list at exit 0. The self-test does cover it (15 failures), so this is a structural note, not an exposure — but for an instrument whose whole doctrine is that the false green is the dangerous direction, a cheap paths.length === 0 assertion in that branch would restore what base had for free.

C. The one line that constitutes the fix is not covered. Severing run()'s use of treeReadingMarker leaves the self-test at 64/64, exit 0. treeReadingMarker is unit-tested three ways, but the split battery reimplements the fallback inline rather than calling it, and run() is never exercised. Commit 40b0845 is titled "name the tree reading's marker choice so the self-test can see it"; the self-test sees the helper, not the choice as wired. Routing the split battery's readWith through treeReadingMarker would close it in one line.

D. ⛔ Nothing in CI runs this self-test — and this is the PR that most enlarges what that leaves unverified. Zero hits for the script name in package.json, .github/** and turbo.json at origin/main; positive control on the same command and scope fired. The run half legitimately cannot be a gate — it judges a deliberately mutated tree — but the --self-test half can, and the repo has a named precedent: lint.yml documents PR #6876 (#5620) adding assertions to a checker that "never executed once on its own CI" because it carried skip-changeset, with the prescribed remedy a deliberately unconditional self-test step. This PR adds 28 cases and raises the battery floor on a script with no such wiring, and it carries skip-changeset. Pre-existing, not introduced here, not gate weakening — but a maintainer-floor item, and it means the only evidence those 64 cases pass at this head is the reviewer's own hand run.

E. Pre-existing header claim carried forward unverified. The header names .claude/skills/dogfood-verification/SKILL.md as one of three places this script is invoked from; that file mentions it 0 times (148 lines, measured). Identical at base, so not a regression — but this PR rewrote the surrounding header specifically to make the instrument's claims exact, and left a checkable one false.

F. Narrowings, declared. No node_modules and no install in that checkout, so the repo's own build, any check:* gate and any CI lane could not be run. The re-spelling was measured on a standalone esbuild 0.28.2 driven with this repo's documented tsup settings, not on a real package build — the quote normalisation and the minify-only colon behaviour are esbuild-level and stable, but a tsup-level transform could in principle intervene and none was observed. CI was read from the API, not re-executed. ⛔ None of these is banked as green.

Implemented-by: claude/issue-19348-ablation-preflight-absent-mode
Reviewed-by: session_01UDXER3sdqfeVYpEWZs5mZx

VERDICT: PASS


Generated by Claude Code

…Marker

The split battery reimplemented `sourceMarker ?? marker` inline instead of
calling the named export, so the one helper the previous commit added to make
the marker choice visible was the one the battery did not use.

This is the coverage half only; it is measured NOT to close the hole on its
own -- see the commit that follows.

Claude-Session: https://claude.ai/code/session_01UDXER3sdqfeVYpEWZs5mZx
Co-authored-by: Claude <noreply@anthropic.com>
…re the marker choice once

Three boundary findings from the contract review, all in this one file.

A. The header and the indeterminate refusal both said tsup "drops the space
   after a colon". Measured on a real build of @objectstack/platform-objects
   with its own tsup config: dist carries `label: "Operator"` 19666 times and
   `label:"Operator"` 0 times -- the quotes are re-spelled, the whitespace is
   not. The no-space form is what --minify emits (the mirror counts, 0 and
   19666), and none of the 21 tsup configs here sets it. The refusal is what an
   author reads while CHOOSING a spelling, so following it literally produced a
   marker this build never emits: 0 dist hits, which in --absent mode is a PASS.
   Both texts now state what was measured, name the minify caveat as the reason
   the claim is build-dependent, and send the author to grep dist/ instead.

B. The clean branch trusted `leg === 'restore'` alone, so a broken inference
   printed "working tree clean against HEAD" with an empty dirty list at exit 0.
   The clean line now requires the inference AND `files.length === 0`; a
   disagreement between them refuses and names the paths. `classifyTree` is
   injectable so the guard has cases -- with the real inference the two can
   never disagree, and a guard no case can reach proves nothing.

C. `run()` owned a second, untested copy of the marker-choice wiring. Severing
   it -- probing with the emitted marker, i.e. the exact pre-split defect --
   left the self-test at 64/64 exit 0. Routing the split battery's readWith
   through treeReadingMarker() did NOT change that (measured: still 64/64):
   the helper was already pinned three ways, the CALL SITE was the gap. The
   reading is now one exported function, readTreeLeg(), which run() and both
   real-tree batteries call; the same sever now reds it.

Claude-Session: https://claude.ai/code/session_01UDXER3sdqfeVYpEWZs5mZx
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l and removed size/m labels Sep 22, 2026
The `treeMarker` field no caller read was dead surface on a function added to
remove a duplicate, which is the wrong direction.

Claude-Session: https://claude.ai/code/session_01UDXER3sdqfeVYpEWZs5mZx
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 250/250 CONTRACT_REVIEW_TIER
Head-sha: ab0f1b7f693275a2259b4b67b73fe8080b67c1d5

Re-review of the fix round, superseding the PASS at comment 5770982139 (which named head 40b0845dc9). Stamp control measured by the adopting seat over the reviewer's transcript: all 250 assistant rows carry the model CONTRACT_REVIEW_TIER names, read from origin/main at 2026-09-22T04:44Z. Total, non-zero, no fallback row. Text below is the reviewer's, adopted.

① Derived judgments

1. ⭐ Flag C — the previous review's own one-line prescription genuinely does NOT close the hole. Verified in all three readings, and the reason is structural rather than an artifact of the sever chosen.

Reproduced in isolated scratch copies of each commit's scripts/ tree. Each copy first reproduced its commit's pristine reading exactly (prev 64, one-liner 64, head 67, all exit 0). Every exit captured before any pipe; every ablation proved on disk by anchor count 1 → 0 and replacement 0 → 1.

  • (1) The hole, at 40b0845dc9 — sever markerPresence(REPO_ROOT, status.entries, treeMarker) → … marker): 64 PASS / 0 FAIL, exit 0.
  • (2) The one-liner alone, at 652cefe535 — first confirmed that commit IS the prescription verbatim, a one-line diff and nothing else. Same sever: 64 PASS / 0 FAIL, exit 0. STILL GREEN.
  • (3) The real fix, at ab0f1b7f69 — equivalent sever inside readTreeLeg: 66 PASS / 1 FAIL, exit 1. REDS.

The explanation holds and is independently provable from structure, not only from the measurement: the self-test never invokes run() — at both prev and head run()'s only call site is the module entrypoint. A change to the self-test's own helper therefore cannot observe what run() does. The one-liner swapped readWith's inline fallback for a call to treeReadingMarker, but readWith still rebuilt its own markerPresence(...) call, so run()'s wiring stayed uncovered — and treeReadingMarker already carried three direct pins, so the one-liner added no discriminating power whatsoever.

⇒ ⭐ The dev did not over-build — this is a genuine correction of a review prescription. The fix is the minimum change that makes the sever observable: at head markerPresence has exactly one call site, where prev and the one-liner each had three. The refactor removed two inline copies of the wiring rather than adding structure — both batteries got shorter. The correction is recorded in the instrument's own docblock, where the next author reads it.

2. ⭐ Flag A — the clause was false, the re-measurement is exact to the digit, and the vacuous pass it caused is reproducible live.

Real tsup build (tsup 8.5.1, esbuild 0.28.2) of @objectstack/platform-objects, counted over dist/**/*.js:

label: " label:"
unminified (the repo's actual build) 19666 0
--minify, same package and config 0 19666

An exact mirror; a byte dump confirms l-a-b-e-l-colon-SPACE-doublequote. Source: 1371 occurrences of label: '. Census on origin/main: 21 tsup.config.ts, zero minify (exit 1); positive control (format) fires on the same command and scope. ⇒ the quotes re-spell single→double and the space is preserved. This also closes the previous review's own declared narrowing F.

⭐ The clause was load-bearing, proved live on the built package. --absent with the FALSE spelling the old clause taught scores exit 0, GREEN — "marker absent from all 44 built files". The true emitted spelling correctly reds at exit 1 — "marker still present in 6 built files". So an author following the old sentence got a green that proved nothing. Mitigation working: even that vacuous green printed the NOT-proved caveat.

All three corrections verified, including the one the previous review did not name — the split battery's EMITTED constant, teaching the false spelling a third time. The false clause is gone at head — zero hits, positive control ("space") at 22 hits, known target outside the radius being prev, where it survives at lines 178 and 641.

Judged on the stated criterion — is an author now led to grep dist/ rather than trust a sentence? Yes. The header states the generalisable lesson rather than swapping one memorable rule for another, and names the failure mode outright: "Getting the positional marker wrong is not a wrong answer, it is a PASS." The refusal line — the text actually read at the moment of choice — carries "⛔ Do not take either spelling from this sentence". The measured spellings are stated but scoped ("measured here") and placed before the imperative that overrides them. Keeping the clause rather than deleting it is the right call.

3. Flag B — a real defensive guard exercised by legitimate fault injection, ⛔ NOT manufactured coverage.

Under the real classifier leg === 'restore' is exactly equivalent to files.length === 0, so the refusal branch is unreachable while classifyTree is correct — which is precisely the condition the guard exists to stop depending on.

Measured: the guard is reached through the real path the moment the inference breaks, with no injection at all. Ablating the inference, then calling readTreeLeg on a real dirty git repo with the default classifier — at head: ok=false, dirty path named, "the leg inference and the tree disagree"; at prev: ok=true, paths: [], "working tree clean against HEAD" — the false green.

It also fails the AGENTS.md phantom-check test in the right direction: a phantom check "evaluates never, and deleting it leaves every gate just as green", whereas treeIsClean is evaluated on every run and severing it reds 65 PASS / 2 FAIL, exit 1. The injection adds no risk surface: readTreeLeg never forwards classify, so the CLI path always takes the default, and there are zero programmatic importers anywhere on origin/main (radius: all tracked files; positive control fired on the sibling script).

4. Counts and floors — tightened, nothing loosened. Pure table 26 → 29, the other four unchanged, SELF_TEST_BATTERY_FLOOR unchanged at 5; 29+3+7+19+9 = 67, matching the printed count. Floors still bind, two-sided at head. parseArgs, exitCodeFor and treeReadingMarker are byte-identical prev vs head, so the argv strictness pinned last round is untouched.

5. Gates — a DERIVED zero, derived independently here. 27 commands derived, same change set (517 changed lines, under the 5000 threshold). Reconciliation exit 0, "27 derived, 27 run, 0 NOT-MEASURED, 0 UNRUN", with the tool disclaiming "⛔ This tool ran none of them; it read the codes you recorded." Control: withholding one row flips it to exit 1 — the reconciliation bites rather than asserting. The staleness warning was read rather than ignored, correctly: the one commit touching dispatch-gates.mjs adds only blocks explicitly "⛔ NOT runnable and ⛔ never in --commands".

6. The two disclosed NOT MEASURED — prerequisite refusals confirmed, and one is MISLABELLED. Neither is in the derived 27. See ③-A.

7. Lint — reproduced, with a firing control. Whole-repo at head: exit 0, 6993 files, 0 errors, 0 warnings — the dev's figures exactly. Positive control: injecting a restricted import makes the same command exit 1; the file was restored and proved byte-identical by blob hash.

8. ⚠️ CI — 31 names, 23 success, 8 skipped, 0 failure, 0 pending; every skip rostered. Raw payload fetched independently: 31 rows, 31 distinct names, no superseded history and no cancelled lane, so no aggregate green rests on one.
All 8 judged by importing EXPECTED_SKIPS from origin/main and testing exact string equality — ⛔ not by grepping, which that file's own self-test prose contaminates: 8/8 rostered. Five positive controls correctly unrostered. The roster tool's own self-test: 99 cases, exit 0; run on this head's real payload: exit 0.
⚠️ The uninterpolated matrix name — verified, not inherited. The roster spells it raw and its reason reads "the raw matrix template IS the name a job skipped before matrix expansion reports". Deliberate coverage, gate-verified by the tool's own self-test.
⛔ No aggregate stands in for member lanes. The bare Dogfood Regression Gate reports success under if: always() while its 3 shards never expanded — its green is not evidence the suite ran. Build Core and Temporal Conformance are REQUIRED contexts whose verdicts the roster defers to the merge-queue build; at this head they are NOT MEASURED, correctly, and their absence is ⛔ not a clearance.

② Semver level

None — no published surface moves. All 83 manifests enumerated and parsed: 13 private, 70 published, every one declaring an explicit files[], and zero naming scripts/ or reaching the repo-root scripts/ tree. Positive control: @objectstack/spec's files[] includes dist, on which the same predicate fires. The diff is one file (+469/−48 vs merge base). skip-changeset remains applied, Check Changeset skipped and rostered, reasoning at 5770778611. scripts/ is not on the governed register and Governed Surface Queue Guard is green.

③ Boundary flags

A. ⚠️ A NOT-MEASURED disclosure labels a FINDINGS exit as a prerequisite refusal. The fix-round report disclosed 「check:dts-closure exit 1 and check:published-readme-exports exit 3 both refuse for a missing prerequisite」. Exit 3 is that gate's prerequisite code; exit 1 is its findings code, and the gate pins the distinction in its own self-test precisely so the two cannot be conflated. The real cause was reproduced: on a cleanly unbuilt tree check-dts-closure answers exit 3; after building one package whose .d.ts were skipped it answers exit 1 with a genuine finding — "1 built package(s) are MISSING declaration files their own package.json promises." So the exit 1 came from the dev's own partially built worktree, ⛔ not from a prerequisite refusal.
The substantive conclusion survives — neither gate is in the derived 27, both read published .d.ts surfaces that 0 of 70 published packages expose to this diff, and CI builds fresh — but the label is wrong in the exact direction this instrument exists to police: "could not measure" reported where "measured, and found something" is what happened. Report-level, ⛔ no code defect.

B. ⚠️ Flag B's justification is stated one step too strong in the report. It argues classifyTree had to become injectable because otherwise the guard "would be a phantom check in the sense AGENTS.md names". It would not: a phantom check never evaluates, whereas treeIsClean is computed on every run — only the refusal branch is unreachable under a correct classifier, and the honest reason for injection is that a defensive branch cannot be unit-pinned without supplying the fault it defends against. ⭐ The code's own comment states this correctly; only the prose overreaches. Recorded because the reasoning, ⛔ not just the conclusion, is what the next author inherits.

C. Narrowings, declared. The 13 pnpm check:* families of the derived 27 were not run here and are ⛔ not banked as green; 4 of the 14 direct-node families refuse at exit 3 for a dependency missing from this tree. CI was read from the API and the raw payload, ⛔ not re-executed. The real tsup build ran with OS_SKIP_DTS=1 — a full dts build fails in this tree for want of built siblings — which does not touch the literal re-spelling measurement, an esbuild-level transform on the JS output. The dev's own stated lint control did not fire in the shape tried; instrument liveness was established instead through the other resolving rule, on the same command and scope.

Implemented-by: claude/issue-19348-ablation-preflight-absent-mode
Reviewed-by: session_01UDXER3sdqfeVYpEWZs5mZx

VERDICT: PASS


Generated by Claude Code

@os-warren
os-warren marked this pull request as ready for review September 22, 2026 05:14
@os-warren
os-warren enabled auto-merge September 22, 2026 05:14
@os-warren
os-warren added this pull request to the merge queue Sep 22, 2026
Merged via the queue into main with commit 79854f7 Sep 22, 2026
33 checks passed
@os-warren
os-warren deleted the claude/issue-19348-ablation-preflight-absent-mode branch September 22, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ablation-dist-preflight --absent exits 1 on any dirty tree, so a DELETE ablation's mutate leg can never pass its own pre-flight

2 participants