From 2cef65ac7ae42ba9b688066a1b06d29398b45ff9 Mon Sep 17 00:00:00 2001 From: Nowaker Date: Mon, 12 Oct 2026 15:33:36 -0500 Subject: [PATCH 1/2] fix(opencode): answer a failed TUI worker request instead of hanging A method behind the TUI worker's RPC channel that threw never got a reply, so the caller's promise never settled. The TUI waits on that channel before it draws anything, so for example `opencode --port 4096` with port 4096 already taken stayed on a blank screen forever. The worker now answers a failed request with the error's message, or its cause's when it has none (a taken port fails with an empty ServeError whose cause names the port), and the call rejects with it, so opencode exits with that message. --- packages/opencode/src/util/rpc.ts | 33 +++++++++++------ packages/opencode/test/fixture/rpc-worker.ts | 18 ++++++++++ packages/opencode/test/util/rpc.test.ts | 38 ++++++++++++++++++++ 3 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 packages/opencode/test/fixture/rpc-worker.ts create mode 100644 packages/opencode/test/util/rpc.test.ts diff --git a/packages/opencode/src/util/rpc.ts b/packages/opencode/src/util/rpc.ts index 02586ebcfc60..1f9d2038c86c 100644 --- a/packages/opencode/src/util/rpc.ts +++ b/packages/opencode/src/util/rpc.ts @@ -1,3 +1,5 @@ +import { errorMessage } from "@/util/error" + type Definition = { [method: string]: (input: any) => any } @@ -5,10 +7,20 @@ type Definition = { export function listen(rpc: Definition) { onmessage = async (evt) => { const parsed = JSON.parse(evt.data) - if (parsed.type === "rpc.request") { - const result = await rpc[parsed.method](parsed.input) - postMessage(JSON.stringify({ type: "rpc.result", result, id: parsed.id })) - } + if (parsed.type !== "rpc.request") return + // A request that throws must still be answered, or its caller waits forever. + const reply = await Promise.resolve() + .then(() => rpc[parsed.method](parsed.input)) + .then( + (result) => ({ type: "rpc.result", result, id: parsed.id }), + (error) => ({ + type: "rpc.result", + // Some failures keep their text on the cause: a taken --port rejects with an empty ServeError. + error: errorMessage(error instanceof Error && !error.message && error.cause ? error.cause : error), + id: parsed.id, + }), + ) + postMessage(JSON.stringify(reply)) } } @@ -20,16 +32,17 @@ export function client(target: { postMessage: (data: string) => void | null onmessage: ((this: Worker, ev: MessageEvent) => any) | null }) { - const pending = new Map void>() + const pending = new Map void; reject: (error: Error) => void }>() const listeners = new Map void>>() let id = 0 target.onmessage = async (evt) => { const parsed = JSON.parse(evt.data) if (parsed.type === "rpc.result") { - const resolve = pending.get(parsed.id) - if (resolve) { - resolve(parsed.result) + const request = pending.get(parsed.id) + if (request) { pending.delete(parsed.id) + if (typeof parsed.error === "string") request.reject(new Error(parsed.error)) + else request.resolve(parsed.result) } } if (parsed.type === "rpc.event") { @@ -44,8 +57,8 @@ export function client(target: { return { call(method: Method, input: Parameters[0]): Promise> { const requestId = id++ - return new Promise((resolve) => { - pending.set(requestId, resolve) + return new Promise((resolve, reject) => { + pending.set(requestId, { resolve, reject }) target.postMessage(JSON.stringify({ type: "rpc.request", method, input, id: requestId })) }) }, diff --git a/packages/opencode/test/fixture/rpc-worker.ts b/packages/opencode/test/fixture/rpc-worker.ts new file mode 100644 index 000000000000..11a89cec878c --- /dev/null +++ b/packages/opencode/test/fixture/rpc-worker.ts @@ -0,0 +1,18 @@ +import { Rpc } from "../../src/util/rpc" + +export const rpc = { + echo(input: string) { + return input + }, + throws() { + throw new Error("thrown in the worker") + }, + async rejects() { + throw new Error("rejected in the worker") + }, + wrapped() { + throw new Error("", { cause: new Error("the cause says why") }) + }, +} + +Rpc.listen(rpc) diff --git a/packages/opencode/test/util/rpc.test.ts b/packages/opencode/test/util/rpc.test.ts new file mode 100644 index 000000000000..1bd9e0f61381 --- /dev/null +++ b/packages/opencode/test/util/rpc.test.ts @@ -0,0 +1,38 @@ +import { afterAll, describe, expect, test } from "bun:test" +import { Rpc } from "../../src/util/rpc" +import type { rpc } from "../fixture/rpc-worker" + +const worker = new Worker(new URL("../fixture/rpc-worker.ts", import.meta.url)) +const client = Rpc.client(worker) + +afterAll(() => worker.terminate()) + +// The protocol has no timeout of its own, so a call is raced against one here: +// an unanswered request then fails the assertion instead of stalling the test. +const settle = (call: Promise) => + Promise.race([ + call.then( + (value) => ({ value }), + (error: Error) => ({ error: error.message }), + ), + Bun.sleep(2000).then(() => "no reply"), + ]) + +describe("util.rpc", () => { + test("resolves with the worker's result", async () => { + expect(await settle(client.call("echo", "hello"))).toEqual({ value: "hello" }) + }) + + test("rejects when the worker's method throws", async () => { + expect(await settle(client.call("throws", undefined))).toEqual({ error: "thrown in the worker" }) + }) + + test("rejects with the cause's message when the error has none of its own", async () => { + expect(await settle(client.call("wrapped", undefined))).toEqual({ error: "the cause says why" }) + }) + + test("rejects when the worker's method rejects, and keeps serving", async () => { + expect(await settle(client.call("rejects", undefined))).toEqual({ error: "rejected in the worker" }) + expect(await settle(client.call("echo", "still there"))).toEqual({ value: "still there" }) + }) +}) From 672e38abe1103a1b49cfb5b6994a57231a95174f Mon Sep 17 00:00:00 2001 From: James Long Date: Tue, 27 Oct 2026 12:45:12 -0400 Subject: [PATCH 2/2] fix(opencode): answer unserializable rpc results and drop flaky rpc test --- packages/opencode/src/util/rpc.ts | 28 +++++++-------- packages/opencode/test/fixture/rpc-worker.ts | 18 ---------- packages/opencode/test/util/rpc.test.ts | 38 -------------------- 3 files changed, 13 insertions(+), 71 deletions(-) delete mode 100644 packages/opencode/test/fixture/rpc-worker.ts delete mode 100644 packages/opencode/test/util/rpc.test.ts diff --git a/packages/opencode/src/util/rpc.ts b/packages/opencode/src/util/rpc.ts index 1f9d2038c86c..02988bb2a3a9 100644 --- a/packages/opencode/src/util/rpc.ts +++ b/packages/opencode/src/util/rpc.ts @@ -11,19 +11,18 @@ export function listen(rpc: Definition) { // A request that throws must still be answered, or its caller waits forever. const reply = await Promise.resolve() .then(() => rpc[parsed.method](parsed.input)) - .then( - (result) => ({ type: "rpc.result", result, id: parsed.id }), - (error) => ({ - type: "rpc.result", - // Some failures keep their text on the cause: a taken --port rejects with an empty ServeError. - error: errorMessage(error instanceof Error && !error.message && error.cause ? error.cause : error), - id: parsed.id, - }), - ) - postMessage(JSON.stringify(reply)) + .then((result) => JSON.stringify({ type: "rpc.result", result, id: parsed.id })) + .catch((error) => JSON.stringify({ type: "rpc.result", error: failureMessage(error), id: parsed.id })) + postMessage(reply) } } +// Some failures keep their text on the cause: a taken --port rejects with an empty ServeError. +function failureMessage(error: unknown): string { + if (error instanceof Error && !error.message && error.cause) return failureMessage(error.cause) + return errorMessage(error) +} + export function emit(event: string, data: unknown) { postMessage(JSON.stringify({ type: "rpc.event", event, data })) } @@ -39,11 +38,10 @@ export function client(target: { const parsed = JSON.parse(evt.data) if (parsed.type === "rpc.result") { const request = pending.get(parsed.id) - if (request) { - pending.delete(parsed.id) - if (typeof parsed.error === "string") request.reject(new Error(parsed.error)) - else request.resolve(parsed.result) - } + if (!request) return + pending.delete(parsed.id) + if (typeof parsed.error === "string") return request.reject(new Error(parsed.error)) + return request.resolve(parsed.result) } if (parsed.type === "rpc.event") { const handlers = listeners.get(parsed.event) diff --git a/packages/opencode/test/fixture/rpc-worker.ts b/packages/opencode/test/fixture/rpc-worker.ts deleted file mode 100644 index 11a89cec878c..000000000000 --- a/packages/opencode/test/fixture/rpc-worker.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { Rpc } from "../../src/util/rpc" - -export const rpc = { - echo(input: string) { - return input - }, - throws() { - throw new Error("thrown in the worker") - }, - async rejects() { - throw new Error("rejected in the worker") - }, - wrapped() { - throw new Error("", { cause: new Error("the cause says why") }) - }, -} - -Rpc.listen(rpc) diff --git a/packages/opencode/test/util/rpc.test.ts b/packages/opencode/test/util/rpc.test.ts deleted file mode 100644 index 1bd9e0f61381..000000000000 --- a/packages/opencode/test/util/rpc.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { afterAll, describe, expect, test } from "bun:test" -import { Rpc } from "../../src/util/rpc" -import type { rpc } from "../fixture/rpc-worker" - -const worker = new Worker(new URL("../fixture/rpc-worker.ts", import.meta.url)) -const client = Rpc.client(worker) - -afterAll(() => worker.terminate()) - -// The protocol has no timeout of its own, so a call is raced against one here: -// an unanswered request then fails the assertion instead of stalling the test. -const settle = (call: Promise) => - Promise.race([ - call.then( - (value) => ({ value }), - (error: Error) => ({ error: error.message }), - ), - Bun.sleep(2000).then(() => "no reply"), - ]) - -describe("util.rpc", () => { - test("resolves with the worker's result", async () => { - expect(await settle(client.call("echo", "hello"))).toEqual({ value: "hello" }) - }) - - test("rejects when the worker's method throws", async () => { - expect(await settle(client.call("throws", undefined))).toEqual({ error: "thrown in the worker" }) - }) - - test("rejects with the cause's message when the error has none of its own", async () => { - expect(await settle(client.call("wrapped", undefined))).toEqual({ error: "the cause says why" }) - }) - - test("rejects when the worker's method rejects, and keeps serving", async () => { - expect(await settle(client.call("rejects", undefined))).toEqual({ error: "rejected in the worker" }) - expect(await settle(client.call("echo", "still there"))).toEqual({ value: "still there" }) - }) -})