trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns1g rEventsFlows

The OpenAPI description lists every route the MCP tools call, and a test fails when a route is missing from it #11331

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/11322-the-openapi-description-lists-every-rout → main 3 commits fork 2 threads · 2 unresolved

  • Merged. scout/11322-the-openapi-description-lists-every-rout was merged into main as 55a0c96.

Review

Sign in to review this change.
1 file changed +184 −9
test/mcp-routes-described.test.ts +184 −9
@@ -2,7 +2,21 @@
22 * The forge's own OpenAPI description is `contract/openapi.ts` (served at
33 * `/contract/openapi.json`; `/api` reads it). Every REST route an MCP tool's
44 * doc names ("as `GET /api/v1/user/pins` is") must be in it, so an agent that
5 * reads the description finds what the tools use.
5 * reads the description finds what the tools use. A tool also writes a route
6 * short, `PATCH …/issues/:number`: the tail must be whole trailing segments of a
7 * described path of that method, and a tail that fits several paths is reported
8 * as ambiguous unless SHORT_MEANS says which path the tool means, or exactly one
9 * fit lies under `/api/v1/repos/` (what a bare `…/` means in the tools).
10 *
11 * That last rule is a convenience with a known gap: when the one repos fit is not
12 * the path the tool meant (it meant an origins or user path, and the repos path
13 * only shares its tail), the test still passes. A tail whose meaning is not the
14 * repository's goes in SHORT_MEANS, where the test holds it to the paths named.
15 *
16 * MCP_ONLY is the table of routes deliberately left out of the description, each
17 * with its reason. Both tables are checked for stale entries: a key no tool names
18 * any more, an MCP-only route that is now described, a SHORT_MEANS tail that no
19 * longer fits several paths, each fails until the line is deleted.
620 */
721 import { expect, test } from "bun:test";
822 import { ROUTES } from "../src/contract/openapi.ts";
@@ -10,13 +24,33 @@ import { TOOLS } from "../src/http/mcp/tools.ts";
1024
1125 const shape = (path: string): string => path.replace(/\{\w+\}|:\w+/g, "{}");
1226
13 const described = new Set(ROUTES.map((r) => `${r.method.toUpperCase()} ${shape(r.path)}`));
27 type Table<T> = Readonly<Record<string, T>>;
1428
15 /** `[tool, "METHOD /path"]` for each route a tool's text names, `PUT`/`DELETE /x` naming both. */
16 function named(): [string, string][] {
29 /** "METHOD /path" or "METHOD …/tail" a tool names → why the OpenAPI description deliberately leaves it out. */
30 const MCP_ONLY: Table<string> = {};
31
32 /** A short route that fits several paths → the paths it means (each must be described). */
33 const SHORT_MEANS: Table<readonly string[]> = {
34 "POST …/{}/sync": ["/api/v1/user/origins/{}/sync"],
35 // `assign` sets the assignee of an issue or of a pull.
36 "PUT …/assignee": ["/api/v1/repos/{}/{}/issues/{}/assignee", "/api/v1/repos/{}/{}/pulls/{}/assignee"],
37 };
38
39 interface Row {
40 readonly method: string;
41 readonly path: string;
42 }
43 interface Doc {
44 readonly name: string;
45 readonly doc?: string;
46 readonly description: string;
47 }
48
49 /** `[tool, "METHOD /path"]` for each full route a tool's text names, `PUT`/`DELETE /x` naming both. */
50 function named(tools: readonly Doc[]): [string, string][] {
1751 const found: [string, string][] = [];
18 for (const tool of TOOLS) {
19 const text = `${tool.doc} ${tool.description}`;
52 for (const tool of tools) {
53 const text = `${tool.doc ?? ""} ${tool.description}`;
2054 for (const m of text.matchAll(/((?:\b[A-Z]{3,6}\b[`\/ ]*){1,4})`?\s*(\/api\/v1\/[^\s`,;)]*)/g)) {
2155 const path = shape(m[2]!.replace(/[.:]+$/, ""));
2256 for (const method of m[1]!.match(/[A-Z]{3,6}/g)!) found.push([tool.name, `${method} ${path}`]);
@@ -25,11 +59,152 @@ function named(): [string, string][] {
2559 return found;
2660 }
2761
62 /** `[tool, method, "/tail"]` for each short route (`METHOD …/tail`) a tool's text names. */
63 function namedShort(tools: readonly Doc[]): [string, string, string][] {
64 const found: [string, string, string][] = [];
65 for (const tool of tools) {
66 const text = `${tool.doc ?? ""} ${tool.description}`;
67 for (const m of text.matchAll(/\b(GET|POST|PUT|PATCH|DELETE)\s*`?…(\/[A-Za-z0-9_/:{}-]+)/g)) {
68 found.push([tool.name, m[1]!, shape(m[2]!)]);
69 }
70 }
71 return found;
72 }
73
74 /**
75 * Each route the tools name that the rows do not list, as a line saying so.
76 * `mcpOnly` maps "METHOD /path" (or "METHOD …/tail") to why it is deliberately
77 * not described; `shortMeans` maps "METHOD …/tail" to the paths it means. A short
78 * tail that fits more than one described path, and neither table nor the repos
79 * rule settles, is reported as ambiguous.
80 */
81 function missingRoutes(
82 tools: readonly Doc[],
83 rows: readonly Row[],
84 mcpOnly: Table<string> = {},
85 shortMeans: Table<readonly string[]> = {},
86 ): string[] {
87 const shaped = rows.map((r) => ({ method: r.method.toUpperCase(), path: shape(r.path) }));
88 const have = new Set(shaped.map((r) => `${r.method} ${r.path}`));
89 const out: string[] = [];
90 for (const [tool, route] of named(tools)) {
91 if (!have.has(route) && mcpOnly[route] === undefined) out.push(`${route} (named by the ${tool} tool)`);
92 }
93 for (const [tool, method, tail] of namedShort(tools)) {
94 if (mcpOnly[`${method} …${tail}`] !== undefined) continue;
95 const fits = shaped.filter((r) => r.method === method && r.path.length > tail.length && r.path.endsWith(tail));
96 if (fits.length === 0) out.push(`${method} …${tail} (named by the ${tool} tool)`);
97 else if (fits.length > 1) {
98 const meant = shortMeans[`${method} …${tail}`];
99 if (meant !== undefined) {
100 for (const path of meant)
101 if (!fits.some((r) => r.path === path)) out.push(`${method} …${tail} means ${path}, which is not described (named by the ${tool} tool)`);
102 continue;
103 }
104 if (fits.filter((r) => r.path.startsWith("/api/v1/repos/")).length === 1) continue;
105 out.push(`${method} …${tail} is ambiguous: it fits ${fits.map((r) => r.path).join(", ")} (named by the ${tool} tool)`);
106 }
107 }
108 return out;
109 }
110
111 /** Each entry in either table that no longer earns its line, as a line saying why. */
112 function staleEntries(
113 tools: readonly Doc[],
114 rows: readonly Row[],
115 mcpOnly: Table<string>,
116 shortMeans: Table<readonly string[]>,
117 ): string[] {
118 const shaped = rows.map((r) => ({ method: r.method.toUpperCase(), path: shape(r.path) }));
119 const said = new Set([
120 ...named(tools).map(([, route]) => route),
121 ...namedShort(tools).map(([, method, tail]) => `${method} …${tail}`),
122 ]);
123 const fits = (key: string): number => {
124 const [method, rest] = key.split(" ", 2) as [string, string];
125 const tail = rest.replace(/^…/, "");
126 return shaped.filter((r) =>
127 r.method === method && (rest.startsWith("…") ? r.path.length > tail.length && r.path.endsWith(tail) : r.path === tail),
128 ).length;
129 };
130 const out: string[] = [];
131 for (const [key, why] of Object.entries(mcpOnly)) {
132 if (!said.has(key)) out.push(`MCP_ONLY ${key}: no tool names it`);
133 if (why.trim().length === 0) out.push(`MCP_ONLY ${key}: no reason`);
134 if (fits(key) > 0) out.push(`MCP_ONLY ${key}: the description lists it now`);
135 }
136 for (const [key, paths] of Object.entries(shortMeans)) {
137 if (!said.has(key)) out.push(`SHORT_MEANS ${key}: no tool names it`);
138 if (fits(key) < 2) out.push(`SHORT_MEANS ${key}: it fits one path or none, so it is not ambiguous`);
139 if (paths.length === 0) out.push(`SHORT_MEANS ${key}: names no path`);
140 }
141 return out;
142 }
143
144 const FIXTURE_ROWS: Row[] = [
145 { method: "get", path: "/api/v1/repos/{owner}/{name}/issues/{number}" },
146 { method: "post", path: "/api/v1/repos/{owner}/{name}/sync" },
147 { method: "post", path: "/api/v1/user/origins/{id}/sync" },
148 { method: "post", path: "/api/v1/repos/{owner}/{name}/pulls/{number}/sync" },
149 { method: "get", path: "/api/v1/user/pins" },
150 ];
151 const tool = (doc: string): Doc => ({ name: "t", doc, description: "" });
152
28153 test("the walk finds the routes the tools name", () => {
29 expect(named().length).toBeGreaterThan(20);
154 expect(named(TOOLS).length).toBeGreaterThan(20);
155 expect(namedShort(TOOLS).length).toBeGreaterThan(10);
30156 });
31157
32158 test("every REST route an MCP tool names is in the published OpenAPI description", () => {
33 const missing = named().filter(([, route]) => !described.has(route));
34 expect(missing.map(([tool, route]) => `${route} (named by the ${tool} tool)`)).toEqual([]);
159 expect(missingRoutes(TOOLS, ROUTES, MCP_ONLY, SHORT_MEANS)).toEqual([]);
160 });
161
162 test("MCP_ONLY and SHORT_MEANS hold no stale entry", () => {
163 expect(staleEntries(TOOLS, ROUTES, MCP_ONLY, SHORT_MEANS)).toEqual([]);
164 });
165
166 test("a stale table entry is reported", () => {
167 const tools = [tool("`GET /api/v1/user/hidden` and `POST …/sync`")];
168 expect(staleEntries(tools, FIXTURE_ROWS, { "GET /api/v1/user/hidden": "only an agent asks for it" }, {})).toEqual([]);
169 expect(staleEntries(tools, FIXTURE_ROWS, { "GET /api/v1/user/gone": "a reason" }, {})).toEqual([
170 "MCP_ONLY GET /api/v1/user/gone: no tool names it",
171 ]);
172 expect(staleEntries(tools, FIXTURE_ROWS, { "GET /api/v1/user/hidden": " " }, {})).toEqual(["MCP_ONLY GET /api/v1/user/hidden: no reason"]);
173 const described = [tool("`GET /api/v1/user/pins`")];
174 expect(staleEntries(described, FIXTURE_ROWS, { "GET /api/v1/user/pins": "was MCP-only once" }, {})).toEqual([
175 "MCP_ONLY GET /api/v1/user/pins: the description lists it now",
176 ]);
177 expect(staleEntries(tools, FIXTURE_ROWS, {}, { "POST …/sync": ["/api/v1/user/origins/{}/sync"] })).toEqual([]);
178 expect(staleEntries(tools, FIXTURE_ROWS, {}, { "POST …/gone": ["/api/v1/x"] })).toEqual([
179 "SHORT_MEANS POST …/gone: no tool names it",
180 "SHORT_MEANS POST …/gone: it fits one path or none, so it is not ambiguous",
181 ]);
182 });
183
184 test("a full route absent from the rows is reported, and listed MCP-only it is not", () => {
185 const t = [tool("as `GET /api/v1/user/nope` is")];
186 expect(missingRoutes(t, FIXTURE_ROWS)).toEqual(["GET /api/v1/user/nope (named by the t tool)"]);
187 expect(missingRoutes(t, FIXTURE_ROWS, { "GET /api/v1/user/nope": "only the MCP reads it" })).toEqual([]);
188 expect(missingRoutes([tool("as `GET /api/v1/user/pins` is")], FIXTURE_ROWS)).toEqual([]);
189 });
190
191 test("a short route is matched on whole trailing segments", () => {
192 expect(missingRoutes([tool("`GET …/issues/:number`")], FIXTURE_ROWS)).toEqual([]);
193 // "/sues/{}" ends the path's text but not at a segment boundary.
194 expect(missingRoutes([tool("`GET …/sues/:number`")], FIXTURE_ROWS)).toEqual(["GET …/sues/{} (named by the t tool)"]);
195 // The wrong method does not count.
196 expect(missingRoutes([tool("`PUT …/issues/:number`")], FIXTURE_ROWS)).toEqual(["PUT …/issues/{} (named by the t tool)"]);
197 expect(missingRoutes([tool("`GET …/gone`")], FIXTURE_ROWS, { "GET …/gone": "MCP-only: no page asks" })).toEqual([]);
198 });
199
200 test("a short route fitting several paths resolves to the repo one, or to SHORT_MEANS, or is ambiguous", () => {
201 const rows: Row[] = [...FIXTURE_ROWS, { method: "get", path: "/api/v1/user/origins/{id}/issues/{number}" }];
202 expect(missingRoutes([tool("`GET …/issues/:number`")], rows)).toEqual([]);
203 const many: Row[] = [...rows, { method: "get", path: "/api/v1/repos/{o}/{n}/x/issues/{number}" }];
204 expect(missingRoutes([tool("`GET …/issues/:number`")], many)[0]).toContain("is ambiguous");
205 const means = { "POST …/{}/sync": ["/api/v1/user/origins/{}/sync"] };
206 expect(missingRoutes([tool("`POST …/:id/sync`")], FIXTURE_ROWS, {}, means)).toEqual([]);
207 const noOrigins = FIXTURE_ROWS.filter((r) => !r.path.includes("/origins/"));
208 expect(missingRoutes([tool("`POST …/:id/sync`")], noOrigins, {}, means)[0]).toContain("which is not described");
209 expect(missingRoutes([tool("`POST …/sync`")], FIXTURE_ROWS)[0]).toContain("is ambiguous");
35210 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs