Skip to content

Commit 551f9c3

Browse files
authored
fix(codex): render plans in approval UI (#3524)
1 parent cd322c7 commit 551f9c3

7 files changed

Lines changed: 457 additions & 79 deletions

File tree

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

Lines changed: 182 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2783,7 +2783,13 @@ describe("CodexAppServerAgent", () => {
27832783
}
27842784

27852785
// permissionOutcome may be a value or a function (per-call, e.g. a pending promise).
2786-
function makePlanAgent(permissionOutcome: unknown) {
2786+
function makePlanAgent(
2787+
permissionOutcome: unknown,
2788+
options: {
2789+
rejectPlanToolUpdates?: boolean;
2790+
stallPlanToolUpdates?: boolean;
2791+
} = {},
2792+
) {
27872793
const stub = makeStubRpc({
27882794
"thread/start": { thread: { id: "t" } },
27892795
"turn/start": { turn: { id: "turn_1" } },
@@ -2796,7 +2802,19 @@ describe("CodexAppServerAgent", () => {
27962802
}> = [];
27972803
const client = {
27982804
sessionUpdate: async (n: unknown) => {
2799-
sessionUpdates.push(n as { update?: Record<string, unknown> });
2805+
const notification = n as { update?: Record<string, unknown> };
2806+
if (
2807+
notification.update?.sessionUpdate === "tool_call" ||
2808+
notification.update?.sessionUpdate === "tool_call_update"
2809+
) {
2810+
if (options.rejectPlanToolUpdates) {
2811+
throw new Error("renderer disconnected");
2812+
}
2813+
if (options.stallPlanToolUpdates) {
2814+
return new Promise(() => {});
2815+
}
2816+
}
2817+
sessionUpdates.push(notification);
28002818
},
28012819
requestPermission: async (params: {
28022820
toolCall: Record<string, unknown>;
@@ -2924,6 +2942,57 @@ describe("CodexAppServerAgent", () => {
29242942
expect(permissionRequests[0].options.map((o) => o.optionId)).toContain(
29252943
"auto",
29262944
);
2945+
expect(sessionUpdates).toContainEqual({
2946+
sessionId: "t",
2947+
update: {
2948+
sessionUpdate: "tool_call",
2949+
toolCallId: "p1:implement",
2950+
title: "Ready to code?",
2951+
kind: "switch_mode",
2952+
content: [
2953+
{
2954+
type: "content",
2955+
content: { type: "text", text: "# The plan\n\n" },
2956+
},
2957+
],
2958+
rawInput: { plan: "# The plan\n\n" },
2959+
status: "in_progress",
2960+
},
2961+
});
2962+
expect(sessionUpdates).toContainEqual({
2963+
sessionId: "t",
2964+
update: {
2965+
sessionUpdate: "tool_call_update",
2966+
toolCallId: "p1:implement",
2967+
status: "in_progress",
2968+
content: [
2969+
{
2970+
type: "content",
2971+
content: { type: "text", text: "# The plan\n\n1. do it" },
2972+
},
2973+
],
2974+
rawInput: { plan: "# The plan\n\n1. do it" },
2975+
},
2976+
});
2977+
expect(sessionUpdates).toContainEqual({
2978+
sessionId: "t",
2979+
update: {
2980+
sessionUpdate: "tool_call_update",
2981+
toolCallId: "p1:implement",
2982+
status: "completed",
2983+
},
2984+
});
2985+
expect(
2986+
sessionUpdates.some((notification) => {
2987+
if (notification.update?.sessionUpdate !== "agent_message_chunk") {
2988+
return false;
2989+
}
2990+
const content = notification.update.content as
2991+
| { type?: string; text?: string }
2992+
| undefined;
2993+
return content?.text?.includes("# The plan") === true;
2994+
}),
2995+
).toBe(false);
29272996

29282997
// Mode flipped to auto and the host was told.
29292998
expect(sessionUpdates).toContainEqual(
@@ -2949,8 +3018,66 @@ describe("CodexAppServerAgent", () => {
29493018
});
29503019
});
29513020

3021+
it("coalesces streamed plan snapshots in notification history", async () => {
3022+
const { agent, stub } = makePlanAgent({
3023+
outcome: { outcome: "selected", optionId: "reject_with_feedback" },
3024+
});
3025+
const { done } = await startPlanTurn(agent, stub);
3026+
3027+
stub.emit("item/plan/delta", { itemId: "p1", delta: "first" });
3028+
stub.emit("item/plan/delta", { itemId: "p1", delta: " second" });
3029+
stub.emit("item/plan/delta", { itemId: "p1", delta: " third" });
3030+
stub.emit("turn/completed", {
3031+
turn: { id: "turn_1", status: "completed" },
3032+
});
3033+
await done;
3034+
3035+
const session = (
3036+
agent as unknown as {
3037+
session: {
3038+
notificationHistory: Array<{ update?: Record<string, unknown> }>;
3039+
};
3040+
}
3041+
).session;
3042+
const streamedUpdates = session.notificationHistory.filter(
3043+
(notification) =>
3044+
notification.update?.sessionUpdate === "tool_call_update" &&
3045+
notification.update.status === "in_progress",
3046+
);
3047+
expect(streamedUpdates).toHaveLength(1);
3048+
expect(streamedUpdates[0].update?.rawInput).toEqual({
3049+
plan: "first second third",
3050+
});
3051+
});
3052+
3053+
it("caps streamed plans before rendering or storing them", async () => {
3054+
const { agent, stub, permissionRequests, sessionUpdates } = makePlanAgent({
3055+
outcome: { outcome: "selected", optionId: "reject_with_feedback" },
3056+
});
3057+
const { done } = await startPlanTurn(agent, stub);
3058+
3059+
stub.emit("item/plan/delta", { itemId: "p1", delta: "a".repeat(75_000) });
3060+
stub.emit("item/plan/delta", { itemId: "p1", delta: "b".repeat(75_000) });
3061+
stub.emit("turn/completed", {
3062+
turn: { id: "turn_1", status: "completed" },
3063+
});
3064+
await done;
3065+
3066+
const renderedPlans = sessionUpdates.flatMap((notification) => {
3067+
const rawInput = notification.update?.rawInput as
3068+
| { plan?: unknown }
3069+
| undefined;
3070+
return typeof rawInput?.plan === "string" ? [rawInput.plan] : [];
3071+
});
3072+
const approvalPlan = permissionRequests[0].toolCall.rawInput as {
3073+
plan: string;
3074+
};
3075+
expect(renderedPlans.every((plan) => plan.length <= 100_000)).toBe(true);
3076+
expect(approvalPlan.plan).toHaveLength(100_000);
3077+
});
3078+
29523079
it("stays in plan mode when the handoff is rejected without feedback", async () => {
2953-
const { agent, stub, permissionRequests } = makePlanAgent({
3080+
const { agent, stub, sessionUpdates, permissionRequests } = makePlanAgent({
29543081
outcome: { outcome: "selected", optionId: "reject_with_feedback" },
29553082
});
29563083
const { done } = await startPlanTurn(agent, stub);
@@ -2964,12 +3091,64 @@ describe("CodexAppServerAgent", () => {
29643091

29653092
expect((await done).stopReason).toBe("end_turn");
29663093
expect(permissionRequests).toHaveLength(1);
3094+
expect(sessionUpdates).toContainEqual({
3095+
sessionId: "t",
3096+
update: {
3097+
sessionUpdate: "tool_call_update",
3098+
toolCallId: "p1:implement",
3099+
status: "failed",
3100+
},
3101+
});
29673102
// No implementation turn started; the picker stays on plan.
29683103
expect(stub.requests.filter((r) => r.method === "turn/start")).toHaveLength(
29693104
1,
29703105
);
29713106
});
29723107

3108+
it("settles the handoff when plan tool-call updates cannot be delivered", async () => {
3109+
const { agent, stub, permissionRequests } = makePlanAgent(
3110+
{
3111+
outcome: { outcome: "selected", optionId: "reject_with_feedback" },
3112+
},
3113+
{ rejectPlanToolUpdates: true },
3114+
);
3115+
const { done } = await startPlanTurn(agent, stub);
3116+
3117+
stub.emit("item/completed", {
3118+
item: { type: "plan", id: "p1", text: "# The plan" },
3119+
});
3120+
stub.emit("turn/completed", {
3121+
turn: { id: "turn_1", status: "completed" },
3122+
});
3123+
3124+
expect((await done).stopReason).toBe("end_turn");
3125+
expect(permissionRequests).toHaveLength(1);
3126+
expect(stub.requests.filter((r) => r.method === "turn/start")).toHaveLength(
3127+
1,
3128+
);
3129+
});
3130+
3131+
it("does not block the handoff when plan tool-call updates never settle", async () => {
3132+
const { agent, stub, permissionRequests } = makePlanAgent(
3133+
{
3134+
outcome: { outcome: "selected", optionId: "reject_with_feedback" },
3135+
},
3136+
{ stallPlanToolUpdates: true },
3137+
);
3138+
const { done } = await startPlanTurn(agent, stub);
3139+
3140+
stub.emit("item/plan/delta", { itemId: "p1", delta: "# The plan" });
3141+
stub.emit("turn/completed", {
3142+
turn: { id: "turn_1", status: "completed" },
3143+
});
3144+
3145+
expect((await done).stopReason).toBe("end_turn");
3146+
expect(permissionRequests).toHaveLength(1);
3147+
expect(stub.requests.filter((r) => r.method === "turn/start")).toHaveLength(
3148+
1,
3149+
);
3150+
});
3151+
29733152
it("feeds handoff feedback into another plan turn", async () => {
29743153
const { agent, stub, permissionRequests } = makePlanAgent({
29753154
outcome: { outcome: "selected", optionId: "reject_with_feedback" },

0 commit comments

Comments
 (0)