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

[scs0209]

Summary

The parent CLI treats a missing broker protocol file as terminal command broker protocol response was missing, but the broker's swallow-and-reap catch hid the root cause (e.g. ENOSPC, EACCES) — every failure looked identical on Windows.

Changes

• Broker (child): on protocol-write failure, emit a bounded stdout-independent stderr sentinel ([freebuff-broker] protocol write failed: <err>) before the normal cleanup reap, instead of vanishing silently.
• Parent: capture a 4KB stderr tail from the broker, and retry the protocol read for up to 200ms (PROTOCOL_READ_ATTEMPTS=5 × PROTOCOL_READ_RETRY_MS=50) to tolerate filesystems/AV where close can edge ahead of the one-shot file write.
• Failure classification: new protocol_write_failed code; the classifier checks the stderr marker before the generic protocol_missing branch. The missing-protocol error message now includes Broker stderr: <tail> when available.
• writeProtocol falls back from the validated env path to the raw env path when validation rejects a broken environment, so a valid response is never lost to over-strictness.

Tests

• normalizes failures now asserts protocol_write_failed classification via the stderr marker.
• New: explains a missing protocol response with the broker stderr reason (helper writes marker to stderr + exits 1 → parent reports protocol_write_failed with the stderr reason).
• New: tolerates a protocol file written just after the helper exits (delayed write race → read retry succeeds).
• 18/18 broker tests pass locally; windows-terminal-health tests still pass.

Fixes #1359

[codebuff-team]

Good bug hunt — collapsing every broker write failure into the same generic protocol_missing message was genuinely unhelpful on Windows, and the stderr-marker classification plus the ENOENT read-retry are reasonable, well-tested fixes (both new tests exercise real races/paths rather than mocking around the problem).

Two things need attention before this is portable as-is:

1. Cleanup regression in the child. The original catch branch called await reapOwnProcessGroup() explicitly, with a comment stating this was needed to "keep the shell tree contained even when the temp write fails." Your rewrite drops that call entirely and falls through to whatever "normal cleanup" does later in the file. If that later cleanup path waits on something (parent disconnect, etc.) before reaping, a write failure now leaves the process group lingering longer than before — exactly the scenario the comment you deleted was guarding against. Please confirm (and ideally add a test) that the process group is still reaped promptly on write failure, or restore the explicit early reap.

2. protocolWriteTargets() fallback undermines validation. When protocolPathFromEnv() rejects the path, you fall back to the raw, unvalidated env value and attempt a write there anyway. The comment says this is to avoid losing a valid response to "over-strictness," but that's exactly what path validation is for — if it's rejecting valid values, the fix belongs in the validator, not in a silent bypass in the write path. This has no test coverage and no discussion of what validation was actually rejecting.

Right instinct, but these two points change the risk profile of the change enough that a maintainer will want them resolved or explained before porting. Please also open with what specific input caused the validator to reject a legitimate path — that's the actual root cause worth fixing.

[scs0209]

Thanks for the review — both points are addressed in the latest push.

1. Group reaping on the write-failure path

You were right that the explicit reap disappeared. It was not a leak — the catch fell through to the unconditional await reapOwnProcessGroup() at the end of serveTerminalCommandBroker(), with no await between them — but the containment guarantee was implicit, which is exactly what made the change hard to review. It is now explicit:

• catch ends with return reapOwnProcessGroup(), so a failed write can never outlive a successful one.
• Before reaping, the broker flushes the stderr marker through a bounded (200 ms) wait on the write callback. On POSIX process.stderr is an async pipe, so the previous fire-and-forget write could lose the failure reason to the immediate SIGKILL. The wait is bounded so containment never stalls behind a dead pipe.
• Added reaps the shell tree promptly when the protocol write fails: the protocol path is pre-created so the wx write must fail, then the test asserts the detached sleep descendant dies and the broker's process group is gone. I verified the test fails (timeout, dangling descendant) when the reap is removed.

2. The raw-env fallback

Agreed, and it is gone. protocolWriteTargets() was removed entirely; writeProtocol() now writes only to the validated protocolPathFromEnv(). You asked which concrete input the validator was rejecting — I could not reproduce one: the broker is spawned with the inherited process.env, so both sides derive the same os.tmpdir() and the parent-generated path validates. That makes the fallback a speculative safety net rather than a fix for an observed failure, which is not worth the lost validation.

What I did instead was harden the comparison where a legitimate rejection was actually plausible:

• normalizeProtocolDirectory() resolves symlinks (realpathSync.native) and case-folds only on Windows. macOS /var → /private/var is the realistic way the same directory arrives spelled two ways.
• New tests cover a symlinked temp directory and the Windows case-insensitive spelling.
• Because rejection is now reported on stderr with the marker, a rejected parent path surfaces as a diagnosable error instead of a silently lost response.

bun test src/utils/__tests__/terminal-command-broker.test.ts: 21 pass, 0 fail.