Public: anyone can read this; the crew can change it.
MCP `pulls`: list a repository's open pulls, briefly, with each one's check and review state #4055
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3401-mcp-pulls-list-a-repository-s-open-pulls → main 2 commits fork 2 threads · 2 unresolved
4 files
changed +220
−1
docs/MCP.md +1
| @@ -35,6 +35,7 @@ for a tunnel and a forge that sleeps. | ||
| 35 | 35 | | `issue` | `{ repo, number }`: one issue's title, state, closeReason, body, labels, assignee, comments (author, authorIsAgent, body, at) and blockedBy/blocks, as `GET …/issues/:number`; a sealed one you cannot see is not found | | |
| 36 | 36 | | `pull` | `{ repo, number }`: a pull's title, state, head and base, and for an open pull whether it merges into the base's current head (`mergeable`) and the paths that stop it (`conflicts`), and `check: { state, baseOid }`: the latest check's state and the base tip it was merged with (empty when it merged nothing); `mergeable` and `conflicts` are the same fields as `GET …/pulls/:number`, `check` is `state` and `run.baseOid` from `GET …/pulls/:number/checks` | | |
| 37 | 37 | | `edit` | a new title or body for an issue or a pull: its author's, or a writer's; an agent edits only what it wrote | yes | |
| 38 | | `pulls` | `{ repo, state?, limit? }`: a repository's pulls, newest first, one brief line each: `number`, `title`, `author`, `head`, `check` (state), `approvals`, `changesRequested`, `mergeable` (null once closed); `state` open (default) or closed, `limit` 1-50 (default 20); no bodies, no diffs | no | | |
| 38 | 39 | | `read_file` | a file (utf-8, or base64 for binary) or a directory listing at a ref, as GET …/contents | no | |
| 39 | 40 | | `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 | |
| 40 | 41 | | `now` | what the crew is doing right now: who, on what, since when, at what cost; a planner that declined the direction and waits for you comes first (`run: null`, its words in `says`) | | |
src/http/mcp.ts +46 −1
| @@ -112,9 +112,10 @@ import { dependenciesOf } from "../core/dependencies.ts"; | ||
| 112 | 112 | import { waitsForPerson } from "../crew/review.ts"; |
| 113 | 113 | import { attachLook } from "../actions/attach_look.ts"; |
| 114 | 114 | import { lookUrl } from "../core/looks.ts"; |
| 115 | import { reviewSummariesFor } from "../core/reviews.ts"; | |
| 115 | 116 | import { pullProposalOf } from "../core/proposal_token.ts"; |
| 116 | 117 | import { resolveOid } from "../git/merge.ts"; |
| 117 | import { getPull, pullDir, pullMergeState } from "../core/pulls.ts"; | |
| 118 | import { getPull, listPulls, pullDir, pullMergeState, pullMergeStates } from "../core/pulls.ts"; | |
| 118 | 119 | import { repoGraph } from "../core/dependencies.ts"; |
| 119 | 120 | import { pillarView } from "../core/pillars.ts"; |
| 120 | 121 | import { createBranch, deleteBranch } from "../actions/branches.ts"; |
| @@ -812,6 +813,50 @@ const TOOLS: readonly Tool[] = [ | ||
| 812 | 813 | }); |
| 813 | 814 | }, |
| 814 | 815 | }, |
| 816 | { | |
| 817 | name: "pulls", | |
| 818 | description: | |
| 819 | "List a repository's pulls briefly, newest first: number, title, author, head, check, approvals, changesRequested, mergeable. `state` open (default) or closed; `limit` 1-50 (default 20).", | |
| 820 | inputSchema: { | |
| 821 | type: "object", | |
| 822 | properties: { | |
| 823 | repo: REPO_ARG, | |
| 824 | state: { type: "string", enum: ["open", "closed"] }, | |
| 825 | limit: { type: "integer", minimum: 1, maximum: 50 }, | |
| 826 | }, | |
| 827 | required: ["repo"], | |
| 828 | }, | |
| 829 | writes: false, | |
| 830 | async run(ctx, user, args) { | |
| 831 | const repo = repoArg(ctx, user, args); | |
| 832 | if (isAnswer(repo)) return repo; | |
| 833 | const state = args["state"] ?? "open"; | |
| 834 | if (state !== "open" && state !== "closed") | |
| 835 | return bad("invalid", "`state` is open or closed"); | |
| 836 | const limit = args["limit"] ?? 20; | |
| 837 | if (typeof limit !== "number" || !Number.isInteger(limit) || limit < 1 || limit > 50) | |
| 838 | return bad("invalid", "`limit` is a whole number from 1 to 50"); | |
| 839 | const db = ctx.app.db; | |
| 840 | const pulls = listPulls(db, repo.id, { state, limit, ...sealedScope(db, repo, user) }); | |
| 841 | const reviews = reviewSummariesFor(db, pulls); | |
The tests only ever see zero approvals and zero changes requested here. Add a pull with real reviews so this mapping is tested. |
||
| 842 | const dirs = await Promise.all( | |
| 843 | pulls.map((p) => pullDir(db, ctx.app.reposRoot, repo, p)), | |
| 844 | ); | |
| 845 | const merge = await pullMergeStates(pulls.map((pull, i) => ({ dir: dirs[i]!, pull }))); | |
| 846 | return good({ | |
| 847 | pulls: pulls.map((p, i) => ({ | |
| 848 | number: p.number, | |
| 849 | title: p.title, | |
| 850 | author: p.authorName, | |
| 851 | head: p.headBranch, | |
| 852 | check: checkSummary(db, p.id, p.headOid).state, | |
| 853 | approvals: reviews.get(p.id)?.approvals ?? 0, | |
| 854 | changesRequested: reviews.get(p.id)?.changesRequested ?? 0, | |
| 855 | mergeable: merge[i]?.mergeable ?? null, | |
| 856 | })), | |
| 857 | }); | |
| 858 | }, | |
| 859 | }, | |
| 815 | 860 | { |
| 816 | 861 | name: "variant", |
| 817 | 862 | description: |
test/mcp-pulls.test.ts added +172
| @@ -0,0 +1,172 @@ | ||
| 1 | /** MCP `pulls`: a brief line per pull, bounded input, private repositories not found, tools/list under its ceiling. */ | |
| 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, type App } from "../src/app.ts"; | |
| 7 | import { commitToBranch } from "../src/actions/repos.ts"; | |
| 8 | import { createPull, type Pull } from "../src/core/pulls.ts"; | |
| 9 | import { setIssueState } from "../src/core/issues.ts"; | |
| 10 | import { submitReview } from "../src/core/reviews.ts"; | |
| 11 | import { setSealed } from "../src/core/sealed.ts"; | |
| 12 | import { claimRun, createRun, finishRun } from "../src/core/runs.ts"; | |
| 13 | import { createRepo } from "../src/core/repos.ts"; | |
| 14 | import { repoDir } from "../src/git/repos.ts"; | |
| 15 | import { createToken } from "../src/core/tokens.ts"; | |
| 16 | import { createUser } from "../src/core/users.ts"; | |
| 17 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 18 | import { quietLogs } from "./helpers/globals.ts"; | |
| 19 | ||
| 20 | quietLogs(); | |
| 21 | ||
| 22 | let dataDir: string; | |
| 23 | let app: App; | |
| 24 | let servers: Servers; | |
| 25 | const tok: Record<string, string> = {}; | |
| 26 | ||
| 27 | const ok = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => { | |
| 28 | if (!r.ok) throw new Error(JSON.stringify(r.error)); | |
| 29 | return r.value; | |
| 30 | }; | |
| 31 | ||
| 32 | async function rpc(token: string, method: string, params?: unknown): Promise<any> { | |
| 33 | const res = await fetch(`${servers.url}/mcp`, { | |
| 34 | method: "POST", | |
| 35 | headers: { | |
| 36 | "content-type": "application/json", | |
| 37 | accept: "application/json, text/event-stream", | |
| 38 | authorization: `Bearer ${token}`, | |
| 39 | }, | |
| 40 | body: JSON.stringify({ jsonrpc: "2.0", id: 1, method, params }), | |
| 41 | }); | |
| 42 | return { text: await res.text() }; | |
| 43 | } | |
| 44 | ||
| 45 | async function pulls(token: string, args: unknown): Promise<{ error?: string; value?: any }> { | |
| 46 | const { text } = await rpc(token, "tools/call", { name: "pulls", arguments: args }); | |
| 47 | const body = JSON.parse(text); | |
| 48 | const out: string = body.result.content[0].text; | |
| 49 | return body.result.isError === true ? { error: out } : { value: JSON.parse(out) }; | |
| 50 | } | |
| 51 | ||
| 52 | beforeAll(async () => { | |
| 53 | dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-pulls-")); | |
| 54 | app = createApp({ dataDir, port: 0 }); | |
| 55 | servers = await serve(app); | |
| 56 | const make = (username: string, adminIfFirst = false) => | |
| 57 | createUser(app.db, { | |
| 58 | origin: "seed", | |
| 59 | trusted: true, | |
| 60 | username, | |
| 61 | email: `${username}@forge.local`, | |
| 62 | password: "a-good-passphrase", | |
| 63 | adminIfFirst, | |
| 64 | }); | |
| 65 | const ada = ok(await make("ada", true)); | |
| 66 | const rita = ok(await make("rita")); | |
| 67 | const sam = ok(await make("sam")); | |
| 68 | const pub = ok(await createRepo(app.db, app.reposRoot, { ownerId: ada.id, name: "svc", visibility: "public" })); | |
| 69 | const priv = ok(await createRepo(app.db, app.reposRoot, { ownerId: ada.id, name: "vault", visibility: "private" })); | |
| 70 | for (const repo of [pub, priv]) { | |
| 71 | ok(await commitToBranch(app, ada, repo, { message: "seed", files: [{ path: "a.txt", content: "a\n" }] })); | |
| 72 | for (const b of ["one", "two"]) | |
| 73 | ok(await commitToBranch(app, ada, repo, { message: b, branch: b, files: [{ path: `${b}.txt`, content: `${b}\n` }] } as any)); | |
| 74 | for (const b of ["one", "two"]) | |
| 75 | ok(await createPull(app.db, repoDir(app.reposRoot, ada.username, repo.name), { | |
| 76 | repoId: repo.id, authorId: ada.id, title: `Pull ${b}`, baseBranch: "main", headBranch: b, | |
| 77 | })); | |
| 78 | } | |
| 79 | // A third repository with the pulls whose state is the point. | |
| 80 | const lab = ok(await createRepo(app.db, app.reposRoot, { ownerId: ada.id, name: "lab", visibility: "public" })); | |
| 81 | ok(await commitToBranch(app, ada, lab, { message: "seed", files: [{ path: "a.txt", content: "a\n" }] })); | |
| 82 | const open = async (b: string, author = ada): Promise<Pull> => { | |
| 83 | ok(await commitToBranch(app, ada, lab, { message: b, branch: b, files: [{ path: `${b}.txt`, content: `${b}\n` }] } as any)); | |
| 84 | return ok(await createPull(app.db, repoDir(app.reposRoot, ada.username, "lab"), { | |
| 85 | repoId: lab.id, authorId: author.id, title: `Pull ${b}`, baseBranch: "main", headBranch: b, | |
| 86 | })); | |
| 87 | }; | |
| 88 | const reviewed = await open("reviewed"); | |
| 89 | const asked = await open("asked"); | |
| 90 | const sealed = await open("sealed", sam); | |
| 91 | const closed = await open("closed"); | |
| 92 | ok(submitReview(app.db, { issueId: reviewed.id, authorId: rita.id, state: "approved", commitOid: reviewed.headOid })); | |
| 93 | ok(submitReview(app.db, { issueId: asked.id, authorId: ada.id, state: "changes_requested", commitOid: asked.headOid })); | |
| 94 | const run = createRun(app.db, { repoId: lab.id, kind: "check", issueId: reviewed.id, commitOid: reviewed.headOid }); | |
| 95 | claimRun(app.db, run.id); | |
| 96 | finishRun(app.db, run.id, { state: "ok", summary: "green" }); | |
| 97 | setSealed(app.db, sealed.id, true); | |
| 98 | setIssueState(app.db, closed.id, "closed"); | |
| 99 | tok["sam"] = createToken(app.db, sam.id, "rw", "rw").token; | |
| 100 | tok["ada"] = createToken(app.db, ada.id, "rw", "rw").token; | |
| 101 | tok["rita"] = createToken(app.db, rita.id, "rw", "rw").token; | |
| 102 | }, 30_000); | |
| 103 | ||
| 104 | afterAll(async () => { | |
| 105 | servers?.stop(); | |
| 106 | app?.close(); | |
| 107 | await rm(dataDir, { recursive: true, force: true }); | |
| 108 | }); | |
| 109 | ||
| 110 | test("lists open pulls in the brief shape, newest first, limited", async () => { | |
| 111 | const all = (await pulls(tok["rita"]!, { repo: "ada/svc" })).value.pulls; | |
| 112 | expect(all.map((p: any) => p.number)).toEqual([2, 1]); | |
| 113 | expect(all[0]).toEqual({ | |
| 114 | number: 2, | |
| 115 | title: "Pull two", | |
| 116 | author: "ada", | |
| 117 | head: "two", | |
| 118 | check: "none", | |
| 119 | approvals: 0, | |
| 120 | changesRequested: 0, | |
| 121 | mergeable: true, | |
| 122 | }); | |
| 123 | expect((await pulls(tok["rita"]!, { repo: "ada/svc", limit: 1 })).value.pulls).toHaveLength(1); | |
| 124 | expect((await pulls(tok["rita"]!, { repo: "ada/svc", state: "closed" })).value.pulls).toEqual([]); | |
| 125 | }); | |
| 126 | ||
| 127 | test("bad input stops at the boundary", async () => { | |
| 128 | for (const args of [ | |
| 129 | { repo: "ada/svc", limit: 0 }, | |
| 130 | { repo: "ada/svc", limit: 51 }, | |
| 131 | { repo: "ada/svc", limit: "5" }, | |
| 132 | { repo: "ada/svc", state: "all" }, | |
| 133 | ]) | |
| 134 | expect((await pulls(tok["ada"]!, args)).error).toBeDefined(); | |
| 135 | }); | |
| 136 | ||
| 137 | test("a private repository the caller cannot read is not found, as pull says", async () => { | |
| 138 | const theirs = await pulls(tok["rita"]!, { repo: "ada/vault" }); | |
| 139 | const missing = await pulls(tok["rita"]!, { repo: "ada/nothing" }); | |
| 140 | expect(theirs.error).toBeDefined(); | |
| 141 | expect(theirs.error).toBe(missing.error); | |
| 142 | expect((await pulls(tok["ada"]!, { repo: "ada/vault" })).value.pulls).toHaveLength(2); | |
| 143 | }); | |
| 144 | ||
| 145 | test("tools/list lists pulls and stays under 40 KB", async () => { | |
| 146 | const { text } = await rpc(tok["ada"]!, "tools/list"); | |
| 147 | expect(Buffer.byteLength(text)).toBeLessThanOrEqual(40_000); | |
| 148 | expect(JSON.parse(text).result.tools.map((t: any) => t.name)).toContain("pulls"); | |
| 149 | }); | |
| 150 | ||
| 151 | const labPulls = async (who: string, args: object = {}): Promise<any[]> => | |
| 152 | (await pulls(tok[who]!, { repo: "ada/lab", ...args })).value.pulls; | |
| 153 | ||
| 154 | test("review and check state are counted, not assumed", async () => { | |
| 155 | const byHead = Object.fromEntries((await labPulls("rita")).map((p) => [p.head, p])); | |
| 156 | expect(byHead["reviewed"]).toMatchObject({ approvals: 1, changesRequested: 0, check: "ok" }); | |
| 157 | expect(byHead["asked"]).toMatchObject({ approvals: 0, changesRequested: 1, check: "none" }); | |
| 158 | }); | |
| 159 | ||
| 160 | test("a closed pull is listed under state closed, with mergeable null", async () => { | |
| 161 | expect((await labPulls("rita")).map((p) => p.head)).not.toContain("closed"); | |
| 162 | const closed = await labPulls("rita", { state: "closed" }); | |
| 163 | expect(closed.map((p) => p.head)).toEqual(["closed"]); | |
| 164 | expect(closed[0].mergeable).toBeNull(); | |
| 165 | }); | |
| 166 | ||
| 167 | test("a sealed pull is seen by a writer and its reporter, not by a plain reader", async () => { | |
| 168 | const heads = async (who: string) => (await labPulls(who)).map((p) => p.head); | |
| 169 | expect(await heads("ada")).toContain("sealed"); | |
| 170 | expect(await heads("sam")).toContain("sealed"); | |
| 171 | expect(await heads("rita")).not.toContain("sealed"); | |
| 172 | }); |
test/mcp.test.ts +1
| @@ -178,6 +178,7 @@ describe("the forge over MCP", () => { | ||
| 178 | 178 | "say", |
| 179 | 179 | "issue", |
| 180 | 180 | "pull", |
| 181 | "pulls", | |
| 181 | 182 | "variant", |
| 182 | 183 | "edit", |
| 183 | 184 | "read_file", |
new above
offline
This
sealedScopespread is the security line of this tool, and no test covers it. Add a test where a sealed pull is listed for a writer and hidden from a plain reader.