Public: anyone can read this; the crew can change it.
The check fails a pull that modifies or deletes a file under src/core/migration-pins/ #6832
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6821-the-check-fails-a-pull-that-modifies-or → main 5 commits fork 3 threads · 3 unresolved
5 files
changed +222
−3
scripts/gate.ts +40 −1
| @@ -180,10 +180,49 @@ export function gateRefusal(present: readonly string[]): string[] { | ||
| 180 | 180 | ]; |
| 181 | 181 | } |
| 182 | 182 | |
| 183 | function git(args: string[], cwd: string = process.cwd()): { ok: boolean; out: string } { | |
| 184 | const r = Bun.spawnSync(["git", ...args], { stderr: "pipe", cwd }); | |
| 185 | return { ok: r.exitCode === 0, out: r.stdout.toString().trim() }; | |
| 186 | } | |
| 187 | ||
| 188 | /** | |
| 189 | * Pins the pull edited, against its base: `$FORGE_CHECK_BASE`, else the | |
| 190 | * merge base with main. A pin whose migration is not on the base was never | |
| 191 | * history, so it is deleted and regenerated rather than left to a new migration. | |
| 192 | * With no base to compare to it says so on stderr and names nothing. | |
| 193 | */ | |
| 194 | // Imported on use: `treeGate` loads this file alone, outside the repository. | |
| 195 | export async function pinRefusal(cwd: string = process.cwd()): Promise<string[]> { | |
| 196 | const { parseNameStatus, pinEdits } = await import("../src/kernel/pin_edits.ts"); | |
| 197 | const env = process.env.FORGE_CHECK_BASE; | |
| 198 | const base = | |
| 199 | env || git(["merge-base", "origin/main", "HEAD"], cwd).out || git(["merge-base", "main", "HEAD"], cwd).out; | |
| 200 | if (!base) { | |
| 201 | console.error("gate: no base to compare to (set FORGE_CHECK_BASE): migration pins not checked"); | |
Fail-open: with no base the gate prints a warning and exits 0. When the check sets FORGE_CHECK_BASE and it does not resolve, that should be exit 2, so a broken base cannot disable the guard unnoticed. Keep the warning only for the local no-env case. |
||
| 202 | return []; | |
| 203 | } | |
| 204 | const diff = git(["diff", "--name-status", "-M", `${base}..HEAD`], cwd); | |
| 205 | if (!diff.ok) { | |
outdated
No test drives this function or the gate.ts entry. Add a temp-git-repo test asserting exit 2 and the named file, plus both message branches. |
||
| 206 | console.error(`gate: git diff against ${base} failed: migration pins not checked`); | |
| 207 | return []; | |
| 208 | } | |
| 209 | const changes = parseNameStatus(diff.out); | |
| 210 | return pinEdits(changes).map((path) => { | |
| 211 | const name = path.split("/").pop()?.replace(/\.pin$/, "") ?? ""; | |
| 212 | const deleted = changes.some((c) => c.path === path && c.status.startsWith("D")); | |
| 213 | const onBase = git(["cat-file", "-e", `${base}:src/core/migrations/${name}.ts`], cwd).ok; | |
| 214 | if (path.endsWith(".pin") && !onBase) | |
| 215 | return `${path} was changed: its migration is not on the base yet, so delete the pin and re-run bun scripts/pin-migrations.ts`; | |
| 216 | return deleted | |
| 217 | ? `${path} was deleted: a migration on main is history, so restore it` | |
| 218 | : `${path} was changed: a migration on main is history, so add a new migration instead`; | |
| 219 | }); | |
| 220 | } | |
| 221 | ||
| 183 | 222 | // `bun scripts/gate.ts`: the audit alone, first in `forge.json`'s check, so |
| 184 | 223 | // a stale exclusion fails in under a second — not after install and tsc. |
| 185 | 224 | if (import.meta.main) { |
| 186 | const refusal = gateRefusal(presentTests()); | |
| 225 | const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal())]; | |
| 187 | 226 | for (const line of refusal) console.error(`gate: ${line}`); |
| 188 | 227 | process.exit(refusal.length === 0 ? 0 : 2); |
| 189 | 228 | } |
src/kernel/pin_edits.ts added +41
| @@ -0,0 +1,41 @@ | ||
| 1 | /** | |
| 2 | * Pins are history: a migration on main is never edited, so neither is its | |
| 3 | * pin. `pinBreaks` cannot see a pull that edits a migration and its pin | |
| 4 | * together; the pull's diff to its base can. | |
| 5 | */ | |
| 6 | ||
| 7 | export type Change = { | |
| 8 | /** git's name-status letter: A, M, D, R, C or T. */ | |
| 9 | status: string; | |
| 10 | path: string; | |
| 11 | }; | |
| 12 | ||
| 13 | const PINS = "src/core/migration-pins/"; | |
| 14 | const FROZEN = "src/core/migration-pins.json"; | |
| 15 | ||
| 16 | /** The paths a pull may not touch: pins and the frozen pins file that it modified, deleted or renamed. */ | |
| 17 | export function pinEdits(changes: readonly Change[]): string[] { | |
| 18 | const edited = new Set<string>(); | |
| 19 | for (const { status, path } of changes) { | |
| 20 | if (!path.startsWith(PINS) && path !== FROZEN) continue; | |
| 21 | if (status.startsWith("A") || status.startsWith("C")) continue; | |
| 22 | edited.add(path); | |
| 23 | } | |
| 24 | return [...edited]; | |
| 25 | } | |
| 26 | ||
| 27 | /** | |
| 28 | * `git diff --name-status` output as changes. A rename is its old path | |
| 29 | * (renamed away) and its new path (added). | |
| 30 | */ | |
| 31 | export function parseNameStatus(out: string): Change[] { | |
| 32 | const changes: Change[] = []; | |
| 33 | for (const line of out.split("\n")) { | |
| 34 | const [status, a, b] = line.split("\t"); | |
| 35 | if (!status || a === undefined) continue; | |
| 36 | if (status.startsWith("R") && b !== undefined) { | |
| 37 | changes.push({ status: "R", path: a }, { status: "A", path: b }); | |
| 38 | } else changes.push({ status, path: a }); | |
| 39 | } | |
| 40 | return changes; | |
| 41 | } |
test/pin-edits.test.ts added +30
| @@ -0,0 +1,30 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { parseNameStatus, pinEdits } from "../src/kernel/pin_edits.ts"; | |
| 3 | ||
| 4 | const pin = "src/core/migration-pins/0001-a.pin"; | |
| 5 | ||
| 6 | test("a modified pin is named", () => { | |
| 7 | expect(pinEdits([{ status: "M", path: pin }])).toEqual([pin]); | |
| 8 | }); | |
| 9 | ||
| 10 | test("a deleted pin is named", () => { | |
| 11 | expect(pinEdits([{ status: "D", path: pin }])).toEqual([pin]); | |
| 12 | }); | |
| 13 | ||
| 14 | test("a renamed pin is named by its old path; the new one is added", () => { | |
| 15 | const changes = parseNameStatus(`R100\t${pin}\tsrc/core/migration-pins/0002-b.pin`); | |
| 16 | expect(pinEdits(changes)).toEqual([pin]); | |
| 17 | }); | |
| 18 | ||
| 19 | test("an added pin passes", () => { | |
| 20 | expect(pinEdits([{ status: "A", path: pin }])).toEqual([]); | |
| 21 | }); | |
| 22 | ||
| 23 | test("paths outside the folder are ignored", () => { | |
| 24 | expect(pinEdits([{ status: "M", path: "src/core/migrations/0001-a.ts" }])).toEqual([]); | |
| 25 | }); | |
| 26 | ||
| 27 | test("the frozen pins file is history too", () => { | |
| 28 | const f = "src/core/migration-pins.json"; | |
| 29 | expect(pinEdits([{ status: "M", path: f }])).toEqual([f]); | |
| 30 | }); |
test/pin-refusal.test.ts added +106
| @@ -0,0 +1,106 @@ | ||
| 1 | import { afterAll, expect, test } from "bun:test"; | |
| 2 | import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; | |
| 3 | import { join } from "node:path"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { pinRefusal } from "../scripts/gate.ts"; | |
| 6 | ||
| 7 | const root = mkdtempSync(join(tmpdir(), "pin-refusal-")); | |
| 8 | afterAll(() => rmSync(root, { recursive: true, force: true })); | |
| 9 | ||
| 10 | const gateScript = join(import.meta.dir, "../scripts/gate.ts"); | |
| 11 | const run = (cwd: string, ...args: string[]) => { | |
| 12 | const r = Bun.spawnSync(["git", "-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd }); | |
| 13 | if (r.exitCode !== 0) throw new Error(r.stderr.toString()); | |
| 14 | return r.stdout.toString().trim(); | |
| 15 | }; | |
| 16 | const put = (dir: string, file: string, text: string) => { | |
| 17 | mkdirSync(join(dir, file, ".."), { recursive: true }); | |
| 18 | writeFileSync(join(dir, file), text); | |
| 19 | }; | |
| 20 | ||
| 21 | /** A repo whose base commit has one pinned migration on it and one pin without. */ | |
| 22 | function repo(name: string) { | |
| 23 | const dir = join(root, name); | |
| 24 | mkdirSync(dir); | |
| 25 | run(dir, "init", "-q", "-b", "work"); | |
| 26 | put(dir, "src/core/migrations/0001-a.ts", "a"); | |
| 27 | put(dir, "src/core/migration-pins/0001-a.pin", "x"); | |
| 28 | put(dir, "src/core/migration-pins/0002-b.pin", "x"); | |
| 29 | run(dir, "add", "-A"); | |
| 30 | run(dir, "commit", "-qm", "base"); | |
| 31 | return { dir, base: run(dir, "rev-parse", "HEAD") }; | |
| 32 | } | |
| 33 | ||
| 34 | const withBase = async (base: string, f: () => Promise<string[]>) => { | |
| 35 | const old = process.env.FORGE_CHECK_BASE; | |
| 36 | process.env.FORGE_CHECK_BASE = base; | |
| 37 | try { | |
| 38 | return await f(); | |
| 39 | } finally { | |
| 40 | if (old === undefined) delete process.env.FORGE_CHECK_BASE; | |
| 41 | else process.env.FORGE_CHECK_BASE = old; | |
| 42 | } | |
| 43 | }; | |
| 44 | ||
| 45 | test("a modified pin whose migration is on the base is told to add a new migration", async () => { | |
| 46 | const { dir, base } = repo("modified"); | |
| 47 | put(dir, "src/core/migration-pins/0001-a.pin", "y"); | |
| 48 | run(dir, "commit", "-qam", "edit"); | |
| 49 | const out = await withBase(base, () => pinRefusal(dir)); | |
| 50 | expect(out).toEqual([ | |
| 51 | "src/core/migration-pins/0001-a.pin was changed: a migration on main is history, so add a new migration instead", | |
| 52 | ]); | |
| 53 | }); | |
| 54 | ||
| 55 | test("a pin whose migration is not on the base is told to delete it and re-run", async () => { | |
| 56 | const { dir, base } = repo("regen"); | |
| 57 | put(dir, "src/core/migration-pins/0002-b.pin", "y"); | |
| 58 | run(dir, "commit", "-qam", "edit"); | |
| 59 | const out = await withBase(base, () => pinRefusal(dir)); | |
| 60 | expect(out).toHaveLength(1); | |
| 61 | expect(out[0]).toContain("0002-b.pin"); | |
| 62 | expect(out[0]).toContain("delete the pin and re-run"); | |
| 63 | }); | |
| 64 | ||
| 65 | test("a deleted pin whose migration is on the base is told to restore it", async () => { | |
| 66 | const { dir, base } = repo("deleted"); | |
| 67 | run(dir, "rm", "-q", "src/core/migration-pins/0001-a.pin"); | |
| 68 | run(dir, "commit", "-qm", "rm"); | |
| 69 | const out = await withBase(base, () => pinRefusal(dir)); | |
| 70 | expect(out).toEqual([ | |
| 71 | "src/core/migration-pins/0001-a.pin was deleted: a migration on main is history, so restore it", | |
| 72 | ]); | |
| 73 | }); | |
| 74 | ||
| 75 | test("an added pin passes", async () => { | |
| 76 | const { dir, base } = repo("added"); | |
| 77 | put(dir, "src/core/migration-pins/0003-c.pin", "x"); | |
| 78 | run(dir, "add", "-A"); | |
| 79 | run(dir, "commit", "-qm", "add"); | |
| 80 | expect(await withBase(base, () => pinRefusal(dir))).toEqual([]); | |
| 81 | }); | |
| 82 | ||
| 83 | test("`bun scripts/gate.ts` exits 2 and names the edited pin", () => { | |
| 84 | const { dir, base } = repo("entry"); | |
| 85 | put(dir, "src/core/migration-pins/0001-a.pin", "y"); | |
| 86 | run(dir, "commit", "-qam", "edit"); | |
| 87 | const r = Bun.spawnSync(["bun", gateScript], { cwd: dir, env: { ...process.env, FORGE_CHECK_BASE: base } }); | |
| 88 | expect(r.exitCode).toBe(2); | |
| 89 | expect(r.stderr.toString()).toContain("src/core/migration-pins/0001-a.pin"); | |
| 90 | }); | |
| 91 | ||
| 92 | test("with no base it warns on stderr instead of passing silently", async () => { | |
| 93 | const { dir } = repo("nobase"); | |
| 94 | const seen: string[] = []; | |
| 95 | const orig = console.error; | |
| 96 | console.error = (...a: unknown[]) => void seen.push(a.join(" ")); | |
| 97 | const old = process.env.FORGE_CHECK_BASE; | |
| 98 | delete process.env.FORGE_CHECK_BASE; | |
| 99 | try { | |
| 100 | expect(await pinRefusal(dir)).toEqual([]); | |
| 101 | } finally { | |
| 102 | console.error = orig; | |
| 103 | if (old !== undefined) process.env.FORGE_CHECK_BASE = old; | |
| 104 | } | |
| 105 | expect(seen.join("\n")).toContain("pins not checked"); | |
| 106 | }); |
test/shards.test.ts +5 −2
| @@ -737,11 +737,13 @@ describe("the gate audit before any shard", () => { | ||
| 737 | 737 | describe("the gate audit alone, first in the check", () => { |
| 738 | 738 | const script = new URL("../scripts/gate.ts", import.meta.url).pathname; |
| 739 | 739 | const listed = presentTests(); |
| 740 | // The check sets FORGE_CHECK_BASE; the scratch dir is no repo, so it has none. | |
| 741 | const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => k !== "FORGE_CHECK_BASE")); | |
| 740 | 742 | const run = async (extra: string[], drop: string[] = []) => { |
| 741 | 743 | const dir = mkdtempSync(join(tmpdir(), "forge-gate-")); |
| 742 | 744 | mkdirSync(join(dir, "test")); |
| 743 | 745 | for (const file of [...listed, ...extra]) if (!drop.includes(file)) writeFileSync(join(dir, file), ""); |
| 744 | const child = Bun.spawn(["bun", script], { cwd: dir, stdin: "ignore", stdout: "pipe", stderr: "pipe" }); | |
| 746 | const child = Bun.spawn(["bun", script], { cwd: dir, env, stdin: "ignore", stdout: "pipe", stderr: "pipe" }); | |
| 745 | 747 | const err = await new Response(child.stderr).text(); |
| 746 | 748 | const code = await child.exited; |
| 747 | 749 | rmSync(dir, { recursive: true, force: true }); |
| @@ -757,7 +759,8 @@ describe("the gate audit alone, first in the check", () => { | ||
| 757 | 759 | ); |
| 758 | 760 | const green = await run(["test/zz-new.test.ts"]); |
| 759 | 761 | expect(green.code).toBe(0); |
| 760 | expect(green.err).toBe(""); | |
| 762 | // The scratch dir is no git repo, so the pin check says it had no base. | |
| 763 | expect(green.err).toBe("gate: no base to compare to (set FORGE_CHECK_BASE): migration pins not checked\n"); | |
| 761 | 764 | }, 30_000); |
| 762 | 765 | |
| 763 | 766 | test("forge.json audits before tsc, and the native tsc runs incremental from the check's cache", async () => { |
new above
offline
If no base resolves, this returns [] and the check passes without checking. Print a one-line warning to stderr.