diff --git a/docs/goal-extension.md b/docs/goal-extension.md index 98900e5d..4ab9c2ac 100644 --- a/docs/goal-extension.md +++ b/docs/goal-extension.md @@ -12,13 +12,13 @@ An agent advertises support in its `initialize` response: "goal": { "version": 1, "controlMethod": "_session/goal", - "actions": ["pause", "resume", "clear"] + "actions": ["set", "pause", "resume", "clear"] } } } ``` -`actions` is the implementation-supported subset of `set`, `pause`, `resume`, and `clear`. Clients must not infer support for an action that is not advertised. The control request contains `sessionId` and `action`; a future version may add action-specific fields such as an objective for `set`. +`actions` is the implementation-supported subset of `set`, `pause`, `resume`, and `clear`. Clients must not infer support for an action that is not advertised. The control request contains `sessionId` and `action`; `set` additionally requires a non-blank `objective`. ## Session state diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 0a2baa46..823e2ef7 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -501,12 +501,17 @@ export class CodexAcpClient { sessionId: string, objective: string, onTurnStarted?: (turnId: string) => void, + onGoalSet?: (goal: ThreadGoal) => void, ): Promise { - return await this.codexClient.runGoalSet({ + const params = { threadId: sessionId, objective, status: "active", - }, onTurnStarted); + } as const; + if (onGoalSet === undefined) { + return await this.codexClient.runGoalSet(params, onTurnStarted); + } + return await this.codexClient.runGoalSet(params, onTurnStarted, undefined, onGoalSet); } async setGoalStatus(sessionId: string, status: ThreadGoalStatus): Promise { @@ -526,11 +531,16 @@ export class CodexAcpClient { async resumeGoal( sessionId: string, onTurnStarted?: (turnId: string) => void, + onGoalSet?: (goal: ThreadGoal) => void, ): Promise { - return await this.codexClient.runGoalSet({ + const params = { threadId: sessionId, status: "active", - }, onTurnStarted); + } as const; + if (onGoalSet === undefined) { + return await this.codexClient.runGoalSet(params, onTurnStarted); + } + return await this.codexClient.runGoalSet(params, onTurnStarted, undefined, onGoalSet); } async clearGoal(sessionId: string): Promise { diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index ea95e903..3c321c86 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -14,7 +14,7 @@ import { import type {McpStartupResult} from "./CodexAppServerClient"; import {type AcpClientConnection, ACPSessionConnection, type UpdateSessionEvent} from "./ACPSessionConnection"; import type {InputModality, ReasoningEffort} from "./app-server"; -import type {Account, Model, ReasoningEffortOption, Thread, ThreadItem, UserInput} from "./app-server/v2"; +import type {Account, Model, ReasoningEffortOption, Thread, ThreadGoal, ThreadItem, UserInput} from "./app-server/v2"; import type {RateLimitsMap} from "./RateLimitsMap"; import {ModelId} from "./ModelId"; import {AgentMode, MODE_CONFIG_ID} from "./AgentMode"; @@ -35,7 +35,7 @@ import { } from "./ModelConfigOption"; import type {TokenCount} from "./TokenCount"; import {toPromptUsage} from "./TokenCount"; -import {CodexCommands} from "./CodexCommands"; +import {CodexCommands, GOAL_CONTINUATION_PROMPT} from "./CodexCommands"; import {SteeringQueue} from "./SteeringQueue"; import type {QuotaMeta} from "./QuotaMeta"; import {logger} from "./Logger"; @@ -175,6 +175,7 @@ export class CodexAcpServer { private readonly closingSessions: Map; private readonly sessionGenerations: Map; private readonly sessionOpenGenerations: Map; + private readonly goalControlGenerations: Map; constructor( connection: AcpClientConnection, @@ -191,6 +192,7 @@ export class CodexAcpServer { this.closingSessions = new Map(); this.sessionGenerations = new Map(); this.sessionOpenGenerations = new Map(); + this.goalControlGenerations = new Map(); this.connection = connection; this.codexAcpClient = codexAcpClient; this.defaultAuthRequest = defaultAuthRequest ?? null; @@ -284,12 +286,51 @@ export class CodexAcpServer { throw RequestError.invalidParams(undefined, `Unknown session: ${methodRequest.params.sessionId}`); } const sessionGeneration = this.getSessionGeneration(sessionState.sessionId); - if (methodRequest.params.action === "pause" || methodRequest.params.action === "resume") { - const status = methodRequest.params.action === "pause" ? "paused" : "active"; - const goal = await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionState.sessionId, status)); + const goalControlGeneration = this.bumpGoalControlGeneration(sessionState.sessionId); + if (methodRequest.params.action === "set") { + const objective = methodRequest.params.objective; + let updatedGoal: ThreadGoal | null = null; + const turnCompleted = await this.runWithProcessCheck(() => this.codexAcpClient.setGoal( + sessionState.sessionId, + objective, + undefined, + (goal) => { + updatedGoal = goal; + }, + )); + if (turnCompleted === null && updatedGoal !== null) { + await this.startGoalContinuationIfCurrent( + sessionState, + sessionGeneration, + goalControlGeneration, + updatedGoal, + ); + } + } else if (methodRequest.params.action === "pause") { + const goal = await this.runWithProcessCheck(() => this.codexAcpClient.setGoalStatus(sessionState.sessionId, "paused")); if (this.goalPublishIsCurrent(sessionState, sessionGeneration)) { await this.publishGoalSnapshot(sessionState, toThreadGoalSnapshot(goal), false); } + } else if (methodRequest.params.action === "resume") { + let updatedGoal: ThreadGoal | null = null; + const turnCompleted = await this.runWithProcessCheck(() => this.codexAcpClient.resumeGoal( + sessionState.sessionId, + undefined, + (goal) => { + updatedGoal = goal; + }, + )); + if (updatedGoal !== null && this.goalPublishIsCurrent(sessionState, sessionGeneration)) { + await this.publishGoalSnapshot(sessionState, toThreadGoalSnapshot(updatedGoal), false); + } + if (turnCompleted === null && updatedGoal !== null) { + await this.startGoalContinuationIfCurrent( + sessionState, + sessionGeneration, + goalControlGeneration, + updatedGoal, + ); + } } else if (methodRequest.params.action === "clear") { await this.runWithProcessCheck(() => this.codexAcpClient.clearGoal(sessionState.sessionId)); if (this.goalPublishIsCurrent(sessionState, sessionGeneration)) { @@ -397,6 +438,12 @@ export class CodexAcpServer { return this.sessionGenerations.get(sessionId) ?? 0; } + private bumpGoalControlGeneration(sessionId: string): number { + const generation = (this.goalControlGenerations.get(sessionId) ?? 0) + 1; + this.goalControlGenerations.set(sessionId, generation); + return generation; + } + private bumpSessionGeneration(sessionId: string): number { const generation = this.getSessionGeneration(sessionId) + 1; this.sessionGenerations.set(sessionId, generation); @@ -618,6 +665,7 @@ export class CodexAcpServer { this.pendingTurnStarts.delete(params.sessionId); this.activePrompts.delete(params.sessionId); this.steeringQueues.delete(params.sessionId); + this.goalControlGenerations.delete(params.sessionId); } this.endSessionCloseFence(params.sessionId); } @@ -1039,25 +1087,58 @@ export class CodexAcpServer { * fails or is cancelled before the turn starts. */ private async startNewTurnFromSteering(params: SessionSteerRequest): Promise { - // A prompt can outlive its turn (post-turn cleanup runs before it leaves - // activePrompts), so a steer can miss the turn while the prompt is still - // winding down. Starting a new turn now would run a second prompt on the - // same session, so wait for the current one to drain first (a no-op when idle). + await this.startNewTurnFromExternalPrompt(params, "Steering"); + return {outcome: "startedNewTurn"}; + } + + private async startGoalContinuationIfCurrent( + sessionState: SessionState, + sessionGeneration: number, + goalControlGeneration: number, + expectedGoal: ThreadGoal, + ): Promise { + await this.startNewTurnFromExternalPrompt({ + sessionId: sessionState.sessionId, + prompt: GOAL_CONTINUATION_PROMPT, + }, "Goal continuation", async () => { + if (!this.goalPublishIsCurrent(sessionState, sessionGeneration) + || this.goalControlGenerations.get(sessionState.sessionId) !== goalControlGeneration) { + return false; + } + const currentGoal = await this.runWithProcessCheck(() => this.codexAcpClient.getGoal(sessionState.sessionId)); + return currentGoal?.status === "active" + && currentGoal.objective === expectedGoal.objective + && currentGoal.createdAt === expectedGoal.createdAt + && this.goalControlGenerations.get(sessionState.sessionId) === goalControlGeneration; + }); + } + + private async startNewTurnFromExternalPrompt( + params: acp.PromptRequest, + source: string, + canStart: () => Promise = async () => true, + ): Promise { + // A prompt can outlive its turn while post-turn cleanup runs. Starting a + // control-triggered turn during that window would run two prompts on the + // same session, so wait for the current prompt to drain first. const previousPrompt = this.activePrompts.get(params.sessionId); await previousPrompt?.completion; if (this.sessionIsClosing(params.sessionId)) { throw RequestError.invalidRequest(`Session ${params.sessionId} is closing`); } + if (!await canStart()) { + return false; + } - return await new Promise((resolve, reject) => { + return await new Promise((resolve, reject) => { let turnStarted = false; const promptDone = this.prompt(params, undefined, () => { turnStarted = true; - logger.log("Steering session started a new turn", {sessionId: params.sessionId}); + logger.log(`${source} started a new turn`, {sessionId: params.sessionId}); // The new turn is now running. This is the success path: answer the // steer immediately ("a turn was started") and let prompt() finish the // turn in the background. - resolve({outcome: "startedNewTurn"}); + resolve(true); }); promptDone.then( (response) => { @@ -1070,7 +1151,7 @@ export class CodexAcpServer { // resolve in the callback above), or the prompt finished // without ever starting a turn and was not cancelled (e.g. a // command-only turn). Both count as a successfully accepted steer. - resolve({outcome: "startedNewTurn"}); + resolve(turnStarted); } }, (error: unknown) => { @@ -1078,7 +1159,7 @@ export class CodexAcpServer { // The turn had already started, so the steer was already // answered "startedNewTurn". This is a failure of a turn running // in the background — nothing to return, just log it. - logger.error(`Steering-started prompt for session ${params.sessionId} failed`, error); + logger.error(`${source} prompt for session ${params.sessionId} failed`, error); } else { // The prompt failed before the turn started. The steer never // took, so surface the failure to the caller. @@ -1995,6 +2076,10 @@ export class CodexAcpServer { }; } + const effectiveParams = commandResult.prompt === undefined + ? params + : {...params, prompt: commandResult.prompt}; + if (this.sessionIsClosing(params.sessionId)) { return this.cancelledPromptResponse(sessionState); } @@ -2011,7 +2096,7 @@ export class CodexAcpServer { }); } - if (!sessionState.supportedInputModalities.includes("image") && params.prompt.some(b => b.type === "image")) { + if (!sessionState.supportedInputModalities.includes("image") && effectiveParams.prompt.some(b => b.type === "image")) { throw RequestError.invalidRequest("The current model does not support image input"); } const agentMode = sessionState.agentMode; @@ -2022,7 +2107,7 @@ export class CodexAcpServer { ensurePendingTurnStart(); const sendPromptPromise = this.runWithProcessCheck( () => this.codexAcpClient.sendPrompt( - params, + effectiveParams, agentMode, modelId, serviceTier, diff --git a/src/CodexCommands.ts b/src/CodexCommands.ts index e6a837ed..1d5e86d2 100644 --- a/src/CodexCommands.ts +++ b/src/CodexCommands.ts @@ -20,9 +20,14 @@ type ParsedSlashCommand = { }; export type CommandHandleResult = - | { handled: false } + | { handled: false, prompt?: acp.ContentBlock[] } | { handled: true, turnCompleted?: TurnCompletedNotification }; +export const GOAL_CONTINUATION_PROMPT: acp.ContentBlock[] = [{ + type: "text", + text: "Continue working toward the active goal.", +}]; + export type CommandHandleOptions = { onTurnStartPending?: () => void; onTurnStarted?: (turnId: string, threadId: string) => void; @@ -371,7 +376,7 @@ export class CodexCommands { private createGoalCommandResult(turnCompleted: TurnCompletedNotification | null): CommandHandleResult { if (turnCompleted === null) { - return { handled: true }; + return { handled: false, prompt: GOAL_CONTINUATION_PROMPT }; } return { handled: true, diff --git a/src/GoalExtension.ts b/src/GoalExtension.ts index 0181af4b..0e4e064f 100644 --- a/src/GoalExtension.ts +++ b/src/GoalExtension.ts @@ -4,7 +4,7 @@ export const GOAL_EXTENSION_VERSION = 1; export const GOAL_CONTROL_METHOD = "_session/goal"; export const LEGACY_GOAL_CONTROL_METHOD = "_codex/session/goal_control"; -export const GOAL_CONTROL_ACTIONS = ["pause", "resume", "clear"] as const; +export const GOAL_CONTROL_ACTIONS = ["set", "pause", "resume", "clear"] as const; export type GoalControlAction = typeof GOAL_CONTROL_ACTIONS[number]; export type GoalCapability = { @@ -28,7 +28,6 @@ export type GoalSnapshot = { controlMethod: typeof GOAL_CONTROL_METHOD; } -export type GoalControlRequest = { - sessionId: SessionId; - action: GoalControlAction; -} +export type GoalControlRequest = + | { sessionId: SessionId; action: "set"; objective: string } + | { sessionId: SessionId; action: Exclude } diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index dfbc1bcc..8bedb332 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -2530,18 +2530,32 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); it('controls an active goal through the out-of-band session extension', async () => { - const { mockFixture, sessionState } = setupPromptFixture(); + const { mockFixture, sessionState, turnStartSpy } = setupPromptFixture(); // @ts-expect-error - registering local session state for the extension request path mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); const pausedGoal = createThreadGoal({status: "paused", timeUsedSeconds: 12}); const activeGoal = createThreadGoal({status: "active", timeUsedSeconds: 12}); + const setGoalSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "setGoal") + .mockImplementation(async (_sessionId, _objective, _onTurnStarted, onGoalSet) => { + onGoalSet?.(activeGoal); + return null; + }); const setStatusSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "setGoalStatus") - .mockResolvedValueOnce(pausedGoal) - .mockResolvedValueOnce(activeGoal); + .mockResolvedValue(pausedGoal); + const resumeGoalSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "resumeGoal") + .mockImplementation(async (_sessionId, _onTurnStarted, onGoalSet) => { + onGoalSet?.(activeGoal); + return null; + }); const clearGoalSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "clearGoal").mockResolvedValue(undefined); - const getGoalSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "getGoal"); + const getGoalSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "getGoal").mockResolvedValue(activeGoal); mockFixture.clearAcpConnectionDump(); + await expect(mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "set", + objective: "Replace the objective", + })).resolves.toEqual({}); await expect(mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { sessionId: "session-id", action: "pause", @@ -2555,10 +2569,12 @@ describe('ACP server test', { timeout: 40_000 }, () => { action: "clear", })).resolves.toEqual({}); + expect(setGoalSpy).toHaveBeenCalledWith("session-id", "Replace the objective", undefined, expect.any(Function)); expect(setStatusSpy).toHaveBeenNthCalledWith(1, "session-id", "paused"); - expect(setStatusSpy).toHaveBeenNthCalledWith(2, "session-id", "active"); + expect(resumeGoalSpy).toHaveBeenCalledWith("session-id", undefined, expect.any(Function)); expect(clearGoalSpy).toHaveBeenCalledWith("session-id"); - expect(getGoalSpy).not.toHaveBeenCalled(); + expect(getGoalSpy).toHaveBeenCalledTimes(2); + expect(turnStartSpy).toHaveBeenCalledTimes(2); const goalUpdates = mockFixture.getAcpConnectionEvents([]).filter(event => event.method === "sessionUpdate" && "args" in event @@ -2589,6 +2605,193 @@ describe('ACP server test', { timeout: 40_000 }, () => { ])); }); + it('waits for an active turn before starting goal work', async () => { + const {mockFixture, sessionState, turnStartSpy} = setupPromptFixture(); + // @ts-expect-error - registering local session state for the extension request path + mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); + const activeTurnCompleted = deferred(); + vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted") + .mockReset() + .mockReturnValueOnce(activeTurnCompleted.promise) + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-work-turn", "completed"), + }); + const goal = createThreadGoal({objective: "Finish the migration", status: "active"}); + vi.spyOn(mockFixture.getCodexAcpClient(), "setGoal") + .mockImplementation(async (_sessionId, _objective, _onTurnStarted, onGoalSet) => { + onGoalSet?.(goal); + return null; + }); + vi.spyOn(mockFixture.getCodexAcpClient(), "getGoal").mockResolvedValue(goal); + + const activePrompt = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "Work already in progress"}], + }); + await vi.waitFor(() => expect(turnStartSpy).toHaveBeenCalledTimes(1)); + + const setGoal = mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "set", + objective: goal.objective, + }); + await flushAsyncWork(); + expect(turnStartSpy).toHaveBeenCalledTimes(1); + + activeTurnCompleted.resolve({ + threadId: "session-id", + turn: createTurn("turn-id", "completed"), + }); + await expect(activePrompt).resolves.toMatchObject({stopReason: "end_turn"}); + await expect(setGoal).resolves.toEqual({}); + expect(turnStartSpy).toHaveBeenCalledTimes(2); + expect(turnStartSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + input: [expect.objectContaining({text: "Continue working toward the active goal."})], + })); + }); + + it('starts goal work when resume routes no app-server turn', async () => { + const {mockFixture, sessionState, turnStartSpy} = setupPromptFixture(); + // @ts-expect-error - registering local session state for the extension request path + mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); + const goal = createThreadGoal({objective: "Finish the migration", status: "active"}); + vi.spyOn(mockFixture.getCodexAcpClient(), "resumeGoal") + .mockImplementation(async (_sessionId, _onTurnStarted, onGoalSet) => { + onGoalSet?.(goal); + return null; + }); + vi.spyOn(mockFixture.getCodexAcpClient(), "getGoal").mockResolvedValue(goal); + + await expect(mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "resume", + })).resolves.toEqual({}); + + expect(turnStartSpy).toHaveBeenCalledTimes(1); + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ + input: [expect.objectContaining({text: "Continue working toward the active goal."})], + })); + }); + + it('starts only the latest goal replacement after an active turn', async () => { + const {mockFixture, sessionState, turnStartSpy} = setupPromptFixture(); + // @ts-expect-error - registering local session state for the extension request path + mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); + const activeTurnCompleted = deferred(); + vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted") + .mockReset() + .mockReturnValueOnce(activeTurnCompleted.promise) + .mockResolvedValue({ + threadId: "session-id", + turn: createTurn("goal-work-turn", "completed"), + }); + const firstGoal = createThreadGoal({objective: "First replacement", createdAt: 1}); + const latestGoal = createThreadGoal({objective: "Latest replacement", createdAt: 2}); + vi.spyOn(mockFixture.getCodexAcpClient(), "setGoal") + .mockImplementation(async (_sessionId, objective, _onTurnStarted, onGoalSet) => { + onGoalSet?.(objective === firstGoal.objective ? firstGoal : latestGoal); + return null; + }); + const getGoal = vi.spyOn(mockFixture.getCodexAcpClient(), "getGoal").mockResolvedValue(latestGoal); + + const activePrompt = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "Work already in progress"}], + }); + await vi.waitFor(() => expect(turnStartSpy).toHaveBeenCalledTimes(1)); + const firstSet = mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "set", + objective: firstGoal.objective, + }); + const latestSet = mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "set", + objective: latestGoal.objective, + }); + await flushAsyncWork(); + expect(turnStartSpy).toHaveBeenCalledTimes(1); + + activeTurnCompleted.resolve({ + threadId: "session-id", + turn: createTurn("turn-id", "completed"), + }); + await expect(activePrompt).resolves.toMatchObject({stopReason: "end_turn"}); + await expect(firstSet).resolves.toEqual({}); + await expect(latestSet).resolves.toEqual({}); + + expect(turnStartSpy).toHaveBeenCalledTimes(2); + expect(getGoal).toHaveBeenCalledTimes(1); + }); + + it('does not start queued goal work after the goal is paused', async () => { + const {mockFixture, sessionState, turnStartSpy} = setupPromptFixture(); + // @ts-expect-error - registering local session state for the extension request path + mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); + const activeTurnCompleted = deferred(); + vi.spyOn(mockFixture.getCodexAppServerClient(), "awaitTurnCompleted") + .mockReset() + .mockReturnValue(activeTurnCompleted.promise); + const goal = createThreadGoal({objective: "Finish the migration", status: "active"}); + vi.spyOn(mockFixture.getCodexAcpClient(), "setGoal") + .mockImplementation(async (_sessionId, _objective, _onTurnStarted, onGoalSet) => { + onGoalSet?.(goal); + return null; + }); + vi.spyOn(mockFixture.getCodexAcpClient(), "setGoalStatus") + .mockResolvedValue(createThreadGoal({objective: goal.objective, status: "paused"})); + + const activePrompt = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "Work already in progress"}], + }); + await vi.waitFor(() => expect(turnStartSpy).toHaveBeenCalledTimes(1)); + const setGoal = mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "set", + objective: goal.objective, + }); + await flushAsyncWork(); + await mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "pause", + }); + + activeTurnCompleted.resolve({ + threadId: "session-id", + turn: createTurn("turn-id", "completed"), + }); + await expect(activePrompt).resolves.toMatchObject({stopReason: "end_turn"}); + await expect(setGoal).resolves.toEqual({}); + expect(turnStartSpy).toHaveBeenCalledTimes(1); + }); + + it('does not duplicate an app-server-routed goal turn', async () => { + const {mockFixture, sessionState, turnStartSpy} = setupPromptFixture(); + // @ts-expect-error - registering local session state for the extension request path + mockFixture.getCodexAcpAgent().sessions.set("session-id", sessionState); + const goal = createThreadGoal({objective: "Finish the migration", status: "active"}); + vi.spyOn(mockFixture.getCodexAcpClient(), "setGoal") + .mockImplementation(async (_sessionId, _objective, _onTurnStarted, onGoalSet) => { + onGoalSet?.(goal); + return { + threadId: "session-id", + turn: createTurn("routed-goal-turn", "completed"), + }; + }); + const getGoal = vi.spyOn(mockFixture.getCodexAcpClient(), "getGoal"); + + await expect(mockFixture.getCodexAcpAgent().extMethod(GOAL_CONTROL_METHOD, { + sessionId: "session-id", + action: "set", + objective: goal.objective, + })).resolves.toEqual({}); + + expect(getGoal).not.toHaveBeenCalled(); + expect(turnStartSpy).not.toHaveBeenCalled(); + }); + it('ignores an older goal refresh that completes after a newer refresh', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpAgent = mockFixture.getCodexAcpAgent(); @@ -2789,7 +2992,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { } }); - it('completes goal slash command when app server starts no continuation turn', async () => { + it('starts a goal work turn when app server starts no continuation turn', async () => { const { mockFixture, turnStartSpy } = setupPromptFixture(); const goalRunSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "runGoalSet") .mockResolvedValue(null); @@ -2805,7 +3008,13 @@ describe('ACP server test', { timeout: 40_000 }, () => { objective: "Ship the migration and keep tests green", status: "active", }, expect.any(Function)); - expect(turnStartSpy).not.toHaveBeenCalled(); + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ + input: [{ + type: "text", + text: "Continue working toward the active goal.", + text_elements: [], + }], + })); }); it('reports missing goal slash command input', async () => { diff --git a/src/__tests__/CodexACPAgent/initialize.test.ts b/src/__tests__/CodexACPAgent/initialize.test.ts index b391fe3f..7b5bd589 100644 --- a/src/__tests__/CodexACPAgent/initialize.test.ts +++ b/src/__tests__/CodexACPAgent/initialize.test.ts @@ -68,7 +68,7 @@ describe('CodexACPAgent - initialize', () => { goal: { version: 1, controlMethod: "_session/goal", - actions: ["pause", "resume", "clear"], + actions: ["set", "pause", "resume", "clear"], }, }, }); diff --git a/src/index.ts b/src/index.ts index 970bf324..0bddc4e2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -32,10 +32,17 @@ const sessionSteerParamsParser = z.object({ prompt: z.array(z.any()), }).passthrough(); -const goalControlParamsParser = z.object({ - sessionId: z.string(), - action: z.enum(["pause", "resume", "clear"]), -}).passthrough(); +const goalControlParamsParser = z.discriminatedUnion("action", [ + z.object({ + sessionId: z.string(), + action: z.literal("set"), + objective: z.string().trim().min(1), + }).passthrough(), + z.object({ + sessionId: z.string(), + action: z.enum(["pause", "resume", "clear"]), + }).passthrough(), +]); if (process.argv.includes("--version")) { console.log(`${packageJson.name} ${packageJson.version}`);