diff --git a/README.md b/README.md index 7cf19dae..66295125 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Use [OpenAI Codex](https://github.com/openai/codex) from [Agent Client Protocol] ## Features - ChatGPT, API key, and client-provided custom gateway authentication. -- Model, reasoning effort, fast mode, approval, and sandbox mode configuration. +- Model, reasoning effort, fast mode, approval reviewer, and approval/sandbox mode configuration. - Text prompts, embedded context, images, resource links, and additional workspace directories. - Shell command, file change, permission request, MCP tool call, terminal output, reasoning, plan, web search, image generation, image view, token usage, and review events. - Subagent launches as standard ACP tool calls, with Codex thread identity and activity details in namespaced `_meta.codex.subagent` metadata. diff --git a/src/ApprovalsReviewerConfig.ts b/src/ApprovalsReviewerConfig.ts new file mode 100644 index 00000000..df11983e --- /dev/null +++ b/src/ApprovalsReviewerConfig.ts @@ -0,0 +1,49 @@ +import type {SessionConfigOption} from "@agentclientprotocol/sdk"; +import type {ApprovalsReviewer} from "./app-server/v2"; + +export const APPROVALS_REVIEWER_CONFIG_ID = "approvals_reviewer"; +export const USER_APPROVALS_REVIEWER = "user"; +export const AUTO_APPROVALS_REVIEWER = "auto_review"; + +export type SelectableApprovalsReviewer = + | typeof USER_APPROVALS_REVIEWER + | typeof AUTO_APPROVALS_REVIEWER; + +export function createApprovalsReviewerConfigOption( + currentValue: SelectableApprovalsReviewer, +): SessionConfigOption { + return { + id: APPROVALS_REVIEWER_CONFIG_ID, + name: "Approval reviewer", + description: "Who reviews eligible approval requests", + category: "_approvals_reviewer", + type: "select", + currentValue, + options: [ + { + value: USER_APPROVALS_REVIEWER, + name: "User", + description: "Ask the user to approve or deny requests", + }, + { + value: AUTO_APPROVALS_REVIEWER, + name: "Auto-review", + description: "Use a reviewer agent to assess eligible requests", + }, + ], + }; +} + +export function parseApprovalsReviewer(value: unknown): SelectableApprovalsReviewer | null { + if (value === USER_APPROVALS_REVIEWER) return USER_APPROVALS_REVIEWER; + if (value === AUTO_APPROVALS_REVIEWER) return AUTO_APPROVALS_REVIEWER; + return null; +} + +export function normalizeApprovalsReviewer( + value: ApprovalsReviewer | null | undefined, +): SelectableApprovalsReviewer { + return value === AUTO_APPROVALS_REVIEWER || value === "guardian_subagent" + ? AUTO_APPROVALS_REVIEWER + : USER_APPROVALS_REVIEWER; +} diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 0c7af207..990df3df 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -44,6 +44,10 @@ import packageJson from "../package.json"; import type {AuthenticationStatusResponse} from "./AcpExtensions"; import {createCodexCollaborationMode} from "./CollaborationModeConfig"; import type {ModeKind} from "./app-server/ModeKind"; +import { + normalizeApprovalsReviewer, + type SelectableApprovalsReviewer, +} from "./ApprovalsReviewerConfig"; /** * Well-known provider id for the client-configurable custom LLM gateway. @@ -344,6 +348,7 @@ export class CodexAcpClient { currentModelId: currentModelId, models: codexModels, collaborationMode: this.getCollaborationMode(response.thread.id), + approvalsReviewer: normalizeApprovalsReviewer(response.approvalsReviewer), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, additionalDirectories, @@ -372,6 +377,7 @@ export class CodexAcpClient { currentModelId: currentModelId, models: codexModels, collaborationMode: this.getCollaborationMode(response.thread.id), + approvalsReviewer: normalizeApprovalsReviewer(response.approvalsReviewer), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, thread: historyResponse.thread, @@ -399,6 +405,7 @@ export class CodexAcpClient { currentModelId: currentModelId, models: codexModels, collaborationMode: this.getCollaborationMode(response.thread.id), + approvalsReviewer: normalizeApprovalsReviewer(response.approvalsReviewer), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, additionalDirectories, @@ -686,6 +693,7 @@ export class CodexAcpClient { async sendPrompt( request: acp.PromptRequest, agentMode: AgentMode, + approvalsReviewer: SelectableApprovalsReviewer, modelId: ModelId, serviceTier: ServiceTier | null, disableSummary: boolean, @@ -704,6 +712,7 @@ export class CodexAcpClient { threadId: request.sessionId, input: input, approvalPolicy: agentMode.approvalPolicy, + approvalsReviewer, sandboxPolicy: addAdditionalDirectoriesToSandboxPolicy(agentMode.sandboxPolicy, additionalDirectories), summary: disableSummary ? "none" : "auto", effort: effort, @@ -898,6 +907,7 @@ export type SessionMetadata = { currentModelId: string, models: Model[], collaborationMode: ModeKind, + approvalsReviewer: SelectableApprovalsReviewer, modelProvider?: string | null, currentServiceTier?: ServiceTier | null, additionalDirectories: string[], diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 928f2b16..68f9f56c 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -90,6 +90,12 @@ import { type ThreadGoalSnapshot, toThreadGoalSnapshot, } from "./ThreadGoalSnapshot"; +import { + APPROVALS_REVIEWER_CONFIG_ID, + createApprovalsReviewerConfigOption, + parseApprovalsReviewer, + type SelectableApprovalsReviewer, +} from "./ApprovalsReviewerConfig"; export interface SessionState { sessionId: string, @@ -98,6 +104,7 @@ export interface SessionState { supportedReasoningEfforts: Array, supportedInputModalities: Array, agentMode: AgentMode, + approvalsReviewer: SelectableApprovalsReviewer, collaborationMode: ModeKind, currentTurnId: string | null; lastTokenUsage: TokenCount | null; @@ -449,6 +456,7 @@ export class CodexAcpServer { supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], agentMode: AgentMode.getInitialAgentMode(), + approvalsReviewer: sessionMetadata.approvalsReviewer, collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -755,6 +763,9 @@ export class CodexAcpServer { case MODE_CONFIG_ID: this.applyModeChange(sessionState, this.stringConfigValue(params)); break; + case APPROVALS_REVIEWER_CONFIG_ID: + this.applyApprovalsReviewerChange(sessionState, this.stringConfigValue(params)); + break; case COLLABORATION_MODE_CONFIG_ID: await this.applyCollaborationModeChange(sessionState, this.stringConfigValue(params)); break; @@ -796,6 +807,14 @@ export class CodexAcpServer { sessionState.agentMode = newMode; } + private applyApprovalsReviewerChange(sessionState: SessionState, value: string): void { + const approvalsReviewer = parseApprovalsReviewer(value); + if (approvalsReviewer === null) { + throw RequestError.invalidParams(); + } + sessionState.approvalsReviewer = approvalsReviewer; + } + private async applyCollaborationModeChange(sessionState: SessionState, value: string): Promise { const mode = parseCollaborationMode(value); if (mode === null) { @@ -1110,6 +1129,7 @@ export class CodexAcpServer { const currentModelId = ModelId.fromString(sessionState.currentModelId); const configOptions = [ sessionState.agentMode.toConfigOption(), + createApprovalsReviewerConfigOption(sessionState.approvalsReviewer), createCollaborationModeConfigOption(sessionState.collaborationMode), createModelConfigOption(sessionState.availableModels, currentModelId.model), ]; @@ -1291,6 +1311,7 @@ export class CodexAcpServer { supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], agentMode: AgentMode.getInitialAgentMode(), + approvalsReviewer: sessionMetadata.approvalsReviewer, collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -1985,6 +2006,7 @@ export class CodexAcpServer { () => this.codexAcpClient.sendPrompt( params, agentMode, + sessionState.approvalsReviewer, modelId, serviceTier, disableSummary, diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index d58bd594..92f5453d 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -2821,6 +2821,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { currentModelId, models: [model], collaborationMode: "default", + approvalsReviewer: "user", additionalDirectories: [], }) .mockResolvedValueOnce({ @@ -2828,6 +2829,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { currentModelId, models: [model], collaborationMode: "default", + approvalsReviewer: "user", additionalDirectories: [], }); const logoutSpy = vi.spyOn(codexAcpClient, "logout").mockResolvedValue(); @@ -2887,6 +2889,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { currentModelId, models: [model], collaborationMode: "default", + approvalsReviewer: "user", modelProvider: "openai", additionalDirectories: [], }); @@ -2940,6 +2943,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { currentModelId, models: [model], collaborationMode: "default", + approvalsReviewer: "user", additionalDirectories: [], }); const logoutSpy = vi.spyOn(codexAcpClient, "logout").mockResolvedValue(); diff --git a/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json b/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json index cad1559e..39b1bc66 100644 --- a/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json +++ b/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json @@ -47,6 +47,7 @@ } ], "approvalPolicy": "on-request", + "approvalsReviewer": "approvalsReviewer", "sandboxPolicy": { "type": "workspaceWrite", "writableRoots": [], diff --git a/src/__tests__/CodexACPAgent/fast-mode-config.test.ts b/src/__tests__/CodexACPAgent/fast-mode-config.test.ts index 6007d529..5d6e12dc 100644 --- a/src/__tests__/CodexACPAgent/fast-mode-config.test.ts +++ b/src/__tests__/CodexACPAgent/fast-mode-config.test.ts @@ -44,6 +44,7 @@ describe("Fast mode session config", () => { currentModelId: "fast-model[medium]", models: [fastModel, slowModel], collaborationMode: "default", + approvalsReviewer: "user", currentServiceTier, additionalDirectories: [], }); diff --git a/src/__tests__/CodexACPAgent/list-sessions.test.ts b/src/__tests__/CodexACPAgent/list-sessions.test.ts index 845ee0fd..a53dddab 100644 --- a/src/__tests__/CodexACPAgent/list-sessions.test.ts +++ b/src/__tests__/CodexACPAgent/list-sessions.test.ts @@ -169,6 +169,7 @@ describe("CodexACPAgent - list sessions", () => { isDefault: true, }], collaborationMode: "default", + approvalsReviewer: "user", currentServiceTier: null, additionalDirectories: ["/repo/extra"], }); diff --git a/src/__tests__/CodexACPAgent/model-filtering.test.ts b/src/__tests__/CodexACPAgent/model-filtering.test.ts index f5344242..4eab0a42 100644 --- a/src/__tests__/CodexACPAgent/model-filtering.test.ts +++ b/src/__tests__/CodexACPAgent/model-filtering.test.ts @@ -129,6 +129,7 @@ describe("Model filtering", () => { currentModelId: "gpt-5.2[medium]", models, collaborationMode: "default", + approvalsReviewer: "user", additionalDirectories: [], }); vi.spyOn(codexAcpClient, "getAccount").mockResolvedValue({account: null, requiresOpenaiAuth: false}); diff --git a/src/__tests__/CodexACPAgent/new-session-logout.test.ts b/src/__tests__/CodexACPAgent/new-session-logout.test.ts index 01a7b429..ba16cef4 100644 --- a/src/__tests__/CodexACPAgent/new-session-logout.test.ts +++ b/src/__tests__/CodexACPAgent/new-session-logout.test.ts @@ -83,6 +83,7 @@ describe("New session logout handling", () => { currentModelId, models: [model], collaborationMode: "default", + approvalsReviewer: "user", modelProvider: "openai", additionalDirectories: [], }) @@ -91,6 +92,7 @@ describe("New session logout handling", () => { currentModelId, models: [model], collaborationMode: "default", + approvalsReviewer: "user", modelProvider: "custom-provider", additionalDirectories: [], }) diff --git a/src/__tests__/CodexACPAgent/session-close.test.ts b/src/__tests__/CodexACPAgent/session-close.test.ts index 5effd4b0..48ca67d7 100644 --- a/src/__tests__/CodexACPAgent/session-close.test.ts +++ b/src/__tests__/CodexACPAgent/session-close.test.ts @@ -458,6 +458,7 @@ async function createSession(options: { currentModelId: "model-id[medium]", models: [model], collaborationMode: "default", + approvalsReviewer: "user", currentServiceTier: null, additionalDirectories: [], }); @@ -505,6 +506,7 @@ function createSessionMetadata(): SessionMetadata { currentModelId: "model-id[medium]", models: [createTestModel()], collaborationMode: "default", + approvalsReviewer: "user", currentServiceTier: null, additionalDirectories: [], }; diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index ab669916..cbc18d28 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -1,5 +1,9 @@ import {describe, expect, it, vi} from "vitest"; -import {createCodexMockTestFixture, createTestModel} from "../acp-test-utils"; +import { + createCodexMockTestFixture, + createTestModel, + setupPromptTestSession, +} from "../acp-test-utils"; import {AgentMode, MODE_CONFIG_ID} from "../../AgentMode"; import { MODEL_CONFIG_ID, @@ -11,6 +15,13 @@ import { COLLABORATION_MODE_CONFIG_ID, PLAN_COLLABORATION_MODE, } from "../../CollaborationModeConfig"; +import { + APPROVALS_REVIEWER_CONFIG_ID, + AUTO_APPROVALS_REVIEWER, + createApprovalsReviewerConfigOption, + type SelectableApprovalsReviewer, + USER_APPROVALS_REVIEWER, +} from "../../ApprovalsReviewerConfig"; const lowEffort: ReasoningEffortOption = {reasoningEffort: "low", description: "Fast"}; const mediumEffort: ReasoningEffortOption = {reasoningEffort: "medium", description: "Balanced"}; @@ -35,7 +46,11 @@ function buildModels(): {fast: Model; slow: Model} { return {fast, slow}; } -async function createSession(currentModelId: string, availableModels: Array) { +async function createSession( + currentModelId: string, + availableModels: Array, + approvalsReviewer: SelectableApprovalsReviewer = USER_APPROVALS_REVIEWER, +) { const fixture = createCodexMockTestFixture(); const codexAcpAgent = fixture.getCodexAcpAgent(); const codexAcpClient = fixture.getCodexAcpClient(); @@ -47,6 +62,7 @@ async function createSession(currentModelId: string, availableModels: Array { - it("exposes mode, model, reasoning_effort and fast-mode in the new session response", async () => { + it("exposes mode, approval reviewer, model, reasoning_effort and fast-mode in the new session response", async () => { const {fast, slow} = buildModels(); const {response} = await createSession("fast-model[medium]", [fast, slow]); const ids = response.configOptions?.map(o => o.id); - expect(ids).toEqual([MODE_CONFIG_ID, COLLABORATION_MODE_CONFIG_ID, MODEL_CONFIG_ID, REASONING_EFFORT_CONFIG_ID, "fast-mode"]); + expect(ids).toEqual([ + MODE_CONFIG_ID, + APPROVALS_REVIEWER_CONFIG_ID, + COLLABORATION_MODE_CONFIG_ID, + MODEL_CONFIG_ID, + REASONING_EFFORT_CONFIG_ID, + "fast-mode", + ]); + + const approvalsReviewerOption = response.configOptions?.find( + o => o.id === APPROVALS_REVIEWER_CONFIG_ID, + ); + expect(approvalsReviewerOption).toEqual( + createApprovalsReviewerConfigOption(USER_APPROVALS_REVIEWER), + ); const modelOption = response.configOptions?.find(o => o.id === MODEL_CONFIG_ID); expect(modelOption).toMatchObject({ @@ -101,7 +131,12 @@ describe("Session config options", () => { const {codexAcpAgent, response} = await createSession("custom-model[high]", [fast, slow]); const ids = response.configOptions?.map(o => o.id); - expect(ids).toEqual([MODE_CONFIG_ID, COLLABORATION_MODE_CONFIG_ID, MODEL_CONFIG_ID]); + expect(ids).toEqual([ + MODE_CONFIG_ID, + APPROVALS_REVIEWER_CONFIG_ID, + COLLABORATION_MODE_CONFIG_ID, + MODEL_CONFIG_ID, + ]); const modelOption = response.configOptions?.find(o => o.id === MODEL_CONFIG_ID); expect(modelOption).toMatchObject({ @@ -154,6 +189,39 @@ describe("Session config options", () => { expect((modeOption as any).currentValue).toBe(AgentMode.ReadOnly.id); }); + it("changes the approval reviewer via setSessionConfigOption", async () => { + const {fast} = buildModels(); + const {codexAcpAgent} = await createSession("fast-model[medium]", [fast]); + + const result = await codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: APPROVALS_REVIEWER_CONFIG_ID, + value: AUTO_APPROVALS_REVIEWER, + }); + + expect(codexAcpAgent.getSessionState("session-id").approvalsReviewer).toBe( + AUTO_APPROVALS_REVIEWER, + ); + expect( + result.configOptions?.find(o => o.id === APPROVALS_REVIEWER_CONFIG_ID), + ).toEqual(createApprovalsReviewerConfigOption(AUTO_APPROVALS_REVIEWER)); + }); + + it("sends the selected approval reviewer with the next turn", async () => { + const {mockFixture, turnStartSpy} = setupPromptTestSession({ + approvalsReviewer: AUTO_APPROVALS_REVIEWER, + }); + + await mockFixture.getCodexAcpAgent().prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "hello"}], + }); + + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ + approvalsReviewer: AUTO_APPROVALS_REVIEWER, + })); + }); + it("changes collaboration mode without starting a model turn", async () => { const {fast} = buildModels(); const {codexAcpAgent, codexAcpClient} = await createSession("fast-model[medium]", [fast]); @@ -280,6 +348,7 @@ describe("Session config options", () => { currentModelId: "fast-model[medium]", models: [fast], collaborationMode: "default", + approvalsReviewer: USER_APPROVALS_REVIEWER, additionalDirectories: [], }); await codexAcpAgent.newSession({cwd: "/test/cwd", mcpServers: []}); @@ -316,7 +385,7 @@ describe("Session config options", () => { expect(codexAcpAgent.getSessionState("session-id").currentModelId).toBe("slow-model[medium]"); }); - it("rejects unknown model, effort, and mode values", async () => { + it("rejects unknown model, effort, mode, and approval reviewer values", async () => { const {fast} = buildModels(); const {codexAcpAgent} = await createSession("fast-model[medium]", [fast]); @@ -337,5 +406,11 @@ describe("Session config options", () => { configId: MODE_CONFIG_ID, value: "no-such-mode", })).rejects.toThrow(); + + await expect(codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: APPROVALS_REVIEWER_CONFIG_ID, + value: "no-such-reviewer", + })).rejects.toThrow(); }); }); diff --git a/src/__tests__/CodexACPAgent/session-delete.test.ts b/src/__tests__/CodexACPAgent/session-delete.test.ts index ba0207ed..6bee0a2c 100644 --- a/src/__tests__/CodexACPAgent/session-delete.test.ts +++ b/src/__tests__/CodexACPAgent/session-delete.test.ts @@ -129,6 +129,7 @@ async function createSession(): Promise<{ currentModelId: "model-id[medium]", models: [model], collaborationMode: "default", + approvalsReviewer: "user", currentServiceTier: null, additionalDirectories: [], }); diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index de4ad962..0f1aa793 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -14,6 +14,7 @@ import {AgentMode} from "../AgentMode"; import {DEFAULT_COLLABORATION_MODE} from "../CollaborationModeConfig"; import {expect, vi} from "vitest"; import type {Model, ReasoningEffortOption} from "../app-server/v2"; +import {USER_APPROVALS_REVIEWER} from "../ApprovalsReviewerConfig"; export type MethodCallEvent = { method: string; args: any[] }; @@ -384,6 +385,7 @@ export function createTestSessionState(overrides?: Partial): Sessi supportedReasoningEfforts: [], supportedInputModalities: ["text", "image"], agentMode: AgentMode.DEFAULT_AGENT_MODE, + approvalsReviewer: USER_APPROVALS_REVIEWER, collaborationMode: DEFAULT_COLLABORATION_MODE, fastModeEnabled: false, currentModelSupportsFast: false,