diff --git a/src/app-server/connection/json-rpc-client.ts b/src/app-server/connection/json-rpc-client.ts index 8ae9f752..d5e1fc14 100644 --- a/src/app-server/connection/json-rpc-client.ts +++ b/src/app-server/connection/json-rpc-client.ts @@ -99,16 +99,27 @@ export class JsonRpcClient { handleLine(line: string): void { if (line.trim().length === 0) return; - let message: RpcInboundMessage; + let parsed: unknown; try { - message = JSON.parse(line) as RpcInboundMessage; + parsed = JSON.parse(line) as unknown; } catch { this.options.onLog(`Invalid app-server JSON: ${line}`); return; } + const message = rpcInboundMessage(parsed); + if (!message) { + this.options.onLog(`Invalid app-server JSON-RPC message: ${line}`); + return; + } + if ("id" in message && "method" in message) { - this.options.onServerRequest(message); + try { + this.options.onServerRequest(message); + } catch (error) { + this.options.onLog(`App-server request handler failed: ${errorMessage(error)}`); + this.reject(message.id, -32603, "Codex Panel failed to handle the app-server request."); + } return; } @@ -118,7 +129,11 @@ export class JsonRpcClient { } if ("method" in message) { - this.options.onNotification(message); + try { + this.options.onNotification(message); + } catch (error) { + this.options.onLog(`App-server notification handler failed: ${errorMessage(error)}`); + } } } @@ -162,6 +177,33 @@ export class JsonRpcClient { } } +function rpcInboundMessage(value: unknown): RpcInboundMessage | null { + if (!isRecord(value)) return null; + const hasId = Object.hasOwn(value, "id"); + const hasMethod = Object.hasOwn(value, "method"); + if (hasId && !isRequestId(value["id"])) return null; + if (hasMethod && (typeof value["method"] !== "string" || value["method"].length === 0 || !isRecord(value["params"]))) return null; + if (hasId && hasMethod) return value as unknown as ServerRequest; + if (hasId) { + const hasResult = Object.hasOwn(value, "result"); + const hasError = Object.hasOwn(value, "error"); + return hasResult !== hasError ? (value as unknown as RpcInboundMessage) : null; + } + return hasMethod ? (value as unknown as ServerNotification) : null; +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function isRequestId(value: unknown): value is RequestId { + return typeof value === "string" || (typeof value === "number" && Number.isFinite(value)); +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + function isRpcError(value: unknown): value is RpcError { return value !== null && typeof value === "object" && typeof (value as { message?: unknown }).message === "string"; } diff --git a/tests/app-server/app-server-client.test.ts b/tests/app-server/app-server-client.test.ts index f36af7ea..f5ac357d 100644 --- a/tests/app-server/app-server-client.test.ts +++ b/tests/app-server/app-server-client.test.ts @@ -213,6 +213,62 @@ describe("AppServerClient", () => { expect(latestSent(getTransport())).toEqual({ id: 99, error: { code: -32601, message: "Request not handled." } }); }); + it("logs malformed JSON-RPC envelopes without throwing from the transport callback", async () => { + const logs: string[] = []; + const notifications = vi.fn(); + const serverRequests = vi.fn(); + let transport!: FakeTransport; + const client = createTestClient({ + handlers: { + onNotification: notifications, + onServerRequest: serverRequests, + onLog: (message) => logs.push(message), + onExit: () => undefined, + }, + transportFactory: (handlers) => { + transport = new FakeTransport(handlers); + return transport; + }, + }); + const connecting = client.connect(); + transport.emitLine({ id: 1, result: { codexHome: "/tmp/codex" } satisfies Partial }); + await connecting; + + for (const message of [null, 42, "text", [], {}, { method: "warning" }, { id: 4 }, { id: 5, method: "request", params: null }]) { + expect(() => transport.emitLine(message)).not.toThrow(); + } + + expect(notifications).not.toHaveBeenCalled(); + expect(serverRequests).not.toHaveBeenCalled(); + expect(logs).toHaveLength(8); + expect(logs.every((message) => message.startsWith("Invalid app-server JSON-RPC message:"))).toBe(true); + }); + + it("contains inbound handler failures at the JSON-RPC boundary", async () => { + const logs: string[] = []; + let transport!: FakeTransport; + const client = createTestClient({ + handlers: { + onNotification: () => { + throw new Error("notification failed"); + }, + onServerRequest: () => undefined, + onLog: (message) => logs.push(message), + onExit: () => undefined, + }, + transportFactory: (handlers) => { + transport = new FakeTransport(handlers); + return transport; + }, + }); + const connecting = client.connect(); + transport.emitLine({ id: 1, result: { codexHome: "/tmp/codex" } satisfies Partial }); + await connecting; + + expect(() => transport.emitLine({ method: "warning", params: { message: "careful" } })).not.toThrow(); + expect(logs).toEqual(["App-server notification handler failed: notification failed"]); + }); + it("sends typed client requests", async () => { const { client, transport } = await connectedClient();