Skip to content
Open
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 @@ -459,7 +459,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
93 changes: 73 additions & 20 deletions src/CodexEventHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import type {
ItemCompletedNotification,
ItemStartedNotification, ThreadItem,
ModelReroutedNotification,
ReasoningSummaryPartAddedNotification,
ReasoningSummaryTextDeltaNotification,
ReasoningTextDeltaNotification,
ThreadTokenUsageUpdatedNotification,
TurnPlanUpdatedNotification,
WarningNotification
Expand Down Expand Up @@ -45,6 +48,7 @@ export class CodexEventHandler {
private readonly sessionState: SessionState;
private failure: RequestError | null = null;
private readonly activeFuzzyFileSearchSessions = new Set<string>();
private readonly seenReasoningDeltaItemIds = new Set<string>();

constructor(connection: acp.AgentSideConnection, sessionState: SessionState) {
this.connection = connection;
Expand Down Expand Up @@ -125,13 +129,13 @@ export class CodexEventHandler {
case "guardianWarning":
return this.createGuardianWarningEvent(notification.params);
case "thread/compacted":
return {
sessionUpdate: "agent_message_chunk",
content: {
type: "text",
text: "*Context compacted to fit the model's context window.*\n\n"
}
};
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 @@ -144,9 +148,6 @@ export class CodexEventHandler {
case "item/autoApprovalReview/completed":
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 @@ -242,6 +243,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 @@ -288,15 +311,10 @@ export class CodexEventHandler {
case "commandExecution":
return this.completeCommandExecutionEvent(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 "collabAgentToolCall":
case "userMessage":
case "hookPrompt":
Expand All @@ -305,11 +323,46 @@ export class CodexEventHandler {
case "imageView":
case "imageGeneration":
case "enteredReviewMode":
case "exitedReviewMode":
case "contextCompaction":
case "plan":
return null;
case "exitedReviewMode":
return this.createExitedReviewModeEvent(event.item);
case "contextCompaction":
return this.createContextCompactedEvent();
}
}

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) {
return null;
}
return {
sessionUpdate: "agent_message_chunk",
content: {
type: "text",
text,
}
};
}

private createContextCompactedEvent(): UpdateSessionEvent {
return {
sessionUpdate: "agent_message_chunk",
content: {
type: "text",
text: "*Context compacted to fit the model's context window.*\n\n"
}
};
}

private createCommandOutputDeltaEvent(event: CommandExecutionOutputDeltaNotification): UpdateSessionEvent {
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 @@ -998,22 +998,22 @@ describe('ACP server test', { timeout: 40_000 }, () => {
return { mockFixture, sessionState, turnStartSpy };
}

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 @@ -1027,7 +1027,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 @@ -1038,7 +1038,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 @@ -51,7 +51,7 @@
"excludeTmpdirEnvVar": false,
"excludeSlashTmp": false
},
"summary": null,
"summary": "auto",
"effort": "effort",
"model": "model",
"serviceTier": null
Expand Down
103 changes: 103 additions & 0 deletions src/__tests__/CodexACPAgent/reasoning-events.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
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",
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",
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