Skip to content

Commit 357f678

Browse files
committed
fix(devx): make --require-stamp cover the spec side, so check:console-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
1 parent 6abc4df commit 357f678

1 file changed

Lines changed: 172 additions & 12 deletions

File tree

‎scripts/check-console-injection.mjs‎

Lines changed: 172 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@
7272
*
7373
* Paying for the build instead does not rescue it, because the headline scenario
7474
* is one this gate deliberately does not test. With the dist and stamp held
75-
* fixed and only the tree varying: spec unbuilt PASSES (expiry not re-checked),
75+
* fixed and only the tree varying: spec unbuilt now FAILS under --require-stamp
76+
* (objectstack#10428; it used to pass with the expiry check silently skipped),
7677
* spec unchanged PASSES, spec MOVED FORWARD PASSES, and only a spec that has
7778
* caught up to the published text FAILS. "Spec moved forward since the dist was
7879
* built" is precisely what a spec trigger would be bought for, and PR
@@ -84,13 +85,34 @@
8485
* tree STATE, not an event, so once it is true today's 6/100 console runs still
8586
* catch it. Widening the filter buys latency, not coverage.
8687
*
87-
* The exit, for whoever asks a third time: objectstack#10428 proposes deriving
88-
* the expiry probe from packages/spec SOURCE text — `describe()` arguments are
89-
* plain string literals — which would make that one assertion BUILDLESS. The
90-
* light job is worthless only because its single meaningful assertion needs a
91-
* build; remove the build and this question reopens on entirely different terms.
92-
* Full working — the paths-filter replay under both picomatch versions, the
93-
* per-commit attribution — is on objectstack#9710's ruling comment.
88+
* The exit proposed for whoever asks a third time was to derive the expiry probe
89+
* from packages/spec SOURCE text — `describe()` arguments are "plain string
90+
* literals present in both" — making that one assertion BUILDLESS and reopening
91+
* the light-job question on different terms. That was PRICED under
92+
* objectstack#10428 and DECLINED. The premise is 98% true and the missing 2% is
93+
* the wrong 2%: of 2993 `.describe()` probe candidates in a built spec, 44 do not
94+
* appear anywhere in `src/**` as literal text (59, or 2.0%, against the narrower
95+
* `.zod.ts` source subset the package actually publishes), by two irreducible
96+
* bundler transforms:
97+
*
98+
* - 25/44 quote-and-escape normalisation. Source writes `'…definition\'s…'`;
99+
* esbuild re-emits `"…definition's…"`. Same characters, different bytes, so a
100+
* literal substring search over source text misses it.
101+
* - 19/44 constant-folded concatenation. Source splits a long description as
102+
* `'… declares ' + '`_packageId`.'` (api/protocol.zod.ts:341-342 is one);
103+
* the bundler folds it to one literal that exists in no source file.
104+
*
105+
* A missed probe here reads as "not expired" — a SILENT PASS, the same failure
106+
* class this gate exists to end, so the 2% lands on exactly the side that cannot
107+
* be tolerated. The reverse channel is worse-behaved still: 260 of 3161 source
108+
* candidates (8.2%) are description text the built package does not ship, and a
109+
* detector matching one of those would report EXPIRED on a healthy tree.
110+
* Symmetric src-vs-src derivation would cancel both, but that is a redesign of
111+
* what assert-console-spec-injection.mjs stamps — and it still cannot make the
112+
* BUNDLE side buildless, which is where the dist dependency actually lives.
113+
* So the dependency stays and is made MANDATORY instead: see --require-stamp
114+
* below. Full working — the paths-filter replay under both picomatch versions,
115+
* the per-commit attribution — is on objectstack#9710's ruling comment.
94116
*
95117
* ## Failure response: FAIL, deliberately, rather than rebuild
96118
*
@@ -101,9 +123,31 @@
101123
* rejected. Failing once, with the eviction command spelled out, is the cheaper
102124
* and more honest response. Every failure below names its remedy.
103125
*
126+
* ## What --require-stamp requires (objectstack#10428)
127+
*
128+
* The flag's job is to refuse a VACUOUS pass: a caller passing it has asserted
129+
* that this run's green means something. That covered the dist side only — a
130+
* missing dist, an unstamped dist — and left the tree side open, so a run with
131+
* packages/spec unbuilt skipped the expiry re-check, printed an `ℹ`, and exited
132+
* 0. Five of six verdicts here are pure functions of the restored dist and its
133+
* stamp; the expiry re-check is the only one that reads the checkout. A run that
134+
* skips it cannot fail on anything about the PR it is guarding.
135+
*
136+
* ci.yml gets a readable spec today only by STEP ADJACENCY — the Console Pin
137+
* Gate happens to run `turbo run build --filter=@objectstack/client...` first,
138+
* and @objectstack/spec is in that closure. Nothing enforced the ordering, so
139+
* reordering the steps or calling this from a job without that build turned the
140+
* assertion off silently. --require-stamp now covers the tree side too, which
141+
* makes the ordering a contract instead of a coincidence and needs no new flag
142+
* at the call site — a flag a new job could forget is the same failure again.
143+
* The bare invocation is unchanged: a checkout with no built spec still gets the
144+
* advisory notice, because there the five bundle assertions genuinely do stand
145+
* on their own.
146+
*
104147
* Exit codes:
105148
* 0 verified; or no dist to verify; or an unstamped dist without --require-stamp
106-
* 1 the restored dist is not one this repo can vouch for (see the message)
149+
* 1 the restored dist is not one this repo can vouch for (see the message); or,
150+
* under --require-stamp, a run that could not make an assertion it promised
107151
* 2 cannot run (unreadable assets, malformed stamp)
108152
*/
109153

@@ -214,8 +258,10 @@ export function evaluate({ distDir, specDir, requireStamp = false, cacheKey = ''
214258
}
215259

216260
// The tree's own spec, for the expiry re-check. Absent when spec is not built
217-
// — a real state for a bare checkout, and not a reason to fail: the bundle
218-
// assertions below stand on their own.
261+
// — a real state for a bare checkout, and not a reason to fail on its own: the
262+
// bundle assertions below stand without it. Under --require-stamp it IS a
263+
// reason to fail, but only once we know a probe was actually skipped; that is
264+
// counted in the loop and answered after it (see `expiryDeferred`).
219265
let treeBlob = null;
220266
let treeBlobNote = '';
221267
try {
@@ -226,6 +272,7 @@ export function evaluate({ distDir, specDir, requireStamp = false, cacheKey = ''
226272
}
227273

228274
let asserted = 0;
275+
let expiryDeferred = 0;
229276
for (const entry of stamp.packages) {
230277
const name = entry?.name || '<unnamed>';
231278

@@ -274,6 +321,13 @@ export function evaluate({ distDir, specDir, requireStamp = false, cacheKey = ''
274321
// Expiry. A stale detector is evidence only while it still separates the two
275322
// specs; once this tree's spec also contains it, a bundle that "passes" is
276323
// proving nothing at all.
324+
//
325+
// Count the probes this run could NOT re-check, rather than reading
326+
// `asserted` afterwards: an entry can have skew and still carry no stale
327+
// detector, and such an entry has no expiry question to defer. Only a probe
328+
// that exists and went unexamined is a skipped assertion.
329+
if (staleDetector && !treeBlob) expiryDeferred += 1;
330+
277331
if (staleDetector && treeBlob && treeBlob.includes(staleDetector)) {
278332
err.push(
279333
`✗ The stamped staleness probe for ${name} has EXPIRED.`,
@@ -301,7 +355,46 @@ export function evaluate({ distDir, specDir, requireStamp = false, cacheKey = ''
301355
if (staleDetector) out.push(` absent (published only): "${staleDetector}"`);
302356
}
303357

304-
if (treeBlobNote && asserted > 0) {
358+
// A probe that was never re-checked is an assertion this run did not make.
359+
// Advisory by default — a bare checkout legitimately has no built spec, and
360+
// the five bundle assertions above still stand. Fatal under --require-stamp,
361+
// for the reason that flag exists: five of this gate's six verdicts are pure
362+
// functions of the restored dist and its stamp, and the expiry re-check is the
363+
// ONLY one that reads this tree. Skipping it leaves a run that cannot fail on
364+
// anything about the checkout it is guarding, which is the vacuous pass
365+
// --require-stamp already refuses one layer in (objectstack#10428).
366+
//
367+
// Exit 1, not 2, deliberately: every other --require-stamp refusal here (no
368+
// dist, no stamp) is a 1, and they are the same kind of statement — the run
369+
// could not prove what this caller promised was provable. 2 stays reserved for
370+
// a tree this script cannot read at all.
371+
if (expiryDeferred > 0) {
372+
if (requireStamp) {
373+
err.push(
374+
`✗ ${expiryDeferred === 1 ? 'A stamped staleness probe was' : `${expiryDeferred} stamped staleness probes were`} never re-checked for expiry.`,
375+
'',
376+
` ${treeBlobNote}`,
377+
'',
378+
' This gate has six failure verdicts and five of them read only the restored',
379+
' dist and its stamp. The expiry re-check is the one that reads THIS TREE, so',
380+
' a run without it cannot fail on anything about the checkout it guards — it',
381+
' would report green while asserting nothing about this PR.',
382+
'',
383+
' --require-stamp callers have asserted that this run is one whose verdict',
384+
' means something, so the skip is a failure here rather than the notice a',
385+
' bare checkout gets.',
386+
'',
387+
' How to clear this: build the spec before this step.',
388+
'',
389+
' pnpm --filter @objectstack/spec build',
390+
'',
391+
' In ci.yml the Console Pin Gate already does, via',
392+
' `turbo run build --filter=@objectstack/client...` (@objectstack/spec is in',
393+
' that closure). If you are seeing this there, the steps were reordered or',
394+
' this check moved to a job that does not build the closure.',
395+
);
396+
return { code: 1, out, err };
397+
}
305398
out.push(`ℹ Probe expiry not re-checked: ${treeBlobNote}`);
306399
out.push(' (build the spec — `pnpm --filter @objectstack/spec build` — to enable it)');
307400
}
@@ -335,6 +428,23 @@ function makeSpecPkg(dir, descriptions) {
335428
return dir;
336429
}
337430

431+
/**
432+
* A spec package that has never been built: the manifest and its exports map are
433+
* there, `dist/` is not. This is what a bare checkout looks like, and what the
434+
* Console Pin Gate would look like if its build step were reordered away.
435+
*/
436+
function makeUnbuiltSpecPkg(dir) {
437+
fs.mkdirSync(dir, { recursive: true });
438+
fs.writeFileSync(
439+
path.join(dir, 'package.json'),
440+
JSON.stringify({
441+
name: '@objectstack/spec',
442+
exports: { '.': { import: { types: './dist/index.d.mts', default: './dist/index.mjs' } } },
443+
}),
444+
);
445+
return dir;
446+
}
447+
338448
/** A minimal console dist: index.html, one JS asset, optionally a stamp. */
339449
function makeDist(dir, assetText, stamp) {
340450
fs.mkdirSync(path.join(dir, 'assets'), { recursive: true });
@@ -440,6 +550,56 @@ function selfTest() {
440550
expect('no dist fails when required', evaluate({ distDir: dist, specDir, requireStamp: true }).code, 1);
441551
}
442552

553+
// 7b. THE SECOND VACUITY PATH (objectstack#10428): an unbuilt spec means the
554+
// expiry re-check — the only one of six verdicts that reads this tree —
555+
// never ran. Advisory on a bare checkout, fatal under --require-stamp.
556+
//
557+
// Asserted on the REJECT side on purpose. "No error" proves nothing here:
558+
// before the fix this fixture exited 0 with an `ℹ`, which is precisely a
559+
// green that asserts nothing, so only a red and its branch-unique wording
560+
// can tell the fixed script from the broken one.
561+
{
562+
const unbuilt = makeUnbuiltSpecPkg(path.join(root, 'spec-unbuilt'));
563+
const dist = makeDist(path.join(root, 'unbuilt-tree'), `console(${JSON.stringify(FRESH)})`, stampFor());
564+
565+
expect('unbuilt spec is advisory by default', evaluate({ distDir: dist, specDir: unbuilt }).code, 0);
566+
567+
const r = evaluate({ distDir: dist, specDir: unbuilt, requireStamp: true });
568+
expect('unbuilt spec is fatal under --require-stamp', r.code, 1);
569+
const text = r.err.join('\n');
570+
checked += 1;
571+
if (!text.includes('never re-checked for expiry')) {
572+
failures.push('skipped-expiry failure must say the probe was never re-checked');
573+
}
574+
checked += 1;
575+
if (!text.includes('pnpm --filter @objectstack/spec build')) {
576+
failures.push('skipped-expiry failure must name the build that clears it');
577+
}
578+
579+
// POSITIVE CONTROL. The normal CI ordering — spec built before the check —
580+
// must still pass under the same flag. Without this the case above would be
581+
// satisfied by a script that fails --require-stamp unconditionally.
582+
expect(
583+
'positive control: built spec still passes under --require-stamp',
584+
evaluate({ distDir: dist, specDir, requireStamp: true }).code,
585+
0,
586+
);
587+
588+
// PRECISION. The refusal is scoped to a probe that actually went unexamined,
589+
// not to "the spec is unbuilt". A stamp recording no skew has no expiry
590+
// question to skip, so an unbuilt tree is still a clean pass for it —
591+
// otherwise this would fail runs over an assertion nobody was owed.
592+
expect(
593+
'no-skew stamp does not fail on an unbuilt spec',
594+
evaluate({
595+
distDir: makeDist(path.join(root, 'noskew-unbuilt'), 'console("anything")', stampFor({ skew: false, freshWitness: null, staleDetector: null })),
596+
specDir: unbuilt,
597+
requireStamp: true,
598+
}).code,
599+
0,
600+
);
601+
}
602+
443603
// 8. A build that found no skew records it, and this gate says so honestly.
444604
{
445605
const dist = makeDist(

0 commit comments

Comments
 (0)