From bd6feac235287a3bc573ac2cb195e12e511e0fa1 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 10 May 2026 13:46:10 +0800 Subject: [PATCH] test(codex): regression coverage for app-server JSON-RPC dispatch (#869) * test(codex): regression test for server-initiated JSON-RPC approval round-trip Verified this test fails against the pre-#866 classifier shape from #758. * test(codex): cover all server-initiated request methods in transport --- .../providers/codex-app-server-agent.test.ts | 213 ++++++++++++++++++ .../codex/app-server-transport.test.ts | 20 +- 2 files changed, 227 insertions(+), 6 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index cf7d1a226..3688410a9 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -98,6 +98,122 @@ function markdownImageSource(markdown: string): string { return match[1].replace(/\\\)/g, ")"); } +function createChildProcessStub(): ChildProcessWithoutNullStreams { + const child = new EventEmitter() as ChildProcessWithoutNullStreams; + child.stdin = new PassThrough() as ChildProcessWithoutNullStreams["stdin"]; + child.stdout = new PassThrough() as ChildProcessWithoutNullStreams["stdout"]; + child.stderr = new PassThrough() as ChildProcessWithoutNullStreams["stderr"]; + child.exitCode = null; + child.signalCode = null; + child.kill = vi.fn((signal?: NodeJS.Signals | number) => { + queueMicrotask(() => child.emit("exit", null, signal ?? null)); + return true; + }) as ChildProcessWithoutNullStreams["kill"]; + return child; +} + +function createScriptedCodexPeer( + child: ChildProcessWithoutNullStreams, + handlers: Record unknown>, +) { + const messages: Record[] = []; + const errors: Error[] = []; + const waiters = new Set<{ + predicate: (message: Record) => boolean; + resolve: (message: Record) => void; + }>(); + let buffer = ""; + + const processMessage = (message: Record) => { + messages.push(message); + for (const waiter of Array.from(waiters)) { + if (waiter.predicate(message)) { + waiters.delete(waiter); + waiter.resolve(message); + } + } + + if (typeof message.id !== "number" || typeof message.method !== "string") { + return; + } + + const handler = handlers[message.method]; + if (!handler) { + errors.push(new Error(`Unexpected Codex app-server request: ${message.method}`)); + return; + } + + Promise.resolve(handler(message.params)) + .then((result) => { + child.stdout.write(`${JSON.stringify({ id: message.id, result })}\n`); + return undefined; + }) + .catch((error) => { + child.stdout.write( + `${JSON.stringify({ + id: message.id, + error: { message: error instanceof Error ? error.message : String(error) }, + })}\n`, + ); + return undefined; + }); + }; + + child.stdin.on("data", (chunk) => { + buffer += chunk.toString(); + for (;;) { + const newlineIndex = buffer.indexOf("\n"); + if (newlineIndex === -1) { + break; + } + const line = buffer.slice(0, newlineIndex).trim(); + buffer = buffer.slice(newlineIndex + 1); + if (!line) { + continue; + } + try { + const parsed: unknown = JSON.parse(line); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + processMessage(parsed as Record); + } + } catch (error) { + errors.push(error instanceof Error ? error : new Error(String(error))); + } + } + }); + + return { + assertNoErrors() { + if (errors.length > 0) { + throw errors[0]; + } + }, + waitForMessage( + predicate: (message: Record) => boolean, + label: string, + ): Promise> { + const existing = messages.find(predicate); + if (existing) { + return Promise.resolve(existing); + } + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + waiters.delete(waiter); + reject(new Error(`Timed out waiting for ${label}`)); + }, 1000); + const waiter = { + predicate, + resolve: (message: Record) => { + clearTimeout(timeout); + resolve(message); + }, + }; + waiters.add(waiter); + }); + }, + }; +} + describe("Codex app-server provider", () => { test("passes ephemeral: true to thread/start when constructed as ephemeral", async () => { const requests: Array<{ method: string; params: unknown }> = []; @@ -184,6 +300,103 @@ describe("Codex app-server provider", () => { } }); + test("round-trips server-initiated command approvals through the real app-server transport", async () => { + const child = createChildProcessStub(); + const peer = createScriptedCodexPeer(child, { + initialize: () => ({}), + "collaborationMode/list": () => ({ data: [] }), + "skills/list": () => ({ data: [] }), + }); + const session = new __codexAppServerInternals.CodexAppServerAgentSession( + createConfig({ cwd: "/workspace/project" }), + null, + createTestLogger(), + async () => child, + ); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + + await session.connect(); + peer.assertNoErrors(); + + const permissionRequested = new Promise< + Extract + >((resolve, reject) => { + const existing = events.find( + (event): event is Extract => + event.type === "permission_requested", + ); + if (existing) { + resolve(existing); + return; + } + const timeout = setTimeout(() => { + unsubscribe(); + reject(new Error("Timed out waiting for permission_requested")); + }, 1000); + const unsubscribe = session.subscribe((event) => { + if (event.type !== "permission_requested") { + return; + } + clearTimeout(timeout); + unsubscribe(); + resolve(event); + }); + }); + + child.stdout.write( + `${JSON.stringify({ + jsonrpc: "2.0", + id: 41, + method: "item/commandExecution/requestApproval", + params: { + itemId: "exec-approval-1", + threadId: "thread-1", + turnId: "turn-1", + command: "git restore README.md", + cwd: "/workspace/project", + reason: "requires escalated permissions", + }, + })}\n`, + ); + + const permissionEvent = await permissionRequested; + expect(permissionEvent.request).toMatchObject({ + id: "permission-exec-approval-1", + provider: "codex", + name: "CodexBash", + kind: "tool", + title: "Run command: git restore README.md", + description: "requires escalated permissions", + input: { + command: "git restore README.md", + cwd: "/workspace/project", + }, + metadata: { + itemId: "exec-approval-1", + threadId: "thread-1", + turnId: "turn-1", + }, + }); + + await session.respondToPermission(permissionEvent.request.id, { behavior: "allow" }); + + await expect( + peer.waitForMessage( + (message) => + message.id === 41 && + !("method" in message) && + JSON.stringify(message.result) === JSON.stringify({ decision: "accept" }), + "command approval response", + ), + ).resolves.toMatchObject({ + id: 41, + result: { decision: "accept" }, + }); + peer.assertNoErrors(); + await session.close(); + }); + test("lists repo skills using WorkspaceGitService repo-root resolution", async () => { const tempDir = await mkdtemp(path.join(tmpdir(), "codex-skills-")); const cwd = path.join(tempDir, "repo", "packages", "app"); diff --git a/packages/server/src/server/agent/providers/codex/app-server-transport.test.ts b/packages/server/src/server/agent/providers/codex/app-server-transport.test.ts index ae9012cc5..2d72266bb 100644 --- a/packages/server/src/server/agent/providers/codex/app-server-transport.test.ts +++ b/packages/server/src/server/agent/providers/codex/app-server-transport.test.ts @@ -32,19 +32,27 @@ describe("Codex app-server transport", () => { child.stdin.end(); }); - test("answers server-initiated requests through registered handlers", async () => { + test.each([ + "item/commandExecution/requestApproval", + "item/fileChange/requestApproval", + "item/tool/requestUserInput", + "tool/requestUserInput", + ])("answers server-initiated %s requests through registered handlers", async (method) => { const child = createChildProcessStub(); const client = new CodexAppServerClient(child, createTestLogger()); - client.setRequestHandler("tool/requestUserInput", async (params) => ({ echoed: params })); + const handlerCalls: unknown[] = []; + client.setRequestHandler(method, async (params) => { + handlerCalls.push(params); + return { ok: true }; + }); const response = new Promise((resolve) => { child.stdin.once("data", (chunk) => resolve(chunk.toString())); }); - child.stdout.write( - '{"id":7,"method":"tool/requestUserInput","params":{"prompt":"Continue?"}}\n', - ); + child.stdout.write(`${JSON.stringify({ jsonrpc: "2.0", id: 7, method, params: {} })}\n`); - await expect(response).resolves.toBe('{"id":7,"result":{"echoed":{"prompt":"Continue?"}}}\n'); + await expect(response).resolves.toBe('{"id":7,"result":{"ok":true}}\n'); + expect(handlerCalls).toEqual([{}]); child.stdout.end(); child.stderr.end(); child.stdin.end();