trevato/forge

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

More
Forks1g fMapg aRuns14g rEventsFlows

MCP `merge`: a person lands a pull from any MCP client, as the pull page's Merge button does #7253

Merged torquecrew opened this

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

ConversationFiles
Merged

torque/forge:torque/6243-mcp-merge-a-person-lands-a-pull-from-any → main 1 commit fork

  • Merged. torque/6243-mcp-merge-a-person-lands-a-pull-from-any was merged into main as 6987cc1.

Review

Sign in to review this change.
5 files changed +298 −0
docs/MCP.md +1
@@ -98,6 +98,7 @@ for a tunnel and a forge that sleeps.
9898 | `limits` | `{ key?, value?, dryRun? }` (forge admin): every limit the forge's owner can hit, with its value, default and bounds; with `key` and `value` (`""` restores the default), planned first (`dryRun` is true unless `false`); a value out of bounds is refused, applying needs an admin-scope token | yes |
9999 | `make_it_yours` | `{ repo, name?, visibility?, dryRun? }`: a fork of an app you can see, as your own app, following the original; code and history copied, data and secrets not; private unless `visibility` is public (a public original only); at most 5 copies an account a day; plans unless `dryRun` is false | yes |
100100 | `map` | an app's plan: ready now, waiting on what, the critical path, where the crew is, parked twice and why (`app`, `done` to list closed issues too); draws the `ui://forge/map` card | |
101 | `merge` | `{ repo, number, strategy?, dryRun? }` (a person, never an agent: a person merges): land a pull as its page's Merge button does, `merge` (default) or `squash`; planned unless `dryRun` is false: the plan is the pull's `head`, `base`, `mergeable`, `conflicts` and `check` as `pull` reads them; landing is `mergePull`'s, with its refusals (conflict, review or check required, not a writer) in the page's words, and takes an rw token | yes |
101102 | `migrate` | `{ url, name?, crew? }` brings a public repository in by URL (history only; nothing reaches back) and answers `{ job, state, url }`; `{ job }` alone reads that job. Same validation, scope (`rw`) and private-address refusal as `POST /api/v1/migrate` | yes |
102103 | `milestones` | `{ app, issue? }`: how far each thing asked for is (and, with `issue`, whether you watch it: `watching`), from the dependency tree: the plan's limbs with their weights, or one milestone opened (who is building, what is blocked and on what, what merged in 7 days, an estimate, `closedPerDay` (14 days) and `paceDate`, and for an owner `spend`: dollars, changes landed ever, the costliest three issues) | |
103104 | `mint_invite` | `{ days? }`: mint a single-use invite link that expires in `days` (1–30, default 7) and is shown once; a person admin's, with an admin token | yes |
src/http/mcp/tools/index.ts +1
@@ -71,6 +71,7 @@ export { tool as limb } from "./limb.ts";
7171 export { tool as limits } from "./limits.ts";
7272 export { tool as make_it_yours } from "./make_it_yours.ts";
7373 export { tool as map } from "./map.ts";
74 export { tool as merge } from "./merge.ts";
7475 export { tool as migrate } from "./migrate.ts";
7576 export { tool as milestones } from "./milestones.ts";
7677 export { tool as mint_invite } from "./mint_invite.ts";
src/http/mcp/tools/merge.ts added +86
@@ -0,0 +1,86 @@
1 import { mayView } from "../../../core/sealed.ts";
2 import { mergePull } from "../../../actions/pulls.ts";
3 import { getPull, pullDir, pullMergeState } from "../../../core/pulls.ts";
4 import { checkSummary } from "../../../core/runs.ts";
5 import { mergeFailure } from "../../../web/pages/pull.ts";
6 import { good, bad, type Tool, REPO_ARG, repoArg, isAnswer } from "../shared.ts";
7
8 /**
9 * The pull page's Merge button from any MCP client: a second door to
10 * `mergePull`, never a looser one. Rule 1 in docs/APPS.md — the crew
11 * proposes, a person lands — is said here before anything is read, and
12 * `mergePull` says it again.
13 */
14 export const tool: Tool = {
15 name: "merge",
16 doc: "`{ repo, number, strategy?, dryRun? }` (a person, never an agent: a person merges): land a pull as its page's Merge button does, `merge` (default) or `squash`; planned unless `dryRun` is false: the plan is the pull's `head`, `base`, `mergeable`, `conflicts` and `check` as `pull` reads them; landing is `mergePull`'s, with its refusals (conflict, review or check required, not a writer) in the page's words, and takes an rw token",
17 description: "Land a pull, as its page's Merge button does; a person's only.",
18 inputSchema: {
19 type: "object",
20 properties: {
21 repo: REPO_ARG,
22 number: { type: "integer", minimum: 1 },
23 strategy: { type: "string", enum: ["merge", "squash"] },
24 dryRun: { type: "boolean" },
25 },
26 required: ["repo", "number"],
27 },
28 writes: true,
29 dryByDefault: true,
30 async run(ctx, user, args) {
31 if (user.kind === "agent")
32 return bad("forbidden", "the crew proposes and a person merges; an agent may not land a pull");
33 const repo = repoArg(ctx, user, args);
34 if (isAnswer(repo)) return repo;
35 const s = args["strategy"];
36 if (s !== undefined && s !== "merge" && s !== "squash")
37 return bad("invalid", "strategy is merge or squash");
38 const strategy = s === "squash" ? "squash" : "merge";
39 const n = args["number"];
40 const pull =
41 typeof n === "number" && Number.isInteger(n) && n > 0
42 ? getPull(ctx.app.db, repo.id, n)
43 : undefined;
44 if (pull === undefined || !mayView(ctx.app.db, repo, pull, user))
45 return bad("no_such_pull", `no pull #${String(n)}`);
46 const url = `/${repo.ownerName}/${repo.name}/pulls/${pull.number}`;
47 if (args["dryRun"] !== false) {
48 // The `pull` tool's reads, so the plan and the reading never disagree.
49 const dir = await pullDir(ctx.app.db, ctx.app.reposRoot, repo, pull);
50 const state = await pullMergeState(dir, pull);
51 const check = checkSummary(ctx.app.db, pull.id, pull.headOid);
52 return good({
53 dryRun: true,
54 number: pull.number,
55 state: pull.mergedAt !== null ? "merged" : pull.state,
56 head: pull.headBranch,
57 base: pull.baseBranch,
58 strategy,
59 mergeable: state?.mergeable ?? false,
60 conflicts: state?.conflicts ?? [],
61 check: { state: check.state, baseOid: check.run?.baseOid ?? "" },
62 url,
63 says: "This is the plan; nothing changed. Send dryRun: false to merge.",
64 });
65 }
66 const merged = await mergePull(ctx.app, user, repo, pull, strategy);
67 if (!merged.ok) {
68 const error = merged.error;
69 const code =
70 typeof error !== "string"
71 ? error.kind
72 : error === "unauthorized"
73 ? "forbidden"
74 : error;
75 return bad(code, mergeFailure(error));
76 }
77 return good({
78 dryRun: false,
79 merged: true,
80 strategy,
81 mergeOid: merged.value.mergeOid,
82 url,
83 says: `Merged #${pull.number} into ${pull.baseBranch}.`,
84 });
85 },
86 };
test/mcp-merge.test.ts added +209
@@ -0,0 +1,209 @@
1 /**
2 * The `merge` tool over MCP (#6243): the pull page's Merge button from any
3 * client. It plans unless `dryRun` is false, the plan is what `pull` reads,
4 * landing is `mergePull`'s with its refusals, and an agent — even one on the
5 * crew, who may write the repository — is refused: a person merges.
6 */
7 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { createApp, type App } from "../src/app.ts";
12 import { openPull } from "../src/actions/pulls.ts";
13 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
14 import { addCrew, createAgentRow } from "../src/core/agents.ts";
15 import { getPull } from "../src/core/pulls.ts";
16 import { getRepo, type Repo } from "../src/core/repos.ts";
17 import { createToken } from "../src/core/tokens.ts";
18 import { createUser, type User } from "../src/core/users.ts";
19 import { serve, type Servers } from "../src/http/server.ts";
20 import { resolveOid } from "../src/git/merge.ts";
21 import { repoDir } from "../src/git/repos.ts";
22 import { promote } from "./helpers/members.ts";
23
24 describe("the merge tool over MCP", () => {
25 let dataDir: string;
26 let app: App;
27 let servers: Servers;
28 let repo: Repo;
29 let alice: User;
30 let aliceRw: string;
31 let aliceRo: string;
32 let bobRw: string;
33 let agentRw: string;
34 let nextId = 1;
35
36 const tool = async (token: string, args: unknown) => {
37 const res = await fetch(`${servers.url}/mcp`, {
38 method: "POST",
39 headers: {
40 "content-type": "application/json",
41 accept: "application/json, text/event-stream",
42 authorization: `Bearer ${token}`,
43 },
44 body: JSON.stringify({
45 jsonrpc: "2.0",
46 id: nextId++,
47 method: "tools/call",
48 params: { name: "merge", arguments: args },
49 }),
50 });
51 const body = (await res.json()) as any;
52 const text: string = body.result.content[0].text;
53 return body.result.isError === true
54 ? { error: text }
55 : { value: JSON.parse(text) };
56 };
57
58 /** A pull on alice/game writing `path`, from a branch off main. */
59 const pullOf = async (branch: string, path = `${branch}.txt`, content = `${branch}\n`) => {
60 const made = await commitToBranch(app, alice, repo, {
61 branch,
62 message: branch,
63 files: [{ path, content }],
64 });
65 if (!made.ok) throw new Error(JSON.stringify(made.error));
66 const opened = await openPull(app, alice, repo, {
67 title: branch,
68 baseBranch: "main",
69 headBranch: branch,
70 });
71 if (!opened.ok) throw new Error(String(opened.error));
72 return opened.value;
73 };
74
75 const mainTip = () =>
76 resolveOid(repoDir(app.reposRoot, "alice", "game"), "refs/heads/main");
77
78 beforeAll(async () => {
79 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-merge-"));
80 app = createApp({ dataDir, port: 0 });
81 servers = await serve(app);
82 const mk = async (username: string, first = false, agent = false) => {
83 const made = await createUser(app.db, {
84 origin: agent ? "agent" : "seed",
85 trusted: true,
86 username,
87 email: `${username}@forge.local`,
88 password: agent ? "" : "a-good-passphrase",
89 adminIfFirst: first,
90 ...(agent ? { agent: true } : {}),
91 });
92 if (!made.ok) throw new Error(made.error);
93 return made.value;
94 };
95 alice = await mk("alice", true);
96 const bob = await mk("bob");
97 const wright = await mk("wright", false, true);
98 createAgentRow(app.db, { userId: wright.id, brief: "Builds." });
99 promote(app.db, alice, bob);
100 aliceRw = createToken(app.db, alice.id, "rw", "rw").token;
101 aliceRo = createToken(app.db, alice.id, "ro", "ro").token;
102 bobRw = createToken(app.db, bob.id, "rw", "rw").token;
103 agentRw = createToken(app.db, wright.id, "rw", "rw").token;
104 const made = await createRepo(app, alice, { name: "game", visibility: "public" });
105 if (!made.ok) throw new Error(String(made.error));
106 await commitToBranch(app, alice, made.value, {
107 message: "seed",
108 files: [{ path: "README.md", content: "# game\n" }],
109 });
110 repo = getRepo(app.db, "alice", "game")!;
111 // On the crew: the agent may write the repository, and still not land.
112 addCrew(app.db, repo.id, wright.id, alice.id);
113 }, 30_000);
114
115 afterAll(async () => {
116 servers.stop();
117 app.close();
118 await rm(dataDir, { recursive: true, force: true });
119 });
120
121 test("an agent's token is refused, dry run or not: a person merges", async () => {
122 const pull = await pullOf("by-agent");
123 const before = await mainTip();
124 for (const dryRun of [undefined, false]) {
125 const refused = await tool(agentRw, {
126 repo: "alice/game",
127 number: pull.number,
128 ...(dryRun === undefined ? {} : { dryRun }),
129 });
130 expect(refused.error).toStartWith("forbidden:");
131 expect(refused.error).toContain("a person merges");
132 }
133 expect(await mainTip()).toBe(before);
134 expect(getPull(app.db, repo.id, pull.number)!.mergedAt).toBeNull();
135 });
136
137 test("the plan says what would land and changes nothing", async () => {
138 const pull = await pullOf("planned");
139 const before = await mainTip();
140 // A read-only token may ask for the plan: it is a read.
141 const plan = await tool(aliceRo, { repo: "alice/game", number: pull.number, strategy: "squash" });
142 expect(plan.value).toMatchObject({
143 dryRun: true,
144 number: pull.number,
145 state: "open",
146 head: "planned",
147 base: "main",
148 strategy: "squash",
149 mergeable: true,
150 conflicts: [],
151 check: { state: expect.any(String), baseOid: expect.any(String) },
152 url: `/alice/game/pulls/${pull.number}`,
153 });
154 expect(await mainTip()).toBe(before);
155 // … and may not land.
156 const refused = await tool(aliceRo, { repo: "alice/game", number: pull.number, dryRun: false });
157 expect(refused.error).toStartWith("forbidden:");
158 expect(getPull(app.db, repo.id, pull.number)!.mergedAt).toBeNull();
159 });
160
161 test("a person's rw token merges a clean pull onto the base", async () => {
162 const pull = await pullOf("clean");
163 const landed = await tool(aliceRw, { repo: "alice/game", number: pull.number, dryRun: false });
164 expect(landed.value).toMatchObject({
165 dryRun: false,
166 merged: true,
167 strategy: "merge",
168 url: `/alice/game/pulls/${pull.number}`,
169 });
170 expect(await mainTip()).toBe(landed.value.mergeOid);
171 expect(getPull(app.db, repo.id, pull.number)!.mergedAt).not.toBeNull();
172 // Once landed, again is mergePull's own refusal.
173 const again = await tool(aliceRw, { repo: "alice/game", number: pull.number, dryRun: false });
174 expect(again.error).toStartWith("already_merged:");
175 });
176
177 test("a conflict is refused in the page's words, and nothing lands", async () => {
178 const first = await pullOf("left", "same.txt", "left\n");
179 const second = await pullOf("right", "same.txt", "right\n");
180 expect((await tool(aliceRw, { repo: "alice/game", number: first.number, dryRun: false })).value.merged).toBe(true);
181 const tip = await mainTip();
182 const plan = await tool(aliceRw, { repo: "alice/game", number: second.number });
183 expect(plan.value.mergeable).toBe(false);
184 expect(plan.value.conflicts).toEqual(["same.txt"]);
185 const refused = await tool(aliceRw, { repo: "alice/game", number: second.number, dryRun: false });
186 expect(refused.error).toStartWith("conflict:");
187 expect(refused.error).toContain("The branches conflict in 1 file");
188 expect(await mainTip()).toBe(tip);
189 });
190
191 test("a person who cannot write is refused by mergePull", async () => {
192 const pull = await pullOf("not-yours");
193 const refused = await tool(bobRw, { repo: "alice/game", number: pull.number, dryRun: false });
194 expect(refused.error).toStartWith("forbidden:");
195 expect(getPull(app.db, repo.id, pull.number)!.mergedAt).toBeNull();
196 });
197
198 test("bad input stops at the boundary", async () => {
199 const pull = await pullOf("bad-input");
200 const base = { repo: "alice/game", number: pull.number, dryRun: false };
201 expect((await tool(aliceRw, { ...base, strategy: "rebase" })).error).toContain(
202 "strategy is merge or squash",
203 );
204 expect((await tool(aliceRw, { ...base, number: 9999 })).error).toContain("no pull #9999");
205 expect((await tool(aliceRw, { ...base, number: "1" })).error).toContain("no_such_pull");
206 expect((await tool(aliceRw, { ...base, repo: "nope" })).error).toStartWith("not_found:");
207 expect(getPull(app.db, repo.id, pull.number)!.mergedAt).toBeNull();
208 });
209 });
test/mcp.test.ts +1
@@ -274,6 +274,7 @@ describe("the forge over MCP", () => {
274274 "origin_apps",
275275 "front_page",
276276 "map",
277 "merge",
277278 "milestones",
278279 "watch",
279280 "runs",

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs