fix(codex): render plans in approval UI

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

@@ -2757,7 +2757,10 @@ describe("CodexAppServerAgent", () => {
   }
 
   // permissionOutcome may be a value or a function (per-call, e.g. a pending promise).
-  function makePlanAgent(permissionOutcome: unknown) {
+  function makePlanAgent(
+    permissionOutcome: unknown,
+    options: { rejectPlanToolUpdates?: boolean } = {},
+  ) {
     const stub = makeStubRpc({
       "thread/start": { thread: { id: "t" } },
       "turn/start": { turn: { id: "turn_1" } },
@@ -2770,7 +2773,15 @@ describe("CodexAppServerAgent", () => {
     }> = [];
     const client = {
       sessionUpdate: async (n: unknown) => {
-        sessionUpdates.push(n as { update?: Record<string, unknown> });
+        const notification = n as { update?: Record<string, unknown> };
+        if (
+          options.rejectPlanToolUpdates &&
+          (notification.update?.sessionUpdate === "tool_call" ||
+            notification.update?.sessionUpdate === "tool_call_update")
+        ) {
+          throw new Error("renderer disconnected");
+        }
+        sessionUpdates.push(notification);
       },
       requestPermission: async (params: {
         toolCall: Record<string, unknown>;
@@ -2898,6 +2909,42 @@ describe("CodexAppServerAgent", () => {
     expect(permissionRequests[0].options.map((o) => o.optionId)).toContain(
       "auto",
     );
+    expect(sessionUpdates).toContainEqual({
+      sessionId: "t",
+      update: {
+        sessionUpdate: "tool_call",
+        toolCallId: "p1:implement",
+        title: "Ready to code?",
+        kind: "switch_mode",
+        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({
+      sessionId: "t",
+      update: {
+        sessionUpdate: "tool_call_update",
+        toolCallId: "p1:implement",
+        status: "completed",
+      },
+    });
+    expect(
+      sessionUpdates.some((notification) => {
+        if (notification.update?.sessionUpdate !== "agent_message_chunk") {
+          return false;
+        }
+        const content = notification.update.content as
+          | { type?: string; text?: string }
+          | undefined;
+        return content?.text?.includes("# The plan") === true;
+      }),
+    ).toBe(false);
 
     // Mode flipped to auto and the host was told.
     expect(sessionUpdates).toContainEqual(
@@ -2924,7 +2971,7 @@ describe("CodexAppServerAgent", () => {
   });
 
   it("stays in plan mode when the handoff is rejected without feedback", async () => {
-    const { agent, stub, permissionRequests } = makePlanAgent({
+    const { agent, stub, sessionUpdates, permissionRequests } = makePlanAgent({
       outcome: { outcome: "selected", optionId: "reject_with_feedback" },
     });
     const { done } = await startPlanTurn(agent, stub);
@@ -2938,12 +2985,43 @@ describe("CodexAppServerAgent", () => {
 
     expect((await done).stopReason).toBe("end_turn");
     expect(permissionRequests).toHaveLength(1);
+    expect(sessionUpdates).toContainEqual({
+      sessionId: "t",
+      update: {
+        sessionUpdate: "tool_call_update",
+        toolCallId: "p1:implement",
+        status: "failed",
+      },
+    });
     // No implementation turn started; the picker stays on plan.
     expect(stub.requests.filter((r) => r.method === "turn/start")).toHaveLength(
       1,
     );
   });
 
+  it("settles the handoff when plan tool-call updates cannot be delivered", async () => {
+    const { agent, stub, permissionRequests } = makePlanAgent(
+      {
+        outcome: { outcome: "selected", optionId: "reject_with_feedback" },
+      },
+      { rejectPlanToolUpdates: true },
+    );
+    const { done } = await startPlanTurn(agent, stub);
+
+    stub.emit("item/completed", {
+      item: { type: "plan", id: "p1", text: "# 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

@@ -1011,6 +1011,11 @@ export class CodexAppServerAgent extends BaseAcpAgent {
       ],
       rawInput: { plan: proposal.text },
     };
+    await this.emitPlanApprovalToolCall({
+      sessionUpdate: "tool_call",
+      ...toolCall,
+      status: "in_progress",
+    });
     const options = [
       {
         optionId: "auto",
@@ -1053,8 +1058,12 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     });
     const settled = await Promise.race([permission, cancelled]);
     this.planHandoffCancel = undefined;
-    if (!settled) return { kind: "stay" };
+    if (!settled) {
+      await this.completePlanApprovalToolCall(toolCallId, "failed");
+      return { kind: "stay" };
+    }
     if (settled.failed) {
+      await this.completePlanApprovalToolCall(toolCallId, "failed");
       this.logger.warn("plan implementation prompt failed; staying in plan", {
         error: String(settled.err),
       });
@@ -1066,25 +1075,63 @@ export class CodexAppServerAgent extends BaseAcpAgent {
     }
     const response = settled.res;
     if (this.session.cancelled || response.outcome.outcome !== "selected") {
+      await this.completePlanApprovalToolCall(toolCallId, "failed");
       return { kind: "stay" };
     }
     const optionId = response.outcome.optionId;
-    if (!offered.has(optionId)) return { kind: "stay" };
-    if (optionId === "auto") return { kind: "implement", mode: "auto" };
+    if (!offered.has(optionId)) {
+      await this.completePlanApprovalToolCall(toolCallId, "failed");
+      return { kind: "stay" };
+    }
+    if (optionId === "auto") {
+      await this.completePlanApprovalToolCall(toolCallId, "completed");
+      return { kind: "implement", mode: "auto" };
+    }
     // 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" };
     }
     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");
         return { kind: "feedback", feedback: feedback.trim() };
       }
     }
+    await this.completePlanApprovalToolCall(toolCallId, "failed");
     return { kind: "stay" };
   }
 
+  private async completePlanApprovalToolCall(
+    toolCallId: string,
+    status: "completed" | "failed",
+  ): Promise<void> {
+    await 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) {
+      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). */
   private broadcastAgentText(text: string): void {
     if (!this.sessionId) return;

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

@@ -42,20 +42,14 @@ describe("mapAppServerNotification", () => {
     });
   });
 
-  it("streams a plan delta as an ACP agent_message_chunk", () => {
+  it("keeps plan deltas out of the agent transcript", () => {
     const result = mapAppServerNotification(
       "s-1",
       APP_SERVER_NOTIFICATIONS.PLAN_DELTA,
       { itemId: "p1", delta: "## Plan\n" },
     );
 
-    expect(result).toEqual({
-      sessionId: "s-1",
-      update: {
-        sessionUpdate: "agent_message_chunk",
-        content: { type: "text", text: "## Plan\n" },
-      },
-    });
+    expect(result).toBeNull();
   });
 
   it("returns null when the delta is missing or empty", () => {

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

@@ -41,18 +41,9 @@ export function mapAppServerNotification(
         },
       };
     }
-    // Plan-mode proposal streaming as agent prose (codex strips it from agentMessage deltas).
-    case APP_SERVER_NOTIFICATIONS.PLAN_DELTA: {
-      const delta = readStringField(params, "delta");
-      if (!delta) return null;
-      return {
-        sessionId,
-        update: {
-          sessionUpdate: "agent_message_chunk",
-          content: { type: "text", text: delta },
-        },
-      };
-    }
+    // Plan deltas are buffered by the adapter for the structured approval UI.
+    case APP_SERVER_NOTIFICATIONS.PLAN_DELTA:
+      return null;
     case APP_SERVER_NOTIFICATIONS.TOKEN_USAGE_UPDATED: {
       // Context indicator: renderer reads `used`/`size`; detailed breakdown comes via `_posthog/usage_update`.
       const usage = readTokenUsage(params);

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

@@ -32,8 +32,8 @@ export const APP_SERVER_NOTIFICATIONS = {
   REASONING_TEXT_DELTA: "item/reasoning/textDelta",
   // Default reasoning stream for gpt-5 models; raw textDelta is off by default, so without this the host sees no reasoning.
   REASONING_SUMMARY_TEXT_DELTA: "item/reasoning/summaryTextDelta",
-  // Plan-mode <proposed_plan> stream. codex strips the plan from agentMessage deltas,
-  // so without this the host sees nothing while the plan is written.
+  // Plan-mode <proposed_plan> stream. The adapter buffers it for the structured
+  // plan approval UI because codex strips it from agentMessage deltas.
   PLAN_DELTA: "item/plan/delta",
   TURN_PLAN_UPDATED: "turn/plan/updated",
   TURN_COMPLETED: "turn/completed",

packages/ui/src/features/sessions/components/session-update/PlanApprovalView.test.tsx

@@ -84,6 +84,15 @@ describe("PlanApprovalView", () => {
     expect(screen.getByText(PLAN_MARKER)).toBeInTheDocument();
   });
 
+  it("shows the rejected status when the plan tool call fails", () => {
+    renderView({ toolCall: makeToolCall({ status: "failed" }) });
+
+    expect(screen.getByText(/\(plan rejected\)/i)).toBeInTheDocument();
+    expect(
+      screen.getByRole("button", { name: /show plan/i }),
+    ).toBeInTheDocument();
+  });
+
   it("omits the toggle when there is no plan text available", () => {
     renderView({
       toolCall: makeToolCall({

packages/ui/src/features/sessions/components/session-update/PlanApprovalView.tsx

@@ -16,7 +16,7 @@ export function PlanApprovalView({
   turnComplete,
 }: ToolViewProps) {
   const { content } = toolCall;
-  const { isComplete, wasCancelled } = useToolCallStatus(
+  const { isComplete, isFailed, wasCancelled } = useToolCallStatus(
     toolCall.status,
     turnCancelled,
     turnComplete,
@@ -45,7 +45,8 @@ export function PlanApprovalView({
     return null;
   }, [content, toolCall.rawInput]);
 
-  const showResult = isComplete || wasCancelled;
+  const wasRejected = isFailed || wasCancelled;
+  const showResult = isComplete || wasRejected;
   const canTogglePlan = showResult && !!planText;
   const planContentId = `plan-content-${toolCall.toolCallId}`;
 
@@ -58,7 +59,7 @@ export function PlanApprovalView({
         Plan approved — proceeding with implementation
       </Text>
     </>
-  ) : wasCancelled ? (
+  ) : wasRejected ? (
     <Text className="text-[13px] text-gray-10">(Plan rejected)</Text>
   ) : null;