Public: anyone can read this; the crew can change it.
The OpenAPI document describes a pull's review threads: list, resolve and reopen #5393
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5381-the-openapi-document-describes-a-pull-s → main 4 commits fork 4 threads · 4 unresolved
4 files
changed +50
−6
src/contract/openapi.ts +21
| @@ -531,6 +531,27 @@ export const ROUTES: readonly Route[] = [ | ||
| 531 | 531 | refusals: { bad_anchor: "`path`, `line` and `side` (old or new) name no line of the pull's current diff (422)" }, |
| 532 | 532 | }, |
| 533 | 533 | { method: "post", path: `${REPO}/pulls/{number}/merge`, summary: "Merge a pull (a person's)", auth: true }, |
| 534 | { method: "get", path: `${REPO}/pulls/{number}/threads`, summary: "List a pull's review threads: every review comment sharing one anchor, and whether the conversation is resolved", auth: false }, | |
| 535 | { | |
| 536 | method: "post", | |
| 537 | path: `${REPO}/pulls/{number}/threads/{anchor}/resolve`, | |
| 538 | summary: "Mark a review thread resolved (rw scope; the thread's author or a writer); answers the thread", | |
| 539 | auth: true, | |
| 540 | refusals: { | |
| 541 | not_found: "the pull has no thread at `anchor` (404)", | |
| 542 | forbidden: "you neither wrote the thread's comments nor can write to the repository (403)", | |
| 543 | }, | |
| 544 | }, | |
| 545 | { | |
| 546 | method: "delete", | |
| 547 | path: `${REPO}/pulls/{number}/threads/{anchor}/resolve`, | |
| 548 | summary: "Reopen a resolved review thread (rw scope; the thread's author or a writer); answers the thread", | |
| 549 | auth: true, | |
| 550 | refusals: { | |
| 551 | not_found: "the pull has no thread at `anchor` (404)", | |
| 552 | forbidden: "you neither wrote the thread's comments nor can write to the repository (403)", | |
| 553 | }, | |
| 554 | }, | |
| 534 | 555 | { |
| 535 | 556 | method: "post", |
| 536 | 557 | path: `${REPO}/pulls/{number}/approve-here`, |
test/openapi-coverage.test.ts −3
| @@ -15,7 +15,6 @@ const NOT_YET_DESCRIBED: readonly string[] = [ | ||
| 15 | 15 | "DELETE /api/v1/repos/{}/{}/labels/{}", |
| 16 | 16 | "DELETE /api/v1/repos/{}/{}/milestones/{}/watch", |
| 17 | 17 | "DELETE /api/v1/repos/{}/{}/pulls/{}/hold", |
| 18 | "DELETE /api/v1/repos/{}/{}/pulls/{}/threads/{}/resolve", | |
| 19 | 18 | "DELETE /api/v1/user/keys/{}", |
| 20 | 19 | "DELETE /api/v1/user/origin-apps/{}", |
| 21 | 20 | "DELETE /api/v1/user/origins/{}", |
| @@ -49,7 +48,6 @@ const NOT_YET_DESCRIBED: readonly string[] = [ | ||
| 49 | 48 | "GET /api/v1/repos/{}/{}/pulls/{}/checks", |
| 50 | 49 | "GET /api/v1/repos/{}/{}/pulls/{}/review-states", |
| 51 | 50 | "GET /api/v1/repos/{}/{}/pulls/{}/reviews", |
| 52 | "GET /api/v1/repos/{}/{}/pulls/{}/threads", | |
| 53 | 51 | "GET /api/v1/repos/{}/{}/reports", |
| 54 | 52 | "GET /api/v1/user", |
| 55 | 53 | "GET /api/v1/user/assigned", |
| @@ -104,7 +102,6 @@ const NOT_YET_DESCRIBED: readonly string[] = [ | ||
| 104 | 102 | "POST /api/v1/repos/{}/{}/pulls/{}/comments", |
| 105 | 103 | "POST /api/v1/repos/{}/{}/pulls/{}/looks", |
| 106 | 104 | "POST /api/v1/repos/{}/{}/pulls/{}/review-states", |
| 107 | "POST /api/v1/repos/{}/{}/pulls/{}/threads/{}/resolve", | |
| 108 | 105 | "POST /api/v1/repos/{}/{}/pulls/{}/variant", |
| 109 | 106 | "POST /api/v1/repos/{}/{}/reports/{}/accept", |
| 110 | 107 | "POST /api/v1/repos/{}/{}/reports/{}/dismiss", |
test/openapi-oa-pull-threads.test.ts added +24
| @@ -0,0 +1,24 @@ | ||
| 1 | /** A pull's review threads are in the served OpenAPI document. */ | |
| 2 | import { expect, test } from "bun:test"; | |
| 3 | import { openapi } from "../src/contract/openapi.ts"; | |
| 4 | ||
| 5 | const T = "/api/v1/repos/{owner}/{name}/pulls/{number}/threads"; | |
| 6 | ||
| 7 | test("list, resolve and reopen are described", () => { | |
| 8 | const paths = openapi("http://x", "1").paths as Record<string, Record<string, any>>; | |
| 9 | expect(paths[T]?.get).toBeDefined(); | |
| 10 | expect(paths[T]?.get.security).toBeUndefined(); | |
| 11 | expect(paths[`${T}/{anchor}/resolve`]?.post).toBeDefined(); | |
| 12 | expect(paths[`${T}/{anchor}/resolve`]?.delete).toBeDefined(); | |
| 13 | expect(paths[`${T}/{anchor}/resolve`]?.post.security).toBeDefined(); | |
| 14 | }); | |
| 15 | ||
| 16 | test("resolve and reopen state their refusals with the statuses the handler answers", () => { | |
| 17 | const paths = openapi("http://x", "1").paths as Record<string, Record<string, any>>; | |
| 18 | for (const method of ["post", "delete"]) { | |
| 19 | const text: string = paths[`${T}/{anchor}/resolve`]![method].responses.default.description; | |
| 20 | expect(text).toContain("`not_found` (the pull has no thread at `anchor` (404))"); | |
| 21 | expect(text).toContain("`forbidden` (you neither wrote the thread's comments nor can write to the repository (403))"); | |
| 22 | expect(text).not.toContain("422"); | |
| 23 | } | |
| 24 | }); |
test/shards.test.ts +5 −3
| @@ -653,10 +653,12 @@ describe("the gate audit before any shard", () => { | ||
| 653 | 653 | }); |
| 654 | 654 | |
| 655 | 655 | test("a new file is in the gate with no list naming it, and explicit files are not audited", async () => { |
| 656 | const last = String(gateFiles().length + 1); | |
| 657 | const got = await run(["test/zz-new.test.ts"], ["--gate", "--shard", `${last}/${last}`]); | |
| 656 | // One slice holds every file, so the count includes the new one; a shard | |
| 657 | // per file would outgrow the three digits a shard spec allows. | |
| 658 | const all = String(gateFiles().length + 1); | |
| 659 | const got = await run(["test/zz-new.test.ts"], ["--gate", "--shard", "1/1"]); | |
| 658 | 660 | expect(got.err).not.toContain("EXCLUDED"); |
| 659 | expect(got.out).toContain(`matrix slice ${last}/${last} (1 of ${last} files)`); | |
| 661 | expect(got.out).toContain(`matrix slice 1/1 (${all} of ${all} files)`); | |
| 660 | 662 | const explicit = await run(["test/zz-new.test.ts"], ["test/zz-new.test.ts"], [listed[0]!]); |
| 661 | 663 | expect(explicit.err).not.toContain("EXCLUDED"); |
| 662 | 664 | }, 30_000); |
new above
offline