diff --git a/tests/agent-install-check.test.cjs b/tests/agent-install-check.test.cjs index ff2391d5d..a20041e6d 100644 --- a/tests/agent-install-check.test.cjs +++ b/tests/agent-install-check.test.cjs @@ -20,7 +20,7 @@ const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { createTempDir, cleanup } = require('./helpers.cjs'); +const { createTempDir, cleanup, captureFdSync } = require('./helpers.cjs'); const AGENT_INSTALL_CHECK_PATH = path.join( __dirname, '..', 'gsd-core', 'bin', 'lib', 'agent-install-check.cjs' @@ -1183,7 +1183,7 @@ describe('cmdValidateAgents surfaces the Codex posture result (#3242 row 20)', ( cleanup(tmpDir); }); - test('row 20: validate agents output carries the posture result for a violating install', (t) => { + test('row 20: validate agents output carries the posture result for a violating install', () => { const { cmdValidateAgents } = require( path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'verify.cjs'), ); @@ -1193,22 +1193,9 @@ describe('cmdValidateAgents surfaces the Codex posture result (#3242 row 20)', ( `name = "${EXPECTED_AGENTS[0]}"\nmodel = "sonnet"\ndeveloper_instructions = '''\nWork.\n'''\n`, ); - const written = []; - const realWriteSync = fs.writeSync; - t.mock.method(fs, 'writeSync', (fd, data, offset, length) => { - if (fd !== 1) { - return realWriteSync.call(fs, fd, data, offset, length); - } - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - written.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }); + const written = captureFdSync(1, () => cmdValidateAgents(tmpDir, false)); - cmdValidateAgents(tmpDir, false); - - const parsed = JSON.parse(written.join('')); + const parsed = JSON.parse(written); assert.ok( parsed.codex_posture, `expected cmdValidateAgents output to carry a codex_posture key, got keys: ${Object.keys(parsed).join(', ')}`, diff --git a/tests/codex-inherit-smoke.test.cjs b/tests/codex-inherit-smoke.test.cjs index 5e9a66f2a..5f7f6cd28 100644 --- a/tests/codex-inherit-smoke.test.cjs +++ b/tests/codex-inherit-smoke.test.cjs @@ -56,7 +56,7 @@ const { install } = require('../bin/install.js'); const { checkCodexModelPosture } = require('../gsd-core/bin/lib/agent-install-check.cjs'); const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs'); const { parseCodexAgentToml } = require('../gsd-core/bin/lib/codex-agent-toml.cjs'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, captureFdSync } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); @@ -149,17 +149,9 @@ function findKeyLine(header, key) { * structure, never prose. */ function runEffortSyncDryRun(codexHome) { - const origWriteSync = fs.writeSync; - let captured = ''; - fs.writeSync = (fd, data) => { - if (fd === 1) { captured += data; return data.length; } - return origWriteSync(fd, data); - }; - try { - cmdEffortSync(codexHome, false, { dryRun: true, configDir: codexHome, runtime: 'codex' }); - } finally { - fs.writeSync = origWriteSync; - } + const captured = captureFdSync(1, () => + cmdEffortSync(codexHome, false, { dryRun: true, configDir: codexHome, runtime: 'codex' }) + ); return JSON.parse(captured); } diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index c8184d768..ed7dab856 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -4343,7 +4343,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, captureFdSync } = require('./helpers.cjs'); const { runNode } = require('./helpers/process-seam.cjs'); const { toLegacyResult } = require('./helpers/git-fixture.cjs'); @@ -4369,18 +4369,7 @@ function makeTmpDir(prefix) { // output() in core.cjs uses fs.writeSync(1, data) — intercept fd=1 writes. // Pass raw=false so output() emits JSON (raw=true emits the plain rawValue string). function captureOutput(fn) { - const origWriteSync = fs.writeSync; - let captured = ''; - fs.writeSync = (fd, data) => { - if (fd === 1) captured += data; - else origWriteSync(fd, data); - }; - try { - fn(); - } finally { - fs.writeSync = origWriteSync; - } - return JSON.parse(captured); + return JSON.parse(captureFdSync(1, fn)); } function makeAgentsDir(tmpDir) { diff --git a/tests/commit-docs-bypass.test.cjs b/tests/commit-docs-bypass.test.cjs index d769ccf66..23d188ba7 100644 --- a/tests/commit-docs-bypass.test.cjs +++ b/tests/commit-docs-bypass.test.cjs @@ -844,7 +844,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const fc = require('./helpers/fast-check-setup.cjs'); -const { cleanup, createTempGitProject } = require('./helpers.cjs'); +const { cleanup, createTempGitProject, captureFdSync } = require('./helpers.cjs'); const { seedPhase } = require('./fixtures/index.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); @@ -881,23 +881,7 @@ function writeConfig(tmpDir, config) { * tests/config-get-default.test.cjs's captureFdWrite. */ function captureFdWrite(fd, fn) { - const orig = fs.writeSync; - let captured = Buffer.alloc(0); - fs.writeSync = (writeFd, ...rest) => { - if (writeFd !== fd) return orig.call(fs, writeFd, ...rest); - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)) - : Buffer.from(String(data), 'utf8'); - captured = Buffer.concat([captured, chunk]); - return chunk.length; - }; - try { - fn(); - } finally { - fs.writeSync = orig; - } - return captured.toString('utf-8'); + return captureFdSync(fd, fn); } function runCommit(tmpDir, message, files) { @@ -912,28 +896,21 @@ function runCommit(tmpDir, message, files) { * ExitError-throwing path (ADR-3889 — error() throws ExitError directly, it * no longer calls process.exit()). */ function runConfigSetExpectError(tmpDir, keyPath, value) { - const origWriteSync = fs.writeSync; io.setJsonErrorMode(true); - let stderr = ''; - fs.writeSync = (fd, ...rest) => { - if (fd !== 2) return origWriteSync.call(fs, fd, ...rest); - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)).toString('utf8') - : String(data); - stderr += chunk; - return Buffer.byteLength(chunk); - }; + let capturedStderr = ''; try { - configCli.cmdConfigSet(tmpDir, keyPath, value, true); - assert.fail('expected cmdConfigSet to throw ExitError'); - } catch (e) { - if (!(e instanceof ExitError)) throw e; + capturedStderr = captureFdSync(2, () => { + try { + configCli.cmdConfigSet(tmpDir, keyPath, value, true); + assert.fail('expected cmdConfigSet to throw ExitError'); + } catch (inner) { + if (!(inner instanceof ExitError)) throw inner; + } + }); } finally { - fs.writeSync = origWriteSync; io.setJsonErrorMode(false); } - const parts = stderr.split('\n').filter(Boolean); + const parts = capturedStderr.split('\n').filter(Boolean); let payload = {}; try { payload = JSON.parse(parts[parts.length - 1]); } catch { /* leave {} */ } return payload; diff --git a/tests/config-get-default.test.cjs b/tests/config-get-default.test.cjs index 2a3bd5abe..450f0cb4c 100644 --- a/tests/config-get-default.test.cjs +++ b/tests/config-get-default.test.cjs @@ -14,7 +14,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, captureFdSync } = require('./helpers.cjs'); // In-process invocation, not execFileSync: cmdConfigGet is a pure CJS // function reachable without spawning `node` as a child. The prior @@ -65,23 +65,7 @@ const { ExitError } = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib * would have hit the fd. */ function captureFdWrite(fd, fn) { - const orig = fs.writeSync; - let captured = Buffer.alloc(0); - fs.writeSync = (writeFd, ...rest) => { - if (writeFd !== fd) return orig.call(fs, writeFd, ...rest); - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)) - : Buffer.from(String(data), 'utf8'); - captured = Buffer.concat([captured, chunk]); - return chunk.length; - }; - try { - fn(); - } finally { - fs.writeSync = orig; - } - return captured.toString('utf-8'); + return captureFdSync(fd, fn); } /** @@ -276,39 +260,28 @@ describe('config-get --default flag (#1893)', () => { function runExpectError(...args) { const { keyPath, raw, defaultValue } = parseConfigGetArgs(args); - const origWriteSync = fs.writeSync; io.setJsonErrorMode(true); - let writeCount = 0; - let stderr = ''; - fs.writeSync = (fd, ...rest) => { - if (fd !== 2) return origWriteSync.call(fs, fd, ...rest); - writeCount++; - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)).toString('utf8') - : String(data); - stderr += chunk; - return Buffer.byteLength(chunk); - }; - const lastError = () => { - const parts = stderr.split('\n').filter(Boolean); - try { return JSON.parse(parts[parts.length - 1]); } catch { return {}; } - }; - // ADR-3889: error() throws ExitError directly; catch it here rather - // than mocking process.exit. let exitCode; + let stderr; try { - config.cmdConfigGet(tmpDir, keyPath, raw, defaultValue); - assert.fail('expected cmdConfigGet to throw ExitError'); - } catch (e) { - if (!(e instanceof ExitError)) throw e; - exitCode = e.code; + stderr = captureFdSync(2, () => { + try { + config.cmdConfigGet(tmpDir, keyPath, raw, defaultValue); + assert.fail('expected cmdConfigGet to throw ExitError'); + } catch (e) { + if (!(e instanceof ExitError)) throw e; + exitCode = e.code; + } + }); } finally { - fs.writeSync = origWriteSync; io.setJsonErrorMode(false); } + const lines = stderr.split('\n').filter(Boolean); + const lastError = () => { + try { return JSON.parse(lines[lines.length - 1]); } catch { return {}; } + }; assert.ok(exitCode !== 0 && exitCode !== undefined, 'Expected non-zero exit code'); - assert.equal(writeCount, 1, 'error() must fire exactly once'); + assert.equal(lines.length, 1, 'error() must emit exactly one stderr line'); const payload = lastError(); return { status: exitCode, reason: payload.reason, message: payload.message, stderr }; } diff --git a/tests/core-utils.test.cjs b/tests/core-utils.test.cjs index 8f08a7783..94cbe2b36 100644 --- a/tests/core-utils.test.cjs +++ b/tests/core-utils.test.cjs @@ -32,6 +32,7 @@ const { SCOPE } = require('../gsd-core/bin/lib/planning-scope.cjs'); const { cleanup, runGsdTools, scrubConfigLocationEnv, saveSessionEnv, restoreSessionEnv, clearSessionEnv, TEST_HOME_SANDBOX_MARKER, + captureFdSync, } = require('./helpers.cjs'); const phaseLocator = require('../gsd-core/bin/lib/phase-locator.cjs'); const phaseId = require('../gsd-core/bin/lib/phase-id.cjs'); @@ -63,23 +64,7 @@ const initMod = require('../gsd-core/bin/lib/init.cjs'); // `~/.gsd` config or leak real session-identity env into the slug output. function captureFd1Sync(fn) { - const chunks = []; - const origWriteSync = fs.writeSync.bind(fs); - fs.writeSync = (fd, data, offset, length) => { - if (fd === 2) return Buffer.isBuffer(data) ? data.length : String(data).length; - if (fd !== 1) return origWriteSync(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - chunks.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }; - try { - fn(); - } finally { - fs.writeSync = origWriteSync; - } - return chunks.join(''); + return captureFdSync(1, fn); } function withHermeticInProcessEnv(dir, fn) { diff --git a/tests/discuss-mode.test.cjs b/tests/discuss-mode.test.cjs index 846279d42..be3656b20 100644 --- a/tests/discuss-mode.test.cjs +++ b/tests/discuss-mode.test.cjs @@ -8,7 +8,7 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { createTempProject, cleanup } = require('./helpers.cjs'); +const { createTempProject, cleanup, captureFdSync } = require('./helpers.cjs'); describe('workflow.discuss_mode config', () => { test('config template includes discuss_mode default', () => { @@ -153,26 +153,10 @@ describe('workflow.discuss_mode config', () => { // cmdInitPlanPhase writes its JSON result directly to fd 1 via // io.cjs's writeAllSync (bypasses console.log — captureConsole() - // cannot observe it). Monkeypatch fs.writeSync and restore it in a - // finally, the project's standard IO-capture seam. - const orig = fs.writeSync; - let captured = Buffer.alloc(0); - fs.writeSync = (fd, ...rest) => { - if (fd !== 1) return orig.call(fs, fd, ...rest); - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)) - : Buffer.from(String(data), 'utf8'); - captured = Buffer.concat([captured, chunk]); - return chunk.length; - }; - let result; - try { - cmdInitPlanPhase(cwd, 'does-not-exist', false, {}); - result = JSON.parse(captured.toString('utf8')); - } finally { - fs.writeSync = orig; - } + // cannot observe it). Delegates to the shared, safe fd-capture helper + // (#4306) — see tests/helpers.cjs's captureFdSync. + const captured = captureFdSync(1, () => cmdInitPlanPhase(cwd, 'does-not-exist', false, {})); + const result = JSON.parse(captured); assert.strictEqual( result.text_mode, true, 'cmdInitPlanPhase result must propagate config.workflow.text_mode' diff --git a/tests/estimate-calibrate.test.cjs b/tests/estimate-calibrate.test.cjs index d5b6b66a1..5d83c0016 100644 --- a/tests/estimate-calibrate.test.cjs +++ b/tests/estimate-calibrate.test.cjs @@ -21,7 +21,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); +const { createTempProject, cleanup, runGsdTools, captureFdSync } = require('./helpers.cjs'); const est = require('../gsd-core/bin/lib/phase-estimation.cjs'); const estimateCli = require('../gsd-core/bin/lib/estimate-cli.cjs'); // io.cjs owns error()/ERROR_REASON/JSON-error-mode — driven directly here so @@ -549,37 +549,28 @@ describe('unreadable phases directory refuses calibration (#3882, ADR-3473 §8.5 * throws (ADR-3889 — error() no longer calls process.exit() directly) with * stderr(fd 2) captured. */ function runCalibrateExpectError(tmpDir) { - const origWriteSync = fs.writeSync; io.setJsonErrorMode(true); - let writeCount = 0; - let stderr = ''; - fs.writeSync = (fd, ...rest) => { - if (fd !== 2) return origWriteSync.call(fs, fd, ...rest); - writeCount++; - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)).toString('utf8') - : String(data); - stderr += chunk; - return Buffer.byteLength(chunk); - }; - const lastError = () => { - const parts = stderr.split('\n').filter(Boolean); - try { return JSON.parse(parts[parts.length - 1]); } catch { return {}; } - }; let exitCode; + let stderr; try { - estimateCli.cmdEstimateCalibrate(tmpDir, [], false); - assert.fail('expected cmdEstimateCalibrate to throw ExitError'); - } catch (e) { - if (!(e instanceof ExitError)) throw e; - exitCode = e.code; + stderr = captureFdSync(2, () => { + try { + estimateCli.cmdEstimateCalibrate(tmpDir, [], false); + assert.fail('expected cmdEstimateCalibrate to throw ExitError'); + } catch (e) { + if (!(e instanceof ExitError)) throw e; + exitCode = e.code; + } + }); } finally { - fs.writeSync = origWriteSync; io.setJsonErrorMode(false); } + const lines = stderr.split('\n').filter(Boolean); + const lastError = () => { + try { return JSON.parse(lines[lines.length - 1]); } catch { return {}; } + }; assert.ok(exitCode !== 0 && exitCode !== undefined, 'expected a non-zero exit code'); - assert.equal(writeCount, 1, 'error() must fire exactly once'); + assert.equal(lines.length, 1, 'error() must emit exactly one stderr line'); return { status: exitCode, ...lastError() }; } diff --git a/tests/gap-checker.property.test.cjs b/tests/gap-checker.property.test.cjs index 635e84b1b..0bf540f1a 100644 --- a/tests/gap-checker.property.test.cjs +++ b/tests/gap-checker.property.test.cjs @@ -955,7 +955,7 @@ describe('#3885 (ADR-3473 §8.5): runGapAnalysis distinguishes unreadable from a describe('#4014 matrix row 15: unreadable-vs-empty identity is consistent across roadmap/gap-checker/init', () => { const fs = require('fs'); const path = require('path'); - const { createTempProject, cleanup } = require('./helpers.cjs'); + const { createTempProject, cleanup, captureFdSync } = require('./helpers.cjs'); const { runGapAnalysis } = require('../gsd-core/bin/lib/gap-checker.cjs'); const roadmapLib = require('../gsd-core/bin/lib/roadmap.cjs'); const initMod = require('../gsd-core/bin/lib/init.cjs'); @@ -976,22 +976,7 @@ describe('#4014 matrix row 15: unreadable-vs-empty identity is consistent across // `output()` writes via `fs.writeSync(1, ...)`, bypassing console.log — see // init.test.cjs's captureFd1 for the identical rationale/pattern. function captureFd1(run) { - const chunks = []; - const origWriteSync = fs.writeSync; - fs.writeSync = function patchedWriteSync(fd, data, offset, length) { - if (fd !== 1) return origWriteSync.apply(fs, arguments); - const buf = Buffer.isBuffer(data) ? data : Buffer.from(data); - const start = offset ?? 0; - const len = length ?? (buf.length - start); - chunks.push(Buffer.from(buf.subarray(start, start + len))); - return len; - }; - try { - run(); - } finally { - fs.writeSync = origWriteSync; - } - return JSON.parse(Buffer.concat(chunks).toString('utf8')); + return JSON.parse(captureFdSync(1, run)); } test('the same unreadable phase directory reports SCOPE.UNREADABLE consistently on all three surfaces', (t) => { diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 2bb62928c..5e47c69fe 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -578,6 +578,68 @@ function mockPartialWriteThenThrow(fsModule, matches, bytesBeforeThrow, options return () => { fsModule.writeFileSync = origWriteFileSync; }; } +/** + * Capture the bytes written to `captureFd` while `fn()` runs, WITHOUT ever + * fabricating a byte count for any fd (#4306). + * + * Every previous hand-rolled version of this idiom across the test suite + * mocked `fs.writeSync`, and on its "success" arm returned a fabricated byte + * count while pushing the bytes into a local array instead of ever calling + * the real `fs.writeSync` — the data reached nowhere but that array. That is + * unsafe: Node's `node:test` runner defaults to `--test-isolation=process` + * (Node >= 22), so each test file's own real stdout is what the PARENT + * runner reads to parse its child-to-parent result/TAP protocol. If the + * runner's own reporter write for an adjacent test lands on the mocked fd + * during this window, a mock that fabricates success without delivering the + * bytes silently swallows that write instead of letting it reach the real + * pipe — the parent then tries to parse a truncated stream, observed in CI + * as "Unable to deserialize cloned data" (Node's generic corrupted/truncated + * v8.deserialize error), not as a thrown exception. + * + * This helper always forwards every write, on every fd, to the real + * `fs.writeSync` first — so nothing is ever swallowed, regardless of what + * else shares the fd during the mocked window — and returns the REAL + * result. Only `captureFd`'s traffic is additionally recorded and returned + * to the caller as a joined UTF-8 string; every other fd's bytes still + * reach their real destination (e.g. a test's own stderr diagnostics still + * physically write to stderr, just outside the returned capture), they are + * simply not included in the returned string. + * + * Standalone — no node:test context required; save/restore in a `finally` + * so a thrown assertion still restores the real `fs.writeSync`. + * + * @param {number} captureFd - the fd to capture and return (1 for stdout, 2 for stderr). + * @param {() => void} fn - synchronous function to run while capturing. + * @returns {string} every byte actually written to `captureFd` during `fn()`. + */ +function captureFdSync(captureFd, fn) { + const chunks = []; + const orig = fs.writeSync; + fs.writeSync = (fd, data, ...rest) => { + const n = orig.call(fs, fd, data, ...rest); + if (fd === captureFd) { + // rest[0] is `offset` only for the buffer-form overload; the + // string-form overload's 2nd arg is `position`, which is irrelevant + // here since a string write has no byte offset into `data` itself. + const offset = Buffer.isBuffer(data) && typeof rest[0] === 'number' ? rest[0] : 0; + const buf = Buffer.isBuffer(data) + ? data.subarray(offset, offset + n) + : Buffer.from(String(data), 'utf8').subarray(0, n); + // Buffered, not decoded per-call: a real short write can split a + // multi-byte UTF-8 codepoint across two writeSync calls, and decoding + // each half separately would corrupt it. Decode once, after joining. + chunks.push(buf); +} + return n; +}; + try { + fn(); + } finally { + fs.writeSync = orig; +} + return Buffer.concat(chunks).toString('utf8'); +} + /** * Read a workflow .md file plus every .md file under its sibling * `/steps/` directory, concatenated in document order @@ -1111,7 +1173,7 @@ function sandboxHome(t, dir) { }); } -module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, tmpRootCandidates, readFileNormalized, readWorkflowCombined, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, absPlanningPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, SESSION_ENV_KEYS, saveSessionEnv, restoreSessionEnv, clearSessionEnv, isolateWorkstreamEnv, restoreWorkstreamEnv, TOOLS_PATH, SESSION_IDENTITY_ENV_KEYS, scrubConfigLocationEnv, installSpawnEnv, installSpawnHome, sandboxHome, TEST_HOME_SANDBOX_MARKER, mockPartialWriteThenThrow }; +module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, tmpRootCandidates, readFileNormalized, readWorkflowCombined, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, absPlanningPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, SESSION_ENV_KEYS, saveSessionEnv, restoreSessionEnv, clearSessionEnv, isolateWorkstreamEnv, restoreWorkstreamEnv, TOOLS_PATH, SESSION_IDENTITY_ENV_KEYS, scrubConfigLocationEnv, installSpawnEnv, installSpawnHome, sandboxHome, TEST_HOME_SANDBOX_MARKER, mockPartialWriteThenThrow, captureFdSync }; // Lazy, for the reason builtLib() is lazy: reading either of these is what // forces the built-lib require, so a test file that needs neither can still diff --git a/tests/init.test.cjs b/tests/init.test.cjs index e415f7580..6c8fb5c54 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -7,7 +7,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const { spawnSync } = require('node:child_process'); -const { runGsdTools, cleanup, absPlanningPath, TOOLS_PATH, parseFrontmatter } = require('./helpers.cjs'); +const { runGsdTools, cleanup, absPlanningPath, TOOLS_PATH, parseFrontmatter, captureFdSync } = require('./helpers.cjs'); const { createFixture, seedPhase } = require('./fixtures/index.cjs'); const { createTempProject, createTempDir } = require('./helpers.cjs'); const { executionContextRefs } = require('../scripts/command-contract-helpers.cjs'); @@ -3266,19 +3266,7 @@ describe('#3057 B3: cmdInitVerifyWork — verification staleness-check indetermi * it were stdout. */ function captureInitVerifyWork(t, cwd, phase) { - const chunks = []; - const origWriteSync = fs.writeSync.bind(fs); - t.mock.method(fs, 'writeSync', (fd, data, offset, length) => { - if (fd === 2) return Buffer.isBuffer(data) ? data.length : String(data).length; - if (fd !== 1) return origWriteSync(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - chunks.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }); - initMod.cmdInitVerifyWork(cwd, phase, false); - const captured = chunks.join(''); + const captured = captureFdSync(1, () => initMod.cmdInitVerifyWork(cwd, phase, false)); assert.ok(captured.length > 0, 'cmdInitVerifyWork produced no stdout output'); return captured; } @@ -3379,19 +3367,7 @@ describe('#3885 (ADR-3473 §8.5): init callers distinguish unreadable from absen }); function captureFd1(t, run) { - const chunks = []; - const origWriteSync = fs.writeSync.bind(fs); - t.mock.method(fs, 'writeSync', (fd, data, offset, length) => { - if (fd === 2) return Buffer.isBuffer(data) ? data.length : String(data).length; - if (fd !== 1) return origWriteSync(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - chunks.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }); - run(); - const captured = chunks.join(''); + const captured = captureFdSync(1, run); assert.ok(captured.length > 0, 'command produced no stdout output'); return JSON.parse(captured); } diff --git a/tests/io.test.cjs b/tests/io.test.cjs index 3b4b534fc..2d94c48e2 100644 --- a/tests/io.test.cjs +++ b/tests/io.test.cjs @@ -395,7 +395,9 @@ function bug1008ChunkOf(data, offset, length) { const end = length === undefined ? data.length : start + length; return data.subarray(start, end).toString('utf8'); } - return String(data); + const str = String(data); + if (length === undefined) return str; + return Buffer.from(str, 'utf8').subarray(0, length).toString('utf8'); } function bug1008WriteError(code, errno) { @@ -415,9 +417,19 @@ describe('bug #1008: io.output() tolerates a full / slow non-blocking pipe', () if (fd !== 1) return orig(fd, data, offset, length); calls += 1; if (calls === 1) throw bug1008WriteError('EAGAIN', -11); // pipe momentarily full - const chunk = bug1008ChunkOf(data, offset, length); - written.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); + // #4306: deliver the retried write for real via `orig` rather than + // fabricating a byte count while discarding it. node:test's own + // process-isolated runner reads this file's real stdout to parse its + // child-to-parent result protocol; a stray write from that protocol + // landing on fd 1 while this mock is installed would otherwise be + // silently swallowed instead of reaching the real pipe, corrupting the + // parent's parse ("Unable to deserialize cloned data"). The pushed + // chunk is derived from `orig`'s real return count (never the + // requested length) so a genuine short write from the real fd is + // reflected accurately, matching the short-write test's pattern below. + const n = orig(fd, data, offset, length); + written.push(bug1008ChunkOf(data, offset, n)); + return n; }); const payload = { ok: true, n: 42 }; @@ -434,9 +446,11 @@ describe('bug #1008: io.output() tolerates a full / slow non-blocking pipe', () if (fd !== 1) return orig(fd, data, offset, length); calls += 1; if (calls === 1) throw bug1008WriteError('EINTR', -4); - const chunk = bug1008ChunkOf(data, offset, length); - written.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); + // #4306: see the EAGAIN test above — deliver the retried write for + // real, and derive the pushed chunk from its real return count. + const n = orig(fd, data, offset, length); + written.push(bug1008ChunkOf(data, offset, n)); + return n; }); assert.doesNotThrow(() => io.output('plain', true, 'PLAIN-RAW')); @@ -449,10 +463,29 @@ describe('bug #1008: io.output() tolerates a full / slow non-blocking pipe', () const orig = fs.writeSync.bind(fs); t.mock.method(fs, 'writeSync', (fd, data, offset, length) => { if (fd !== 1) return orig(fd, data, offset, length); + if (!Buffer.isBuffer(data)) { + // Only Buffer-form writes (what io.output actually produces) are + // subject to the simulated short-write cap below. Anything else + // sharing fd 1 during this window (e.g. node:test's own interleaved + // report traffic, which can be string-form) must pass through with + // its real arguments — forcing it through the Buffer-shaped + // truncation math below would corrupt fs.writeSync's string-form + // overload (its 3rd/4th args are position/encoding, not + // offset/length). + return orig(fd, data, offset, length); + } const chunk = bug1008ChunkOf(data, offset, length); const part = chunk.slice(0, CAP); - written.push(part); - return Buffer.byteLength(part, 'utf8'); + const partLen = Buffer.byteLength(part, 'utf8'); + // #4306: deliver the truncated slice for real via `orig`, at the + // simulated cap, instead of fabricating a byte count while discarding + // the write — see the EAGAIN test above for why a swallowed write on a + // shared fd is unsafe. `orig`'s own return is used below so a real + // short write from the kernel (rather than our simulated one) is still + // reflected accurately. + const n = orig(fd, data, offset ?? 0, partLen); + written.push(bug1008ChunkOf(data, offset, n)); + return n; }); const payload = { message: 'a reasonably long ascii payload to force many short writes' }; @@ -508,9 +541,13 @@ describe('bug #1008: io.error() tolerates a full non-blocking stderr pipe', () = if (fd !== 2) return restore(fd, data, offset, length); calls += 1; if (calls === 1) throw bug1008WriteError('EAGAIN', -11); - const chunk = bug1008ChunkOf(data, offset, length); - written.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); + // #4306: forward the retried write to the real writeSync instead of + // fabricating a byte count — see the io.output() EAGAIN test above. + // The pushed chunk is derived from the real return count, not the + // requested length. + const n = restore(fd, data, offset, length); + written.push(bug1008ChunkOf(data, offset, n)); + return n; }; try { assert.throws( diff --git a/tests/milestone-lock.test.cjs b/tests/milestone-lock.test.cjs index 1ae00a6d1..9110f93a2 100644 --- a/tests/milestone-lock.test.cjs +++ b/tests/milestone-lock.test.cjs @@ -31,7 +31,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const helpers = require('./helpers.cjs'); -const { runGsdTools, createTempProject, cleanup, TOOLS_PATH } = helpers; +const { runGsdTools, createTempProject, cleanup, TOOLS_PATH, captureFdSync } = helpers; const processSeam = require('./helpers/process-seam.cjs'); const { collectSection } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs'); @@ -552,24 +552,16 @@ describe('#3311 guard: phase.complete does not lose a concurrent STATE.md write' }, }); - // Swallow fd-1 writes (io.output writes JSON straight to fd 1). - const origWriteSync = fs.writeSync; + // Capture fd-1 writes (io.output writes JSON straight to fd 1). + // Delegates to the shared, safe fd-capture helper (#4306) — see + // tests/helpers.cjs's captureFdSync. const captured = []; - fs.writeSync = function patchedWriteSync(fd, data, ...rest) { - if (fd === 1) { - captured.push(String(data)); - return String(data).length; - } - return origWriteSync(fd, data, ...rest); - }; - let threw = null; try { - phaseMod.cmdPhaseComplete(tmpDir, '1', true); + captured.push(captureFdSync(1, () => phaseMod.cmdPhaseComplete(tmpDir, '1', true))); } catch (e) { threw = e; } finally { - fs.writeSync = origWriteSync; stateMod._resetStateLockTestHooks(); stateMod._resetLockProbes(); } diff --git a/tests/phase-locator.test.cjs b/tests/phase-locator.test.cjs index ac2fba663..3150a89a3 100644 --- a/tests/phase-locator.test.cjs +++ b/tests/phase-locator.test.cjs @@ -28,6 +28,7 @@ const phaseLocator = require('../gsd-core/bin/lib/phase-locator.cjs'); const planDependencyGraph = require('../gsd-core/bin/lib/plan-dependency-graph.cjs'); const { runGsdTools, createTempProject, createTempDir, cleanup, isolateWorkstreamEnv, restoreWorkstreamEnv, + captureFdSync, } = require('./helpers.cjs'); const driftGuard = require('../scripts/lint-phase-enumeration-drift.cjs'); @@ -1613,25 +1614,9 @@ describe('sentinel-range boundaries, driven through the new API (#3882 rows D1-D // ─── E. Migrated call sites ───────────────────────────────────────────── describe('migrated exemptions behave identically (#3882 rows E1/E2)', () => { - /** Capture whatever a synchronous fn writes to `fd` via fs.writeSync, without touching the real fd. */ + /** Capture whatever a synchronous fn writes to `fd` via fs.writeSync — delegates to the shared, safe helper (#4306) that always delivers real bytes rather than fabricating a byte count. */ function captureFdWrite(fd, fn) { - const orig = fs.writeSync; - let captured = Buffer.alloc(0); - fs.writeSync = (writeFd, ...rest) => { - if (writeFd !== fd) return orig.call(fs, writeFd, ...rest); - const [data, offset = 0, length] = rest; - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset, offset + (length ?? data.length - offset)) - : Buffer.from(String(data), 'utf8'); - captured = Buffer.concat([captured, chunk]); - return chunk.length; - }; - try { - fn(); - } finally { - fs.writeSync = orig; - } - return captured.toString('utf-8'); + return captureFdSync(fd, fn); } /** diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index 957065f3d..1d4494bbe 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -10,7 +10,7 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { runGsdTools, createTempProject, cleanup, captureFdSync } = require('./helpers.cjs'); const { scanFencedBlocks } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs'); describe('roadmap get-phase command', () => { @@ -1202,19 +1202,7 @@ describe('#3057 B3: roadmap update-plan-progress — verification staleness-chec * corrupts the captured payload into two concatenated JSON objects). */ function captureUpdatePlanProgress(t, cwd, phaseNum) { - const chunks = []; - const origWriteSync = fs.writeSync.bind(fs); - t.mock.method(fs, 'writeSync', (fd, data, offset, length) => { - if (fd === 2) return Buffer.isBuffer(data) ? data.length : String(data).length; - if (fd !== 1) return origWriteSync(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - chunks.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }); - roadmapMod.cmdRoadmapUpdatePlanProgress(cwd, phaseNum, false); - const captured = chunks.join(''); + const captured = captureFdSync(1, () => roadmapMod.cmdRoadmapUpdatePlanProgress(cwd, phaseNum, false)); assert.ok(captured.length > 0, 'cmdRoadmapUpdatePlanProgress produced no stdout output'); return captured; } @@ -4374,26 +4362,16 @@ describe('#3957 (epic #3473 B9): no-op decline reports the real condition', () = // Mirrors state.test.cjs's captureCliIO — see that file's doc comment. function captureCliIO(fn) { - const originalWriteSync = fs.writeSync; const originalStderrWrite = process.stderr.write.bind(process.stderr); - let stdout = ''; let stderr = ''; - fs.writeSync = (fd, data, offset, length) => { - if (fd !== 1) return originalWriteSync(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - stdout += chunk; - return Buffer.byteLength(chunk, 'utf8'); - }; process.stderr.write = (chunk) => { stderr += String(chunk); return true; }; + let stdout; try { - fn(); + stdout = captureFdSync(1, fn); } finally { - fs.writeSync = originalWriteSync; process.stderr.write = originalStderrWrite; } return { stdout, stderr }; diff --git a/tests/state-rebuild-cli.test.cjs b/tests/state-rebuild-cli.test.cjs index 70f1030f6..67f9d1cc5 100644 --- a/tests/state-rebuild-cli.test.cjs +++ b/tests/state-rebuild-cli.test.cjs @@ -18,6 +18,7 @@ const { createTempProject, cleanup, runGsdTools, + captureFdSync, } = require('./helpers.cjs'); const { withFaultyFs } = require('./helpers/faulty-deps.cjs'); const stateMod = require('../gsd-core/bin/lib/state.cjs'); @@ -216,27 +217,12 @@ function hasRebuildLogSection(content) { * Run `fn` while capturing every fd-1 write `cmdStateRebuild`'s `output()` * performs (it writes via a raw `fs.writeSync(1, ...)`, never * `console.log`/`process.stdout.write` — same seam `tests/io.test.cjs` - * exercises for bug #1008). Standalone helper with no test context, so the - * try/finally restore is CONTRIBUTING-compliant (same shape as - * `withFaultyFs`). + * exercises for bug #1008). Delegates to the shared, safe helper (#4306) — + * see `tests/helpers.cjs`'s `captureFdSync` for why a hand-rolled version + * that fabricates a byte count instead of forwarding is unsafe. */ function captureStdout(fn) { - const chunks = []; - const original = fs.writeSync; - fs.writeSync = (fd, data, offset, length) => { - if (fd !== 1) return original(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - chunks.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }; - try { - fn(); - } finally { - fs.writeSync = original; - } - return chunks.join(''); + return captureFdSync(1, fn); } // --------------------------------------------------------------------------- diff --git a/tests/state.test.cjs b/tests/state.test.cjs index ae796c657..ee531fec1 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -12,7 +12,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup } = require('./helpers.cjs'); +const { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, captureFdSync } = require('./helpers.cjs'); const { createFixture, seedWorkstream, writeState } = require('./fixtures/index.cjs'); // ADR-3473 §8.7 (#3872): git-fixture spawns for the state_head rows (10/11) // go through the throw-preserving wrapper, never a raw execFileSync. @@ -125,22 +125,7 @@ function bodyProgressPercent(stateMdContent) { * subprocess would not observe the parent process's mock). */ function captureStdout(fn) { - const chunks = []; - const original = fs.writeSync; - fs.writeSync = (fd, data, offset, length) => { - if (fd !== 1) return original(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - chunks.push(chunk); - return Buffer.byteLength(chunk, 'utf8'); - }; - try { - fn(); - } finally { - fs.writeSync = original; - } - return chunks.join(''); + return captureFdSync(1, fn); } function readShippedStateTemplateBody(replacements) { @@ -19505,26 +19490,16 @@ describe('#3957 (epic #3473 B9): no-op decline reports the real condition', () = * subprocess-based runGsdTools drops stderr on a clean exit. */ function captureCliIO(fn) { - const originalWriteSync = fs.writeSync; const originalStderrWrite = process.stderr.write.bind(process.stderr); let stdout = ''; let stderr = ''; - fs.writeSync = (fd, data, offset, length) => { - if (fd !== 1) return originalWriteSync(fd, data, offset, length); - const chunk = Buffer.isBuffer(data) - ? data.subarray(offset ?? 0, length === undefined ? data.length : (offset ?? 0) + length).toString('utf8') - : String(data); - stdout += chunk; - return Buffer.byteLength(chunk, 'utf8'); - }; process.stderr.write = (chunk) => { stderr += String(chunk); return true; }; try { - fn(); + stdout = captureFdSync(1, fn); } finally { - fs.writeSync = originalWriteSync; process.stderr.write = originalStderrWrite; } return { stdout, stderr }; diff --git a/tests/task-command-router-resolve-content.test.cjs b/tests/task-command-router-resolve-content.test.cjs index 67e7c9c7c..d6e8e1285 100644 --- a/tests/task-command-router-resolve-content.test.cjs +++ b/tests/task-command-router-resolve-content.test.cjs @@ -14,12 +14,12 @@ * injection convention — no subprocess spawn needed for these rows. */ -const { test, describe, mock } = require('node:test'); +const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { createTempDir, cleanup } = require('./helpers.cjs'); +const { createTempDir, cleanup, captureFdSync } = require('./helpers.cjs'); const { routeResolveContent } = require('../gsd-core/bin/lib/task-command-router.cjs'); const { ExitError } = require('../gsd-core/bin/lib/cli-exit.cjs'); @@ -42,21 +42,7 @@ function writePlan(dir, taskXml) { * body still work). */ function captureStdout(fn) { - const chunks = []; - const origWriteSync = fs.writeSync.bind(fs); - const writeMock = mock.method(fs, 'writeSync', (fd, buffer, ...rest) => { - if (fd === 1) { - chunks.push(Buffer.isBuffer(buffer) ? buffer.toString('utf8') : String(buffer)); - return Buffer.isBuffer(buffer) ? buffer.length : Buffer.byteLength(String(buffer)); - } - return origWriteSync(fd, buffer, ...rest); - }); - try { - fn(); - } finally { - writeMock.mock.restore(); - } - return chunks.join(''); + return captureFdSync(1, fn); } /** @@ -72,21 +58,7 @@ function captureStdout(fn) { * `runCalibrateExpectError`. */ function captureStderr(fn) { - const chunks = []; - const origWriteSync = fs.writeSync.bind(fs); - const writeMock = mock.method(fs, 'writeSync', (fd, buffer, ...rest) => { - if (fd === 2) { - chunks.push(Buffer.isBuffer(buffer) ? buffer.toString('utf8') : String(buffer)); - return Buffer.isBuffer(buffer) ? buffer.length : Buffer.byteLength(String(buffer)); - } - return origWriteSync(fd, buffer, ...rest); - }); - try { - fn(); - } finally { - writeMock.mock.restore(); - } - return chunks.join(''); + return captureFdSync(2, fn); } const RESOLVABLE_TASK = 'xdo the thing'; diff --git a/tests/worktree-base-ref.test.cjs b/tests/worktree-base-ref.test.cjs index 1533a54d3..75b97cd51 100644 --- a/tests/worktree-base-ref.test.cjs +++ b/tests/worktree-base-ref.test.cjs @@ -681,11 +681,13 @@ describe('cmdWorktreeBaseCheck', () => { const chunks = []; const original = fs.writeSync; fs.writeSync = (fd, buf, offset, length) => { + const n = original.call(fs, fd, buf, offset, length); fds.push(fd); + // Chunk is derived from the REAL return count `n`, not the requested + // extent — a genuine short write must be reflected accurately (#4306). const start = offset ?? 0; - const end = length ?? (typeof buf === 'string' ? buf.length : buf.length); - chunks.push(typeof buf === 'string' ? buf.slice(start, end) : buf.toString('utf8', start, end)); - return end - start; + chunks.push(typeof buf === 'string' ? buf.slice(start, start + n) : buf.toString('utf8', start, start + n)); + return n; }; t.after(() => { fs.writeSync = original; }); const result = cmdWorktreeBaseCheck('/repo', [], {