feat: add review command

Author: gabrielbergamini

src/CodexAcpClient.ts

@@ -27,6 +27,7 @@ import type {
     ListMcpServerStatusParams,
     ListMcpServerStatusResponse,
     Model,
+    ReviewTarget,
     SkillsListParams,
     SkillsListResponse,
     Thread,
@@ -397,6 +398,14 @@ export class CodexAcpClient {
         });
     }
 
+    async runReview(threadId: string, target: ReviewTarget): Promise<TurnCompletedNotification> {
+        return await this.codexClient.runReview({
+            threadId,
+            target,
+            delivery: "inline",
+        });
+    }
+
     async listSkills(params?: SkillsListParams): Promise<SkillsListResponse> {
         return this.codexClient.listSkills(params ?? {});
     }

src/CodexAcpServer.ts

@@ -554,11 +554,14 @@ export class CodexAcpServer implements acp.Agent {
         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: `${entered ? "Entered" : "Exited"} review mode: ${item.review}`,
+                text: `${text}\n\n`,
             },
         };
     }
@@ -742,8 +745,19 @@ export class CodexAcpServer implements acp.Agent {
                 approvalHandler,
                 elicitationHandler);
 
-            if (await this.availableCommands.tryHandle(params.prompt, sessionState)) {
+            const commandResult = await this.availableCommands.tryHandle(params.prompt, sessionState);
+            if (commandResult.handled) {
                 logger.log("Prompt handled by a command");
+                if (commandResult.turnCompleted?.turn.status === "interrupted") {
+                    return await this.createInterruptedPromptResponse(params.sessionId, sessionState);
+                }
+                if (commandResult.turnCompleted) {
+                    const error = eventHandler.getFailure()
+                    if (error) {
+                        // noinspection ExceptionCaughtLocallyJS
+                        throw error;
+                    }
+                }
                 return {
                     stopReason: "end_turn",
                     usage: this.buildPromptUsage(sessionState.lastTokenUsage),
@@ -772,21 +786,7 @@ export class CodexAcpServer implements acp.Agent {
 
             // Check if turn was interrupted (cancelled)
             if (turnCompleted.turn.status === "interrupted") {
-                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()
@@ -809,6 +809,27 @@ export class CodexAcpServer implements acp.Agent {
         }
     }
 
+    private async createInterruptedPromptResponse(
+        sessionId: string,
+        sessionState: SessionState
+    ): Promise<acp.PromptResponse> {
+        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;
 

src/CodexAppServerClient.ts

@@ -22,6 +22,8 @@ import type {
     McpServerStatusUpdatedNotification,
     ModelListParams,
     ModelListResponse,
+    ReviewStartParams,
+    ReviewStartResponse,
     SkillsListParams,
     SkillsListResponse,
     ThreadLoadedListParams,
@@ -160,6 +162,10 @@ export class CodexAppServerClient {
         return await this.sendRequest({ method: "turn/start", params: params });
     }
 
+    async reviewStart(params: ReviewStartParams): Promise<ReviewStartResponse> {
+        return await this.sendRequest({ method: "review/start", params: params });
+    }
+
     async runTurn(params: TurnStartParams): Promise<TurnCompletedNotification> {
         const capturedCompletions: Array<TurnCompletedNotification> = [];
         const releaseCapture = this.captureTurnCompletions(params.threadId, (event) => {
@@ -181,6 +187,31 @@ export class CodexAppServerClient {
         }
     }
 
+    async runReview(params: ReviewStartParams): Promise<TurnCompletedNotification> {
+        if (params.delivery === "detached") {
+            throw new Error("runReview only supports inline review delivery");
+        }
+        const capturedCompletions: Array<TurnCompletedNotification> = [];
+        const releaseCapture = this.captureTurnCompletions(params.threadId, (event) => {
+            capturedCompletions.push(event);
+        });
+
+        try {
+            const reviewStarted = await this.reviewStart(params);
+            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<TurnInterruptResponse> {
         return await this.sendRequest({ method: "turn/interrupt", params: params });
     }

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;
@@ -41,12 +45,12 @@ export class CodexCommands {
         }
     }
 
-    async tryHandle(prompt: acp.ContentBlock[], sessionState: SessionState): Promise<boolean> {
+    async tryHandle(prompt: acp.ContentBlock[], sessionState: SessionState): Promise<CommandHandlingResult> {
         const command = this.parseCommand(prompt);
         if (command) {
             return this.handleCommand(command, sessionState);
         }
-        return false;
+        return { handled: false };
     }
 
     private buildAvailableCommands(skillsEntries: SkillsListEntry[]): AvailableCommand[] {
@@ -81,6 +85,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.",
@@ -119,7 +128,7 @@ export class CodexCommands {
         };
     }
 
-    async handleCommand(command: ParsedCommand, sessionState: SessionState): Promise<boolean> {
+    async handleCommand(command: ParsedCommand, sessionState: SessionState): Promise<CommandHandlingResult> {
         const sessionId = sessionState.sessionId;
 
         switch (command.name) {
@@ -130,7 +139,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());
@@ -139,7 +148,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());
@@ -156,7 +165,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());
@@ -177,14 +186,33 @@ 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)
+                );
+                return { handled: true, turnCompleted };
             }
             default:
                 await this.sendUnknownCommandMessage(command.name, sessionId);
-                return true;
+                return { handled: true };
         }
     }
 
+    private async sendCommandMessage(sessionId: string, text: string): Promise<void> {
+        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<void> {
         const lines = this.getBuiltinCommands().map(command => `- /${command.name}: ${command.description}`);
         const text = [

src/CodexEventHandler.ts

@@ -227,6 +227,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":
@@ -235,7 +237,6 @@ export class CodexEventHandler {
             case "webSearch":
             case "imageView":
             case "imageGeneration":
-            case "enteredReviewMode":
             case "exitedReviewMode":
             case "contextCompaction":
             case "plan":
@@ -280,13 +281,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",