From c47e1f178b5b37f182a5c7b92e0269c361f82c14 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 29 Oct 2026 08:51:20 +0100 Subject: [PATCH] fix(providers): allow optional exact MCP preapproval Separate automatic approval convenience from strict tool-policy requirements so Hub can launch any available provider. Unsupported providers retain ordinary MCP injection and native permissions; strict requests still fail closed. Co-authored-by: Ryan Winograd --- packages/protocol/src/agent-types.ts | 2 ++ packages/protocol/src/messages.test.ts | 35 +++++++++++++++++++ packages/protocol/src/messages.ts | 4 ++- .../src/server/agent/agent-manager.test.ts | 8 ++++- .../server/src/server/agent/agent-manager.ts | 3 ++ .../src/server/agent/provider-options.test.ts | 23 ++++++++++++ .../src/server/agent/provider-registry.ts | 9 +++-- 7 files changed, 79 insertions(+), 5 deletions(-) diff --git a/packages/protocol/src/agent-types.ts b/packages/protocol/src/agent-types.ts index c7fffe8ce9..8a573b6434 100644 --- a/packages/protocol/src/agent-types.ts +++ b/packages/protocol/src/agent-types.ts @@ -509,6 +509,8 @@ export interface McpToolRef { } export interface ToolPolicy { + /** Missing or required fails closed; if-supported requests approval convenience only. */ + enforcement?: "required" | "if-supported"; preapproved: McpToolRef[]; } diff --git a/packages/protocol/src/messages.test.ts b/packages/protocol/src/messages.test.ts index 373cef409b..8b0617f75c 100644 --- a/packages/protocol/src/messages.test.ts +++ b/packages/protocol/src/messages.test.ts @@ -571,3 +571,38 @@ test.each(["all", "base", "uncommitted"] as const)( expect(parsed.attachments).toEqual([attachment]); }, ); + +test("native agent creation accepts optional approval without weakening legacy strict acknowledgement", () => { + const toolPolicy = { + preapproved: [{ kind: "mcp", server: "hub", tool: "finish_execution" }], + enforcement: "if-supported", + }; + const parsed = SessionInboundMessageSchema.parse({ + type: "create_agent_request", + requestId: "req", + config: { provider: "pi", cwd: "/tmp", toolPolicy }, + }); + expect(parsed).toMatchObject({ config: { toolPolicy } }); + expect( + SessionInboundMessageSchema.safeParse({ + type: "hub.execution.agent.create.request", + requestId: "req", + executionId: "exec", + provider: "pi", + cwd: "/tmp", + prompt: "test", + toolPolicy: { preapproved: toolPolicy.preapproved }, + }).success, + ).toBe(true); + expect( + SessionInboundMessageSchema.safeParse({ + type: "hub.execution.agent.create.request", + requestId: "req", + executionId: "exec", + provider: "pi", + cwd: "/tmp", + prompt: "test", + toolPolicy, + }).success, + ).toBe(false); +}); diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index d20c91d773..d853230848 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -490,7 +490,9 @@ const AgentSessionConfigSchema = z.object({ featureValues: z.record(z.string(), z.unknown()).optional(), title: z.string().trim().min(1).max(MAX_EXPLICIT_AGENT_TITLE_CHARS).optional().nullable(), providerOptions: ProviderOptionsSchema.optional(), - toolPolicy: ToolPolicySchema.optional(), + toolPolicy: ToolPolicySchema.extend({ + enforcement: z.enum(["required", "if-supported"]).optional(), + }).optional(), systemPrompt: z.string().optional(), mcpServers: z.record(z.string(), McpServerConfigSchema).optional(), }); diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index bb521abc71..c3cb539786 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -3822,7 +3822,7 @@ test("resumeAgentFromPersistence drops stored internal paseo MCP when runtime in expect(snapshot.config.mcpServers).toBeUndefined(); }); -test("createAgent preserves a user-provided paseo MCP config", async () => { +test("createAgent preserves injected MCP when optional approval is unavailable", async () => { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-test-")); const storagePath = join(workdir, "agents"); const storage = new AgentStorage(storagePath, logger); @@ -3851,6 +3851,10 @@ test("createAgent preserves a user-provided paseo MCP config", async () => { { provider: "codex", cwd: workdir, + toolPolicy: { + enforcement: "if-supported", + preapproved: [{ kind: "mcp", server: "paseo", tool: "finish_execution" }], + }, mcpServers: { paseo: { type: "http", @@ -3868,6 +3872,8 @@ test("createAgent preserves a user-provided paseo MCP config", async () => { url: "https://example.com/custom-paseo", }, }); + expect(snapshot.config.toolPolicy).toBeUndefined(); + expect(client.lastConfig?.toolPolicy).toBeUndefined(); expect(client.lastConfig?.mcpServers).toEqual(snapshot.config.mcpServers); }); diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index ae9e424d78..b7d8e934b3 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -5458,6 +5458,9 @@ export class AgentManager { const definition = this.providerDefinitions.get(config.provider); this.validateToolPolicyServers(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`, ); diff --git a/packages/server/src/server/agent/provider-options.test.ts b/packages/server/src/server/agent/provider-options.test.ts index f5b97d064a..f9bfc35f4a 100644 --- a/packages/server/src/server/agent/provider-options.test.ts +++ b/packages/server/src/server/agent/provider-options.test.ts @@ -135,6 +135,29 @@ describe("provider-owned option schemas", () => { }); describe("exact MCP preapproval mappings", () => { + test("optional exact preapproval permits unsupported providers without changing their MCP config", () => { + const registry = buildProviderRegistry(createTestLogger()); + const config = { + provider: "pi", + cwd: "/tmp", + mcpServers: { hub: { type: "http" as const, url: "http://localhost/hub" } }, + }; + expect( + registry.pi.applyToolPolicy(config, { ...hubPolicy, enforcement: "if-supported" }), + ).toEqual({ ...config, toolPolicy: undefined }); + }); + test.each(["claude", "codex", "opencode"])( + "optional grants retain exact approval for %s", + (provider) => { + const registry = buildProviderRegistry(createTestLogger()); + const config = { provider, cwd: "/tmp" }; + expect( + registry[provider]!.applyToolPolicy(config, { ...hubPolicy, enforcement: "if-supported" }) + .toolPolicy, + ).toEqual(hubPolicy); + }, + ); + test("unsupported providers fail closed with an unattended-execution error", () => { const registry = buildProviderRegistry(createTestLogger()); expect(() => diff --git a/packages/server/src/server/agent/provider-registry.ts b/packages/server/src/server/agent/provider-registry.ts index 0f3c39868a..f082324657 100644 --- a/packages/server/src/server/agent/provider-registry.ts +++ b/packages/server/src/server/agent/provider-registry.ts @@ -652,12 +652,15 @@ function createRegistryEntry( supportsExactMcpPreapproval: resolved.contract.supportsExactMcpPreapproval, applyToolPolicy: (config, toolPolicy) => { if (toolPolicy && !resolved.contract.supportsExactMcpPreapproval) { - throw new ToolPolicyUnsupportedError(provider); + if (toolPolicy.enforcement !== "if-supported") + throw new ToolPolicyUnsupportedError(provider); + return { ...config, toolPolicy: undefined }; } + const exactPolicy = toolPolicy ? { preapproved: toolPolicy.preapproved } : undefined; return { ...config, - toolPolicy: toolPolicy - ? (resolved.contract.applyToolPolicy?.(provider, toolPolicy) ?? toolPolicy) + toolPolicy: exactPolicy + ? (resolved.contract.applyToolPolicy?.(provider, exactPolicy) ?? exactPolicy) : undefined, }; },