diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 429f92e9..05e93889 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -2125,7 +2125,13 @@ export class ClaudeAcpAgent implements Agent { ? applyAvailableModelsAllowlist(initializationResult.models, settingsAvailableModels) : initializationResult.models; - const models = await getAvailableModels(q, allowedModels, settingsManager, this.logger); + const models = await getAvailableModels( + q, + allowedModels, + initializationResult.models, + settingsManager, + this.logger, + ); // Gate `auto` (and future model-specific modes) on the resolved model's // `ModelInfo`. See `buildAvailableModes` for the canonical SDK signal. @@ -2609,12 +2615,14 @@ function applyAvailableModelsAllowlist(sdkModels: ModelInfo[], allowlist: string async function getAvailableModels( query: Query, models: ModelInfo[], + sdkModels: ModelInfo[], settingsManager: SettingsManager, logger: Logger, ): Promise { const settings = settingsManager.getSettings(); let currentModel = models[0]; + let resolvedFromInput: string | undefined; // Model priority (highest to lowest): // 1. ANTHROPIC_MODEL environment variable @@ -2624,15 +2632,33 @@ async function getAvailableModels( const match = resolveModelPreference(models, process.env.ANTHROPIC_MODEL); if (match) { currentModel = match; + resolvedFromInput = process.env.ANTHROPIC_MODEL; } - } else { + } else if (typeof settings.model === "string") { const match = resolveSettingsModel(models, settings.model, logger); if (match) { currentModel = match; + resolvedFromInput = settings.model; } } - await query.setModel(currentModel.value); + // Skip the setModel round-trip when we can prove the SDK has already landed + // on the same model. Two cases qualify: + // (a) No override applied — currentModel stayed at models[0]; the SDK is on + // its own default and we have nothing to sync. + // (b) The resolver returned the user's input verbatim AND that value exists + // in the SDK's original model list — meaning no fuzzy match or + // allowlist rewrite was involved, and the SDK (which reads the same + // ANTHROPIC_MODEL / settings.json) will have arrived at the same entry. + // Anything else (fuzzy match, allowlist-synthesized value, alias) gets a + // setModel call so we don't drift from the user's intended pin. + const sdkSawSameValue = sdkModels.some((m) => m.value === currentModel.value); + const skipSetModel = + resolvedFromInput === undefined || + (currentModel.value === resolvedFromInput && sdkSawSameValue); + if (!skipSetModel) { + await query.setModel(currentModel.value); + } return { availableModels: models.map((model) => ({ diff --git a/src/tests/acp-agent-settings.test.ts b/src/tests/acp-agent-settings.test.ts index 9d357783..14c9af04 100644 --- a/src/tests/acp-agent-settings.test.ts +++ b/src/tests/acp-agent-settings.test.ts @@ -222,7 +222,9 @@ describe("ClaudeAcpAgent settings", () => { }); // Bad model is ignored at the usage site; falls back to the first SDK model. - expect(setModelSpy).toHaveBeenCalledWith("claude-sonnet-4-6"); + // No setModel call is needed because no override was applied — the SDK is + // already on its own default. + expect(setModelSpy).not.toHaveBeenCalled(); expect(response.models.currentModelId).toBe("claude-sonnet-4-6"); }); @@ -687,4 +689,94 @@ describe("ClaudeAcpAgent settings", () => { expect(setModelSpy).toHaveBeenCalledWith("claude-opus-4-6-1m"); expect(response.models.currentModelId).toBe("claude-opus-4-6-1m"); }); + + it("skips the initial setModel when the resolved value matches the SDK's model list verbatim", async () => { + // Covers the launcher case from PR #646: the launcher bakes the model into + // ANTHROPIC_MODEL, the SDK already starts on that model, and a second + // setModel call would be a redundant round-trip (and on some launcher + // setups, more fragile than launch-time selection). + const originalEnv = process.env.ANTHROPIC_MODEL; + process.env.ANTHROPIC_MODEL = "claude-opus-4-6"; + + const projectDir = path.join(tempDir, "project"); + await fs.promises.mkdir(projectDir, { recursive: true }); + + const setModelSpy = vi.fn(); + querySpy.mockImplementation(() => { + return { + initializationResult: async () => ({ + models: [ + { value: "default", displayName: "Default", description: "" }, + { value: "claude-sonnet-4-6", displayName: "Claude Sonnet 4.6", description: "" }, + { value: "claude-opus-4-6", displayName: "Claude Opus 4.6", description: "" }, + ], + }), + setModel: setModelSpy, + supportedCommands: async () => [], + } as any; + }); + + try { + 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 }, + }); + + expect(setModelSpy).not.toHaveBeenCalled(); + expect(response.models.currentModelId).toBe("claude-opus-4-6"); + } finally { + if (originalEnv === undefined) { + delete process.env.ANTHROPIC_MODEL; + } else { + process.env.ANTHROPIC_MODEL = originalEnv; + } + } + }); + + it("still calls setModel when the allowlist synthesizes a value the SDK has not surfaced", async () => { + // The allowlist may rewrite a model's `value` to the user's literal ID + // (e.g., `claude-haiku-4-5`) even when the SDK only exposed an alias + // (`haiku`). In that case the SDK has not independently arrived at the + // user's preferred ID, so we must sync via setModel. + await fs.promises.writeFile( + path.join(tempDir, "settings.json"), + JSON.stringify({ + availableModels: ["claude-haiku-4-5"], + model: "claude-haiku-4-5", + }), + ); + + const projectDir = path.join(tempDir, "project"); + await fs.promises.mkdir(projectDir, { recursive: true }); + + const setModelSpy = vi.fn(); + querySpy.mockImplementation(() => { + return { + initializationResult: async () => ({ + models: [ + { value: "default", displayName: "Default", description: "" }, + { value: "haiku", displayName: "Haiku", description: "Fast" }, + ], + }), + setModel: setModelSpy, + supportedCommands: async () => [], + } as any; + }); + + 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 }, + }); + + expect(setModelSpy).toHaveBeenCalledWith("claude-haiku-4-5"); + expect(response.models.currentModelId).toBe("claude-haiku-4-5"); + }); });