fix(#224): return byte counts for --pick stdout capture (#226)

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');
 }
 
 /**

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'
+    );
   });
 });

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'
+    );
+  });
+});