Public: anyone can read this; the crew can change it.
MCP `close`: close or reopen an issue with the same rules as PATCH …/issues/{number} #4179
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3361-mcp-close-close-or-reopen-an-issue-with → main 7 commits fork 2 threads · 2 unresolved
5 files
changed +142
−31
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 | | `close` | `{ repo, number, state?, reason?, into? }`: close an issue or a pull (`state` closed by default) or reopen it (`open`); the same rules and refusals as `PATCH …/issues/:number`: an agent closes only what the rules give it and reopens nothing; answers `state` and `closeReason` | yes | | |
| 38 | 39 | | `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 | |
| 39 | 40 | | `read_file` | a file (utf-8, or base64 for binary) or a directory listing at a ref, as GET …/contents | no | |
| 40 | 41 | | `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 | |
src/actions/issues.ts +25
| @@ -27,6 +27,7 @@ import { moveBlockersToChildren } from "../core/dependencies.ts"; | ||
| 27 | 27 | import { |
| 28 | 28 | addComment as insertComment, |
| 29 | 29 | createIssue, |
| 30 | CLOSE_REASONS, | |
| 30 | 31 | getIssue, |
| 31 | 32 | getIssueById, |
| 32 | 33 | getLabel, |
| @@ -615,6 +616,30 @@ export interface Closing { | ||
| 615 | 616 | readonly triage?: boolean; |
| 616 | 617 | } |
| 617 | 618 | |
| 619 | /** | |
| 620 | * `reason` and `into` of a close request, or undefined when they are not | |
| 621 | * the shapes they must be (a bad reason word, `into` not a list of numbers). | |
| 622 | */ | |
| 623 | export function closingFrom(input: Record<string, unknown>): Closing | undefined { | |
| 624 | const reason = input.reason; | |
| 625 | if (reason !== undefined && !CLOSE_REASONS.includes(reason as CloseReason)) | |
| 626 | return undefined; | |
| 627 | let into: number[] | undefined; | |
| 628 | if (input.into !== undefined) { | |
| 629 | if ( | |
| 630 | !Array.isArray(input.into) || | |
| 631 | input.into.length > 50 || | |
| 632 | !input.into.every((n) => Number.isInteger(n) && n > 0) | |
| 633 | ) | |
| 634 | return undefined; | |
| 635 | into = input.into as number[]; | |
| 636 | } | |
| 637 | return { | |
| 638 | ...(reason === undefined ? {} : { reason: reason as CloseReason }), | |
| 639 | ...(into === undefined ? {} : { into }), | |
| 640 | }; | |
| 641 | } | |
| 642 | ||
| 618 | 643 | /** |
| 619 | 644 | * Replace an issue's (or pull's) label set. Writers only — labels are the |
| 620 | 645 | * work queue. Labels may be named by id or by name; an unknown one rejects |
src/http/api.ts +2 −30
| @@ -347,12 +347,8 @@ import { isEnvName } from "../apps/data.ts"; | ||
| 347 | 347 | import { getAppByRepo } from "../core/apps.ts"; |
| 348 | 348 | import { crewRole, isCrew, type Agent } from "../core/agents.ts"; |
| 349 | 349 | import { listApps, type AppRecord } from "../core/apps.ts"; |
| 350 | import { | |
| 351 | CLOSE_REASONS, | |
| 352 | listAssignedTo, | |
| 353 | type CloseReason, | |
| 354 | } from "../core/issues.ts"; | |
| 355 | import type { Closing } from "../actions/issues.ts"; | |
| 350 | import { listAssignedTo } from "../core/issues.ts"; | |
| 351 | import { closingFrom } from "../actions/issues.ts"; | |
| 356 | 352 | import { |
| 357 | 353 | checkSummary, |
| 358 | 354 | checksForHead, |
| @@ -841,30 +837,6 @@ const NOTHING_TO_CHANGE = () => | ||
| 841 | 837 | "send a new title or body, a state, or both", |
| 842 | 838 | ); |
| 843 | 839 | |
| 844 | /** | |
| 845 | * `reason` and `into` of a close request, or undefined when they are not | |
| 846 | * the shapes they must be (a bad reason word, `into` not a list of numbers). | |
| 847 | */ | |
| 848 | function closingFrom(input: Record<string, unknown>): Closing | undefined { | |
| 849 | const reason = input.reason; | |
| 850 | if (reason !== undefined && !CLOSE_REASONS.includes(reason as CloseReason)) | |
| 851 | return undefined; | |
| 852 | let into: number[] | undefined; | |
| 853 | if (input.into !== undefined) { | |
| 854 | if ( | |
| 855 | !Array.isArray(input.into) || | |
| 856 | input.into.length > 50 || | |
| 857 | !input.into.every((n) => Number.isInteger(n) && n > 0) | |
| 858 | ) | |
| 859 | return undefined; | |
| 860 | into = input.into as number[]; | |
| 861 | } | |
| 862 | return { | |
| 863 | ...(reason === undefined ? {} : { reason: reason as CloseReason }), | |
| 864 | ...(into === undefined ? {} : { into }), | |
| 865 | }; | |
| 866 | } | |
| 867 | ||
| 868 | 840 | /** |
| 869 | 841 | * Which status a branch refusal deserves. A conflict is something that |
| 870 | 842 | * changed under the caller (or was already there); everything else is the |
src/http/mcp.ts +51 −1
| @@ -45,9 +45,11 @@ import { chainOfRepo, FOLLOW_MODES, forgeLinkOf } from "../spawn/chain.ts"; | ||
| 45 | 45 | import { watchMilestone } from "../actions/watches.ts"; |
| 46 | 46 | import { |
| 47 | 47 | addCommentThrough, |
| 48 | closingFrom, | |
| 48 | 49 | editIssue, |
| 49 | 50 | setIssueLabelsThrough, |
| 50 | 51 | setIssueSealed, |
| 52 | setIssueStateThrough, | |
| 51 | 53 | TAKEN_UP, |
| 52 | 54 | } from "../actions/issues.ts"; |
| 53 | 55 | import { |
| @@ -103,6 +105,7 @@ import { share } from "../feed/share.ts"; | ||
| 103 | 105 | import { parseTargets } from "./feed.ts"; |
| 104 | 106 | import { appsRuntime, envsFor } from "../apps/runtime.ts"; |
| 105 | 107 | import { |
| 108 | CLOSE_REASONS, | |
| 106 | 109 | getIssue, |
| 107 | 110 | getIssueById, |
| 108 | 111 | labelsFor, |
| @@ -293,7 +296,6 @@ const str = (v: unknown): string => (typeof v === "string" ? v.trim() : ""); | ||
| 293 | 296 | |
| 294 | 297 | const REPO_ARG = { |
| 295 | 298 | type: "string", |
| 296 | description: "owner/name", | |
| 297 | 299 | } as const; |
| 298 | 300 | |
| 299 | 301 | /** A repository the caller may read, by `owner/name`. */ |
| @@ -950,6 +952,54 @@ const TOOLS: readonly Tool[] = [ | ||
| 950 | 952 | }); |
| 951 | 953 | }, |
| 952 | 954 | }, |
| 955 | { | |
| 956 | name: "close", | |
| 957 | description: | |
| 958 | "Close an issue or pull, or reopen it (`state` open); the PATCH route's refusals apply. `reason`: done, dropped, or split (with `into`).", | |
| 959 | inputSchema: { | |
| 960 | type: "object", | |
| 961 | properties: { | |
| 962 | repo: REPO_ARG, | |
| 963 | number: { type: "integer", minimum: 1 }, | |
| 964 | state: { type: "string", enum: ["closed", "open"] }, | |
| 965 | reason: { type: "string", enum: [...CLOSE_REASONS] }, | |
| 966 | into: { | |
| 967 | type: "array", | |
| 968 | items: { type: "integer", minimum: 1 }, | |
| 969 | maxItems: 50, | |
| 970 | }, | |
| 971 | }, | |
| 972 | required: ["repo", "number"], | |
| 973 | }, | |
| 974 | writes: true, | |
| 975 | async run(ctx, user, args) { | |
| 976 | const repo = repoArg(ctx, user, args); | |
| 977 | if (isAnswer(repo)) return repo; | |
| 978 | const issue = issueArg(ctx, repo, args, "number"); | |
| 979 | if ("ok" in issue) return issue; | |
| 980 | const state = args["state"] ?? "closed"; | |
| 981 | if (state !== "closed" && state !== "open") | |
| 982 | return bad("invalid_state", "state is closed or open"); | |
| 983 | // The PATCH route's own check of `reason` and `into`. | |
| 984 | const closing = closingFrom(args); | |
| 985 | if (closing === undefined) | |
| 986 | return bad("invalid_reason", "a reason is done, dropped, or split (with `into`: up to 50 issue numbers)"); | |
| 987 | const changed = await setIssueStateThrough(ctx.app, user, repo, issue, state, closing); | |
| 988 | if (!changed.ok) | |
Low: any error other than |
||
| 989 | return bad( | |
| 990 | changed.error, | |
| 991 | changed.error === "forbidden" | |
| 992 | ? "this is not yours to close or reopen" | |
| 993 | : "that state could not be set", | |
| 994 | ); | |
| 995 | return good({ | |
| 996 | says: `#${issue.number} is ${changed.value.state}.`, | |
| 997 | number: issue.number, | |
| 998 | state: changed.value.state, | |
| 999 | closeReason: changed.value.closeReason, | |
| 1000 | }); | |
| 1001 | }, | |
| 1002 | }, | |
| 953 | 1003 | { |
| 954 | 1004 | name: "read_file", |
| 955 | 1005 | description: |
test/mcp.test.ts +63
| @@ -13,6 +13,8 @@ import { createApp, type App } from "../src/app.ts"; | ||
| 13 | 13 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; |
| 14 | 14 | import { pullDir } from "../src/core/pulls.ts"; |
| 15 | 15 | import { resolveOid } from "../src/git/merge.ts"; |
| 16 | import { openIssue } from "../src/actions/issues.ts"; | |
| 17 | import { setCrewRole } from "../src/core/agents.ts"; | |
| 16 | 18 | import { openPull } from "../src/actions/pulls.ts"; |
| 17 | 19 | import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts"; |
| 18 | 20 | import { createApp as insertApp } from "../src/core/apps.ts"; |
| @@ -181,6 +183,7 @@ describe("the forge over MCP", () => { | ||
| 181 | 183 | "pulls", |
| 182 | 184 | "variant", |
| 183 | 185 | "edit", |
| 186 | "close", | |
| 184 | 187 | "read_file", |
| 185 | 188 | "asks", |
| 186 | 189 | "asks_act", |
| @@ -456,6 +459,66 @@ describe("the forge over MCP", () => { | ||
| 456 | 459 | ).toContain("not_found"); |
| 457 | 460 | }); |
| 458 | 461 | |
| 462 | test("close is the PATCH route's action: the same refusals, the new state answered", async () => { | |
| 463 | const made = await createAgent(app, alice, { username: "closer", brief: "x" }); | |
| 464 | if (!made.ok) throw new Error(String(made.error)); | |
| 465 | expect(addToCrew(app, alice, game, "closer").ok).toBe(true); | |
| 466 | setCrewRole(app.db, game.id, made.value.user.id, "planner"); | |
| 467 | const botToken = createToken(app.db, made.value.user.id, "bot", "rw").token; | |
| 468 | const mine = (title: string) => { | |
| 469 | const o = openIssue(app, made.value.user, game, { title }); | |
| 470 | if (!o.ok) throw new Error(String(o.error)); | |
| 471 | return o.value; | |
| 472 | }; | |
| 473 | const patch = async (token: string, number: number, payload: unknown) => { | |
| 474 | const res = await fetch(`${base}/api/v1/repos/alice/game/issues/${number}`, { | |
| 475 | method: "PATCH", | |
| 476 | headers: { "content-type": "application/json", authorization: `Bearer ${token}` }, | |
| 477 | body: JSON.stringify(payload), | |
| 478 | }); | |
| 479 | return { status: res.status, json: (await res.json()) as any }; | |
| 480 | }; | |
| 481 | ||
| 482 | // Done with no commit naming it: refused, with the API's code. | |
| 483 | const noNote = mine("a thing nobody did"); | |
| 484 | const viaApi = await patch(botToken, noNote.number, { state: "closed", reason: "done" }); | |
| 485 | expect(viaApi.json.error).toBe("forbidden"); | |
| 486 | const refused = await tool(botToken, "close", { repo: "alice/game", number: noNote.number, reason: "done" }); | |
| 487 | expect(refused.error).toContain("forbidden"); | |
| 488 | ||
| 489 | // Its own, dropped: closed, and the answer says so. | |
| 490 | const own = mine("a thing to drop"); | |
| 491 | const dropped = await tool(botToken, "close", { | |
| 492 | repo: "alice/game", | |
| 493 | number: own.number, | |
| 494 | reason: "dropped", | |
| 495 | }); | |
| 496 | expect(dropped.value).toMatchObject({ number: own.number, state: "closed", closeReason: "dropped" }); | |
| 497 | ||
| 498 | // A person's ask is not an agent's to drop. | |
| 499 | const asked = openIssue(app, alice, game, { title: "a person asks" }); | |
| 500 | if (!asked.ok) throw new Error(String(asked.error)); | |
| 501 | expect( | |
| 502 | (await tool(botToken, "close", { repo: "alice/game", number: asked.value.number, reason: "dropped" })).error, | |
| 503 | ).toContain("forbidden"); | |
| 504 | // Nor does an agent reopen. | |
| 505 | expect((await tool(botToken, "close", { repo: "alice/game", number: own.number, state: "open" })).error).toContain( | |
| 506 | "forbidden", | |
| 507 | ); | |
| 508 | ||
| 509 | // A person reopens it; closing defaults to done. | |
| 510 | const reopened = await tool(rw, "close", { repo: "alice/game", number: own.number, state: "open" }); | |
| 511 | expect(reopened.value).toMatchObject({ state: "open", closeReason: null }); | |
| 512 | const closed = await tool(rw, "close", { repo: "alice/game", number: own.number }); | |
| 513 | expect(closed.value).toMatchObject({ state: "closed", closeReason: "done" }); | |
| 514 | ||
| 515 | // The boundary: bad words are refused before the action. | |
| 516 | expect((await tool(rw, "close", { repo: "alice/game", number: own.number, state: "x" })).error).toContain("invalid_state"); | |
| 517 | expect((await tool(rw, "close", { repo: "alice/game", number: own.number, reason: "x" })).error).toContain("invalid_reason"); | |
| 518 | expect((await tool(rw, "close", { repo: "alice/game", number: own.number, into: [0] })).error).toContain("invalid_reason"); | |
| 519 | expect((await tool(ro, "close", { repo: "alice/game", number: own.number })).error).toContain("forbidden: this token may read, not act"); | |
| 520 | }); | |
| 521 | ||
| 459 | 522 | test("issue reads body, comments, labels and what it waits for; a sealed one is not found; a reader is marked untrusted", async () => { |
| 460 | 523 | const mk = (title: string, body = "", sealed = false) => { |
| 461 | 524 | const r = createIssue(app.db, { repoId: game.id, authorId: alice.id, title, body, sealed }); |
new above
offline
Low, scope: dropping
description: "owner/name"fromREPO_ARGtakes the format hint off all 37 tools'repoargument. That isn't part of #3361. It was cut only to fit the 40 KBtools/listceiling, and main has since trimmed the same descriptions. If there is room now, put it back. If there isn't, get the bytes from prose descriptions: the shape of an argument is what a harness reads first.