From 87552aae303ed0fa482c86f428cc5d053c8265bf Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 22 Jul 2026 16:29:16 +0000 Subject: [PATCH 1/4] fix(coding-agent/launch): woke ready waits on sticky marker not live state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hub start and for:"ready" waits polled the live daemon state, so a process that flipped starting→ready→exited within one 50ms poll interval was only ever observed as "exited" and the wait blocked for the full readiness timeout — despite #markReady durably recording readyAt. A pre-ready exit had the same failure since terminal states only woke the wait during broker shutdown. Wake both waits on readyAt !== undefined || terminalState(state); readyTimedOut = !ready then falls out. The start renderer reports "Process exited before readiness was observed." for a pre-ready exit. Adds two regression tests that hang to their caps on the old code. Fixes #6303 --- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/launch/broker.ts | 17 +++- packages/coding-agent/src/tools/hub/launch.ts | 5 ++ .../coding-agent/test/tools/launch.test.ts | 90 +++++++++++++++++++ 4 files changed, 114 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 945a4ed1c..b1c9160ee 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `hub start`/`for:"ready"` waits blocking for the full readiness timeout when the launched process became ready and exited within one poll interval (or exited before ever becoming ready). The wait now wakes on the sticky `readyAt` marker or any terminal state instead of sampling the transient live state, and `start` reports `Process exited before readiness was observed.` for a pre-ready exit ([#6303](https://github.com/can1357/oh-my-pi/issues/6303)). + ## [17.0.7] - 2026-07-21 ### Fixed diff --git a/packages/coding-agent/src/launch/broker.ts b/packages/coding-agent/src/launch/broker.ts index 3120e4ff4..0b2537312 100644 --- a/packages/coding-agent/src/launch/broker.ts +++ b/packages/coding-agent/src/launch/broker.ts @@ -491,8 +491,17 @@ class DaemonBroker { await this.#launch(record); let readyTimedOut = false; if (spec.ready && !terminalState(record.snapshot.state)) { - const ready = await this.#waitUntil(record, () => record.snapshot.state === "ready", spec.ready.timeoutMs); - readyTimedOut = !ready && !terminalState(record.snapshot.state); + // Wake on the sticky readyAt marker or any terminal state, not the live + // state: a fast process flips starting→ready→exited within one poll + // interval, so sampling `state === "ready"` never observes readiness even + // though #markReady durably recorded readyAt. A pre-ready exit must also + // wake the wait rather than block for the full timeout. + const ready = await this.#waitUntil( + record, + () => record.snapshot.readyAt !== undefined || terminalState(record.snapshot.state), + spec.ready.timeoutMs, + ); + readyTimedOut = !ready; } await record.persistQueue; return { op: "start", daemon: record.snapshot, readyTimedOut }; @@ -820,6 +829,10 @@ class DaemonBroker { return true; } if (operation.for === "exit") return terminalState(record.snapshot.state); + // A settled daemon that flipped ready→exited within a poll interval still + // satisfies for:"ready" via the sticky readyAt marker; a terminal state + // wakes the wait so it never blocks for the full timeout. + if (record.snapshot.readyAt !== undefined || terminalState(record.snapshot.state)) return true; return record.snapshot.state === "ready" || (record.snapshot.state === "running" && !record.spec.ready); }; const reached = condition() || (await this.#waitUntil(record, condition, operation.timeoutMs)); diff --git a/packages/coding-agent/src/tools/hub/launch.ts b/packages/coding-agent/src/tools/hub/launch.ts index e110664ad..eb3731513 100644 --- a/packages/coding-agent/src/tools/hub/launch.ts +++ b/packages/coding-agent/src/tools/hub/launch.ts @@ -74,6 +74,9 @@ const KEY_INPUT: Record = { LEFT: "\u001b[D", }; +/** Terminal daemon lifecycle states — the process is no longer running. */ +const TERMINAL_STATES: Partial> = { exited: true, failed: true }; + /** Structured launch state retained for compact TUI rendering. */ export interface LaunchToolDetails { op: LaunchParams["op"]; @@ -229,6 +232,8 @@ function toolContent(result: DaemonRpcResult, params: LaunchParams): string { lines.push( `NOT ready — readiness timed out after ${params.ready?.timeout ?? 30}s${cause}. The process is still running (state: ${daemon.state}); follow its logs or stop it.`, ); + } else if (params.ready && daemon.readyAt === undefined && TERMINAL_STATES[daemon.state]) { + lines.push("Process exited before readiness was observed."); } return lines.join("\n"); } diff --git a/packages/coding-agent/test/tools/launch.test.ts b/packages/coding-agent/test/tools/launch.test.ts index 7d6978307..14d282091 100644 --- a/packages/coding-agent/test/tools/launch.test.ts +++ b/packages/coding-agent/test/tools/launch.test.ts @@ -478,4 +478,94 @@ esac await shutdown(client); } }, 20_000); + + // Regression: a process that flips starting→ready→exited within one 50ms poll + // interval used to hang `start` for the full readiness timeout, because + // #waitUntil sampled the live (already "exited") state instead of the sticky + // readyAt marker #markReady durably recorded. + it("returns promptly when the process becomes ready then exits within a poll", async () => { + const projectDir = await tempDir("omp-daemon-fast-project-"); + const runtimeDir = await tempDir("omp-daemon-fast-runtime-"); + const scriptPath = path.join(projectDir, "fast.ts"); + await Bun.write(scriptPath, `process.stdout.write("done\\n");\n`); + const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + try { + const spec: DaemonSpec = { + name: "fast", + application: process.execPath, + args: [scriptPath], + env: {}, + cwd: projectDir, + pty: false, + ready: { log: ".+", timeoutMs: 60_000 }, + restart: "no", + persist: false, + detached: false, + }; + const t0 = Date.now(); + const started = await client.request({ op: "start", spec }); + const elapsed = Date.now() - t0; + expect(started.op).toBe("start"); + if (started.op !== "start") throw new Error("unexpected start result"); + // Woke on readyAt/terminal, not the full 60s timeout. + expect(elapsed).toBeLessThan(10_000); + expect(started.readyTimedOut).toBeFalse(); + expect(started.daemon.readyAt).toBeDefined(); + } finally { + await shutdown(client); + } + }, 20_000); + + // Regression: a process that exits before ever becoming ready used to block the + // caller for the full timeout, and a for:"ready" wait on the settled daemon did + // the same. Terminal states now wake both waits immediately. + it('wakes start and for:"ready" waits when the process exits before readiness', async () => { + const projectDir = await tempDir("omp-daemon-preexit-project-"); + const runtimeDir = await tempDir("omp-daemon-preexit-runtime-"); + const scriptPath = path.join(projectDir, "preexit.ts"); + // Exits without ever printing the ready pattern. + await Bun.write(scriptPath, `process.stdout.write("nope\\n"); process.exit(0);\n`); + const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + try { + const spec: DaemonSpec = { + name: "preexit", + application: process.execPath, + args: [scriptPath], + env: {}, + cwd: projectDir, + pty: false, + ready: { log: "LISTENING", timeoutMs: 60_000 }, + restart: "no", + persist: false, + detached: false, + }; + const t0 = Date.now(); + const started = await client.request({ op: "start", spec }); + const startElapsed = Date.now() - t0; + expect(started.op).toBe("start"); + if (started.op !== "start") throw new Error("unexpected start result"); + expect(startElapsed).toBeLessThan(10_000); + // Woke on the terminal exit rather than timing out; the readyAt marker is + // absent because the ready pattern never matched. + expect(started.readyTimedOut).toBeFalse(); + expect(started.daemon.readyAt).toBeUndefined(); + expect(["exited", "failed"]).toContain(started.daemon.state); + + // A for:"ready" wait on the already-settled daemon must not hang either. + const t1 = Date.now(); + const waited = await client.request({ + op: "wait", + name: "preexit", + for: "ready", + timeoutMs: 60_000, + }); + const waitElapsed = Date.now() - t1; + expect(waited.op).toBe("wait"); + if (waited.op !== "wait") throw new Error("unexpected wait result"); + expect(waitElapsed).toBeLessThan(10_000); + expect(waited.timedOut).toBeFalse(); + } finally { + await shutdown(client); + } + }, 20_000); }); From 4a000330c68431fbd97f1d195cbb19b36a47cff4 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 22 Jul 2026 16:36:24 +0000 Subject: [PATCH 2/4] fix(coding-agent/launch): cleared stale readiness on restart entry readyAt/readyMatch belong to the exited generation but were only reset by #launch, which runs after the restart backoff delay. During the "restarting" window the sticky-marker predicate from the prior commit therefore reported a dead service as ready, letting start and for:"ready" waits race it. Clear both markers when #settle enters "restarting"; #launch re-sets them once the new child is up. Adds a regression test that observes the backoff window. Fixes #6303 --- packages/coding-agent/src/launch/broker.ts | 5 ++ .../coding-agent/test/tools/launch.test.ts | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/packages/coding-agent/src/launch/broker.ts b/packages/coding-agent/src/launch/broker.ts index 0b2537312..e15742fbd 100644 --- a/packages/coding-agent/src/launch/broker.ts +++ b/packages/coding-agent/src/launch/broker.ts @@ -757,6 +757,11 @@ class DaemonBroker { const uptime = Date.now() - record.snapshot.startedAt; record.consecutiveFailures = uptime >= 30_000 ? 0 : record.consecutiveFailures + 1; record.snapshot.restartCount++; + // Readiness belongs to the exited generation; clear it before the backoff + // so start / for:"ready" waits don't treat a dead service as ready during + // the restart window (readyAt is re-set by #launch once the child is up). + record.snapshot.readyAt = undefined; + record.snapshot.readyMatch = undefined; record.snapshot.state = "restarting"; const delay = Math.min(1_000 * 2 ** Math.min(record.consecutiveFailures, 5), RESTART_MAX_DELAY_MS); record.log?.append( diff --git a/packages/coding-agent/test/tools/launch.test.ts b/packages/coding-agent/test/tools/launch.test.ts index 14d282091..2499c5ad4 100644 --- a/packages/coding-agent/test/tools/launch.test.ts +++ b/packages/coding-agent/test/tools/launch.test.ts @@ -568,4 +568,54 @@ esac await shutdown(client); } }, 20_000); + + // Regression (PR #6305 review): readyAt belongs to the exited generation, so a + // daemon in the restart backoff window must not report readiness. #settle now + // clears readyAt/readyMatch when entering "restarting"; without that, start and + // for:"ready" waits race a dead service during the backoff. + it("clears stale readiness while a daemon is restarting", async () => { + const projectDir = await tempDir("omp-daemon-restart-project-"); + const runtimeDir = await tempDir("omp-daemon-restart-runtime-"); + const scriptPath = path.join(projectDir, "flap.ts"); + // Becomes ready (prints the pattern), then crashes shortly after. + await Bun.write(scriptPath, `process.stdout.write("READY\\n"); setTimeout(() => process.exit(1), 50);\n`); + const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + try { + const spec: DaemonSpec = { + name: "flap", + application: process.execPath, + args: [scriptPath], + env: {}, + cwd: projectDir, + pty: false, + ready: { log: "READY", timeoutMs: 60_000 }, + restart: "on-failure", + persist: false, + detached: false, + }; + const started = await client.request({ op: "start", spec }); + expect(started.op).toBe("start"); + if (started.op !== "start") throw new Error("unexpected start result"); + expect(started.daemon.readyAt).toBeDefined(); + + // Catch the backoff window: once restarting, readiness must be cleared. + const restarting = await waitUntil(async () => { + const listed = await client.request({ op: "list" }); + if (listed.op !== "list") return false; + const daemon = listed.daemons.find(d => d.name === "flap"); + return daemon?.state === "restarting"; + }, 15_000); + expect(restarting).toBeTrue(); + const listed = await client.request({ op: "list" }); + if (listed.op !== "list") throw new Error("unexpected list result"); + const daemon = listed.daemons.find(d => d.name === "flap"); + expect(daemon?.state).toBe("restarting"); + expect(daemon?.readyAt).toBeUndefined(); + expect(daemon?.readyMatch).toBeUndefined(); + + await client.request({ op: "stop", name: "flap", timeoutMs: 2_000 }); + } finally { + await shutdown(client); + } + }, 30_000); }); From 3c1fbdd3f3fd9643b542b8fa78e26d6443226fbd Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 22 Jul 2026 16:42:23 +0000 Subject: [PATCH 3/4] fix(coding-agent/launch): surfaced pre-ready exits in TUI The model-facing start content reported when a process exited before readiness, but launchRenderResult rebuilt the interactive result solely from structured details and dropped that explanation. Mirror the terminal-without-readyAt condition in the TUI start renderer and add a renderer contract test. Fixes #6303 --- packages/coding-agent/src/tools/hub/launch.ts | 2 ++ .../test/tools/launch-renderer.test.ts | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/coding-agent/src/tools/hub/launch.ts b/packages/coding-agent/src/tools/hub/launch.ts index eb3731513..6aeca9ba7 100644 --- a/packages/coding-agent/src/tools/hub/launch.ts +++ b/packages/coding-agent/src/tools/hub/launch.ts @@ -442,6 +442,8 @@ export function launchRenderResult( : "Readiness timed out; the process is still running.", ), ); + } else if (params.ready && daemon && daemon.readyAt === undefined && TERMINAL_STATES[daemon.state]) { + body.push(theme.fg("warning", "Process exited before readiness was observed.")); } break; } diff --git a/packages/coding-agent/test/tools/launch-renderer.test.ts b/packages/coding-agent/test/tools/launch-renderer.test.ts index a0572a5d1..182806773 100644 --- a/packages/coding-agent/test/tools/launch-renderer.test.ts +++ b/packages/coding-agent/test/tools/launch-renderer.test.ts @@ -167,6 +167,34 @@ describe("hub launch rendering", () => { expect(rendered.some(line => line.includes("spawn bun ENOENT"))).toBe(true); }); + it("surfaces a pre-ready exit in the interactive start result", async () => { + const uiTheme = await theme(); + const rendered = lines( + hubToolRenderer.renderResult( + { + content: [{ type: "text", text: "Process exited before readiness was observed." }], + details: { + op: "start", + timedOut: false, + daemon: daemon({ + state: "exited", + pid: undefined, + exitedAt: Date.now(), + exitCode: 0, + readyAt: undefined, + }), + } satisfies LaunchToolDetails, + }, + { expanded: false, isPartial: false }, + uiTheme, + { op: "start", name: "web", application: "bun", ready: { log: "LISTENING" } }, + ), + ); + expect(rendered[0]).toContain("Launch start"); + expect(rendered[0]).toContain("exited"); + expect(rendered.some(line => line.includes("Process exited before readiness was observed."))).toBe(true); + }); + it("names the unmet readiness condition instead of a contradictory Ready + timed-out pair", async () => { const uiTheme = await theme(); const rendered = lines( From b2ac625377920080cef335751830015944ef2b54 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 22 Jul 2026 16:49:34 +0000 Subject: [PATCH 4/4] fix(coding-agent/launch): kept pre-ready exits as not-ready in wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A for:"ready" wait woke on any terminal state, but reported timedOut:false even when readiness was never observed — the only success signal on the wait result — so callers could chain work against a dead process. Split the wake predicate from the ready-observed check: the wait still wakes on a terminal exit, but timedOut now reflects whether readiness was actually observed (readyAt, live ready, or a running daemon with no ready spec). Fixes #6303 --- packages/coding-agent/src/launch/broker.ts | 22 +++++++++++++------ .../coding-agent/test/tools/launch.test.ts | 20 +++++++++++++++-- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/coding-agent/src/launch/broker.ts b/packages/coding-agent/src/launch/broker.ts index e15742fbd..989f9657e 100644 --- a/packages/coding-agent/src/launch/broker.ts +++ b/packages/coding-agent/src/launch/broker.ts @@ -826,6 +826,12 @@ class DaemonBroker { throw new Error(`Invalid wait regex: ${error instanceof Error ? error.message : String(error)}`); } } + // Readiness was actually observed: the sticky readyAt survives a fast + // ready→exit, a live "ready" state, or a "running" daemon with no ready spec. + const readyObserved = (): boolean => + record.snapshot.readyAt !== undefined || + record.snapshot.state === "ready" || + (record.snapshot.state === "running" && !record.spec.ready); const condition = (): boolean => { if (pattern) { const match = pattern.exec(record.readinessBuffer); @@ -834,14 +840,16 @@ class DaemonBroker { return true; } if (operation.for === "exit") return terminalState(record.snapshot.state); - // A settled daemon that flipped ready→exited within a poll interval still - // satisfies for:"ready" via the sticky readyAt marker; a terminal state - // wakes the wait so it never blocks for the full timeout. - if (record.snapshot.readyAt !== undefined || terminalState(record.snapshot.state)) return true; - return record.snapshot.state === "ready" || (record.snapshot.state === "running" && !record.spec.ready); + // Wake on observed readiness or any terminal state so the wait never + // blocks for the full timeout; success is judged by readyObserved below. + return readyObserved() || terminalState(record.snapshot.state); }; - const reached = condition() || (await this.#waitUntil(record, condition, operation.timeoutMs)); - return { op: "wait", daemon: record.snapshot, matched, timedOut: !reached }; + const woke = condition() || (await this.#waitUntil(record, condition, operation.timeoutMs)); + // A for:"ready" wait that woke on a terminal exit without ever observing + // readiness is still "not ready" — surface it as timed out so callers and the + // renderer don't chain work against a dead process. + const timedOut = operation.for === "ready" && !pattern ? !readyObserved() : !woke; + return { op: "wait", daemon: record.snapshot, matched, timedOut }; } async #send(operation: Extract): Promise { diff --git a/packages/coding-agent/test/tools/launch.test.ts b/packages/coding-agent/test/tools/launch.test.ts index 2499c5ad4..fb3764837 100644 --- a/packages/coding-agent/test/tools/launch.test.ts +++ b/packages/coding-agent/test/tools/launch.test.ts @@ -511,6 +511,19 @@ esac expect(elapsed).toBeLessThan(10_000); expect(started.readyTimedOut).toBeFalse(); expect(started.daemon.readyAt).toBeDefined(); + + // A for:"ready" wait on the settled daemon reports success via the sticky + // readyAt marker even though the process has already exited. + const waited = await client.request({ + op: "wait", + name: "fast", + for: "ready", + timeoutMs: 60_000, + }); + expect(waited.op).toBe("wait"); + if (waited.op !== "wait") throw new Error("unexpected wait result"); + expect(waited.timedOut).toBeFalse(); + expect(waited.daemon.readyAt).toBeDefined(); } finally { await shutdown(client); } @@ -551,7 +564,9 @@ esac expect(started.daemon.readyAt).toBeUndefined(); expect(["exited", "failed"]).toContain(started.daemon.state); - // A for:"ready" wait on the already-settled daemon must not hang either. + // A for:"ready" wait on the already-settled daemon must wake immediately, + // but a process that never became ready is surfaced as not ready + // (timedOut) so callers don't chain work against a dead process. const t1 = Date.now(); const waited = await client.request({ op: "wait", @@ -563,7 +578,8 @@ esac expect(waited.op).toBe("wait"); if (waited.op !== "wait") throw new Error("unexpected wait result"); expect(waitElapsed).toBeLessThan(10_000); - expect(waited.timedOut).toBeFalse(); + expect(waited.timedOut).toBeTrue(); + expect(waited.daemon.readyAt).toBeUndefined(); } finally { await shutdown(client); }