fix(codex): harden plan approval lifecycle

Generated-By: PostHog Code
Task-Id: 2cd7c8b3-ddf5-4e4c-994d-1f5bbafabff0

Author: MattPua

packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts

@@ -2759,7 +2759,10 @@ describe("CodexAppServerAgent", () => {
   // permissionOutcome may be a value or a function (per-call, e.g. a pending promise).
   function makePlanAgent(
     permissionOutcome: unknown,
-    options: { rejectPlanToolUpdates?: boolean } = {},
+    options: {
+      rejectPlanToolUpdates?: boolean;
+      stallPlanToolUpdates?: boolean;
+    } = {},
   ) {
     const stub = makeStubRpc({
       "thread/start": { thread: { id: "t" } },
@@ -2775,11 +2778,15 @@ describe("CodexAppServerAgent", () => {
       sessionUpdate: async (n: unknown) => {
         const notification = n as { update?: Record<string, unknown> };
         if (
-          options.rejectPlanToolUpdates &&
-          (notification.update?.sessionUpdate === "tool_call" ||
-            notification.update?.sessionUpdate === "tool_call_update")
+          notification.update?.sessionUpdate === "tool_call" ||
+          notification.update?.sessionUpdate === "tool_call_update"
         ) {
-          throw new Error("renderer disconnected");
+          if (options.rejectPlanToolUpdates) {
+            throw new Error("renderer disconnected");
+          }
+          if (options.stallPlanToolUpdates) {
+            return new Promise(() => {});
+          }
         }
         sessionUpdates.push(notification);
       },
@@ -2916,14 +2923,29 @@ describe("CodexAppServerAgent", () => {
         toolCallId: "p1:implement",
         title: "Ready to code?",
         kind: "switch_mode",
+        content: [
+          {
+            type: "content",
+            content: { type: "text", text: "# The plan\n\n" },
+          },
+        ],
+        rawInput: { plan: "# The plan\n\n" },
+        status: "in_progress",
+      },
+    });
+    expect(sessionUpdates).toContainEqual({
+      sessionId: "t",
+      update: {
+        sessionUpdate: "tool_call_update",
+        toolCallId: "p1:implement",
+        status: "in_progress",
         content: [
           {
             type: "content",
             content: { type: "text", text: "# The plan\n\n1. do it" },
           },
         ],
         rawInput: { plan: "# The plan\n\n1. do it" },
-        status: "in_progress",
       },
     });
     expect(sessionUpdates).toContainEqual({
@@ -3022,6 +3044,27 @@ describe("CodexAppServerAgent", () => {
     );
   });
 
+  it("does not block the handoff when plan tool-call updates never settle", async () => {
+    const { agent, stub, permissionRequests } = makePlanAgent(
+      {
+        outcome: { outcome: "selected", optionId: "reject_with_feedback" },
+      },
+      { stallPlanToolUpdates: true },
+    );
+    const { done } = await startPlanTurn(agent, stub);
+
+    stub.emit("item/plan/delta", { itemId: "p1", delta: "# The plan" });
+    stub.emit("turn/completed", {
+      turn: { id: "turn_1", status: "completed" },
+    });
+
+    expect((await done).stopReason).toBe("end_turn");
+    expect(permissionRequests).toHaveLength(1);
+    expect(stub.requests.filter((r) => r.method === "turn/start")).toHaveLength(
+      1,
+    );
+  });
+
   it("feeds handoff feedback into another plan turn", async () => {
     const { agent, stub, permissionRequests } = makePlanAgent({
       outcome: { outcome: "selected", optionId: "reject_with_feedback" },

packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts

@@ -242,6 +242,8 @@ export class CodexAppServerAgent extends BaseAcpAgent {
   private workspaceDirectory?: string;
   /** The in-flight turn's <proposed_plan>, streamed or completed (drives the implement handoff). */
   private planProposal?: { itemId: string; text: string };
+  /** Structured plan tool call already emitted while the proposal streams. */
+  private streamedPlanToolCallId?: string;
   /** Idle signal deferred while the plan handoff keeps this prompt busy. */
   private deferredTurnComplete?: { usage: PromptResponse["usage"] };
   /** Settles the pending plan-approval race on cancel/close/preempting prompt. */
@@ -879,6 +881,7 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     this.lastAgentMessage = "";
     this.resetUsage();
     this.planProposal = undefined;
+    this.streamedPlanToolCallId = undefined;
     // A new turn owns the idle boundary; its own completion emits the signal.
     this.deferredTurnComplete = undefined;
     const { completion, turn } = this.turns.begin();
@@ -936,13 +939,22 @@ export class CodexAppServerAgent extends BaseAcpAgent {
         // Re-check after the await: a cancel that raced the response wins, so a
         // late accept can never start implementation on a cancelled prompt.
         if (this.session.cancelled) {
+          if (outcome.kind === "implement") {
+            this.completePlanApprovalToolCall(outcome.toolCallId, "failed");
+          }
           result = { ...result, stopReason: "cancelled" };
           break;
         }
         // A picker change while approval was open owns the mode. Never let a
         // stale approval overwrite it with a broader implementation mode.
-        if (this.config.mode !== "plan") break;
+        if (this.config.mode !== "plan") {
+          if (outcome.kind === "implement") {
+            this.completePlanApprovalToolCall(outcome.toolCallId, "failed");
+          }
+          break;
+        }
         if (outcome.kind === "implement") {
+          this.completePlanApprovalToolCall(outcome.toolCallId, "completed");
           this.config.setOption("mode", outcome.mode);
           this.emitCurrentMode(outcome.mode);
           this.emitConfigOptions();
@@ -994,28 +1006,17 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     itemId: string;
     text: string;
   }): Promise<
-    | { kind: "implement"; mode: "auto" | "full-access" }
+    | {
+        kind: "implement";
+        mode: "auto" | "full-access";
+        toolCallId: string;
+      }
     | { kind: "feedback"; feedback: string }
     | { kind: "stay" }
   > {
     const toolCallId = `${proposal.itemId}:implement`;
-    const toolCall = {
-      toolCallId,
-      title: "Ready to code?",
-      kind: "switch_mode",
-      content: [
-        {
-          type: "content" as const,
-          content: { type: "text" as const, text: proposal.text },
-        },
-      ],
-      rawInput: { plan: proposal.text },
-    };
-    await this.emitPlanApprovalToolCall({
-      sessionUpdate: "tool_call",
-      ...toolCall,
-      status: "in_progress",
-    });
+    const toolCall = this.buildPlanApprovalToolCall(proposal);
+    this.emitPlanProposal(toolCall, proposal.text);
     const options = [
       {
         optionId: "auto",
@@ -1059,11 +1060,11 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     const settled = await Promise.race([permission, cancelled]);
     this.planHandoffCancel = undefined;
     if (!settled) {
-      await this.completePlanApprovalToolCall(toolCallId, "failed");
+      this.completePlanApprovalToolCall(toolCallId, "failed");
       return { kind: "stay" };
     }
     if (settled.failed) {
-      await this.completePlanApprovalToolCall(toolCallId, "failed");
+      this.completePlanApprovalToolCall(toolCallId, "failed");
       this.logger.warn("plan implementation prompt failed; staying in plan", {
         error: String(settled.err),
       });
@@ -1075,61 +1076,106 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     }
     const response = settled.res;
     if (this.session.cancelled || response.outcome.outcome !== "selected") {
-      await this.completePlanApprovalToolCall(toolCallId, "failed");
+      this.completePlanApprovalToolCall(toolCallId, "failed");
       return { kind: "stay" };
     }
     const optionId = response.outcome.optionId;
     if (!offered.has(optionId)) {
-      await this.completePlanApprovalToolCall(toolCallId, "failed");
+      this.completePlanApprovalToolCall(toolCallId, "failed");
       return { kind: "stay" };
     }
     if (optionId === "auto") {
-      await this.completePlanApprovalToolCall(toolCallId, "completed");
-      return { kind: "implement", mode: "auto" };
+      return { kind: "implement", mode: "auto", toolCallId };
     }
     // Double-gated: only ever offered under ALLOW_BYPASS, and re-checked here.
     if (optionId === "full-access" && ALLOW_BYPASS) {
-      await this.completePlanApprovalToolCall(toolCallId, "completed");
-      return { kind: "implement", mode: "full-access" };
+      return { kind: "implement", mode: "full-access", toolCallId };
     }
     if (optionId === "reject_with_feedback") {
       const feedback = (response as { _meta?: { customInput?: unknown } })._meta
         ?.customInput;
       if (typeof feedback === "string" && feedback.trim()) {
-        await this.completePlanApprovalToolCall(toolCallId, "failed");
+        this.completePlanApprovalToolCall(toolCallId, "failed");
         return { kind: "feedback", feedback: feedback.trim() };
       }
     }
-    await this.completePlanApprovalToolCall(toolCallId, "failed");
+    this.completePlanApprovalToolCall(toolCallId, "failed");
     return { kind: "stay" };
   }
 
-  private async completePlanApprovalToolCall(
+  private buildPlanApprovalToolCall(proposal: {
+    itemId: string;
+    text: string;
+  }): {
+    toolCallId: string;
+    title: string;
+    kind: "switch_mode";
+    content: Array<{
+      type: "content";
+      content: { type: "text"; text: string };
+    }>;
+    rawInput: { plan: string };
+  } {
+    return {
+      toolCallId: `${proposal.itemId}:implement`,
+      title: "Ready to code?",
+      kind: "switch_mode",
+      content: [
+        {
+          type: "content",
+          content: { type: "text", text: proposal.text },
+        },
+      ],
+      rawInput: { plan: proposal.text },
+    };
+  }
+
+  private emitPlanProposal(
+    toolCall: ReturnType<CodexAppServerAgent["buildPlanApprovalToolCall"]>,
+    text: string,
+  ): void {
+    if (this.streamedPlanToolCallId === toolCall.toolCallId) {
+      this.emitPlanApprovalToolCall({
+        sessionUpdate: "tool_call_update",
+        toolCallId: toolCall.toolCallId,
+        status: "in_progress",
+        content: [{ type: "content", content: { type: "text", text } }],
+        rawInput: { plan: text },
+      });
+      return;
+    }
+    this.streamedPlanToolCallId = toolCall.toolCallId;
+    this.emitPlanApprovalToolCall({
+      sessionUpdate: "tool_call",
+      ...toolCall,
+      status: "in_progress",
+    });
+  }
+
+  private completePlanApprovalToolCall(
     toolCallId: string,
     status: "completed" | "failed",
-  ): Promise<void> {
-    await this.emitPlanApprovalToolCall({
+  ): void {
+    this.emitPlanApprovalToolCall({
       sessionUpdate: "tool_call_update",
       toolCallId,
       status,
     });
   }
 
-  private async emitPlanApprovalToolCall(
-    update: Record<string, unknown>,
-  ): Promise<void> {
-    try {
-      await this.client.sessionUpdate({
-        sessionId: this.sessionId,
-        update,
-      } as unknown as Parameters<AgentSideConnection["sessionUpdate"]>[0]);
-    } catch (error) {
+  private emitPlanApprovalToolCall(update: Record<string, unknown>): void {
+    const notification = {
+      sessionId: this.sessionId,
+      update,
+    } as unknown as Parameters<AgentSideConnection["sessionUpdate"]>[0];
+    this.appendNotification(this.sessionId, notification);
+    void this.client.sessionUpdate(notification).catch((error) => {
       this.logger.warn("Failed to emit plan approval tool call update", {
         error: String(error),
         sessionUpdate: update.sessionUpdate,
         toolCallId: update.toolCallId,
       });
-    }
+    });
   }
 
   /** Emit a plain agent message (user-facing status the model didn't produce). */
@@ -1414,6 +1460,12 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     )?.item;
     if (item?.type === "plan" && typeof item.text === "string" && item.text) {
       this.planProposal = { itemId: item.id ?? "codex-plan", text: item.text };
+      if (this.config.mode === "plan" && this.streamedPlanToolCallId) {
+        this.emitPlanProposal(
+          this.buildPlanApprovalToolCall(this.planProposal),
+          this.planProposal.text,
+        );
+      }
     }
   }
 
@@ -1431,6 +1483,12 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     const previousText =
       this.planProposal?.itemId === proposalId ? this.planProposal.text : "";
     this.planProposal = { itemId: proposalId, text: previousText + delta };
+    if (this.config.mode === "plan") {
+      this.emitPlanProposal(
+        this.buildPlanApprovalToolCall(this.planProposal),
+        this.planProposal.text,
+      );
+    }
   }
 
   /** Compaction started: emit `_posthog/status` so the host sets `isCompacting` (gates steer/queue). */

packages/agent/src/adapters/codex-app-server/mapping.test.ts

@@ -603,15 +603,25 @@ describe("mapHistoryItem", () => {
     ]);
   });
 
-  it("replays a persisted plan item as an agent_message_chunk", () => {
+  it("replays a persisted plan item as a historical plan tool call", () => {
     expect(
       mapHistoryItem("s-1", { type: "plan", id: "p1", text: "# The plan" }),
     ).toEqual([
       {
         sessionId: "s-1",
         update: {
-          sessionUpdate: "agent_message_chunk",
-          content: { type: "text", text: "# The plan" },
+          sessionUpdate: "tool_call",
+          toolCallId: "p1:implement",
+          title: "Plan",
+          kind: "switch_mode",
+          status: "completed",
+          content: [
+            {
+              type: "content",
+              content: { type: "text", text: "# The plan" },
+            },
+          ],
+          rawInput: { plan: "# The plan", historical: true },
         },
       },
     ]);