diff --git a/ts/packages/defaultAgentProvider/src/mcp/mcpSeed.ts b/ts/packages/defaultAgentProvider/src/mcp/mcpSeed.ts index 6df7fee63b..31832ddf43 100644 --- a/ts/packages/defaultAgentProvider/src/mcp/mcpSeed.ts +++ b/ts/packages/defaultAgentProvider/src/mcp/mcpSeed.ts @@ -22,18 +22,22 @@ function scriptCommand(serverScript: string): string | undefined { * config so shipped servers can be seeded through the dynamic source rather than * a separate hard-coded mechanism. * - * Returns `undefined` when the entry cannot be expressed as a static normalized - * config — specifically when its `serverScriptArgs` is an `ArgDefinitions` - * object (interactive per-instance arguments, e.g. the filesystem server's - * allowed directories). Those depend on runtime instance config and remain on - * the legacy provider until that flow is migrated. `resolveScriptPath` resolves - * a relative `serverScript` to an absolute path (the shipped scripts live under - * the package). + * Returns `undefined` when the entry cannot be expressed as a normalized + * config. Interactive `serverScriptArgs` require a runtime resolver; unresolved + * entries remain on the legacy provider. `resolveScriptPath` resolves a relative + * `serverScript` to an absolute path (the shipped scripts live under the package). */ export function mcpInfoToNormalized( name: string, info: McpAppAgentInfo, resolveScriptPath: (p: string) => string = (p) => p, + resolveServerArgs?: ( + name: string, + definitions: Exclude< + NonNullable, + string[] + >, + ) => string[] | undefined, ): NormalizedMcpServerConfig | undefined { const base: Pick< NormalizedMcpServerConfig, @@ -71,11 +75,12 @@ export function mcpInfoToNormalized( if (info.serverScript === undefined) { return undefined; } - // ArgDefinitions (interactive per-instance args) cannot be seeded statically. - if ( - info.serverScriptArgs !== undefined && - !Array.isArray(info.serverScriptArgs) - ) { + const serverScriptArgs = Array.isArray(info.serverScriptArgs) + ? info.serverScriptArgs + : info.serverScriptArgs === undefined + ? [] + : resolveServerArgs?.(name, info.serverScriptArgs); + if (serverScriptArgs === undefined) { return undefined; } const command = scriptCommand(info.serverScript); @@ -83,25 +88,37 @@ export function mcpInfoToNormalized( return undefined; } const scriptPath = resolveScriptPath(info.serverScript); - const args = [scriptPath, ...(info.serverScriptArgs ?? [])]; + const args = [scriptPath, ...serverScriptArgs]; return { ...base, transport: { kind: "stdio", command, args } }; } /** * Build the seed map of normalized shipped-server configs from the provider - * config's `mcpServers`. Entries that cannot be statically seeded (interactive - * arg definitions) are skipped and left to the legacy provider. + * config's `mcpServers`. Entries that cannot be seeded, including unresolved + * interactive argument definitions, are left to the legacy provider. */ export function buildMcpSeed( servers: Record | undefined, resolveScriptPath: (p: string) => string = (p) => p, + resolveServerArgs?: ( + name: string, + definitions: Exclude< + NonNullable, + string[] + >, + ) => string[] | undefined, ): Record { const seed: Record = {}; if (servers === undefined) { return seed; } for (const [name, info] of Object.entries(servers)) { - const normalized = mcpInfoToNormalized(name, info, resolveScriptPath); + const normalized = mcpInfoToNormalized( + name, + info, + resolveScriptPath, + resolveServerArgs, + ); if (normalized !== undefined) { seed[name] = normalized; } diff --git a/ts/packages/defaultAgentProvider/src/mcpDefaultAgentProvider.ts b/ts/packages/defaultAgentProvider/src/mcpDefaultAgentProvider.ts index 7ae302f077..b3897c3b60 100644 --- a/ts/packages/defaultAgentProvider/src/mcpDefaultAgentProvider.ts +++ b/ts/packages/defaultAgentProvider/src/mcpDefaultAgentProvider.ts @@ -18,19 +18,61 @@ import { import type { NormalizedMcpServerConfig } from "./mcp/mcpServerConfig.js"; import type { McpHostServices } from "./mcp/mcpServerProvider.js"; import type { McpConfigDiscoveryResult } from "./mcp/mcpConfigDiscovery.js"; +import fs from "node:fs"; +import path from "node:path"; import registerDebug from "debug"; const MCP_CLIENT_INFO = { name: "typeagent", version: "0.0.1" }; const debug = registerDebug("typeagent:mcp:discovery"); +const FILESYSTEM_SERVER_NAME = "mcpfilesystem"; let mcpAppAgentProvider: AppAgentProvider | undefined; -// The shipped-server seed the dynamic MCP source vends: every `data/config.json` -// mcpServers entry that can be expressed as a static normalized config. Entries -// with interactive arg definitions (e.g. the filesystem server) are NOT here — -// they stay on the legacy provider. Script paths resolve against the package. -function getShippedSeed(): Record { - return buildMcpSeed(getProviderConfig().mcpServers, getPackageFilePath); +function isDirectory(directory: string): boolean { + try { + return fs.statSync(directory).isDirectory(); + } catch { + return false; + } +} + +function resolveShippedServerArgs( + instanceConfigs: InstanceConfigProvider, + name: string, +): string[] | undefined { + if (name !== FILESYSTEM_SERVER_NAME) { + return undefined; + } + + const configured = + instanceConfigs.getInstanceConfig().mcpServers?.[name] + ?.serverScriptArgs; + const validConfigured = (configured ?? []) + .map((directory) => path.resolve(directory)) + .filter(isDirectory); + if (validConfigured.length > 0) { + return validConfigured; + } + + const instanceDir = instanceConfigs.getInstanceDir(); + if (instanceDir === undefined) { + return undefined; + } + const sandbox = path.join(instanceDir, "mcp-filesystem"); + fs.mkdirSync(sandbox, { recursive: true }); + return [sandbox]; +} + +function getShippedSeed( + instanceConfigs?: InstanceConfigProvider, +): Record { + return buildMcpSeed( + getProviderConfig().mcpServers, + getPackageFilePath, + instanceConfigs === undefined + ? undefined + : (name) => resolveShippedServerArgs(instanceConfigs, name), + ); } function initializeMcpAppAgentProvider( @@ -41,11 +83,7 @@ function initializeMcpAppAgentProvider( return undefined; } - // Hand every statically-convertible shipped server to the dynamic MCP - // source (see getMcpAppAgentSource); the legacy provider keeps only the ones - // that cannot be seeded (interactive arg definitions), so no name is - // registered by two providers. - const seeded = new Set(Object.keys(getShippedSeed())); + const seeded = new Set(Object.keys(getShippedSeed(instanceConfigs))); for (const name of Object.keys(servers)) { if (seeded.has(name)) { delete servers[name]; @@ -113,7 +151,7 @@ export function createMcpAppAgentSourceForInstance( "Internal error: MCP app agent source requires an instance directory.", ); } - const seed = { ...getShippedSeed(), ...runtimeSeed }; + const seed = { ...getShippedSeed(instanceConfigs), ...runtimeSeed }; // Reserve ALL shipped server names (both seeded and legacy) so the user // store can never register a name owned by another provider. const reserved = new Set([ diff --git a/ts/packages/defaultAgentProvider/test/mcpDefaultAgentProvider.spec.ts b/ts/packages/defaultAgentProvider/test/mcpDefaultAgentProvider.spec.ts new file mode 100644 index 0000000000..22e808312e --- /dev/null +++ b/ts/packages/defaultAgentProvider/test/mcpDefaultAgentProvider.spec.ts @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + createMcpAppAgentSourceForInstance, + getDefaultMcpAppAgentProvider, +} from "../src/mcpDefaultAgentProvider.js"; +import { getInstanceConfigProvider } from "../src/utils/config.js"; + +const tempDirs: string[] = []; + +function tmpDir(prefix: string): string { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.push(directory); + return directory; +} + +function tmpInstanceDir(): string { + return tmpDir("ta-mcp-default-"); +} + +function filesystemArgs(instanceDir: string) { + const configs = getInstanceConfigProvider(instanceDir); + const source = createMcpAppAgentSourceForInstance(configs); + const server = source.testApi.getServer("shipped:mcpfilesystem"); + if (server?.transport.kind !== "stdio") { + throw new Error("Expected a shipped stdio filesystem server"); + } + return server.transport.args ?? []; +} + +describe("default MCP filesystem server", () => { + afterEach(() => { + for (const directory of tempDirs.splice(0)) { + fs.rmSync(directory, { recursive: true, force: true }); + } + }); + + it("uses a dedicated per-instance sandbox and leaves the legacy provider", () => { + const instanceDir = tmpInstanceDir(); + const args = filesystemArgs(instanceDir); + const sandbox = path.join(instanceDir, "mcp-filesystem"); + + expect(args).toHaveLength(2); + expect(args[1]).toBe(sandbox); + expect(fs.statSync(sandbox).isDirectory()).toBe(true); + expect( + getDefaultMcpAppAgentProvider( + getInstanceConfigProvider(instanceDir), + ), + ).toBeUndefined(); + }); + + it("honors existing configured directories and ignores invalid ones", () => { + const instanceDir = tmpInstanceDir(); + const configuredRoot = tmpDir("ta-mcp-root-"); + const configs = getInstanceConfigProvider(instanceDir); + configs.setInstanceConfig({ + mcpServers: { + mcpfilesystem: { + serverScriptArgs: [ + path.join(instanceDir, "missing"), + configuredRoot, + ], + }, + }, + }); + + const source = createMcpAppAgentSourceForInstance(configs); + const server = source.testApi.getServer("shipped:mcpfilesystem"); + expect(server?.transport).toMatchObject({ + kind: "stdio", + args: [expect.any(String), configuredRoot], + }); + }); + + it("initializes the official server and lists its tools", async () => { + const instanceDir = tmpInstanceDir(); + const source = createMcpAppAgentSourceForInstance( + getInstanceConfigProvider(instanceDir), + ); + + const result = await source.testApi.testServer("shipped:mcpfilesystem"); + + expect(result.protocolVersion).toBeDefined(); + expect(result.tools).toEqual( + expect.arrayContaining(["read_file", "list_directory"]), + ); + }); +}); diff --git a/ts/packages/defaultAgentProvider/test/mcpSeed.spec.ts b/ts/packages/defaultAgentProvider/test/mcpSeed.spec.ts index 25f34ba20e..47bf09a8e5 100644 --- a/ts/packages/defaultAgentProvider/test/mcpSeed.spec.ts +++ b/ts/packages/defaultAgentProvider/test/mcpSeed.spec.ts @@ -81,6 +81,25 @@ describe("mcpInfoToNormalized", () => { expect(normalized).toBeUndefined(); }); + it("converts ArgDefinitions when runtime arguments are resolved", () => { + const normalized = mcpInfoToNormalized( + "fs", + info({ + serverScript: "server.js", + serverScriptArgs: { + dirs: { type: "string", description: "dirs" }, + } as any, + }), + (p) => `/abs/${p}`, + (name) => (name === "fs" ? ["/safe/root"] : undefined), + ); + expect(normalized?.transport).toEqual({ + kind: "stdio", + command: "node", + args: ["/abs/server.js", "/safe/root"], + }); + }); + it("returns undefined for a server with no url or script", () => { expect(mcpInfoToNormalized("empty", info({}))).toBeUndefined(); }); diff --git a/ts/packages/defaultAgentProvider/test/translateTestCommon.ts b/ts/packages/defaultAgentProvider/test/translateTestCommon.ts index 6bdf65e6c6..9c25e5bf8f 100644 --- a/ts/packages/defaultAgentProvider/test/translateTestCommon.ts +++ b/ts/packages/defaultAgentProvider/test/translateTestCommon.ts @@ -6,6 +6,7 @@ loadConfigSync(); import { getPackageFilePath } from "../src/utils/getPackageFilePath.js"; import { getDefaultAppAgentProviders } from "../src/defaultAgentProviders.js"; +import { createMcpAppAgentSourceForInstance } from "../src/mcpDefaultAgentProvider.js"; import { awaitCommand, CommandResult, @@ -222,6 +223,10 @@ export async function defineTranslateTest( const defaultAppAgentProviders = getDefaultAppAgentProviders( instanceConfigProvider, ); + const mcpSource = + instanceConfigProvider?.getInstanceDir() === undefined + ? undefined + : createMcpAppAgentSourceForInstance(instanceConfigProvider); const inputs: TranslateTestEntry[] = ( await Promise.all( dataFiles.map>(async (f) => { @@ -304,6 +309,9 @@ export async function defineTranslateTest( "cli test translate", { appAgentProviders: defaultAppAgentProviders, + ...(mcpSource === undefined + ? {} + : { appAgentSources: [mcpSource] }), agents: { actions: false, commands: ["dispatcher"], diff --git a/ts/packages/defaultAgentProvider/test/translate_mcpfs.test.ts b/ts/packages/defaultAgentProvider/test/translate_mcpfs.test.ts index a67f365fa1..10f0be53fa 100644 --- a/ts/packages/defaultAgentProvider/test/translate_mcpfs.test.ts +++ b/ts/packages/defaultAgentProvider/test/translate_mcpfs.test.ts @@ -21,7 +21,7 @@ try { } catch (e) {} await defineTranslateTest("translate mcp filesystem", dataFiles, { - getInstanceDir: () => undefined, + getInstanceDir: () => tempDir, getInstanceConfig: () => { return { mcpServers: {