diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8918d536e2..ea5e66d91c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -937,6 +937,8 @@ jobs: tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts tests/boundary/mcp/rawBinaryMcp.test.ts tests/boundary/mcp/advertisedDialect.test.ts + tests/boundary/mcp/absoluteLocalPathSchema.test.ts + tests/boundary/mcp/toolSchemaValidity.test.ts tests/boundary/filesystem/safeOutputTree.test.ts tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index e069dc8e85..69e27e7566 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -279,6 +279,11 @@ const PATH_SCOPES = { "tests/boundary/mcp/rawBinaryMcp.test.ts", "src/contracts/advertisedDialect*.ts", "tests/boundary/mcp/advertisedDialect.test.ts", + // The advertised absolute-path pattern is host-specific (#7151). + "src/domain/localPath*.ts", + "tests/boundary/mcp/absoluteLocalPathSchema.test.ts", + // Live advertisement, independent of the POSIX-generated catalog. + "tests/boundary/mcp/toolSchemaValidity.test.ts", "tests/boundary/filesystem/safeOutputTree.test.ts", "tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts", "tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts", diff --git a/scripts/lib/development-tests.mjs b/scripts/lib/development-tests.mjs index 22fcc23559..66e7a7af01 100644 --- a/scripts/lib/development-tests.mjs +++ b/scripts/lib/development-tests.mjs @@ -46,7 +46,7 @@ const artifactConsumers = { "artifacts:mcp-catalog": [ "tests/boundary/mcp/deferredRuntime.test.ts", "tests/boundary/mcp/contractPresentation.test.ts", - "tests/boundary/mcp/toolSchemaValidity.test.ts", + "tests/boundary/mcp/generatedToolCatalog.test.ts", ], "artifacts:product-catalog": [ "tests/boundary/filesystem/documentationFacts.test.ts", diff --git a/tests/boundary/mcp/generatedToolCatalog.test.ts b/tests/boundary/mcp/generatedToolCatalog.test.ts new file mode 100644 index 0000000000..7e05f238e5 --- /dev/null +++ b/tests/boundary/mcp/generatedToolCatalog.test.ts @@ -0,0 +1,62 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { McpServer } from "@modelcontextprotocol/server"; +import { expect, it } from "vitest"; + +import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; +import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; +import { + advertisedSchemaErrors, + localInputReferences, + recursiveReferences, +} from "../../fixtures/mcpAdvertisedSchemaValidity.js"; +import { GENERATED_MCP_TOOL_CATALOG } from "../../fixtures/mcpToolCatalog.js"; + +// The generated catalog is built on the POSIX build host; live advertisement +// checks that must also hold on Windows stay in toolSchemaValidity.test.ts. + +it("ships nonrecursive input and output schemas in the generated catalog", () => { + expect(advertisedSchemaErrors(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); + expect(recursiveReferences(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); +}); + +it("ships input schemas without local references in the generated catalog", () => { + expect(localInputReferences(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); +}); + +it("generates the catalog the MCP server advertises", async () => { + const server = new McpServer({ name: "generated-catalog", version: "0" }); + const client = new Client({ name: "generated-catalog", version: "0" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + for (const contract of TOOL_CONTRACTS) + server.registerTool( + contract.name, + toolRegistrationOptions(contract), + async () => ({ content: [] }), + ); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + const { tools } = await client.listTools(); + const project = (tool: (typeof tools)[number]) => ({ + name: tool.name, + title: tool.title, + description: tool.description, + inputSchema: tool.inputSchema, + outputSchema: tool.outputSchema, + annotations: tool.annotations, + }); + expect(tools.map(project)).toEqual( + GENERATED_MCP_TOOL_CATALOG.map((tool) => ({ + name: tool.name, + title: tool.title, + description: tool.description, + inputSchema: tool.inputSchema, + outputSchema: tool.outputSchema, + annotations: tool.annotations, + })), + ); + } finally { + await Promise.allSettled([client.close(), server.close()]); + } +}); diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index 9d81bb7b02..063bfbfd0b 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -5,20 +5,19 @@ import { describe, expect, it } from "vitest"; import { z } from "zod"; import { advertisedInputExamples } from "../../../src/contracts/advertisedInputExamples.js"; -import { - schemaArrays as schemaArrayKeywords, - schemaChildren as schemaChildKeywords, - schemaMaps as schemaMapKeywords, -} from "../../../src/contracts/jsonSchemaKeywords.js"; import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; import { emptyArraySchema } from "../../../src/domain/emptyArraySchema.js"; import { processScenarioSchema } from "../../../src/domain/process/processScenario.js"; -import { GENERATED_MCP_TOOL_CATALOG } from "../../fixtures/mcpToolCatalog.js"; import { arrayItemSchemaError, unsupportedInputLiteralSchemaError, } from "../../fixtures/mcpClientSchemaConstraints.js"; -import { advertisedSchemaErrors } from "../../fixtures/mcpAdvertisedSchemaValidity.js"; +import { + advertisedSchemaErrors, + localInputReferences, + recursiveReferences, + resolveReference, +} from "../../fixtures/mcpAdvertisedSchemaValidity.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; import { servedCatalog } from "../../../src/application/environment/environmentState.js"; import { servedFrom } from "../../fixtures/environmentStateServing.js"; @@ -208,23 +207,6 @@ function expectMeaningfulPropertyDescriptions( const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); -function resolveReference(schema: unknown, reference: string): unknown { - if (reference === "#") return schema; - if (!reference.startsWith("#/")) return undefined; - let value = schema; - for (const token of reference.slice(2).split("/")) { - const key = token.replaceAll("~1", "/").replaceAll("~0", "~"); - if ( - typeof value !== "object" || - value === null || - !Object.hasOwn(value, key) - ) - return undefined; - value = Reflect.get(value, key); - } - return value; -} - /** Follow SDK-shared local definitions at each schema path component. */ function schemaAt( root: unknown, @@ -307,66 +289,6 @@ function expectManagedLocalSignatureSchema( expect(validateLocals({ has_context_dependent_types: false })).toBe(false); } -function recursiveReferences(tools: readonly ToolSchemas[]): string[] { - return tools.flatMap((tool) => - (["inputSchema", "outputSchema"] as const).flatMap((kind) => { - const schema = tool[kind]; - const found = new Set(); - const visit = (node: unknown, active: readonly string[]): void => { - if (typeof node !== "object" || node === null) return; - for (const [key, child] of Object.entries(node)) { - if ( - [ - "$defs", - "definitions", - "examples", - "default", - "const", - "enum", - ].includes(key) - ) - continue; - if (key !== "$ref" || typeof child !== "string") visit(child, active); - else if (active.includes(child)) found.add(child); - else visit(resolveReference(schema, child), [...active, child]); - } - }; - visit(schema, []); - return [...found].map( - (reference) => `${tool.name}.${kind}: ${reference}`, - ); - }), - ); -} - -function localInputReferences(tools: readonly ToolSchemas[]): string[] { - return tools.flatMap((tool) => { - const references: string[] = []; - const visit = (node: unknown, path: string): void => { - if (!isRecord(node)) return; - if (typeof node.$ref === "string") references.push(`${path}.$ref`); - for (const [key, value] of Object.entries(node)) { - if (schemaMapKeywords.has(key) && isRecord(value)) { - for (const [name, schema] of Object.entries(value)) - visit(schema, `${path}.${key}.${name}`); - } else if (schemaArrayKeywords.has(key) && Array.isArray(value)) { - value.forEach((schema, index) => - visit(schema, `${path}.${key}[${index}]`), - ); - } else if (schemaChildKeywords.has(key)) { - if (Array.isArray(value)) - value.forEach((schema, index) => - visit(schema, `${path}.${key}[${index}]`), - ); - else visit(value, `${path}.${key}`); - } - } - }; - visit(tool.inputSchema, `${tool.name}.inputSchema`); - return references; - }); -} - it("finds references only in input schema positions, not literal data", () => { const schema = { type: "object", @@ -575,24 +497,6 @@ describe("MCP JSON Schema validity", () => { ), }, ); - const catalogProjection = tools.map((tool) => ({ - name: tool.name, - title: tool.title, - description: tool.description, - inputSchema: tool.inputSchema, - outputSchema: tool.outputSchema, - annotations: tool.annotations, - })); - expect(catalogProjection).toEqual( - GENERATED_MCP_TOOL_CATALOG.map((tool) => ({ - name: tool.name, - title: tool.title, - description: tool.description, - inputSchema: tool.inputSchema, - outputSchema: tool.outputSchema, - annotations: tool.annotations, - })), - ); for (const contract of TOOL_CONTRACTS) { const tool = byName.get(contract.name); expect(tool?.title?.trim(), contract.name).toBeTruthy(); @@ -885,15 +789,6 @@ describe("MCP process input JSON Schema", () => { }); }); -it("ships nonrecursive input and output schemas in the generated catalog", () => { - expect(advertisedSchemaErrors(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); - expect(recursiveReferences(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); -}); - -it("ships input schemas without local references in the generated catalog", () => { - expect(localInputReferences(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); -}); - it("rejects unresolved output-schema references even for uncalled tools", () => { const errors = advertisedSchemaErrors([ { diff --git a/tests/fixtures/mcpAdvertisedSchemaValidity.ts b/tests/fixtures/mcpAdvertisedSchemaValidity.ts index 30f76b9631..f7d50018e7 100644 --- a/tests/fixtures/mcpAdvertisedSchemaValidity.ts +++ b/tests/fixtures/mcpAdvertisedSchemaValidity.ts @@ -1,5 +1,10 @@ import { Ajv2020 } from "ajv/dist/2020.js"; +import { + schemaArrays as schemaArrayKeywords, + schemaChildren as schemaChildKeywords, + schemaMaps as schemaMapKeywords, +} from "../../src/contracts/jsonSchemaKeywords.js"; import { arrayItemSchemaError, unsupportedInputLiteralSchemaError, @@ -65,3 +70,90 @@ export function advertisedSchemaErrors( }), ); } + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +/** Resolve a local JSON Pointer reference inside one schema document. */ +export function resolveReference(schema: unknown, reference: string): unknown { + if (reference === "#") return schema; + if (!reference.startsWith("#/")) return undefined; + let value = schema; + for (const token of reference.slice(2).split("/")) { + const key = token.replaceAll("~1", "/").replaceAll("~0", "~"); + if ( + typeof value !== "object" || + value === null || + !Object.hasOwn(value, key) + ) + return undefined; + value = Reflect.get(value, key); + } + return value; +} + +/** Local `$ref` cycles in each advertised schema; clients reject recursion. */ +export function recursiveReferences( + tools: readonly AdvertisedToolSchemas[], +): string[] { + return tools.flatMap((tool) => + (["inputSchema", "outputSchema"] as const).flatMap((kind) => { + const schema = tool[kind]; + const found = new Set(); + const visit = (node: unknown, active: readonly string[]): void => { + if (typeof node !== "object" || node === null) return; + for (const [key, child] of Object.entries(node)) { + if ( + [ + "$defs", + "definitions", + "examples", + "default", + "const", + "enum", + ].includes(key) + ) + continue; + if (key !== "$ref" || typeof child !== "string") visit(child, active); + else if (active.includes(child)) found.add(child); + else visit(resolveReference(schema, child), [...active, child]); + } + }; + visit(schema, []); + return [...found].map( + (reference) => `${tool.name}.${kind}: ${reference}`, + ); + }), + ); +} + +/** Local `$ref`s in input schema positions, which some clients cannot resolve. */ +export function localInputReferences( + tools: readonly AdvertisedToolSchemas[], +): string[] { + return tools.flatMap((tool) => { + const references: string[] = []; + const visit = (node: unknown, path: string): void => { + if (!isRecord(node)) return; + if (typeof node.$ref === "string") references.push(`${path}.$ref`); + for (const [key, value] of Object.entries(node)) { + if (schemaMapKeywords.has(key) && isRecord(value)) { + for (const [name, schema] of Object.entries(value)) + visit(schema, `${path}.${key}.${name}`); + } else if (schemaArrayKeywords.has(key) && Array.isArray(value)) { + value.forEach((schema, index) => + visit(schema, `${path}.${key}[${index}]`), + ); + } else if (schemaChildKeywords.has(key)) { + if (Array.isArray(value)) + value.forEach((schema, index) => + visit(schema, `${path}.${key}[${index}]`), + ); + else visit(value, `${path}.${key}`); + } + } + }; + visit(tool.inputSchema, `${tool.name}.inputSchema`); + return references; + }); +} diff --git a/tests/process-global/curatedWindowsLane.test.ts b/tests/process-global/curatedWindowsLane.test.ts index cf31c7fd96..cdea873b03 100644 --- a/tests/process-global/curatedWindowsLane.test.ts +++ b/tests/process-global/curatedWindowsLane.test.ts @@ -117,6 +117,18 @@ describe("curated Windows test lane", () => { ); }); + it("keeps advertised schema checks in the lane", async () => { + // Windows hosts advertise their own absolute-path pattern (#7151); only + // this lane compiles it and validates host paths against it. + expect(await explicitPaths("windows")).toEqual( + expect.arrayContaining([ + "tests/boundary/mcp/advertisedDialect.test.ts", + "tests/boundary/mcp/absoluteLocalPathSchema.test.ts", + "tests/boundary/mcp/toolSchemaValidity.test.ts", + ]), + ); + }); + it("keeps Windows runner cancellation coverage in the lane", async () => { expect(await explicitPaths("windows")).toEqual( expect.arrayContaining([