From 4c80c7b114a459a90bc67bddfe0b410d2055ae2b Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 2 Aug 2026 05:40:14 +0000 Subject: [PATCH] fix(mnemopi): exempted initialization from embed timeout Kept the hard timeout and worker reap for steady-state embed requests while allowing first-use runtime installation and model bootstrap to finish without SIGKILL stranding the install lock. Added deterministic coverage for initialization outliving the embed timeout budget and corrected the changelog contract. Fixes #7352 --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/mnemopi/embed-client.ts | 26 ++++++----- .../test/issue-7352-repro.test.ts | 46 +++++++++++++------ 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e952b3443..9a797e9b5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed headless `-p` / `--mode json` runs with `memory.backend: mnemopi` hanging after a completed turn and leaving `__omp_worker_mnemopi_embed` unreaped when the embed worker's fastembed/onnxruntime runtime wedged. Embed-worker IPC requests (`init`/`embed`) were unbounded, so a stuck native runtime blocked the turn's memory recall or the shutdown consolidation forever; requests are now bounded and the wedged worker is reaped on timeout so the next call respawns a fresh child (regression of [#5753](https://github.com/can1357/oh-my-pi/issues/5753); [#7352](https://github.com/can1357/oh-my-pi/issues/7352)). +- Fixed headless `-p` / `--mode json` runs with `memory.backend: mnemopi` hanging after a completed turn and leaving `__omp_worker_mnemopi_embed` unreaped when the embed worker's fastembed/onnxruntime runtime wedged. Steady-state embed requests were unbounded, so a stuck native runtime blocked the turn's memory recall or shutdown consolidation forever; embeds are now bounded and the wedged worker is reaped on timeout so the next call respawns a fresh child, while initialization remains unbounded so first-time runtime installation and model bootstrap are not killed mid-install (regression of [#5753](https://github.com/can1357/oh-my-pi/issues/5753); [#7352](https://github.com/can1357/oh-my-pi/issues/7352)). ## [17.2.4] - 2026-08-01 diff --git a/packages/coding-agent/src/mnemopi/embed-client.ts b/packages/coding-agent/src/mnemopi/embed-client.ts index a1dd561f1..936048b6e 100644 --- a/packages/coding-agent/src/mnemopi/embed-client.ts +++ b/packages/coding-agent/src/mnemopi/embed-client.ts @@ -85,13 +85,14 @@ export interface MnemopiSubprocessEmbeddingModel { } /** - * Upper bound on how long a single embed-worker IPC round-trip (init or embed) - * may block before the worker is treated as wedged. fastembed's steady-state - * embed and even a cold model load settle well within this; a longer stall + * Upper bound on a steady-state embed IPC round-trip. Initialization is + * intentionally exempt: bundled installs may spend several minutes installing + * fastembed and bootstrapping the model, and killing that worker can strand the + * runtime install lock. Once initialization succeeds, a longer embed stall * means a hung native runtime (issue #4792) that would otherwise pin whatever * awaits the embed — a turn's memory recall or the headless shutdown * consolidation — indefinitely, leaving the process alive with an unreaped - * `__omp_worker_mnemopi_embed` child (issue #7352). On expiry the request fails + * `__omp_worker_mnemopi_embed` child (issue #7352). On expiry the embed fails * and the worker is SIGKILL-reaped so the next request respawns a fresh one. */ const EMBED_REQUEST_TIMEOUT_MS = 120_000; @@ -135,7 +136,7 @@ export class MnemopiEmbedClient { this.#pending.set(id, { kind: "init", model, resolve }); try { worker.send({ type: "init", id, model, cacheDir }); - const ok = await this.#awaitRequest(promise); + const ok = await promise; if (!ok) return null; } finally { this.#pending.delete(id); @@ -195,13 +196,14 @@ export class MnemopiEmbedClient { } /** - * Await one embed-worker IPC reply, bounded by {@link EMBED_REQUEST_TIMEOUT_MS}. - * The timeout timer is `unref`'d so a pending request never keeps the parent - * event loop alive on its own (the awaiting caller does). On expiry the - * wedged worker is SIGKILL-reaped via {@link terminate} — faulting any other - * in-flight request and letting the next call respawn a fresh child — before - * the request rejects, so a hung native runtime cannot pin a turn's recall or - * the shutdown consolidation forever (issue #7352). + * Await one steady-state embed reply, bounded by + * {@link EMBED_REQUEST_TIMEOUT_MS}. The timeout timer is `unref`'d so a + * pending request never keeps the parent event loop alive on its own (the + * awaiting caller does). On expiry the wedged worker is SIGKILL-reaped via + * {@link terminate} — faulting any other in-flight request and letting the + * next call respawn a fresh child — before the request rejects, so a hung + * native runtime cannot pin a turn's recall or shutdown consolidation + * forever (issue #7352). */ async #awaitRequest(promise: Promise): Promise { const { promise: timedOut, resolve: fire } = Promise.withResolvers(); diff --git a/packages/coding-agent/test/issue-7352-repro.test.ts b/packages/coding-agent/test/issue-7352-repro.test.ts index 946586f4c..87dcc827e 100644 --- a/packages/coding-agent/test/issue-7352-repro.test.ts +++ b/packages/coding-agent/test/issue-7352-repro.test.ts @@ -4,18 +4,19 @@ * A headless `omp --mode json --no-session -p @` run with * `memory.backend: mnemopi` hung after its turn completed and left an * unreaped `__omp_worker_mnemopi_embed` child. The embed-worker IPC request - * (`init` / `embed`) had no timeout, so a wedged native runtime (fastembed / + * (`embed`) had no timeout, so a wedged native runtime (fastembed / * onnxruntime hanging, cf. #4792) blocked whatever awaited the embed — the * turn's memory recall or the shutdown consolidation — forever. #5753 only * bounded the dispose-time consolidate *await*; the embed IPC underneath it * stayed unbounded, so the wedge escaped that budget. * - * The fix bounds every embed-worker request: on expiry the request fails and + * The fix bounds steady-state embed requests: on expiry the request fails and * the wedged worker is SIGKILL-reaped so the next call respawns a fresh child. - * These tests drive the client with fake, deliberately-silent workers so the - * contract is exercised without fastembed/onnxruntime. + * Initialization stays unbounded because first use may install fastembed and + * bootstrap the model. These tests use fake workers so the contract is + * exercised without fastembed/onnxruntime. */ -import { describe, expect, it } from "bun:test"; +import { describe, expect, it, vi } from "bun:test"; import { MnemopiEmbedClient, type MnemopiEmbedWorkerHandle } from "@oh-my-pi/pi-coding-agent/mnemopi/embed-client"; import type { MnemopiEmbedWorkerInbound, @@ -112,30 +113,49 @@ describe("issue #7352 — mnemopi embed requests are bounded and reap a wedged w } }, 10_000); - it("returns null and reaps the worker when init itself wedges", async () => { + it("allows initialization to outlive the steady-state embed budget", async () => { + vi.useFakeTimers(); const state = { spawns: 0, terminated: 0 }; - // Worker that never answers anything, including init. + const { promise: initStarted, resolve: markInitStarted } = Promise.withResolvers(); + let completeInit: (() => void) | undefined; const client = new MnemopiEmbedClient(() => { state.spawns += 1; + let handler: ((message: MnemopiEmbedWorkerOutbound) => void) | undefined; return { - send() {}, - onMessage() { - return () => {}; + send(message) { + if (message.type !== "init") return; + completeInit = () => handler?.({ type: "ready", id: message.id }); + markInitStarted(); + }, + onMessage(next) { + handler = next; + return () => { + if (handler === next) handler = undefined; + }; }, onError() { return () => {}; }, async terminate() { state.terminated += 1; + handler = undefined; }, }; }, 50); try { - const model = await client.initialize("fast-bge-base-en-v1.5", undefined); - expect(model).toBeNull(); - expect(state.terminated).toBeGreaterThanOrEqual(1); + const initializing = client.initialize("fast-bge-base-en-v1.5", undefined); + await initStarted; + vi.advanceTimersByTime(10_000); + expect(completeInit).toBeDefined(); + completeInit?.(); + + const model = await initializing; + expect(model).not.toBeNull(); + expect(state.spawns).toBe(1); + expect(state.terminated).toBe(0); } finally { await client.terminate(); + vi.useRealTimers(); } }, 10_000); });