From 7b573b3b8bd77bf385d4f247e44558c6ea64aeda Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:53:07 +0900 Subject: [PATCH 1/5] fix: leave Stop-hook headroom so a timed-out review gate can report The stop-review child timeout matched the Stop hook's 900s budget, so Claude Code killed the hook before it could emit the ETIMEDOUT reason. Lower the inner spawnSync timeout to 14 minutes so a slow review still returns the bypass/manual-review message instead of ending the turn silently. Fixes #766. --- .../codex/scripts/stop-review-gate-hook.mjs | 4 ++-- tests/commands.test.mjs | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..bab91a59a 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -13,7 +13,7 @@ import { sortJobsNewestFirst } from "./lib/job-control.mjs"; import { SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; -const STOP_REVIEW_TIMEOUT_MS = 15 * 60 * 1000; +const STOP_REVIEW_TIMEOUT_MS = 14 * 60 * 1000; const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); const ROOT_DIR = path.resolve(SCRIPT_DIR, ".."); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; @@ -113,7 +113,7 @@ function runStopReview(cwd, input = {}) { return { ok: false, reason: - "The stop-time Codex review task timed out after 15 minutes. Run /codex:review --wait manually or bypass the gate." + "The stop-time Codex review task timed out after 14 minutes. Run /codex:review --wait manually or bypass the gate." }; } diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..11797760d 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -210,6 +210,27 @@ test("hooks keep session-end cleanup and stop gating enabled", () => { assert.match(source, /session-lifecycle-hook\.mjs/); }); +test("stop-review task timeout leaves headroom under the Stop hook timeout", () => { + const hookSource = read("scripts/stop-review-gate-hook.mjs"); + const hooks = JSON.parse(read("hooks/hooks.json")); + const stopHook = hooks.hooks.Stop.flatMap((entry) => entry.hooks).find((hook) => + String(hook.command ?? "").includes("stop-review-gate-hook.mjs") + ); + const hookTimeoutMs = Number(stopHook?.timeout) * 1000; + const timeoutMatch = hookSource.match(/const STOP_REVIEW_TIMEOUT_MS = ([^;]+);/); + assert.ok(timeoutMatch, "STOP_REVIEW_TIMEOUT_MS should be declared"); + assert.match(timeoutMatch[1], /^[\d\s.*+\-/()]+$/); + const innerTimeoutMs = new Function(`return (${timeoutMatch[1]});`)(); + const timeoutMessageMatch = hookSource.match(/timed out after ([0-9]+) minutes/); + + assert.equal(Number.isFinite(hookTimeoutMs), true); + assert.equal(hookTimeoutMs, 900000); + assert.equal(Number.isFinite(innerTimeoutMs), true); + assert.equal(innerTimeoutMs < hookTimeoutMs, true); + assert.equal(hookTimeoutMs - innerTimeoutMs >= 30000, true); + assert.equal(Number(timeoutMessageMatch?.[1]) * 60 * 1000, innerTimeoutMs); +}); + test("setup command can offer Codex install and still points users to codex login", () => { const setup = read("commands/setup.md"); const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); 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 2/5] 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); +}); From 26de90c413ab325bc7a6f329fcc6d05521f65a5f Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Mon, 21 Sep 2026 00:05:53 +0900 Subject: [PATCH 3/5] fix: surface status --wait timeouts instead of looking successful status --wait already set waitTimedOut on the JSON snapshot, but text output still rendered a normal running job and the process exited 0. Callers that poll with /codex:status --wait therefore treated a timed-out wait as a finished status check. Print the timeout and exit non-zero while keeping waitTimedOut in the JSON payload. --- plugins/codex/scripts/codex-companion.mjs | 12 ++++- plugins/codex/scripts/lib/render.mjs | 7 ++- tests/runtime.test.mjs | 59 ++++++++++++++++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..7a34eafa5 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -895,7 +895,17 @@ async function handleStatus(argv) { pollIntervalMs: options["poll-interval-ms"] }) : buildSingleJobSnapshot(cwd, reference); - outputCommandResult(snapshot, renderJobStatusReport(snapshot.job), options.json); + if (snapshot.waitTimedOut) { + process.exitCode = 1; + } + outputCommandResult( + snapshot, + renderJobStatusReport(snapshot.job, { + waitTimedOut: snapshot.waitTimedOut, + timeoutMs: snapshot.timeoutMs + }), + options.json + ); return; } diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec185236..42b6d75c6 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -374,8 +374,13 @@ export function renderStatusReport(report) { return `${lines.join("\n").trimEnd()}\n`; } -export function renderJobStatusReport(job) { +export function renderJobStatusReport(job, options = {}) { const lines = ["# Codex Job Status", ""]; + if (options.waitTimedOut) { + const timeoutSeconds = Math.max(1, Math.round((Number(options.timeoutMs) || 0) / 1000)); + lines.push(`Timed out after ${timeoutSeconds}s while the job was still ${job.status || "active"}.`); + lines.push(""); + } pushJobDetails(lines, job, { showElapsed: job.status === "queued" || job.status === "running", showDuration: job.status !== "queued" && job.status !== "running", diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..db156ad17 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1345,13 +1345,70 @@ test("status --wait times out cleanly when a job is still active", () => { cwd: workspace }); - assert.equal(result.status, 0, result.stderr); + assert.equal(result.status > 0, true, result.stderr); const payload = JSON.parse(result.stdout); assert.equal(payload.job.id, "task-live"); assert.equal(payload.job.status, "running"); assert.equal(payload.waitTimedOut, true); }); +test("status --wait reports a timeout in text output when the job is still active", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + const jobsDir = path.join(stateDir, "jobs"); + fs.mkdirSync(jobsDir, { recursive: true }); + + const logFile = path.join(jobsDir, "task-live.log"); + fs.writeFileSync(logFile, "[2026-03-18T15:30:00.000Z] Starting Codex Task.\n", "utf8"); + fs.writeFileSync( + path.join(jobsDir, "task-live.json"), + JSON.stringify( + { + id: "task-live", + status: "running", + title: "Codex Task", + logFile + }, + null, + 2 + ), + "utf8" + ); + + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify( + { + version: 1, + config: { stopReviewGate: false }, + jobs: [ + { + id: "task-live", + status: "running", + title: "Codex Task", + jobClass: "task", + summary: "Investigate flaky test", + logFile, + createdAt: "2026-03-18T15:30:00.000Z", + startedAt: "2026-03-18T15:30:01.000Z", + updatedAt: "2026-03-18T15:30:02.000Z" + } + ] + }, + null, + 2 + )}\n`, + "utf8" + ); + + const result = run("node", [SCRIPT, "status", "task-live", "--wait", "--timeout-ms", "25"], { + cwd: workspace + }); + + assert.match(result.stdout, /Timed out after \d+s while the job was still running/i); + assert.equal(result.status > 0, true); +}); + test("result returns the stored output for the latest finished job by default", () => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace); From 24f9fd14b738ab947308a262257be8a78d557fb1 Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Mon, 21 Sep 2026 00:11:00 +0900 Subject: [PATCH 4/5] fix: do not crash when a fileChange start event omits changes describeStartedItem assumed item.changes was always present on item/started fileChange notifications. Codex can emit the start event before the change list is known, which threw TypeError and aborted the turn. Treat a missing or non-array changes field as zero file changes so the job can finish. --- plugins/codex/scripts/lib/codex.mjs | 5 ++++- tests/fake-codex-fixture.mjs | 7 +++++++ tests/runtime.test.mjs | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..dd7efec1c 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -248,7 +248,10 @@ function describeStartedItem(state, item) { phase: looksLikeVerificationCommand(item.command) ? "verifying" : "running" }; case "fileChange": - return { message: `Applying ${item.changes.length} file change(s).`, phase: "editing" }; + return { + message: `Applying ${Array.isArray(item.changes) ? item.changes.length : 0} file change(s).`, + phase: "editing" + }; case "mcpToolCall": return { message: `Calling ${item.server}/${item.tool}.`, phase: "investigating" }; case "dynamicToolCall": diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..5743b9dce 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -568,6 +568,13 @@ rl.on("line", (line) => { } const items = [ + ...(BEHAVIOR === "file-change-started-without-changes" + ? [ + { + started: { type: "fileChange", id: "fc_" + turnId } + } + ] + : []), ...(BEHAVIOR === "with-reasoning" ? [ { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..6d0efbe78 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -175,6 +175,25 @@ test("task runs when the active provider does not require OpenAI login", () => { assert.match(result.stdout, /Handled the requested task/); }); +test("task survives fileChange started items that omit changes", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "file-change-started-without-changes"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "apply the patch"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Handled the requested task/); + assert.doesNotMatch(result.stderr, /Cannot read properties of undefined/); +}); + test("task runs without auth preflight so Codex can refresh an expired session", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 852a934b72c521490f356418e2205b0fe101d488 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 16:01:56 +0000 Subject: [PATCH 5/5] docs: record the four imports of this batch Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UXfvnjSC72HsM6EEPVt2Tg --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index b6229b1b9..862064378 100644 --- a/README.md +++ b/README.md @@ -423,6 +423,10 @@ Broker and background-job lifecycle: | [#725](https://github.com/openai/codex-plugin-cc/pull/725) | nothing is spawned through the user's shell on Windows, where MSYS path conversion mangled switches like `taskkill /PID` and left background workers unkillable under Git Bash (this supersedes [#735](https://github.com/openai/codex-plugin-cc/pull/735)) | | [#656](https://github.com/openai/codex-plugin-cc/pull/656) | `/codex:cancel` exits non-zero when neither the turn interrupt nor the worker kill confirmed the job stopped, instead of reporting a cancellation nothing proved | | [#770](https://github.com/openai/codex-plugin-cc/pull/770) | `npm run build` works on Windows: npm runs scripts through cmd.exe, where `mkdir -p` made a stray `-p` directory and then failed every build after the first | +| [#775](https://github.com/openai/codex-plugin-cc/pull/775) | a `fileChange` start event without its change list no longer throws and aborts the turn | +| [#772](https://github.com/openai/codex-plugin-cc/pull/772) | the stop-review gate keeps a minute of headroom under the Stop hook's budget, so a timed-out review can still say so instead of ending the turn silently | +| [#774](https://github.com/openai/codex-plugin-cc/pull/774) | `status --wait` prints its timeout and exits non-zero, instead of looking like a finished status check | +| [#773](https://github.com/openai/codex-plugin-cc/pull/773) | a broker connect that never completes is given up on after 2s and falls back to a direct app-server (the probe half of that PR is not taken: ours already bounds each attempt *and* reports why it failed) | Commands and flags: