Public: anyone can read this; the crew can change it.
The landing gate itself lands an upstream pull as a merge, so the upstream's commits stay ancestors of main #4220
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3558-the-landing-gate-itself-lands-an-upstrea → main 1 commit fork
3 files
changed +54
−2
src/core/upstream_follow.ts +9
| @@ -97,6 +97,15 @@ export function getFollow(db: Database, repoId: string): UpstreamFollow | undefi | ||
| 97 | 97 | return row ? fromRow(row) : undefined; |
| 98 | 98 | } |
| 99 | 99 | |
| 100 | /** | |
| 101 | * Whether this pull is the one the follow opened for the upstream: its | |
| 102 | * commits must land as a merge, so the upstream's tip stays an ancestor of | |
| 103 | * the base and the next check reads nothing behind (#3558). | |
| 104 | */ | |
| 105 | export function isUpstreamPull(db: Database, pull: { id: string; repoId: string; headBranch: string }): boolean { | |
| 106 | return pull.headBranch.startsWith("upstream/") && getFollow(db, pull.repoId)?.pendingId === pull.id; | |
| 107 | } | |
| 108 | ||
| 100 | 109 | /** Every repository whose following is on. */ |
| 101 | 110 | export function listFollowing(db: Database): UpstreamFollow[] { |
| 102 | 111 | return db |
src/crew/autonomy.ts +4 −1
| @@ -30,6 +30,7 @@ import { mergeable, resolveOid, treeOf } from "../git/merge.ts"; | ||
| 30 | 30 | import { repoDir } from "../git/repos.ts"; |
| 31 | 31 | import { CLOSED_NOTE } from "../core/done_on_main.ts"; |
| 32 | 32 | import { mergePull } from "../actions/pulls.ts"; |
| 33 | import { isUpstreamPull } from "../core/upstream_follow.ts"; | |
| 33 | 34 | import { |
| 34 | 35 | listCrewRoles, |
| 35 | 36 | type Agent, |
| @@ -2017,7 +2018,9 @@ async function landGated( | ||
| 2017 | 2018 | by: "rule", |
| 2018 | 2019 | ms: 0, |
| 2019 | 2020 | }); |
| 2020 | const merged = await mergePull(app, by, repo, pull, "squash", expect); | |
| 2021 | // An upstream's commits are kept, not squashed: they stay ancestors of main. | |
| 2022 | const strategy = isUpstreamPull(app.db, pull) ? "merge" : "squash"; | |
| 2023 | const merged = await mergePull(app, by, repo, pull, strategy, expect); | |
| 2021 | 2024 | if (!merged.ok) { |
| 2022 | 2025 | const why = mergeWhy(merged.error); |
| 2023 | 2026 | log.info("green but not landed", { |
test/upstream_follow.test.ts +41 −1
| @@ -12,8 +12,11 @@ import { createApp, type App } from "../src/app.ts"; | ||
| 12 | 12 | import { createRepo } from "../src/actions/repos.ts"; |
| 13 | 13 | import { audienceFilter } from "../src/core/event_audience.ts"; |
| 14 | 14 | import { listEvents } from "../src/core/events.ts"; |
| 15 | import { provideChecksForTest, type ChecksHandle } from "../src/checks/service.ts"; | |
| 16 | import { landIfGreen } from "../src/crew/autonomy.ts"; | |
| 17 | import { claimRun, createRun, finishRun } from "../src/core/runs.ts"; | |
| 15 | 18 | import { getPull, listPulls } from "../src/core/pulls.ts"; |
| 16 | import type { Repo } from "../src/core/repos.ts"; | |
| 19 | import { getRepoById, type Repo } from "../src/core/repos.ts"; | |
| 17 | 20 | import { createUser, type User } from "../src/core/users.ts"; |
| 18 | 21 | import { checkDue, checkUpstream, mayCarryCredential } from "../src/actions/upstream_follow.ts"; |
| 19 | 22 | import { createOrigin } from "../src/core/origins.ts"; |
| @@ -37,6 +40,8 @@ let repo: Repo; | ||
| 37 | 40 | let target: Repo; |
| 38 | 41 | let dir = ""; |
| 39 | 42 | |
| 43 | const getRepoByIdFor = (id: string): Repo => getRepoById(app.db, id)!; | |
| 44 | ||
| 40 | 45 | const ok = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => { |
| 41 | 46 | if (!r.ok) throw new Error(JSON.stringify(r.error)); |
| 42 | 47 | return r.value; |
| @@ -323,6 +328,41 @@ describe("checking an upstream (layer 2)", () => { | ||
| 323 | 328 | } |
| 324 | 329 | }); |
| 325 | 330 | |
| 331 | test("the landing gate lands an upstream pull as a merge: the upstream's tip stays an ancestor of main", async () => { | |
| 332 | const t = await staged("lands", "base\n", "new\n"); | |
| 333 | try { | |
| 334 | // Main moved too, so a merge and a squash cannot be told apart by a fast-forward. | |
| 335 | const blob = await t.git(["hash-object", "-w", "--stdin"], "base\n"); | |
| 336 | const tree = await t.git(["mktree"], `100644 blob ${blob}\tf.txt\n100644 blob ${blob}\tg.txt\n`); | |
| 337 | const main = await t.git(["commit-tree", tree, "-p", t.main, "-m", "ours"]); | |
| 338 | await t.git(["update-ref", `refs/heads/${t.r.defaultBranch}`, main]); | |
| 339 | const opened = ok(await checkUpstream(app, t.r)); | |
| 340 | const pull = getPull(app.db, t.r.id, opened.pull!)!; | |
| 341 | app.db.query("UPDATE repo SET autonomy = 'autonomous', autonomy_by = ?2 WHERE id = ?1").run(t.r.id, alice.id); | |
| 342 | const run = createRun(app.db, { | |
| 343 | repoId: t.r.id, | |
| 344 | kind: "check", | |
| 345 | issueId: pull.id, | |
| 346 | env: `pull-${pull.number}`, | |
| 347 | commitOid: pull.headOid, | |
| 348 | reason: "pull", | |
| 349 | }); | |
| 350 | claimRun(app.db, run.id); | |
| 351 | finishRun(app.db, run.id, { state: "ok", summary: "check passed" }); | |
| 352 | provideChecksForTest(app, { gate: async () => ({ ok: true, error: "", lines: [] }) } as unknown as ChecksHandle); | |
| 353 | const fresh = getRepoByIdFor(t.r.id); | |
| 354 | expect(await landIfGreen(app, fresh, getPull(app.db, t.r.id, pull.number)!)).toBe(true); | |
| 355 | expect(getPull(app.db, t.r.id, pull.number)!.mergedAt).not.toBeNull(); | |
| 356 | const isAncestor = (await runGit(["merge-base", "--is-ancestor", t.up, `refs/heads/${t.r.defaultBranch}`], { cwd: t.d })).ok; | |
| 357 | expect(isAncestor).toBe(true); | |
| 358 | expect(await t.git(["rev-list", "--parents", "-n1", `refs/heads/${t.r.defaultBranch}`])).toContain(t.up); | |
| 359 | const after = ok(await checkUpstream(app, t.r)); | |
| 360 | expect([after.behind, after.outcome]).toEqual([0, "none"]); | |
| 361 | } finally { | |
| 362 | t.srv.server.stop(true); | |
| 363 | } | |
| 364 | }); | |
| 365 | ||
| 326 | 366 | test("a later tip moves the open pull instead of opening another", async () => { |
| 327 | 367 | const t = await staged("moving", "base\n", "new\n"); |
| 328 | 368 | try { |
new above
offline