From 85acb9ab70bd79efd49f1a248f22dd04048e91c6 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 14 Aug 2026 14:23:15 +0200 Subject: [PATCH] fix(rpc): surface drained stderr when startup fails under load - RpcClient: when stdout closes before ready, race child.exited (which settles only after ptree drains the stderr tail for nonzero exits) for 250ms before rejecting, so the earlier-registered exit watcher rejects with the real stderr text instead of an empty 'Stderr:'. - mock-rpc-agent fixture: await the stderr pipe write callback before process.exit so failure text cannot be dropped unflushed. - sdk-tool-activation: restore the default extension-handler budget once the intentionally stalled activation times out, so full-suite machine load cannot also time out the genuine recovery registration. Both tests flaked only under concurrent full-suite chunk load; 10 concurrent stress runs pass post-fix. --- packages/coding-agent/src/modes/rpc/rpc-client.ts | 7 +++++++ packages/coding-agent/test/fixtures/mock-rpc-agent.ts | 9 ++++++++- packages/coding-agent/test/sdk-tool-activation.test.ts | 9 +++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/src/modes/rpc/rpc-client.ts b/packages/coding-agent/src/modes/rpc/rpc-client.ts index 71af2c246..d4367121d 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-client.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-client.ts @@ -353,6 +353,13 @@ export class RpcClient { // failures are reaped by the readyPromise catch below; established // workers are reaped here so pending requests cannot hang indefinitely. if (!readySettled) { + // Stdout can close before the exit reaper finishes draining stderr. + // child.exited settles only after the stderr tail is complete (for + // nonzero exits), so give it a bounded head start: the exit watcher + // below was registered first and rejects with the real stderr text + // instead of an empty "Stderr:" (flaked under full-suite load). + await Promise.race([child.exited.catch(() => {}), Bun.sleep(250)]); + if (readySettled) return; readySettled = true; readyReject(new Error(`Agent output stream ended before ready. Stderr: ${child.peekStderr()}`)); return; diff --git a/packages/coding-agent/test/fixtures/mock-rpc-agent.ts b/packages/coding-agent/test/fixtures/mock-rpc-agent.ts index d7f4ebfe7..ea1c111d9 100755 --- a/packages/coding-agent/test/fixtures/mock-rpc-agent.ts +++ b/packages/coding-agent/test/fixtures/mock-rpc-agent.ts @@ -30,7 +30,14 @@ const legacyState = { }; if (Bun.env.MOCK_RPC_EXIT_BEFORE_READY) { - process.stderr.write(Bun.env.MOCK_RPC_EXIT_STDERR ?? ""); + const message = Bun.env.MOCK_RPC_EXIT_STDERR ?? ""; + if (message) { + // Await the pipe write: exiting immediately can drop unflushed stderr + // bytes, leaving the client's startup error without the failure text. + const { promise, resolve } = Promise.withResolvers(); + process.stderr.write(message, () => resolve()); + await promise; + } process.exit(Number(Bun.env.MOCK_RPC_EXIT_BEFORE_READY)); } diff --git a/packages/coding-agent/test/sdk-tool-activation.test.ts b/packages/coding-agent/test/sdk-tool-activation.test.ts index f5f5863bc..d9a38bbb4 100644 --- a/packages/coding-agent/test/sdk-tool-activation.test.ts +++ b/packages/coding-agent/test/sdk-tool-activation.test.ts @@ -1247,6 +1247,11 @@ describe("createAgentSession defaultInactive tool activation", () => { const errors: string[] = []; const unsubscribe = runner.onError(error => { errors.push(error.error); + // The 10ms budget exists only to reap the stalled first handler + // quickly; handlers run sequentially and the budget is read per + // handler, so restoring it here keeps machine load from timing out + // the genuine recovery registration too (flaked in full-suite runs). + testSetExtensionHandlerTimeoutMs(EXTENSION_HANDLER_TIMEOUT_MS); }); testSetExtensionHandlerTimeoutMs(10); @@ -1454,6 +1459,10 @@ describe("createAgentSession defaultInactive tool activation", () => { releaseStalledRegistration.resolve(); const failure = await detachedFailure.promise; + // Restore the default budget before the recovered registration flush: + // the 10ms budget was only for reaping the stalled activation, and the + // real presentation pass can exceed it under full-suite load. + testSetExtensionHandlerTimeoutMs(EXTENSION_HANDLER_TIMEOUT_MS); releaseRecoveredRegistration.resolve(); await recoveredActivation.promise;