-
Notifications
You must be signed in to change notification settings - Fork 31
feat: support additional schemas via additionalSchemas parameter #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
21b04da
c0371bd
314d4d2
904b91b
0d437ca
e7aff85
3cc9931
006190c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import * as z from "zod/v4"; | |
| import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; | ||
| import { jsonResult, errorResult } from "../utils/response-formatter.js"; | ||
| import { createLogger } from "../utils/logger.js"; | ||
| import { SCHEMAS, VALID_SCHEMAS } from "../data/schemas/index.js"; | ||
| import { SCHEMAS, VALID_SCHEMAS, type SchemaEntry } from "../data/schemas/index.js"; | ||
| import { getExample, searchExamples, getExamplesForResource } from "../data/examples/index.js"; | ||
|
|
||
| const log = createLogger("tool:harness-schema"); | ||
|
|
@@ -96,10 +96,13 @@ function navigateToPath( | |
| */ | ||
| function getSummary(schema: Record<string, unknown>, resourceType: string): Record<string, unknown> { | ||
| const definitions = schema.definitions as Record<string, Record<string, unknown>> | undefined; | ||
| const sections = definitions ? Object.keys(definitions[resourceType] ?? {}) : []; | ||
|
|
||
| // Get the root resource definition | ||
| const rootDef = definitions?.[resourceType]?.[resourceType] as Record<string, unknown> | undefined; | ||
| // Harness-generated schemas nest the root definition under definitions[type][type]. | ||
| // Plain JSON Schemas (extension schemas) place properties at the root level. | ||
| const harnessRootDef = definitions?.[resourceType]?.[resourceType] as Record<string, unknown> | undefined; | ||
| const rootDef = harnessRootDef ?? (schema.properties ? schema : undefined) as Record<string, unknown> | undefined; | ||
|
|
||
| const sections = definitions ? Object.keys(definitions[resourceType] ?? {}) : []; | ||
| const properties = rootDef?.properties as Record<string, unknown> | undefined; | ||
| const required = rootDef?.required as string[] | undefined; | ||
|
|
||
|
|
@@ -124,8 +127,21 @@ function getSummary(schema: Record<string, unknown>, resourceType: string): Reco | |
| }; | ||
| } | ||
|
|
||
| export function registerSchemaTool(server: McpServer): void { | ||
| const availableSchemas = VALID_SCHEMAS; | ||
| export function registerSchemaTool( | ||
| server: McpServer, | ||
| additionalSchemas?: Record<string, SchemaEntry>, | ||
| ): void { | ||
| if (additionalSchemas) { | ||
| for (const key of Object.keys(additionalSchemas)) { | ||
| if (key in SCHEMAS) { | ||
| throw new Error(`additionalSchemas key '${key}' conflicts with a built-in schema name`); | ||
| } | ||
| } | ||
| } | ||
| const allSchemas: Record<string, Record<string, any>> = additionalSchemas | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This accepts arbitrary
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ? { ...SCHEMAS, ...Object.fromEntries(Object.entries(additionalSchemas).map(([k, v]) => [k, v.schema])) } | ||
| : { ...SCHEMAS }; | ||
| const availableSchemas = Object.keys(allSchemas); | ||
|
|
||
| server.registerTool( | ||
| "harness_schema", | ||
|
|
@@ -213,7 +229,7 @@ export function registerSchemaTool(server: McpServer): void { | |
| return errorResult("resource_type is required for schema lookups. Use example_search to search examples without specifying a resource type."); | ||
| } | ||
|
|
||
| const schema = SCHEMAS[args.resource_type as keyof typeof SCHEMAS] as Record<string, unknown>; | ||
| const schema = allSchemas[args.resource_type] as Record<string, unknown>; | ||
|
|
||
| // No path → return summary with available examples | ||
| if (!args.path) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,10 +14,11 @@ import { registerSearchTool } from "./harness-search.js"; | |
| import { registerDescribeTool } from "./harness-describe.js"; | ||
| import { registerStatusTool } from "./harness-status.js"; | ||
| import { registerSchemaTool } from "./harness-schema.js"; | ||
| import type { SchemaEntry } from "../data/schemas/index.js"; | ||
| import "../data/examples/load-all.js"; | ||
|
|
||
|
|
||
| export function registerAllTools(server: McpServer, registry: Registry, client: HarnessClient, config: Config): void { | ||
| export function registerAllTools(server: McpServer, registry: Registry, client: HarnessClient, config: Config, additionalSchemas?: Record<string, SchemaEntry>): void { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still doesn't make the new schema path reachable from the shipped server.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is expected for now. |
||
| registerListTool(server, registry, client); | ||
| registerGetTool(server, registry, client); | ||
| registerCreateTool(server, registry, client); | ||
|
|
@@ -28,5 +29,5 @@ export function registerAllTools(server: McpServer, registry: Registry, client: | |
| registerSearchTool(server, registry, client); | ||
| registerDescribeTool(server, registry); | ||
| registerStatusTool(server, registry, client, config); | ||
| registerSchemaTool(server); | ||
| registerSchemaTool(server, additionalSchemas); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| import { describe, it, expect, vi } from "vitest"; | ||
| import type { ToolResult } from "../../src/utils/response-formatter.js"; | ||
| import type { SchemaEntry } from "../../src/data/schemas/index.js"; | ||
| import { registerSchemaTool } from "../../src/tools/harness-schema.js"; | ||
|
|
||
| function entry(schema: Record<string, any>): SchemaEntry { | ||
| return { schema, description: "test", group: "test" }; | ||
| } | ||
|
|
||
| function makeMcpServer() { | ||
| const tools = new Map<string, { handler: (...args: unknown[]) => Promise<ToolResult> }>(); | ||
| return { | ||
| registerTool: vi.fn((name: string, _schema: unknown, handler: (...args: unknown[]) => Promise<ToolResult>) => { | ||
| tools.set(name, { handler }); | ||
| }), | ||
| async call(name: string, args: Record<string, unknown>): Promise<ToolResult> { | ||
| const tool = tools.get(name); | ||
| if (!tool) throw new Error(`Tool "${name}" not registered`); | ||
| const extra = { signal: new AbortController().signal, sendNotification: vi.fn(), _meta: {} }; | ||
| return tool.handler(args, extra) as Promise<ToolResult>; | ||
| }, | ||
| } as any; | ||
| } | ||
|
|
||
| function parseResult(result: ToolResult): unknown { | ||
| return JSON.parse(result.content[0]!.text); | ||
| } | ||
|
|
||
| describe("registerSchemaTool additionalSchemas", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important: these new cases only prove that the internal helper works when called directly. They do not exercise Please add at least one test or smoke check that goes through the real bootstrap and verifies the extra schema is visible from the actual exported server surface, not just from |
||
| it("accepts additionalSchemas without throwing", () => { | ||
| const server = makeMcpServer(); | ||
| expect(() => | ||
| registerSchemaTool(server, { DashboardContract: entry({ type: "object", properties: { id: { type: "string" } } }) }) | ||
| ).not.toThrow(); | ||
| }); | ||
|
|
||
| it("registers without additionalSchemas (backwards compat)", () => { | ||
| const server = makeMcpServer(); | ||
| expect(() => registerSchemaTool(server)).not.toThrow(); | ||
| }); | ||
|
|
||
| it("throws when additionalSchemas key collides with a built-in schema name", () => { | ||
| const server = makeMcpServer(); | ||
| expect(() => | ||
| registerSchemaTool(server, { pipeline: entry({ type: "object" }) }), | ||
| ).toThrow("conflicts with a built-in schema name"); | ||
| }); | ||
|
|
||
| it("handler returns fields from a Harness-layout extension schema (definitions[type][type])", async () => { | ||
| const server = makeMcpServer(); | ||
| const dashboardSchema = entry({ | ||
| definitions: { | ||
| DashboardContract: { | ||
| DashboardContract: { | ||
| type: "object", | ||
| properties: { title: { type: "string" }, widgets: { type: "array" } }, | ||
| required: ["title"], | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| registerSchemaTool(server, { DashboardContract: dashboardSchema }); | ||
|
|
||
| const result = await server.call("harness_schema", { resource_type: "DashboardContract" }); | ||
| const parsed = parseResult(result) as Record<string, unknown>; | ||
|
|
||
| expect(parsed.resource_type).toBe("DashboardContract"); | ||
| expect(parsed.fields).toEqual([ | ||
| { name: "title", type: "string", required: true }, | ||
| { name: "widgets", type: "array", required: false }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("handler returns fields from a plain JSON Schema extension schema (root-level properties)", async () => { | ||
| const server = makeMcpServer(); | ||
| registerSchemaTool(server, { | ||
| MyExtension: entry({ | ||
| type: "object", | ||
| properties: { name: { type: "string" }, count: { type: "number" } }, | ||
| required: ["name"], | ||
| }), | ||
| }); | ||
|
|
||
| const result = await server.call("harness_schema", { resource_type: "MyExtension" }); | ||
| const parsed = parseResult(result) as Record<string, unknown>; | ||
|
|
||
| expect(parsed.resource_type).toBe("MyExtension"); | ||
| expect(parsed.fields).toEqual([ | ||
| { name: "name", type: "string", required: true }, | ||
| { name: "count", type: "number", required: false }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("handler still returns built-in schema content when no additionalSchemas", async () => { | ||
| const server = makeMcpServer(); | ||
| registerSchemaTool(server); | ||
|
|
||
| const result = await server.call("harness_schema", { resource_type: "pipeline" }); | ||
| const parsed = parseResult(result) as Record<string, unknown>; | ||
|
|
||
| expect(parsed.resource_type).toBe("pipeline"); | ||
| expect(Array.isArray(parsed.fields)).toBe(true); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This merge silently allows an extension schema to shadow a built-in name after only logging a warning. That means callers can replace canonical contracts like
pipelineortriggerin bothschema:///...andharness_schema(...), which is the opposite of the repo's fail-loud guidance. Duplicate keys should throw instead of overriding built-ins.