fix(cli): surface terminal command broker protocol write failures through stderr

The parent treats a missing protocol file as 'protocol response was missing',
but the broker's swallow-and-reap catch hid the underlying write error
(ENOSPC, EACCES, ...). The broker now emits a bounded stderr marker before
reaping itself, and the parent captures a 4KB stderr tail and retries the
protocol read for 200ms (5x50ms) to tolerate tmpfs/AV races where close edges
ahead of the file write. Classified as protocol_write_failed.

Fixes #1359

Author: scs0209

cli/src/utils/__tests__/terminal-command-broker.test.ts

@@ -105,6 +105,13 @@ describe('terminal command broker', () => {
         new Error('terminal command broker protocol response was missing'),
       ),
     ).toBe('protocol_missing')
+    expect(
+      classifyTerminalBrokerFailure(
+        new Error(
+          'terminal command broker protocol response was missing\nBroker stderr: [freebuff-broker] protocol write failed: ENOSPC',
+        ),
+      ),
+    ).toBe('protocol_write_failed')
     expect(sanitizeWindowsCliVersion('0.0.142')).toBe('0.0.142')
     expect(sanitizeWindowsCliVersion('private path/and details')).toBe(
       'unknown',
@@ -353,6 +360,65 @@ describe('terminal command broker', () => {
     ])
   })
 
+  test('explains a missing protocol response with the broker stderr reason', async () => {
+    if (process.platform === 'win32') return
+    const failures: Array<{ stage: string; failureCode: string }> = []
+    const broker = createTerminalCommandBroker({
+      invocation: () => ({
+        executable: '/bin/sh',
+        args: [
+          '-c',
+          'echo "[freebuff-broker] protocol write failed: ENOSPC" >&2; exit 1',
+        ],
+      }),
+      reportFailure: (failure) => failures.push(failure),
+    })
+
+    let failureMessage = ''
+    try {
+      await runTerminalCommand({
+        command: `printf 'must not run'`,
+        process_type: 'SYNC',
+        cwd: process.cwd(),
+        timeout_seconds: 10,
+        terminalCommandBroker: broker,
+      })
+    } catch (error) {
+      failureMessage = error instanceof Error ? error.message : String(error)
+    }
+
+    expect(failureMessage).toContain('protocol response was missing')
+    expect(failureMessage).toContain(
+      '[freebuff-broker] protocol write failed: ENOSPC',
+    )
+    expect(failures).toEqual([
+      { stage: 'completion', failureCode: 'protocol_write_failed' },
+    ])
+  })
+
+  test('tolerates a protocol file written just after the helper exits', async () => {
+    if (process.platform === 'win32') return
+    const broker = createTerminalCommandBroker({
+      invocation: () => ({
+        executable: '/bin/sh',
+        args: [
+          '-c',
+          `(sleep 0.04; printf '%s\\n' '{"ok":true,"exitCode":0}' > "$CODEBUFF_TERMINAL_COMMAND_BROKER_PROTOCOL") & exit 0`,
+        ],
+      }),
+    })
+
+    const result = await runTerminalCommand({
+      command: `printf 'must not run'`,
+      process_type: 'SYNC',
+      cwd: process.cwd(),
+      timeout_seconds: 10,
+      terminalCommandBroker: broker,
+    })
+
+    expect(result[0].value).toMatchObject({ exitCode: 0 })
+  })
+
   test('does not add broker recovery guidance to a command spawn failure', async () => {
     const missingCwd = path.join(
       tmpdir(),

cli/src/utils/terminal-command-broker.ts

@@ -23,6 +23,12 @@ const MAX_REQUEST_BYTES = 4 * 1024 * 1024
 const MAX_PROTOCOL_BYTES = 64 * 1024
 const PROTOCOL_FILE_PREFIX = 'freebuff-terminal-command-broker-'
 const TERMINAL_COMMAND_BROKER_RECOVERY = 'Restart Freebuff and try again.'
+const PROTOCOL_READ_RETRY_MS = 50
+const PROTOCOL_READ_ATTEMPTS = 5
+const MAX_BROKER_STDERR_TAIL_BYTES = 4 * 1024
+// Cross-process contract: the detached broker emits this marker on stderr
+// before reaping itself so the parent can surface the write failure reason.
+const BROKER_STDERR_MARKER = '[freebuff-broker] protocol write failed:'
 
 export type TerminalBrokerFailureStage = 'spawn' | 'stdio' | 'completion'
 export type TerminalBrokerFailureCode =
@@ -33,6 +39,7 @@ export type TerminalBrokerFailureCode =
   | 'epipe'
   | 'invalid_response'
   | 'protocol_missing'
+  | 'protocol_write_failed'
   | 'response_too_large'
   | 'unknown'
 
@@ -64,6 +71,7 @@ export function classifyTerminalBrokerFailure(
   const message = errorMessage(error).toLowerCase()
   if (message.includes('failed to connect')) return 'failed_to_connect'
   if (message.includes('invalid response')) return 'invalid_response'
+  if (message.includes(BROKER_STDERR_MARKER)) return 'protocol_write_failed'
   if (message.includes('protocol response was missing')) {
     return 'protocol_missing'
   }
@@ -154,18 +162,41 @@ function removeProtocolFile(protocolPath: string): void {
   }
 }
 
+function protocolWriteTargets(): string[] {
+  try {
+    return [protocolPathFromEnv()]
+  } catch {
+    // Path validation can only reject a broken environment; still attempt the
+    // parent-provided path so a valid response is not lost to over-strictness.
+    const raw = getSystemProcessEnv()[TERMINAL_COMMAND_BROKER_PROTOCOL_ENV]
+    return raw ? [path.resolve(raw)] : []
+  }
+}
+
 function writeProtocol(message: BrokerProtocol): void {
   const payload = `${JSON.stringify(message)}\n`
   if (Buffer.byteLength(payload) > MAX_PROTOCOL_BYTES) {
     throw new Error('terminal command broker response was too large')
   }
-  // A constrained one-shot file avoids Bun's unreliable custom stdio pipes on
-  // Windows. `wx` ensures even an accidentally reused path is never replaced.
-  writeFileSync(protocolPathFromEnv(), payload, {
-    encoding: 'utf8',
-    flag: 'wx',
-    mode: 0o600,
-  })
+  let lastError: unknown = new Error(
+    'terminal command broker protocol path was invalid',
+  )
+  for (const target of protocolWriteTargets()) {
+    try {
+      // A constrained one-shot file avoids Bun's unreliable custom stdio pipes on
+      // Windows. `wx` ensures even an accidentally reused path is never replaced.
+      writeFileSync(target, payload, {
+        encoding: 'utf8',
+        flag: 'wx',
+        mode: 0o600,
+      })
+      return
+    } catch (error) {
+      lastError = error
+    }
+  }
+  if (lastError instanceof Error) throw lastError
+  throw new Error(errorMessage(lastError))
 }
 
 function waitForParentDisconnect(): Promise<void> {
@@ -267,10 +298,14 @@ export async function serveTerminalCommandBroker(): Promise<void> {
 
   try {
     writeProtocol(outcome.message)
-  } catch {
-    // Without a protocol response, the parent reports an actionable broker
-    // failure. Keep the shell tree contained even when the temp write fails.
-    await reapOwnProcessGroup()
+  } catch (error) {
+    // Never vanish silently: relay the write failure on stderr so the parent
+    // can explain the missing protocol instead of a generic ENOENT message.
+    try {
+      process.stderr.write(`\n${BROKER_STDERR_MARKER} ${errorMessage(error)}\n`)
+    } catch {
+      // The pipe may already be gone; the parent still sees the missing file.
+    }
   }
 
   // Normal cleanup belongs to this detached process. In particular, Windows
@@ -417,28 +452,50 @@ export function createTerminalCommandBroker({
       child.stdin.on('error', () => {})
       child.stdin.end(JSON.stringify(request))
 
+      // The broker writes the one-shot file immediately before its own exit,
+      // but close can still edge ahead on some filesystems (tmpfs, AV). ENOENT
+      // is retried, and a bounded stderr tail explains an unrecoverable miss.
+      let brokerStderrTail = ''
+      child.stderr.on('data', (chunk: Buffer) => {
+        const next = brokerStderrTail + chunk.toString('utf8')
+        brokerStderrTail =
+          next.length > MAX_BROKER_STDERR_TAIL_BYTES
+            ? next.slice(next.length - MAX_BROKER_STDERR_TAIL_BYTES)
+            : next
+      })
+
       const closed = new Promise<void>((resolve, reject) => {
         child.once('error', reject)
         child.once('close', () => resolve())
       })
-      const completion = closed
-        .then(() => {
-          let payload: Buffer
+
+      const readProtocol = async (): Promise<BrokerProtocol> => {
+        for (let attempt = 0; attempt < PROTOCOL_READ_ATTEMPTS; attempt++) {
           try {
-            payload = readFileSync(protocolPath)
+            const payload = readFileSync(protocolPath)
+            if (payload.byteLength > MAX_PROTOCOL_BYTES) {
+              throw new Error('terminal command broker response was too large')
+            }
+            return parseProtocol(payload.toString('utf8').trim())
           } catch (error) {
-            if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
-              throw new Error(
-                'terminal command broker protocol response was missing',
+            if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error
+            if (attempt < PROTOCOL_READ_ATTEMPTS - 1) {
+              await new Promise((resolve) =>
+                setTimeout(resolve, PROTOCOL_READ_RETRY_MS),
               )
             }
-            throw error
-          }
-          if (payload.byteLength > MAX_PROTOCOL_BYTES) {
-            throw new Error('terminal command broker response was too large')
           }
-          return parseProtocol(payload.toString('utf8').trim())
-        })
+        }
+        const tail = brokerStderrTail.trim()
+        throw new Error(
+          tail
+            ? `terminal command broker protocol response was missing\nBroker stderr: ${tail}`
+            : 'terminal command broker protocol response was missing',
+        )
+      }
+
+      const completion = closed
+        .then(readProtocol)
         .catch((error) => {
           if (!terminationRequested) report('completion', error)
           throw brokerFailure(error)

cli/src/utils/windows-terminal-health.ts

@@ -18,6 +18,7 @@ export type WindowsTerminalFailure = {
     | 'epipe'
     | 'invalid_response'
     | 'protocol_missing'
+    | 'protocol_write_failed'
     | 'response_too_large'
     | 'exit_nonzero'
     | 'terminated'