From 6448a8575575d213da8bc927455b4ec554de5c63 Mon Sep 17 00:00:00 2001 From: jim Date: Thu, 27 Aug 2026 17:04:35 +0200 Subject: [PATCH 1/3] fix: keep idle steering fallback host-owned (cherry picked from commit 84bfbe8318400b139214e9aa51352585aae19368) --- src/AcpExtensions.ts | 12 +++- src/CodexAcpServer.ts | 32 ++++++++-- .../CodexACPAgent/steer-events.test.ts | 59 +++++++++++++++++++ src/__tests__/SteeringQueue.test.ts | 6 +- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/AcpExtensions.ts b/src/AcpExtensions.ts index b450c8bd0..dbd80cec7 100644 --- a/src/AcpExtensions.ts +++ b/src/AcpExtensions.ts @@ -122,11 +122,17 @@ export async function legacySetSessionModel( export type SessionSteerRequest = { sessionId: SessionId; prompt: ContentBlock[]; + _meta?: { + steering?: { + /** Leave idle input unconsumed so the host can own a normal prompt lifecycle. */ + idleBehavior?: "promptRequired"; + }; + }; } -export type SessionSteeringResponse = { - outcome: "injected" | "startedNewTurn" | "failed"; -} +export type SessionSteeringResponse = + | {outcome: "injected" | "startedNewTurn" | "failed"} + | {outcome: "promptRequired", reason: "noRunningTurn"} export type SessionSteeringExtRequest = { method: typeof SESSION_STEERING_METHOD; diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 63ae8011c..df881b189 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -3,6 +3,7 @@ import {RequestError, type SessionId, type SessionModeState} from "@agentclientp import {CodexEventHandler, type CompletedPlan} from "./CodexEventHandler"; import {attachmentFileUri, desktopAttachmentHistory} from "./DesktopAttachmentHistory"; import {CodexApprovalHandler} from "./permissions/CodexApprovalHandler"; +import {isRecord} from "./permissions/json"; import {PermissionLifecycleContext} from "./permissions/lifecycle"; import {CodexElicitationHandler} from "./CodexElicitationHandler"; import {type CodexAuthRequest, getCodexAuthMethods, isCodexAuthRequest} from "./CodexAuthMethod"; @@ -1755,8 +1756,9 @@ export class CodexAcpServer { * check guards against deleting a queue a later request has since reused). * * @param params The target session id and the prompt to steer with. - * @returns Whether the prompt joined the active turn ("injected"), started a - * new one ("startedNewTurn"), or could not be applied ("failed"); see + * @returns Whether the prompt joined the active turn ("injected"), needs a + * host-owned prompt ("promptRequired"), started a new one + * ("startedNewTurn"), or could not be applied ("failed"); see * {@link performSteeringRequest}. */ async executeOrQueueSteeringRequest(params: SessionSteerRequest): Promise { @@ -1794,11 +1796,13 @@ export class CodexAcpServer { /** * Delivers a steering prompt to the session: injects it into the live turn - * when there is one, otherwise starts a new turn. + * when there is one. On an idle race, an opted-in host keeps ownership of + * the content; legacy clients retain the adapter-started turn behavior. * * @param params The target session id and the prompt to steer with. - * @returns "injected" when the prompt joined an existing turn, otherwise the - * outcome of starting a new turn. + * @returns "injected" when the prompt joined an existing turn, + * "promptRequired" when the host requested an owned idle fallback, or + * the outcome of starting a legacy fallback turn. */ private async performSteeringRequest(params: SessionSteerRequest): Promise { logger.log("Steering session requested", { @@ -1816,6 +1820,9 @@ export class CodexAcpServer { return {outcome: "injected"}; } } + if (params._meta?.steering?.idleBehavior === "promptRequired") { + return {outcome: "promptRequired", reason: "noRunningTurn"}; + } return await this.startNewTurnFromSteering(params); } @@ -1996,9 +2003,24 @@ export class CodexAcpServer { if (typeof sessionId !== "string" || !Array.isArray(prompt)) { throw RequestError.invalidParams(); } + const meta = params["_meta"]; + if (meta !== undefined && !isRecord(meta)) { + throw RequestError.invalidParams(undefined, "steering _meta must be an object"); + } + const steering = meta?.["steering"]; + if (steering !== undefined && !isRecord(steering)) { + throw RequestError.invalidParams(undefined, "steering _meta.steering must be an object"); + } + const idleBehavior = steering?.["idleBehavior"]; + if (idleBehavior !== undefined && idleBehavior !== "promptRequired") { + throw RequestError.invalidParams(undefined, "unsupported steering idleBehavior"); + } return { sessionId: sessionId, prompt: prompt as acp.ContentBlock[], + ...(idleBehavior === "promptRequired" ? { + _meta: {steering: {idleBehavior}}, + } : {}), }; } diff --git a/src/__tests__/CodexACPAgent/steer-events.test.ts b/src/__tests__/CodexACPAgent/steer-events.test.ts index c13a719be..aa4006de9 100644 --- a/src/__tests__/CodexACPAgent/steer-events.test.ts +++ b/src/__tests__/CodexACPAgent/steer-events.test.ts @@ -109,6 +109,23 @@ describe('_session/steering', () => { }); }); + it('leaves idle input unconsumed when the host requests an owned prompt fallback', async () => { + const mockFixture = createCodexMockTestFixture(); + const sessionState = createTestSessionState(); + vi.spyOn(mockFixture.getCodexAcpAgent(), "getSessionState").mockReturnValue(sessionState); + const turnStartSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnStart"); + const turnSteerSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnSteer"); + + await expect(mockFixture.getCodexAcpAgent().extMethod(SESSION_STEERING_METHOD, { + sessionId: "session-id", + prompt: [{type: "text", text: "host-owned follow-up"}], + _meta: {steering: {idleBehavior: "promptRequired"}}, + })).resolves.toEqual({outcome: "promptRequired", reason: "noRunningTurn"}); + + expect(turnStartSpy).not.toHaveBeenCalled(); + expect(turnSteerSpy).not.toHaveBeenCalled(); + }); + it('starts a new turn when Codex reports that the tracked turn is no longer active', async () => { const {mockFixture, sessionState, turnCompleted} = startActiveTurn(); const nextTurnCompleted = deferred(); @@ -152,6 +169,38 @@ describe('_session/steering', () => { }); }); + it('returns the host-owned fallback when the tracked turn wins the idle race', async () => { + const {mockFixture, sessionState, turnCompleted} = startActiveTurn(); + const turnStartSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnStart"); + vi.spyOn(mockFixture.getCodexAppServerClient(), "turnSteer").mockImplementation(async () => { + turnCompleted.resolve({ + threadId: "session-id", + turn: createTurn("turn-id", "completed"), + }); + throw Object.assign(new Error("Internal error"), { + data: {details: "no active turn to steer"}, + }); + }); + + const promptPromise = mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "long running prompt"}], + }); + await vi.waitFor(() => { + expect(sessionState.currentTurnId).toBe("turn-id"); + }); + + await expect(mockFixture.getCodexAcpAgent().extMethod(SESSION_STEERING_METHOD, { + sessionId: "session-id", + prompt: [{type: "text", text: "racing follow-up"}], + _meta: {steering: {idleBehavior: "promptRequired"}}, + })).resolves.toEqual({outcome: "promptRequired", reason: "noRunningTurn"}); + + await expect(promptPromise).resolves.toMatchObject({stopReason: "end_turn"}); + expect(turnStartSpy).toHaveBeenCalledTimes(1); + expect(sessionState.currentTurnId).toBeNull(); + }); + it('serializes concurrent late steering requests without dropping either prompt', async () => { const mockFixture = createCodexMockTestFixture(); const sessionState = createTestSessionState(); @@ -212,6 +261,16 @@ describe('_session/steering', () => { })).rejects.toThrow(RequestError); }); + it('rejects an unsupported idle behavior', async () => { + const mockFixture = createCodexMockTestFixture(); + + await expect(mockFixture.getCodexAcpAgent().extMethod(SESSION_STEERING_METHOD, { + sessionId: "session-id", + prompt: [{type: "text", text: "follow-up"}], + _meta: {steering: {idleBehavior: "startDetachedTurn"}}, + })).rejects.toThrow("unsupported steering idleBehavior"); + }); + it('rejects image input when the model does not support it', async () => { const {mockFixture} = startActiveTurn({supportedInputModalities: ["text"]}); const turnSteerSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnSteer"); diff --git a/src/__tests__/SteeringQueue.test.ts b/src/__tests__/SteeringQueue.test.ts index 54b5e63b6..3b56c5463 100644 --- a/src/__tests__/SteeringQueue.test.ts +++ b/src/__tests__/SteeringQueue.test.ts @@ -58,7 +58,11 @@ describe("SteeringQueue", () => { }); it("delivers each handler result to its own caller", async () => { - const outcomes: SessionSteeringResponse["outcome"][] = ["injected", "startedNewTurn", "injected"]; + const outcomes: Array> = [ + "injected", + "startedNewTurn", + "injected", + ]; let call = 0; const queue = new SteeringQueue(async () => ({outcome: outcomes[call++]!})); From a1382ac5005de4d3791857c420dd26d5211ff2f7 Mon Sep 17 00:00:00 2001 From: lihong Date: Thu, 8 Oct 2026 11:05:13 +0800 Subject: [PATCH 2/3] test: cover host-owned steering during plan approval --- README.md | 28 ++++++++++++++ .../CodexACPAgent/plan-review-events.test.ts | 38 +++++++++++++++++++ .../CodexACPAgent/steer-events.test.ts | 25 +++++++++++- 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 0c4ba6678..7878c5b8c 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,34 @@ The adapter advertises ACP auth methods during initialization. Clients can authe - API key via `CODEX_API_KEY` or `OPENAI_API_KEY`. - A custom OpenAI-compatible gateway, when the client opts in to the gateway auth capability. +## Host-owned steering fallback + +Clients can opt in to host-owned fallback when calling `_session/steering`: + +```json +{ + "sessionId": "session-id", + "prompt": [{ "type": "text", "text": "Adjust the plan" }], + "_meta": { "steering": { "idleBehavior": "promptRequired" } } +} +``` + +When a Codex turn is running, the input is injected into that turn and the +response is `{ "outcome": "injected" }`. If no turn can accept it, the adapter +returns `{ "outcome": "promptRequired", "reason": "noRunningTurn" }` without +consuming the input or starting a new prompt. This also returns while the +original prompt is waiting for plan approval. + +The client keeps the input queued, answers any pending permission request, and +waits for the original `session/prompt` response before sending the queued input +through its normal `session/prompt` lifecycle. That request owns completion, +usage, and failure reporting. `promptRequired` does not cancel an active prompt +or answer its permission requests. + +Calls without this opt-in retain the existing `startedNewTurn` fallback. +The initialization capability `_meta.steering.supported` advertises steering +in general; it does not distinguish support for this newer opt-in contract. + ## Runtime options - `CODEX_API_KEY` - API key used when the API-key auth method is selected. Takes precedence over `OPENAI_API_KEY`. diff --git a/src/__tests__/CodexACPAgent/plan-review-events.test.ts b/src/__tests__/CodexACPAgent/plan-review-events.test.ts index a5b3ab0e7..73d7839aa 100644 --- a/src/__tests__/CodexACPAgent/plan-review-events.test.ts +++ b/src/__tests__/CodexACPAgent/plan-review-events.test.ts @@ -2,6 +2,7 @@ import * as acp from "@agentclientprotocol/sdk"; import {beforeEach, describe, expect, it, vi} from "vitest"; import {PLAN_COLLABORATION_MODE} from "../../CollaborationModeConfig"; import {ClientCapabilities} from "../../tool-calls/ClientCapabilities"; +import {SESSION_STEERING_METHOD} from "../../AcpExtensions"; import { createCodexMockTestFixture, createTestSessionState, @@ -160,6 +161,43 @@ describe("CodexACPAgent - plan review", () => { return {promptPromise, sessionState, turnStart, implementationTurn}; } + it.each(["revise", "cancel"])("returns host-owned steering immediately during plan approval, allowing %s", async (action) => { + const permission = deferred(); + const {promptPromise, turnStart} = await startPlanPrompt(null, { + permissionResponse: permission.promise, + emitCompletionNotification: true, + }); + await vi.waitFor(() => expect(fixture.getAcpConnectionEvents([]) + .some(event => event.method === "requestPermission")).toBe(true)); + // The plan turn has finished, but its prompt still owns the approval. + // Codex rejects steering even if the adapter's cached turn id remains. + vi.spyOn(fixture.getCodexAppServerClient(), "turnSteer") + .mockRejectedValue(new Error("no active turn to steer")); + let promptFinished = false; + void promptPromise.then(() => { promptFinished = true; }); + try { + await expect(fixture.getCodexAcpAgent().extMethod(SESSION_STEERING_METHOD, { + sessionId, + prompt: [{type: "text", text: "revise this plan"}], + _meta: {steering: {idleBehavior: "promptRequired"}}, + })).resolves.toEqual({outcome: "promptRequired", reason: "noRunningTurn"}); + expect(promptFinished).toBe(false); + expect(turnStart).toHaveBeenCalledTimes(1); + if (action === "cancel") { + await fixture.getCodexAcpAgent().cancel({sessionId}); + // Cancelling the completed turn does not answer its pending permission request. + permission.resolve({outcome: {outcome: "cancelled"}}); + await expect(promptPromise).resolves.toMatchObject({stopReason: "end_turn"}); + } else { + permission.resolve({outcome: {outcome: "selected", optionId: "revise_plan"}}); + await expect(promptPromise).resolves.toMatchObject({stopReason: "end_turn"}); + } + expect(turnStart).toHaveBeenCalledTimes(1); + } finally { + permission.resolve({outcome: {outcome: "cancelled"}}); + } + }, 2000); + it("requests plan permission and starts one implementation turn when approved", async () => { const {promptPromise, sessionState, turnStart, implementationTurn} = await startPlanPrompt("implement_plan"); diff --git a/src/__tests__/CodexACPAgent/steer-events.test.ts b/src/__tests__/CodexACPAgent/steer-events.test.ts index aa4006de9..09b6c5d45 100644 --- a/src/__tests__/CodexACPAgent/steer-events.test.ts +++ b/src/__tests__/CodexACPAgent/steer-events.test.ts @@ -49,7 +49,7 @@ describe('_session/steering', () => { vi.clearAllMocks(); }); - it('reports injected when the input joins the active turn', async () => { + it.each([undefined, {steering: {idleBehavior: "promptRequired"}}])('reports injected when the input joins the active turn (metadata: %j)', async (_meta) => { const {mockFixture, sessionState, turnCompleted} = startActiveTurn(); const turnSteerSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnSteer") .mockResolvedValue({turnId: "turn-id"}); @@ -65,6 +65,7 @@ describe('_session/steering', () => { await expect(mockFixture.getCodexAcpAgent().extMethod(SESSION_STEERING_METHOD, { sessionId: "session-id", prompt: [{type: "text", text: "also keep backward compatibility"}], + _meta, })).resolves.toEqual({outcome: "injected"}); expect(turnSteerSpy).toHaveBeenCalledWith({ @@ -271,6 +272,28 @@ describe('_session/steering', () => { })).rejects.toThrow("unsupported steering idleBehavior"); }); + it.each([ + null, + [], + false, + "invalid", + {steering: null}, + {steering: []}, + {steering: "invalid"}, + {steering: {idleBehavior: null}}, + ])('rejects malformed steering metadata without starting or injecting a turn (%j)', async (_meta) => { + const mockFixture = createCodexMockTestFixture(); + const turnStartSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnStart"); + const turnSteerSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnSteer"); + await expect(mockFixture.getCodexAcpAgent().extMethod(SESSION_STEERING_METHOD, { + sessionId: "session-id", + prompt: [{type: "text", text: "do not consume this"}], + _meta, + })).rejects.toThrow(RequestError); + expect(turnStartSpy).not.toHaveBeenCalled(); + expect(turnSteerSpy).not.toHaveBeenCalled(); + }); + it('rejects image input when the model does not support it', async () => { const {mockFixture} = startActiveTurn({supportedInputModalities: ["text"]}); const turnSteerSpy = vi.spyOn(mockFixture.getCodexAppServerClient(), "turnSteer"); From 0eb5498de6cbd9b03dce0c3b27d551d2e5d22b45 Mon Sep 17 00:00:00 2001 From: lihong Date: Thu, 8 Oct 2026 11:07:31 +0800 Subject: [PATCH 3/3] docs: clarify steering idle fallback outcomes --- src/CodexAcpServer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index df881b189..ce5afd8f0 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1843,10 +1843,10 @@ export class CodexAcpServer { * A failed injection is fatal only when the turn is still the session's * current turn and Codex reported something other than "no active turn to * steer". Otherwise the turn has already ended underneath us and the caller - * should start a new turn instead. + * should use its configured idle fallback instead. * * @returns true when the prompt was injected; false when the caller should - * fall back to starting a new turn. + * return "promptRequired" or use the legacy new-turn fallback. */ private async injectSteerIntoActiveTurn( params: SessionSteerRequest,