Public: anyone can read this; the crew can change it.
A pull's check tier comes from the base branch's forge.json: the 43 pulls opened before #1200 still run the full gate on every push #1347
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1275-a-pull-s-check-tier-comes-from-the-base → main 2 commits fork 1 thread · 1 unresolved
3 files
changed +122
−16
src/checks/service.ts +37 −3
| @@ -449,7 +449,42 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle { | ||
| 449 | 449 | }; |
| 450 | 450 | if (declared.value === undefined) |
| 451 | 451 | return { ok: false, error: "this commit declares no check" }; |
| 452 | const manifest = declared.value; | |
| 452 | // A pull does not choose its own gate: the commands and the timeout come | |
| 453 | // from the base branch's `forge.json` where the pull left it. The | |
| 454 | // branch's own supplies only what it needs to run (install, egress, | |
| 455 | // limits). | |
| 456 | let manifest = declared.value; | |
| 457 | let baseTip: string | undefined; | |
| 458 | if (x.pull !== undefined) { | |
| 459 | baseTip = await resolveOid(bare, `refs/heads/${x.pull.baseBranch}`); | |
| 460 | if (baseTip === undefined) | |
| 461 | return { ok: false, error: "the pull's base branch could not be found" }; | |
| 462 | const [baseJson, basePackage] = await Promise.all([ | |
| 463 | readFileAt(bare, baseTip, "forge.json"), | |
| 464 | readFileAt(bare, baseTip, "package.json"), | |
| 465 | ]); | |
| 466 | const gate = parseCheck(baseJson, basePackage); | |
| 467 | if (!gate.ok) | |
| 468 | return { | |
| 469 | ok: false, | |
| 470 | error: | |
| 471 | gate.error.kind === "invalid" | |
| 472 | ? `${x.pull.baseBranch}: ${gate.error.message}` | |
| 473 | : "the base branch declares no check", | |
| 474 | }; | |
| 475 | if (gate.value !== undefined) { | |
| 476 | manifest = { | |
| 477 | ...manifest, | |
| 478 | run: gate.value.run, | |
| 479 | fast: gate.value.fast, | |
| 480 | full: gate.value.full, | |
| 481 | timeout: gate.value.timeout, | |
| 482 | }; | |
| 483 | write( | |
| 484 | `tier ${x.tier} from ${x.pull.baseBranch}@${baseTip.slice(0, 7)}`, | |
| 485 | ); | |
| 486 | } | |
| 487 | } | |
| 453 | 488 | const command = |
| 454 | 489 | x.tier === "full" |
| 455 | 490 | ? manifest.full |
| @@ -461,8 +496,7 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle { | ||
| 461 | 496 | const fast = x.tier === "fast" && manifest.fast !== null && x.pull !== undefined; |
| 462 | 497 | let base: string | undefined; |
| 463 | 498 | if (fast) { |
| 464 | const tip = await resolveOid(bare, `refs/heads/${x.pull!.baseBranch}`); | |
| 465 | base = tip === undefined ? undefined : await mergeBase(bare, tip, run.commitOid); | |
| 499 | base = await mergeBase(bare, baseTip!, run.commitOid); | |
| 466 | 500 | if (base === undefined) |
| 467 | 501 | return { ok: false, error: "the pull's fork point could not be found" }; |
| 468 | 502 | } |
test/check-tiers.test.ts +56 −4
| @@ -267,7 +267,6 @@ describe("the tiers in the forge's own checks service", () => { | ||
| 267 | 267 | |
| 268 | 268 | test("a red full gate names its tests and the pull learns the files", async () => { |
| 269 | 269 | const red = await commitToBranch(app, alice, repo, { |
| 270 | branch: "probe", | |
| 271 | 270 | message: "Full goes red", |
| 272 | 271 | files: [ |
| 273 | 272 | { |
| @@ -296,8 +295,7 @@ describe("the tiers in the forge's own checks service", () => { | ||
| 296 | 295 | test("landing runs the full gate on the merge with its base; red is told and taught", async () => { |
| 297 | 296 | const pull0 = listPulls(app.db, repo.id, { state: "open", limit: 5 })[0]!; |
| 298 | 297 | const pushed = await commitToBranch(app, alice, repo, { |
| 299 | branch: "feature", | |
| 300 | message: "Feature goes red on land", | |
| 298 | message: "Main goes red on land", | |
| 301 | 299 | files: [ |
| 302 | 300 | { |
| 303 | 301 | path: "forge.json", |
| @@ -308,7 +306,7 @@ describe("the tiers in the forge's own checks service", () => { | ||
| 308 | 306 | ], |
| 309 | 307 | }); |
| 310 | 308 | if (!pushed.ok) throw new Error(JSON.stringify(pushed.error)); |
| 311 | const pull = { ...pull0, headOid: pushed.value.commitOid }; | |
| 309 | const pull = pull0; | |
| 312 | 310 | const base = (await resolveOid( |
| 313 | 311 | repoDir(app.reposRoot, repo.ownerName, repo.name), |
| 314 | 312 | "refs/heads/main", |
| @@ -324,4 +322,58 @@ describe("the tiers in the forge's own checks service", () => { | ||
| 324 | 322 | expect(verdict.comment).toContain("- lands red"); |
| 325 | 323 | expect(readLearned(dataDir, pull.id)).toContain("test/y.test.ts"); |
| 326 | 324 | }); |
| 325 | ||
| 326 | const finished = async (pullId: string, oid: string): Promise<Run> => { | |
| 327 | const deadline = Date.now() + 30_000; | |
| 328 | let run: Run | undefined; | |
| 329 | while (Date.now() < deadline) { | |
| 330 | run = listRuns(app.db, repo.id, { kind: "check", limit: 50 }).find( | |
| 331 | (r) => r.issueId === pullId && r.commitOid === oid, | |
| 332 | ); | |
| 333 | if (run && (run.state === "ok" || run.state === "failed")) return run; | |
| 334 | await Bun.sleep(50); | |
| 335 | } | |
| 336 | throw new Error("the check did not finish"); | |
| 337 | }; | |
| 338 | ||
| 339 | test("the tier comes from the base: a head without check.fast, or editing it, is checked with the base's", async () => { | |
| 340 | const own = (fast: string | undefined): string => | |
| 341 | JSON.stringify({ | |
| 342 | check: { run: "echo HEAD-RUN", ...(fast ? { fast } : {}) }, | |
| 343 | }); | |
| 344 | const old = await commitToBranch(app, alice, repo, { | |
| 345 | branch: "old", | |
| 346 | message: "Branched before the fast tier", | |
| 347 | files: [{ path: "forge.json", content: own(undefined) }], | |
| 348 | }); | |
| 349 | if (!old.ok) throw new Error(JSON.stringify(old.error)); | |
| 350 | const weak = await commitToBranch(app, alice, repo, { | |
| 351 | branch: "weak", | |
| 352 | message: "Weakens its own gate", | |
| 353 | files: [{ path: "forge.json", content: own("echo HEAD-FAST") }], | |
| 354 | }); | |
| 355 | if (!weak.ok) throw new Error(JSON.stringify(weak.error)); | |
| 356 | const main = await commitToBranch(app, alice, repo, { | |
| 357 | message: "Base declares the tier", | |
| 358 | files: [{ path: "forge.json", content: forgeJson("echo FULL") }], | |
| 359 | }); | |
| 360 | if (!main.ok) throw new Error(JSON.stringify(main.error)); | |
| 361 | for (const [head, oid] of [ | |
| 362 | ["old", old.value.commitOid], | |
| 363 | ["weak", weak.value.commitOid], | |
| 364 | ] as const) { | |
| 365 | const opened = await openPull(app, alice, repo, { | |
| 366 | title: head, | |
| 367 | baseBranch: "main", | |
| 368 | headBranch: head, | |
| 369 | }); | |
| 370 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 371 | const run = await finished(opened.value.id, oid); | |
| 372 | expect(run.state).toBe("ok"); | |
| 373 | const log = readRunLog(dataDir, run.id).lines.join("\n"); | |
| 374 | expect(log).toContain(`tier fast from main@${main.value.commitOid.slice(0, 7)}`); | |
| 375 | expect(log).toContain("FAST base="); | |
| 376 | expect(log).not.toContain("HEAD-"); | |
| 377 | } | |
| 378 | }); | |
| 327 | 379 | }); |
test/checks.test.ts +29 −9
| @@ -278,10 +278,16 @@ describe("checks", () => { | ||
| 278 | 278 | }); |
| 279 | 279 | |
| 280 | 280 | test("a pull is checked when it opens and each time its branch moves; a failure names the exit", async () => { |
| 281 | // A pull is checked by the base's check, not its own. | |
| 282 | const gate = await commitToBranch(app, alice, repo, { | |
| 283 | message: "The base's check", | |
| 284 | files: [{ path: "forge.json", content: checkJson("test -f NOPE.md") }], | |
| 285 | }); | |
| 286 | if (!gate.ok) throw new Error(JSON.stringify(gate.error)); | |
| 281 | 287 | const branched = await commitToBranch(app, alice, repo, { |
| 282 | 288 | branch: "feature", |
| 283 | message: "Break the check", | |
| 284 | files: [{ path: "forge.json", content: checkJson("test -f NOPE.md") }], | |
| 289 | message: "Leave the check failing", | |
| 290 | files: [{ path: "a.txt", content: "a\n" }], | |
| 285 | 291 | }); |
| 286 | 292 | if (!branched.ok) throw new Error(JSON.stringify(branched.error)); |
| 287 | 293 | // A branch that heads no pull is nobody's to check yet. |
| @@ -326,11 +332,17 @@ describe("checks", () => { | ||
| 326 | 332 | }, 60_000); |
| 327 | 333 | |
| 328 | 334 | test("a bun test failure in the check step is summarised: how many passed, how many failed, and their names", async () => { |
| 329 | const branched = await commitToBranch(app, alice, repo, { | |
| 330 | branch: "red-tests", | |
| 331 | message: "A check that runs bun test, with two failures", | |
| 335 | const gate = await commitToBranch(app, alice, repo, { | |
| 336 | message: "The base's check runs bun test", | |
| 332 | 337 | files: [ |
| 333 | 338 | { path: "forge.json", content: checkJson("bun test check.test.ts") }, |
| 339 | ], | |
| 340 | }); | |
| 341 | if (!gate.ok) throw new Error(JSON.stringify(gate.error)); | |
| 342 | const branched = await commitToBranch(app, alice, repo, { | |
| 343 | branch: "red-tests", | |
| 344 | message: "Two failing tests", | |
| 345 | files: [ | |
| 334 | 346 | { |
| 335 | 347 | path: "check.test.ts", |
| 336 | 348 | content: [ |
| @@ -366,6 +378,11 @@ describe("checks", () => { | ||
| 366 | 378 | }, 30_000); |
| 367 | 379 | |
| 368 | 380 | test("a reader's fork pull is checked at the base's snapshot once a writer approves each head", async () => { |
| 381 | const gate = await commitToBranch(app, alice, repo, { | |
| 382 | message: "The base's check passes", | |
| 383 | files: [{ path: "forge.json", content: checkJson("true") }], | |
| 384 | }); | |
| 385 | if (!gate.ok) throw new Error(JSON.stringify(gate.error)); | |
| 369 | 386 | const forked = await forkRepo(app, bob, repo); |
| 370 | 387 | if (!forked.ok) throw new Error(String(forked.error)); |
| 371 | 388 | const fork = forked.value; |
| @@ -604,13 +621,16 @@ describe("checks", () => { | ||
| 604 | 621 | }); |
| 605 | 622 | test("an open pull whose head was never checked is checked when the service boots", async () => { |
| 606 | 623 | await checks.stop(); |
| 624 | // The pull is checked by the base's check, so the base must ask for it. | |
| 625 | const gate = await commitToBranch(app, alice, repo, { | |
| 626 | message: "The base's check wants QUIET.md", | |
| 627 | files: [{ path: "forge.json", content: checkJson("test -f QUIET.md") }], | |
| 628 | }); | |
| 629 | if (!gate.ok) throw new Error(JSON.stringify(gate.error)); | |
| 607 | 630 | const branch = await commitToBranch(app, alice, repo, { |
| 608 | 631 | branch: "quiet", |
| 609 | 632 | message: "Quiet", |
| 610 | files: [ | |
| 611 | { path: "QUIET.md", content: "quiet\n" }, | |
| 612 | { path: "forge.json", content: checkJson("test -f QUIET.md") }, | |
| 613 | ], | |
| 633 | files: [{ path: "QUIET.md", content: "quiet\n" }], | |
| 614 | 634 | }); |
| 615 | 635 | if (!branch.ok) throw new Error(JSON.stringify(branch.error)); |
| 616 | 636 | // Opened while no checks service was listening: nothing asked for a check. |
new above
offline
Log the base ref only when the base declares a check. When it does not, the head's own gate runs silently, so write a line saying so; otherwise the log hides which gate ran.