diff --git a/src/AgentMode.ts b/src/AgentMode.ts index aed9710c..c8c1ad03 100644 --- a/src/AgentMode.ts +++ b/src/AgentMode.ts @@ -1,41 +1,63 @@ -import type {AskForApproval, SandboxMode, SandboxPolicy} from "./app-server/v2"; +import type {ApprovalsReviewer, AskForApproval, SandboxMode, SandboxPolicy} from "./app-server/v2"; import type {SessionConfigOption, SessionMode, SessionModeState} from "@agentclientprotocol/sdk"; export const MODE_CONFIG_ID = "mode"; +type AgentModeKind = "plan" | "auto_review" | "standard" | "full_access"; + export class AgentMode { readonly id: string; readonly name: string; readonly description: string; + readonly kind: AgentModeKind; readonly approvalPolicy: AskForApproval; + readonly approvalsReviewer: ApprovalsReviewer; readonly sandboxPolicy: SandboxPolicy; readonly sandboxMode: SandboxMode; - private constructor(id: string, name: string, description: string, approval: AskForApproval, sandbox: SandboxPolicy, sandboxMode: SandboxMode) { + private constructor( + id: string, + name: string, + description: string, + kind: AgentModeKind, + approvalPolicy: AskForApproval, + approvalsReviewer: ApprovalsReviewer, + sandboxPolicy: SandboxPolicy, + sandboxMode: SandboxMode, + ) { this.id = id; this.name = name; this.description = description; - this.approvalPolicy = approval; - this.sandboxPolicy = sandbox; + this.kind = kind; + this.approvalPolicy = approvalPolicy; + this.approvalsReviewer = approvalsReviewer; + this.sandboxPolicy = sandboxPolicy; this.sandboxMode = sandboxMode; // same as sandboxPolicy, need to look for } static readonly ReadOnly = new AgentMode( "read-only", - "Read-only", - "Requires approval to edit files and run commands.", + "Ask for approval", + "Always ask to edit external files and use the internet", + "standard", "on-request", + "user", { - "type": "readOnly", - "networkAccess": false + type: "workspaceWrite", + writableRoots: [], + networkAccess: false, + excludeTmpdirEnvVar: false, + excludeSlashTmp: false, }, - "read-only" + "workspace-write", ); static readonly Agent = new AgentMode( "agent", - "Agent", - "Read and edit files, and run commands.", + "Approve for me", + "Only ask for actions detected as potentially unsafe", + "auto_review", "on-request", + "auto_review", { type: "workspaceWrite", writableRoots: [], @@ -43,15 +65,17 @@ export class AgentMode { excludeTmpdirEnvVar: false, excludeSlashTmp: false }, - "workspace-write" + "workspace-write", ); static readonly AgentFullAccess = new AgentMode( "agent-full-access", - "Agent (full access)", - "Codex can edit files outside this workspace and run commands with network access. Exercise caution when using.", + "Full access", + "Unrestricted access to the internet and any file on your computer", + "full_access", "never", + "user", {"type": "dangerFullAccess"}, - "danger-full-access" + "danger-full-access", ); static DEFAULT_AGENT_MODE = AgentMode.Agent; @@ -61,6 +85,7 @@ export class AgentMode { id: this.id, name: this.name, description: this.description, + _meta: {kind: this.kind}, }; } @@ -83,6 +108,7 @@ export class AgentMode { value: mode.id, name: mode.name, description: mode.description, + _meta: {kind: mode.kind}, })), }; } diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 10e724e7..bec75265 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -861,6 +861,7 @@ export class CodexAcpClient { threadId: request.sessionId, input: input, approvalPolicy: agentMode.approvalPolicy, + approvalsReviewer: agentMode.approvalsReviewer, sandboxPolicy: addAdditionalDirectoriesToSandboxPolicy(agentMode.sandboxPolicy, additionalDirectories), summary: disableSummary ? "none" : "auto", effort: effort, diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 305982d3..67cd14b3 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -937,6 +937,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { sessionId: "session-id", cwd: "/workspace", additionalDirectories: ["/workspace/extra"], + agentMode: AgentMode.Agent, })); await codexAcpAgent.prompt({ @@ -955,6 +956,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { type: "workspaceWrite", writableRoots: ["/workspace/extra"], }); + expect(turnStartSpy.mock.calls[0]![0].approvalsReviewer).toBe("auto_review"); }); function loadNotifications(){ 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/e2e/acp-e2e-file-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts index 5cd2140c..b80920d0 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts @@ -44,11 +44,11 @@ describeE2E("E2E file approval tests", () => { }); }); -describeE2E("E2E Agent mode file permission tests", () => { +describeE2E("E2E read-only mode file permission tests", () => { let fixture: SpawnedAgentFixture; beforeEach(async () => { - fixture = await createAuthenticatedFixture(AgentMode.Agent); + fixture = await createAuthenticatedFixture(AgentMode.ReadOnly); }); afterEach(async () => { @@ -65,7 +65,7 @@ describeE2E("E2E Agent mode file permission tests", () => { }); }); -describeE2E("E2E Agent with full access file permission tests", () => { +describeE2E("E2E full-access mode file permission tests", () => { let fixture: SpawnedAgentFixture; beforeEach(async () => { diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts index 89387e5b..b0c087d7 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts @@ -17,15 +17,21 @@ import { const FIRST_FILE_NAME = "approval-first.txt"; const SECOND_FILE_NAME = "approval-second.txt"; -const COMMAND = `if [ -e ${FIRST_FILE_NAME} ]; then touch ${SECOND_FILE_NAME}; else touch ${FIRST_FILE_NAME}; fi`; describeE2E("E2E shell approval tests", () => { let fixture: SpawnedAgentFixture; let sessionId: string; + let firstFilePath: string; + let secondFilePath: string; + let command: string; beforeEach(async () => { fixture = await createAuthenticatedFixture(AgentMode.ReadOnly); sessionId = (await fixture.createSession()).sessionId; + const outsideWorkspace = createDirOutsideWorkspace(fixture); + firstFilePath = path.join(outsideWorkspace, FIRST_FILE_NAME); + secondFilePath = path.join(outsideWorkspace, SECOND_FILE_NAME); + command = `if [ -e '${firstFilePath}' ]; then touch '${secondFilePath}'; else touch '${firstFilePath}'; fi`; }); afterEach(async () => { @@ -34,8 +40,8 @@ describeE2E("E2E shell approval tests", () => { async function promptShellCommandTwice(): Promise { for (const text of [ - `Use your shell tool to run exactly \`${COMMAND}\`.`, - `Use your shell tool to run exactly the same command again: \`${COMMAND}\`.`, + `Use your shell tool to run exactly \`${command}\`.`, + `Use your shell tool to run exactly the same command again: \`${command}\`.`, ]) { expectEndTurn(await fixture.connection.prompt({ sessionId, @@ -52,33 +58,33 @@ describeE2E("E2E shell approval tests", () => { : null )); await promptShellCommandTwice(); - expect(fs.existsSync(path.join(fixture.workspaceDir, FIRST_FILE_NAME))).toBe(true); - expect(fs.existsSync(path.join(fixture.workspaceDir, SECOND_FILE_NAME))).toBe(false); + expect(fs.existsSync(firstFilePath)).toBe(true); + expect(fs.existsSync(secondFilePath)).toBe(false); expectPermissionRequests(fixture, sessionId, {execute: 2, edit: 0}); }); it("skips subsequent approvals when allow_for_session is selected", async () => { fixture.setPermissionResponder(createPermissionResponder("execute", ApprovalOptionId.AllowForSession)); await promptShellCommandTwice(); - expect(fs.existsSync(path.join(fixture.workspaceDir, FIRST_FILE_NAME))).toBe(true); - expect(fs.existsSync(path.join(fixture.workspaceDir, SECOND_FILE_NAME))).toBe(true); + expect(fs.existsSync(firstFilePath)).toBe(true); + expect(fs.existsSync(secondFilePath)).toBe(true); expectPermissionRequests(fixture, sessionId, {execute: 1, edit: 0}); }); it("cancels every command when cancel is selected", async () => { fixture.setPermissionResponder(createPermissionResponder("execute", ApprovalOptionId.Cancel)); await promptShellCommandTwice(); - expect(fs.existsSync(path.join(fixture.workspaceDir, FIRST_FILE_NAME))).toBe(false); - expect(fs.existsSync(path.join(fixture.workspaceDir, SECOND_FILE_NAME))).toBe(false); + expect(fs.existsSync(firstFilePath)).toBe(false); + expect(fs.existsSync(secondFilePath)).toBe(false); expectPermissionRequests(fixture, sessionId, {execute: 2, edit: 0}); }); }); -describeE2E("E2E Agent mode shell permission tests", () => { +describeE2E("E2E read-only mode shell permission tests", () => { let fixture: SpawnedAgentFixture; beforeEach(async () => { - fixture = await createAuthenticatedFixture(AgentMode.Agent); + fixture = await createAuthenticatedFixture(AgentMode.ReadOnly); }); afterEach(async () => { @@ -99,7 +105,7 @@ describeE2E("E2E Agent mode shell permission tests", () => { }); }); -describeE2E("E2E Agent with full access shell permission tests", () => { +describeE2E("E2E full-access mode shell permission tests", () => { let fixture: SpawnedAgentFixture; beforeEach(async () => { diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts index 9d5284c5..defb0c38 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts @@ -80,7 +80,7 @@ describeE2E("E2E tests", () => { }); it("respects INITIAL_AGENT_MODE when seeding the initial session mode", async () => { - const initialMode = AgentMode.ReadOnly; + const initialMode = AgentMode.Agent; fixture = await createAuthenticatedFixture(initialMode); const session = await fixture.createSession(); diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index ab669916..e895d282 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -90,6 +90,23 @@ describe("Session config options", () => { category: "mode", currentValue: AgentMode.DEFAULT_AGENT_MODE.id, type: "select", + options: [ + { + value: "read-only", + name: "Ask for approval", + description: "Always ask to edit external files and use the internet", + }, + { + value: "agent", + name: "Approve for me", + description: "Only ask for actions detected as potentially unsafe", + }, + { + value: "agent-full-access", + name: "Full access", + description: "Unrestricted access to the internet and any file on your computer", + }, + ], }); expect((modeOption as any).options.map((o: any) => o.value)).toEqual( AgentMode.all().map(m => m.id) @@ -146,12 +163,12 @@ describe("Session config options", () => { const result = await codexAcpAgent.setSessionConfigOption({ sessionId: "session-id", configId: MODE_CONFIG_ID, - value: AgentMode.ReadOnly.id, + value: AgentMode.Agent.id, }); - expect(codexAcpAgent.getSessionState("session-id").agentMode).toBe(AgentMode.ReadOnly); + expect(codexAcpAgent.getSessionState("session-id").agentMode).toBe(AgentMode.Agent); const modeOption = result.configOptions?.find(o => o.id === MODE_CONFIG_ID); - expect((modeOption as any).currentValue).toBe(AgentMode.ReadOnly.id); + expect((modeOption as any).currentValue).toBe(AgentMode.Agent.id); }); it("changes collaboration mode without starting a model turn", async () => {