From ed28c57e20bc43cec96e17d0851913022f26b349 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 7 Jul 2026 15:48:08 +0100 Subject: [PATCH] fix(codex): validate maintained app-server types (#101726) Co-authored-by: Vincent Koc --- .../codex/src/app-server/config.test.ts | 15 +++ extensions/codex/src/app-server/config.ts | 24 ++-- extensions/codex/src/app-server/protocol.ts | 79 +++++++++---- scripts/check-codex-app-server-protocol.ts | 110 ++++++++++++++++++ .../lib/codex-app-server-protocol-source.ts | 44 +++++++ .../codex-app-server-protocol-source.test.ts | 40 +++++++ 6 files changed, 275 insertions(+), 37 deletions(-) diff --git a/extensions/codex/src/app-server/config.test.ts b/extensions/codex/src/app-server/config.test.ts index 9533a93f940..87c5725bee0 100644 --- a/extensions/codex/src/app-server/config.test.ts +++ b/extensions/codex/src/app-server/config.test.ts @@ -23,6 +23,7 @@ import { resolveOpenClawExecPolicyForCodexAppServer, resolveCodexPluginsPolicy, shouldAutoApproveCodexAppServerApprovals, + withMcpElicitationsApprovalPolicy, } from "./config.js"; type RuntimeOptionsParams = NonNullable[0]>; @@ -31,6 +32,20 @@ function resolveRuntimeForTest(params: RuntimeOptionsParams = {}) { return resolveCodexAppServerRuntimeOptions({ env: {}, requirementsToml: null, ...params }); } +describe("withMcpElicitationsApprovalPolicy", () => { + it("returns every field required by Codex granular approval policy", () => { + expect(withMcpElicitationsApprovalPolicy("never")).toEqual({ + granular: { + mcp_elicitations: true, + request_permissions: false, + rules: false, + sandbox_approval: false, + skill_approval: false, + }, + }); + }); +}); + function envRef(id: string) { return { source: "env" as const, provider: "default", id }; } diff --git a/extensions/codex/src/app-server/config.ts b/extensions/codex/src/app-server/config.ts index 235cd6ffe42..c68ad1defca 100644 --- a/extensions/codex/src/app-server/config.ts +++ b/extensions/codex/src/app-server/config.ts @@ -21,7 +21,13 @@ import { import { normalizeTrimmedStringList } from "openclaw/plugin-sdk/string-coerce-runtime"; import { detectWindowsSpawnCommandInlineArgs } from "openclaw/plugin-sdk/windows-spawn"; import { z } from "zod"; -import type { CodexSandboxPolicy, CodexServiceTier, JsonObject, JsonValue } from "./protocol.js"; +import type { + CodexApprovalPolicy, + CodexSandboxPolicy, + CodexServiceTier, + JsonObject, + JsonValue, +} from "./protocol.js"; const START_OPTIONS_KEY_SECRET_SYMBOL = Symbol.for("openclaw.codexAppServerStartOptionsKeySecret"); const START_OPTIONS_KEY_SECRET = getStartOptionsKeySecret(); @@ -60,17 +66,7 @@ type CodexAppServerDefaultPolicy = { }; export type CodexAppServerApprovalPolicy = "never" | "on-request" | "on-failure" | "untrusted"; export type CodexAppServerApprovalPolicySource = "config" | "env" | "requirements" | "implicit"; -export type CodexAppServerEffectiveApprovalPolicy = - | CodexAppServerApprovalPolicy - | { - granular: { - mcp_elicitations: boolean; - rules: boolean; - sandbox_approval: boolean; - request_permissions?: boolean; - skill_approval?: boolean; - }; - }; +export type CodexAppServerEffectiveApprovalPolicy = CodexApprovalPolicy; export type CodexAppServerSandboxMode = "read-only" | "workspace-write" | "danger-full-access"; type CodexAppServerApprovalsReviewer = "user" | "auto_review" | "guardian_subagent"; type CodexAppServerCommandSource = "managed" | "resolved-managed" | "config" | "env"; @@ -1053,6 +1049,8 @@ export function withMcpElicitationsApprovalPolicy( mcp_elicitations: true, rules: false, sandbox_approval: false, + request_permissions: false, + skill_approval: false, }, }; } @@ -1061,6 +1059,8 @@ export function withMcpElicitationsApprovalPolicy( mcp_elicitations: true, rules: true, sandbox_approval: true, + request_permissions: true, + skill_approval: true, }, }; } diff --git a/extensions/codex/src/app-server/protocol.ts b/extensions/codex/src/app-server/protocol.ts index 6b96d71c8d2..6de8815be3b 100644 --- a/extensions/codex/src/app-server/protocol.ts +++ b/extensions/codex/src/app-server/protocol.ts @@ -2,6 +2,23 @@ export type JsonValue = null | boolean | number | string | JsonValue[] | JsonObject; export type JsonObject = { [key: string]: JsonValue }; export type CodexServiceTier = string; +export type CodexApprovalPolicy = + | "untrusted" + | "on-failure" + | "on-request" + | { + granular: { + sandbox_approval: boolean; + rules: boolean; + skill_approval: boolean; + request_permissions: boolean; + mcp_elicitations: boolean; + }; + } + | "never"; +export type CodexApprovalsReviewer = "user" | "auto_review" | "guardian_subagent"; +export type CodexSandboxMode = "read-only" | "workspace-write" | "danger-full-access"; +export type CodexPersonality = "none" | "friendly" | "pragmatic"; export type CodexAppServerRequestMethod = keyof CodexAppServerRequestResultMap | (string & {}); export type CodexAppServerRequestParams = @@ -57,7 +74,10 @@ export type CodexUserInput = | { type: "text"; text: string; - text_elements?: JsonValue[]; + text_elements: Array<{ + byteRange: { start: number; end: number }; + placeholder: string | null; + }>; } | { type: "image"; @@ -103,10 +123,10 @@ export type CodexThreadStartParams = JsonObject & { cwd?: string; model?: string; modelProvider?: string | null; - personality?: string | null; - approvalPolicy?: string | JsonObject; - approvalsReviewer?: string | null; - sandbox?: string; + personality?: CodexPersonality | null; + approvalPolicy?: CodexApprovalPolicy | null; + approvalsReviewer?: CodexApprovalsReviewer | null; + sandbox?: CodexSandboxMode | null; serviceTier?: CodexServiceTier | null; dynamicTools?: CodexDynamicToolSpec[] | null; developerInstructions?: string; @@ -118,10 +138,10 @@ export type CodexThreadResumeParams = JsonObject & { threadId: string; model?: string; modelProvider?: string | null; - personality?: string | null; - approvalPolicy?: string | JsonObject; - approvalsReviewer?: string | null; - sandbox?: string; + personality?: CodexPersonality | null; + approvalPolicy?: CodexApprovalPolicy | null; + approvalsReviewer?: CodexApprovalsReviewer | null; + sandbox?: CodexSandboxMode | null; serviceTier?: CodexServiceTier | null; config?: JsonObject; developerInstructions?: string; @@ -137,7 +157,7 @@ export type CodexThreadForkParams = CodexThreadStartParams & { threadId: string; baseInstructions?: string; ephemeral?: boolean; - threadSource?: string | JsonObject; + threadSource?: string | null; excludeTurns?: boolean; }; @@ -217,25 +237,37 @@ export type CodexTurnInterruptParams = JsonObject & { export type CodexTurnStartParams = JsonObject & { threadId: string; - input?: CodexUserInput[]; + input: CodexUserInput[]; cwd?: string; model?: string; - approvalPolicy?: string | JsonObject; - approvalsReviewer?: string | null; + approvalPolicy?: CodexApprovalPolicy | null; + approvalsReviewer?: CodexApprovalsReviewer | null; sandboxPolicy?: CodexSandboxPolicy; serviceTier?: CodexServiceTier | null; effort?: string | null; - personality?: string | null; + personality?: CodexPersonality | null; environments?: CodexTurnEnvironmentParams[] | null; collaborationMode?: { - mode: string; - settings: JsonObject & { + mode: "plan" | "default"; + settings: { + model: string; + reasoning_effort: string | null; developer_instructions: string | null; }; } | null; }; -export type CodexSandboxPolicy = string | JsonObject; +export type CodexSandboxPolicy = + | { type: "dangerFullAccess" } + | { type: "readOnly"; networkAccess: boolean } + | { type: "externalSandbox"; networkAccess: "restricted" | "enabled" } + | { + type: "workspaceWrite"; + writableRoots: string[]; + networkAccess: boolean; + excludeTmpdirEnvVar: boolean; + excludeSlashTmp: boolean; + }; export type CodexTurnStartResponse = { turn: CodexTurn; @@ -243,11 +275,11 @@ export type CodexTurnStartResponse = { export type CodexTurn = { id: string; - threadId: string; + threadId?: string; status?: string; - error?: CodexErrorNotification["error"]; - startedAt?: string | null; - completedAt?: string | null; + error?: CodexErrorNotification["error"] | null; + startedAt?: number | null; + completedAt?: number | null; durationMs?: number | null; items: CodexThreadItem[]; }; @@ -374,10 +406,7 @@ export type CodexDynamicToolCallOutputContentItem = export type CodexErrorNotification = { error: { message?: string; - codexErrorInfo?: { - message?: string; - [key: string]: unknown; - }; + codexErrorInfo?: string | JsonObject | null; additionalDetails?: string | null; [key: string]: unknown; }; diff --git a/scripts/check-codex-app-server-protocol.ts b/scripts/check-codex-app-server-protocol.ts index ef256abaab4..4273949e143 100644 --- a/scripts/check-codex-app-server-protocol.ts +++ b/scripts/check-codex-app-server-protocol.ts @@ -1,3 +1,4 @@ +import { spawnSync } from "node:child_process"; // Check Codex App Server Protocol script supports OpenClaw repository automation. import fs from "node:fs/promises"; import path from "node:path"; @@ -93,6 +94,7 @@ async function main(): Promise { try { await compareGeneratedProtocolMirror(source.jsonRoot); + await checkMaintainedProtocolTypes(source.typescriptRoot); for (const check of checks) { const filePath = path.join(source.typescriptRoot, check.file); @@ -129,6 +131,114 @@ async function main(): Promise { ); } +async function checkMaintainedProtocolTypes(sourceRoot: string): Promise { + // Raw requests go to Codex; raw responses flow into OpenClaw. Keep the + // assignability direction explicit so the probe permits deliberate projections. + const probePath = path.join(sourceRoot, "openclaw-protocol-compatibility.ts"); + const protocolPath = path.resolve(process.cwd(), "extensions/codex/src/app-server/protocol.ts"); + const protocolImport = relativeTypeScriptImport(probePath, protocolPath); + const generatedImport = (file: string) => + relativeTypeScriptImport(probePath, path.join(sourceRoot, file)); + const probe = ` +import type { + CodexDynamicToolSpec, + CodexDynamicToolCallParams, + CodexErrorNotification, + CodexModelListResponse, + CodexThreadForkParams, + CodexThreadForkResponse, + CodexThreadResumeParams, + CodexThreadResumeResponse, + CodexThreadStartParams, + CodexThreadStartResponse, + CodexTurnEnvironmentParams, + CodexTurnInterruptParams, + CodexTurnStartParams, +} from ${JSON.stringify(protocolImport)}; +import type { DynamicToolCallParams } from ${JSON.stringify(generatedImport("v2/DynamicToolCallParams.ts"))}; +import type { DynamicToolSpec } from ${JSON.stringify(generatedImport("v2/DynamicToolSpec.ts"))}; +import type { ErrorNotification } from ${JSON.stringify(generatedImport("v2/ErrorNotification.ts"))}; +import type { ModelListResponse } from ${JSON.stringify(generatedImport("v2/ModelListResponse.ts"))}; +import type { ThreadForkParams } from ${JSON.stringify(generatedImport("v2/ThreadForkParams.ts"))}; +import type { ThreadForkResponse } from ${JSON.stringify(generatedImport("v2/ThreadForkResponse.ts"))}; +import type { ThreadResumeParams } from ${JSON.stringify(generatedImport("v2/ThreadResumeParams.ts"))}; +import type { ThreadResumeResponse } from ${JSON.stringify(generatedImport("v2/ThreadResumeResponse.ts"))}; +import type { ThreadStartParams } from ${JSON.stringify(generatedImport("v2/ThreadStartParams.ts"))}; +import type { ThreadStartResponse } from ${JSON.stringify(generatedImport("v2/ThreadStartResponse.ts"))}; +import type { TurnEnvironmentParams } from ${JSON.stringify(generatedImport("v2/TurnEnvironmentParams.ts"))}; +import type { TurnInterruptParams } from ${JSON.stringify(generatedImport("v2/TurnInterruptParams.ts"))}; +import type { TurnStartParams } from ${JSON.stringify(generatedImport("v2/TurnStartParams.ts"))}; + +declare const openClawDynamicToolSpec: CodexDynamicToolSpec; +const generatedDynamicToolSpec: DynamicToolSpec = openClawDynamicToolSpec; +declare const openClawTurnEnvironmentParams: CodexTurnEnvironmentParams; +const generatedTurnEnvironmentParams: TurnEnvironmentParams = openClawTurnEnvironmentParams; +declare const openClawThreadStartParams: CodexThreadStartParams; +const generatedThreadStartParams: ThreadStartParams = openClawThreadStartParams; +declare const openClawThreadResumeParams: CodexThreadResumeParams; +const generatedThreadResumeParams: ThreadResumeParams = openClawThreadResumeParams; +declare const openClawThreadForkParams: CodexThreadForkParams; +const generatedThreadForkParams: ThreadForkParams = openClawThreadForkParams; +declare const openClawTurnInterruptParams: CodexTurnInterruptParams; +const generatedTurnInterruptParams: TurnInterruptParams = openClawTurnInterruptParams; +declare const openClawTurnStartParams: CodexTurnStartParams; +const generatedTurnStartParams: TurnStartParams = openClawTurnStartParams; + +declare const generatedDynamicToolCallParams: Omit; +const openClawDynamicToolCallParams: Omit = + generatedDynamicToolCallParams; +declare const generatedErrorNotification: ErrorNotification; +const openClawErrorNotification: CodexErrorNotification = generatedErrorNotification; +declare const generatedModelListResponse: ModelListResponse; +const openClawModelListResponse: CodexModelListResponse = generatedModelListResponse; + +// Thread and turn bodies are normalized behind checked-in JSON schemas. Their +// raw generated shapes must not be confused with the projector-facing types. +declare const generatedThreadForkResponse: Omit; +const openClawThreadForkResponse: Omit = + generatedThreadForkResponse; +declare const generatedThreadResumeResponse: Omit; +const openClawThreadResumeResponse: Omit = + generatedThreadResumeResponse; +declare const generatedThreadStartResponse: Omit; +const openClawThreadStartResponse: Omit = + generatedThreadStartResponse; + +export {}; +`; + await fs.writeFile(probePath, probe); + const result = spawnSync( + process.execPath, + [ + "scripts/run-tsgo.mjs", + "--ignoreConfig", + "--noEmit", + "--allowImportingTsExtensions", + "--strict", + "--skipLibCheck", + "--module", + "nodenext", + "--moduleResolution", + "nodenext", + probePath, + ], + { cwd: process.cwd(), encoding: "utf8" }, + ); + if (result.error) { + failures.push(`maintained protocol types: failed to start tsgo (${result.error.message})`); + return; + } + if (result.status !== 0) { + const output = `${result.stdout}${result.stderr}`.trim(); + failures.push(`maintained protocol types differ from generated Codex types\n${output}`); + } +} + +function relativeTypeScriptImport(fromFile: string, toFile: string): string { + const relative = path.relative(path.dirname(fromFile), toFile).replaceAll(path.sep, "/"); + return relative.startsWith(".") ? relative : `./${relative}`; +} + async function compareGeneratedProtocolMirror(sourceJsonRoot: string): Promise { for (const schema of selectedCodexAppServerJsonSchemas) { const sourcePath = path.join(sourceJsonRoot, schema); diff --git a/scripts/lib/codex-app-server-protocol-source.ts b/scripts/lib/codex-app-server-protocol-source.ts index f55f3d75d77..ab4a4a66f6c 100644 --- a/scripts/lib/codex-app-server-protocol-source.ts +++ b/scripts/lib/codex-app-server-protocol-source.ts @@ -158,6 +158,7 @@ export async function generateExperimentalCodexAppServerProtocolSource( repoRoot = process.cwd(), ): Promise { const { codexRepo } = await resolveCodexAppServerProtocolSource(repoRoot); + await validateCodexProtocolSourceVersion({ codexRepo, repoRoot }); const root = await fs.mkdtemp(path.join(repoRoot, ".tmp-codex-app-server-protocol-")); const generatedRoot = path.join(root, "generated"); const typescriptRoot = path.join(root, "typescript"); @@ -187,6 +188,41 @@ export async function generateExperimentalCodexAppServerProtocolSource( }; } +export function readCargoWorkspacePackageVersion(manifest: string): string | undefined { + const header = /^\s*\[workspace\.package\]\s*(?:#.*)?$/m.exec(manifest); + if (!header) { + return undefined; + } + const remainder = manifest.slice(header.index + header[0].length); + const nextSection = /^\s*\[/m.exec(remainder); + const workspacePackage = remainder.slice(0, nextSection?.index ?? remainder.length); + return /^\s*version\s*=\s*"([^"]+)"\s*(?:#.*)?$/m.exec(workspacePackage)?.[1]; +} + +export async function validateCodexProtocolSourceVersion(params: { + codexRepo: string; + repoRoot: string; +}): Promise { + const packageManifest = JSON.parse( + await fs.readFile(path.join(params.repoRoot, "extensions/codex/package.json"), "utf8"), + ) as { dependencies?: Record }; + const expectedVersion = packageManifest.dependencies?.["@openai/codex"]; + if (typeof expectedVersion !== "string" || expectedVersion.length === 0) { + throw new Error("extensions/codex/package.json must pin @openai/codex to an exact version"); + } + + const cargoManifest = await fs.readFile( + path.join(params.codexRepo, "codex-rs/Cargo.toml"), + "utf8", + ); + const sourceVersion = readCargoWorkspacePackageVersion(cargoManifest); + if (sourceVersion !== expectedVersion) { + throw new Error( + `Codex protocol source version ${sourceVersion ?? ""} does not match @openai/codex ${expectedVersion}. Check out rust-v${expectedVersion} in ${params.codexRepo}.`, + ); + } +} + async function collectCodexRepoCandidates(repoRoot: string): Promise { const candidates = [ process.env.OPENCLAW_CODEX_REPO, @@ -326,6 +362,9 @@ function runCargoProtocolGenerator(codexRepo: string, args: string[]): void { cwd: codexRepo, stdio: "inherit", }); + if (result.error) { + throw new Error(`Failed to start cargo: ${result.error.message}`, { cause: result.error }); + } if (result.status !== 0) { throw new Error(`cargo ${args.join(" ")} failed with exit code ${result.status ?? "unknown"}`); } @@ -346,6 +385,11 @@ function formatGeneratedTypeScript(repoRoot: string, root: string): void { stdio: "inherit", windowsVerbatimArguments: command.windowsVerbatimArguments, }); + if (result.error) { + throw new Error(`Failed to start protocol formatter: ${result.error.message}`, { + cause: result.error, + }); + } if (result.status !== 0) { throw new Error( `pnpm exec oxfmt --write --threads=1 ${root} failed with exit code ${ diff --git a/test/scripts/codex-app-server-protocol-source.test.ts b/test/scripts/codex-app-server-protocol-source.test.ts index b34aa172022..b3dc02cef7f 100644 --- a/test/scripts/codex-app-server-protocol-source.test.ts +++ b/test/scripts/codex-app-server-protocol-source.test.ts @@ -6,10 +6,12 @@ import { buildCodexProtocolExportArgs, canonicalizeCodexAppServerProtocolJson, formatCodexAppServerProtocolJsonText, + readCargoWorkspacePackageVersion, resolveCodexAppServerProtocolSource, resolveCodexProtocolCargoTargetDir, resolveCodexProtocolMinFreeBytes, resolveCodexProtocolPnpmCommand, + validateCodexProtocolSourceVersion, validateCodexProtocolGenerationHeadroom, } from "../../scripts/lib/codex-app-server-protocol-source.js"; import { createScriptTestHarness } from "./test-helpers.js"; @@ -26,6 +28,44 @@ afterEach(() => { }); describe("codex app-server protocol source resolver", () => { + it("reads the Cargo workspace package version without matching sibling sections", () => { + expect( + readCargoWorkspacePackageVersion(` +[workspace] +members = [] + +[workspace.package] # shared crate metadata +version = "0.142.5" +edition = "2024" + +[workspace.dependencies] +version = "9.9.9" +`), + ).toBe("0.142.5"); + expect(readCargoWorkspacePackageVersion('[workspace.dependencies]\nversion = "9.9.9"\n')).toBe( + undefined, + ); + }); + + it("rejects a Codex checkout that differs from the pinned package version", async () => { + const repoRoot = createTempDir("openclaw-protocol-version-root-"); + const codexRepo = createTempDir("openclaw-protocol-version-codex-"); + fs.mkdirSync(path.join(repoRoot, "extensions/codex"), { recursive: true }); + fs.mkdirSync(path.join(codexRepo, "codex-rs"), { recursive: true }); + fs.writeFileSync( + path.join(repoRoot, "extensions/codex/package.json"), + JSON.stringify({ dependencies: { "@openai/codex": "0.142.5" } }), + ); + fs.writeFileSync( + path.join(codexRepo, "codex-rs/Cargo.toml"), + '[workspace.package]\nversion = "0.142.4"\n', + ); + + await expect(validateCodexProtocolSourceVersion({ codexRepo, repoRoot })).rejects.toThrow( + /0\.142\.4 does not match @openai\/codex 0\.142\.5/, + ); + }); + it("uses the app-server protocol export binary instead of compiling the full codex cli", () => { expect(buildCodexProtocolExportArgs("/codex/codex-rs/Cargo.toml", "/tmp/protocol")).toEqual([ "run",