diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index 57e6d31ea..11aeb4c36 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -514,38 +514,19 @@ async function main() { cwd = findProjectRoot(cwd); } - // When --pick is active, intercept stdout to extract the requested field. + // When --pick is active, capture stdout and extract the requested field. if (pickField) { - const origWriteSync = fs.writeSync; - let captured = ''; - fs.writeSync = function (fd, data, ...rest) { - if (fd === 1) { - captured += String(data); - return; - } - return origWriteSync.call(fs, fd, data, ...rest); - }; - const cleanup = () => { - fs.writeSync = origWriteSync; - let jsonStr = captured; - if (jsonStr.startsWith('@file:')) { - jsonStr = fs.readFileSync(jsonStr.slice(6), 'utf-8'); - } - try { - const obj = JSON.parse(jsonStr); - const value = extractField(obj, pickField); - const result = value === null || value === undefined ? '' : String(value); - origWriteSync.call(fs, 1, result); - } catch { - origWriteSync.call(fs, 1, captured); - } - }; - try { + const captured = await captureStdoutSyncWrites(async () => { await runCommand(command, args, cwd, raw, defaultValue, originalCommand); - cleanup(); - } catch (e) { - fs.writeSync = origWriteSync; - throw e; + }); + const resolved = resolveAtFileOutput(captured); + try { + const obj = JSON.parse(resolved); + const value = extractField(obj, pickField); + const result = value === null || value === undefined ? '' : String(value); + fs.writeSync(1, result); + } catch { + fs.writeSync(1, captured); } return; } @@ -555,24 +536,49 @@ async function main() { // already resolves this, but the normal path wrote @file: to stdout, forcing // every workflow to have a bash-specific `if [[ "$INIT" == @file:* ]]` check // that breaks on PowerShell and other non-bash shells. - const origWriteSync2 = fs.writeSync; - let captured = ''; - fs.writeSync = function (fd, data, ...rest) { - if (fd === 1) { - captured += String(data); - return; - } - return origWriteSync2.call(fs, fd, data, ...rest); - }; - try { + const captured = await captureStdoutSyncWrites(async () => { await runCommand(command, args, cwd, raw, defaultValue, originalCommand); - } finally { - fs.writeSync = origWriteSync2; - } - if (captured.startsWith('@file:')) { - captured = fs.readFileSync(captured.slice(6), 'utf-8'); - } - origWriteSync2.call(fs, 1, captured); + }); + fs.writeSync(1, resolveAtFileOutput(captured)); +} + +function captureStdoutSyncWrites(run) { + const originalWriteSync = fs.writeSync; + let captured = ''; + + fs.writeSync = function patchedWriteSync(fd, data, ...rest) { + if (fd === 1) { + if (Buffer.isBuffer(data)) { + captured += data.toString('utf-8'); + return data.length; + } + const text = String(data); + captured += text; + let encoding = 'utf-8'; + if (typeof rest[1] === 'string') encoding = rest[1]; + return Buffer.byteLength(text, encoding); + } + return originalWriteSync.call(fs, fd, data, ...rest); + }; + + const restore = () => { + fs.writeSync = originalWriteSync; + }; + + return Promise.resolve() + .then(() => run()) + .then(() => { + restore(); + return captured; + }, (err) => { + restore(); + throw err; + }); +} + +function resolveAtFileOutput(captured) { + if (!captured.startsWith('@file:')) return captured; + return fs.readFileSync(captured.slice(6), 'utf-8'); } /** diff --git a/tests/bug-1891-file-resolution.test.cjs b/tests/bug-1891-file-resolution.test.cjs index f85699697..b9789be88 100644 --- a/tests/bug-1891-file-resolution.test.cjs +++ b/tests/bug-1891-file-resolution.test.cjs @@ -48,17 +48,14 @@ describe('bug #1891: @file: resolution in gsd-tools.cjs', () => { }); test('stdout interception wraps runCommand in the non-pick path', () => { - // The main function should intercept fs.writeSync for fd=1 - // in BOTH the pick path AND the normal path + // The main function should resolve @file: output in BOTH --pick and + // non-pick paths. This can be either two inline checks or a shared helper. const mainFunc = src.slice(src.indexOf('async function main()')); - const pickInterception = mainFunc.indexOf('// When --pick is active'); - const fileResolution = mainFunc.indexOf('@file:'); - - // There should be at least two @file: resolution points: - // one in the --pick path and one in the normal path - const firstAt = mainFunc.indexOf("'@file:'"); - const secondAt = mainFunc.indexOf("'@file:'", firstAt + 1); - assert.ok(secondAt > firstAt, - 'Both --pick and normal paths should resolve @file: references'); + const resolveCalls = (mainFunc.match(/resolveAtFileOutput\(/g) || []).length; + const inlineAtFileChecks = (mainFunc.match(/@file:/g) || []).length; + assert.ok( + resolveCalls >= 2 || inlineAtFileChecks >= 2, + 'Both --pick and normal paths should resolve @file: references' + ); }); }); diff --git a/tests/bug-224-pick-stdout-capture.test.cjs b/tests/bug-224-pick-stdout-capture.test.cjs new file mode 100644 index 000000000..bc164e6ab --- /dev/null +++ b/tests/bug-224-pick-stdout-capture.test.cjs @@ -0,0 +1,40 @@ +// allow-test-rule: structural-implementation-guard +// Bug #224 is a platform-specific (Node 24 + Windows) flake in `--pick` where +// stdout interception can produce non-deterministic failures. We lock the seam +// contract structurally until we have a deterministic Windows reproduction +// harness in CI. + +'use strict'; + +const { describe, test, before } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools } = require('./helpers.cjs'); + +const GSD_TOOLS_SRC = path.join(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); + +describe('bug #224: --pick stdout capture contract', () => { + let src; + + before(() => { + src = fs.readFileSync(GSD_TOOLS_SRC, 'utf-8'); + }); + + test('--pick output still succeeds for current-timestamp command', () => { + const result = runGsdTools(['current-timestamp', '--pick', 'timestamp']); + assert.strictEqual(result.success, true, result.error || 'expected command to succeed'); + assert.match(result.output, /^\d{4}-\d{2}-\d{2}T/, 'expected ISO timestamp output'); + }); + + test('stdout interception for fd=1 returns a byte count (never undefined)', () => { + const mainStart = src.indexOf('async function main()'); + assert.ok(mainStart !== -1, 'main() must exist'); + const mainSrc = src.slice(mainStart); + + assert.ok( + mainSrc.includes('Buffer.byteLength(') || mainSrc.includes('return data.length'), + 'stdout interception must return written-byte counts for fd=1 captures' + ); + }); +});