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

An approval is recorded against the head and the base it was given for, and says when either moved #12164

Merged opened this

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

Merged

scout/forge:scout/12158-an-approval-is-recorded-against-the-head → main 3 commits fork

  • Merged. scout/12158-an-approval-is-recorded-against-the-head was merged into main as fb6f20c.

Review

Sign in to review this change.
17 files changed +263 −11
docs/features/approval-base.md added +10
@@ -0,0 +1,10 @@
1 # An approval says what it was given for
2
3 An approval is recorded against the head and the base tip it was given for. When the base moves after it (another pull landed), the approval is still counted, and every door says the base moved.
4
5 - **Page:** the reviews chip in the merge box on the pull's Files step reads "the base moved since the approval" in red. Nothing is added while the approval is current.
6 - **API:** `GET /api/v1/repos/:owner/:name/pulls/:number` carries `approvedAt: { at, head, base, current }`, or `null` when no one's latest verdict approves. `base` is `null` for an approval given before the base was recorded; `current` is `false` once the head or the base moved.
7 - **MCP:** `pulls` with a `repo` carries `movedSinceApproval` on each row; the card says "moved since approval".
8 - **Terminal:** the pull screen's merge box adds "the base moved since the approval" after the approval count.
9
10 It is information, not a gate: the base moves with every landing, so refusing a merge on it would ask for a new approval after every unrelated merge. A moved head still dismisses the approval, as before.
src/actions/pulls.ts +2
@@ -854,6 +854,7 @@ export async function submitReview(
854854 state: input.state,
855855 body: input.body ?? "",
856856 commitOid: status.headOid,
857 baseOid: status.baseOid,
857858 });
858859 if (!recorded.ok) return recorded;
859860 app.bus.append({
@@ -1034,6 +1035,7 @@ export async function approveHere(
10341035 state: "approved",
10351036 body: "",
10361037 commitOid: status.headOid,
1038 baseOid: status.baseOid,
10371039 });
10381040 if (!recorded.ok) return err("not_found");
10391041 app.bus.append({
src/core/migration-pins/20261114070000-review-base.pin added +1
@@ -0,0 +1,1 @@
1 30e84ed139e687b40c02ab983058eb9539453e650e97244b9c7e7fcaef8ae41e
src/core/migrations/20261114070000-review-base.ts added +6
@@ -0,0 +1,6 @@
1 // An approval is given against a head and the base tip it was given for. `base_oid` is that base
2 // tip, written where approvals are written; '' is a review made before the column, whose base is
3 // not known (it stays current by its head alone). Additive.
4 export default `
5 ALTER TABLE review ADD COLUMN base_oid TEXT NOT NULL DEFAULT '';
6 `;
src/core/pulls.ts +12 −2
@@ -41,7 +41,7 @@ import {
4141 type Issue,
4242 type ListIssuesOptions,
4343 } from "./issues.ts";
44 import { reviewSummary } from "./reviews.ts";
44 import { approvalRecord, listReviews, reviewSummary, type ApprovalRecord } from "./reviews.ts";
4545 import { checkStale, checksHold } from "../checks/stale.ts";
4646 import { checkSummary } from "./runs.ts";
4747 import { writeStamp } from "./write_clock.ts";
@@ -781,6 +781,11 @@ export interface PullReviewStatus {
781781 /** Whether this repo demands an approval at all (`repo.requireReview`). */
782782 readonly required: boolean;
783783 readonly mergeable: boolean;
784 /**
785 * The approval the pull stands on, with the head and base it was given for; null when none,
786 * or when the caller named no base.
787 */
788 readonly approval: ApprovalRecord | null;
784789 }
785790
786791 /**
@@ -830,9 +835,14 @@ export function reviewGate(
830835 gate: ReviewGate,
831836 pull: Pull,
832837 headOid: string,
838 baseOid?: string,
833839 ): PullReviewStatus {
834840 const summary = reviewSummary(gate.db, pull.id, headOid);
835841 return {
842 approval:
843 baseOid === undefined
844 ? null
845 : approvalRecord(listReviews(gate.db, pull.id), headOid, baseOid),
836846 approved: summary.approvals,
837847 changesRequested: summary.changesRequested,
838848 required: gate.requireReview,
@@ -864,7 +874,7 @@ export async function pullStatus(
864874 ]);
865875 const baseOid = liveBase ?? pull.baseOid;
866876 const headOid = liveHead ?? pull.headOid;
867 const reviews = gate === undefined ? null : reviewGate(gate, pull, headOid);
877 const reviews = gate === undefined ? null : reviewGate(gate, pull, headOid, baseOid);
868878 // A fork's pull combined on our side merges as its combined branch (#9012).
869879 const lands = pull.crossRepo
870880 ? ((await combinedTip(dir, pull.number, headOid)) ?? headOid)
src/core/reviews.ts +43 −3
@@ -210,6 +210,8 @@ export interface Review {
210210 readonly body: string;
211211 /** The head the verdict was passed on. A verdict on an older head is stale. */
212212 readonly commitOid: string;
213 /** The base tip the verdict was given against; '' when it was not recorded. */
214 readonly baseOid: string;
213215 readonly createdAt: string;
214216 }
215217
@@ -231,6 +233,8 @@ export interface SubmitReviewInput {
231233 /** May be empty: an approval needs no words. */
232234 body?: string;
233235 commitOid: string;
236 /** The base tip the verdict is given against. */
237 baseOid?: string;
234238 /** Set when the review came from an origin rather than from here. */
235239 originRemoteId?: string;
236240 originAuthor?: string;
@@ -254,8 +258,8 @@ export function submitReview(
254258 const write = db.transaction(() => {
255259 db.query(
256260 `INSERT INTO review (id, issue_id, author_id, state, body, commit_oid,
257 origin_remote_id, origin_author, created_at)
258 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)`,
261 origin_remote_id, origin_author, created_at, base_oid)
262 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)`,
259263 ).run(
260264 reviewId,
261265 input.issueId,
@@ -266,6 +270,7 @@ export function submitReview(
266270 input.originRemoteId ?? null,
267271 input.originAuthor ?? null,
268272 now,
273 input.baseOid ?? "",
269274 );
270275 db.query("UPDATE issue SET updated_at = ?1 WHERE id = ?2").run(
271276 new Date().toISOString(),
@@ -284,13 +289,14 @@ interface ReviewRowRaw {
284289 state: ReviewState;
285290 body: string;
286291 commit_oid: string;
292 base_oid: string;
287293 created_at: string;
288294 }
289295
290296 const REVIEW_SELECT = `
291297 SELECT review.id, review.author_id, user.username AS author_name,
292298 review.origin_author, review.state, review.body, review.commit_oid,
293 review.created_at
299 review.base_oid, review.created_at
294300 FROM review JOIN user ON user.id = review.author_id`;
295301
296302 const toReview = (row: ReviewRowRaw): Review => ({
@@ -301,6 +307,7 @@ const toReview = (row: ReviewRowRaw): Review => ({
301307 state: row.state,
302308 body: row.body,
303309 commitOid: row.commit_oid,
310 baseOid: row.base_oid,
304311 createdAt: row.created_at,
305312 });
306313
@@ -617,3 +624,36 @@ export function getReviewByRemoteId(
617624 .get(issueId, remoteId);
618625 return row ? toReview(row) : undefined;
619626 }
627
628 /**
629 * The approval a pull stands on, said with what it was given for: the head
630 * and the base tip. `current` is false once either moved. Null when nobody's
631 * latest verdict approves. A review made before the base was recorded
632 * (`base` null) is judged by its head alone.
633 */
634 export interface ApprovalRecord {
635 readonly at: string;
636 readonly head: string;
637 readonly base: string | null;
638 readonly current: boolean;
639 }
640
641 export function approvalRecord(
642 reviews: readonly Review[],
643 headOid: string,
644 baseOid: string,
645 ): ApprovalRecord | null {
646 let latest: Review | undefined;
647 for (const review of latestByAuthor(reviews).values())
648 if (review.state === "approved" && (latest === undefined || review.id > latest.id))
649 latest = review;
650 if (latest === undefined) return null;
651 return {
652 at: latest.createdAt,
653 head: latest.commitOid,
654 base: latest.baseOid === "" ? null : latest.baseOid,
655 current:
656 latest.commitOid === headOid &&
657 (latest.baseOid === "" || latest.baseOid === baseOid),
658 };
659 }
src/http/api.ts +3
@@ -2783,6 +2783,9 @@ export function apiRoutes(router: Router<Handler>): void {
27832783 ...withLabels(ctx, found.pull),
27842784 check: checkWord(checkSummary(ctx.app.db, found.pull.id, status.headOid).run ?? undefined, status.headOid),
27852785 checkStale: checkStaleNow,
2786 // The approval and what it was given for; `current` is false once the head or the base
2787 // moved.
2788 approvedAt: status.reviews?.approval ?? null,
27862789 variantSiblings: variantSiblings(ctx.app.db, [found.pull])[0] ?? [],
27872790 ...(live ? { mergeable: status.mergeable, conflicts: status.conflicts } : {}),
27882791 proposal: pullProposalOf(ctx.app.db, found.pull.id) ?? null,
src/http/mcp/tools/pulls.ts +18 −2
@@ -1,6 +1,7 @@
1 import { refTip } from "../../../git/read.ts";
12 import { canRead, getReposByIds } from "../../../core/repos.ts";
23 import { mayView, sealedScope } from "../../../core/sealed.ts";
3 import { reviewSummariesFor } from "../../../core/reviews.ts";
4 import { approvalRecord, listReviewsFor, reviewSummariesFor } from "../../../core/reviews.ts";
45 import { listPulls, openPullPileRows, pullDir, pullMergeStates, pullsByIssueIds } from "../../../core/pulls.ts";
56 import { checkSummary } from "../../../core/runs.ts";
67 import type { Ctx } from "../../context.ts";
@@ -11,7 +12,7 @@ import { good, bad, type Args, type Answer, type Tool, REPO_ARG, repoArg, isAnsw
1112
1213 export const tool: Tool = {
1314 name: "pulls",
14 doc: "`{ repo?, state?, limit?, cursor? }`: without `repo`, every open pull you may read, oldest first, as `{ pulls, next_cursor }` (limit 1-200, default 50): `repo`, `number`, `title`, `check`, `verdict` (approved, changes_requested or none) and `gate` (what holds it, or null; the `why` module's cause); with `repo`, a repository's pulls, newest first, one brief line each: `number`, `title`, `author`, `head`, `check` (state), `approvals`, `changesRequested`, `mergeable` (null once closed); `state` open (default) or closed, `limit` 1-50 (default 20); no bodies, no diffs",
15 doc: "`{ repo?, state?, limit?, cursor? }`: without `repo`, every open pull you may read, oldest first, as `{ pulls, next_cursor }` (limit 1-200, default 50): `repo`, `number`, `title`, `check`, `verdict` (approved, changes_requested or none) and `gate` (what holds it, or null; the `why` module's cause); with `repo`, a repository's pulls, newest first, one brief line each: `number`, `title`, `author`, `head`, `check` (state), `approvals`, `movedSinceApproval` (the approval was given for a head or base that has since moved), `changesRequested`, `mergeable` (null once closed); `state` open (default) or closed, `limit` 1-50 (default 20); no bodies, no diffs",
1516 description:
1617 "A repository's pulls, newest first; no `repo`: all open pulls you may read, oldest first.",
1718 inputSchema: {
@@ -41,6 +42,10 @@ export const tool: Tool = {
4142 pulls.map((p) => pullDir(db, ctx.app.reposRoot, repo, p)),
4243 );
4344 const merge = await pullMergeStates(pulls.map((pull, i) => ({ dir: dirs[i]!, pull })));
45 const approvals = listReviewsFor(db, pulls.map((p) => p.id));
46 const bases = await Promise.all(
47 pulls.map((p, i) => (p.state === "open" ? refTip(dirs[i]!, `refs/heads/${p.baseBranch}`) : undefined)),
48 );
4449 const answer = good({
4550 pulls: pulls.map((p, i) => ({
4651 number: p.number,
@@ -49,6 +54,7 @@ export const tool: Tool = {
4954 head: p.headBranch,
5055 check: checkSummary(db, p.id, p.headOid).state,
5156 approvals: reviews.get(p.id)?.approvals ?? 0,
57 movedSinceApproval: movedSince(approvals.get(p.id) ?? [], p.headOid, bases[i]),
5258 changesRequested: reviews.get(p.id)?.changesRequested ?? 0,
5359 mergeable: merge[i]?.mergeable ?? null,
5460 })),
@@ -97,3 +103,13 @@ function pile(ctx: Ctx, user: User, args: Args): Answer {
97103 next_cursor,
98104 });
99105 }
106
107 /**
108 * An approval stands for a head and a base: true when either moved since. An unread base is
109 * not a move.
110 */
111 function movedSince(reviews: Parameters<typeof approvalRecord>[0], head: string, base: string | undefined): boolean {
112 const approval = approvalRecord(reviews, head, base ?? "");
113 if (approval === null) return false;
114 return base === undefined ? approval.head !== head : !approval.current;
115 }
src/tui/screens/pull.ts +9 −3
@@ -168,6 +168,8 @@ export interface Standing {
168168 readonly blocking: readonly string[];
169169 /** Verdicts a push has dismissed. */
170170 readonly stale: number;
171 /** Approvals given for a base that has since moved. */
172 readonly baseMoved?: number;
171173 }
172174
173175 export function standingOf(reviews: readonly ReviewFact[]): Standing {
@@ -175,16 +177,19 @@ export function standingOf(reviews: readonly ReviewFact[]): Standing {
175177 for (const review of reviews) latest.set(review.author, review);
176178 let approvals = 0;
177179 let stale = 0;
180 let baseMoved = 0;
178181 const blocking: string[] = [];
179182 for (const review of latest.values()) {
180183 if (!review.current) {
181184 if (review.state !== "commented") stale++;
182185 continue;
183186 }
184 if (review.state === "approved") approvals++;
185 else if (review.state === "changes_requested") blocking.push(review.author);
187 if (review.state === "approved") {
188 approvals++;
189 if (review.baseMoved === true) baseMoved++;
190 } else if (review.state === "changes_requested") blocking.push(review.author);
186191 }
187 return { approvals, blocking, stale };
192 return { approvals, blocking, stale, baseMoved };
188193 }
189194
190195 /** `2 approvals · changes requested by bob` — the merge box's second line. */
@@ -199,6 +204,7 @@ export function standingText(standing: Standing, required: boolean): string {
199204 ? ""
200205 : `changes requested by ${standing.blocking.join(", ")}`,
201206 standing.stale === 0 ? "" : `${standing.stale} dismissed by a push`,
207 (standing.baseMoved ?? 0) === 0 ? "" : "the base moved since the approval",
202208 ].filter((part) => part !== "");
203209 return parts.join(" · ");
204210 }
src/view/local.ts +4
@@ -322,6 +322,10 @@ export function localSource(
322322 commitOid: review.commitOid,
323323 createdAt: review.createdAt,
324324 current: review.commitOid === status.headOid,
325 baseMoved:
326 review.commitOid === status.headOid &&
327 review.baseOid !== "" &&
328 review.baseOid !== status.baseOid,
325329 })),
326330 };
327331 }
src/view/source.ts +2
@@ -172,6 +172,8 @@ export interface ReviewFact {
172172 readonly createdAt: string;
173173 /** False when the verdict was passed on an older head — dismissed. */
174174 readonly current: boolean;
175 /** True when this verdict is current but was given for a base that has since moved. */
176 readonly baseMoved?: boolean;
175177 }
176178
177179 /** What a pull adds to a conversation. */
src/web/pages/pull.ts +2
@@ -260,6 +260,8 @@ function reviewsChip(
260260 ${blocking.map((s) => s.who).join(", ")}</span
261261 >`,
262262 );
263 if (reviews.approved > 0 && reviews.approval !== null && !reviews.approval.current)
264 parts.push(html`<span class="bad">${icon("alert")}the base moved since the approval</span>`);
263265 if (parts.length === 0) {
264266 if (!reviews.required) return html``;
265267 parts.push(html`<span class="muted">No reviews yet</span>`);
src/web/studio/cards.ts +1 −1
@@ -777,7 +777,7 @@ export function cardFor(
777777 const slug = text(p["repo"]) || text(extra["repo"]);
778778 const path = `/${slug}/pulls/${number}`;
779779 const verdict = text(p["verdict"]);
780 const review = reviewWord(verdict, Number(p["approvals"]), Number(p["changesRequested"]));
780 const review = reviewWord(verdict, Number(p["approvals"]), Number(p["changesRequested"])) + (p["movedSinceApproval"] === true ? " · moved since approval" : "");
781781 rows.push({
782782 label: `#${number} ${clip(p["title"])}`.trim(),
783783 value: `${checkWord(check) === "none" ? "no check" : `check ${checkWord(check)}`} · ${review}`,
test/approval-base.test.ts added +114
@@ -0,0 +1,114 @@
1 /**
2 * An approval is recorded against the head and the base tip it was given
3 * for, and stops being current when either moves: the API carries
4 * `approvedAt.base`, the pull page's reviews chip says when the base moved.
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 { openPull, submitReview } from "../src/actions/pulls.ts";
11 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
12 import { createApp, type App } from "../src/app.ts";
13 import { getPull } from "../src/core/pulls.ts";
14 import type { Repo } from "../src/core/repos.ts";
15 import { approvalRecord, type Review } from "../src/core/reviews.ts";
16 import { createToken } from "../src/core/tokens.ts";
17 import { createUser, type User } from "../src/core/users.ts";
18 import { forgeHttp, type ForgeHttp } from "../src/http/server.ts";
19 import { quietLogs } from "./helpers/globals.ts";
20 import { promote } from "./helpers/members.ts";
21
22 quietLogs();
23
24 describe("approvalRecord", () => {
25 const review = (over: Partial<Review>): Review => ({
26 id: "1", authorId: "a", authorName: "a", originAuthor: null, state: "approved",
27 body: "", commitOid: "h1", baseOid: "b1", createdAt: "t", ...over,
28 });
29 test("current only while the head and the base are those it was given for", () => {
30 const r = [review({})];
31 expect(approvalRecord(r, "h1", "b1")?.current).toBe(true);
32 expect(approvalRecord(r, "h1", "b2")?.current).toBe(false);
33 expect(approvalRecord(r, "h2", "b1")?.current).toBe(false);
34 });
35 test("an approval from before the base was recorded is judged by its head", () => {
36 const r = [review({ baseOid: "" })];
37 expect(approvalRecord(r, "h1", "b9")).toEqual({ at: "t", head: "h1", base: null, current: true });
38 });
39 test("a later request for changes by the same person withdraws it", () => {
40 expect(approvalRecord([review({}), review({ id: "2", state: "changes_requested" })], "h1", "b1")).toBeNull();
41 });
42 });
43
44 describe("approvedAt over the API and the page", () => {
45 let dataDir: string;
46 let app: App;
47 let http: ForgeHttp;
48 let alice: User;
49 let bob: User;
50 let repo: Repo;
51 let token = "";
52 const url = "http://forge.test";
53
54 const get = (path: string, auth = true) =>
55 http.handle(new Request(`${url}${path}`, { headers: auth ? { authorization: `Bearer ${token}` } : {} }));
56
57 beforeAll(async () => {
58 dataDir = await mkdtemp(join(tmpdir(), "forge-approval-base-"));
59 app = createApp({ dataDir, port: 0, host: "forge.test" });
60 http = forgeHttp(app);
61 const mk = async (username: string, first: boolean) => {
62 const made = await createUser(app.db, {
63 origin: "seed", trusted: true, username, email: `${username}@forge.local`,
64 password: "a-good-passphrase", adminIfFirst: first,
65 });
66 if (!made.ok) throw new Error(made.error);
67 return made.value;
68 };
69 alice = await mk("alice", true);
70 bob = await mk("bob", false);
71 const made = await createRepo(app, alice, { name: "lib", visibility: "public" });
72 if (!made.ok) throw new Error(String(made.error));
73 repo = made.value;
74 promote(app.db, bob);
75 app.db.run("UPDATE user SET is_admin = 1 WHERE id = ?1", [bob.id]);
76 bob = { ...bob, isAdmin: true };
77 token = createToken(app.db, alice.id, "t", "rw").token;
78 }, 60_000);
79
80 afterAll(async () => {
81 app.close();
82 await rm(dataDir, { recursive: true, force: true });
83 });
84
85 test("the approval names its head and base, and goes stale when the base moves", async () => {
86 const first = await commitToBranch(app, alice, repo, { message: "one", files: [{ path: "a.md", content: "a\n" }] });
87 if (!first.ok) throw new Error(JSON.stringify(first.error));
88 const side = await commitToBranch(app, alice, repo, { branch: "feat", base: "main", message: "feat", files: [{ path: "f.md", content: "f\n" }] });
89 if (!side.ok) throw new Error(JSON.stringify(side.error));
90 const opened = await openPull(app, alice, repo, { title: "Feat", baseBranch: "main", headBranch: "feat" });
91 if (!opened.ok) throw new Error(String(opened.error));
92 const number = opened.value.number;
93 const path = `/api/v1/repos/alice/lib/pulls/${number}?diff=0`;
94
95 expect((await (await get(path)).json() as { approvedAt: unknown }).approvedAt).toBeNull();
96
97 const pull = getPull(app.db, repo.id, number)!;
98 const approved = await submitReview(app, bob, repo, pull, { state: "approved" });
99 if (!approved.ok) throw new Error(String(approved.error));
100 const body = (await (await get(path)).json()) as { approvedAt: { head: string; base: string; current: boolean; at: string } };
101 expect(body.approvedAt.head).toBe(side.value.commitOid);
102 expect(body.approvedAt.base).toBe(first.value.commitOid);
103 expect(body.approvedAt.current).toBe(true);
104
105 expect(await (await get(`/alice/lib/pulls/${number}/files`)).text()).not.toContain("the base moved since the approval");
106
107 const moved = await commitToBranch(app, alice, repo, { message: "two", files: [{ path: "b.md", content: "b\n" }] });
108 if (!moved.ok) throw new Error(JSON.stringify(moved.error));
109 const after = (await (await get(path)).json()) as { approvedAt: { head: string; base: string; current: boolean } };
110 expect(after.approvedAt.base).toBe(first.value.commitOid);
111 expect(after.approvedAt.current).toBe(false);
112 expect(await (await get(`/alice/lib/pulls/${number}/files`)).text()).toContain("the base moved since the approval");
113 }, 60_000);
114 });
test/mcp-cards.test.ts +6
@@ -80,6 +80,12 @@ describe("explore, pulls and feed cards", () => {
8080 await rm(dataDir, { recursive: true, force: true });
8181 });
8282
83 test("a pull row says when its approval moved", () => {
84 const rows = cardFor("pulls", { pulls: [{ repo: "a/b", number: 1, title: "t", check: "ok", verdict: "approved", approvals: 1, movedSinceApproval: true }, { repo: "a/b", number: 2, title: "t", check: "ok", verdict: "approved", approvals: 1, movedSinceApproval: false }] }, "https://forge.test", {})!.rows;
85 expect(rows[0]!.value).toContain(" · moved since approval");
86 expect(rows[1]!.value).not.toContain("moved since approval");
87 });
88
8389 test("explore draws its card for a stranger: the ranked public app, linked, and not the private one", async () => {
8490 const c = await card("explore", {});
8591 expect(c.kind).toBe("explore");
test/mcp-pulls.test.ts +11
@@ -117,6 +117,7 @@ test("lists open pulls in the brief shape, newest first, limited", async () => {
117117 head: "two",
118118 check: "none",
119119 approvals: 0,
120 movedSinceApproval: false,
120121 changesRequested: 0,
121122 mergeable: true,
122123 });
@@ -178,3 +179,13 @@ test("a sealed pull is seen by a writer and its reporter, not by a plain reader"
178179 expect(await heads("sam")).toContain("sealed");
179180 expect(await heads("rita")).not.toContain("sealed");
180181 });
182
183 test("an approval given for a base that has since moved says so", async () => {
184 const before = (await labPulls("rita")).find((p) => p.head === "reviewed");
185 expect(before.movedSinceApproval).toBe(false);
186 // The approval was given against a base tip that is not the base's tip now.
187 app.db.run("UPDATE review SET base_oid = ?1 WHERE commit_oid != ''", ["0".repeat(40)]);
188 const after = (await labPulls("rita")).find((p) => p.head === "reviewed");
189 expect(after.movedSinceApproval).toBe(true);
190 expect((await labPulls("rita")).find((p) => p.head === "asked").movedSinceApproval).toBe(false);
191 });
test/pull-next-tui.test.ts +19
@@ -6,6 +6,8 @@ import { openPull } from "../src/actions/pulls.ts";
66 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
77 import { createApp, type App } from "../src/app.ts";
88 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
9 import { repoDir } from "../src/git/repos.ts";
10 import { submitReview } from "../src/core/reviews.ts";
911 import { createUser, type User } from "../src/core/users.ts";
1012 import { createForgeApp } from "../src/tui/forge.ts";
1113 import { pullScreen } from "../src/tui/screens/pull.ts";
@@ -99,3 +101,20 @@ test("with reviews off, a green pull's screen says it lands by itself, as the pa
99101 const text = await screenSaying(topic, "next: queue, green, reviews are off: it lands");
100102 if (text.includes("next: reviewer")) throw new Error(text);
101103 });
104
105 test("the terminal's pull screen says when an approval was given for a base that has moved", async () => {
106 const topic = await openTopic("lib-moved", true);
107 const bob = await createUser(app.db, {
108 origin: "seed", trusted: true, username: "bob", email: "[email protected]", password: "a-good-passphrase",
109 });
110 if (!bob.ok) throw new Error(String(bob.error));
111 const head = (await Bun.$`git --git-dir=${repoDir(app.reposRoot, alice.username, topic.repo.name)} rev-parse refs/heads/topic`.text()).trim();
112 const base = (await Bun.$`git --git-dir=${repoDir(app.reposRoot, alice.username, topic.repo.name)} rev-parse refs/heads/main`.text()).trim();
113 const given = submitReview(app.db, { issueId: topic.pull.id, authorId: bob.value.id, state: "approved", commitOid: head, baseOid: base });
114 if (!given.ok) throw new Error(String(given.error));
115 const quiet = await screenSaying(topic, "1 approval");
116 if (quiet.includes("the base moved since the approval")) throw new Error(quiet);
117 const moved = await commitToBranch(app, alice, topic.repo, { branch: "main", message: "more", files: [{ path: "more.txt", content: "m\n" }] });
118 if (!moved.ok) throw new Error(JSON.stringify(moved.error));
119 await screenSaying(topic, "the base moved since the approval");
120 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs