trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

Adding or changing an MCP tool no longer has to edit docs/MCP.md: the tool table stops being a checked, committed file #8313

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/8304-adding-or-changing-an-mcp-tool-no-longer → main 1 commit fork

  • Merged. scout/8304-adding-or-changing-an-mcp-tool-no-longer was merged into main as 00d621d.

Review

Sign in to review this change.
11 files changed +85 −159
CLAUDE.md +2 −2
@@ -260,8 +260,8 @@ flag exists only in the stdio dev runner and is never reachable over SSH.
260260 exporting `route`; then `bun scripts/openapi-routes.ts` writes its line in the
261261 generated index. Never list it by hand.
262262 - **Add an MCP tool:** one file, `http/mcp/tools/<name>.ts`, exporting `tool`;
263 then `bun scripts/mcp-tools.ts` writes its line in the index and its row in
264 docs/MCP.md (docs/MCP.md, "Adding a tool"). Never list it by hand.
263 then `bun scripts/mcp-tools.ts` writes its line in the index; its `doc` is
264 its line in /llms.txt, served live, so no docs file changes. Never list it by hand.
265265 - **Add a TUI screen:** one file under `tui/screens/`, exporting
266266 `(f: Session, ctx: Ctx, …) => Screen`. Compose `ui/widgets`; take navigation
267267 from `screens/common.ts` (`ListState`, `Scroll`, `Filter`, `drawList`,
scripts/mcp-tools.ts +16 −52
@@ -1,27 +1,24 @@
11 /**
2 * Writes the MCP tools' generated files from `src/http/mcp/tools/*.ts` (#5924):
2 * Writes the MCP tools' generated index, `src/http/mcp/tools/index.ts`, from
3 * `src/http/mcp/tools/*.ts` (#5924): one sorted line per tool file, so a pull
4 * that adds a tool adds one line and never edits another pull's.
35 *
4 * - `src/http/mcp/tools/index.ts`: one sorted line per tool file, so a pull
5 * that adds a tool adds one line and never edits another pull's.
6 * - the `## Tools` table in `docs/MCP.md`, between its `tools:begin` and
7 * `tools:end` markers, from each tool's `doc` and `writes`.
6 * It writes no docs (#8304): the list of tools a reader sees is served live,
7 * from each tool's `doc`, in /llms.txt's `## MCP tools` (`src/web/llms_tools.ts`).
88 *
9 * bun scripts/mcp-tools.ts write both
10 * bun scripts/mcp-tools.ts --check exit 1, naming the file, when either is stale
9 * bun scripts/mcp-tools.ts write the index
10 * bun scripts/mcp-tools.ts --check exit 1, naming the file, when it is stale
1111 *
1212 * `test/mcp-tools.test.ts` runs the check, so a pull that adds a tool file
1313 * without running this goes red with the command to run.
1414 */
1515 import { readdirSync, readFileSync, writeFileSync } from "node:fs";
1616 import { join } from "node:path";
17 import { byName } from "../src/http/mcp/shared.ts";
1817
1918 const ROOT = join(import.meta.dir, "..");
2019 const DIR = join(ROOT, "src/http/mcp/tools");
2120 const INDEX = join(DIR, "index.ts");
22 const DOC = join(ROOT, "docs/MCP.md");
23 const BEGIN = "<!-- tools:begin: generated by `bun scripts/mcp-tools.ts` from each tool's `doc` -->";
24 const END = "<!-- tools:end -->";
21 const INDEX_NAME = "src/http/mcp/tools/index.ts";
2522
2623 /** The tool files, by name: every `<name>.ts` here but the index. */
2724 export function toolFiles(dir = DIR): string[] {
@@ -40,50 +37,17 @@ export function indexText(names: readonly string[]): string {
4037 ].join("\n");
4138 }
4239
43 type Documented = {
44 readonly name: string;
45 readonly doc?: string;
46 readonly writes: boolean;
47 readonly needsAdminScope?: true;
48 };
49
50 export function tableText(tools: readonly Documented[]): string {
51 const rows = [...tools]
52 .filter((t) => t.doc !== undefined)
53 .sort(byName)
54 // An admin-scope tool is marked so, as `tools/list` hides it from a lesser token (#7650).
55 .map((t) => `| \`${t.name}\` | ${t.needsAdminScope === true ? "(admin scope) " : ""}${t.doc} | ${t.writes ? "yes" : ""} |`);
56 return [BEGIN, "", "| tool | does | writes |", "| ---- | ---- | ------ |", ...rows, "", END].join("\n");
57 }
58
59 /** docs/MCP.md with its table replaced by `table`. */
60 export function withTable(md: string, table: string): string {
61 const from = md.indexOf(BEGIN);
62 const to = md.indexOf(END);
63 if (from < 0 || to < from) throw new Error(`docs/MCP.md has no ${BEGIN} … ${END} markers`);
64 return md.slice(0, from) + table + md.slice(to + END.length);
65 }
66
6740 if (import.meta.main) {
6841 const check = process.argv.includes("--check");
69 const stale: string[] = [];
7042 const index = indexText(toolFiles());
71 if (readFileSync(INDEX, "utf8") !== index) {
72 stale.push("src/http/mcp/tools/index.ts");
73 if (!check) writeFileSync(INDEX, index);
74 }
75 // After the index is current, so a new file's row is in the table.
76 const { TOOLS } = await import("../src/http/mcp/tools.ts");
77 const { REPORT_TOOL } = await import("../src/http/mcp.ts");
78 const md = readFileSync(DOC, "utf8");
79 const doc = withTable(md, tableText([...TOOLS, REPORT_TOOL]));
80 if (md !== doc) {
81 stale.push("docs/MCP.md");
82 if (!check) writeFileSync(DOC, doc);
83 }
84 if (check && stale.length > 0) {
85 console.error(`stale: ${stale.join(", ")}; run \`bun scripts/mcp-tools.ts\``);
43 const stale = readFileSync(INDEX, "utf8") !== index;
44 if (check && stale) {
45 console.error(`stale: ${INDEX_NAME}; run \`bun scripts/mcp-tools.ts\``);
8646 process.exit(1);
8747 }
88 if (!check) console.log(stale.length > 0 ? `wrote ${stale.join(", ")}` : "current");
48 if (stale && !check) writeFileSync(INDEX, index);
49 if (!check) {
50 console.log(`${stale ? "wrote" : "current"}: ${INDEX_NAME}, the only file this writes`);
51 console.log("the tool list is served live from each tool's `doc` in /llms.txt; docs/MCP.md is not written");
52 }
8953 }
src/checks/shared_red.ts +4 −7
@@ -49,20 +49,18 @@ export const NO_TEST_NAME = "(unnamed)";
4949 * reserving it ("keeper", "feed", "favicon.ico") failed it, main green.
5050 * OpenAPI coverage (#8140): each pull adding an /api/v1 route without its
5151 * row, or a route file without regenerating the index, failed it, main green.
52 * MCP tools (#7906): each pull adding a tool or editing its `doc` without
53 * `bun scripts/mcp-tools.ts` failed the index or table check, main green.
52 * MCP tools (#7906): each pull adding a tool without `bun scripts/mcp-tools.ts`
53 * failed the index check, main green.
5454 * The rest are top-level tests named whole (#8184): each reads only the tree.
5555 */
5656 export const TREE_GATES: readonly string[] = [
5757 "design check: nothing added outside the system (#2678) > ",
5858 "no polling in src/web > ",
5959 "reserved names > every top-level page of the forge is a reserved name",
60 // A route or tool a pull added without its row in docs/openapi or docs/MCP.md.
60 // A route a pull added without its row in docs/openapi.
6161 "every registered /api/v1 route has an OpenAPI row or is listed as not yet described",
6262 "NOT_YET_DESCRIBED has no entry that is now described",
6363 "NOT_YET_DESCRIBED is sorted and names registered routes",
64 "every tool in tools/list has a row in docs/MCP.md",
65 "every row in docs/MCP.md names a tool that exists",
6664 // An import a pull added from scripts/ or test/ into src/.
6765 "nothing under src/ imports from outside src/ (scripts/, test/, …)",
6866 // A README a pull grew past its limit, or a docs/ link it broke.
@@ -70,9 +68,8 @@ export const TREE_GATES: readonly string[] = [
7068 "every docs/ link in README.md resolves",
7169 // A route file a pull added without regenerating the index.
7270 "the gathered routes > the index is current: run `bun scripts/openapi-routes.ts`",
73 // A tool file, or a tool's `doc`, a pull changed without regenerating (#7906).
71 // A tool file a pull added without regenerating the index (#7906).
7472 "the gathered tools > the index is current: run `bun scripts/mcp-tools.ts`",
75 "the gathered tools > docs/MCP.md's tool table is current: run `bun scripts/mcp-tools.ts`",
7673 ];
7774
7875 /** Whether a failing test's name is one of {@link TREE_GATES}': under a suite prefix (`… > `), or a test's full name exactly. */
src/http/mcp.ts +1 −1
@@ -234,7 +234,7 @@ function rpcOne(
234234 */
235235 export const REPORT_TOOL = {
236236 name: "report",
237 /** Its row in docs/MCP.md (`bun scripts/mcp-tools.ts`); never sent to a client. */
237 /** Its line in /llms.txt's `## MCP tools`; never sent to a client. */
238238 doc: "`{ repo, title, body?, page?, agent? }`: report a problem in a public repository, the one tool open without a token; it still stores the report, which a person reads first; plain text. With a token the report carries your account; `agent` without one is shown as a claim",
239239 writes: true,
240240 description:
src/http/mcp/shared.ts +2 −2
@@ -45,7 +45,7 @@ export const bad = (error: string, message: string): Answer => ({
4545
4646 export interface Tool {
4747 readonly name: string;
48 /** Its row in docs/MCP.md's tool table: markdown, one line; absent, the tool has no row yet. */
48 /** Its line in /llms.txt's `## MCP tools`: markdown, one line; absent, the line reads `description`. */
4949 readonly doc?: string;
5050 readonly description: string;
5151 readonly inputSchema: Record<string, unknown>;
@@ -338,6 +338,6 @@ export function boardFailed(e: BoardError): Answer {
338338 return bad(e, BOARD_NOT_SAVED);
339339 }
340340
341 /** Orders by name's code points, never the machine's locale: the tool list and its docs table read the same everywhere. */
341 /** Orders by name's code points, never the machine's locale: the tool list reads the same everywhere. */
342342 export const byName = (a: { readonly name: string }, b: { readonly name: string }): number =>
343343 a.name < b.name ? -1 : a.name > b.name ? 1 : 0;
src/http/server.ts +2 −2
@@ -506,7 +506,7 @@ import { viaGate } from "./via_gate.ts";
506506 import { checksRecheckRoutes } from "./checks_recheck.ts";
507507 import { feedRoutes } from "./feed.ts";
508508 import type { Database } from "bun:sqlite";
509 import { mcpRoutes } from "./mcp.ts";
509 import { mcpRoutes, REPORT_TOOL } from "./mcp.ts";
510510 import { TOOLS } from "./mcp/tools.ts";
511511 import { setMcpTools } from "../web/llms_tools.ts";
512512 import { openapiRoute } from "../contract/served.ts";
@@ -1180,7 +1180,7 @@ export function buildRouter(): Router<Handler> {
11801180 // reclaim a path under `/api/`.
11811181 // The forge as an MCP server: the studio's verbs for an assistant.
11821182 mcpRoutes(router);
1183 setMcpTools(TOOLS);
1183 setMcpTools([...TOOLS, REPORT_TOOL]);
11841184 // The public API as OpenAPI, for any client that reads it.
11851185 openapiRoute(router);
11861186 oauthRoutes(router);
src/web/llms_tools.ts +10 −5
@@ -1,19 +1,24 @@
11 /**
22 * The `## MCP tools` section of /llms.txt: one line per tool in the generated
3 * index, `name: description`, then `(admin)` and `(writes)` where they apply.
3 * index and `report`, `name: doc` (its `description` when it has no `doc`),
4 * then `(admin)` and `(writes)` where they apply. The live list of every tool,
5 * so no committed file has to be regenerated when a tool changes (#8304).
46 * Built once at boot and the same for every reader, whoever they are.
57 */
68 import type { Tool } from "../http/mcp/shared.ts";
79
10 /** What a line reads: `report` is not a `Tool`, having no `run`, but is listed. */
11 export type Listed = Pick<Tool, "name" | "description" | "doc" | "writes" | "adminOnly" | "needsAdminScope">;
12
813 const oneLine = (s: string): string => s.replace(/\s+/g, " ").trim();
914
10 export function toolLine(t: Tool): string {
15 export function toolLine(t: Listed): string {
1116 const admin = t.adminOnly === true || t.needsAdminScope === true ? " (admin)" : "";
1217 const writes = t.writes ? " (writes)" : "";
13 return `- ${t.name}: ${oneLine(t.description)}${admin}${writes}`;
18 return `- ${t.name}: ${oneLine(t.doc ?? t.description)}${admin}${writes}`;
1419 }
1520
16 export const toolsSection = (tools: readonly Tool[]): string[] => [
21 export const toolsSection = (tools: readonly Listed[]): string[] => [
1722 "## MCP tools",
1823 "",
1924 ...tools.map(toolLine),
@@ -23,7 +28,7 @@ export const toolsSection = (tools: readonly Tool[]): string[] => [
2328 // The index is handed in at boot by the server rather than imported here:
2429 // importing it from a page module closes an import cycle through the actions.
2530 let section: readonly string[] = [];
26 export const setMcpTools = (tools: readonly Tool[]): void => {
31 export const setMcpTools = (tools: readonly Listed[]): void => {
2732 section = toolsSection(tools);
2833 };
2934 export const mcpToolsSection = (): readonly string[] => section;
test/llms-tools.test.ts +6 −4
@@ -1,11 +1,13 @@
1 /** /llms.txt's `## MCP tools` section is the generated tool index, line for line. */
1 /** /llms.txt's `## MCP tools` section is the generated tool index and `report`, line for line, each by its `doc`. */
22 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
33 import { mkdtemp, rm } from "node:fs/promises";
44 import { tmpdir } from "node:os";
55 import { join } from "node:path";
66 import { createApp } from "../src/app.ts";
77 import { serve, type Servers } from "../src/http/server.ts";
8 import { REPORT_TOOL } from "../src/http/mcp.ts";
89 import { TOOLS } from "../src/http/mcp/tools.ts";
10 import type { Listed } from "../src/web/llms_tools.ts";
911
1012 describe("/llms.txt MCP tools", () => {
1113 let dataDir: string;
@@ -20,7 +22,7 @@ describe("/llms.txt MCP tools", () => {
2022 await rm(dataDir, { recursive: true, force: true });
2123 });
2224
23 test("lists exactly the tools in the index, with (admin) and (writes)", async () => {
25 test("lists exactly the tools in the index and `report`, by their `doc`, with (admin) and (writes)", async () => {
2426 const res = await fetch(`${servers.url}/llms.txt`);
2527 expect(res.status).toBe(200);
2628 expect(res.headers.get("cache-control")).toContain("max-age");
@@ -29,9 +31,9 @@ describe("/llms.txt MCP tools", () => {
2931 expect(start).toBeGreaterThan(-1);
3032 const end = lines.findIndex((l, i) => i > start && l.startsWith("## "));
3133 const listed = lines.slice(start + 1, end).filter((l) => l.startsWith("- "));
32 const expected = TOOLS.map((t) => {
34 const expected = [...TOOLS, REPORT_TOOL].map((t: Listed) => {
3335 const admin = t.adminOnly === true || t.needsAdminScope === true ? " (admin)" : "";
34 return `- ${t.name}: ${t.description.replace(/\s+/g, " ").trim()}${admin}${t.writes ? " (writes)" : ""}`;
36 return `- ${t.name}: ${(t.doc ?? t.description).replace(/\s+/g, " ").trim()}${admin}${t.writes ? " (writes)" : ""}`;
3537 });
3638 expect(listed).toEqual(expected);
3739 expect(TOOLS.some((t) => t.writes)).toBe(true);
test/mcp-docs.test.ts deleted −65
@@ -1,65 +0,0 @@
1 /** docs/MCP.md never falls behind the server: every tool in tools/list has a row, every row names a tool. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp } from "../src/app.ts";
7 import { createToken } from "../src/core/tokens.ts";
8 import { createUser } from "../src/core/users.ts";
9 import { serve, type Servers } from "../src/http/server.ts";
10 import { quietLogs } from "./helpers/globals.ts";
11 import { listAt } from "./mcp-list.ts";
12
13 quietLogs();
14
15 let dataDir: string;
16 let servers: Servers;
17 let served: string[];
18
19 beforeAll(async () => {
20 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-docs-"));
21 const app = createApp({ dataDir, port: 0 });
22 servers = await serve(app);
23 const made = await createUser(app.db, {
24 origin: "seed",
25 trusted: true,
26 username: "ada",
27 email: "[email protected]",
28 password: "a-good-passphrase",
29 adminIfFirst: true,
30 });
31 if (!made.ok) throw new Error(JSON.stringify(made.error));
32 const token = createToken(app.db, made.value.id, "admin", "admin").token;
33 served = (await listAt(servers.url, token)).result.tools.map((t: { name: string }) => t.name);
34 });
35
36 afterAll(async () => {
37 await servers.stop();
38 await rm(dataDir, { recursive: true, force: true });
39 });
40
41 async function documented(): Promise<string[]> {
42 const md = await Bun.file(join(import.meta.dir, "..", "docs", "MCP.md")).text();
43 const names: string[] = [];
44 let inTable = false;
45 for (const line of md.split("\n")) {
46 if (/^\|\s*tool\s*\|/.test(line)) inTable = true;
47 else if (inTable && !line.startsWith("|")) break;
48 else if (inTable) {
49 const m = /^\|\s*`([^`]+)`\s*\|/.exec(line);
50 if (m) names.push(m[1]!);
51 }
52 }
53 return names;
54 }
55
56 test("every tool in tools/list has a row in docs/MCP.md", async () => {
57 const rows = new Set(await documented());
58 expect(rows.size).toBeGreaterThan(0);
59 expect(served.filter((n) => !rows.has(n))).toEqual([]);
60 });
61
62 test("every row in docs/MCP.md names a tool that exists", async () => {
63 const live = new Set(served);
64 expect((await documented()).filter((n) => !live.has(n))).toEqual([]);
65 });
test/mcp-tools.test.ts +39 −14
@@ -1,14 +1,19 @@
11 /**
22 * One MCP tool per file (#5924): the contract every file under
3 * src/http/mcp/tools/ keeps, and the generated index and docs table kept
4 * current. A tool's own behaviour is tested in test/mcp-<name>.test.ts.
3 * src/http/mcp/tools/ keeps, the generated index kept current, and the
4 * live list in /llms.txt carrying every tool's `doc` (#8304), so no docs
5 * file is regenerated when a tool changes. A tool's own behaviour is tested in test/mcp-<name>.test.ts.
56 */
67 import { describe, expect, test } from "bun:test";
78 import { readFileSync } from "node:fs";
9 import { mkdtemp, rm, writeFile } from "node:fs/promises";
10 import { tmpdir } from "node:os";
811 import { join } from "node:path";
9 import { indexText, tableText, toolFiles, withTable } from "../scripts/mcp-tools.ts";
12 import { indexText, toolFiles } from "../scripts/mcp-tools.ts";
1013 import { REPORT_TOOL, toolNames } from "../src/http/mcp.ts";
14 import { byName } from "../src/http/mcp/shared.ts";
1115 import { TOOLS } from "../src/http/mcp/tools.ts";
16 import { toolsSection } from "../src/web/llms_tools.ts";
1217
1318 const ROOT = join(import.meta.dir, "..");
1419 const read = (path: string) => readFileSync(join(ROOT, path), "utf8");
@@ -45,20 +50,10 @@ describe("the gathered tools", () => {
4550 expect(read("src/http/mcp/tools/index.ts")).toBe(indexText(files));
4651 });
4752
48 test("docs/MCP.md's tool table is current: run `bun scripts/mcp-tools.ts`", () => {
49 const md = read("docs/MCP.md");
50 expect(md).toBe(withTable(md, tableText([...TOOLS, REPORT_TOOL])));
51 });
52
5353 test("are ordered by code point, not the machine's locale", () => {
5454 const names = TOOLS.map((t) => t.name);
5555 expect(names).toEqual([...names].sort());
56 const rows = tableText([
57 { name: "b", doc: "x", writes: false },
58 { name: "B", doc: "x", writes: false },
59 { name: "a", doc: "x", writes: false },
60 ]).split("\n").filter((l) => l.startsWith("| `"));
61 expect(rows.map((l) => l.split("`")[1])).toEqual(["B", "a", "b"]);
56 expect([{ name: "b" }, { name: "B" }, { name: "a" }].sort(byName).map((t) => t.name)).toEqual(["B", "a", "b"]);
6257 });
6358
6459 test("a new file is one new line in the index, touching no other", () => {
@@ -68,3 +63,33 @@ describe("the gathered tools", () => {
6863 expect(before.filter((l) => !after.includes(l))).toEqual([]);
6964 });
7065 });
66
67 describe("the live tool list", () => {
68 test("every tool's `doc` is its line in /llms.txt's tools section", () => {
69 const section = toolsSection([...TOOLS, REPORT_TOOL]);
70 for (const t of [...TOOLS, REPORT_TOOL]) {
71 const line = section.find((l) => l.startsWith(`- ${t.name}: `));
72 expect([t.name, line?.includes(t.doc ?? t.description) ?? false]).toEqual([t.name, true]);
73 }
74 expect(section.filter((l) => l.startsWith("- ")).length).toBe(TOOLS.length + 1);
75 });
76
77 test("a pull adding a tool file and its index line needs no docs file", async () => {
78 const dir = await mkdtemp(join(tmpdir(), "forge-mcp-tools-"));
79 try {
80 for (const n of files) await writeFile(join(dir, `${n}.ts`), "");
81 await writeFile(join(dir, "zz_fixture.ts"), "");
82 const names = toolFiles(dir);
83 expect(names).toEqual([...files, "zz_fixture"]);
84 // The index gains the one line, and the script writes nothing else.
85 const added = indexText(names).split("\n").filter((l) => !indexText(files).split("\n").includes(l));
86 expect(added).toEqual(['export { tool as zz_fixture } from "./zz_fixture.ts";']);
87 expect(read("scripts/mcp-tools.ts").match(/writeFileSync\(\w+/g)).toEqual(["writeFileSync(INDEX"]);
88 // Its `doc` reaches the live list with no committed file regenerated.
89 const fixture = { name: "zz_fixture", doc: "`{}`: a fixture", description: "Fixture.", writes: false };
90 expect(toolsSection([...TOOLS, fixture])).toContain("- zz_fixture: `{}`: a fixture");
91 } finally {
92 await rm(dir, { recursive: true, force: true });
93 }
94 });
95 });
test/shared-red.test.ts +3 −5
@@ -113,12 +113,10 @@ describe("a red many pulls share", () => {
113113 expect(shared(red("i3", [longer]))).toBe(true);
114114 });
115115
116 test("three pulls each changing a tool without regenerating docs/MCP.md are three pulls' own reds (#7906)", () => {
117 const table = "the gathered tools > docs/MCP.md's tool table is current: run `bun scripts/mcp-tools.ts`";
116 test("three pulls each adding a tool without regenerating the index are three pulls' own reds (#7906)", () => {
118117 const index = "the gathered tools > the index is current: run `bun scripts/mcp-tools.ts`";
119 for (const i of ["i1", "i2", "i3"]) red(i, [table, index]);
120 expect(shared(red("i4", [table]))).toBe(false);
121 expect(shared(red("i5", [index]))).toBe(false);
118 for (const i of ["i1", "i2", "i3"]) red(i, [index]);
119 expect(shared(red("i4", [index]))).toBe(false);
122120 expect(sharedTests(db)).toEqual([]);
123121 // The rest of the suite is no gate: three pulls on it still share it.
124122 const other = "the gathered tools > are ordered by code point, not the machine's locale";

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs