trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns16g rEventsFlows

Automate a review finding as a lint or test: Direction: the repo says pipeline first (#6534, #6611, #5276, #3316, #6217, #642 #8842

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/8839-automate-a-review-finding-as-a-lint-or-t → main 2 commits fork

  • Merged. scout/8839-automate-a-review-finding-as-a-lint-or-t was merged into main as eb699a1.

Review

Sign in to review this change.
3 files changed +137 −1
scripts/gate.ts +20 −1
@@ -255,10 +255,29 @@ export async function pinRefusal(cwd: string = process.cwd()): Promise<string[]>
255255 return [...edits, ...landed, ...missing];
256256 }
257257
258 /**
259 * A pull that adds a feature must say, in a commit message, why it goes
260 * ahead of the pipeline (#8839). Same base as `pinRefusal`; with no base to
261 * compare to it passes, since that function already says so.
262 */
263 export async function whyNowRefusal(cwd: string = process.cwd()): Promise<string[]> {
264 const { parseNameStatus } = await import("../src/kernel/pin_edits.ts");
265 const { whyNowProblems } = await import("../src/kernel/why_now.ts");
266 const base =
267 process.env.FORGE_CHECK_BASE ||
268 git(["merge-base", "origin/main", "HEAD"], cwd).out ||
269 git(["merge-base", "main", "HEAD"], cwd).out;
270 if (!base) return [];
271 const diff = git(["diff", "--name-status", "-M", `${base}..HEAD`], cwd);
272 const log = git(["log", "--format=%B%x00", `${base}..HEAD`], cwd);
273 if (!diff.ok || !log.ok) return [];
274 return whyNowProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean));
275 }
276
258277 // `bun scripts/gate.ts`: the audit alone, first in `forge.json`'s check, so
259278 // a stale exclusion fails in under a second — not after install and tsc.
260279 if (import.meta.main) {
261 const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal())];
280 const refusal = [...gateRefusal(presentTests()), ...(await pinRefusal()), ...(await whyNowRefusal())];
262281 for (const line of refusal) console.error(`gate: ${line}`);
263282 process.exit(refusal.length === 0 ? 0 : 2);
264283 }
src/kernel/why_now.ts added +31
@@ -0,0 +1,31 @@
1 /**
2 * The direction issue puts the pipeline first (#8839), so a new feature
3 * says why it goes ahead of it. A feature is a file a pull adds, at any
4 * depth, under an action, a page, an API route, an MCP tool or a terminal
5 * screen; the pull's commits must then say either `Pipeline: #N` (it is
6 * pipeline work) or `Why now: <reason>` (it goes ahead, and why). The check
7 * cannot read the pull description, so the answer lives in a commit
8 * message. It is a nudge, not a proof: it checks a reason was written, not
9 * that it is a good one.
10 */
11
12 import type { Change } from "./pin_edits.ts";
13
14 const FEATURE = /^src\/(actions|web\/pages|http\/api|http\/mcp\/tools|tui)\/.+\.ts$/;
15 const PIPELINE = /^Pipeline:\s*#\d+/im;
16 const WHY_NOW = /^Why now:\s*\S[^\n]{14,}/im;
17
18 /** The files a pull adds that make a new feature. */
19 export function featuresAdded(changes: readonly Change[]): string[] {
20 return changes.filter((c) => c.status.startsWith("A") && FEATURE.test(c.path)).map((c) => c.path);
21 }
22
23 /** One line per problem: none when no feature is added or a commit answers the direction. */
24 export function whyNowProblems(changes: readonly Change[], messages: readonly string[]): string[] {
25 const added = featuresAdded(changes);
26 if (added.length === 0) return [];
27 if (messages.some((m) => PIPELINE.test(m) || WHY_NOW.test(m))) return [];
28 return [
29 `rule "say why it goes ahead of the pipeline": this pull adds ${added.join(", ")} but no commit message has a "Why now: <reason>" line (15+ characters) or a "Pipeline: #N" line`,
30 ];
31 }
test/why-now.test.ts added +86
@@ -0,0 +1,86 @@
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 { whyNowRefusal } from "../scripts/gate.ts";
6 import { featuresAdded, whyNowProblems } from "../src/kernel/why_now.ts";
7
8 const add = (path: string) => ({ status: "A", path });
9
10 test("a new action, page or route is a feature; edits, tests and other files are not", () => {
11 expect(featuresAdded([add("src/actions/x.ts"), add("src/web/pages/y.ts"), add("src/http/api/z.ts")])).toHaveLength(3);
12 expect(featuresAdded([add("src/http/mcp/tools/x.ts"), add("src/tui/app/screen.ts"), add("src/http/api/v1/y.ts")])).toHaveLength(3);
13 expect(featuresAdded([{ status: "M", path: "src/actions/x.ts" }, add("test/x.test.ts"), add("src/core/x.ts")])).toEqual([]);
14 });
15
16 test("a feature with no answer to the direction is refused, naming the rule and the file", () => {
17 const [p] = whyNowProblems([add("src/actions/x.ts")], ["Add x\n\nIt does x."]);
18 expect(p).toContain("say why it goes ahead of the pipeline");
19 expect(p).toContain("src/actions/x.ts");
20 });
21
22 test("a Why now line or a Pipeline line answers it; a thin reason does not", () => {
23 const changes = [add("src/actions/x.ts")];
24 expect(whyNowProblems(changes, ["Add x\n\nWhy now: the owner asked for it in #1234 this week"])).toEqual([]);
25 expect(whyNowProblems(changes, ["Add x\n\nPipeline: #6534"])).toEqual([]);
26 expect(whyNowProblems(changes, ["Add x\n\nWhy now: soon"])).toHaveLength(1);
27 });
28
29 test("a pull that adds no feature is never asked", () => {
30 expect(whyNowProblems([add("test/a.test.ts")], ["Fix a"])).toEqual([]);
31 });
32
33 const root = mkdtempSync(join(tmpdir(), "why-now-"));
34 afterAll(() => rmSync(root, { recursive: true, force: true }));
35
36 const git = (cwd: string, ...args: string[]) => {
37 const r = Bun.spawnSync(["git", "-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd });
38 if (r.exitCode !== 0) throw new Error(r.stderr.toString());
39 return r.stdout.toString().trim();
40 };
41
42 /** A repo with a base commit, then one commit adding src/actions/x.ts with this message. */
43 function pull(name: string, message: string) {
44 const dir = join(root, name);
45 mkdirSync(join(dir, "src/actions"), { recursive: true });
46 git(dir, "init", "-q", "-b", "work");
47 writeFileSync(join(dir, "README"), "a");
48 git(dir, "add", "-A");
49 git(dir, "commit", "-qm", "base");
50 const base = git(dir, "rev-parse", "HEAD");
51 writeFileSync(join(dir, "src/actions/x.ts"), "export {};\n");
52 git(dir, "add", "-A");
53 git(dir, "commit", "-qm", message);
54 return { dir, base };
55 }
56
57 const withBase = async (base: string, f: () => Promise<string[]>) => {
58 const old = process.env.FORGE_CHECK_BASE;
59 process.env.FORGE_CHECK_BASE = base;
60 try {
61 return await f();
62 } finally {
63 if (old === undefined) delete process.env.FORGE_CHECK_BASE;
64 else process.env.FORGE_CHECK_BASE = old;
65 }
66 };
67
68 test("whyNowRefusal reads a real repo: a feature with no Why now line is refused", async () => {
69 const { dir, base } = pull("bare", "Add x");
70 const out = await withBase(base, () => whyNowRefusal(dir));
71 expect(out).toHaveLength(1);
72 expect(out[0]).toContain("say why it goes ahead of the pipeline");
73 expect(out[0]).toContain("src/actions/x.ts");
74 });
75
76 test("whyNowRefusal passes when any commit on the branch carries the line", async () => {
77 const { dir, base } = pull("answered", "Add x\n\nWhy now: the owner asked for it this week");
78 expect(await withBase(base, () => whyNowRefusal(dir))).toEqual([]);
79 });
80
81 test("whyNowRefusal asks nothing of a branch that adds no feature", async () => {
82 const { dir, base } = pull("quiet", "Add x");
83 git(dir, "rm", "-q", "src/actions/x.ts");
84 git(dir, "commit", "-qm", "drop x");
85 expect(await withBase(base, () => whyNowRefusal(dir))).toEqual([]);
86 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs