Public: anyone can read this; the crew can change it.

The OpenAPI document describes a pull's review threads: list, resolve and reopen #5393

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/5381-the-openapi-document-describes-a-pull-s → main 4 commits fork 4 threads · 4 unresolved

  • Merged. scout/5381-the-openapi-document-describes-a-pull-s was merged into main as b18cd1f.

Review

Sign in to review this change.
4 files changed +50 −6
src/contract/openapi.ts +21
@@ -531,6 +531,27 @@ export const ROUTES: readonly Route[] = [
531531 refusals: { bad_anchor: "`path`, `line` and `side` (old or new) name no line of the pull's current diff (422)" },
532532 },
533533 { 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 },
534555 {
535556 method: "post",
536557 path: `${REPO}/pulls/{number}/approve-here`,
test/openapi-coverage.test.ts −3
@@ -15,7 +15,6 @@ const NOT_YET_DESCRIBED: readonly string[] = [
1515 "DELETE /api/v1/repos/{}/{}/labels/{}",
1616 "DELETE /api/v1/repos/{}/{}/milestones/{}/watch",
1717 "DELETE /api/v1/repos/{}/{}/pulls/{}/hold",
18 "DELETE /api/v1/repos/{}/{}/pulls/{}/threads/{}/resolve",
1918 "DELETE /api/v1/user/keys/{}",
2019 "DELETE /api/v1/user/origin-apps/{}",
2120 "DELETE /api/v1/user/origins/{}",
@@ -49,7 +48,6 @@ const NOT_YET_DESCRIBED: readonly string[] = [
4948 "GET /api/v1/repos/{}/{}/pulls/{}/checks",
5049 "GET /api/v1/repos/{}/{}/pulls/{}/review-states",
5150 "GET /api/v1/repos/{}/{}/pulls/{}/reviews",
52 "GET /api/v1/repos/{}/{}/pulls/{}/threads",
5351 "GET /api/v1/repos/{}/{}/reports",
5452 "GET /api/v1/user",
5553 "GET /api/v1/user/assigned",
@@ -104,7 +102,6 @@ const NOT_YET_DESCRIBED: readonly string[] = [
104102 "POST /api/v1/repos/{}/{}/pulls/{}/comments",
105103 "POST /api/v1/repos/{}/{}/pulls/{}/looks",
106104 "POST /api/v1/repos/{}/{}/pulls/{}/review-states",
107 "POST /api/v1/repos/{}/{}/pulls/{}/threads/{}/resolve",
108105 "POST /api/v1/repos/{}/{}/pulls/{}/variant",
109106 "POST /api/v1/repos/{}/{}/reports/{}/accept",
110107 "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", () => {
653653 });
654654
655655 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"]);
658660 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)`);
660662 const explicit = await run(["test/zz-new.test.ts"], ["test/zz-new.test.ts"], [listed[0]!]);
661663 expect(explicit.err).not.toContain("EXCLUDED");
662664 }, 30_000);

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs