fix: require advertised ACP model support
Some checks are pending
CI / scope (push) Waiting to run
CI / ${{ matrix.name }} (pnpm run build, Build) (push) Blocked by required conditions
CI / ${{ matrix.name }} (pnpm run conformance:run -- --case acp.v1.initialize.handshake, Conformance Smoke) (push) Blocked by required conditions
CI / ${{ matrix.name }} (pnpm run format:check, Format) (push) Blocked by required conditions
CI / ${{ matrix.name }} (pnpm run lint, Lint) (push) Blocked by required conditions
CI / ${{ matrix.name }} (pnpm run test:coverage, Test, 22) (push) Blocked by required conditions
CI / ${{ matrix.name }} (pnpm run typecheck, Typecheck) (push) Blocked by required conditions
CI / Docs (push) Blocked by required conditions

This commit is contained in:
Peter Steinberger 2026-04-25 21:52:18 +01:00
parent 119b84ee31
commit 2480c48806
No known key found for this signature in database
11 changed files with 231 additions and 35 deletions

View File

@ -12,6 +12,8 @@ Repo: https://github.com/openclaw/acpx
### Fixes
- CLI/models: fail clearly when `--model` targets a non-Claude ACP agent that does not advertise ACP model support, and reject model ids outside an adapter's advertised `availableModels` instead of silently falling back to the adapter default.
## 2026.4.25 (v0.6.0)
### Changes

View File

@ -102,22 +102,22 @@ or close a PR if you run it against a live repository.
All global options:
| Option | Description | Details |
| ---------------------------------------- | ---------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `--agent <command>` | Raw ACP agent command (escape hatch) | Do not combine with positional agent token. |
| `--cwd <dir>` | Working directory | Defaults to current directory. Stored as absolute path for scoping. |
| `--approve-all` | Auto-approve all permissions | Permission mode `approve-all`. |
| `--approve-reads` | Auto-approve reads/searches, prompt for others | Default permission mode. |
| `--deny-all` | Deny all permissions | Permission mode `deny-all`. |
| `--format <fmt>` | Output format | `text` (default), `json`, `quiet`. |
| `--suppress-reads` | Suppress read file contents | Replaces raw read payloads with `[read output suppressed]`. |
| `--json-strict` | Strict JSON mode | Requires `--format json`; suppresses non-JSON stderr output. |
| `--no-terminal` | Disable ACP terminal capability | Advertises `clientCapabilities.terminal: false` during ACP initialize for new agent clients. |
| `--non-interactive-permissions <policy>` | Non-TTY prompt policy | `deny` (default) or `fail` when approval prompt cannot be shown. |
| `--timeout <seconds>` | Max wait time for agent response | Must be positive. Decimal seconds allowed. |
| `--ttl <seconds>` | Queue owner idle TTL before shutdown | Default `300`. `0` disables TTL. |
| `--model <id>` | Set agent model | Passed through to agent-specific session creation metadata when applicable; if the agent advertises models, `acpx` also applies it via ACP `session/set_model`. |
| `--verbose` | Enable verbose logs | Prints ACP/debug details to stderr. |
| Option | Description | Details |
| ---------------------------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `--agent <command>` | Raw ACP agent command (escape hatch) | Do not combine with positional agent token. |
| `--cwd <dir>` | Working directory | Defaults to current directory. Stored as absolute path for scoping. |
| `--approve-all` | Auto-approve all permissions | Permission mode `approve-all`. |
| `--approve-reads` | Auto-approve reads/searches, prompt for others | Default permission mode. |
| `--deny-all` | Deny all permissions | Permission mode `deny-all`. |
| `--format <fmt>` | Output format | `text` (default), `json`, `quiet`. |
| `--suppress-reads` | Suppress read file contents | Replaces raw read payloads with `[read output suppressed]`. |
| `--json-strict` | Strict JSON mode | Requires `--format json`; suppresses non-JSON stderr output. |
| `--no-terminal` | Disable ACP terminal capability | Advertises `clientCapabilities.terminal: false` during ACP initialize for new agent clients. |
| `--non-interactive-permissions <policy>` | Non-TTY prompt policy | `deny` (default) or `fail` when approval prompt cannot be shown. |
| `--timeout <seconds>` | Max wait time for agent response | Must be positive. Decimal seconds allowed. |
| `--ttl <seconds>` | Queue owner idle TTL before shutdown | Default `300`. `0` disables TTL. |
| `--model <id>` | Set agent model | Claude-compatible adapters may consume session creation metadata; other agents must advertise ACP models and support `session/set_model`, otherwise `acpx` fails clearly instead of silently falling back. |
| `--verbose` | Enable verbose logs | Prints ACP/debug details to stderr. |
Permission flags are mutually exclusive. Using more than one of `--approve-all`, `--approve-reads`, `--deny-all` is a usage error.

View File

@ -155,7 +155,7 @@ Behavior:
- `set-mode` mode ids are adapter-defined; unsupported values are rejected by the adapter (often `Invalid params`).
- `set`: calls ACP `session/set_config_option`.
- For codex, `thought_level` is accepted as a compatibility alias for codex-acp `reasoning_effort`.
- `--model <id>`: passed through to agent-specific session creation metadata when applicable; if the agent advertises models, `acpx` also applies it via `session/set_model`.
- `--model <id>`: Claude-compatible adapters may consume session creation metadata; other agents must advertise ACP models and support `session/set_model`, otherwise `acpx` fails clearly instead of silently falling back.
- `set model <id>`: calls `session/set_model`. This is the generic ACP method for mid-session model switching.
- `set-mode`/`set` route through queue-owner IPC when active, otherwise reconnect directly.
@ -202,7 +202,7 @@ Behavior:
- `--suppress-reads`: suppress raw read-file contents while preserving the selected format
- `--timeout <seconds>`: max wait time (positive number)
- `--ttl <seconds>`: queue owner idle TTL before shutdown (default `300`, `0` disables TTL)
- `--model <id>`: request an agent model during session creation; when the agent advertises models, `acpx` also applies it via `session/set_model`
- `--model <id>`: request an agent model during session creation; non-Claude agents must advertise ACP models and support `session/set_model`
- `--verbose`: verbose ACP/debug logs to stderr
Permission flags are mutually exclusive.

51
src/acp/model-support.ts Normal file
View File

@ -0,0 +1,51 @@
import type { SessionModelState } from "@agentclientprotocol/sdk";
import { isClaudeAcpCommand } from "./agent-command.js";
import { splitCommandLine } from "./client-process.js";
export class RequestedModelUnsupportedError extends Error {
constructor(message: string) {
super(message);
this.name = "RequestedModelUnsupportedError";
}
}
export function supportsLegacyClaudeCodeModelMetadata(agentCommand: string | undefined): boolean {
if (!agentCommand) {
return false;
}
const { command, args } = splitCommandLine(agentCommand);
return isClaudeAcpCommand(command, args);
}
export function formatAvailableModelIds(models: SessionModelState | undefined): string {
const ids =
models?.availableModels
.map((model) => model.modelId.trim())
.filter((modelId) => modelId.length > 0) ?? [];
return ids.length > 0 ? ids.join(", ") : "none advertised";
}
export function assertRequestedModelSupported(params: {
requestedModel: string;
models: SessionModelState | undefined;
agentCommand?: string;
context: "apply" | "replay";
}): void {
if (!params.models) {
if (supportsLegacyClaudeCodeModelMetadata(params.agentCommand)) {
return;
}
const action = params.context === "replay" ? "replay saved model" : "apply --model";
throw new RequestedModelUnsupportedError(
`Cannot ${action} "${params.requestedModel}": the ACP agent did not advertise model support. Generic model selection requires ACP models plus session/set_model support, or an adapter-specific startup model flag.`,
);
}
const advertised = new Set(params.models.availableModels.map((model) => model.modelId));
if (!advertised.has(params.requestedModel)) {
const action = params.context === "replay" ? "replay saved model" : "apply --model";
throw new RequestedModelUnsupportedError(
`Cannot ${action} "${params.requestedModel}": the ACP agent did not advertise that model. Available models: ${formatAvailableModelIds(params.models)}.`,
);
}
}

View File

@ -1,4 +1,5 @@
import type { AcpClient, SessionCreateResult } from "../../acp/client.js";
import { assertRequestedModelSupported } from "../../acp/model-support.js";
import { withTimeout } from "../../async-control.js";
export async function applyRequestedModelIfAdvertised(params: {
@ -6,11 +7,21 @@ export async function applyRequestedModelIfAdvertised(params: {
sessionId: string;
requestedModel: string | undefined;
models: SessionCreateResult["models"];
agentCommand?: string;
timeoutMs?: number;
}): Promise<boolean> {
const requestedModel =
typeof params.requestedModel === "string" ? params.requestedModel.trim() : "";
if (!requestedModel || !params.models) {
if (!requestedModel) {
return false;
}
assertRequestedModelSupported({
requestedModel,
models: params.models,
agentCommand: params.agentCommand,
context: "apply",
});
if (!params.models) {
return false;
}
if (params.models.currentModelId === requestedModel) {

View File

@ -4,6 +4,7 @@ import {
isRetryablePromptError,
normalizeOutputError,
} from "../../acp/error-normalization.js";
import { assertRequestedModelSupported } from "../../acp/model-support.js";
import { InterruptedError, withInterrupt, withTimeout } from "../../async-control.js";
export { InterruptedError, TimeoutError } from "../../async-control.js";
import { formatPerfMetric, measurePerf, startPerfTimer } from "../../perf-metrics.js";
@ -138,10 +139,26 @@ async function applyPromptModelIfAdvertised(params: {
}): Promise<void> {
const requestedModel =
typeof params.requestedModel === "string" ? params.requestedModel.trim() : "";
if (!requestedModel || !Array.isArray(params.record.acpx?.available_models)) {
if (!requestedModel) {
return;
}
if (params.record.acpx.current_model_id === requestedModel) {
const availableModels = params.record.acpx?.available_models;
assertRequestedModelSupported({
requestedModel,
models: Array.isArray(availableModels)
? {
currentModelId: params.record.acpx?.current_model_id ?? "",
availableModels: availableModels.map((modelId) => ({ modelId, name: modelId })),
}
: undefined,
agentCommand: params.record.agentCommand,
context: "apply",
});
if (!Array.isArray(availableModels)) {
return;
}
if (params.record.acpx?.current_model_id === requestedModel) {
setDesiredModelId(params.record, requestedModel);
return;
}
@ -737,6 +754,7 @@ export async function runOnce(options: RunOnceOptions): Promise<RunPromptResult>
sessionId,
requestedModel: options.sessionOptions?.model,
models: createdSession.models,
agentCommand: options.agentCommand,
timeoutMs: options.timeoutMs,
});

View File

@ -102,6 +102,7 @@ async function createSessionRecordWithClient(
sessionId,
requestedModel: options.sessionOptions?.model,
models: sessionModels,
agentCommand: options.agentCommand,
timeoutMs: options.timeoutMs,
});
} catch (error) {
@ -122,6 +123,7 @@ async function createSessionRecordWithClient(
sessionId,
requestedModel: options.sessionOptions?.model,
models: sessionModels,
agentCommand: options.agentCommand,
timeoutMs: options.timeoutMs,
});
}

View File

@ -5,6 +5,7 @@ import {
isAcpQueryClosedBeforeResponseError,
isAcpResourceNotFoundError,
} from "../../acp/error-normalization.js";
import { assertRequestedModelSupported } from "../../acp/model-support.js";
import { InterruptedError, TimeoutError, withTimeout } from "../../async-control.js";
import {
SessionConfigOptionReplayError,
@ -154,18 +155,25 @@ async function replayDesiredModel(params: {
sessionId: string;
desiredModelId: string | undefined;
previousSessionId: string;
record: SessionRecord;
models: import("../../acp/client.js").SessionLoadResult["models"] | undefined;
timeoutMs?: number;
verbose?: boolean;
}): Promise<void> {
if (!params.desiredModelId || !params.models) {
return;
}
if (params.models.currentModelId === params.desiredModelId) {
if (!params.desiredModelId) {
return;
}
try {
assertRequestedModelSupported({
requestedModel: params.desiredModelId,
models: params.models,
agentCommand: params.record.agentCommand,
context: "replay",
});
if (!params.models || params.models.currentModelId === params.desiredModelId) {
return;
}
await withTimeout(
params.client.setSessionModel(params.sessionId, params.desiredModelId),
params.timeoutMs,
@ -331,6 +339,7 @@ export async function connectAndLoadSession(
sessionId,
desiredModelId,
previousSessionId: originalSessionId,
record,
models: sessionModels,
timeoutMs: options.timeoutMs,
verbose: options.verbose,

View File

@ -634,6 +634,62 @@ test("connectAndLoadSession replays desired model on a fresh session", async ()
});
});
test("connectAndLoadSession fails clearly when saved model cannot be replayed generically", async () => {
await withTempHome(async (homeDir) => {
const cwd = path.join(homeDir, "workspace");
await fs.mkdir(cwd, { recursive: true });
const record = makeSessionRecord({
acpxRecordId: "model-replay-unsupported-record",
acpSessionId: "stale-session",
agentCommand: "agent",
cwd,
acpx: {
session_options: {
model: "gpt-5.4",
},
},
});
const client: FakeClient = {
hasReusableSession: () => false,
start: async () => {},
getAgentLifecycleSnapshot: () => ({
running: true,
}),
supportsLoadSession: () => false,
loadSessionWithOptions: async () => {
throw new Error("loadSessionWithOptions should not be called");
},
createSession: async () => ({
sessionId: "fresh-session",
agentSessionId: "fresh-runtime",
}),
setSessionMode: async () => {},
setSessionModel: async () => {
throw new Error("setSessionModel should not be called");
},
};
await assert.rejects(
async () =>
await connectAndLoadSession({
client: client as never,
record,
activeController: ACTIVE_CONTROLLER,
}),
(error: unknown) => {
assert(error instanceof Error);
assert.equal(error.name, "SessionModelReplayError");
assert.match(error.message, /did not advertise model support/);
return true;
},
);
assert.equal(record.acpSessionId, "stale-session");
});
});
test("connectAndLoadSession restores the original session when desired model replay fails", async () => {
await withTempHome(async (homeDir) => {
const cwd = path.join(homeDir, "workspace");

View File

@ -865,14 +865,22 @@ test("integration: qoder session reuse preserves persisted startup flags", async
test("integration: exec forwards model, allowed-tools, and max-turns in session/new _meta", async () => {
await withTempHome(async (homeDir) => {
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "acpx-integration-cwd-"));
const claudeCompatibleAgentCommand = `${MOCK_AGENT_COMMAND} --claude-agent-acp`;
try {
const created = await runCli([...baseAgentArgs(cwd), "sessions", "new"], homeDir);
const created = await runCli(
["--agent", claudeCompatibleAgentCommand, "--approve-all", "--cwd", cwd, "sessions", "new"],
homeDir,
);
assert.equal(created.code, 0, created.stderr);
const result = await runCli(
[
...baseAgentArgs(cwd),
"--agent",
claudeCompatibleAgentCommand,
"--approve-all",
"--cwd",
cwd,
"--format",
"json",
"--model",
@ -967,7 +975,7 @@ test("integration: exec --model calls session/set_model when agent advertises mo
});
});
test("integration: exec --model skips session/set_model when agent does not advertise models", async () => {
test("integration: exec --model fails when agent does not advertise models", async () => {
await withTempHome(async (homeDir) => {
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "acpx-integration-cwd-"));
@ -976,11 +984,11 @@ test("integration: exec --model skips session/set_model when agent does not adve
[...baseAgentArgs(cwd), "--format", "json", "--model", "sonnet", "exec", "echo hello"],
homeDir,
);
assert.equal(result.code, 0, result.stderr);
assert.notEqual(result.code, 0, "expected non-zero exit");
assert.match(`${result.stderr}\n${result.stdout}`, /did not advertise model support/);
const payloads = parseJsonRpcOutputLines(result.stdout);
// _meta.claudeCode.options.model should still be sent
const createRequest = payloads.find((payload) => payload.method === "session/new") as
| { params?: { _meta?: Record<string, unknown> } }
| undefined;
@ -998,6 +1006,41 @@ test("integration: exec --model skips session/set_model when agent does not adve
});
});
test("integration: exec --model rejects models not advertised by the agent", async () => {
await withTempHome(async (homeDir) => {
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "acpx-integration-cwd-"));
const modelAgentCommand = `${MOCK_AGENT_COMMAND} --advertise-models`;
try {
const result = await runCli(
[
"--agent",
modelAgentCommand,
"--approve-all",
"--cwd",
cwd,
"--format",
"json",
"--model",
"missing-model",
"exec",
"echo hello",
],
homeDir,
);
assert.notEqual(result.code, 0, "expected non-zero exit");
assert.match(`${result.stderr}\n${result.stdout}`, /did not advertise that model/);
assert.match(`${result.stderr}\n${result.stdout}`, /default-model, fast-model, smart-model/);
const payloads = parseJsonRpcOutputLines(result.stdout);
const setModelRequest = payloads.find((payload) => payload.method === "session/set_model");
assert.equal(setModelRequest, undefined, "session/set_model should not be called");
} finally {
await fs.rm(cwd, { recursive: true, force: true });
}
});
});
test("integration: prompt --model updates existing session model before prompt", async () => {
await withTempHome(async (homeDir) => {
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "acpx-integration-cwd-"));
@ -1056,7 +1099,7 @@ test("integration: exec --model fails when session/set_model fails", async () =>
"--format",
"quiet",
"--model",
"bad-model",
"fast-model",
"exec",
"echo hello",
],
@ -1085,7 +1128,7 @@ test("integration: sessions new --model fails when session/set_model fails", asy
"--cwd",
cwd,
"--model",
"bad-model",
"fast-model",
"sessions",
"new",
],
@ -1191,7 +1234,7 @@ test("integration: status shows model after session creation with --model", asyn
"--cwd",
cwd,
"--model",
"gpt-5.4",
"smart-model",
"sessions",
"new",
],
@ -1211,7 +1254,7 @@ test("integration: status shows model after session creation with --model", asyn
mode?: string;
availableModels?: string[];
};
assert.equal(statusPayload.model, "gpt-5.4");
assert.equal(statusPayload.model, "smart-model");
assert(Array.isArray(statusPayload.availableModels), "expected availableModels array");
// Check status text
@ -1220,7 +1263,7 @@ test("integration: status shows model after session creation with --model", asyn
homeDir,
);
assert.equal(statusText.code, 0, statusText.stderr);
assert.match(statusText.stdout, /model: gpt-5\.4/);
assert.match(statusText.stdout, /model: smart-model/);
} finally {
await fs.rm(cwd, { recursive: true, force: true });
}

View File

@ -381,6 +381,10 @@ function parseMockAgentOptions(argv: string[]): MockAgentOptions {
continue;
}
if (token === "--claude-agent-acp") {
continue;
}
if (token === "--load-replay-text") {
supportsLoadSession = true;
replayLoadSessionUpdates = true;