From 6336ed4166c5fe35d3e8e49e70320f66ba92575d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 26 Apr 2026 04:32:06 +0100 Subject: [PATCH] fix: gate codex acp route hints --- CHANGELOG.md | 1 + docs/gateway/doctor.md | 12 +- docs/plugins/codex-harness.md | 5 + docs/providers/openai.md | 2 + src/agents/skills/plugin-skills.test.ts | 42 ++++++- src/agents/skills/plugin-skills.ts | 7 +- src/agents/subagent-system-prompt.ts | 4 +- src/agents/system-prompt.test.ts | 18 +++ src/agents/system-prompt.ts | 2 +- .../shared/codex-route-warnings.test.ts | 83 +++++++++++++ .../doctor/shared/codex-route-warnings.ts | 112 ++++++++++++++++++ .../doctor/shared/preview-warnings.ts | 3 + 12 files changed, 282 insertions(+), 9 deletions(-) create mode 100644 src/commands/doctor/shared/codex-route-warnings.test.ts create mode 100644 src/commands/doctor/shared/codex-route-warnings.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 15c4d7e14ef..2ce7126696a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,7 @@ Docs: https://docs.openclaw.ai - Browser automation: keep stable tab ids and labels attached when Chromium replaces the raw target after form submissions or other action-triggered navigations, and return the replacement `targetId` from `/act` when the match is provable. Fixes #46137. - QQ Bot: make `qqbot_remind` schedule, list, and remove Gateway cron jobs directly for owner-authorized senders instead of returning `cronParams` and relying on a follow-up generic `cron` tool call. Fixes #70865. (#70937) Thanks @GaosCode. - Agents/ACP: hide `sessions_spawn` ACP runtime options unless an ACP backend is loaded, and make `/acp doctor` call out `plugins.allow` blocking bundled `acpx`. Thanks @vincentkoc. +- Agents/Codex: keep ACP prompt/skill routing hidden unless an ACP runtime backend is available, and warn in doctor when enabled Codex plugin configs still route `openai-codex/*` models through PI. Thanks @vincentkoc. - Media delivery: avoid sending generated image attachments twice when the assistant reply already includes explicit `MEDIA:` lines for the same turn, and reject unsafe remote `MEDIA:` URLs before delivery. Thanks @pashpashpash. - Codex harness: ignore retryable app-server error notifications after Codex recovers, and preserve the real nested error message for terminal app-server failures instead of replacing it with a generic failure. Thanks @pashpashpash. - Agents/Codex: prepare native Codex sub-agent session metadata without a diff --git a/docs/gateway/doctor.md b/docs/gateway/doctor.md index a120c319f14..3dc59c5f380 100644 --- a/docs/gateway/doctor.md +++ b/docs/gateway/doctor.md @@ -249,7 +249,7 @@ doctor prints platform-specific fix guidance. On macOS with a Homebrew Node, the fix is usually `brew postinstall ca-certificates`. With `--deep`, the probe runs even if the gateway is healthy. -### 2c) Codex OAuth provider overrides +### 2e) Codex OAuth provider overrides If you previously added legacy OpenAI transport settings under `models.providers.openai-codex`, they can shadow the built-in Codex OAuth @@ -259,6 +259,16 @@ the stale transport override and get the built-in routing/fallback behavior back. Custom proxies and header-only overrides are still supported and do not trigger this warning. +### 2f) Codex plugin route warnings + +When the bundled Codex plugin is enabled, doctor also checks whether +`openai-codex/*` primary model refs still resolve through the default PI runner. +That combination is valid when you want Codex OAuth/subscription auth through +PI, but it is easy to confuse with the native Codex app-server harness. Doctor +warns and points to the explicit app-server shape: +`openai/*` plus `embeddedHarness.runtime: "codex"` or +`OPENCLAW_AGENT_RUNTIME=codex`. + ### 3) Legacy state migrations (disk layout) Doctor can migrate older on-disk layouts into the current structure: diff --git a/docs/plugins/codex-harness.md b/docs/plugins/codex-harness.md index 998efdd5cf4..1c94739209e 100644 --- a/docs/plugins/codex-harness.md +++ b/docs/plugins/codex-harness.md @@ -53,6 +53,11 @@ want native app-server execution. Legacy `codex/*` model refs still auto-select the harness for compatibility, but runtime-backed legacy provider prefixes are not shown as normal model/provider choices. +If the `codex` plugin is enabled but the primary model is still +`openai-codex/*`, `openclaw doctor` warns instead of changing the route. That is +intentional: `openai-codex/*` remains the PI Codex OAuth/subscription path, and +native app-server execution stays an explicit runtime choice. + ## Pick the right model prefix OpenAI-family routes are prefix-specific. Use `openai-codex/*` when you want diff --git a/docs/providers/openai.md b/docs/providers/openai.md index 95036c1adc0..2b49c3fb810 100644 --- a/docs/providers/openai.md +++ b/docs/providers/openai.md @@ -42,6 +42,8 @@ Enabling the OpenAI plugin, or selecting an `openai-codex/*` model, does not enable the bundled Codex app-server plugin. OpenClaw enables that plugin only when you explicitly select the native Codex harness with `embeddedHarness.runtime: "codex"` or use a legacy `codex/*` model ref. +If the bundled `codex` plugin is enabled but `openai-codex/*` still resolves +through PI, `openclaw doctor` warns and leaves the route unchanged. ## OpenClaw feature coverage diff --git a/src/agents/skills/plugin-skills.test.ts b/src/agents/skills/plugin-skills.test.ts index 0fdd1757fed..8e965d3abd3 100644 --- a/src/agents/skills/plugin-skills.test.ts +++ b/src/agents/skills/plugin-skills.test.ts @@ -1,6 +1,10 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { + __testing as acpRuntimeTesting, + registerAcpRuntimeBackend, +} from "../../acp/runtime/registry.js"; import type { OpenClawConfig } from "../../config/config.js"; import type { PluginManifestRegistry } from "../../plugins/manifest-registry.js"; import { createTrackedTempDirs } from "../../test-utils/tracked-temp-dirs.js"; @@ -109,9 +113,30 @@ async function setupPluginOutsideSkills() { return { workspaceDir, pluginRoot, outsideSkills }; } +function registerHealthyAcpBackend() { + registerAcpRuntimeBackend({ + id: "acpx", + runtime: { + async ensureSession(input) { + return { + sessionKey: input.sessionKey, + backend: "acpx", + runtimeSessionName: input.sessionKey, + }; + }, + async *runTurn() { + yield { type: "done" as const }; + }, + async cancel() {}, + async close() {}, + }, + }); +} + afterEach(async () => { hoisted.loadPluginManifestRegistryForInstalledIndex.mockReset(); hoisted.loadPluginRegistrySnapshot.mockReset(); + acpRuntimeTesting.resetAcpRuntimeBackendsForTests(); await tempDirs.cleanup(); }); @@ -132,8 +157,9 @@ describe("resolvePluginSkillDirs", () => { it.each([ { - name: "keeps acpx plugin skills when ACP is enabled", + name: "keeps acpx plugin skills when ACP runtime is available", acpEnabled: true, + backendAvailable: true, expectedDirs: ({ acpxRoot, helperRoot }: { acpxRoot: string; helperRoot: string }) => [ path.resolve(acpxRoot, "skills"), path.resolve(helperRoot, "skills"), @@ -142,12 +168,24 @@ describe("resolvePluginSkillDirs", () => { { name: "skips acpx plugin skills when ACP is disabled", acpEnabled: false, + backendAvailable: true, expectedDirs: ({ helperRoot }: { acpxRoot: string; helperRoot: string }) => [ path.resolve(helperRoot, "skills"), ], }, - ])("$name", async ({ acpEnabled, expectedDirs }) => { + { + name: "skips acpx plugin skills when no ACP runtime backend is loaded", + acpEnabled: true, + backendAvailable: false, + expectedDirs: ({ helperRoot }: { acpxRoot: string; helperRoot: string }) => [ + path.resolve(helperRoot, "skills"), + ], + }, + ])("$name", async ({ acpEnabled, backendAvailable, expectedDirs }) => { const { workspaceDir, acpxRoot, helperRoot } = await setupAcpxAndHelperRegistry(); + if (backendAvailable) { + registerHealthyAcpBackend(); + } const dirs = resolvePluginSkillDirs({ workspaceDir, diff --git a/src/agents/skills/plugin-skills.ts b/src/agents/skills/plugin-skills.ts index 68cae6d570e..66f92b0b6a1 100644 --- a/src/agents/skills/plugin-skills.ts +++ b/src/agents/skills/plugin-skills.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import { isAcpRuntimeSpawnAvailable } from "../../acp/runtime/availability.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { @@ -61,7 +62,7 @@ export function resolvePluginSkillDirs(params: { params.config?.plugins, createRegistryPluginIdNormalizer(registry), ); - const acpEnabled = params.config?.acp?.enabled !== false; + const acpRuntimeAvailable = isAcpRuntimeSpawnAvailable({ config: params.config }); const memorySlot = normalizedPlugins.slots.memory; let selectedMemoryPluginId: string | null = null; const seen = new Set(); @@ -81,8 +82,8 @@ export function resolvePluginSkillDirs(params: { if (!activationState.activated) { continue; } - // ACP router skills should not be attached when ACP is explicitly disabled. - if (!acpEnabled && record.id === "acpx") { + // ACP router skills should not be attached unless ACP can actually spawn. + if (!acpRuntimeAvailable && record.id === "acpx") { continue; } const memoryDecision = resolveMemorySlotDecision({ diff --git a/src/agents/subagent-system-prompt.ts b/src/agents/subagent-system-prompt.ts index 8ca92350356..d1a1b3f79f9 100644 --- a/src/agents/subagent-system-prompt.ts +++ b/src/agents/subagent-system-prompt.ts @@ -7,7 +7,7 @@ export function buildSubagentSystemPrompt(params: { childSessionKey: string; label?: string; task?: string; - /** Whether ACP-specific routing guidance should be included. Defaults to true. */ + /** Whether ACP-specific routing guidance should be included. Defaults to false. */ acpEnabled?: boolean; /** Registered runtime slash/native command names such as `codex`. */ nativeCommandNames?: string[]; @@ -25,7 +25,7 @@ export function buildSubagentSystemPrompt(params: { typeof params.maxSpawnDepth === "number" ? params.maxSpawnDepth : DEFAULT_SUBAGENT_MAX_SPAWN_DEPTH; - const acpEnabled = params.acpEnabled !== false; + const acpEnabled = params.acpEnabled === true; const nativeCodexCommandAvailable = (params.nativeCommandNames ?? []).some( (name) => name.trim().replace(/^\/+/, "").toLowerCase() === "codex", ); diff --git a/src/agents/system-prompt.test.ts b/src/agents/system-prompt.test.ts index 41c24574985..35c56877dc1 100644 --- a/src/agents/system-prompt.test.ts +++ b/src/agents/system-prompt.test.ts @@ -304,6 +304,7 @@ describe("buildAgentSystemPrompt", () => { const prompt = buildAgentSystemPrompt({ workspaceDir: "/tmp/openclaw", toolNames: ["sessions_spawn"], + acpEnabled: true, }); expect(prompt).toContain("sessions_spawn"); @@ -318,6 +319,7 @@ describe("buildAgentSystemPrompt", () => { workspaceDir: "/tmp/openclaw", toolNames: ["sessions_spawn", "subagents", "agents_list", "exec"], nativeCommandNames: ["codex"], + acpEnabled: true, }); expect(prompt).toContain("Native Codex app-server plugin is available"); @@ -358,6 +360,7 @@ describe("buildAgentSystemPrompt", () => { const prompt = buildAgentSystemPrompt({ workspaceDir: "/tmp/openclaw", toolNames: ["sessions_spawn", "subagents", "agents_list", "exec"], + acpEnabled: true, sandboxInfo: { enabled: true, }, @@ -935,6 +938,7 @@ describe("buildSubagentSystemPrompt", () => { task: "research task", childDepth: 1, maxSpawnDepth: 2, + acpEnabled: true, }); expect(prompt).toContain("## Sub-Agent Spawning"); @@ -981,6 +985,19 @@ describe("buildSubagentSystemPrompt", () => { expect(prompt).toContain("You CAN spawn your own sub-agents"); }); + it("omits ACP spawning guidance by default", () => { + const prompt = buildSubagentSystemPrompt({ + childSessionKey: "agent:main:subagent:abc", + task: "research task", + childDepth: 1, + maxSpawnDepth: 2, + }); + + expect(prompt).not.toContain('runtime: "acp"'); + expect(prompt).not.toContain("For ACP harness sessions (claudecode/gemini/opencode"); + expect(prompt).toContain("You CAN spawn your own sub-agents"); + }); + it("prefers native Codex commands over Codex ACP when available", () => { const prompt = buildSubagentSystemPrompt({ childSessionKey: "agent:main:subagent:abc", @@ -988,6 +1005,7 @@ describe("buildSubagentSystemPrompt", () => { childDepth: 1, maxSpawnDepth: 2, nativeCommandNames: ["codex"], + acpEnabled: true, }); expect(prompt).toContain("Native Codex app-server plugin is available"); diff --git a/src/agents/system-prompt.ts b/src/agents/system-prompt.ts index e53b1798746..315152ab832 100644 --- a/src/agents/system-prompt.ts +++ b/src/agents/system-prompt.ts @@ -496,7 +496,7 @@ export function buildAgentSystemPrompt(params: { memoryCitationsMode?: MemoryCitationsMode; promptContribution?: ProviderSystemPromptContribution; }) { - const acpEnabled = params.acpEnabled !== false; + const acpEnabled = params.acpEnabled === true; const sandboxedRuntime = params.sandboxInfo?.enabled === true; const acpSpawnRuntimeEnabled = acpEnabled && !sandboxedRuntime; const coreToolSummaries: Record = { diff --git a/src/commands/doctor/shared/codex-route-warnings.test.ts b/src/commands/doctor/shared/codex-route-warnings.test.ts new file mode 100644 index 00000000000..cf4dc42966f --- /dev/null +++ b/src/commands/doctor/shared/codex-route-warnings.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../../config/types.openclaw.js"; +import { collectCodexRouteWarnings } from "./codex-route-warnings.js"; + +function codexPluginConfig(): Pick { + return { + plugins: { + entries: { + codex: { enabled: true }, + }, + }, + } as Pick; +} + +describe("collectCodexRouteWarnings", () => { + it("warns when the Codex plugin is enabled but openai-codex models still route through PI", () => { + const warnings = collectCodexRouteWarnings({ + cfg: { + ...codexPluginConfig(), + agents: { + defaults: { + model: "openai-codex/gpt-5.5", + }, + }, + } as OpenClawConfig, + }); + + expect(warnings).toEqual([expect.stringContaining("Codex plugin is enabled")]); + expect(warnings[0]).toContain("agents.defaults.model"); + expect(warnings[0]).toContain('runtime "pi"'); + expect(warnings[0]).toContain('embeddedHarness.runtime: "codex"'); + }); + + it("does not warn when the native Codex runtime is selected", () => { + const warnings = collectCodexRouteWarnings({ + cfg: { + ...codexPluginConfig(), + agents: { + defaults: { + model: "openai-codex/gpt-5.5", + embeddedHarness: { + runtime: "codex", + }, + }, + }, + } as OpenClawConfig, + }); + + expect(warnings).toEqual([]); + }); + + it("does not warn when OPENCLAW_AGENT_RUNTIME selects native Codex", () => { + const warnings = collectCodexRouteWarnings({ + cfg: { + ...codexPluginConfig(), + agents: { + defaults: { + model: "openai-codex/gpt-5.5", + }, + }, + } as OpenClawConfig, + env: { + OPENCLAW_AGENT_RUNTIME: "codex", + }, + }); + + expect(warnings).toEqual([]); + }); + + it("does not warn unless the Codex plugin is explicitly enabled or allowed", () => { + const warnings = collectCodexRouteWarnings({ + cfg: { + agents: { + defaults: { + model: "openai-codex/gpt-5.5", + }, + }, + } as OpenClawConfig, + }); + + expect(warnings).toEqual([]); + }); +}); diff --git a/src/commands/doctor/shared/codex-route-warnings.ts b/src/commands/doctor/shared/codex-route-warnings.ts new file mode 100644 index 00000000000..f4b904508dc --- /dev/null +++ b/src/commands/doctor/shared/codex-route-warnings.ts @@ -0,0 +1,112 @@ +import type { + AgentEmbeddedHarnessConfig, + AgentModelConfig, +} from "../../../config/types.agents-shared.js"; +import type { OpenClawConfig } from "../../../config/types.openclaw.js"; + +type CodexPiRouteHit = { + path: string; + model: string; + runtime: string; +}; + +function normalizeString(value: unknown): string | undefined { + return typeof value === "string" && value.trim() ? value.trim().toLowerCase() : undefined; +} + +function normalizeModelRef(model: AgentModelConfig | undefined): string | undefined { + if (typeof model === "string") { + return model.trim() || undefined; + } + return typeof model?.primary === "string" && model.primary.trim() + ? model.primary.trim() + : undefined; +} + +function isOpenAICodexModelRef(model: string | undefined): model is string { + return normalizeString(model)?.startsWith("openai-codex/") === true; +} + +function isCodexPluginEnabled(cfg: OpenClawConfig): boolean { + const plugins = cfg.plugins; + if (plugins?.enabled === false) { + return false; + } + const allow = plugins?.allow; + if (Array.isArray(allow) && !allow.map((entry) => normalizeString(entry)).includes("codex")) { + return false; + } + return ( + plugins?.entries?.codex?.enabled === true || + (Array.isArray(allow) && allow.map((entry) => normalizeString(entry)).includes("codex")) + ); +} + +function resolveRuntime(params: { + env?: NodeJS.ProcessEnv; + agentHarness?: AgentEmbeddedHarnessConfig; + defaultsHarness?: AgentEmbeddedHarnessConfig; +}): string { + return ( + normalizeString(params.env?.OPENCLAW_AGENT_RUNTIME) ?? + normalizeString(params.agentHarness?.runtime) ?? + normalizeString(params.defaultsHarness?.runtime) ?? + "pi" + ); +} + +function collectOpenAICodexPiRouteHits( + cfg: OpenClawConfig, + env?: NodeJS.ProcessEnv, +): CodexPiRouteHit[] { + const defaults = cfg.agents?.defaults; + const defaultsHarness = defaults?.embeddedHarness; + const hits: CodexPiRouteHit[] = []; + const defaultModel = normalizeModelRef(defaults?.model); + const defaultRuntime = resolveRuntime({ env, defaultsHarness }); + if (isOpenAICodexModelRef(defaultModel) && defaultRuntime !== "codex") { + hits.push({ path: "agents.defaults.model", model: defaultModel, runtime: defaultRuntime }); + } + + for (const agent of cfg.agents?.list ?? []) { + const model = normalizeModelRef(agent.model); + if (!isOpenAICodexModelRef(model)) { + continue; + } + const runtime = resolveRuntime({ + env, + agentHarness: agent.embeddedHarness, + defaultsHarness, + }); + if (runtime === "codex") { + continue; + } + const id = typeof agent.id === "string" && agent.id.trim() ? agent.id.trim() : ""; + hits.push({ path: `agents.list.${id}.model`, model, runtime }); + } + + return hits; +} + +export function collectCodexRouteWarnings(params: { + cfg: OpenClawConfig; + env?: NodeJS.ProcessEnv; +}): string[] { + if (!isCodexPluginEnabled(params.cfg)) { + return []; + } + const hits = collectOpenAICodexPiRouteHits(params.cfg, params.env); + if (hits.length === 0) { + return []; + } + return [ + [ + "- Codex plugin is enabled, but `openai-codex/*` model refs still use the OpenClaw PI runner unless `embeddedHarness.runtime` is `codex`.", + ...hits.map( + (hit) => `- ${hit.path}: ${hit.model} currently resolves with runtime "${hit.runtime}".`, + ), + '- To use native Codex app-server, set the model to `openai/` and set `agents.defaults.embeddedHarness.runtime: "codex"` (or the agent-level equivalent).', + "- Leave this unchanged if you intentionally want Codex OAuth/subscription auth through PI.", + ].join("\n"), + ]; +} diff --git a/src/commands/doctor/shared/preview-warnings.ts b/src/commands/doctor/shared/preview-warnings.ts index 8899304ec64..7329a548607 100644 --- a/src/commands/doctor/shared/preview-warnings.ts +++ b/src/commands/doctor/shared/preview-warnings.ts @@ -139,6 +139,9 @@ export async function collectDoctorPreviewWarnings(params: { }).join("\n"), ); } + + const { collectCodexRouteWarnings } = await import("./codex-route-warnings.js"); + warnings.push(...collectCodexRouteWarnings({ cfg: params.cfg, env })); } if (hasPluginLoadPaths(params.cfg)) {