mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-14 13:22:37 +00:00
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>
This commit is contained in:
@@ -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[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -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<string, string> = {},
|
||||
) {
|
||||
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<typeof buildRegistry>,
|
||||
model: string,
|
||||
): Promise<AgentSession> {
|
||||
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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, string>;
|
||||
waitForInitialCommands?: boolean;
|
||||
@@ -979,6 +981,7 @@ export class ACPAgentClient implements AgentClient {
|
||||
async createSession(
|
||||
config: AgentSessionConfig,
|
||||
launchContext?: AgentLaunchContext,
|
||||
options?: AgentCreateSessionOptions,
|
||||
): Promise<AgentSession> {
|
||||
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<string>;
|
||||
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)) {
|
||||
|
||||
Reference in New Issue
Block a user