From a88d7d5a5f8b54645fffbab4a0a444ba885edd6f Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Mon, 21 Sep 2026 00:00:48 +0900 Subject: [PATCH] fix: bound hung broker connects instead of waiting forever waitForBrokerEndpoint only resolved on connect or error, so a named pipe or Unix socket that never completed left ensureBrokerSession and later commands hung past their readiness budget. Time out each connect attempt, apply the same deadline when opening a broker client, and treat ETIMEDOUT like the other connection failures that already fall back to a direct app-server. --- plugins/codex/scripts/lib/app-server.mjs | 14 +++++- .../codex/scripts/lib/broker-lifecycle.mjs | 43 ++++++++++++++++--- plugins/codex/scripts/lib/codex.mjs | 3 +- tests/broker-lifecycle.test.mjs | 31 +++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 tests/broker-lifecycle.test.mjs diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index 72b30a764..4097b12c2 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -287,11 +287,23 @@ class BrokerCodexAppServerClient extends AppServerClientBase { const target = parseBrokerEndpoint(this.endpoint); this.socket = net.createConnection({ path: target.path }); this.socket.setEncoding("utf8"); - this.socket.on("connect", resolve); + const timeoutMs = this.options.connectTimeoutMs ?? 2000; + const timer = setTimeout(() => { + const error = Object.assign(new Error("Timed out connecting to the Codex app-server broker."), { + code: "ETIMEDOUT" + }); + this.socket.destroy(); + reject(error); + }, timeoutMs); + this.socket.on("connect", () => { + clearTimeout(timer); + resolve(); + }); this.socket.on("data", (chunk) => { this.handleChunk(chunk); }); this.socket.on("error", (error) => { + clearTimeout(timer); if (!this.exitResolved) { reject(error); } diff --git a/plugins/codex/scripts/lib/broker-lifecycle.mjs b/plugins/codex/scripts/lib/broker-lifecycle.mjs index ef763819c..cb56026b3 100644 --- a/plugins/codex/scripts/lib/broker-lifecycle.mjs +++ b/plugins/codex/scripts/lib/broker-lifecycle.mjs @@ -24,18 +24,49 @@ function connectToEndpoint(endpoint) { export async function waitForBrokerEndpoint(endpoint, timeoutMs = 2000) { const start = Date.now(); while (Date.now() - start < timeoutMs) { + const remainingMs = timeoutMs - (Date.now() - start); + if (remainingMs <= 0) { + break; + } const ready = await new Promise((resolve) => { + let settled = false; + const finish = (value) => { + if (settled) { + return; + } + settled = true; + resolve(value); + }; + const socket = connectToEndpoint(endpoint); - socket.on("connect", () => { - socket.end(); - resolve(true); - }); - socket.on("error", () => resolve(false)); + const attemptTimeoutMs = Math.max(1, Math.min(100, remainingMs)); + const timer = setTimeout(() => { + socket.destroy(); + finish(false); + }, attemptTimeoutMs); + + const onDone = (value) => { + clearTimeout(timer); + if (value) { + socket.end(); + } else { + socket.destroy(); + } + finish(value); + }; + + socket.setTimeout(attemptTimeoutMs, () => onDone(false)); + socket.on("connect", () => onDone(true)); + socket.on("error", () => onDone(false)); }); if (ready) { return true; } - await new Promise((resolve) => setTimeout(resolve, 50)); + const waitMs = Math.min(50, Math.max(0, timeoutMs - (Date.now() - start))); + if (waitMs <= 0) { + break; + } + await new Promise((resolve) => setTimeout(resolve, waitMs)); } return false; } diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..371194fb4 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -621,7 +621,8 @@ async function withAppServer(cwd, fn) { const brokerRequested = client?.transport === "broker" || Boolean(process.env[BROKER_ENDPOINT_ENV]); const shouldRetryDirect = (client?.transport === "broker" && error?.rpcCode === BROKER_BUSY_RPC_CODE) || - (brokerRequested && (error?.code === "ENOENT" || error?.code === "ECONNREFUSED")); + (brokerRequested && + (error?.code === "ENOENT" || error?.code === "ECONNREFUSED" || error?.code === "ETIMEDOUT")); if (client) { await client.close().catch(() => {}); diff --git a/tests/broker-lifecycle.test.mjs b/tests/broker-lifecycle.test.mjs new file mode 100644 index 000000000..ef20cebfc --- /dev/null +++ b/tests/broker-lifecycle.test.mjs @@ -0,0 +1,31 @@ +import net from "node:net"; +import test from "node:test"; +import assert from "node:assert/strict"; + +import { waitForBrokerEndpoint } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; + +function hangingConnection() { + return { + setTimeout() {}, + on() { + return this; + }, + removeAllListeners() {}, + end() {}, + destroy() {} + }; +} + +test("waitForBrokerEndpoint returns false when connect hangs past the timeout", { timeout: 2000 }, async (t) => { + const originalCreateConnection = net.createConnection; + net.createConnection = hangingConnection; + t.after(() => { + net.createConnection = originalCreateConnection; + }); + + const started = Date.now(); + const ready = await waitForBrokerEndpoint("unix:/tmp/codex-hung-broker.sock", 150); + + assert.equal(ready, false); + assert.equal(Date.now() - started < 1000, true); +});