trevato/forge

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

More
Forks1g fMapg aRuns18g rEventsFlows

Before a fork's pull merges, its preview runs the combined build, and conflicts are resolved in a branch on our side with the decisions written for the author #9027

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9012-before-a-fork-s-pull-merges-its-preview → main 4 commits fork

  • Merged. scout/9012-before-a-fork-s-pull-merges-its-preview was merged into main as 5b3363b.

Review

Sign in to review this change.
14 files changed +722 −13
docs/features/upstream-landing.md +9 −1
@@ -20,6 +20,14 @@ Test: `test/kept-fork-landing.test.ts` runs two clones of two repositories throu
2020
2121 The rule lives in `landingKind` (`src/core/landing_kind.ts`) and `mergePull` applies it, so the API, the MCP `merge` tool and both screens land a kept fork as a merge. The choice is not offered where it would be ignored: the web merge box shows "Lands as a merge commit" in place of the strategy menu, and the terminal's merge confirm (`m` on a pull) drops Squash and says the same. A pull by or from an agent, or inside one repository, keeps both choices.
2222
23 ## Combined on our side
24
25 When main has moved since a kept fork's head, the forge merges main into that head in a branch of the base repository, `combined/N` (`src/git/combined.ts`), on opening the pull, on every push to the fork, and again just before landing. The author's branch is never written. Each conflicting block keeps main's side, with no review: the author's side of that block is dropped from the landed tree, and the note says so and how to get it back (merge main into their branch, resolve, push: a head that already has main is its own combined build). A conflict git cannot pick a side of (modify/delete, rename) is left open. The combined commit's message is the note, and its `Combined-Base` and `Combined-Kept` trailers are the facts the pull renders from (`src/core/combined_view.ts`): a "Combined with main" card on the page and the same section on the terminal's pull screen. The preview builds `combined/N`, under the approval of the head it carries (read from git, `headUnder`), and landing merges it, so the author's head stays an ancestor of main. Commits pushed on top of `combined/N` by a writer are kept by the next combining.
26
27 A push to main alone does not recombine: the card marks itself stale, naming the main commit it was built on, until the next push to the fork or the landing recombines. Checks run on the pull's head, not on the combined commit, and the card says that too.
28
29 Test: `test/combined-fork.test.ts` (open, page, terminal, a push recombining, stale, landing) and `test/combined-git.test.ts` (a writer's commit surviving, modify/delete left open, the head a preview runs under).
30
2331 ## Not yet
2432
25 Landing a very large fork in reviewed slices that still end as a merge of the author's head; routing a pull opened on the Forgejo mirror to trunk; a preview of the combined build with a crew-written resolution note; and running his code under the guest rule until a person approves (#8903 is not in this repository). Resolving conflicts on our side is today's resolve page (`docs/PULLS.md`).
33 Landing a very large fork in reviewed slices that still end as a merge of the author's head; routing a pull opened on the Forgejo mirror to trunk; a crew run sent to refine `combined/N` beyond main's side; recombining when main alone moves; running the check on the combined commit; and running his code under the guest rule until a person approves (#8903 is not in this repository). Resolving conflicts on our side is today's resolve page (`docs/PULLS.md`).
src/actions/apps.ts +3 −4
@@ -59,6 +59,7 @@ import { canAdmin, canEnterApp, canWrite, getRepo, type Repo } from "../core/rep
5959 import type { Run } from "../core/runs.ts";
6060 import { type User } from "../core/users.ts";
6161 import { resolveOid } from "../git/merge.ts";
62 import { previewCommit } from "../git/combined.ts";
6263 import { repoDir } from "../git/repos.ts";
6364 import { err, ok, type Result } from "../kernel/result.ts";
6465 import {
@@ -767,11 +768,9 @@ async function commitFor(
767768 return resolveOid(dir, `refs/heads/${repo.defaultBranch}`);
768769 const pull = getPull(app.db, repo.id, Number(env.slice(5)));
769770 if (!pull || pull.state !== "open") return undefined;
770 const ref = pull.crossRepo
771 ? `refs/pull/${pull.number}/head`
772 : `refs/heads/${pull.headBranch}`;
773771 const at = await pullDir(app.db, app.reposRoot, repo, pull);
774 return (await resolveOid(at, ref)) ?? (pull.headOid || undefined);
772 // A fork's pull combined on our side previews the combined build (#9012).
773 return previewCommit(at, pull);
775774 }
776775
777776 export { previewEnv };
src/apps/runtime.ts +9 −5
@@ -56,6 +56,7 @@ import {
5656 import { Gate } from "../git/env.ts";
5757 import { resolveOid } from "../git/merge.ts";
5858 import { repoDir } from "../git/repos.ts";
59 import { headUnder, previewCommit } from "../git/combined.ts";
5960 import { createLog } from "../kernel/log.ts";
6061 import type { Result } from "../kernel/result.ts";
6162 import { holdLease, STALE_AFTER_MS } from "../runs/lease.ts";
@@ -808,10 +809,8 @@ export class AppsRuntime {
808809
809810 private async pullHead(repo: Repo, pull: Pull): Promise<string | undefined> {
810811 const dir = await pullDir(this.app.db, this.app.reposRoot, repo, pull);
811 const ref = pull.crossRepo
812 ? `refs/pull/${pull.number}/head`
813 : `refs/heads/${pull.headBranch}`;
814 return (await resolveOid(dir, ref)) ?? (pull.headOid || undefined);
812 // A fork's pull combined on our side previews the combined build (#9012).
813 return previewCommit(dir, pull);
815814 }
816815
817816 // ---- events ----
@@ -1674,7 +1673,12 @@ export class AppsRuntime {
16741673 const number = envPull(env);
16751674 const pull =
16761675 number === undefined ? undefined : getPull(this.app.db, repo.id, number);
1677 if (pull !== undefined && pullMayRun(this.app.db, repo, pull, commitOid))
1676 // A combined preview runs under the head it carries, read from git (#9012).
1677 const head =
1678 pull === undefined
1679 ? commitOid
1680 : headUnder(repoDir(this.app.reposRoot, repo.ownerName, repo.name), pull, commitOid);
1681 if (pull !== undefined && pullMayRun(this.app.db, repo, pull, head))
16781682 return true;
16791683 log.info("preview waits for a writer's approval", {
16801684 app: record.slug,
src/core/combined_view.ts added +58
@@ -0,0 +1,58 @@
1 /**
2 * What a fork's pull shows of its combined branch (#9012), the same on the
3 * page and in the terminal: the branch and commit the preview builds, the
4 * base it was built on and whether main has moved since, what each conflict
5 * kept, and that the checks ran on the head, not on this commit.
6 */
7
8 import type { Pull } from "./pulls.ts";
9 import { keptLine, readCombined, TAKE_BACK, type Kept } from "../git/combined.ts";
10
11 export interface CombinedView {
12 readonly branch: string;
13 readonly oid: string;
14 /** The main commit this combining was built on; "" when its note does not say. */
15 readonly builtOn: string;
16 /** Main has moved past `builtOn`: the preview is not what lands until it recombines. */
17 readonly stale: boolean;
18 readonly files: readonly Kept[];
19 }
20
21 /** The combined branch of an open fork's pull, when it differs from the head. */
22 export async function combinedView(
23 dir: string,
24 pull: Pull,
25 headOid: string,
26 baseOid: string,
27 ): Promise<CombinedView | undefined> {
28 if (!pull.crossRepo || pull.state !== "open" || pull.mergedAt !== null) return undefined;
29 const read = await readCombined(dir, pull.number, headOid);
30 // Main already in the head: the preview is the head itself, nothing to say.
31 if (read === undefined || read.oid === headOid) return undefined;
32 return {
33 branch: read.branch,
34 oid: read.oid,
35 builtOn: read.baseOid,
36 stale: read.baseOid !== "" && read.baseOid !== baseOid,
37 files: read.files,
38 };
39 }
40
41 /** The view, worded: the lead, the per-file lines, and the notes after them. */
42 export function combinedWords(
43 view: CombinedView,
44 number: number,
45 ): { lead: string; files: string[]; notes: string[] } {
46 const notes: string[] = [];
47 if (view.stale)
48 notes.push(
49 `Stale: main has moved since ${view.builtOn.slice(0, 10)}, which this was built on. It recombines on the next push to the fork, and merging recombines before it lands, so what lands may differ from the preview.`,
50 );
51 notes.push("Checks run on the pull's head, not on this combined commit.");
52 if (view.files.some((f) => f.kept === "main")) notes.push(TAKE_BACK);
53 return {
54 lead: `The preview is built from ${view.branch} (${view.oid.slice(0, 10)}): main merged into this pull's head. Merging lands that branch.`,
55 files: view.files.map((f) => keptLine(f, number)),
56 notes,
57 };
58 }
src/core/following.ts +8
@@ -41,6 +41,8 @@ import { runGit } from "../git/env.ts";
4141 import { readTipOrAbsent } from "../git/tip.ts";
4242 import { readTips } from "../git/tips.ts";
4343 import { repoDir } from "../git/repos.ts";
44 import { combine } from "../git/combined.ts";
45 import { fromKeptFork } from "./landing_kind.ts";
4446 import { forkFollowMode } from "./fork_follow.ts";
4547 import { createLog } from "../kernel/log.ts";
4648 import { err, ok, type Result } from "../kernel/result.ts";
@@ -453,6 +455,12 @@ export async function refreshPullHead(
453455 forge.db
454456 .query("UPDATE pull SET head_oid = ?2 WHERE issue_id = ?1")
455457 .run(pull.id, tip);
458 // A kept fork's pull is combined with main on our side (#9012).
459 if (!apart && fromKeptFork(forge.db, pull)) {
460 const baseTip = await resolveOid(baseDir, `refs/heads/${pull.baseBranch}`);
461 if (baseTip !== undefined)
462 await combine(baseDir, { number: pull.number, baseOid: baseTip, headOid: tip });
463 }
456464 return tip;
457465 }
458466
src/core/pulls.ts +19 −3
@@ -13,6 +13,8 @@ import type { Database } from "bun:sqlite";
1313 import { diffRange, type Diff, type DiffLaps } from "../git/diff.ts";
1414 import { isSafeRef } from "../git/guard.ts";
1515 import { refTip } from "../git/read.ts";
16 import { combine, combinedTip } from "../git/combined.ts";
17 import { fromKeptFork } from "./landing_kind.ts";
1618 import { readTips } from "../git/tips.ts";
1719 import {
1820 aheadCount,
@@ -310,7 +312,11 @@ export async function createPull(
310312 // and (for cross-repo) the only place the base repo holds the fork's tip.
311313 await writeRef(at, `refs/pull/${issue.value.number}/head`, headOid);
312314
313 return ok(join(db, issue.value)!);
315 const pull = join(db, issue.value)!;
316 // A kept fork's pull is combined with main on our side (#9012).
317 if (at === dir && fromKeptFork(db, pull))
318 await combine(dir, { number: pull.number, baseOid, headOid });
319 return ok(pull);
314320 }
315321
316322 export function getPull(
@@ -834,10 +840,14 @@ export async function pullStatus(
834840 const baseOid = liveBase ?? pull.baseOid;
835841 const headOid = liveHead ?? pull.headOid;
836842 const reviews = gate === undefined ? null : reviewGate(gate, pull, headOid);
843 // A fork's pull combined on our side merges as its combined branch (#9012).
844 const lands = pull.crossRepo
845 ? ((await combinedTip(dir, pull.number, headOid)) ?? headOid)
846 : headOid;
837847 const { ahead, mergeable: clean, conflicts } = await mergeReading(
838848 dir,
839849 baseOid,
840 headOid,
850 lands,
841851 );
842852 return { baseOid, headOid, ahead, mergeable: clean, conflicts, reviews };
843853 }
@@ -1151,10 +1161,16 @@ export async function mergePull(
11511161 )
11521162 return err("checks_required");
11531163 }
1164 // A kept fork lands its combined branch, brought up to the base first:
1165 // the author's head is its ancestor, so it lands too (#9012).
1166 const combined =
1167 input.sealedFrom === undefined && fromKeptFork(db, pull)
1168 ? await combine(dir, { number: pull.number, baseOid: status.baseOid, headOid: status.headOid })
1169 : undefined;
11541170 const landed = await merge(dir, {
11551171 baseBranch: pull.baseBranch,
11561172 baseOid: status.baseOid,
1157 headOid: status.headOid,
1173 headOid: combined?.oid ?? status.headOid,
11581174 strategy: input.strategy,
11591175 message: input.message,
11601176 author: input.author,
src/git/combined.ts added +229
@@ -0,0 +1,229 @@
1 /**
2 * A kept fork's pull, combined on our side (#9012): main merged into the
3 * author's head, in a branch of the base repository (`combined/N`). The
4 * author's branch is never written. Each conflicting block keeps main's
5 * side, and the combined commit's message is the note the author reads:
6 * every conflicted file and what was kept. The preview builds this commit
7 * and the landing merges it, so the author's head stays an ancestor of main.
8 *
9 * Commits pushed on top of the combined branch (a resolution done by hand)
10 * are kept: the next combining starts from the branch's tip as long as it
11 * still carries the author's head.
12 */
13
14 import { gitEnv, runGit, runGitCapture } from "./env.ts";
15 import { isSafeOid, isSafeTreePath } from "./guard.ts";
16 import { refTip } from "./read.ts";
17 import { mergeBase, resolveOid } from "./merge.ts";
18 import { parseConflicts } from "./conflicts.ts";
19
20 /** The branch a pull's combined tree lives on, in the base repository. */
21 export const combinedBranch = (number: number): string => `combined/${number}`;
22 const combinedRef = (number: number): string => `refs/heads/${combinedBranch(number)}`;
23
24 /** One conflicted file and what the combining kept of it. */
25 export interface Kept {
26 readonly path: string;
27 /** Conflicting blocks in the file; 0 when it is not a text conflict. */
28 readonly blocks: number;
29 /** "main": main's side of every block; "open": git could not pick, it awaits a hand. */
30 readonly kept: "main" | "open";
31 }
32
33 const IDENTITY = {
34 GIT_AUTHOR_NAME: "forge",
35 GIT_AUTHOR_EMAIL: "forge@localhost",
36 GIT_COMMITTER_NAME: "forge",
37 GIT_COMMITTER_EMAIL: "forge@localhost",
38 };
39
40 const isAncestor = async (dir: string, a: string, b: string): Promise<boolean> =>
41 a === b || (await mergeBase(dir, a, b)) === a;
42
43 /** The note the author reads: what was combined and what was kept, file by file. */
44 export function combinedNote(number: number, baseOid: string, files: readonly Kept[]): string {
45 const lines = [
46 `Main merged into #${number}'s head on our side (${combinedBranch(number)})`,
47 "",
48 "Your branch is untouched. The preview builds this combined tree, and landing merges it, so your head stays an ancestor of main.",
49 "",
50 ];
51 if (files.length === 0) lines.push("No file conflicted: this is a clean merge of main into your head.");
52 else {
53 lines.push("Conflicted files, and what was kept:");
54 for (const f of files) lines.push(`- ${keptLine(f, number)}`);
55 if (files.some((f) => f.kept === "main")) lines.push("", TAKE_BACK);
56 }
57 // The same facts as trailers: what readers render from, not the words above.
58 lines.push("", `${BASE_TRAILER}: ${baseOid}`);
59 for (const f of files) lines.push(`${KEPT_TRAILER}: ${f.kept} ${f.blocks} ${f.path}`);
60 return lines.join("\n");
61 }
62
63 /** One conflicted file, said to its author. */
64 export const keptLine = (f: Kept, number: number): string =>
65 f.kept === "main"
66 ? `${f.path}: ${f.blocks} block${f.blocks === 1 ? "" : "s"}, main's side kept; your side was dropped from the landed tree.`
67 : `${f.path}: left open (not a line conflict); it is resolved by hand on ${combinedBranch(number)}.`;
68
69 /** How the author gets a dropped side back. */
70 export const TAKE_BACK =
71 "To get your side back: merge main into your branch, resolve these files as you want them, and push. A head that already has main is its own combined build, so what you resolved is what lands.";
72
73 const BASE_TRAILER = "Combined-Base";
74 const KEPT_TRAILER = "Combined-Kept";
75
76 /** The trailers of a combining commit, read back as data. */
77 export function parseCombined(message: string): { baseOid: string; files: Kept[] } {
78 let baseOid = "";
79 const files: Kept[] = [];
80 for (const line of message.split("\n")) {
81 const base = line.startsWith(`${BASE_TRAILER}: `) ? line.slice(BASE_TRAILER.length + 2).trim() : "";
82 if (isSafeOid(base)) baseOid = base;
83 const kept = /^Combined-Kept: (main|open) (\d{1,6}) (.+)$/.exec(line);
84 if (kept !== null && isSafeTreePath(kept[3]!))
85 files.push({ kept: kept[1] as Kept["kept"], blocks: Number(kept[2]), path: kept[3]! });
86 }
87 return { baseOid, files };
88 }
89
90 /** `merge-tree --write-tree --name-only`: the tree, and the paths still in conflict. */
91 async function mergeTree(
92 dir: string,
93 ours: string,
94 theirs: string,
95 options: readonly string[],
96 ): Promise<{ tree: string; files: string[] } | undefined> {
97 const out = await runGitCapture(
98 ["merge-tree", "--write-tree", "--name-only", ...options, ours, theirs],
99 { cwd: dir, timeoutMs: 60_000 },
100 );
101 if (out.code !== 0 && out.code !== 1) return undefined;
102 const [first, ...rest] = out.stdout.split("\n");
103 const tree = (first ?? "").trim();
104 if (!isSafeOid(tree)) return undefined;
105 const files: string[] = [];
106 if (out.code === 1)
107 for (const line of rest) {
108 if (line.trim() === "") break;
109 if (isSafeTreePath(line)) files.push(line);
110 }
111 return { tree, files };
112 }
113
114 /**
115 * Write (or bring up to date) the combined branch of pull `number`: main at
116 * `baseOid` merged into `headOid`. Answers the combined tip, and the files
117 * this combining resolved (null when the branch already carried both).
118 */
119 export async function combine(
120 dir: string,
121 input: { readonly number: number; readonly baseOid: string; readonly headOid: string },
122 ): Promise<{ oid: string; files: Kept[] | null } | undefined> {
123 const { number, baseOid, headOid } = input;
124 if (!Number.isInteger(number) || number < 1 || !isSafeOid(baseOid) || !isSafeOid(headOid))
125 return undefined;
126 const ref = combinedRef(number);
127 const prior = await resolveOid(dir, ref);
128 const from = prior !== undefined && (await isAncestor(dir, headOid, prior)) ? prior : headOid;
129 if (from === prior && (await isAncestor(dir, baseOid, from))) return { oid: from, files: null };
130
131 const update = async (oid: string): Promise<boolean> =>
132 oid === prior ||
133 (await runGit(["update-ref", ref, oid, ...(prior === undefined ? [] : [prior])], { cwd: dir })).ok;
134 // Main is already in it: the head (or the branch) is the combined build.
135 if (await isAncestor(dir, baseOid, from)) return (await update(from)) ? { oid: from, files: [] } : undefined;
136
137 const plain = await mergeTree(dir, baseOid, from, []);
138 if (plain === undefined) return undefined;
139 let tree = plain.tree;
140 const files: Kept[] = [];
141 if (plain.files.length > 0) {
142 // Main's side of each block; what git still cannot pick stays open.
143 const picked = await mergeTree(dir, baseOid, from, ["-X", "ours"]);
144 const open = new Set(picked?.files ?? plain.files);
145 if (picked !== undefined) tree = picked.tree;
146 for (const path of plain.files) {
147 const text = await runGit(["cat-file", "blob", `${plain.tree}:${path}`], { cwd: dir });
148 const blocks = text.ok ? parseConflicts(text.value).blocks.length : 0;
149 files.push({ path, blocks, kept: open.has(path) || blocks === 0 ? "open" : "main" });
150 }
151 }
152 const commit = await runGit(
153 ["commit-tree", tree, "-p", from, "-p", baseOid, "-m", combinedNote(number, baseOid, files)],
154 { cwd: dir, env: IDENTITY },
155 );
156 if (!commit.ok) return undefined;
157 const oid = commit.value.trim();
158 return (await update(oid)) ? { oid, files } : undefined;
159 }
160
161 /** The combined tip for this head, when the branch exists and still carries it. */
162 export async function combinedTip(
163 dir: string,
164 number: number,
165 headOid: string,
166 ): Promise<string | undefined> {
167 if (!Number.isInteger(number) || number < 1 || !isSafeOid(headOid)) return undefined;
168 const tip = await refTip(dir, combinedRef(number));
169 if (tip === undefined || !(await isAncestor(dir, headOid, tip))) return undefined;
170 return tip;
171 }
172
173 /** The combined tip, the base it was built on, and what its combining kept. */
174 export async function readCombined(
175 dir: string,
176 number: number,
177 headOid: string,
178 ): Promise<{ oid: string; branch: string; baseOid: string; files: Kept[] } | undefined> {
179 const oid = await combinedTip(dir, number, headOid);
180 if (oid === undefined) return undefined;
181 // The newest combining commit of those the head lacks says what was kept.
182 const log = await runGit(
183 ["log", "-1", "--author=forge@localhost", "--format=%B", oid, `^${headOid}`],
184 { cwd: dir },
185 );
186 return { oid, branch: combinedBranch(number), ...parseCombined(log.ok ? log.value : "") };
187 }
188
189 /**
190 * The head a preview commit runs under, read from git as it stands: the
191 * pull's head when `commit` is its combined tip and still carries it, the
192 * commit itself otherwise. Synchronous, for the runtime's approval gate.
193 */
194 export function headUnder(
195 dir: string,
196 pull: { readonly crossRepo: boolean; readonly number: number; readonly headOid: string },
197 commit: string,
198 ): string {
199 if (!pull.crossRepo || commit === pull.headOid || !isSafeOid(commit) || !isSafeOid(pull.headOid))
200 return commit;
201 const git = (args: string[]) =>
202 Bun.spawnSync(["git", ...args], { cwd: dir, env: gitEnv(), stdout: "pipe", stderr: "ignore" });
203 const tip = git(["rev-parse", "--verify", "--quiet", combinedRef(pull.number)]);
204 if (tip.exitCode !== 0 || tip.stdout.toString().trim() !== commit) return commit;
205 return git(["merge-base", "--is-ancestor", pull.headOid, commit]).exitCode === 0 ? pull.headOid : commit;
206 }
207
208 /**
209 * The commit a pull's preview builds: the head (the branch, or a fork's
210 * snapshot), or the combined tip when a fork's pull has one that carries it.
211 */
212 export async function previewCommit(
213 dir: string,
214 pull: { readonly crossRepo: boolean; readonly number: number; readonly headBranch: string; readonly headOid: string },
215 ): Promise<string | undefined> {
216 return (await previewHead(dir, pull))?.commit;
217 }
218
219 /** The head a preview commit stands for: a combined tip answers as the head it carries. */
220 export async function previewHead(
221 dir: string,
222 pull: { readonly crossRepo: boolean; readonly number: number; readonly headBranch: string; readonly headOid: string },
223 ): Promise<{ commit: string; head: string } | undefined> {
224 const ref = pull.crossRepo ? `refs/pull/${pull.number}/head` : `refs/heads/${pull.headBranch}`;
225 const head = (await resolveOid(dir, ref)) ?? (pull.headOid || undefined);
226 if (head === undefined) return undefined;
227 const combined = pull.crossRepo ? await combinedTip(dir, pull.number, head) : undefined;
228 return { commit: combined ?? head, head };
229 }
src/tui/screens/pull.ts +46
@@ -49,6 +49,7 @@ import { openEnvUrl } from "../../actions/apps.ts";
4949 import { listRepoRuns, mayReadRunLog } from "../../actions/runs.ts";
5050 import { pullLooks, type PullLook } from "../../looks/readable.ts";
5151 import { pullLooksScreen } from "./pulllooks.ts";
52 import { combinedView, combinedWords, type CombinedView } from "../../core/combined_view.ts";
5253 import { previewEnv } from "../../apps/data.ts";
5354 import { findingsWords, pullPreviewFindings } from "../../core/preview_findings.ts";
5455 import {
@@ -266,6 +267,8 @@ export function pullScreen(
266267 let preview: EnvStatus | undefined;
267268 /** The pictures taken of the preview, as the API would list them to this reader. */
268269 let looks: PullLook[] = [];
270 /** A fork's pull combined with main on our side: what its preview builds (#9012). */
271 let combined: CombinedView | undefined;
269272 /** The deploy of the *current* head — what the merge box calls the checks. */
270273 let check: CheckSummary = NO_CHECK;
271274 /** The pull's base branch, to name what the check was merged with. */
@@ -329,6 +332,14 @@ export function pullScreen(
329332 return base === undefined ? undefined : wallOf(dir, base, facts.headOid);
330333 })()
331334 : undefined;
335 combined =
336 repo !== undefined && row !== undefined && facts !== null
337 ? await (async () => {
338 const dir = await pullDir(f.app.db, f.app.reposRoot, repo, row);
339 const base = await resolveOid(dir, `refs/heads/${row.baseBranch}`);
340 return base === undefined ? undefined : combinedView(dir, row, facts.headOid, base);
341 })()
342 : undefined;
332343 // The app half: the preview environment (in memory, from the runtime),
333344 // the checks at the live head (a row), and what the crew has done here.
334345 const record = repo === undefined ? undefined : getAppByRepo(f.db, repo.id);
@@ -1175,6 +1186,7 @@ export function pullScreen(
11751186 width = body!.w;
11761187 lines = [
11771188 ...threadLines(entries(), Math.max(20, body!.w - 1), c),
1189 ...combinedLines(combined, number, Math.max(20, body!.w - 1), c),
11781190 ...looksLines(looks, lookOrigin(f.app.config), Math.max(20, body!.w - 1), c),
11791191 ];
11801192 }
@@ -1894,6 +1906,40 @@ const lookOrigin = (config: {
18941906 * The Looks section under the conversation: one line each, absolute URL so a
18951907 * terminal can open it. Nothing at all when the pull has none.
18961908 */
1909 /** The combined branch, said as the page's card says it (`web/pages/pull-combined.ts`). */
1910 function combinedLines(
1911 view: CombinedView | undefined,
1912 number: number,
1913 width: number,
1914 c: Tokens,
1915 ): Span[][] {
1916 if (view === undefined) return [];
1917 const words = combinedWords(view, number);
1918 const clean = (t: string): string => t.replace(/[\p{Cc}\p{Cf}\u2028\u2029]/gu, " ");
1919 const wrap = (text: string, indent: string): string[] => {
1920 const room = Math.max(10, width - indent.length);
1921 const out: string[] = [];
1922 let line = "";
1923 for (const word of clean(text).split(" ")) {
1924 if (line !== "" && line.length + 1 + word.length > room) {
1925 out.push(indent + line);
1926 line = word;
1927 } else line = line === "" ? word : `${line} ${word}`;
1928 }
1929 if (line !== "") out.push(indent + line);
1930 return out;
1931 };
1932 const say = (text: string, indent: string, fg: Color): Span[][] =>
1933 wrap(text, indent).map((t): Span[] => [{ text: t, style: style(fg, c.bg) }]);
1934 return [
1935 [],
1936 [{ text: ` Combined with main${view.stale ? " · stale" : ""}`, style: style(c.fg, c.bg, BOLD) }],
1937 ...say(words.lead, " ", c.fg),
1938 ...words.files.flatMap((l) => say(`- ${l}`, " ", c.fg)),
1939 ...words.notes.flatMap((l) => say(l, " ", c.mutedFg)),
1940 ];
1941 }
1942
18971943 function looksLines(
18981944 items: readonly PullLook[],
18991945 origin: string,
src/web/pages/pull-combined.ts added +32
@@ -0,0 +1,32 @@
1 /**
2 * The combined card on a fork's pull (#9012): main merged into the author's
3 * head on our side, which the preview builds and the landing merges, with
4 * each conflicted file and what was kept. The terminal says the same
5 * (`core/combined_view.ts`).
6 */
7
8 import type { Pull } from "../../core/pulls.ts";
9 import { combinedView, combinedWords } from "../../core/combined_view.ts";
10 import { html, type Html } from "../html.ts";
11 import { card, icon } from "../ui.ts";
12
13 export async function combinedCard(
14 dir: string,
15 pull: Pull,
16 headOid: string,
17 baseOid: string,
18 ): Promise<Html> {
19 const view = await combinedView(dir, pull, headOid, baseOid);
20 if (view === undefined) return html``;
21 const words = combinedWords(view, pull.number);
22 return card(
23 html`<p>${words.lead}</p>
24 ${words.files.length === 0 ? null : html`<ul>${words.files.map((i) => html`<li>${i}</li>`)}</ul>`}
25 ${words.notes.map((w) => html`<p class="muted small">${w}</p>`)}`,
26 {
27 extra: "mb",
28 head: html`${icon("merge")}
29 <h2>Combined with main${view.stale ? " · stale" : ""}</h2>`,
30 },
31 );
32 }
src/web/pages/pull.ts +2
@@ -30,6 +30,7 @@ import { parseCheck } from "../../apps/manifest.ts";
3030 import { RED_ALONE_WAIT, redAloneAt, redMark, type RedMark } from "../../checks/queue.ts";
3131 import { tierLine, type TierState } from "../../checks/tiers.ts";
3232 import { fromKeptFork } from "../../core/landing_kind.ts";
33 import { combinedCard } from "./pull-combined.ts";
3334 import { getAppByRepo } from "../../core/apps.ts";
3435 import { labelsFor, listLabels } from "../../core/issues.ts";
3536 import { marksByName } from "../../core/marks.ts";
@@ -1023,6 +1024,7 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
10231024 ${holdCard(ctx, repo, pull, { act: false })}
10241025 ${teamsCard(teamVerdicts(ctx.app.db, { ...pull, headOid: status.headOid }))}
10251026 ${mergeBox(ctx, repo, pull, status, mayMerge, by, check, isApp, tier, stale)}
1027 ${await combinedCard(dir, pull, status.headOid, status.baseOid)}
10261028 <div class="split">
10271029 <div>
10281030 ${guestCard(ctx, repo, pull, `${thread}/approve-guest`)}
src/web/studio/change.ts +7
@@ -66,6 +66,7 @@ import { previewCard } from "../pages/apps.ts";
6666 import { lookCard } from "../pages/looks.ts";
6767 import { approveHereCard, pullChecksLineFor, teamsCard } from "../pages/pull.ts";
6868 import { guestCard } from "../pages/pull-guest.ts";
69 import { combinedCard } from "../pages/pull-combined.ts";
6970 import { teamVerdicts } from "../../core/issue_teams.ts";
7071 import { liveRegionsJs } from "../client.ts";
7172 import { pullState } from "../pages/pulls.ts";
@@ -673,10 +674,16 @@ async function changeConversation(
673674 <summary class="muted small">More: hold, take it back</summary>
674675 ${holdForm}${takeBackSection}
675676 </details>`;
677 // A fork's pull combined with main on our side says so, with its note (#9012).
678 const combined =
679 status === undefined
680 ? null
681 : await lap(spent, "diff", () => combinedCard(wordCtx.dir, pull, headOid, status.baseOid));
676682 const gate = tick(
677683 spent,
678684 "gate",
679685 () => html`${held ? holdCard(ctx, repo, pull, { act: true }) : null}
686 ${combined}
680687 ${guestCard(ctx, repo, pull, `${thread}/approve-guest`)}
681688 ${approveHereCard(ctx, repo, pull, headOid, `${thread}/approve-here`)}
682689 ${teamsCard(teamVerdicts(db, { ...pull, headOid }))}
test/combined-fork.test.ts added +201
@@ -0,0 +1,201 @@
1 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
2 import { mkdtemp, rm } from "node:fs/promises";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { createApp, type App } from "../src/app.ts";
6 import { serve, type Servers } from "../src/http/server.ts";
7 import { gitClientEnv } from "./helpers/git-client.ts";
8 import { proveAll } from "./helpers/verified.ts";
9 import { promote } from "./helpers/members.ts";
10 import { getPull } from "../src/core/pulls.ts";
11 import { getRepo } from "../src/core/repos.ts";
12 import { previewCommit } from "../src/git/combined.ts";
13 import { repoDir } from "../src/git/repos.ts";
14 import { tokenFor } from "./helpers/tokens.ts";
15 import { refreshPullHead } from "../src/core/following.ts";
16 import { getUserByName } from "../src/core/users.ts";
17 import { createForgeApp } from "../src/tui/forge.ts";
18 import { pullScreen } from "../src/tui/screens/pull.ts";
19 import { local } from "../src/tui/screens/where.ts";
20 import { headlessTerminal } from "../src/tui/term/terminal.ts";
21 import { settleApp } from "./helpers/tui.ts";
22
23 const gitEnv = gitClientEnv({
24 GIT_AUTHOR_NAME: "Jay Moran",
25 GIT_AUTHOR_EMAIL: "[email protected]",
26 GIT_COMMITTER_NAME: "Jay Moran",
27 GIT_COMMITTER_EMAIL: "[email protected]",
28 });
29
30 describe("a conflicting fork pull is combined on our side (#9012)", () => {
31 let dataDir: string;
32 let work: string;
33 let app: App;
34 let servers: Servers;
35 let base: string;
36 let alice = "";
37 let bob = "";
38
39 async function git(args: string[], cwd: string): Promise<string> {
40 const p = Bun.spawn(["git", ...args], { cwd, env: gitEnv, stdout: "pipe", stderr: "pipe" });
41 const [out, err, code] = await Promise.all([
42 new Response(p.stdout).text(),
43 new Response(p.stderr).text(),
44 p.exited,
45 ]);
46 if (code !== 0) throw new Error(`git ${args.join(" ")}: ${err}`);
47 return out.trim();
48 }
49 const call = (path: string, token: string, body?: unknown): Promise<Response> =>
50 fetch(`${base}${path}`, {
51 method: body === undefined ? "GET" : "POST",
52 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
53 ...(body === undefined ? {} : { body: JSON.stringify(body) }),
54 });
55 const authed = (user: string, token: string) =>
56 `${base.replace("http://", `http://${user}:${token}@`)}`;
57
58 async function signup(username: string): Promise<string> {
59 await fetch(`${base}/signup`, {
60 method: "POST",
61 body: new URLSearchParams({ username, email: `${username}@f.dev`, password: "kept-fork-pass" }),
62 redirect: "manual",
63 });
64 return tokenFor(app, username, "admin", "k");
65 }
66
67 beforeAll(async () => {
68 dataDir = await mkdtemp(join(tmpdir(), "forge-cb-"));
69 work = await mkdtemp(join(tmpdir(), "forge-cb-work-"));
70 app = createApp({ dataDir, port: 0 });
71 servers = await serve(app);
72 base = servers.url;
73 alice = await signup("alice");
74 bob = await signup("bob");
75 proveAll(app);
76 promote(app.db);
77 await call("/api/v1/repos", alice, { name: "app", visibility: "public" });
78 await git(["clone", `${authed("alice", alice)}/alice/app.git`, "a"], work);
79 await Bun.write(join(work, "a/one.txt"), "one\n");
80 await git(["add", "."], join(work, "a"));
81 await git(["commit", "-m", "seed"], join(work, "a"));
82 await git(["push", "origin", "main"], join(work, "a"));
83 await call("/api/v1/repos/alice/app/forks", bob, {});
84 });
85
86 afterAll(async () => {
87 servers.stop();
88 app.close();
89 await rm(dataDir, { recursive: true, force: true });
90 await rm(work, { recursive: true, force: true });
91 });
92
93 test("the combined branch, its note, its preview, and the landing keep the author's head", async () => {
94 const a = join(work, "a");
95 const b = join(work, "b");
96 await git(["clone", `${authed("bob", bob)}/bob/app.git`, "b"], work);
97 await Bun.write(join(b, "one.txt"), "bob\n");
98 await git(["commit", "-am", "bob's one"], b);
99 await git(["push", "origin", "main"], b);
100 const bobHead = await git(["rev-parse", "HEAD"], b);
101
102 await Bun.write(join(a, "one.txt"), "alice\n");
103 await Bun.write(join(a, "two.txt"), "two\n");
104 await git(["add", "."], a);
105 await git(["commit", "-m", "alice moves on"], a);
106 await git(["push", "origin", "main"], a);
107 const aliceMain = await git(["rev-parse", "HEAD"], a);
108
109 const opened = await call("/api/v1/repos/alice/app/pulls", bob, {
110 title: "Jay's work",
111 base: "main",
112 head: "main",
113 headRepo: "bob/app",
114 });
115 expect(opened.status).toBe(201);
116 const { number } = (await opened.json()) as { number: number };
117
118 // The branch on our side: main merged into his head, main's side kept.
119 const bare = repoDir(app.reposRoot, "alice", "app");
120 const tip = await git(["rev-parse", `refs/heads/combined/${number}`], bare);
121 expect((await git(["log", "-1", "--format=%P", tip], bare)).split(" ")).toEqual([bobHead, aliceMain]);
122 expect(await git(["show", `${tip}:one.txt`], bare)).toBe("alice");
123 expect(await git(["show", `${tip}:two.txt`], bare)).toBe("two");
124 const note = await git(["log", "-1", "--format=%B", tip], bare);
125 expect(note).toContain("- one.txt: 1 block, main's side kept");
126 // His branch is untouched.
127 expect(await git(["ls-remote", "origin", "refs/heads/main"], b)).toStartWith(bobHead);
128
129 // The preview builds the combined tree.
130 const repo = getRepo(app.db, "alice", "app")!;
131 const pull = getPull(app.db, repo.id, number)!;
132 expect(await previewCommit(bare, pull)).toBe(tip);
133
134 // The pull says so, with the note.
135 const page = await call(`/alice/app/pulls/${number}`, alice);
136 const body = await page.text();
137 expect(body).toContain(`combined/${number}`);
138 expect(body).toContain("one.txt: 1 block, main&#39;s side kept");
139
140 expect(body).toContain("your side was dropped from the landed tree");
141 expect(body).toContain("To get your side back");
142 expect(body).toContain("Checks run on the pull&#39;s head, not on this combined commit.");
143 expect(body).not.toContain("Stale: main has moved");
144
145 // The terminal's pull screen says the same (#9012).
146 const screenText = async (): Promise<string> => {
147 const user = getUserByName(app.db, "alice")!;
148 const { app: tui, session } = createForgeApp(
149 app,
150 { sshUsername: "alice", remoteAddress: "127.0.0.1", user },
151 { terminal: headlessTerminal(200, 80), exitHooks: false },
152 );
153 tui.start();
154 await settleApp(tui);
155 tui.push(pullScreen(session, tui.ctx, local(session, repo), number));
156 await settleApp(tui);
157 const out = tui.screen.toText();
158 tui.stop();
159 return out;
160 };
161 const shown = await screenText();
162 expect(shown).toContain("Combined with main");
163 expect(shown).toContain(`combined/${number} (${tip.slice(0, 10)})`);
164 expect(shown).toContain("one.txt: 1 block, main's side kept; your side was dropped");
165
166 // A push to the fork recombines: the new head is the branch's first parent.
167 await Bun.write(join(b, "three.txt"), "three\n");
168 await git(["add", "."], b);
169 await git(["commit", "-m", "bob's three"], b);
170 await git(["push", "origin", "main"], b);
171 const bobNext = await git(["rev-parse", "HEAD"], b);
172 // The push's own follow may have refreshed it already; this is the same door.
173 await refreshPullHead(app, repo, getPull(app.db, repo.id, number)!);
174 let retip = "";
175 for (let i = 0; i < 40; i++) {
176 retip = await git(["rev-parse", `refs/heads/combined/${number}`], bare);
177 if ((await git(["log", "-1", "--format=%P", retip], bare)).startsWith(bobNext)) break;
178 await Bun.sleep(50);
179 }
180 expect(getPull(app.db, repo.id, number)!.headOid).toBe(bobNext);
181 expect((await git(["log", "-1", "--format=%P", retip], bare)).split(" ")).toEqual([bobNext, aliceMain]);
182 expect(await previewCommit(bare, getPull(app.db, repo.id, number)!)).toBe(retip);
183
184 // Main moves on its own: the card says the preview is stale, and on what it was built.
185 await Bun.write(join(a, "four.txt"), "four\n");
186 await git(["add", "."], a);
187 await git(["commit", "-m", "alice's four"], a);
188 await git(["push", "origin", "main"], a);
189 const stale = await (await call(`/alice/app/pulls/${number}`, alice)).text();
190 expect(stale).toContain(`Stale: main has moved since ${aliceMain.slice(0, 10)}`);
191 expect(await screenText()).toContain("Combined with main · stale");
192
193 // The merge lands the combined branch: his head is an ancestor of main.
194 const merged = await call(`/api/v1/repos/alice/app/pulls/${number}/merge`, alice, { strategy: "merge" });
195 expect(merged.status).toBe(200);
196 await git(["pull", "--no-rebase", "origin", "main"], a);
197 await git(["merge-base", "--is-ancestor", bobNext, "HEAD"], a);
198 expect(await Bun.file(join(a, "four.txt")).text()).toBe("four\n");
199 expect(await Bun.file(join(a, "one.txt")).text()).toBe("alice\n");
200 }, 30_000);
201 });
test/combined-git.test.ts added +98
@@ -0,0 +1,98 @@
1 /** The combined branch, in git alone (#9012): what a recombine keeps, what it leaves open, whose approval it runs under. */
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 { combine, headUnder, parseCombined, readCombined } from "../src/git/combined.ts";
7 import { gitClientEnv } from "./helpers/git-client.ts";
8
9 const env = gitClientEnv({
10 GIT_AUTHOR_NAME: "Jay Moran",
11 GIT_AUTHOR_EMAIL: "[email protected]",
12 GIT_COMMITTER_NAME: "Jay Moran",
13 GIT_COMMITTER_EMAIL: "[email protected]",
14 });
15 let dir = "";
16 /** The repository itself, as the forge holds it: refs read from files, as in a bare one. */
17 const gitDir = (): string => join(dir, ".git");
18
19 function git(...args: string[]): string {
20 const p = Bun.spawnSync(["git", ...args], { cwd: dir, env, stdout: "pipe", stderr: "pipe" });
21 if (p.exitCode !== 0) throw new Error(`git ${args.join(" ")}: ${p.stderr.toString()}`);
22 return p.stdout.toString().trim();
23 }
24 function commitOn(branch: string, files: Record<string, string | null>, message: string): string {
25 if (git("symbolic-ref", "--short", "HEAD") !== branch) git("checkout", "-q", branch);
26 for (const [path, text] of Object.entries(files))
27 if (text === null) git("rm", "-q", path);
28 else {
29 Bun.spawnSync(["sh", "-c", 'printf "%s" "$1" > "$2"', "_", text, path], { cwd: dir });
30 git("add", path);
31 }
32 git("commit", "-q", "-m", message);
33 return git("rev-parse", "HEAD");
34 }
35
36 beforeAll(async () => {
37 dir = await mkdtemp(join(tmpdir(), "forge-combined-git-"));
38 git("init", "-q", "-b", "main");
39 commitOn("main", { "one.txt": "one\n", "gone.txt": "gone\n" }, "seed");
40 git("branch", "fork");
41 });
42 afterAll(async () => {
43 await rm(dir, { recursive: true, force: true });
44 });
45
46 test("a modify/delete conflict is left open, a line conflict keeps main's side", async () => {
47 const head = commitOn("fork", { "one.txt": "fork\n", "gone.txt": "changed\n" }, "fork edits");
48 const base = commitOn("main", { "one.txt": "main\n", "gone.txt": null }, "main deletes");
49 const made = await combine(gitDir(), { number: 7, baseOid: base, headOid: head });
50 expect(made).toBeDefined();
51 const files = [...(made!.files ?? [])].sort((a, b) => a.path.localeCompare(b.path));
52 expect(files).toEqual([
53 { path: "gone.txt", blocks: 0, kept: "open" },
54 { path: "one.txt", blocks: 1, kept: "main" },
55 ]);
56 expect(git("show", `${made!.oid}:one.txt`)).toBe("main");
57 // The page and the terminal read the same facts back, from trailers.
58 const read = await readCombined(gitDir(), 7, head);
59 expect(read?.baseOid).toBe(base);
60 expect([...read!.files].sort((a, b) => a.path.localeCompare(b.path))).toEqual(files);
61 const note = git("log", "-1", "--format=%B", made!.oid);
62 expect(note).toContain("your side was dropped from the landed tree");
63 expect(note).toContain("gone.txt: left open");
64 });
65
66 test("a writer's commit on the combined branch survives main moving on", async () => {
67 const head = git("rev-parse", "fork");
68 const tip = git("rev-parse", "combined/7");
69 const fixed = commitOn("combined/7", { "one.txt": "both\n", "gone.txt": "changed\n" }, "a writer resolves");
70 expect(git("rev-parse", "combined/7")).not.toBe(tip);
71 const base = commitOn("main", { "two.txt": "two\n" }, "main moves");
72 const again = await combine(gitDir(), { number: 7, baseOid: base, headOid: head });
73 expect(git("log", "-1", "--format=%P", again!.oid).split(" ")).toEqual([fixed, base]);
74 expect(git("show", `${again!.oid}:one.txt`)).toBe("both");
75 expect(git("show", `${again!.oid}:two.txt`)).toBe("two");
76 });
77
78 test("a combined preview runs under the head it carries, and nothing else does", () => {
79 const head = git("rev-parse", "fork");
80 const tip = git("rev-parse", "combined/7");
81 const pull = { crossRepo: true, number: 7, headOid: head };
82 expect(headUnder(gitDir(), pull, tip)).toBe(head);
83 // Not the combined tip: it answers for itself, so its own approval is asked.
84 const main = git("rev-parse", "main");
85 expect(headUnder(gitDir(), pull, main)).toBe(main);
86 // A pull whose head the branch no longer carries: the tip answers for itself.
87 const elsewhere = git("commit-tree", "4b825dc642cb6eb9a060e54bf8d69288fbee4904", "-m", "elsewhere");
88 expect(headUnder(gitDir(), { ...pull, headOid: elsewhere }, tip)).toBe(tip);
89 // A branch pull is never combined.
90 expect(headUnder(gitDir(), { ...pull, crossRepo: false }, tip)).toBe(tip);
91 });
92
93 test("trailers that are not ours are not read as facts", () => {
94 expect(parseCombined("Combined-Base: nope\nCombined-Kept: maybe 1 x\nCombined-Kept: main 2 ../x")).toEqual({
95 baseOid: "",
96 files: [],
97 });
98 });
test/spawn-pipes.test.ts +1
@@ -24,6 +24,7 @@ const PIPING: Record<string, { sites: string[]; reason: string }> = {
2424 "src/crew/sandbox.ts": { sites: ["stderr"], reason: "stderr awaited with the exit; stdout ignored" },
2525 "src/crew/workdir.ts": { sites: ["stderr"], reason: "stderr read to its end alongside the exit; stdout ignored" },
2626 "src/git/env.ts": { sites: ["stdout+stderr", "stdout+stderr"], reason: "stdout and stderr both read to the end, finally clears the timer" },
27 "src/git/combined.ts": { sites: ["stdout"], reason: "spawnSync buffers rev-parse stdout; stderr ignored" },
2728 "src/git/local.ts": { sites: ["stdout+stderr", "stdout+stderr"], reason: "stdout and stderr both read to the end" },
2829 "src/git/read.ts": { sites: ["stdout", "stdout", "stdout"], reason: "cat-file stdout is read by the long-lived reader, by blobStream until its window ends or the reader hangs up, and by blobHead until its bytes are in (then killed); stderr ignored" },
2930 "src/git/smart-http.ts": { sites: ["stdout"], reason: "stdout is a pull-pipe FIFO (the response body, read on demand); stderr ignored" },

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs