diff --git a/backend/TESTING.md b/backend/TESTING.md index 266c10299..7db16901a 100644 --- a/backend/TESTING.md +++ b/backend/TESTING.md @@ -99,9 +99,13 @@ Frontend testing is documented separately at `frontend/TESTING.md`. Contracts te ./dev.sh test:integration # Tier 1 against Docker Compose services (./dev.sh up required) ``` -## Node 26 kills any suite whose require graph reaches `buffer-equal-constant-time` +## Node 25+ kills any suite whose require graph reaches `buffer-equal-constant-time` -Node 26 removed `SlowBuffer`. `buffer-equal-constant-time/index.js:37` reads +Node 25 removed `SlowBuffer` — 22 and 24 still have it, 25 and 26 do not +(measured: `node -e "typeof require('buffer').SlowBuffer"` is `function` on 22 +and 24, `undefined` on 25 and 26; the removal landed at 25, so "Node 26" — how +this was first described — understates the range). +`buffer-equal-constant-time/index.js:37` reads `SlowBuffer.prototype.equal` at **module scope**, so it throws the moment it is required — before any test runs: @@ -117,7 +121,7 @@ never runs. The unconditional **read** at `:37` is the one that fires. **`jsonwebtoken` is the common importer, not the failing package.** The chain is `jsonwebtoken` → `jws` → `jwa` → `buffer-equal-constant-time`, and both `require('jsonwebtoken')` and `require('buffer-equal-constant-time')` throw at -the identical frame. So a "will this suite die on 26?" check must ask whether +the identical frame. So a "will this suite die on 25+?" check must ask whether that leaf is in the require graph — grepping a suite for the string `jsonwebtoken` misses every suite that reaches it transitively, and blames the wrong package when it hits. @@ -153,7 +157,7 @@ comparison function is a faithful replacement rather than a test-only shim. Note that is a stub of the *leaf*, not of `jsonwebtoken` — the caution below about stubbing `jsonwebtoken` doesn't apply, because real signing and verifying still run against a real comparison. -That is the move **if Node 26 ever becomes mandatory** — not now, while 26 is +That is the move **if Node 25+ ever becomes mandatory** — not now, while 25+ is optional and Node 22 is the right answer. Note the remedy has to intercept the `require`. Lines 36-37 are dead by @@ -192,7 +196,7 @@ actually signs or verifies a token, which is most of the runtime-token service suites. Suites with no jwt in their graph are unaffected — `mongoose` and -`mongodb-memory-server` both load clean on 26. +`mongodb-memory-server` both load clean on 25+. ## CI diff --git a/backend/__tests__/utils/globalSetup.js b/backend/__tests__/utils/globalSetup.js index 8fc0fea1e..525f54610 100644 --- a/backend/__tests__/utils/globalSetup.js +++ b/backend/__tests__/utils/globalSetup.js @@ -9,6 +9,26 @@ const sleep = (ms) => new Promise((resolve) => { setTimeout(resolve, ms); }); // cached binary and never race the download lock — the root cause of the flaky // "Cannot unlock file ... .lock" failures in CI. module.exports = async () => { + // Name the remedy before the failure, because the failure does not. On Node + // 25+ every suite whose require graph reaches jsonwebtoken dies with a bare + // `TypeError: Cannot read properties of undefined (reading 'prototype')` + // raised inside buffer-equal-constant-time — a package the test never + // imports, four frames under jws. Measured cost of not saying this: a seat + // read the stack, installed deps, wrote a SlowBuffer shim, and only then + // found TESTING.md. One line here is cheaper than that detour. + if (Number(process.versions.node.split('.')[0]) >= 25) { + // console is not stubbed in this process (that happens per-worker in + // setup.js), but stderr is the honest target either way. + process.stderr.write( + `[globalSetup] Node ${process.versions.node} removed buffer.SlowBuffer, which ` + + 'buffer-equal-constant-time reads at module scope — any suite reaching ' + + 'jsonwebtoken will fail to load. Run on Node 22 (what CI pins):\n' + + ' PATH=/opt/homebrew/opt/node@22/bin:$PATH npx jest \n' + + ' ...or without a local install: npx -y -p node@22 node node_modules/jest/bin/jest.js \n' + + 'See backend/TESTING.md for the full picture.\n', + ); + } + // Integration runs use a real Mongo (MONGO_URI); no in-memory binary needed. if (process.env.INTEGRATION_TEST === 'true') return; diff --git a/docs/development/agent-experience-audit.md b/docs/development/agent-experience-audit.md index cd2623904..cce5a4b91 100644 --- a/docs/development/agent-experience-audit.md +++ b/docs/development/agent-experience-audit.md @@ -3760,3 +3760,34 @@ orientation/tool-description tests; and align the CLI fork frame with the same AND semantics. The decision tool posts its own ask, remains advisory (not privileged-action consent), and does not require unrelated work to stop while a ruling is pending. + +## 57. A documented remedy that its own failure never points at (2026-09-18, sprint-impl) + +*Origin observation: sprint-impl running TASK-130's backend suites in this +checkout; verification: `backend/TESTING.md` ("Node 25+ kills any suite whose +require graph reaches `buffer-equal-constant-time`"), `backend/__tests__/utils/globalSetup.js`, +measured with `npx -y -p node@{22,24,25,26} node -e "typeof require('buffer').SlowBuffer"`.* + +`backend/TESTING.md` already carried the whole story of this trap, including the +exact Node-22 invocation. The seat still lost the detour, because the failure it +describes points nowhere near the doc. Two dependency-shaped errors arrived in a +row, neither naming a Node version: first +`Module ts-jest in the transform option was not found` (this checkout simply had +no `backend/node_modules`), then, after `npm ci`, +`TypeError: Cannot read properties of undefined (reading 'prototype')` raised +four frames under `jws` inside a package the test never imports. The remedy is a +paragraph in a testing doc — findable only if the version is already suspected. + +Two of the doc's own facts were also off by one release boundary, so even a +reader who found it could be misled: measured, `buffer.SlowBuffer` is a +`function` on Node 22 and 24 and `undefined` on 25 and 26. The heading said +"Node 26", which understates the range and sends a Node 25 user looking +elsewhere. + +**Repair:** the doc now states the measured boundary rather than the version on +which it was first noticed, and `globalSetup` writes the remedy to stderr when +Node ≥ 25, before any worker fails. One line where the consumer is, instead of a +paragraph where the consumer is not. Rule: when a doc exists to explain a +failure that a bare dependency error will produce, put the pointer at the +failure, not only in the doc — and state removals as measured boundaries, not as +the version of the machine you happened to hit them on.