fix(agents): validate optional MCP requirements before plugin injection

Keep optional approval decisions in the provider registry and preserve required plugin servers. Remove requirements alongside stripped internal servers so persistence cannot retain dangling metadata.
This commit is contained in:
Mohamed Boudra
2026-10-29 09:14:58 +01:00
parent 2325a2f32d
commit e2a2d1588d
4 changed files with 59 additions and 15 deletions
@@ -3850,15 +3850,12 @@ test.each([
clients: {
[provider]: client,
},
providerDefinitions:
provider === "pi"
? {
pi: {
enabled: true,
applyToolPolicy: buildProviderRegistry(logger).pi.applyToolPolicy,
},
}
: undefined,
providerDefinitions: {
[provider]: {
enabled: true,
applyToolPolicy: buildProviderRegistry(logger)[provider]!.applyToolPolicy,
},
},
registry: storage,
logger,
mcpBaseUrl: "http://127.0.0.1:6767/mcp/agents",
@@ -3898,8 +3895,12 @@ test.each([
expect(manager.getAgent(snapshot.id)?.config.mcpServerRequirements).toEqual({
paseo: "optional",
});
expect(snapshot.config.toolPolicy).toBeUndefined();
expect(client.lastConfig?.toolPolicy).toBeUndefined();
const expectedPolicy =
provider === "codex"
? { preapproved: [{ kind: "mcp", server: "paseo", tool: "finish_execution" }] }
: undefined;
expect(snapshot.config.toolPolicy).toEqual(expectedPolicy);
expect(client.lastConfig?.toolPolicy).toEqual(expectedPolicy);
expect(client.lastConfig?.mcpServers).toEqual(snapshot.config.mcpServers);
},
);
@@ -12608,3 +12609,21 @@ test("an out-of-band command racing idle cleanup waits for the resumed runtime",
expect(h.commands).toEqual(["/maintenance"]);
expect(h.prompts).toEqual([]);
});
test("MCP requirements cannot mark a later plugin server optional", async () => {
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-test-"));
const manager = new AgentManager({
clients: { codex: new TestAgentClient() },
registry: new AgentStorage(join(workdir, "agents"), logger),
logger,
});
await expect(
manager.createAgent(
{ provider: "codex", cwd: workdir, mcpServerRequirements: { plugin: "optional" } },
undefined,
{ workspaceId: undefined },
),
).rejects.toThrow(
"MCP server requirement 'plugin' requires MCP server 'plugin' in the same agent request",
);
});
@@ -5465,10 +5465,8 @@ export class AgentManager {
private applyProviderConfiguration(config: AgentSessionConfig): AgentSessionConfig {
const definition = this.providerDefinitions.get(config.provider);
this.validateToolPolicyServers(config);
this.validateMcpServerRequirements(config);
if (config.toolPolicy && !definition?.applyToolPolicy) {
if (config.toolPolicy.enforcement === "if-supported") {
return { ...config, toolPolicy: undefined };
}
throw new Error(
`Provider '${config.provider}' cannot preapprove exact MCP tools for unattended execution`,
);
@@ -5478,6 +5476,16 @@ export class AgentManager {
: config;
}
private validateMcpServerRequirements(config: AgentSessionConfig): void {
for (const name of Object.keys(config.mcpServerRequirements ?? {})) {
if (!Object.hasOwn(config.mcpServers ?? {}, name)) {
throw new Error(
`MCP server requirement '${name}' requires MCP server '${name}' in the same agent request`,
);
}
}
}
private validateToolPolicyServers(config: AgentSessionConfig): void {
if (!config.toolPolicy) return;
const serverNames = new Set(Object.keys(config.mcpServers ?? {}));
@@ -1,7 +1,7 @@
import { describe, expect, test } from "vitest";
import type { AgentSessionConfig } from "./agent-sdk-types.js";
import { withRuntimePaseoMcpServer } from "./runtime-mcp-config.js";
import { stripInternalPaseoMcpServer, withRuntimePaseoMcpServer } from "./runtime-mcp-config.js";
const BASE_CONFIG: AgentSessionConfig = {
provider: "claude",
@@ -49,3 +49,15 @@ describe("withRuntimePaseoMcpServer", () => {
expect(result.mcpServers).toBeUndefined();
});
});
test("stripping an internal MCP server also strips its launch requirement", () => {
const result = stripInternalPaseoMcpServer({
...BASE_CONFIG,
mcpServerRequirements: { paseo: "optional", hub: "optional" },
mcpServers: {
paseo: { type: "http", url: "http://127.0.0.1/mcp/agents?callerAgentId=agent-1" },
hub: { type: "http", url: "https://hub.test/mcp" },
},
});
expect(result.mcpServerRequirements).toEqual({ hub: "optional" });
});
@@ -18,6 +18,11 @@ export function stripInternalPaseoMcpServer(config: AgentSessionConfig): AgentSe
delete nextMcpServers[PASEO_MCP_SERVER_NAME];
const next = { ...config };
if (next.mcpServerRequirements?.[PASEO_MCP_SERVER_NAME]) {
next.mcpServerRequirements = { ...next.mcpServerRequirements };
delete next.mcpServerRequirements[PASEO_MCP_SERVER_NAME];
if (Object.keys(next.mcpServerRequirements).length === 0) delete next.mcpServerRequirements;
}
if (Object.keys(nextMcpServers).length > 0) {
next.mcpServers = nextMcpServers;
} else {