Public: anyone can read this; the crew can change it.
A trusted source's big first fold opens one pull, not a flood, and lands by true merge after the crew reviews it #12079
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9024-a-trusted-source-s-big-first-fold-opens → main 2 commits fork 3 threads · 3 unresolved
1 file
changed +129
−5
test/follow-sources.test.ts +129 −5
| @@ -16,7 +16,11 @@ import { openPull } from "../src/actions/pulls.ts"; | ||
| 16 | 16 | import { createRepo } from "../src/actions/repos.ts"; |
| 17 | 17 | import { addTrust } from "../src/actions/trusted_sources.ts"; |
| 18 | 18 | import { checkDue, checkSource } from "../src/actions/upstream_follow.ts"; |
| 19 | import { reviewed } from "../src/crew/autonomy.ts"; | |
| 19 | import { landingStrategy, reviewed, sweepGreen } from "../src/crew/autonomy.ts"; | |
| 20 | import { provideChecksForTest, type ChecksHandle } from "../src/checks/service.ts"; | |
| 21 | import { lastWaits } from "../src/crew/waits.ts"; | |
| 22 | import { claimRun, createRun, finishRun } from "../src/core/runs.ts"; | |
| 23 | import { id as newId } from "../src/kernel/id.ts"; | |
| 20 | 24 | import { createApp, type App } from "../src/app.ts"; |
| 21 | 25 | import { sourcesScreen } from "../src/tui/screens/sources.ts"; |
| 22 | 26 | import type { Ctx, Screen } from "../src/tui/app/app.ts"; |
| @@ -145,11 +149,12 @@ describe("following several sources", () => { | ||
| 145 | 149 | const ident = { GIT_AUTHOR_NAME: "t", GIT_AUTHOR_EMAIL: "t@x", GIT_COMMITTER_NAME: "t", GIT_COMMITTER_EMAIL: "t@x" }; |
| 146 | 150 | |
| 147 | 151 | /** A fake Forgejo: answers `info/refs` for each path with the tip it is given. */ |
| 148 | function forgejo(tips: Map<string, string>) { | |
| 152 | function forgejo(tips: Map<string, string>, hold?: Promise<void>) { | |
| 149 | 153 | const server = Bun.serve({ |
| 150 | 154 | port: 0, |
| 151 | 155 | hostname: "127.0.0.1", |
| 152 | fetch(req) { | |
| 156 | async fetch(req) { | |
| 157 | await hold; | |
| 153 | 158 | const url = new URL(req.url); |
| 154 | 159 | const path = url.pathname.replace(/\/info\/refs$/, ""); |
| 155 | 160 | const tip = tips.get(path); |
| @@ -161,7 +166,7 @@ describe("following several sources", () => { | ||
| 161 | 166 | return { server, host: `127.0.0.1:${server.port}`, url: (path: string) => `http://127.0.0.1:${server.port}${path}` }; |
| 162 | 167 | } |
| 163 | 168 | |
| 164 | /** A repository whose main holds f.txt = `mainText`, and a commit tree builder. */ | |
| 169 | /** A repository whose main holds src/f.txt = `mainText`, and a commit tree builder. */ | |
| 165 | 170 | async function repoWith(name: string, mainText: string) { |
| 166 | 171 | const r = ok(await createRepo(app, alice, { name, visibility: "public" })); |
| 167 | 172 | askForReviews(app.db, r.id); |
| @@ -170,7 +175,8 @@ describe("following several sources", () => { | ||
| 170 | 175 | ok(await runGit(args, { cwd: d, env: ident, ...(input === undefined ? {} : { stdin: input }) })).trim(); |
| 171 | 176 | const commit = async (text: string, parent?: string) => { |
| 172 | 177 | const blob = await git(["hash-object", "-w", "--stdin"], text); |
| 173 | const tree = await git(["mktree"], `100644 blob ${blob}\tf.txt\n`); | |
| 178 | const inner = await git(["mktree"], `100644 blob ${blob}\tf.txt\n`); | |
| 179 | const tree = await git(["mktree"], `040000 tree ${inner}\tsrc\n`); | |
| 174 | 180 | return git(["commit-tree", tree, ...(parent ? ["-p", parent] : []), "-m", text]); |
| 175 | 181 | }; |
| 176 | 182 | const base = await commit("base\n"); |
| @@ -312,6 +318,124 @@ describe("following several sources", () => { | ||
| 312 | 318 | } |
| 313 | 319 | }); |
| 314 | 320 | |
| 321 | /** `n` commits on `parent`, all of one tree (`text`): a long history built in `n` spawns. */ | |
| 322 | async function chain(t: Awaited<ReturnType<typeof repoWith>>, parent: string, n: number, text: string) { | |
| 323 | const ref = `refs/tmp/chain-${chains++}`; | |
| 324 | let stream = ""; | |
| 325 | for (let i = 0; i < n; i++) | |
| 326 | stream += | |
| 327 | `commit ${ref}\ncommitter t <t@x> ${1_700_000_000 + i} +0000\ndata 2\nc${i % 10}\n` + | |
| 328 | (i === 0 ? `from ${parent}\n` : "") + | |
| 329 | `M 100644 inline src/f.txt\ndata ${text.length}\n${text}\n`; | |
| 330 | ok(await runGit(["fast-import", "--quiet"], { cwd: t.d, env: ident, stdin: stream })); | |
| 331 | const tip = await t.git(["rev-parse", ref]); | |
| 332 | await t.git(["update-ref", "-d", ref]); | |
| 333 | return tip; | |
| 334 | } | |
| 335 | let chains = 0; | |
| 336 | ||
| 337 | test("a first fold 120 commits ahead is one pull however many polls race for it, and later commits move that pull", async () => { | |
| 338 | const t = await repoWith("flood", "base\n"); | |
outdated
This race passes only if the three checks overlap in time. Gate the fake Forgejo response on a promise released after all three have started, so exactly one winner is deterministic, not timing luck. |
||
| 339 | const tip = await chain(t, t.base, 120, "new\n"); | |
| 340 | const served = new Map([["/jmoran/flood.git", tip]]); | |
| 341 | let release!: () => void; | |
| 342 | const f = forgejo(served, new Promise<void>((go) => (release = go))); | |
| 343 | try { | |
| 344 | ok(addTrust(app, alice, { host: f.host, account: "jmoran", until: "2099-01-01" })); | |
| 345 | const s = await follow(t, f, "/jmoran/flood.git", tip); | |
| 346 | const repo = getRepoById(app.db, t.r.id)!; | |
| 347 | // A person's check, the clock and a second click all arrive together. | |
| 348 | // The fake forge holds every answer until all three checks have started. | |
| 349 | const started = [checkSource(app, repo, s), checkSource(app, repo, s), checkDue(app)] as const; | |
| 350 | release(); | |
| 351 | const [one, two] = await Promise.all(started); | |
| 352 | expect([one.ok, two.ok].sort()).toEqual([false, true]); | |
| 353 | expect(one.ok ? two : one).toEqual({ ok: false, error: "busy" }); | |
| 354 | expect(pullCount(t.r.id)).toBe(1); | |
| 355 | const again = ok(await checkSource(app, repo, listSources(app.db, t.r.id)[0]!)); | |
| 356 | expect(again.behind).toBe(120); | |
| 357 | expect(pullCount(t.r.id)).toBe(1); | |
| 358 | const pull = getPull(app.db, t.r.id, again.pull!)!; | |
| 359 | expect(pullSourceOf(app.db, pull.id)).toMatchObject({ host: f.host, account: "jmoran" }); | |
| 360 | expect(needsCrewReview(app.db, pull.id)).toBe(true); | |
| 361 | ||
| 362 | // Later commits on the source update that pull while it is open. | |
| 363 | const more = await chain(t, tip, 3, "newer\n"); | |
| 364 | served.set("/jmoran/flood.git", more); | |
| 365 | await t.git(["update-ref", sourceRef(s.id), more]); | |
| 366 | const moved = ok(await checkSource(app, repo, listSources(app.db, t.r.id)[0]!)); | |
| 367 | expect(moved.pull).toBe(again.pull); | |
| 368 | expect(getPull(app.db, t.r.id, moved.pull!)!.headOid).toBe(more); | |
| 369 | expect(pullCount(t.r.id)).toBe(1); | |
| 370 | expect(needsCrewReview(app.db, pull.id)).toBe(true); | |
| 371 | } finally { | |
| 372 | f.server.stop(true); | |
| 373 | } | |
| 374 | }); | |
| 375 | ||
| 376 | test("a trusted source's big first fold that conflicts opens no pull: the crew resolves it, reviews it, and it lands by true merge", async () => { | |
| 377 | const t = await repoWith("bigclash", "ours\n"); | |
| 378 | const tip = await chain(t, t.base, 120, "theirs\n"); | |
| 379 | const f = forgejo(new Map([["/jmoran/bigclash.git", tip]])); | |
| 380 | try { | |
| 381 | ok(addTrust(app, alice, { host: f.host, account: "jmoran", until: "2099-01-01" })); | |
| 382 | const bot = ok(await createAgent(app, alice, { username: "bot-bigclash", brief: "Be brief." })); | |
| 383 | expect(addToCrew(app, alice, getRepoById(app.db, t.r.id)!, "bot-bigclash").ok).toBe(true); | |
| 384 | updateRepoSettings(app.db, t.r, { autonomy: "assisted", autonomyBy: alice.id }); | |
| 385 | askForReviews(app.db, t.r.id); | |
| 386 | const s = await follow(t, f, "/jmoran/bigclash.git", tip); | |
| 387 | const r = getRepoById(app.db, t.r.id)!; | |
| 388 | const found = ok(await checkSource(app, r, s)); | |
| 389 | expect(found.outcome).toBe("conflict"); | |
| 390 | expect(found.behind).toBe(120); | |
| 391 | // No pull, so no no-review path; one issue for the crew. | |
outdated
Box 3 is inferred from |
||
| 392 | expect(pullCount(t.r.id)).toBe(0); | |
| 393 | const issues = listIssues(app.db, t.r.id, { state: "all" }).filter((i) => i.title.startsWith("Upstream: conflict at ")); | |
| 394 | expect(issues.length).toBe(1); | |
| 395 | // A second poll of the same head asks nobody again. | |
| 396 | ok(await checkSource(app, r, listSources(app.db, t.r.id)[0]!)); | |
| 397 | expect(pullCount(t.r.id)).toBe(0); | |
| 398 | expect(listIssues(app.db, t.r.id, { state: "all" }).filter((i) => i.title.startsWith("Upstream: conflict at ")).length).toBe(1); | |
| 399 | ||
| 400 | // The crew's resolution merges the source's head, so its commits are ancestors of the pull. | |
| 401 | const mainOid = await t.git(["rev-parse", `refs/heads/${r.defaultBranch}`]); | |
| 402 | const resolved = await t.git(["commit-tree", `${tip}^{tree}`, "-p", mainOid, "-p", tip, "-m", "resolve"]); | |
| 403 | await t.git(["update-ref", "refs/heads/bot/resolve", resolved]); | |
| 404 | const mine = ok(await openPull(app, bot.user, r, { title: "Resolve", body: `Closes #${issues[0]!.number}`, baseBranch: r.defaultBranch, headBranch: "bot/resolve" })); | |
| 405 | expect(needsCrewReview(app.db, mine.id)).toBe(true); | |
| 406 | // Trusted, yet held for the crew's review rather than passed on its check. | |
| 407 | expect(foldsIn(app.db, mine.id)).toBe(true); | |
| 408 | expect(await reviewed(app, getRepoById(app.db, t.r.id)!, mine)).toBe("hold"); | |
| 409 | expect(await landingStrategy(app, getRepoById(app.db, t.r.id)!, mine, mainOid)).toBe("merge"); | |
| 410 | ||
| 411 | // The crew's reviewer approves and its check is green: it lands. | |
| 412 | provideChecksForTest(app, { | |
| 413 | gate: async () => ({ ok: true, error: "", lines: [] }), | |
| 414 | spareSlots: () => 0, | |
| 415 | } as unknown as ChecksHandle); | |
| 416 | const run = createRun(app.db, { repoId: t.r.id, kind: "check", issueId: mine.id, env: `pull-${mine.number}`, commitOid: mine.headOid, reason: "pull" }); | |
| 417 | claimRun(app.db, run.id); | |
| 418 | finishRun(app.db, run.id, { state: "ok", summary: "ok" }); | |
| 419 | app.db | |
| 420 | .query("INSERT INTO review (id, issue_id, author_id, state, commit_oid, created_at) VALUES (?1, ?2, ?3, 'approved', ?4, ?5)") | |
| 421 | .run(newId(), mine.id, alice.id, mine.headOid, new Date().toISOString()); | |
| 422 | app.db.query("UPDATE repo SET autonomy = 'autonomous', autonomy_by = ?2 WHERE id = ?1").run(t.r.id, alice.id); | |
| 423 | const landed = await sweepGreen(app, getRepoById(app.db, t.r.id)!); | |
| 424 | expect([landed, lastWaits(t.r.id)]).toEqual([1, { queued: 1 }]); | |
| 425 | expect(getPull(app.db, t.r.id, mine.number)!.mergedAt).not.toBeNull(); | |
| 426 | ||
| 427 | // A true merge: the source's commits are ancestors of main, so merging main into | |
| 428 | // the source's tip is a fast-forward (the tip is the merge base), never a conflict. | |
| 429 | const after = await t.git(["rev-parse", `refs/heads/${r.defaultBranch}`]); | |
| 430 | expect((await t.git(["rev-list", "--parents", "-n", "1", after])).split(" ").length).toBe(3); | |
| 431 | await t.git(["merge-base", "--is-ancestor", tip, after]); | |
| 432 | expect(await t.git(["merge-base", tip, after])).toBe(tip); | |
| 433 | expect(await t.git(["merge-base", "--is-ancestor", mainOid, after]).then(() => true)).toBe(true); | |
| 434 | } finally { | |
| 435 | f.server.stop(true); | |
| 436 | } | |
| 437 | }); | |
| 438 | ||
| 315 | 439 | test("a conflict goes to the crew once; the crew's resolving pull carries the source, and only an agent's", async () => { |
| 316 | 440 | const t = await repoWith("clash", "ours\n"); |
| 317 | 441 | const up = await t.commit("theirs\n", t.base); |
new above
offline
120 sequential spawns per call, called from two tests. Build the chain with one
git fast-importso the history costs one spawn and the test stays under the 1 s budget (#1242).