trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

When a fork's work lands, its author gets a thank-you note with what landed and what changed on the way #9166

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9013-when-a-fork-s-work-lands-its-author-gets → main 4 commits fork

  • Merged. scout/9013-when-a-fork-s-work-lands-its-author-gets was merged into main as 392c381.

Review

Sign in to review this change.
6 files changed +673 −40
docs/features/thanks.md added +37
@@ -0,0 +1,37 @@
1 # The thank-you note
2
3 When a pull whose author lives elsewhere lands, its author gets a note on the pull. It thanks them by name, links the
4 landing commit and says what changed between their head and the base on the way (`src/origins/thanks.ts`, called by
5 `mergePull` in `src/actions/pulls.ts` after the landing).
6
7 **Who is thanked** (`whomToThank`), first match wins:
8
9 - a pull brought in from the read-only mirror (`mirror-pulls.ts`) thanks its author by their name there, bare
10 (`tdobbertin-espo`), so it never mentions a user here who happens to share the name;
11 - work from another forge (a `pull_source` row) thanks `account@host`;
12 - a fork a person keeps thanks its owner as `@user`, which notifies them.
13
14 **Who is not thanked:** a pull written by an agent (a crew pull), and a pull made inside the repository.
15
16 **What the note says:**
17
18 - `Thank you, <name>: pull #N landed in <commit>`, with the commit linked when the forge knows its URL;
19 - how many commits by whom touched how many files, and whether it landed as a merge (so `git merge main` on their
20 side stays clean) or was squashed into one commit;
21 - what changed from their head on the way:
22 - the slices that landed before the last one (`Slice-of` merges on the base);
23 - every merge of the base into their branch, which is where conflicts were resolved;
24 - a squash that made several commits into one.
25
26 When none of these happened, the note says it landed as they pushed it.
27
28 **Once per landing.** A pull that already carries a note by the person who landed it, naming the landing commit,
29 gets no second note (`landingThanked` in `src/core/issues.ts`). A comment anyone else posts that looks like the note
30 does not count, so it cannot suppress the real one.
31
32 **The copy on the origin pull.** For a pull brought from the mirror, the same note is posted as a comment on the
33 mirror pull, through the mirror's provider (`thankOnOrigin`). It is sent after the merge answers, so a landing never
34 waits on another host. A failure is logged as a warning and nothing is retried.
35
36 For work from another forge (`pull_source`), the note is written only on the pull here: the forge does not store
37 which pull it came from on the other side, so there is nowhere to post the copy.
src/actions/pulls.ts +9 −25
@@ -65,8 +65,9 @@ import { grantRun } from "../core/run_grant.ts";
6565 import { commitIdentity } from "../core/commit_identity.ts";
6666 import { getUserById, getUserByName, type User } from "../core/users.ts";
6767 import { coAuthorLines, messagesBetween } from "../git/coauthors.ts";
68 import { carriedSummary, resolveOid, type MergeStrategy } from "../git/merge.ts";
69 import { landingKind, thanksNote } from "../core/landing_kind.ts";
68 import { resolveOid, type MergeStrategy } from "../git/merge.ts";
69 import { landingKind } from "../core/landing_kind.ts";
70 import { thankAuthor } from "../origins/thanks.ts";
7071 import { repoDir } from "../git/repos.ts";
7172 import { importIssue, importReviewStates } from "../origins/import.ts";
7273 import { err, ok, type Result } from "../kernel/result.ts";
@@ -501,14 +502,6 @@ export async function mergePull(
501502 ...kept,
502503 ...(from === undefined ? [] : [sourceTrailers(from, ownHost(app.config))]),
503504 ].join("\n");
504 const carriedBefore =
505 strategy === "merge" && pull.crossRepo
506 ? await carriedSummary(
507 baseDir,
508 (await resolveOid(baseDir, `refs/heads/${pull.baseBranch}`)) ?? pull.baseOid,
509 (getPull(app.db, repo.id, pull.number) ?? pull).headOid,
510 )
511 : undefined;
512505 const merged = await landPull(
513506 app.db,
514507 baseDir,
@@ -533,21 +526,12 @@ export async function mergePull(
533526 },
534527 );
535528 if (!merged.ok) return merged;
536 if (carriedBefore !== undefined) {
537 const thanks = thanksNote({ number: pull.number, ...carriedBefore });
538 const noted = addIssueComment(app.db, {
539 issueId: pull.id,
540 authorId: writer.value.id,
541 body: thanks,
542 });
543 if (noted.ok)
544 app.bus.append({
545 kind: "pull.commented",
546 repoId: repo.id,
547 actorId: writer.value.id,
548 payload: { number: pull.number, body: thanks, commentId: noted.value.id },
549 });
550 }
529 // Its author, when they live elsewhere, reads what landed and what changed on the way (#9013).
530 await thankAuthor(app, repo, getPull(app.db, repo.id, pull.number) ?? pull, {
531 mergeOid: merged.value.mergeOid,
532 strategy,
533 by: writer.value.id,
534 });
551535 // What came of the decision to show it, when there was one.
552536 resolveOpen(app.db, pull.id, "land", "shipped");
553537 app.bus.append({
src/core/issues.ts +18
@@ -621,6 +621,24 @@ export function addComment(
621621 return ok(getComment(db, commentId)!);
622622 }
623623
624 /**
625 * A thank-you note (#9013) for this landing already stands on the pull: one
626 * written by the person who landed it, naming the landing commit. Keyed on
627 * its author, so a note anyone else posts cannot stand in for it.
628 */
629 export function landingThanked(
630 db: Database,
631 input: { pullId: string; authorId: string; mergeOid: string },
632 ): boolean {
633 return (
634 db
635 .query<{ n: number }, [string, string, string]>(
636 "SELECT count(*) AS n FROM comment WHERE issue_id = ?1 AND author_id = ?2 AND body LIKE 'Thank you, %' AND instr(body, ?3) > 0",
637 )
638 .get(input.pullId, input.authorId, input.mergeOid.slice(0, 12))!.n > 0
639 );
640 }
641
624642 function getComment(db: Database, commentId: string): Comment | undefined {
625643 const row = db
626644 .query<
src/core/landing_kind.ts −15
@@ -32,18 +32,3 @@ export function landingKind(
3232 ): MergeStrategy {
3333 return fromKeptFork(db, pull) ? "merge" : asked;
3434 }
35
36 /** The note the author reads when their work landed: what came across and under which names. */
37 export function thanksNote(landed: {
38 readonly number: number;
39 readonly commits: number;
40 readonly authors: readonly string[];
41 readonly files: number;
42 }): string {
43 const who = landed.authors.length === 0 ? "you" : landed.authors.join(", ");
44 return [
45 `Thank you: pull #${landed.number} landed as a merge, so your commits are ancestors of main and \`git merge main\` on your side stays clean.`,
46 `${landed.commits} commit${landed.commits === 1 ? "" : "s"} by ${who} touched ${landed.files} file${landed.files === 1 ? "" : "s"}.`,
47 ]
48 .join("\n");
49 }
src/origins/thanks.ts added +233
@@ -0,0 +1,233 @@
1 /**
2 * The thank-you note (#9013): when a pull whose author lives elsewhere lands
3 * — a fork a person keeps, a pull brought from the mirror, work from another
4 * forge — its author reads, on the pull and, where the origin takes a
5 * comment, on the origin pull, that it landed, in which commit, and what
6 * changed between their head and main on the way: slices landed first,
7 * merges of the base into their branch (conflicts resolved), a squash.
8 *
9 * Never for crew pulls, and once per landing: the note names the landing
10 * commit, and a pull that already carries a note naming it gets no second.
11 */
12
13 import type { App } from "../app.ts";
14 import { addComment, getIssueById, landingThanked } from "../core/issues.ts";
15 import { fromKeptFork } from "../core/landing_kind.ts";
16 import { getMirrorTarget } from "../core/migrate_mirror.ts";
17 import { getOriginById } from "../core/origins.ts";
18 import type { Pull } from "../core/pulls.ts";
19 import type { Repo } from "../core/repos.ts";
20 import { pullSourceOf } from "../core/trusted_sources.ts";
21 import { getUserById } from "../core/users.ts";
22 import { runGit } from "../git/env.ts";
23 import { isSafeOid } from "../git/guard.ts";
24 import { carriedSummary, type MergeStrategy } from "../git/merge.ts";
25 import { repoDir } from "../git/repos.ts";
26 import { landedSlices } from "../git/slices.ts";
27 import { createLog } from "../kernel/log.ts";
28 import { providerForOrigin } from "./credential.ts";
29 import type { Provider } from "./types.ts";
30
31 const log = createLog("thanks");
32
33 /** Who is thanked, and the origin pull to tell as well, when there is one. */
34 export interface Thanked {
35 /** As the note writes it: `@user` here, a bare name or `account@host` elsewhere. */
36 readonly name: string;
37 readonly originUrl: string | null;
38 }
39
40 /** A merge of the base into the author's branch, on the way to landing. */
41 export interface Resolution {
42 readonly oid: string;
43 readonly subject: string;
44 }
45
46 export interface Landed {
47 readonly name: string;
48 readonly number: number;
49 readonly mergeOid: string;
50 /** The landing commit's page, when the forge knows its own URL. */
51 readonly commitUrl: string | null;
52 readonly strategy: MergeStrategy;
53 readonly baseBranch: string;
54 readonly commits: number;
55 readonly authors: readonly string[];
56 readonly files: number;
57 /** The slices that landed before this one, oldest first: their merge commits. */
58 readonly slices: readonly string[];
59 readonly resolved: readonly Resolution[];
60 }
61
62 /** A name as it may stand in a note: no markdown, no line breaks, bounded. */
63 const cleanName = (s: string): string => s.replace(/[^A-Za-z0-9._@-]/g, "").slice(0, 100);
64
65 /** A commit author's name as it may stand in a note: letters, spaces and marks, no markdown. */
66 const cleanAuthor = (s: string): string =>
67 s.replace(/[^\p{L}\p{N} .'@-]/gu, "").replace(/\s+/g, " ").trim().slice(0, 100);
68
69 /** A commit subject as it may stand in a code span. */
70 const cleanSubject = (s: string): string =>
71 s.replace(/[\u0000-\u001f\u007f`]+/g, " ").trim().slice(0, 80);
72
73 const plural = (n: number, one: string): string => `${n} ${one}${n === 1 ? "" : "s"}`;
74
75 /** Whom a landed pull thanks, or nobody: crew pulls and pulls made here are not thanked. */
76 export function whomToThank(app: App, pull: Pull): Thanked | undefined {
77 const author = getUserById(app.db, pull.authorId);
78 if (author === undefined || author.kind === "agent") return undefined;
79 const issue = getIssueById(app.db, pull.id);
80 const fromMirror = issue?.originAuthor ?? null;
81 if (fromMirror !== null && cleanName(fromMirror) !== "")
82 return { name: cleanName(fromMirror), originUrl: issue?.originUrl ?? null };
83 const source = pullSourceOf(app.db, pull.id);
84 if (source !== undefined) {
85 const name = cleanName(`${source.account}@${source.host}`);
86 if (name !== "") return { name, originUrl: null };
87 }
88 if (fromKeptFork(app.db, pull)) return { name: `@${cleanName(author.username)}`, originUrl: null };
89 return undefined;
90 }
91
92 /** The note itself: plain data in, markdown out. */
93 export function thanksNote(l: Landed): string {
94 const short = l.mergeOid.slice(0, 12);
95 const commit = l.commitUrl === null ? `\`${short}\`` : `[\`${short}\`](${l.commitUrl})`;
96 const who = l.authors.length === 0 ? "you" : l.authors.map(cleanAuthor).filter(Boolean).join(", ") || "you";
97 const how =
98 l.strategy === "merge"
99 ? `as a merge, so your commits are ancestors of ${l.baseBranch} and \`git merge ${l.baseBranch}\` on your side stays clean`
100 : `squashed into one commit on ${l.baseBranch}`;
101 const lines = [
102 `Thank you, ${l.name}: pull #${l.number} landed in ${commit}.`,
103 "",
104 `${plural(l.commits, "commit")} by ${who} touched ${plural(l.files, "file")}, landed ${how}.`,
105 ];
106 const changed: string[] = [];
107 if (l.slices.length > 0)
108 changed.push(
109 `- landed in ${plural(l.slices.length + 1, "slice")}: ${[...l.slices.map((s) => `\`${s.slice(0, 12)}\``), commit].join(", ")}`,
110 );
111 if (l.resolved.length > 0)
112 changed.push(
113 `- conflicts resolved: ${plural(l.resolved.length, "merge")} of ${l.baseBranch} into your branch (${l.resolved
114 .map((r) => `\`${r.oid.slice(0, 12)}\` ${cleanSubject(r.subject)}`.trim())
115 .join("; ")})`,
116 );
117 if (l.strategy === "squash" && l.commits > 1) changed.push(`- your ${plural(l.commits, "commit")} became one`);
118 lines.push("");
119 if (changed.length === 0) lines.push("Nothing changed on the way: it landed as you pushed it.");
120 else lines.push("What changed from your head on the way:", ...changed);
121 return lines.join("\n");
122 }
123
124 /** Merges of the base into the pull's range: where its conflicts were resolved. */
125 async function resolutions(dir: string, baseOid: string, headOid: string): Promise<Resolution[]> {
126 if (!isSafeOid(baseOid) || !isSafeOid(headOid)) return [];
127 const out = await runGit(["log", "--merges", "--max-count=20", "--format=%H %s", `${baseOid}..${headOid}`], {
128 cwd: dir,
129 });
130 if (!out.ok) return [];
131 return out.value
132 .split("\n")
133 .filter(Boolean)
134 .map((line) => ({ oid: line.slice(0, 40), subject: line.slice(41) }))
135 .reverse();
136 }
137
138 /** The commit's first parent: the base as it stood before the landing. */
139 async function firstParent(dir: string, oid: string): Promise<string | undefined> {
140 if (!isSafeOid(oid)) return undefined;
141 const out = await runGit(["rev-parse", `${oid}^1`], { cwd: dir });
142 const p = out.ok ? out.value.trim() : "";
143 return isSafeOid(p) ? p : undefined;
144 }
145
146 function commitUrl(app: App, repo: Repo, oid: string): string | null {
147 const base = app.config.url;
148 if (base === undefined || !/^https?:\/\//i.test(base)) return null;
149 return `${base.replace(/\/+$/, "")}/${encodeURIComponent(repo.ownerName)}/${encodeURIComponent(repo.name)}/commit/${oid}`;
150 }
151
152 /** The origin pull's number, read from its link back. */
153 const originNumber = (url: string): number | undefined => {
154 const m = /\/(\d{1,9})\/?$/.exec(URL.parse(url)?.pathname ?? "");
155 return m === null ? undefined : Number(m[1]);
156 };
157
158 /** Tell the origin pull, through the mirror this repository pushes to. Never throws. */
159 export async function thankOnOrigin(
160 app: App,
161 repo: Repo,
162 originUrl: string,
163 body: string,
164 provider?: Provider,
165 ): Promise<boolean> {
166 const number = originNumber(originUrl);
167 const target = getMirrorTarget(app.db, repo.id);
168 if (number === undefined || target === undefined) return false;
169 let p = provider;
170 if (p === undefined) {
171 const origin = getOriginById(app.db, target.originId);
172 if (origin === undefined) return false;
173 const made = providerForOrigin(app, origin);
174 if (!made.ok) return false;
175 p = made.value;
176 }
177 try {
178 const said = await p.createComment(target.fullName, number, body);
179 if (!said.ok) log.warn("thanks not posted on the origin", { repo: repo.id, number, error: said.error.message });
180 return said.ok;
181 } catch (error) {
182 log.warn("thanks not posted on the origin", { repo: repo.id, number, error: String(error) });
183 return false;
184 }
185 }
186
187 /**
188 * Thank a landed pull's author, once: the note on the pull, written before
189 * this returns, and nothing when it is not theirs to get. The origin's copy
190 * is sent without waiting, so a landing never waits on another host; it
191 * never throws, a failure is logged (`thankOnOrigin`).
192 */
193 export async function thankAuthor(
194 app: App,
195 repo: Repo,
196 pull: Pull,
197 landing: { readonly mergeOid: string; readonly strategy: MergeStrategy; readonly by: string },
198 ): Promise<string | undefined> {
199 const who = whomToThank(app, pull);
200 if (who === undefined || !isSafeOid(landing.mergeOid)) return undefined;
201 if (landingThanked(app.db, { pullId: pull.id, authorId: landing.by, mergeOid: landing.mergeOid })) return undefined;
202 const dir = repoDir(app.reposRoot, repo.ownerName, repo.name);
203 const before = (await firstParent(dir, landing.mergeOid)) ?? pull.baseOid;
204 const head = pull.headOid;
205 const [carried, slices, resolved] = await Promise.all([
206 carriedSummary(dir, before, head),
207 landedSlices(dir, landing.mergeOid, pull.number, pull.createdAt),
208 resolutions(dir, before, head),
209 ]);
210 // Another landing of the same pull raced this one while git was read.
211 if (landingThanked(app.db, { pullId: pull.id, authorId: landing.by, mergeOid: landing.mergeOid })) return undefined;
212 const note = thanksNote({
213 name: who.name,
214 number: pull.number,
215 mergeOid: landing.mergeOid,
216 commitUrl: commitUrl(app, repo, landing.mergeOid),
217 strategy: landing.strategy,
218 baseBranch: pull.baseBranch,
219 ...carried,
220 slices: slices.map((s) => s.mergeOid),
221 resolved,
222 });
223 const noted = addComment(app.db, { issueId: pull.id, authorId: landing.by, body: note });
224 if (!noted.ok) return undefined;
225 app.bus.append({
226 kind: "pull.commented",
227 repoId: repo.id,
228 actorId: landing.by,
229 payload: { number: pull.number, body: note, commentId: noted.value.id },
230 });
231 if (who.originUrl !== null) void thankOnOrigin(app, repo, who.originUrl, note);
232 return note;
233 }
test/thanks.test.ts added +376
@@ -0,0 +1,376 @@
1 /**
2 * The thank-you note (#9013): a pull brought from the mirror lands, and its
3 * author is thanked by name on the pull and on the mirror pull, with the
4 * landing commit linked and what changed on the way; once per landing,
5 * never for a crew pull or one made here.
6 */
7
8 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
9 import { mkdtemp, rm } from "node:fs/promises";
10 import { tmpdir } from "node:os";
11 import { join } from "node:path";
12 import { landSlice } from "../src/actions/pull_slices.ts";
13 import { mergePull, openPull } from "../src/actions/pulls.ts";
14 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
15 import { createApp, type App } from "../src/app.ts";
16 import { setMirrorTarget } from "../src/core/migrate_mirror.ts";
17 import { createOrigin } from "../src/core/origins.ts";
18 import { getPull } from "../src/core/pulls.ts";
19 import type { Repo } from "../src/core/repos.ts";
20 import { getRepo } from "../src/core/repos.ts";
21 import { recordPullSource } from "../src/core/trusted_sources.ts";
22 import { createUser, getUserByName, type User } from "../src/core/users.ts";
23 import { serve, type Servers } from "../src/http/server.ts";
24 import { gitClientEnv } from "./helpers/git-client.ts";
25 import { promote } from "./helpers/members.ts";
26 import { tokenFor } from "./helpers/tokens.ts";
27 import { proveAll } from "./helpers/verified.ts";
28 import { resolveOid } from "../src/git/merge.ts";
29 import { repoDir } from "../src/git/repos.ts";
30 import { approveGuest } from "../src/origins/guest.ts";
31 import { bringMirrorPulls, type FetchHead } from "../src/origins/mirror-pulls.ts";
32 import { ForgejoProvider } from "../src/origins/providers/forgejo.ts";
33 import { thankAuthor, thanksNote, whomToThank } from "../src/origins/thanks.ts";
34 import { staticCredential } from "../src/origins/types.ts";
35 import { newFakeState, serveFakeForge, type FakeForge, type FakeState } from "./fake-origin.ts";
36 import { quietLogs } from "./helpers/globals.ts";
37
38 quietLogs();
39
40 const TOKEN = "tok-thanks-1234";
41 const OID = "a".repeat(40);
42
43 describe("the note's words", () => {
44 test("names the author, links the landing commit and lists what changed", () => {
45 const note = thanksNote({
46 name: "[email protected]",
47 number: 7,
48 mergeOid: OID,
49 commitUrl: `https://forge.test/alice/app/commit/${OID}`,
50 strategy: "merge",
51 baseBranch: "main",
52 commits: 3,
53 authors: ["Jay Moran"],
54 files: 2,
55 slices: ["b".repeat(40)],
56 resolved: [{ oid: "c".repeat(40), subject: "Merge `main` into fix" }],
57 });
58 expect(note).toStartWith("Thank you, [email protected]: pull #7 landed in");
59 expect(note).toContain(`[\`aaaaaaaaaaaa\`](https://forge.test/alice/app/commit/${OID})`);
60 expect(note).toContain("3 commits by Jay Moran touched 2 files");
61 expect(note).toContain("landed in 2 slices: `bbbbbbbbbbbb`");
62 expect(note).toContain("conflicts resolved: 1 merge of main into your branch (`cccccccccccc` Merge main into fix)");
63 });
64
65 test("a squash that changed nothing else says so", () => {
66 const note = thanksNote({
67 name: "@bob",
68 number: 2,
69 mergeOid: OID,
70 commitUrl: null,
71 strategy: "squash",
72 baseBranch: "main",
73 commits: 1,
74 authors: [],
75 files: 1,
76 slices: [],
77 resolved: [],
78 });
79 expect(note).toContain("landed in `aaaaaaaaaaaa`.");
80 expect(note).toContain("Nothing changed on the way");
81 });
82 });
83
84 describe("a landed mirror pull thanks its author once", () => {
85 let dataDir: string;
86 let app: App;
87 let alice: User;
88 let repo: Repo;
89 let fake: FakeForge;
90 let state: FakeState;
91
92 const fetchHead: FetchHead = async (a, r, _t, _p, number, ref) => {
93 const made = await commitToBranch(a, alice, r, {
94 branch: ref.replace(/^refs\/heads\//, ""),
95 message: `Mirror pull ${number}`,
96 files: [{ path: `m${number}.md`, content: "from the mirror\n" }],
97 });
98 return made.ok ? resolveOid(repoDir(a.reposRoot, r.ownerName, r.name), ref) : undefined;
99 };
100 const provider = () =>
101 new ForgejoProvider(staticCredential({ kind: "forgejo", apiUrl: fake.apiUrl, webUrl: fake.url, token: TOKEN }));
102 const notesOn = (number: number): string[] =>
103 app.db
104 .query<{ body: string }, [string, number]>(
105 "SELECT body FROM comment WHERE issue_id = (SELECT id FROM issue WHERE repo_id = ?1 AND number = ?2) AND body LIKE 'Thank you, %'",
106 )
107 .all(repo.id, number)
108 .map((r) => r.body);
109
110 beforeAll(async () => {
111 dataDir = await mkdtemp(join(tmpdir(), "forge-thanks-"));
112 app = createApp({ dataDir, port: 0, url: "https://forge.test" });
113 state = newFakeState("forgejo", "trevato", TOKEN);
114 state.repos.push({ id: 1, fullName: "trevato-apps/forge", cloneUrl: "http://127.0.0.1/none.git", admin: true });
115 fake = serveFakeForge(state);
116 const made = await createUser(app.db, {
117 origin: "seed",
118 trusted: true,
119 username: "alice",
120 email: "[email protected]",
121 password: "a-good-passphrase",
122 adminIfFirst: true,
123 });
124 if (!made.ok) throw new Error(made.error);
125 alice = made.value;
126 const r = await createRepo(app, alice, { name: "forge", visibility: "public" });
127 if (!r.ok) throw new Error(String(r.error));
128 repo = r.value;
129 const seeded = await commitToBranch(app, alice, repo, {
130 message: "Start",
131 files: [{ path: "README.md", content: "forge\n" }],
132 });
133 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
134 const origin = createOrigin(app.db, dataDir, {
135 userId: alice.id,
136 kind: "forgejo",
137 apiUrl: fake.apiUrl,
138 webUrl: fake.url,
139 login: "trevato",
140 token: TOKEN,
141 });
142 if (!origin.ok) throw new Error(origin.error);
143 setMirrorTarget(app.db, {
144 repoId: repo.id,
145 originId: origin.value.id,
146 fullName: "trevato-apps/forge",
147 cloneUrl: `${fake.url}/trevato-apps/forge.git`,
148 tips: {},
149 });
150 }, 60_000);
151
152 afterAll(async () => {
153 fake.stop();
154 app.close();
155 await rm(dataDir, { recursive: true, force: true });
156 });
157
158 test("on the pull and on the mirror pull, naming the author and the commit; never twice", async () => {
159 state.issues.set("trevato-apps/forge", [
160 {
161 id: 501,
162 number: 1,
163 title: "Fix 1",
164 body: "It was broken.",
165 user: "tdobbertin-espo",
166 pull: { base: "main", head: "fix-1", headRepo: "tdobbertin-espo/forge", headSha: "1".padStart(40, "a") },
167 },
168 ]);
169 const report = await bringMirrorPulls(app, repo, { provider: provider(), fetchHead });
170 const pull = getPull(app.db, repo.id, report.opened[0]!)!;
171 expect(approveGuest(app.db, repo, pull, alice).ok).toBe(true);
172 const merged = await mergePull(app, alice, repo, pull, "merge");
173 if (!merged.ok) throw new Error(String(merged.error));
174 const oid = merged.value.mergeOid;
175
176 const here = notesOn(pull.number);
177 expect(here).toHaveLength(1);
178 expect(here[0]).toStartWith(`Thank you, tdobbertin-espo: pull #${pull.number} landed in`);
179 expect(here[0]).toContain(`https://forge.test/alice/forge/commit/${oid}`);
180 expect(here[0]).toContain("1 commit by alice touched 1 file");
181
182 // The mirror's copy goes out after the merge answers.
183 const onMirror = () => (state.comments.get("trevato-apps/forge") ?? []).filter((c) => c.body.startsWith("Thank you"));
184 for (let i = 0; i < 50 && onMirror().length === 0; i++) await Bun.sleep(20);
185 expect(onMirror()).toHaveLength(1);
186 expect(onMirror()[0]!.issueNumber).toBe(1);
187 expect(onMirror()[0]!.body).toBe(here[0]!);
188
189 // A second call for the same landing writes nothing.
190 const again = await thankAuthor(app, repo, getPull(app.db, repo.id, pull.number)!, {
191 mergeOid: oid,
192 strategy: "merge",
193 by: alice.id,
194 });
195 expect(again).toBeUndefined();
196 expect(notesOn(pull.number)).toHaveLength(1);
197 expect(onMirror()).toHaveLength(1);
198 });
199
200 test("a pull made here, or by the crew, is not thanked", async () => {
201 const branch = await commitToBranch(app, alice, repo, {
202 branch: "topic",
203 message: "inside",
204 files: [{ path: "in.md", content: "in\n" }],
205 });
206 if (!branch.ok) throw new Error(JSON.stringify(branch.error));
207 const opened = await openPull(app, alice, repo, { title: "inside", body: "", baseBranch: "main", headBranch: "topic" });
208 if (!opened.ok) throw new Error(String(opened.error));
209 expect(whomToThank(app, opened.value)).toBeUndefined();
210 const merged = await mergePull(app, alice, repo, opened.value, "squash");
211 expect(merged.ok).toBe(true);
212 expect(notesOn(opened.value.number)).toHaveLength(0);
213
214 // A crew pull brought from the mirror still has an agent for an author here.
215 const mirrored = getPull(app.db, repo.id, 1)!;
216 app.db.query("UPDATE user SET is_agent = 1 WHERE id = ?1").run(mirrored.authorId);
217 expect(whomToThank(app, mirrored)).toBeUndefined();
218 app.db.query("UPDATE user SET is_agent = 0 WHERE id = ?1").run(mirrored.authorId);
219 });
220
221 test("a note someone else posts naming the landing does not stand in for the real one", async () => {
222 const mallory = await createUser(app.db, {
223 origin: "seed",
224 trusted: true,
225 username: "mallory",
226 email: "[email protected]",
227 password: "a-good-passphrase",
228 });
229 if (!mallory.ok) throw new Error(mallory.error);
230 const pull = getPull(app.db, repo.id, 1)!;
231 const oid = (await resolveOid(repoDir(app.reposRoot, repo.ownerName, repo.name), "refs/heads/main"))!;
232 app.db.query("DELETE FROM comment WHERE issue_id = ?1 AND body LIKE 'Thank you, %'").run(pull.id);
233 app.db
234 .query("INSERT INTO comment (id, issue_id, author_id, body) VALUES ('forged', ?1, ?2, ?3)")
235 .run(pull.id, mallory.value.id, `Thank you, nobody: landed in ${oid.slice(0, 12)}`);
236 const note = await thankAuthor(app, repo, pull, { mergeOid: oid, strategy: "merge", by: alice.id });
237 expect(note).toStartWith("Thank you, tdobbertin-espo");
238 });
239
240 test("work from another forge is thanked as account@host", async () => {
241 const branch = await commitToBranch(app, alice, repo, {
242 branch: "from-afar",
243 message: "afar",
244 files: [{ path: "afar.md", content: "afar\n" }],
245 });
246 if (!branch.ok) throw new Error(JSON.stringify(branch.error));
247 const opened = await openPull(app, alice, repo, { title: "afar", body: "", baseBranch: "main", headBranch: "from-afar" });
248 if (!opened.ok) throw new Error(String(opened.error));
249 recordPullSource(app.db, { pullId: opened.value.id, host: "other.forge", account: "jay", via: [] });
250 expect(whomToThank(app, opened.value)).toEqual({ name: "[email protected]", originUrl: null });
251 });
252
253 });
254
255 const gitEnv = gitClientEnv({
256 GIT_AUTHOR_NAME: "Bob Kept",
257 GIT_AUTHOR_EMAIL: "[email protected]",
258 GIT_COMMITTER_NAME: "Bob Kept",
259 GIT_COMMITTER_EMAIL: "[email protected]",
260 });
261
262 describe("a kept fork's note lists what changed on the way", () => {
263 let dataDir: string;
264 let work: string;
265 let app: App;
266 let servers: Servers;
267 let base: string;
268 let alice = "";
269 let bob = "";
270
271 async function git(args: string[], cwd: string): Promise<string> {
272 const p = Bun.spawn(["git", ...args], { cwd, env: gitEnv, stdout: "pipe", stderr: "pipe" });
273 const [out, e, code] = await Promise.all([new Response(p.stdout).text(), new Response(p.stderr).text(), p.exited]);
274 if (code !== 0) throw new Error(`git ${args.join(" ")}: ${e}`);
275 return out.trim();
276 }
277 const call = (path: string, token: string, body?: unknown): Promise<Response> =>
278 fetch(`${base}${path}`, {
279 method: body === undefined ? "GET" : "POST",
280 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
281 ...(body === undefined ? {} : { body: JSON.stringify(body) }),
282 });
283 const authed = (user: string, token: string) => base.replace("http://", `http://${user}:${token}@`);
284 async function signup(username: string): Promise<string> {
285 await fetch(`${base}/signup`, {
286 method: "POST",
287 body: new URLSearchParams({ username, email: `${username}@f.dev`, password: "thanks-pass-123" }),
288 redirect: "manual",
289 });
290 return tokenFor(app, username, "admin", "k");
291 }
292 async function commit(dir: string, file: string): Promise<string> {
293 await Bun.write(join(dir, file), `${file}\n`);
294 await git(["add", "."], dir);
295 await git(["commit", "-m", `add ${file}`], dir);
296 return git(["rev-parse", "HEAD"], dir);
297 }
298 async function open(head: string): Promise<number> {
299 const r = await call("/api/v1/repos/alice/app/pulls", bob, { title: head, base: "main", head, headRepo: "bob/app" });
300 expect(r.status).toBe(201);
301 return ((await r.json()) as { number: number }).number;
302 }
303 const notesOn = (number: number): string[] =>
304 app.db
305 .query<{ body: string }, [number]>(
306 "SELECT body FROM comment WHERE issue_id = (SELECT i.id FROM issue i JOIN repo r ON r.id = i.repo_id WHERE r.name = 'app' AND i.number = ?1 AND i.kind = 'pull' AND r.owner_id = (SELECT id FROM user WHERE username = 'alice')) AND body LIKE 'Thank you, %'",
307 )
308 .all(number)
309 .map((r) => r.body);
310
311 beforeAll(async () => {
312 dataDir = await mkdtemp(join(tmpdir(), "forge-thanks-kf-"));
313 work = await mkdtemp(join(tmpdir(), "forge-thanks-kf-work-"));
314 app = createApp({ dataDir, port: 0 });
315 servers = await serve(app);
316 base = servers.url;
317 alice = await signup("alice");
318 bob = await signup("bob");
319 proveAll(app);
320 promote(app.db);
321 await call("/api/v1/repos", alice, { name: "app", visibility: "public" });
322 await git(["clone", `${authed("alice", alice)}/alice/app.git`, "a"], work);
323 await commit(join(work, "a"), "seed.txt");
324 await git(["push", "origin", "main"], join(work, "a"));
325 await call("/api/v1/repos/alice/app/forks", bob, {});
326 await git(["clone", `${authed("bob", bob)}/bob/app.git`, "b"], work);
327 }, 60_000);
328
329 afterAll(async () => {
330 servers.stop();
331 app.close();
332 await rm(dataDir, { recursive: true, force: true });
333 await rm(work, { recursive: true, force: true });
334 });
335
336 test("a branch that merged main in: thanked as @bob, the merge named as conflicts resolved", async () => {
337 const a = join(work, "a");
338 const b = join(work, "b");
339 await git(["checkout", "-b", "fix"], b);
340 await commit(b, "fix.txt");
341 await commit(a, "moved.txt");
342 await git(["push", "origin", "main"], a);
343 await git(["fetch", `${authed("alice", alice)}/alice/app.git`, "main"], b);
344 await git(["merge", "--no-edit", "-m", "Merge main into fix", "FETCH_HEAD"], b);
345 const resolved = await git(["rev-parse", "HEAD"], b);
346 await git(["push", "origin", "fix"], b);
347 const number = await open("fix");
348 const merged = await call(`/api/v1/repos/alice/app/pulls/${number}/merge`, alice, { strategy: "merge" });
349 expect(merged.status).toBe(200);
350 const notes = notesOn(number);
351 expect(notes).toHaveLength(1);
352 expect(notes[0]).toStartWith(`Thank you, @bob: pull #${number} landed in`);
353 expect(notes[0]).toContain(`conflicts resolved: 1 merge of main into your branch (\`${resolved.slice(0, 12)}\` Merge main into fix)`);
354 });
355
356 test("a pull landed in slices: the note lists the slice before the last", async () => {
357 const b = join(work, "b");
358 await git(["fetch", `${authed("alice", alice)}/alice/app.git`, "main"], b);
359 await git(["checkout", "-b", "long", "FETCH_HEAD"], b);
360 const first = await commit(b, "one.txt");
361 await commit(b, "two.txt");
362 await git(["push", "origin", "long"], b);
363 const number = await open("long");
364 const repo = getRepo(app.db, "alice", "app")!;
365 const lander = getUserByName(app.db, "alice")!;
366 const slice = await landSlice(app, lander, repo, getPull(app.db, repo.id, number)!, first);
367 if (!slice.ok) throw new Error(String(slice.error));
368 expect(slice.value.last).toBe(false);
369 const merged = await mergePull(app, lander, repo, getPull(app.db, repo.id, number)!, "merge");
370 if (!merged.ok) throw new Error(String(merged.error));
371 const notes = notesOn(number);
372 expect(notes).toHaveLength(1);
373 expect(notes[0]).toContain(`landed in 2 slices: \`${slice.value.mergeOid.slice(0, 12)}\`, \`${merged.value.mergeOid.slice(0, 12)}\``);
374 expect(notes[0]).not.toContain("conflicts resolved");
375 });
376 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs