mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(acpx): keep leaked non-openai model out of the Codex ACP thinking slot (#95852)
* fix(acpx): keep leaked non-openai model out of the Codex ACP thinking slot Codex ACP spawn mis-routed an inherited non-OpenAI fleet default into the reasoning-effort slot and aborted (#95780). Replace the splitter with a closed classifier and make the spawn path provenance-aware: drop an inherited leaked default so Codex starts on its own default, but fail closed with ACP_INVALID_RUNTIME_OPTION when a caller explicitly selects an unsupported or malformed model. Thread a modelExplicit flag from resolveAcpSpawnRuntimeOptions through the ACP runtime ensure contract; strip it before the acpx delegate. Dropping the inherited default only at ensureSession was not enough: the manager still persisted the leaked model in runtimeOptions, and the first turn replayed it through applyRuntimeControls -> setConfigOption(model), which the new fail-closed Codex control path then rejected. ensureSession now reports the effective model it applied on the returned handle (applied | dropped), and the manager persists that effective model, so a dropped inherited default is never saved or replayed as a model control before the first turn. Explicit unsupported selections still fail closed at spawn and never persist. * test(acp): split runtime config validation tests --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -1020,6 +1020,269 @@ describe("AcpxRuntime fresh reset wrapper", () => {
|
||||
).not.toContain("gpt-5.6-sol/medium");
|
||||
});
|
||||
|
||||
it.each<{ model: string | undefined; thinking?: string; override: Record<string, string> }>([
|
||||
{ model: "openai/gpt-5.4", override: { model: "gpt-5.4" } },
|
||||
{
|
||||
model: "openai/gpt-5.4",
|
||||
thinking: "high",
|
||||
override: { model: "gpt-5.4", reasoningEffort: "high" },
|
||||
},
|
||||
{ model: "gpt-5.4/high", override: { model: "gpt-5.4", reasoningEffort: "high" } },
|
||||
{ model: "gpt-5.4", override: { model: "gpt-5.4" } },
|
||||
{ model: undefined, thinking: "low", override: { reasoningEffort: "low" } },
|
||||
{ model: "", override: {} },
|
||||
])(
|
||||
"classifies supported Codex ACP model request ($model, $thinking) as an override",
|
||||
({ model, thinking, override }) => {
|
||||
expect(testing.classifyCodexAcpModelRequest(model, thinking)).toEqual({
|
||||
kind: "override",
|
||||
override,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each<{ model: string; thinking?: string; expected: Record<string, unknown> }>([
|
||||
{ model: "google/gemini-3.1-flash-lite", expected: { kind: "unsupported" } },
|
||||
{
|
||||
model: "google/gemini-3.1-flash-lite",
|
||||
thinking: "low",
|
||||
expected: { kind: "unsupported", thinkingOverride: { reasoningEffort: "low" } },
|
||||
},
|
||||
{ model: "gpt-5.4/ultra", expected: { kind: "unsupported" } },
|
||||
{ model: "/high", expected: { kind: "unsupported" } },
|
||||
])(
|
||||
"classifies unsupported Codex ACP model request ($model, $thinking) without thinking-slot routing",
|
||||
({ model, thinking, expected }) => {
|
||||
expect(testing.classifyCodexAcpModelRequest(model, thinking)).toEqual(expected);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["openai/foo/bar", "openai/", "openai//high"])(
|
||||
"fails closed on malformed openai-qualified Codex ACP model request %s",
|
||||
(model) => {
|
||||
expect(() => testing.classifyCodexAcpModelRequest(model)).toThrow(AcpRuntimeError);
|
||||
},
|
||||
);
|
||||
|
||||
it("fails closed on an unsupported Codex ACP thinking value", () => {
|
||||
expect(() => testing.classifyCodexAcpModelRequest(undefined, "superhigh")).toThrow(
|
||||
AcpRuntimeError,
|
||||
);
|
||||
});
|
||||
|
||||
it("starts Codex ACP without injecting a leaked non-openai default model", 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: "google/gemini-3.1-flash-lite",
|
||||
});
|
||||
|
||||
const ensureInput = readFirstEnsureSessionInput(ensure);
|
||||
expect(ensureInput).toEqual({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
});
|
||||
expect(ensureInput).not.toHaveProperty("model");
|
||||
expect(ensureInput).not.toHaveProperty("sessionOptions");
|
||||
});
|
||||
|
||||
it("reports a dropped leaked non-openai default on the returned handle", 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"],
|
||||
},
|
||||
});
|
||||
vi.spyOn(delegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "codex",
|
||||
});
|
||||
|
||||
const handle = await runtime.ensureSession({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
model: "google/gemini-3.1-flash-lite",
|
||||
});
|
||||
|
||||
expect(handle.appliedModel).toEqual({ kind: "dropped" });
|
||||
});
|
||||
|
||||
it("reports a supported codex model as applied on the returned handle", 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"],
|
||||
},
|
||||
});
|
||||
vi.spyOn(delegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "codex",
|
||||
});
|
||||
|
||||
const handle = await runtime.ensureSession({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
model: "openai/gpt-5.5",
|
||||
});
|
||||
|
||||
expect(handle.appliedModel).toEqual({ kind: "applied", model: "openai/gpt-5.5" });
|
||||
});
|
||||
|
||||
it("applies explicit Codex ACP thinking while dropping a leaked non-openai default model", 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: "google/gemini-3.1-flash-lite",
|
||||
thinking: "low",
|
||||
});
|
||||
|
||||
const ensureInput = readFirstEnsureSessionInput(ensure);
|
||||
expect(ensureInput).not.toHaveProperty("model");
|
||||
expect(ensureInput).not.toHaveProperty("sessionOptions");
|
||||
expect(ensureInput).toMatchObject({ thinking: "low" });
|
||||
});
|
||||
|
||||
it("drops a leaked malformed Codex ACP default at spawn instead of failing the session", 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: "gpt-5.4/ultra",
|
||||
});
|
||||
|
||||
const ensureInput = readFirstEnsureSessionInput(ensure);
|
||||
expect(ensureInput).not.toHaveProperty("model");
|
||||
expect(ensureInput).not.toHaveProperty("sessionOptions");
|
||||
});
|
||||
|
||||
it.each(["google/gemini-3.1-flash-lite", "gpt-5.4/ultra"])(
|
||||
"fails closed on an explicit unsupported Codex ACP spawn model %s without calling the delegate",
|
||||
async (model) => {
|
||||
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 expect(
|
||||
runtime.ensureSession({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
model,
|
||||
modelExplicit: true,
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "ACP_INVALID_RUNTIME_OPTION" });
|
||||
expect(ensure).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("passes an explicit supported Codex ACP spawn model through without leaking the provenance flag", 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/gpt-5.5",
|
||||
modelExplicit: true,
|
||||
});
|
||||
|
||||
const ensureInput = readFirstEnsureSessionInput(ensure);
|
||||
expect(ensureInput).not.toHaveProperty("modelExplicit");
|
||||
expect(ensureInput).toMatchObject({
|
||||
model: "gpt-5.5",
|
||||
sessionOptions: { model: "gpt-5.5" },
|
||||
});
|
||||
});
|
||||
|
||||
it("normalizes Codex ACP model config controls to adapter ids", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => ({
|
||||
@@ -1051,6 +1314,32 @@ describe("AcpxRuntime fresh reset wrapper", () => {
|
||||
expect(setConfigOption).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each(["google/gemini-3.1-flash-lite", "gpt-5.4/ultra", "openai/foo/bar"])(
|
||||
"fails closed on Codex ACP model config control %s without re-injecting it",
|
||||
async (value) => {
|
||||
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 expect(runtime.setConfigOption({ handle, key: "model", value })).rejects.toMatchObject({
|
||||
code: "ACP_INVALID_RUNTIME_OPTION",
|
||||
});
|
||||
expect(setConfigOption).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("normalizes Codex ACP slash reasoning suffixes to config controls", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => ({
|
||||
|
||||
+136
-84
@@ -60,6 +60,7 @@ type AcpxRuntimeTestOptions = Record<string, unknown> & {
|
||||
};
|
||||
type OpenClawRuntimeTurnInput = Parameters<NonNullable<AcpRuntime["startTurn"]>>[0];
|
||||
type OpenClawRuntimeEnsureInput = Parameters<AcpRuntime["ensureSession"]>[0];
|
||||
type OpenClawRuntimeHandle = Awaited<ReturnType<AcpRuntime["ensureSession"]>>;
|
||||
type AcpxDelegateEnsureInput = Parameters<BaseAcpxRuntime["ensureSession"]>[0];
|
||||
type AcpxMcpServer = NonNullable<AcpRuntimeOptions["mcpServers"]>[number];
|
||||
|
||||
@@ -340,7 +341,6 @@ const OPENCLAW_BRIDGE_SUBCOMMAND = "acp";
|
||||
const CODEX_ACP_AGENT_ID = "codex";
|
||||
const CODEX_ACP_OPENCLAW_PREFIX = "openai/";
|
||||
const CLAUDE_ACP_OPENCLAW_PREFIX = "anthropic/";
|
||||
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"],
|
||||
@@ -360,6 +360,10 @@ type CodexAcpModelOverride = {
|
||||
reasoningEffort?: string;
|
||||
};
|
||||
|
||||
type CodexAcpModelClassification =
|
||||
| { kind: "override"; override: CodexAcpModelOverride }
|
||||
| { kind: "unsupported"; thinkingOverride?: CodexAcpModelOverride };
|
||||
|
||||
function normalizeAgentName(value: string | undefined): string | undefined {
|
||||
const normalized = value?.trim().toLowerCase();
|
||||
return normalized ? normalized : undefined;
|
||||
@@ -531,46 +535,74 @@ function normalizeCodexAcpReasoningEffort(rawThinking: string | undefined): stri
|
||||
return CODEX_ACP_THINKING_ALIASES.get(normalized);
|
||||
}
|
||||
|
||||
function normalizeCodexAcpModelOverride(
|
||||
function isCodexAcpReasoningEffortAlias(value: string | undefined): boolean {
|
||||
const normalized = value?.trim().toLowerCase();
|
||||
return Boolean(normalized && CODEX_ACP_THINKING_ALIASES.has(normalized));
|
||||
}
|
||||
|
||||
function classifyCodexAcpModelRequest(
|
||||
rawModel: string | undefined,
|
||||
rawThinking?: string,
|
||||
): CodexAcpModelOverride | undefined {
|
||||
): CodexAcpModelClassification {
|
||||
const raw = rawModel?.trim();
|
||||
const thinkingReasoningEffort = normalizeCodexAcpReasoningEffort(rawThinking);
|
||||
|
||||
const thinkingOnlyOverride = thinkingReasoningEffort
|
||||
? { reasoningEffort: thinkingReasoningEffort }
|
||||
: undefined;
|
||||
if (!raw) {
|
||||
return thinkingReasoningEffort ? { reasoningEffort: thinkingReasoningEffort } : undefined;
|
||||
return { kind: "override", override: thinkingOnlyOverride ?? {} };
|
||||
}
|
||||
|
||||
let value = raw;
|
||||
let hadOpenAiQualifier = false;
|
||||
if (value.toLowerCase().startsWith(CODEX_ACP_OPENCLAW_PREFIX)) {
|
||||
value = value.slice(CODEX_ACP_OPENCLAW_PREFIX.length);
|
||||
hadOpenAiQualifier = true;
|
||||
}
|
||||
const parts = value.split("/");
|
||||
if (parts.length > 2) {
|
||||
|
||||
let model = value.trim();
|
||||
let modelReasoningEffort: string | undefined;
|
||||
const slashIndex = value.lastIndexOf("/");
|
||||
if (slashIndex >= 0 && isCodexAcpReasoningEffortAlias(value.slice(slashIndex + 1))) {
|
||||
modelReasoningEffort = normalizeCodexAcpReasoningEffort(value.slice(slashIndex + 1));
|
||||
model = value.slice(0, slashIndex).trim();
|
||||
}
|
||||
|
||||
if (hadOpenAiQualifier && (!model || model.includes("/"))) {
|
||||
failUnsupportedCodexAcpModel(
|
||||
raw,
|
||||
`Codex ACP model "${raw}" is not supported. Use openai/<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/<model> or <model>/<reasoning-effort>.`,
|
||||
);
|
||||
if (!model || model.includes("/")) {
|
||||
return thinkingOnlyOverride
|
||||
? { kind: "unsupported", thinkingOverride: thinkingOnlyOverride }
|
||||
: { kind: "unsupported" };
|
||||
}
|
||||
|
||||
const reasoningEffort = thinkingReasoningEffort ?? modelReasoningEffort;
|
||||
if (reasoningEffort && !CODEX_ACP_REASONING_EFFORTS.has(reasoningEffort)) {
|
||||
failUnsupportedCodexAcpThinking(reasoningEffort);
|
||||
}
|
||||
return {
|
||||
model,
|
||||
...(reasoningEffort ? { reasoningEffort } : {}),
|
||||
kind: "override",
|
||||
override: {
|
||||
model,
|
||||
...(reasoningEffort ? { reasoningEffort } : {}),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function withCodexSessionModel<T extends { model?: string }>(
|
||||
input: T,
|
||||
override: CodexAcpModelOverride | undefined,
|
||||
): T {
|
||||
const next = { ...input };
|
||||
if (override?.model) {
|
||||
next.model = override.model;
|
||||
} else {
|
||||
delete next.model;
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
function normalizeClaudeAcpModelOverride(rawModel: string | undefined): string | undefined {
|
||||
const raw = rawModel?.trim();
|
||||
if (!raw) {
|
||||
@@ -586,8 +618,9 @@ function withAcpxSessionOptions(input: OpenClawRuntimeEnsureInput): AcpxDelegate
|
||||
const existingOptions = (input as { sessionOptions?: SessionAgentOptions }).sessionOptions;
|
||||
const model = input.model?.trim() || existingOptions?.model;
|
||||
const sessionOptions = model ? { ...existingOptions, model } : existingOptions;
|
||||
const { modelExplicit: _modelExplicit, ...rest } = input;
|
||||
return {
|
||||
...input,
|
||||
...rest,
|
||||
...(sessionOptions ? { sessionOptions } : {}),
|
||||
} as AcpxDelegateEnsureInput;
|
||||
}
|
||||
@@ -1066,21 +1099,44 @@ export class AcpxRuntime implements AcpRuntime {
|
||||
|
||||
async ensureSession(
|
||||
input: Parameters<AcpRuntime["ensureSession"]>[0],
|
||||
): Promise<AcpRuntimeHandle> {
|
||||
): Promise<OpenClawRuntimeHandle> {
|
||||
assertSupportedRuntimeSessionMode(input.mode);
|
||||
const command = resolveAgentCommand({
|
||||
agentName: input.agent,
|
||||
agentRegistry: this.agentRegistry,
|
||||
});
|
||||
const delegate = this.resolveDelegateForSession({ command, sessionKey: input.sessionKey });
|
||||
const isCodexAcp =
|
||||
normalizeAgentName(input.agent) === CODEX_ACP_AGENT_ID && isCodexAcpCommand(command);
|
||||
const claudeModelOverride = isClaudeAcpCommand(command)
|
||||
? normalizeClaudeAcpModelOverride(input.model)
|
||||
: undefined;
|
||||
const codexClassification = isCodexAcp
|
||||
? classifyCodexAcpModelRequest(input.model, input.thinking)
|
||||
: undefined;
|
||||
if (codexClassification?.kind === "unsupported" && input.modelExplicit) {
|
||||
failUnsupportedCodexAcpModel(input.model ?? "");
|
||||
}
|
||||
const classifiedCodexOverride =
|
||||
codexClassification?.kind === "override"
|
||||
? codexClassification.override
|
||||
: codexClassification?.thinkingOverride;
|
||||
const codexModelOverride =
|
||||
normalizeAgentName(input.agent) === CODEX_ACP_AGENT_ID && isCodexAcpCommand(command)
|
||||
? normalizeCodexAcpModelOverride(input.model, input.thinking)
|
||||
classifiedCodexOverride && Object.keys(classifiedCodexOverride).length > 0
|
||||
? classifiedCodexOverride
|
||||
: undefined;
|
||||
const ensureInput = claudeModelOverride ? { ...input, model: claudeModelOverride } : input;
|
||||
const requestedModel = input.model?.trim();
|
||||
const appliedModel: OpenClawRuntimeHandle["appliedModel"] =
|
||||
isCodexAcp && requestedModel
|
||||
? codexModelOverride?.model
|
||||
? { kind: "applied", model: requestedModel }
|
||||
: { kind: "dropped" }
|
||||
: undefined;
|
||||
const ensureInput = isCodexAcp
|
||||
? withCodexSessionModel(input, codexModelOverride)
|
||||
: claudeModelOverride
|
||||
? { ...input, model: claudeModelOverride }
|
||||
: input;
|
||||
const stableLaunchCommand =
|
||||
codexModelOverride && command
|
||||
? appendCodexAcpConfigOverrides(command, codexModelOverride)
|
||||
@@ -1093,37 +1149,32 @@ export class AcpxRuntime implements AcpRuntime {
|
||||
resumeSessionId: input.resumeSessionId,
|
||||
}));
|
||||
|
||||
if (!codexModelOverride) {
|
||||
return await this.runWithLaunchLease({
|
||||
sessionKey: ensureInput.sessionKey,
|
||||
command: stableLaunchCommand,
|
||||
enabled: shouldStartWithLease,
|
||||
run: () =>
|
||||
this.withCodexWrapperDiagnostics({
|
||||
command: stableLaunchCommand,
|
||||
fallbackCode: "ACP_SESSION_INIT_FAILED",
|
||||
run: () => ensureDelegateSessionWithModelFallback(delegate, ensureInput),
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
const normalizedInput = {
|
||||
...ensureInput,
|
||||
...(codexModelOverride.model ? { model: codexModelOverride.model } : {}),
|
||||
};
|
||||
return await this.runWithLaunchLease({
|
||||
sessionKey: input.sessionKey,
|
||||
command: stableLaunchCommand,
|
||||
enabled: shouldStartWithLease,
|
||||
run: () =>
|
||||
this.codexAcpModelOverrideScope.run(codexModelOverride, () =>
|
||||
this.withCodexWrapperDiagnostics({
|
||||
command: stableLaunchCommand,
|
||||
fallbackCode: "ACP_SESSION_INIT_FAILED",
|
||||
run: () => delegate.ensureSession(withAcpxSessionOptions(normalizedInput)),
|
||||
}),
|
||||
),
|
||||
});
|
||||
const handle = !codexModelOverride
|
||||
? await this.runWithLaunchLease({
|
||||
sessionKey: ensureInput.sessionKey,
|
||||
command: stableLaunchCommand,
|
||||
enabled: shouldStartWithLease,
|
||||
run: () =>
|
||||
this.withCodexWrapperDiagnostics({
|
||||
command: stableLaunchCommand,
|
||||
fallbackCode: "ACP_SESSION_INIT_FAILED",
|
||||
run: () => ensureDelegateSessionWithModelFallback(delegate, ensureInput),
|
||||
}),
|
||||
})
|
||||
: await this.runWithLaunchLease({
|
||||
sessionKey: input.sessionKey,
|
||||
command: stableLaunchCommand,
|
||||
enabled: shouldStartWithLease,
|
||||
run: () =>
|
||||
this.codexAcpModelOverrideScope.run(codexModelOverride, () =>
|
||||
this.withCodexWrapperDiagnostics({
|
||||
command: stableLaunchCommand,
|
||||
fallbackCode: "ACP_SESSION_INIT_FAILED",
|
||||
run: () => delegate.ensureSession(withAcpxSessionOptions(ensureInput)),
|
||||
}),
|
||||
),
|
||||
});
|
||||
return appliedModel ? { ...handle, appliedModel } : handle;
|
||||
}
|
||||
|
||||
async *runTurn(input: Parameters<AcpRuntime["runTurn"]>[0]): AsyncIterable<AcpRuntimeEvent> {
|
||||
@@ -1305,36 +1356,37 @@ export class AcpxRuntime implements AcpRuntime {
|
||||
return;
|
||||
}
|
||||
if (isCodexAcp) {
|
||||
if (
|
||||
key === "model" ||
|
||||
key === "thinking" ||
|
||||
key === "thought_level" ||
|
||||
key === "reasoning_effort"
|
||||
) {
|
||||
const override =
|
||||
key === "model"
|
||||
? normalizeCodexAcpModelOverride(input.value)
|
||||
: normalizeCodexAcpModelOverride(undefined, input.value);
|
||||
if (!override && 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,
|
||||
});
|
||||
}
|
||||
if (key === "model") {
|
||||
const classification = classifyCodexAcpModelRequest(input.value);
|
||||
if (classification.kind === "unsupported") {
|
||||
failUnsupportedCodexAcpModel(input.value);
|
||||
}
|
||||
const { override } = classification;
|
||||
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;
|
||||
}
|
||||
if (key === "thinking" || key === "thought_level" || key === "reasoning_effort") {
|
||||
const classification = classifyCodexAcpModelRequest(undefined, input.value);
|
||||
const reasoningEffort =
|
||||
classification.kind === "override" ? classification.override.reasoningEffort : undefined;
|
||||
if (!reasoningEffort) {
|
||||
return;
|
||||
}
|
||||
await delegate.setConfigOption({
|
||||
...input,
|
||||
key: "reasoning_effort",
|
||||
value: reasoningEffort,
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (isClaudeAcpCommand(command) && key === "model") {
|
||||
@@ -1400,10 +1452,10 @@ export {
|
||||
export const testing = {
|
||||
appendCodexAcpConfigOverrides,
|
||||
assertSupportedRuntimeSessionMode,
|
||||
classifyCodexAcpModelRequest,
|
||||
isClaudeAcpCommand,
|
||||
isCodexAcpCommand,
|
||||
normalizeClaudeAcpModelOverride,
|
||||
normalizeCodexAcpModelOverride,
|
||||
};
|
||||
|
||||
export type { AcpAgentRegistry, AcpRuntimeOptions, AcpSessionRecord, AcpSessionStore };
|
||||
|
||||
@@ -32,6 +32,13 @@ export type AcpRuntimeHandle = {
|
||||
backendSessionId?: string;
|
||||
/** Upstream harness session identifier, if exposed by adapter/runtime. */
|
||||
agentSessionId?: string;
|
||||
/**
|
||||
* Effective model the backend applied during session creation, when it can differ from the
|
||||
* requested model. A backend that drops an unsupported inherited default reports `dropped` so
|
||||
* the manager omits that model from persisted runtime controls instead of replaying a rejected
|
||||
* model before the first turn. Absent when the backend did not deviate from the request.
|
||||
*/
|
||||
appliedModel?: { kind: "applied"; model: string } | { kind: "dropped" };
|
||||
};
|
||||
|
||||
export type AcpRuntimeEnsureInput = {
|
||||
@@ -42,6 +49,12 @@ export type AcpRuntimeEnsureInput = {
|
||||
resumeSessionId?: string;
|
||||
/** Optional runtime model override that must be available during session creation. */
|
||||
model?: string;
|
||||
/**
|
||||
* Whether `model` was an explicit caller selection rather than an inherited default. A backend
|
||||
* that cannot honor an explicit unsupported model must fail closed; an unsupported inherited
|
||||
* default may be dropped so the backend starts on its own default.
|
||||
*/
|
||||
modelExplicit?: boolean;
|
||||
/** Optional runtime thinking/reasoning override that must be available during session creation. */
|
||||
thinking?: string;
|
||||
cwd?: string;
|
||||
|
||||
@@ -59,6 +59,7 @@ export async function runManagerInitializeSession(params: {
|
||||
mode: input.mode,
|
||||
resumeSessionId: input.resumeSessionId,
|
||||
...(requestedModel ? { model: requestedModel } : {}),
|
||||
...(requestedModel && input.modelExplicit ? { modelExplicit: true } : {}),
|
||||
...(requestedThinking ? { thinking: requestedThinking } : {}),
|
||||
cwd: requestedCwd,
|
||||
}),
|
||||
@@ -66,8 +67,13 @@ export async function runManagerInitializeSession(params: {
|
||||
fallbackMessage: "Could not initialize ACP session runtime.",
|
||||
});
|
||||
const effectiveCwd = normalizeText(handle.cwd) ?? requestedCwd;
|
||||
const effectiveModel = resolveEffectiveSessionModel({
|
||||
requestedModel,
|
||||
appliedModel: handle.appliedModel,
|
||||
});
|
||||
const effectiveRuntimeOptions = normalizeRuntimeOptions({
|
||||
...initialRuntimeOptions,
|
||||
model: effectiveModel,
|
||||
...(effectiveCwd ? { cwd: effectiveCwd } : {}),
|
||||
});
|
||||
|
||||
@@ -130,6 +136,17 @@ export async function runManagerInitializeSession(params: {
|
||||
};
|
||||
}
|
||||
|
||||
function resolveEffectiveSessionModel(params: {
|
||||
requestedModel: string | undefined;
|
||||
appliedModel: AcpRuntimeHandle["appliedModel"];
|
||||
}): string | undefined {
|
||||
const { appliedModel } = params;
|
||||
if (!appliedModel) {
|
||||
return params.requestedModel;
|
||||
}
|
||||
return appliedModel.kind === "applied" ? appliedModel.model : undefined;
|
||||
}
|
||||
|
||||
async function persistInitializedSessionMeta(params: {
|
||||
cfg: OpenClawConfig;
|
||||
sessionKey: string;
|
||||
|
||||
@@ -0,0 +1,173 @@
|
||||
/** Tests ACP runtime config validation and backend-applied model persistence. */
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
AcpSessionManager,
|
||||
baseCfg,
|
||||
createRuntime,
|
||||
expectMockCallFields,
|
||||
expectNoMockCallFields,
|
||||
expectRejectedRecord,
|
||||
hoisted,
|
||||
installAcpSessionManagerTestLifecycle,
|
||||
readySessionMeta,
|
||||
type SessionAcpMeta,
|
||||
} from "./manager.test-helpers.js";
|
||||
|
||||
describe("AcpSessionManager runtime config validation", () => {
|
||||
installAcpSessionManagerTestLifecycle();
|
||||
|
||||
it("rejects invalid runtime option values before backend controls run", async () => {
|
||||
const runtimeState = createRuntime();
|
||||
hoisted.requireAcpRuntimeBackendMock.mockReturnValue({
|
||||
id: "acpx",
|
||||
runtime: runtimeState.runtime,
|
||||
});
|
||||
hoisted.readAcpSessionEntryMock.mockReturnValue({
|
||||
sessionKey: "agent:codex:acp:session-1",
|
||||
storeSessionKey: "agent:codex:acp:session-1",
|
||||
acp: readySessionMeta(),
|
||||
});
|
||||
|
||||
const manager = new AcpSessionManager();
|
||||
await expectRejectedRecord(
|
||||
manager.setSessionConfigOption({
|
||||
cfg: baseCfg,
|
||||
sessionKey: "agent:codex:acp:session-1",
|
||||
key: "timeout",
|
||||
value: "not-a-number",
|
||||
}),
|
||||
{ code: "ACP_INVALID_RUNTIME_OPTION" },
|
||||
);
|
||||
expect(runtimeState.setConfigOption).not.toHaveBeenCalled();
|
||||
|
||||
await expectRejectedRecord(
|
||||
manager.updateSessionRuntimeOptions({
|
||||
cfg: baseCfg,
|
||||
sessionKey: "agent:codex:acp:session-1",
|
||||
patch: { cwd: "relative/path" },
|
||||
}),
|
||||
{ code: "ACP_INVALID_RUNTIME_OPTION" },
|
||||
);
|
||||
});
|
||||
|
||||
it("never replays an inherited non-openai default that the backend dropped at session init", async () => {
|
||||
const sessionKey = "agent:codex:acp:session-dropped-default";
|
||||
const runtimeState = createRuntime();
|
||||
runtimeState.ensureSession.mockImplementation(async (input) => ({
|
||||
sessionKey: input.sessionKey,
|
||||
backend: "acpx",
|
||||
runtimeSessionName: `${input.sessionKey}:runtime`,
|
||||
appliedModel: { kind: "dropped" as const },
|
||||
}));
|
||||
hoisted.requireAcpRuntimeBackendMock.mockReturnValue({
|
||||
id: "acpx",
|
||||
runtime: runtimeState.runtime,
|
||||
});
|
||||
|
||||
let persistedMeta: SessionAcpMeta | undefined;
|
||||
hoisted.upsertAcpSessionMetaMock.mockImplementation(
|
||||
async (payload: {
|
||||
mutate: (current: SessionAcpMeta | undefined) => SessionAcpMeta | null | undefined;
|
||||
}) => {
|
||||
persistedMeta = payload.mutate(undefined) ?? undefined;
|
||||
return persistedMeta
|
||||
? { sessionKey, storeSessionKey: sessionKey, acp: persistedMeta }
|
||||
: null;
|
||||
},
|
||||
);
|
||||
|
||||
const manager = new AcpSessionManager();
|
||||
await manager.initializeSession({
|
||||
cfg: baseCfg,
|
||||
sessionKey,
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
runtimeOptions: {
|
||||
model: "google/gemini-3.1-flash-lite",
|
||||
thinking: "low",
|
||||
},
|
||||
});
|
||||
|
||||
expect(persistedMeta?.runtimeOptions).toEqual({ thinking: "low" });
|
||||
|
||||
hoisted.readAcpSessionEntryMock.mockReturnValue({
|
||||
sessionKey,
|
||||
storeSessionKey: sessionKey,
|
||||
acp: persistedMeta as SessionAcpMeta,
|
||||
});
|
||||
|
||||
await manager.runTurn({
|
||||
cfg: baseCfg,
|
||||
sessionKey,
|
||||
text: "do work",
|
||||
mode: "prompt",
|
||||
requestId: "run-dropped-default",
|
||||
provenance: "system",
|
||||
});
|
||||
|
||||
expectNoMockCallFields(runtimeState.setConfigOption, { key: "model" });
|
||||
expectMockCallFields(runtimeState.setConfigOption, { key: "thinking", value: "low" });
|
||||
expect(runtimeState.runTurn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("persists and replays a supported codex model the backend applied at session init", async () => {
|
||||
const sessionKey = "agent:codex:acp:session-applied-model";
|
||||
const runtimeState = createRuntime();
|
||||
runtimeState.ensureSession.mockImplementation(async (input) => ({
|
||||
sessionKey: input.sessionKey,
|
||||
backend: "acpx",
|
||||
runtimeSessionName: `${input.sessionKey}:runtime`,
|
||||
appliedModel: { kind: "applied" as const, model: input.model ?? "" },
|
||||
}));
|
||||
hoisted.requireAcpRuntimeBackendMock.mockReturnValue({
|
||||
id: "acpx",
|
||||
runtime: runtimeState.runtime,
|
||||
});
|
||||
|
||||
let persistedMeta: SessionAcpMeta | undefined;
|
||||
hoisted.upsertAcpSessionMetaMock.mockImplementation(
|
||||
async (payload: {
|
||||
mutate: (current: SessionAcpMeta | undefined) => SessionAcpMeta | null | undefined;
|
||||
}) => {
|
||||
persistedMeta = payload.mutate(undefined) ?? undefined;
|
||||
return persistedMeta
|
||||
? { sessionKey, storeSessionKey: sessionKey, acp: persistedMeta }
|
||||
: null;
|
||||
},
|
||||
);
|
||||
|
||||
const manager = new AcpSessionManager();
|
||||
await manager.initializeSession({
|
||||
cfg: baseCfg,
|
||||
sessionKey,
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
runtimeOptions: {
|
||||
model: "openai/gpt-5.5",
|
||||
},
|
||||
});
|
||||
|
||||
expect(persistedMeta?.runtimeOptions).toEqual({ model: "openai/gpt-5.5" });
|
||||
|
||||
hoisted.readAcpSessionEntryMock.mockReturnValue({
|
||||
sessionKey,
|
||||
storeSessionKey: sessionKey,
|
||||
acp: persistedMeta as SessionAcpMeta,
|
||||
});
|
||||
|
||||
await manager.runTurn({
|
||||
cfg: baseCfg,
|
||||
sessionKey,
|
||||
text: "do work",
|
||||
mode: "prompt",
|
||||
requestId: "run-applied-model",
|
||||
provenance: "system",
|
||||
});
|
||||
|
||||
expectMockCallFields(runtimeState.setConfigOption, {
|
||||
key: "model",
|
||||
value: "openai/gpt-5.5",
|
||||
});
|
||||
expect(runtimeState.runTurn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -995,38 +995,4 @@ describe("AcpSessionManager runtime config", () => {
|
||||
});
|
||||
expect(nextOptions).toEqual({ permissionProfile: "strict" });
|
||||
});
|
||||
|
||||
it("rejects invalid runtime option values before backend controls run", async () => {
|
||||
const runtimeState = createRuntime();
|
||||
hoisted.requireAcpRuntimeBackendMock.mockReturnValue({
|
||||
id: "acpx",
|
||||
runtime: runtimeState.runtime,
|
||||
});
|
||||
hoisted.readAcpSessionEntryMock.mockReturnValue({
|
||||
sessionKey: "agent:codex:acp:session-1",
|
||||
storeSessionKey: "agent:codex:acp:session-1",
|
||||
acp: readySessionMeta(),
|
||||
});
|
||||
|
||||
const manager = new AcpSessionManager();
|
||||
await expectRejectedRecord(
|
||||
manager.setSessionConfigOption({
|
||||
cfg: baseCfg,
|
||||
sessionKey: "agent:codex:acp:session-1",
|
||||
key: "timeout",
|
||||
value: "not-a-number",
|
||||
}),
|
||||
{ code: "ACP_INVALID_RUNTIME_OPTION" },
|
||||
);
|
||||
expect(runtimeState.setConfigOption).not.toHaveBeenCalled();
|
||||
|
||||
await expectRejectedRecord(
|
||||
manager.updateSessionRuntimeOptions({
|
||||
cfg: baseCfg,
|
||||
sessionKey: "agent:codex:acp:session-1",
|
||||
patch: { cwd: "relative/path" },
|
||||
}),
|
||||
{ code: "ACP_INVALID_RUNTIME_OPTION" },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -49,6 +49,7 @@ export type AcpInitializeSessionInput = {
|
||||
mode: AcpRuntimeSessionMode;
|
||||
resumeSessionId?: string;
|
||||
runtimeOptions?: Partial<AcpSessionRuntimeOptions>;
|
||||
modelExplicit?: boolean;
|
||||
cwd?: string;
|
||||
backendId?: string;
|
||||
};
|
||||
|
||||
@@ -1017,8 +1017,11 @@ function resolveAcpSpawnRuntimeOptions(params: {
|
||||
model?: string;
|
||||
thinking?: string;
|
||||
runTimeoutSeconds?: number;
|
||||
}): { ok: true; runtimeOptions?: AcpSpawnRuntimeOptions } | { ok: false; error: string } {
|
||||
}):
|
||||
| { ok: true; runtimeOptions?: AcpSpawnRuntimeOptions; modelExplicit: boolean }
|
||||
| { ok: false; error: string } {
|
||||
const policyAgentId = params.configAgentId ?? params.targetAgentId;
|
||||
const modelExplicit = normalizeOptionalString(params.model) !== undefined;
|
||||
const model = resolveConfiguredSubagentSpawnModelSelection({
|
||||
cfg: params.cfg,
|
||||
agentId: policyAgentId,
|
||||
@@ -1059,7 +1062,7 @@ function resolveAcpSpawnRuntimeOptions(params: {
|
||||
...(timeoutSeconds ? { timeoutSeconds } : {}),
|
||||
}
|
||||
: undefined;
|
||||
return { ok: true, runtimeOptions };
|
||||
return { ok: true, runtimeOptions, modelExplicit };
|
||||
}
|
||||
|
||||
async function initializeAcpSpawnRuntime(params: {
|
||||
@@ -1069,6 +1072,7 @@ async function initializeAcpSpawnRuntime(params: {
|
||||
runtimeMode: AcpRuntimeSessionMode;
|
||||
resumeSessionId?: string;
|
||||
runtimeOptions?: AcpSpawnRuntimeOptions;
|
||||
modelExplicit?: boolean;
|
||||
cwd?: string;
|
||||
}): Promise<AcpSpawnInitializedRuntime> {
|
||||
const storePath = resolveStorePath(params.cfg.session?.store, { agentId: params.targetAgentId });
|
||||
@@ -1096,6 +1100,7 @@ async function initializeAcpSpawnRuntime(params: {
|
||||
mode: params.runtimeMode,
|
||||
resumeSessionId: params.resumeSessionId,
|
||||
runtimeOptions: params.runtimeOptions,
|
||||
modelExplicit: params.modelExplicit,
|
||||
cwd: params.cwd,
|
||||
backendId: params.cfg.acp?.backend,
|
||||
});
|
||||
@@ -1495,6 +1500,7 @@ export async function spawnAcpDirect(
|
||||
runtimeMode,
|
||||
resumeSessionId: params.resumeSessionId,
|
||||
runtimeOptions: runtimeOptionsResult.runtimeOptions,
|
||||
modelExplicit: runtimeOptionsResult.modelExplicit,
|
||||
cwd: runtimeCwd,
|
||||
});
|
||||
initializedRuntime = initializedSession.runtimeCloseHandle;
|
||||
|
||||
Reference in New Issue
Block a user