Skip to content

fix(devx): make --require-stamp cover the spec side, so check:console-injection can't skip its only tree-sensitive assertion - #10597

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10428-console-injection-spec-blob
Aug 21, 2026
Merged

fix(devx): make --require-stamp cover the spec side, so check:console-injection can't skip its only tree-sensitive assertion#10597
os-zhuang merged 1 commit into
mainfrom
claude/issue-10428-console-injection-spec-blob

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10428

check:console-injection has six failure verdicts. Five are pure functions of the restored console dist and its stamp; exactly one reads this tree — the probe-expiry re-check:

if(staleDetector&&treeBlob&&treeBlob.includes(staleDetector)){EXPIRED}

treeBlob comes from readSpecBlob(specDir), which resolves packages/spec's exports map and therefore needs packages/spec/dist/**. With the spec unbuilt, readSpecBlob throws ProbeError, the script catches it, treeBlob stays null, the expiry branch is skipped — and the run exits 0 printing an . --require-stamp exists precisely to refuse a vacuous pass, but it covered the dist side only, so a run could satisfy it while asserting nothing whatsoever about the checkout it guards.

Honest scope: this is not live today.ci.yml's Console Pin Gate runs turbo run build --filter=@objectstack/client... before the check and @objectstack/spec is inside that closure. But that is step adjacency, not a contract — reorder the steps, or call this from a job that does not build the closure, and the only tree-sensitive assertion turns off silently while still reporting green. This PR converts the coincidence into a contract.

Option 3 was priced first, and declined on measurement

The card's cheaper variant — derive the expiry probe from packages/spec/src/** source text, making the assertion buildless — is strictly better than enforcing a dependency, so it was measured before anything was written. It does not hold. Its premise, that .describe() arguments are "plain string literals present in both", is ~98% true, and the missing 2% falls on the side that cannot be tolerated.

Driving the real describeCandidates() / readSpecBlob() over a freshly built packages/spec (dist blob 9,693,406 bytes; 595 non-test source files):

channelmeasurementconsequence
dist probe candidates2993the realistic probe pool
absent from src/** as literal text44 (1.5%) — 59 (2.0%) against the narrower published .zod.ts subsetprobe reads "not expired" ⇒ silent pass
src candidates absent from dist260 of 3161 (8.2%)detector matching one ⇒ EXPIRED on a healthy tree

The 44 split into two irreducible bundler transforms, both verified by hand:

  • 25/44 — quote-and-escape normalisation. Source writes '…definition\'s…'; esbuild re-emits "…definition's…". Same characters, different bytes, so a literal substring search over source misses it. (20 of the 25 have their pre-quote prefix present in source, confirming the text is there, only spelled differently.)

  • 19/44 — constant-folded concatenation. Source splits long descriptions across lines and the bundler folds them into one literal that exists in no source file. Concretely, packages/spec/src/api/protocol.zod.ts:341-342:

    'Owning package machine id. Present only when the resolved item declares '+'`_packageId`.',

A missed probe here reads as "not expired" — a silent pass, the same failure class this gate exists to end. Symmetric src-vs-src derivation would cancel both channels, but that is a redesign of what assert-console-spec-injection.mjs stamps, and it still cannot make the bundle side buildless, which is where the dist dependency actually lives. So the dependency stays and is made mandatory instead. The full measurement is recorded in the script header so the next person does not re-price it.

The change

--require-stamp now also requires that every stamped staleness probe was actually re-checked. Deliberately scoped to a probe that exists and went unexamined, not to "the spec is unbuilt": a stamp recording no skew has no expiry question to skip and still passes. Exit 1, matching every other --require-stamp refusal (no dist, no stamp) — 2 stays reserved for a tree the script cannot read at all.

No ci.yml change, by design. The requirement rides the flag the gate already passes, so it cannot be forgotten — a new --require-spec-blob that a future job neglects to pass would reproduce exactly the failure this card is about. The bare invocation is untouched: a checkout with no built spec keeps the advisory , because there the five bundle assertions genuinely do stand on their own.

Validation

Four rows driven through the real exported evaluate(), dist and stamp held fixed, only the tree varying. Both flag modes, because the card's table did not state one:

Before

tree staterequireStamp: falserequireStamp: true (what ci.yml runs)
spec not builtPASSPASSexpiry check skipped
spec built, unchangedPASSPASS
spec built, moved forwardPASSPASS
spec built, caught upFAIL(1) — EXPIREDFAIL(1) — EXPIRED

After

tree staterequireStamp: falserequireStamp: true (what ci.yml runs)
spec not builtPASS (advisory , unchanged)FAIL(1)✗ A stamped staleness probe was never re-checked for expiry.
spec built, unchangedPASSPASS
spec built, moved forwardPASSPASS
spec built, caught upFAIL(1) — EXPIREDFAIL(1) — EXPIRED

Row 1 changes verdict under the flag CI actually passes; rows 2-4 are untouched in both modes, and the bare mode is untouched entirely.

Reject side asserted positively. A "no error" test proves nothing here — before the fix this fixture exited 0 with an , which is itself a green that asserts nothing. Self-test case 7b constructs the unbuilt-spec tree (makeUnbuiltSpecPkg: real manifest and exports map, no dist/) and asserts the red plus its branch-unique wording, alongside a positive control (built spec, same flag, still passes — without which the case would be satisfied by a script that fails --require-stamp unconditionally) and a precision case (no-skew stamp on an unbuilt tree still passes).

Ablation. Reverting only the new refusal (if (expiryDeferred > 0)if (expiryDeferred > 0 && !requireStamp)) turns the self-test red with exactly the predicted 3 failures, and no others:

✗ check-console-injection --self-test -- 3 failure(s)
unbuilt spec is fatal under --require-stamp: expected 1, got 0
skipped-expiry failure must say the probe was never re-checked
skipped-expiry failure must name the build that clears it

The mutation was confirmed on disk before the run (removed-text count 1→0, injected-text count 0→1, plus the diff hunk), and the restore leg was confirmed the same way (mutation text absent, original present, worktree clean against the commit). No build is involved — these scripts run from source, nothing resolves through a dist/.

Real-CLI control, against the actual built packages/spec rather than a fixture: --require-stamp with the real spec exits 0; the same dist and stamp with an unbuilt spec exits 1 with the new message.

Gates, derived from the real change set via node scripts/pm/dispatch-gates.mjs (not a recalled list) at 357f67864b, all green:

  • pnpm check:console-injection✓ … --self-test: 27 assertions over real fixture trees (real evaluate() path)
  • pnpm check:cross-package-test-inputsAll 60 self-test cases passed. / OK: 12 package(s) read outside themselves…
  • node scripts/check-nul-bytes.mjsOK (scanned 6187 text file(s) … no raw ASCII control bytes)
  • eslint scripts/check-console-injection.mjs --no-inline-config — exit 0

Changeset

This PR carries no changeset and takes the skip-changeset label instead. It changes one repo-internal CI gate script; the root package is private: true and no published package ships scripts/**, so it releases nothing. The gate's own error text names that as the skip-changeset case, and explicitly rules out an empty-frontmatter changeset as an alternative.

Out of scope

Filed #10595 (unassigned, finding): --require-stamp still passes on a stamp whose packages array is empty — the same "satisfies the flag while asserting nothing" shape, one input over. Not reachable from the real producer today, and whether it should refuse is a judgement call rather than something existing evidence pins, so it is recorded rather than fixed here.

Refs: #9710 (where this was measured) · #9706 · #9667 · #8134

Generated by Claude Code


Generated by Claude Code

…-injection cannot skip its only tree-sensitive assertion
check:console-injection has six failure verdicts; five are pure functions of
the restored console dist and its stamp, and exactly one reads this tree —
the probe-expiry re-check. That one needs packages/spec/dist, because
readSpecBlob resolves the package's exports map. With the spec unbuilt,
readSpecBlob throws ProbeError, the script catches it, treeBlob stays null,
the expiry branch is skipped and the run exits 0 printing only an info line.
--require-stamp exists precisely to refuse a vacuous pass, but it covered the
dist side only. So a run could satisfy it while asserting nothing about the
checkout it guards. ci.yml gets a readable spec today by STEP ADJACENCY —
the Console Pin Gate happens to run turbo build --filter=@objectstack/client...
first — not by any contract; reorder the steps and the assertion turns off
silently while still reporting green.
--require-stamp now also requires that every stamped staleness probe was
actually re-checked. Scoped to a probe that exists and went unexamined, not to
"the spec is unbuilt": a stamp recording no skew has no expiry question to
skip and still passes. The bare invocation is unchanged, so a checkout with no
built spec keeps the advisory notice. No ci.yml change: the requirement rides
the flag the gate already passes, and a flag a new job could forget would
reproduce the same failure.
Deriving the probe from packages/spec source text instead was priced first and
declined; the measurement is recorded in the file header.
Refs #9710, #9706, #9667, #8134
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT, arming. Option 3 was priced honestly and the refusal to take it is the right call.

Reviewed against the tree and the PR, not against the report. Head 357f6786, 28/28 checks green, 0 failing, 0 running (latest run per check name).

⭐ The option-3 pricing is the substance here

My brief told you to price the buildless route first and report the numbers even if you rejected it. You did, and the premise I handed you turned out to be false in exactly the direction that matters:

.describe() arguments are plain string literals present in both

True for 98.5% of the probe pool. The 1.5% that is not (2.0% against the published .zod.ts subset) fails as a false negative — a missed probe reads as "not expired", i.e. a silent pass, which is the one direction this gate exists to eliminate. With two named mechanisms rather than a hand-wave:

  • 25/44 quote-and-escape normalisation — source writes '…definition\'s…', esbuild re-emits "…definition's…". Same characters, different bytes. 20 of the 25 have their pre-quote prefix present in src, which is what proves the text is there and merely spelled differently.
  • 19/44 constant-folded concatenationprotocol.zod.ts:341-342 writes 'Owning package machine id. Present only when the resolved item declares ' + '\_packageId`.'`; the bundler folds it into a literal present in no source file.

And you reproduced the counts against an independently built spec in a sibling worktree, so this is structural rather than dist/src skew. ⭐ Recording the measurement in the script header so the next asker does not re-price it is worth as much as the fix.

On rejecting option 2 — I under-specified, you corrected it

My brief presented options 1 and 2 as near-equivalent. Your correction stands and is the sharper reading:

option 2 (--require-spec-blob passed in ci.yml) reproduces this card's own failure mode one level up — an invariant a call site can forget

That is precisely the "step adjacency, not a contract" problem the card is about. A fix that re-creates the defect one level out is not a fix. Riding the flag the gate already passes means no ci.yml change at all, which also keeps the Console Pin Gate step and the #9465 changeset fence untouched — cleanest possible blast radius for a card of this kind.

What earns the ACCEPT

  • Scoped to the right condition. The refusal fires on a probe that exists and went unexamined (expiryDeferred), not on "the spec is unbuilt". A no-skew stamp has no expiry question to skip and still passes — that precision control is what stops this from becoming a gate that reddens over an assertion nobody was owed.
  • Both four-row tables driven through the real exported evaluate(), in both flag modes, dist+stamp fixed and only the tree varying. Row 1 flips; rows 2–4 do not. That is the whole claim, shown rather than asserted.
  • The reject side is asserted positively (case 7b builds a real unbuilt-spec tree and pins branch-unique wording), with your reason for why the weaker form would have been worthless: "before the fix that fixture exited 0 with an info line, which is itself a green asserting nothing."
  • Two levels of positive control, including a real-CLI run against the actual 9.7 MB built spec — so case 7b cannot be satisfied by a script that fails --require-stamp unconditionally.
  • Ablation predicted before running (red, exactly 3 failures, controls staying green) and matched exactly; mutation confirmed on disk by grep -c of both removed and injected text, not by an editor's exit code.
  • ⭐ You corrected the #9710 ruling prose in the header that your own change falsified ("spec unbuilt PASSES"). Leaving a comment that contradicts the code it sits on is how the next reader gets misled; catching it in the same diff is the standard.
  • check:console-injection --require-stamp still passes on a stamp with an empty packages array #10595 filed rather than folded, with the reason it failed the in-scope test (not reachable from the real producer; whether it should refuse is a judgement call existing evidence does not pin). Correctly filed unassigned — I have triaged it into the queue.

Your open question — A, ratified. skip-changeset stays.

And the error was mine, not a judgement call you needed to escalate: AGENTS.md:943 already says"Pure bug fixes do not require a changeset", pr-automation.yml's Check Changeset job names the label as the explicit "declares no release of its own" exemption, and PR #10502scripts/**-only, skip-changesetmerged today. My brief's "a changeset is expected" line was a template defect that would have fabricated a release on every scripts-only card. Three agents contradicted it tonight; it is fixed at the source.

Noting your read-back discipline on the label (['size/m','skip-changeset'], additive POST preserving the bot's write) — that is the #5533 failure mode, and checking it rather than assuming is right.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 04:54
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 71a0198Aug 21, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10428-console-injection-spec-blob branch August 21, 2026 05:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check:console-injection silently drops its only tree-sensitive assertion when packages/spec is unbuilt — and nothing requires the spec blob

2 participants

@os-zhuang@claude