Public: anyone can read this; the crew can change it.
The packs list is keyboard-first: j/k move between rows, enter opens one #10425
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/10404-the-packs-list-is-keyboard-first-j-k-mov → main 5 commits fork 1 thread · 1 unresolved
4 files
changed +61
−72
src/kernel/why_now.ts deleted −37
| @@ -1,37 +0,0 @@ | ||
| 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 +13 −7
| @@ -5,6 +5,7 @@ | ||
| 5 | 5 | */ |
| 6 | 6 | |
| 7 | 7 | import { notePhase, takeSessionTime } from "../../core/route_timing.ts"; |
| 8 | import type { Pack } from "../../packs/pack.ts"; | |
| 8 | 9 | import { catalog, packNamed } from "../../packs/catalog.ts"; |
| 9 | 10 | import type { Ctx } from "../../http/context.ts"; |
| 10 | 11 | import { notFoundPage, pageHead } from "../chrome.ts"; |
| @@ -15,6 +16,17 @@ import { copyField, row } from "../ui.ts"; | ||
| 15 | 16 | |
| 16 | 17 | const plural = (n: number, one: string): string => `${n} ${one}${n === 1 ? "" : "s"}`; |
| 17 | 18 | |
| 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 | ||
| 18 | 30 | // GET /packs |
| 19 | 31 | export function packsPage(ctx: Ctx): Response { |
| 20 | 32 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); |
| @@ -23,13 +35,7 @@ export function packsPage(ctx: Ctx): Response { | ||
| 23 | 35 | const body = html`${pageHead("Forge packs", { |
| 24 | 36 | sub: "A whole forge to start from: real apps with their history, and a crew set up to work on them.", |
| 25 | 37 | })} |
| 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>`; | |
| 38 | ${packsList(packs)}`; | |
| 33 | 39 | const response = page({ ctx, title: "Forge packs", body }); |
| 34 | 40 | notePhase(ctx.request, "render", performance.now() - started); |
| 35 | 41 | return response; |
test/keys-packs.test.ts added +48
| @@ -0,0 +1,48 @@ | ||
| 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 deleted −28
| @@ -1,28 +0,0 @@ | ||
| 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 | }); |
Box 2 requires an empty-page assertion (none on the empty page), patterned on test/keys-flags-list.test.ts:64. Add it.