diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index d911342b..928f2b16 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1469,7 +1469,7 @@ export class CodexAcpServer { case "contextCompaction": return [createCompletedContextCompactionUpdate(item)]; case "plan": - return [this.createPlanUpdate(item)]; + return [this.createPlanMessageUpdate(item)]; } } @@ -1520,16 +1520,14 @@ export class CodexAcpServer { }; } - private createPlanUpdate( + private createPlanMessageUpdate( item: ThreadItem & { type: "plan" } ): UpdateSessionEvent { - return { - sessionUpdate: "agent_message_chunk", - content: { - type: "text", - text: `Plan:\n${item.text}`, - }, - }; + return createAgentTextMessageChunk( + item.text, + item.id, + createCodexMessagePhaseMeta("final_answer"), + ); } private userInputToContentBlocks(input: UserInput): acp.ContentBlock[] { diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 5739e44e..89e91eb5 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -19,6 +19,7 @@ import type { ItemStartedNotification, ThreadItem, ModelReroutedNotification, + PlanDeltaNotification, ReasoningSummaryPartAddedNotification, ReasoningSummaryTextDeltaNotification, ReasoningTextDeltaNotification, @@ -76,6 +77,7 @@ export class CodexEventHandler { private readonly activeGuardianApprovalReviews = new Set(); private readonly activeImageGenerationItems = new Set(); private readonly emittedImageViewItems = new Set(); + private readonly planDeltaTextByItemId = new Map(); private readonly seenReasoningDeltaItemIds = new Set(); private readonly terminalCommandIds = new Set(); private readonly terminalCommandOutputIds = new Set(); @@ -110,6 +112,8 @@ export class CodexEventHandler { switch (notification.method) { case "item/agentMessage/delta": return await this.createTextEvent(notification.params); + case "item/plan/delta": + return this.createPlanDeltaEvent(notification.params); case "item/started": return await this.createItemEvent(notification.params); case "item/completed": @@ -223,7 +227,6 @@ export class CodexEventHandler { case "rawResponseItem/completed": case "rawResponse/completed": case "thread/started": - case "item/plan/delta": case "remoteControl/status/changed": case "app/list/updated": case "thread/settings/updated": @@ -293,6 +296,15 @@ export class CodexEventHandler { return this.createAgentThoughtEvent(event.delta, event.itemId); } + private createPlanDeltaEvent(event: PlanDeltaNotification): UpdateSessionEvent | null { + if (event.delta.length === 0) { + return null; + } + const text = this.planDeltaTextByItemId.get(event.itemId) ?? ""; + this.planDeltaTextByItemId.set(event.itemId, text + event.delta); + return null; + } + private createReasoningSectionBreakEvent(event: ReasoningSummaryPartAddedNotification): UpdateSessionEvent { this.seenReasoningDeltaItemIds.add(event.itemId); return this.createAgentThoughtEvent("\n\n", event.itemId); @@ -389,6 +401,11 @@ export class CodexEventHandler { case "agentMessage": this.rememberAgentMessagePhase(event.item); return null; + case "plan": { + const deltaText = this.planDeltaTextByItemId.get(event.item.id) ?? ""; + this.planDeltaTextByItemId.delete(event.item.id); + return this.createCompletedPlanEvent(event.item, deltaText); + } case "exitedReviewMode": return this.createExitedReviewModeEvent(event.item); case "contextCompaction": @@ -404,7 +421,6 @@ export class CodexEventHandler { case "userMessage": case "hookPrompt": case "enteredReviewMode": - case "plan": return null; } @@ -423,6 +439,25 @@ export class CodexEventHandler { return this.createAgentThoughtEvent(text, item.id); } + private createCompletedPlanEvent( + item: ThreadItem & { type: "plan" }, + deltaText: string, + ): UpdateSessionEvent | null { + const text = item.text.length > 0 ? item.text : deltaText; + if (text.length === 0) { + return null; + } + return this.createPlanTextEvent(text, item.id); + } + + private createPlanTextEvent(text: string, messageId: string): UpdateSessionEvent { + return createAgentTextMessageChunk( + text, + messageId, + createCodexMessagePhaseMeta("final_answer"), + ); + } + private createExitedReviewModeEvent(item: ThreadItem & { type: "exitedReviewMode" }): UpdateSessionEvent | null { const text = item.review.trim(); if (text.length === 0) { diff --git a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json index b314d869..a7420a6c 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json +++ b/src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json @@ -151,9 +151,15 @@ "sessionId": "session-legacy", "update": { "sessionUpdate": "agent_message_chunk", + "messageId": "item-plan-1", "content": { "type": "text", - "text": "Plan:\nInspect project files" + "text": "Inspect project files" + }, + "_meta": { + "codex": { + "phase": "final_answer" + } } } } diff --git a/src/__tests__/CodexACPAgent/data/plan-checklist-update.json b/src/__tests__/CodexACPAgent/data/plan-checklist-update.json new file mode 100644 index 00000000..ab0367f1 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/plan-checklist-update.json @@ -0,0 +1,23 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "plan", + "entries": [ + { + "status": "completed", + "content": "Add the event mapping", + "priority": "medium" + }, + { + "status": "in_progress", + "content": "Verify it in Zed", + "priority": "medium" + } + ] + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/plan-completed-fallback.json b/src/__tests__/CodexACPAgent/data/plan-completed-fallback.json new file mode 100644 index 00000000..4642c754 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/plan-completed-fallback.json @@ -0,0 +1,21 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_message_chunk", + "messageId": "plan-2", + "content": { + "type": "text", + "text": "### Fallback plan\n\n1. Use the completed item." + }, + "_meta": { + "codex": { + "phase": "final_answer" + } + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/plan-delta-fallback.json b/src/__tests__/CodexACPAgent/data/plan-delta-fallback.json new file mode 100644 index 00000000..87a37988 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/plan-delta-fallback.json @@ -0,0 +1,21 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_message_chunk", + "messageId": "plan-2", + "content": { + "type": "text", + "text": "### Buffered plan\n\n1. Use the buffered fallback." + }, + "_meta": { + "codex": { + "phase": "final_answer" + } + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/plan-deltas.json b/src/__tests__/CodexACPAgent/data/plan-deltas.json new file mode 100644 index 00000000..e2036870 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/plan-deltas.json @@ -0,0 +1,21 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_message_chunk", + "messageId": "plan-1", + "content": { + "type": "text", + "text": "Completed text should not duplicate the streamed plan." + }, + "_meta": { + "codex": { + "phase": "final_answer" + } + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/plan-events.test.ts b/src/__tests__/CodexACPAgent/plan-events.test.ts new file mode 100644 index 00000000..67ca0a92 --- /dev/null +++ b/src/__tests__/CodexACPAgent/plan-events.test.ts @@ -0,0 +1,176 @@ +import {beforeEach, describe, expect, it, vi} from "vitest"; +import type {ServerNotification} from "../../app-server"; +import {AgentMode} from "../../AgentMode"; +import type {SessionState} from "../../CodexAcpServer"; +import { + createCodexMockTestFixture, + createTestSessionState, + setupPromptAndSendNotifications, + type CodexMockTestFixture, +} from "../acp-test-utils"; + +describe("CodexEventHandler - plan events", () => { + let mockFixture: CodexMockTestFixture; + const sessionId = "test-session-id"; + + beforeEach(() => { + mockFixture = createCodexMockTestFixture(); + vi.clearAllMocks(); + }); + + const sessionState: SessionState = createTestSessionState({ + sessionId, + currentModelId: "model-id[effort]", + agentMode: AgentMode.DEFAULT_AGENT_MODE, + }); + + it("emits the authoritative completed plan after buffering deltas", async () => { + const notifications: ServerNotification[] = [ + { + method: "item/started", + params: { + threadId: sessionId, + turnId: "turn-1", + startedAtMs: 0, + item: { + type: "plan", + id: "plan-1", + text: "", + }, + }, + }, + { + method: "item/plan/delta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "plan-1", + delta: "### Implementation plan\n\n", + }, + }, + { + method: "item/plan/delta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "plan-1", + delta: "1. Add the event mapping.\n2. Verify it.", + }, + }, + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-1", + completedAtMs: 0, + item: { + type: "plan", + id: "plan-1", + text: "Completed text should not duplicate the streamed plan.", + }, + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/plan-deltas.json", + ); + }); + + it("falls back to buffered deltas when the completed plan is empty", async () => { + const notifications: ServerNotification[] = [ + { + method: "item/plan/delta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "plan-2", + delta: "### Buffered plan\n\n", + }, + }, + { + method: "item/plan/delta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "plan-2", + delta: "1. Use the buffered fallback.", + }, + }, + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-1", + completedAtMs: 0, + item: { + type: "plan", + id: "plan-2", + text: "", + }, + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/plan-delta-fallback.json", + ); + }); + + it("emits the completed plan when no deltas streamed", async () => { + const notifications: ServerNotification[] = [ + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-1", + completedAtMs: 0, + item: { + type: "plan", + id: "plan-2", + text: "### Fallback plan\n\n1. Use the completed item.", + }, + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/plan-completed-fallback.json", + ); + }); + + it("keeps turn plan updates as ACP checklist updates", async () => { + const notifications: ServerNotification[] = [ + { + method: "turn/plan/updated", + params: { + threadId: sessionId, + turnId: "turn-1", + explanation: "Implement and verify the mapping.", + plan: [ + { + step: "Add the event mapping", + status: "completed", + }, + { + step: "Verify it in Zed", + status: "inProgress", + }, + ], + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/plan-checklist-update.json", + ); + }); +});