diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 550fc9c5..13e107a9 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -480,7 +480,7 @@ export class CodexAcpClient { input: input, approvalPolicy: agentMode.approvalPolicy, sandboxPolicy: agentMode.sandboxPolicy, - summary: disableSummary ? "none" : null, + summary: disableSummary ? "none" : "auto", effort: effort, model: modelId.model, serviceTier: serviceTier, diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index ab1615fd..06305317 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -20,6 +20,9 @@ import type { ItemStartedNotification, ThreadItem, ModelReroutedNotification, + ReasoningSummaryPartAddedNotification, + ReasoningSummaryTextDeltaNotification, + ReasoningTextDeltaNotification, ThreadGoalClearedNotification, ThreadGoalUpdatedNotification, ThreadTokenUsageUpdatedNotification, @@ -60,6 +63,7 @@ export class CodexEventHandler { private readonly activeGuardianApprovalReviews = new Set(); private readonly activeImageGenerationItems = new Set(); private readonly emittedImageViewItems = new Set(); + private readonly seenReasoningDeltaItemIds = new Set(); constructor(connection: acp.AgentSideConnection, sessionState: SessionState) { this.connection = connection; @@ -145,6 +149,12 @@ export class CodexEventHandler { return this.handleGuardianApprovalReviewCompleted(notification.params); case "thread/compacted": return this.createContextCompactedEvent(); + case "item/reasoning/summaryTextDelta": + return this.createReasoningDeltaEvent(notification.params); + case "item/reasoning/textDelta": + return this.createReasoningDeltaEvent(notification.params); + case "item/reasoning/summaryPartAdded": + return this.createReasoningSectionBreakEvent(notification.params); case "model/rerouted": return this.createModelReroutedEvent(notification.params); case "fuzzyFileSearch/sessionUpdated": @@ -159,9 +169,6 @@ export class CodexEventHandler { case "command/exec/outputDelta": case "hook/started": case "hook/completed": - case "item/reasoning/summaryTextDelta": - case "item/reasoning/summaryPartAdded": - case "item/reasoning/textDelta": case "turn/diff/updated": case "item/commandExecution/terminalInteraction": case "item/fileChange/outputDelta": @@ -290,6 +297,28 @@ export class CodexEventHandler { }; } + private createReasoningDeltaEvent( + event: ReasoningSummaryTextDeltaNotification | ReasoningTextDeltaNotification + ): UpdateSessionEvent { + this.seenReasoningDeltaItemIds.add(event.itemId); + return this.createAgentThoughtEvent(event.delta); + } + + private createReasoningSectionBreakEvent(event: ReasoningSummaryPartAddedNotification): UpdateSessionEvent { + this.seenReasoningDeltaItemIds.add(event.itemId); + return this.createAgentThoughtEvent("\n\n"); + } + + private createAgentThoughtEvent(text: string): UpdateSessionEvent { + return { + sessionUpdate: "agent_thought_chunk", + content: { + type: "text", + text, + } + }; + } + private async createItemEvent(event: ItemStartedNotification): Promise { switch (event.item.type) { case "fileChange": @@ -351,15 +380,10 @@ export class CodexEventHandler { } return createImageGenerationUpdate(event.item); case "reasoning": - const summary = event.item.summary[0]; - if (!summary) return null; - return { - sessionUpdate: "agent_thought_chunk", - content: { - type: "text", - text: summary - } + if (this.seenReasoningDeltaItemIds.delete(event.item.id)) { + return null; } + return this.createCompletedReasoningEvent(event.item); case "webSearch": return createWebSearchCompleteUpdate(event.item); case "collabAgentToolCall": @@ -376,6 +400,15 @@ export class CodexEventHandler { } } + private createCompletedReasoningEvent(item: ThreadItem & { type: "reasoning" }): UpdateSessionEvent | null { + const parts = item.summary.length > 0 ? item.summary : item.content; + const text = parts.filter(part => part.length > 0).join("\n\n"); + if (text.length === 0) { + return null; + } + return this.createAgentThoughtEvent(text); + } + private createExitedReviewModeEvent(item: ThreadItem & { type: "exitedReviewMode" }): UpdateSessionEvent | null { const text = item.review.trim(); if (text.length === 0) { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 485fdc13..e7d14c0e 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1179,7 +1179,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { }; } - it ('should disable resasoning.summary if key authorization is used', async () => { + it ('should disable reasoning.summary if key authorization is used', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "apiKey" } }); await mockFixture.getCodexAcpAgent().prompt({ sessionId: "id", prompt: [{ type: "text", text: "test" }] }); @@ -1187,14 +1187,14 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "none" })); }); - it ('should not disable resasoning.summary by default', async () => { + it ('should enable reasoning.summary by default', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "chatgpt", email: "test@example.com", planType: "pro" }, }); await mockFixture.getCodexAcpAgent().prompt({ sessionId: "id", prompt: [{ type: "text", text: "test" }] }); - expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: null })); + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "auto" })); }); it ('should disable reasoning.summary when model lacks reasoning', async () => { @@ -1208,7 +1208,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "none" })); }); - it ('should not disable reasoning.summary when model supports reasoning', async () => { + it ('should enable reasoning.summary when model supports reasoning', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "chatgpt", email: "test@example.com", planType: "pro" }, supportedReasoningEfforts: [ @@ -1219,7 +1219,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { await mockFixture.getCodexAcpAgent().prompt({ sessionId: "id", prompt: [{ type: "text", text: "test" }] }); - expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: null })); + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "auto" })); }); it ('should reject prompt with images when model does not support image input', async () => { diff --git a/src/__tests__/CodexACPAgent/data/reasoning-completed-parts.json b/src/__tests__/CodexACPAgent/data/reasoning-completed-parts.json new file mode 100644 index 00000000..1a8241ec --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/reasoning-completed-parts.json @@ -0,0 +1,15 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_thought_chunk", + "content": { + "type": "text", + "text": "First summary\n\nSecond summary" + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/reasoning-deltas-and-section-break.json b/src/__tests__/CodexACPAgent/data/reasoning-deltas-and-section-break.json new file mode 100644 index 00000000..43ff8453 --- /dev/null +++ b/src/__tests__/CodexACPAgent/data/reasoning-deltas-and-section-break.json @@ -0,0 +1,45 @@ +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_thought_chunk", + "content": { + "type": "text", + "text": "First thought" + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_thought_chunk", + "content": { + "type": "text", + "text": "\n\n" + } + } + } + ] +} +{ + "method": "sessionUpdate", + "args": [ + { + "sessionId": "test-session-id", + "update": { + "sessionUpdate": "agent_thought_chunk", + "content": { + "type": "text", + "text": "Raw reasoning detail" + } + } + } + ] +} \ No newline at end of file diff --git a/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json b/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json index 4ddf4cba..47394626 100644 --- a/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json +++ b/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json @@ -45,7 +45,7 @@ "excludeTmpdirEnvVar": false, "excludeSlashTmp": false }, - "summary": null, + "summary": "auto", "effort": "effort", "model": "model", "serviceTier": null diff --git a/src/__tests__/CodexACPAgent/reasoning-events.test.ts b/src/__tests__/CodexACPAgent/reasoning-events.test.ts new file mode 100644 index 00000000..3d9ef367 --- /dev/null +++ b/src/__tests__/CodexACPAgent/reasoning-events.test.ts @@ -0,0 +1,105 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { ServerNotification } from "../../app-server"; +import type { SessionState } from "../../CodexAcpServer"; +import { AgentMode } from "../../AgentMode"; +import { + createCodexMockTestFixture, + createTestSessionState, + setupPromptAndSendNotifications, + type CodexMockTestFixture +} from "../acp-test-utils"; + +describe("CodexEventHandler - reasoning 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("streams reasoning deltas and section breaks without duplicating the completed item", async () => { + const notifications: ServerNotification[] = [ + { + method: "item/reasoning/summaryTextDelta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "reasoning-1", + summaryIndex: 0, + delta: "First thought", + }, + }, + { + method: "item/reasoning/summaryPartAdded", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "reasoning-1", + summaryIndex: 1, + }, + }, + { + method: "item/reasoning/textDelta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "reasoning-1", + contentIndex: 0, + delta: "Raw reasoning detail", + }, + }, + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-1", + completedAtMs: 0, + item: { + type: "reasoning", + id: "reasoning-1", + summary: ["Completed summary should not duplicate"], + content: ["Completed content should not duplicate"], + }, + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/reasoning-deltas-and-section-break.json" + ); + }); + + it("emits all completed reasoning parts when no deltas streamed", async () => { + const notifications: ServerNotification[] = [ + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-1", + completedAtMs: 0, + item: { + type: "reasoning", + id: "reasoning-2", + summary: ["First summary", "Second summary"], + content: ["Raw content fallback"], + }, + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( + "data/reasoning-completed-parts.json" + ); + }); +});