From a162e1075f2ca83db38e8fd409cbe0bce0440e96 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:15:19 +0200 Subject: [PATCH] fix(acp): apply configured models the agent does not advertise (#6273) * fix(acp): apply configured models the agent does not advertise A model added through a provider's models or additionalModels was listed in the picker, but an ACP session only accepted ids the agent advertised in session/new, so the agent silently ran on its default model. The registry now passes configured model ids to the session, which applies them through session/set_model. Unconfigured, unadvertised ids are still skipped with a warning. * fix(acp): send configured models when the agent advertises none Also covers the resume path in the regression test. * test(acp): keep configured-model tests in the ACP agent suite --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com> --- .../src/server/agent/agent-sdk-types.ts | 7 + .../src/server/agent/provider-registry.ts | 5 +- .../server/agent/providers/acp-agent.test.ts | 152 +++++++++++++++++- .../src/server/agent/providers/acp-agent.ts | 54 ++++--- 4 files changed, 194 insertions(+), 24 deletions(-) diff --git a/packages/server/src/server/agent/agent-sdk-types.ts b/packages/server/src/server/agent/agent-sdk-types.ts index 3e5e739577..34d36c9e16 100644 --- a/packages/server/src/server/agent/agent-sdk-types.ts +++ b/packages/server/src/server/agent/agent-sdk-types.ts @@ -628,6 +628,11 @@ export interface AgentCreateSessionOptions { * Defaults to true. Providers that cannot honor false should no-op. */ persistSession?: boolean; + /** + * Model ids added by provider configuration (`models` / `additionalModels`). + * Providers that validate against runtime-advertised models must accept these. + */ + configuredModelIds?: readonly string[]; } /** What a resumed session is for: driving the agent, or reading what it already did. */ @@ -637,6 +642,8 @@ export type AgentResumePurpose = "interactive" | "history"; export interface AgentResumeSessionOptions { /** Defaults to interactive. History loading may be read-only for archived native sessions. */ purpose?: AgentResumePurpose; + /** See AgentCreateSessionOptions.configuredModelIds. */ + configuredModelIds?: readonly string[]; } /** diff --git a/packages/server/src/server/agent/provider-registry.ts b/packages/server/src/server/agent/provider-registry.ts index bd22d37df3..2698a3b2ac 100644 --- a/packages/server/src/server/agent/provider-registry.ts +++ b/packages/server/src/server/agent/provider-registry.ts @@ -492,6 +492,7 @@ function wrapClientProvider( const listFeatures = inner.listFeatures?.bind(inner); const archiveNativeSession = inner.archiveNativeSession?.bind(inner); const unarchiveNativeSession = inner.unarchiveNativeSession?.bind(inner); + const configuredModelIds = [...profileModels, ...additionalModels].map((model) => model.id); return { provider, @@ -509,7 +510,7 @@ function wrapClientProvider( providerOptions: mergeProviderOptions(providerOptions, config.providerOptions), }, launchContext, - options, + { ...options, configuredModelIds }, ), ), resumeSession: async (handle, overrides, launchContext, options) => @@ -526,7 +527,7 @@ function wrapClientProvider( providerOptions: mergeProviderOptions(providerOptions, overrides?.providerOptions), }, launchContext, - options, + { ...options, configuredModelIds }, ), ), fetchCatalog: async (options, context) => { diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 95d7e25a21..33c9696900 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -5,7 +5,7 @@ import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; -import { afterEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { AgentSideConnection, ClientSideConnection, @@ -54,8 +54,10 @@ import type { AgentStreamEvent } from "../agent-sdk-types.js"; import type { AgentCapabilityFlags, AgentPersistenceHandle, + AgentSession, ProviderRefreshContext, } from "../agent-sdk-types.js"; +import { buildProviderRegistry } from "../provider-registry.js"; import { createTestLogger } from "../../../test-utils/test-logger.js"; import { buildStringCommandShellInvocation } from "../../../utils/string-command-shell.js"; import { asInternals } from "../../test-utils/class-mocks.js"; @@ -4370,3 +4372,151 @@ describe("ACPAgentSession close() with an unresponsive provider", () => { expect(isProcessAlive(pid)).toBe(false); }, 15_000); }); + +// An ACP agent that advertises two models (or none, with ACP_ADVERTISE_MODELS=none) but, +// like Cline, runs any model id it is given. +const PERMISSIVE_MODEL_ACP_AGENT = ` +import { Readable, Writable } from "node:stream"; +const { AgentSideConnection, PROTOCOL_VERSION, ndJsonStream } = await import(process.env.ACP_SDK_URL); +let currentModelId = "advertised-default"; +const modelState = () => + process.env.ACP_ADVERTISE_MODELS === "none" + ? undefined + : { + currentModelId, + availableModels: [ + { modelId: "advertised-default", name: "Advertised default" }, + { modelId: "advertised-other", name: "Advertised other" }, + ], + }; +new AgentSideConnection( + () => ({ + async initialize() { + return { protocolVersion: PROTOCOL_VERSION, agentCapabilities: { loadSession: true } }; + }, + async newSession() { + return { sessionId: "permissive-session", models: modelState() }; + }, + async loadSession() { + return { models: modelState() }; + }, + async unstable_setSessionModel({ modelId }) { + currentModelId = modelId; + return {}; + }, + async authenticate() {}, + async cancel() {}, + async prompt() { + return { stopReason: "end_turn" }; + }, + }), + ndJsonStream(Writable.toWeb(process.stdout), Readable.toWeb(process.stdin)), +); +`; + +describe("custom ACP provider with configured models", () => { + let dir: string; + let agentScript: string; + let session: AgentSession | null = null; + + beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), "paseo-acp-configured-models-")); + agentScript = path.join(dir, "agent.mjs"); + await writeFile(agentScript, PERMISSIVE_MODEL_ACP_AGENT); + }); + + afterEach(async () => { + await session?.close(); + session = null; + await rm(dir, { recursive: true, force: true }); + }); + + function buildRegistry( + override: { + models?: Array<{ id: string; label: string }>; + additionalModels?: Array<{ id: string; label: string }>; + }, + agentEnv: Record = {}, + ) { + return buildProviderRegistry(createTestLogger(), { + providerOverrides: { + permissive: { + extends: "acp", + label: "Permissive", + command: [process.execPath, agentScript], + env: { + ACP_SDK_URL: pathToFileURL( + createRequire(import.meta.url).resolve("@agentclientprotocol/sdk"), + ).href, + ...agentEnv, + }, + ...override, + }, + }, + }); + } + + async function createSessionOnModel( + registry: ReturnType, + model: string, + ): Promise { + const client = registry.permissive.createClient(createTestLogger()); + session = await client.createSession({ provider: "permissive", cwd: dir, model }); + return session; + } + + test("runs on a model added through additionalModels that the agent does not advertise", async () => { + const registry = buildRegistry({ + additionalModels: [{ id: "stealth/pixel-canary", label: "Pixel Canary" }], + }); + + const created = await createSessionOnModel(registry, "stealth/pixel-canary"); + + expect((await created.getRuntimeInfo()).model).toBe("stealth/pixel-canary"); + }); + + test("runs on a model from a replacement models list that the agent does not advertise", async () => { + const registry = buildRegistry({ + models: [{ id: "stealth/pixel-canary", label: "Pixel Canary" }], + }); + + const created = await createSessionOnModel(registry, "stealth/pixel-canary"); + + expect((await created.getRuntimeInfo()).model).toBe("stealth/pixel-canary"); + }); + + test("runs on a model from additionalModels when the agent advertises no models", async () => { + const registry = buildRegistry( + { additionalModels: [{ id: "stealth/pixel-canary", label: "Pixel Canary" }] }, + { ACP_ADVERTISE_MODELS: "none" }, + ); + + const created = await createSessionOnModel(registry, "stealth/pixel-canary"); + + expect((await created.getRuntimeInfo()).model).toBe("stealth/pixel-canary"); + }); + + test("resumes on a model added through additionalModels that the agent does not advertise", async () => { + const registry = buildRegistry({ + additionalModels: [{ id: "stealth/pixel-canary", label: "Pixel Canary" }], + }); + const client = registry.permissive.createClient(createTestLogger()); + + session = await client.resumeSession( + { provider: "permissive", sessionId: "permissive-session", metadata: { cwd: dir } }, + { model: "stealth/pixel-canary" }, + ); + + expect((await session.getRuntimeInfo()).model).toBe("stealth/pixel-canary"); + }); + + test("keeps the agent's model when the requested model is neither advertised nor configured", async () => { + const registry = buildRegistry({ + additionalModels: [{ id: "stealth/pixel-canary", label: "Pixel Canary" }], + }); + + const created = await createSessionOnModel(registry, "stale/removed-model"); + + expect((await created.getRuntimeInfo()).model).toBe("advertised-default"); + }); +}); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index e5d1e4c48e..6bded49feb 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -77,6 +77,7 @@ import { type AgentPromptContentBlock, type AgentPromptInput, type AgentResumePurpose, + type AgentCreateSessionOptions, type AgentResumeSessionOptions, type AgentRunOptions, type AgentRunResult, @@ -512,6 +513,7 @@ interface ACPAgentSessionOptions { extensionCommandsParser?: ACPExtensionCommandsParser; handle?: AgentPersistenceHandle; resumePurpose?: AgentResumePurpose; + configuredModelIds?: readonly string[]; agentId?: string; launchEnv?: Record; waitForInitialCommands?: boolean; @@ -979,6 +981,7 @@ export class ACPAgentClient implements AgentClient { async createSession( config: AgentSessionConfig, launchContext?: AgentLaunchContext, + options?: AgentCreateSessionOptions, ): Promise { this.assertProvider(config); const providerOptions = ACPProviderOptionsSchema.parse(config.providerOptions ?? {}); @@ -1006,6 +1009,7 @@ export class ACPAgentClient implements AgentClient { supportsMcpServers: providerOptions.supportsMcpServers ?? this.capabilities.supportsMcpServers, }, + configuredModelIds: options?.configuredModelIds, agentId: launchContext?.agentId, launchEnv: launchContext?.env, extensionCommandsParser: this.extensionCommandsParser, @@ -1064,6 +1068,7 @@ export class ACPAgentClient implements AgentClient { }, handle, resumePurpose: options?.purpose, + configuredModelIds: options?.configuredModelIds, agentId: launchContext?.agentId, launchEnv: launchContext?.env, extensionCommandsParser: this.extensionCommandsParser, @@ -1692,6 +1697,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { configOptions: SessionConfigOption[], ) => SessionConfigOption[]; private readonly configFeatureOptions: ACPConfigFeatureOption[]; + private readonly configuredModelIds: ReadonlySet; private readonly clientCapabilities?: ACPClientCapabilities; private readonly clientCapabilityMeta?: ACPClientCapabilityMeta; private readonly modeIdTransformer?: (modeId: string) => string | null; @@ -1760,6 +1766,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.sessionResponseTransformer = options.sessionResponseTransformer; this.configOptionsTransformer = options.configOptionsTransformer; this.configFeatureOptions = options.configFeatureOptions ?? []; + this.configuredModelIds = new Set(options.configuredModelIds); this.clientCapabilities = options.clientCapabilities; this.clientCapabilityMeta = options.clientCapabilityMeta; this.modeIdTransformer = options.modeIdTransformer; @@ -2194,12 +2201,20 @@ export class ACPAgentSession implements AgentSession, ACPClient { return; } + await this.setModelWithSelection({ modelId, selection: this.resolveModelSelection(modelId) }); + } + + private resolveModelSelection(modelId: string): ACPModelSelection { const selection = resolveACPModelSelection({ modelId, availableModels: this.availableModels, configOptions: this.configOptions, }); - await this.setModelWithSelection({ modelId, selection }); + if (selection.availableModel || !this.configuredModelIds.has(modelId)) { + return selection; + } + // A model the user added in provider config is applied even when the agent does not advertise it. + return { ...selection, availableModel: { modelId, name: modelId } }; } private async setModelWithSelection({ @@ -2213,21 +2228,20 @@ export class ACPAgentSession implements AgentSession, ACPClient { throw new Error("ACP session not initialized"); } - if (selection.hasAvailableModels) { - if (!selection.availableModel) { - this.warnInvalidSelection( - modelId, - `is not a valid ${this.provider} model. Available options: ${this.availableModels - ?.map((model) => model.modelId) - .join(", ")}`, - ); - return; - } - - if (typeof this.connection.unstable_setSessionModel !== "function") { - throw new Error(this.modelSelectionUnavailableMessage()); - } + if (selection.hasAvailableModels && !selection.availableModel) { + this.warnInvalidSelection( + modelId, + `is not a valid ${this.provider} model. Available options: ${this.availableModels + ?.map((model) => model.modelId) + .join(", ")}`, + ); + return; + } + if ( + selection.availableModel && + typeof this.connection.unstable_setSessionModel === "function" + ) { try { await this.connection.unstable_setSessionModel({ sessionId: this.sessionId, @@ -2922,13 +2936,11 @@ export class ACPAgentSession implements AgentSession, ACPClient { const configuredModelId = this.config.model; let switchedModel = false; if (configuredModelId && configuredModelId !== this.currentModel) { - const selection = resolveACPModelSelection({ - modelId: configuredModelId, - availableModels: this.availableModels, - configOptions: this.configOptions, - }); try { - await this.setModelWithSelection({ modelId: configuredModelId, selection }); + await this.setModelWithSelection({ + modelId: configuredModelId, + selection: this.resolveModelSelection(configuredModelId), + }); switchedModel = true; } catch (error) { if (!this.isModelSelectionUnavailableError(error)) {