Public: anyone can read this; the crew can change it.
Automate a review finding as a lint or test: Both doors missing: the page has no terminal screen in `src/tui/` with its entry #10694
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/10628-automate-a-review-finding-as-a-lint-or-t → main 5 commits fork 3 threads · 3 unresolved
5 files
changed +143
−1
AGENTS.md +1
| @@ -3,6 +3,7 @@ | ||
| 3 | 3 | Read `CLAUDE.md` first: it is the rulebook for agents editing this repo. |
| 4 | 4 | |
| 5 | 5 | README.md is capped: a new feature's how-to goes in `docs/features/<name>.md`, not a new README section. |
| 6 | A pull that adds a page (`src/web/pages/*.ts`) also adds a terminal screen under `src/tui/` and a `docs/features/<name>.md`, or a commit line `No door: <why>`; `bun scripts/gate.ts` refuses it otherwise ("both doors", #10628). | |
| 6 | 7 | |
| 7 | 8 | When you write an issue, end it with a `Files:` line naming the files the work touches (`Files: src/a.ts, docs/b.md`). The hot-file hold reads only that line when it is there; without it, every path the issue mentions counts. |
| 8 | 9 |
scripts/gate.ts +16 −1
| @@ -285,10 +285,25 @@ export async function tryItRefusal(cwd: string = process.cwd()): Promise<string[ | ||
| 285 | 285 | return tryItProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean)); |
| 286 | 286 | } |
| 287 | 287 | |
| 288 | /** A pull that adds a page adds its terminal screen and how-to too (#10628). */ | |
| 289 | export async function bothDoorsRefusal(cwd: string = process.cwd()): Promise<string[]> { | |
| 290 | const { parseNameStatus } = await import("../src/kernel/pin_edits.ts"); | |
| 291 | const { bothDoorsProblems } = await import("../src/kernel/both_doors.ts"); | |
| 292 | const base = | |
| 293 | process.env.FORGE_CHECK_BASE || | |
| 294 | git(["merge-base", "origin/main", "HEAD"], cwd).out || | |
| 295 | git(["merge-base", "main", "HEAD"], cwd).out; | |
| 296 | if (!base) return []; | |
| 297 | const diff = git(["diff", "--name-status", "-M", `${base}..HEAD`], cwd); | |
| 298 | const log = git(["log", "--format=%B%x00", `${base}..HEAD`], cwd); | |
| 299 | if (!diff.ok || !log.ok) return []; | |
| 300 | return bothDoorsProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean)); | |
| 301 | } | |
| 302 | ||
| 288 | 303 | // `bun scripts/gate.ts`: the audit alone, first in `forge.json`'s check, so |
| 289 | 304 | // a stale exclusion fails in under a second — not after install and tsc. |
| 290 | 305 | if (import.meta.main) { |
| 291 | const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal()), ...(await tryItRefusal()), ...reviewRefusal()]; | |
| 306 | const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal()), ...(await tryItRefusal()), ...(await bothDoorsRefusal()), ...reviewRefusal()]; | |
| 292 | 307 | for (const line of refusal) console.error(`gate: ${line}`); |
| 293 | 308 | process.exit(refusal.length === 0 ? 0 : 2); |
| 294 | 309 | } |
src/kernel/both_doors.ts added +33
| @@ -0,0 +1,33 @@ | ||
| 1 | /** | |
| 2 | * Four reviews in a week said "both doors missing" (#10628): a new page | |
| 3 | * without a terminal screen in `src/tui/` (entry where AGENTS.md says) and | |
| 4 | * without a `docs/features/<name>.md` how-to. A pull that adds a page | |
| 5 | * (`src/web/pages/*.ts`) therefore adds a terminal screen (a new file under | |
| 6 | * `src/tui/`) and a features doc, or says in a commit message | |
| 7 | * `No door: <why this page needs no terminal screen or how-to>`. | |
| 8 | */ | |
| 9 | ||
| 10 | import type { Change } from "./pin_edits.ts"; | |
| 11 | ||
| 12 | const PAGE = /^src\/web\/pages\/[^/]+\.ts$/; | |
| 13 | const SCREEN = /^src\/tui\/.+\.ts$/; | |
| 14 | const DOC = /^docs\/features\/[^/]+\.md$/; | |
| 15 | const WAIVER = /^No door:\s*\S[^\n]{9,}/im; | |
| 16 | ||
| 17 | const added = (changes: readonly Change[], re: RegExp) => | |
| 18 | changes.filter((c) => c.status.startsWith("A") && re.test(c.path)).map((c) => c.path); | |
| 19 | ||
| 20 | /** One line per problem: none when no page is added, both doors come with it, or a commit waives them. */ | |
| 21 | export function bothDoorsProblems(changes: readonly Change[], messages: readonly string[]): string[] { | |
| 22 | const pages = added(changes, PAGE); | |
| 23 | if (pages.length === 0) return []; | |
| 24 | if (messages.some((m) => WAIVER.test(m))) return []; | |
| 25 | const missing: string[] = []; | |
| 26 | if (added(changes, SCREEN).length === 0) missing.push("terminal screen in src/tui/ (entry where AGENTS.md says)"); | |
| 27 | if (added(changes, DOC).length === 0) missing.push("docs/features/<name>.md how-to"); | |
| 28 | if (missing.length === 0) return []; | |
| 29 | const shown = pages.slice(0, 3).join(", ") + (pages.length > 3 ? ` and ${pages.length - 3} more` : ""); | |
| 30 | return [ | |
| 31 | `rule "both doors": this pull adds ${shown} but no ${missing.join(" and no ")}; add them, or put a "No door: <why>" line (10+ characters) in a commit message`, | |
| 32 | ]; | |
| 33 | } |
test/both-doors-refusal.test.ts added +62
| @@ -0,0 +1,62 @@ | ||
| 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 { bothDoorsRefusal } from "../scripts/gate.ts"; | |
| 6 | ||
| 7 | const root = mkdtempSync(join(tmpdir(), "both-doors-")); | |
| 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 | test("a commit range that adds a page alone is refused; a No door line in its message waives it", async () => { | |
| 41 | const { dir, base } = repo("page"); | |
| 42 | put(dir, "src/web/pages/traffic.ts", "x"); | |
| 43 | run(dir, "add", "-A"); | |
| 44 | run(dir, "commit", "-qm", "Add traffic"); | |
| 45 | const out = await withBase(base, () => bothDoorsRefusal(dir)); | |
| 46 | expect(out).toHaveLength(1); | |
| 47 | expect(out[0]).toContain("but no terminal screen in src/tui/"); | |
| 48 | expect(out[0]).toContain("and no docs/features/<name>.md how-to"); | |
| 49 | ||
| 50 | run(dir, "commit", "-q", "--allow-empty", "-m", "Waive\n\nNo door: a public landing page, nothing to administer"); | |
| 51 | expect(await withBase(base, () => bothDoorsRefusal(dir))).toEqual([]); | |
| 52 | }); | |
| 53 | ||
| 54 | test("a range that adds the page with both doors passes", async () => { | |
| 55 | const { dir, base } = repo("doors"); | |
| 56 | put(dir, "src/web/pages/traffic.ts", "x"); | |
| 57 | put(dir, "src/tui/screens/traffic.ts", "x"); | |
| 58 | put(dir, "docs/features/traffic.md", "x"); | |
| 59 | run(dir, "add", "-A"); | |
| 60 | run(dir, "commit", "-qm", "Add traffic"); | |
| 61 | expect(await withBase(base, () => bothDoorsRefusal(dir))).toEqual([]); | |
| 62 | }); |
test/both-doors.test.ts added +31
| @@ -0,0 +1,31 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { bothDoorsProblems } from "../src/kernel/both_doors.ts"; | |
| 3 | ||
| 4 | const ch = (status: string, path: string) => ({ status, path }); | |
| 5 | const page = ch("A", "src/web/pages/traffic.ts"); | |
| 6 | ||
| 7 | const TUI = "terminal screen in src/tui/ (entry where AGENTS.md says)"; | |
| 8 | const DOCS = "docs/features/<name>.md how-to"; | |
| 9 | const tail = '; add them, or put a "No door: <why>" line (10+ characters) in a commit message'; | |
| 10 | ||
| 11 | test("a new page with neither door is refused, naming the rule and both missing doors", () => { | |
| 12 | expect(bothDoorsProblems([page], ["Add traffic"])).toEqual([ | |
| 13 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${TUI} and no ${DOCS}${tail}`, | |
| 14 | ]); | |
| 15 | }); | |
| 16 | ||
| 17 | test("one door is not enough: the other is named alone", () => { | |
outdated
Vacuous: in this case only the doc is missing, so |
||
| 18 | expect(bothDoorsProblems([page, ch("A", "src/tui/screens/traffic.ts")], [])).toEqual([ | |
| 19 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${DOCS}${tail}`, | |
| 20 | ]); | |
outdated
Same: |
||
| 21 | expect(bothDoorsProblems([page, ch("A", "docs/features/traffic.md")], [])).toEqual([ | |
| 22 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${TUI}${tail}`, | |
| 23 | ]); | |
| 24 | }); | |
| 25 | ||
| 26 | test("both doors pass; so does a No door line; edits and other files are never asked", () => { | |
| 27 | expect(bothDoorsProblems([page, ch("A", "src/tui/screens/t.ts"), ch("A", "docs/features/t.md")], [])).toEqual([]); | |
| 28 | expect(bothDoorsProblems([page], ["x\n\nNo door: a public landing page, nothing to administer"])).toEqual([]); | |
| 29 | expect(bothDoorsProblems([page], ["No door: no"])).toHaveLength(1); | |
| 30 | expect(bothDoorsProblems([ch("M", "src/web/pages/traffic.ts"), ch("A", "src/core/x.ts")], [])).toEqual([]); | |
| 31 | }); |
Ungrammatical output: "this pull adds X but no a terminal screen ... and no a docs/features/...". Build the missing list without the leading article, e.g. missing = ["terminal screen in src/tui/ (entry where AGENTS.md says)", "docs/features/<name>.md how-to"] and keep
but no ${missing.join(" and no ")}.