trevato/forge

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

More
Forks1g fMapg aRuns13g rEventsFlowsAPI

Four more ways a landing goes wrong, found by the landing simulator: stale co… #12181

Merged torquecrew opened this in answer to trevatoowner

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

ConversationFiles
Merged

torque/forge:torque/12151-four-more-ways-a-landing-goes-wrong-foun → main 4 commits fork

  • Merged. torque/12151-four-more-ways-a-landing-goes-wrong-foun was merged into main as 7189963.

Review

Sign in to review this change.
12 files changed +225 −43
docs/features/combined-rebuild-stopped.md added +13
@@ -0,0 +1,13 @@
1 # A rebuild that stopped
2
3 A kept fork's pull is built as main merged into its head (`combined/N`). When main deletes, renames or changes the mode of a file the pull also changed, no side can be picked for it, and nothing lands that nobody chose (#12151). The rebuild stops and the pull says so:
4
5 - "The rebuild stopped: gone.txt was deleted, renamed or changed in a way that conflicts with main, and no side is picked for it. Push to the fork with it resolved."
6 - If `combined/N` holds only the forge's own combinings, it is deleted: the card reads "Not combined with main" and the pull answers as its head does (conflicting with main).
7 - If it carries someone's own commits, it is kept as it was, and the card reads "Combined with main · stale" with the sentence under it.
8 - Other stops word themselves: "The rebuild failed: the pull's head is missing…", "…git could not merge main into the pull's head", and "The rebuild failed unexpectedly" (the detail is in the log). A good rebuild clears the sentence.
9 - A landing rebuilds the other kept forks' branches as a push to main does; passes never overlap, and calls during one share it (`core/recombine.ts`).
10
11 - **Web:** the combined card on the pull. **Terminal:** the same card in `forge tui`'s pull screen. Both read `combinedView()` and `combinedWords()` in `src/core/combined_view.ts`.
12 - Try it: open a pull from a kept fork that edits a file, delete that file on main, open the pull.
13 - Tests: `test/combined-git.test.ts`, `test/combined-recombine.test.ts`.
src/actions/pulls.ts +8
@@ -4,6 +4,7 @@
44 * issues — a pull is an issue with a diff.
55 */
66
7 import { recombine } from "../core/recombine.ts";
78 import { teamsHolding } from "../core/issue_teams.ts";
89 import { isContributionFork, type GuestRefusal } from "../core/contribute.ts";
910 import { takeGuestPull } from "./guests.ts";
@@ -592,6 +593,13 @@ export async function mergePull(
592593 },
593594 );
594595 if (!merged.ok) return merged;
596 // A landing moves the default branch as a push to it does: every other kept
597 // fork's `combined/N` is built on the old tip, and its check with it, so
598 // they rebuild now (`core/recombine.ts`). Off the answer, as the hook's is.
599 if (pull.baseBranch === repo.defaultBranch)
600 void recombine({ db: app.db, reposRoot: app.reposRoot, log: app.log }, repo.id).catch((error) =>
601 app.log.warn("could not recombine after a landing", { repo: `${repo.ownerName}/${repo.name}`, error: String(error) }),
602 );
595603 // Its author, when they live elsewhere, reads what landed and what changed on the way (#9013).
596604 await thankAuthor(app, repo, getPull(app.db, repo.id, pull.number) ?? pull, {
597605 mergeOid: merged.value.mergeOid,
src/core/combined_view.ts +15 −2
@@ -6,7 +6,7 @@
66 */
77
88 import type { Pull } from "./pulls.ts";
9 import { keptLine, readCombined, readFailure, TAKE_BACK, type Kept } from "../git/combined.ts";
9 import { combinedBranch, keptLine, readCombined, readFailure, TAKE_BACK, type Kept } from "../git/combined.ts";
1010
1111 export interface CombinedView {
1212 readonly branch: string;
@@ -29,8 +29,15 @@ export async function combinedView(
2929 ): Promise<CombinedView | undefined> {
3030 if (!pull.crossRepo || pull.state !== "open" || pull.mergedAt !== null) return undefined;
3131 const read = await readCombined(dir, pull.number, headOid);
32 if (read === undefined) {
33 // A refusal deleted the branch: only its sentence is left to say (#12151).
34 const failure = (await readFailure(dir, pull.number)) ?? "";
35 return failure === ""
36 ? undefined
37 : { branch: combinedBranch(pull.number), oid: "", builtOn: "", stale: false, files: [], failure };
38 }
3239 // Main already in the head: the preview is the head itself, nothing to say.
33 if (read === undefined || read.oid === headOid) return undefined;
40 if (read.oid === headOid) return undefined;
3441 const stale = read.baseOid !== "" && read.baseOid !== baseOid;
3542 return {
3643 failure: stale ? ((await readFailure(dir, pull.number)) ?? "") : "",
@@ -48,6 +55,12 @@ export function combinedWords(
4855 number: number,
4956 ): { lead: string; files: string[]; notes: string[] } {
5057 const notes: string[] = [];
58 if (view.oid === "")
59 return {
60 lead: "There is no combined preview: main could not be merged into this pull's head.",
61 files: [],
62 notes: [view.failure],
63 };
5164 if (view.stale)
5265 notes.push(
5366 `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.`,
src/core/pulls.ts +9 −8
@@ -1177,7 +1177,7 @@ export async function mergePull(
11771177 // What the pull proposes is already on the base: there is nothing to
11781178 // land, so nothing is committed — the pull closes as merged into the
11791179 // commit that carried it, and the base does not move.
1180 const carrier = await carriedBy(dir, pull, status, input.strategy);
1180 const carrier = await carriedBy(dir, pull, status);
11811181 if (carrier !== undefined) {
11821182 closeAsMerged(db, pull.id, {
11831183 mergeOid: carrier,
@@ -1239,19 +1239,20 @@ export async function mergePull(
12391239 }
12401240
12411241 /**
1242 * The commit on the base that already carries a squash of this pull, when
1243 * squashing it again would commit the base's own tree: the pull's own
1244 * landing if the base still holds it, else the base's tip. Undefined when
1245 * the squash would change something — or is not a squash, or does not
1246 * merge cleanly, which the merge itself reports.
1242 * The commit on the base that already carries this pull's changes, when
1243 * landing it again, squashed or merged, would commit the base's own tree: the
1244 * pull's own landing if the base still holds it, else the base's tip. A
1245 * pull stacked on another lands that way when the one above it landed
1246 * first (#12151): it closes as merged, its author credited, and the base
1247 * does not move. Undefined when landing would change something, or the
1248 * pull does not merge cleanly, which the merge itself reports.
12471249 */
12481250 async function carriedBy(
12491251 dir: string,
12501252 pull: Pull,
12511253 status: { baseOid: string; headOid: string; ahead: number; mergeable: boolean },
1252 strategy: MergeStrategy,
12531254 ): Promise<string | undefined> {
1254 if (strategy !== "squash" || status.ahead === 0 || !status.mergeable)
1255 if (status.ahead === 0 || !status.mergeable)
12551256 return undefined;
12561257 if (!(await contentOnBase(dir, status.baseOid, status.headOid)))
12571258 return undefined;
src/core/recombine.ts +12 −8
@@ -7,7 +7,7 @@
77 */
88
99 import type { Database } from "bun:sqlite";
10 import { combine, setFailure } from "../git/combined.ts";
10 import { combine, readFailure, setFailure } from "../git/combined.ts";
1111 import { repoDir } from "../git/repos.ts";
1212 import { resolveOid } from "../git/merge.ts";
1313 import type { Log } from "../kernel/log.ts";
@@ -55,13 +55,17 @@ async function pass(forge: RecombineForge, repoId: string): Promise<void> {
5555 headOid === undefined ? undefined : await combine(dir, { number: pull.number, baseOid, headOid });
5656 if (made === undefined) {
5757 forge.log.warn("recombine skipped a pull", { repo: repoId, pull: pull.number });
58 await setFailure(
59 dir,
60 pull.number,
61 headOid === undefined
62 ? "The rebuild failed: the pull's head is missing from the repository. Push to the fork again to rebuild."
63 : "The rebuild failed: git could not merge main into the pull's head. Push to the fork to rebuild it.",
64 );
58 // A refusal has written its own sentence (naming the files);
59 // only an unexplained one gets this.
60 if ((await readFailure(dir, pull.number)) === undefined) {
61 await setFailure(
62 dir,
63 pull.number,
64 headOid === undefined
65 ? "The rebuild failed: the pull's head is missing from the repository. Push to the fork again to rebuild."
66 : "The rebuild failed: git could not merge main into the pull's head. Push to the fork to rebuild it.",
67 );
68 }
6569 }
6670 } catch (e) {
6771 const error = e instanceof Error ? e.message : String(e);
src/git/combined.ts +26 −4
@@ -40,6 +40,12 @@ const IDENTITY = {
4040 const isAncestor = async (dir: string, a: string, b: string): Promise<boolean> =>
4141 a === b || (await mergeBase(dir, a, b)) === a;
4242
43 /** Whether the branch has a commit beyond the head that the forge's own combining did not make. */
44 async function carriesWriter(dir: string, headOid: string, tip: string): Promise<boolean> {
45 const out = await runGit(["log", "--format=%ae", `${headOid}..${tip}`], { cwd: dir });
46 return out.ok && out.value.split("\n").some((e) => e.trim() !== "" && e.trim() !== "forge@localhost");
47 }
48
4349 /** The note the author reads: what was combined and what was kept, file by file. */
4450 export function combinedNote(number: number, baseOid: string, files: readonly Kept[]): string {
4551 const lines = [
@@ -120,10 +126,9 @@ export async function combine(
120126 dir: string,
121127 input: { readonly number: number; readonly baseOid: string; readonly headOid: string },
122128 ): Promise<{ oid: string; files: Kept[] | null } | undefined> {
123 const made = await build(dir, input);
124 // A rebuild that worked ends any earlier failure's sentence, whoever asked for it.
125 if (made !== undefined) await setFailure(dir, input.number, undefined);
126 return made;
129 // Any earlier failure's sentence ends with this try; a refusal writes its own.
130 await setFailure(dir, input.number, undefined);
131 return build(dir, input);
127132 }
128133
129134 async function build(
@@ -159,6 +164,23 @@ async function build(
159164 files.push({ path, blocks, kept: open.has(path) || blocks === 0 ? "open" : "main" });
160165 }
161166 }
167 // A delete, a rename or a mode change that meets a change on main has no
168 // side git can pick, and `-X ours` leaves whatever it likes (the file kept,
169 // or under both names, or back at its old mode). Nothing lands that nobody
170 // chose (#12151): no rebuild is written. A branch of forge's own combinings
171 // goes, and the pull answers as its head does (conflicting with main); one
172 // that carries a writer's commits stays, as they are theirs to keep.
173 const unresolved = files.filter((f) => f.kept === "open").map((f) => f.path);
174 if (unresolved.length > 0) {
175 if (prior !== undefined && !(await carriesWriter(dir, headOid, prior)))
176 await runGit(["update-ref", "-d", ref], { cwd: dir });
177 await setFailure(
178 dir,
179 number,
180 `The rebuild stopped: ${unresolved.join(", ")} was deleted, renamed or changed in a way that conflicts with main, and no side is picked for it. Push to the fork with it resolved.`,
181 );
182 return undefined;
183 }
162184 const commit = await runGit(
163185 ["commit-tree", tree, "-p", from, "-p", baseOid, "-m", combinedNote(number, baseOid, files)],
164186 { cwd: dir, env: IDENTITY },
src/tui/screens/pull.ts +1 −1
@@ -2088,7 +2088,7 @@ function combinedLines(
20882088 wrap(text, indent).map((t): Span[] => [{ text: t, style: style(fg, c.bg) }]);
20892089 return [
20902090 [],
2091 [{ text: ` Combined with main${view.stale ? " · stale" : ""}`, style: style(c.fg, c.bg, BOLD) }],
2091 [{ text: ` ${view.oid === "" ? "Not combined with main" : "Combined with main"}${view.stale ? " · stale" : ""}`, style: style(c.fg, c.bg, BOLD) }],
20922092 ...say(words.lead, " ", c.fg),
20932093 ...words.files.flatMap((l) => say(`- ${l}`, " ", c.fg)),
20942094 ...words.notes.flatMap((l) => say(l, " ", c.mutedFg)),
src/web/pages/pull-combined.ts +1 −1
@@ -26,7 +26,7 @@ export async function combinedCard(
2626 {
2727 extra: "mb",
2828 head: html`${icon("merge")}
29 <h2>Combined with main${view.stale ? " · stale" : ""}</h2>`,
29 <h2>${view.oid === "" ? "Not combined with main" : "Combined with main"}${view.stale ? " · stale" : ""}</h2>`,
3030 },
3131 );
3232 }
test/combined-git.test.ts +33 −12
@@ -3,7 +3,7 @@ import { afterAll, beforeAll, expect, test } from "bun:test";
33 import { mkdtemp, rm } from "node:fs/promises";
44 import { tmpdir } from "node:os";
55 import { join } from "node:path";
6 import { combine, headUnder, parseCombined, readCombined } from "../src/git/combined.ts";
6 import { combine, headUnder, parseCombined, readCombined, readFailure } from "../src/git/combined.ts";
77 import { gitClientEnv } from "./helpers/git-client.ts";
88
99 const env = gitClientEnv({
@@ -43,30 +43,39 @@ afterAll(async () => {
4343 await rm(dir, { recursive: true, force: true });
4444 });
4545
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");
46 test("a line conflict keeps main's side, and says so in trailers", async () => {
47 const head = commitOn("fork", { "one.txt": "fork\n" }, "fork edits");
48 const base = commitOn("main", { "one.txt": "main\n" }, "main edits");
4949 const made = await combine(gitDir(), { number: 7, baseOid: base, headOid: head });
5050 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 ]);
51 const files = [...(made!.files ?? [])];
52 expect(files).toEqual([{ path: "one.txt", blocks: 1, kept: "main" }]);
5653 expect(git("show", `${made!.oid}:one.txt`)).toBe("main");
5754 // The page and the terminal read the same facts back, from trailers.
5855 const read = await readCombined(gitDir(), 7, head);
5956 expect(read?.baseOid).toBe(base);
60 expect([...read!.files].sort((a, b) => a.path.localeCompare(b.path))).toEqual(files);
57 expect([...read!.files]).toEqual(files);
6158 const note = git("log", "-1", "--format=%B", made!.oid);
6259 expect(note).toContain("your side was dropped from the landed tree");
63 expect(note).toContain("gone.txt: left open");
60 });
61
62 test("a modify/delete conflict has no side to pick: no combined branch, and the reason is kept (#12151)", async () => {
63 git("branch", "doomed", "fork");
64 const head = commitOn("doomed", { "gone.txt": "changed\n" }, "fork edits gone.txt");
65 const base = commitOn("main", { "gone.txt": null }, "main deletes gone.txt");
66 expect(await combine(gitDir(), { number: 8, baseOid: base, headOid: head })).toBeUndefined();
67 expect(Bun.spawnSync(["git", "rev-parse", "--verify", "--quiet", "refs/heads/combined/8"], { cwd: gitDir() }).exitCode).not.toBe(0);
68 expect(await readFailure(gitDir(), 8)).toContain("gone.txt");
69 // A rebuild that works ends the sentence.
70 const fixed = commitOn("doomed", { "gone.txt": null }, "fork agrees");
71 expect(await combine(gitDir(), { number: 8, baseOid: base, headOid: fixed })).toBeDefined();
72 expect(await readFailure(gitDir(), 8)).toBeUndefined();
6473 });
6574
6675 test("a writer's commit on the combined branch survives main moving on", async () => {
6776 const head = git("rev-parse", "fork");
6877 const tip = git("rev-parse", "combined/7");
69 const fixed = commitOn("combined/7", { "one.txt": "both\n", "gone.txt": "changed\n" }, "a writer resolves");
78 const fixed = commitOn("combined/7", { "one.txt": "both\n" }, "a writer resolves");
7079 expect(git("rev-parse", "combined/7")).not.toBe(tip);
7180 const base = commitOn("main", { "two.txt": "two\n" }, "main moves");
7281 const again = await combine(gitDir(), { number: 7, baseOid: base, headOid: head });
@@ -96,3 +105,15 @@ test("trailers that are not ours are not read as facts", () => {
96105 files: [],
97106 });
98107 });
108
109 test("a combined branch carrying a writer's commits is kept when main later deletes a file they touched (#12151)", async () => {
110 git("branch", "wr", "main");
111 const head = commitOn("wr", { "two.txt": "fork\n" }, "fork edits two.txt");
112 const main = git("rev-parse", "main");
113 expect(await combine(gitDir(), { number: 9, baseOid: main, headOid: head })).toBeDefined();
114 const written = commitOn("combined/9", { "two.txt": "writer\n" }, "a writer resolves");
115 const base = commitOn("main", { "two.txt": null }, "main deletes two.txt");
116 expect(await combine(gitDir(), { number: 9, baseOid: base, headOid: head })).toBeUndefined();
117 expect(git("rev-parse", "combined/9")).toBe(written);
118 expect(await readFailure(gitDir(), 9)).toContain("two.txt");
119 });
test/combined-recombine.test.ts +70 −2
@@ -1,5 +1,5 @@
11 /** `recombine` (#10039): main moves, every kept-fork pull's combined branch follows, one pass at a time. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
2 import { afterAll, beforeAll, expect, spyOn, test } from "bun:test";
33 import { mkdtemp, rm } from "node:fs/promises";
44 import { tmpdir } from "node:os";
55 import { join } from "node:path";
@@ -8,9 +8,12 @@ import { serve, type Servers } from "../src/http/server.ts";
88 import { gitClientEnv } from "./helpers/git-client.ts";
99 import { proveAll } from "./helpers/verified.ts";
1010 import { promote } from "./helpers/members.ts";
11 import { combine, readFailure } from "../src/git/combined.ts";
11 import { combine, readFailure, setFailure } from "../src/git/combined.ts";
1212 import { idle, recombine, runsStarted } from "../src/core/recombine.ts";
1313 import { getRepo } from "../src/core/repos.ts";
14 import { getUserByName } from "../src/core/users.ts";
15 import { mergePull } from "../src/actions/pulls.ts";
16 import { getPull } from "../src/core/pulls.ts";
1417 import { repoDir } from "../src/git/repos.ts";
1518 import { tokenFor } from "./helpers/tokens.ts";
1619
@@ -240,3 +243,68 @@ test("a main git cannot merge puts its sentence on the combined card, and a fixe
240243 expect((await parents(`refs/heads/combined/${first}`))[1]).toBe(fixed);
241244 expect(head).toBe(await git(["rev-parse", "refs/pull/" + first + "/head"], bare));
242245 });
246
247 test("landing a pull rebuilds the other kept fork's combined branch, once, with a second call joining it (#12151)", async () => {
248 const user = getUserByName(app.db, "alice")!;
249 const repo = getRepo(app.db, "alice", "app")!;
250 await idle(repoId);
251 const before = await git(["rev-parse", `refs/heads/combined/${first}`], bare);
252 const runs = runsStarted(repoId);
253 const landed = await mergePull(app, user, repo, getPull(app.db, repo.id, second)!, "merge");
254 expect(landed.ok).toBe(true);
255 // A second call while the landing's own pass runs joins it: two passes at most, never two
256 // at once.
257 const joined = recombine(forge(), repoId);
258 await idle(repoId);
259 expect(await joined).toBeLessThanOrEqual(2);
260 expect(runsStarted(repoId)).toBe(runs + 1);
261 const main = await git(["rev-parse", "refs/heads/main"], bare);
262 expect(main).not.toBe(before);
263 expect((await parents(`refs/heads/combined/${first}`))[1]).toBe(main);
264 });
265
266 test("a refusal's own sentence is kept, not replaced by the generic one", async () => {
267 await git(["fetch", "-q", "origin"], a);
268 await git(["reset", "-q", "--hard", "origin/main"], a);
269 await git(["fetch", "-q", "origin"], b);
270 await git(["checkout", "-q", "-b", "three", "origin/main"], b);
271 await Bun.write(join(b, "one.txt"), "bob\n");
272 await git(["add", "."], b);
273 await git(["commit", "-m", "bob three"], b);
274 await git(["push", "origin", "three"], b);
275 const r = await call("/api/v1/repos/alice/app/pulls", bob, { title: "three", base: "main", head: "three", headRepo: "bob/app" });
276 expect(r.status).toBe(201);
277 const third = ((await r.json()) as { number: number }).number;
278 await git(["rm", "-q", "one.txt"], a);
279 await git(["commit", "-m", "main deletes one.txt"], a);
280 await git(["push", "origin", "main"], a);
281 await idle(repoId);
282 const said = await readFailure(bare, third);
283 expect(said).toContain("The rebuild stopped: one.txt");
284 expect(said).not.toContain("git could not merge");
285 // The page and the terminal word the card from one view: the sentence is on both.
286 const { combinedView, combinedWords } = await import("../src/core/combined_view.ts");
287 const head = await git(["rev-parse", `refs/pull/${third}/head`], bare);
288 const view = await combinedView(bare, getPull(app.db, repoId, third)!, head, await git(["rev-parse", "refs/heads/main"], bare));
289 expect(combinedWords(view!, third).notes).toEqual([said!]);
290 const page = (await (await call(`/alice/app/pulls/${third}`, alice)).text()).replace(/&#39;/g, "'");
291 expect(page).toContain("Not combined with main");
292 expect(page).toContain("The rebuild stopped: one.txt");
293 });
294
295 test("a build that throws still leaves a sentence", async () => {
296 const third = Number((await git(["config", "--local", "--get-regexp", "^recombine.failed"], bare)).split("\n").pop()!.split(" ")[0]!.replace("recombine.failed", ""));
297 await setFailure(bare, third, undefined);
298 const real = Bun.spawn;
299 const spy = spyOn(Bun, "spawn").mockImplementation(((cmd: string[], ...rest: unknown[]) => {
300 if (cmd.includes("merge-tree")) throw new Error("boom");
301 return (real as (...a: unknown[]) => unknown)(cmd, ...rest);
302 }) as typeof Bun.spawn);
303 try {
304 await moveMain("m9.txt");
305 await idle(repoId);
306 } finally {
307 spy.mockRestore();
308 }
309 expect(await readFailure(bare, third)).toContain("The rebuild failed unexpectedly");
310 });
test/lands-once.test.ts +33
@@ -13,6 +13,7 @@ import { join } from "node:path";
1313 import { createApp, type App } from "../src/app.ts";
1414 import { mergePull, openPull } from "../src/actions/pulls.ts";
1515 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
16 import { idle } from "../src/core/recombine.ts";
1617 import { getPull, type Pull } from "../src/core/pulls.ts";
1718 import { getRepoById, type Repo } from "../src/core/repos.ts";
1819 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
@@ -67,6 +68,7 @@ describe("a pull lands once", () => {
6768 });
6869
6970 afterAll(async () => {
71 await idle(repo.id);
7072 await app.close?.();
7173 await rm(dataDir, { recursive: true, force: true });
7274 });
@@ -173,4 +175,35 @@ describe("a pull lands once", () => {
173175 if (!again.ok) expect(again.error).toBe("already_merged");
174176 expect(mainTip()).toBe(tip);
175177 });
178
179 test("a stacked pull landing as a merge commit closes as merged, and main does not move (#12151)", async () => {
180 await push("feature/stack-a", "stack-a.ts", "export const a = 1;\n");
181 const upper = await commitToBranch(app, alice, repo, {
182 branch: "feature/stack-b",
183 base: "feature/stack-a",
184 message: "stack-b",
185 files: [{ path: "stack-b.ts", content: "export const b = 1;\n" }],
186 });
187 if (!upper.ok) throw new Error(JSON.stringify(upper.error));
188 const open = async (name: string): Promise<Pull> => {
189 const o = await openPull(app, alice, repo, {
190 title: name,
191 baseBranch: "main",
192 headBranch: `feature/${name}`,
193 });
194 if (!o.ok) throw new Error(String(o.error));
195 return getPull(app.db, repo.id, o.value.number)!;
196 };
197 const lower = await open("stack-a");
198 const top = await open("stack-b");
199 const landed = await mergePull(app, alice, fresh(), top, "squash");
200 expect(landed.ok).toBe(true);
201 const tip = mainTip();
202 const done = await mergePull(app, alice, fresh(), lower, "merge");
203 expect(done.ok).toBe(true);
204 expect(mainTip()).toBe(tip);
205 const closed = getPull(app.db, repo.id, lower.number)!;
206 expect(closed.state).toBe("closed");
207 expect(closed.mergedOid).toBe(tip);
208 });
176209 });
test/support/landsim.ts +4 −5
@@ -45,7 +45,7 @@ import {
4545 } from "../../src/core/runs.ts";
4646 import { commitIdentity } from "../../src/core/commit_identity.ts";
4747 import { getPull, pullStatus, type Pull } from "../../src/core/pulls.ts";
48 import { recombine } from "../../src/core/recombine.ts";
48 import { idle, recombine } from "../../src/core/recombine.ts";
4949 import { refreshPullHead } from "../../src/core/following.ts";
5050 import { getRepoById, type Repo } from "../../src/core/repos.ts";
5151 import { reviewSummary } from "../../src/core/reviews.ts";
@@ -1114,6 +1114,7 @@ export class Sim {
11141114 this.row(st),
11151115 step.how,
11161116 );
1117 await idle(this.repo.id);
11171118 if (!merged.ok)
11181119 this.refuse(
11191120 `land: ${typeof merged.error === "string" ? merged.error : merged.error.kind}`,
@@ -1127,6 +1128,7 @@ export class Sim {
11271128 world.onGate.set(this.repo.id, () => this.exec(during));
11281129 }
11291130 await sweepGreen(app, this.repo);
1131 await idle(this.repo.id);
11301132 world.onGate.delete(this.repo.id);
11311133 return;
11321134 }
@@ -1224,6 +1226,7 @@ export class Sim {
12241226 this.row(st),
12251227 st.how,
12261228 );
1229 await idle(this.repo.id);
12271230 if (!merged.ok) {
12281231 st.lastRefusal = typeof merged.error === "string" ? merged.error : merged.error.kind;
12291232 this.refuse(`settle: ${st.lastRefusal}`);
@@ -1682,10 +1685,6 @@ export const KNOWN_VIOLATIONS: readonly { prefix: string; issue: string; why: st
16821685 { prefix: "reviewedIsLanded/removed-line-survives-by-combine", issue: "#12104", why: "the same, for a line the pull removed or rewrote" },
16831686 { prefix: "noSilentLoss/dropped-by-combine", issue: "#12104", why: "the same, seen from main's side" },
16841687 { prefix: "noDroppedBlocksLand/combined-kept", issue: "#12104", why: "a tree with Combined-Kept blocks lands" },
1685 { prefix: "liveness/queue-named-never-lands-fork", issue: "unfiled", why: "after one pull lands, the other kept forks' combined/N are stale and landing fires no hook: the check is stale (checks_required), the pull says queue, and neither the queue nor a person can land it until something else pushes to main" },
1686 { prefix: "reviewedIsLanded/removed-line-survives", issue: "unfiled", why: "a kept fork's delete or rename that conflicts with main lands through combine with the file kept or under both names, so a line the pull removed survives (or a later pull's edit reaches one copy of it)" },
1687 { prefix: "reviewedIsLanded/mode-lost", issue: "unfiled", why: "a kept fork's chmod on a file main deleted lands through combine with the file back at the old mode" },
1688 { prefix: "authorshipKept/repo", issue: "unfiled", why: "a pull whose commits already landed inside a stacked pull lands again as an empty squash by the lander, crediting nobody" },
16891688 { prefix: "additiveMergesCommute/needs-a-person", issue: "#12104", why: "two in-repo pulls that only add lines at one place do not merge: git conflicts, and the pull waits for a crew run or a person" },
16901689 ];
16911690

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs