Public: anyone can read this; the crew can change it.
MCP `issue`: read one issue with its body, comments, labels and what it waits for #3354
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3342-mcp-issue-read-one-issue-with-its-body-c → main 6 commits fork 4 threads · 4 unresolved
3 files
changed +171
−2
docs/MCP.md +1
| @@ -32,6 +32,7 @@ for a tunnel and a forge that sleeps. | ||
| 32 | 32 | | `repo` | `{ repo }`: one repository's description, visibility, autonomy and `audience { level, sentence }`, the same as `GET /api/v1/repos/:owner/:name` | | |
| 33 | 33 | | `tell` | a sentence becomes an issue, a builder is picked, a run starts on the crew's next tick | yes | |
| 34 | 34 | | `say` | speak on an issue or a pull; a working builder hears it as its next step | yes | |
| 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 | | | |
| 35 | 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` | | |
| 36 | 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 | |
| 37 | 38 | | `read_file` | a file (utf-8, or base64 for binary) or a directory listing at a ref, as GET …/contents | no | |
src/http/mcp.ts +81 −1
| @@ -100,7 +100,15 @@ import { | ||
| 100 | 100 | import { share } from "../feed/share.ts"; |
| 101 | 101 | import { parseTargets } from "./feed.ts"; |
| 102 | 102 | import { appsRuntime, envsFor } from "../apps/runtime.ts"; |
| 103 | import { getIssue, getIssueById, listOpenAgentPullsVisibleTo } from "../core/issues.ts"; | |
| 103 | import { | |
| 104 | getIssue, | |
| 105 | getIssueById, | |
| 106 | labelsFor, | |
| 107 | listComments, | |
| 108 | listOpenAgentPullsVisibleTo, | |
| 109 | } from "../core/issues.ts"; | |
| 110 | import { isCrew } from "../core/agents.ts"; | |
| 111 | import { dependenciesOf } from "../core/dependencies.ts"; | |
| 104 | 112 | import { waitsForPerson } from "../crew/review.ts"; |
| 105 | 113 | import { attachLook } from "../actions/attach_look.ts"; |
| 106 | 114 | import { lookUrl } from "../core/looks.ts"; |
| @@ -140,6 +148,7 @@ import { audit } from "../contract/audit.ts"; | ||
| 140 | 148 | import { probeApp } from "../contract/probes.ts"; |
| 141 | 149 | import { convergeFork, convergeState } from "../crew/converge.ts"; |
| 142 | 150 | import { |
| 151 | markReaders, | |
| 143 | 152 | DIRECTION_NOTE, |
| 144 | 153 | directionNote, |
| 145 | 154 | OUTSIDERS_NOTE, |
| @@ -690,6 +699,77 @@ const TOOLS: readonly Tool[] = [ | ||
| 690 | 699 | : bad("refused", "that could not be said here"); |
| 691 | 700 | }, |
| 692 | 701 | }, |
| 702 | { | |
| 703 | name: "issue", | |
| 704 | description: | |
| 705 | "Read one issue: title, state, closeReason, body, labels, assignee, its comments (author, whether the author is an agent, body, time), and `blockedBy` and `blocks` (what it waits for, and what waits for it). Text written by someone who cannot write the repository carries `untrusted: true`, as in GET /api/v1/repos/:owner/:name/issues/:number (shown to a crew agent only). A sealed or private issue you cannot see is not found.", | |
| 706 | inputSchema: { | |
| 707 | type: "object", | |
| 708 | properties: { | |
| 709 | repo: REPO_ARG, | |
| 710 | number: { type: "integer", minimum: 1 }, | |
| 711 | }, | |
| 712 | required: ["repo", "number"], | |
| 713 | }, | |
| 714 | writes: false, | |
| 715 | async run(ctx, user, args) { | |
| 716 | const repo = repoArg(ctx, user, args); | |
| 717 | if (isAnswer(repo)) return repo; | |
| 718 | const n = args["number"]; | |
| 719 | const issue = | |
| 720 | typeof n === "number" && Number.isInteger(n) && n > 0 | |
| 721 | ? getIssue(ctx.app.db, repo.id, n) | |
| 722 | : undefined; | |
| 723 | if ( | |
| 724 | issue === undefined || | |
| 725 | issue.kind !== "issue" || | |
| 726 | !mayView(ctx.app.db, repo, issue, user) | |
| 727 | ) | |
| 728 | return bad("not_found", `no issue #${String(n)}`); | |
| 729 | const db = ctx.app.db; | |
| 730 | // Marked only for a crew agent, as the API does: to anyone else the | |
| 731 | // mark would say which commenters cannot write. | |
| 732 | const marking = user.kind === "agent" && isCrew(db, repo.id, user.id); | |
| 733 | const mark = <T extends { authorName: string; originAuthor: string | null }>( | |
| 734 | items: readonly T[], | |
| 735 | ): (T & { untrusted?: string })[] => | |
| 736 | marking ? markReaders(db, repo, items) : [...items]; | |
| 737 | const comments = mark(listComments(db, issue.id)); | |
| 738 | const authors = getUsersByIds(db, [...new Set(comments.map((c) => c.authorId))]); | |
| 739 | const deps = dependenciesOf(db, issue.id); | |
| 740 | const brief = (i: typeof issue) => ({ | |
| 741 | number: i.number, | |
| 742 | title: i.title, | |
| 743 | state: i.state, | |
| 744 | ...(i.closeReason === null ? {} : { closeReason: i.closeReason }), | |
| 745 | }); | |
| 746 | const seen = (i: typeof issue) => mayView(db, repo, i, user); | |
| 747 | const flag = (x: { untrusted?: string }) => | |
| 748 | x.untrusted === undefined ? {} : { untrusted: true }; | |
| 749 | const [self] = mark([issue]); | |
| 750 | return good({ | |
| 751 | number: issue.number, | |
| 752 | title: issue.title, | |
| 753 | state: issue.state, | |
| 754 | closeReason: issue.closeReason, | |
| 755 | body: issue.body, | |
| 756 | author: issue.authorName, | |
No test covers this line: with it removed, the suite stays green. Add a test where bob files the issue and the crew agent reads |
||
| 757 | authorIsAgent: issue.authorIsAgent, | |
| 758 | ...flag(self!), | |
| 759 | labels: labelsFor(db, issue.id).map((l) => l.name), | |
| 760 | assignee: issue.assigneeName, | |
| 761 | comments: comments.map((c) => ({ | |
| 762 | author: c.authorName, | |
| 763 | authorIsAgent: authors.get(c.authorId)?.kind === "agent", | |
| 764 | body: c.body, | |
| 765 | at: c.createdAt, | |
| 766 | ...flag(c), | |
| 767 | })), | |
| 768 | blockedBy: deps.blockedBy.filter(seen).map(brief), | |
| 769 | blocks: deps.blocks.filter(seen).map(brief), | |
| 770 | }); | |
| 771 | }, | |
| 772 | }, | |
| 693 | 773 | { |
| 694 | 774 | name: "pull", |
| 695 | 775 | description: |
test/mcp.test.ts +89 −1
| @@ -16,7 +16,8 @@ import { resolveOid } from "../src/git/merge.ts"; | ||
| 16 | 16 | import { openPull } from "../src/actions/pulls.ts"; |
| 17 | 17 | import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts"; |
| 18 | 18 | import { createApp as insertApp } from "../src/core/apps.ts"; |
| 19 | import { listIssues } from "../src/core/issues.ts"; | |
| 19 | import { addComment, createIssue, listIssues } from "../src/core/issues.ts"; | |
| 20 | import { addDependency } from "../src/core/dependencies.ts"; | |
| 20 | 21 | import { getRepoById, updateRepoSettings, type Repo } from "../src/core/repos.ts"; |
| 21 | 22 | import { createRun, listRuns, setRunBase } from "../src/core/runs.ts"; |
| 22 | 23 | import { getAccountCeiling } from "../src/core/account_ceiling.ts"; |
| @@ -175,6 +176,7 @@ describe("the forge over MCP", () => { | ||
| 175 | 176 | "tell", |
| 176 | 177 | "share", |
| 177 | 178 | "say", |
| 179 | "issue", | |
| 178 | 180 | "pull", |
| 179 | 181 | "variant", |
| 180 | 182 | "edit", |
| @@ -452,6 +454,92 @@ describe("the forge over MCP", () => { | ||
| 452 | 454 | ).toContain("not_found"); |
| 453 | 455 | }); |
| 454 | 456 | |
| 457 | test("issue reads body, comments, labels and what it waits for; a sealed one is not found; a reader is marked untrusted", async () => { | |
| 458 | const mk = (title: string, body = "", sealed = false) => { | |
| 459 | const r = createIssue(app.db, { repoId: game.id, authorId: alice.id, title, body, sealed }); | |
| 460 | if (!r.ok) throw new Error(String(r.error)); | |
| 461 | return r.value; | |
| 462 | }; | |
| 463 | const waiting = mk("wait for it", "the body"); | |
| 464 | const first = mk("first"); | |
| 465 | expect(addDependency(app.db, waiting, first, alice.id).ok).toBe(true); | |
| 466 | const bot = await createAgent(app, alice, { username: "issuebot", brief: "x" }); | |
| 467 | if (!bot.ok) throw new Error(String(bot.error)); | |
| 468 | expect(addToCrew(app, alice, game, "issuebot").ok).toBe(true); | |
| 469 | addComment(app.db, { issueId: waiting.id, authorId: alice.id, body: "one" }); | |
| 470 | addComment(app.db, { issueId: waiting.id, authorId: bot.value.user.id, body: "two" }); | |
| 471 | ||
| 472 | const read = (await tool(rw, "issue", { repo: "alice/game", number: waiting.number })).value; | |
| 473 | expect(read).toMatchObject({ | |
| 474 | number: waiting.number, | |
| 475 | title: "wait for it", | |
| 476 | state: "open", | |
| 477 | closeReason: null, | |
| 478 | body: "the body", | |
| 479 | labels: [], | |
| 480 | assignee: null, | |
| 481 | blockedBy: [{ number: first.number, title: "first", state: "open" }], | |
| 482 | blocks: [], | |
outdated
This comment says a pull's number is not found, but no assertion checks it. Add |
||
| 483 | }); | |
| 484 | expect(read.comments.map((c: any) => [c.author, c.authorIsAgent, c.body])).toEqual([ | |
| 485 | ["alice", false, "one"], | |
| 486 | ["issuebot", true, "two"], | |
| 487 | ]); | |
| 488 | expect(read.comments[0].at).toBeString(); | |
| 489 | expect(read.untrusted).toBeUndefined(); | |
| 490 | const other = (await tool(rw, "issue", { repo: "alice/game", number: first.number })).value; | |
| 491 | expect(other.blocks.map((b: any) => b.number)).toEqual([waiting.number]); | |
| 492 | ||
| 493 | // Not found: no such number, and a pull's number is not an issue's. | |
| 494 | expect((await tool(rw, "issue", { repo: "alice/game", number: 9999 })).error).toContain("not_found"); | |
| 495 | const branched = await commitToBranch(app, alice, game, { | |
| 496 | branch: "issue-kind", | |
| 497 | message: "a change", | |
| 498 | files: [{ path: "kind.txt", content: "k\n" }], | |
| 499 | }); | |
| 500 | if (!branched.ok) throw new Error(JSON.stringify(branched.error)); | |
| 501 | const opened = await openPull(app, alice, game, { | |
| 502 | title: "Not an issue", | |
| 503 | baseBranch: "main", | |
| 504 | headBranch: "issue-kind", | |
| 505 | }); | |
| 506 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 507 | expect((await tool(rw, "issue", { repo: "alice/game", number: opened.value.number })).error).toContain("not_found"); | |
| 508 | ||
| 509 | // Sealed: the writer reads it; a reader of a public repo is told not found. | |
| 510 | const pub = await createRepo(app, alice, { name: "sealedpub", visibility: "public" }); | |
| 511 | if (!pub.ok) throw new Error(String(pub.error)); | |
| 512 | const secret = createIssue(app.db, { repoId: pub.value.id, authorId: alice.id, title: "hole", sealed: true }); | |
| 513 | if (!secret.ok) throw new Error(String(secret.error)); | |
| 514 | const open = createIssue(app.db, { repoId: pub.value.id, authorId: alice.id, title: "visible" }); | |
| 515 | if (!open.ok) throw new Error(String(open.error)); | |
| 516 | expect((await tool(rw, "issue", { repo: "alice/sealedpub", number: secret.value.number })).value.title).toBe("hole"); | |
| 517 | expect((await tool(bobRw, "issue", { repo: "alice/sealedpub", number: secret.value.number })).error).toContain("not_found"); | |
| 518 | ||
| 519 | // A public issue waiting on the sealed one: the reader must not learn its title. | |
| 520 | expect(addDependency(app.db, open.value, secret.value, alice.id).ok).toBe(true); | |
| 521 | expect((await tool(bobRw, "issue", { repo: "alice/sealedpub", number: open.value.number })).value.blockedBy).toEqual([]); | |
| 522 | const waits = (await tool(rw, "issue", { repo: "alice/sealedpub", number: open.value.number })).value.blockedBy; | |
| 523 | expect(waits).toHaveLength(1); | |
| 524 | expect(waits[0]).toMatchObject({ number: secret.value.number, title: "hole" }); | |
| 525 | expect((await tool(bobRw, "issue", { repo: "alice/sealedpub", number: open.value.number })).value.blockedBy).not.toContainEqual(expect.objectContaining({ title: "hole" })); | |
| 526 | ||
| 527 | // Untrusted: bob cannot write alice/sealedpub; a crew agent is told so. | |
| 528 | addComment(app.db, { issueId: open.value.id, authorId: bob.id, body: "from a reader" }); | |
| 529 | addComment(app.db, { issueId: open.value.id, authorId: alice.id, body: "from the owner" }); | |
| 530 | expect(addToCrew(app, alice, pub.value, "issuebot").ok).toBe(true); | |
| 531 | const botToken = createToken(app.db, bot.value.user.id, "bot", "ro").token; | |
| 532 | const marked = (await tool(botToken, "issue", { repo: "alice/sealedpub", number: open.value.number })).value; | |
| 533 | expect(marked.comments.map((c: any) => c.untrusted)).toEqual([true, undefined]); | |
| 534 | expect(marked.untrusted).toBeUndefined(); | |
| 535 | ||
| 536 | // An issue a reader filed is marked itself, for the crew agent and not for the owner. | |
| 537 | const theirs = createIssue(app.db, { repoId: pub.value.id, authorId: bob.id, title: "from bob" }); | |
| 538 | if (!theirs.ok) throw new Error(String(theirs.error)); | |
| 539 | expect((await tool(botToken, "issue", { repo: "alice/sealedpub", number: theirs.value.number })).value.untrusted).toBe(true); | |
| 540 | expect((await tool(rw, "issue", { repo: "alice/sealedpub", number: theirs.value.number })).value.untrusted).toBeUndefined(); | |
| 541 | }); | |
| 542 | ||
| 455 | 543 | test("following, take_updates and converge are the fork's page", async () => { |
| 456 | 544 | const pub = await createRepo(app, alice, { |
| 457 | 545 | name: "open", |
new above
offline
Nothing tests this filter. Without it, a sealed blocker's title reaches anyone who can read the public issue that waits on it. Add a test where a reader (
bobRw) reads a public issue blocked by a sealed one and getsblockedBy: [], while the writer still sees the sealed one.