From 1e00988c1ae05643d3e21047ca9b549c79e9d205 Mon Sep 17 00:00:00 2001
From: Theo Ephraim <theo@dmno.dev>
Date: Mon, 2 Nov 2026 12:36:57 -0700
Subject: [PATCH] Settle spawnAsync on close so stdout/stderr are fully read

exit can fire before the stdio pipes are drained, so a fast failing command
could reject with empty stderr (bitwarden then reported 'unknown' instead of
re-unlocking). Settle on close instead, falling back 500ms after exit for
children that leave a daemon holding the pipes.
---
 .bumpy/spawn-async-stdio-race.md         | 10 ++++++
 packages/utils/src/exec-helpers.ts       | 23 ++++++++++++-
 packages/utils/test/exec-helpers.test.ts | 42 ++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 .bumpy/spawn-async-stdio-race.md

diff --git a/.bumpy/spawn-async-stdio-race.md b/.bumpy/spawn-async-stdio-race.md
new file mode 100644
index 000000000..a646f4e71
--- /dev/null
+++ b/.bumpy/spawn-async-stdio-race.md
@@ -0,0 +1,10 @@
+---
+varlock: patch
+"@varlock/1password-plugin": patch
+"@varlock/bitwarden-plugin": patch
+"@varlock/keepass-plugin": patch
+"@varlock/pass-plugin": patch
+"@varlock/proton-pass-plugin": patch
+---
+
+Fix CLI calls occasionally losing stdout/stderr when the command exits quickly under load, which could surface as a vague "unknown" error instead of the real one
diff --git a/packages/utils/src/exec-helpers.ts b/packages/utils/src/exec-helpers.ts
index 419c139f0..6ca2f50cc 100644
--- a/packages/utils/src/exec-helpers.ts
+++ b/packages/utils/src/exec-helpers.ts
@@ -11,6 +11,9 @@ export class ExecError extends Error {
   }
 }
 
+/** how long to wait for stdio to drain after the child exits, see below */
+const CLOSE_GRACE_PERIOD_MS = 500;
+
 export function spawnAsyncHelper(
   command: string,
   args: Array<string>,
@@ -36,7 +39,12 @@ export function spawnAsyncHelper(
     childProcess.on('error', (err) => {
       reject(err);
     });
-    childProcess.on('exit', (exitCode, signal) => {
+    let settled = false;
+    let graceTimer: ReturnType<typeof setTimeout> | undefined;
+    const settle = (exitCode: number | null, signal: NodeJS.Signals | null) => {
+      if (settled) return;
+      settled = true;
+      clearTimeout(graceTimer);
       // A child killed by a signal exits with a null code, so only an explicit 0 counts as
       // success. Otherwise whatever partial stdout arrived before the kill would be returned.
       if (exitCode === 0) {
@@ -48,6 +56,19 @@ export function spawnAsyncHelper(
           stderrData || (signal ? `command terminated by ${signal}` : ''),
         ));
       }
+    };
+    // `exit` can fire before stdout/stderr are drained, so settling there could drop output
+    // (eg. an empty stderr on failure). `close` fires once the stdio streams have ended.
+    childProcess.on('close', (exitCode, signal) => settle(exitCode, signal));
+    // But if the child leaves a grandchild (eg. a background daemon) holding the stdio pipes,
+    // `close` never fires. After `exit`, give the streams a short grace period to drain, then
+    // settle with whatever output arrived and stop waiting on the pipes.
+    childProcess.on('exit', (exitCode, signal) => {
+      graceTimer = setTimeout(() => {
+        settle(exitCode, signal);
+        childProcess.stdout?.destroy();
+        childProcess.stderr?.destroy();
+      }, CLOSE_GRACE_PERIOD_MS);
     });
   });
 
diff --git a/packages/utils/test/exec-helpers.test.ts b/packages/utils/test/exec-helpers.test.ts
index ea5e4de72..9fa0ccd1e 100644
--- a/packages/utils/test/exec-helpers.test.ts
+++ b/packages/utils/test/exec-helpers.test.ts
@@ -37,4 +37,46 @@ describe('spawnAsync', () => {
     expect((result as ExecError).exitCode).not.toBe(0);
     expect((result as ExecError).data).toContain('SIGTERM');
   });
+
+  // `exit` can fire before the stdio pipes are drained, so settling on it used to drop
+  // output under load (eg. bitwarden saw an empty stderr and could not tell the vault was
+  // locked). To make that gap deterministic, the child exits immediately and a grandchild
+  // sharing its stdio writes the output a little later, before the pipes close.
+  const delayedWriteScript = (fd: 1 | 2, text: string, exitCode: number) => `
+    const { spawn } = require('node:child_process');
+    spawn(process.execPath, ['-e', ${JSON.stringify(`setTimeout(() => require('node:fs').writeSync(${fd}, ${JSON.stringify(text)}), 100)`)}], { stdio: 'inherit' }).unref();
+    process.exit(${exitCode});
+  `;
+
+  test('rejection carries stderr written after the child exited but before the pipes closed', async () => {
+    const result = await spawnAsync(
+      process.execPath,
+      ['-e', delayedWriteScript(2, 'Vault is locked.', 1)],
+    ).catch((err) => err);
+
+    expect(result).toBeInstanceOf(ExecError);
+    expect((result as ExecError).exitCode).toBe(1);
+    expect((result as ExecError).data).toBe('Vault is locked.');
+  });
+
+  test('resolves with stdout written after the child exited but before the pipes closed', async () => {
+    const output = await spawnAsync(process.execPath, ['-e', delayedWriteScript(1, 'LATE_OUTPUT', 0)]);
+    expect(output).toBe('LATE_OUTPUT');
+  });
+
+  test('does not hang when a grandchild keeps the stdio pipes open after the child exits', async () => {
+    // the child spawns a detached grandchild that inherits stdout/stderr and outlives it,
+    // like a CLI that starts a background daemon. `close` never fires in that case.
+    const startAt = Date.now();
+    const output = await spawnAsync(process.execPath, [
+      '-e', `
+      const { spawn } = require('node:child_process');
+      const gc = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 10000)'], { stdio: 'inherit', detached: true });
+      gc.unref();
+      process.stdout.write('DONE');
+    `,
+    ]);
+    expect(output).toBe('DONE');
+    expect(Date.now() - startAt).toBeLessThan(5000);
+  });
 });