Public: anyone can read this; the crew can change it.
Automate a review finding as a lint or test: Both doors: add src/tui/ screen for the roadmap and its how-to as docs/features/ #12370
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/12369-automate-a-review-finding-as-a-lint-or-t → main 7 commits fork 3 threads · 3 unresolved
7 files
changed +195
−14
AGENTS.md +1 −1
| @@ -3,7 +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 | A pull that adds a page (`src/web/pages/*.ts`) also adds a terminal screen under `src/tui/screens/` that another module imports (its palette entry) and a `docs/features/<name>.md`, or a commit line `No door: <why>`; `bun scripts/gate.ts` refuses it otherwise ("both doors", #10628). | |
| 7 | 7 | A pull that changes `src/web/` (a `.ts` or `.css`) names, in a commit line `Looks: <path>`, the page of `app.looks` in forge.json whose before/after pictures show it, or says `No look: <why>`; `bun scripts/gate.ts` refuses it otherwise ("before/after looks", #12362). |
| 8 | 8 | |
| 9 | 9 | 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. |
scripts/gate.ts +6 −1
| @@ -489,7 +489,12 @@ export async function bothDoorsRefusal(cwd: string = process.cwd()): Promise<str | ||
| 489 | 489 | const diff = git(["diff", "--name-status", "-M", `${base}..HEAD`], cwd); |
| 490 | 490 | const log = git(["log", "--format=%B%x00", `${base}..HEAD`], cwd); |
| 491 | 491 | if (!diff.ok || !log.ok) return []; |
| 492 | return bothDoorsProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean)); | |
| 492 | const { readTui, screenOpened } = await import("../src/kernel/tui_entry.ts"); | |
outdated
|
||
| 493 | const changes = parseNameStatus(diff.out); | |
| 494 | const added = new Set(changes.filter((c) => c.status.startsWith("A")).map((c) => c.path)); | |
| 495 | const files = readTui(cwd); | |
| 496 | const imported = (screen: string) => screenOpened(screen, files, added); | |
| 497 | return bothDoorsProblems(changes, log.out.split("\0").map((m) => m.trim()).filter(Boolean), imported); | |
| 493 | 498 | } |
| 494 | 499 | |
| 495 | 500 | /** A pull that changes a screen says which pictured page shows it (#12362). */ |
src/kernel/both_doors.ts +15 −4
| @@ -10,20 +10,31 @@ | ||
| 10 | 10 | import type { Change } from "./pin_edits.ts"; |
| 11 | 11 | |
| 12 | 12 | const PAGE = /^src\/web\/pages\/[^/]+\.ts$/; |
| 13 | const SCREEN = /^src\/tui\/.+\.ts$/; | |
| 13 | const SCREEN = /^src\/tui\/screens\/[^/]+\.ts$/; | |
| 14 | 14 | const DOC = /^docs\/features\/[^/]+\.md$/; |
| 15 | 15 | const WAIVER = /^No door:\s*\S[^\n]{9,}/im; |
| 16 | 16 | |
| 17 | 17 | const added = (changes: readonly Change[], re: RegExp) => |
| 18 | 18 | changes.filter((c) => c.status.startsWith("A") && re.test(c.path)).map((c) => c.path); |
| 19 | 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[] { | |
| 20 | /** | |
| 21 | * One line per problem: none when no page is added, both doors come with it, or a commit | |
| 22 | * waives them. The screen is a file in `src/tui/screens/` (a helper under `src/tui/ui/` is | |
| 23 | * no door), and at least one added screen is opened (`imported`, see tui_entry.ts): a screen | |
| 24 | * nobody can open is not a door. | |
| 25 | */ | |
| 26 | export function bothDoorsProblems( | |
| 27 | changes: readonly Change[], | |
| 28 | messages: readonly string[], | |
| 29 | imported: (screen: string) => boolean, | |
| 30 | ): string[] { | |
| 22 | 31 | const pages = added(changes, PAGE); |
| 23 | 32 | if (pages.length === 0) return []; |
| 24 | 33 | if (messages.some((m) => WAIVER.test(m))) return []; |
| 25 | 34 | const missing: string[] = []; |
| 26 | if (added(changes, SCREEN).length === 0) missing.push("terminal screen in src/tui/ (entry where AGENTS.md says)"); | |
| 35 | const screens = added(changes, SCREEN); | |
| 36 | if (screens.length === 0) missing.push("terminal screen in src/tui/screens/ (entry where AGENTS.md says)"); | |
Because |
||
| 37 | else if (!screens.some(imported)) missing.push(`entry that opens ${screens[0]} (add its palette entry per AGENTS.md)`); | |
| 27 | 38 | if (added(changes, DOC).length === 0) missing.push("docs/features/<name>.md how-to"); |
| 28 | 39 | if (missing.length === 0) return []; |
| 29 | 40 | const shown = pages.slice(0, 3).join(", ") + (pages.length > 3 ? ` and ${pages.length - 3} more` : ""); |
src/kernel/tui_entry.ts added +50
| @@ -0,0 +1,50 @@ | ||
| 1 | /** | |
| 2 | * Is a new terminal screen reachable (#10628)? A screen is opened when a module that | |
| 3 | * already existed before the pull (one added alongside it can be another orphan) imports | |
| 4 | * it by a path that resolves to the screen's file: `from "./x.ts"`, `from './x.ts'`, | |
| 5 | * `import "./x.ts"` or `import("./x.ts")`. A basename match is no proof (`ui/x.ts` is | |
| 6 | * not `screens/x.ts`), and the screen's own imports of itself do not count. | |
| 7 | */ | |
| 8 | ||
| 9 | import { existsSync, readdirSync, readFileSync } from "node:fs"; | |
| 10 | import { join, posix } from "node:path"; | |
| 11 | ||
| 12 | export type Source = { path: string; text: string }; | |
| 13 | ||
| 14 | const SPECIFIER = /\b(?:from\s*|import\s*\(?\s*)(["'])([^"'\n]+)\1/g; | |
| 15 | ||
| 16 | /** Relative specifiers a module names, in any static or dynamic form. */ | |
| 17 | export function specifiers(text: string): string[] { | |
| 18 | return [...text.matchAll(SPECIFIER)].map((m) => m[2]!).filter((s) => s.startsWith(".")); | |
| 19 | } | |
| 20 | ||
| 21 | /** The repository path a specifier from `importer` names (`.ts` added when absent). */ | |
| 22 | export function resolveSpecifier(importer: string, spec: string): string { | |
| 23 | const path = posix.normalize(posix.join(posix.dirname(importer), spec)); | |
| 24 | return /\.[cm]?[jt]s$/.test(path) ? path : `${path}.ts`; | |
| 25 | } | |
| 26 | ||
| 27 | /** True when a file in `files` outside `added` (and not the screen) imports `screen`. */ | |
| 28 | export function screenOpened(screen: string, files: readonly Source[], added: ReadonlySet<string>): boolean { | |
| 29 | return files.some( | |
| 30 | (f) => | |
| 31 | f.path !== screen && | |
| 32 | !added.has(f.path) && | |
| 33 | specifiers(f.text).some((s) => resolveSpecifier(f.path, s) === screen), | |
| 34 | ); | |
| 35 | } | |
| 36 | ||
| 37 | /** Every `.ts` file under `src/tui` in `cwd`; none when the directory is absent. */ | |
| 38 | export function readTui(cwd: string): Source[] { | |
| 39 | const out: Source[] = []; | |
| 40 | const walk = (dir: string) => { | |
| 41 | if (!existsSync(join(cwd, dir))) return; | |
| 42 | for (const e of readdirSync(join(cwd, dir), { withFileTypes: true })) { | |
| 43 | const rel = `${dir}/${e.name}`; | |
| 44 | if (e.isDirectory()) walk(rel); | |
| 45 | else if (e.name.endsWith(".ts")) out.push({ path: rel, text: readFileSync(join(cwd, rel), "utf8") }); | |
| 46 | } | |
| 47 | }; | |
| 48 | walk("src/tui"); | |
| 49 | return out; | |
| 50 | } |
test/both-doors-refusal.test.ts +60
| @@ -32,6 +32,8 @@ function repo(name: string) { | ||
| 32 | 32 | mkdirSync(dir); |
| 33 | 33 | run(dir, "init", "-q", "-b", "work"); |
| 34 | 34 | put(dir, "README.md", "x"); |
| 35 | put(dir, "src/tui/screens/entries.ts", "export const ENTRIES = [];\n"); | |
| 36 | put(dir, "src/tui/ui/traffic.ts", "export const x = 1;\n"); | |
| 35 | 37 | run(dir, "add", "-A"); |
| 36 | 38 | run(dir, "commit", "-qm", "base"); |
| 37 | 39 | return { dir, base: run(dir, "rev-parse", "HEAD") }; |
| @@ -55,8 +57,66 @@ test("a range that adds the page with both doors passes", async () => { | ||
| 55 | 57 | const { dir, base } = repo("doors"); |
| 56 | 58 | put(dir, "src/web/pages/traffic.ts", "x"); |
| 57 | 59 | put(dir, "src/tui/screens/traffic.ts", "x"); |
| 60 | put(dir, "src/tui/screens/entries.ts", "import { entry } from './traffic.ts';"); | |
| 58 | 61 | put(dir, "docs/features/traffic.md", "x"); |
| 59 | 62 | run(dir, "add", "-A"); |
| 60 | 63 | run(dir, "commit", "-qm", "Add traffic"); |
| 61 | 64 | expect(await withBase(base, () => bothDoorsRefusal(dir))).toEqual([]); |
| 62 | 65 | }); |
| 66 | ||
| 67 | test("a screen nothing imports is refused, naming the entry it lacks", async () => { | |
| 68 | const { dir, base } = repo("orphan"); | |
| 69 | put(dir, "src/web/pages/traffic.ts", "x"); | |
| 70 | put(dir, "src/tui/screens/traffic.ts", "x"); | |
| 71 | put(dir, "docs/features/traffic.md", "x"); | |
| 72 | run(dir, "add", "-A"); | |
| 73 | run(dir, "commit", "-qm", "Add traffic"); | |
| 74 | const out = await withBase(base, () => bothDoorsRefusal(dir)); | |
| 75 | expect(out).toHaveLength(1); | |
| 76 | expect(out[0]).toContain("no entry that opens src/tui/screens/traffic.ts"); | |
| 77 | }); | |
| 78 | ||
| 79 | const addOnly = (name: string, files: Record<string, string>) => { | |
| 80 | const { dir, base } = repo(name); | |
| 81 | put(dir, "src/web/pages/traffic.ts", "x"); | |
| 82 | put(dir, "src/tui/screens/traffic.ts", "x"); | |
| 83 | put(dir, "docs/features/traffic.md", "x"); | |
| 84 | for (const [k, v] of Object.entries(files)) put(dir, k, v); | |
| 85 | run(dir, "add", "-A"); | |
| 86 | run(dir, "commit", "-qm", "Add traffic"); | |
| 87 | return withBase(base, () => bothDoorsRefusal(dir)); | |
| 88 | }; | |
| 89 | ||
| 90 | test("a helper with the screen's basename importing it is not an entry", async () => { | |
| 91 | const out = await addOnly("helper", { "src/tui/ui/traffic.ts": 'import "./traffic.ts";' }); | |
| 92 | expect(out[0]).toContain("no entry that opens"); | |
| 93 | }); | |
| 94 | ||
| 95 | test("a second new screen importing the first is not an entry", async () => { | |
| 96 | const out = await addOnly("pair", { | |
| 97 | "src/tui/screens/other.ts": 'import "./traffic.ts";', | |
| 98 | "src/tui/screens/traffic.ts": 'import "./other.ts";', | |
| 99 | }); | |
| 100 | expect(out[0]).toContain("no entry that opens"); | |
| 101 | }); | |
| 102 | ||
| 103 | test("a single-quoted or dynamic import from the entries module opens it", async () => { | |
| 104 | expect(await addOnly("single", { "src/tui/screens/entries.ts": "import { e } from './traffic.ts';" })).toEqual([]); | |
| 105 | expect(await addOnly("dynamic", { "src/tui/screens/entries.ts": 'const m = () => import("./traffic.ts");' })).toEqual([]); | |
| 106 | }); | |
| 107 | ||
| 108 | test("a repository without src/tui before the pull does not crash the gate", async () => { | |
| 109 | const dir = join(root, "bare"); | |
| 110 | mkdirSync(dir); | |
| 111 | run(dir, "init", "-q", "-b", "work"); | |
| 112 | put(dir, "README.md", "x"); | |
| 113 | run(dir, "add", "-A"); | |
| 114 | run(dir, "commit", "-qm", "base"); | |
| 115 | const base = run(dir, "rev-parse", "HEAD"); | |
| 116 | put(dir, "src/web/pages/traffic.ts", "x"); | |
| 117 | put(dir, "src/tui/screens/traffic.ts", "x"); | |
| 118 | put(dir, "docs/features/traffic.md", "x"); | |
| 119 | run(dir, "add", "-A"); | |
| 120 | run(dir, "commit", "-qm", "Add traffic"); | |
| 121 | expect((await withBase(base, () => bothDoorsRefusal(dir)))[0]).toContain("no entry that opens"); | |
| 122 | }); |
test/both-doors.test.ts +19 −8
| @@ -4,28 +4,39 @@ import { bothDoorsProblems } from "../src/kernel/both_doors.ts"; | ||
| 4 | 4 | const ch = (status: string, path: string) => ({ status, path }); |
| 5 | 5 | const page = ch("A", "src/web/pages/traffic.ts"); |
| 6 | 6 | |
| 7 | const TUI = "terminal screen in src/tui/ (entry where AGENTS.md says)"; | |
| 7 | const TUI = "terminal screen in src/tui/screens/ (entry where AGENTS.md says)"; | |
| 8 | 8 | const DOCS = "docs/features/<name>.md how-to"; |
| 9 | 9 | const tail = '; add them, or put a "No door: <why>" line (10+ characters) in a commit message'; |
| 10 | 10 | |
| 11 | 11 | test("a new page with neither door is refused, naming the rule and both missing doors", () => { |
| 12 | expect(bothDoorsProblems([page], ["Add traffic"])).toEqual([ | |
| 12 | expect(bothDoorsProblems([page], ["Add traffic"], () => true)).toEqual([ | |
| 13 | 13 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${TUI} and no ${DOCS}${tail}`, |
| 14 | 14 | ]); |
| 15 | 15 | }); |
| 16 | 16 | |
| 17 | 17 | test("one door is not enough: the other is named alone", () => { |
| 18 | expect(bothDoorsProblems([page, ch("A", "src/tui/screens/traffic.ts")], [])).toEqual([ | |
| 18 | expect(bothDoorsProblems([page, ch("A", "src/tui/screens/traffic.ts")], [], () => true)).toEqual([ | |
| 19 | 19 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${DOCS}${tail}`, |
| 20 | 20 | ]); |
| 21 | expect(bothDoorsProblems([page, ch("A", "docs/features/traffic.md")], [])).toEqual([ | |
| 21 | expect(bothDoorsProblems([page, ch("A", "docs/features/traffic.md")], [], () => true)).toEqual([ | |
| 22 | 22 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${TUI}${tail}`, |
| 23 | 23 | ]); |
| 24 | 24 | }); |
| 25 | 25 | |
| 26 | 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([]); | |
| 27 | expect(bothDoorsProblems([page, ch("A", "src/tui/screens/t.ts"), ch("A", "docs/features/t.md")], [], () => true)).toEqual([]); | |
| 28 | expect(bothDoorsProblems([page], ["x\n\nNo door: a public landing page, nothing to administer"], () => true)).toEqual([]); | |
| 29 | expect(bothDoorsProblems([page], ["No door: no"], () => true)).toHaveLength(1); | |
| 30 | expect(bothDoorsProblems([ch("M", "src/web/pages/traffic.ts"), ch("A", "src/core/x.ts")], [], () => true)).toEqual([]); | |
| 31 | }); | |
| 32 | ||
| 33 | test("a helper under src/tui/ is not a screen, and a screen nothing imports is not a door", () => { | |
| 34 | expect(bothDoorsProblems([page, ch("A", "src/tui/ui/traffic.ts"), ch("A", "docs/features/traffic.md")], [], () => true)).toEqual([ | |
| 35 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no ${TUI}${tail}`, | |
| 36 | ]); | |
| 37 | const both = [page, ch("A", "src/tui/screens/traffic.ts"), ch("A", "docs/features/traffic.md")]; | |
| 38 | expect(bothDoorsProblems(both, [], () => true)).toEqual([]); | |
| 39 | expect(bothDoorsProblems(both, [], () => false)).toEqual([ | |
| 40 | `rule "both doors": this pull adds src/web/pages/traffic.ts but no entry that opens src/tui/screens/traffic.ts (add its palette entry per AGENTS.md)${tail}`, | |
| 41 | ]); | |
| 31 | 42 | }); |
test/tui-entry.test.ts added +44
| @@ -0,0 +1,44 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { mkdtempSync } from "node:fs"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { readTui, resolveSpecifier, screenOpened, specifiers } from "../src/kernel/tui_entry.ts"; | |
| 6 | ||
| 7 | const SCREEN = "src/tui/screens/traffic.ts"; | |
| 8 | const none = new Set<string>(); | |
| 9 | const f = (path: string, text: string) => ({ path, text }); | |
| 10 | ||
| 11 | test("specifiers read double, single quoted, bare and dynamic imports", () => { | |
| 12 | const text = `import { a } from "./a.ts";\nimport {b} from './b.ts'\nimport "./c.ts";\nconst d = await import('./d.ts');\nimport x from "node:fs";`; | |
| 13 | expect(specifiers(text)).toEqual(["./a.ts", "./b.ts", "./c.ts", "./d.ts"]); | |
| 14 | }); | |
| 15 | ||
| 16 | test("a specifier resolves against its importer", () => { | |
| 17 | expect(resolveSpecifier("src/tui/screens/entries.ts", "./traffic.ts")).toBe(SCREEN); | |
| 18 | expect(resolveSpecifier("src/tui/forge.ts", "./screens/traffic")).toBe(SCREEN); | |
| 19 | expect(resolveSpecifier("src/tui/ui/x.ts", "../screens/traffic.ts")).toBe(SCREEN); | |
| 20 | }); | |
| 21 | ||
| 22 | test("an existing module importing the screen's real path opens it, in any quote style", () => { | |
| 23 | for (const line of [`import { e } from "./traffic.ts";`, `import { e } from './traffic.ts';`, `const m = await import("./traffic.ts");`]) { | |
| 24 | expect(screenOpened(SCREEN, [f("src/tui/screens/entries.ts", line)], none)).toBe(true); | |
| 25 | } | |
| 26 | expect(screenOpened(SCREEN, [f("src/tui/forge.ts", `import { e } from "./screens/traffic.ts";`)], none)).toBe(true); | |
| 27 | }); | |
| 28 | ||
| 29 | test("an import with the same basename from another directory does not open it", () => { | |
| 30 | expect(screenOpened(SCREEN, [f("src/tui/ui/traffic.ts", `import "./traffic.ts";`)], none)).toBe(false); | |
| 31 | expect(screenOpened(SCREEN, [f("src/tui/forge.ts", `import { x } from "./ui/traffic.ts";`)], none)).toBe(false); | |
| 32 | expect(screenOpened(SCREEN, [f("src/tui/forge.ts", `import { x } from "../web/traffic.ts";`)], none)).toBe(false); | |
| 33 | }); | |
| 34 | ||
| 35 | test("a module added in the same pull, or the screen itself, is no entry", () => { | |
| 36 | const other = "src/tui/screens/other.ts"; | |
| 37 | const files = [f(other, `import "./traffic.ts";`), f(SCREEN, `import "./traffic.ts";`)]; | |
| 38 | expect(screenOpened(SCREEN, files, new Set([other, SCREEN]))).toBe(false); | |
| 39 | expect(screenOpened(SCREEN, files, new Set([SCREEN]))).toBe(true); | |
| 40 | }); | |
| 41 | ||
| 42 | test("readTui finds nothing when src/tui is absent", () => { | |
| 43 | expect(readTui(mkdtempSync(join(tmpdir(), "tui-entry-")))).toEqual([]); | |
| 44 | }); |
new above
offline
Basename-only match: any import ending
/<name>.tscounts, even from a helper that is not an entry. Resolve to the screen's real path. Single-quoted and dynamic imports are missed too.