Public: anyone can read this; the crew can change it.
MCP `hooks` and `repo_labels` should page like the routes they mirror #7001
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6997-mcp-hooks-and-repo-labels-should-page-li → main 2 commits fork 3 threads · 3 unresolved
6 files
changed +77
−10
docs/MCP.md +2 −2
| @@ -85,7 +85,7 @@ for a tunnel and a forge that sleeps. | ||
| 85 | 85 | | `front_page` | `{ pinned?, hidden?, weights?, dryRun? }`: the front page as a stranger sees it, ranked, with numbers; a site admin sets what is pinned (first public one featured), hidden, and what counts (weights of forks, ships, use, fresh, 0 to 10) | yes | |
| 86 | 86 | | `health` | how this forge is: the version and commit it runs, draining or not, the rollout and its incidents | | |
| 87 | 87 | | `hold` | `{ app, pull | path, why?, until?, release?, dryRun? }`: hold a pull so no rule reviews or lands it, or let it go; a person who can write, never an agent; or with `path` (no `pull`) hold new work on a file so the picker starts no build whose issue names it, or list the holds and who set each; an admin of the app; plans unless `dryRun: false` | yes | |
| 88 | | `hooks` | `{ repo, action, url, secret, kinds, id }`: `list`, `add` (`secret`, 16–200 characters, is required) or `remove` the repository's webhooks, as `…/hooks`; an admin-scope token on a repository you administer, and a secret is never shown | yes | | |
| 88 | | `hooks` | `{ repo, action, url, secret, kinds, id }`: `list` (pages: `limit` 1–200, `cursor`; `{ items, next_cursor }`), `add` (`secret`, 16–200 characters, is required) or `remove` the repository's webhooks, as `…/hooks`; an admin-scope token on a repository you administer, and a secret is never shown | yes | | |
| 89 | 89 | | `invites` | `{ action, id? }`: `list` the outstanding invites (state and expiry, never a link) or `revoke` one by `id`; a person admin with an admin token, stricter than the API's list, which an `ro` token may read | yes | |
| 90 | 90 | | `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; draws the `ui://forge/issue` card: title, state, assignee, labels and what it waits for, never the body or comments | | |
| 91 | 91 | | `issue_teams` | `{ repo, number, teams? }`: the crew teams an issue names; with `teams` (at most two slugs, `[]` clears) names them, and each reviews the closing pull | yes | |
| @@ -125,7 +125,7 @@ for a tunnel and a forge that sleeps. | ||
| 125 | 125 | | `read_file` | a file (utf-8, or base64 for binary) or a directory listing at a ref, as GET …/contents | | |
| 126 | 126 | | `releases` | `{}`: the forge's kept releases newest first: short sha, version, the pull that landed it, when, and for the live one `live: true` and `since`; for whoever may read /releases, others get its refusal; no rollback, a person does that on the page | | |
| 127 | 127 | | `repo` | `{ repo }`: one repository's description, visibility, autonomy, `audience { level, sentence }` and `provenance` (where a migrated one came from, `null` when hidden), the same as `GET /api/v1/repos/:owner/:name` | | |
| 128 | | `repo_labels` | `{ repo, action?, name?, color?, id? }`: list (`list`, the default; a read-only token may), create (`create`, `name`, `color` six hex digits) or remove (`remove`, `id`) the repository's labels, as `GET`/`POST …/labels` and `DELETE …/labels/:id`; create and remove need an admin token and a repository admin | yes | | |
| 128 | | `repo_labels` | `{ repo, action?, name?, color?, id? }`: list (`list`, the default, paged by `limit` 1–200 and `cursor`, as `{ items, next_cursor }`; a read-only token may), create (`create`, `name`, `color` six hex digits) or remove (`remove`, `id`) the repository's labels, as `GET`/`POST …/labels` and `DELETE …/labels/:id`; create and remove need an admin token and a repository admin | yes | | |
| 129 | 129 | | `repo_settings` | `{ repo, description?, visibility?, flowOnly? }` (admin): change a repository's description, visibility and FLOW ONLY switch as `PATCH /api/v1/repos/:owner/:name` does, for the same callers (a repository admin); answers `{ repo, description, visibility, flowOnly }` (with none to change it only reads); a visibility other than public or private is `invalid` | yes | |
| 130 | 130 | | `reports` | `{ repo, id?, action? }`: the waiting reports from outsiders, each `from: "outside"` (their words are unchecked); with `id` and `action` (`accept` opens it as an issue marked as from outside, `dismiss` closes it). A person who writes the repository only; needs an `rw` token | yes | |
| 131 | 131 | | `restore_drill` | no input: restore the newest snapshot as a drill and answer `{ verdict, failedStep?, says }`, worded as the Backups page's button; no store, a failure or a red drill is an answer, not an error; a forge admin's, with an admin token | yes | |
src/http/mcp/page.ts added +27
| @@ -0,0 +1,27 @@ | ||
| 1 | /** | |
| 2 | * `limit`/`cursor` for an MCP list over rows already in key order, as the | |
| 3 | * routes they mirror page: `{ items, next_cursor }`, limit 1..200 (default | |
| 4 | * 50), and a cursor whose row is gone is `invalid_cursor`. | |
| 5 | */ | |
| 6 | ||
| 7 | import { bad, good, type Answer, type Args } from "./shared.ts"; | |
| 8 | ||
| 9 | const LIMIT_DEFAULT = 50; | |
| 10 | const LIMIT_MAX = 200; | |
| 11 | ||
| 12 | export function pageRows<T>(rows: readonly T[], keyOf: (row: T) => string, args: Args): Answer { | |
| 13 | const rawLimit = args["limit"]; | |
| 14 | const limit = rawLimit === undefined ? LIMIT_DEFAULT : rawLimit; | |
| 15 | if (typeof limit !== "number" || !Number.isInteger(limit) || limit < 1 || limit > LIMIT_MAX) | |
| 16 | return bad("invalid_limit", `limit is a whole number from 1 to ${LIMIT_MAX}`); | |
| 17 | const cursor = args["cursor"]; | |
| 18 | let start = 0; | |
| 19 | if (cursor !== undefined) { | |
| 20 | const at = typeof cursor === "string" ? rows.findIndex((r) => keyOf(r) === cursor) : -1; | |
| 21 | if (at < 0) return bad("invalid_cursor", "cursor is the next_cursor of a page before"); | |
| 22 | start = at + 1; | |
| 23 | } | |
| 24 | const items = rows.slice(start, start + limit); | |
| 25 | const more = start + limit < rows.length; | |
| 26 | return good({ items, next_cursor: more ? keyOf(items[items.length - 1]!) : null }); | |
| 27 | } |
src/http/mcp/tools/hooks.ts +5 −2
| @@ -1,6 +1,7 @@ | ||
| 1 | 1 | import { canAdmin } from "../../../core/repos.ts"; |
| 2 | 2 | import { listWebhooks } from "../../../core/webhooks.ts"; |
| 3 | 3 | import { createWebhook, deleteWebhook } from "../../../actions/repos.ts"; |
| 4 | import { pageRows } from "../page.ts"; | |
| 4 | 5 | import { good, bad, type Answer, type Args, type Tool, REPO_ARG, repoArg, isAnswer, str } from "../shared.ts"; |
| 5 | 6 | |
| 6 | 7 | const HOOK_LIMITS = { url: 2048, kinds: 200, secret: 200, id: 64 } as const; |
| @@ -19,7 +20,7 @@ const notAdmin = (): Answer => bad("not_found", "no such app or repository, or y | ||
| 19 | 20 | |
| 20 | 21 | export const tool: Tool = { |
| 21 | 22 | name: "hooks", |
| 22 | doc: "`{ repo, action, url, secret, kinds, id }`: `list`, `add` (`secret`, 16–200 characters, is required) or `remove` the repository's webhooks, as `…/hooks`; an admin-scope token on a repository you administer, and a secret is never shown", | |
| 23 | doc: "`{ repo, action, url, secret, kinds, id }`: `list` (pages: `limit` 1–200, `cursor`; `{ items, next_cursor }`), `add` (`secret`, 16–200 characters, is required) or `remove` the repository's webhooks, as `…/hooks`; an admin-scope token on a repository you administer, and a secret is never shown", | |
| 23 | 24 | description: "List, add (`url`, `secret`, `kinds`) or remove (`id`) webhooks; admins.", |
| 24 | 25 | inputSchema: { |
| 25 | 26 | type: "object", |
| @@ -30,6 +31,8 @@ export const tool: Tool = { | ||
| 30 | 31 | kinds: { type: "string", maxLength: 200 }, |
| 31 | 32 | secret: { type: "string", maxLength: 200 }, |
| 32 | 33 | id: { type: "string", maxLength: 64 }, |
| 34 | limit: { type: "number" }, | |
| 35 | cursor: { type: "string" }, | |
| 33 | 36 | }, |
| 34 | 37 | required: ["repo", "action"], |
| 35 | 38 | }, |
| @@ -41,7 +44,7 @@ export const tool: Tool = { | ||
| 41 | 44 | const action = args["action"]; |
| 42 | 45 | if (action === "list") { |
| 43 | 46 | if (!canAdmin(ctx.app.db, repo, user)) return notAdmin(); |
| 44 | return good(listWebhooks(ctx.app.db, repo.id)); | |
| 47 | return pageRows(listWebhooks(ctx.app.db, repo.id), (h) => h.id, args); | |
| 45 | 48 | } |
| 46 | 49 | if (action === "add") { |
| 47 | 50 | const long = hookTooLong(args); |
src/http/mcp/tools/repo_labels.ts +5 −2
| @@ -2,11 +2,12 @@ import { listLabels } from "../../../core/issues.ts"; | ||
| 2 | 2 | import { scopeAllows } from "../../../core/tokens.ts"; |
| 3 | 3 | import { createLabel, deleteLabel } from "../../../actions/repos.ts"; |
| 4 | 4 | import { LABEL_WORDS } from "../../../actions/label_words.ts"; |
| 5 | import { pageRows } from "../page.ts"; | |
| 5 | 6 | import { good, bad, type Tool, REPO_ARG, repoArg, isAnswer } from "../shared.ts"; |
| 6 | 7 | |
| 7 | 8 | export const tool: Tool = { |
| 8 | 9 | name: "repo_labels", |
| 9 | doc: "`{ repo, action?, name?, color?, id? }`: list (`list`, the default; a read-only token may), create (`create`, `name`, `color` six hex digits) or remove (`remove`, `id`) the repository's labels, as `GET`/`POST …/labels` and `DELETE …/labels/:id`; create and remove need an admin token and a repository admin", | |
| 10 | doc: "`{ repo, action?, name?, color?, id? }`: list (`list`, the default, paged by `limit` 1–200 and `cursor`, as `{ items, next_cursor }`; a read-only token may), create (`create`, `name`, `color` six hex digits) or remove (`remove`, `id`) the repository's labels, as `GET`/`POST …/labels` and `DELETE …/labels/:id`; create and remove need an admin token and a repository admin", | |
| 10 | 11 | description: |
| 11 | 12 | "Repo labels: list (default), create, remove (admin token).", |
| 12 | 13 | inputSchema: { |
| @@ -17,6 +18,8 @@ export const tool: Tool = { | ||
| 17 | 18 | name: { type: "string" }, |
| 18 | 19 | color: { type: "string" }, |
| 19 | 20 | id: { type: "string" }, |
| 21 | limit: { type: "number" }, | |
| 22 | cursor: { type: "string" }, | |
| 20 | 23 | }, |
| 21 | 24 | required: ["repo"], |
| 22 | 25 | }, |
| @@ -27,7 +30,7 @@ export const tool: Tool = { | ||
| 27 | 30 | const repo = repoArg(ctx, user, args); |
| 28 | 31 | if (isAnswer(repo)) return repo; |
| 29 | 32 | const action = args["action"] ?? "list"; |
| 30 | if (action === "list") return good({ items: listLabels(ctx.app.db, repo.id) }); | |
| 33 | if (action === "list") return pageRows(listLabels(ctx.app.db, repo.id), (l) => l.id, args); | |
outdated
The route (#6972 |
||
| 31 | 34 | if (action !== "create" && action !== "remove") |
| 32 | 35 | return bad("invalid", "action is list, create or remove"); |
| 33 | 36 | // The API asks for an admin token on both routes; so does this. |
test/mcp-hooks.test.ts +23 −4
| @@ -66,20 +66,39 @@ describe("MCP hooks", () => { | ||
| 66 | 66 | expect(added.value.kinds).toBe("*"); |
| 67 | 67 | expect(JSON.stringify(added)).not.toContain("s3cret-value-long-enough"); |
| 68 | 68 | const listed = await tool(aliceAdmin, { repo: "alice/game", action: "list" }); |
| 69 | expect(listed.value.map((h: any) => h.id)).toEqual([added.value.id]); | |
| 69 | expect(listed.value.items.map((h: any) => h.id)).toEqual([added.value.id]); | |
| 70 | 70 | expect(JSON.stringify(listed)).not.toContain("s3cret-value-long-enough"); |
| 71 | 71 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "remove", id: added.value.id })).value).toEqual({}); |
| 72 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list" })).value).toEqual([]); | |
| 72 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list" })).value).toEqual({ items: [], next_cursor: null }); | |
| 73 | 73 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "remove", id: added.value.id })).error).toContain("not_found"); |
| 74 | 74 | }); |
| 75 | 75 | |
| 76 | test("list pages by limit and cursor, and refuses what the route refuses", async () => { | |
| 77 | const ids: string[] = []; | |
| 78 | for (let i = 0; i < 3; i++) { | |
| 79 | const a = await tool(aliceAdmin, { repo: "alice/game", action: "add", url: `https://example.com/p${i}`, secret: "0123456789abcdef" }); | |
| 80 | ids.push(a.value.id); | |
| 81 | } | |
| 82 | ids.sort(); | |
| 83 | const first = await tool(aliceAdmin, { repo: "alice/game", action: "list", limit: 2 }); | |
| 84 | expect(first.value.items.map((h: any) => h.id)).toEqual(ids.slice(0, 2)); | |
| 85 | expect(first.value.next_cursor).toBe(ids[1]); | |
| 86 | const second = await tool(aliceAdmin, { repo: "alice/game", action: "list", limit: 2, cursor: first.value.next_cursor }); | |
| 87 | expect(second.value.items.map((h: any) => h.id)).toEqual([ids[2]]); | |
| 88 | expect(second.value.next_cursor).toBeNull(); | |
| 89 | for (const limit of [0, 201, 1.5, "5"]) | |
| 90 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list", limit })).error).toContain("invalid_limit"); | |
| 91 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list", cursor: "gone" })).error).toContain("invalid_cursor"); | |
| 92 | for (const id of ids) await tool(aliceAdmin, { repo: "alice/game", action: "remove", id }); | |
| 93 | }); | |
| 94 | ||
| 76 | 95 | test("a reader is refused and nothing is added", async () => { |
| 77 | 96 | for (const args of [ |
| 78 | 97 | { repo: "alice/game", action: "add", url: "https://example.com/x", secret: "0123456789abcdef" }, |
| 79 | 98 | { repo: "alice/game", action: "list" }, |
| 80 | 99 | ]) |
| 81 | 100 | expect((await tool(bobAdmin, args)).error).toContain("not_found"); |
| 82 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list" })).value).toEqual([]); | |
| 101 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list" })).value).toEqual({ items: [], next_cursor: null }); | |
| 83 | 102 | }); |
| 84 | 103 | |
| 85 | 104 | test("an invalid URL is refused with the API's error", async () => { |
| @@ -98,7 +117,7 @@ describe("MCP hooks", () => { | ||
| 98 | 117 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "remove", id: "i".repeat(65) })).error).toContain("id is text"); |
| 99 | 118 | for (const secret of [undefined, "short"]) |
| 100 | 119 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "add", url: "https://example.com/h", secret })).error).toContain("secret is required"); |
| 101 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list" })).value).toEqual([]); | |
| 120 | expect((await tool(aliceAdmin, { repo: "alice/game", action: "list" })).value).toEqual({ items: [], next_cursor: null }); | |
| 102 | 121 | }); |
| 103 | 122 | |
| 104 | 123 | test("it is listed, and callable, only with an admin-scope token", async () => { |
test/mcp-repo-labels.test.ts +15
| @@ -105,6 +105,21 @@ describe("MCP repo_labels", () => { | ||
| 105 | 105 | expect((await call(admin, { repo: R, action: "remove", id: "nope" })).error).toContain("not_found"); |
| 106 | 106 | }); |
| 107 | 107 | |
| 108 | test("list pages by limit and cursor, and refuses what the route refuses", async () => { | |
| 109 | for (const name of ["pa", "pb", "pc"]) await call(admin, { repo: R, action: "create", name, color: "aabbcc" }); | |
| 110 | const first = await call(admin, { repo: R, action: "list", limit: 2 }); | |
| 111 | expect(first.value.items.map((l: any) => l.name)).toEqual(["pa", "pb"]); | |
| 112 | expect(first.value.next_cursor).toBe(first.value.items[1].id); | |
outdated
Once the cursor is the id, expect the id of |
||
| 113 | const second = await call(admin, { repo: R, action: "list", limit: 3, cursor: first.value.next_cursor }); | |
| 114 | expect(second.value.items.map((l: any) => l.name)).toEqual(["pc", "ui"]); | |
| 115 | expect(second.value.next_cursor).toBeNull(); | |
| 116 | for (const limit of [0, 201, 1.5, "5"]) | |
| 117 | expect((await call(admin, { repo: R, action: "list", limit })).error).toContain("invalid_limit"); | |
| 118 | expect((await call(admin, { repo: R, action: "list", cursor: "gone" })).error).toContain("invalid_cursor"); | |
| 119 | for (const l of (await call(admin, { repo: R, action: "list" })).value.items) | |
| 120 | await call(admin, { repo: R, action: "remove", id: l.id }); | |
| 121 | }); | |
| 122 | ||
| 108 | 123 | test("a read-only token lists, named or by default, and cannot change", async () => { |
| 109 | 124 | expect(Array.isArray((await call(ro, { repo: R })).value.items)).toBe(true); |
| 110 | 125 | expect(Array.isArray((await call(ro, { repo: R, action: "list" })).value.items)).toBe(true); |
new above
offline
Nit: every other string here has a
maxLength, and the API caps a keyed cursor at 160 (KEYED_CURSOR_MAX). Givecursorthe same cap (64 is enough for an id), here and inrepo_labels, so the schema says what the tool accepts.