From 5b3bed18b55f358c13935f852b1875555baaed2a Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 7 Aug 2026 13:11:40 +0000 Subject: [PATCH] fix(launch): accepted empty daemon regex matches Preserved zero-width readiness and wait matches across the daemon wire protocol, and isolated malformed completion events from unrelated pending RPCs. Fixes #7908 --- packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/src/launch/client.ts | 12 +- packages/coding-agent/src/launch/protocol.ts | 9 +- .../coding-agent/test/launch/protocol.test.ts | 47 +++++- .../coding-agent/test/tools/launch.test.ts | 139 ++++++++++++++++++ 5 files changed, 205 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index aa016442f..7a7d32ca4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,8 @@ - Fixed `/reload-plugins` and the Agent Control Center leaving the running session's `task` tool on its create-time agent definitions; refreshed `.omp/agents/*.md` names and descriptions now reach existing tools without restarting ([#7940](https://github.com/can1357/oh-my-pi/issues/7940)). - Fixed legacy Pi extensions failing to load when they call `pi.unregisterProvider()`, and made provider replacement calls take effect immediately after extension runtime initialization ([#7914](https://github.com/can1357/oh-my-pi/issues/7914)). +- Fixed zero-width daemon readiness and wait regex matches being rejected by the hub wire decoder; persisted empty-match completion replays can now be acknowledged without rejecting later side-effecting RPCs ([#7908](https://github.com/can1357/oh-my-pi/issues/7908)). + - Fixed proxy discovery preferring the bundled catalog name over the proxy-reported name, so `omp models refresh` now updates stale display names (e.g. a proxy serving `longcat-2.0` as `"LongCat"` no longer shows the raw id). - Fixed the compiled binary build on Windows: `Bun.Glob.scan` yields backslash-separated paths, which the legacy Pi virtual module used verbatim for export keys and generated identifiers, producing invalid JavaScript. - Fixed Ctrl+O (`app.tools.expand`) not expanding truncated tool output while a tool-approval prompt or other selection dialog held keyboard focus, by promoting the shortcut to a global input listener that fires regardless of focus (it still defers to fullscreen overlays and the tree selector's own Ctrl+O filter cycle) ([#7837](https://github.com/can1357/oh-my-pi/issues/7837)). diff --git a/packages/coding-agent/src/launch/client.ts b/packages/coding-agent/src/launch/client.ts index 96392a48a..07e489279 100644 --- a/packages/coding-agent/src/launch/client.ts +++ b/packages/coding-agent/src/launch/client.ts @@ -367,7 +367,17 @@ class SocketDaemonClient implements DaemonBrokerClient { try { message = parseDaemonWireMessage(decoded); } catch (error) { - this.#rejectPending(error instanceof Error ? error : new Error(String(error))); + const parseError = error instanceof Error ? error : new Error(String(error)); + if ( + typeof decoded === "object" && + decoded !== null && + "event" in decoded && + decoded.event === "daemon-completed" + ) { + logger.warn("Ignoring malformed daemon completion", { error: parseError.message }); + continue; + } + this.#rejectPending(parseError); continue; } if ("event" in message) { diff --git a/packages/coding-agent/src/launch/protocol.ts b/packages/coding-agent/src/launch/protocol.ts index adb1ff090..befa630ac 100644 --- a/packages/coding-agent/src/launch/protocol.ts +++ b/packages/coding-agent/src/launch/protocol.ts @@ -168,6 +168,11 @@ function optionalString(value: unknown, label: string): string | undefined { return stringValue(value, label); } +function optionalRawString(value: unknown, label: string): string | undefined { + if (value === undefined) return undefined; + return rawString(value, label); +} + function booleanValue(value: unknown, label: string): boolean { if (typeof value !== "boolean") throw new Error(`${label} must be a boolean`); return value; @@ -272,7 +277,7 @@ export function parseDaemonSnapshot(value: unknown): DaemonSnapshot { restartCount: numberValue(source.restartCount, "daemon.restartCount"), outputBytes: numberValue(source.outputBytes, "daemon.outputBytes"), owner: optionalString(source.owner, "daemon.owner"), - readyMatch: optionalString(source.readyMatch, "daemon.readyMatch"), + readyMatch: optionalRawString(source.readyMatch, "daemon.readyMatch"), readyPending: source.readyPending === undefined ? undefined : readyPendingList(source.readyPending), persist: booleanValue(source.persist, "daemon.persist"), detached: source.detached === undefined ? false : booleanValue(source.detached, "daemon.detached"), @@ -427,7 +432,7 @@ export function parseDaemonRpcResult(operation: DaemonOperation, value: unknown) return { op: "wait", daemon: parseDaemonSnapshot(source.daemon), - matched: optionalString(source.matched, "result.matched"), + matched: optionalRawString(source.matched, "result.matched"), timedOut: booleanValue(source.timedOut, "result.timedOut"), }; case "send": diff --git a/packages/coding-agent/test/launch/protocol.test.ts b/packages/coding-agent/test/launch/protocol.test.ts index b34018e43..085ba7082 100644 --- a/packages/coding-agent/test/launch/protocol.test.ts +++ b/packages/coding-agent/test/launch/protocol.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "bun:test"; -import { type DaemonOperation, parseDaemonRpcResult, parseDaemonWireRequest } from "../../src/launch/protocol"; +import { + type DaemonOperation, + parseDaemonRpcResult, + parseDaemonSnapshot, + parseDaemonWireRequest, +} from "../../src/launch/protocol"; const operation: Extract = { op: "logs", @@ -18,6 +23,18 @@ const baseResult = { state: "running" as const, }; +const baseSnapshot = { + name: "web", + id: "daemon-1", + state: "ready" as const, + createdAt: 1, + startedAt: 1, + restartCount: 0, + outputBytes: 5, + persist: false, + detached: false, +}; + describe("launch logs protocol", () => { it("decodes terminal rows without changing their bytes", () => { const terminalRows = ["\x1b[0m\x1b[1;38;5;2mready", "", "界e\u0301"]; @@ -74,3 +91,31 @@ describe("launch logs compatibility", () => { expect("terminalText" in result ? result.terminalText : undefined).toBe("progress\rready"); }); }); + +describe("regex-derived protocol fields", () => { + it("preserves an empty readiness match", () => { + expect(parseDaemonSnapshot({ ...baseSnapshot, readyMatch: "" }).readyMatch).toBe(""); + }); + + it("preserves an empty wait pattern match", () => { + const waitOperation: Extract = { + op: "wait", + name: "web", + for: "ready", + pattern: "^", + timeoutMs: 1_000, + }; + expect( + parseDaemonRpcResult(waitOperation, { + daemon: baseSnapshot, + matched: "", + timedOut: false, + }), + ).toEqual({ + op: "wait", + daemon: baseSnapshot, + matched: "", + timedOut: false, + }); + }); +}); diff --git a/packages/coding-agent/test/tools/launch.test.ts b/packages/coding-agent/test/tools/launch.test.ts index 687e2bd09..99e933646 100644 --- a/packages/coding-agent/test/tools/launch.test.ts +++ b/packages/coding-agent/test/tools/launch.test.ts @@ -171,6 +171,61 @@ afterEach(async () => { }); describe("daemon broker", () => { + it("keeps a valid RPC response authoritative after a malformed completion", async () => { + const projectDir = await tempDir("omp-daemon-malformed-completion-project-"); + const runtimeDir = await tempDir("omp-daemon-malformed-completion-runtime-"); + const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + const server = net.createServer(socket => { + let buffer = ""; + socket.setEncoding("utf8"); + socket.on("data", chunk => { + buffer += chunk; + const newline = buffer.indexOf("\n"); + if (newline < 0) return; + const request: unknown = JSON.parse(buffer.slice(0, newline)); + if ( + typeof request !== "object" || + request === null || + !("id" in request) || + typeof request.id !== "string" + ) { + socket.destroy(new Error("request id missing")); + return; + } + socket.write( + `${JSON.stringify({ + event: "daemon-completed", + completionId: "malformed-completion", + owner: "completion-owner", + daemon: null, + })}\n`, + ); + socket.write( + `${JSON.stringify({ + id: request.id, + ok: true, + result: { projectDir }, + })}\n`, + ); + }); + }); + const listening = Promise.withResolvers(); + server.once("error", listening.reject); + server.listen(daemonBrokerEndpoint(projectDir, runtimeDir), listening.resolve); + await listening.promise; + try { + expect(await client.request({ op: "ping" })).toEqual({ op: "ping", projectDir }); + } finally { + client.close(); + const closed = Promise.withResolvers(); + server.close(error => { + if (error) closed.reject(error); + else closed.resolve(); + }); + await closed.promise; + } + }); + it("shares PTY output and input across project clients", async () => { const projectDir = await tempDir("omp-daemon-project-"); const runtimeDir = await tempDir("omp-daemon-runtime-"); @@ -756,6 +811,90 @@ esac } }, 12_000); + it("replays a zero-width completion without poisoning the next start", async () => { + const projectDir = await tempDir("omp-daemon-empty-ready-project-"); + const runtimeDir = await tempDir("omp-daemon-empty-ready-runtime-"); + const markerPath = path.join(projectDir, "victim-ran"); + const owner = "empty-ready-owner"; + const first = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + let recovered: DaemonBrokerClient | undefined; + let victimError: Error | undefined; + try { + first.onCompletion(owner, () => { + throw new Error("leave completion pending for reconnect"); + }); + await first.request({ op: "ping" }); + await first + .request({ + op: "start", + spec: { + name: "empty-ready-poison", + application: process.execPath, + args: ["-e", 'console.log("READY")'], + env: {}, + cwd: projectDir, + pty: false, + ready: { log: "^", timeoutMs: 5_000 }, + restart: "no", + persist: false, + detached: false, + }, + owner, + }) + .catch(() => undefined); + const metaPath = path.join(runtimeDir, "daemons", "empty-ready-poison", "meta.json"); + expect( + await waitUntil(async () => { + const metadata: unknown = await Bun.file(metaPath).json(); + return ( + typeof metadata === "object" && + metadata !== null && + "completionPending" in metadata && + metadata.completionPending === true + ); + }, 3_000), + ).toBeTrue(); + first.close(); + + const completions: DaemonSnapshot[] = []; + recovered = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + recovered.onCompletion(owner, notification => { + completions.push(notification.daemon); + }); + const victim = await recovered + .request({ + op: "start", + spec: { + name: "empty-ready-victim", + application: process.execPath, + args: ["-e", `await Bun.write(${JSON.stringify(markerPath)}, "yes"); console.log("SECOND")`], + env: {}, + cwd: projectDir, + pty: false, + ready: { log: "SECOND", timeoutMs: 5_000 }, + restart: "no", + persist: false, + detached: false, + }, + }) + .catch(error => { + victimError = error instanceof Error ? error : new Error(String(error)); + return undefined; + }); + + expect(await waitUntil(() => Bun.file(markerPath).exists(), 3_000)).toBeTrue(); + expect(await Bun.file(markerPath).text()).toBe("yes"); + expect(victimError).toBeUndefined(); + if (victim?.op !== "start") throw new Error("victim start result missing"); + expect(victim.daemon).toMatchObject({ name: "empty-ready-victim", readyMatch: "SECOND" }); + expect(await waitUntil(() => completions.length === 1, 2_000)).toBeTrue(); + expect(completions[0]).toMatchObject({ name: "empty-ready-poison", readyMatch: "", state: "exited" }); + } finally { + first.close(); + if (recovered) await shutdown(recovered); + } + }, 12_000); + it("replays a recovered non-detached daemon exit", async () => { if (process.platform === "win32") return; const projectDir = await tempDir("omp-daemon-attached-recovery-project-");