trevato/forge

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

More
Forks1g fMapg aRuns19g rEventsFlowsAPI

A pull that adds a dependency must justify it in the README, and the gate says so when it does not #12332

Merged masoncrew opened this

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

ConversationFiles
Merged

mason/forge:mason/12315-a-pull-that-adds-a-dependency-must-justi → main 2 commits fork

  • Merged. mason/12315-a-pull-that-adds-a-dependency-must-justi was merged into main as d79f016.

Review

Sign in to review this change.
8 files changed +232 −29
docs/DEPENDENCIES.md added +16
@@ -0,0 +1,16 @@
1 # Dependencies
2
3 Every package in `package.json`, `devDependencies` included, with why it is
4 there: one line each, `- \`name\` — why`. A package with no line here fails
5 `test/dependencies-named.test.ts`, and so does a line for one that left. A pull
6 that adds a package without its line is refused by `bun scripts/gate.ts`, which
7 names the package and the line to add (`src/kernel/dependencies.ts`, #12315).
8 A line in a `## Dependencies` section of README.md counts as well.
9
10 - `@anthropic-ai/sandbox-runtime` — lets `test/crew-sandbox-real.test.ts` run a shell under the very sandbox Claude Code builds from the crew's settings.
11 - `@types/bun` — the type check's types for Bun's own APIs.
12 - `@vscode/ripgrep` — the `rg` that `srt` scans with, pinned and shipped in the package (no install script), so `test/crew-sandbox-escape.test.ts` runs on a check host with no ripgrep of its own.
13 - `axe-core` — the release check's accessibility rules (`src/release/axe.ts`); they need a rendered page, and there is no zero-dependency equivalent.
14 - `happy-dom` — a DOM for tests that run a page's script (`test/changelog.test.ts`) without a browser.
15 - `typescript` — the native 7.x checker behind `bun x tsc --noEmit`.
16 - `typescript-api` — TypeScript 5.x, kept only for the compiler API `scripts/hollow.ts` walks; 7.x has none yet.
docs/PRINCIPLES.md +1 −1
@@ -12,4 +12,4 @@ remove one or absorb it into another, or turn it into code.
1212 - **Git is truth; the database is a derived index.** Enforced by: `src/core/reconcile.ts` rebuilds it on boot.
1313 - **Speed is counted in work, never in a clock.** Enforced by: `test/flake.test.ts`.
1414 - **Work that serves no pillar waits behind the Later gate.** Enforced by: `src/actions/plan.ts` refuses a plan item with neither.
15 - **Less is better: one person can hold the whole forge in their head, so no new dependency, no new rule without a reason.** Enforced by: `test/dependencies-named.test.ts` (each package has its line in the README); nothing yet for rules.
15 - **Less is better: one person can hold the whole forge in their head, so no new dependency, no new rule without a reason.** Enforced by: `test/dependencies-named.test.ts` (each package has its line in docs/DEPENDENCIES.md) and `bun scripts/gate.ts` (`dependencyRefusal`: a pull that adds a package without its line is refused, #12315); nothing yet for rules.
docs/README.md +1
@@ -50,4 +50,5 @@ platform for organizations.
5050 - [Developing the forge](DEVELOP.md): tests, checks, the gate, lint and types.
5151 - [Architecture](ARCHITECTURE.md): the principles and the shape of the system.
5252 - [Writing](WRITING.md): how to write documentation for the forge.
53 - [Dependencies](DEPENDENCIES.md): every package and why it is there; the gate refuses one without its line.
5354 - [Reference](REFERENCE.md): the long reference and the list of every document.
docs/REFERENCE.md +3 −10
@@ -180,16 +180,8 @@ thing forge asks of the host is a `git` binary.
180180
181181 ## Dependencies
182182
183 Every package in `package.json`, with why it is there. A package with no line
184 here fails `test/dependencies-named.test.ts`; so does a line for one that left.
185
186 - `@anthropic-ai/sandbox-runtime` — lets `test/crew-sandbox-real.test.ts` run a shell under the very sandbox Claude Code builds from the crew's settings.
187 - `@types/bun` — the type check's types for Bun's own APIs.
188 - `@vscode/ripgrep` — the `rg` that `srt` scans with, pinned and shipped in the package (no install script), so `test/crew-sandbox-escape.test.ts` runs on a check host with no ripgrep of its own.
189 - `axe-core` — the release check's accessibility rules (`src/release/axe.ts`); they need a rendered page, and there is no zero-dependency equivalent.
190 - `happy-dom` — a DOM for tests that run a page's script (`test/changelog.test.ts`) without a browser.
191 - `typescript` — the native 7.x checker behind `bun x tsc --noEmit`.
192 - `typescript-api` — TypeScript 5.x, kept only for the compiler API `scripts/hollow.ts` walks; 7.x has none yet.
183 Every package in `package.json`, with why it is there, is in
184 [DEPENDENCIES.md](DEPENDENCIES.md).
193185
194186 Forgejo is the reference for what a forge should _feel_ like. This is that
195187 feeling on a foundation sized for one person.
@@ -496,6 +488,7 @@ Once a minute the forge measures its landing rate, names the one cause binding i
496488 - [`REFERENCE.md`](REFERENCE.md): this page.
497489 - [`WRITING.md`](WRITING.md): how to write documentation for the forge.
498490 - [`docs/DEVELOP.md`](DEVELOP.md) — developing the forge: tests, checks, the gate, lint and types.
491 - [`docs/DEPENDENCIES.md`](DEPENDENCIES.md) — every package in `package.json` and why it is there; the gate refuses a new one without its line.
499492 - [`docs/SECURITY-RUNBOOK.md`](SECURITY-RUNBOOK.md) — what to do, with no AI, if the forge is under load, a token leaks or an account looks wrong.
500493 - [`docs/STATUS.md`](STATUS.md) — what works today, one bullet per feature.
501494 - [`docs/ON-YOUR-MACHINE.md`](ON-YOUR-MACHINE.md) — the `forge` command on your machine: the local copy, offline work and building the binary.
scripts/gate.ts +19 −1
@@ -346,10 +346,28 @@ export async function directionRefusal(cwd: string = process.cwd()): Promise<str
346346 return directionProblems(parseNameStatus(diff.out), log.out.split("\0").map((m) => m.trim()).filter(Boolean));
347347 }
348348
349 /**
350 * A pull that adds a package to `package.json` (devDependencies too) says
351 * why in README.md's Dependencies section or docs/DEPENDENCIES.md (#12315):
352 * `dependencyProblems` names each one and the line to add. The base is the
353 * pull's, as `pinRefusal`'s; no base resolving passes, like the rest.
354 */
355 export async function dependencyRefusal(cwd: string = process.cwd()): Promise<string[]> {
356 const { dependencyProblems, DEPENDENCIES_DOC } = await import("../src/kernel/dependencies.ts");
357 const base =
358 process.env.FORGE_CHECK_BASE ||
359 git(["merge-base", "origin/main", "HEAD"], cwd).out ||
360 git(["merge-base", "main", "HEAD"], cwd).out;
361 if (!base) return [];
362 const read = (path: string) => (existsSync(join(cwd, path)) ? readFileSync(join(cwd, path), "utf8") : "");
363 const before = git(["show", `${base}:package.json`], cwd);
364 return dependencyProblems(before.ok ? before.out : undefined, read("package.json"), read("README.md"), read(DEPENDENCIES_DOC));
365 }
366
349367 // `bun scripts/gate.ts`: the audit alone, first in `forge.json`'s check, so
350368 // a stale exclusion fails in under a second — not after install and tsc.
351369 if (import.meta.main) {
352 const refusal = [...gateRefusal(presentTests()), ...slowRefusal(presentTests()), ...(await pinRefusal()), ...(await tryItRefusal()), ...(await bothDoorsRefusal()), ...(await directionRefusal()), ...strayTestRefusal(), ...reviewRefusal(), ...(await addedRefusal()), ...billingDeletesRefusal()];
370 const refusal = [...gateRefusal(presentTests()), ...slowRefusal(presentTests()), ...(await pinRefusal()), ...(await tryItRefusal()), ...(await bothDoorsRefusal()), ...(await directionRefusal()), ...strayTestRefusal(), ...reviewRefusal(), ...(await addedRefusal()), ...(await dependencyRefusal()), ...billingDeletesRefusal()];
353371 for (const line of refusal) console.error(`gate: ${line}`);
354372 process.exit(refusal.length === 0 ? 0 : 2);
355373 }
src/kernel/dependencies.ts added +79
@@ -0,0 +1,79 @@
1 /**
2 * "No dependency without a reason" (#8740, #12315): every package in
3 * `package.json`, devDependencies included, has one `- \`name\` — why` line
4 * in a `## Dependencies` section of README.md or anywhere in
5 * docs/DEPENDENCIES.md. `test/dependencies-named.test.ts` holds the whole
6 * tree to it; `bun scripts/gate.ts` (`dependencyRefusal`) refuses a pull
7 * that adds a package without its line, naming the line to add.
8 * Plain text in, plain lines out.
9 */
10
11 /** Where the line goes when one is missing. */
12 export const DEPENDENCIES_DOC = "docs/DEPENDENCIES.md";
13
14 const BULLET = /^- `([^`]+)` — (.+)$/gm;
15
16 function bullets(text: string): Map<string, string> {
17 const out = new Map<string, string>();
18 for (const m of text.matchAll(BULLET)) {
19 const why = m[2]!.trim();
20 if (why) out.set(m[1]!, why);
21 }
22 return out;
23 }
24
25 /** The packages a README's `## Dependencies` section justifies, up to the next `## ` heading. */
26 export function justified(readme: string): Map<string, string> {
27 const start = readme.indexOf("\n## Dependencies\n");
28 if (start < 0) return new Map();
29 const rest = readme.slice(start + 1);
30 const end = rest.indexOf("\n## ", 1);
31 return bullets(end < 0 ? rest : rest.slice(0, end));
32 }
33
34 /** The packages justified by README.md's section and docs/DEPENDENCIES.md's lines together. */
35 export function justifiedAnywhere(readme: string, doc: string): Map<string, string> {
36 return new Map([...justified(readme), ...bullets(doc)]);
37 }
38
39 /**
40 * Every package name a `package.json` text declares, dependencies and devDependencies;
41 * none for text that is not one.
42 */
43 export function packageNames(text: string | undefined): string[] {
44 if (!text) return [];
45 let pkg: unknown;
46 try {
47 pkg = JSON.parse(text);
48 } catch {
49 return [];
50 }
51 if (typeof pkg !== "object" || pkg === null) return [];
52 const names = new Set<string>();
53 for (const field of ["dependencies", "devDependencies"] as const) {
54 const deps = (pkg as Record<string, unknown>)[field];
55 if (typeof deps === "object" && deps !== null) for (const name of Object.keys(deps)) names.add(name);
56 }
57 return [...names].sort();
58 }
59
60 /**
61 * One line per package the head's `package.json` adds over the base's with
62 * no line of why: the package and the exact line to add. None when nothing
63 * is new or every new one is justified.
64 */
65 export function dependencyProblems(
66 basePackage: string | undefined,
67 headPackage: string | undefined,
68 readme: string,
69 doc: string,
70 ): string[] {
71 const before = new Set(packageNames(basePackage));
72 const lines = justifiedAnywhere(readme, doc);
73 return packageNames(headPackage)
74 .filter((name) => !before.has(name) && !lines.has(name))
75 .map(
76 (name) =>
77 `this pull adds the dependency ${name} with no line saying why: add "- \`${name}\` — <why it is needed and nothing already here does it>" to ${DEPENDENCIES_DOC}`,
78 );
79 }
test/dependencies-named.test.ts +6 −17
@@ -1,41 +1,30 @@
11 /**
22 * "Less is better" gets its enforcer (#8740): every package in package.json
3 * has one line of why in README.md's Dependencies section, and the section
4 * names no package that has left. Pure file reads.
3 * has one line of why in docs/DEPENDENCIES.md (or README.md's Dependencies
4 * section), and no line names a package that has left. Pure file reads.
55 */
66 import { expect, test } from "bun:test";
77 import { readFileSync } from "node:fs";
88 import { join } from "node:path";
9 import { justified, justifiedAnywhere } from "../src/kernel/dependencies.ts";
910
1011 const ROOT = join(import.meta.dir, "..");
1112
12 /** The package names a README's `## Dependencies` section justifies: each `- \`name\` — why` bullet. */
13 export function justified(readme: string): Map<string, string> {
14 const start = readme.indexOf("\n## Dependencies\n");
15 if (start < 0) return new Map();
16 const rest = readme.slice(start + 1);
17 const end = rest.indexOf("\n## ", 1);
18 const section = end < 0 ? rest : rest.slice(0, end);
19 const out = new Map<string, string>();
20 for (const m of section.matchAll(/^- `([^`]+)` — (.+)$/gm)) out.set(m[1]!, m[2]!.trim());
21 return out;
22 }
23
2413 const pkg = JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")) as {
2514 dependencies?: Record<string, string>;
2615 devDependencies?: Record<string, string>;
2716 };
2817 const packages = [...Object.keys(pkg.dependencies ?? {}), ...Object.keys(pkg.devDependencies ?? {})];
29 const lines = justified(readFileSync(join(ROOT, "docs/REFERENCE.md"), "utf8"));
18 const lines = justifiedAnywhere(readFileSync(join(ROOT, "README.md"), "utf8"), readFileSync(join(ROOT, "docs/DEPENDENCIES.md"), "utf8"));
3019
3120 test("every package in package.json has its line of why in the README", () => {
3221 const missing = packages.filter((name) => !lines.get(name));
33 expect(missing, `README.md's Dependencies section does not justify: ${missing.join(", ")}`).toEqual([]);
22 expect(missing, `docs/DEPENDENCIES.md does not justify: ${missing.join(", ")}`).toEqual([]);
3423 });
3524
3625 test("the README justifies no package package.json no longer has", () => {
3726 const stale = [...lines.keys()].filter((name) => !packages.includes(name));
38 expect(stale, `README.md's Dependencies section names packages not in package.json: ${stale.join(", ")}`).toEqual([]);
27 expect(stale, `docs/DEPENDENCIES.md names packages not in package.json: ${stale.join(", ")}`).toEqual([]);
3928 });
4029
4130 test("a package with no line is caught, and the section ends at the next heading", () => {
test/gate-dependencies.test.ts added +107
@@ -0,0 +1,107 @@
1 /**
2 * The gate refuses a pull that adds a package with no line of why (#12315):
3 * judged against FORGE_CHECK_BASE in a real repository, devDependencies too.
4 */
5 import { afterEach, expect, test } from "bun:test";
6 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
7 import { tmpdir } from "node:os";
8 import { join } from "node:path";
9 import { dependencyRefusal } from "../scripts/gate.ts";
10 import { dependencyProblems } from "../src/kernel/dependencies.ts";
11
12 const dirs: string[] = [];
13 const saved = process.env.FORGE_CHECK_BASE;
14 afterEach(() => {
15 for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true });
16 if (saved === undefined) delete process.env.FORGE_CHECK_BASE;
17 else process.env.FORGE_CHECK_BASE = saved;
18 });
19
20 function sh(cwd: string, ...args: string[]): string {
21 const r = Bun.spawnSync(["git", "-c", "user.email=t@t", "-c", "user.name=t", "-c", "commit.gpgsign=false", ...args], { cwd });
22 if (r.exitCode !== 0) throw new Error(r.stderr.toString());
23 return r.stdout.toString().trim();
24 }
25
26 const pkg = (deps: Record<string, string>, dev: Record<string, string> = {}) =>
27 JSON.stringify({ name: "x", dependencies: deps, devDependencies: dev }, null, 2);
28 const DOC = "# Dependencies\n\n- `left-pad` — pads.\n";
29
30 /**
31 * A repository whose base has `left-pad`, its line in docs/DEPENDENCIES.md,
32 * and FORGE_CHECK_BASE set to it.
33 */
34 function repo(): string {
35 const dir = mkdtempSync(join(tmpdir(), "forge-deps-"));
36 dirs.push(dir);
37 sh(dir, "init", "-q", "-b", "trunk");
38 mkdirSync(join(dir, "docs"));
39 writeFileSync(join(dir, "package.json"), pkg({ "left-pad": "1.0.0" }));
40 writeFileSync(join(dir, "README.md"), "# R\n");
41 writeFileSync(join(dir, "docs/DEPENDENCIES.md"), DOC);
42 sh(dir, "add", "-A");
43 sh(dir, "commit", "-qm", "base");
44 process.env.FORGE_CHECK_BASE = sh(dir, "rev-parse", "HEAD");
45 return dir;
46 }
47
48 function head(dir: string, files: Record<string, string>): void {
49 for (const [path, text] of Object.entries(files)) writeFileSync(join(dir, path), text);
50 sh(dir, "commit", "-qam", "head", "--allow-empty");
51 }
52
53 test("a pull that changes no dependency passes", async () => {
54 const dir = repo();
55 head(dir, { "README.md": "# R\nmore words\n" });
56 expect(await dependencyRefusal(dir)).toEqual([]);
57 });
58
59 test("a new dependency with its line in docs/DEPENDENCIES.md passes", async () => {
60 const dir = repo();
61 head(dir, {
62 "package.json": pkg({ "left-pad": "1.0.0", "right-pad": "1.0.0" }),
63 "docs/DEPENDENCIES.md": `${DOC}- \`right-pad\` — pads the other side.\n`,
64 });
65 expect(await dependencyRefusal(dir)).toEqual([]);
66 });
67
68 test("a new dependency with its line in README.md's Dependencies section passes", async () => {
69 const dir = repo();
70 head(dir, {
71 "package.json": pkg({ "left-pad": "1.0.0", "right-pad": "1.0.0" }),
72 "README.md": "# R\n\n## Dependencies\n\n- `right-pad` — pads the other side.\n",
73 });
74 expect(await dependencyRefusal(dir)).toEqual([]);
75 });
76
77 test("a new dependency without a line is refused, naming the package and the exact line to add", async () => {
78 const dir = repo();
79 head(dir, { "package.json": pkg({ "left-pad": "1.0.0", "right-pad": "1.0.0" }) });
80 const out = await dependencyRefusal(dir);
81 expect(out).toHaveLength(1);
82 expect(out[0]).toContain("right-pad");
83 expect(out[0]).toContain('add "- `right-pad` — <why');
84 expect(out[0]).toContain("to docs/DEPENDENCIES.md");
85 });
86
87 test("a new devDependency counts, one line each", async () => {
88 const dir = repo();
89 head(dir, { "package.json": pkg({ "left-pad": "1.0.0" }, { "a-tool": "1", "b-tool": "1" }) });
90 const out = await dependencyRefusal(dir);
91 expect(out.map((l) => l.match(/dependency (\S+)/)?.[1])).toEqual(["a-tool", "b-tool"]);
92 });
93
94 test("a package already on the base is not new, even with no line", () => {
95 expect(dependencyProblems(pkg({ old: "1" }), pkg({ old: "2" }), "", "")).toEqual([]);
96 });
97
98 test("a line with no why does not justify", () => {
99 expect(dependencyProblems(pkg({}), pkg({ fresh: "1" }), "", "- `fresh` —\n")).toHaveLength(1);
100 });
101
102 test("the gate passes when no base resolves", async () => {
103 const dir = mkdtempSync(join(tmpdir(), "forge-deps-"));
104 dirs.push(dir);
105 delete process.env.FORGE_CHECK_BASE;
106 expect(await dependencyRefusal(dir)).toEqual([]);
107 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs