refactor(coding-agent): await the spawn event instead of a startup sleep by snimu · Pull Request #1854 · PrimeIntellect-ai/prime-agent

snimu · 2026-08-27T12:14:22Z

Note: this PR also contains the full change from #1840 (removal of blanket RPC timeouts in favor of real child-lifecycle boundaries (spawn error, stdout close, exit/close finalize) with pending-request rejection) - that PR is closed and merges here as one unit.

What was wrong

RpcClient.start() guessed process readiness by sleeping 100ms and then sampling the exit code — a timer standing in for a real signal. Children that died at 150ms were "ready"; healthy children paid 100ms for nothing (audit: timeouts as synchronization, timeouts.md finding 6).

The fix

The sleep and exit-code sampling are deleted. start() awaits the child's OS spawn event (node:events.once) — the readiness signal the child actually emits. Pre-spawn errors surface through the recorded transport error with the existing message format; fast post-spawn exits surface on first use via the stdout-close handler with captured stderr — strictly more honest than the old sample. +14/−6.

How it's verified

Reviewer traced both failure orderings (error-before-spawn, exit-after-spawn), confirmed events.once cleans its listeners on both paths with no missed-event race, and validated the new fake-timer test is meaningful (old code hangs under fake timers, new code resolves). Focused RPC suites green; full CI-style failures match the stack base exactly (dist-build extras pass after building). Two-model implement/review loop, approved first pass.

Stacked on #1743 (test the whole stack at the leaf; merge base-first).

Note: intentionally no Linear ticket for this cleanup stack, so that check stays red.

Note

Medium Risk
Public RpcClient API behavior changes (no default timeouts, removed export, prompt errors propagate), though failures are more explicit; lifecycle fixes reduce hang risk on restart/stop.

Overview
RpcClient no longer applies fixed client-side timeouts on send, refine, waitForIdle, collectEvents, or promptAndWait — work completes on a real response or agent_end, or fails when the transport dies. Callers can still pass an explicit timeout to the wait helpers. REFINE_REQUEST_TIMEOUT_MS is removed.

Startup awaits the child spawn event instead of a 100ms sleep and immediate exit-code check.

Process lifecycle is generation-scoped: restart/stop/error/exit/stdout-close call failPendingOperations so pending RPCs and event waiters reject with stderr context; late output from a replaced child cannot satisfy the new session. stop() fails in-flight work, uses SIGTERM with a 1s SIGKILL fallback, and can restart even when a grandchild keeps stdio pipes open. prompt() now surfaces server-side prompt failures via the response instead of returning after send only.

Tests add a hanging-child fixture and rpc-client-timeout.test.ts; the refine extended-timeout unit test is dropped.

^{Reviewed by Cursor Bugbot for commit 2cccab2. Bugbot is set up for automated code reviews on this repo. Configure here.}

Linear ticket: ENG-5662
(ticket linked above)

Note

Replace RPC client startup sleep with `spawn` event and remove blanket timeouts

RpcClient.start() now resolves when the child emits spawn instead of after a fixed delay, and rejects cleanly on spawn errors so the client remains restartable
Removed client-side response timeouts from send(), refine(), waitForIdle(), collectEvents(), and promptAndWait(); these now run indefinitely by default unless an explicit timeout is passed
Added failPendingOperations() which caches a terminal transportError and rejects all pending requests and event waiters when the process errors, exits, or output closes
prompt() now throws on unsuccessful responses via getData<void>() instead of silently returning
stop() tolerates grandchild-held stdio pipes, proactively rejects pending work, and SIGKILLs after a 1s fallback
Behavioral Change: long-running RPC commands and agent turns no longer time out by default; consumers relying on the old fixed timeouts (including REFINE_REQUEST_TIMEOUT_MS) must pass explicit timeouts or handle indefinite waits

^{Macroscope summarized 2cccab2.}

…ries

…es cannot hang shutdown stop() waited for the child 'close' event with no fallback: a grandchild inheriting the stdio pipes (or a child that never dies) kept 'close' from firing, hanging stop() forever and leaving this.process set so a later start() threw 'Client already started'. Wait on 'exit' instead, resolve after the SIGKILL escalation as a fallback, and clear this.process explicitly (guarded against a restarted child).

# Conflicts: # packages/coding-agent/test/rpc-client-timeout.test.ts

…hild Late error/stdout-close events from a replaced child could set transportError on a freshly restarted client, rejecting its pending work. Guard every child handler with this.process === child, fail pending operations on 'exit' so a grandchild holding stdout cannot leave requests and idle waiters hanging, and finalize on 'close' as well so a failed spawn (which emits 'error'/'close' but never 'exit') clears this.process and start() can be retried.

… start() can be retried once(child, 'spawn') rejects on the child 'error' event, which for a failed spawn fires before the 'close' event where the finalizer clears this.process; the rejection therefore won the race and a retried start() threw 'Client already started'. Clear this.process in the rejection path: an error before 'spawn' can only mean the child never came up, so no live process is abandoned, and the finalizer's process-identity guard makes the later 'close' a no-op.

…n child exit Node may emit 'exit' while a complete response is still buffered in the stdout pipe (docs guarantee only that 'close' runs after the streams drain), so the exit finalizer could reject a request whose answer was already in flight. Defer failPendingOperations until stdout closes, with a 1s unref'd fallback so pipes held open by a grandchild still fail promptly, and skip the deferred failure if the client was restarted.

…start If start() ran inside a dead child's stdout-drain window, the deferred failure skipped (anti-poisoning guard) and the old generation's pending requests and idle waiters hung forever; the old JSONL reader also stayed attached, so late output from the dead child's pipe could resolve the new session's waiters. start() now detaches the previous reader and fails any leftover pending operations synchronously before spawning.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8c392f1. Configure here.}

The ghost grandchild now also writes a sentinel to the inherited stderr, which still feeds the client's shared stderr accumulator after restart. The test awaits the sentinel before asserting the new session's waiter is untouched, so the still-pending assertion cannot pass vacuously if the stdin-EOF trigger ever stops firing.

snimu added 2 commits August 27, 2026 14:04

fix(coding-agent): remove blanket RPC timeouts, reject on real bounda…

4130037

…ries

refactor(coding-agent): await the spawn event instead of a startup sleep

c33e516