Skip to content

Commit f169e0a

Browse files
kevinslinkevinslin
authored
fix(codex): guard against stale codex app snapshots leading to plugin invocation failure (#83807)
* feat(codex): add plugin enable disable list commands * fix(codex): escape plugin management output * test(codex): narrow plugin command coverage * fix(codex): gate plugin management writes * test(codex): type command plugin context * fix(codex): recover plugin app bindings * fix(codex): fail closed on missing app inventory * fix(codex): restore plugin thread config log signal * revert(codex): drop plugin management commands * fix(codex): warn on missing plugin app inventory * fix(codex): trim plugin binding debug logs * fix(codex): restore thread lifecycle json import * chore(codex): remove plugin app debug logs * fix(codex): redact plugin thread config logs
1 parent 6f7d973 commit f169e0a

11 files changed

Lines changed: 522 additions & 25 deletions

‎extensions/codex/harness.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export function createCodexAppServerAgentHarness(options?: {
1414
label?: string;
1515
providerIds?: Iterable<string>;
1616
pluginConfig?: unknown;
17+
resolvePluginConfig?: () => unknown;
1718
}): AgentHarness {
1819
const providerIds = new Set(
1920
[...(options?.providerIds ?? DEFAULT_CODEX_HARNESS_PROVIDER_IDS)].map((id) =>
@@ -39,20 +40,22 @@ export function createCodexAppServerAgentHarness(options?: {
3940
runAttempt: async (params) => {
4041
const { runCodexAppServerAttempt } = await import("./src/app-server/run-attempt.js");
4142
return runCodexAppServerAttempt(params, {
42-
pluginConfig: options?.pluginConfig,
43+
pluginConfig: options?.resolvePluginConfig?.() ?? options?.pluginConfig,
4344
nativeHookRelay: { enabled: true },
4445
});
4546
},
4647
runSideQuestion: async (params) => {
4748
const { runCodexAppServerSideQuestion } = await import("./src/app-server/side-question.js");
4849
return runCodexAppServerSideQuestion(params, {
49-
pluginConfig: options?.pluginConfig,
50+
pluginConfig: options?.resolvePluginConfig?.() ?? options?.pluginConfig,
5051
nativeHookRelay: { enabled: true },
5152
});
5253
},
5354
compact: async (params) => {
5455
const { maybeCompactCodexAppServerSession } = await import("./src/app-server/compact.js");
55-
return maybeCompactCodexAppServerSession(params, { pluginConfig: options?.pluginConfig });
56+
return maybeCompactCodexAppServerSession(params, {
57+
pluginConfig: options?.resolvePluginConfig?.() ?? options?.pluginConfig,
58+
});
5659
},
5760
reset: async (params) => {
5861
if (params.sessionFile) {

‎extensions/codex/index.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,64 @@ describe("codex plugin", () => {
150150
);
151151
});
152152

153+
it("passes live Codex plugin config into public Codex app-server attempts", async () => {
154+
const registerAgentHarness = vi.fn();
155+
const liveConfig = {
156+
plugins: {
157+
entries: {
158+
codex: {
159+
config: {
160+
codexPlugins: {
161+
enabled: true,
162+
plugins: {
163+
"google-calendar": {
164+
marketplaceName: "openai-curated",
165+
pluginName: "google-calendar",
166+
},
167+
},
168+
},
169+
},
170+
},
171+
},
172+
},
173+
};
174+
plugin.register(
175+
createTestPluginApi({
176+
id: "codex",
177+
name: "Codex",
178+
source: "test",
179+
config: {},
180+
pluginConfig: { codexPlugins: { enabled: false } },
181+
runtime: {
182+
config: {
183+
current: () => liveConfig,
184+
},
185+
} as never,
186+
registerAgentHarness,
187+
registerCommand: vi.fn(),
188+
registerMediaUnderstandingProvider: vi.fn(),
189+
registerMigrationProvider: vi.fn(),
190+
registerProvider: vi.fn(),
191+
on: vi.fn(),
192+
}),
193+
);
194+
const harness = mockCallArg(registerAgentHarness) as ReturnType<
195+
typeof createCodexAppServerAgentHarness
196+
>;
197+
const result = { success: true };
198+
runCodexAppServerAttemptMock.mockResolvedValueOnce(result);
199+
200+
await expect(harness.runAttempt({ prompt: "calendar" } as never)).resolves.toBe(result);
201+
202+
expect(runCodexAppServerAttemptMock).toHaveBeenCalledWith(
203+
{ prompt: "calendar" },
204+
{
205+
pluginConfig: liveConfig.plugins.entries.codex.config,
206+
nativeHookRelay: { enabled: true },
207+
},
208+
);
209+
});
210+
153211
it("enables the native hook relay for public Codex side questions", async () => {
154212
const harness = createCodexAppServerAgentHarness({ pluginConfig: { appServer: {} } });
155213
const runSideQuestion = harness.runSideQuestion;

‎extensions/codex/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ export default definePluginEntry({
3131
"codex",
3232
api.pluginConfig as Record<string, unknown>,
3333
) ?? api.pluginConfig;
34-
api.registerAgentHarness(createCodexAppServerAgentHarness({ pluginConfig: api.pluginConfig }));
34+
api.registerAgentHarness(
35+
createCodexAppServerAgentHarness({ resolvePluginConfig: resolveCurrentPluginConfig }),
36+
);
3537
api.registerProvider(buildCodexProvider({ pluginConfig: api.pluginConfig }));
3638
api.registerMediaUnderstandingProvider(
3739
buildCodexMediaUnderstandingProvider({ pluginConfig: api.pluginConfig }),

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

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it, vi } from "vitest";
2-
import { CodexAppInventoryCache, buildCodexAppInventoryCacheKey } from "./app-inventory-cache.js";
2+
import {
3+
CodexAppInventoryCache,
4+
buildCodexAppInventoryCacheKey,
5+
serializeCodexAppInventoryError,
6+
} from "./app-inventory-cache.js";
37
import type { v2 } from "./protocol.js";
48

59
describe("Codex app inventory cache", () => {
@@ -27,7 +31,27 @@ describe("Codex app inventory cache", () => {
2731
expect(fresh.snapshot?.apps.map((item) => item.id)).toEqual(["app-1", "app-2"]);
2832
});
2933

30-
it("uses stale inventory for the current read while refreshing asynchronously", async () => {
34+
it("can read missing inventory without scheduling app/list", async () => {
35+
const cache = new CodexAppInventoryCache({ ttlMs: 100 });
36+
const request = vi.fn(async () => {
37+
return {
38+
data: [app("app-1")],
39+
nextCursor: null,
40+
} satisfies v2.AppsListResponse;
41+
});
42+
43+
const read = cache.read({
44+
key: "runtime",
45+
request,
46+
suppressRefresh: true,
47+
});
48+
49+
expect(read.state).toBe("missing");
50+
expect(read.refreshScheduled).toBe(false);
51+
expect(request).not.toHaveBeenCalled();
52+
});
53+
54+
it("uses stale inventory for the current read while still refreshing asynchronously", async () => {
3155
const cache = new CodexAppInventoryCache({ ttlMs: 10 });
3256
const request = vi.fn(async () => {
3357
return {
@@ -38,7 +62,7 @@ describe("Codex app inventory cache", () => {
3862
const key = "runtime";
3963
await cache.refreshNow({ key, request, nowMs: 0 });
4064

41-
const stale = cache.read({ key, request, nowMs: 11 });
65+
const stale = cache.read({ key, request, nowMs: 11, suppressRefresh: true });
4266
expect(stale.state).toBe("stale");
4367
expect(stale.snapshot?.apps.map((item) => item.id)).toEqual(["app-1"]);
4468
expect(stale.refreshScheduled).toBe(true);
@@ -75,6 +99,17 @@ describe("Codex app inventory cache", () => {
7599
expect(read.diagnostic?.message).toBe("app list failed");
76100
});
77101

102+
it("omits challenge HTML when serializing app/list errors", () => {
103+
const error = new Error(
104+
'failed to list apps: Request failed with status 403 Forbidden: <html><script src="/backend-api/connectors/directory/list?__cf_chl_tk=secret-token"></script></html>',
105+
);
106+
const serialized = serializeCodexAppInventoryError(error);
107+
108+
expect(serialized.message).toBe(
109+
"failed to list apps: Request failed with status 403 Forbidden: [HTML response body omitted]",
110+
);
111+
});
112+
78113
it("forces a post-install refresh past an older in-flight app/list", async () => {
79114
const cache = new CodexAppInventoryCache({ ttlMs: 1_000 });
80115
const key = "runtime";

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

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
import type { v2 } from "./protocol.js";
1+
import { embeddedAgentLog } from "openclaw/plugin-sdk/agent-harness-runtime";
2+
import type { JsonValue, v2 } from "./protocol.js";
23

34
export const CODEX_APP_INVENTORY_CACHE_TTL_MS = 60 * 60 * 1_000;
5+
const MAX_SERIALIZED_ERROR_MESSAGE_LENGTH = 500;
46

57
export type CodexAppInventoryRequest = (
68
method: "app/list",
@@ -50,12 +52,15 @@ type RefreshParams = {
5052
request: CodexAppInventoryRequest;
5153
nowMs?: number;
5254
forceRefetch?: boolean;
55+
suppressRefresh?: boolean;
5356
};
5457

5558
export class CodexAppInventoryCache {
5659
private readonly ttlMs: number;
5760
private readonly entries = new Map<string, CacheEntry>();
5861
private readonly inFlight = new Map<string, Promise<CodexAppInventorySnapshot>>();
62+
// Per-key refresh generation. Each refresh attempt claims the next token so
63+
// an older request that finishes late cannot overwrite a newer snapshot.
5964
private readonly refreshTokens = new Map<string, number>();
6065
private readonly diagnostics = new Map<string, CodexAppInventoryCacheDiagnostic>();
6166
private revision = 0;
@@ -68,7 +73,7 @@ export class CodexAppInventoryCache {
6873
const nowMs = params.nowMs ?? Date.now();
6974
const entry = this.entries.get(params.key);
7075
if (!entry) {
71-
const refreshScheduled = this.scheduleRefresh(params);
76+
const refreshScheduled = params.suppressRefresh ? false : this.scheduleRefresh(params);
7277
return {
7378
state: "missing",
7479
key: params.key,
@@ -168,26 +173,49 @@ export class CodexAppInventoryCache {
168173
expiresAtMs: nowMs + this.ttlMs,
169174
revision: this.revision,
170175
};
176+
// Only publish this snapshot if no newer refresh started for the same key
177+
// while this request was in flight.
171178
if (this.refreshTokens.get(params.key) === refreshToken) {
172179
this.entries.set(params.key, { ...snapshot, invalidated: false });
173180
this.diagnostics.delete(params.key);
174181
}
175182
return snapshot;
176183
} catch (error) {
177184
const diagnostic = {
178-
message: error instanceof Error ? error.message : String(error),
185+
message: sanitizeErrorMessage(error instanceof Error ? error.message : String(error)),
179186
atMs: nowMs,
180187
};
181188
this.diagnostics.set(params.key, diagnostic);
182189
const entry = this.entries.get(params.key);
183190
if (entry) {
184191
entry.lastError = diagnostic;
185192
}
193+
embeddedAgentLog.warn("codex app inventory refresh failed", {
194+
forceRefetch: params.forceRefetch === true,
195+
keyFingerprint: fingerprintInventoryCacheKey(params.key),
196+
error: serializeCodexAppInventoryError(error),
197+
});
186198
throw error;
187199
}
188200
}
189201
}
190202

203+
export function serializeCodexAppInventoryError(error: unknown): Record<string, unknown> {
204+
const record = isRecord(error) ? error : undefined;
205+
const data = record && "data" in record ? redactErrorData(record.data) : undefined;
206+
return {
207+
name:
208+
error instanceof Error
209+
? error.name
210+
: typeof record?.name === "string"
211+
? record.name
212+
: undefined,
213+
message: sanitizeErrorMessage(error instanceof Error ? error.message : String(error)),
214+
...(typeof record?.code === "number" ? { code: record.code } : {}),
215+
...(data !== undefined ? { data } : {}),
216+
};
217+
}
218+
191219
export const defaultCodexAppInventoryCache = new CodexAppInventoryCache();
192220

193221
export function buildCodexAppInventoryCacheKey(input: CodexAppInventoryCacheKeyInput): string {
@@ -223,3 +251,74 @@ function stripEntryState(entry: CacheEntry): CodexAppInventorySnapshot {
223251
const { invalidated: _invalidated, ...snapshot } = entry;
224252
return snapshot;
225253
}
254+
255+
function fingerprintInventoryCacheKey(key: string): string {
256+
let hash = 0;
257+
for (let index = 0; index < key.length; index += 1) {
258+
hash = (hash * 31 + key.charCodeAt(index)) >>> 0;
259+
}
260+
return hash.toString(16).padStart(8, "0");
261+
}
262+
263+
function isRecord(value: unknown): value is Record<string, unknown> {
264+
return Boolean(value && typeof value === "object" && !Array.isArray(value));
265+
}
266+
267+
function redactErrorData(value: unknown, depth = 0): JsonValue | undefined {
268+
if (value === undefined) {
269+
return undefined;
270+
}
271+
if (value === null || typeof value === "boolean" || typeof value === "number") {
272+
return value;
273+
}
274+
if (depth > 6) {
275+
return "[truncated]";
276+
}
277+
if (Array.isArray(value)) {
278+
return value.map((entry) => redactErrorData(entry, depth + 1) ?? null);
279+
}
280+
if (isRecord(value)) {
281+
const redacted: Record<string, JsonValue> = {};
282+
for (const [key, entry] of Object.entries(value)) {
283+
redacted[key] = isSensitiveErrorDataKey(key)
284+
? "<redacted>"
285+
: (redactErrorData(entry, depth + 1) ?? null);
286+
}
287+
return redacted;
288+
}
289+
if (typeof value === "string" && value.length > 500) {
290+
return `${value.slice(0, 500)}...`;
291+
}
292+
if (typeof value === "string") {
293+
return value;
294+
}
295+
if (typeof value === "bigint") {
296+
return value.toString();
297+
}
298+
if (typeof value === "symbol") {
299+
return value.description ? `Symbol(${value.description})` : "Symbol()";
300+
}
301+
if (typeof value === "function") {
302+
return value.name ? `[function ${value.name}]` : "[function]";
303+
}
304+
return "[unserializable]";
305+
}
306+
307+
function sanitizeErrorMessage(message: string): string {
308+
const htmlStart = message.search(/<html[\s>]/i);
309+
const withoutHtml =
310+
htmlStart >= 0
311+
? `${message.slice(0, htmlStart).trimEnd()} [HTML response body omitted]`
312+
: message;
313+
const redacted = withoutHtml.replace(
314+
/([?&][^=\s"'<>]*(?:api[_-]?key|authorization|cookie|credential|password|secret|token|tk)[^=\s"'<>]*=)[^&\s"'<>]+/gi,
315+
"$1<redacted>",
316+
);
317+
return redacted.length > MAX_SERIALIZED_ERROR_MESSAGE_LENGTH
318+
? `${redacted.slice(0, MAX_SERIALIZED_ERROR_MESSAGE_LENGTH)}...`
319+
: redacted;
320+
}
321+
322+
function isSensitiveErrorDataKey(key: string): boolean {
323+
return /api[_-]?key|authorization|cookie|credential|password|secret|token/i.test(key);
324+
}

0 commit comments

Comments
 (0)