trevato/forge

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

More
Forks1g fMapg aRuns14g rEventsFlows

The OpenAPI document describes editing a pull, its reviews, approving it here and assigning an issue or pull #5261

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/5241-the-openapi-document-describes-editing-a → main 1 commit fork

  • Merged. scout/5241-the-openapi-document-describes-editing-a was merged into main as edc5e3c.

Review

Sign in to review this change.
3 files changed +159 −3
src/contract/openapi.ts +52 −2
@@ -285,6 +285,19 @@ export const ROUTES: readonly Route[] = [
285285 { method: "get", path: "/api/v1/telemetry/sources", summary: "List telemetry sources; answers { sources }; the owner or an agent holding telemetry:read", auth: true },
286286 { method: "get", path: "/api/v1/telemetry/history", summary: "Read telemetry history; answers { points }; the owner or an agent holding telemetry:read", auth: true },
287287 { method: "get", path: `${REPO}/pulls/{number}`, summary: "Read one pull with its diff and review gate (diff=0 leaves the diff out)", auth: false, query: { diff: "string" } },
288 {
289 method: "patch",
290 path: `${REPO}/pulls/{number}`,
291 summary: "Edit a pull's title or body and open or close it (rw scope); answers the pull",
292 auth: true,
293 body: { title: "string", body: "string", state: "string" },
294 refusals: {
295 invalid_text: "`title` or `body` is not a string (422)",
296 invalid_state: "`state` is neither open nor closed (422)",
297 invalid_reason: "`reason` or `into` is given; a pull has none (422)",
298 nothing_to_change: "none of `title`, `body` and `state` is given (422)",
299 },
300 },
288301 { method: "get", path: `${REPO}/issues/{number}/comments`, summary: "Page through an issue's or pull's comments, oldest first: {comments, next}; limit 1 to 100 (default 50), after a comment id", auth: false, query: { after: "string", limit: "integer" } },
289302 {
290303 method: "post",
@@ -383,14 +396,51 @@ export const ROUTES: readonly Route[] = [
383396 auth: false,
384397 query: { kind: "string" },
385398 },
399 { method: "get", path: `${REPO}/pulls/{number}/reviews`, summary: "List a pull's line-anchored review comments, each with its `anchor` and whether it is `outdated`", auth: false },
386400 {
387401 method: "post",
388402 path: `${REPO}/pulls/{number}/reviews`,
389 summary: "Give a verdict on a pull",
403 summary: "Comment on one line of a pull's diff (rw scope); answers 201 with the comment; a verdict goes to …/review-states",
390404 auth: true,
391 body: { verdict: "string", body: "string" },
405 status: 201,
406 body: { path: "string", line: "integer", side: "string", body: "string" },
407 refusals: { bad_anchor: "`path`, `line` and `side` (old or new) name no line of the pull's current diff (422)" },
392408 },
393409 { method: "post", path: `${REPO}/pulls/{number}/merge`, summary: "Merge a pull (a person's)", auth: true },
410 {
411 method: "post",
412 path: `${REPO}/pulls/{number}/approve-here`,
413 summary: "Let an origin fork's pull run here: a writer approves its current head (rw scope); answers 201",
414 auth: true,
415 status: 201,
416 refusals: {
417 not_foreign: "the pull needs no approval to run (422)",
418 self_review: "you cannot approve your own pull (422)",
419 not_open: "the pull is closed (422)",
420 },
421 },
422 {
423 method: "put",
424 path: `${REPO}/issues/{number}/assignee`,
425 summary: "Assign an issue to a person or crew member, or to no one with null (rw scope); answers the issue",
426 auth: true,
427 body: { assignee: "string" },
428 refusals: {
429 invalid_assignee: "`assignee` is neither a username nor null (422)",
430 no_such_user: "`assignee` names no account (422)",
431 },
432 },
433 {
434 method: "put",
435 path: `${REPO}/pulls/{number}/assignee`,
436 summary: "Assign a pull to a person or crew member, or to no one with null (rw scope); answers the pull",
437 auth: true,
438 body: { assignee: "string" },
439 refusals: {
440 invalid_assignee: "`assignee` is neither a username nor null (422)",
441 no_such_user: "`assignee` names no account (422)",
442 },
443 },
394444 {
395445 method: "patch",
396446 path: REPO,
test/openapi-pulls.test.ts added +104
@@ -0,0 +1,104 @@
1 /** Editing a pull, its reviews, approving it here and assigning are in the served OpenAPI document, and the refusals match the routes. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { openPull } from "../src/actions/pulls.ts";
7 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
8 import { openIssue } from "../src/actions/issues.ts";
9 import { createApp, type App } from "../src/app.ts";
10 import { openapi } from "../src/contract/openapi.ts";
11 import { createToken } from "../src/core/tokens.ts";
12 import { createUser } from "../src/core/users.ts";
13 import { forgeHttp, type ForgeHttp } from "../src/http/server.ts";
14 import { quietLogs } from "./helpers/globals.ts";
15
16 quietLogs();
17
18 const R = "/api/v1/repos/{owner}/{name}";
19 const WANT: readonly [string, string][] = [
20 ["patch", `${R}/pulls/{number}`],
21 ["get", `${R}/pulls/{number}/reviews`],
22 ["post", `${R}/pulls/{number}/reviews`],
23 ["post", `${R}/pulls/{number}/approve-here`],
24 ["put", `${R}/issues/{number}/assignee`],
25 ["put", `${R}/pulls/{number}/assignee`],
26 ];
27
28 let dir: string;
29 let app: App;
30 let http: ForgeHttp;
31 let token: string;
32 let pull: number;
33 let issue: number;
34
35 beforeAll(async () => {
36 dir = await mkdtemp(join(tmpdir(), "forge-openapi-pulls-"));
37 app = createApp({ dataDir: dir, port: 0, host: "forge.test" });
38 const made = await createUser(app.db, {
39 origin: "seed",
40 trusted: true,
41 username: "alice",
42 email: "[email protected]",
43 password: "a-good-passphrase",
44 adminIfFirst: true,
45 });
46 if (!made.ok) throw new Error(made.error);
47 const alice = made.value;
48 const repo = await createRepo(app, alice, { name: "lib", visibility: "public" });
49 if (!repo.ok) throw new Error(String(repo.error));
50 const first = await commitToBranch(app, alice, repo.value, {
51 message: "Start",
52 files: [{ path: "README.md", content: "# lib\n" }],
53 });
54 if (!first.ok) throw new Error(JSON.stringify(first.error));
55 const committed = await commitToBranch(app, alice, repo.value, {
56 branch: "feat",
57 message: "A change",
58 files: [{ path: "a.md", content: "hi\n" }],
59 });
60 if (!committed.ok) throw new Error(JSON.stringify(committed.error));
61 const opened = await openPull(app, alice, repo.value, { title: "Mine", baseBranch: "main", headBranch: "feat" });
62 if (!opened.ok) throw new Error(String(opened.error));
63 pull = opened.value.number;
64 const filed = openIssue(app, alice, repo.value, { title: "An issue" });
65 if (!filed.ok) throw new Error(String(filed.error));
66 issue = filed.value.number;
67 token = createToken(app.db, alice.id, "t", "rw").token;
68 http = forgeHttp(app);
69 }, 120_000);
70
71 afterAll(async () => {
72 http?.close();
73 app.close();
74 await rm(dir, { recursive: true, force: true });
75 });
76
77 const call = (method: string, path: string, body?: unknown) =>
78 http.handle(
79 new Request(`http://forge.test/api/v1/repos/alice/lib${path}`, {
80 method,
81 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
82 body: body === undefined ? undefined : JSON.stringify(body),
83 }),
84 );
85
86 test("each path and method is in the document", () => {
87 const paths = openapi("http://x", "1").paths as Record<string, Record<string, unknown>>;
88 for (const [method, path] of WANT) expect(paths[path]?.[method]).toBeDefined();
89 });
90
91 test("one refusal per row is what the route answers", async () => {
92 const code = async (res: Response) => ((await res.json()) as { error: string }).error;
93 const patched = await call("PATCH", `/pulls/${pull}`, { state: "bogus" });
94 expect([patched.status, await code(patched)]).toEqual([422, "invalid_state"]);
95 const reviewed = await call("POST", `/pulls/${pull}/reviews`, { path: "nope.md", line: 1, body: "x" });
96 expect([reviewed.status, await code(reviewed)]).toEqual([422, "bad_anchor"]);
97 const approved = await call("POST", `/pulls/${pull}/approve-here`);
98 expect(await code(approved)).toBe("not_foreign");
99 for (const [kind, number] of [["issues", issue], ["pulls", pull]] as const) {
100 const assigned = await call("PUT", `/${kind}/${number}/assignee`, { assignee: "nobody-here" });
101 expect([assigned.status, await code(assigned)]).toEqual([422, "no_such_user"]);
102 }
103 expect((await call("GET", `/pulls/${pull}/reviews`)).status).toBe(200);
104 });
test/openapi.test.ts +3 −1
@@ -29,7 +29,9 @@ test("every http route the manifest names is described", () => {
2929 test("every described route is registered in the router", () => {
3030 const src = ["src/http/server.ts", "src/http/api.ts", "src/http/mcp.ts"]
3131 .map((f) => readFileSync(f, "utf8"))
32 .join("\n");
32 .join("\n")
33 // A route registered in a loop over issues and pulls is two routes.
34 .replace(/"(PUT)",\s*`([^`]*)\$\{kind\}([^`]*)`/g, (_, m, a, b) => `"${m}", "${a}issues${b}"\n"${m}", "${a}pulls${b}"`);
3335 const registered = new Set(
3436 [...src.matchAll(/"(GET|POST|PATCH|DELETE|PUT)",\s*"([^"]+)"/g)].map((m) => `${m[1]} ${m[2]}`),
3537 );

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs