fix(opencode): prevent truncated exports during provider switching

[Juliusolsson05]

Problem

OpenCode 1.18.30 writes JSON to stdout and then explicitly exits without waiting for asynchronous pipe writes to drain. Agent Code could receive a successful exit with only a truncated prefix, so session:switch-provider failed while parsing JSON, before any target-specific conversion.

A read-only probe of an affected native session returned 65,536 bytes of invalid JSON over a pipe. The same session returned 14,566,990 bytes of valid JSON with 804 messages when stdout was a regular file. The configured 256 MiB execFile buffer was not the cause. This PR contains no private transcript text and no session identifiers.

Fix

• Finite OpenCode CLI commands get a private regular-file stdout descriptor. No shell is invoked.
• Process/stdio close is awaited before the capture is read or released, including after post-spawn errors.
• Output overflow is detected while the command runs, and the final 256 MiB limit is checked before JSON is materialized. Polling is a soft disk bound, not a hard quota.
• Only a bounded stderr prefix is kept, with an explicit truncation marker.
• Malformed exports still fail. There are no JSON repair or retry heuristics, and parse errors contain no transcript excerpts.
• The fix sits at the shared CLI boundary, so large resolved-config output used for inbound switching is protected too.
• Documented rule: output that always fits one pipe buffer (db path, --version) may stay on a pipe. Anything that can exceed ~64 KiB must use this capture.

Merged with main (e9ac8bd)

Main's e9ac8bd (Refs #864) had bounded every CLI call with execFile's timeout plus an owned abort listener, both SIGKILL, with a 30 s default. That keeps a hung empty-session import from holding OpenCode Terminal startup or surviving stop(). This branch replaced execFile with spawn, so the merge (36bf958) rebuilt both bounds by hand and kept main's invariants:

• settle only from close;
• cancellation wins over timeout or overflow;
• an already-stopped caller creates nothing.

User-visible change: a timeout now reads OpenCode <command> failed: timed out after N ms instead of execFile's Command failed: .... No caller parses that text; OpencodeTerminalSession branches on signal.aborted.

The review found that the rebuild had lost one property of execFile's kill: destroying stdio before signalling. That is fixed below.

Process ownership and capture lifetime (review rounds)

• Spawn failure without stdio. On EMFILE/ENFILE, Node's spawn() returns with no pid and no stderr stream, then emits error on the next tick. error and close listeners are now attached right after spawn, and stderr is optional. Before this, the child.stderr! dereference threw first, the timers leaked, and the unlistened error became an uncaught exception, which the crash hooks turn into an app exit.

• Process group. On POSIX, commands now start with detached: true (without unref). The deadline, stop() and the size guard share one terminate():

  1. destroy child.stderr, so close cannot be held open by a descendant that inherited fd 2;
  2. SIGKILL the whole group (process.kill(-pid)), falling back to child.kill('SIGKILL'). This happens at most once, and only while the direct child is unreaped. After Node has reaped the leader, only step 1 runs, because the group id may already belong to an unrelated group.

  Why: OpenCode's npm launcher (packages/opencode/bin/opencode, v1.18.30) spawns the native binary with stdio: "inherit" and forwards only SIGINT/SIGTERM/SIGHUP. Before this change, SIGKILL reached only the launcher, the native process kept both descriptors, and none of the three bounds ever settled.

  Resolving the native binary instead was rejected. It would couple Agent Code to OpenCode's install layout (env override, cached copy, or platform/arch/baseline/musl packages) and would still miss helpers the native CLI starts itself.

  Consequence: these children have no controlling terminal, so a dev-terminal Ctrl+C no longer reaches them directly, and the CLI runs to its own end. The deadline is not a property of the child: it is a timer in Agent Code's main process and ends with it. A Ctrl+C that kills Electron therefore leaves a wedged CLI unbounded, where the old non-detached child sat in the terminal's foreground group and received the SIGINT (which the npm launcher forwards). Packaged quits already orphaned these children before this change.

• Unlinked capture. The capture is opened wx+ with mode 0600, then unlinked and its private directory removed before spawn. The child inherits the descriptor, not a path.

  • The parent keeps the fstat size guard on its handle.
  • After close, the parent reads the capture with positional reads. FileHandle.readFile would return nothing, because the child's writes moved the shared offset to end-of-file (verified).
  • An app quit, a crash or an orphaned child can no longer leave a plaintext transcript copy, because the inode is freed when the last descriptor closes.
  • Out of scope: the payload that opencode import <path> reads by path still needs a name while the child runs. importOpencodeSession's finally still removes it.

Transform timeouts

loadOpencodeSnapshot (export for switch, duplicate, rewind and prompt listing) and the projected-session import in transcriptEngine.ts now pass timeoutMs: 5 * 60_000. The terminal startup empty-session import and the debug config / models probes keep the 30 s default.

Transform calls carry no AbortSignal, so this deadline is their only bound. When it fires, it SIGKILLs the process group (while the leader is unreaped).

The duration of a large export remains unmeasured. The 14.6 MB / 804-message live probe ran before any deadline existed. It was not re-run against real sessions in any review round.

Verification

Node 24.14.1, final head ab5606aa. CI on this head: quality-gate passed (11m29s) and minimum-node-fixture-gate passed, run 34728966548.

• Typecheck on the round-3 code: raw tsc -p tsconfig.node.json and tsc -p tsconfig.web.json both clean.
• opencodeCliSessions.system.test.ts: 15 tests (13 on 36bf958).
  • New cases:
    • a spawn-failure double with no stderr that errors on the next tick;
    • a stop that lands during capture setup (late-abort replay).
  • Tightened cases:
    • the export child observes a link count of 0 on its stdout;
    • no agent-code-opencode-output-* entry exists while an import child is running;
    • the missing binary must report ENOENT.
  • Main's hung-import stop/timeout cases now sit behind the readiness gate. The child reports before its deadline or stop() can fire, is owned from spawn, and must be dead by SIGKILL within 2 s of the trigger.
• opencodeCliSessions.processTree.system.test.ts: 7 tests on real two-process launcher trees. All sit behind the shared readiness gate (testing/spawnReadiness.ts) and use shared polling helpers (testing/processWait.ts). The cases:
  • timeout, stop and output overflow, each settling within its bound with the descendant dead and no capture left;
  • an escaped descendant holding stderr, where the timeout still settles;
  • an escaped descendant whose readiness never arrived, reclaimed through the launcher's spawn-time pid record;
  • a leader that exits 0 while an escaped helper holds stderr, where no negative-pid signal is ever requested (a regression is recorded, never delivered);
  • timeout followed by stop, with exactly one group signal and cancellation still winning.
• Focused runs on the round-3 code: CLI + process-tree pair run: 2 files, 22/22 (15 + 7) at load average ~40–75, with no fixture process left afterwards.
• Round-2 head 3096ed2:
  • CI: quality-gate and minimum-node-fixture-gate passed.
  • Pair runs of both CLI files: 21/21, and 20/21 in a run where model discovery hit vitest's 5 s timeout at load average ~44 while the whole file took 16.1 s instead of ~4 s. The two verbose reruns right after passed 15/15, with model discovery at 73 ms and 104 ms.
  • Session control system test on its own: 10/10.
  • transcriptEngine and opencodeTerminalSession unit tests: 20/20.
  • vitest related: 396/397. The one failure was the same load-bound 5 s timeout, in a control case that never reaches the changed code.
  • libuv 1.51.0 implements detached on macOS by adding POSIX_SPAWN_SETSID to the same posix_spawn call, so the process group does not change how children are spawned. An interleaved benchmark showed no detached penalty.
• Slow-start probe for the readiness gate: with the fake CLI delayed 1.5 s before reporting, the pre-gate hung-import test failed both the stop and timeout cases with timed out waiting for import child status. The gated version passed both.
• Mutation checks. Each fix was reverted and the matching test confirmed to fail:
  • listeners after stderr wiring → uncaught EMFILE;
  • direct-child kill only → every tree case pending past its bound;
  • no stderr destroy → both escaped-helper cases pending;
  • no group kill → descendants survive;
  • named capture → link count 1, plus a named capture directory present during both import cases;
  • no late-abort replay → late stop pending;
  • group signal after the leader was reaped → a recorded -pid SIGKILL;
  • no once-only guard → two group signals;
  • cleanup ignoring the spawn-time descendant record → the forced-readiness-failure case fails with timed out waiting for escaped descendant reclaimed.
• Before the merge: 70 switching/OpenCode tests passed, and the affected native session passed an opted-in, read-only export through the production API. It also passed real Claude, Codex and OpenCode native projection without publishing targets or modifying source sessions. That live run was not repeated after the merge.

Review round

Round 1

Codex (request changes) and Claude (approve with comments) reviewed 36bf958.

Finding	Disposition
Codex 1 (major): EMFILE/ENFILE spawn leaves stderr undefined; uncaught child error exits the app	Fixed in b18032b
Codex 2 (major): killing the npm launcher leaves the native writer alive; timeout, stop and overflow never settle	Fixed in 7ad30b7 (process group + terminate())
Codex 3 (major): app quit can leave a plaintext capture	Fixed in c1be521 (unlinked before spawn). The suggested app-wide shutdown drain for finite CLI operations is declined: with the unlink nothing persists across a quit, an orphaned child on quit predates this PR (execFile had the same exposure), and an app-owned operation drain belongs to the quit lifecycle and operation ownership work in #919 / #918
Codex 4 (nit): comments falsely claim spawn lacks timeout/killSignal	Fixed in 7ad30b7 (real reasons for owned handlers), tidied in de80797
Codex note: the 30 s default now applies to large transform exports/imports	Fixed in 8f6c0ef (5 min for transforms). Duration remains unmeasured
Claude F1 (minor): hand-built timeout/overflow kill no longer destroys stderr, so a descendant holding fd 2 blocks settlement	Fixed in 7ad30b7
Claude F2 (minor): false WHY premise about spawn	Fixed in 7ad30b7
Claude F3 (minor): PR body predates the merge; 30 s default vs large export; re-run the live export; quality-gate pending	Body updated; transform deadline fixed in 8f6c0ef. Re-running the live export is declined because live runs against real user sessions were not authorized, so duration is stated as unmeasured
Claude F4 (nit): missing-binary test asserts only failed	Fixed in c3aa47a (ENOENT)
Claude F5 (nit): late-abort replay untested	Fixed in c3aa47a
Claude F6 (nit): plan not updated for the merge	Fixed in 8c94da7
Claude F7 (nit): why pipe-captured db path / --version are safe is undocumented	Fixed in c1be521

Round 2

Both reviewers re-checked 8c94da7 and confirmed every round-1 disposition. Codex requested changes; Claude approved with comments.

Finding	Disposition
Codex R2 #1 (major), same as Claude R2-1 (minor): terminate() could SIGKILL a released, reusable process-group id after Node reaped the leader but before close. A timeout or overflow followed by stop() also signalled twice	Fixed in 623a52b. The group is signalled at most once, and only while exitCode and signalCode are both null; after that only stderr is destroyed. Regressions: a launcher that exits 0 while an escaped helper holds stderr (no negative-pid signal requested), and timeout followed by stop (exactly one group signal, cancellation still wins). The trade-off (a same-group member that outlives an exited leader is no longer killed) is documented
Codex R2 #2 (minor): real timeout tests could kill the fixture before it was ready; the launcher pid was inferred from ppid; no per-test timeout	Fixed in 5d15002. The tree tests moved to opencodeCliSessions.processTree.system.test.ts, behind a spawn seam that returns only after the fixture reports, so readiness precedes every bound. The launcher passes its own pid, the test owns the launcher's ChildProcess from spawn, and each case has a 30 s timeout
Claude R2-2 (nit): a failed readiness wait left the command and its tree running past the test	Fixed in 5d15002 (tree tests) and a89e5b4 (main's stop/timeout tests). Cleanup aborts the command before awaiting its outcome
Claude R2-3 (nit): "runs to its deadline" is wrong once the parent dies	Fixed in 6359107 and in this body. Plan updated in a380c2a
Found during round-2 verification: inherited readiness race in main's stop/timeout tests (e9ac8bd waited 2 s for the fake CLI while the 1 s deadline was already running; the stop case failed before stop() ran)	Fixed in a89e5b4 (plan in 3096ed2). Both cases use the shared readiness gate (testing/spawnReadiness.ts), own the child from spawn, check SIGKILL on the ChildProcess handle within 2 s of the trigger, and abort in cleanup. No budget was simply widened

Round 3

Both reviewers approved 3096ed2 with comments and verified the production fixes with real processes. The remaining items were test-only or record keeping.

Finding	Disposition
Codex R3 #1 (minor): cleanup cannot reclaim an escaped fixture descendant when readiness fails, because it found descendants only through the ready status	Fixed in d0562d9. The launcher fixture records the descendant's pid as soon as spawn() returns. release() kills that owned identity after the command settles, whether or not readiness succeeded. A new forced-readiness-failure case for the escaped fixture proves no survivor
Claude R3-1 (minor): PR body not updated for rounds 2 and 3	Fixed in this body revision: the Consequence wording matches the code comment; round 2 and round 3 tables were added; Verification moved to the final head with the 15 + 7 split; the leader-exit trade-off is in Known limitations; CI is filled for the final head. Plan recorded in ab5606a
Claude R3-2 (nit): a failed test can leave its spawn gate queued for the next test	Fixed in c319e90. Both system files call vi.mocked(spawn).mockReset() in afterEach
Claude R3-3 (nit): the reaped-leader spy would deliver a real group SIGKILL under the mutation it guards	Fixed in fef39a4. The spy swallows negative-pid requests and passes positive pids through, so a regression is recorded, never delivered
Claude R3-4 (nit): alive / within / waitUntil duplicated across the split files	Fixed in c319e90. They moved to testing/processWait.ts; the hoisted vi.mock factories stay per file

Known limitations

• A same-group process that outlives an exited leader is not killed. It can only append to the unlinked capture, and the read is bounded by the fstat size.
• In development, a Ctrl+C that kills Electron leaves a wedged CLI child unbounded, because the deadline is a main-process timer and ends with it.
• A descendant that leaves the process group can still survive termination. The call itself settles, because stderr is destroyed, and no named file remains. If such a descendant keeps only stdout, it can append after close; the read is bounded by the fstat size that enforced the limit.
• The import payload file is path-based and is still removed only by the async finally.
• The native import is not transactional. A timeout or kill mid-import is reported as a failure but can leave provider state partially imported (pre-existing).
• On Windows, only the direct child is killed. Agent Code packages for macOS.
• The duration of a large export under the 5 minute transform deadline is unmeasured.

Scope

Independent of Grok feature work. No app launch, real session import/switch/retirement, or PR merge was performed, and no database-access workaround was used. Existing dependency advisories are outside this fix. The running app keeps the old behavior until this branch is integrated and rebuilt.

Fixes #845

🤖 Generated with Claude Code

https://claude.ai/code/session_01S4AoJJUBex14FAEZZfHPsV

[Juliusolsson05]

The final live-probe commit also runs the affected large native export through decodeOpencodeConversation and the real Claude, Codex and OpenCode native-resume projectors. All retained nonempty conversation content. This is read-only conversion verification, not a claim that target CLIs were started or source/target sessions switched. Explicit new-test typechecking uses the app source alias map; a bare standalone tsc invocation otherwise resolves stale compiled parser declarations from the parent installation.

[Juliusolsson05]

CI attempt1: all11 new OpenCode CLI system tests passed, but an unchanged WebSocketSessionFeed integration case failed at line294 (expected fixture u-live, observed empty entries). Its fake manager uses Claude and does not execute the modified CLI helper. All13 tests in that file pass locally. Rerunning the failed job on the identical commit to check intermittency; no assertions/timeouts or unrelated production code have been changed.

[Juliusolsson05]

Final verification: quality-gate and minimum-node-fixture-gate are green on the identical commit after rerunning the failed job. Core/system/renderer, coverage, typechecks and distributable verification passed. The independent intermittent WebSocket bootstrap test failure is recorded in #847; no assertion/timeout changes were made here. Source export/projection verification and local81 deterministic tests remain green. Ready for review; no merge or running-app change performed.