diff --git a/BUILD-LOG.md b/BUILD-LOG.md index d2859a13..165880e7 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -3266,3 +3266,14 @@ reports 107/107 node tests and 22/22 mutations caught. The canonical suite list in AGENTS.md. Carried into A2 before genesis: N8, N11, N5, N10, N12 and Filbert's r1 notes P1 to P3 (agents/darkwing/work/queue-a2/). The nested `node --test` fix (N13) is a separate commit. + +## 2026-09-27: Nested node --test fix in two suites (N13, #1508) + +`test-foundation.sh` and `test-discord.sh` started a nested `node --test` +that inherited NODE_TEST_CONTEXT, so under a parent runner a failing +test there would exit 0. Both now clear it, and each has one new check +that fails the suite if the clearing is lost (foundation 43 to 44, +discord 63 to 64). Darkwing wrote it (n13.patch 00868b2f; the suites +hash 60f04822 and 2ad3be74), and Filbert approved it (e464be6c). The +nine suites passed on an index export. Latent until now: nothing ran +those suites under a node runner. diff --git a/agents/darkwing/work/queue-a1/n13-check.sh b/agents/darkwing/work/queue-a1/n13-check.sh new file mode 100755 index 00000000..7185c27e --- /dev/null +++ b/agents/darkwing/work/queue-a1/n13-check.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# N13 check (#1508): a failing nested `node --test` must fail test-foundation.sh +# and test-discord.sh even when a parent runner's NODE_TEST_CONTEXT is set. +# Runs in a scratch clone only: n13-check.sh CLONE (a clone of HEAD with +# node_modules linked). It copies this checkout's two suites into the clone, +# plants a failing test in each suite's test directory, and restores after. +set -uo pipefail +SRC="$(cd "$(dirname "$0")/../../../.." && pwd)" +CLONE="${1:?usage: n13-check.sh CLONE}" +[ "$(cd "$CLONE" && pwd)" != "$SRC" ] || { echo "refusing to run in the canonical checkout" >&2; exit 4; } +cd "$CLONE" || exit 1 + +PLANT='import { test } from "node:test"; +import assert from "node:assert/strict"; +test("N13 planted failure", () => assert.equal(1, 2));' +FAILS=0 +expect() { # NAME WANT GOT + if [ "$2" = "$3" ]; then echo "ok $1 (rc $3)"; else echo "FAIL $1 (want rc $2, got $3)"; FAILS=$((FAILS+1)); fi +} +run() { # SUITE -> rc, output in /tmp/n13--.txt + NODE_TEST_CONTEXT=child-v8 NO_COLOR=1 "scripts/test-$1.sh" >"/tmp/n13-$1-$2.txt" 2>&1 +} + +for pair in foundation:scripts/foundation discord:packages/discord/tests; do + suite=${pair%%:*} dir=${pair#*:} + git checkout -q -- "scripts/test-$suite.sh" + printf '%s\n' "$PLANT" >"$dir/zz-n13-planted.test.mjs" + run "$suite" head-planted; expect "$suite at HEAD, planted failure, parent context set" 0 $? + cp -p "$SRC/scripts/test-$suite.sh" "scripts/test-$suite.sh" + run "$suite" fixed-planted; expect "$suite fixed, planted failure, parent context set" 1 $? + rm -f "$dir/zz-n13-planted.test.mjs" + run "$suite" fixed-clean; expect "$suite fixed, no planted failure, parent context set" 0 $? + sed -i 's/node_tests() { env -u NODE_TEST_CONTEXT node --test/node_tests() { node --test/' "scripts/test-$suite.sh" + run "$suite" mutant-clean; expect "$suite with the clearing removed, no planted failure" 1 $? + git checkout -q -- "scripts/test-$suite.sh" +done +git status --short +echo "n13 check: $FAILS failed" +[ "$FAILS" -eq 0 ] diff --git a/agents/darkwing/work/queue-a1/n13.md b/agents/darkwing/work/queue-a1/n13.md new file mode 100644 index 00000000..b592fc85 --- /dev/null +++ b/agents/darkwing/work/queue-a1/n13.md @@ -0,0 +1,59 @@ +# N13: nested `node --test` in two suites (#1508) + +Darkwing, 2026-09-26. Filbert's note N13, put in DEFERRED by Sage as a +small reviewed item before A2. Nothing is committed, staged or pushed. + +## The defect + +`scripts/test-foundation.sh:76` and `scripts/test-discord.sh:142` start a +nested `node --test` without clearing `NODE_TEST_CONTEXT`. Under a parent +test runner the nested run reports to that runner and exits 0 whatever its +tests do. I reproduced it at HEAD 40a02d2b: with a failing test planted in +each suite's test directory and `NODE_TEST_CONTEXT=child-v8` set, both +suites exit 0. The check line reads `OK node --test ... (summary +missing)`. In a control run of foundation without the variable, the same +planted test fails the suite, so only a run under a parent runner is +blind. I ran that control for foundation only. + +## The change + +`n13.patch` (sha256 00868b2f) changes only the two suites, +20 −2 lines. +Each suite now has a `node_tests` function that runs `env -u +NODE_TEST_CONTEXT node --test`, and uses it for its test run. Each also +gets one new check. It writes a failing test into the sandbox, runs it +through `node_tests` with `NODE_TEST_CONTEXT=child-v8` set, and requires +exit 1 and `✖ planted failure` in the output. If someone drops the `env +-u`, that check fails on every run, not only under a parent runner. + +| File | sha256 | +|---|---| +| `scripts/test-foundation.sh` | 60f04822 | +| `scripts/test-discord.sh` | 2ad3be74 | +| `n13-check.sh` | 6a231759 | + +## The check + +`n13-check.sh CLONE` runs in a scratch clone only and refuses the +canonical checkout. For each suite it plants a failing test in the real +test directory and runs the whole suite with `NODE_TEST_CONTEXT=child-v8`: + +| Suite | Case | Exit | +|---|---|---| +| foundation | HEAD, planted failure | 0 (the defect) | +| foundation | fixed, planted failure | 1 | +| foundation | fixed, no planted failure | 0 | +| foundation | fixed but `env -u` removed, no planted failure | 1 | +| discord | HEAD, planted failure | 0 (the defect) | +| discord | fixed, planted failure | 1 | +| discord | fixed, no planted failure | 0 | +| discord | fixed but `env -u` removed, no planted failure | 1 | + +In the fixed runs with the planted failure, the suite prints `FAIL node +--test ...` with the pass count and the planted test's name. The last row +of each is the mutation: the new check alone fails the suite. + +All nine suites pass at 40a02d2b with this change and the queue A1 +candidate: foundation 44 and discord 64, one more check each than before. + +I found no other nested `node --test` in the suites. `test-queue.sh` and +`queue-commit.sh` already clear the variable. diff --git a/agents/darkwing/work/queue-a1/n13.patch b/agents/darkwing/work/queue-a1/n13.patch new file mode 100644 index 00000000..fe4c66d6 --- /dev/null +++ b/agents/darkwing/work/queue-a1/n13.patch @@ -0,0 +1,44 @@ +diff --git a/scripts/test-discord.sh b/scripts/test-discord.sh +index 6044100c..9ec4ddda 100755 +--- a/scripts/test-discord.sh ++++ b/scripts/test-discord.sh +@@ -139,7 +139,16 @@ else + fi + + # --- the seven offline groups --- +-node --test --test-reporter=spec packages/discord/tests/ >"$SANDBOX/node-test.log" 2>&1 ++# A nested `node --test` inherits a parent runner's NODE_TEST_CONTEXT, reports ++# to that runner and exits 0 whatever its tests do, so the suite clears it ++# (#1508 N13). The planted failing test proves a failure still fails here. ++node_tests() { env -u NODE_TEST_CONTEXT node --test "$@"; } ++mkdir -p "$SANDBOX/planted" ++printf '%s\n' 'import { test } from "node:test";' 'import assert from "node:assert/strict";' 'test("planted failure", () => assert.equal(1, 2));' >"$SANDBOX/planted/planted.test.mjs" ++NODE_TEST_CONTEXT=child-v8 node_tests "$SANDBOX/planted/" >"$SANDBOX/planted.log" 2>&1 ++[ $? -eq 1 ] && grep -q '^✖ planted failure' "$SANDBOX/planted.log" ++check "a failing nested test fails the run under a parent runner's NODE_TEST_CONTEXT" $? ++node_tests --test-reporter=spec packages/discord/tests/ >"$SANDBOX/node-test.log" 2>&1 + NODE_RC=$? + check "node --test packages/discord/tests/ ($(grep -E '^ℹ pass' "$SANDBOX/node-test.log" | tr -d '\n' || echo 'summary missing'))" $NODE_RC + if [ "$NODE_RC" -ne 0 ]; then +diff --git a/scripts/test-foundation.sh b/scripts/test-foundation.sh +index 251eb674..b94dab5c 100755 +--- a/scripts/test-foundation.sh ++++ b/scripts/test-foundation.sh +@@ -73,7 +73,16 @@ done + check "checked-in demo bundles equal a fresh generation" $DEMO_OK + + # --- unit, CLI, privacy, non-effect and fixture-index tests --- +-node --test scripts/foundation/ >"$SANDBOX/node-test.log" 2>&1 ++# A nested `node --test` inherits a parent runner's NODE_TEST_CONTEXT, reports ++# to that runner and exits 0 whatever its tests do, so the suite clears it ++# (#1508 N13). The planted failing test proves a failure still fails here. ++node_tests() { env -u NODE_TEST_CONTEXT node --test "$@"; } ++mkdir -p "$SANDBOX/planted" ++printf '%s\n' 'import { test } from "node:test";' 'import assert from "node:assert/strict";' 'test("planted failure", () => assert.equal(1, 2));' >"$SANDBOX/planted/planted.test.mjs" ++NODE_TEST_CONTEXT=child-v8 node_tests "$SANDBOX/planted/" >"$SANDBOX/planted.log" 2>&1 ++[ $? -eq 1 ] && grep -q '^✖ planted failure' "$SANDBOX/planted.log" ++check "a failing nested test fails the run under a parent runner's NODE_TEST_CONTEXT" $? ++node_tests scripts/foundation/ >"$SANDBOX/node-test.log" 2>&1 + NODE_RC=$? + check "node --test scripts/foundation/ ($(grep -E '^ℹ pass' "$SANDBOX/node-test.log" | tr -d '\n' || echo 'summary missing'))" $NODE_RC + [ "$NODE_RC" -ne 0 ] && grep -E "^✖|AssertionError" "$SANDBOX/node-test.log" | head -20 diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 6552dc37..ca253817 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -417,3 +417,4 @@ are never rewritten or removed; corrections are new entries. 2026-09-26T23:58:21Z | Sage (T3 Claude Code, thread 1ef1e4f8) | #1507 CHAT-03 brief r2 | Rocko R2 revise (07b938fb), one blocking finding on stop proof vs interruption; V-1 accepted with limits, lead decision 25; r3 to Dewey. A2 carry-forward 773dbd75 accepted. 2026-09-27T00:04:42Z | Filbert (T3 Claude Code, thread 9cb9731e) | Queue A1 (#1508) r1 re-review and N13 | approved: delta-r1 b733b894, manifest-r1 85a8a453, n13.patch 00868b2f; review agents/filbert/work/queue-a1-review-r1-2026-09-26.md e464be6c; three non-blocking notes 2026-09-27T00:07:44Z | Sage (T3 Claude Code, thread 1ef1e4f8) | #1508 queue A1 commit | Filbert approved r1 (e464be6c); 20 files match 85a8a453; nine suites green on the index incl. queue; test-queue.sh added to AGENTS.md suites; lead decision 26 (P1-P3 to A2, N13-a declined). A2 started. +2026-09-27T00:10:31Z | Sage (T3 Claude Code, thread 1ef1e4f8) | #1508 N13 commit | nested node --test fix in test-foundation/test-discord; pins 60f04822/2ad3be74 match; nine suites green on the index (foundation 44, discord 64); DEFERRED entry closed. diff --git a/docs/plans/DEFERRED.md b/docs/plans/DEFERRED.md index 67bc617b..948edbf3 100644 --- a/docs/plans/DEFERRED.md +++ b/docs/plans/DEFERRED.md @@ -163,17 +163,6 @@ at every gate. Started 2026-09-12 during the control board MVP. Sage asked the SetSpark lead to use a one-word role. The rule stays as it is. (2026-09-26, #1506) -- **A nested `node --test` passes even when its tests fail.** A child - `node --test` inherits NODE_TEST_CONTEXT from its parent and exits 0 - whatever its results. Darkwing found it building queue A1 and fixed that - path with `env -u NODE_TEST_CONTEXT`, plus a test. Other suites that - start `node --test` from inside a test run have not been checked, so - they may be hiding failures. (2026-09-26, #1508) - Filbert's A1 review (6933b885, N13) found two: `test-foundation.sh` line - 76 and `test-discord.sh` line 142 start a nested `node --test` without - clearing NODE_TEST_CONTEXT. Latent today, because neither runs under a - parent runner. Darkwing fixes both after the A1 delta (lead decision 23). - ## Queue Moved to `docs/plans/QUEUE.md` on 2026-09-13. This file holds only gaps. @@ -187,4 +176,4 @@ Moved to `docs/plans/QUEUE.md` on 2026-09-13. This file holds only gaps. - Control board checked the bind address but not Host or Origin (2026-09-26, #1507, found by Dewey): closed by the board guard commit. `foreignRequest` refuses a non-loopback Host, a wrong port, userinfo or a path in Host, and any foreign Origin on every route. Dewey authored it, and Rocko approved it (`agents/rocko/work/board-guard-review-2026-09-26.md`, 5a12f08e). Rocko's one low finding: URL normalization accepts a root slash, empty userinfo and an empty query in Host, so the notes' wording overstates what the raw-syntax check refuses. No bypass was found. - Discord engine: leaked fake pi and the timeout gap in `busy` (2026-09-26, #1509): fixed in 3edb15eb (Darkwing 6b R2, Rocko approved, review ed5510a0). Tests stop the engine in `finally`. A turn pi never started now stops pi with exit 1 instead of guessing, and the unit restarts it. Known limit: see the open entry on wedge restarts. - Combined concurrent test hangs (2026-09-14, #1509): closed on evidence from 3edb15eb. At default concurrency the union passed 406/406 three times on the R2 snapshot and 408/408 on the commit's index, where clean HEAD had hung four runs out of four. The inherited-intermittency entry from 2026-09-15 stays open until a later run shows it is the same cause. - +- A nested `node --test` passes even when its tests fail (2026-09-26, #1508): closed. Queue A1 (34a72af9) clears NODE_TEST_CONTEXT on its own path. N13 fixes `test-foundation.sh` and `test-discord.sh`, each with a check that fails the suite if the clearing is lost. Darkwing wrote it (n13.patch 00868b2f) and Filbert approved it (e464be6c). Sage's grep on 2026-09-27 finds no other nested `node --test`: in scripts, only the two suites, `test-queue.sh` and `queue-commit.sh`, and in JS, only the queue test that covers the fix. diff --git a/scripts/test-discord.sh b/scripts/test-discord.sh index 6044100c..9ec4ddda 100755 --- a/scripts/test-discord.sh +++ b/scripts/test-discord.sh @@ -139,7 +139,16 @@ else fi # --- the seven offline groups --- -node --test --test-reporter=spec packages/discord/tests/ >"$SANDBOX/node-test.log" 2>&1 +# A nested `node --test` inherits a parent runner's NODE_TEST_CONTEXT, reports +# to that runner and exits 0 whatever its tests do, so the suite clears it +# (#1508 N13). The planted failing test proves a failure still fails here. +node_tests() { env -u NODE_TEST_CONTEXT node --test "$@"; } +mkdir -p "$SANDBOX/planted" +printf '%s\n' 'import { test } from "node:test";' 'import assert from "node:assert/strict";' 'test("planted failure", () => assert.equal(1, 2));' >"$SANDBOX/planted/planted.test.mjs" +NODE_TEST_CONTEXT=child-v8 node_tests "$SANDBOX/planted/" >"$SANDBOX/planted.log" 2>&1 +[ $? -eq 1 ] && grep -q '^✖ planted failure' "$SANDBOX/planted.log" +check "a failing nested test fails the run under a parent runner's NODE_TEST_CONTEXT" $? +node_tests --test-reporter=spec packages/discord/tests/ >"$SANDBOX/node-test.log" 2>&1 NODE_RC=$? check "node --test packages/discord/tests/ ($(grep -E '^ℹ pass' "$SANDBOX/node-test.log" | tr -d '\n' || echo 'summary missing'))" $NODE_RC if [ "$NODE_RC" -ne 0 ]; then diff --git a/scripts/test-foundation.sh b/scripts/test-foundation.sh index 251eb674..b94dab5c 100755 --- a/scripts/test-foundation.sh +++ b/scripts/test-foundation.sh @@ -73,7 +73,16 @@ done check "checked-in demo bundles equal a fresh generation" $DEMO_OK # --- unit, CLI, privacy, non-effect and fixture-index tests --- -node --test scripts/foundation/ >"$SANDBOX/node-test.log" 2>&1 +# A nested `node --test` inherits a parent runner's NODE_TEST_CONTEXT, reports +# to that runner and exits 0 whatever its tests do, so the suite clears it +# (#1508 N13). The planted failing test proves a failure still fails here. +node_tests() { env -u NODE_TEST_CONTEXT node --test "$@"; } +mkdir -p "$SANDBOX/planted" +printf '%s\n' 'import { test } from "node:test";' 'import assert from "node:assert/strict";' 'test("planted failure", () => assert.equal(1, 2));' >"$SANDBOX/planted/planted.test.mjs" +NODE_TEST_CONTEXT=child-v8 node_tests "$SANDBOX/planted/" >"$SANDBOX/planted.log" 2>&1 +[ $? -eq 1 ] && grep -q '^✖ planted failure' "$SANDBOX/planted.log" +check "a failing nested test fails the run under a parent runner's NODE_TEST_CONTEXT" $? +node_tests scripts/foundation/ >"$SANDBOX/node-test.log" 2>&1 NODE_RC=$? check "node --test scripts/foundation/ ($(grep -E '^ℹ pass' "$SANDBOX/node-test.log" | tr -d '\n' || echo 'summary missing'))" $NODE_RC [ "$NODE_RC" -ne 0 ] && grep -E "^✖|AssertionError" "$SANDBOX/node-test.log" | head -20