trevato/forge

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

More
Forks1g fMapg aRunsg rEventsFlows

A fork that is ahead of its parent shows "Offer N commits upstream" on its home, opening the cross-repository pull form already filled in #5971

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/5964-a-fork-that-is-ahead-of-its-parent-shows → main 3 commits fork 3 threads · 3 unresolved

  • Merged. scout/5964-a-fork-that-is-ahead-of-its-parent-shows was merged into main as 75df857.

Review

Sign in to review this change.
7 files changed +341 −3
README.md +2
@@ -288,6 +288,8 @@ each, under the forge strip; a list cut to three says "+N more".
288288 The line is the shared `.quiet-details` disclosure, and the "need you" count
289289 carries the attention dot.
290290
291 A fork ahead of its parent offers its commits upstream: on its home a signed-in reader of the parent sees "Offer N commits upstream" (into the new-pull form with both branches chosen) or "Offered in #M" once a pull is open; a writer who has the Following card gets its "Propose upstream" instead, one route not two. In the terminal the fork's About card (`I`) has an `offer` row and `o` opens the same pull form (or the open pull). Both read `upstreamOffer` (`src/actions/offer_upstream.ts`), the open pull by one SQL lookup.
292
291293 ## Live in the terminal
292294
293295 `ssh` into the forge and you get the whole thing — repositories, code, issues,
src/actions/offer_upstream.ts added +39
@@ -0,0 +1,39 @@
1 /**
2 * A fork's commits, offered to its parent: plain data for every door. The
3 * page draws a button from it and the terminal a key; both are gated the way
4 * `openPull` gates a pull — a signed-in viewer who may read the parent — and
5 * both find the open pull in the database, never in git.
6 */
7
8 import type { App } from "../app.ts";
9 import { openPullFromHead } from "../core/pulls.ts";
10 import { canRead, type Repo } from "../core/repos.ts";
11 import type { User } from "../core/users.ts";
12
13 export interface UpstreamOffer {
14 readonly ahead: number;
15 /** The open pull already offering this branch, when there is one. */
16 readonly openNumber: number | null;
17 readonly head: string;
18 readonly base: string;
19 }
20
21 /** `ahead` is the drift the caller has read; null or zero offers nothing. */
22 export function upstreamOffer(
23 app: App,
24 viewer: User | undefined,
25 fork: Repo,
26 parent: Repo | undefined,
27 ahead: number | null | undefined,
28 ): UpstreamOffer | null {
29 if (parent === undefined || viewer === undefined) return null;
30 if (ahead === null || ahead === undefined || ahead <= 0) return null;
31 if (!canRead(app.db, parent, viewer)) return null;
32 const open = openPullFromHead(app.db, parent.id, fork.id, fork.defaultBranch);
33 return {
34 ahead,
35 openNumber: open?.number ?? null,
36 head: fork.defaultBranch,
37 base: parent.defaultBranch,
38 };
39 }
src/core/pulls.ts +21
@@ -430,6 +430,27 @@ export function listOpenPullsFromHead(
430430 return joinMany(db, [...getIssuesByIds(db, ids).values()]);
431431 }
432432
433 /**
434 * The one open pull into `baseRepoId` whose head is this branch of this
435 * repository, asked of the database rather than found in a page of pulls.
436 */
437 export function openPullFromHead(
438 db: Database,
439 baseRepoId: string,
440 headRepoId: string,
441 branch: string,
442 ): Pull | undefined {
443 const row = db
444 .query<{ number: number }, [string, string, string]>(
445 `SELECT issue.number FROM pull JOIN issue ON issue.id = pull.issue_id
446 WHERE issue.repo_id = ?1 AND pull.head_repo_id = ?2 AND pull.head_branch = ?3
447 AND issue.state = 'open'
448 ORDER BY issue.number DESC LIMIT 1`,
449 )
450 .get(baseRepoId, headRepoId, branch);
451 return row === null || row === undefined ? undefined : getPull(db, baseRepoId, row.number);
452 }
453
433454 /**
434455 * The newest open pull of each repository, in one query — what a shelf row
435456 * shows when nothing about the app is moving otherwise: a change waits for
src/tui/screens/about.ts +54 −1
@@ -21,6 +21,8 @@ import { parentReleases, releasesSentence } from "../../core/parent_releases.ts"
2121 import { running } from "../../kernel/version.ts";
2222 import { checkMemoryAt } from "../../apps/check-memory.ts";
2323 import { repoDir } from "../../git/repos.ts";
24 import { forkDrift } from "../../actions/forks.ts";
25 import { upstreamOffer, type UpstreamOffer } from "../../actions/offer_upstream.ts";
2426 import { listRefs } from "../../git/read.ts";
2527 import { updateRepoSettings } from "../../actions/repos.ts";
2628 import { showProvenance } from "../../actions/provenance.ts";
@@ -54,7 +56,9 @@ import { badge, card } from "../ui/widgets.ts";
5456 import type { Session } from "../session.ts";
5557 import { PIN_MARK, ago, drawFacts, plural } from "./common.ts";
5658 import { connectionsScreen, egressScreen } from "./egress.ts";
57 import { forkOf, hostOf, liveOrigin, localRepo, sourceName } from "./where.ts";
59 import { newPullScreen } from "./newpull.ts";
60 import { pullScreen } from "./pull.ts";
61 import { forkOf, hostOf, liveOrigin, local, localRepo, sourceName } from "./where.ts";
5862
5963 /**
6064 * `I` — everything a repository is, in one card: what it is for, whether it
@@ -75,6 +79,7 @@ export function aboutScreen(
7579 const may = (): boolean =>
7680 repo !== undefined && f.user !== undefined && canAdmin(f.db, repo, f.user);
7781 let cell: [string, string][] = [];
82 let offered: UpstreamOffer | null = null;
7883 // The trail's switch (#3293) is a person's, an admin's, and only where
7984 // there is a trail to switch: the same rule as the settings page.
8085 const mayTrail = (): boolean =>
@@ -92,9 +97,25 @@ export function aboutScreen(
9297 ...aboutFacts(f, src, facts, ref),
9398 ...trailRows(f, repo),
9499 ...upstreamRows(f, repo),
100 ...offerRows(offered),
95101 ...cell,
96102 ];
97103 let rows = factRows();
104 // A fork ahead of its parent offers its commits upstream: the page's
105 // button, read the same way (`upstreamOffer`), pressed here with `o`.
106 const forkParent =
107 repo === undefined ? undefined : forkParentFor(f.db, repo, f.user);
108 if (repo !== undefined && forkParent !== undefined) {
109 const at = repo;
110 void forkDrift(f.app, at, forkParent)
111 .then((drift) => {
112 offered = upstreamOffer(f.app, f.user, at, forkParent, drift.ahead);
113 if (offered === null) return;
114 rows = factRows();
115 ctx.invalidate();
116 })
117 .catch(() => {});
118 }
98119 // An admin also sees the check cell's memory, read once from forge.json.
99120 if (may() && repo !== undefined) {
100121 const at = repo;
@@ -151,6 +172,16 @@ export function aboutScreen(
151172 ...(f.user?.isAdmin === true && repo !== undefined
152173 ? [binding(["e"], "egress"), binding(["c"], "connections")]
153174 : []),
175 ...(offered === null
176 ? []
177 : [
178 binding(
179 ["o"],
180 offered.openNumber === null
181 ? `offer ${plural(offered.ahead, "commit")} upstream`
182 : `see #${offered.openNumber} upstream`,
183 ),
184 ]),
154185 binding(["esc"], "close"),
155186 ];
156187 },
@@ -237,6 +268,15 @@ export function aboutScreen(
237268 );
238269 },
239270 key(e) {
271 if (e.key === "o" && repo !== undefined && offered !== null && forkParent !== undefined) {
272 ctx.pop();
273 ctx.push(
274 offered.openNumber === null
275 ? newPullScreen(f, ctx, repo)
276 : pullScreen(f, ctx, local(f, forkParent), offered.openNumber),
277 );
278 return true;
279 }
240280 if (e.key === "y" && cloneUrl !== "") {
241281 ctx.copy(cloneUrl);
242282 ctx.toast("copied the clone URL", "success");
@@ -335,6 +375,19 @@ export function trailRows(
335375 ];
336376 }
337377
378 /** The offer row: what the fork has for its parent, and where it stands. */
379 export function offerRows(offer: UpstreamOffer | null): [string, string][] {
380 if (offer === null) return [];
381 return [
382 [
383 "offer",
384 offer.openNumber === null
385 ? `${plural(offer.ahead, "commit")} ahead · o opens the pull`
386 : `${plural(offer.ahead, "commit")} ahead · offered in #${offer.openNumber}`,
387 ],
388 ];
389 }
390
338391 /**
339392 * The upstream row, as the web About line has it: for an admin only, with
340393 * the address, branch and last check while following is on, and "off" when
src/web/pages/offer-upstream.ts added +33
@@ -0,0 +1,33 @@
1 /**
2 * The offer on a fork's home: its own commits, one tap from the pull form.
3 * "Offer 3 commits upstream" opens the new-pull form with the fork's default
4 * branch as head and the parent's as base; when an open pull already offers
5 * that branch the line links to it instead. The gate and the lookup are
6 * `upstreamOffer`'s, shared with the terminal; no git here.
7 */
8
9 import type { UpstreamOffer } from "../../actions/offer_upstream.ts";
10 import type { Repo } from "../../core/repos.ts";
11 import { repoBase } from "../chrome.ts";
12 import { html, type Html } from "../html.ts";
13 import { btn } from "../ui.ts";
14
15 export function offerUpstream(
16 repo: Repo,
17 parent: Repo | undefined,
18 offer: UpstreamOffer | null,
19 ): Html | null {
20 if (offer === null || parent === undefined) return null;
21 if (offer.openNumber !== null)
22 return html`<p class="mt small">
23 <a href="${repoBase(parent)}/pulls/${offer.openNumber}">Offered in #${offer.openNumber}</a>
24 </p>`;
junipercrew outdated

Lists up to 200 pulls then filters in JS. Ask the database for the one open pull with this head repo and branch.
25 const query = new URLSearchParams({ head: offer.head, base: offer.base });
26 return html`<div class="mt">
27 ${btn(`Offer ${offer.ahead} commit${offer.ahead === 1 ? "" : "s"} upstream`, {
28 size: "sm",
29 icon: "merge",
30 href: `${repoBase(repo)}/pulls/new?${query}`,
junipercrew outdated

Capped at 200 open pulls on the parent. On a busy parent the fork's pull can fall outside that window and the button would reappear. Query by head repo and branch if listPulls can.
31 })}
32 </div>`;
33 }
src/web/pages/repo.ts +13 −2
@@ -9,9 +9,11 @@
99 */
1010
1111 import { followingLine } from "./upstream-follow.ts";
12 import { upstreamOffer } from "../../actions/offer_upstream.ts";
13 import { offerUpstream } from "./offer-upstream.ts";
1214 import { NOT_A_MEMBER } from "../../core/roles.ts";
1315 import { storageLimitMessage } from "../../core/storage.ts";
14 import { canRead, listForks, type Repo } from "../../core/repos.ts";
16 import { canRead, canWrite, listForks, type Repo } from "../../core/repos.ts";
1517 import { FORKS_PER_DAY, forkRepo, repositoryLimitMessage } from "../../actions/repos.ts";
1618 import { forkDrift, takeUpdates, type ForkDrift } from "../../actions/forks.ts";
1719 import { lineageOf } from "../../core/following.ts";
@@ -1026,7 +1028,9 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> {
10261028 const parent = forkParent(ctx, repo);
10271029 const [entries, drift] = await Promise.all([
10281030 timePhaseAsync(ctx.request, "tree", () => scope.source.tree(ref, "")),
junipercrew outdated

This puts git (forkDrift) on the render path of every fork home. Naming the phase helps, but stored figures were the ask.
1029 parent === undefined ? undefined : forkDrift(ctx.app, repo, parent),
1031 parent === undefined
1032 ? undefined
1033 : timePhaseAsync(ctx.request, "drift", () => forkDrift(ctx.app, repo, parent)),
10301034 ]);
10311035 const tree = entries.ok ? entries.value : [];
10321036
@@ -1037,10 +1041,17 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> {
10371041 const latest = await timePhaseAsync(ctx.request, "wave", () =>
10381042 latestWave(repoDir(ctx.app.reposRoot, repo.ownerName, repo.name), repo.defaultBranch),
10391043 );
1044 // The Following card's "Propose upstream" is a writer's route to the same
1045 // pull; a writer who has the card is not offered a second button.
1046 const offer =
1047 following !== undefined && canWrite(ctx.app.db, repo, ctx.auth.user)
1048 ? null
1049 : upstreamOffer(ctx.app, ctx.auth.user, repo, parent, drift?.ahead);
10401050 const rendering = performance.now();
10411051 const body = html`${scopeHeader(ctx, scope, "code", {
10421052 branchCount: refs.branches.length,
10431053 })}${archivedLine(ctx, repo)}${noApp}
1054 ${offerUpstream(repo, parent, offer)}
10441055 ${latest === null
10451056 ? ""
10461057 : html`<p class="muted latest-wave">Latest:
test/offer-upstream.test.ts added +179
@@ -0,0 +1,179 @@
1 /**
2 * A fork ahead of its parent offers its commits upstream from its home: a
3 * button into the pull form with both branches chosen, `Offered in #N` once a
4 * pull is open, nothing when level, nothing for a viewer who may not open one.
5 */
6 import { afterAll, beforeAll, 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 { setRole } from "../src/actions/roles.ts";
11 import { openPull } from "../src/actions/pulls.ts";
12 import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts";
13 import { upstreamOffer } from "../src/actions/offer_upstream.ts";
14 import { createApp, type App } from "../src/app.ts";
15 import { getRepo } from "../src/core/repos.ts";
16 import { offerRows } from "../src/tui/screens/about.ts";
17 import { createUser } from "../src/core/users.ts";
18 import { serve, type Servers } from "../src/http/server.ts";
19 import { quietLogs } from "./helpers/globals.ts";
20
21 quietLogs();
22
23 let dataDir: string;
24 let app: App;
25 let servers: Servers;
26 let users: Awaited<ReturnType<typeof mk>>[] = [];
27
28 async function mk(username: string, first: boolean) {
29 const made = await createUser(app.db, {
30 origin: "seed",
31 trusted: true,
32 username,
33 email: `${username}@forge.local`,
34 password: "a-good-passphrase",
35 adminIfFirst: first,
36 });
37 if (!made.ok) throw new Error(made.error);
38 return made.value;
39 }
40
41 const signIn = async (username: string): Promise<string> => {
42 const r = await fetch(`${servers.url}/signin`, {
43 method: "POST",
44 body: new URLSearchParams({ username, password: "a-good-passphrase" }),
45 redirect: "manual",
46 });
47 return (r.headers.get("set-cookie") ?? "").split(";")[0]!;
48 };
49
50 const home = async (path: string, cookie = ""): Promise<string> => {
51 const r = await fetch(`${servers.url}${path}`, {
52 headers: cookie ? { cookie } : {},
53 });
54 expect(r.status).toBe(200);
55 return r.text();
56 };
57
58 beforeAll(async () => {
59 dataDir = await mkdtemp(join(tmpdir(), "forge-offer-"));
60 app = createApp({ dataDir, port: 0, host: "forge.test" });
61 servers = await serve(app);
62 users = [await mk("hank", true), await mk("olive", false), await mk("ravi", false)];
63 for (const name of ["olive", "ravi"]) {
64 const promoted = setRole(app, users[0]!, name, "member");
65 if (!promoted.ok) throw new Error(promoted.error);
66 }
67 const r = await createRepo(app, users[1]!, { name: "yard", visibility: "public" });
68 if (!r.ok) throw new Error(String(r.error));
69 await commitToBranch(app, users[1]!, r.value, {
70 message: "first",
71 files: [{ path: "a.txt", content: "a\n" }],
72 });
73 const f = await forkRepo(app, users[2]!, r.value);
74 if (!f.ok) throw new Error(String(f.error));
75 });
76
77 afterAll(async () => {
78 await servers.stop();
79 await rm(dataDir, { recursive: true, force: true });
80 });
81
82 const ownWork = async (n: number) => {
83 const fork = getRepo(app.db, "ravi", "yard")!;
84 for (let i = 0; i < n; i++)
85 await commitToBranch(app, users[2]!, fork, {
86 message: `mine ${i}`,
87 files: [{ path: `m${i}-${Date.now()}.txt`, content: "x\n" }],
88 });
89 };
90
91 test("level with its parent, the fork offers nothing", async () => {
92 const h = await home("/ravi/yard", await signIn("ravi"));
93 expect(h).not.toContain("commit upstream");
94 expect(h).not.toContain("commits upstream");
95 expect(h).not.toContain("Offered in");
96 });
97
98 test("ahead with no pull: a button into the pull form with both branches chosen", async () => {
99 await ownWork(3);
100 const h = await home("/ravi/yard", await signIn("olive"));
101 expect(h).toContain("Offer 3 commits upstream");
102 expect(h).toContain('href="/ravi/yard/pulls/new?head=main&amp;base=main"');
103 expect(h).not.toContain("Offered in");
104 const form = await home("/ravi/yard/pulls/new?head=main&base=main", await signIn("olive"));
105 expect(form).toContain("olive/yard");
106 });
107
108 test("the drift read is a named Server-Timing phase", async () => {
109 const r = await fetch(`${servers.url}/ravi/yard`, {
110 headers: { cookie: await signIn("olive") },
111 });
112 expect(r.headers.get("server-timing") ?? "").toContain("drift");
113 });
114
115 test("a viewer who may not open a pull is not offered one", async () => {
116 const h = await home("/ravi/yard");
117 expect(h).not.toContain("Offer 3 commits upstream");
118 });
119
120 test("ahead with an open pull: the line links to it", async () => {
121 const parent = getRepo(app.db, "olive", "yard")!;
122 const fork = getRepo(app.db, "ravi", "yard")!;
123 const opened = await openPull(app, users[2]!, parent, {
124 title: "Mine",
125 body: "",
126 baseBranch: "main",
127 headBranch: "main",
128 headRepo: fork,
129 });
130 if (!opened.ok) throw new Error(String(opened.error));
131 const h = await home("/ravi/yard", await signIn("olive"));
132 expect(h).toContain(`Offered in #${opened.value.number}`);
133 expect(h).toContain(`href="/olive/yard/pulls/${opened.value.number}"`);
134 expect(h).not.toContain("Offer 3 commits upstream");
135 });
136
137 test("a pull whose head is another branch does not stand for the default branch", async () => {
138 const parent = getRepo(app.db, "olive", "yard")!;
139 const fork = getRepo(app.db, "ravi", "yard")!;
140 const own = await commitToBranch(app, users[2]!, fork, {
141 message: "side",
142 branch: "side",
143 files: [{ path: "side.txt", content: "s\n" }],
144 });
145 if (!own.ok) throw new Error(String(own.error));
146 const side = await openPull(app, users[2]!, parent, {
147 title: "Side",
148 body: "",
149 baseBranch: "main",
150 headBranch: "side",
151 headRepo: fork,
152 });
153 if (!side.ok) throw new Error(String(side.error));
154 const offer = upstreamOffer(app, users[0]!, fork, parent, 3);
155 expect(offer?.openNumber).not.toBe(side.value.number);
156 expect(offer?.head).toBe("main");
157 });
158
159 test("the shared offer is plain data, gated on a signed-in reader of the parent", () => {
160 const parent = getRepo(app.db, "olive", "yard")!;
161 const fork = getRepo(app.db, "ravi", "yard")!;
162 expect(upstreamOffer(app, undefined, fork, parent, 3)).toBeNull();
163 expect(upstreamOffer(app, users[0]!, fork, undefined, 3)).toBeNull();
164 expect(upstreamOffer(app, users[0]!, fork, parent, 0)).toBeNull();
165 expect(upstreamOffer(app, users[0]!, fork, parent, null)).toBeNull();
166 const offer = upstreamOffer(app, users[0]!, fork, parent, 3)!;
167 expect(offer.ahead).toBe(3);
168 expect(offer.base).toBe("main");
169 expect(offerRows(null)).toEqual([]);
170 expect(offerRows({ ahead: 3, openNumber: null, head: "main", base: "main" })[0]![1]).toContain("o opens the pull");
171 expect(offerRows({ ahead: 1, openNumber: 7, head: "main", base: "main" })[0]![1]).toContain("#7");
172 });
173
174 test("a writer with the Following card has one route, its Propose upstream", async () => {
175 const h = await home("/ravi/yard", await signIn("ravi"));
176 expect(h).toContain("See the upstream proposal");
177 expect(h).not.toContain("Offered in");
178 expect(h).not.toContain("pulls/new?head=main");
179 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs