Public: anyone can read this; the crew can change it.
Automate a review finding as a lint or test: Direction: the repository is PIPELINE FIRST (#6534, #6611, #5276, #3316, #6217, #11037
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11036-automate-a-review-finding-as-a-lint-or-t → main 2 commits fork 2 threads · 2 unresolved
4 files
changed +141
−1
scripts/gate.ts +16 −1
| @@ -305,10 +305,25 @@ export async function bothDoorsRefusal(cwd: string = process.cwd()): Promise<str | ||
| 305 | 305 | return bothDoorsProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean)); |
| 306 | 306 | } |
| 307 | 307 | |
| 308 | /** A pull that adds a feature without touching the pipeline says why (#11036). */ | |
| 309 | export async function directionRefusal(cwd: string = process.cwd()): Promise<string[]> { | |
| 310 | const { parseNameStatus } = await import("../src/kernel/pin_edits.ts"); | |
| 311 | const { directionProblems } = await import("../src/kernel/direction.ts"); | |
| 312 | const base = | |
| 313 | process.env.FORGE_CHECK_BASE || | |
| 314 | git(["merge-base", "origin/main", "HEAD"], cwd).out || | |
| 315 | git(["merge-base", "main", "HEAD"], cwd).out; | |
| 316 | if (!base) return []; | |
| 317 | const diff = git(["diff", "--name-status", "-M", `${base}..HEAD`], cwd); | |
| 318 | const log = git(["log", "--format=%B%x00", `${base}..HEAD`], cwd); | |
| 319 | if (!diff.ok || !log.ok) return []; | |
| 320 | return directionProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean)); | |
| 321 | } | |
| 322 | ||
| 308 | 323 | // `bun scripts/gate.ts`: the audit alone, first in `forge.json`'s check, so |
| 309 | 324 | // a stale exclusion fails in under a second — not after install and tsc. |
| 310 | 325 | if (import.meta.main) { |
| 311 | const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal()), ...(await tryItRefusal()), ...(await bothDoorsRefusal()), ...reviewRefusal()]; | |
| 326 | const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal()), ...(await tryItRefusal()), ...(await bothDoorsRefusal()), ...(await directionRefusal()), ...reviewRefusal()]; | |
| 312 | 327 | for (const line of refusal) console.error(`gate: ${line}`); |
| 313 | 328 | process.exit(refusal.length === 0 ? 0 : 2); |
| 314 | 329 | } |
src/kernel/direction.ts added +26
| @@ -0,0 +1,26 @@ | ||
| 1 | /** | |
| 2 | * Four reviews in a week said "pipeline first, before any feature" (#11036). | |
| 3 | * A pull that adds a feature surface — a page (`src/web/pages/*.ts`), a | |
| 4 | * terminal screen (`src/tui/`) or a how-to (`docs/features/*.md`) — and | |
| 5 | * touches no pipeline code (`scripts/`, `src/flow/`, `src/nightly/`, | |
| 6 | * `src/checks/`, `forge.json`) must say why it goes ahead of the pipeline | |
| 7 | * items, as `Direction: <why>` in a commit message. | |
| 8 | */ | |
| 9 | ||
| 10 | import type { Change } from "./pin_edits.ts"; | |
| 11 | ||
| 12 | const FEATURE = /^(src\/web\/pages\/[^/]+\.ts|src\/tui\/.+\.ts|docs\/features\/[^/]+\.md)$/; | |
| 13 | const PIPELINE = /^(scripts\/|src\/flow\/|src\/nightly\/|src\/checks\/|forge\.json$)/; | |
| 14 | const WAIVER = /^Direction:[ \t]*\S[^\n]{9,}/im; | |
| 15 | ||
|
||
| 16 | /** One line per problem: none unless a feature is added with no pipeline change and no Direction line. */ | |
| 17 | export function directionProblems(changes: readonly Change[], messages: readonly string[]): string[] { | |
| 18 | const features = changes.filter((c) => c.status.startsWith("A") && FEATURE.test(c.path)).map((c) => c.path); | |
| 19 | if (features.length === 0) return []; | |
| 20 | if (changes.some((c) => PIPELINE.test(c.path))) return []; | |
| 21 | if (messages.some((m) => WAIVER.test(m))) return []; | |
| 22 | const shown = features.slice(0, 3).join(", ") + (features.length > 3 ? ` and ${features.length - 3} more` : ""); | |
| 23 | return [ | |
| 24 | `rule "pipeline first": this pull adds a feature (${shown}) and no pipeline change (scripts/, src/flow/, src/nightly/, src/checks/, forge.json); change the pipeline, or put a "Direction: <why this goes ahead of the pipeline items>" line (10+ characters) in a commit message`, | |
| 25 | ]; | |
| 26 | } |
test/direction-refusal.test.ts added +71
| @@ -0,0 +1,71 @@ | ||
| 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 { directionRefusal } from "../scripts/gate.ts"; | |
| 6 | ||
| 7 | const root = mkdtempSync(join(tmpdir(), "direction-")); | |
| 8 | afterAll(() => rmSync(root, { recursive: true, force: true })); | |
| 9 | ||
| 10 | const run = (cwd: string, ...args: string[]) => { | |
| 11 | const r = Bun.spawnSync(["git", "-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd }); | |
| 12 | if (r.exitCode !== 0) throw new Error(r.stderr.toString()); | |
| 13 | return r.stdout.toString().trim(); | |
| 14 | }; | |
| 15 | const put = (dir: string, file: string, text: string) => { | |
| 16 | mkdirSync(join(dir, file, ".."), { recursive: true }); | |
| 17 | writeFileSync(join(dir, file), text); | |
| 18 | }; | |
| 19 | const withBase = async (base: string | undefined, f: () => Promise<string[]>) => { | |
| 20 | const old = process.env.FORGE_CHECK_BASE; | |
| 21 | if (base === undefined) delete process.env.FORGE_CHECK_BASE; | |
| 22 | else process.env.FORGE_CHECK_BASE = base; | |
| 23 | try { | |
| 24 | return await f(); | |
| 25 | } finally { | |
| 26 | if (old === undefined) delete process.env.FORGE_CHECK_BASE; | |
| 27 | else process.env.FORGE_CHECK_BASE = old; | |
| 28 | } | |
| 29 | }; | |
| 30 | function repo(name: string) { | |
| 31 | const dir = join(root, name); | |
| 32 | mkdirSync(dir); | |
| 33 | run(dir, "init", "-q", "-b", "work"); | |
| 34 | put(dir, "README.md", "x"); | |
| 35 | run(dir, "add", "-A"); | |
| 36 | run(dir, "commit", "-qm", "base"); | |
| 37 | return { dir, base: run(dir, "rev-parse", "HEAD") }; | |
| 38 | } | |
| 39 | ||
| 40 | const gateMain = (cwd: string, base: string) => { | |
| 41 | const r = Bun.spawnSync(["bun", join(import.meta.dir, "../scripts/gate.ts")], { | |
| 42 | cwd, | |
| 43 | env: { ...process.env, FORGE_CHECK_BASE: base }, | |
| 44 | }); | |
| 45 | return r.stderr.toString(); | |
| 46 | }; | |
| 47 | ||
| 48 | test("a range that adds a page alone is refused; a Direction line in its message waives it", async () => { | |
| 49 | const { dir, base } = repo("page"); | |
| 50 | put(dir, "src/web/pages/machines.ts", "x"); | |
| 51 | run(dir, "add", "-A"); | |
| 52 | run(dir, "commit", "-qm", "Add machines"); | |
| 53 | const out = await withBase(base, () => directionRefusal(dir)); | |
| 54 | expect(out).toHaveLength(1); | |
| 55 | expect(out[0]).toContain('rule "pipeline first"'); | |
| 56 | // the gate's own entry point runs the rule, so removing its call goes red | |
| 57 | expect(gateMain(dir, base)).toContain('rule "pipeline first"'); | |
| 58 | ||
| 59 | run(dir, "commit", "-q", "--allow-empty", "-m", "Waive\n\nDirection: a small door onto the spawn path, owner asked"); | |
| 60 | expect(await withBase(base, () => directionRefusal(dir))).toEqual([]); | |
| 61 | expect(gateMain(dir, base)).not.toContain('rule "pipeline first"'); | |
| 62 | }); | |
| 63 | ||
| 64 | test("a range that adds the page with a pipeline change passes", async () => { | |
| 65 | const { dir, base } = repo("pipe"); | |
| 66 | put(dir, "src/web/pages/machines.ts", "x"); | |
| 67 | put(dir, "scripts/lint.ts", "x"); | |
| 68 | run(dir, "add", "-A"); | |
| 69 | run(dir, "commit", "-qm", "Add machines"); | |
| 70 | expect(await withBase(base, () => directionRefusal(dir))).toEqual([]); | |
| 71 | }); |
test/direction.test.ts added +28
| @@ -0,0 +1,28 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { directionProblems } from "../src/kernel/direction.ts"; | |
| 3 | ||
| 4 | const ch = (status: string, path: string) => ({ status, path }); | |
| 5 | const page = ch("A", "src/web/pages/machines.ts"); | |
| 6 | ||
| 7 | test("a new feature with no pipeline change is refused, naming the rule", () => { | |
| 8 | const out = directionProblems([page], ["Add machines"]); | |
| 9 | expect(out).toHaveLength(1); | |
| 10 | expect(out[0]).toContain('rule "pipeline first"'); | |
| 11 | expect(out[0]).toContain("src/web/pages/machines.ts"); | |
| 12 | expect(out[0]).toContain('"Direction: <why'); | |
| 13 | }); | |
| 14 | ||
| 15 | test("a pipeline change, or a Direction line of 10+ characters, passes", () => { | |
| 16 | expect(directionProblems([page, ch("M", "scripts/gate.ts")], [])).toEqual([]); | |
| 17 | expect(directionProblems([page], ["x\n\nDirection: a small door onto the spawn path, owner asked on #7642"])).toEqual([]); | |
| 18 | expect(directionProblems([page], ["Direction: no"])).toHaveLength(1); | |
| 19 | }); | |
| 20 | ||
| 21 | test("edits and non-feature files are never asked", () => { | |
| 22 | expect(directionProblems([ch("M", "src/web/pages/machines.ts"), ch("A", "src/core/x.ts")], [])).toEqual([]); | |
| 23 | expect(directionProblems([ch("A", "docs/features/x.md")], [])).toHaveLength(1); | |
| 24 | }); | |
| 25 | ||
| 26 | test("the reason must sit on the Direction line itself, not a later paragraph", () => { | |
| 27 | expect(directionProblems([page], ["Direction:\n\nsome later paragraph of text"])).toHaveLength(1); | |
| 28 | }); |
Untested wiring. Add a git-repo test, as for
bothDoorsRefusal, that fails if this function or its call in the main list is removed.