From 050919a493878045ed4ade0cd1cbb391174920b8 Mon Sep 17 00:00:00 2001 From: jinhyuk9714 Date: Sat, 6 Jun 2026 00:31:11 +0900 Subject: [PATCH 1/3] fix(nodejs): handle stdio stdin errors --- nodejs/src/client.ts | 14 ++++++++++---- nodejs/test/client.test.ts | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 8dc35b8d70..aa2e7807b9 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -2089,10 +2089,16 @@ export class CopilotClient { throw new Error("CLI process not started"); } - // Add error handler to stdin to prevent unhandled rejections during forceStop - this.cliProcess.stdin?.on("error", (err) => { - if (!this.forceStopping) { - throw err; + // Keep stdin pipe errors inside the normal JSON-RPC teardown path. + this.cliProcess.stdin?.on("error", () => { + if (this.forceStopping) { + return; + } + this.state = "error"; + try { + this.connection?.dispose(); + } catch { + // The connection may already be closing after the child process exited. } }); diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 657ec7c9cf..b60541d707 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { describe, expect, it, onTestFinished, vi } from "vitest"; +import { PassThrough } from "stream"; import { approveAll, CopilotClient, @@ -13,6 +14,21 @@ import { defaultJoinSessionPermissionHandler } from "../src/types.js"; // This file is for unit tests. Where relevant, prefer to add e2e tests in e2e/*.test.ts instead describe("CopilotClient", () => { + it("disposes the stdio connection when child stdin emits an error", async () => { + const client = new CopilotClient(); + onTestFinished(() => client.forceStop()); + + const stdin = new PassThrough(); + const stdout = new PassThrough(); + (client as any).cliProcess = { stdin, stdout }; + await (client as any).connectToChildProcessViaStdio(); + + const dispose = vi.spyOn((client as any).connection, "dispose"); + + expect(() => stdin.emit("error", new Error("broken pipe"))).not.toThrow(); + expect(dispose).toHaveBeenCalledOnce(); + }); + it("does not respond to v3 permission requests when handler returns no-result", async () => { const session = new CopilotSession("session-1", {} as any); session.registerPermissionHandler(() => ({ kind: "no-result" })); From aafa12ffbbbbeb82e0a833f550e7afeb2863c1cd Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Wed, 24 Jun 2026 14:12:14 +0100 Subject: [PATCH 2/3] fix(nodejs): preserve stdin error reason during stdio teardown Keep the stdin "error" handler's no-throw teardown, but stash the failure reason on a private field and surface it via the existing logLevel-gated debug logger instead of discarding it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 13 ++++++++++++- nodejs/test/client.test.ts | 4 +++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index a2ebfa8a5d..522b39779c 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -414,6 +414,7 @@ export class CopilotClient { private runtimePort: number | null = null; private actualHost: string = "localhost"; private state: "disconnected" | "connecting" | "connected" | "error" = "disconnected"; + private lastError: Error | null = null; private sessions: Map = new Map(); private stderrBuffer: string = ""; // Captures CLI stderr for error messages /** Resolved connection mode chosen in the constructor. */ @@ -490,6 +491,13 @@ export class CopilotClient { } } + private logDebug(message: string): void { + const level = this.options.logLevel?.toLowerCase(); + if (level === "debug" || level === "all") { + process.stderr.write(`[copilot-sdk] ${message}\n`); + } + } + /** * Creates a new CopilotClient instance. * @@ -2305,11 +2313,14 @@ export class CopilotClient { } // Keep stdin pipe errors inside the normal JSON-RPC teardown path. - this.cliProcess.stdin?.on("error", () => { + // Preserve the failure reason rather than discarding it. + this.cliProcess.stdin?.on("error", (err) => { if (this.forceStopping) { return; } this.state = "error"; + this.lastError = err instanceof Error ? err : new Error(String(err)); + this.logDebug(`stdin pipe error: ${this.lastError.stack ?? this.lastError.message}`); try { this.connection?.dispose(); } catch { diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 44d3560d9a..b6e1caf0fc 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -26,8 +26,10 @@ describe("CopilotClient", () => { const dispose = vi.spyOn((client as any).connection, "dispose"); - expect(() => stdin.emit("error", new Error("broken pipe"))).not.toThrow(); + const boom = new Error("broken pipe"); + expect(() => stdin.emit("error", boom)).not.toThrow(); expect(dispose).toHaveBeenCalledOnce(); + expect((client as any).lastError).toBe(boom); }); it("does not respond to v3 permission requests when handler returns no-result", async () => { From 945e954f7b5224a3a571539a6d3765ca5c58255f Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Wed, 24 Jun 2026 14:24:41 +0100 Subject: [PATCH 3/3] fix(nodejs): drop unused lastError field; keep gated debug log Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 7 +++---- nodejs/test/client.test.ts | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 522b39779c..751d79b9fc 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -414,7 +414,6 @@ export class CopilotClient { private runtimePort: number | null = null; private actualHost: string = "localhost"; private state: "disconnected" | "connecting" | "connected" | "error" = "disconnected"; - private lastError: Error | null = null; private sessions: Map = new Map(); private stderrBuffer: string = ""; // Captures CLI stderr for error messages /** Resolved connection mode chosen in the constructor. */ @@ -2313,14 +2312,14 @@ export class CopilotClient { } // Keep stdin pipe errors inside the normal JSON-RPC teardown path. - // Preserve the failure reason rather than discarding it. + // Preserve the failure reason via the gated debug log rather than discarding it. this.cliProcess.stdin?.on("error", (err) => { if (this.forceStopping) { return; } this.state = "error"; - this.lastError = err instanceof Error ? err : new Error(String(err)); - this.logDebug(`stdin pipe error: ${this.lastError.stack ?? this.lastError.message}`); + const reason = err instanceof Error ? (err.stack ?? err.message) : String(err); + this.logDebug(`stdin pipe error: ${reason}`); try { this.connection?.dispose(); } catch { diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index b6e1caf0fc..5319e22cd4 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -29,7 +29,6 @@ describe("CopilotClient", () => { const boom = new Error("broken pipe"); expect(() => stdin.emit("error", boom)).not.toThrow(); expect(dispose).toHaveBeenCalledOnce(); - expect((client as any).lastError).toBe(boom); }); it("does not respond to v3 permission requests when handler returns no-result", async () => {