From ef40c529f82c87b090bf2efdf8eea2ce448aec90 Mon Sep 17 00:00:00 2001 From: prateek Date: Sat, 7 Mar 2026 20:27:01 +0530 Subject: [PATCH] fix: handle permissions=skip correctly in codex plugin (#337) * fix: handle permissions=skip correctly in codex plugin * refactor: rename permission mode to permissionless and align agent mappings --------- Co-authored-by: Jayesh Sharma --- agent-orchestrator.yaml.example | 6 ++- packages/core/src/config.ts | 7 ++- packages/core/src/session-manager.ts | 4 +- packages/core/src/types.ts | 36 +++++++++++++- .../plugins/agent-aider/src/index.test.ts | 18 +++++-- packages/plugins/agent-aider/src/index.ts | 12 ++++- .../agent-claude-code/src/index.test.ts | 19 ++++++-- .../plugins/agent-claude-code/src/index.ts | 15 +++++- .../plugins/agent-codex/src/index.test.ts | 47 ++++++++++++++----- packages/plugins/agent-codex/src/index.ts | 20 ++++++-- 10 files changed, 151 insertions(+), 33 deletions(-) diff --git a/agent-orchestrator.yaml.example b/agent-orchestrator.yaml.example index 99daf79e6..285a2d853 100644 --- a/agent-orchestrator.yaml.example +++ b/agent-orchestrator.yaml.example @@ -45,7 +45,11 @@ projects: # Agent-specific config # agentConfig: - # permissions: skip # --dangerously-skip-permissions + # permissions: permissionless # modes: permissionless | default | auto-edit | suggest + # # - permissionless: no interactive permission prompts + # # - default: agent defaults + # # - auto-edit: auto-approve edits where supported + # # - suggest: conservative/untrusted-approval mode where supported # model: opus # Inline rules included in every agent prompt for this project diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 945a3bca5..f9e92288f 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -51,9 +51,14 @@ const NotifierConfigSchema = z }) .passthrough(); +const AgentPermissionSchema = z + .enum(["permissionless", "default", "auto-edit", "suggest", "skip"]) + .default("permissionless") + .transform((value) => (value === "skip" ? "permissionless" : value)); + const AgentSpecificConfigSchema = z .object({ - permissions: z.enum(["skip", "default"]).default("skip"), + permissions: AgentPermissionSchema, model: z.string().optional(), }) .passthrough(); diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index dabb5bba3..1e55b2090 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -718,11 +718,11 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { } // Get agent launch config — uses systemPromptFile, no issue/tracker interaction. - // Orchestrator ALWAYS gets skip permissions — it must run ao CLI commands autonomously. + // Orchestrator ALWAYS gets permissionless mode — it must run ao CLI commands autonomously. const agentLaunchConfig = { sessionId, projectConfig: project, - permissions: "skip" as const, + permissions: "permissionless" as const, model: project.agentConfig?.model, systemPromptFile, }; diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 056ce3695..df21abf20 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -329,7 +329,7 @@ export interface AgentLaunchConfig { projectConfig: ProjectConfig; issueId?: string; prompt?: string; - permissions?: "skip" | "default"; + permissions?: AgentPermissionMode; model?: string; /** * System prompt to pass to the agent for orchestrator context. @@ -922,11 +922,43 @@ export interface NotifierConfig { } export interface AgentSpecificConfig { - permissions?: "skip" | "default"; + permissions?: AgentPermissionMode; model?: string; [key: string]: unknown; } +/** + * Canonical cross-agent permission policy mode. + * + * Semantics: + * - permissionless: run without interactive permission prompts (most permissive mode). + * - default: use the agent's normal/default permission model. + * - auto-edit: automatically approve edit actions where the agent supports granular approval policies. + * - suggest: conservative mode that asks for approval on higher-risk/untrusted actions where supported. + * + * Note: Not every agent exposes all granular policies; plugins map these modes to + * their closest supported behavior. + */ +export type AgentPermissionMode = "permissionless" | "default" | "auto-edit" | "suggest"; + +/** Backward-compatible legacy alias accepted in config parsing. */ +export type LegacyAgentPermissionMode = "skip"; + +/** Raw permission input (supports legacy aliases). */ +export type AgentPermissionInput = AgentPermissionMode | LegacyAgentPermissionMode; + +/** Normalize legacy aliases to canonical permission modes. */ +export function normalizeAgentPermissionMode( + mode: string | undefined, +): AgentPermissionMode | undefined { + if (!mode) return undefined; + if (mode !== "permissionless" && mode !== "default" && mode !== "auto-edit" && mode !== "suggest") { + if (mode === "skip") return "permissionless"; + return undefined; + } + return mode; +} + // ============================================================================= // PLUGIN SYSTEM // ============================================================================= diff --git a/packages/plugins/agent-aider/src/index.test.ts b/packages/plugins/agent-aider/src/index.test.ts index 3a31a5f28..7f5045587 100644 --- a/packages/plugins/agent-aider/src/index.test.ts +++ b/packages/plugins/agent-aider/src/index.test.ts @@ -114,8 +114,20 @@ describe("getLaunchCommand", () => { expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("aider"); }); - it("includes --yes when permissions=skip", () => { - const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "skip" })); + it("includes --yes when permissions=permissionless", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "permissionless" })); + expect(cmd).toContain("--yes"); + }); + + it("treats legacy permissions=skip as permissionless", () => { + const cmd = agent.getLaunchCommand( + makeLaunchConfig({ permissions: "skip" as unknown as AgentLaunchConfig["permissions"] }), + ); + expect(cmd).toContain("--yes"); + }); + + it("maps permissions=auto-edit to no-prompt mode on Aider", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "auto-edit" })); expect(cmd).toContain("--yes"); }); @@ -131,7 +143,7 @@ describe("getLaunchCommand", () => { it("combines all options", () => { const cmd = agent.getLaunchCommand( - makeLaunchConfig({ permissions: "skip", model: "sonnet", prompt: "Go" }), + makeLaunchConfig({ permissions: "permissionless", model: "sonnet", prompt: "Go" }), ); expect(cmd).toBe("aider --yes --model 'sonnet' --message 'Go'"); }); diff --git a/packages/plugins/agent-aider/src/index.ts b/packages/plugins/agent-aider/src/index.ts index b103248ca..9f0e11270 100644 --- a/packages/plugins/agent-aider/src/index.ts +++ b/packages/plugins/agent-aider/src/index.ts @@ -18,6 +18,15 @@ import { constants } from "node:fs"; const execFileAsync = promisify(execFile); +function normalizePermissionMode(mode: string | undefined): "permissionless" | "default" | "auto-edit" | "suggest" | undefined { + if (!mode) return undefined; + if (mode === "skip") return "permissionless"; + if (mode === "permissionless" || mode === "default" || mode === "auto-edit" || mode === "suggest") { + return mode; + } + return undefined; +} + // ============================================================================= // Aider Activity Detection Helpers // ============================================================================= @@ -75,7 +84,8 @@ function createAiderAgent(): Agent { getLaunchCommand(config: AgentLaunchConfig): string { const parts: string[] = ["aider"]; - if (config.permissions === "skip") { + const permissionMode = normalizePermissionMode(config.permissions); + if (permissionMode === "permissionless" || permissionMode === "auto-edit") { parts.push("--yes"); } diff --git a/packages/plugins/agent-claude-code/src/index.test.ts b/packages/plugins/agent-claude-code/src/index.test.ts index dabeafd61..27b689e6d 100644 --- a/packages/plugins/agent-claude-code/src/index.test.ts +++ b/packages/plugins/agent-claude-code/src/index.test.ts @@ -149,8 +149,20 @@ describe("getLaunchCommand", () => { expect(cmd).not.toContain("unset"); }); - it("includes --dangerously-skip-permissions when permissions=skip", () => { - const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "skip" })); + it("includes --dangerously-skip-permissions when permissions=permissionless", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "permissionless" })); + expect(cmd).toContain("--dangerously-skip-permissions"); + }); + + it("treats legacy permissions=skip as permissionless", () => { + const cmd = agent.getLaunchCommand( + makeLaunchConfig({ permissions: "skip" as unknown as AgentLaunchConfig["permissions"] }), + ); + expect(cmd).toContain("--dangerously-skip-permissions"); + }); + + it("maps permissions=auto-edit to no-prompt mode on Claude", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "auto-edit" })); expect(cmd).toContain("--dangerously-skip-permissions"); }); @@ -167,7 +179,7 @@ describe("getLaunchCommand", () => { it("combines all options without prompt", () => { const cmd = agent.getLaunchCommand( - makeLaunchConfig({ permissions: "skip", model: "opus", prompt: "Hello" }), + makeLaunchConfig({ permissions: "permissionless", model: "opus", prompt: "Hello" }), ); expect(cmd).toBe("claude --dangerously-skip-permissions --model 'opus'"); }); @@ -658,4 +670,3 @@ describe("getSessionInfo", () => { }); }); }); - diff --git a/packages/plugins/agent-claude-code/src/index.ts b/packages/plugins/agent-claude-code/src/index.ts index db8b475ee..7b463277f 100644 --- a/packages/plugins/agent-claude-code/src/index.ts +++ b/packages/plugins/agent-claude-code/src/index.ts @@ -23,6 +23,15 @@ import { promisify } from "node:util"; const execFileAsync = promisify(execFile); +function normalizePermissionMode(mode: string | undefined): "permissionless" | "default" | "auto-edit" | "suggest" | undefined { + if (!mode) return undefined; + if (mode === "skip") return "permissionless"; + if (mode === "permissionless" || mode === "default" || mode === "auto-edit" || mode === "suggest") { + return mode; + } + return undefined; +} + // ============================================================================= // Metadata Updater Hook Script // ============================================================================= @@ -635,7 +644,8 @@ function createClaudeCodeAgent(): Agent { // This command must be safe for both shell and execFile contexts. const parts: string[] = ["claude"]; - if (config.permissions === "skip") { + const permissionMode = normalizePermissionMode(config.permissions); + if (permissionMode === "permissionless" || permissionMode === "auto-edit") { parts.push("--dangerously-skip-permissions"); } @@ -794,7 +804,8 @@ function createClaudeCodeAgent(): Agent { // Build resume command const parts: string[] = ["claude", "--resume", shellEscape(sessionUuid)]; - if (project.agentConfig?.permissions === "skip") { + const permissionMode = normalizePermissionMode(project.agentConfig?.permissions); + if (permissionMode === "permissionless" || permissionMode === "auto-edit") { parts.push("--dangerously-skip-permissions"); } diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 363825244..c6ecabef8 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import type { Session, RuntimeHandle, AgentLaunchConfig } from "@composio/ao-core"; +import type { Session, RuntimeHandle, AgentLaunchConfig, AgentSpecificConfig } from "@composio/ao-core"; // --------------------------------------------------------------------------- // Hoisted mocks — available inside vi.mock factories @@ -213,24 +213,27 @@ describe("getLaunchCommand", () => { expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("'codex'"); }); - it("includes --dangerously-bypass-approvals-and-sandbox when permissions=skip", () => { - const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "skip" })); + it("includes bypass flag when permissions=permissionless", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "permissionless" })); expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); + expect(cmd).not.toContain("--ask-for-approval"); expect(cmd).not.toContain("--full-auto"); }); - it("includes --ask-for-approval never when permissions=auto-edit", () => { - // Cast needed: "auto-edit" not yet in AgentLaunchConfig type union + it("treats legacy permissions=skip as permissionless", () => { const cmd = agent.getLaunchCommand( - makeLaunchConfig({ permissions: "auto-edit" as AgentLaunchConfig["permissions"] }), + makeLaunchConfig({ permissions: "skip" as unknown as AgentLaunchConfig["permissions"] }), ); + expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); + }); + + it("includes --ask-for-approval never when permissions=auto-edit", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "auto-edit" })); expect(cmd).toContain("--ask-for-approval never"); }); it("includes --ask-for-approval untrusted when permissions=suggest", () => { - const cmd = agent.getLaunchCommand( - makeLaunchConfig({ permissions: "suggest" as AgentLaunchConfig["permissions"] }), - ); + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "suggest" })); expect(cmd).toContain("--ask-for-approval untrusted"); }); @@ -253,7 +256,7 @@ describe("getLaunchCommand", () => { it("combines all options", () => { const cmd = agent.getLaunchCommand( - makeLaunchConfig({ permissions: "skip", model: "o3", prompt: "Go" }), + makeLaunchConfig({ permissions: "permissionless", model: "o3", prompt: "Go" }), ); expect(cmd).toBe("'codex' --dangerously-bypass-approvals-and-sandbox --model 'o3' -c model_reasoning_effort=high -- 'Go'"); }); @@ -965,7 +968,7 @@ describe("getRestoreCommand", () => { expect(cmd).toContain("thread-abc-123"); }); - it("includes --dangerously-bypass-approvals-and-sandbox from project config", async () => { + it("includes bypass flag when project config permissions=permissionless", async () => { const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-1" }, @@ -978,7 +981,27 @@ describe("getRestoreCommand", () => { const session = makeSession({ workspacePath: "/workspace/test" }); const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ - agentConfig: { permissions: "skip" }, + agentConfig: { permissions: "permissionless" }, + })); + + expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); + expect(cmd).not.toContain("--ask-for-approval"); + }); + + it("treats legacy project config permissions=skip as permissionless", async () => { + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { threadId: "thread-1" }, + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + setupMockStream(content); + mockReadFile.mockResolvedValue(content); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ + agentConfig: { permissions: "skip" as unknown as AgentSpecificConfig["permissions"] }, })); expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 0ebb69039..1f48c383e 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -24,6 +24,15 @@ import { randomBytes } from "node:crypto"; const execFileAsync = promisify(execFile); +function normalizePermissionMode(mode: string | undefined): "permissionless" | "default" | "auto-edit" | "suggest" | undefined { + if (!mode) return undefined; + if (mode === "skip") return "permissionless"; + if (mode === "permissionless" || mode === "default" || mode === "auto-edit" || mode === "suggest") { + return mode; + } + return undefined; +} + /** Shared bin directory for ao shell wrappers (prepended to PATH) */ const AO_BIN_DIR = join(homedir(), ".ao", "bin"); @@ -516,11 +525,12 @@ export async function resolveCodexBinary(): Promise { /** Append approval-policy flags to a command parts array */ function appendApprovalFlags(parts: string[], permissions: string | undefined): void { - if (permissions === "skip") { + const mode = normalizePermissionMode(permissions); + if (mode === "permissionless") { parts.push("--dangerously-bypass-approvals-and-sandbox"); - } else if (permissions === "auto-edit") { + } else if (mode === "auto-edit") { parts.push("--ask-for-approval", "never"); - } else if (permissions === "suggest") { + } else if (mode === "suggest") { parts.push("--ask-for-approval", "untrusted"); } } @@ -571,7 +581,7 @@ function createCodexAgent(): Agent { const binary = resolvedBinary ?? "codex"; const parts: string[] = [shellEscape(binary)]; - appendApprovalFlags(parts, config.permissions as string | undefined); + appendApprovalFlags(parts, config.permissions); appendModelFlags(parts, config.model); if (config.systemPromptFile) { @@ -760,7 +770,7 @@ function createCodexAgent(): Agent { const binary = resolvedBinary ?? "codex"; const parts: string[] = [shellEscape(binary), "resume"]; - appendApprovalFlags(parts, project.agentConfig?.permissions as string | undefined); + appendApprovalFlags(parts, project.agentConfig?.permissions); const effectiveModel = (project.agentConfig?.model ?? data.model) as string | undefined; appendModelFlags(parts, effectiveModel ?? undefined);