Skip to content

Commit 389c355

Browse files
kevinslinkevinslin
authored
fix(codex): preserve per-app approval reviewer policy (#97327)
* fix(codex): version app inventory cache keys * fix(codex): refresh and replay plugin app policy * fix(codex): preserve user reviewer for plugin turns * fix(codex): gate plugin reviewer from startup policy * fix(codex): route app approvals to user reviewer * fix(codex): prompt destructive app tools * fix(codex): scope app approval reviewers * test(codex): complete app policy fixture * fix(codex): avoid pre-start app inventory gate * Revert "fix(codex): prompt destructive app tools" This reverts commit d1cb0d5. # Conflicts: # extensions/codex/src/app-server/plugin-thread-config.test.ts # extensions/codex/src/app-server/plugin-thread-config.ts
1 parent ff1e7e1 commit 389c355

8 files changed

Lines changed: 253 additions & 29 deletions

‎extensions/codex/src/app-server/app-inventory-cache.test.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ describe("Codex app inventory cache", () => {
1818
} satisfies v2.AppsListResponse;
1919
});
2020

21-
const key = buildCodexAppInventoryCacheKey({ codexHome: "/codex", authProfileId: "work" });
21+
const key = buildCodexAppInventoryCacheKey(
22+
{ codexHome: "/codex", authProfileId: "work" },
23+
"2026.6.27",
24+
"2026.6.27",
25+
);
2226
const read = cache.read({ key, request, nowMs: 0 });
2327
expect(read.state).toBe("missing");
2428
expect(read.refreshScheduled).toBe(true);
@@ -33,6 +37,14 @@ describe("Codex app inventory cache", () => {
3337
expect(fresh.snapshot?.apps.map((item) => item.id)).toEqual(["app-1", "app-2"]);
3438
});
3539

40+
it("changes the cache key when either build version changes", () => {
41+
const input = { codexHome: "/codex", authProfileId: "work" };
42+
const baseline = buildCodexAppInventoryCacheKey(input, "2026.6.27", "2026.6.27");
43+
44+
expect(buildCodexAppInventoryCacheKey(input, "2026.6.28", "2026.6.27")).not.toBe(baseline);
45+
expect(buildCodexAppInventoryCacheKey(input, "2026.6.27", "2026.6.28")).not.toBe(baseline);
46+
});
47+
3648
it("can read missing inventory without scheduling app/list", async () => {
3749
const cache = new CodexAppInventoryCache({ ttlMs: 100 });
3850
const request = vi.fn(async () => {
@@ -76,10 +88,7 @@ describe("Codex app inventory cache", () => {
7688

7789
expect(snapshot.apps.map((item) => item.id)).toEqual(["app-1", "google-calendar-app"]);
7890
expect(request).toHaveBeenCalledTimes(2);
79-
expect(request.mock.calls.map(([, params]) => params.cursor ?? null)).toEqual([
80-
null,
81-
"page-2",
82-
]);
91+
expect(request.mock.calls.map(([, params]) => params.cursor ?? null)).toEqual([null, "page-2"]);
8392
});
8493

8594
it("uses stale inventory for the current read while still refreshing asynchronously", async () => {

‎extensions/codex/src/app-server/app-inventory-cache.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,15 @@ export function serializeCodexAppInventoryError(error: unknown): Record<string,
252252
/** Shared app inventory cache used by Codex app-server runtime paths. */
253253
export const defaultCodexAppInventoryCache = new CodexAppInventoryCache();
254254

255-
/** Builds a stable cache key from runtime identity fields. */
256-
export function buildCodexAppInventoryCacheKey(input: CodexAppInventoryCacheKeyInput): string {
255+
/** Builds a stable cache key from build versions and runtime identity fields. */
256+
export function buildCodexAppInventoryCacheKey(
257+
input: CodexAppInventoryCacheKeyInput,
258+
openClawVersion: string,
259+
codexPluginVersion: string,
260+
): string {
257261
return JSON.stringify({
262+
openClawVersion,
263+
codexPluginVersion,
258264
codexHome: input.codexHome ?? null,
259265
endpoint: input.endpoint ?? null,
260266
runtimeIdentity: normalizeRuntimeIdentityForCacheKey(input.runtimeIdentity),

‎extensions/codex/src/app-server/plugin-app-cache-key.ts‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@
33
* auth, account, and version inputs without storing secret material.
44
*/
55
import { createHash } from "node:crypto";
6+
import { createRequire } from "node:module";
7+
import { OPENCLAW_VERSION } from "openclaw/plugin-sdk/agent-harness-runtime";
8+
import { readPluginPackageVersion } from "openclaw/plugin-sdk/extension-shared";
69
import {
710
buildCodexAppInventoryCacheKey,
811
type CodexAppInventoryCacheKeyInput,
912
} from "./app-inventory-cache.js";
1013
import { resolveCodexAppServerHomeDir } from "./auth-bridge.js";
11-
import type { CodexAppServerRuntimeOptions, CodexAppServerStartOptions } from "./config.js";
1214
import type { CodexAppServerRuntimeIdentity } from "./client.js";
15+
import type { CodexAppServerRuntimeOptions, CodexAppServerStartOptions } from "./config.js";
16+
17+
const require = createRequire(import.meta.url);
18+
const CODEX_PLUGIN_VERSION = readPluginPackageVersion({ require });
1319

1420
/** Inputs that identify the Codex app inventory cache scope for one runtime. */
1521
export type CodexPluginAppCacheKeyParams = Omit<
@@ -23,17 +29,21 @@ export type CodexPluginAppCacheKeyParams = Omit<
2329

2430
/** Builds the full app inventory cache key for Codex plugin/app discovery. */
2531
export function buildCodexPluginAppCacheKey(params: CodexPluginAppCacheKeyParams): string {
26-
return buildCodexAppInventoryCacheKey({
27-
codexHome:
28-
params.runtimeIdentity?.codexHome ??
29-
resolveCodexPluginAppCacheCodexHome(params.appServer, params.agentDir),
30-
endpoint: resolveCodexPluginAppCacheEndpoint(params.appServer),
31-
authProfileId: params.authProfileId,
32-
accountId: params.accountId,
33-
envApiKeyFingerprint: params.envApiKeyFingerprint,
34-
appServerVersion: params.appServerVersion ?? params.runtimeIdentity?.serverVersion,
35-
runtimeIdentity: params.runtimeIdentity,
36-
});
32+
return buildCodexAppInventoryCacheKey(
33+
{
34+
codexHome:
35+
params.runtimeIdentity?.codexHome ??
36+
resolveCodexPluginAppCacheCodexHome(params.appServer, params.agentDir),
37+
endpoint: resolveCodexPluginAppCacheEndpoint(params.appServer),
38+
authProfileId: params.authProfileId,
39+
accountId: params.accountId,
40+
envApiKeyFingerprint: params.envApiKeyFingerprint,
41+
appServerVersion: params.appServerVersion ?? params.runtimeIdentity?.serverVersion,
42+
runtimeIdentity: params.runtimeIdentity,
43+
},
44+
OPENCLAW_VERSION,
45+
CODEX_PLUGIN_VERSION,
46+
);
3747
}
3848

3949
/** Builds a durable thread-binding fingerprint for one initialized app-server runtime. */

‎extensions/codex/src/app-server/plugin-thread-config.test.ts‎

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vitest";
33
import { CodexAppInventoryCache } from "./app-inventory-cache.js";
44
import { CODEX_PLUGINS_MARKETPLACE_NAME } from "./config.js";
55
import {
6+
buildCodexPluginAppsConfigPatchFromPolicyContext,
67
buildCodexPluginThreadConfig,
78
buildCodexPluginThreadConfigInputFingerprint,
89
isCodexPluginThreadBindingStale,
@@ -68,6 +69,9 @@ describe("Codex plugin thread config", () => {
6869
},
6970
},
7071
});
72+
expect(config.configPatch).not.toHaveProperty("approvals_reviewer");
73+
const apps = config.configPatch?.apps as Record<string, unknown> | undefined;
74+
expect(apps?.["_default"]).not.toHaveProperty("approvals_reviewer");
7175
expect(config.policyContext.apps["google-calendar-app"]).toEqual({
7276
configKey: "google-calendar",
7377
marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME,
@@ -104,6 +108,7 @@ describe("Codex plugin thread config", () => {
104108
default_tools_approval_mode: "auto",
105109
});
106110
expect(disabledApps?.["google-calendar-app"]).not.toHaveProperty("default_tools_enabled");
111+
expect(disabledApps?.["google-calendar-app"]).not.toHaveProperty("approvals_reviewer");
107112
expect(disabledApps?.["google-calendar-app"]).not.toHaveProperty("tools");
108113
expect(
109114
pluginOverrideDisabled.policyContext.apps["google-calendar-app"]?.allowDestructiveActions,
@@ -135,6 +140,7 @@ describe("Codex plugin thread config", () => {
135140
open_world_enabled: true,
136141
default_tools_approval_mode: "auto",
137142
});
143+
expect(enabledApps?.["google-calendar-app"]).not.toHaveProperty("approvals_reviewer");
138144
expect(
139145
pluginOverrideEnabled.policyContext.apps["google-calendar-app"]?.allowDestructiveActions,
140146
).toBe(true);
@@ -164,13 +170,14 @@ describe("Codex plugin thread config", () => {
164170
open_world_enabled: true,
165171
default_tools_approval_mode: "auto",
166172
});
173+
expect(apps?.["google-calendar-app"]).not.toHaveProperty("approvals_reviewer");
167174
expect(config.policyContext.apps["google-calendar-app"]).toMatchObject({
168175
allowDestructiveActions: true,
169176
destructiveApprovalMode: "auto",
170177
});
171178
});
172179

173-
it("exposes destructive app access while clearing only durable approval overrides for always mode", async () => {
180+
it("routes destructive approvals to the user while clearing durable overrides for always mode", async () => {
174181
const appCache = new CodexAppInventoryCache();
175182
await appCache.refreshNow({
176183
key: "runtime",
@@ -258,10 +265,12 @@ describe("Codex plugin thread config", () => {
258265
const apps = config.configPatch?.apps as Record<string, unknown> | undefined;
259266
expect(apps?.["google-calendar-app"]).toEqual({
260267
enabled: true,
268+
approvals_reviewer: "user",
261269
destructive_enabled: true,
262270
open_world_enabled: true,
263271
default_tools_approval_mode: "auto",
264272
});
273+
expect(config.configPatch).not.toHaveProperty("approvals_reviewer");
265274
expect(config.policyContext.apps["google-calendar-app"]).toMatchObject({
266275
allowDestructiveActions: true,
267276
destructiveApprovalMode: "always",
@@ -285,6 +294,89 @@ describe("Codex plugin thread config", () => {
285294
});
286295
});
287296

297+
it.each([
298+
["auto", "auto", undefined],
299+
["boolean true", true, undefined],
300+
["boolean false", false, undefined],
301+
["always", "always", "user"],
302+
] as const)(
303+
"applies the resolved per-plugin %s reviewer policy over global always",
304+
async (_name, pluginOverride, expectedReviewer) => {
305+
const config = await buildReadyGoogleCalendarThreadConfig({
306+
codexPlugins: {
307+
enabled: true,
308+
allow_destructive_actions: "always",
309+
plugins: {
310+
"google-calendar": {
311+
marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME,
312+
pluginName: "google-calendar",
313+
allow_destructive_actions: pluginOverride,
314+
},
315+
},
316+
},
317+
});
318+
319+
const apps = config.configPatch?.apps as Record<string, unknown> | undefined;
320+
const app = apps?.["google-calendar-app"] as Record<string, unknown> | undefined;
321+
expect(app?.approvals_reviewer).toBe(expectedReviewer);
322+
expect(config.policyContext.apps["google-calendar-app"]?.destructiveApprovalMode).toBe(
323+
pluginOverride === true ? "allow" : pluginOverride === false ? "deny" : pluginOverride,
324+
);
325+
},
326+
);
327+
328+
it("rebuilds persisted app policy with the same reviewer precedence", () => {
329+
const configPatch = buildCodexPluginAppsConfigPatchFromPolicyContext({
330+
fingerprint: "policy",
331+
apps: {
332+
"always-app": {
333+
configKey: "always",
334+
marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME,
335+
pluginName: "always",
336+
allowDestructiveActions: true,
337+
destructiveApprovalMode: "always",
338+
mcpServerNames: ["always"],
339+
},
340+
"auto-app": {
341+
configKey: "auto",
342+
marketplaceName: CODEX_PLUGINS_MARKETPLACE_NAME,
343+
pluginName: "auto",
344+
allowDestructiveActions: true,
345+
destructiveApprovalMode: "auto",
346+
mcpServerNames: ["auto"],
347+
},
348+
},
349+
pluginAppIds: {
350+
always: ["always-app"],
351+
auto: ["auto-app"],
352+
},
353+
});
354+
355+
expect(configPatch).toEqual({
356+
apps: {
357+
_default: {
358+
enabled: false,
359+
destructive_enabled: false,
360+
open_world_enabled: false,
361+
},
362+
"always-app": {
363+
enabled: true,
364+
approvals_reviewer: "user",
365+
destructive_enabled: true,
366+
open_world_enabled: true,
367+
default_tools_approval_mode: "auto",
368+
},
369+
"auto-app": {
370+
enabled: true,
371+
destructive_enabled: true,
372+
open_world_enabled: true,
373+
default_tools_approval_mode: "auto",
374+
},
375+
},
376+
});
377+
expect(configPatch).not.toHaveProperty("approvals_reviewer");
378+
});
379+
288380
it("omits always policy apps when cwd effective approval overrides remain after cleanup", async () => {
289381
const appCache = new CodexAppInventoryCache();
290382
await appCache.refreshNow({
@@ -958,6 +1050,7 @@ describe("Codex plugin thread config", () => {
9581050
request,
9591051
});
9601052

1053+
expect(config.configPatch).not.toHaveProperty("approvals_reviewer");
9611054
expect(config.configPatch?.apps).toEqual({
9621055
_default: {
9631056
enabled: false,
@@ -1049,6 +1142,7 @@ describe("Codex plugin thread config", () => {
10491142
request,
10501143
});
10511144

1145+
expect(config.configPatch).not.toHaveProperty("approvals_reviewer");
10521146
expect(config.configPatch?.apps).toEqual({
10531147
_default: {
10541148
enabled: false,
@@ -1552,6 +1646,9 @@ async function buildReadyGoogleCalendarThreadConfig(
15521646
if (method === "plugin/read") {
15531647
return pluginDetail("google-calendar", [appSummary("google-calendar-app")]);
15541648
}
1649+
if (method === "config/read") {
1650+
return { config: {} };
1651+
}
15551652
throw new Error(`unexpected request ${method}`);
15561653
},
15571654
});

‎extensions/codex/src/app-server/plugin-thread-config.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,9 @@ export async function buildCodexPluginThreadConfig(
269269
open_world_enabled: true,
270270
default_tools_approval_mode: "auto",
271271
};
272+
if (record.policy.destructiveApprovalMode === "always") {
273+
appConfig.approvals_reviewer = "user";
274+
}
272275
apps[app.id] = appConfig;
273276
policyApps[app.id] = {
274277
configKey: record.policy.configKey,
@@ -369,6 +372,31 @@ function buildDisabledAppsConfigPatch(): JsonObject {
369372
};
370373
}
371374

375+
/** Rebuilds the safe per-thread apps patch persisted with a Codex thread binding. */
376+
export function buildCodexPluginAppsConfigPatchFromPolicyContext(
377+
policyContext: PluginAppPolicyContext,
378+
): JsonObject {
379+
const apps: JsonObject = {
380+
_default: {
381+
enabled: false,
382+
destructive_enabled: false,
383+
open_world_enabled: false,
384+
},
385+
};
386+
for (const [appId, policy] of Object.entries(policyContext.apps).toSorted(([left], [right]) =>
387+
left.localeCompare(right),
388+
)) {
389+
apps[appId] = {
390+
enabled: true,
391+
destructive_enabled: policy.allowDestructiveActions,
392+
open_world_enabled: true,
393+
default_tools_approval_mode: "auto",
394+
...(policy.destructiveApprovalMode === "always" ? { approvals_reviewer: "user" } : {}),
395+
};
396+
}
397+
return { apps };
398+
}
399+
372400
function buildPluginAppPolicyContext(
373401
apps: Record<string, PluginAppPolicyContextEntry>,
374402
pluginAppIds: Record<string, string[]>,

0 commit comments

Comments
 (0)