From dab989b0684e625e9e4f769f2e6b4a29ea8081df Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Mon, 14 Sep 2026 16:27:29 +0200 Subject: [PATCH] fix(mailbox-tests): the owed-set suite's own gate refused to let it run on node 24 scripts/test-owed-withdrawal.sh has not executed a single assertion since the image moved to node 24. Its precondition line node --experimental-strip-types --check "$SRC" exits 2 on an unmodified extensions/pi/mempalace.ts, and the script is `set -euo pipefail` with `|| exit 2`, so all 17 assertions and both regression guards were skipped. Measured on pi-devbox v1.9.1, node v24.21.0. CAUSE, MEASURED AND NARROWER THAN IT LOOKS. The reported error names the inline type-import at mempalace.ts:92, which invites the reading "that import is unusual". It is not the import: `node --check` does not type-strip AT ALL. A file whose entire content is `const x: number = 1;` fails identically, with or without --experimental-strip-types, while `node --experimental-strip-types ` executes it fine. --check has never been type-aware; execution is what gained stripping. So this gate could never validate TypeScript, on any node. WHY IT USED TO PASS -- INFERENCE, NOT MEASUREMENT. e2b060a's message records this suite running on 2026-09-09 with a control pass and four mutation kills, when the image shipped node v22.23.2; v1.9.1 ships v24.21.0. No node 22 exists on the box where this was diagnosed, so the counterfactual was not executed. Treat "22 stripped for --check and 24 stopped" as a hypothesis consistent with the record, not as a measured cause. What IS measured is the present-tense behaviour above. WHY THIS MATTERS MORE THAN A RED TEST. This suite exists because isWithdrawn is the one rule in the extension that can go wrong SILENTLY -- a wrong rule does not throw and does not log, it makes a real unanswered ask vanish from a mailbox forever. The rule shipped in e2b060a and has been baked since v1.9.1; the thing that makes its failure mode visible has been dark for the same period. The mitigating half: it failed CLOSED (exit 2, loud), never vacuously green. A gate that cannot run must not pass, and it did not. THE FIX. Strip first, then syntax-check the emitted JS: version-stable, and it still refuses malformed input. `mode: "strip"` blanks type syntax without moving anything, so offsets and line numbers survive and a reported error line still points at the right line of the original .ts (69157 B in, 69157 B out). THE EXIT CODES ARE NOW SPLIT, AND THAT IS THE POINT. 3 = the gate itself cannot run (no module.stripTypeScriptTypes, i.e. node < 22.13). 2 = the source does not parse. Collapsing the two is how this defect disguised itself: it printed "mempalace.ts does not parse" while mempalace.ts was fine, sending a reader to inspect the wrong file. Note the stripper is itself a parser, so a genuine syntax error surfaces as an exception from the strip call rather than from --check; that path is caught and reported as 2, not 3. Both remain failures. Neither passes. VERIFIED IN SIX DIRECTIONS on this image, each expectation written down first: unmutated source -> rc=0, PASSED (17/17) malformed TypeScript -> rc=2, "does not parse: Expression expected" stripper made unavailable -> rc=3, "cannot strip", and NOT "does not parse" (simulated by doctoring the runtime through NODE_OPTIONS, so the shipped line ran as shipped) M1 marker requirement removed -> FAILED: 3 (no-marker, other-thread, prose) M2 third-party guard removed -> FAILED: 1 M3 to_agent guard removed -> FAILED: 2 (broadcast and wrong-device share it) M4 ordering guard removed -> FAILED: 1 The four kill counts are the same ones e2b060a recorded, so sensitivity is restored rather than merely asserted. WORTH KNOWING FOR THE NEXT PERSON WHO MUTATES THIS: the extractor carries its own guard that rejects an isWithdrawn which no longer mentions "withdraws", so the obvious M1 (delete the marker check outright) is refused before any assertion runs -- correctly, but it looks like a crash. Mutate to `... || true` instead, which removes the requirement while keeping the key mentioned. SC2016 is disabled on the node invocation with a stated reason: the single quotes are deliberate, the payload is JavaScript and `${process.version}` must reach node rather than the shell. Checked that the file is shellcheck-clean at default severity, as it was before this change, and at the -S error severity the pi-devbox gate uses. NOT FIXED HERE. Nothing in CI runs this suite -- it is a script an operator invokes, which is precisely why a gate that fails loudly still went unnoticed across a node bump. The harness's own runner two hundred lines below still passes --experimental-strip-types, deliberately: it is a no-op on 24 and required on 22, so it keeps the suite runnable on both. And the real reason this was found at all is unrelated to CI: isWithdrawn was being exercised end-to-end on a released image against the live logstream for the first time (project/pi-devbox seq 140), and the suite was reached for as corroboration. --- scripts/test-owed-withdrawal.sh | 47 ++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/scripts/test-owed-withdrawal.sh b/scripts/test-owed-withdrawal.sh index 9c9cb5b..d2c7c27 100755 --- a/scripts/test-owed-withdrawal.sh +++ b/scripts/test-owed-withdrawal.sh @@ -23,7 +23,9 @@ # withdrawal had no effect, so tor-ms22 was still being told it owed a reply 41h # later for a release it never installed. # -# Usage: scripts/test-owed-withdrawal.sh [source.ts] (exit 0 = all rules behave) +# Usage: scripts/test-owed-withdrawal.sh [source.ts] +# exit 0 = all rules behave 1 = a rule broke +# exit 2 = source unreadable or does not parse 3 = the gate itself cannot run # # The optional argument exists so the suite can be pointed at a deliberately # MUTATED copy of the source to prove it is sensitive — a suite that has never @@ -39,8 +41,47 @@ trap 'rm -rf "$WORK"' EXIT [ -r "$SRC" ] || { echo "FAIL: cannot read $SRC" >&2; exit 2; } # A gate that cannot run must not pass — the standing rule in this repo. -node --experimental-strip-types --check "$SRC" \ - || { echo "FAIL: $SRC does not parse" >&2; exit 2; } +# +# NOT `node --check`: it does not type-strip, so it rejects ANY TypeScript — +# `const x: number = 1` included, not just the inline type-import at the top of +# mempalace.ts. It passed on node 22.x and stopped passing on node 24.x +# (measured on pi-devbox v1.9.1, node v24.21.0: this gate exited 2 on an +# unmodified mempalace.ts, so all 17 assertions below refused to run). Strip +# first, then syntax-check the emitted JS. `mode: "strip"` blanks type syntax +# without moving anything, so byte offsets and line numbers survive and a +# reported error line still points at the right line of the ORIGINAL .ts. +# +# The two failure modes are reported separately and on purpose. "Cannot strip" +# is a fact about the toolchain; "does not parse" is a fact about the source. +# Collapsing them is what made this very defect present itself as +# "mempalace.ts does not parse" when mempalace.ts was fine. +# +# SC2016 is disabled deliberately: the single quotes are the point. What follows +# is JavaScript, and `${process.version}` must reach node, not be expanded by the +# shell first. +# shellcheck disable=SC2016 +node --no-warnings -e ' + const { readFileSync, writeFileSync } = require("node:fs"); + const { stripTypeScriptTypes } = require("node:module"); + if (typeof stripTypeScriptTypes !== "function") { + console.error(`FAIL: node ${process.version} cannot strip TypeScript ` + + `(module.stripTypeScriptTypes needs >= 22.13) — the gate is unavailable, ` + + `which is NOT a statement about the source`); + process.exit(3); + } + let js; + try { + js = stripTypeScriptTypes(readFileSync(process.argv[1], "utf8"), { mode: "strip" }); + } catch (err) { + // The stripper is itself a parser, so a syntax error lands HERE, not in the + // --check below. This is a statement about the source: exit 2, not 3. + console.error(`FAIL: ${process.argv[1]} does not parse: ${err.message}`); + process.exit(2); + } + writeFileSync(process.argv[2], js); +' "$SRC" "$WORK/stripped.mjs" || exit $? +node --check "$WORK/stripped.mjs" \ + || { echo "FAIL: $SRC does not parse (stripped output rejected)" >&2; exit 2; } # ---------------------------------------------------------------- extractor --- cat >"$WORK/extract.mjs" <<'EXTRACT'