From 98c284bb871a96710596dfdc2b9ea25d2400f6b2 Mon Sep 17 00:00:00 2001 From: Hao Zhou Date: Tue, 30 Jun 2026 15:02:32 -0400 Subject: [PATCH] fix: apply modelOverrides when resolving availableModels allowlist (#827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When `claude-agent-acp` runs against a Bedrock or Vertex deployment where the enterprise enforces **short model aliases** in `availableModels` (e.g. `claude-opus-4-6`), session creation fails immediately with: ``` API Error (claude-opus-4-6): 400 The provided model identifier is invalid. Try switching to us.anthropic.claude-opus-4-5-20251101-v1:0. ``` ### Root cause `applyAvailableModelsAllowlist` rewrites each allowlisted model's `value` to the user's literal entry (the short alias), and that value is what gets passed to `query.setModel()`. The provider API rejects short aliases — it requires provider-specific IDs such as `global.anthropic.claude-opus-4-6-v1[1m]`. The `modelOverrides` map — which exists precisely to translate aliases to provider IDs — was never consulted during allowlist resolution. So even with a correct `modelOverrides` configured (via `settings.json` or `CLAUDE_MODEL_CONFIG`), the alias still reached the API unchanged. This is reproducible on the Amazon-internal Toolbox Claude distribution, which regenerates `~/.claude/settings.json` on every launch with `availableModels` pinned to short aliases — so editing `settings.json` cannot work around it. ### Trace (with temporary debug logging) ``` settings.model = "global.anthropic.claude-opus-4-6-v1[1m]" (desired) getAvailableModels currentModel.value = "claude-opus-4-6" (WRONG) calling query.setModel("claude-opus-4-6") (rejected) ``` ## Fix `applyAvailableModelsAllowlist` now accepts `settings.modelOverrides`. When an allowlist alias has an override mapping, the **overridden provider ID** becomes the surfaced model value, so `setModel` passes an ID the provider accepts. After the fix: ``` getAvailableModels currentModel.value = "global.anthropic.claude-opus-4-6-v1[1m]" calling query.setModel("global.anthropic.claude-opus-4-6-v1[1m]") ``` and `acpx claude exec "what model are you using?"` responds normally: > I'm running on Claude Opus 4.6 with a 1M token context window. ## Testing - Added a regression test (`maps allowlist aliases through modelOverrides to provider IDs`) under the existing `availableModels allowlist from settings` suite. - Full suite: `411 passed | 17 skipped`. - `npm run check` (eslint + prettier) clean. - `tsc` build clean. - Manually verified end-to-end via `acpx` against a Bedrock-routed Toolbox Claude binary. --------- Co-authored-by: Hao Zhou Co-authored-by: Ben Brandt Co-authored-by: Claude Sonnet 5 --- src/acp-agent.ts | 25 +++++++++++--- src/tests/acp-agent-settings.test.ts | 49 ++++++++++++++++++++++++++++ src/tests/model-resolution.test.ts | 32 ++++++++++++++++++ 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 7314a158..05635391 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -3486,8 +3486,13 @@ export class ClaudeAcpAgent { // configOptions, the current-model resolver, and the stored modelInfos // consistent with what the user configured. const settingsAvailableModels = settingsManager.getSettings().availableModels; + const settingsModelOverrides = settingsManager.getSettings().modelOverrides; const allowedModels = Array.isArray(settingsAvailableModels) - ? applyAvailableModelsAllowlist(initializationResult.models, settingsAvailableModels) + ? applyAvailableModelsAllowlist( + initializationResult.models, + settingsAvailableModels, + settingsModelOverrides, + ) : initializationResult.models; const models = await getAvailableModels( @@ -4166,6 +4171,7 @@ function resolveSettingsModel( export function applyAvailableModelsAllowlist( sdkModels: ModelInfo[], allowlist: string[], + settingsModelOverrides?: Record, ): ModelInfo[] { // Default is always preserved per the docs. Synthesize one if the SDK // didn't surface it so downstream code (e.g. `getAvailableModels` picking @@ -4180,17 +4186,28 @@ export function applyAvailableModelsAllowlist( const sdkModelsWithoutDefault = sdkModels.filter((m) => m.value !== "default"); + // Bedrock/Vertex deployments enforce short aliases (e.g. "claude-opus-4-6") + // in availableModels but require provider-specific IDs at the API. We still + // resolve `sdkMatch` against the alias (`trimmed`) — that's what the + // matching heuristics above are built for, and override targets (ARNs, + // opaque provider IDs) often won't textually resemble anything in + // `sdkModelsWithoutDefault`. Only the entry's surfaced `value` becomes the + // override target, so it's what `setModel` ends up passing to the API. for (const entry of allowlist) { const trimmed = entry.trim(); if (!trimmed || seen.has(trimmed)) continue; + const overridden = settingsModelOverrides?.[trimmed]; + const effective = overridden ?? trimmed; + if (seen.has(effective)) continue; + const sdkMatch = resolveModelPreference(sdkModelsWithoutDefault, trimmed); if (sdkMatch) { - result.push({ ...sdkMatch, value: trimmed }); + result.push({ ...sdkMatch, value: effective }); } else { - result.push({ value: trimmed, displayName: trimmed, description: "" }); + result.push({ value: effective, displayName: trimmed, description: "" }); } - seen.add(trimmed); + seen.add(effective); } // The custom model option (ANTHROPIC_CUSTOM_MODEL_OPTION) is exempt from the diff --git a/src/tests/acp-agent-settings.test.ts b/src/tests/acp-agent-settings.test.ts index d000dc59..dec54ba8 100644 --- a/src/tests/acp-agent-settings.test.ts +++ b/src/tests/acp-agent-settings.test.ts @@ -811,6 +811,55 @@ describe("ClaudeAcpAgent settings", () => { } } }); + + it("maps allowlist aliases through modelOverrides to provider IDs", async () => { + // Bedrock/Vertex deployments enforce short aliases in availableModels + // but require provider-specific IDs at the API. Without applying + // modelOverrides during allowlist resolution, the alias is surfaced as + // the model value and passed verbatim to setModel, which the provider + // API rejects with a 400 "invalid model identifier". + await fs.promises.writeFile( + path.join(tempDir, "settings.json"), + JSON.stringify({ + availableModels: ["claude-opus-4-6", "claude-sonnet-4-6"], + model: "claude-opus-4-6", + modelOverrides: { + "claude-opus-4-6": "global.anthropic.claude-opus-4-6-v1[1m]", + "claude-sonnet-4-6": "global.anthropic.claude-sonnet-4-6[1m]", + }, + }), + ); + + const projectDir = path.join(tempDir, "project"); + await fs.promises.mkdir(projectDir, { recursive: true }); + + const { setModelSpy } = mockQueryWithModels([ + { value: "default", displayName: "Default", description: "Default model" }, + { value: "opus", displayName: "Opus", description: "Claude Opus 4.6" }, + { value: "sonnet", displayName: "Sonnet", description: "Claude Sonnet 4.6" }, + ]); + + const { ClaudeAcpAgent } = await import("../acp-agent.js"); + const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient()); + + const response = await (agent as any).createSession({ + cwd: projectDir, + mcpServers: [], + _meta: { disableBuiltInTools: true }, + }); + + const modelOption = response.configOptions.find((o: any) => o.id === "model"); + expect(modelOption.options.map((o: any) => o.value)).toEqual([ + "default", + "global.anthropic.claude-opus-4-6-v1[1m]", + "global.anthropic.claude-sonnet-4-6[1m]", + ]); + // The allowlist synthesized the overridden ID (the SDK only surfaced the + // `opus` alias), so the adapter syncs the provider ID via setModel — never + // the raw alias that Bedrock would reject. + expect(setModelSpy).toHaveBeenCalledWith("global.anthropic.claude-opus-4-6-v1[1m]"); + expect(modelOption.currentValue).toBe("global.anthropic.claude-opus-4-6-v1[1m]"); + }); }); it("resolves model aliases like opus[1m] to the correct model", async () => { diff --git a/src/tests/model-resolution.test.ts b/src/tests/model-resolution.test.ts index 67530fd0..10a7137a 100644 --- a/src/tests/model-resolution.test.ts +++ b/src/tests/model-resolution.test.ts @@ -79,6 +79,38 @@ describe("applyAvailableModelsAllowlist - resolvedModel matching", () => { }); }); +describe("applyAvailableModelsAllowlist - modelOverrides", () => { + const SDK_MODELS: ModelInfo[] = [ + { value: "default", displayName: "Default", description: "Default model" }, + { + value: "opus", + displayName: "Opus", + description: "Claude Opus 4.6", + supportsEffort: true, + supportedEffortLevels: ["low", "high"], + }, + ]; + + it("surfaces the override target as the value while keeping the alias's display metadata", () => { + // The override target is an opaque Bedrock ARN with no "opus" substring, + // so matching against it (instead of the "claude-opus-4-6" alias) would + // find nothing and silently drop displayName/description/supportsEffort. + const overrides = { + "claude-opus-4-6": "arn:aws:bedrock:us-west-2:111122223333:inference-profile/custom-7f3a", + }; + const result = applyAvailableModelsAllowlist(SDK_MODELS, ["claude-opus-4-6"], overrides); + + const entry = result.find((m) => m.value === overrides["claude-opus-4-6"]); + expect(entry).toEqual({ + value: overrides["claude-opus-4-6"], + displayName: "Opus", + description: "Claude Opus 4.6", + supportsEffort: true, + supportedEffortLevels: ["low", "high"], + }); + }); +}); + // Runs against the real SDK (no mocks) so the fixture above is flagged the // moment the SDK's actual model list shape drifts from it. Requires a usable // ANTHROPIC_API_KEY, hence opt-in via RUN_INTEGRATION_TESTS.