From e82d19fb06b5c8d3c227a9c65ef304ffece3121b Mon Sep 17 00:00:00 2001 From: Kevin Lin Date: Sun, 14 Jun 2026 18:00:38 -0700 Subject: [PATCH] feat(codex): add auto plugin approvals (#92625) * feat(codex): add on-request plugin approvals * feat(codex): rename plugin approval policy to auto * fix(codex): update binding schema version callers --- docs/plugins/codex-native-plugins.md | 18 +- extensions/codex/doctor-contract-api.test.ts | 81 ++++++ extensions/codex/doctor-contract-api.ts | 56 +++- extensions/codex/openclaw.plugin.json | 6 +- .../codex/src/app-server/config.test.ts | 81 ++++++ extensions/codex/src/app-server/config.ts | 40 ++- .../src/app-server/elicitation-bridge.test.ts | 273 ++++++++++++++++++ .../src/app-server/elicitation-bridge.ts | 182 ++++++++++-- .../src/app-server/plugin-activation.test.ts | 1 + .../app-server/plugin-approval-roundtrip.ts | 4 +- .../app-server/plugin-thread-config.test.ts | 38 +++ .../src/app-server/plugin-thread-config.ts | 5 + .../src/app-server/session-binding.test.ts | 80 ++++- .../codex/src/app-server/session-binding.ts | 50 +++- .../codex/src/app-server/thread-lifecycle.ts | 2 +- .../codex/src/command-plugins-management.ts | 2 +- extensions/codex/src/commands.test.ts | 2 +- extensions/codex/src/migration/apply.ts | 1 + extensions/codex/src/migration/plan.ts | 127 ++++++-- .../codex/src/migration/provider.test.ts | 81 +++++- 20 files changed, 1047 insertions(+), 83 deletions(-) diff --git a/docs/plugins/codex-native-plugins.md b/docs/plugins/codex-native-plugins.md index cdaf45874983..46149d552e1d 100644 --- a/docs/plugins/codex-native-plugins.md +++ b/docs/plugins/codex-native-plugins.md @@ -200,11 +200,12 @@ enabled. OpenClaw sets app-level `destructive_enabled` from the effective global or per-plugin `allow_destructive_actions` policy and lets Codex enforce -destructive tool metadata from its native app tool annotations. The `_default` -app config is disabled with `open_world_enabled: false`. Enabled plugin apps -are emitted with `open_world_enabled: true`; OpenClaw does not expose a separate -plugin open-world policy knob and does not maintain per-plugin destructive -tool-name deny lists. +destructive tool metadata from its native app tool annotations. `true` and +`"auto"` both set `destructive_enabled: true`; `false` sets it false. The +`_default` app config is disabled with `open_world_enabled: false`. Enabled +plugin apps are emitted with `open_world_enabled: true`; OpenClaw does not +expose a separate plugin open-world policy knob and does not maintain +per-plugin destructive tool-name deny lists. Tool approval mode is automatic by default for plugin apps so non-destructive read tools can run without a same-thread approval UI. Destructive tools remain @@ -221,6 +222,9 @@ plugins, while unsafe schemas and ambiguous ownership still fail closed: - When policy is `false`, OpenClaw returns a deterministic decline. - When policy is `true`, OpenClaw auto-accepts only safe schemas it can map to an approval response, such as a boolean approve field. +- When policy is `"auto"`, OpenClaw exposes destructive plugin actions to + Codex but turns ownership-proven MCP approval elicitations into OpenClaw + plugin approvals before returning the Codex approval response. - Missing plugin identity, ambiguous ownership, a missing turn id, a wrong turn id, or an unsafe elicitation schema declines instead of prompting. @@ -268,8 +272,8 @@ Codex thread bindings keep the app config they started with until OpenClaw establishes a new harness session or replaces a stale binding. **Destructive action is declined:** check the global and per-plugin -`allow_destructive_actions` values. Even when policy is true, unsafe elicitation -schemas and ambiguous plugin identity still fail closed. +`allow_destructive_actions` values. Even when policy is true or `"auto"`, +unsafe elicitation schemas and ambiguous plugin identity still fail closed. ## Related diff --git a/extensions/codex/doctor-contract-api.test.ts b/extensions/codex/doctor-contract-api.test.ts index 3d2fec66d7b1..44da53aa6616 100644 --- a/extensions/codex/doctor-contract-api.test.ts +++ b/extensions/codex/doctor-contract-api.test.ts @@ -13,6 +13,31 @@ describe("codex doctor contract", () => { expect(legacyConfigRules[0]?.match({ codexDynamicToolsLoading: "direct" })).toBe(false); }); + it("reports old approval-routed destructive plugin policy values", () => { + expect( + legacyConfigRules[1]?.match({ + allow_destructive_actions: "on-request", + plugins: {}, + }), + ).toBe(true); + expect( + legacyConfigRules[1]?.match({ + allow_destructive_actions: true, + plugins: { + "google-calendar": { allow_destructive_actions: "on-request" }, + }, + }), + ).toBe(true); + expect( + legacyConfigRules[1]?.match({ + allow_destructive_actions: "auto", + plugins: { + "google-calendar": { allow_destructive_actions: true }, + }, + }), + ).toBe(false); + }); + it("removes the retired dynamic tools profile without dropping other Codex config", () => { const original = { plugins: { @@ -42,4 +67,60 @@ describe("codex doctor contract", () => { }); expect(original.plugins.entries.codex.config).toHaveProperty("codexDynamicToolsProfile"); }); + + it("renames old approval-routed destructive plugin policy values", () => { + const original = { + plugins: { + entries: { + codex: { + enabled: true, + config: { + codexDynamicToolsProfile: "openclaw-compat", + codexPlugins: { + enabled: true, + allow_destructive_actions: "on-request", + plugins: { + "google-calendar": { + enabled: true, + allow_destructive_actions: "on-request", + }, + slack: { + enabled: true, + allow_destructive_actions: false, + }, + }, + }, + }, + }, + }, + }, + }; + + const result = normalizeCompatibilityConfig({ cfg: original }); + + expect(result.changes).toEqual([ + "Removed retired plugins.entries.codex.config.codexDynamicToolsProfile; Codex app-server always keeps Codex-native workspace tools native.", + 'Renamed plugins.entries.codex.config.codexPlugins allow_destructive_actions="on-request" values to "auto".', + ]); + expect(result.config.plugins?.entries?.codex?.config).toEqual({ + codexPlugins: { + enabled: true, + allow_destructive_actions: "auto", + plugins: { + "google-calendar": { + enabled: true, + allow_destructive_actions: "auto", + }, + slack: { + enabled: true, + allow_destructive_actions: false, + }, + }, + }, + }); + expect( + original.plugins.entries.codex.config.codexPlugins.plugins["google-calendar"] + .allow_destructive_actions, + ).toBe("on-request"); + }); }); diff --git a/extensions/codex/doctor-contract-api.ts b/extensions/codex/doctor-contract-api.ts index a89fd2615f79..bf761a2ed9e3 100644 --- a/extensions/codex/doctor-contract-api.ts +++ b/extensions/codex/doctor-contract-api.ts @@ -21,6 +21,20 @@ function hasRetiredDynamicToolsProfile(value: unknown): boolean { return Object.hasOwn(asRecord(value) ?? {}, "codexDynamicToolsProfile"); } +function hasLegacyPluginDestructivePolicy(value: unknown): boolean { + const codexPlugins = asRecord(value); + if (!codexPlugins) { + return false; + } + if (codexPlugins.allow_destructive_actions === "on-request") { + return true; + } + const plugins = asRecord(codexPlugins.plugins); + return Object.values(plugins ?? {}).some( + (plugin) => asRecord(plugin)?.allow_destructive_actions === "on-request", + ); +} + /** Legacy Codex config keys that doctor should report or repair. */ export const legacyConfigRules: LegacyConfigRule[] = [ { @@ -29,6 +43,12 @@ export const legacyConfigRules: LegacyConfigRule[] = [ 'plugins.entries.codex.config.codexDynamicToolsProfile is retired; Codex app-server always keeps Codex-native workspace tools native. Run "openclaw doctor --fix".', match: hasRetiredDynamicToolsProfile, }, + { + path: ["plugins", "entries", "codex", "config", "codexPlugins"], + message: + 'plugins.entries.codex.config.codexPlugins.allow_destructive_actions="on-request" was renamed to "auto". Run "openclaw doctor --fix".', + match: hasLegacyPluginDestructivePolicy, + }, ]; /** @@ -40,7 +60,11 @@ export function normalizeCompatibilityConfig({ cfg }: { cfg: OpenClawConfig }): } { const rawEntry = asRecord(cfg.plugins?.entries?.codex); const rawPluginConfig = asRecord(rawEntry?.config); - if (!rawPluginConfig || !hasRetiredDynamicToolsProfile(rawPluginConfig)) { + const rawCodexPlugins = asRecord(rawPluginConfig?.codexPlugins); + const shouldRemoveDynamicToolsProfile = + rawPluginConfig !== null && hasRetiredDynamicToolsProfile(rawPluginConfig); + const shouldRewriteDestructivePolicy = hasLegacyPluginDestructivePolicy(rawCodexPlugins); + if (!rawPluginConfig || (!shouldRemoveDynamicToolsProfile && !shouldRewriteDestructivePolicy)) { return { config: cfg, changes: [] }; } @@ -55,12 +79,34 @@ export function normalizeCompatibilityConfig({ cfg }: { cfg: OpenClawConfig }): return { config: cfg, changes: [] }; } - delete nextPluginConfig.codexDynamicToolsProfile; + const changes: string[] = []; + if (shouldRemoveDynamicToolsProfile) { + delete nextPluginConfig.codexDynamicToolsProfile; + changes.push( + "Removed retired plugins.entries.codex.config.codexDynamicToolsProfile; Codex app-server always keeps Codex-native workspace tools native.", + ); + } + + if (shouldRewriteDestructivePolicy) { + const nextCodexPlugins = asRecord(nextPluginConfig.codexPlugins); + if (nextCodexPlugins?.allow_destructive_actions === "on-request") { + nextCodexPlugins.allow_destructive_actions = "auto"; + } + const nextPluginPolicies = asRecord(nextCodexPlugins?.plugins); + for (const plugin of Object.values(nextPluginPolicies ?? {})) { + const nextPlugin = asRecord(plugin); + if (nextPlugin?.allow_destructive_actions === "on-request") { + nextPlugin.allow_destructive_actions = "auto"; + } + } + changes.push( + 'Renamed plugins.entries.codex.config.codexPlugins allow_destructive_actions="on-request" values to "auto".', + ); + } + return { config: nextConfig, - changes: [ - "Removed retired plugins.entries.codex.config.codexDynamicToolsProfile; Codex app-server always keeps Codex-native workspace tools native.", - ], + changes, }; } diff --git a/extensions/codex/openclaw.plugin.json b/extensions/codex/openclaw.plugin.json index dc65964ee850..65b8f3cfaaa3 100644 --- a/extensions/codex/openclaw.plugin.json +++ b/extensions/codex/openclaw.plugin.json @@ -100,7 +100,7 @@ "default": false }, "allow_destructive_actions": { - "type": "boolean", + "oneOf": [{ "type": "boolean" }, { "const": "auto" }], "default": true }, "plugins": { @@ -120,7 +120,7 @@ "type": "string" }, "allow_destructive_actions": { - "type": "boolean" + "oneOf": [{ "type": "boolean" }, { "const": "auto" }] } } } @@ -290,7 +290,7 @@ }, "codexPlugins.allow_destructive_actions": { "label": "Allow Destructive Plugin Actions", - "help": "Default policy for plugin app write or destructive action elicitations. Defaults to true.", + "help": "Default policy for plugin app write or destructive action elicitations. Use true to accept safe schemas without prompting, false to decline, or auto to ask through plugin approvals.", "advanced": true }, "codexPlugins.plugins": { diff --git a/extensions/codex/src/app-server/config.test.ts b/extensions/codex/src/app-server/config.test.ts index 666e29421ac8..561b35f1dbb1 100644 --- a/extensions/codex/src/app-server/config.test.ts +++ b/extensions/codex/src/app-server/config.test.ts @@ -859,6 +859,7 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] configured: true, enabled: true, allowDestructiveActions: false, + destructiveApprovalMode: "deny", pluginPolicies: [ { configKey: "google-calendar", @@ -866,6 +867,7 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] pluginName: "google-calendar", enabled: true, allowDestructiveActions: true, + destructiveApprovalMode: "allow", }, { configKey: "slack", @@ -873,11 +875,88 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] pluginName: "slack", enabled: false, allowDestructiveActions: false, + destructiveApprovalMode: "deny", }, ], }); }); + it("parses auto native Codex plugin destructive policy", () => { + const config = readCodexPluginConfig({ + codexPlugins: { + enabled: true, + allow_destructive_actions: "auto", + plugins: { + "google-calendar": { + marketplaceName: "openai-curated", + pluginName: "google-calendar", + }, + slack: { + marketplaceName: "openai-curated", + pluginName: "slack", + allow_destructive_actions: false, + }, + gmail: { + marketplaceName: "openai-curated", + pluginName: "gmail", + allow_destructive_actions: true, + }, + }, + }, + }); + + expect(config.codexPlugins?.allow_destructive_actions).toBe("auto"); + expect(resolveCodexPluginsPolicy(config)).toEqual({ + configured: true, + enabled: true, + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + pluginPolicies: [ + { + configKey: "gmail", + marketplaceName: "openai-curated", + pluginName: "gmail", + enabled: true, + allowDestructiveActions: true, + destructiveApprovalMode: "allow", + }, + { + configKey: "google-calendar", + marketplaceName: "openai-curated", + pluginName: "google-calendar", + enabled: true, + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + }, + { + configKey: "slack", + marketplaceName: "openai-curated", + pluginName: "slack", + enabled: true, + allowDestructiveActions: false, + destructiveApprovalMode: "deny", + }, + ], + }); + }); + + it("rejects unsupported native Codex plugin destructive policy strings", () => { + const config = readCodexPluginConfig({ + codexPlugins: { + enabled: true, + allow_destructive_actions: "ask", + plugins: { + slack: { + marketplaceName: "openai-curated", + pluginName: "slack", + }, + }, + }, + }); + + expect(config.codexPlugins).toBeUndefined(); + }); + it("defaults native Codex plugin destructive policy to enabled", () => { const policy = resolveCodexPluginsPolicy({ codexPlugins: { @@ -895,6 +974,7 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] configured: true, enabled: true, allowDestructiveActions: true, + destructiveApprovalMode: "allow", pluginPolicies: [ { configKey: "slack", @@ -902,6 +982,7 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] pluginName: "slack", enabled: true, allowDestructiveActions: true, + destructiveApprovalMode: "allow", }, ], }); diff --git a/extensions/codex/src/app-server/config.ts b/extensions/codex/src/app-server/config.ts index ed6482965b6a..c26d55eab8b3 100644 --- a/extensions/codex/src/app-server/config.ts +++ b/extensions/codex/src/app-server/config.ts @@ -67,7 +67,8 @@ export type CodexAppServerSandboxMode = "read-only" | "workspace-write" | "dange type CodexAppServerApprovalsReviewer = "user" | "auto_review" | "guardian_subagent"; type CodexAppServerCommandSource = "managed" | "resolved-managed" | "config" | "env"; export type CodexDynamicToolsLoading = "searchable" | "direct"; -export type CodexPluginDestructivePolicy = boolean; +export type CodexPluginDestructivePolicy = boolean | "auto"; +export type CodexPluginDestructiveApprovalMode = "allow" | "deny" | "auto"; export const CODEX_PLUGINS_MARKETPLACE_NAME = "openai-curated"; @@ -115,13 +116,15 @@ export type ResolvedCodexPluginPolicy = { marketplaceName: typeof CODEX_PLUGINS_MARKETPLACE_NAME; pluginName: string; enabled: boolean; - allowDestructiveActions: CodexPluginDestructivePolicy; + allowDestructiveActions: boolean; + destructiveApprovalMode: CodexPluginDestructiveApprovalMode; }; export type ResolvedCodexPluginsPolicy = { configured: boolean; enabled: boolean; - allowDestructiveActions: CodexPluginDestructivePolicy; + allowDestructiveActions: boolean; + destructiveApprovalMode: CodexPluginDestructiveApprovalMode; pluginPolicies: ResolvedCodexPluginPolicy[]; }; @@ -258,6 +261,7 @@ const codexAppServerApprovalPolicySchema = z.enum([ const codexAppServerSandboxSchema = z.enum(["read-only", "workspace-write", "danger-full-access"]); const codexAppServerApprovalsReviewerSchema = z.enum(["user", "auto_review", "guardian_subagent"]); const codexDynamicToolsLoadingSchema = z.enum(["searchable", "direct"]); +const codexPluginDestructivePolicySchema = z.union([z.boolean(), z.literal("auto")]); const codexAppServerServiceTierSchema = z .preprocess( (value) => (value === null ? null : normalizeCodexServiceTier(value)), @@ -275,14 +279,14 @@ const codexPluginEntryConfigSchema = z enabled: z.boolean().optional(), marketplaceName: z.literal(CODEX_PLUGINS_MARKETPLACE_NAME).optional(), pluginName: z.string().trim().min(1).optional(), - allow_destructive_actions: z.boolean().optional(), + allow_destructive_actions: codexPluginDestructivePolicySchema.optional(), }) .strict(); const codexPluginsConfigSchema = z .object({ enabled: z.boolean().optional(), - allow_destructive_actions: z.boolean().optional(), + allow_destructive_actions: codexPluginDestructivePolicySchema.optional(), plugins: z.record(z.string(), codexPluginEntryConfigSchema).optional(), }) .strict(); @@ -380,19 +384,25 @@ export function resolveCodexPluginsPolicy(pluginConfig?: unknown): ResolvedCodex const config = readCodexPluginConfig(pluginConfig).codexPlugins; const configured = config !== undefined; const enabled = config?.enabled === true; - const allowDestructiveActions = config?.allow_destructive_actions ?? true; + const destructivePolicy = resolveCodexPluginDestructivePolicy( + config?.allow_destructive_actions ?? true, + ); const pluginPolicies = Object.entries(config?.plugins ?? {}) .flatMap(([configKey, entry]): ResolvedCodexPluginPolicy[] => { if (entry.marketplaceName !== CODEX_PLUGINS_MARKETPLACE_NAME || !entry.pluginName) { return []; } + const entryDestructivePolicy = resolveCodexPluginDestructivePolicy( + entry.allow_destructive_actions ?? config?.allow_destructive_actions ?? true, + ); return [ { configKey, marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: entry.pluginName, enabled: enabled && entry.enabled !== false, - allowDestructiveActions: entry.allow_destructive_actions ?? allowDestructiveActions, + allowDestructiveActions: entryDestructivePolicy.allowDestructiveActions, + destructiveApprovalMode: entryDestructivePolicy.destructiveApprovalMode, }, ]; }) @@ -400,11 +410,25 @@ export function resolveCodexPluginsPolicy(pluginConfig?: unknown): ResolvedCodex return { configured, enabled, - allowDestructiveActions, + allowDestructiveActions: destructivePolicy.allowDestructiveActions, + destructiveApprovalMode: destructivePolicy.destructiveApprovalMode, pluginPolicies, }; } +function resolveCodexPluginDestructivePolicy(policy: CodexPluginDestructivePolicy): { + allowDestructiveActions: boolean; + destructiveApprovalMode: CodexPluginDestructiveApprovalMode; +} { + if (policy === "auto") { + return { allowDestructiveActions: true, destructiveApprovalMode: "auto" }; + } + return { + allowDestructiveActions: policy, + destructiveApprovalMode: policy ? "allow" : "deny", + }; +} + export function resolveCodexAppServerRuntimeOptions( params: { pluginConfig?: unknown; diff --git a/extensions/codex/src/app-server/elicitation-bridge.test.ts b/extensions/codex/src/app-server/elicitation-bridge.test.ts index b69d4ce0d4cf..b070af06d55a 100644 --- a/extensions/codex/src/app-server/elicitation-bridge.test.ts +++ b/extensions/codex/src/app-server/elicitation-bridge.test.ts @@ -157,6 +157,7 @@ function buildConnectorPluginApprovalElicitation(overrides: Record; } = {}, ) { @@ -177,6 +178,9 @@ function createPluginAppPolicyContext( marketplaceName: "openai-curated" as const, pluginName: app.pluginName, allowDestructiveActions: params.allowDestructiveActions ?? false, + ...(params.destructiveApprovalMode + ? { destructiveApprovalMode: params.destructiveApprovalMode } + : {}), mcpServerNames: app.mcpServerNames, }, ]), @@ -831,6 +835,275 @@ describe("Codex app-server elicitation bridge", () => { expect(mockCallGatewayTool).not.toHaveBeenCalled(); }); + for (const { name, requestedSchema } of [ + { + name: "declines connector-id plugin app elicitations with non-object schemas", + requestedSchema: { type: "string", properties: {} }, + }, + { + name: "declines connector-id plugin app elicitations without object properties", + requestedSchema: { type: "object" }, + }, + ]) { + it(name, async () => { + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation({ requestedSchema }), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + }); + + expect(result).toEqual({ action: "decline", content: null, _meta: null }); + expect(mockCallGatewayTool).not.toHaveBeenCalled(); + }); + } + + it("routes auto connector-id plugin app elicitations through plugin approvals", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-calendar", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-calendar", decision: "allow-once" }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation(), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + }); + + expect(result).toEqual({ + action: "accept", + content: null, + _meta: null, + }); + expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([ + "plugin.approval.request", + "plugin.approval.waitDecision", + ]); + expect(gatewayToolArg(0, 2)).toMatchObject({ + allowedDecisions: ["allow-once", "deny"], + title: "Allow Google Calendar to create an event?", + toolName: "codex_mcp_tool_approval", + twoPhase: true, + }); + }); + + it("maps auto plugin allow-always only when Codex offers always persistence", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-calendar-always", status: "accepted" }) + .mockResolvedValueOnce({ + id: "plugin:approval-calendar-always", + decision: "allow-always", + }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation({ + _meta: { + codex_approval_kind: "mcp_tool_call", + source: "connector", + connector_id: "connector_google_calendar", + connector_name: "Google Calendar", + persist: ["session", "always"], + tool_title: "create_event", + }, + }), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + }); + + expect(result).toEqual({ + action: "accept", + content: null, + _meta: { + persist: "always", + }, + }); + expect(gatewayToolArg(0, 2)).toMatchObject({ + allowedDecisions: ["allow-once", "allow-always", "deny"], + }); + }); + + it("does not expose allow-always for auto plugin session-only persistence", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-calendar-session", status: "accepted" }) + .mockResolvedValueOnce({ + id: "plugin:approval-calendar-session", + decision: "allow-once", + }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation({ + _meta: { + codex_approval_kind: "mcp_tool_call", + source: "connector", + connector_id: "connector_google_calendar", + connector_name: "Google Calendar", + persist: ["session"], + tool_title: "create_event", + }, + requestedSchema: { + type: "object", + properties: { + approve: { + type: "boolean", + title: "Approve this app action", + }, + persist: { + type: "string", + title: "Persist choice", + enum: ["session", "always"], + }, + }, + required: ["approve"], + }, + }), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + }); + + expect(result).toEqual({ + action: "accept", + content: { + approve: true, + }, + _meta: null, + }); + expect(gatewayToolArg(0, 2)).toMatchObject({ + allowedDecisions: ["allow-once", "deny"], + }); + }); + + it("declines denied auto plugin app approvals", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-calendar-deny", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-calendar-deny", decision: "deny" }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation(), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + }); + + expect(result).toEqual({ action: "decline", content: null, _meta: null }); + }); + + it("fails closed when auto plugin approval routing is unavailable", async () => { + mockCallGatewayTool.mockResolvedValueOnce({ + id: "plugin:approval-calendar-unavailable", + decision: null, + }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation(), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + }); + + expect(result).toEqual({ action: "decline", content: null, _meta: null }); + expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([ + "plugin.approval.request", + ]); + }); + + it("cancels auto plugin app approvals when the turn aborts", async () => { + const abortController = new AbortController(); + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-calendar-abort", status: "accepted" }) + .mockImplementationOnce(() => { + abortController.abort(new Error("turn stopped")); + return new Promise(() => {}); + }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildConnectorPluginApprovalElicitation(), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + pluginAppPolicyContext: createPluginAppPolicyContext({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + apps: [ + { + appId: "connector_google_calendar", + pluginName: "google-calendar", + mcpServerNames: [], + }, + ], + }), + signal: abortController.signal, + }); + + expect(result).toEqual({ action: "cancel", content: null, _meta: null }); + }); + it("declines connector-id plugin app elicitations when destructive actions are disabled", async () => { const result = await handleCodexAppServerElicitationRequest({ requestParams: buildConnectorPluginApprovalElicitation(), diff --git a/extensions/codex/src/app-server/elicitation-bridge.ts b/extensions/codex/src/app-server/elicitation-bridge.ts index 549fc8cf9680..847a84300dfb 100644 --- a/extensions/codex/src/app-server/elicitation-bridge.ts +++ b/extensions/codex/src/app-server/elicitation-bridge.ts @@ -9,6 +9,7 @@ import { mapExecDecisionToOutcome, requestPluginApproval, type AppServerApprovalOutcome, + type ExecApprovalDecision, waitForPluginApprovalDecision, } from "./plugin-approval-roundtrip.js"; import type { @@ -28,6 +29,8 @@ type BridgeableApprovalElicitation = { description: string; requestedSchema: JsonObject; meta: JsonObject; + persistHintsMode?: "legacy" | "explicit"; + allowedDecisions?: ExecApprovalDecision[]; }; type PluginElicitationResolution = @@ -111,7 +114,12 @@ export async function handleCodexAppServerElicitationRequest(params: { logPluginElicitationDecline("missing_active_turn", requestParams); return declineElicitationResponse(); } - return buildPluginPolicyElicitationResponse(pluginResolution.entry, requestParams); + return await buildPluginPolicyElicitationResponse({ + entry: pluginResolution.entry, + requestParams, + paramsForRun: params.paramsForRun, + signal: params.signal, + }); } const approvalPrompt = @@ -125,9 +133,10 @@ export async function handleCodexAppServerElicitationRequest(params: { paramsForRun: params.paramsForRun, title: approvalPrompt.title, description: approvalPrompt.description, + allowedDecisions: approvalPrompt.allowedDecisions, signal: params.signal, }); - return buildElicitationResponse(approvalPrompt.requestedSchema, approvalPrompt.meta, outcome); + return buildElicitationResponse(approvalPrompt, outcome); } function matchesCurrentThread(requestParams: JsonObject | undefined, threadId: string): boolean { @@ -284,28 +293,111 @@ function normalizePluginIdentityText(value: string): string { return value.toLowerCase().replace(/[^a-z0-9]+/g, ""); } -function buildPluginPolicyElicitationResponse( - entry: PluginAppPolicyContextEntry, - requestParams: JsonObject, -): JsonValue { - if (!entry.allowDestructiveActions) { - logPluginElicitationDecline("destructive_actions_disabled", requestParams); +async function buildPluginPolicyElicitationResponse(params: { + entry: PluginAppPolicyContextEntry; + requestParams: JsonObject; + paramsForRun: EmbeddedRunAttemptParams; + signal?: AbortSignal; +}): Promise { + const mode = resolvePluginDestructiveApprovalMode(params.entry); + if (mode === "deny") { + logPluginElicitationDecline("destructive_actions_disabled", params.requestParams); return declineElicitationResponse(); } + const approvalPrompt = readPluginApprovalElicitation(params.entry, params.requestParams); + if (!approvalPrompt) { + logPluginElicitationDecline("unsupported_schema", params.requestParams); + return declineElicitationResponse(); + } + const response = buildElicitationResponse(approvalPrompt, "approved-once"); + if (isJsonObject(response) && response.action === "accept") { + if (mode === "allow") { + return response; + } + const outcome = await requestPluginApprovalOutcome({ + paramsForRun: params.paramsForRun, + title: approvalPrompt.title, + description: approvalPrompt.description, + allowedDecisions: approvalPrompt.allowedDecisions, + signal: params.signal, + }); + return buildElicitationResponse(approvalPrompt, outcome); + } + logPluginElicitationDecline("unmappable_schema", params.requestParams); + return declineElicitationResponse(); +} + +function resolvePluginDestructiveApprovalMode( + entry: PluginAppPolicyContextEntry, +): "allow" | "deny" | "auto" { + return entry.destructiveApprovalMode ?? (entry.allowDestructiveActions ? "allow" : "deny"); +} + +function readPluginApprovalElicitation( + entry: PluginAppPolicyContextEntry, + requestParams: JsonObject, +): BridgeableApprovalElicitation | undefined { if ( readString(requestParams, "mode") !== "form" || !isJsonObject(requestParams.requestedSchema) ) { - logPluginElicitationDecline("unsupported_schema", requestParams); - return declineElicitationResponse(); + return undefined; } + const requestedSchema = requestParams.requestedSchema; + if ( + readString(requestedSchema, "type") !== "object" || + !isJsonObject(requestedSchema.properties) + ) { + return undefined; + } + const meta = isJsonObject(requestParams["_meta"]) ? requestParams["_meta"] : {}; - const response = buildElicitationResponse(requestParams.requestedSchema, meta, "approved-once"); - if (isJsonObject(response) && response.action === "accept") { - return response; + const title = + sanitizeDisplayText(readString(requestParams, "message") ?? "") || "Codex plugin approval"; + const descriptionMeta: JsonObject = { ...meta }; + if (!readString(descriptionMeta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY)) { + descriptionMeta[MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY] = entry.pluginName; } - logPluginElicitationDecline("unmappable_schema", requestParams); - return declineElicitationResponse(); + return { + title, + description: buildApprovalDescription({ + title, + meta: descriptionMeta, + requestedSchema, + serverName: sanitizeOptionalDisplayText(readString(requestParams, "serverName")), + }), + requestedSchema, + meta, + persistHintsMode: "explicit", + allowedDecisions: buildApprovalAllowedDecisions(requestedSchema, meta), + }; +} + +function buildApprovalAllowedDecisions( + requestedSchema: JsonObject, + meta: JsonObject, +): ExecApprovalDecision[] { + return canMapPersistentApproval(requestedSchema, meta) + ? ["allow-once", "allow-always", "deny"] + : ["allow-once", "deny"]; +} + +function canMapPersistentApproval(requestedSchema: JsonObject, meta: JsonObject): boolean { + const persistHints = readPersistHints(meta, "explicit"); + if (persistHints.length > 0) { + return persistHints.includes("always"); + } + const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : {}; + return Object.entries(properties).some(([name, value]) => { + const schema = isJsonObject(value) ? value : undefined; + if (!schema) { + return false; + } + return ( + isPersistField({ name, schema, required: false }) && + chooseAlwaysPersistOptionValue(readEnumOptions(schema)) !== undefined + ); + }); } function declineElicitationResponse(): JsonValue { @@ -558,6 +650,7 @@ async function requestPluginApprovalOutcome(params: { paramsForRun: EmbeddedRunAttemptParams; title: string; description: string; + allowedDecisions?: ExecApprovalDecision[]; signal?: AbortSignal; }): Promise { try { @@ -567,6 +660,7 @@ async function requestPluginApprovalOutcome(params: { description: params.description, severity: "warning", toolName: "codex_mcp_tool_approval", + allowedDecisions: params.allowedDecisions, }); const approvalId = requestResult?.id; @@ -584,10 +678,13 @@ async function requestPluginApprovalOutcome(params: { } function buildElicitationResponse( - requestedSchema: JsonObject, - meta: JsonObject, + approvalPrompt: Pick< + BridgeableApprovalElicitation, + "requestedSchema" | "meta" | "persistHintsMode" + >, outcome: AppServerApprovalOutcome, ): JsonValue { + const { requestedSchema, meta } = approvalPrompt; if (outcome === "cancelled") { return { action: "cancel", content: null, _meta: null }; } @@ -595,13 +692,13 @@ function buildElicitationResponse( return { action: "decline", content: null, _meta: null }; } - const content = buildAcceptedContent(requestedSchema, meta, outcome); + const content = buildAcceptedContent(approvalPrompt, outcome); if (!content) { if (hasNoSchemaProperties(requestedSchema)) { return { action: "accept", content: null, - _meta: buildAcceptedMeta(meta, outcome), + _meta: buildAcceptedMeta(meta, outcome, approvalPrompt.persistHintsMode ?? "legacy"), }; } embeddedAgentLog.warn("codex MCP approval elicitation approved without a mappable response", { @@ -611,14 +708,21 @@ function buildElicitationResponse( }); return { action: "decline", content: null, _meta: null }; } - return { action: "accept", content, _meta: buildAcceptedMeta(meta, outcome) }; + return { + action: "accept", + content, + _meta: buildAcceptedMeta(meta, outcome, approvalPrompt.persistHintsMode ?? "legacy"), + }; } function buildAcceptedContent( - requestedSchema: JsonObject, - meta: JsonObject, + approvalPrompt: Pick< + BridgeableApprovalElicitation, + "requestedSchema" | "meta" | "persistHintsMode" + >, outcome: AppServerApprovalOutcome, ): JsonObject | undefined { + const { requestedSchema, meta } = approvalPrompt; const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : undefined; @@ -641,7 +745,7 @@ function buildAcceptedContent( const property = { name, schema, required: required.has(name) }; const next = readApprovalFieldValue(property, outcome) ?? - readPersistFieldValue(property, meta, outcome) ?? + readPersistFieldValue(property, meta, outcome, approvalPrompt.persistHintsMode ?? "legacy") ?? readFallbackFieldValue(property, outcome); if (next === undefined) { @@ -691,11 +795,12 @@ function readPersistFieldValue( property: ApprovalPropertyContext, meta: JsonObject, outcome: AppServerApprovalOutcome, + persistHintsMode: "legacy" | "explicit", ): JsonValue | undefined { if (!isPersistField(property) || outcome !== "approved-session") { return undefined; } - const persistHints = readPersistHints(meta); + const persistHints = readPersistHints(meta, persistHintsMode); const options = readEnumOptions(property.schema); if (options.length === 0) { return undefined; @@ -707,6 +812,9 @@ function readPersistFieldValue( ); return match?.value; } + if (persistHintsMode === "explicit") { + return chooseAlwaysPersistOptionValue(options); + } return undefined; } @@ -744,7 +852,7 @@ function propertyText(property: ApprovalPropertyContext): string { .join(" "); } -function readPersistHints(meta: JsonObject): string[] { +function readPersistHints(meta: JsonObject, mode: "legacy" | "explicit" = "legacy"): string[] { const raw = meta.persist; if (typeof raw === "string") { return [raw]; @@ -752,14 +860,18 @@ function readPersistHints(meta: JsonObject): string[] { if (Array.isArray(raw)) { return raw.filter((entry): entry is string => typeof entry === "string"); } - return ["session", "always"]; + return mode === "legacy" ? ["session", "always"] : []; } -function buildAcceptedMeta(meta: JsonObject, outcome: AppServerApprovalOutcome): JsonObject | null { +function buildAcceptedMeta( + meta: JsonObject, + outcome: AppServerApprovalOutcome, + persistHintsMode: "legacy" | "explicit", +): JsonObject | null { if (outcome !== "approved-session") { return null; } - const persist = choosePersistHint(readPersistHints(meta)); + const persist = choosePersistHint(readPersistHints(meta, persistHintsMode)); return persist ? { persist } : null; } @@ -773,6 +885,20 @@ function choosePersistHint(persistHints: string[]): "always" | "session" | undef return undefined; } +function chooseAlwaysPersistOptionValue( + options: Array<{ value: string; label: string }>, +): string | undefined { + const always = options.find((option) => optionMatchesPersist(option, "always")); + return always?.value; +} + +function optionMatchesPersist( + option: { value: string; label: string }, + persist: "always" | "session", +): boolean { + return option.value.toLowerCase() === persist || option.label.toLowerCase() === persist; +} + function hasNoSchemaProperties(requestedSchema: JsonObject): boolean { const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : {}; return Object.keys(properties).length === 0; diff --git a/extensions/codex/src/app-server/plugin-activation.test.ts b/extensions/codex/src/app-server/plugin-activation.test.ts index 800349a51414..ead0fd9ab1cb 100644 --- a/extensions/codex/src/app-server/plugin-activation.test.ts +++ b/extensions/codex/src/app-server/plugin-activation.test.ts @@ -303,6 +303,7 @@ function identity(pluginName: string): ResolvedCodexPluginPolicy { pluginName, enabled: true, allowDestructiveActions: false, + destructiveApprovalMode: "deny", }; } diff --git a/extensions/codex/src/app-server/plugin-approval-roundtrip.ts b/extensions/codex/src/app-server/plugin-approval-roundtrip.ts index a4d9c8491e99..24d540f3c5da 100644 --- a/extensions/codex/src/app-server/plugin-approval-roundtrip.ts +++ b/extensions/codex/src/app-server/plugin-approval-roundtrip.ts @@ -12,7 +12,7 @@ const DEFAULT_CODEX_APPROVAL_TIMEOUT_MS = 120_000; const MAX_PLUGIN_APPROVAL_TITLE_LENGTH = 80; const MAX_PLUGIN_APPROVAL_DESCRIPTION_LENGTH = 256; -type ExecApprovalDecision = "allow-once" | "allow-always" | "deny"; +export type ExecApprovalDecision = "allow-once" | "allow-always" | "deny"; /** Normalized Codex app-server approval outcome after a gateway decision. */ export type AppServerApprovalOutcome = @@ -40,6 +40,7 @@ export async function requestPluginApproval(params: { severity: "info" | "warning"; toolName: string; toolCallId?: string; + allowedDecisions?: ExecApprovalDecision[]; }): Promise { const timeoutMs = DEFAULT_CODEX_APPROVAL_TIMEOUT_MS; return callGatewayTool( @@ -60,6 +61,7 @@ export async function requestPluginApproval(params: { turnSourceThreadId: params.paramsForRun.currentThreadTs, timeoutMs, twoPhase: true, + ...(params.allowedDecisions ? { allowedDecisions: params.allowedDecisions } : {}), }, { expectFinal: false }, ) as Promise; diff --git a/extensions/codex/src/app-server/plugin-thread-config.test.ts b/extensions/codex/src/app-server/plugin-thread-config.test.ts index eb9ddd8fb75a..1a7704663ae9 100644 --- a/extensions/codex/src/app-server/plugin-thread-config.test.ts +++ b/extensions/codex/src/app-server/plugin-thread-config.test.ts @@ -73,6 +73,7 @@ describe("Codex plugin thread config", () => { marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: "google-calendar", allowDestructiveActions: true, + destructiveApprovalMode: "allow", mcpServerNames: ["google-calendar"], }); expect(config.diagnostics).toStrictEqual([]); @@ -107,6 +108,9 @@ describe("Codex plugin thread config", () => { expect( pluginOverrideDisabled.policyContext.apps["google-calendar-app"]?.allowDestructiveActions, ).toBe(false); + expect( + pluginOverrideDisabled.policyContext.apps["google-calendar-app"]?.destructiveApprovalMode, + ).toBe("deny"); const pluginOverrideEnabled = await buildReadyGoogleCalendarThreadConfig({ codexPlugins: { @@ -134,6 +138,36 @@ describe("Codex plugin thread config", () => { expect( pluginOverrideEnabled.policyContext.apps["google-calendar-app"]?.allowDestructiveActions, ).toBe(true); + expect( + pluginOverrideEnabled.policyContext.apps["google-calendar-app"]?.destructiveApprovalMode, + ).toBe("allow"); + }); + + it("exposes destructive app access while marking auto approval mode", async () => { + const config = await buildReadyGoogleCalendarThreadConfig({ + codexPlugins: { + enabled: true, + allow_destructive_actions: "auto", + plugins: { + "google-calendar": { + marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, + pluginName: "google-calendar", + }, + }, + }, + }); + + const apps = config.configPatch?.apps as Record | undefined; + expect(apps?.["google-calendar-app"]).toEqual({ + enabled: true, + destructive_enabled: true, + open_world_enabled: true, + default_tools_approval_mode: "auto", + }); + expect(config.policyContext.apps["google-calendar-app"]).toMatchObject({ + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + }); }); it("builds a restrictive app config when native plugin support is disabled", async () => { @@ -267,6 +301,7 @@ describe("Codex plugin thread config", () => { marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: "google-calendar", allowDestructiveActions: true, + destructiveApprovalMode: "allow", mcpServerNames: [], }); expect(config.diagnostics).toStrictEqual([]); @@ -338,6 +373,7 @@ describe("Codex plugin thread config", () => { pluginName: "google-calendar", enabled: true, allowDestructiveActions: true, + destructiveApprovalMode: "allow", }, message: "google-calendar-app is not accessible or enabled for google-calendar.", }, @@ -408,6 +444,7 @@ describe("Codex plugin thread config", () => { marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: "google-calendar", allowDestructiveActions: true, + destructiveApprovalMode: "allow", mcpServerNames: [], }); expect(config.diagnostics).toStrictEqual([]); @@ -498,6 +535,7 @@ describe("Codex plugin thread config", () => { marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: "google-calendar", allowDestructiveActions: true, + destructiveApprovalMode: "allow", mcpServerNames: [], }); expect(config.diagnostics).toStrictEqual([]); diff --git a/extensions/codex/src/app-server/plugin-thread-config.ts b/extensions/codex/src/app-server/plugin-thread-config.ts index 67dbff6b2e80..34fb5fce6eff 100644 --- a/extensions/codex/src/app-server/plugin-thread-config.ts +++ b/extensions/codex/src/app-server/plugin-thread-config.ts @@ -13,6 +13,7 @@ import { } from "./app-inventory-cache.js"; import { resolveCodexPluginsPolicy, + type CodexPluginDestructiveApprovalMode, type ResolvedCodexPluginPolicy, type ResolvedCodexPluginsPolicy, } from "./config.js"; @@ -36,6 +37,7 @@ export type PluginAppPolicyContextEntry = { marketplaceName: ResolvedCodexPluginPolicy["marketplaceName"]; pluginName: string; allowDestructiveActions: boolean; + destructiveApprovalMode?: CodexPluginDestructiveApprovalMode; mcpServerNames: string[]; }; @@ -246,6 +248,7 @@ export async function buildCodexPluginThreadConfig( marketplaceName: record.policy.marketplaceName, pluginName: record.policy.pluginName, allowDestructiveActions: record.policy.allowDestructiveActions, + destructiveApprovalMode: record.policy.destructiveApprovalMode, mcpServerNames: [...(record.detail?.mcpServers ?? [])].toSorted(), }; } @@ -425,12 +428,14 @@ function policyFingerprint(policy: ResolvedCodexPluginsPolicy): JsonValue { return { enabled: policy.enabled, allowDestructiveActions: policy.allowDestructiveActions, + destructiveApprovalMode: policy.destructiveApprovalMode, plugins: policy.pluginPolicies.map((plugin) => ({ configKey: plugin.configKey, marketplaceName: plugin.marketplaceName, pluginName: plugin.pluginName, enabled: plugin.enabled, allowDestructiveActions: plugin.allowDestructiveActions, + destructiveApprovalMode: plugin.destructiveApprovalMode, })), }; } diff --git a/extensions/codex/src/app-server/session-binding.test.ts b/extensions/codex/src/app-server/session-binding.test.ts index 0f2f881a1e5c..06851341ac49 100644 --- a/extensions/codex/src/app-server/session-binding.test.ts +++ b/extensions/codex/src/app-server/session-binding.test.ts @@ -67,7 +67,7 @@ describe("codex app-server session binding", () => { const binding = await readCodexAppServerBinding(sessionFile); - expect(binding?.schemaVersion).toBe(1); + expect(binding?.schemaVersion).toBe(2); expect(binding?.threadId).toBe("thread-123"); expect(binding?.sessionFile).toBe(sessionFile); expect(binding?.cwd).toBe(tempDir); @@ -108,6 +108,84 @@ describe("codex app-server session binding", () => { expect(binding?.pluginAppPolicyContext).toEqual(pluginAppPolicyContext); }); + it("round-trips plugin app policy context destructive approval mode", async () => { + const sessionFile = path.join(tempDir, "session.json"); + const pluginAppPolicyContext = { + fingerprint: "plugin-policy-1", + apps: { + "google-calendar-app": { + configKey: "google-calendar", + marketplaceName: "openai-curated" as const, + pluginName: "google-calendar", + allowDestructiveActions: true, + destructiveApprovalMode: "auto" as const, + mcpServerNames: ["google-calendar"], + }, + }, + pluginAppIds: { + "google-calendar": ["google-calendar-app"], + }, + }; + await writeCodexAppServerBinding(sessionFile, { + threadId: "thread-123", + cwd: tempDir, + pluginAppPolicyContext, + }); + + const binding = await readCodexAppServerBinding(sessionFile); + + expect(binding?.pluginAppPolicyContext).toEqual(pluginAppPolicyContext); + }); + + it("normalizes v1 plugin app policy context destructive approval modes", async () => { + const sessionFile = path.join(tempDir, "session.json"); + await fs.writeFile( + resolveCodexAppServerBindingPath(sessionFile), + JSON.stringify({ + schemaVersion: 1, + threadId: "thread-123", + cwd: tempDir, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + pluginAppPolicyContext: { + fingerprint: "plugin-policy-1", + apps: { + "auto-accept-app": { + configKey: "gmail", + marketplaceName: "openai-curated", + pluginName: "gmail", + allowDestructiveActions: true, + destructiveApprovalMode: "auto", + mcpServerNames: ["gmail"], + }, + "approval-routed-app": { + configKey: "google-calendar", + marketplaceName: "openai-curated", + pluginName: "google-calendar", + allowDestructiveActions: true, + destructiveApprovalMode: "on-request", + mcpServerNames: ["google-calendar"], + }, + }, + pluginAppIds: { + gmail: ["auto-accept-app"], + "google-calendar": ["approval-routed-app"], + }, + }, + }), + ); + + const binding = await readCodexAppServerBinding(sessionFile); + + expect(binding?.schemaVersion).toBe(2); + expect(binding?.pluginAppPolicyContext?.apps["auto-accept-app"]?.destructiveApprovalMode).toBe( + "allow", + ); + expect( + binding?.pluginAppPolicyContext?.apps["approval-routed-app"]?.destructiveApprovalMode, + ).toBe("auto"); + }); + it("round-trips context-engine binding metadata", async () => { const sessionFile = path.join(tempDir, "session.json"); await writeCodexAppServerBinding(sessionFile, { diff --git a/extensions/codex/src/app-server/session-binding.ts b/extensions/codex/src/app-server/session-binding.ts index b43e2aea7c60..571e9517d000 100644 --- a/extensions/codex/src/app-server/session-binding.ts +++ b/extensions/codex/src/app-server/session-binding.ts @@ -56,7 +56,7 @@ export type CodexAppServerAuthProfileLookup = { /** Durable sidecar binding connecting an OpenClaw session file to a Codex thread. */ export type CodexAppServerThreadBinding = { - schemaVersion: 1; + schemaVersion: 2; threadId: string; sessionFile: string; cwd: string; @@ -157,14 +157,16 @@ export async function readCodexAppServerBinding( return undefined; } try { - const parsed = JSON.parse(raw) as Partial; - if (parsed.schemaVersion !== 1 || typeof parsed.threadId !== "string") { + const parsed = JSON.parse(raw) as Record; + const schemaVersion = + parsed.schemaVersion === 1 || parsed.schemaVersion === 2 ? parsed.schemaVersion : undefined; + if (schemaVersion === undefined || typeof parsed.threadId !== "string") { return undefined; } const authProfileId = typeof parsed.authProfileId === "string" ? parsed.authProfileId : undefined; return { - schemaVersion: 1, + schemaVersion: 2, threadId: parsed.threadId, sessionFile, cwd: typeof parsed.cwd === "string" ? parsed.cwd : "", @@ -203,7 +205,10 @@ export async function readCodexAppServerBinding( typeof parsed.pluginAppsInputFingerprint === "string" ? parsed.pluginAppsInputFingerprint : undefined, - pluginAppPolicyContext: readPluginAppPolicyContext(parsed.pluginAppPolicyContext), + pluginAppPolicyContext: readPluginAppPolicyContext( + parsed.pluginAppPolicyContext, + schemaVersion, + ), contextEngine: readContextEngineBinding(parsed.contextEngine), environmentSelectionFingerprint: typeof parsed.environmentSelectionFingerprint === "string" @@ -232,7 +237,7 @@ export async function writeCodexAppServerBinding( await withCodexAppServerBindingLock(sessionFile, async () => { const now = new Date().toISOString(); const payload: CodexAppServerThreadBinding = { - schemaVersion: 1, + schemaVersion: 2, sessionFile, threadId: binding.threadId, cwd: binding.cwd, @@ -309,7 +314,10 @@ function readContextEngineProjectionBinding( }; } -function readPluginAppPolicyContext(value: unknown): PluginAppPolicyContext | undefined { +function readPluginAppPolicyContext( + value: unknown, + bindingSchemaVersion: 1 | 2, +): PluginAppPolicyContext | undefined { if (!value || typeof value !== "object" || Array.isArray(value)) { return undefined; } @@ -327,12 +335,17 @@ function readPluginAppPolicyContext(value: unknown): PluginAppPolicyContext | un return undefined; } const entry = rawEntry as Record; + const destructiveApprovalMode = readDestructiveApprovalMode( + entry.destructiveApprovalMode, + bindingSchemaVersion, + ); if ( "appId" in entry || typeof entry.configKey !== "string" || entry.marketplaceName !== CODEX_PLUGINS_MARKETPLACE_NAME || typeof entry.pluginName !== "string" || typeof entry.allowDestructiveActions !== "boolean" || + destructiveApprovalMode === "invalid" || !Array.isArray(entry.mcpServerNames) || entry.mcpServerNames.some((serverName) => typeof serverName !== "string") ) { @@ -343,6 +356,7 @@ function readPluginAppPolicyContext(value: unknown): PluginAppPolicyContext | un marketplaceName: entry.marketplaceName, pluginName: entry.pluginName, allowDestructiveActions: entry.allowDestructiveActions, + ...(destructiveApprovalMode ? { destructiveApprovalMode } : {}), mcpServerNames: entry.mcpServerNames, }; } @@ -366,6 +380,28 @@ function readPluginAppPolicyContext(value: unknown): PluginAppPolicyContext | un }; } +function readDestructiveApprovalMode( + value: unknown, + bindingSchemaVersion: 1 | 2, +): PluginAppPolicyContext["apps"][string]["destructiveApprovalMode"] | undefined | "invalid" { + if (value === undefined) { + return undefined; + } + if (value === "deny") { + return "deny"; + } + if (value === "allow") { + return "allow"; + } + if (value === "auto") { + return bindingSchemaVersion === 1 ? "allow" : "auto"; + } + if (value === "on-request" && bindingSchemaVersion === 1) { + return "auto"; + } + return "invalid"; +} + /** Removes the Codex app-server binding sidecar if present. */ export async function clearCodexAppServerBinding( sessionFile: string, diff --git a/extensions/codex/src/app-server/thread-lifecycle.ts b/extensions/codex/src/app-server/thread-lifecycle.ts index c89bb9998376..31ef25841acc 100644 --- a/extensions/codex/src/app-server/thread-lifecycle.ts +++ b/extensions/codex/src/app-server/thread-lifecycle.ts @@ -769,7 +769,7 @@ export async function startOrResumeThread(params: { action: rotatedContextEngineBinding ? "rotated" : "started", }); return { - schemaVersion: 1, + schemaVersion: 2, threadId: response.thread.id, sessionFile: params.params.sessionFile, cwd: params.cwd, diff --git a/extensions/codex/src/command-plugins-management.ts b/extensions/codex/src/command-plugins-management.ts index faa954b18305..5053f5944337 100644 --- a/extensions/codex/src/command-plugins-management.ts +++ b/extensions/codex/src/command-plugins-management.ts @@ -23,7 +23,7 @@ export type CodexPluginConfigEntry = { enabled?: boolean; marketplaceName?: string; pluginName?: string; - allow_destructive_actions?: boolean; + allow_destructive_actions?: boolean | "auto"; }; export type CodexPluginsConfigBlock = { diff --git a/extensions/codex/src/commands.test.ts b/extensions/codex/src/commands.test.ts index 2ea89219a358..9881857cf02e 100644 --- a/extensions/codex/src/commands.test.ts +++ b/extensions/codex/src/commands.test.ts @@ -2563,7 +2563,7 @@ describe("codex command", () => { await firstConfirmBindingRead; } return { - schemaVersion: 1 as const, + schemaVersion: 2 as const, threadId: "thread-race", cwd: "/repo", sessionFile: bindingSessionFile, diff --git a/extensions/codex/src/migration/apply.ts b/extensions/codex/src/migration/apply.ts index 65a73f960a72..19f9ff820a04 100644 --- a/extensions/codex/src/migration/apply.ts +++ b/extensions/codex/src/migration/apply.ts @@ -508,6 +508,7 @@ function readCodexPluginPolicy(item: MigrationItem): ResolvedCodexPluginPolicy | pluginName, enabled: true, allowDestructiveActions: true, + destructiveApprovalMode: "allow", }; } diff --git a/extensions/codex/src/migration/plan.ts b/extensions/codex/src/migration/plan.ts index 66b0835565ec..42518b5293ba 100644 --- a/extensions/codex/src/migration/plan.ts +++ b/extensions/codex/src/migration/plan.ts @@ -43,6 +43,7 @@ export type CodexPluginMigrationConfigEntry = { configKey: string; pluginName: string; enabled: boolean; + allowDestructiveActions?: "auto"; }; type CodexPluginMigrationBlockSkipDetails = { @@ -134,9 +135,11 @@ function hasExistingCodexPluginEntry( existingEntries: Record, configKey: string, pluginName: string, + nextEntry: Record, ): boolean { - if (existingEntries[configKey] !== undefined) { - return true; + const existingEntry = existingEntries[configKey]; + if (existingEntry !== undefined) { + return !isLegacyDestructivePolicyRepair(existingEntry, nextEntry); } return Object.values(existingEntries).some((entry) => { if (!isRecord(entry)) { @@ -146,6 +149,36 @@ function hasExistingCodexPluginEntry( }); } +function isLegacyDestructivePolicyRepair( + existing: unknown, + nextEntry: Record, +): boolean { + const existingEntry = isRecord(existing) ? existing : undefined; + if ( + existingEntry?.allow_destructive_actions !== "on-request" || + nextEntry.allow_destructive_actions !== "auto" + ) { + return false; + } + const normalizedExisting = { ...existingEntry, allow_destructive_actions: "auto" }; + const normalizedEntries = Object.entries(normalizedExisting); + return ( + normalizedEntries.length === Object.keys(nextEntry).length && + normalizedEntries.every(([key, value]) => nextEntry[key] === value) + ); +} + +function isLegacyDestructivePolicyConfigEntryRepair( + existing: unknown, + pluginName: string, +): boolean { + const existingEntry = isRecord(existing) ? existing : undefined; + return ( + existingEntry?.allow_destructive_actions === "on-request" && + existingEntry.pluginName === pluginName + ); +} + function buildPluginItems( ctx: MigrationProviderContext, plugins: readonly CodexPluginSource[], @@ -166,9 +199,25 @@ function buildPluginItems( plugin.pluginName ) { const configKey = uniquePluginConfigKey(plugin, baseCounts, usedCounts); + const plannedEntry = { + enabled: true, + marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, + pluginName: plugin.pluginName, + ...(isLegacyDestructivePolicyConfigEntryRepair( + existingPluginEntries[configKey], + plugin.pluginName, + ) + ? { allow_destructive_actions: "auto" } + : {}), + }; const conflict = !ctx.overwrite && - hasExistingCodexPluginEntry(existingPluginEntries, configKey, plugin.pluginName); + hasExistingCodexPluginEntry( + existingPluginEntries, + configKey, + plugin.pluginName, + plannedEntry, + ); items.push( createMigrationItem({ id: `plugin:${configKey}`, @@ -185,6 +234,9 @@ function buildPluginItems( pluginName: plugin.pluginName, sourceInstalled: plugin.installed === true, sourceEnabled: plugin.enabled === true, + ...(plannedEntry.allow_destructive_actions === "auto" + ? { allowDestructiveActions: "auto" } + : {}), ...(plugin.apps && plugin.apps.length > 0 && !shouldVerifyPluginApps(ctx) ? { sourceAppVerification: CODEX_PLUGIN_SOURCE_APP_VERIFICATION_UNVERIFIED } : {}), @@ -253,17 +305,44 @@ export function readCodexPluginMigrationConfigEntry( ) { return undefined; } - return { configKey, pluginName, enabled }; + const allowDestructiveActions = item.details?.allowDestructiveActions; + return { + configKey, + pluginName, + enabled, + ...(allowDestructiveActions === "auto" ? { allowDestructiveActions: "auto" } : {}), + }; } function readExistingAllowDestructiveActions( config: MigrationProviderContext["config"], -): boolean | undefined { +): boolean | "auto" | undefined { const value = readMigrationConfigPath(config as Record, [ ...CODEX_PLUGIN_NATIVE_CONFIG_PATH, "allow_destructive_actions", ]); - return asBoolean(value); + return normalizeExistingAllowDestructiveActions(value); +} + +function normalizeExistingAllowDestructiveActions(value: unknown): boolean | "auto" | undefined { + return value === "auto" || value === "on-request" ? "auto" : asBoolean(value); +} + +function readExistingPluginPolicyRepairs( + config: MigrationProviderContext["config"] | undefined, +): Record { + if (config === undefined) { + return {}; + } + return Object.fromEntries( + Object.entries(readExistingCodexPluginEntries(config)).flatMap(([configKey, entry]) => { + const pluginEntry = isRecord(entry) ? entry : undefined; + if (pluginEntry?.allow_destructive_actions !== "on-request") { + return []; + } + return [[configKey, { ...pluginEntry, allow_destructive_actions: "auto" }]]; + }), + ); } export function buildCodexPluginsConfigValue( @@ -272,18 +351,24 @@ export function buildCodexPluginsConfigValue( config?: MigrationProviderContext["config"]; } = {}, ): Record { - const plugins = Object.fromEntries( - entries - .toSorted((a, b) => a.configKey.localeCompare(b.configKey)) - .map((entry) => [ - entry.configKey, - { - enabled: entry.enabled, - marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, - pluginName: entry.pluginName, - }, - ]), - ); + const plugins = { + ...readExistingPluginPolicyRepairs(params.config), + ...Object.fromEntries( + entries + .toSorted((a, b) => a.configKey.localeCompare(b.configKey)) + .map((entry) => [ + entry.configKey, + { + enabled: entry.enabled, + marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, + pluginName: entry.pluginName, + ...(entry.allowDestructiveActions + ? { allow_destructive_actions: entry.allowDestructiveActions } + : {}), + }, + ]), + ), + }; const config: Record = { codexPlugins: { enabled: true, @@ -329,9 +414,12 @@ export function hasCodexPluginConfigConflict( return true; } const allowDestructiveActions = nativeConfig.allow_destructive_actions; + const existingAllowDestructiveActions = normalizeExistingAllowDestructiveActions( + existingNativeConfig.allow_destructive_actions, + ); if ( existingNativeConfig.allow_destructive_actions !== undefined && - existingNativeConfig.allow_destructive_actions !== allowDestructiveActions + existingAllowDestructiveActions !== allowDestructiveActions ) { return true; } @@ -347,6 +435,7 @@ export function hasCodexPluginConfigConflict( readExistingCodexPluginEntries(config), configKey, typeof plugin.pluginName === "string" ? plugin.pluginName : configKey, + plugin, ); }); } diff --git a/extensions/codex/src/migration/provider.test.ts b/extensions/codex/src/migration/provider.test.ts index 4497ef57df7d..00c44da71b70 100644 --- a/extensions/codex/src/migration/provider.test.ts +++ b/extensions/codex/src/migration/provider.test.ts @@ -1885,6 +1885,7 @@ describe("buildCodexMigrationProvider", () => { enabled: true, marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: "slack", + allow_destructive_actions: "on-request", }, }, }, @@ -1952,6 +1953,7 @@ describe("buildCodexMigrationProvider", () => { enabled: true, marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, pluginName: "slack", + allow_destructive_actions: "auto", }, }, enabled: true, @@ -2011,7 +2013,6 @@ describe("buildCodexMigrationProvider", () => { stateDir: fixture.stateDir, workspaceDir: fixture.workspaceDir, config: configState, - overwrite: true, }), ); @@ -2029,6 +2030,84 @@ describe("buildCodexMigrationProvider", () => { }); }); + it("repairs old approval-routed destructive plugin policy during migration", async () => { + const fixture = await createCodexFixture(); + const configState: MigrationProviderContext["config"] = { + plugins: { + entries: { + codex: { + enabled: true, + config: { + codexPlugins: { + enabled: true, + allow_destructive_actions: "on-request", + plugins: { + "google-calendar": { + enabled: true, + marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, + pluginName: "google-calendar", + allow_destructive_actions: "on-request", + }, + }, + }, + }, + }, + }, + }, + agents: { defaults: { workspace: fixture.workspaceDir } }, + } as MigrationProviderContext["config"]; + appServerRequest.mockImplementation(async ({ method }: { method: string }) => { + if (method === "plugin/list") { + return pluginList([pluginSummary("google-calendar", { installed: true, enabled: true })]); + } + if (method === "plugin/read") { + return pluginRead("google-calendar"); + } + if (method === "plugin/install") { + return { authPolicy: "ON_USE", appsNeedingAuth: [] } satisfies v2.PluginInstallResponse; + } + if (method === "skills/list") { + return { data: [] } satisfies v2.SkillsListResponse; + } + if (method === "hooks/list") { + return { data: [] } satisfies v2.HooksListResponse; + } + if (method === "config/mcpServer/reload") { + return {}; + } + if (method === "app/list") { + return appsList([]); + } + throw new Error(`unexpected request ${method}`); + }); + const provider = buildCodexMigrationProvider({ + runtime: createConfigRuntime(configState), + }); + + const result = await provider.apply( + makeContext({ + source: fixture.codexHome, + stateDir: fixture.stateDir, + workspaceDir: fixture.workspaceDir, + config: configState, + }), + ); + + expectRecordFields(findItem(result.items, "config:codex-plugins"), { status: "migrated" }); + expect(configState.plugins?.entries?.codex?.config?.codexPlugins).toEqual({ + enabled: true, + allow_destructive_actions: "auto", + plugins: { + "google-calendar": { + enabled: true, + marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME, + pluginName: "google-calendar", + allow_destructive_actions: "auto", + }, + }, + }); + }); + it("records auth-required plugin installs as disabled explicit config entries", async () => { const fixture = await createCodexFixture(); const configState: MigrationProviderContext["config"] = {