Skip to content

Commit b22db51

Browse files
fix(scripts): stop the bump self-test reporting a crashed digest probe as a range verdict (#19034)
Fixes #18354 Clause-②: no ## What was wrong `scripts/bump-objectui.selftest.sh` ran the digest's `--check-walkable` probe with both streams discarded, and then hard-coded the failure wording as a **verdict about the objectui range**: ``` node "$DIGEST_SCRIPT" ... --check-walkable >/dev/null 2>&1 || walk_rc=$? ... bad "fixture: the range does not walk BEFORE breaking the blob (rc=${walk_rc}) — not this card's state" ``` `--check-walkable` derives nothing and prints nothing on stdout — the digest's own self-test asserts that emptiness — so **stderr was the entire diagnostic**, and it was thrown away before anyone could read it. A probe that *crashed* (missing module, syntax error, killed process) was therefore reported as "the range does not walk": a confident, wrong diagnosis pointing at another subsystem, with the only clue already gone. Not hypothetical: in the #16421 round a new import made the digest die, that wording sent the dev to investigate shallow clones and `fetch --unshallow`, and the real cause was a copy manifest three directories away. Their report calls it the longest part of the round. ## What changed One file, both probe sites in `case_5`: - `check_walkable` runs the probe keeping the child's **stderr** in a file under the existing `TMPROOT` (so the EXIT trap still cleans it up). stdout stays discarded — the probe writes none. - `walk_answered` forks on **exit 2 or 3 = a verdict, anything else = it never answered**. That criterion is **taken from `bump-objectui.sh`**, which already forks on exactly it, rather than invented a second time — so the two cannot drift into two different ideas of what "the range does not walk" means. - `walk_replay_stderr` replays the child's stderr, prefixed, **before** the `bad` line. - The no-answer wording says the probe never answered, names the exit and the two verdicts, and states that this says nothing about the objectui range. The header comment's pointer at this card was replaced with a pointer at the fork that now exists in the file, so it does not outlive the card as a dangling reference. `scripts/bump-objectui.sh` is **not touched**, per the card. ## Scope: why two of the eight `dev/null` occurrences `dev/null` appears 8 times in the file. Two of them — the `--check-walkable` probes — are the carded shape: stderr swallowed **and** a hard-coded failure sentence that is a verdict about another subsystem. The other six are not, and are left alone: | line (pre-change) | what it is | why out of scope | |---|---|---| | 242 `git cat-file -e "$blob" 2>/dev/null` | existence probe | the exit code IS the answer; the message it produces is about the fixture it just built, not a verdict about another subsystem | | 261 `git cat-file -e "${sha}^{commit}" 2>/dev/null` | existence probe | same | | 265 `git log -1 --format=%s "$sha" >/dev/null 2>&1` | existence probe | same | | 273 `git rev-parse HEAD 2>/dev/null` | existence probe, `rc` already captured and printed | same | | 374 | the pin file read with `tr`, stderr discarded, falling back with a `true` on absence | tolerated-absence read; no verdict sentence at all | | 431 `break_changeset_blob … >/dev/null` | stdout only | stderr already flows through, and the function calls `bad` itself with its own evidence | Deliberately **not** measured here: whether this shape exists elsewhere in the repo. The card claims this one file's two sites, not a population, and a population would be a new finding rather than a licence to widen this PR. ## Both directions, measured Acceptance item 3. Each leg mutates the tree, proves the mutation reached disk by an occurrence count, runs the real self-test, and restores under a trap; every leg ends with both touched files hashing byte-identical to `HEAD` (`git hash-object` vs the `HEAD` blob), and the final `git status --porcelain` and `git diff HEAD --stat` are both empty. **Direction A — a real "range does not walk"** (the fixture deletes the `from` endpoint's commit object, so the probe returns its verdict **2**): - before: `✗ fixture: the range does not walk BEFORE breaking the blob (rc=2) — not this card's state` - after: the same sentence, now preceded by the probe's own stderr (`PREREQUISITE NOT MET — an endpoint of … is not present as a commit object …`, plus the `fetch --unshallow` / `fetch origin` remedies and git's own `fatal:` line). So a real verdict still reads as a verdict about the range. No regression. **Direction B — a crash** (the digest imports a name `./invoked-as.mjs` does not export; exit **1**, which is neither verdict): - before: `✗ fixture: the range does not walk BEFORE breaking the blob (rc=1) — not this card's state` — the wrong sentence, and nothing else. - after: ``` ↳ the walkability probe's own stderr: | file:///…/scripts/objectui-changeset-digest.mjs:213 | import { isEntrypointTHISEXPORTDOESNOTEXIST as isEntrypoint } from './invoked-as.mjs'; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | SyntaxError: The requested module './invoked-as.mjs' does not provide an export named 'isEntrypointTHISEXPORTDOESNOTEXIST' | at ModuleJob._instantiate (node:internal/modules/esm/module_job:226:21) … ✗ fixture: the walkability probe never ANSWERED before the blob was broken (exit 1; its verdicts are 2 and 3) — this says nothing about the objectui range; read the probe's stderr above ``` Both directions were also driven through the **second** probe site (breaking the walk, and breaking the digest, between the two probes): - A at site 2, before and after: `✗ fixture: --check-walkable now exits 2 — the blob deletion broke the WALK, not just the blob read` (after: with the probe's stderr above it). - B at site 2, before: the same sentence with `exits 1` — wrong. After: `✗ fixture: the walkability probe never ANSWERED after the blob deletion (exit 1; its verdicts are 2 and 3) — the WALK is UNMEASURED here, not broken; read the probe's stderr above`, with the SyntaxError replayed. Note on the vehicle: removing an import *specifier* outright cannot reach these probes — `firstPartyModuleClosure` throws on a first-party module that does not exist, so `new_framework_with_digest` dies first (the #16421 hardening working). A missing *named export* from an existing module is the same ERR-class load failure and does reach them. ## Verification | run | exit | |---|---| | `pnpm check:objectui-bump` on the pre-change file (restored from the merge base, restore proven byte-identical afterwards) | 0 — 20 assertions across 5 cases | | `pnpm check:objectui-bump` after the change | 0 — 20 assertions across 5 cases | | `bash scripts/bump-objectui.selftest.sh` directly, after the change | 0 (same run; the gate is exactly this command) | Gate families derived in-worktree with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (1 changed path vs merge base `72c164050`): **26 commands**. 25 ran green here (exit 0), including `check:objectui-bump`, `check:nul-bytes`, `check:parse-guard`, `check:entry-guard`, `check:self-test-wired`, `check:scripts-symbol-anchors`, `check:bash32-floor` and `check:agent-test-spelling`. The 26th, `check:pm-dispatch-gates`, was run detached and waited on to completion: **exit 0**, and it printed `the battery took 999.5s on this box` (a contended box; the prescription quotes 430-450s). The tool's own warning applies: that list is **not** a complete account of what CI runs on this PR. `pnpm lint` is not a reading for this diff and is not claimed as one: eslint's universe is JS/TS, and the only changed file is a `.sh` file it does not lint. `skip-changeset` was **measured, not asserted**: the root manifest is `private: true` with no `files[]`, and no package's `files[]` names anything under the repo-root `scripts/` directory (positive control: the same loop prints `dist` for 5+ packages). Nothing this diff touches is published. ## Acceptance notes - Observation, not filed: the same "swallow both streams, then assert a hard-coded sentence" shape may exist elsewhere in this repo. This PR deliberately did not sweep for it, because the card claims two sites and not a population. - Observation, not filed: the two probe sites both re-run the identical `--check-walkable` invocation, differing only in which side of the blob deletion they sit on. They are now one helper; collapsing the surrounding assertions further would change what case 5 pins and is not in scope here. --- _Generated by [Claude Code](https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent abd63ed commit b22db51

1 file changed

Lines changed: 63 additions & 7 deletions

File tree

‎scripts/bump-objectui.selftest.sh‎

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ case_begin() { CASE="$1"; echo " • ${CASE}"; }
8484
# case 5 failed on the refusal's wording instead. (⚠️ `bump-objectui.sh` itself
8585
# is NOT at fault and is not to be touched for this: it swallows no stderr, and
8686
# its `WALK_RC` branch already separates a verdict from a no-answer and refuses
87-
# to offer `--unshallow` for a crash. See #18354 for what IS carded.) The
87+
# to offer `--unshallow` for a crash. What WAS at fault was this file, and it
88+
# is fixed below: `check_walkable` keeps the child's stderr and
89+
# `walk_answered` forks on the same 2-or-3 criterion, so a probe that never
90+
# answered is no longer reported as a verdict about the objectui range.) The
8891
# derivation lives in `first-party-closure.mjs`, shared with the JS sites.
8992
#
9093
# ⚠️ THE BASENAME IS SPELLED ALONE AND THE DIRECTORY IS INTERPOLATED ONTO IT —
@@ -291,6 +294,51 @@ run_bump() {
291294

292295
log_has() { grep -qF -- "$1" "$LOG"; }
293296

297+
# --- the walkability probe, and the ONE criterion for reading its exit -------
298+
#
299+
# THE PROBE'S STDERR IS THE ONLY DIAGNOSTIC IT EMITS. `--check-walkable`
300+
# derives nothing and prints nothing on stdout — the digest's own self-test
301+
# asserts that emptiness — so everything it has to say arrives on stderr: a
302+
# real verdict's explanation, and equally a crash's `ERR_MODULE_NOT_FOUND`.
303+
# Both call sites below used to send stderr to the bit bucket along with
304+
# stdout, which threw the only clue away before anyone could read it.
305+
#
306+
# ⭐ AND A NON-ZERO EXIT IS NOT AUTOMATICALLY A VERDICT ABOUT THE RANGE.
307+
# `bump-objectui.sh` already forks on exactly this, and the criterion is taken
308+
# from there rather than re-invented, so the two cannot drift into two
309+
# different ideas of what "the range does not walk" means: 2 and 3 are the
310+
# probe's two VERDICTS (2 = an endpoint is missing, 3 = the endpoints are here
311+
# but the history stops inside the range); any other non-zero exit means it
312+
# never reached one — no node, a missing module, a syntax error, a killed
313+
# process. Reporting that as "the range does not walk" is a confident, wrong
314+
# diagnosis pointing at another subsystem, and it has cost a round already
315+
# (#16421): a new import made the digest die, the wording sent that dev to
316+
# investigate shallow clones and `fetch --unshallow`, and the real cause was a
317+
# copy manifest three directories away. Their report calls it the longest part
318+
# of the round.
319+
WALK_ERR=""
320+
check_walkable() {
321+
local oui="$1" from="$2" to="$3" rc=0
322+
WALK_ERR="${TMPROOT}/walk-$$-${RANDOM}.err"
323+
node "$DIGEST_SCRIPT" --objectui-root "$oui" --from "$from" --to "$to" \
324+
--check-walkable >/dev/null 2>"$WALK_ERR" || rc=$?
325+
return "$rc"
326+
}
327+
328+
# True for the probe's two verdicts, false for every "it never answered" exit.
329+
walk_answered() { [[ "$1" -eq 2 || "$1" -eq 3 ]]; }
330+
331+
# Replay the child's stderr BEFORE the `bad` that reports it, so the failure
332+
# carries its evidence instead of only pointing somewhere.
333+
walk_replay_stderr() {
334+
echo " ↳ the walkability probe's own stderr:" >&2
335+
if [[ -s "$WALK_ERR" ]]; then
336+
sed 's#^# | #' "$WALK_ERR" >&2
337+
else
338+
echo " | (nothing — the probe wrote no stderr at all)" >&2
339+
fi
340+
}
341+
294342
# --- case 1: unreadable commit object ⇒ refusal, pin file untouched ----------
295343
case_1() {
296344
case_begin 'unreadable commit object ⇒ refuses, .objectui-sha byte-identical'
@@ -421,10 +469,14 @@ case_5() {
421469
new_framework_with_digest "$fw" "$old_sha"
422470

423471
local walk_rc=0
424-
node "$DIGEST_SCRIPT" --objectui-root "$oui" --from "$old_sha" --to "$new_sha" \
425-
--check-walkable >/dev/null 2>&1 || walk_rc=$?
472+
check_walkable "$oui" "$old_sha" "$new_sha" || walk_rc=$?
426473
if [[ "$walk_rc" -ne 0 ]]; then
427-
bad "fixture: the range does not walk BEFORE breaking the blob (rc=${walk_rc}) — not this card's state"
474+
walk_replay_stderr
475+
if walk_answered "$walk_rc"; then
476+
bad "fixture: the range does not walk BEFORE breaking the blob (rc=${walk_rc}) — not this card's state"
477+
else
478+
bad "fixture: the walkability probe never ANSWERED before the blob was broken (exit ${walk_rc}; its verdicts are 2 and 3) — this says nothing about the objectui range; read the probe's stderr above"
479+
fi
428480
return 0
429481
fi
430482

@@ -433,12 +485,16 @@ case_5() {
433485
# Re-assert walkability AFTER breaking the blob — the whole point of this
434486
# case is that the commit/tree walk stays green while the blob read fails.
435487
walk_rc=0
436-
node "$DIGEST_SCRIPT" --objectui-root "$oui" --from "$old_sha" --to "$new_sha" \
437-
--check-walkable >/dev/null 2>&1 || walk_rc=$?
488+
check_walkable "$oui" "$old_sha" "$new_sha" || walk_rc=$?
438489
if [[ "$walk_rc" -eq 0 ]]; then
439490
ok 'fixture: --check-walkable still exits 0 after the blob is deleted (walk is commits/trees, not blobs)'
440491
else
441-
bad "fixture: --check-walkable now exits ${walk_rc} — the blob deletion broke the WALK, not just the blob read"
492+
walk_replay_stderr
493+
if walk_answered "$walk_rc"; then
494+
bad "fixture: --check-walkable now exits ${walk_rc} — the blob deletion broke the WALK, not just the blob read"
495+
else
496+
bad "fixture: the walkability probe never ANSWERED after the blob deletion (exit ${walk_rc}; its verdicts are 2 and 3) — the WALK is UNMEASURED here, not broken; read the probe's stderr above"
497+
fi
442498
return 0
443499
fi
444500

0 commit comments

Comments
 (0)