From 13e1259efbf5e5806396c85758368efd0760b619 Mon Sep 17 00:00:00 2001 From: Claude Lin & Lay Date: Sun, 14 Jun 2026 16:24:21 +0900 Subject: [PATCH] test(mcp-server): add CI guard for proxy/Worker search schema drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add scripts/check-schema-drift.mjs and run it in CI to assert the proxy static search schema (mcp-server/server/index.js TOOLS) mirrors the Worker zod schema (src/mcp.ts). Exact param-set match; drift fails the build. proxy は tools/list を Worker へ転送せず静的スキーマを返すため、Worker に param を 足して proxy を忘れると黙って出荷される穴があった（graph_expand が v0.9.0 でこれで 欠落 = gh#157）。忘れ得る手順を CI ガード（構造）へ置換する。 Also record the schema-parity invariant in docs/0-requirements{,.ja}.md. Refs #159 --- .github/workflows/ci.yml | 1 + docs/0-requirements.ja.md | 8 ++++ docs/0-requirements.md | 9 ++++ scripts/check-schema-drift.mjs | 86 ++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+) create mode 100644 scripts/check-schema-drift.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 130a3de..740c45c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,7 @@ jobs: with: node-version: "22" - run: npm ci + - run: node scripts/check-schema-drift.mjs - run: npx tsc --noEmit - run: npx wrangler deploy --dry-run --outdir dist diff --git a/docs/0-requirements.ja.md b/docs/0-requirements.ja.md index 23794ec..2610b70 100644 --- a/docs/0-requirements.ja.md +++ b/docs/0-requirements.ja.md @@ -83,6 +83,14 @@ Responsibilities: - semantic retrieval と structured retrieval の tool を公開する - downstream agent がそのまま使える形式で state を返す +スキーマの source of truth は Worker 側の tool 定義。client proxy +(`mcp-server/server/index.js`) は `tools/list` を Worker へ転送せず、`search` +params の静的ミラーから応答する（起動時を auth/network なしに保つため）。ミラーは +手動保守ゆえ、Worker に param を足して proxy を忘れると、MCP クライアントが +`additionalProperties: false` で黙って落とし Worker に届かない（gh#157）。 +`scripts/check-schema-drift.mjs` を CI で実行し、proxy の `search` スキーマが Worker +からズレたら build を失敗させる。両者は同期せずに出荷できない。 + ### 2. Webhook Receiver webhook receiver は GitHub event を near real time で取り込む。 diff --git a/docs/0-requirements.md b/docs/0-requirements.md index c39bc13..b6eeaf5 100644 --- a/docs/0-requirements.md +++ b/docs/0-requirements.md @@ -83,6 +83,15 @@ Responsibilities: - expose semantic and structured retrieval tools - return state in a format that downstream agents can consume directly +Schema source of truth: the Worker tool definitions are authoritative. The client +proxy (`mcp-server/server/index.js`) answers `tools/list` from a static mirror of +the `search` params — it does not forward `tools/list` to the Worker, keeping +startup auth-free and network-free. Because that mirror is hand-maintained, a param +added to the Worker but forgotten in the proxy is silently dropped by MCP clients +(`additionalProperties: false`) and never reaches the Worker (gh#157). +`scripts/check-schema-drift.mjs` runs in CI and fails the build when the proxy +`search` schema drifts from the Worker, so the two cannot ship out of sync. + ### 2. Webhook Receiver The webhook receiver ingests GitHub events in near real time. diff --git a/scripts/check-schema-drift.mjs b/scripts/check-schema-drift.mjs new file mode 100644 index 0000000..7350e5a --- /dev/null +++ b/scripts/check-schema-drift.mjs @@ -0,0 +1,86 @@ +#!/usr/bin/env node +// Guards against drift between the Worker `search` tool param schema and the +// client proxy's static mirror of it. +// +// source of truth : src/mcp.ts (zod object passed to this.server.tool("search", ...)) +// mirror : mcp-server/server/index.js (TOOLS[0].inputSchema.properties) +// +// Why this exists (gh#157 / gh#159): the proxy answers tools/list from a +// hand-maintained static schema (it does NOT forward to the Worker, to keep +// startup auth-free / network-free). A param added to the Worker but forgotten +// in the proxy is silently stripped by MCP clients (additionalProperties:false) +// and never reaches the Worker — exactly how graph_expand shipped broken in +// v0.9.0. This check turns that "forgot to sync" procedure into a CI gate: the +// build fails instead of shipping a stale schema. + +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; + +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), ".."); +const workerPath = join(repoRoot, "src", "mcp.ts"); +const proxyPath = join(repoRoot, "mcp-server", "server", "index.js"); + +/** Return the substring of `text` from the first `startMarker` to the next `endMarker`. */ +function region(text, startMarker, endMarker, label) { + const start = text.indexOf(startMarker); + if (start === -1) throw new Error(`drift-check: start marker not found (${label}): ${startMarker}`); + const end = text.indexOf(endMarker, start + startMarker.length); + if (end === -1) throw new Error(`drift-check: end marker not found (${label}): ${endMarker}`); + return text.slice(start, end); +} + +// Worker: zod object is the 3rd arg of this.server.tool("search", "", { ... }, handler). +// Top-level param keys are 8-space-indented `: z`. Bounded from the tool +// name to the `async ({` handler so other zod objects in the file are excluded. +function workerParams(text) { + const toolIdx = text.indexOf('"search"'); + if (toolIdx === -1) throw new Error("drift-check: `\"search\"` tool not found in Worker (src/mcp.ts)"); + const schema = region(text.slice(toolIdx), "{", "async ({", "worker search schema"); + return new Set([...schema.matchAll(/^ {8}(\w+): z\b/gm)].map((m) => m[1])); +} + +// Proxy: TOOLS[0].inputSchema.properties — top-level keys are 8-space-indented +// `: {`. Bounded from `properties: {` to the sibling `annotations:`. +function proxyParams(text) { + const props = region(text, "properties: {", "annotations:", "proxy search schema"); + return new Set([...props.matchAll(/^ {8}(\w+): \{/gm)].map((m) => m[1])); +} + +const worker = workerParams(readFileSync(workerPath, "utf8")); +const proxy = proxyParams(readFileSync(proxyPath, "utf8")); + +// Extractor sanity guard: a structural change could make a regex match nothing, +// turning the comparison into a meaningless empty==empty pass. Refuse that. +for (const [label, set] of [["worker", worker], ["proxy", proxy]]) { + if (set.size < 5 || !set.has("query") || !set.has("repo")) { + console.error( + `drift-check: ${label} param extraction looks wrong ` + + `(got ${set.size}: ${[...set].join(", ") || ""}). ` + + "The source structure likely changed — update scripts/check-schema-drift.mjs.", + ); + process.exit(2); + } +} + +const missingInProxy = [...worker].filter((k) => !proxy.has(k)); +const extraInProxy = [...proxy].filter((k) => !worker.has(k)); + +if (missingInProxy.length || extraInProxy.length) { + console.error("drift-check: proxy search schema is out of sync with the Worker."); + if (missingInProxy.length) + console.error( + " missing in proxy (add to mcp-server/server/index.js TOOLS search inputSchema.properties): " + + missingInProxy.join(", "), + ); + if (extraInProxy.length) + console.error( + " extra in proxy (not present in Worker src/mcp.ts search schema): " + extraInProxy.join(", "), + ); + process.exit(1); +} + +console.log( + `drift-check OK: proxy mirrors Worker search params (${worker.size}): ` + + `${[...worker].sort().join(", ")}`, +);