Public: anyone can read this; the crew can change it.
MCP `wait_for`: make an issue wait for another, or stop waiting, with the same rules as the issue page #3659
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3647-mcp-wait-for-make-an-issue-wait-for-anot → main 3 commits fork 3 threads · 3 unresolved
4 files
changed +166
−57
docs/MCP.md +1 −2
| @@ -60,8 +60,7 @@ for a tunnel and a forge that sleeps. | ||
| 60 | 60 | | `watch` | `{ app, issue, watch? }`: watch a milestone, or stop with `watch: false`: one item in your feed that follows it, and one push to your phone when it lands | yes | |
| 61 | 61 | | `usage` | `{ repo, days? }`: what the crew's runs spent — tokens, cost, wall time — by subscription, agent, issue, and each account's 5-hour and weekly standing; a writer's | | |
| 62 | 62 | | `labels` | `{ repo, number, labels }`: set an issue's or a pull's labels to exactly these (`[]` clears); only labels the repository has, by a writer | yes | |
| 63 | | `link` | `{ app, issue, blockedBy }`: the issue waits for another; loops and cross-repository are refused | yes | | |
| 64 | | `unlink` | `{ app, issue, blockedBy }`: the issue stops waiting for it | yes | | |
| 63 | | `wait_for` | `{ app, issue, blockedBy, stop? }`: the issue waits for another, or stops with `stop: true`; the page's rules, and the answer carries the issue's URL | yes | | |
| 65 | 64 | | `hold` | `{ app, pull, why?, release?, dryRun? }`: hold a pull so no rule reviews or lands it, or let it go; a person who can write, never an agent; plans unless `dryRun: false` | yes | |
| 66 | 65 | | `plan` | `{ app, issues }`: open up to 12 linked issues, all or none (see below) | yes | |
| 67 | 66 |
src/http/mcp.ts +19 −43
| @@ -2466,66 +2466,42 @@ const TOOLS: readonly Tool[] = [ | ||
| 2466 | 2466 | }, |
| 2467 | 2467 | }, |
| 2468 | 2468 | { |
| 2469 | name: "link", | |
| 2470 | description: | |
| 2471 | "Say that an issue waits for another in the same app. Refused when it would make a loop, crosses repositories, or names an issue that does not exist.", | |
| 2469 | name: "wait_for", | |
| 2470 | description: "Make an issue wait for another in the same app, or stop waiting with `stop: true`.", | |
| 2472 | 2471 | inputSchema: { |
| 2473 | 2472 | type: "object", |
| 2474 | 2473 | properties: { |
| 2475 | 2474 | app: APP_ARG, |
| 2476 | 2475 | issue: { type: "integer", minimum: 1 }, |
| 2477 | 2476 | blockedBy: { type: "integer", minimum: 1 }, |
| 2477 | stop: { type: "boolean" }, | |
| 2478 | 2478 | }, |
| 2479 | 2479 | required: ["app", "issue", "blockedBy"], |
| 2480 | 2480 | }, |
| 2481 | 2481 | writes: true, |
| 2482 | 2482 | run(ctx, user, args) { |
| 2483 | if (args["stop"] !== undefined && typeof args["stop"] !== "boolean") | |
| 2484 | return bad("invalid_stop", "stop is true or false"); | |
| 2483 | 2485 | const repo = planRepo(ctx, user, args); |
| 2484 | 2486 | if (isAnswer(repo)) return repo; |
| 2485 | 2487 | const issue = issueArg(ctx, repo, args, "issue"); |
| 2486 | 2488 | if ("ok" in issue) return issue; |
| 2487 | 2489 | const blocker = issueArg(ctx, repo, args, "blockedBy"); |
| 2488 | 2490 | if ("ok" in blocker) return blocker; |
| 2489 | const linked = linkIssue(ctx.app, user, repo, issue, blocker); | |
| 2490 | return linked.ok | |
| 2491 | ? good({ | |
| 2492 | says: `#${issue.number} now waits for #${blocker.number}.`, | |
| 2493 | issue: issue.number, | |
| 2494 | blockedBy: blocker.number, | |
| 2495 | }) | |
| 2496 | : bad( | |
| 2497 | linked.error, | |
| 2498 | dependencyWords[linked.error] ?? | |
| 2499 | "you need write access to plan here", | |
| 2500 | ); | |
| 2501 | }, | |
| 2502 | }, | |
| 2503 | { | |
| 2504 | name: "unlink", | |
| 2505 | description: "Say that an issue no longer waits for another.", | |
| 2506 | inputSchema: { | |
| 2507 | type: "object", | |
| 2508 | properties: { | |
| 2509 | app: APP_ARG, | |
| 2510 | issue: { type: "integer", minimum: 1 }, | |
| 2511 | blockedBy: { type: "integer", minimum: 1 }, | |
| 2512 | }, | |
| 2513 | required: ["app", "issue", "blockedBy"], | |
| 2514 | }, | |
| 2515 | writes: true, | |
| 2516 | run(ctx, user, args) { | |
| 2517 | const repo = planRepo(ctx, user, args); | |
| 2518 | if (isAnswer(repo)) return repo; | |
| 2519 | const issue = issueArg(ctx, repo, args, "issue"); | |
| 2520 | if ("ok" in issue) return issue; | |
| 2521 | const blocker = issueArg(ctx, repo, args, "blockedBy"); | |
| 2522 | if ("ok" in blocker) return blocker; | |
| 2523 | const unlinked = unlinkIssue(ctx.app, user, repo, issue, blocker); | |
| 2524 | return unlinked.ok | |
| 2525 | ? good({ | |
| 2526 | says: `#${issue.number} no longer waits for #${blocker.number}.`, | |
| 2527 | }) | |
| 2528 | : bad(unlinked.error, "you need write access to plan here"); | |
| 2491 | const stop = args["stop"] === true; | |
| 2492 | const done = stop | |
| 2493 | ? unlinkIssue(ctx.app, user, repo, issue, blocker) | |
| 2494 | : linkIssue(ctx.app, user, repo, issue, blocker); | |
| 2495 | if (!done.ok) | |
| 2496 | return bad( | |
| 2497 | done.error, | |
| 2498 | dependencyWords[done.error] ?? | |
| 2499 | "You need write access, or a place on the crew, to plan here.", | |
| 2500 | ); | |
| 2501 | return good({ | |
| 2502 | says: `#${issue.number} ${stop ? "no longer waits" : "waits"} for #${blocker.number}.`, | |
| 2503 | url: `${ctx.url.origin}/${repo.ownerName}/${repo.name}/issues/${issue.number}`, | |
| 2504 | }); | |
| 2529 | 2505 | }, |
| 2530 | 2506 | }, |
| 2531 | 2507 | { |
test/mcp-wait-for.test.ts added +135
| @@ -0,0 +1,135 @@ | ||
| 1 | /** | |
| 2 | * The `wait_for` tool is the issue page's dependency action over MCP: a | |
| 3 | * writer's call works, a reader's is refused in the page's words, and bad | |
| 4 | * input is refused at the boundary. | |
| 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 { createRepo } from "../src/actions/repos.ts"; | |
| 12 | import { createApp as insertApp } from "../src/core/apps.ts"; | |
| 13 | import { createToken } from "../src/core/tokens.ts"; | |
| 14 | import { createUser } from "../src/core/users.ts"; | |
| 15 | import { promote } from "./helpers/members.ts"; | |
| 16 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 17 | ||
| 18 | describe("wait_for over MCP", () => { | |
| 19 | let dataDir: string; | |
| 20 | let app: App; | |
| 21 | let servers: Servers; | |
| 22 | let rw: string; | |
| 23 | let ro: string; | |
| 24 | let bobRw: string; | |
| 25 | let id = 1; | |
| 26 | ||
| 27 | const tool = async (token: string, name: string, args: unknown = {}) => { | |
| 28 | const res = await fetch(`${servers.url}/mcp`, { | |
| 29 | method: "POST", | |
| 30 | headers: { | |
| 31 | "content-type": "application/json", | |
| 32 | accept: "application/json, text/event-stream", | |
| 33 | authorization: `Bearer ${token}`, | |
| 34 | }, | |
| 35 | body: JSON.stringify({ | |
| 36 | jsonrpc: "2.0", | |
| 37 | id: id++, | |
| 38 | method: "tools/call", | |
| 39 | params: { name, arguments: args }, | |
| 40 | }), | |
| 41 | }); | |
| 42 | const body = (await res.json()) as any; | |
| 43 | const text: string = body.result.content[0].text; | |
| 44 | return body.result.isError === true | |
| 45 | ? { error: text } | |
| 46 | : { value: JSON.parse(text) }; | |
| 47 | }; | |
| 48 | const tell = async (text: string): Promise<number> => | |
| 49 | (await tool(rw, "tell", { repo: "alice/game", text })).value.issue; | |
| 50 | ||
| 51 | beforeAll(async () => { | |
| 52 | dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-wait-")); | |
| 53 | app = createApp({ dataDir, port: 0 }); | |
| 54 | servers = await serve(app); | |
| 55 | const a = await createUser(app.db, { | |
| 56 | origin: "seed", | |
| 57 | trusted: true, | |
| 58 | username: "alice", | |
| 59 | email: "[email protected]", | |
| 60 | password: "a-good-passphrase", | |
| 61 | adminIfFirst: true, | |
| 62 | }); | |
| 63 | if (!a.ok) throw new Error(a.error); | |
| 64 | rw = createToken(app.db, a.value.id, "rw", "rw").token; | |
| 65 | ro = createToken(app.db, a.value.id, "ro", "ro").token; | |
| 66 | const b = await createUser(app.db, { | |
| 67 | origin: "seed", | |
| 68 | trusted: true, | |
| 69 | username: "bob", | |
| 70 | email: "[email protected]", | |
| 71 | password: "a-good-passphrase", | |
| 72 | }); | |
| 73 | if (!b.ok) throw new Error(b.error); | |
| 74 | promote(app.db, b.value); | |
| 75 | bobRw = createToken(app.db, b.value.id, "rw", "rw").token; | |
| 76 | const open = await createRepo(app, a.value, { name: "open", visibility: "public" }); | |
| 77 | if (!open.ok) throw new Error(String(open.error)); | |
| 78 | const made = await createRepo(app, a.value, { name: "game", visibility: "private" }); | |
| 79 | if (!made.ok) throw new Error(String(made.error)); | |
| 80 | const rec = insertApp(app.db, { repoId: made.value.id, slug: "game", access: "private" }); | |
| 81 | if (!rec.ok) throw new Error(rec.error); | |
| 82 | }, 30_000); | |
| 83 | ||
| 84 | afterAll(async () => { | |
| 85 | servers.stop(); | |
| 86 | app.close(); | |
| 87 | await rm(dataDir, { recursive: true, force: true }); | |
| 88 | }); | |
| 89 | ||
| 90 | test("a writer makes an issue wait, then stop waiting", async () => { | |
| 91 | const a = await tell("first"); | |
| 92 | const b = await tell("second"); | |
| 93 | const set = await tool(rw, "wait_for", { app: "alice/game", issue: b, blockedBy: a }); | |
| 94 | expect(set.value.says).toBe(`#${b} waits for #${a}.`); | |
| 95 | expect(set.value.url).toBe(`${servers.url}/alice/game/issues/${b}`); | |
| 96 | const map = (await tool(rw, "map", { app: "alice/game" })).value; | |
| 97 | expect(map.edges).toContainEqual({ issue: b, blockedBy: a }); | |
| 98 | // The page's rule: no loops. | |
| 99 | expect( | |
| 100 | (await tool(rw, "wait_for", { app: "alice/game", issue: a, blockedBy: b })).error, | |
| 101 | ).toContain("loop"); | |
| 102 | const stopped = await tool(rw, "wait_for", { app: "alice/game", issue: b, blockedBy: a, stop: true }); | |
| 103 | expect(stopped.value.says).toBe(`#${b} no longer waits for #${a}.`); | |
| 104 | const after = (await tool(rw, "map", { app: "alice/game" })).value; | |
| 105 | expect(after.edges).not.toContainEqual({ issue: b, blockedBy: a }); | |
| 106 | }); | |
| 107 | ||
| 108 | test("a reader is refused, and nothing changes", async () => { | |
| 109 | const a = await tell("third"); | |
| 110 | const b = await tell("fourth"); | |
| 111 | const refused = await tool(ro, "wait_for", { app: "alice/game", issue: b, blockedBy: a }); | |
| 112 | expect(refused.error).toContain("forbidden"); | |
| 113 | const unchanged = (await tool(rw, "map", { app: "alice/game" })).value; | |
| 114 | expect(unchanged.edges).not.toContainEqual({ issue: b, blockedBy: a }); | |
| 115 | // A signed-in stranger on a public app is refused in the page's words. | |
| 116 | const a2 = (await tool(rw, "tell", { repo: "alice/open", text: "x" })).value.issue; | |
| 117 | const b2 = (await tool(rw, "tell", { repo: "alice/open", text: "y" })).value.issue; | |
| 118 | const stranger = await tool(bobRw, "wait_for", { app: "alice/open", issue: b2, blockedBy: a2 }); | |
| 119 | expect(stranger.error).toBe("forbidden: You need write access, or a place on the crew, to plan here."); | |
| 120 | const map = (await tool(ro, "map", { app: "alice/game" })).value; | |
Nit: this assertion repeats the |
||
| 121 | expect(map.edges).not.toContainEqual({ issue: b, blockedBy: a }); | |
| 122 | }); | |
| 123 | ||
| 124 | test("bad input is refused at the boundary", async () => { | |
| 125 | const a = await tell("fifth"); | |
| 126 | const w = (args: Record<string, unknown>) => | |
| 127 | tool(rw, "wait_for", { app: "alice/game", ...args }); | |
| 128 | expect((await w({ issue: a, blockedBy: a })).error).toContain("itself"); | |
| 129 | expect((await w({ issue: a, blockedBy: 9999 })).error).toContain("no_such_issue"); | |
| 130 | expect((await w({ issue: "1", blockedBy: a })).error).toContain("no_such_issue"); | |
| 131 | expect((await w({ issue: a, blockedBy: a + 0.5 })).error).toContain("no_such_issue"); | |
| 132 | expect((await w({ issue: a, blockedBy: 1, stop: "yes" })).error).toContain("invalid_stop"); | |
| 133 | expect((await tool(rw, "wait_for", { issue: a, blockedBy: 1 })).error).toBeDefined(); | |
| 134 | }); | |
| 135 | }); |
test/mcp.test.ts +11 −12
| @@ -224,8 +224,7 @@ describe("the forge over MCP", () => { | ||
| 224 | 224 | "spend", |
| 225 | 225 | "usage", |
| 226 | 226 | "labels", |
| 227 | "link", | |
| 228 | "unlink", | |
| 227 | "wait_for", | |
| 229 | 228 | "attach_look", |
| 230 | 229 | "hold", |
| 231 | 230 | "plan", |
| @@ -640,7 +639,7 @@ describe("the forge over MCP", () => { | ||
| 640 | 639 | expect(stale.error).toContain("already ended"); |
| 641 | 640 | }); |
| 642 | 641 | |
| 643 | test("map, link and unlink are the plan's own actions", async () => { | |
| 642 | test("map and wait_for are the plan's own actions", async () => { | |
| 644 | 643 | const a = (await tool(rw, "tell", { repo: "alice/game", text: "first" })) |
| 645 | 644 | .value.issue; |
| 646 | 645 | const b = (await tool(rw, "tell", { repo: "alice/game", text: "second" })) |
| @@ -652,12 +651,12 @@ describe("the forge over MCP", () => { | ||
| 652 | 651 | t.annotations.readOnlyHint, |
| 653 | 652 | ]), |
| 654 | 653 | ); |
| 655 | expect([hints.map, hints.link, hints.unlink]).toEqual([true, false, false]); | |
| 654 | expect([hints.map, hints.wait_for]).toEqual([true, false]); | |
| 656 | 655 | |
| 657 | 656 | expect( |
| 658 | (await tool(rw, "link", { app: "alice/game", issue: b, blockedBy: a })) | |
| 659 | .value.blockedBy, | |
| 660 | ).toBe(a); | |
| 657 | (await tool(rw, "wait_for", { app: "alice/game", issue: b, blockedBy: a })) | |
| 658 | .value.says, | |
| 659 | ).toBe(`#${b} waits for #${a}.`); | |
| 661 | 660 | const map = (await tool(ro, "map", { app: "alice/game" })).value; |
| 662 | 661 | expect(map.ready.map((i: any) => i.number)).toContain(a); |
| 663 | 662 | expect(map.blockedOn.map((i: any) => i.number)).toContain(a); |
| @@ -666,15 +665,15 @@ describe("the forge over MCP", () => { | ||
| 666 | 665 | |
| 667 | 666 | // The API's words, for a loop, a stranger issue, and a read token. |
| 668 | 667 | expect( |
| 669 | (await tool(rw, "link", { app: "alice/game", issue: a, blockedBy: b })) | |
| 668 | (await tool(rw, "wait_for", { app: "alice/game", issue: a, blockedBy: b })) | |
| 670 | 669 | .error, |
| 671 | 670 | ).toBe("cycle: that would make a loop"); |
| 672 | 671 | expect( |
| 673 | (await tool(rw, "link", { app: "alice/game", issue: a, blockedBy: 999 })) | |
| 672 | (await tool(rw, "wait_for", { app: "alice/game", issue: a, blockedBy: 999 })) | |
| 674 | 673 | .error, |
| 675 | 674 | ).toBe("no_such_issue: no issue #999"); |
| 676 | 675 | expect( |
| 677 | (await tool(ro, "link", { app: "alice/game", issue: a, blockedBy: b })) | |
| 676 | (await tool(ro, "wait_for", { app: "alice/game", issue: a, blockedBy: b })) | |
| 678 | 677 | .error, |
| 679 | 678 | ).toContain("forbidden"); |
| 680 | 679 | // A private app is no such app to someone who cannot read it. |
| @@ -682,11 +681,11 @@ describe("the forge over MCP", () => { | ||
| 682 | 681 | "not_found", |
| 683 | 682 | ); |
| 684 | 683 | expect( |
| 685 | (await tool(bobRw, "link", { app: "alice/game", issue: b, blockedBy: a })) | |
| 684 | (await tool(bobRw, "wait_for", { app: "alice/game", issue: b, blockedBy: a })) | |
| 686 | 685 | .error, |
| 687 | 686 | ).toContain("not_found"); |
| 688 | 687 | |
| 689 | await tool(rw, "unlink", { app: "alice/game", issue: b, blockedBy: a }); | |
| 688 | await tool(rw, "wait_for", { app: "alice/game", issue: b, blockedBy: a, stop: true }); | |
| 690 | 689 | const after = (await tool(ro, "map", { app: "alice/game" })).value; |
| 691 | 690 | expect(after.edges).not.toContainEqual({ issue: b, blockedBy: a }); |
| 692 | 691 | expect(after.ready.map((i: any) => i.number)).toContain(b); |
new above
offline
Same assertion as the line above. Drop it, or check what the issue says: that the map has no edge after the refusal.