Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CodexAcpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
55 changes: 44 additions & 11 deletions src/CodexEventHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import type {
ItemStartedNotification,
ThreadItem,
ModelReroutedNotification,
ReasoningSummaryPartAddedNotification,
ReasoningSummaryTextDeltaNotification,
ReasoningTextDeltaNotification,
ThreadGoalClearedNotification,
ThreadGoalUpdatedNotification,
ThreadTokenUsageUpdatedNotification,
Expand Down Expand Up @@ -60,6 +63,7 @@ export class CodexEventHandler {
private readonly activeGuardianApprovalReviews = new Set<string>();
private readonly activeImageGenerationItems = new Set<string>();
private readonly emittedImageViewItems = new Set<string>();
private readonly seenReasoningDeltaItemIds = new Set<string>();

constructor(connection: acp.AgentSideConnection, sessionState: SessionState) {
this.connection = connection;
Expand Down Expand Up @@ -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":
Expand All @@ -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":
Expand Down Expand Up @@ -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<UpdateSessionEvent | null> {
switch (event.item.type) {
case "fileChange":
Expand Down Expand Up @@ -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":
Expand All @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions src/__tests__/CodexACPAgent/CodexAcpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1179,22 +1179,22 @@ 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" }] });

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 () => {
Expand All @@ -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: [
Expand All @@ -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 () => {
Expand Down
15 changes: 15 additions & 0 deletions src/__tests__/CodexACPAgent/data/reasoning-completed-parts.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"excludeTmpdirEnvVar": false,
"excludeSlashTmp": false
},
"summary": null,
"summary": "auto",
"effort": "effort",
"model": "model",
"serviceTier": null
Expand Down
105 changes: 105 additions & 0 deletions src/__tests__/CodexACPAgent/reasoning-events.test.ts
Original file line number Diff line number Diff line change
@@ -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"
);
});
});
Loading