Handle ACP session-config rejections inside the ACP lane instead of falling back to CLI

Review feedback on the previous fix: the point of ACPX is to stop
maintaining the CLI adapter, so recovering from a codex-acp
set_config_option rejection by switching lanes was the wrong shape.

ACP-native handling instead:
- The codex model is now applied via a `-c model=...` startup config
  override on the codex-acp command (mirroring ANTHROPIC_MODEL for
  claude). The override feeds the same config surface as
  ~/.codex/config.toml, which accepts manual model ids; verified against
  codex-acp 0.12.0 that session/new then reports the manual id as
  currentModelId while set_config_option validation rejects unlisted ids
  on older versions.
- session/set_config_option application is now best-effort: a rejected
  option (e.g. ACP -32602) logs loudly, surfaces in the run's command
  notes, and the run continues on the agent default instead of dying
  with acpx_session_config_failed.
- The ACP->CLI session-config fallback and its error-code constant are
  reverted; auto lane falls back to CLI only for startup failures, as
  before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Fable 5 committed 2026-07-10 14:37:48 +00:00
1 parent 8c46620678
commit 6b2d2ef1e8
7 files changed
+186 -132

No files matched your search

@@ -5,11 +5,6 @@ export const DEFAULT_ACP_ENGINE_NON_INTERACTIVE_PERMISSIONS = "deny";
export const DEFAULT_ACP_ENGINE_TIMEOUT_SEC = 0;
export const DEFAULT_ACP_ENGINE_WARM_HANDLE_IDLE_MS = 0;
// The ACP server rejected a session/set_config_option value (for example a
// manual model id its validation list does not include). Adapters with a
// non-ACP lane can treat this as recoverable and retry there.
export const ACPX_SESSION_CONFIG_FAILED_ERROR_CODE = "acpx_session_config_failed";
export const ACPX_ADAPTER_AGENT_IDS = {
claude_local: "claude",
codex_local: "codex",
@@ -75,15 +75,16 @@ async function runExecutor(
executionTransport?: Record<string, unknown>;
authToken?: string;
executionTarget?: Record<string, unknown>;
runtimeOverrides?: Record<string, unknown>;
} = {},
) {
const runtimeOptions: Record<string, unknown>[] = [];
const meta: Record<string, unknown>[] = [];
const logs: Array<{ stream: string; text: string }> = [];
const execute = createAcpxEngineExecutor({
createRuntime: (options) => {
runtimeOptions.push(options as unknown as Record<string, unknown>);
return buildRuntime() as never;
createRuntime: (createOptions) => {
runtimeOptions.push(createOptions as unknown as Record<string, unknown>);
return { ...buildRuntime(), ...options.runtimeOverrides } as never;
},
});
@@ -1166,3 +1167,112 @@ describe("shared ACP engine execution timeouts", () => {
expect(cancelReasons).toContain(expectedMessage);
}, 15_000);
});
describe("ACPX session config application", () => {
it("applies the codex model via a -c startup config override instead of set_config_option", async () => {
const root = await makeTempRoot();
const stateDir = path.join(root, "state");
const setConfigOption = [] as Array<{ key: string; value: string }>;
const { meta } = await runExecutor(
{
agent: "codex",
agentCommand: "node ./fake-codex-acp.js",
model: "gpt-5.5-pro",
stateDir,
env: { CODEX_HOME: path.join(root, "codex-home") },
},
{
runtimeOverrides: {
setConfigOption: async (input: { key: string; value: string }) => {
setConfigOption.push({ key: input.key, value: input.value });
},
},
},
);
const wrappers = await fs.readdir(path.join(stateDir, "wrappers"));
const wrapperFile = wrappers.find((name) => name.endsWith(".sh"));
expect(wrapperFile).toBeTruthy();
const wrapper = await fs.readFile(path.join(stateDir, "wrappers", wrapperFile!), "utf8");
expect(wrapper).toContain('exec node ./fake-codex-acp.js -c \'model="gpt-5.5-pro"\'');
expect(setConfigOption).toEqual([]);
const commandNotes = (meta[0]?.commandNotes ?? []) as string[];
expect(commandNotes.join("\n")).toContain("set via -c model=... startup config override");
});
it("still applies codex reasoning effort via set_config_option alongside the model startup override", async () => {
const root = await makeTempRoot();
const setConfigOption = [] as Array<{ key: string; value: string }>;
await runExecutor(
{
agent: "codex",
agentCommand: "node ./fake-codex-acp.js",
model: "gpt-5.5-pro",
modelReasoningEffort: "high",
stateDir: path.join(root, "state"),
env: { CODEX_HOME: path.join(root, "codex-home") },
},
{
runtimeOverrides: {
setConfigOption: async (input: { key: string; value: string }) => {
setConfigOption.push({ key: input.key, value: input.value });
},
},
},
);
expect(setConfigOption).toEqual([{ key: "reasoning_effort", value: "high" }]);
});
it("degrades gracefully when the ACP server rejects a session config option", async () => {
const root = await makeTempRoot();
const { logs, meta, result } = await runExecutor(
{
agent: "custom",
agentCommand: "node ./fake-acp.js",
model: "gpt-5.5-pro",
stateDir: path.join(root, "state"),
},
{
runtimeOverrides: {
setConfigOption: async () => {
throw new Error("Invalid params (ACP -32602)");
},
},
},
);
expect(result.exitCode).toBe(0);
expect(result.errorCode ?? null).toBeNull();
const stderrText = logs
.filter((entry) => entry.stream === "stderr")
.map((entry) => entry.text)
.join("\n");
expect(stderrText).toContain('rejected session config model=gpt-5.5-pro');
expect(stderrText).toContain("continuing with the agent's default model");
const commandNotes = (meta[0]?.commandNotes ?? []) as string[];
expect(commandNotes.join("\n")).toContain("continuing with the agent's default model");
});
it("warns and continues when the ACPX runtime lacks session config controls", async () => {
const root = await makeTempRoot();
const { logs, result } = await runExecutor({
agent: "custom",
agentCommand: "node ./fake-acp.js",
thinkingEffort: "high",
stateDir: path.join(root, "state"),
});
expect(result.exitCode).toBe(0);
const stderrText = logs
.filter((entry) => entry.stream === "stderr")
.map((entry) => entry.text)
.join("\n");
expect(stderrText).toContain("does not expose session config controls");
expect(stderrText).toContain("continuing without effort override(s)");
});
});
@@ -54,7 +54,6 @@ import {
type AcpRuntimeTurnResult,
} from "acpx/runtime";
import {
ACPX_SESSION_CONFIG_FAILED_ERROR_CODE,
DEFAULT_ACP_ENGINE_AGENT,
DEFAULT_ACP_ENGINE_MODE,
DEFAULT_ACP_ENGINE_NON_INTERACTIVE_PERMISSIONS,
@@ -1076,6 +1075,16 @@ async function buildRuntime(input: {
agentCommand = normalized;
}
}
// For the codex agent, set the model via a `-c model=...` startup config
// override rather than session/set_config_option — the override feeds the
// same config surface as ~/.codex/config.toml, which accepts manual model
// ids, while some codex-acp versions validate set_config_option values
// against their advertised model list and reject unlisted ids (ACP -32602).
// This mirrors the ANTHROPIC_MODEL startup env approach for claude below.
if (requestedModel && acpxAgent === "codex" && agentCommandShell) {
agentCommandShell = `${agentCommandShell} -c ${shellQuote(`model=${JSON.stringify(requestedModel)}`)}`;
agentCommand = agentCommandShell;
}
const childStderrDir = path.join(stateDir, "run-stderr");
const childStderrLogPath = agentCommand ? path.join(childStderrDir, `${runId}.log`) : null;
const wrapper = agentCommand
@@ -1156,9 +1165,10 @@ async function buildRuntime(input: {
function sessionConfigOptions(prepared: AcpxPreparedRuntime): Array<{ key: string; value: string }> {
const options: Array<{ key: string; value: string }> = [];
// Model for the claude agent is pre-set via ANTHROPIC_MODEL env var at
// startup; skip set_config_option to avoid ACP-server model-name validation
// that rejects bare IDs like "claude-opus-4-7" in some runtime versions.
if (prepared.requestedModel && prepared.acpxAgent !== "claude") {
// startup, and for the codex agent via a `-c model=...` startup config
// override; skip set_config_option for both to avoid ACP-server model-name
// validation that rejects bare/manual IDs in some runtime versions.
if (prepared.requestedModel && prepared.acpxAgent !== "claude" && prepared.acpxAgent !== "codex") {
options.push({ key: "model", value: prepared.requestedModel });
}
if (prepared.requestedThinkingEffort) {
@@ -1176,31 +1186,47 @@ function sessionConfigOptions(prepared: AcpxPreparedRuntime): Array<{ key: strin
return options;
}
// Session config options are preferences, not prerequisites: a value the ACP
// server rejects (e.g. -32602 for a model id its validation list does not
// advertise) must degrade the run, never kill it. Failures are logged loudly
// and returned as notes so they surface in the run's command metadata.
async function applySessionConfigOptions(input: {
runtime: AcpRuntime;
handle: AcpRuntimeHandle;
prepared: AcpxPreparedRuntime;
onLog: AdapterExecutionContext["onLog"];
}) {
}): Promise<string[]> {
const options = sessionConfigOptions(input.prepared);
if (options.length === 0) return;
if (options.length === 0) return [];
const notes: string[] = [];
if (!input.runtime.setConfigOption) {
const message =
"ACPX runtime does not expose session config controls; upgrade ACPX or remove configured model, effort, and fast mode overrides.";
`ACPX runtime does not expose session config controls; continuing without ` +
`${options.map((option) => option.key).join(", ")} override(s). Upgrade ACPX to apply them.`;
await input.onLog("stderr", `[paperclip] ${message}\n`);
throw new Error(message);
return [message];
}
for (const option of options) {
await input.runtime.setConfigOption({
handle: input.handle,
key: option.key,
value: option.value,
});
await input.onLog(
"stdout",
`[paperclip] Applied ACPX ${input.prepared.acpxAgent} config ${option.key}=${option.value}\n`,
);
try {
await input.runtime.setConfigOption({
handle: input.handle,
key: option.key,
value: option.value,
});
await input.onLog(
"stdout",
`[paperclip] Applied ACPX ${input.prepared.acpxAgent} config ${option.key}=${option.value}\n`,
);
} catch (err) {
const reason = err instanceof Error ? err.message : String(err);
const message =
`ACPX ${input.prepared.acpxAgent} rejected session config ${option.key}=${option.value} ` +
`(${reason}); continuing with the agent's default ${option.key}.`;
await input.onLog("stderr", `[paperclip] ${message}\n`);
notes.push(message);
}
}
return notes;
}
function renderPaperclipEnvNote(env: Record<string, string>): string {
@@ -1378,7 +1404,7 @@ function resultErrorMessage(result: AcpRuntimeTurnResult): string | null {
return result.error.message;
}
type AcpxExecutionPhase = "ensure_session" | "configure_session" | "turn";
type AcpxExecutionPhase = "ensure_session" | "turn";
function describeErrorDiagnostics(err: unknown): {
errorName: string;
@@ -1443,7 +1469,6 @@ function classifyError(
if (acpCode === "ACP_BACKEND_MISSING") return "acpx_backend_missing";
if (acpCode === "ACP_BACKEND_UNAVAILABLE") return "acpx_backend_unavailable";
if (phase === "ensure_session") return "acpx_session_init_failed";
if (phase === "configure_session") return ACPX_SESSION_CONFIG_FAILED_ERROR_CODE;
if (phase === "turn") return "acpx_turn_failed";
return null;
})();
@@ -1721,49 +1746,12 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) {
};
}
const sessionHandle = handle;
try {
await applySessionConfigOptions({
runtime,
handle: sessionHandle,
prepared,
onLog: ctx.onLog,
});
} catch (err) {
const { classified, message } = await emitAcpxFailure({
ctx,
prepared,
err,
phase: "configure_session",
});
await runtime.close({
handle: sessionHandle,
reason: "paperclip config cleanup",
discardPersistentState: false,
}).catch(() => {});
const existing = warmHandles.get(prepared.sessionKey);
if (warmHandleMatches(existing, runtime, sessionHandle) && existing) {
clearWarmHandleTimer(existing);
warmHandles.delete(prepared.sessionKey);
}
return {
exitCode: 1,
signal: null,
timedOut: false,
errorMessage: message,
...classified,
provider: "acpx",
model: prepared.requestedModel || null,
clearSession,
resultJson: {
phase: "configure_session",
agent: prepared.acpxAgent,
requestedModel: prepared.requestedModel || null,
requestedThinkingEffort: prepared.requestedThinkingEffort || null,
fastMode: prepared.fastMode,
},
summary: message,
};
}
const sessionConfigNotes = await applySessionConfigOptions({
runtime,
handle: sessionHandle,
prepared,
onLog: ctx.onLog,
});
const { prompt, promptMetrics, commandNotes } = await buildPrompt(ctx, resumedSession, prepared.env);
const runPrompt = joinPromptSections([prepared.skillPromptInstructions, prompt]);
await emitAcpxLog(ctx, {
@@ -1791,11 +1779,14 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) {
? [
prepared.acpxAgent === "claude"
? `Requested ACPX model: ${prepared.requestedModel} (set via ANTHROPIC_MODEL env at startup).`
: `Requested ACPX model: ${prepared.requestedModel}.`,
: prepared.acpxAgent === "codex"
? `Requested ACPX model: ${prepared.requestedModel} (set via -c model=... startup config override).`
: `Requested ACPX model: ${prepared.requestedModel}.`,
]
: []),
...(prepared.requestedThinkingEffort ? [`Requested ACPX thinking effort: ${prepared.requestedThinkingEffort}.`] : []),
...(prepared.fastMode ? ["Requested ACPX Codex fast mode."] : []),
...sessionConfigNotes,
...(Array.isArray(prepared.skillsIdentity.commandNotes)
? prepared.skillsIdentity.commandNotes.filter((note): note is string => typeof note === "string")
: []),
+1 -1
View File
@@ -104,5 +104,5 @@ Notes:
- Some model/tool combinations reject certain effort levels (for example minimal with web search enabled).
- Fast mode is supported on GPT-5.5, GPT-5.4 and manual model IDs. When enabled for those models, Paperclip applies \`service_tier="fast"\` and \`features.fast_mode=true\`.
- When Paperclip realizes a workspace/runtime for a run, it injects PAPERCLIP_WORKSPACE_* and PAPERCLIP_RUNTIME_* env vars for agent-side tooling.
- Codex ACP is the preferred auto lane when Node >=22.13.0 and the Codex ACP server are available. It reuses shared ACP prompt/runtime guidance, selected skill materialization into CODEX_HOME/skills, model/reasoning/fast-mode session config, and existing quota-window reporting. Auto selection falls back to CLI when ACP prerequisites are unavailable or the ACP server rejects the session configuration (for example a manual model id its set_config_option validation does not list); explicit engine="acp" fails loudly.
- Codex ACP is the preferred auto lane when Node >=22.13.0 and the Codex ACP server are available. It reuses shared ACP prompt/runtime guidance, selected skill materialization into CODEX_HOME/skills, model/reasoning/fast-mode session config, and existing quota-window reporting. The configured model is applied through a \`-c model=...\` startup config override, which accepts manual model ids; other session config options are best-effort, so a rejected option is logged and the run continues on the agent default instead of failing. Auto selection falls back to CLI only when ACP prerequisites are unavailable; explicit engine="acp" fails loudly.
`;
@@ -368,12 +368,17 @@ describe("codex_local ACP lane", () => {
mode: "persistent",
cwd: root,
});
// The model is applied via a `-c model=...` startup config override (it
// accepts manual model ids some codex-acp versions reject through
// set_config_option); only effort and fast mode go through session config.
expect(runtimes[0]?.setConfigInputs.map((input) => [input.key, input.value])).toEqual([
["model", "gpt-5.5"],
["reasoning_effort", "high"],
["service_tier", "fast"],
["features.fast_mode", "true"],
]);
expect(meta[0]?.commandNotes?.join("\n")).toContain(
"Requested ACPX model: gpt-5.5 (set via -c model=... startup config override).",
);
expect(meta[0]?.commandNotes?.join("\n")).toContain("Prepared ACPX Codex skill home");
expect(meta[0]?.env?.CODEX_HOME).toBe(path.join(root, "codex-home"));
});
@@ -159,58 +159,28 @@ describe("codex_local ACP startup fallback", () => {
expect(runAdapterExecutionTargetProcess).not.toHaveBeenCalled();
});
it("falls back to Codex CLI when auto-selected ACP rejects session configuration", async () => {
executeCodexAcp.mockResolvedValueOnce({
exitCode: 1,
it("returns successful structured ACP results without touching the CLI lane", async () => {
const succeeded = {
exitCode: 0,
signal: null,
timedOut: false,
errorMessage:
'Agent rejected session/set_config_option for "model"="gpt-5.5-pro": Invalid params (ACP -32602).',
errorCode: "acpx_session_config_failed",
errorMessage: null,
errorCode: null,
provider: "acpx",
model: "gpt-5.5-pro",
resultJson: { phase: "configure_session" },
summary: "config rejected",
} as never);
resultJson: { status: "completed" },
summary: "acp turn done",
};
executeCodexAcp.mockResolvedValueOnce(succeeded as never);
const ctx = buildContext({ model: "gpt-5.5-pro" });
const result = await execute(ctx as never);
expect(result.exitCode).toBe(0);
expect(result.summary).toBe("hello");
expect(executeCodexAcp).toHaveBeenCalledTimes(1);
expect(runAdapterExecutionTargetProcess).toHaveBeenCalledTimes(1);
expect(ctx.onLog).toHaveBeenCalledWith(
"stderr",
expect.stringContaining("Codex ACP rejected session configuration"),
);
expect(ctx.onLog).toHaveBeenCalledWith(
"stderr",
expect.stringContaining("gpt-5.5-pro"),
);
});
it("returns the session-config failure unchanged when engine=acp is explicit", async () => {
const failed = {
exitCode: 1,
signal: null,
timedOut: false,
errorMessage: "Agent rejected session/set_config_option",
errorCode: "acpx_session_config_failed",
provider: "acpx",
resultJson: { phase: "configure_session" },
summary: "config rejected",
};
executeCodexAcp.mockResolvedValueOnce(failed as never);
const ctx = buildContext({ engine: "acp" });
const result = await execute(ctx as never);
expect(result).toBe(failed);
expect(result).toBe(succeeded);
expect(runAdapterExecutionTargetProcess).not.toHaveBeenCalled();
});
it("does not fall back for other failed ACP results", async () => {
it("does not fall back to the CLI lane for failed structured ACP results", async () => {
const failed = {
exitCode: 1,
signal: null,
@@ -64,7 +64,6 @@ import {
formatOutputInactivityMonitorErrorMessage,
resolveCodexInactivityTimeout,
} from "./output-inactivity-monitor.js";
import { ACPX_SESSION_CONFIG_FAILED_ERROR_CODE } from "@paperclipai/adapter-utils/acpx-engine/constants";
import {
createCodexAcpExecutor,
formatCodexAcpFallbackMessage,
@@ -331,9 +330,8 @@ export async function ensureCodexSkillsInjected(
export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExecutionResult> {
const engineSelection = await resolveCodexExecutionEngineForRun(ctx);
if (engineSelection.engine === "acp") {
let acpResult: AdapterExecutionResult | null = null;
try {
acpResult = await executeCodexAcp(ctx);
return await executeCodexAcp(ctx);
} catch (err) {
if (engineSelection.explicit) throw err;
const reason = err instanceof Error ? err.message : String(err);
@@ -342,21 +340,6 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
formatCodexAcpFallbackMessage(`Codex ACP startup failed: ${reason}`),
);
}
if (acpResult) {
// A session/set_config_option rejection (e.g. a manual model id the ACP
// server's validation list does not include) is recoverable: the CLI
// lane applies model/effort/fast-mode via startup args instead of ACP
// session controls. Explicit engine=acp stays strict and fails loudly.
const sessionConfigRejected =
acpResult.errorCode === ACPX_SESSION_CONFIG_FAILED_ERROR_CODE;
if (engineSelection.explicit || !sessionConfigRejected) return acpResult;
await ctx.onLog(
"stderr",
formatCodexAcpFallbackMessage(
`Codex ACP rejected session configuration: ${acpResult.errorMessage ?? "unknown error"}`,
),
);
}
}
if (!engineSelection.explicit && engineSelection.fallbackReason) {
await ctx.onLog("stderr", formatCodexAcpFallbackMessage(engineSelection.fallbackReason));