Public: anyone can read this; the crew can change it.
A release never moves main: the release record leaves the tree, and a base move disjoint from a gate does not void it #6641
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6640-a-release-never-moves-main-the-release-r → main 3 commits fork 3 threads · 3 unresolved
2 files
changed +144
−2
src/checks/queue.ts +52 −2
| @@ -951,12 +951,19 @@ function split<T>(steps: readonly T[], k: number): T[][] { | ||
| 951 | 951 | * Whether every step landed. |
| 952 | 952 | */ |
| 953 | 953 | async function land(candidate: Candidate, steps: readonly Waiting[]): Promise<boolean> { |
| 954 | let at = candidate.baseOid; | |
| 954 | const moved = await movedDisjoint(candidate); | |
| 955 | let at = moved ?? candidate.baseOid; | |
| 955 | 956 | let landed: Queued | undefined; |
| 956 | 957 | try { |
| 957 | 958 | for (const [i, step] of candidate.steps.entries()) { |
| 958 | 959 | const w = steps[i]!; |
| 959 | const tip = await step.entry.land({ baseOid: at, treeOid: step.treeOid }); | |
| 960 | // On a moved base the tree is the step's merge on the tip it lands on: | |
| 961 | // a clean merge there is the re-verification the move needs. | |
| 962 | const rebased = moved === undefined ? undefined : await mergeable(candidate.dir, at, step.entry.pull.headOid); | |
| 963 | const tip = | |
| 964 | rebased !== undefined && !rebased.ok | |
| 965 | ? undefined | |
| 966 | : await step.entry.land({ baseOid: at, treeOid: rebased?.value.treeOid ?? step.treeOid }); | |
| 960 | 967 | if (tip === undefined) { |
| 961 | 968 | log.warn("merge queue: a gated step did not land", { |
| 962 | 969 | pull: step.entry.pull.number, |
| @@ -976,6 +983,49 @@ async function land(candidate: Candidate, steps: readonly Waiting[]): Promise<bo | ||
| 976 | 983 | } |
| 977 | 984 | } |
| 978 | 985 | |
| 986 | /** | |
| 987 | * The base's tip, when it moved since the candidate was gated and the move | |
| 988 | * touched only the release record, which the candidate does not change: the | |
| 989 | * green gate still speaks for the candidate, so it lands on that tip | |
| 990 | * instead of being voided. Undefined when the base is where it was, or the | |
| 991 | * move overlaps the candidate, or anything cannot be read. | |
| 992 | */ | |
| 993 | async function movedDisjoint(candidate: Candidate): Promise<string | undefined> { | |
| 994 | const first = candidate.steps[0]; | |
| 995 | if (first === undefined) return undefined; | |
| 996 | const { dir } = candidate; | |
| 997 | const tip = await resolveOid(dir, `refs/heads/${first.entry.pull.baseBranch}`); | |
| 998 | if (tip === undefined || tip === candidate.baseOid) return undefined; | |
| 999 | // Only a move forward of the gated base: a rewind is not a move to land on. | |
| 1000 | const ahead = await runGit(["merge-base", "--is-ancestor", candidate.baseOid, tip], { cwd: dir }); | |
| 1001 | if (!ahead.ok) return undefined; | |
| 1002 | const theirs = await changedFiles(dir, candidate.baseOid, tip); | |
| 1003 | const ours = await changedFiles(dir, candidate.baseOid, candidate.tipOid); | |
| 1004 | if (theirs === undefined || ours === undefined) return undefined; | |
| 1005 | // Clear of the candidate is not enough: files that share no path can | |
| 1006 | // still break each other. Only a move of inert paths keeps the gate. | |
| 1007 | if (!theirs.every(inert)) return undefined; | |
| 1008 | if (theirs.some((f) => ours.includes(f))) return undefined; | |
| 1009 | log.info("merge queue: the base moved clear of the candidate; it lands on the new tip", { | |
| 1010 | from: candidate.baseOid.slice(0, 8), | |
| 1011 | to: tip.slice(0, 8), | |
| 1012 | files: theirs.length, | |
| 1013 | }); | |
| 1014 | return tip; | |
| 1015 | } | |
| 1016 | ||
Drop the docs/*.md clause: tests read docs, so a docs-only move can break a gate that is already green, and the re-merged tree lands without a gate. |
||
| 1017 | /** The release record: the one file a release cut moves that no gated test depends on. */ | |
| 1018 | function inert(path: string): boolean { | |
| 1019 | return path === "src/core/releases.json"; | |
| 1020 | } | |
| 1021 | ||
A hard-coded path is a stopgap. Delete this exemption when #6642 moves the release record out of the tree, and name #6642 in the comment so it is not forgotten. |
||
| 1022 | /** The paths that differ between two commits, or undefined when git cannot say. */ | |
| 1023 | async function changedFiles(dir: string, from: string, to: string): Promise<string[] | undefined> { | |
| 1024 | const out = await runGit(["diff", "--name-only", "-z", "--no-renames", from, to], { cwd: dir }); | |
| 1025 | if (!out.ok) return undefined; | |
| 1026 | return out.value.split("\0").filter((f) => f !== ""); | |
| 1027 | } | |
| 1028 | ||
| 979 | 1029 | /** |
| 980 | 1030 | * The candidate of `batch` on its base's tip now — or on `on`'s tip, to |
| 981 | 1031 | * gate it ahead of `on` landing: each pull's squash on the step before it. |
test/merge-queue.test.ts +92
| @@ -358,6 +358,98 @@ describe("the merge queue", () => { | ||
| 358 | 358 | expect(tree(repo, "refs/heads/main")).toBe(tree(repo, gatesOf(repo)[0]!.commit)); |
| 359 | 359 | }); |
| 360 | 360 | |
| 361 | test("a release cut while a gate is green and in flight does not void it: the pulls land on the new tip, one gate (#6640)", async () => { | |
| 362 | const repo = await autonomousRepo("release-mid-gate"); | |
| 363 | const pulls = []; | |
| 364 | for (const name of ["a.ts", "b.ts"]) pulls.push(await greenPull(repo, name)); | |
| 365 | // The release record is pushed to main while the gate runs. | |
| 366 | held = (async () => { | |
| 367 | const cut = await commitToBranch(app, alice, repo, { | |
| 368 | message: "Release 0.0.1", | |
| 369 | files: [{ path: "src/core/releases.json", content: "[]\n" }], | |
| 370 | }); | |
| 371 | if (!cut.ok) throw new Error(JSON.stringify(cut.error)); | |
| 372 | })(); | |
| 373 | const base = git(repo, "rev-parse", "refs/heads/main"); | |
| 374 | try { | |
| 375 | expect(await sweepGreen(app, repo)).toBe(2); | |
| 376 | } finally { | |
| 377 | held = undefined; | |
| 378 | } | |
| 379 | expect(git(repo, "rev-parse", "refs/heads/main")).not.toBe(base); | |
| 380 | expect(gatesOf(repo)).toHaveLength(1); | |
| 381 | for (const pull of pulls) expect(merged(repo, pull)).toBe(true); | |
| 382 | const files = git(repo, "ls-tree", "--name-only", "refs/heads/main"); | |
| 383 | expect(git(repo, "ls-tree", "-r", "--name-only", "refs/heads/main")).toContain("src/core/releases.json"); | |
| 384 | expect(files).toContain("a.ts"); | |
| 385 | expect(files).toContain("b.ts"); | |
| 386 | }); | |
| 387 | ||
| 388 | test("a base move that touches a file the candidate changes still voids the gate (#6640)", async () => { | |
| 389 | const repo = await autonomousRepo("overlap-mid-gate"); | |
| 390 | const pull = await greenPull(repo, "a.ts"); | |
| 391 | held = (async () => { | |
| 392 | const moved = await commitToBranch(app, alice, repo, { | |
| 393 | message: "Same file", | |
| 394 | files: [{ path: "a.ts", content: "export const n = 'other';\n" }], | |
| 395 | }); | |
| 396 | if (!moved.ok) throw new Error(JSON.stringify(moved.error)); | |
| 397 | })(); | |
| 398 | const base = git(repo, "rev-parse", "refs/heads/main"); | |
| 399 | try { | |
| 400 | await sweepGreen(app, repo); | |
| 401 | } finally { | |
| 402 | held = undefined; | |
| 403 | } | |
| 404 | expect(merged(repo, pull)).toBe(false); | |
| 405 | // Main moved by the competing commit alone, and nothing is left half-landed. | |
| 406 | expect(git(repo, "rev-list", "--count", `${base}..refs/heads/main`)).toBe("1"); | |
| 407 | expect(git(repo, "log", "-1", "--format=%s", "refs/heads/main")).toBe("Same file"); | |
| 408 | expect(gatesOf(repo).length).toBeLessThanOrEqual(1); | |
| 409 | }); | |
| 410 | ||
| 411 | test("a disjoint move of live files that breaks the gate is no exemption: the pull does not land (#6640)", async () => { | |
| 412 | const repo = await autonomousRepo("breaking-mid-gate"); | |
| 413 | const pull = await greenPull(repo, "a.ts"); | |
| 414 | held = (async () => { | |
| 415 | const moved = await commitToBranch(app, alice, repo, { | |
| 416 | message: "Breaks the gate", | |
| 417 | files: [{ path: "red-other.ts", content: "export const x = 1;\n" }], | |
| 418 | }); | |
| 419 | if (!moved.ok) throw new Error(JSON.stringify(moved.error)); | |
| 420 | })(); | |
| 421 | try { | |
| 422 | await sweepGreen(app, repo); | |
| 423 | } finally { | |
| 424 | held = undefined; | |
| 425 | } | |
| 426 | expect(merged(repo, pull)).toBe(false); | |
| 427 | expect(git(repo, "ls-tree", "-r", "--name-only", "refs/heads/main")).not.toContain("a.ts"); | |
| 428 | }); | |
| 429 | ||
| 430 | test("a docs-only move voids the gate: tests read docs, so the pull is gated again on the new base (#6640)", async () => { | |
| 431 | const repo = await autonomousRepo("docs-mid-gate"); | |
| 432 | const pull = await greenPull(repo, "a.ts"); | |
| 433 | held = (async () => { | |
| 434 | const moved = await commitToBranch(app, alice, repo, { | |
| 435 | message: "A doc", | |
| 436 | files: [{ path: "docs/NOTE.md", content: "# note\n" }], | |
| 437 | }); | |
| 438 | if (!moved.ok) throw new Error(JSON.stringify(moved.error)); | |
| 439 | })(); | |
| 440 | try { | |
| 441 | await sweepGreen(app, repo); | |
| 442 | } finally { | |
| 443 | held = undefined; | |
| 444 | } | |
| 445 | // Voided: not landed on the gate that ran before the move. | |
| 446 | expect(merged(repo, pull)).toBe(false); | |
| 447 | expect(gatesOf(repo)).toHaveLength(1); | |
| 448 | await sweepGreen(app, repo); | |
| 449 | expect(gatesOf(repo)).toHaveLength(2); | |
| 450 | expect(merged(repo, pull)).toBe(true); | |
| 451 | }); | |
| 452 | ||
| 361 | 453 | test("a gated step lands only on the base and as the tree it was gated with", async () => { |
| 362 | 454 | const repo = await autonomousRepo("guarded"); |
| 363 | 455 | const pull = await greenPull(repo, "guard.ts"); |
new above
offline
This merge is the only verification a moved base gets. Gate this tree with the fast re-check, or limit movedDisjoint to inert paths.