feat: add review command

Author: gabrielbergamini

src/CodexAcpClient.ts

@@ -28,6 +28,7 @@ import type {
     GetAccountResponse,
     ListMcpServerStatusResponse,
     Model,
+    ReviewTarget,
     SkillsListParams,
     SkillsListResponse,
     Thread,
@@ -474,6 +475,14 @@ export class CodexAcpClient {
         this.codexClient.markTurnStale(params.threadId, params.turnId);
     }
 
+    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

@@ -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<acp.PromptResponse> {
+        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;
 

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<ReviewStartResponse> {
+        return await this.sendRequest({ method: "review/start", params: params });
+    }
+
     async runTurn(params: TurnStartParams, onTurnStarted?: (turnId: string) => void): Promise<TurnCompletedNotification> {
         const capturedCompletions: Array<TurnCompletedNotification> = [];
         const releaseCapture = this.captureTurnCompletions(params.threadId, (event) => {
@@ -211,6 +217,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;
@@ -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,25 +110,33 @@ 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<boolean> {
-        const commandName = this.getCommandName(prompt);
-        if (commandName === null) return false;
-        if (commandName.startsWith("$")) return false;
+    async tryHandleCommand(
+        prompt: acp.ContentBlock[],
+        sessionState: SessionState
+    ): Promise<CommandHandlingResult> {
+        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);
                 await session.update({
                     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,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(commandName, sessionId);
-                return true;
+                await this.sendUnknownCommandMessage(command.name, sessionId);
+                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 = [
@@ -343,4 +379,7 @@ export class CodexCommands {
     }
 }
 
-type ParsedCommand = { name: string; };
+type ParsedCommand = {
+    name: string;
+    input: string | null;
+};

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",