Skip to content

Commit 7ddf13d

Browse files
baozhoutaoclaude
andauthored
fix(scripts): refuse to record the sdui lockstep from a tree the pin does not name (#17268)
* fix(scripts): refuse to record the sdui lockstep from a tree the pin does not name `gen:sdui-lockstep` read the record's content from whatever the objectui checkout had at HEAD while stamping `recordedAgainstPin` from `.objectui-sha`. Nothing joined those two, so a checkout off the pin wrote a record that named the pin while describing a different tree — silently, at exit 0. `judge()`'s `pin-moved` clause cannot reach that: it compares the stamped side against the pin file, two values from the same file that agree by construction. The join has to happen in the generator, the only moment both the tree and the pin are in hand. `judgeRecordability` is that join, as a pure function so `--self-test` can pin both directions: on the pin it records as before, off the pin it refuses and names both revisions plus the two ways to move. `recordedAgainstPin` is now stamped from the value the refusal compared, so what is checked and what is stamped cannot drift apart again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU * fix(scripts): scope the "is the guard wired" assertions to update()'s own body The self-test's wiring cases scanned the WHOLE FILE for `judgeRecordability({ head:`, which matched the battery's own test code. An ablation proved the consequence: deleting the generator's call to the judge left the self-test GREEN at exit 0 — the assertion was pinning itself, and the battery it claimed to protect was a phantom check. Scoped to a slice of `update()`'s own body, with the slice bounds asserted so a rename cannot make the case vacuously green instead. The stamp case is scoped the same way, for the same reason. Also stops the off-pin case from crashing when the guard is ablated: reading `[0].includes(...)` off an empty array threw a TypeError where a named failure belongs, so the self-test redded for the right reason with the wrong words. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f8fea00 commit 7ddf13d

1 file changed

Lines changed: 187 additions & 6 deletions

File tree

‎scripts/check-sdui-lockstep.mjs‎

Lines changed: 187 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@
5454
* (`packages/spec/react-declaration-parity.baseline.json` records a diff and
5555
* ratchets it); this record holds ONE SIDE, so there is no diff in it to bless.
5656
*
57+
* ## And why `--update` REFUSES off the pin
58+
*
59+
* `--update` reads the objectui checkout at HEAD but stamps `recordedAgainstPin`
60+
* from `PIN_FILE`, so those two used to be joined by nothing: a checkout not
61+
* sitting on the pin wrote a record that NAMED THE PIN WHILE DESCRIBING A
62+
* DIFFERENT TREE, silently, at exit 0. The record was self-certifying — stamped
63+
* with the identity of something other than what it described, with the only
64+
* check that compared anything (`judge()`'s `pin-moved`) comparing the STAMP
65+
* against the pin file, two values from one file. So the generator now refuses
66+
* unless HEAD is the pin; see `judgeRecordability`, which carries the
67+
* measurement and the reason the resolve-it-for-them alternative was declined.
68+
*
5769
* ## Why the pin is read (`.objectui-sha`)
5870
*
5971
* `.objectui-sha` is the objectui commit whose `@object-ui/console` build this
@@ -95,8 +107,9 @@
95107
*
96108
* Every input this gate needs is asserted before any verdict: the record file,
97109
* its shape, the region delimiter in this tree, a non-empty code set, a
98-
* resolvable identifier at every code position, and the pin. A missing one exits
99-
* 1 saying which — never a `⚠` and exit 0. A guard whose success condition and
110+
* resolvable identifier at every code position, and the pin. `--update` asserts
111+
* the same way before any RECORD: a readable pin, a HEAD that parses, and the
112+
* two being equal. A missing one exits 1 saying which — never a `⚠` and exit 0. A guard whose success condition and
100113
* whose total-failure condition are the same exit code is worse than no guard
101114
* (#13014, and #4690 for the incident that named the class).
102115
*/
@@ -403,6 +416,69 @@ export function resolveObjectui(env = process.env) {
403416
return { root: candidate, why: null };
404417
}
405418

419+
/**
420+
* Whether an objectui checkout may be RECORDED FROM: its `HEAD` must be the
421+
* commit `.objectui-sha` names. Empty means it may; otherwise one `[tag]`
422+
* finding per reason, in `judge()`'s shape so both are read the same way.
423+
*
424+
* ## Why the generator needs this and the checker cannot supply it
425+
*
426+
* The record's CONTENT is read from whatever the objectui checkout has at HEAD,
427+
* while `recordedAgainstPin` is stamped from `PIN_FILE`. Nothing joined those
428+
* two, so a checkout not sitting on the pin wrote a record that NAMED THE PIN
429+
* WHILE DESCRIBING A DIFFERENT TREE — silently, at exit 0.
430+
*
431+
* ⚠️ And `judge()`'s `pin-moved` clause cannot catch it, which is the whole
432+
* point: that clause compares `record.recordedAgainstPin` against the live pin,
433+
* i.e. the STAMPED side against the pin file — two values that came from the
434+
* same file and therefore agree by construction. The record was self-certifying:
435+
* it was stamped with the identity of something other than what it described,
436+
* and the one check comparing anything compared the stamp. The join has to
437+
* happen HERE, at the only moment both the tree and the pin are in hand.
438+
*
439+
* ⛔ Never repair that by stamping HEAD instead. It would make the field honest
440+
* and useless — the field exists to say WHICH PIN the description belongs to —
441+
* and it would make `judge()`'s comparison pass forever, destroying a working
442+
* check along with the record.
443+
*
444+
* Measured, deterministically, on the `a472b07167a3` → `53ded82bf7a4` bump: read
445+
* at objectui tip the record carried 25 diagnostic codes, read at the pin it
446+
* carried 24. ⚠️ `parse.ts` is byte-identical between those two revisions while
447+
* the `PARSER_SRC` tree is not, and that is exactly where the difference lives —
448+
* so a reader checking the obvious file sees nothing wrong, and an equality
449+
* assertion is the only thing that does.
450+
*
451+
* Refusing rather than resolving the checkout to the pin ourselves is deliberate:
452+
* a read from the pin's tree needs git WRITES inside somebody else's checkout,
453+
* whose behaviour over uncommitted work there would have to be defined. The
454+
* operator parking their own checkout needs no new machinery.
455+
*/
456+
export function judgeRecordability({ head, livePin }) {
457+
const findings = [];
458+
if (typeof livePin !== 'string' || !/^[0-9a-f]{40}$/.test(livePin)) {
459+
findings.push(
460+
`[pin-unusable] ${PIN_FILE} does not hold a 40-character commit id, so nothing can be checked\n`
461+
+ ` against it. It reads: ${JSON.stringify(livePin)}.`,
462+
);
463+
return findings;
464+
}
465+
if (typeof head !== 'string' || !/^[0-9a-f]{40}$/.test(head)) {
466+
findings.push(
467+
`[head-unusable] \`git rev-parse HEAD\` in the objectui checkout did not yield a 40-character\n`
468+
+ ` commit id, so it cannot be compared with the pin. It yielded: ${JSON.stringify(head)}.`,
469+
);
470+
return findings;
471+
}
472+
if (head !== livePin) {
473+
findings.push(
474+
`[head-off-pin] the objectui checkout is at ${head.slice(0, 12)}, and ${PIN_FILE} names\n`
475+
+ ` ${livePin.slice(0, 12)}. Recording now would write a record stamped with the pin while\n`
476+
+ ' describing a different tree — the exact silent-wrong-record this refusal exists to prevent.',
477+
);
478+
}
479+
return findings;
480+
}
481+
406482
function update() {
407483
const { root: objectui, why } = resolveObjectui();
408484
if (objectui === null) {
@@ -417,6 +493,36 @@ function update() {
417493
}
418494

419495
const git = (...args) => execFileSync('git', ['-C', objectui, ...args], { encoding: 'utf8' }).trim();
496+
497+
// The HEAD-vs-pin join, BEFORE anything is read or written. It comes first
498+
// because on the wrong revision every later reading is a reading of the wrong
499+
// tree, and a dirty-tree or missing-delimiter complaint about it is noise.
500+
let livePin = null;
501+
try {
502+
livePin = readFileSync(join(ROOT, PIN_FILE), 'utf8').trim();
503+
} catch (error) {
504+
console.error(
505+
`\ncheck-sdui-lockstep --update: REFUSED — ${PIN_FILE} could not be read (${error.message}).\n`
506+
+ ' Nothing was recorded. The pin is what the record is stamped against, so a record taken\n'
507+
+ ' without it would name no revision at all.\n',
508+
);
509+
process.exit(1);
510+
}
511+
const recordability = judgeRecordability({ head: git('rev-parse', 'HEAD'), livePin });
512+
if (recordability.length > 0) {
513+
console.error('\ncheck-sdui-lockstep --update: REFUSED — nothing was recorded.\n');
514+
for (const finding of recordability) console.error(` ${finding}\n`);
515+
console.error(
516+
` Park the checkout on the pin and re-run, from ${objectui}:\n\n`
517+
+ ` git -C ${objectui} fetch origin && git -C ${objectui} checkout ${livePin}\n`
518+
+ ' pnpm gen:sdui-lockstep\n\n'
519+
+ ' Or, if you meant to record a DIFFERENT revision, move the pin to it first — that is a\n'
520+
+ ` decision recorded in an issue, never a side effect of this generator:\n\n`
521+
+ ' scripts/bump-objectui.sh <sha>\n',
522+
);
523+
process.exit(1);
524+
}
525+
420526
const dirty = git('status', '--porcelain', '--', PARSER_SRC);
421527
if (dirty !== '') {
422528
console.error(
@@ -461,7 +567,9 @@ function update() {
461567
source: PARSER_SRC,
462568
files: theirs.files,
463569
},
464-
recordedAgainstPin: readFileSync(join(ROOT, PIN_FILE), 'utf8').trim(),
570+
// The same bytes the refusal above compared HEAD against — re-reading the file here
571+
// would reopen the gap between what was CHECKED and what is STAMPED.
572+
recordedAgainstPin: livePin,
465573
grammarRegion: {
466574
file: REGION_FILE,
467575
delimiter: REGION_DELIMITER,
@@ -535,12 +643,13 @@ const SELF_TEST_BATTERIES = Object.freeze({
535643
'The region reader': 7,
536644
'The verdict': 4,
537645
'Absence is loud: every degraded input REFUSES rather than passing': 8,
538-
'The wiring this gate needs to be reachable at all': 9,
646+
'The generator will not record a tree the pin does not name': 7,
647+
'The wiring this gate needs to be reachable at all': 12,
539648
});
540649

541650
// DELETING an entry silences that battery's floor exactly as effectively as
542651
// zeroing it, so the roster's own size is pinned too.
543-
const SELF_TEST_BATTERY_FLOOR = 5;
652+
const SELF_TEST_BATTERY_FLOOR = 6;
544653

545654
// The key an assertion is filed under when no battery is open. It is not a
546655
// declared battery, so it reds by the same set difference rather than silently
@@ -712,6 +821,54 @@ export function selfTest() {
712821
=== 'code-set-empty',
713822
);
714823

824+
// ── The generator will not record a tree the pin does not name ───────────
825+
//
826+
// `judge()`'s `pin-moved` clause cannot reach this: it compares the STAMPED
827+
// side against the pin file, two values from the same file. So the agreeing
828+
// case below is asserted alongside the firing ones — a guard that fires on
829+
// everything is as useless as one that fires on nothing, and the REVERSE
830+
// direction is what says the good path still records.
831+
battery('The generator will not record a tree the pin does not name');
832+
const PIN_A = '5'.repeat(40);
833+
const PIN_B = '7'.repeat(40);
834+
const recKinds = (f) => f.map((x) => x.slice(1, x.indexOf(']'))).join(',');
835+
check(
836+
'⭐ REVERSE: a checkout sitting ON the pin is recordable — no finding, so recording proceeds',
837+
judgeRecordability({ head: PIN_A, livePin: PIN_A }).length === 0,
838+
recKinds(judgeRecordability({ head: PIN_A, livePin: PIN_A })),
839+
);
840+
check(
841+
'a checkout one commit off the pin is refused, not recorded',
842+
recKinds(judgeRecordability({ head: PIN_B, livePin: PIN_A })) === 'head-off-pin',
843+
);
844+
// `?? ''` rather than `[0].includes(...)`: with the guard ablated the array is
845+
// EMPTY, and an uncaught TypeError there is a crash where a named failure
846+
// belongs — the self-test would red for the right reason with the wrong words.
847+
const offPinText = judgeRecordability({ head: PIN_B, livePin: PIN_A })[0] ?? '';
848+
check(
849+
'the refusal names BOTH revisions, so the operator can see which way to move',
850+
offPinText.includes(PIN_B.slice(0, 12)) && offPinText.includes(PIN_A.slice(0, 12)),
851+
offPinText || '(no finding at all)',
852+
);
853+
// Absence and malformation are loud here too, and they are DISTINGUISHED: a
854+
// pin that cannot be compared must not read as a checkout off the pin.
855+
for (const [label, pin] of [
856+
['empty', ''],
857+
['a short sha', PIN_A.slice(0, 12)],
858+
['undefined', undefined],
859+
]) {
860+
check(
861+
`a pin that is ${label} refuses as pin-unusable, never as head-off-pin`,
862+
recKinds(judgeRecordability({ head: PIN_A, livePin: pin })) === 'pin-unusable',
863+
recKinds(judgeRecordability({ head: PIN_A, livePin: pin })),
864+
);
865+
}
866+
check(
867+
'a HEAD that did not parse to a commit id refuses as head-unusable, never as head-off-pin',
868+
recKinds(judgeRecordability({ head: 'HEAD', livePin: PIN_A })) === 'head-unusable',
869+
recKinds(judgeRecordability({ head: 'HEAD', livePin: PIN_A })),
870+
);
871+
715872
// ── The wiring this gate needs to be reachable at all ────────────────────
716873
battery('The wiring this gate needs to be reachable at all');
717874
const pkg = JSON.parse(readFileSync(join(ROOT, 'package.json'), 'utf8'));
@@ -757,6 +914,29 @@ export function selfTest() {
757914
'the plain literal is still spelled too, so the file the gate opens is named as written',
758915
inCode.includes(`'${PIN_FILE}'`),
759916
);
917+
// ⚠️ The two cases below are scoped to `update()`'s OWN BODY, never to the
918+
// whole file. Measured: a whole-file scan for `judgeRecordability({ head:`
919+
// matches the battery above, so deleting the generator's call left the
920+
// self-test GREEN — the assertion was pinning its own test code. Every
921+
// "is it wired" case has that shape; the slice is what makes it real.
922+
const updateOpen = inCode.indexOf('\nfunction update() {');
923+
const updateClose = inCode.indexOf('\n}\n', updateOpen);
924+
check(
925+
"the generator's body is still addressable, so the cases below are not vacuous",
926+
updateOpen !== -1 && updateClose > updateOpen && updateClose - updateOpen > 1000,
927+
`open ${updateOpen}, close ${updateClose}`,
928+
);
929+
const updateSrc = inCode.slice(updateOpen, updateClose);
930+
// A `judgeRecordability` nothing calls is a phantom check: every case in the
931+
// battery above stays green while the generator records from any tree it likes.
932+
check(
933+
'the generator CALLS the recordability judge, so that battery is not phantom',
934+
/judgeRecordability\(/.test(updateSrc),
935+
);
936+
check(
937+
'and it stamps `recordedAgainstPin` from the value it compared, not from a second read',
938+
/recordedAgainstPin: livePin,/.test(updateSrc) && !/recordedAgainstPin: readFileSync/.test(updateSrc),
939+
);
760940

761941
// ── The floor: every declared battery RAN, and ran its cases (#13489) ───
762942
//
@@ -810,7 +990,8 @@ export function selfTest() {
810990
}
811991
console.log(
812992
'check:sdui-lockstep --self-test passed (the constant-vs-literal decomposition and its unresolvable '
813-
+ 'direction, the region reader in both drift directions, all four refusal classes, and the CI wiring)',
993+
+ 'direction, the region reader in both drift directions, all four refusal classes, the generator\'s '
994+
+ 'HEAD-vs-pin refusal in both directions, and the CI wiring)',
814995
);
815996

816997
return SELF_TEST_VERDICT;

0 commit comments

Comments
 (0)