From 82460348e6378acea8f19fd2b1620d46861fe371 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 11:52:57 +0200 Subject: [PATCH 01/18] feat: Add /goal support --- src/CodexAcpClient.ts | 20 +++++ src/CodexAppServerClient.ts | 12 +++ src/CodexCommands.ts | 43 ++++++++++ .../CodexACPAgent/CodexAcpClient.test.ts | 78 ++++++++++++++++++- .../data/available-commands-build-in.json | 7 ++ .../data/available-commands-skills.json | 7 ++ .../data/load-session-history.json | 7 ++ ...ession-response-item-history-fallback.json | 7 ++ 8 files changed, 180 insertions(+), 1 deletion(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index f8be1d59..e6219870 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -33,6 +33,7 @@ import type { SkillsListResponse, SandboxPolicy, Thread, + ThreadGoalStatus, ThreadSourceKind, TurnCompletedNotification, UserInput, @@ -322,6 +323,25 @@ export class CodexAcpClient { await this.codexClient.runCompact({threadId: sessionId}); } + async setGoal(sessionId: string, objective: string): Promise { + await this.codexClient.threadGoalSet({ + threadId: sessionId, + objective, + status: "active", + }); + } + + async setGoalStatus(sessionId: string, status: ThreadGoalStatus): Promise { + await this.codexClient.threadGoalSet({ + threadId: sessionId, + status, + }); + } + + async clearGoal(sessionId: string): Promise { + await this.codexClient.threadGoalClear({threadId: sessionId}); + } + async awaitMcpServerStartup(serverNames: Array, afterVersion: number): Promise { return await this.codexClient.awaitMcpServerStartup(serverNames, afterVersion); } diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 969e161d..711bb8d9 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -30,6 +30,10 @@ import type { ThreadArchiveResponse, ThreadCompactStartParams, ThreadCompactStartResponse, + ThreadGoalClearParams, + ThreadGoalClearResponse, + ThreadGoalSetParams, + ThreadGoalSetResponse, ThreadLoadedListParams, ThreadLoadedListResponse, ThreadListParams, @@ -314,6 +318,14 @@ export class CodexAppServerClient { return await this.sendRequest({ method: "thread/compact/start", params: params }); } + async threadGoalSet(params: ThreadGoalSetParams): Promise { + return await this.sendRequest({ method: "thread/goal/set", params: params }); + } + + async threadGoalClear(params: ThreadGoalClearParams): Promise { + return await this.sendRequest({ method: "thread/goal/clear", params: params }); + } + async listMcpServerStatus(params: ListMcpServerStatusParams): Promise { return await this.sendRequest({ method: "mcpServerStatus/list", params }); } diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index e4c6f8c8..f199df2f 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -112,6 +112,11 @@ export class CodexCommands { description: "Summarize conversation to avoid hitting the context limit.", input: null }, + { + name: "goal", + description: "Set, pause, resume, or clear a task goal.", + input: { hint: "[|clear|edit|pause|resume]" } + }, { name: "logout", description: "Sign out of Codex. This option is available when you are logged in via ChatGPT.", @@ -151,6 +156,10 @@ export class CodexCommands { await this.runWithProcessCheck(() => this.codexAcpClient.runCompact(sessionId)); return { handled: true }; } + case "goal": { + await this.runGoalCommand(sessionId, command.rest); + return { handled: true }; + } case "review": { const target = this.buildReviewTarget(command.rest); const turnCompleted = await this.runReviewCommand(sessionState, target); @@ -251,6 +260,40 @@ export class CodexCommands { )); } + private async runGoalCommand(sessionId: string, rest: string): Promise { + const argument = rest.trim(); + if (argument.length === 0) { + await this.sendCommandUsageMessage("goal", "[|clear|edit|pause|resume]", sessionId); + return; + } + + switch (argument.toLowerCase()) { + case "pause": + await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "paused")); + return; + case "resume": + await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "active")); + return; + case "clear": + await this.runWithProcessCheck(() => this.codexAcpClient.clearGoal(sessionId)); + return; + } + + if (argument.length > 4000) { + const session = new ACPSessionConnection(this.connection, sessionId); + await session.update({ + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: 'Command "/goal" requires goal text of at most 4000 characters.' + } + }); + return; + } + + await this.runWithProcessCheck(() => this.codexAcpClient.setGoal(sessionId, argument)); + } + private buildReviewTarget(instructions: string): ReviewTarget { if (instructions.length === 0) { return { type: "uncommittedChanges" }; diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 70582711..3edb78c0 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -13,7 +13,7 @@ import { import type {ServerNotification} from "../../app-server"; import type {SessionState} from "../../CodexAcpServer"; import {AgentMode} from "../../AgentMode"; -import type {Model, ReviewStartResponse, TurnCompletedNotification, TurnStartParams} from "../../app-server/v2"; +import type {Model, ReviewStartResponse, ThreadGoal, TurnCompletedNotification, TurnStartParams} from "../../app-server/v2"; import type {RateLimitsMap} from "../../RateLimitsMap"; import {ModelId} from "../../ModelId"; @@ -1172,6 +1172,68 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(mockFixture.getAcpConnectionDump([])).toContain("Context compacted"); }); + it('handles goal slash commands through Codex app server', async () => { + const { mockFixture, turnStartSpy } = setupPromptFixture(); + const goalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + .mockResolvedValue({ goal: createThreadGoal() }); + const goalClearSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalClear") + .mockResolvedValue({ cleared: true }); + + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }); + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal pause" }], + }); + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal resume" }], + }); + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal clear" }], + }); + + expect(goalSetSpy).toHaveBeenNthCalledWith(1, { + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + expect(goalSetSpy).toHaveBeenNthCalledWith(2, { + threadId: "session-id", + status: "paused", + }); + expect(goalSetSpy).toHaveBeenNthCalledWith(3, { + threadId: "session-id", + status: "active", + }); + expect(goalClearSpy).toHaveBeenCalledWith({ threadId: "session-id" }); + expect(turnStartSpy).not.toHaveBeenCalled(); + }); + + it('reports missing goal slash command input', async () => { + const { mockFixture } = setupPromptFixture(); + const goalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + .mockResolvedValue({ goal: createThreadGoal() }); + const goalClearSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalClear") + .mockResolvedValue({ cleared: true }); + + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal" }], + }); + + expect(goalSetSpy).not.toHaveBeenCalled(); + expect(goalClearSpy).not.toHaveBeenCalled(); + const [event] = mockFixture.getAcpConnectionEvents([]); + expect(event).toBeDefined(); + expect(event!.args[0].update.content.text).toBe( + 'Command "/goal" requires [|clear|edit|pause|resume].' + ); + }); + it('reports missing review slash command input', async () => { const { mockFixture } = setupPromptFixture(); const reviewStartSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "reviewStart") @@ -1407,6 +1469,20 @@ describe('ACP server test', { timeout: 40_000 }, () => { }; } + function createThreadGoal(overrides?: Partial): ThreadGoal { + return { + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + tokenBudget: null, + tokensUsed: 0, + timeUsedSeconds: 0, + createdAt: 1710000000, + updatedAt: 1710000000, + ...overrides, + }; + } + it ('should disable reasoning.summary if key authorization is used', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "apiKey" } }); diff --git a/src/__tests__/CodexACPAgent/data/available-commands-build-in.json b/src/__tests__/CodexACPAgent/data/available-commands-build-in.json index d805f6b0..b8bf95c8 100644 --- a/src/__tests__/CodexACPAgent/data/available-commands-build-in.json +++ b/src/__tests__/CodexACPAgent/data/available-commands-build-in.json @@ -47,6 +47,13 @@ "description": "Summarize conversation to avoid hitting the context limit.", "input": null }, + { + "name": "goal", + "description": "Set, pause, resume, or clear a task goal.", + "input": { + "hint": "[|clear|edit|pause|resume]" + } + }, { "name": "logout", "description": "Sign out of Codex. This option is available when you are logged in via ChatGPT.", diff --git a/src/__tests__/CodexACPAgent/data/available-commands-skills.json b/src/__tests__/CodexACPAgent/data/available-commands-skills.json index dc1b0101..d009fa8d 100644 --- a/src/__tests__/CodexACPAgent/data/available-commands-skills.json +++ b/src/__tests__/CodexACPAgent/data/available-commands-skills.json @@ -47,6 +47,13 @@ "description": "Summarize conversation to avoid hitting the context limit.", "input": null }, + { + "name": "goal", + "description": "Set, pause, resume, or clear a task goal.", + "input": { + "hint": "[|clear|edit|pause|resume]" + } + }, { "name": "logout", "description": "Sign out of Codex. This option is available when you are logged in via ChatGPT.", diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index c8d0b1eb..9bcc33f0 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -47,6 +47,13 @@ "description": "Summarize conversation to avoid hitting the context limit.", "input": null }, + { + "name": "goal", + "description": "Set, pause, resume, or clear a task goal.", + "input": { + "hint": "[|clear|edit|pause|resume]" + } + }, { "name": "logout", "description": "Sign out of Codex. This option is available when you are logged in via ChatGPT.", 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 94a65ad8..5e5237ad 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 @@ -47,6 +47,13 @@ "description": "Summarize conversation to avoid hitting the context limit.", "input": null }, + { + "name": "goal", + "description": "Set, pause, resume, or clear a task goal.", + "input": { + "hint": "[|clear|edit|pause|resume]" + } + }, { "name": "logout", "description": "Sign out of Codex. This option is available when you are logged in via ChatGPT.", From 5d23882d08090447ff3917d91cd70c13209e79f1 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:01:08 +0200 Subject: [PATCH 02/18] Suppress multiple identical notifications --- src/CodexAcpServer.ts | 7 ++ src/CodexEventHandler.ts | 39 ++++++++- .../CodexACPAgent/thread-goal-events.test.ts | 79 ++++++++++++++++--- 3 files changed, 112 insertions(+), 13 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 11c33a7e..727965ac 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -66,6 +66,12 @@ import packageJson from "../package.json"; import {isJetBrains2026_1Client} from "./JBUtils"; import {resolveTerminalOutputMode, type TerminalOutputMode} from "./TerminalOutputMode"; +export interface ThreadGoalSnapshot { + objective: string; + status: string; + tokenBudget: number | null; +} + export interface SessionState { sessionId: string, currentModelId: string, @@ -85,6 +91,7 @@ export interface SessionState { currentModelSupportsFast: boolean; sessionMcpServers?: Array; terminalOutputMode: TerminalOutputMode; + currentGoal?: ThreadGoalSnapshot | null; } interface PendingMcpStartupSession { diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index eb22e74b..a333302f 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -3,7 +3,8 @@ import type { FuzzyFileSearchSessionUpdatedNotification, ServerNotification } from "./app-server"; -import type {SessionState} from "./CodexAcpServer"; +import type {SessionState, ThreadGoalSnapshot} from "./CodexAcpServer"; +import * as acp from "@agentclientprotocol/sdk"; import {type PlanEntry, RequestError} from "@agentclientprotocol/sdk"; import {ACPSessionConnection, type AcpClientConnection, type UpdateSessionEvent} from "./ACPSessionConnection"; import type { @@ -266,9 +267,15 @@ export class CodexEventHandler { }; } - private createThreadGoalUpdatedEvent(event: ThreadGoalUpdatedNotification): UpdateSessionEvent { + private createThreadGoalUpdatedEvent(event: ThreadGoalUpdatedNotification): UpdateSessionEvent | null { + const goalSnapshot = this.createThreadGoalSnapshot(event); + if (this.sameThreadGoalSnapshot(this.sessionState.currentGoal, goalSnapshot)) { + return null; + } + this.sessionState.currentGoal = goalSnapshot; + const status = this.formatThreadGoalStatus(event.goal.status); - const objective = event.goal.objective.trim(); + const objective = goalSnapshot.objective; const text = objective.includes("\n") ? `Goal updated (${status}):\n${objective}` : `Goal updated (${status}): ${objective}`; @@ -298,7 +305,12 @@ export class CodexEventHandler { } } - private createThreadGoalClearedEvent(_event: ThreadGoalClearedNotification): UpdateSessionEvent { + private createThreadGoalClearedEvent(_event: ThreadGoalClearedNotification): UpdateSessionEvent | null { + if (this.sessionState.currentGoal === null) { + return null; + } + this.sessionState.currentGoal = null; + return { sessionUpdate: "agent_message_chunk", content: { @@ -308,6 +320,25 @@ export class CodexEventHandler { }; } + private createThreadGoalSnapshot(event: ThreadGoalUpdatedNotification): ThreadGoalSnapshot { + return { + objective: event.goal.objective.trim(), + status: event.goal.status, + tokenBudget: event.goal.tokenBudget, + }; + } + + private sameThreadGoalSnapshot( + left: ThreadGoalSnapshot | null | undefined, + right: ThreadGoalSnapshot + ): boolean { + return left !== null + && left !== undefined + && left.objective === right.objective + && left.status === right.status + && left.tokenBudget === right.tokenBudget; + } + private createReasoningDeltaEvent( event: ReasoningSummaryTextDeltaNotification | ReasoningTextDeltaNotification ): UpdateSessionEvent { diff --git a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts index d121cf85..dce3841e 100644 --- a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts +++ b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts @@ -18,12 +18,6 @@ describe("CodexEventHandler - thread goal events", () => { vi.clearAllMocks(); }); - const sessionState: SessionState = createTestSessionState({ - sessionId, - currentModelId: "model-id[effort]", - agentMode: AgentMode.DEFAULT_AGENT_MODE, - }); - it("should send thread goal updates as agent messages", async () => { const goalUpdatedNotification: ServerNotification = { method: "thread/goal/updated", @@ -43,7 +37,7 @@ describe("CodexEventHandler - thread goal events", () => { }, }; - await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [goalUpdatedNotification]); + await setupPromptAndSendNotifications(mockFixture, sessionId, createSessionState(), [goalUpdatedNotification]); await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( "data/thread-goal-updated.json" @@ -69,7 +63,7 @@ describe("CodexEventHandler - thread goal events", () => { }, }; - await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [goalUpdatedNotification]); + await setupPromptAndSendNotifications(mockFixture, sessionId, createSessionState(), [goalUpdatedNotification]); await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( "data/thread-goal-updated-multiline.json" @@ -84,10 +78,77 @@ describe("CodexEventHandler - thread goal events", () => { }, }; - await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, [goalClearedNotification]); + await setupPromptAndSendNotifications(mockFixture, sessionId, createSessionState(), [goalClearedNotification]); await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot( "data/thread-goal-cleared.json" ); }); + + it("should suppress duplicate thread goal updates", async () => { + const goalUpdatedNotification: ServerNotification = { + method: "thread/goal/updated", + params: { + threadId: sessionId, + turnId: "turn-1", + goal: { + threadId: sessionId, + objective: "Ship the goal update", + status: "active", + tokenBudget: null, + tokensUsed: 42, + timeUsedSeconds: 12, + createdAt: 1710000000, + updatedAt: 1710000012, + }, + }, + }; + const duplicateGoalUpdatedNotification: ServerNotification = { + method: "thread/goal/updated", + params: { + ...goalUpdatedNotification.params, + goal: { + ...goalUpdatedNotification.params.goal, + tokensUsed: 84, + timeUsedSeconds: 24, + updatedAt: 1710000024, + }, + }, + }; + + await setupPromptAndSendNotifications(mockFixture, sessionId, createSessionState(), [ + goalUpdatedNotification, + duplicateGoalUpdatedNotification, + ]); + + const events = mockFixture.getAcpConnectionEvents([]); + expect(events).toHaveLength(1); + expect(events[0]!.args[0].update.content.text).toBe("Goal updated (active): Ship the goal update"); + }); + + it("should suppress duplicate thread goal cleared notifications", async () => { + const goalClearedNotification: ServerNotification = { + method: "thread/goal/cleared", + params: { + threadId: sessionId, + }, + }; + + await setupPromptAndSendNotifications(mockFixture, sessionId, createSessionState(), [ + goalClearedNotification, + goalClearedNotification, + ]); + + const events = mockFixture.getAcpConnectionEvents([]); + expect(events).toHaveLength(1); + expect(events[0]!.args[0].update.content.text).toBe("Goal cleared."); + }); + + function createSessionState(): SessionState { + return createTestSessionState({ + sessionId, + currentModelId: "model-id[effort]", + agentMode: AgentMode.DEFAULT_AGENT_MODE, + }); + } }); From 832155139d4c0b0d9c6cac18a53362fd7ec7a5df Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:09:31 +0200 Subject: [PATCH 03/18] Wait on turns --- src/CodexAcpClient.ts | 4 ++ src/CodexAcpServer.ts | 38 ++++++++++++++- src/CodexCommands.ts | 18 +++---- .../CodexACPAgent/CodexAcpClient.test.ts | 48 +++++++++++++++++++ 4 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index e6219870..08b4ab41 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -342,6 +342,10 @@ export class CodexAcpClient { await this.codexClient.threadGoalClear({threadId: sessionId}); } + async awaitTurnCompleted(params: { threadId: string, turnId: string }): Promise { + return await this.codexClient.awaitTurnCompleted(params.threadId, params.turnId); + } + async awaitMcpServerStartup(serverNames: Array, afterVersion: number): Promise { return await this.codexClient.awaitMcpServerStartup(serverNames, afterVersion); } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 727965ac..14bdb59d 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -14,6 +14,7 @@ import type { ReasoningEffortOption, Thread, ThreadItem, + TurnCompletedNotification, UserInput } from "./app-server/v2"; import type {RateLimitsMap} from "./RateLimitsMap"; @@ -1249,6 +1250,30 @@ export class CodexAcpServer { return turnId; } + private async awaitCurrentCommandTurnCompletion( + sessionState: SessionState, + activePrompt: ActivePrompt, + ): Promise { + const turnId = sessionState.currentTurnId; + if (!turnId) { + return undefined; + } + + const turnCompleted = await Promise.race([ + this.runWithProcessCheck(() => this.codexAcpClient.awaitTurnCompleted({ + threadId: sessionState.sessionId, + turnId, + })), + activePrompt.closeSignal, + ]); + if (turnCompleted === null) { + return null; + } + + await this.codexAcpClient.waitForSessionNotifications(sessionState.sessionId); + return turnCompleted; + } + async prompt(params: acp.PromptRequest): Promise { logger.log("Prompt received", { sessionId: params.sessionId, @@ -1276,7 +1301,18 @@ export class CodexAcpServer { if (commandResult.handled) { logger.log("Prompt handled by a command"); await this.codexAcpClient.waitForSessionNotifications(params.sessionId); - if (commandResult.turnCompleted?.turn.status === "interrupted") { + let turnCompleted: TurnCompletedNotification | null | undefined = commandResult.turnCompleted; + if (!turnCompleted && commandResult.waitForCurrentTurnCompletion) { + turnCompleted = await this.awaitCurrentCommandTurnCompletion(sessionState, activePrompt); + if (turnCompleted === null) { + return { + stopReason: "cancelled", + usage: this.buildPromptUsage(sessionState.lastTokenUsage), + _meta: this.buildQuotaMeta(sessionState), + }; + } + } + if (turnCompleted?.turn.status === "interrupted") { if (!this.sessionIsClosing(params.sessionId) && this.sessions.has(params.sessionId)) { await this.connection.notify(acp.methods.client.session.update, { sessionId: params.sessionId, diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index f199df2f..772831ea 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -15,7 +15,7 @@ type ParsedSlashCommand = { export type CommandHandleResult = | { handled: false } - | { handled: true, turnCompleted?: TurnCompletedNotification }; + | { handled: true, turnCompleted?: TurnCompletedNotification, waitForCurrentTurnCompletion?: boolean }; export class CodexCommands { private readonly connection: AcpClientConnection; @@ -157,8 +157,7 @@ export class CodexCommands { return { handled: true }; } case "goal": { - await this.runGoalCommand(sessionId, command.rest); - return { handled: true }; + return await this.runGoalCommand(sessionId, command.rest); } case "review": { const target = this.buildReviewTarget(command.rest); @@ -260,23 +259,23 @@ export class CodexCommands { )); } - private async runGoalCommand(sessionId: string, rest: string): Promise { + private async runGoalCommand(sessionId: string, rest: string): Promise { const argument = rest.trim(); if (argument.length === 0) { await this.sendCommandUsageMessage("goal", "[|clear|edit|pause|resume]", sessionId); - return; + return { handled: true }; } switch (argument.toLowerCase()) { case "pause": await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "paused")); - return; + return { handled: true }; case "resume": await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "active")); - return; + return { handled: true, waitForCurrentTurnCompletion: true }; case "clear": await this.runWithProcessCheck(() => this.codexAcpClient.clearGoal(sessionId)); - return; + return { handled: true }; } if (argument.length > 4000) { @@ -288,10 +287,11 @@ export class CodexCommands { text: 'Command "/goal" requires goal text of at most 4000 characters.' } }); - return; + return { handled: true }; } await this.runWithProcessCheck(() => this.codexAcpClient.setGoal(sessionId, argument)); + return { handled: true, waitForCurrentTurnCompletion: true }; } private buildReviewTarget(instructions: string): ReviewTarget { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 3edb78c0..0ce22084 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1213,6 +1213,54 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(turnStartSpy).not.toHaveBeenCalled(); }); + it('waits for goal slash command turn completion', async () => { + const { mockFixture } = setupPromptFixture(); + vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + .mockImplementation(async () => { + mockFixture.sendServerNotification({ + method: "turn/started", + params: { + threadId: "session-id", + turn: createTurn("goal-turn-id", "inProgress"), + }, + }); + return { goal: createThreadGoal() }; + }); + let completeGoal: (value: TurnCompletedNotification) => void = () => {}; + const goalCompletedPromise = new Promise((resolve) => { + completeGoal = resolve; + }); + vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted") + .mockReturnValue(goalCompletedPromise); + + let promptResolved = false; + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }).then((response) => { + promptResolved = true; + return response; + }); + + await vi.waitFor(() => { + expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).toHaveBeenCalledWith( + "session-id", + "goal-turn-id", + ); + }); + await Promise.resolve(); + expect(promptResolved).toBe(false); + + completeGoal({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + await expect(promptPromise).resolves.toEqual(expect.objectContaining({ + stopReason: "end_turn", + })); + expect(promptResolved).toBe(true); + }); + it('reports missing goal slash command input', async () => { const { mockFixture } = setupPromptFixture(); const goalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") From ed5cbb761319e01ba9a36ed8d9e8e7f7257d3fab Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:20:17 +0200 Subject: [PATCH 04/18] Wait for goal turn --- src/CodexAcpClient.ts | 24 ++++-- src/CodexAcpServer.ts | 36 +-------- src/CodexAppServerClient.ts | 73 +++++++++++++++++++ src/CodexCommands.ts | 30 ++++++-- .../CodexACPAgent/CodexAcpClient.test.ts | 70 ++++++++++++++---- 5 files changed, 168 insertions(+), 65 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 08b4ab41..30f88dd8 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -323,12 +323,16 @@ export class CodexAcpClient { await this.codexClient.runCompact({threadId: sessionId}); } - async setGoal(sessionId: string, objective: string): Promise { - await this.codexClient.threadGoalSet({ + async setGoal( + sessionId: string, + objective: string, + onTurnStarted?: (turnId: string) => void, + ): Promise { + return await this.codexClient.runGoalSet({ threadId: sessionId, objective, status: "active", - }); + }, onTurnStarted); } async setGoalStatus(sessionId: string, status: ThreadGoalStatus): Promise { @@ -338,12 +342,18 @@ export class CodexAcpClient { }); } - async clearGoal(sessionId: string): Promise { - await this.codexClient.threadGoalClear({threadId: sessionId}); + async resumeGoal( + sessionId: string, + onTurnStarted?: (turnId: string) => void, + ): Promise { + return await this.codexClient.runGoalSet({ + threadId: sessionId, + status: "active", + }, onTurnStarted); } - async awaitTurnCompleted(params: { threadId: string, turnId: string }): Promise { - return await this.codexClient.awaitTurnCompleted(params.threadId, params.turnId); + async clearGoal(sessionId: string): Promise { + await this.codexClient.threadGoalClear({threadId: sessionId}); } async awaitMcpServerStartup(serverNames: Array, afterVersion: number): Promise { diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 14bdb59d..b4eed9c2 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1250,30 +1250,6 @@ export class CodexAcpServer { return turnId; } - private async awaitCurrentCommandTurnCompletion( - sessionState: SessionState, - activePrompt: ActivePrompt, - ): Promise { - const turnId = sessionState.currentTurnId; - if (!turnId) { - return undefined; - } - - const turnCompleted = await Promise.race([ - this.runWithProcessCheck(() => this.codexAcpClient.awaitTurnCompleted({ - threadId: sessionState.sessionId, - turnId, - })), - activePrompt.closeSignal, - ]); - if (turnCompleted === null) { - return null; - } - - await this.codexAcpClient.waitForSessionNotifications(sessionState.sessionId); - return turnCompleted; - } - async prompt(params: acp.PromptRequest): Promise { logger.log("Prompt received", { sessionId: params.sessionId, @@ -1301,17 +1277,7 @@ export class CodexAcpServer { if (commandResult.handled) { logger.log("Prompt handled by a command"); await this.codexAcpClient.waitForSessionNotifications(params.sessionId); - let turnCompleted: TurnCompletedNotification | null | undefined = commandResult.turnCompleted; - if (!turnCompleted && commandResult.waitForCurrentTurnCompletion) { - turnCompleted = await this.awaitCurrentCommandTurnCompletion(sessionState, activePrompt); - if (turnCompleted === null) { - return { - stopReason: "cancelled", - usage: this.buildPromptUsage(sessionState.lastTokenUsage), - _meta: this.buildQuotaMeta(sessionState), - }; - } - } + const turnCompleted = commandResult.turnCompleted; if (turnCompleted?.turn.status === "interrupted") { if (!this.sessionIsClosing(params.sessionId) && this.sessions.has(params.sessionId)) { await this.connection.notify(acp.methods.client.session.update, { diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 711bb8d9..5138f3d3 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -119,6 +119,7 @@ export class CodexAppServerClient { private readonly pendingTurnCompletionResolvers = new Map void>>(); private readonly pendingCompactionCompletionResolvers = new Map void>>(); private readonly turnCompletionCaptures = new Map void>>(); + private readonly turnRoutingCaptures = new Map void>>(); private readonly staleTurnIds = new Map>(); constructor(connection: MessageConnection) { @@ -151,6 +152,7 @@ export class CodexAppServerClient { } return; } + this.recordTurnRouting(routing); this.notify(serverNotification); for (const callback of this.codexEventHandlers) { callback({ eventType: "notification", ...serverNotification }); @@ -266,6 +268,47 @@ export class CodexAppServerClient { } } + async runGoalSet(params: ThreadGoalSetParams, onTurnStarted?: (turnId: string) => void): Promise { + const capturedCompletions: Array = []; + const releaseCompletionCapture = this.captureTurnCompletions(params.threadId, (event) => { + capturedCompletions.push(event); + }); + let goalTurnId: string | null = null; + let resolveGoalTurnStarted: () => void = () => {}; + const goalTurnStarted = new Promise((resolve) => { + resolveGoalTurnStarted = resolve; + }); + const releaseRoutingCapture = this.captureTurnRoutings(params.threadId, (turnId) => { + if (goalTurnId !== null) { + return; + } + goalTurnId = turnId; + onTurnStarted?.(turnId); + resolveGoalTurnStarted(); + }); + + try { + await this.threadGoalSet(params); + if (goalTurnId === null) { + await goalTurnStarted; + } + const turnId = goalTurnId; + if (turnId === null) { + throw new Error("Goal command did not start a turn"); + } + const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); + releaseCompletionCapture(); + releaseRoutingCapture(); + if (earlyCompletion) { + return earlyCompletion; + } + return await this.awaitTurnCompleted(params.threadId, turnId); + } finally { + releaseCompletionCapture(); + releaseRoutingCapture(); + } + } + async runCompact(params: ThreadCompactStartParams): Promise { const compactionCompleted = this.awaitCompactionCompleted(params.threadId); await this.threadCompactStart(params); @@ -478,6 +521,19 @@ export class CodexAppServerClient { } } + private recordTurnRouting(routing: { threadId: string | null, turnId: string | null }): void { + if (routing.threadId === null || routing.turnId === null) { + return; + } + const captures = this.turnRoutingCaptures.get(routing.threadId); + if (!captures) { + return; + } + for (const capture of captures) { + capture(routing.turnId); + } + } + private isStaleTurn(threadId: string | null, turnId: string | null): boolean { if (threadId === null || turnId === null) { return false; @@ -523,6 +579,23 @@ export class CodexAppServerClient { }; } + private captureTurnRoutings(threadId: string, capture: (turnId: string) => void): () => void { + const captures = this.turnRoutingCaptures.get(threadId) ?? new Set<(turnId: string) => void>(); + captures.add(capture); + this.turnRoutingCaptures.set(threadId, captures); + let released = false; + return () => { + if (released) { + return; + } + released = true; + captures.delete(capture); + if (captures.size === 0) { + this.turnRoutingCaptures.delete(threadId); + } + }; + } + private resolveMcpServerStartupResolvers(): void { const pendingResolvers: Array = []; for (const resolver of this.mcpServerStartupResolvers) { diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index 772831ea..62f046c4 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -15,7 +15,7 @@ type ParsedSlashCommand = { export type CommandHandleResult = | { handled: false } - | { handled: true, turnCompleted?: TurnCompletedNotification, waitForCurrentTurnCompletion?: boolean }; + | { handled: true, turnCompleted?: TurnCompletedNotification }; export class CodexCommands { private readonly connection: AcpClientConnection; @@ -157,7 +157,7 @@ export class CodexCommands { return { handled: true }; } case "goal": { - return await this.runGoalCommand(sessionId, command.rest); + return await this.runGoalCommand(sessionState, command.rest); } case "review": { const target = this.buildReviewTarget(command.rest); @@ -259,7 +259,8 @@ export class CodexCommands { )); } - private async runGoalCommand(sessionId: string, rest: string): Promise { + private async runGoalCommand(sessionState: SessionState, rest: string): Promise { + const sessionId = sessionState.sessionId; const argument = rest.trim(); if (argument.length === 0) { await this.sendCommandUsageMessage("goal", "[|clear|edit|pause|resume]", sessionId); @@ -271,8 +272,15 @@ export class CodexCommands { await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "paused")); return { handled: true }; case "resume": - await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "active")); - return { handled: true, waitForCurrentTurnCompletion: true }; + return { + handled: true, + turnCompleted: await this.runWithProcessCheck(() => this.codexAcpClient.resumeGoal( + sessionId, + (turnId) => { + sessionState.currentTurnId = turnId; + }, + )), + }; case "clear": await this.runWithProcessCheck(() => this.codexAcpClient.clearGoal(sessionId)); return { handled: true }; @@ -290,8 +298,16 @@ export class CodexCommands { return { handled: true }; } - await this.runWithProcessCheck(() => this.codexAcpClient.setGoal(sessionId, argument)); - return { handled: true, waitForCurrentTurnCompletion: true }; + return { + handled: true, + turnCompleted: await this.runWithProcessCheck(() => this.codexAcpClient.setGoal( + sessionId, + argument, + (turnId) => { + sessionState.currentTurnId = turnId; + }, + )), + }; } private buildReviewTarget(instructions: string): ReviewTarget { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 0ce22084..272e5c75 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1174,7 +1174,12 @@ describe('ACP server test', { timeout: 40_000 }, () => { it('handles goal slash commands through Codex app server', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture(); - const goalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + const goalRunSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "runGoalSet") + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + const goalStatusSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") .mockResolvedValue({ goal: createThreadGoal() }); const goalClearSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalClear") .mockResolvedValue({ cleared: true }); @@ -1196,19 +1201,19 @@ describe('ACP server test', { timeout: 40_000 }, () => { prompt: [{ type: "text", text: "/goal clear" }], }); - expect(goalSetSpy).toHaveBeenNthCalledWith(1, { + expect(goalRunSpy).toHaveBeenNthCalledWith(1, { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }); - expect(goalSetSpy).toHaveBeenNthCalledWith(2, { + }, expect.any(Function)); + expect(goalStatusSpy).toHaveBeenCalledWith({ threadId: "session-id", status: "paused", }); - expect(goalSetSpy).toHaveBeenNthCalledWith(3, { + expect(goalRunSpy).toHaveBeenNthCalledWith(2, { threadId: "session-id", status: "active", - }); + }, expect.any(Function)); expect(goalClearSpy).toHaveBeenCalledWith({ threadId: "session-id" }); expect(turnStartSpy).not.toHaveBeenCalled(); }); @@ -1216,16 +1221,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { it('waits for goal slash command turn completion', async () => { const { mockFixture } = setupPromptFixture(); vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") - .mockImplementation(async () => { - mockFixture.sendServerNotification({ - method: "turn/started", - params: { - threadId: "session-id", - turn: createTurn("goal-turn-id", "inProgress"), - }, - }); - return { goal: createThreadGoal() }; - }); + .mockResolvedValue({ goal: createThreadGoal() }); let completeGoal: (value: TurnCompletedNotification) => void = () => {}; const goalCompletedPromise = new Promise((resolve) => { completeGoal = resolve; @@ -1242,6 +1238,48 @@ describe('ACP server test', { timeout: 40_000 }, () => { return response; }); + await vi.waitFor(() => { + expect(mockFixture.getCodexAppServerClient().threadGoalSet).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + await Promise.resolve(); + expect(promptResolved).toBe(false); + expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal: createThreadGoal(), + }, + }); + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { + type: "active", + activeFlags: [], + }, + }, + }); + await Promise.resolve(); + expect(promptResolved).toBe(false); + expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "I", + }, + }); await vi.waitFor(() => { expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).toHaveBeenCalledWith( "session-id", From 00efa077cf5c97240e9579f6c4e22a78c31c57bb Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:22:43 +0200 Subject: [PATCH 05/18] Add newlines after notifications --- src/CodexEventHandler.ts | 4 ++-- src/__tests__/CodexACPAgent/data/thread-goal-cleared.json | 2 +- .../CodexACPAgent/data/thread-goal-updated-multiline.json | 2 +- src/__tests__/CodexACPAgent/data/thread-goal-updated.json | 2 +- src/__tests__/CodexACPAgent/thread-goal-events.test.ts | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index a333302f..6b76c237 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -283,7 +283,7 @@ export class CodexEventHandler { sessionUpdate: "agent_message_chunk", content: { type: "text", - text, + text: `${text}\n\n`, }, }; } @@ -315,7 +315,7 @@ export class CodexEventHandler { sessionUpdate: "agent_message_chunk", content: { type: "text", - text: "Goal cleared.", + text: "Goal cleared.\n\n", }, }; } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json b/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json index bfd4ac7a..8d51d915 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json @@ -7,7 +7,7 @@ "sessionUpdate": "agent_message_chunk", "content": { "type": "text", - "text": "Goal cleared." + "text": "Goal cleared.\n\n" } } } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json b/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json index 79c6d2c0..d3cb54f2 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json @@ -7,7 +7,7 @@ "sessionUpdate": "agent_message_chunk", "content": { "type": "text", - "text": "Goal updated (budget limited):\nFirst task\nSecond task" + "text": "Goal updated (budget limited):\nFirst task\nSecond task\n\n" } } } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-updated.json b/src/__tests__/CodexACPAgent/data/thread-goal-updated.json index 19e32e97..9518a0a5 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-updated.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-updated.json @@ -7,7 +7,7 @@ "sessionUpdate": "agent_message_chunk", "content": { "type": "text", - "text": "Goal updated (active): Ship the goal update" + "text": "Goal updated (active): Ship the goal update\n\n" } } } diff --git a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts index dce3841e..cfae5e15 100644 --- a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts +++ b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts @@ -123,7 +123,7 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(1); - expect(events[0]!.args[0].update.content.text).toBe("Goal updated (active): Ship the goal update"); + expect(events[0]!.args[0].update.content.text).toBe("Goal updated (active): Ship the goal update\n\n"); }); it("should suppress duplicate thread goal cleared notifications", async () => { @@ -141,7 +141,7 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(1); - expect(events[0]!.args[0].update.content.text).toBe("Goal cleared."); + expect(events[0]!.args[0].update.content.text).toBe("Goal cleared.\n\n"); }); function createSessionState(): SessionState { From 9b0a563c475fa252a9f94c75ea5afb514e72fabb Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:31:54 +0200 Subject: [PATCH 06/18] Remove unsupported edit subcommand --- src/CodexCommands.ts | 4 ++-- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts | 2 +- .../CodexACPAgent/data/available-commands-build-in.json | 2 +- .../CodexACPAgent/data/available-commands-skills.json | 2 +- src/__tests__/CodexACPAgent/data/load-session-history.json | 2 +- .../data/load-session-response-item-history-fallback.json | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index 62f046c4..e8ff6041 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -115,7 +115,7 @@ export class CodexCommands { { name: "goal", description: "Set, pause, resume, or clear a task goal.", - input: { hint: "[|clear|edit|pause|resume]" } + input: { hint: "[|clear|pause|resume]" } }, { name: "logout", @@ -263,7 +263,7 @@ export class CodexCommands { const sessionId = sessionState.sessionId; const argument = rest.trim(); if (argument.length === 0) { - await this.sendCommandUsageMessage("goal", "[|clear|edit|pause|resume]", sessionId); + await this.sendCommandUsageMessage("goal", "[|clear|pause|resume]", sessionId); return { handled: true }; } diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 272e5c75..f54f44dd 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1316,7 +1316,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { const [event] = mockFixture.getAcpConnectionEvents([]); expect(event).toBeDefined(); expect(event!.args[0].update.content.text).toBe( - 'Command "/goal" requires [|clear|edit|pause|resume].' + 'Command "/goal" requires [|clear|pause|resume].' ); }); diff --git a/src/__tests__/CodexACPAgent/data/available-commands-build-in.json b/src/__tests__/CodexACPAgent/data/available-commands-build-in.json index b8bf95c8..26e78cbb 100644 --- a/src/__tests__/CodexACPAgent/data/available-commands-build-in.json +++ b/src/__tests__/CodexACPAgent/data/available-commands-build-in.json @@ -51,7 +51,7 @@ "name": "goal", "description": "Set, pause, resume, or clear a task goal.", "input": { - "hint": "[|clear|edit|pause|resume]" + "hint": "[|clear|pause|resume]" } }, { diff --git a/src/__tests__/CodexACPAgent/data/available-commands-skills.json b/src/__tests__/CodexACPAgent/data/available-commands-skills.json index d009fa8d..59987072 100644 --- a/src/__tests__/CodexACPAgent/data/available-commands-skills.json +++ b/src/__tests__/CodexACPAgent/data/available-commands-skills.json @@ -51,7 +51,7 @@ "name": "goal", "description": "Set, pause, resume, or clear a task goal.", "input": { - "hint": "[|clear|edit|pause|resume]" + "hint": "[|clear|pause|resume]" } }, { diff --git a/src/__tests__/CodexACPAgent/data/load-session-history.json b/src/__tests__/CodexACPAgent/data/load-session-history.json index 9bcc33f0..bb272cf7 100644 --- a/src/__tests__/CodexACPAgent/data/load-session-history.json +++ b/src/__tests__/CodexACPAgent/data/load-session-history.json @@ -51,7 +51,7 @@ "name": "goal", "description": "Set, pause, resume, or clear a task goal.", "input": { - "hint": "[|clear|edit|pause|resume]" + "hint": "[|clear|pause|resume]" } }, { 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 5e5237ad..c5d03f99 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 @@ -51,7 +51,7 @@ "name": "goal", "description": "Set, pause, resume, or clear a task goal.", "input": { - "hint": "[|clear|edit|pause|resume]" + "hint": "[|clear|pause|resume]" } }, { From 6807a4b9fe91924c19463ea91061e4f1c3450cde Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:43:14 +0200 Subject: [PATCH 07/18] Handle goal commands without continuation turns --- src/CodexAcpClient.ts | 4 +- src/CodexAppServerClient.ts | 35 ++++++++++++---- src/CodexCommands.ts | 36 ++++++++-------- .../CodexACPAgent/CodexAcpClient.test.ts | 41 +++++++++++++++++++ 4 files changed, 89 insertions(+), 27 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 30f88dd8..9c0f83e4 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -327,7 +327,7 @@ export class CodexAcpClient { sessionId: string, objective: string, onTurnStarted?: (turnId: string) => void, - ): Promise { + ): Promise { return await this.codexClient.runGoalSet({ threadId: sessionId, objective, @@ -345,7 +345,7 @@ export class CodexAcpClient { async resumeGoal( sessionId: string, onTurnStarted?: (turnId: string) => void, - ): Promise { + ): Promise { return await this.codexClient.runGoalSet({ threadId: sessionId, status: "active", diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 5138f3d3..44c0da49 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -105,6 +105,8 @@ const McpServerElicitationRequest = new RequestType< void >('mcpServer/elicitation/request'); +const GOAL_TURN_START_GRACE_MS = 1_000; + /** * A type-safe client over the Codex App Server's JSON-RPC API. * Maps each request to its expected response and exposes clear, typed methods for supported JSON-RPC operations. @@ -268,14 +270,18 @@ export class CodexAppServerClient { } } - async runGoalSet(params: ThreadGoalSetParams, onTurnStarted?: (turnId: string) => void): Promise { + async runGoalSet( + params: ThreadGoalSetParams, + onTurnStarted?: (turnId: string) => void, + turnStartGraceMs = GOAL_TURN_START_GRACE_MS, + ): Promise { const capturedCompletions: Array = []; const releaseCompletionCapture = this.captureTurnCompletions(params.threadId, (event) => { capturedCompletions.push(event); }); let goalTurnId: string | null = null; - let resolveGoalTurnStarted: () => void = () => {}; - const goalTurnStarted = new Promise((resolve) => { + let resolveGoalTurnStarted: (turnId: string) => void = () => {}; + const goalTurnStarted = new Promise((resolve) => { resolveGoalTurnStarted = resolve; }); const releaseRoutingCapture = this.captureTurnRoutings(params.threadId, (turnId) => { @@ -284,17 +290,14 @@ export class CodexAppServerClient { } goalTurnId = turnId; onTurnStarted?.(turnId); - resolveGoalTurnStarted(); + resolveGoalTurnStarted(turnId); }); try { await this.threadGoalSet(params); - if (goalTurnId === null) { - await goalTurnStarted; - } - const turnId = goalTurnId; + const turnId = goalTurnId ?? await this.waitForGoalTurnStarted(goalTurnStarted, turnStartGraceMs); if (turnId === null) { - throw new Error("Goal command did not start a turn"); + return null; } const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); releaseCompletionCapture(); @@ -309,6 +312,20 @@ export class CodexAppServerClient { } } + private async waitForGoalTurnStarted(goalTurnStarted: Promise, timeoutMs: number): Promise { + let timeout: ReturnType | null = null; + const timeoutPromise = new Promise((resolve) => { + timeout = setTimeout(() => resolve(null), timeoutMs); + }); + try { + return await Promise.race([goalTurnStarted, timeoutPromise]); + } finally { + if (timeout !== null) { + clearTimeout(timeout); + } + } + } + async runCompact(params: ThreadCompactStartParams): Promise { const compactionCompleted = this.awaitCompactionCompleted(params.threadId); await this.threadCompactStart(params); diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index e8ff6041..506024ec 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -272,15 +272,12 @@ export class CodexCommands { await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "paused")); return { handled: true }; case "resume": - return { - handled: true, - turnCompleted: await this.runWithProcessCheck(() => this.codexAcpClient.resumeGoal( - sessionId, - (turnId) => { - sessionState.currentTurnId = turnId; - }, - )), - }; + return this.createGoalCommandResult(await this.runWithProcessCheck(() => this.codexAcpClient.resumeGoal( + sessionId, + (turnId) => { + sessionState.currentTurnId = turnId; + }, + ))); case "clear": await this.runWithProcessCheck(() => this.codexAcpClient.clearGoal(sessionId)); return { handled: true }; @@ -298,15 +295,22 @@ export class CodexCommands { return { handled: true }; } + return this.createGoalCommandResult(await this.runWithProcessCheck(() => this.codexAcpClient.setGoal( + sessionId, + argument, + (turnId) => { + sessionState.currentTurnId = turnId; + }, + ))); + } + + private createGoalCommandResult(turnCompleted: TurnCompletedNotification | null): CommandHandleResult { + if (turnCompleted === null) { + return { handled: true }; + } return { handled: true, - turnCompleted: await this.runWithProcessCheck(() => this.codexAcpClient.setGoal( - sessionId, - argument, - (turnId) => { - sessionState.currentTurnId = turnId; - }, - )), + turnCompleted, }; } diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index f54f44dd..d60a037e 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1299,6 +1299,47 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(promptResolved).toBe(true); }); + it('does not hang when goal set starts no continuation turn', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({ goal: createThreadGoal() }); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted"); + + const result = await codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, undefined, 0); + + expect(result).toBeNull(); + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + }); + + it('completes goal slash command when app server starts no continuation turn', async () => { + const { mockFixture, turnStartSpy } = setupPromptFixture(); + const goalRunSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "runGoalSet") + .mockResolvedValue(null); + + const response = await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }); + + expect(response.stopReason).toBe("end_turn"); + expect(goalRunSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, expect.any(Function)); + expect(turnStartSpy).not.toHaveBeenCalled(); + }); + it('reports missing goal slash command input', async () => { const { mockFixture } = setupPromptFixture(); const goalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") From 4a88b2311830e5290d2475de04a35a738b0ea33c Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 12:45:56 +0200 Subject: [PATCH 08/18] Separate goal status updates from prior text --- src/CodexEventHandler.ts | 4 +- .../data/thread-goal-cleared.json | 2 +- .../data/thread-goal-updated-multiline.json | 2 +- .../data/thread-goal-updated.json | 2 +- .../CodexACPAgent/thread-goal-events.test.ts | 41 ++++++++++++++++++- 5 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 6b76c237..c915d625 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -283,7 +283,7 @@ export class CodexEventHandler { sessionUpdate: "agent_message_chunk", content: { type: "text", - text: `${text}\n\n`, + text: `\n\n${text}\n\n`, }, }; } @@ -315,7 +315,7 @@ export class CodexEventHandler { sessionUpdate: "agent_message_chunk", content: { type: "text", - text: "Goal cleared.\n\n", + text: "\n\nGoal cleared.\n\n", }, }; } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json b/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json index 8d51d915..c6fc87f1 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-cleared.json @@ -7,7 +7,7 @@ "sessionUpdate": "agent_message_chunk", "content": { "type": "text", - "text": "Goal cleared.\n\n" + "text": "\n\nGoal cleared.\n\n" } } } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json b/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json index d3cb54f2..b9336936 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json @@ -7,7 +7,7 @@ "sessionUpdate": "agent_message_chunk", "content": { "type": "text", - "text": "Goal updated (budget limited):\nFirst task\nSecond task\n\n" + "text": "\n\nGoal updated (budget limited):\nFirst task\nSecond task\n\n" } } } diff --git a/src/__tests__/CodexACPAgent/data/thread-goal-updated.json b/src/__tests__/CodexACPAgent/data/thread-goal-updated.json index 9518a0a5..264c1e34 100644 --- a/src/__tests__/CodexACPAgent/data/thread-goal-updated.json +++ b/src/__tests__/CodexACPAgent/data/thread-goal-updated.json @@ -7,7 +7,7 @@ "sessionUpdate": "agent_message_chunk", "content": { "type": "text", - "text": "Goal updated (active): Ship the goal update\n\n" + "text": "\n\nGoal updated (active): Ship the goal update\n\n" } } } diff --git a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts index cfae5e15..2db982de 100644 --- a/src/__tests__/CodexACPAgent/thread-goal-events.test.ts +++ b/src/__tests__/CodexACPAgent/thread-goal-events.test.ts @@ -123,7 +123,44 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(1); - expect(events[0]!.args[0].update.content.text).toBe("Goal updated (active): Ship the goal update\n\n"); + expect(events[0]!.args[0].update.content.text).toBe("\n\nGoal updated (active): Ship the goal update\n\n"); + }); + + it("should separate completed goal updates from preceding agent text", async () => { + const goalCompletedNotification: ServerNotification = { + method: "thread/goal/updated", + params: { + threadId: sessionId, + turnId: "turn-1", + goal: { + threadId: sessionId, + objective: "tell me a joke", + status: "complete", + tokenBudget: null, + tokensUsed: 42, + timeUsedSeconds: 12, + createdAt: 1710000000, + updatedAt: 1710000012, + }, + }, + }; + + await setupPromptAndSendNotifications(mockFixture, sessionId, createSessionState(), [ + { + method: "item/agentMessage/delta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "message-1", + delta: "Because they kept losing interest in `any`.", + }, + }, + goalCompletedNotification, + ]); + + const events = mockFixture.getAcpConnectionEvents([]); + expect(events).toHaveLength(2); + expect(events[1]!.args[0].update.content.text).toBe("\n\nGoal updated (complete): tell me a joke\n\n"); }); it("should suppress duplicate thread goal cleared notifications", async () => { @@ -141,7 +178,7 @@ describe("CodexEventHandler - thread goal events", () => { const events = mockFixture.getAcpConnectionEvents([]); expect(events).toHaveLength(1); - expect(events[0]!.args[0].update.content.text).toBe("Goal cleared.\n\n"); + expect(events[0]!.args[0].update.content.text).toBe("\n\nGoal cleared.\n\n"); }); function createSessionState(): SessionState { From 5edcfe03e8ac9dd55d551ea452550e71b1cab966 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 15:15:16 +0200 Subject: [PATCH 09/18] Potential fix for pull request finding 'Unused variable, import, function or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/CodexEventHandler.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index c915d625..37384d6c 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -4,7 +4,6 @@ import type { ServerNotification } from "./app-server"; import type {SessionState, ThreadGoalSnapshot} from "./CodexAcpServer"; -import * as acp from "@agentclientprotocol/sdk"; import {type PlanEntry, RequestError} from "@agentclientprotocol/sdk"; import {ACPSessionConnection, type AcpClientConnection, type UpdateSessionEvent} from "./ACPSessionConnection"; import type { From 5f8491aeeaa65621c84387f884e9a70010f29745 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 15:56:56 +0200 Subject: [PATCH 10/18] Interrupt late-started goal commands on cancel --- src/CodexCommands.ts | 31 +++++++++---- .../CodexACPAgent/CodexAcpClient.test.ts | 45 +++++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index 8d9a4f7d..70d9eb2f 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -165,7 +165,7 @@ export class CodexCommands { return { handled: true }; } case "goal": { - return await this.runGoalCommand(sessionState, command.rest); + return await this.runGoalCommand(sessionState, command.rest, options); } case "review": { const target = this.buildReviewTarget(command.rest); @@ -266,16 +266,16 @@ export class CodexCommands { sessionState.sessionId, target, (turnId, threadId) => { - if (options.onTurnStarted) { - options.onTurnStarted(turnId, threadId); - } else { - sessionState.currentTurnId = turnId; - } + this.handleCommandTurnStarted(sessionState, options, turnId, threadId); }, )); } - private async runGoalCommand(sessionState: SessionState, rest: string): Promise { + private async runGoalCommand( + sessionState: SessionState, + rest: string, + options: CommandHandleOptions, + ): Promise { const sessionId = sessionState.sessionId; const argument = rest.trim(); if (argument.length === 0) { @@ -291,7 +291,7 @@ export class CodexCommands { return this.createGoalCommandResult(await this.runWithProcessCheck(() => this.codexAcpClient.resumeGoal( sessionId, (turnId) => { - sessionState.currentTurnId = turnId; + this.handleCommandTurnStarted(sessionState, options, turnId, sessionId); }, ))); case "clear": @@ -315,11 +315,24 @@ export class CodexCommands { sessionId, argument, (turnId) => { - sessionState.currentTurnId = turnId; + this.handleCommandTurnStarted(sessionState, options, turnId, sessionId); }, ))); } + private handleCommandTurnStarted( + sessionState: SessionState, + options: CommandHandleOptions, + turnId: string, + threadId: string, + ): void { + if (options.onTurnStarted) { + options.onTurnStarted(turnId, threadId); + } else { + sessionState.currentTurnId = turnId; + } + } + private createGoalCommandResult(turnCompleted: TurnCompletedNotification | null): CommandHandleResult { if (turnCompleted === null) { return { handled: true }; diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 00e94a24..4bf48f58 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1491,6 +1491,51 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(promptResolved).toBe(true); }); + it('interrupts a late-started goal slash command after the ACP prompt request is cancelled', async () => { + const { mockFixture } = setupPromptFixture(); + const goalCompleted = deferred(); + let startGoalTurn: () => void = () => {}; + const goalRunSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "runGoalSet") + .mockImplementation((_params, onTurnStarted) => { + startGoalTurn = () => onTurnStarted?.("goal-turn-id"); + return goalCompleted.promise; + }); + const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt") + .mockImplementation(async ({threadId, turnId}) => { + goalCompleted.resolve({ + threadId, + turn: createTurn(turnId, "interrupted"), + }); + }); + const controller = new AbortController(); + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }, controller.signal); + + await vi.waitFor(() => { + expect(goalRunSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, expect.any(Function)); + }); + + controller.abort(); + await expect(promptPromise).resolves.toMatchObject({stopReason: "cancelled"}); + expect(turnInterruptSpy).not.toHaveBeenCalled(); + + startGoalTurn(); + + await vi.waitFor(() => { + expect(turnInterruptSpy).toHaveBeenCalledWith({ + threadId: "session-id", + turnId: "goal-turn-id", + }); + }); + }); + it('does not hang when goal set starts no continuation turn', async () => { const mockFixture = createCodexMockTestFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); From f6aa545453626163289609243baaac359efdfbdb Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 21:28:14 +0200 Subject: [PATCH 11/18] Suppress stale turn notifications after routing them --- src/CodexAppServerClient.ts | 28 +++++++---- .../CodexACPAgent/CodexAcpClient.test.ts | 46 +++++++++++++++++++ 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 496dda04..da32ddad 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -144,17 +144,13 @@ export class CodexAppServerClient { this.recordCompactionCompleted(serverNotification); } const routing = extractTurnRouting(serverNotification); - const staleTurnNotification = this.isStaleTurn(routing.threadId, routing.turnId); - if (staleTurnNotification) { - if (isTurnCompletedNotification(serverNotification) && routing.threadId !== null && routing.turnId !== null) { - this.clearStaleTurn(routing.threadId, routing.turnId); - } - for (const callback of this.codexEventHandlers) { - callback({ eventType: "notification", ...serverNotification }); - } + if (this.handleStaleTurnNotification(serverNotification, routing)) { return; } this.recordTurnRouting(routing); + if (this.handleStaleTurnNotification(serverNotification, routing)) { + return; + } this.notify(serverNotification); for (const callback of this.codexEventHandlers) { callback({ eventType: "notification", ...serverNotification }); @@ -554,6 +550,22 @@ export class CodexAppServerClient { } } + private handleStaleTurnNotification( + notification: ServerNotification, + routing: { threadId: string | null, turnId: string | null }, + ): boolean { + if (!this.isStaleTurn(routing.threadId, routing.turnId)) { + return false; + } + if (isTurnCompletedNotification(notification) && routing.threadId !== null && routing.turnId !== null) { + this.clearStaleTurn(routing.threadId, routing.turnId); + } + for (const callback of this.codexEventHandlers) { + callback({ eventType: "notification", ...notification }); + } + return true; + } + private isStaleTurn(threadId: string | null, turnId: string | null): boolean { if (threadId === null || turnId === null) { return false; diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 4bf48f58..2391c148 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1536,6 +1536,52 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); }); + it('suppresses the first routed goal notification after cancellation marks the turn stale', async () => { + const { mockFixture } = setupPromptFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({ goal: createThreadGoal() }); + const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt") + .mockResolvedValue(undefined); + const controller = new AbortController(); + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }, controller.signal); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + controller.abort(); + await expect(promptPromise).resolves.toMatchObject({stopReason: "cancelled"}); + mockFixture.clearAcpConnectionDump(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "leaked goal output", + }, + }); + + await vi.waitFor(() => { + expect(turnInterruptSpy).toHaveBeenCalledWith({ + threadId: "session-id", + turnId: "goal-turn-id", + }); + }); + await flushAsyncWork(); + expect(mockFixture.getAcpConnectionDump([])).not.toContain("leaked goal output"); + }); + it('does not hang when goal set starts no continuation turn', async () => { const mockFixture = createCodexMockTestFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); From 476f879701b5b3f588b95528f4e2495086582d59 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 22:07:33 +0200 Subject: [PATCH 12/18] Wait for thread idle when no goal turn starts --- src/CodexAppServerClient.ts | 71 ++++++++++++++----- .../CodexACPAgent/CodexAcpClient.test.ts | 71 ++++++++++++++++++- 2 files changed, 121 insertions(+), 21 deletions(-) diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index da32ddad..2ac5cc85 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -26,6 +26,8 @@ import type { SkillsExtraRootsSetParams, SkillsListParams, SkillsListResponse, + ThreadStatus, + ThreadStatusChangedNotification, ThreadArchiveParams, ThreadArchiveResponse, ThreadCompactStartParams, @@ -105,8 +107,6 @@ const McpServerElicitationRequest = new RequestType< void >('mcpServer/elicitation/request'); -const GOAL_TURN_START_GRACE_MS = 1_000; - /** * A type-safe client over the Codex App Server's JSON-RPC API. * Maps each request to its expected response and exposes clear, typed methods for supported JSON-RPC operations. @@ -122,6 +122,7 @@ export class CodexAppServerClient { private readonly pendingCompactionCompletionResolvers = new Map void>>(); private readonly turnCompletionCaptures = new Map void>>(); private readonly turnRoutingCaptures = new Map void>>(); + private readonly threadStatusCaptures = new Map void>>(); private readonly staleTurnIds = new Map>(); constructor(connection: MessageConnection) { @@ -143,6 +144,9 @@ export class CodexAppServerClient { if (isCompactionCompletedNotification(serverNotification)) { this.recordCompactionCompleted(serverNotification); } + if (isThreadStatusChangedNotification(serverNotification)) { + this.recordThreadStatusChanged(serverNotification.params); + } const routing = extractTurnRouting(serverNotification); if (this.handleStaleTurnNotification(serverNotification, routing)) { return; @@ -272,7 +276,6 @@ export class CodexAppServerClient { async runGoalSet( params: ThreadGoalSetParams, onTurnStarted?: (turnId: string) => void, - turnStartGraceMs = GOAL_TURN_START_GRACE_MS, ): Promise { const capturedCompletions: Array = []; const releaseCompletionCapture = this.captureTurnCompletions(params.threadId, (event) => { @@ -283,6 +286,10 @@ export class CodexAppServerClient { const goalTurnStarted = new Promise((resolve) => { resolveGoalTurnStarted = resolve; }); + let resolveNoGoalTurnStarted: () => void = () => {}; + const noGoalTurnStarted = new Promise((resolve) => { + resolveNoGoalTurnStarted = () => resolve(null); + }); const releaseRoutingCapture = this.captureTurnRoutings(params.threadId, (turnId) => { if (goalTurnId !== null) { return; @@ -291,16 +298,23 @@ export class CodexAppServerClient { onTurnStarted?.(turnId); resolveGoalTurnStarted(turnId); }); + const releaseStatusCapture = this.captureThreadStatuses(params.threadId, (status) => { + if (goalTurnId !== null || status.type === "active") { + return; + } + resolveNoGoalTurnStarted(); + }); try { await this.threadGoalSet(params); - const turnId = goalTurnId ?? await this.waitForGoalTurnStarted(goalTurnStarted, turnStartGraceMs); + const turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted]); if (turnId === null) { return null; } const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); releaseCompletionCapture(); releaseRoutingCapture(); + releaseStatusCapture(); if (earlyCompletion) { return earlyCompletion; } @@ -308,20 +322,7 @@ export class CodexAppServerClient { } finally { releaseCompletionCapture(); releaseRoutingCapture(); - } - } - - private async waitForGoalTurnStarted(goalTurnStarted: Promise, timeoutMs: number): Promise { - let timeout: ReturnType | null = null; - const timeoutPromise = new Promise((resolve) => { - timeout = setTimeout(() => resolve(null), timeoutMs); - }); - try { - return await Promise.race([goalTurnStarted, timeoutPromise]); - } finally { - if (timeout !== null) { - clearTimeout(timeout); - } + releaseStatusCapture(); } } @@ -537,6 +538,16 @@ export class CodexAppServerClient { } } + private recordThreadStatusChanged(event: ThreadStatusChangedNotification): void { + const captures = this.threadStatusCaptures.get(event.threadId); + if (!captures) { + return; + } + for (const capture of captures) { + capture(event.status); + } + } + private recordTurnRouting(routing: { threadId: string | null, turnId: string | null }): void { if (routing.threadId === null || routing.turnId === null) { return; @@ -628,6 +639,23 @@ export class CodexAppServerClient { }; } + private captureThreadStatuses(threadId: string, capture: (status: ThreadStatus) => void): () => void { + const captures = this.threadStatusCaptures.get(threadId) ?? new Set<(status: ThreadStatus) => void>(); + captures.add(capture); + this.threadStatusCaptures.set(threadId, captures); + let released = false; + return () => { + if (released) { + return; + } + released = true; + captures.delete(capture); + if (captures.size === 0) { + this.threadStatusCaptures.delete(threadId); + } + }; + } + private resolveMcpServerStartupResolvers(): void { const pendingResolvers: Array = []; for (const resolver of this.mcpServerStartupResolvers) { @@ -732,6 +760,13 @@ function isTurnCompletedNotification(notification: ServerNotification): notifica return notification.method === "turn/completed"; } +function isThreadStatusChangedNotification(notification: ServerNotification): notification is { + method: "thread/status/changed"; + params: ThreadStatusChangedNotification; +} { + return notification.method === "thread/status/changed"; +} + function isCompactionCompletedNotification(notification: ServerNotification): notification is CompactionCompletedNotification { if (notification.method === "thread/compacted") { return true; diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 2391c148..2b88a229 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1588,14 +1588,28 @@ describe('ACP server test', { timeout: 40_000 }, () => { const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") .mockResolvedValue({ goal: createThreadGoal() }); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted"); + let resultSettled = false; - const result = await codexAppServerClient.runGoalSet({ + const resultPromise = codexAppServerClient.runGoalSet({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }, undefined, 0); + }).finally(() => { + resultSettled = true; + }); + + await Promise.resolve(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { type: "idle" }, + }, + }); - expect(result).toBeNull(); + await expect(resultPromise).resolves.toBeNull(); expect(threadGoalSetSpy).toHaveBeenCalledWith({ threadId: "session-id", objective: "Ship the migration and keep tests green", @@ -1604,6 +1618,57 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); + it('keeps goal set pending after elapsed startup time until a turn is routed', async () => { + vi.useFakeTimers(); + try { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({ goal: createThreadGoal() }); + const goalCompleted = deferred(); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") + .mockReturnValue(goalCompleted.promise); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }).finally(() => { + resultSettled = true; + }); + + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(10_000); + await Promise.resolve(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "late goal output", + }, + }); + + await Promise.resolve(); + await Promise.resolve(); + expect(awaitTurnCompletedSpy).toHaveBeenCalledWith("session-id", "goal-turn-id"); + + goalCompleted.resolve({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + await expect(resultPromise).resolves.toMatchObject({ + turn: createTurn("goal-turn-id", "completed"), + }); + } finally { + vi.useRealTimers(); + } + }); + it('completes goal slash command when app server starts no continuation turn', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture(); const goalRunSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "runGoalSet") From 239557d0e822554438de60e738c84d59203cfc9d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 22:56:02 +0200 Subject: [PATCH 13/18] Handle cancellation before goal turn routing --- src/CodexAcpServer.ts | 19 +++- src/CodexAppServerClient.ts | 6 ++ src/CodexCommands.ts | 4 + .../CodexACPAgent/CodexAcpClient.test.ts | 102 +++++++++++++++--- 4 files changed, 111 insertions(+), 20 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 20a0ddd5..fbd18d30 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1342,6 +1342,13 @@ export class CodexAcpServer { sessionState.lastTokenUsage = null; const activePrompt = this.trackActivePrompt(params.sessionId); let pendingTurnStart: PendingTurnStart | null = null; + const ensurePendingTurnStart = (): PendingTurnStart => { + if (pendingTurnStart === null) { + pendingTurnStart = this.createPendingTurnStart(); + this.pendingTurnStarts.set(params.sessionId, pendingTurnStart); + } + return pendingTurnStart; + }; const disposePromptRequestCancellation = this.observePromptRequestCancellation(signal, sessionState, activePrompt); try { @@ -1361,6 +1368,9 @@ export class CodexAcpServer { } const commandPromise = this.availableCommands.tryHandleCommand(params.prompt, sessionState, { + onTurnStartPending: () => { + ensurePendingTurnStart(); + }, onTurnStarted: (turnId, threadId) => { const turn = {threadId, turnId}; activePrompt.currentTurn = turn; @@ -1369,6 +1379,7 @@ export class CodexAcpServer { return; } sessionState.currentTurnId = turnId; + pendingTurnStart?.resolve(turnId); }, }); void commandPromise.catch((err) => { @@ -1427,8 +1438,7 @@ export class CodexAcpServer { sessionState.fastModeEnabled, sessionState.currentModelSupportsFast, ); - pendingTurnStart = this.createPendingTurnStart(); - this.pendingTurnStarts.set(params.sessionId, pendingTurnStart); + ensurePendingTurnStart(); const sendPromptPromise = this.runWithProcessCheck( () => this.codexAcpClient.sendPrompt( params, @@ -1490,10 +1500,11 @@ export class CodexAcpServer { logger.log("Prompt completed", {sessionId: params.sessionId}); disposePromptRequestCancellation(); sessionState.currentTurnId = null; - if (pendingTurnStart !== null && this.pendingTurnStarts.get(params.sessionId) === pendingTurnStart) { + const registeredPendingTurnStart = this.pendingTurnStarts.get(params.sessionId); + if (registeredPendingTurnStart !== undefined) { this.pendingTurnStarts.delete(params.sessionId); + registeredPendingTurnStart.resolve(null); } - pendingTurnStart?.resolve(null); activePrompt.complete(); } } diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 2ac5cc85..dcef908e 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -307,6 +307,12 @@ export class CodexAppServerClient { try { await this.threadGoalSet(params); + if (goalTurnId === null) { + const threadResponse = await this.threadRead({ threadId: params.threadId }); + if (goalTurnId === null && threadResponse.thread.status.type !== "active") { + resolveNoGoalTurnStarted(); + } + } const turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted]); if (turnId === null) { return null; diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index 70d9eb2f..b9111ffd 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -18,6 +18,7 @@ export type CommandHandleResult = | { handled: true, turnCompleted?: TurnCompletedNotification }; export type CommandHandleOptions = { + onTurnStartPending?: () => void; onTurnStarted?: (turnId: string, threadId: string) => void; }; @@ -262,6 +263,7 @@ export class CodexCommands { target: ReviewTarget, options: CommandHandleOptions, ): Promise { + options.onTurnStartPending?.(); return await this.runWithProcessCheck(() => this.codexAcpClient.runReview( sessionState.sessionId, target, @@ -288,6 +290,7 @@ export class CodexCommands { await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionId, "paused")); return { handled: true }; case "resume": + options.onTurnStartPending?.(); return this.createGoalCommandResult(await this.runWithProcessCheck(() => this.codexAcpClient.resumeGoal( sessionId, (turnId) => { @@ -311,6 +314,7 @@ export class CodexCommands { return { handled: true }; } + options.onTurnStartPending?.(); return this.createGoalCommandResult(await this.runWithProcessCheck(() => this.codexAcpClient.setGoal( sessionId, argument, diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 2b88a229..f75125ee 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1414,6 +1414,8 @@ describe('ACP server test', { timeout: 40_000 }, () => { const { mockFixture } = setupPromptFixture(); vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") .mockResolvedValue({ goal: createThreadGoal() }); + vi.spyOn(mockFixture.getCodexAppServerClient(), "threadRead") + .mockResolvedValue(createThreadReadResponse("active")); let completeGoal: (value: TurnCompletedNotification) => void = () => {}; const goalCompletedPromise = new Promise((resolve) => { completeGoal = resolve; @@ -1536,11 +1538,77 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); }); + it('interrupts a goal slash command when ACP cancel arrives before the first routed turn', async () => { + const { mockFixture, sessionState } = setupPromptFixture(); + // @ts-expect-error - registering local session state for the ACP cancel path + mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({ goal: createThreadGoal() }); + const threadRead = deferred>(); + vi.spyOn(codexAppServerClient, "threadRead") + .mockReturnValue(threadRead.promise); + const goalCompleted = deferred(); + vi.spyOn(codexAppServerClient, "awaitTurnCompleted") + .mockReturnValue(goalCompleted.promise); + const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt") + .mockImplementation(async ({threadId, turnId}) => { + goalCompleted.resolve({ + threadId, + turn: createTurn(turnId, "interrupted"), + }); + }); + let cancelResolved = false; + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + const cancelPromise = mockFixture.getCodexAcpAgent().cancel({sessionId: "session-id"}) + .then(() => { + cancelResolved = true; + }); + await flushAsyncWork(); + expect(cancelResolved).toBe(false); + + threadRead.resolve(createThreadReadResponse("active")); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "goal output", + }, + }); + + await vi.waitFor(() => { + expect(turnInterruptSpy).toHaveBeenCalledWith({ + threadId: "session-id", + turnId: "goal-turn-id", + }); + }); + await expect(cancelPromise).resolves.toBeUndefined(); + await expect(promptPromise).resolves.toMatchObject({stopReason: "cancelled"}); + }); + it('suppresses the first routed goal notification after cancellation marks the turn stale', async () => { const { mockFixture } = setupPromptFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") .mockResolvedValue({ goal: createThreadGoal() }); + vi.spyOn(codexAppServerClient, "threadRead") + .mockResolvedValue(createThreadReadResponse("active")); const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt") .mockResolvedValue(undefined); const controller = new AbortController(); @@ -1587,34 +1655,23 @@ describe('ACP server test', { timeout: 40_000 }, () => { const codexAppServerClient = mockFixture.getCodexAppServerClient(); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") .mockResolvedValue({ goal: createThreadGoal() }); + const threadReadSpy = vi.spyOn(codexAppServerClient, "threadRead") + .mockResolvedValue(createThreadReadResponse("idle")); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted"); - let resultSettled = false; - const resultPromise = codexAppServerClient.runGoalSet({ + const result = await codexAppServerClient.runGoalSet({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }).finally(() => { - resultSettled = true; }); - await Promise.resolve(); - expect(resultSettled).toBe(false); - - mockFixture.sendServerNotification({ - method: "thread/status/changed", - params: { - threadId: "session-id", - status: { type: "idle" }, - }, - }); - - await expect(resultPromise).resolves.toBeNull(); + expect(result).toBeNull(); expect(threadGoalSetSpy).toHaveBeenCalledWith({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", }); + expect(threadReadSpy).toHaveBeenCalledWith({threadId: "session-id"}); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); @@ -1625,6 +1682,8 @@ describe('ACP server test', { timeout: 40_000 }, () => { const codexAppServerClient = mockFixture.getCodexAppServerClient(); vi.spyOn(codexAppServerClient, "threadGoalSet") .mockResolvedValue({ goal: createThreadGoal() }); + vi.spyOn(codexAppServerClient, "threadRead") + .mockResolvedValue(createThreadReadResponse("active")); const goalCompleted = deferred(); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") .mockReturnValue(goalCompleted.promise); @@ -1985,6 +2044,17 @@ describe('ACP server test', { timeout: 40_000 }, () => { }; } + function createThreadReadResponse(status: "active" | "idle") { + return { + thread: { + id: "session-id", + status: status === "active" + ? { type: "active", activeFlags: [] } + : { type: "idle" }, + } as any, + }; + } + it ('should disable reasoning.summary if key authorization is used', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "apiKey" } }); From 4461c54eeb05d2e1b1b488585dc8f4fb9406c554 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 25 Jun 2026 23:11:04 +0200 Subject: [PATCH 14/18] Wait for goal updates before finishing goal set --- src/CodexAppServerClient.ts | 86 +++++++++++- .../CodexACPAgent/CodexAcpClient.test.ts | 132 +++++++++++++++++- 2 files changed, 209 insertions(+), 9 deletions(-) diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index dcef908e..aa3c2dc4 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -26,6 +26,8 @@ import type { SkillsExtraRootsSetParams, SkillsListParams, SkillsListResponse, + ThreadGoal, + ThreadGoalUpdatedNotification, ThreadStatus, ThreadStatusChangedNotification, ThreadArchiveParams, @@ -123,6 +125,7 @@ export class CodexAppServerClient { private readonly turnCompletionCaptures = new Map void>>(); private readonly turnRoutingCaptures = new Map void>>(); private readonly threadStatusCaptures = new Map void>>(); + private readonly threadGoalUpdateCaptures = new Map void>>(); private readonly staleTurnIds = new Map>(); constructor(connection: MessageConnection) { @@ -147,6 +150,9 @@ export class CodexAppServerClient { if (isThreadStatusChangedNotification(serverNotification)) { this.recordThreadStatusChanged(serverNotification.params); } + if (isThreadGoalUpdatedNotification(serverNotification)) { + this.recordThreadGoalUpdated(serverNotification.params); + } const routing = extractTurnRouting(serverNotification); if (this.handleStaleTurnNotification(serverNotification, routing)) { return; @@ -290,6 +296,13 @@ export class CodexAppServerClient { const noGoalTurnStarted = new Promise((resolve) => { resolveNoGoalTurnStarted = () => resolve(null); }); + let acceptNoGoalTurnStatus = false; + let expectedGoal: ThreadGoal | null = null; + let resolveGoalUpdated: () => void = () => {}; + const goalUpdated = new Promise((resolve) => { + resolveGoalUpdated = resolve; + }); + const capturedGoalUpdates: Array = []; const releaseRoutingCapture = this.captureTurnRoutings(params.threadId, (turnId) => { if (goalTurnId !== null) { return; @@ -299,28 +312,48 @@ export class CodexAppServerClient { resolveGoalTurnStarted(turnId); }); const releaseStatusCapture = this.captureThreadStatuses(params.threadId, (status) => { - if (goalTurnId !== null || status.type === "active") { + if (!acceptNoGoalTurnStatus || goalTurnId !== null || status.type === "active") { return; } resolveNoGoalTurnStarted(); }); + const releaseGoalUpdateCapture = this.captureThreadGoalUpdates(params.threadId, (event) => { + capturedGoalUpdates.push(event); + if (expectedGoal !== null && goalsMatch(event.goal, expectedGoal)) { + resolveGoalUpdated(); + } + }); try { - await this.threadGoalSet(params); + const goalSetResponse = await this.threadGoalSet(params); + expectedGoal = goalSetResponse.goal; + if (capturedGoalUpdates.some(event => goalsMatch(event.goal, expectedGoal!))) { + resolveGoalUpdated(); + } if (goalTurnId === null) { const threadResponse = await this.threadRead({ threadId: params.threadId }); if (goalTurnId === null && threadResponse.thread.status.type !== "active") { - resolveNoGoalTurnStarted(); + await goalUpdated; + if (goalTurnId === null) { + resolveNoGoalTurnStarted(); + } + } else { + acceptNoGoalTurnStatus = true; } } - const turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted]); + let turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted]); if (turnId === null) { - return null; + await goalUpdated; + turnId = goalTurnId; + if (turnId === null) { + return null; + } } const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); releaseCompletionCapture(); releaseRoutingCapture(); releaseStatusCapture(); + releaseGoalUpdateCapture(); if (earlyCompletion) { return earlyCompletion; } @@ -329,6 +362,7 @@ export class CodexAppServerClient { releaseCompletionCapture(); releaseRoutingCapture(); releaseStatusCapture(); + releaseGoalUpdateCapture(); } } @@ -554,6 +588,16 @@ export class CodexAppServerClient { } } + private recordThreadGoalUpdated(event: ThreadGoalUpdatedNotification): void { + const captures = this.threadGoalUpdateCaptures.get(event.threadId); + if (!captures) { + return; + } + for (const capture of captures) { + capture(event); + } + } + private recordTurnRouting(routing: { threadId: string | null, turnId: string | null }): void { if (routing.threadId === null || routing.turnId === null) { return; @@ -662,6 +706,23 @@ export class CodexAppServerClient { }; } + private captureThreadGoalUpdates(threadId: string, capture: (event: ThreadGoalUpdatedNotification) => void): () => void { + const captures = this.threadGoalUpdateCaptures.get(threadId) ?? new Set<(event: ThreadGoalUpdatedNotification) => void>(); + captures.add(capture); + this.threadGoalUpdateCaptures.set(threadId, captures); + let released = false; + return () => { + if (released) { + return; + } + released = true; + captures.delete(capture); + if (captures.size === 0) { + this.threadGoalUpdateCaptures.delete(threadId); + } + }; + } + private resolveMcpServerStartupResolvers(): void { const pendingResolvers: Array = []; for (const resolver of this.mcpServerStartupResolvers) { @@ -773,6 +834,13 @@ function isThreadStatusChangedNotification(notification: ServerNotification): no return notification.method === "thread/status/changed"; } +function isThreadGoalUpdatedNotification(notification: ServerNotification): notification is { + method: "thread/goal/updated"; + params: ThreadGoalUpdatedNotification; +} { + return notification.method === "thread/goal/updated"; +} + function isCompactionCompletedNotification(notification: ServerNotification): notification is CompactionCompletedNotification { if (notification.method === "thread/compacted") { return true; @@ -780,6 +848,14 @@ function isCompactionCompletedNotification(notification: ServerNotification): no return notification.method === "item/completed" && notification.params.item.type === "contextCompaction"; } +function goalsMatch(left: ThreadGoal, right: ThreadGoal): boolean { + return left.threadId === right.threadId + && left.objective === right.objective + && left.status === right.status + && left.tokenBudget === right.tokenBudget + && left.updatedAt === right.updatedAt; +} + function extractThreadId(notification: ServerNotification): string | null { const params = notification.params as { threadId?: unknown } | undefined; if (params && typeof params.threadId === "string") { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index f75125ee..13d3dc47 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1493,6 +1493,112 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(promptResolved).toBe(true); }); + it('does not complete no-turn goal slash command before the goal update notification is handled', async () => { + const { mockFixture } = setupPromptFixture(); + const goal = createThreadGoal({updatedAt: 1710000100}); + const threadGoalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + .mockResolvedValue({ goal }); + vi.spyOn(mockFixture.getCodexAppServerClient(), "threadRead") + .mockResolvedValue(createThreadReadResponse("idle")); + let promptResolved = false; + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }).then((response) => { + promptResolved = true; + return response; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + await flushAsyncWork(); + expect(promptResolved).toBe(false); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + + await expect(promptPromise).resolves.toEqual(expect.objectContaining({ + stopReason: "end_turn", + })); + expect(mockFixture.getAcpConnectionDump([])).toContain("Goal updated (active): Ship the migration and keep tests green"); + }); + + it('ignores stale idle status while goal set is in flight when thread later reads active', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000200}); + const threadGoalSet = deferred<{goal: ThreadGoal}>(); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockReturnValue(threadGoalSet.promise); + vi.spyOn(codexAppServerClient, "threadRead") + .mockResolvedValue(createThreadReadResponse("active")); + const goalCompleted = deferred(); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") + .mockReturnValue(goalCompleted.promise); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { type: "idle" }, + }, + }); + threadGoalSet.resolve({goal}); + await flushAsyncWork(); + expect(resultSettled).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "late goal output", + }, + }); + + await vi.waitFor(() => { + expect(awaitTurnCompletedSpy).toHaveBeenCalledWith("session-id", "goal-turn-id"); + }); + goalCompleted.resolve({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + await expect(resultPromise).resolves.toMatchObject({ + turn: createTurn("goal-turn-id", "completed"), + }); + }); + it('interrupts a late-started goal slash command after the ACP prompt request is cancelled', async () => { const { mockFixture } = setupPromptFixture(); const goalCompleted = deferred(); @@ -1653,25 +1759,43 @@ describe('ACP server test', { timeout: 40_000 }, () => { it('does not hang when goal set starts no continuation turn', async () => { const mockFixture = createCodexMockTestFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000300}); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") - .mockResolvedValue({ goal: createThreadGoal() }); + .mockResolvedValue({ goal }); const threadReadSpy = vi.spyOn(codexAppServerClient, "threadRead") .mockResolvedValue(createThreadReadResponse("idle")); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted"); + let resultSettled = false; - const result = await codexAppServerClient.runGoalSet({ + const resultPromise = codexAppServerClient.runGoalSet({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", + }).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadReadSpy).toHaveBeenCalledWith({threadId: "session-id"}); + }); + await flushAsyncWork(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, }); - expect(result).toBeNull(); + await expect(resultPromise).resolves.toBeNull(); expect(threadGoalSetSpy).toHaveBeenCalledWith({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", }); - expect(threadReadSpy).toHaveBeenCalledWith({threadId: "session-id"}); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); From 37bcbdcba839266447623439a3cb2e9192458855 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 26 Jun 2026 15:03:44 +0200 Subject: [PATCH 15/18] Add grace periods for goal turn routing --- src/CodexAppServerClient.ts | 175 ++++++-- .../CodexACPAgent/CodexAcpClient.test.ts | 424 ++++++++++++++---- 2 files changed, 473 insertions(+), 126 deletions(-) diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index aa3c2dc4..0e3da177 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -109,6 +109,9 @@ const McpServerElicitationRequest = new RequestType< void >('mcpServer/elicitation/request'); +const GOAL_RUNTIME_EFFECTS_GRACE_MS = 1_000; +const GOAL_TURN_COMPLETION_GRACE_MS = 250; + /** * A type-safe client over the Codex App Server's JSON-RPC API. * Maps each request to its expected response and exposes clear, typed methods for supported JSON-RPC operations. @@ -282,83 +285,78 @@ export class CodexAppServerClient { async runGoalSet( params: ThreadGoalSetParams, onTurnStarted?: (turnId: string) => void, + runtimeEffectsGraceMs = GOAL_RUNTIME_EFFECTS_GRACE_MS, + turnCompletionGraceMs = GOAL_TURN_COMPLETION_GRACE_MS, ): Promise { + let goalTurnId: string | null = null; const capturedCompletions: Array = []; + let resolveGoalTurnCompleted: (event: TurnCompletedNotification) => void = () => {}; + const goalTurnCompleted = new Promise((resolve) => { + resolveGoalTurnCompleted = resolve; + }); const releaseCompletionCapture = this.captureTurnCompletions(params.threadId, (event) => { capturedCompletions.push(event); + if (goalTurnId === event.turn.id) { + resolveGoalTurnCompleted(event); + } }); - let goalTurnId: string | null = null; let resolveGoalTurnStarted: (turnId: string) => void = () => {}; const goalTurnStarted = new Promise((resolve) => { resolveGoalTurnStarted = resolve; }); - let resolveNoGoalTurnStarted: () => void = () => {}; - const noGoalTurnStarted = new Promise((resolve) => { - resolveNoGoalTurnStarted = () => resolve(null); - }); - let acceptNoGoalTurnStatus = false; + let goalUpdateHandled = false; let expectedGoal: ThreadGoal | null = null; - let resolveGoalUpdated: () => void = () => {}; - const goalUpdated = new Promise((resolve) => { - resolveGoalUpdated = resolve; - }); + const noGoalTurnStarted = this.createNoGoalTurnStartedPromise(runtimeEffectsGraceMs); const capturedGoalUpdates: Array = []; const releaseRoutingCapture = this.captureTurnRoutings(params.threadId, (turnId) => { - if (goalTurnId !== null) { + if (!goalUpdateHandled || goalTurnId !== null) { return; } goalTurnId = turnId; onTurnStarted?.(turnId); resolveGoalTurnStarted(turnId); }); - const releaseStatusCapture = this.captureThreadStatuses(params.threadId, (status) => { - if (!acceptNoGoalTurnStatus || goalTurnId !== null || status.type === "active") { - return; - } - resolveNoGoalTurnStarted(); - }); const releaseGoalUpdateCapture = this.captureThreadGoalUpdates(params.threadId, (event) => { capturedGoalUpdates.push(event); if (expectedGoal !== null && goalsMatch(event.goal, expectedGoal)) { - resolveGoalUpdated(); + goalUpdateHandled = true; + noGoalTurnStarted.goalUpdated(); + } + }); + const releaseStatusCapture = this.captureThreadStatuses(params.threadId, (status) => { + if (!goalUpdateHandled || goalTurnId !== null) { + return; } + noGoalTurnStarted.threadStatusChanged(status); }); try { const goalSetResponse = await this.threadGoalSet(params); expectedGoal = goalSetResponse.goal; if (capturedGoalUpdates.some(event => goalsMatch(event.goal, expectedGoal!))) { - resolveGoalUpdated(); - } - if (goalTurnId === null) { - const threadResponse = await this.threadRead({ threadId: params.threadId }); - if (goalTurnId === null && threadResponse.thread.status.type !== "active") { - await goalUpdated; - if (goalTurnId === null) { - resolveNoGoalTurnStarted(); - } - } else { - acceptNoGoalTurnStatus = true; - } - } - let turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted]); - if (turnId === null) { - await goalUpdated; - turnId = goalTurnId; - if (turnId === null) { - return null; - } + goalUpdateHandled = true; + noGoalTurnStarted.goalUpdated(); } - const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); - releaseCompletionCapture(); + const turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted.promise]); + noGoalTurnStarted.release(); releaseRoutingCapture(); releaseStatusCapture(); releaseGoalUpdateCapture(); + if (turnId === null) { + return null; + } + const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); if (earlyCompletion) { return earlyCompletion; } - return await this.awaitTurnCompleted(params.threadId, turnId); + const completionGrace = this.createGoalTurnCompletionGracePromise(turnCompletionGraceMs); + try { + return await Promise.race([goalTurnCompleted, completionGrace.promise]); + } finally { + completionGrace.release(); + } } finally { + noGoalTurnStarted.release(); releaseCompletionCapture(); releaseRoutingCapture(); releaseStatusCapture(); @@ -366,6 +364,101 @@ export class CodexAppServerClient { } } + private createNoGoalTurnStartedPromise( + runtimeEffectsGraceMs: number, + ): { + promise: Promise, + release: () => void, + goalUpdated: () => void, + threadStatusChanged: (status: ThreadStatus) => void, + } { + let released = false; + let resolved = false; + let goalUpdated = false; + let activeAfterGoalUpdate = false; + let timeout: ReturnType | null = null; + let resolveNoGoalTurnStarted: () => void = () => {}; + const clearTimer = () => { + if (timeout !== null) { + clearTimeout(timeout); + timeout = null; + } + }; + const resolveNoTurn = () => { + if (released || resolved) { + return; + } + resolved = true; + clearTimer(); + resolveNoGoalTurnStarted(); + }; + const scheduleNoTurnTimer = () => { + if (released || resolved || !goalUpdated || activeAfterGoalUpdate || timeout !== null) { + return; + } + timeout = setTimeout(resolveNoTurn, runtimeEffectsGraceMs); + }; + const release = () => { + if (released) { + return; + } + released = true; + clearTimer(); + }; + const promise = new Promise((resolve) => { + resolveNoGoalTurnStarted = () => { + resolve(null); + }; + }); + const handleGoalUpdated = () => { + goalUpdated = true; + scheduleNoTurnTimer(); + }; + const handleThreadStatusChanged = (status: ThreadStatus) => { + if (!goalUpdated || released || resolved) { + return; + } + if (status.type === "active") { + activeAfterGoalUpdate = true; + clearTimer(); + return; + } + if (activeAfterGoalUpdate) { + resolveNoTurn(); + } + }; + return { + promise, + release, + goalUpdated: handleGoalUpdated, + threadStatusChanged: handleThreadStatusChanged, + }; + } + + private createGoalTurnCompletionGracePromise( + turnCompletionGraceMs: number, + ): { promise: Promise, release: () => void } { + let released = false; + let timeout: ReturnType | null = null; + const release = () => { + if (released) { + return; + } + released = true; + if (timeout !== null) { + clearTimeout(timeout); + } + }; + const promise = new Promise((resolve) => { + const resolveNoGoalTurnCompleted = () => { + timeout = null; + resolve(null); + }; + timeout = setTimeout(resolveNoGoalTurnCompleted, turnCompletionGraceMs); + }); + return {promise, release}; + } + async runCompact(params: ThreadCompactStartParams): Promise { const compactionCompleted = this.awaitCompactionCompleted(params.threadId); await this.threadCompactStart(params); diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index fcb51035..d06b107d 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1508,18 +1508,12 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(turnStartSpy).not.toHaveBeenCalled(); }); - it('waits for goal slash command turn completion', async () => { + it('waits for goal slash command turn routing', async () => { const { mockFixture } = setupPromptFixture(); + const goal = createThreadGoal(); vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") - .mockResolvedValue({ goal: createThreadGoal() }); - vi.spyOn(mockFixture.getCodexAppServerClient(), "threadRead") - .mockResolvedValue(createThreadReadResponse("active")); - let completeGoal: (value: TurnCompletedNotification) => void = () => {}; - const goalCompletedPromise = new Promise((resolve) => { - completeGoal = resolve; - }); - vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted") - .mockReturnValue(goalCompletedPromise); + .mockResolvedValue({ goal }); + const awaitTurnCompletedSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted"); let promptResolved = false; const promptPromise = mockFixture.getCodexAcpAgent().prompt({ @@ -1546,7 +1540,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { params: { threadId: "session-id", turnId: null, - goal: createThreadGoal(), + goal, }, }); mockFixture.sendServerNotification({ @@ -1573,31 +1567,19 @@ describe('ACP server test', { timeout: 40_000 }, () => { }, }); await vi.waitFor(() => { - expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).toHaveBeenCalledWith( - "session-id", - "goal-turn-id", - ); - }); - await Promise.resolve(); - expect(promptResolved).toBe(false); - - completeGoal({ - threadId: "session-id", - turn: createTurn("goal-turn-id", "completed"), + expect(promptResolved).toBe(true); }); await expect(promptPromise).resolves.toEqual(expect.objectContaining({ stopReason: "end_turn", })); - expect(promptResolved).toBe(true); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); - it('does not complete no-turn goal slash command before the goal update notification is handled', async () => { + it('does not complete no-turn goal slash command before the goal update and runtime grace are handled', async () => { const { mockFixture } = setupPromptFixture(); const goal = createThreadGoal({updatedAt: 1710000100}); const threadGoalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") .mockResolvedValue({ goal }); - vi.spyOn(mockFixture.getCodexAppServerClient(), "threadRead") - .mockResolvedValue(createThreadReadResponse("idle")); let promptResolved = false; const promptPromise = mockFixture.getCodexAcpAgent().prompt({ @@ -1627,31 +1609,159 @@ describe('ACP server test', { timeout: 40_000 }, () => { }, }); + await flushAsyncWork(); + expect(promptResolved).toBe(false); + await expect(promptPromise).resolves.toEqual(expect.objectContaining({ stopReason: "end_turn", })); expect(mockFixture.getAcpConnectionDump([])).toContain("Goal updated (active): Ship the migration and keep tests green"); }); - it('ignores stale idle status while goal set is in flight when thread later reads active', async () => { + it('completes goal slash command when a turn routes after the goal update', async () => { + const { mockFixture } = setupPromptFixture(); + const goal = createThreadGoal({updatedAt: 1710000150}); + const threadGoalSetSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + .mockResolvedValue({ goal }); + const awaitTurnCompletedSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted") + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + let promptResolved = false; + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal Ship the migration and keep tests green" }], + }).then((response) => { + promptResolved = true; + return response; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + + await flushAsyncWork(); + expect(promptResolved).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "late goal output", + }, + }); + + await expect(promptPromise).resolves.toEqual(expect.objectContaining({ + stopReason: "end_turn", + })); + expect(promptResolved).toBe(true); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + }); + + it('does not start the no-turn grace period before the goal update is handled', async () => { + vi.useFakeTimers(); + try { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000200}); + const threadGoalSet = deferred<{goal: ThreadGoal}>(); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockReturnValue(threadGoalSet.promise); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, undefined, undefined, 0).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + threadGoalSet.resolve({goal}); + await vi.advanceTimersByTimeAsync(10_000); + await Promise.resolve(); + expect(resultSettled).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + await Promise.resolve(); + expect(resultSettled).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "late goal output", + }, + }); + + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toBeNull(); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it('completes goal set when a turn routes after the goal update', async () => { const mockFixture = createCodexMockTestFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); const goal = createThreadGoal({updatedAt: 1710000200}); - const threadGoalSet = deferred<{goal: ThreadGoal}>(); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") - .mockReturnValue(threadGoalSet.promise); - vi.spyOn(codexAppServerClient, "threadRead") - .mockResolvedValue(createThreadReadResponse("active")); - const goalCompleted = deferred(); + .mockResolvedValue({goal}); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") - .mockReturnValue(goalCompleted.promise); + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); let resultSettled = false; const resultPromise = codexAppServerClient.runGoalSet({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }).finally(() => { + }, undefined, undefined, 0).finally(() => { resultSettled = true; }); @@ -1664,13 +1774,13 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); mockFixture.sendServerNotification({ - method: "thread/status/changed", + method: "thread/goal/updated", params: { threadId: "session-id", - status: { type: "idle" }, + turnId: null, + goal, }, }); - threadGoalSet.resolve({goal}); await flushAsyncWork(); expect(resultSettled).toBe(false); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); @@ -1685,16 +1795,144 @@ describe('ACP server test', { timeout: 40_000 }, () => { }, }); - await vi.waitFor(() => { - expect(awaitTurnCompletedSpy).toHaveBeenCalledWith("session-id", "goal-turn-id"); - }); - goalCompleted.resolve({ - threadId: "session-id", - turn: createTurn("goal-turn-id", "completed"), - }); - await expect(resultPromise).resolves.toMatchObject({ - turn: createTurn("goal-turn-id", "completed"), - }); + await expect(resultPromise).resolves.toBeNull(); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + }); + + it('keeps goal set pending while the thread is active before a turn routes', async () => { + vi.useFakeTimers(); + try { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000225}); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({goal}); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, undefined, undefined, 0).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { + type: "active", + activeFlags: [], + }, + }, + }); + + await vi.advanceTimersByTimeAsync(10_000); + await Promise.resolve(); + expect(resultSettled).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "late goal output", + }, + }); + + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toBeNull(); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it('completes goal set when active thread returns idle without routing a turn', async () => { + vi.useFakeTimers(); + try { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000250}); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({goal}); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, undefined, undefined, 0).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { + type: "active", + activeFlags: [], + }, + }, + }); + + await vi.advanceTimersByTimeAsync(10_000); + await Promise.resolve(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { type: "idle" }, + }, + }); + + await expect(resultPromise).resolves.toBeNull(); + } finally { + vi.useRealTimers(); + } }); it('interrupts a late-started goal slash command after the ACP prompt request is cancelled', async () => { @@ -1747,19 +1985,17 @@ describe('ACP server test', { timeout: 40_000 }, () => { // @ts-expect-error - registering local session state for the ACP cancel path mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal(); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") - .mockResolvedValue({ goal: createThreadGoal() }); - const threadRead = deferred>(); - vi.spyOn(codexAppServerClient, "threadRead") - .mockReturnValue(threadRead.promise); - const goalCompleted = deferred(); - vi.spyOn(codexAppServerClient, "awaitTurnCompleted") - .mockReturnValue(goalCompleted.promise); + .mockResolvedValue({ goal }); const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt") .mockImplementation(async ({threadId, turnId}) => { - goalCompleted.resolve({ - threadId, - turn: createTurn(turnId, "interrupted"), + mockFixture.sendServerNotification({ + method: "turn/completed", + params: { + threadId, + turn: createTurn(turnId, "interrupted"), + }, }); }); let cancelResolved = false; @@ -1784,7 +2020,14 @@ describe('ACP server test', { timeout: 40_000 }, () => { await flushAsyncWork(); expect(cancelResolved).toBe(false); - threadRead.resolve(createThreadReadResponse("active")); + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); mockFixture.sendServerNotification({ method: "item/agentMessage/delta", @@ -1809,10 +2052,9 @@ describe('ACP server test', { timeout: 40_000 }, () => { it('suppresses the first routed goal notification after cancellation marks the turn stale', async () => { const { mockFixture } = setupPromptFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal(); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") - .mockResolvedValue({ goal: createThreadGoal() }); - vi.spyOn(codexAppServerClient, "threadRead") - .mockResolvedValue(createThreadReadResponse("active")); + .mockResolvedValue({ goal }); const turnInterruptSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "turnInterrupt") .mockResolvedValue(undefined); const controller = new AbortController(); @@ -1834,6 +2076,15 @@ describe('ACP server test', { timeout: 40_000 }, () => { await expect(promptPromise).resolves.toMatchObject({stopReason: "cancelled"}); mockFixture.clearAcpConnectionDump(); + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + mockFixture.sendServerNotification({ method: "item/agentMessage/delta", params: { @@ -1860,8 +2111,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { const goal = createThreadGoal({updatedAt: 1710000300}); const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") .mockResolvedValue({ goal }); - const threadReadSpy = vi.spyOn(codexAppServerClient, "threadRead") - .mockResolvedValue(createThreadReadResponse("idle")); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted"); let resultSettled = false; @@ -1869,12 +2118,16 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }).finally(() => { + }, undefined, 0).finally(() => { resultSettled = true; }); await vi.waitFor(() => { - expect(threadReadSpy).toHaveBeenCalledWith({threadId: "session-id"}); + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); }); await flushAsyncWork(); expect(resultSettled).toBe(false); @@ -1889,11 +2142,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); await expect(resultPromise).resolves.toBeNull(); - expect(threadGoalSetSpy).toHaveBeenCalledWith({ - threadId: "session-id", - objective: "Ship the migration and keep tests green", - status: "active", - }); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); @@ -1902,20 +2150,21 @@ describe('ACP server test', { timeout: 40_000 }, () => { try { const mockFixture = createCodexMockTestFixture(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000400}); vi.spyOn(codexAppServerClient, "threadGoalSet") - .mockResolvedValue({ goal: createThreadGoal() }); - vi.spyOn(codexAppServerClient, "threadRead") - .mockResolvedValue(createThreadReadResponse("active")); - const goalCompleted = deferred(); + .mockResolvedValue({ goal }); const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") - .mockReturnValue(goalCompleted.promise); + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); let resultSettled = false; const resultPromise = codexAppServerClient.runGoalSet({ threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }).finally(() => { + }, undefined, undefined, 0).finally(() => { resultSettled = true; }); @@ -1924,6 +2173,17 @@ describe('ACP server test', { timeout: 40_000 }, () => { await Promise.resolve(); expect(resultSettled).toBe(false); + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + await Promise.resolve(); + expect(resultSettled).toBe(false); + mockFixture.sendServerNotification({ method: "item/agentMessage/delta", params: { @@ -1936,15 +2196,9 @@ describe('ACP server test', { timeout: 40_000 }, () => { await Promise.resolve(); await Promise.resolve(); - expect(awaitTurnCompletedSpy).toHaveBeenCalledWith("session-id", "goal-turn-id"); - - goalCompleted.resolve({ - threadId: "session-id", - turn: createTurn("goal-turn-id", "completed"), - }); - await expect(resultPromise).resolves.toMatchObject({ - turn: createTurn("goal-turn-id", "completed"), - }); + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toBeNull(); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); } From a7e480867b864357b09ebf8a2046caf0c522d728 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 26 Jun 2026 15:58:14 +0200 Subject: [PATCH 16/18] Wait for goal notifications in ACP client --- src/CodexAcpClient.ts | 4 +- src/CodexAppServerClient.ts | 221 +++++++++++++++--- .../CodexACPAgent/CodexAcpClient.test.ts | 179 +++++++++++++- 3 files changed, 359 insertions(+), 45 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 576e62c3..d82f0725 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -343,7 +343,7 @@ export class CodexAcpClient { } async setGoalStatus(sessionId: string, status: ThreadGoalStatus): Promise { - await this.codexClient.threadGoalSet({ + await this.codexClient.runGoalSet({ threadId: sessionId, status, }); @@ -360,7 +360,7 @@ export class CodexAcpClient { } async clearGoal(sessionId: string): Promise { - await this.codexClient.threadGoalClear({threadId: sessionId}); + await this.codexClient.runGoalClear({threadId: sessionId}); } async awaitMcpServerStartup(serverNames: Array, afterVersion: number): Promise { diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 0e3da177..f0ccaba7 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -34,6 +34,7 @@ import type { ThreadArchiveResponse, ThreadCompactStartParams, ThreadCompactStartResponse, + ThreadGoalClearedNotification, ThreadGoalClearParams, ThreadGoalClearResponse, ThreadGoalSetParams, @@ -85,6 +86,11 @@ export type McpStartupResult = { cancelled: Array; }; +type ServerNotificationItemType = Extract< + ServerNotification, + { params: { item: { type: unknown } } } +>["params"]["item"]["type"]; + const CommandExecutionApprovalRequest = new RequestType< CommandExecutionRequestApprovalParams, CommandExecutionRequestApprovalResponse, @@ -110,7 +116,45 @@ const McpServerElicitationRequest = new RequestType< >('mcpServer/elicitation/request'); const GOAL_RUNTIME_EFFECTS_GRACE_MS = 1_000; -const GOAL_TURN_COMPLETION_GRACE_MS = 250; +const VISIBLE_TURN_ACTIVITY_METHODS: ReadonlySet = new Set([ + "error", + "turn/plan/updated", + "item/autoApprovalReview/started", + "item/autoApprovalReview/completed", + "item/agentMessage/delta", + "item/commandExecution/outputDelta", + "item/commandExecution/terminalInteraction", + "item/mcpToolCall/progress", + "item/reasoning/summaryTextDelta", + "item/reasoning/summaryPartAdded", + "item/reasoning/textDelta", + "model/rerouted", + "warning", + "configWarning", + "fuzzyFileSearch/sessionUpdated", + "fuzzyFileSearch/sessionCompleted", +]); +const VISIBLE_STARTED_ITEM_TYPES: ReadonlySet = new Set([ + "fileChange", + "commandExecution", + "mcpToolCall", + "dynamicToolCall", + "webSearch", + "imageView", + "imageGeneration", + "collabAgentToolCall", +]); +const VISIBLE_COMPLETED_ITEM_TYPES: ReadonlySet = new Set([ + "fileChange", + "commandExecution", + "mcpToolCall", + "dynamicToolCall", + "webSearch", + "imageView", + "imageGeneration", + "collabAgentToolCall", + "contextCompaction", +]); /** * A type-safe client over the Codex App Server's JSON-RPC API. @@ -127,8 +171,10 @@ export class CodexAppServerClient { private readonly pendingCompactionCompletionResolvers = new Map void>>(); private readonly turnCompletionCaptures = new Map void>>(); private readonly turnRoutingCaptures = new Map void>>(); + private readonly turnActivityCaptures = new Map void>>(); private readonly threadStatusCaptures = new Map void>>(); private readonly threadGoalUpdateCaptures = new Map void>>(); + private readonly threadGoalClearedCaptures = new Map void>>(); private readonly staleTurnIds = new Map>(); constructor(connection: MessageConnection) { @@ -156,6 +202,9 @@ export class CodexAppServerClient { if (isThreadGoalUpdatedNotification(serverNotification)) { this.recordThreadGoalUpdated(serverNotification.params); } + if (isThreadGoalClearedNotification(serverNotification)) { + this.recordThreadGoalCleared(serverNotification.params); + } const routing = extractTurnRouting(serverNotification); if (this.handleStaleTurnNotification(serverNotification, routing)) { return; @@ -164,6 +213,7 @@ export class CodexAppServerClient { if (this.handleStaleTurnNotification(serverNotification, routing)) { return; } + this.recordVisibleTurnActivity(serverNotification, routing); this.notify(serverNotification); for (const callback of this.codexEventHandlers) { callback({ eventType: "notification", ...serverNotification }); @@ -286,7 +336,6 @@ export class CodexAppServerClient { params: ThreadGoalSetParams, onTurnStarted?: (turnId: string) => void, runtimeEffectsGraceMs = GOAL_RUNTIME_EFFECTS_GRACE_MS, - turnCompletionGraceMs = GOAL_TURN_COMPLETION_GRACE_MS, ): Promise { let goalTurnId: string | null = null; const capturedCompletions: Array = []; @@ -304,6 +353,14 @@ export class CodexAppServerClient { const goalTurnStarted = new Promise((resolve) => { resolveGoalTurnStarted = resolve; }); + let resolveGoalUpdateHandled: () => void = () => {}; + const matchingGoalUpdateHandled = new Promise((resolve) => { + resolveGoalUpdateHandled = () => resolve(null); + }); + let resolveGoalTurnActivity: () => void = () => {}; + const goalTurnActivity = new Promise((resolve) => { + resolveGoalTurnActivity = () => resolve(null); + }); let goalUpdateHandled = false; let expectedGoal: ThreadGoal | null = null; const noGoalTurnStarted = this.createNoGoalTurnStartedPromise(runtimeEffectsGraceMs); @@ -316,10 +373,17 @@ export class CodexAppServerClient { onTurnStarted?.(turnId); resolveGoalTurnStarted(turnId); }); + const releaseActivityCapture = this.captureTurnActivities(params.threadId, (turnId) => { + if (!goalUpdateHandled || goalTurnId !== turnId) { + return; + } + resolveGoalTurnActivity(); + }); const releaseGoalUpdateCapture = this.captureThreadGoalUpdates(params.threadId, (event) => { capturedGoalUpdates.push(event); if (expectedGoal !== null && goalsMatch(event.goal, expectedGoal)) { goalUpdateHandled = true; + resolveGoalUpdateHandled(); noGoalTurnStarted.goalUpdated(); } }); @@ -335,35 +399,60 @@ export class CodexAppServerClient { expectedGoal = goalSetResponse.goal; if (capturedGoalUpdates.some(event => goalsMatch(event.goal, expectedGoal!))) { goalUpdateHandled = true; + resolveGoalUpdateHandled(); noGoalTurnStarted.goalUpdated(); } + if (expectedGoal.status !== "active") { + await matchingGoalUpdateHandled; + return null; + } const turnId = goalTurnId ?? await Promise.race([goalTurnStarted, noGoalTurnStarted.promise]); noGoalTurnStarted.release(); releaseRoutingCapture(); releaseStatusCapture(); releaseGoalUpdateCapture(); if (turnId === null) { + releaseActivityCapture(); return null; } const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); if (earlyCompletion) { + releaseActivityCapture(); return earlyCompletion; } - const completionGrace = this.createGoalTurnCompletionGracePromise(turnCompletionGraceMs); - try { - return await Promise.race([goalTurnCompleted, completionGrace.promise]); - } finally { - completionGrace.release(); - } + return await Promise.race([goalTurnCompleted, goalTurnActivity]); } finally { noGoalTurnStarted.release(); releaseCompletionCapture(); releaseRoutingCapture(); + releaseActivityCapture(); releaseStatusCapture(); releaseGoalUpdateCapture(); } } + async runGoalClear(params: ThreadGoalClearParams): Promise { + let goalClearedHandled = false; + let resolveGoalClearedHandled: () => void = () => {}; + const matchingGoalClearedHandled = new Promise((resolve) => { + resolveGoalClearedHandled = () => resolve(); + }); + const releaseGoalClearedCapture = this.captureThreadGoalClears(params.threadId, () => { + goalClearedHandled = true; + resolveGoalClearedHandled(); + }); + + try { + const response = await this.threadGoalClear(params); + if (!response.cleared || goalClearedHandled) { + return; + } + await matchingGoalClearedHandled; + } finally { + releaseGoalClearedCapture(); + } + } + private createNoGoalTurnStartedPromise( runtimeEffectsGraceMs: number, ): { @@ -435,30 +524,6 @@ export class CodexAppServerClient { }; } - private createGoalTurnCompletionGracePromise( - turnCompletionGraceMs: number, - ): { promise: Promise, release: () => void } { - let released = false; - let timeout: ReturnType | null = null; - const release = () => { - if (released) { - return; - } - released = true; - if (timeout !== null) { - clearTimeout(timeout); - } - }; - const promise = new Promise((resolve) => { - const resolveNoGoalTurnCompleted = () => { - timeout = null; - resolve(null); - }; - timeout = setTimeout(resolveNoGoalTurnCompleted, turnCompletionGraceMs); - }); - return {promise, release}; - } - async runCompact(params: ThreadCompactStartParams): Promise { const compactionCompleted = this.awaitCompactionCompleted(params.threadId); await this.threadCompactStart(params); @@ -691,6 +756,16 @@ export class CodexAppServerClient { } } + private recordThreadGoalCleared(event: ThreadGoalClearedNotification): void { + const captures = this.threadGoalClearedCaptures.get(event.threadId); + if (!captures) { + return; + } + for (const capture of captures) { + capture(); + } + } + private recordTurnRouting(routing: { threadId: string | null, turnId: string | null }): void { if (routing.threadId === null || routing.turnId === null) { return; @@ -704,6 +779,22 @@ export class CodexAppServerClient { } } + private recordVisibleTurnActivity( + notification: ServerNotification, + routing: { threadId: string | null, turnId: string | null }, + ): void { + if (routing.threadId === null || routing.turnId === null || !isVisibleTurnActivityNotification(notification)) { + return; + } + const captures = this.turnActivityCaptures.get(routing.threadId); + if (!captures) { + return; + } + for (const capture of captures) { + capture(routing.turnId); + } + } + private handleStaleTurnNotification( notification: ServerNotification, routing: { threadId: string | null, turnId: string | null }, @@ -782,6 +873,23 @@ export class CodexAppServerClient { }; } + private captureTurnActivities(threadId: string, capture: (turnId: string) => void): () => void { + const captures = this.turnActivityCaptures.get(threadId) ?? new Set<(turnId: string) => void>(); + captures.add(capture); + this.turnActivityCaptures.set(threadId, captures); + let released = false; + return () => { + if (released) { + return; + } + released = true; + captures.delete(capture); + if (captures.size === 0) { + this.turnActivityCaptures.delete(threadId); + } + }; + } + private captureThreadStatuses(threadId: string, capture: (status: ThreadStatus) => void): () => void { const captures = this.threadStatusCaptures.get(threadId) ?? new Set<(status: ThreadStatus) => void>(); captures.add(capture); @@ -816,6 +924,23 @@ export class CodexAppServerClient { }; } + private captureThreadGoalClears(threadId: string, capture: () => void): () => void { + const captures = this.threadGoalClearedCaptures.get(threadId) ?? new Set<() => void>(); + captures.add(capture); + this.threadGoalClearedCaptures.set(threadId, captures); + let released = false; + return () => { + if (released) { + return; + } + released = true; + captures.delete(capture); + if (captures.size === 0) { + this.threadGoalClearedCaptures.delete(threadId); + } + }; + } + private resolveMcpServerStartupResolvers(): void { const pendingResolvers: Array = []; for (const resolver of this.mcpServerStartupResolvers) { @@ -934,6 +1059,13 @@ function isThreadGoalUpdatedNotification(notification: ServerNotification): noti return notification.method === "thread/goal/updated"; } +function isThreadGoalClearedNotification(notification: ServerNotification): notification is { + method: "thread/goal/cleared"; + params: ThreadGoalClearedNotification; +} { + return notification.method === "thread/goal/cleared"; +} + function isCompactionCompletedNotification(notification: ServerNotification): notification is CompactionCompletedNotification { if (notification.method === "thread/compacted") { return true; @@ -949,6 +1081,31 @@ function goalsMatch(left: ThreadGoal, right: ThreadGoal): boolean { && left.updatedAt === right.updatedAt; } +function isVisibleTurnActivityNotification(notification: ServerNotification): boolean { + if (notification.method === "item/started") { + return isVisibleStartedItemType(notification.params.item.type); + } + if (notification.method === "item/completed") { + const item = notification.params.item; + if (item.type === "reasoning") { + return item.summary.length > 0 || item.content.length > 0; + } + if (item.type === "exitedReviewMode") { + return item.review.trim().length > 0; + } + return isVisibleCompletedItemType(item.type); + } + return VISIBLE_TURN_ACTIVITY_METHODS.has(notification.method); +} + +function isVisibleStartedItemType(itemType: ServerNotificationItemType): boolean { + return VISIBLE_STARTED_ITEM_TYPES.has(itemType); +} + +function isVisibleCompletedItemType(itemType: ServerNotificationItemType): boolean { + return VISIBLE_COMPLETED_ITEM_TYPES.has(itemType); +} + function extractThreadId(notification: ServerNotification): string | null { const params = notification.params as { threadId?: unknown } | undefined; if (params && typeof params.threadId === "string") { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index d06b107d..693ffc94 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1469,10 +1469,8 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", turn: createTurn("goal-turn-id", "completed"), }); - const goalStatusSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") - .mockResolvedValue({ goal: createThreadGoal() }); - const goalClearSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalClear") - .mockResolvedValue({ cleared: true }); + const goalClearSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "runGoalClear") + .mockResolvedValue(undefined); await mockFixture.getCodexAcpAgent().prompt({ sessionId: "session-id", @@ -1496,11 +1494,11 @@ describe('ACP server test', { timeout: 40_000 }, () => { objective: "Ship the migration and keep tests green", status: "active", }, expect.any(Function)); - expect(goalStatusSpy).toHaveBeenCalledWith({ + expect(goalRunSpy).toHaveBeenNthCalledWith(2, { threadId: "session-id", status: "paused", }); - expect(goalRunSpy).toHaveBeenNthCalledWith(2, { + expect(goalRunSpy).toHaveBeenNthCalledWith(3, { threadId: "session-id", status: "active", }, expect.any(Function)); @@ -1557,6 +1555,17 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(promptResolved).toBe(false); expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).not.toHaveBeenCalled(); + mockFixture.sendServerNotification({ + method: "turn/started", + params: { + threadId: "session-id", + turn: createTurn("goal-turn-id", "inProgress"), + }, + }); + await Promise.resolve(); + expect(promptResolved).toBe(false); + expect(mockFixture.getCodexAppServerClient().awaitTurnCompleted).not.toHaveBeenCalled(); + mockFixture.sendServerNotification({ method: "item/agentMessage/delta", params: { @@ -1696,7 +1705,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }, undefined, undefined, 0).finally(() => { + }, undefined, undefined).finally(() => { resultSettled = true; }); @@ -1761,7 +1770,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }, undefined, undefined, 0).finally(() => { + }, undefined, undefined).finally(() => { resultSettled = true; }); @@ -1818,7 +1827,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }, undefined, undefined, 0).finally(() => { + }, undefined, undefined).finally(() => { resultSettled = true; }); @@ -1872,6 +1881,78 @@ describe('ACP server test', { timeout: 40_000 }, () => { } }); + it('keeps goal set pending after turn start until visible turn activity routes', async () => { + vi.useFakeTimers(); + try { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({updatedAt: 1710000235}); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockResolvedValue({goal}); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted") + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-turn-id", "completed"), + }); + const onTurnStarted = vi.fn(); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }, onTurnStarted).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "Ship the migration and keep tests green", + status: "active", + }); + }); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + mockFixture.sendServerNotification({ + method: "turn/started", + params: { + threadId: "session-id", + turn: createTurn("goal-turn-id", "inProgress"), + }, + }); + + await vi.advanceTimersByTimeAsync(10_000); + await Promise.resolve(); + expect(onTurnStarted).toHaveBeenCalledWith("goal-turn-id"); + expect(resultSettled).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "late goal output", + }, + }); + + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toBeNull(); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + it('completes goal set when active thread returns idle without routing a turn', async () => { vi.useFakeTimers(); try { @@ -1886,7 +1967,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }, undefined, undefined, 0).finally(() => { + }, undefined, undefined).finally(() => { resultSettled = true; }); @@ -1935,6 +2016,82 @@ describe('ACP server test', { timeout: 40_000 }, () => { } }); + it('waits for paused goal update before completing goal status set', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const goal = createThreadGoal({status: "paused", updatedAt: 1710000260}); + const threadGoalSet = deferred<{goal: ThreadGoal}>(); + const threadGoalSetSpy = vi.spyOn(codexAppServerClient, "threadGoalSet") + .mockReturnValue(threadGoalSet.promise); + const awaitTurnCompletedSpy = vi.spyOn(codexAppServerClient, "awaitTurnCompleted"); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalSet({ + threadId: "session-id", + status: "paused", + }).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalSetSpy).toHaveBeenCalledWith({ + threadId: "session-id", + status: "paused", + }); + }); + + threadGoalSet.resolve({goal}); + await flushAsyncWork(); + expect(resultSettled).toBe(false); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + + await expect(resultPromise).resolves.toBeNull(); + expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); + }); + + it('waits for goal cleared notification before completing goal clear', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + const threadGoalClear = deferred<{cleared: boolean}>(); + const threadGoalClearSpy = vi.spyOn(codexAppServerClient, "threadGoalClear") + .mockReturnValue(threadGoalClear.promise); + let resultSettled = false; + + const resultPromise = codexAppServerClient.runGoalClear({ + threadId: "session-id", + }).finally(() => { + resultSettled = true; + }); + + await vi.waitFor(() => { + expect(threadGoalClearSpy).toHaveBeenCalledWith({ + threadId: "session-id", + }); + }); + + threadGoalClear.resolve({cleared: true}); + await flushAsyncWork(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification({ + method: "thread/goal/cleared", + params: { + threadId: "session-id", + }, + }); + + await expect(resultPromise).resolves.toBeUndefined(); + }); + it('interrupts a late-started goal slash command after the ACP prompt request is cancelled', async () => { const { mockFixture } = setupPromptFixture(); const goalCompleted = deferred(); @@ -2164,7 +2321,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { threadId: "session-id", objective: "Ship the migration and keep tests green", status: "active", - }, undefined, undefined, 0).finally(() => { + }, undefined, undefined).finally(() => { resultSettled = true; }); From ebb5b68b3e395af7bd97292c7d2c58f122e7d721 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 26 Jun 2026 18:38:57 +0200 Subject: [PATCH 17/18] Wait for goal turn completion before resolving --- src/CodexAppServerClient.ts | 119 +------------ .../CodexACPAgent/CodexAcpClient.test.ts | 158 +++++++++++++++++- 2 files changed, 153 insertions(+), 124 deletions(-) diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index f0ccaba7..d1bc210c 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -86,11 +86,6 @@ export type McpStartupResult = { cancelled: Array; }; -type ServerNotificationItemType = Extract< - ServerNotification, - { params: { item: { type: unknown } } } ->["params"]["item"]["type"]; - const CommandExecutionApprovalRequest = new RequestType< CommandExecutionRequestApprovalParams, CommandExecutionRequestApprovalResponse, @@ -116,45 +111,6 @@ const McpServerElicitationRequest = new RequestType< >('mcpServer/elicitation/request'); const GOAL_RUNTIME_EFFECTS_GRACE_MS = 1_000; -const VISIBLE_TURN_ACTIVITY_METHODS: ReadonlySet = new Set([ - "error", - "turn/plan/updated", - "item/autoApprovalReview/started", - "item/autoApprovalReview/completed", - "item/agentMessage/delta", - "item/commandExecution/outputDelta", - "item/commandExecution/terminalInteraction", - "item/mcpToolCall/progress", - "item/reasoning/summaryTextDelta", - "item/reasoning/summaryPartAdded", - "item/reasoning/textDelta", - "model/rerouted", - "warning", - "configWarning", - "fuzzyFileSearch/sessionUpdated", - "fuzzyFileSearch/sessionCompleted", -]); -const VISIBLE_STARTED_ITEM_TYPES: ReadonlySet = new Set([ - "fileChange", - "commandExecution", - "mcpToolCall", - "dynamicToolCall", - "webSearch", - "imageView", - "imageGeneration", - "collabAgentToolCall", -]); -const VISIBLE_COMPLETED_ITEM_TYPES: ReadonlySet = new Set([ - "fileChange", - "commandExecution", - "mcpToolCall", - "dynamicToolCall", - "webSearch", - "imageView", - "imageGeneration", - "collabAgentToolCall", - "contextCompaction", -]); /** * A type-safe client over the Codex App Server's JSON-RPC API. @@ -171,7 +127,6 @@ export class CodexAppServerClient { private readonly pendingCompactionCompletionResolvers = new Map void>>(); private readonly turnCompletionCaptures = new Map void>>(); private readonly turnRoutingCaptures = new Map void>>(); - private readonly turnActivityCaptures = new Map void>>(); private readonly threadStatusCaptures = new Map void>>(); private readonly threadGoalUpdateCaptures = new Map void>>(); private readonly threadGoalClearedCaptures = new Map void>>(); @@ -213,7 +168,6 @@ export class CodexAppServerClient { if (this.handleStaleTurnNotification(serverNotification, routing)) { return; } - this.recordVisibleTurnActivity(serverNotification, routing); this.notify(serverNotification); for (const callback of this.codexEventHandlers) { callback({ eventType: "notification", ...serverNotification }); @@ -357,10 +311,6 @@ export class CodexAppServerClient { const matchingGoalUpdateHandled = new Promise((resolve) => { resolveGoalUpdateHandled = () => resolve(null); }); - let resolveGoalTurnActivity: () => void = () => {}; - const goalTurnActivity = new Promise((resolve) => { - resolveGoalTurnActivity = () => resolve(null); - }); let goalUpdateHandled = false; let expectedGoal: ThreadGoal | null = null; const noGoalTurnStarted = this.createNoGoalTurnStartedPromise(runtimeEffectsGraceMs); @@ -373,12 +323,6 @@ export class CodexAppServerClient { onTurnStarted?.(turnId); resolveGoalTurnStarted(turnId); }); - const releaseActivityCapture = this.captureTurnActivities(params.threadId, (turnId) => { - if (!goalUpdateHandled || goalTurnId !== turnId) { - return; - } - resolveGoalTurnActivity(); - }); const releaseGoalUpdateCapture = this.captureThreadGoalUpdates(params.threadId, (event) => { capturedGoalUpdates.push(event); if (expectedGoal !== null && goalsMatch(event.goal, expectedGoal)) { @@ -412,20 +356,17 @@ export class CodexAppServerClient { releaseStatusCapture(); releaseGoalUpdateCapture(); if (turnId === null) { - releaseActivityCapture(); return null; } const earlyCompletion = capturedCompletions.find(event => event.turn.id === turnId); if (earlyCompletion) { - releaseActivityCapture(); return earlyCompletion; } - return await Promise.race([goalTurnCompleted, goalTurnActivity]); + return await goalTurnCompleted; } finally { noGoalTurnStarted.release(); releaseCompletionCapture(); releaseRoutingCapture(); - releaseActivityCapture(); releaseStatusCapture(); releaseGoalUpdateCapture(); } @@ -779,22 +720,6 @@ export class CodexAppServerClient { } } - private recordVisibleTurnActivity( - notification: ServerNotification, - routing: { threadId: string | null, turnId: string | null }, - ): void { - if (routing.threadId === null || routing.turnId === null || !isVisibleTurnActivityNotification(notification)) { - return; - } - const captures = this.turnActivityCaptures.get(routing.threadId); - if (!captures) { - return; - } - for (const capture of captures) { - capture(routing.turnId); - } - } - private handleStaleTurnNotification( notification: ServerNotification, routing: { threadId: string | null, turnId: string | null }, @@ -873,23 +798,6 @@ export class CodexAppServerClient { }; } - private captureTurnActivities(threadId: string, capture: (turnId: string) => void): () => void { - const captures = this.turnActivityCaptures.get(threadId) ?? new Set<(turnId: string) => void>(); - captures.add(capture); - this.turnActivityCaptures.set(threadId, captures); - let released = false; - return () => { - if (released) { - return; - } - released = true; - captures.delete(capture); - if (captures.size === 0) { - this.turnActivityCaptures.delete(threadId); - } - }; - } - private captureThreadStatuses(threadId: string, capture: (status: ThreadStatus) => void): () => void { const captures = this.threadStatusCaptures.get(threadId) ?? new Set<(status: ThreadStatus) => void>(); captures.add(capture); @@ -1081,31 +989,6 @@ function goalsMatch(left: ThreadGoal, right: ThreadGoal): boolean { && left.updatedAt === right.updatedAt; } -function isVisibleTurnActivityNotification(notification: ServerNotification): boolean { - if (notification.method === "item/started") { - return isVisibleStartedItemType(notification.params.item.type); - } - if (notification.method === "item/completed") { - const item = notification.params.item; - if (item.type === "reasoning") { - return item.summary.length > 0 || item.content.length > 0; - } - if (item.type === "exitedReviewMode") { - return item.review.trim().length > 0; - } - return isVisibleCompletedItemType(item.type); - } - return VISIBLE_TURN_ACTIVITY_METHODS.has(notification.method); -} - -function isVisibleStartedItemType(itemType: ServerNotificationItemType): boolean { - return VISIBLE_STARTED_ITEM_TYPES.has(itemType); -} - -function isVisibleCompletedItemType(itemType: ServerNotificationItemType): boolean { - return VISIBLE_COMPLETED_ITEM_TYPES.has(itemType); -} - function extractThreadId(notification: ServerNotification): string | null { const params = notification.params as { threadId?: unknown } | undefined; if (params && typeof params.threadId === "string") { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 693ffc94..2c909c84 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1575,6 +1575,11 @@ describe('ACP server test', { timeout: 40_000 }, () => { delta: "I", }, }); + await Promise.resolve(); + expect(promptResolved).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + await vi.waitFor(() => { expect(promptResolved).toBe(true); }); @@ -1677,6 +1682,10 @@ describe('ACP server test', { timeout: 40_000 }, () => { delta: "late goal output", }, }); + await flushAsyncWork(); + expect(promptResolved).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); await expect(promptPromise).resolves.toEqual(expect.objectContaining({ stopReason: "end_turn", @@ -1685,6 +1694,90 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); + it('waits for goal turn completion after the goal completes before streamed output finishes', async () => { + const { mockFixture } = setupPromptFixture(); + const goal = createThreadGoal({updatedAt: 1710000160}); + const completedGoal = createThreadGoal({ + status: "complete", + updatedAt: 1710000170, + tokensUsed: 42, + timeUsedSeconds: 8, + }); + vi.spyOn(mockFixture.getCodexAppServerClient(), "threadGoalSet") + .mockResolvedValue({ goal }); + let promptResolved = false; + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{ type: "text", text: "/goal tell me a joke" }], + }).then((response) => { + promptResolved = true; + return response; + }); + + await vi.waitFor(() => { + expect(mockFixture.getCodexAppServerClient().threadGoalSet).toHaveBeenCalledWith({ + threadId: "session-id", + objective: "tell me a joke", + status: "active", + }); + }); + + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: null, + goal, + }, + }); + mockFixture.sendServerNotification({ + method: "thread/status/changed", + params: { + threadId: "session-id", + status: { + type: "active", + activeFlags: [], + }, + }, + }); + mockFixture.sendServerNotification({ + method: "thread/goal/updated", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + goal: completedGoal, + }, + }); + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: "Why", + }, + }); + await flushAsyncWork(); + expect(promptResolved).toBe(false); + + mockFixture.sendServerNotification({ + method: "item/agentMessage/delta", + params: { + threadId: "session-id", + turnId: "goal-turn-id", + itemId: "goal-message-id", + delta: " did the test wait?", + }, + }); + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + + await expect(promptPromise).resolves.toEqual(expect.objectContaining({ + stopReason: "end_turn", + })); + expect(promptResolved).toBe(true); + }); + it('does not start the no-turn grace period before the goal update is handled', async () => { vi.useFakeTimers(); try { @@ -1746,7 +1839,18 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); await vi.advanceTimersByTimeAsync(0); - await expect(resultPromise).resolves.toBeNull(); + await Promise.resolve(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toMatchObject({ + threadId: "session-id", + turn: { + id: "goal-turn-id", + status: "completed", + }, + }); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); @@ -1804,7 +1908,17 @@ describe('ACP server test', { timeout: 40_000 }, () => { }, }); - await expect(resultPromise).resolves.toBeNull(); + await flushAsyncWork(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + await expect(resultPromise).resolves.toMatchObject({ + threadId: "session-id", + turn: { + id: "goal-turn-id", + status: "completed", + }, + }); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); }); @@ -1874,14 +1988,25 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); await vi.advanceTimersByTimeAsync(0); - await expect(resultPromise).resolves.toBeNull(); + await Promise.resolve(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toMatchObject({ + threadId: "session-id", + turn: { + id: "goal-turn-id", + status: "completed", + }, + }); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); } }); - it('keeps goal set pending after turn start until visible turn activity routes', async () => { + it('keeps goal set pending after turn start until turn completion routes', async () => { vi.useFakeTimers(); try { const mockFixture = createCodexMockTestFixture(); @@ -1946,7 +2071,18 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); await vi.advanceTimersByTimeAsync(0); - await expect(resultPromise).resolves.toBeNull(); + await Promise.resolve(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toMatchObject({ + threadId: "session-id", + turn: { + id: "goal-turn-id", + status: "completed", + }, + }); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); @@ -2354,7 +2490,17 @@ describe('ACP server test', { timeout: 40_000 }, () => { await Promise.resolve(); await Promise.resolve(); await vi.advanceTimersByTimeAsync(0); - await expect(resultPromise).resolves.toBeNull(); + expect(resultSettled).toBe(false); + + mockFixture.sendServerNotification(createTurnCompletedNotification("session-id", "goal-turn-id")); + await vi.advanceTimersByTimeAsync(0); + await expect(resultPromise).resolves.toMatchObject({ + threadId: "session-id", + turn: { + id: "goal-turn-id", + status: "completed", + }, + }); expect(awaitTurnCompletedSpy).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); From 0500e3435a65664297129588452bbdb3bb435fd6 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 26 Jun 2026 18:48:06 +0200 Subject: [PATCH 18/18] Potential fix for pull request finding 'Unused variable, import, function or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 2c909c84..e645610d 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -2993,17 +2993,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { }; } - function createThreadReadResponse(status: "active" | "idle") { - return { - thread: { - id: "session-id", - status: status === "active" - ? { type: "active", activeFlags: [] } - : { type: "idle" }, - } as any, - }; - } - it ('should disable reasoning.summary if key authorization is used', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "apiKey" } });