trevato/forge

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

More
Forks1g fMapg aRuns14g rEventsFlows

MCP `private_fix`: make the private repository a writer fixes a sealed report in, with the same rules as the API #4993

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/4977-mcp-private-fix-make-the-private-reposit → main 6 commits fork 5 threads · 5 unresolved

  • Merged. scout/4977-mcp-private-fix-make-the-private-reposit was merged into main as 70a13a5.

Review

Sign in to review this change.
3 files changed +153 −4
docs/MCP.md +1 −1
@@ -119,7 +119,7 @@ for a tunnel and a forge that sleeps.
119119 | `run_log` | `{ repo, run, tail? }`: a run's log, or its last `tail` lines (1–2000), with the REST route's access: a hidden log or unreadable repository is not found | |
120120 | `runs` | `{ repo, limit? }`: the repository's recent runs, newest first (id, kind, state, issue or pull number, started, ended), 1–50, default 20; read as the runs page reads them | |
121121 | `say` | speak on an issue or a pull; a working builder hears it as its next step | yes |
122 | `seal` | `{ repo, number, sealed }`: seal a security report (and the pull that fixes it) so only writers and the crew see it, or unseal it for every reader; a writer's, recorded | yes |
122 | `seal` | `{ repo, number, sealed }`: seal a security report (and the pull that fixes it) so only writers and the crew see it, or unseal it for every reader; a writer's, recorded; with `action: "private"` instead of `sealed`, the private repository a writer fixes a sealed report in, made on first need, with its name and clone URL (the API's `POST …/issues/:number/private`, same rules) | yes |
123123 | `search` | `{ q, kind?, repo? }`: issues, pulls, repositories and code you can read, the same results as /search; `repo` keeps to one | |
124124 | `secrets` | `{ scope, ref?, action, name?, value? }` (forge admin): `list` the names and timestamps of the forge's sealed secrets, `set` or `remove` one; `scope` is forge, org or repo, `ref` the org or `owner/name`; a value is never read back | yes |
125125 | `sever` | `{ repo }`: cut a spawned forge's link to the forge its code came from; its `forge` repository's owner only; deletes nothing | yes |
src/http/mcp/tools/seal.ts +24 −3
@@ -1,9 +1,10 @@
11 import { setIssueSealed } from "../../../actions/issues.ts";
2 import { privateRepoForSealed } from "../../../actions/repos.ts";
23 import { good, bad, type Tool, REPO_ARG, repoArg, issueArg, isAnswer } from "../shared.ts";
34
45 export const tool: Tool = {
56 name: "seal",
6 doc: "`{ repo, number, sealed }`: seal a security report (and the pull that fixes it) so only writers and the crew see it, or unseal it for every reader; a writer's, recorded",
7 doc: "`{ repo, number, sealed }`: seal a security report (and the pull that fixes it) so only writers and the crew see it, or unseal it for every reader; a writer's, recorded; with `action: \"private\"` instead of `sealed`, the private repository a writer fixes a sealed report in, made on first need, with its name and clone URL (the API's `POST …/issues/:number/private`, same rules)",
78 description:
89 "Seal or unseal an issue or pull: a sealed one is seen only by the app's writers and crew.",
910 inputSchema: {
@@ -12,15 +13,35 @@ export const tool: Tool = {
1213 repo: REPO_ARG,
1314 number: { type: "number" },
1415 sealed: { type: "boolean" },
16 action: { enum: ["private"] },
1517 },
16 required: ["repo", "number", "sealed"],
18 required: ["repo", "number"],
1719 },
1820 writes: true,
19 run(ctx, user, args) {
21 async run(ctx, user, args) {
2022 const repo = repoArg(ctx, user, args);
2123 if (isAnswer(repo)) return repo;
2224 const issue = issueArg(ctx, repo, args, "number");
2325 if ("ok" in issue) return issue;
26 if (args["action"] !== undefined && args["action"] !== "private")
sentinelcrew

Right guard: an unknown action must not fall through to the sealed write. It has no test, though (#6465).
27 return bad("invalid", "action is private");
28 if (args["action"] === "private") {
29 const made = await privateRepoForSealed(ctx.app, user, repo, issue);
30 if (!made.ok)
31 return made.error === "not_found"
32 ? bad("not_found", "no such issue")
33 : made.error === "not_sealed"
34 ? bad("not_sealed", "only a sealed report has a private place to fix it")
35 : made.error === "unverified"
36 ? bad("unverified", "verify your email first")
37 : bad(made.error, made.error);
38 const there = `${made.value.ownerName}/${made.value.name}`;
39 return good({
40 repo: there,
41 clone: `${ctx.url.origin}/${there}.git`,
42 says: `Fix #${issue.number} in ${there}: clone ${ctx.url.origin}/${there}.git`,
43 });
44 }
2445 if (typeof args["sealed"] !== "boolean")
2546 return bad("invalid", "sealed is true or false");
2647 const done = setIssueSealed(ctx.app, user, repo, issue, args["sealed"]);
test/mcp-private-fix.test.ts added +128
@@ -0,0 +1,128 @@
1 /**
2 * `seal` with `action: "private"` is POST …/issues/:number/private over MCP: the same
3 * action, so the same rules, and the clone URL comes from the forge's origin.
4 */
5 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
6 import { mkdtemp, rm } from "node:fs/promises";
7 import { tmpdir } from "node:os";
8 import { join } from "node:path";
9 import { createApp, type App } from "../src/app.ts";
10 import { createRepo } from "../src/actions/repos.ts";
11 import { createIssue } from "../src/core/issues.ts";
12 import { createToken } from "../src/core/tokens.ts";
13 import { createUser } from "../src/core/users.ts";
14 import { serve, type Servers } from "../src/http/server.ts";
15
16 describe("MCP seal action private", () => {
17 let dataDir: string;
18 let app: App;
19 let servers: Servers;
20 let alice: string;
21 let bob: string;
22 let carol: string;
23 let sealedNo: number;
24 let openNo: number;
25 let id = 1;
26
27 const rpc = async (token: string, method: string, params?: unknown) => {
28 const res = await fetch(`${servers.url}/mcp`, {
29 method: "POST",
30 headers: {
31 "content-type": "application/json",
32 accept: "application/json, text/event-stream",
33 authorization: `Bearer ${token}`,
34 },
35 body: JSON.stringify({ jsonrpc: "2.0", id: id++, method, params }),
36 });
37 return (await res.json()) as any;
38 };
39 const tool = async (token: string, args: unknown) => {
40 const body = await rpc(token, "tools/call", { name: "seal", arguments: { action: "private", ...(args as object) } });
41 const text: string = body.result.content[0].text;
42 return { text, isError: body.result.isError === true };
43 };
44
45 beforeAll(async () => {
46 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-private-fix-"));
47 app = createApp({ dataDir, port: 0 });
48 servers = await serve(app);
49 const mk = async (username: string, first = false) => {
50 const u = await createUser(app.db, {
51 origin: "seed",
52 trusted: true,
53 username,
54 email: `${username}@forge.local`,
55 password: "a-good-passphrase",
56 adminIfFirst: first,
57 });
58 if (!u.ok) throw new Error(u.error);
59 return u.value;
60 };
61 const a = await mk("alice", true);
62 const b = await mk("bob");
63 const c = await mk("carol");
64 alice = createToken(app.db, a.id, "rw", "rw").token;
65 bob = createToken(app.db, b.id, "rw", "rw").token;
66 carol = createToken(app.db, c.id, "rw", "rw").token;
67 const repo = await createRepo(app, a, { name: "game", visibility: "public" });
68 if (!repo.ok) throw new Error(String(repo.error));
69 const sealed = createIssue(app.db, { repoId: repo.value.id, authorId: a.id, title: "Embargoed", sealed: true });
70 const open = createIssue(app.db, { repoId: repo.value.id, authorId: a.id, title: "Open" });
71 if (!sealed.ok || !open.ok) throw new Error("issues");
72 sealedNo = sealed.value.number;
73 openNo = open.value.number;
74 }, 30_000);
75
76 afterAll(async () => {
77 servers.stop();
78 app.close();
79 await rm(dataDir, { recursive: true, force: true });
80 });
81
82 test("seal takes the private action in its schema", async () => {
83 const listed = await rpc(alice, "tools/list");
84 const t = listed.result.tools.find((x: any) => x.name === "seal");
85 expect(t.inputSchema.properties.action.enum).toEqual(["private"]);
86 expect(listed.result.tools.some((x: any) => x.name === "private_fix")).toBe(false);
87 });
88
89 test("a writer gets the private repository, cloned from the forge's origin", async () => {
90 const r = await tool(alice, { repo: "alice/game", number: sealedNo });
91 expect(r.isError).toBe(false);
92 const body = JSON.parse(r.text);
93 expect(body.repo).toMatch(/^alice\/.+/);
94 expect(body.clone).toBe(`${servers.url}/${body.repo}.git`);
95 expect(typeof body.says).toBe("string");
96 const again = JSON.parse((await tool(alice, { repo: "alice/game", number: sealedNo })).text);
97 expect(again.repo).toBe(body.repo);
98 });
99
100 test("a reader and a stranger are refused as the route refuses them", async () => {
101 for (const t of [bob, carol]) {
102 const r = await tool(t, { repo: "alice/game", number: sealedNo });
103 expect(r.isError).toBe(true);
104 expect(r.text).toContain("no_such_issue");
105 }
106 });
107
108 test("an issue that is not sealed is refused in the route's words", async () => {
109 const r = await tool(alice, { repo: "alice/game", number: openNo });
110 expect(r.isError).toBe(true);
111 expect(r.text).toContain("not_sealed");
112 });
113
114 test("a bad argument is a refusal, not a throw", async () => {
115 for (const args of [
116 {},
117 { repo: "alice/game" },
118 { repo: "alice/game", number: "1" },
119 { repo: "alice/game", number: -1 },
120 { repo: "alice/game", number: 1.5 },
121 { repo: 7, number: 1 },
122 { repo: "alice/game", number: 9999 },
123 ]) {
124 const r = await tool(alice, args);
125 expect(r.isError).toBe(true);
126 }
127 });
128 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs