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

MCP `pulls` draws a card in MCP Apps hosts: one row per pull with its check, each linked to its page #7396

Merged opened this

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

Merged

mason/forge:mason/7375-mcp-pulls-draws-a-card-in-mcp-apps-hosts → main 2 commits fork 3 threads · 3 unresolved

  • Merged. mason/7375-mcp-pulls-draws-a-card-in-mcp-apps-hosts was merged into main as 67d82bf.

Review

Sign in to review this change.
4 files changed +169 −10
src/http/mcp/tools/pulls.ts +4 −1
@@ -35,7 +35,7 @@ export const tool: Tool = {
3535 pulls.map((p) => pullDir(db, ctx.app.reposRoot, repo, p)),
3636 );
3737 const merge = await pullMergeStates(pulls.map((pull, i) => ({ dir: dirs[i]!, pull })));
38 return good({
38 const answer = good({
3939 pulls: pulls.map((p, i) => ({
4040 number: p.number,
4141 title: p.title,
@@ -47,5 +47,8 @@ export const tool: Tool = {
4747 mergeable: merge[i]?.mergeable ?? null,
4848 })),
4949 });
50 // What the card needs and the text does not say: where the pulls live, and
51 // the limit that may have cut the list short.
outdated

The card can't tell "20 open" from "the first 20 of 40 open". Pass limit here (or a real open count) so the status can say 20+ open.

52 return { ...answer, card: { repo: `${repo.ownerName}/${repo.name}`, state, limit } };
5053 },
5154 };
src/web/studio/cards.ts +37 −9
@@ -10,7 +10,7 @@
1010 import { html, raw } from "../html.ts";
1111 import { timeAgo } from "../ui.ts";
1212
13 export type CardKind = "app" | "run" | "decision" | "wave" | "map" | "feed" | "pull" | "explore" | "issue";
13 export type CardKind = "app" | "run" | "decision" | "wave" | "map" | "feed" | "pull" | "pulls" | "explore" | "issue";
1414
1515 export interface CardRow {
1616 readonly label: string;
@@ -39,7 +39,7 @@ const here = (origin: string, path: string): boolean => origin !== "" && /^\/(?!
3939
4040 export const CARD_MIME = "text/html;profile=mcp-app";
4141 export const CARD_URI = (kind: CardKind): string => `ui://forge/${kind}`;
42 export const CARD_KINDS: readonly CardKind[] = ["app", "run", "decision", "wave", "map", "feed", "pull", "explore", "issue"];
42 export const CARD_KINDS: readonly CardKind[] = ["app", "run", "decision", "wave", "map", "feed", "pull", "pulls", "explore", "issue"];
4343
4444 /** The tool whose answer each card draws. */
4545 export const CARD_OF_TOOL: Readonly<Record<string, CardKind>> = {
@@ -53,6 +53,7 @@ export const CARD_OF_TOOL: Readonly<Record<string, CardKind>> = {
5353 explore: "explore",
5454 new_app: "app",
5555 pull: "pull",
56 pulls: "pulls",
5657 issue: "issue",
5758 };
5859
@@ -65,6 +66,7 @@ const CARD_TITLE: Readonly<Record<CardKind, string>> = {
6566 feed: "Feed",
6667 explore: "Explore",
6768 pull: "Pull",
69 pulls: "Pulls",
6870 issue: "Issue",
6971 };
7072
@@ -76,9 +78,18 @@ const text = (v: unknown): string =>
7678 typeof v === "string" ? v : typeof v === "number" ? String(v) : "";
7779 const LIMIT = 12;
7880 const FEED_LIMIT = 10;
81 const PULLS_LIMIT = 20;
7982 /** The forge's id shape (src/kernel/id.ts): 26 lowercase Crockford base32 characters. */
8083 const RUN_ID = /^[0-9a-hjkmnp-tv-z]{26}$/;
8184
85 /** A run state as one check word: what a pull's check says at a glance. */
86 const checkWord = (state: string): string =>
87 state === "ok" ? "green"
88 : state === "failed" ? "red"
89 : state === "queued" || state === "running" ? "running"
90 : state === "cancelled" ? "cancelled"
91 : "none";
92
8293 /** An absolute http(s) address: the only origin a card builds links on. */
8394 const web = (u: string): boolean => /^https?:\/\//.test(u);
8495 /** A repository named `owner/name`. */
@@ -121,16 +132,11 @@ export function cardFor(
121132 const p = obj(value);
122133 const state = text(p["state"]);
123134 const check = text(obj(p["check"])["state"]);
124 const checkWord =
125 check === "ok" ? "check green"
126 : check === "failed" ? "check red"
127 : check === "queued" || check === "running" ? "check running"
128 : check === "cancelled" ? "check cancelled"
129 : "";
135 const word = checkWord(check);
130136 const conflicts = list(p["conflicts"]).map(text).filter((c) => c !== "");
131137 status = [
132138 state,
133 state === "open" ? checkWord : "",
139 state === "open" && word !== "none" ? `check ${word}` : "",
134140 state === "open" && conflicts.length > 0 ? "conflicts" : "",
135141 ]
136142 .filter((w) => w !== "")
@@ -150,6 +156,28 @@ export function cardFor(
150156 if (web(preview)) rows.push({ label: "Preview", value: preview, link: preview });
151157 return { kind, title: `#${number} ${text(p["title"])}`.trim(), status, rows, actions };
152158 }
159 if (kind === "pulls") {
160 // `pulls` answers `{ pulls: [{ number, title, check, … }] }`; the repo and
outdated

pulls.length is the number returned, capped by limit. It is not the number open. Make the status true when the answer was cut off at the limit, and add a test for it.

161 // the state asked for come in `extra`.
162 const pulls = list(obj(value)["pulls"]).map(obj);
163 // The tool stops at `limit`: a full answer may be the first of more.
164 const cut = typeof extra["limit"] === "number" && pulls.length >= extra["limit"];
165 status = `${pulls.length}${cut ? "+" : ""} ${extra["state"] === "closed" ? "closed" : "open"}`;
166 const slug = text(extra["repo"]);
167 for (const p of pulls.slice(0, PULLS_LIMIT)) {
168 const number = text(p["number"]);
169 const check = text(p["check"]);
outdated

This repeats the pull card's check mapping a few lines above, and the two already disagree on cancelled. Use one helper for both.

170 const path = `/${slug}/pulls/${number}`;
171 rows.push({
172 label: `#${number} ${text(p["title"])}`.trim(),
173 value: checkWord(check),
174 ...(web(origin) && slugOf(slug) && /^[1-9]\d*$/.test(number) && here(origin, path)
175 ? { link: origin + path }
176 : {}),
177 });
178 }
179 return { kind, title: CARD_TITLE[kind], status, rows, actions };
180 }
153181 if (kind === "issue") {
154182 const v = obj(value);
155183 const number = text(v["number"]);
test/mcp-pulls-card.test.ts added +126
@@ -0,0 +1,126 @@
1 /**
2 * MCP `pulls` draws the `ui://forge/pulls` card: `N open`, one row per pull
3 * with its check word, each label linked to its pull page on this forge.
4 * The text answer is unchanged and a title with markup arrives as text.
5 */
6 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
7 import { mkdtemp, rm } from "node:fs/promises";
8 import { tmpdir } from "node:os";
9 import { join } from "node:path";
10 import { createApp, type App } from "../src/app.ts";
11 import { openPull } from "../src/actions/pulls.ts";
12 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
13 import { createToken } from "../src/core/tokens.ts";
14 import { createUser } from "../src/core/users.ts";
15 import { serve, type Servers } from "../src/http/server.ts";
16 import { cardFor } from "../src/web/studio/cards.ts";
17 import { quietLogs } from "./helpers/globals.ts";
18
19 quietLogs();
20
21 const EVIL = `<img src=x onerror=alert(1)>`;
22
23 describe("MCP pulls card", () => {
24 let dataDir: string;
25 let app: App;
26 let servers: Servers;
27 let token = "";
28 let id = 1;
29 const numbers: number[] = [];
30
31 const rpc = async (method: string, params: unknown = {}) => {
32 const res = await fetch(`${servers.url}/mcp`, {
33 method: "POST",
34 headers: {
35 "content-type": "application/json",
36 accept: "application/json, text/event-stream",
37 authorization: `Bearer ${token}`,
38 },
39 body: JSON.stringify({ jsonrpc: "2.0", id: id++, method, params }),
40 });
41 return (await res.json()) as any;
42 };
43
44 beforeAll(async () => {
45 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-pulls-card-"));
46 app = createApp({ dataDir, port: 0 });
47 servers = await serve(app);
48 const a = await createUser(app.db, { origin: "seed", trusted: true,
49 username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true });
50 if (!a.ok) throw new Error("users");
51 token = createToken(app.db, a.value.id, "ro", "ro").token;
52 const made = await createRepo(app, a.value, { name: "game", visibility: "private" });
53 if (!made.ok) throw new Error(String(made.error));
54 const seeded = await commitToBranch(app, a.value, made.value, {
55 branch: "main", message: "seed", files: [{ path: "README.md", content: "# game\n" }],
56 });
57 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
58 for (const [branch, title] of [["one", "First"], ["two", EVIL]] as const) {
59 const c = await commitToBranch(app, a.value, made.value, {
60 branch, message: branch, files: [{ path: `${branch}.txt`, content: "x\n" }],
61 });
62 if (!c.ok) throw new Error(JSON.stringify(c.error));
63 const p = await openPull(app, a.value, made.value, { title, baseBranch: "main", headBranch: branch });
64 if (!p.ok) throw new Error(String(p.error));
65 numbers.push(p.value.number);
66 }
67 });
68
69 afterAll(async () => {
70 servers.stop();
71 app.close();
72 await rm(dataDir, { recursive: true, force: true });
73 });
74
75 test("resources/list offers ui://forge/pulls", async () => {
76 const uris = (await rpc("resources/list")).result.resources.map((r: any) => r.uri);
77 expect(uris).toContain("ui://forge/pulls");
78 });
79
80 test("tools/call pulls carries the card beside an unchanged text answer", async () => {
81 const body = await rpc("tools/call", { name: "pulls", arguments: { repo: "alice/game" } });
82 const card = body.result.structuredContent.card;
83 expect(card.kind).toBe("pulls");
84 expect(card.title).toBe("Pulls");
85 expect(card.status).toBe("2 open");
86 const [two, one] = [numbers[1]!, numbers[0]!];
87 expect(card.rows).toEqual([
88 { label: `#${two} ${EVIL}`, value: "none", link: `${servers.url}/alice/game/pulls/${two}` },
89 { label: `#${one} First`, value: "none", link: `${servers.url}/alice/game/pulls/${one}` },
90 ]);
91 const text = JSON.parse(body.result.content[0].text);
92 expect(text.pulls.map((p: any) => p.number)).toEqual([two, one]);
93 expect("card" in text).toBe(false);
94
95 const closed = await rpc("tools/call", { name: "pulls", arguments: { repo: "alice/game", state: "closed" } });
96 expect(closed.result.structuredContent.card.status).toBe("0 closed");
97 expect(closed.result.structuredContent.card.rows).toEqual([]);
98
99 // More open than `limit`: the status says the list was cut, not "1 open".
100 const cut = await rpc("tools/call", { name: "pulls", arguments: { repo: "alice/game", limit: 1 } });
101 expect(cut.result.structuredContent.card.status).toBe("1+ open");
102 expect(cut.result.structuredContent.card.rows.length).toBe(1);
103 });
104
105 test("the check word, the cut-off status, the 20-row ceiling, and links only on this forge", () => {
106 const pulls = ["ok", "failed", "running", "queued", "none", "cancelled"].map((check, i) => ({
107 number: i + 1, title: `P${i + 1}`, check,
108 }));
109 const card = cardFor("pulls", { pulls }, "https://forge.test", { repo: "alice/game", state: "open" })!;
110 expect(card.rows.map((r) => r.value)).toEqual(["green", "red", "running", "running", "none", "cancelled"]);
111 expect(card.rows[0]!.link).toBe("https://forge.test/alice/game/pulls/1");
112
113 const many = Array.from({ length: 30 }, (_, i) => ({ number: i + 1, title: "t", check: "ok" }));
114 const big = cardFor("pulls", { pulls: many }, "https://forge.test", { repo: "alice/game" })!;
115 expect(big.status).toBe("30 open");
116 const capped = cardFor("pulls", { pulls: many.slice(0, 20) }, "https://forge.test", { repo: "alice/game", limit: 20 })!;
117 expect(capped.status).toBe("20+ open");
118 expect(big.rows.length).toBe(20);
119
120 // No origin, a repo that is not `owner/name`, or a number that is not one: no link.
121 expect(cardFor("pulls", { pulls }, "", { repo: "alice/game" })!.rows[0]!.link).toBeUndefined();
122 expect(cardFor("pulls", { pulls }, "https://forge.test", { repo: "/evil.example/x" })!.rows[0]!.link).toBeUndefined();
123 const odd = cardFor("pulls", { pulls: [{ number: "//evil", title: "x", check: "ok" }] }, "https://forge.test", { repo: "alice/game" })!;
124 expect(odd.rows[0]!.link).toBeUndefined();
125 });
126 });
test/mcp.test.ts +2
@@ -1218,6 +1218,7 @@ describe("the forge over MCP", () => {
12181218 "ui://forge/issue",
12191219 "ui://forge/map",
12201220 "ui://forge/pull",
1221 "ui://forge/pulls",
12211222 "ui://forge/run",
12221223 "ui://forge/wave",
12231224 ]);
@@ -1240,6 +1241,7 @@ describe("the forge over MCP", () => {
12401241 expect(meta("issue")).toBe("ui://forge/issue");
12411242 expect(meta("explore")).toBe("ui://forge/explore");
12421243 expect(meta("pull")).toBe("ui://forge/pull");
1244 expect(meta("pulls")).toBe("ui://forge/pulls");
12431245 });
12441246
12451247 test("tools/call pull carries the card beside an unchanged text answer", async () => {

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs