diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 0f873e4b..1ed3ca35 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -28,6 +28,7 @@ import type { GetAccountResponse, ListMcpServerStatusResponse, Model, + ReviewTarget, SkillsListParams, SkillsListResponse, Thread, @@ -474,6 +475,18 @@ export class CodexAcpClient { this.codexClient.markTurnStale(params.threadId, params.turnId); } + async runReview( + threadId: string, + target: ReviewTarget, + onTurnStarted?: (turnId: string) => void + ): Promise { + return await this.codexClient.runReview({ + threadId, + target, + delivery: "inline", + }, onTurnStarted); + } + async listSkills(params?: SkillsListParams): Promise { return this.codexClient.listSkills(params ?? {}); } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 0003c058..d05dd38d 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1223,8 +1223,20 @@ export class CodexAcpServer implements acp.Agent { approvalHandler, elicitationHandler); - if (await this.availableCommands.tryHandleCommand(params.prompt, sessionState)) { + const commandResult = await this.availableCommands.tryHandleCommand(params.prompt, sessionState); + if (commandResult.handled) { logger.log("Prompt handled by a command"); + if (commandResult.turnCompleted) { + await this.codexAcpClient.waitForSessionNotifications(params.sessionId); + if (commandResult.turnCompleted.turn.status === "interrupted") { + return await this.createInterruptedPromptResponse(params.sessionId, sessionState); + } + const error = eventHandler.getFailure() + if (error) { + // noinspection ExceptionCaughtLocallyJS + throw error; + } + } return { stopReason: "end_turn", usage: this.buildPromptUsage(sessionState.lastTokenUsage), @@ -1302,23 +1314,7 @@ export class CodexAcpServer implements acp.Agent { // Check if turn was interrupted (cancelled) if (turnCompleted.turn.status === "interrupted") { - if (!this.sessionIsClosing(params.sessionId) && this.sessions.has(params.sessionId)) { - await this.connection.sessionUpdate({ - sessionId: params.sessionId, - update: { - sessionUpdate: "agent_message_chunk", - content: { - type: "text", - text: "*Conversation interrupted*" - } - } - }); - } - return { - stopReason: "cancelled", - usage: this.buildPromptUsage(sessionState.lastTokenUsage), - _meta: this.buildQuotaMeta(sessionState), - }; + return await this.createInterruptedPromptResponse(params.sessionId, sessionState); } const error = eventHandler.getFailure() @@ -1346,6 +1342,29 @@ export class CodexAcpServer implements acp.Agent { } } + private async createInterruptedPromptResponse( + sessionId: string, + sessionState: SessionState + ): Promise { + if (!this.sessionIsClosing(sessionId) && this.sessions.has(sessionId)) { + await this.connection.sessionUpdate({ + sessionId, + update: { + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: "*Conversation interrupted*" + } + } + }); + } + return { + stopReason: "cancelled", + usage: this.buildPromptUsage(sessionState.lastTokenUsage), + _meta: this.buildQuotaMeta(sessionState), + }; + } + private buildQuotaMeta(sessionState: SessionState): { quota: QuotaMeta } { const lastTokenUsage = sessionState.lastTokenUsage; diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index d5145346..50cb1a84 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -21,6 +21,8 @@ import type { McpServerStatusUpdatedNotification, ModelListParams, ModelListResponse, + ReviewStartParams, + ReviewStartResponse, SkillsExtraRootsSetParams, SkillsListParams, SkillsListResponse, @@ -189,6 +191,10 @@ export class CodexAppServerClient { return await this.sendRequest({ method: "turn/start", params: params }); } + async reviewStart(params: ReviewStartParams): Promise { + return await this.sendRequest({ method: "review/start", params: params }); + } + async runTurn(params: TurnStartParams, onTurnStarted?: (turnId: string) => void): Promise { const capturedCompletions: Array = []; const releaseCapture = this.captureTurnCompletions(params.threadId, (event) => { @@ -211,6 +217,35 @@ export class CodexAppServerClient { } } + async runReview( + params: ReviewStartParams, + onTurnStarted?: (turnId: string) => void + ): Promise { + if (params.delivery === "detached") { + throw new Error("runReview only supports inline review delivery"); + } + const capturedCompletions: Array = []; + const releaseCapture = this.captureTurnCompletions(params.threadId, (event) => { + capturedCompletions.push(event); + }); + + try { + const reviewStarted = await this.reviewStart(params); + onTurnStarted?.(reviewStarted.turn.id); + const earlyCompletion = capturedCompletions.find(event => + event.threadId === reviewStarted.reviewThreadId + && event.turn.id === reviewStarted.turn.id + ); + releaseCapture(); + if (earlyCompletion) { + return earlyCompletion; + } + return await this.awaitTurnCompleted(reviewStarted.reviewThreadId, reviewStarted.turn.id); + } finally { + releaseCapture(); + } + } + async turnInterrupt(params: TurnInterruptParams): Promise { return await this.sendRequest({ method: "turn/interrupt", params: params }); } diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index a4ca15e4..5ad18496 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -2,12 +2,16 @@ import type * as acp from "@agentclientprotocol/sdk"; import type {AgentSideConnection, AvailableCommand} from "@agentclientprotocol/sdk"; import {ACPSessionConnection} from "./ACPSessionConnection"; import type {CodexAcpClient} from "./CodexAcpClient"; -import type {RateLimitSnapshot, SkillsListEntry} from "./app-server/v2"; +import type {RateLimitSnapshot, ReviewTarget, SkillsListEntry, TurnCompletedNotification} from "./app-server/v2"; import type {SessionState} from "./CodexAcpServer"; import type {RateLimitsMap} from "./RateLimitsMap"; import type {TokenCount} from "./TokenCount"; import {logger} from "./Logger"; +type CommandHandlingResult = + | { handled: false } + | { handled: true; turnCompleted?: TurnCompletedNotification }; + export class CodexCommands { private readonly connection: AgentSideConnection; private readonly codexAcpClient: CodexAcpClient; @@ -73,6 +77,11 @@ export class CodexCommands { description: "List configured Model Context Protocol (MCP) tools.", input: null }, + { + name: "review", + description: "Review uncommitted changes.", + input: null + }, { name: "skills", description: "List available skills.", @@ -91,7 +100,7 @@ export class CodexCommands { ]; } - private getCommandName(prompt: acp.ContentBlock[]): string | null { + private parseCommand(prompt: acp.ContentBlock[]): ParsedCommand | null { const firstBlock = prompt[0]; if (!firstBlock || firstBlock.type != "text") return null; @@ -101,17 +110,25 @@ export class CodexCommands { const commandText = text.slice(1).trim(); if (commandText.length === 0) return null; - const [name] = commandText.split(/\s+/); - return name?.toLowerCase() ?? null; + const separatorIndex = commandText.search(/\s/); + if (separatorIndex === -1) { + return {name: commandText.toLowerCase(), input: null}; + } + const name = commandText.slice(0, separatorIndex).toLowerCase(); + const input = commandText.slice(separatorIndex).trim(); + return {name, input: input.length > 0 ? input : null}; } - async tryHandleCommand(prompt: acp.ContentBlock[], sessionState: SessionState): Promise { - const commandName = this.getCommandName(prompt); - if (commandName === null) return false; - if (commandName.startsWith("$")) return false; + async tryHandleCommand( + prompt: acp.ContentBlock[], + sessionState: SessionState + ): Promise { + const command = this.parseCommand(prompt); + if (command === null) return {handled: false}; + if (command.name.startsWith("$")) return {handled: false}; const sessionId = sessionState.sessionId; - switch (commandName) { + switch (command.name) { case "status": { const session = new ACPSessionConnection(this.connection, sessionId); const message = this.buildStatusMessage(sessionState); @@ -119,7 +136,7 @@ export class CodexCommands { sessionUpdate: "agent_message_chunk", content: { type: "text", text: message } }); - return true; + return {handled: true}; } case "logout": { await this.runWithProcessCheck(() => this.codexAcpClient.logout()); @@ -128,7 +145,7 @@ export class CodexCommands { sessionUpdate: "agent_message_chunk", content: { type: "text", text: "Logged out from Codex account." } }); - return true; + return {handled: true}; } case "skills": { const response = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills()); @@ -145,7 +162,7 @@ export class CodexCommands { sessionUpdate: "agent_message_chunk", content: { type: "text", text } }); - return true; + return {handled: true}; } case "mcp": { const servers = await this.runWithProcessCheck(() => this.codexAcpClient.listMcpServers()); @@ -166,14 +183,35 @@ export class CodexCommands { sessionUpdate: "agent_message_chunk", content: { type: "text", text } }); - return true; + return {handled: true}; + } + case "review": { + if (command.input !== null) { + await this.sendCommandMessage(sessionId, "/review does not accept arguments yet."); + return {handled: true}; + } + const target: ReviewTarget = {type: "uncommittedChanges"}; + const turnCompleted = await this.runWithProcessCheck(() => + this.codexAcpClient.runReview(sessionId, target, (turnId) => { + sessionState.currentTurnId = turnId; + }) + ); + return {handled: true, turnCompleted}; } default: - await this.sendUnknownCommandMessage(commandName, sessionId); - return true; + await this.sendUnknownCommandMessage(command.name, sessionId); + return {handled: true}; } } + private async sendCommandMessage(sessionId: string, text: string): Promise { + const session = new ACPSessionConnection(this.connection, sessionId); + await session.update({ + sessionUpdate: "agent_message_chunk", + content: {type: "text", text} + }); + } + private async sendUnknownCommandMessage(name: string, sessionId: string): Promise { const lines = this.getBuiltinCommands().map(command => `- /${command.name}: ${command.description}`); const text = [ @@ -343,4 +381,7 @@ export class CodexCommands { } } -type ParsedCommand = { name: string; }; +type ParsedCommand = { + name: string; + input: string | null; +}; diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 70c31314..20db1a8d 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -255,6 +255,8 @@ export class CodexEventHandler { return await createMcpToolCallUpdate(event.item); case "dynamicToolCall": return await createDynamicToolCallUpdate(event.item); + case "enteredReviewMode": + return this.createReviewModeEvent(event.item, true); case "collabAgentToolCall": case "userMessage": case "hookPrompt": @@ -263,7 +265,6 @@ export class CodexEventHandler { case "webSearch": case "imageView": case "imageGeneration": - case "enteredReviewMode": case "exitedReviewMode": case "contextCompaction": case "plan": @@ -308,13 +309,31 @@ export class CodexEventHandler { case "imageView": case "imageGeneration": case "enteredReviewMode": + return null; case "exitedReviewMode": + return this.createReviewModeEvent(event.item, false); case "contextCompaction": case "plan": return null; } } + private createReviewModeEvent( + item: ThreadItem & { type: "enteredReviewMode" | "exitedReviewMode" }, + entered: boolean + ): UpdateSessionEvent { + const text = entered + ? "Code review started: current changes" + : `${item.review}\n\nCode review finished`; + return { + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: `${text}\n\n`, + }, + }; + } + private createCommandOutputDeltaEvent(event: CommandExecutionOutputDeltaNotification): UpdateSessionEvent { return { sessionUpdate: "tool_call_update", diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 355e330a..940f9798 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -13,7 +13,7 @@ import { import type {ServerNotification} from "../../app-server"; import type {SessionState} from "../../CodexAcpServer"; import {AgentMode} from "../../AgentMode"; -import type {Model, TurnStartParams} from "../../app-server/v2"; +import type {Model, TurnCompletedNotification, TurnStartParams} from "../../app-server/v2"; import type {RateLimitsMap} from "../../RateLimitsMap"; import {ModelId} from "../../ModelId"; @@ -912,6 +912,184 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(transportDump).contain(`**Session:** \`${newSessionResponse.sessionId}\``); }); + it('handles review command', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpAgent = mockFixture.getCodexAcpAgent(); + const sessionState: SessionState = createTestSessionState({sessionId: "session-id"}); + const turnCompleted: TurnCompletedNotification = { + threadId: "session-id", + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "completed", + error: null, + startedAt: null, + completedAt: null, + durationMs: null + } + }; + const reviewSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "runReview").mockResolvedValue(turnCompleted); + vi.spyOn(codexAcpAgent, "getSessionState").mockReturnValue(sessionState); + + const response = await codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "/review"}] + }); + + expect(response.stopReason).toBe("end_turn"); + expect(reviewSpy).toHaveBeenCalledWith( + "session-id", + {type: "uncommittedChanges"}, + expect.any(Function) + ); + }); + + it('rejects review command arguments', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpAgent = mockFixture.getCodexAcpAgent(); + const sessionState: SessionState = createTestSessionState({sessionId: "session-id"}); + const reviewSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "runReview"); + vi.spyOn(codexAcpAgent, "getSessionState").mockReturnValue(sessionState); + + const response = await codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "/review check race conditions"}] + }); + + expect(response.stopReason).toBe("end_turn"); + expect(reviewSpy).not.toHaveBeenCalled(); + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/command-review-arguments.json"); + }); + + it('returns cancelled when review command is interrupted', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpAgent = mockFixture.getCodexAcpAgent(); + const sessionState: SessionState = createTestSessionState({sessionId: "session-id"}); + vi.spyOn(mockFixture.getCodexAcpClient(), "runReview").mockResolvedValue({ + threadId: "session-id", + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "interrupted", + error: null, + startedAt: null, + completedAt: null, + durationMs: null + } + }); + vi.spyOn(codexAcpAgent, "getSessionState").mockReturnValue(sessionState); + // @ts-expect-error - exercising interruption behavior for a registered session + codexAcpAgent.sessions.set("session-id", sessionState); + + const response = await codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "/review"}] + }); + + expect(response.stopReason).toBe("cancelled"); + expect(mockFixture.getAcpConnectionDump([])).toContain("*Conversation interrupted*"); + }); + + it('interrupts an active review command when cancelled', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpAgent = mockFixture.getCodexAcpAgent(); + const sessionState: SessionState = createTestSessionState({sessionId: "session-id"}); + vi.spyOn(codexAcpAgent, "getSessionState").mockReturnValue(sessionState); + // @ts-expect-error - exercising cancellation for a registered session + codexAcpAgent.sessions.set("session-id", sessionState); + + const reviewStartSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "reviewStart").mockResolvedValue({ + reviewThreadId: "session-id", + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "inProgress", + error: null, + startedAt: null, + completedAt: null, + durationMs: null + } + }); + let completeReview: (value: TurnCompletedNotification) => void = () => {}; + const reviewCompleted = new Promise((resolve) => { + completeReview = resolve; + }); + vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted").mockReturnValue(reviewCompleted); + const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt").mockResolvedValue(); + + const promptPromise = codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "/review"}] + }); + await vi.waitFor(() => { + expect(reviewStartSpy).toHaveBeenCalled(); + }); + + await codexAcpAgent.cancel({sessionId: "session-id"}); + completeReview({ + threadId: "session-id", + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "interrupted", + error: null, + startedAt: null, + completedAt: null, + durationMs: null + } + }); + await expect(promptPromise).resolves.toMatchObject({stopReason: "cancelled"}); + + expect(turnInterruptSpy).toHaveBeenCalledWith({ + threadId: "session-id", + turnId: "review-turn" + }); + }); + + it('propagates review command event handler failures', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpAgent = mockFixture.getCodexAcpAgent(); + const sessionState: SessionState = createTestSessionState({sessionId: "session-id"}); + vi.spyOn(mockFixture.getCodexAcpClient(), "runReview").mockImplementation(async () => { + mockFixture.sendServerNotification({ + method: "error", + params: { + threadId: "session-id", + turnId: "review-turn", + willRetry: false, + error: { + message: "auth failed", + codexErrorInfo: "unauthorized", + additionalDetails: null + } + } + }); + return { + threadId: "session-id", + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "completed", + error: null, + startedAt: null, + completedAt: null, + durationMs: null + } + }; + }); + vi.spyOn(codexAcpAgent, "getSessionState").mockReturnValue(sessionState); + + await expect(codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "/review"}] + })).rejects.toBeTruthy(); + }); + const mockModels: Model[] = [ { id: '5.2-codex', diff --git a/src/__tests__/CodexACPAgent/data/available-commands-build-in.json b/src/__tests__/CodexACPAgent/data/available-commands-build-in.json index 42999913..5163fbb9 100644 --- a/src/__tests__/CodexACPAgent/data/available-commands-build-in.json +++ b/src/__tests__/CodexACPAgent/data/available-commands-build-in.json @@ -11,6 +11,11 @@ "description": "List configured Model Context Protocol (MCP) tools.", "input": null }, + { + "name": "review", + "description": "Review uncommitted changes.", + "input": null + }, { "name": "skills", "description": "List available skills.", diff --git a/src/__tests__/CodexACPAgent/data/available-commands-skills.json b/src/__tests__/CodexACPAgent/data/available-commands-skills.json index cc64ab60..71f86094 100644 --- a/src/__tests__/CodexACPAgent/data/available-commands-skills.json +++ b/src/__tests__/CodexACPAgent/data/available-commands-skills.json @@ -11,6 +11,11 @@ "description": "List configured Model Context Protocol (MCP) tools.", "input": null }, + { + "name": "review", + "description": "Review uncommitted changes.", + "input": null + }, { "name": "skills", "description": "List available skills.", diff --git a/src/__tests__/CodexACPAgent/data/command-review-arguments.json b/src/__tests__/CodexACPAgent/data/command-review-arguments.json new file mode 100644 index 00000000..6f5621f3 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/command-review-arguments.json @@ -0,0 +1,15 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "session-id", + "update": { + "sessionUpdate": "agent_message_chunk", + "content": { + "type": "text", + "text": "/review does not accept arguments yet." + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 8e962238..2b23f985 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -11,6 +11,11 @@ "description": "List configured Model Context Protocol (MCP) tools.", "input": null }, + { + "name": "review", + "description": "Review uncommitted changes.", + "input": null + }, { "name": "skills", "description": "List available skills.", diff --git a/src/__tests__/CodexACPAgent/data/review-entered.json b/src/__tests__/CodexACPAgent/data/review-entered.json new file mode 100644 index 00000000..16979959 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/review-entered.json @@ -0,0 +1,15 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_message_chunk", + "content": { + "type": "text", + "text": "Code review started: current changes\n\n" + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/review-exited.json b/src/__tests__/CodexACPAgent/data/review-exited.json new file mode 100644 index 00000000..0baee982 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/review-exited.json @@ -0,0 +1,15 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_message_chunk", + "content": { + "type": "text", + "text": "No findings.\n\nCode review finished\n\n" + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/review-events.test.ts b/src/__tests__/CodexACPAgent/review-events.test.ts new file mode 100644 index 00000000..deb81632 --- /dev/null +++ b/src/__tests__/CodexACPAgent/review-events.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { SessionState } from "../../CodexAcpServer"; +import type { ServerNotification } from "../../app-server"; +import { + createCodexMockTestFixture, + createTestSessionState, + setupPromptAndSendNotifications, + type CodexMockTestFixture +} from "../acp-test-utils"; +import { AgentMode } from "../../AgentMode"; + +describe("CodexEventHandler - review events", () => { + let mockFixture: CodexMockTestFixture; + const sessionId = "test-session-id"; + + beforeEach(() => { + mockFixture = createCodexMockTestFixture(); + vi.clearAllMocks(); + }); + + const sessionState: SessionState = createTestSessionState({ + sessionId, + currentModelId: "model-id[effort]", + agentMode: AgentMode.DEFAULT_AGENT_MODE + }); + + it("renders entered review mode", async () => { + const notification: ServerNotification = { + method: "item/started", + params: { + threadId: sessionId, + turnId: "review-turn", + startedAtMs: 0, + item: { + type: "enteredReviewMode", + id: "review-turn", + review: "uncommitted changes", + }, + }, + }; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [notification]); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/review-entered.json"); + }); + + it("renders exited review mode", async () => { + const notification: ServerNotification = { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "review-turn", + completedAtMs: 0, + item: { + type: "exitedReviewMode", + id: "review-turn", + review: "No findings.", + }, + }, + }; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [notification]); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/review-exited.json"); + }); +}); diff --git a/src/__tests__/CodexAppServerClient/review.test.ts b/src/__tests__/CodexAppServerClient/review.test.ts new file mode 100644 index 00000000..89364223 --- /dev/null +++ b/src/__tests__/CodexAppServerClient/review.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it, vi } from "vitest"; +import type { MessageConnection } from "vscode-jsonrpc/node"; +import { CodexAppServerClient } from "../../CodexAppServerClient"; +import type { ServerNotification } from "../../app-server"; + +describe("CodexAppServerClient review support", () => { + function createClient( + sendRequest: (request: unknown) => Promise + ): { client: CodexAppServerClient; sendNotification: (notification: ServerNotification) => void } { + let unhandledNotificationHandler: ((notification: ServerNotification) => void) | null = null; + const connection = { + sendRequest: vi.fn(sendRequest), + onUnhandledNotification: vi.fn((handler: (notification: ServerNotification) => void) => { + unhandledNotificationHandler = handler; + }), + onRequest: vi.fn(), + } as unknown as MessageConnection; + + return { + client: new CodexAppServerClient(connection), + sendNotification(notification: ServerNotification): void { + if (!unhandledNotificationHandler) { + throw new Error("No notification handler registered"); + } + unhandledNotificationHandler(notification); + }, + }; + } + + it("sends review/start with typed params", async () => { + const sendRequest = vi.fn().mockResolvedValue({ + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "inProgress", + error: null, + startedAt: null, + completedAt: null, + durationMs: null, + }, + reviewThreadId: "session-id", + }); + const { client } = createClient(sendRequest); + + await client.reviewStart({ + threadId: "session-id", + target: { type: "uncommittedChanges" }, + delivery: "inline", + }); + + expect(sendRequest).toHaveBeenCalledWith("review/start", { + threadId: "session-id", + target: { type: "uncommittedChanges" }, + delivery: "inline", + }); + }); + + it("returns an early review completion observed before review/start resolves", async () => { + let fixture!: ReturnType; + const earlyCompletion: ServerNotification = { + method: "turn/completed", + params: { + threadId: "session-id", + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "completed", + error: null, + startedAt: null, + completedAt: null, + durationMs: null, + }, + }, + }; + + fixture = createClient(async () => { + fixture.sendNotification(earlyCompletion); + return { + turn: { + id: "review-turn", + items: [], + itemsView: "full", + status: "inProgress", + error: null, + startedAt: null, + completedAt: null, + durationMs: null, + }, + reviewThreadId: "session-id", + }; + }); + + const completed = await fixture.client.runReview({ + threadId: "session-id", + target: { type: "uncommittedChanges" }, + delivery: "inline", + }); + + expect(completed).toEqual(earlyCompletion.params); + }); + + it("rejects detached review delivery without starting review", async () => { + const sendRequest = vi.fn(); + const { client } = createClient(sendRequest); + + await expect(client.runReview({ + threadId: "session-id", + target: { type: "uncommittedChanges" }, + delivery: "detached", + })).rejects.toThrow("runReview only supports inline review delivery"); + + expect(sendRequest).not.toHaveBeenCalled(); + }); +});