trevato/forge

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

More
Forks1g fMapg aRunsg rEventsFlows

Revert "The packs list is keyboard-first: j/k move between rows, enter opens one" #10515

trevatoowner opened this

Check · Review · Land · Live — It conflicts with its base. Next: builder, it conflicts with its base.

ConversationFiles
Conflicts with main

revert-10425 → main 1 commit

Review

Sign in to review this change.
4 files changed +72 −61
src/kernel/why_now.ts added +37
@@ -0,0 +1,37 @@
1 /**
2 * Retired 2026-11-09: the direction no longer puts the pipeline first —
3 * order is set by the pillars and the captain's ranking — so the gate no
4 * longer asks (`scripts/gate.ts`). A finished, approved pull closed unmerged
5 * on this rule (#10317); nothing reads it now. Kept with its test until the
6 * next sweep of unused modules.
7 *
8 * The direction issue put the pipeline first (#8839), so a new feature
9 * said why it went ahead of it. A feature is a file a pull adds, at any
10 * depth, under an action, a page, an API route, an MCP tool or a terminal
11 * screen; the pull's commits must then say either `Pipeline: #N` (it is
12 * pipeline work) or `Why now: <reason>` (it goes ahead, and why). The check
13 * cannot read the pull description, so the answer lives in a commit
14 * message. It is a nudge, not a proof: it checks a reason was written, not
15 * that it is a good one.
16 */
17
18 import type { Change } from "./pin_edits.ts";
19
20 const FEATURE = /^src\/(actions|web\/pages|http\/api|http\/mcp\/tools|tui)\/.+\.ts$/;
21 const PIPELINE = /^Pipeline:\s*#\d+/im;
22 const WHY_NOW = /^Why now:\s*\S[^\n]{14,}/im;
23
24 /** The files a pull adds that make a new feature. */
25 export function featuresAdded(changes: readonly Change[]): string[] {
26 return changes.filter((c) => c.status.startsWith("A") && FEATURE.test(c.path)).map((c) => c.path);
27 }
28
29 /** One line per problem: none when no feature is added or a commit answers the direction. */
30 export function whyNowProblems(changes: readonly Change[], messages: readonly string[]): string[] {
31 const added = featuresAdded(changes);
32 if (added.length === 0) return [];
33 if (messages.some((m) => PIPELINE.test(m) || WHY_NOW.test(m))) return [];
34 return [
35 `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`,
36 ];
37 }
src/web/pages/packs.ts +7 −13
@@ -5,7 +5,6 @@
55 */
66
77 import { notePhase, takeSessionTime } from "../../core/route_timing.ts";
8 import type { Pack } from "../../packs/pack.ts";
98 import { catalog, packNamed } from "../../packs/catalog.ts";
109 import type { Ctx } from "../../http/context.ts";
1110 import { notFoundPage, pageHead } from "../chrome.ts";
@@ -16,17 +15,6 @@ import { copyField, row } from "../ui.ts";
1615
1716 const plural = (n: number, one: string): string => `${n} ${one}${n === 1 ? "" : "s"}`;
1817
19 /** The list of packs as rows; each row is one j/k stop. */
20 export const packsList = (packs: readonly Pack[]) =>
21 html`<ul class="list">${packs.map((p) =>
22 row({
23 title: html`<a href="/packs/${p.name}">${p.title}</a>`,
24 meta: html`${p.description} ${plural(p.apps.length, "app")}, ${plural(p.crew.length, "crew member")}.`,
25 trail: html`v${p.version}`,
26 nav: true,
27 }),
28 )}</ul>`;
29
3018 // GET /packs
3119 export function packsPage(ctx: Ctx): Response {
3220 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
@@ -35,7 +23,13 @@ export function packsPage(ctx: Ctx): Response {
3523 const body = html`${pageHead("Forge packs", {
3624 sub: "A whole forge to start from: real apps with their history, and a crew set up to work on them.",
3725 })}
38 ${packsList(packs)}`;
26 <ul class="list">${packs.map((p) =>
27 row({
28 title: html`<a href="/packs/${p.name}">${p.title}</a>`,
29 meta: html`${p.description} ${plural(p.apps.length, "app")}, ${plural(p.crew.length, "crew member")}.`,
30 trail: html`v${p.version}`,
31 }),
32 )}</ul>`;
3933 const response = page({ ctx, title: "Forge packs", body });
4034 notePhase(ctx.request, "render", performance.now() - started);
4135 return response;
test/keys-packs.test.ts deleted −48
@@ -1,48 +0,0 @@
1 /** The packs list joins the shell's j/k row traversal (#10404): one data-nav per linked row. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { catalog } from "../src/packs/catalog.ts";
8 import { packsList } from "../src/web/pages/packs.ts";
9 import { serve, type Servers } from "../src/http/server.ts";
10
11 let dataDir: string;
12 let app: App;
13 let servers: Servers;
14
15 beforeAll(async () => {
16 dataDir = await mkdtemp(join(tmpdir(), "forge-keys-packs-"));
17 app = createApp({ dataDir, port: 0 });
18 servers = await serve(app);
19 });
20
21 afterAll(async () => {
22 await servers.stop();
23 app.close();
24 await rm(dataDir, { recursive: true, force: true });
25 });
26
27 const navRows = (text: string) => text.split("<li").filter((li) => /class="row"[^>]*data-nav tabindex="-1"/.test(li));
28
29 test("each pack on /packs is one data-nav row whose first link opens the pack", async () => {
30 const { packs } = catalog();
31 expect(packs.length).toBeGreaterThan(0);
32 const rows = navRows(await (await fetch(`${servers.url}/packs`)).text());
33 expect(rows.length).toBe(packs.length);
34 for (const [i, p] of packs.entries()) {
35 expect(rows[i]!.match(/<a href="([^"]+)"/)![1]).toBe(`/packs/${p.name}`);
36 }
37 });
38
39 test("a pack's own page keeps its rows out of the traversal", async () => {
40 const name = catalog().packs[0]!.name;
41 const text = await (await fetch(`${servers.url}/packs/${name}`)).text();
42 expect(navRows(text).length).toBe(0);
43 });
44
45 test("an empty catalog draws no data-nav row", () => {
46 const text = String(packsList([]));
47 expect(text).not.toMatch(/class="row"[^>]*data-nav tabindex="-1"/);
48 });
test/why-now.test.ts added +28
@@ -0,0 +1,28 @@
1 /** Retired 2026-11-09 with the pipeline-first direction: the gate no longer asks (`scripts/gate.ts`); the pure rule keeps its tests until the module goes. */
2 import { expect, test } from "bun:test";
3 import { featuresAdded, whyNowProblems } from "../src/kernel/why_now.ts";
4
5 const add = (path: string) => ({ status: "A", path });
6
7 test("a new action, page or route is a feature; edits, tests and other files are not", () => {
8 expect(featuresAdded([add("src/actions/x.ts"), add("src/web/pages/y.ts"), add("src/http/api/z.ts")])).toHaveLength(3);
9 expect(featuresAdded([add("src/http/mcp/tools/x.ts"), add("src/tui/app/screen.ts"), add("src/http/api/v1/y.ts")])).toHaveLength(3);
10 expect(featuresAdded([{ status: "M", path: "src/actions/x.ts" }, add("test/x.test.ts"), add("src/core/x.ts")])).toEqual([]);
11 });
12
13 test("a feature with no answer to the direction is refused, naming the rule and the file", () => {
14 const [p] = whyNowProblems([add("src/actions/x.ts")], ["Add x\n\nIt does x."]);
15 expect(p).toContain("say why it goes ahead of the pipeline");
16 expect(p).toContain("src/actions/x.ts");
17 });
18
19 test("a Why now line or a Pipeline line answers it; a thin reason does not", () => {
20 const changes = [add("src/actions/x.ts")];
21 expect(whyNowProblems(changes, ["Add x\n\nWhy now: the owner asked for it in #1234 this week"])).toEqual([]);
22 expect(whyNowProblems(changes, ["Add x\n\nPipeline: #6534"])).toEqual([]);
23 expect(whyNowProblems(changes, ["Add x\n\nWhy now: soon"])).toHaveLength(1);
24 });
25
26 test("a pull that adds no feature is never asked", () => {
27 expect(whyNowProblems([add("test/a.test.ts")], ["Fix a"])).toEqual([]);
28 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs