fix(acp): pass Codex ACP model thinking overrides

Fix ACP Codex model/thinking override propagation.\n\nThanks @91wan.
This commit is contained in:
91wan
2026-04-26 02:56:03 +08:00
committed by GitHub
parent e9d9726f2d
commit bb2b68b34e
12 changed files with 693 additions and 9 deletions

View File

@@ -1,6 +1,6 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { AcpRuntime } from "../runtime-api.js";
import { AcpxRuntime } from "./runtime.js";
import { AcpxRuntime, __testing } from "./runtime.js";
type TestSessionStore = {
load(sessionId: string): Promise<Record<string, unknown> | undefined>;
@@ -9,6 +9,8 @@ type TestSessionStore = {
const DOCUMENTED_OPENCLAW_BRIDGE_COMMAND =
"env OPENCLAW_HIDE_BANNER=1 OPENCLAW_SUPPRESS_NOTES=1 openclaw acp --url ws://127.0.0.1:18789 --token-file ~/.openclaw/gateway.token --session agent:main:main";
const CODEX_ACP_COMMAND = "npx @zed-industries/codex-acp@^0.11.1";
const CODEX_ACP_WRAPPER_COMMAND = `node "/tmp/openclaw/acpx/codex-acp-wrapper.mjs"`;
function makeRuntime(
baseStore: TestSessionStore,
@@ -20,6 +22,7 @@ function makeRuntime(
close: AcpRuntime["close"];
ensureSession: AcpRuntime["ensureSession"];
getStatus: NonNullable<AcpRuntime["getStatus"]>;
setConfigOption: NonNullable<AcpRuntime["setConfigOption"]>;
isHealthy(): boolean;
probeAvailability(): Promise<void>;
};
@@ -27,6 +30,7 @@ function makeRuntime(
close: AcpRuntime["close"];
ensureSession: AcpRuntime["ensureSession"];
getStatus: NonNullable<AcpRuntime["getStatus"]>;
setConfigOption: NonNullable<AcpRuntime["setConfigOption"]>;
isHealthy(): boolean;
probeAvailability(): Promise<void>;
};
@@ -55,6 +59,7 @@ function makeRuntime(
close: AcpRuntime["close"];
ensureSession: AcpRuntime["ensureSession"];
getStatus: NonNullable<AcpRuntime["getStatus"]>;
setConfigOption: NonNullable<AcpRuntime["setConfigOption"]>;
isHealthy(): boolean;
probeAvailability(): Promise<void>;
};
@@ -66,6 +71,7 @@ function makeRuntime(
close: AcpRuntime["close"];
ensureSession: AcpRuntime["ensureSession"];
getStatus: NonNullable<AcpRuntime["getStatus"]>;
setConfigOption: NonNullable<AcpRuntime["setConfigOption"]>;
isHealthy(): boolean;
probeAvailability(): Promise<void>;
};
@@ -79,6 +85,274 @@ describe("AcpxRuntime fresh reset wrapper", () => {
vi.restoreAllMocks();
});
it("normalizes OpenClaw Codex model ids for ACP startup", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => undefined),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore, {
agentRegistry: {
resolve: (agentName: string) => (agentName === "codex" ? CODEX_ACP_COMMAND : agentName),
list: () => ["codex", "openclaw"],
},
});
const ensure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "codex",
});
await runtime.ensureSession({
sessionKey: "agent:codex:acp:test",
agent: "codex",
mode: "persistent",
model: "openai-codex/gpt-5.4",
});
expect(ensure).toHaveBeenCalledWith(
expect.objectContaining({
model: "gpt-5.4",
}),
);
});
it("leaves Codex ACP startup defaults alone when no model or thinking is provided", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => undefined),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore, {
agentRegistry: {
resolve: (agentName: string) => (agentName === "codex" ? CODEX_ACP_COMMAND : agentName),
list: () => ["codex", "openclaw"],
},
});
const ensure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "codex",
});
await runtime.ensureSession({
sessionKey: "agent:codex:acp:test",
agent: "codex",
mode: "persistent",
});
expect(ensure).toHaveBeenCalledWith(
expect.objectContaining({
agent: "codex",
}),
);
expect(ensure.mock.calls[0]?.[0]).not.toHaveProperty("model");
expect(ensure.mock.calls[0]?.[0]).not.toHaveProperty("thinking");
});
it("does not normalize model startup for non-Codex ACP agents", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => undefined),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore, {
agentRegistry: {
resolve: (agentName: string) => (agentName === "main" ? CODEX_ACP_COMMAND : agentName),
list: () => ["main", "codex", "openclaw"],
},
});
const ensure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
sessionKey: "agent:main:acp:test",
backend: "acpx",
runtimeSessionName: "main",
});
await runtime.ensureSession({
sessionKey: "agent:main:acp:test",
agent: "main",
mode: "persistent",
model: "openai-codex/gpt-5.5",
});
expect(ensure).toHaveBeenCalledWith(
expect.objectContaining({
agent: "main",
model: "openai-codex/gpt-5.5",
}),
);
});
it("injects Codex ACP startup config into the scoped registry", () => {
expect(__testing.isCodexAcpCommand(CODEX_ACP_COMMAND)).toBe(true);
expect(__testing.isCodexAcpCommand(CODEX_ACP_WRAPPER_COMMAND)).toBe(true);
expect(
__testing.appendCodexAcpConfigOverrides(CODEX_ACP_COMMAND, {
model: "gpt-5.4",
reasoningEffort: "medium",
}),
).toBe(
"npx @zed-industries/codex-acp@^0.11.1 -c model=gpt-5.4 -c model_reasoning_effort=medium",
);
expect(__testing.isCodexAcpCommand("openclaw acp")).toBe(false);
});
it("passes gpt-5.5 Codex ACP startup through instead of blocking it", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => undefined),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore, {
agentRegistry: {
resolve: (agentName: string) => (agentName === "codex" ? CODEX_ACP_COMMAND : agentName),
list: () => ["codex", "openclaw"],
},
});
const ensure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "codex",
});
await runtime.ensureSession({
sessionKey: "agent:codex:acp:test",
agent: "codex",
mode: "persistent",
model: "openai-codex/gpt-5.5",
});
expect(ensure).toHaveBeenCalledWith(
expect.objectContaining({
model: "gpt-5.5",
}),
);
});
it("maps explicit Codex ACP thinking to startup reasoning effort", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => undefined),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore, {
agentRegistry: {
resolve: (agentName: string) => (agentName === "codex" ? CODEX_ACP_COMMAND : agentName),
list: () => ["codex", "openclaw"],
},
});
const ensure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "codex",
});
await runtime.ensureSession({
sessionKey: "agent:codex:acp:test",
agent: "codex",
mode: "persistent",
model: "openai-codex/gpt-5.4",
thinking: "x-high",
});
expect(ensure).toHaveBeenCalledWith(
expect.objectContaining({
model: "gpt-5.4/xhigh",
}),
);
});
it("normalizes Codex ACP model config controls to adapter ids", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => ({
acpxRecordId: "agent:codex:acp:test",
agentCommand: CODEX_ACP_COMMAND,
})),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore);
const setConfigOption = vi.spyOn(delegate, "setConfigOption").mockResolvedValue(undefined);
const handle: Parameters<NonNullable<AcpRuntime["setConfigOption"]>>[0]["handle"] = {
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "agent:codex:acp:test",
acpxRecordId: "agent:codex:acp:test",
};
await runtime.setConfigOption({
handle,
key: "model",
value: "openai-codex/gpt-5.4",
});
expect(setConfigOption).toHaveBeenNthCalledWith(1, {
handle,
key: "model",
value: "gpt-5.4",
});
expect(setConfigOption).toHaveBeenCalledOnce();
});
it("normalizes Codex ACP slash reasoning suffixes to config controls", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => ({
acpxRecordId: "agent:codex:acp:test",
agentCommand: CODEX_ACP_COMMAND,
})),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore);
const setConfigOption = vi.spyOn(delegate, "setConfigOption").mockResolvedValue(undefined);
const handle: Parameters<NonNullable<AcpRuntime["setConfigOption"]>>[0]["handle"] = {
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "agent:codex:acp:test",
acpxRecordId: "agent:codex:acp:test",
};
await runtime.setConfigOption({
handle,
key: "model",
value: "openai-codex/gpt-5.4/high",
});
expect(setConfigOption).toHaveBeenNthCalledWith(1, {
handle,
key: "model",
value: "gpt-5.4",
});
expect(setConfigOption).toHaveBeenNthCalledWith(2, {
handle,
key: "reasoning_effort",
value: "high",
});
});
it("normalizes Codex ACP thinking config controls to reasoning effort", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => ({
acpxRecordId: "agent:codex:acp:test",
agentCommand: CODEX_ACP_COMMAND,
})),
save: vi.fn(async () => {}),
};
const { runtime, delegate } = makeRuntime(baseStore);
const setConfigOption = vi.spyOn(delegate, "setConfigOption").mockResolvedValue(undefined);
const handle: Parameters<NonNullable<AcpRuntime["setConfigOption"]>>[0]["handle"] = {
sessionKey: "agent:codex:acp:test",
backend: "acpx",
runtimeSessionName: "agent:codex:acp:test",
acpxRecordId: "agent:codex:acp:test",
};
await runtime.setConfigOption({
handle,
key: "thinking",
value: "minimal",
});
expect(setConfigOption).toHaveBeenCalledWith({
handle,
key: "reasoning_effort",
value: "low",
});
});
it("keeps stale persistent loads hidden until a fresh record is saved", async () => {
const baseStore: TestSessionStore = {
load: vi.fn(async () => ({ acpxRecordId: "stale" }) as never),

View File

@@ -1,3 +1,4 @@
import { AsyncLocalStorage } from "node:async_hooks";
import {
ACPX_BACKEND_ID,
AcpxRuntime as BaseAcpxRuntime,
@@ -13,7 +14,7 @@ import {
type AcpRuntimeOptions,
type AcpRuntimeStatus,
} from "acpx/runtime";
import type { AcpRuntime } from "../runtime-api.js";
import { AcpRuntimeError, type AcpRuntime } from "../runtime-api.js";
type AcpSessionStore = AcpRuntimeOptions["sessionStore"];
type AcpSessionRecord = Parameters<AcpSessionStore["save"]>[0];
@@ -60,6 +61,27 @@ function createResetAwareSessionStore(baseStore: AcpSessionStore): ResetAwareSes
const OPENCLAW_BRIDGE_EXECUTABLE = "openclaw";
const OPENCLAW_BRIDGE_SUBCOMMAND = "acp";
const CODEX_ACP_AGENT_ID = "codex";
const CODEX_ACP_OPENCLAW_PREFIX = "openai-codex/";
const CODEX_ACP_REASONING_EFFORTS = new Set(["low", "medium", "high", "xhigh"]);
const CODEX_ACP_THINKING_ALIASES = new Map<string, string | undefined>([
["off", undefined],
["minimal", "low"],
["low", "low"],
["medium", "medium"],
["high", "high"],
["x-high", "xhigh"],
["x_high", "xhigh"],
["extra-high", "xhigh"],
["extra_high", "xhigh"],
["extra high", "xhigh"],
["xhigh", "xhigh"],
]);
type CodexAcpModelOverride = {
model?: string;
reasoningEffort?: string;
};
function normalizeAgentName(value: string | undefined): string | undefined {
const normalized = value?.trim().toLowerCase();
@@ -175,6 +197,149 @@ function isOpenClawBridgeCommand(command: string | undefined): boolean {
return /^openclaw(?:\.[cm]?js)?$/i.test(scriptName) && parts[2] === OPENCLAW_BRIDGE_SUBCOMMAND;
}
function isCodexAcpPackageSpec(value: string): boolean {
return /^@zed-industries\/codex-acp(?:@.+)?$/i.test(value.trim());
}
function isCodexAcpCommand(command: string | undefined): boolean {
if (!command) {
return false;
}
const parts = unwrapEnvCommand(splitCommandParts(command.trim()));
if (!parts.length) {
return false;
}
if (parts.some(isCodexAcpPackageSpec)) {
return true;
}
const commandName = basename(parts[0] ?? "");
if (/^codex-acp(?:\.exe)?$/i.test(commandName)) {
return true;
}
if (commandName !== "node") {
return false;
}
const scriptName = basename(parts[1] ?? "");
return /^codex-acp(?:-wrapper)?(?:\.[cm]?js)?$/i.test(scriptName);
}
function failUnsupportedCodexAcpModel(rawModel: string, detail?: string): never {
throw new AcpRuntimeError(
"ACP_INVALID_RUNTIME_OPTION",
detail ??
`Codex ACP model "${rawModel}" is not supported. Use openai-codex/<model> or <model>/<reasoning-effort>.`,
);
}
function failUnsupportedCodexAcpThinking(rawThinking: string): never {
throw new AcpRuntimeError(
"ACP_INVALID_RUNTIME_OPTION",
`Codex ACP thinking level "${rawThinking}" is not supported. Use off, minimal, low, medium, high, or xhigh.`,
);
}
function normalizeCodexAcpReasoningEffort(rawThinking: string | undefined): string | undefined {
const normalized = rawThinking?.trim().toLowerCase();
if (!normalized) {
return undefined;
}
if (!CODEX_ACP_THINKING_ALIASES.has(normalized)) {
failUnsupportedCodexAcpThinking(rawThinking ?? "");
}
return CODEX_ACP_THINKING_ALIASES.get(normalized);
}
function normalizeCodexAcpModelOverride(
rawModel: string | undefined,
rawThinking?: string,
): CodexAcpModelOverride | undefined {
const raw = rawModel?.trim();
const thinkingReasoningEffort = normalizeCodexAcpReasoningEffort(rawThinking);
if (!raw) {
return thinkingReasoningEffort ? { reasoningEffort: thinkingReasoningEffort } : undefined;
}
let value = raw;
if (value.toLowerCase().startsWith(CODEX_ACP_OPENCLAW_PREFIX)) {
value = value.slice(CODEX_ACP_OPENCLAW_PREFIX.length);
}
const parts = value.split("/");
if (parts.length > 2) {
failUnsupportedCodexAcpModel(
raw,
`Codex ACP model "${raw}" is not supported. Use openai-codex/<model> or <model>/<reasoning-effort>.`,
);
}
const model = (parts[0] ?? "").trim();
const modelReasoningEffort = normalizeCodexAcpReasoningEffort(parts[1]);
if (!model) {
failUnsupportedCodexAcpModel(
raw,
`Codex ACP model "${raw}" is not supported. Use openai-codex/<model> or <model>/<reasoning-effort>.`,
);
}
const reasoningEffort = thinkingReasoningEffort ?? modelReasoningEffort;
if (reasoningEffort && !CODEX_ACP_REASONING_EFFORTS.has(reasoningEffort)) {
failUnsupportedCodexAcpThinking(reasoningEffort);
}
return {
model,
...(reasoningEffort ? { reasoningEffort } : {}),
};
}
function codexAcpSessionModelId(override: CodexAcpModelOverride): string {
if (!override.model) {
return "";
}
return override.reasoningEffort
? `${override.model}/${override.reasoningEffort}`
: override.model;
}
function quoteShellArg(value: string): string {
if (/^[A-Za-z0-9_./:=@+-]+$/.test(value)) {
return value;
}
return `'${value.replace(/'/g, "'\\''")}'`;
}
function appendCodexAcpConfigOverrides(command: string, override: CodexAcpModelOverride): string {
const configArgs = override.model ? [`model=${override.model}`] : [];
if (override.reasoningEffort) {
configArgs.push(`model_reasoning_effort=${override.reasoningEffort}`);
}
if (configArgs.length === 0) {
return command;
}
return `${command} ${configArgs.map((arg) => `-c ${quoteShellArg(arg)}`).join(" ")}`;
}
function createModelScopedAgentRegistry(params: {
agentRegistry: AcpAgentRegistry;
scope: AsyncLocalStorage<CodexAcpModelOverride | undefined>;
}): AcpAgentRegistry {
return {
resolve(agentName: string): string | undefined {
const command = params.agentRegistry.resolve(agentName);
const override = params.scope.getStore();
if (
!override ||
normalizeAgentName(agentName) !== CODEX_ACP_AGENT_ID ||
typeof command !== "string" ||
!isCodexAcpCommand(command)
) {
return command;
}
return appendCodexAcpConfigOverrides(command, override);
},
list(): string[] {
return params.agentRegistry.list();
},
};
}
function resolveAgentCommand(params: {
agentName: string | undefined;
agentRegistry: AcpAgentRegistry;
@@ -211,6 +376,10 @@ function shouldUseDistinctBridgeDelegate(options: AcpRuntimeOptions): boolean {
export class AcpxRuntime implements AcpRuntime {
private readonly sessionStore: ResetAwareSessionStore;
private readonly agentRegistry: AcpAgentRegistry;
private readonly scopedAgentRegistry: AcpAgentRegistry;
private readonly codexAcpModelOverrideScope = new AsyncLocalStorage<
CodexAcpModelOverride | undefined
>();
private readonly delegate: BaseAcpxRuntime;
private readonly bridgeSafeDelegate: BaseAcpxRuntime;
private readonly probeDelegate: BaseAcpxRuntime;
@@ -221,9 +390,14 @@ export class AcpxRuntime implements AcpRuntime {
) {
this.sessionStore = createResetAwareSessionStore(options.sessionStore);
this.agentRegistry = options.agentRegistry;
this.scopedAgentRegistry = createModelScopedAgentRegistry({
agentRegistry: this.agentRegistry,
scope: this.codexAcpModelOverrideScope,
});
const sharedOptions = {
...options,
sessionStore: this.sessionStore,
agentRegistry: this.scopedAgentRegistry,
};
this.delegate = new BaseAcpxRuntime(sharedOptions, testOptions);
this.bridgeSafeDelegate = shouldUseDistinctBridgeDelegate(options)
@@ -259,6 +433,18 @@ export class AcpxRuntime implements AcpRuntime {
return this.resolveDelegateForAgent(readAgentFromHandle(handle));
}
private async resolveCommandForHandle(handle: AcpRuntimeHandle): Promise<string | undefined> {
const record = await this.sessionStore.load(handle.acpxRecordId ?? handle.sessionKey);
const recordCommand = readAgentCommandFromRecord(record);
if (recordCommand) {
return recordCommand;
}
return resolveAgentCommandForName({
agentName: readAgentFromHandle(handle),
agentRegistry: this.agentRegistry,
});
}
isHealthy(): boolean {
return this.probeDelegate.isHealthy();
}
@@ -271,8 +457,32 @@ export class AcpxRuntime implements AcpRuntime {
return this.probeDelegate.doctor();
}
ensureSession(input: Parameters<AcpRuntime["ensureSession"]>[0]): Promise<AcpRuntimeHandle> {
return this.resolveDelegateForAgent(input.agent).ensureSession(input);
async ensureSession(
input: Parameters<AcpRuntime["ensureSession"]>[0],
): Promise<AcpRuntimeHandle> {
const command = resolveAgentCommandForName({
agentName: input.agent,
agentRegistry: this.agentRegistry,
});
const delegate = this.resolveDelegateForCommand(command);
const codexModelOverride =
normalizeAgentName(input.agent) === CODEX_ACP_AGENT_ID && isCodexAcpCommand(command)
? normalizeCodexAcpModelOverride(input.model, input.thinking)
: undefined;
if (!codexModelOverride) {
return delegate.ensureSession(input);
}
const normalizedInput = {
...input,
...(codexAcpSessionModelId(codexModelOverride)
? { model: codexAcpSessionModelId(codexModelOverride) }
: {}),
};
return this.codexAcpModelOverrideScope.run(codexModelOverride, () =>
delegate.ensureSession(normalizedInput),
);
}
async *runTurn(input: Parameters<AcpRuntime["runTurn"]>[0]): AsyncIterable<AcpRuntimeEvent> {
@@ -299,6 +509,39 @@ export class AcpxRuntime implements AcpRuntime {
input: Parameters<NonNullable<AcpRuntime["setConfigOption"]>>[0],
): Promise<void> {
const delegate = await this.resolveDelegateForHandle(input.handle);
const command = await this.resolveCommandForHandle(input.handle);
if (
(input.key === "model" ||
input.key === "thinking" ||
input.key === "thought_level" ||
input.key === "reasoning_effort") &&
isCodexAcpCommand(command)
) {
const override =
input.key === "model"
? normalizeCodexAcpModelOverride(input.value)
: normalizeCodexAcpModelOverride(undefined, input.value);
if (!override && input.key !== "model") {
return;
}
if (override) {
if (override.model) {
await delegate.setConfigOption({
...input,
key: "model",
value: override.model,
});
}
if (override.reasoningEffort) {
await delegate.setConfigOption({
...input,
key: "reasoning_effort",
value: override.reasoningEffort,
});
}
return;
}
}
await delegate.setConfigOption(input);
}
@@ -334,4 +577,11 @@ export {
encodeAcpxRuntimeHandleState,
};
export const __testing = {
appendCodexAcpConfigOverrides,
codexAcpSessionModelId,
isCodexAcpCommand,
normalizeCodexAcpModelOverride,
};
export type { AcpAgentRegistry, AcpRuntimeOptions, AcpSessionRecord, AcpSessionStore };