From e4f6f322eacb38e0a6681ba9d62e88818076bd1a Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 09:48:41 +0100 Subject: [PATCH 1/6] chore(agents): spec-lint.mjs checks a spec's structure before a checker reads it, and runnable.mjs reads a feature's Depends on --- .agents/scripts/runnable.mjs | 73 ++++ .agents/scripts/spec-body.mjs | 253 ++++++++++++ .agents/scripts/spec-lint.mjs | 709 ++++++++++++++++++++++++++++++++++ src/runnable.test.ts | 94 +++++ src/spec-lint.test.ts | 582 ++++++++++++++++++++++++++++ 5 files changed, 1711 insertions(+) create mode 100644 .agents/scripts/runnable.mjs create mode 100644 .agents/scripts/spec-body.mjs create mode 100644 .agents/scripts/spec-lint.mjs create mode 100644 src/runnable.test.ts create mode 100644 src/spec-lint.test.ts diff --git a/.agents/scripts/runnable.mjs b/.agents/scripts/runnable.mjs new file mode 100644 index 000000000..9da8df0f7 --- /dev/null +++ b/.agents/scripts/runnable.mjs @@ -0,0 +1,73 @@ +#!/usr/bin/env node +// `node .agents/scripts/runnable.mjs [--world ]`: which of a feature's tasks the +// deliver skill may start now (delivery rule 1), so nobody reads the Depends on by hand. It +// prints the Depends on facts of the open sub-issues, each as `B Depends on: ...` or `N +// ...` (as spec-lint.mjs does): a number that does not exist and a cycle block; a closed issue +// is a note; an open issue outside the feature is `depends-outside # (open, feature #)`. +// Then `chain of tasks` (the longest Depends on path, counted in tasks, over every +// sub-issue), one `waiting #: ` line per open task it leaves out, and last `runnable: +// # #` (or `runnable: none`). +// +// A task is runnable when it is open, `task:ready` and unassigned, and every issue under its +// Depends on is closed, or is a sub-issue of this feature with an open PR out of draft that +// closes it. Exit 1 on a cycle or a missing number, 2 on a usage error. `--world ` reads +// the issues from a JSON file (see spec-body.mjs) instead of GitHub. The repo is GH_REPO, else +// the origin remote. +import { dependsOn, graph, issues, printFinding, readWorld } from "./spec-body.mjs"; +import { option } from "./cli-args.mjs"; + +const args = process.argv.slice(2); +const feature = Number(args[0]); +if (!/^\d+$/.test(args[0] ?? "")) { + console.error("usage: runnable.mjs [--world ]"); + process.exit(2); +} +const world = readWorld(option("--world")); + +const root = issues([feature], world).get(feature); +if (root === undefined) { + console.error(`runnable: #${feature} does not exist`); + process.exit(2); +} +const tasks = [...issues(root.subIssues, world).values()]; +const open = tasks.filter((t) => t.state === "open"); +const known = issues( + open.flatMap((t) => dependsOn(t.body)), + world, +); +const result = graph(tasks, known); +for (const f of result.findings) printFinding(f); +if (result.chain !== null) console.log(`chain ${result.chain} of ${tasks.length} tasks`); + +const subs = new Map(tasks.map((t) => [t.number, t])); +const ready = []; +for (const task of open) { + const why = blocker(task); + if (why === null) ready.push(`#${task.number}`); + else console.log(`waiting #${task.number}: ${why}`); +} +console.log(`runnable: ${result.cycle === null && ready.length > 0 ? ready.join(" ") : "none"}`); +process.exit(result.findings.some((f) => f.level === "B") ? 1 : 0); + +function blocker(task) { + if (!task.labels.includes("task:ready")) { + return `labelled ${task.labels.filter((l) => l.startsWith("task:")).join(", ") || "no task label"}`; + } + if (task.assignees.length > 0) return `assigned to ${task.assignees.join(", ")}`; + const waits = dependsOn(task.body).filter((d) => !settled(d)); + return waits.length === 0 ? null : `on ${waits.map(describe).join(", ")}`; +} + +function settled(dep) { + const target = subs.get(dep) ?? known.get(dep); + if (target === undefined) return false; + if (target.state === "closed") return true; + return subs.has(dep) && target.prs.some((p) => !p.draft); +} + +function describe(dep) { + const target = subs.get(dep) ?? known.get(dep); + if (target === undefined) return `#${dep} (does not exist)`; + if (!subs.has(dep)) return `#${dep} (open, outside this feature)`; + return `#${dep} (${target.prs.length === 0 ? "no PR" : "PR in draft"})`; +} diff --git a/.agents/scripts/spec-body.mjs b/.agents/scripts/spec-body.mjs new file mode 100644 index 000000000..fb621aa40 --- /dev/null +++ b/.agents/scripts/spec-body.mjs @@ -0,0 +1,253 @@ +// What `spec-lint.mjs` and `runnable.mjs` share: reading a feature or task body as GitHub holds +// it, splitting it by heading, and walking the Depends on graph of a feature's tasks. Not run on +// its own. +// +// An issue is `{ number, state, labels, assignees, parent, body, subIssues, prs }`: `state` is +// `open` or `closed`, `labels` and `assignees` are names, `parent` a number or null, `subIssues` +// numbers, `prs` the open PRs that close it as `{ number, draft }`. `issues(numbers, world)` +// reads them through one GraphQL query, or from `world` (the `--world ` JSON, `{ "": +// issue }`) without touching GitHub; a number GitHub does not know is absent from the result. +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; + +export const IS_FOOTER = /^[*_]Written by an agent\.[*_]$/; + +const run = (cmd, args) => + execFileSync(cmd, args, { + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); + +export function readWorld(path) { + return path === null ? null : JSON.parse(readFileSync(path, "utf8")); +} + +function nwo() { + return ( + process.env.GH_REPO ?? + run("git", ["remote", "get-url", "origin"]) + .trim() + .replace(/\.git$/, "") + .replace(/.*[/:]([^/:]+\/[^/]+)$/, "$1") + ); +} + +const FIELDS = `number state body + labels(first:20){nodes{name}} assignees(first:5){nodes{login}} parent{number} + subIssues(first:50){nodes{number}} + closedByPullRequestsReferences(first:10, includeClosedPrs:false){nodes{number isDraft state}}`; + +function fromGraphql(node) { + return { + number: node.number, + state: node.state.toLowerCase(), + body: node.body ?? "", + labels: node.labels.nodes.map((l) => l.name), + assignees: node.assignees.nodes.map((a) => a.login), + parent: node.parent?.number ?? null, + subIssues: node.subIssues.nodes.map((s) => s.number), + prs: node.closedByPullRequestsReferences.nodes + .filter((p) => p.state === "OPEN") + .map((p) => ({ number: p.number, draft: p.isDraft })), + }; +} + +function normalise(n, raw) { + return { + number: n, + state: raw.state ?? "open", + body: raw.body ?? "", + labels: raw.labels ?? [], + assignees: raw.assignees ?? [], + parent: raw.parent ?? null, + subIssues: raw.subIssues ?? [], + prs: raw.prs ?? [], + }; +} + +export function issues(numbers, world) { + const wanted = [...new Set(numbers)]; + if (world !== null) return fromWorld(wanted, world); + return wanted.length === 0 ? new Map() : fromGitHub(wanted); +} + +function fromWorld(wanted, world) { + const found = new Map(); + for (const n of wanted) if (world[n] !== undefined) found.set(n, normalise(n, world[n])); + return found; +} + +function graphql(query) { + try { + return run("gh", ["api", "graphql", "-f", `query=${query}`]); + } catch (error) { + // why: GraphQL answers a number it cannot resolve with an error beside the data, and gh then + // exits 1 with the whole reply on stdout; the other aliases are still good. + if (!error.stdout) throw error; + return error.stdout; + } +} + +function fromGitHub(wanted) { + const [owner, repo] = nwo().split("/"); + const aliases = wanted.map((n) => `i${n}: issue(number:${n}){ ${FIELDS} }`).join("\n"); + const reply = JSON.parse( + graphql(`query{ repository(owner:"${owner}",name:"${repo}"){ ${aliases} } }`), + ); + const data = reply.data?.repository ?? {}; + const found = new Map(); + for (const n of wanted) if (data[`i${n}`]) found.set(n, fromGraphql(data[`i${n}`])); + return found; +} + +export function stripComments(text) { + return text.replace(//g, ""); +} + +export function splitBody(body) { + const lines = body.replace(/\r/g, "").split("\n"); + const heads = []; + let fence = false; + lines.forEach((line, at) => { + if (/^\s*(```|~~~)/.test(line)) fence = !fence; + const h = fence ? null : /^(#{2,4})\s+(.*\S)\s*$/.exec(line); + if (h !== null) heads.push({ level: h[1].length, title: h[2], at }); + }); + const sections = heads.map((h, i) => { + const next = heads.slice(i + 1).find((o) => o.level <= h.level); + return { ...h, lines: lines.slice(h.at + 1, next === undefined ? lines.length : next.at) }; + }); + return { lines, preamble: lines.slice(0, heads[0]?.at ?? lines.length), sections }; +} + +export function section(parsed, title, level) { + return parsed.sections.find( + (s) => s.title === title && (level === undefined || s.level === level), + ); +} + +export function prose(lines) { + let fence = false; + return stripComments(lines.join("\n")) + .split("\n") + .filter((line) => { + if (/^\s*(```|~~~)/.test(line)) { + fence = !fence; + return false; + } + return !fence; + }); +} + +export function bullets(lines) { + const out = []; + for (const line of prose(lines)) { + if (/^[-*] /.test(line)) out.push(line.slice(2).trim()); + else if (/^\s+\S/.test(line) && out.length > 0 && !/^\s+[-*] /.test(line)) { + out[out.length - 1] += ` ${line.trim()}`; + } + } + return out; +} + +export function cells(line) { + return line + .trim() + .replace(/^\||\|$/g, "") + .split(/(? c.trim()); +} + +export function tables(lines) { + const out = []; + let current = null; + for (const line of prose(lines)) { + if (!/^\s*\|/.test(line)) { + current = null; + continue; + } + if (current === null) { + current = { header: cells(line), rows: [] }; + out.push(current); + } else if (!/^\s*\|[\s:|-]+\|\s*$/.test(line)) { + current.rows.push(cells(line)); + } + } + return out; +} + +export function dependsOn(body) { + const s = section(splitBody(body), "Depends on", 2); + if (s === undefined) return []; + return bullets(s.lines) + .map((b) => /^#(\d+)\b/.exec(b)) + .filter((m) => m !== null) + .map((m) => Number(m[1])); +} + +const finding = (level, issue, text) => ({ level, issue, section: "Depends on", text }); + +function outsideFinding(task, dep, target, subs) { + if (target === undefined) return finding("B", task, `#${dep} does not exist; fix the line`); + if (subs.has(dep)) return null; + if (target.state === "closed") { + return finding("N", task, `#${dep} is closed, so the line orders nothing; drop it`); + } + const parent = target.parent === null ? "no feature" : `feature #${target.parent}`; + return finding("N", task, `depends-outside #${dep} (open, ${parent})`); +} + +function duplicateFindings(task, deps) { + return deps + .filter((d, i) => deps.indexOf(d) !== i) + .map((d) => finding("N", task, `#${d} is listed twice`)); +} + +function longest(task, edges, memo, path) { + if (path.includes(task)) return { cycle: [...path.slice(path.indexOf(task)), task] }; + if (memo.has(task)) return memo.get(task); + let best = { length: 1 }; + for (const dep of edges.get(task) ?? []) { + const sub = longest(dep, edges, memo, [...path, task]); + if (sub.cycle) return sub; + if (sub.length + 1 > best.length) best = { length: sub.length + 1 }; + } + memo.set(task, best); + return best; +} + +export function graph(tasks, known) { + const subs = new Set(tasks.map((t) => t.number)); + const edges = new Map(); + const findings = []; + for (const task of tasks) { + const deps = dependsOn(task.body); + edges.set( + task.number, + [...new Set(deps)].filter((d) => subs.has(d)), + ); + if (task.state !== "open") continue; + findings.push(...duplicateFindings(task.number, deps)); + for (const dep of new Set(deps)) { + const f = outsideFinding(task.number, dep, known.get(dep), subs); + if (f !== null) findings.push(f); + } + } + const memo = new Map(); + let chain = 0; + for (const task of tasks) { + const result = longest(task.number, edges, memo, []); + if (result.cycle) { + const loop = result.cycle.map((n) => `#${n}`).join(" → "); + findings.push(finding("B", task.number, `a cycle: ${loop}; drop one of its lines`)); + return { findings, chain: null, cycle: result.cycle, edges }; + } + chain = Math.max(chain, result.length); + } + return { findings, chain, cycle: null, edges }; +} + +export function printFinding(f) { + console.log(`${f.level} ${f.issue ?? "-"} ${f.section}: ${f.text}`); +} diff --git a/.agents/scripts/spec-lint.mjs b/.agents/scripts/spec-lint.mjs new file mode 100644 index 000000000..4d036210f --- /dev/null +++ b/.agents/scripts/spec-lint.mjs @@ -0,0 +1,709 @@ +#!/usr/bin/env node +// `node .agents/scripts/spec-lint.mjs [] [--feature] [--body-file ] [--parent-file ] +// [--world ]`: the structural half of the spec checker's categories, run before any checker +// starts and when a task gets `task:ready`. It reads the posted body (a task with its parent, a +// feature with its sub-issues) and prints one line per finding, `B
: ` for a blocking one and `N ...` for a note, where is the issue the line is +// about (`-` for a draft with no number yet). Then `spec-lint: blocking, notes`; exit 1 +// when anything blocks, 2 on a usage error. +// +// A task (task template, delivery rule 7, toolchain "Slow lane" and "Contract shapes"): +// - `Part of #N.` first, equal to the GitHub parent; then `## Scope`, `## In short`, `## +// Technical spec` with `### Facts`, `### Modules touched`, (`### Surface and extension +// points`), `### Contract and event changes`, `### Other code on the same state`, `### +// Failure modes`, `### Rules in play`, `### Tests`, then `## Done when`, `## Out of scope`, +// `## Depends on`, in that order; the last line is the agent footer; +// - no template leftovers: `#NNN`, a row of `...` cells, a `- ...` bullet, a template comment; +// - In short holds a mermaid block; +// - Facts holds `Commit .`, unique F numbers (a note when out of order), and a Source in +// every row that is not docs, memory, "to re-check" or empty (docs alone is a note when the +// Fact names code); every `path:line[-line]` in Facts, Modules touched and Rules in play +// (a backticked `:line` takes the path before it) is a file at that commit with that many +// lines. A path may be a unique suffix (`state/inbox.ts`). `node_modules/...` and +// `v:` are another tool's source and are skipped; any other path this repo +// lacks blocks; +// - Modules touched and Other code on the same state each name their searches; more than 20 +// distinct `src//` paths need a Scope line saying why it cannot split; +// - Contract and event changes is `none`, or says `bumps the protocol by one`; it never names +// a protocol version, and names `EVENTS.md` when it names an event; +// - Failure modes is a `Call | Outcome | What happens | Limit, counted from` table, or none; +// - Tests opens with a line naming an existing test file; more than 10 bullets is a note and +// more than 30 blocks, unless Scope says why it cannot split; with the parent read, its +// Behaviours rows for this task number the Tests bullets; +// - a Done when line about the slow lane, a real device or "on a Mac with" names an +// `e2e/slow-*.test.ts` that exists, or that Modules touched adds. +// A feature (feature template, spec-session step 7): `Request: #N` or `Request: none`, then its +// sections in template order and the footer, no leftovers; How it works holds a mermaid block; +// Words used is not empty; Open questions is empty or `none` once `feature:planned` (or +// closed); per sub-issue, its Behaviours rows equal its Tests bullets, each bullet shares 60% of +// its words with one of its rows, every Task cell names a sub-issue; over the open sub-issues, +// each Depends on number exists, a closed one is a note, an open issue of another feature is a +// `depends-outside` note, a cycle blocks, and `chain of tasks` is printed. +// +// `--feature` reads as a feature (also when its body starts with `Request:`). +// `--body-file` reads the body from a file and `--parent-file` the parent's; either one, or +// `--world ` (the issues as JSON, see spec-body.mjs), reads nothing from GitHub, so a draft +// can be checked before it is posted. Paths resolve in the git repository of the working +// directory, which needs the body's commit. The repo is GH_REPO, else the origin remote. +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { + bullets, + cells, + dependsOn, + graph, + IS_FOOTER, + issues, + printFinding, + prose, + readWorld, + section, + splitBody, + stripComments, + tables, +} from "./spec-body.mjs"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); +const TASK_H2 = ["Scope", "In short", "Technical spec", "Done when", "Out of scope", "Depends on"]; +const TASK_H3 = [ + "Facts", + "Modules touched", + "Surface and extension points", + "Contract and event changes", + "Other code on the same state", + "Failure modes", + "Rules in play", + "Tests", +]; +const OPTIONAL = new Set(["Surface and extension points"]); +const FEATURE_H2 = [ + "Problem", + "Who it is for", + "Outcome", + "How it works for the user", + "Examples", + "What could go wrong", + "Words used", + "Non-goals", + "Completion conditions", + "Open questions", + "Decisions", + "Shared values", + "Behaviours", + "Tasks", +]; +const FAILURE_HEADER = ["Call", "Outcome", "What happens", "Limit, counted from"]; +const CANNOT_SPLIT = + /\b(cannot|can't|can not|does not|doesn't|won't) split\b|\bone task, not\b|\bkept? (it )?whole\b/i; +const HARDWARE = /slow lane|real (simulator|emulator|device|iPhone|phone)|on a Mac with/i; +const SLOW_TEST = /e2e\/slow-[\w-]+\.test\.ts/g; +const EXTENSIONS = + "ts|tsx|js|mjs|cjs|json|jsonc|md|mdx|yml|yaml|sh|swift|kt|kts|java|toml|txt|html|css|plist|py|xml|gradle"; +const PATH_LINE = new RegExp( + `^(v\\d[\\w.-]*:)?([\\w@./-]*[\\w@-]\\.(?:${EXTENSIONS})):(\\d+)(?:-(\\d+))?$`, +); +const PLAIN_PATH_LINE = new RegExp( + `(? { + const at = argv.indexOf(name); + if (at === -1) return false; + argv.splice(at, 1); + return true; +}; +const value = (name) => { + const at = argv.indexOf(name); + if (at === -1) return null; + const v = argv[at + 1]; + if (v === undefined) usage(); + argv.splice(at, 2); + return v; +}; +const asFeature = flag("--feature"); +const bodyFile = value("--body-file"); +const parentFile = value("--parent-file"); +const worldFile = value("--world"); +if (argv.length > 1 || (argv.length === 1 && !/^\d+$/.test(argv[0]))) usage(); +const number = argv.length === 1 ? Number(argv[0]) : null; +if (number === null && bodyFile === null) usage(); +const offline = bodyFile !== null || parentFile !== null || worldFile !== null; +const world = offline ? (readWorld(worldFile) ?? {}) : null; + +const findings = []; +const add = (level, where, text, issue = number) => + findings.push({ level, issue, section: where, text }); + +const run = (cmd, args) => + execFileSync(cmd, args, { + encoding: "utf8", + maxBuffer: 256 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); +const git = (...a) => run("git", a); + +const self = number === null ? null : (issues([number], world).get(number) ?? null); +if (bodyFile === null && self === null) { + console.error(`spec-lint: #${number} does not exist`); + process.exit(2); +} +const body = bodyFile === null ? self.body : readFileSync(bodyFile, "utf8"); +const parsed = splitBody(body); +const isFeature = asFeature || /^\s*Request:/.test(body); + +checkLeftovers(); +checkFooter(); +if (isFeature) lintFeature(); +else lintTask(); + +for (const f of findings.filter((x) => x.level === "B")) printFinding(f); +for (const f of findings.filter((x) => x.level === "N")) printFinding(f); +const blocking = findings.filter((f) => f.level === "B").length; +console.log(`spec-lint: ${blocking} blocking, ${findings.length - blocking} notes`); +process.exit(blocking > 0 ? 1 : 0); + +function usage() { + console.error( + "usage: spec-lint.mjs [] [--feature] [--body-file ] [--parent-file ] [--world ]", + ); + process.exit(2); +} + +function text(s) { + return s === undefined ? "" : prose(s.lines).join("\n").trim(); +} + +function h3Within(name) { + const parent = section(parsed, name, 2); + if (parent === undefined) return []; + const end = parent.at + parent.lines.length; + return parsed.sections.filter((s) => s.level === 3 && s.at > parent.at && s.at <= end); +} + +function checkOrder(found, expected, where, prefix) { + const titles = found.map((s) => s.title); + let last = -1; + for (const name of expected) { + const at = titles.indexOf(name); + if (at === -1) { + if (!OPTIONAL.has(name)) add("B", where, `\`${prefix} ${name}\` is missing; add it`); + } else if (at < last) { + add("B", where, `\`${prefix} ${name}\` is out of template order; move it`); + } else { + last = at; + } + } + for (const t of titles.filter((x) => !expected.includes(x))) { + add("N", where, `\`${prefix} ${t}\` is not a template section`); + } +} + +function templateComments() { + const out = []; + for (const name of ["task.md", "feature.md"]) { + const raw = readFileSync(join(ROOT, "docs", "internal", "templates", name), "utf8"); + for (const m of raw.matchAll(//g)) out.push(squash(m[1]).slice(0, 40)); + } + return out; +} + +function squash(s) { + return s.replace(/\s+/g, " ").trim(); +} + +function checkTemplateComments() { + const known = templateComments(); + for (const m of body.matchAll(//g)) { + const start = squash(m[1]).slice(0, 40); + if (start !== "" && known.includes(start)) { + add("B", "body", `template comment "${start}…" is still there; delete it`); + } + } +} + +function checkLeftovers() { + checkTemplateComments(); + const lines = prose(body.split("\n")); + if (lines.some((l) => /#NNN\b/.test(l))) add("B", "body", "`#NNN` placeholder; name the issue"); + if (lines.some((l) => /^\s*[-*]\s+\.\.\.\s*$/.test(l))) { + add("B", "body", "a `- ...` placeholder bullet; fill it in or delete it"); + } + if (lines.some((l) => /^\s*\|/.test(l) && cells(l).every((c) => c === "..." || c === ""))) { + add("B", "body", "a table row of `...` cells; fill it in or delete it"); + } +} + +function checkFooter() { + const last = body.replace(/\s+$/, "").split("\n").at(-1)?.trim() ?? ""; + if (!IS_FOOTER.test(last)) add("B", "body", "the last line is not `*Written by an agent.*`"); +} + +function lacksMermaid(s) { + return s !== undefined && !s.lines.some((l) => /^\s*```mermaid\s*$/.test(l)); +} + +function lintTask() { + const tech = h3Within("Technical spec"); + checkOrder( + parsed.sections.filter((s) => s.level === 2), + TASK_H2, + "body", + "##", + ); + checkOrder(tech, TASK_H3, "Technical spec", "###"); + checkPartOf(); + if (lacksMermaid(section(parsed, "In short", 2))) { + add("B", "In short", "no ```mermaid block; add the small diagram the template asks for"); + } + const sha = checkCommit(); + checkFacts(); + for (const name of ["Facts", "Modules touched", "Rules in play"]) checkPaths(name, sha); + checkSearches("Modules touched"); + checkSearches("Other code on the same state"); + checkModuleCount(); + checkContract(); + checkFailureModes(); + checkTests(sha); + checkHardware(sha); + checkOwnBehaviours(); +} + +function checkPartOf() { + const first = parsed.preamble.map((l) => l.trim()).find((l) => l !== "") ?? ""; + const m = /^Part of #(\d+)\.?$/.exec(first); + if (m === null) { + add("B", "body", "the first line is not `Part of #.`"); + return; + } + if (self === null) return; + if (self.parent !== Number(m[1])) { + const actual = self.parent === null ? "no parent" : `#${self.parent}`; + add("B", "body", `\`Part of #${m[1]}\` but GitHub's parent is ${actual}; make them agree`); + } +} + +function canSplitReason() { + return CANNOT_SPLIT.test(text(section(parsed, "Scope", 2))); +} + +function checkCommit() { + const m = /^Commit\b[^\n]*?\b([0-9a-f]{7,40})\b/m.exec( + text(section(parsed, "Technical spec", 2)), + ); + if (m === null) { + add( + "B", + "Facts", + "no `Commit .` line, so no path:line can be checked; add the commit read", + ); + return null; + } + try { + return git("rev-parse", "--verify", "--quiet", `${m[1]}^{commit}`).trim(); + } catch { + add("B", "Facts", `commit ${m[1]} is not in this clone; fetch it, or cite one that is`); + return null; + } +} + +function factRows() { + const facts = section(parsed, "Facts", 3); + if (facts === undefined) return []; + return tables(facts.lines) + .filter((t) => t.header[0] === "#") + .flatMap((t) => t.rows) + .filter((r) => /^F\d+$/.test(r[0])); +} + +function checkSource(id, fact, source) { + if (source === "" || source === "...") { + add("B", "Facts", `${id} has no Source; cite the line or command it was read from`); + } else if (/^(docs?|memory|to re-?check|tbd|todo)\.?$/i.test(source)) { + add("B", "Facts", `${id}'s Source is "${source}", which is no source; cite the line read`); + } else if (DOC_PATH.test(source) && !CODE_PATH.test(source) && /`/.test(fact)) { + add("N", "Facts", `${id} names code but cites only docs; cite the code line`); + } +} + +function checkFacts() { + const rows = factRows(); + const seen = new Set(); + let previous = 0; + for (const row of rows) { + const id = row[0]; + const n = Number(id.slice(1)); + if (seen.has(id)) add("B", "Facts", `${id} is used twice; renumber one`); + else if (n < previous) add("N", "Facts", `${id} comes after F${previous}`); + seen.add(id); + previous = Math.max(previous, n); + checkSource(id, row[1] ?? "", row.at(-1) ?? ""); + } +} + +function pathRefs(lines) { + const refs = []; + for (const line of prose(lines)) { + let last = null; + for (const m of line.matchAll(/`([^`]+)`/g)) { + const full = PATH_LINE.exec(m[1]); + const tail = /^:(\d+)(?:-(\d+))?$/.exec(m[1]); + if (full !== null) { + last = full[1] === undefined ? full[2] : null; + refs.push({ + tool: full[1] !== undefined, + path: full[2], + from: +full[3], + to: +(full[4] ?? full[3]), + }); + } else if (tail !== null && last !== null) { + refs.push({ tool: false, path: last, from: +tail[1], to: +(tail[2] ?? tail[1]) }); + } + } + for (const m of line.replace(/`[^`]*`/g, " ").matchAll(PLAIN_PATH_LINE)) { + refs.push({ tool: false, path: m[1], from: +m[2], to: +(m[3] ?? m[2]) }); + } + } + return refs; +} + +function tree(sha) { + if (!trees.has(sha)) { + trees.set(sha, new Set(git("ls-tree", "-r", "--name-only", sha).split("\n").filter(Boolean))); + } + return trees.get(sha); +} + +function resolvePath(sha, path, named = new Set()) { + const files = tree(sha); + const clean = path.replace(/^\.\//, ""); + if (files.has(clean)) return [clean]; + const found = [...files].filter((f) => f.endsWith(`/${clean}`)); + const preferred = found.filter((f) => named.has(f)); + return preferred.length === 1 ? preferred : found; +} + +function namedPaths(lines) { + return new Set( + [ + ...prose(lines) + .join("\n") + .matchAll(/`([\w@./-]+\/[\w@.-]+)(?::[\d-]+)?`/g), + ].map((m) => m[1]), + ); +} + +function lineCount(sha, path) { + const key = `${sha}:${path}`; + if (!counts.has(key)) { + const content = git("cat-file", "-p", key); + counts.set(key, content.split("\n").length - (content.endsWith("\n") ? 1 : 0)); + } + return counts.get(key); +} + +function checkRef(where, sha, ref, named) { + const shown = `${ref.path}:${ref.from}${ref.to === ref.from ? "" : `-${ref.to}`}`; + if (ref.tool || ref.path.startsWith("node_modules/")) return; + const found = resolvePath(sha, ref.path, named); + if (found.length === 0) { + add( + "B", + where, + `\`${shown}\` is no file at ${sha.slice(0, 9)}; fix the path, or cite another tool as \`node_modules//...\` or \`v:\``, + ); + } else if (found.length > 1) { + add("N", where, `\`${shown}\` matches ${found.length} files; give the full path`); + } else if (ref.to < ref.from || ref.to > lineCount(sha, found[0])) { + add( + "B", + where, + `\`${shown}\` is past the end of ${found[0]} (${lineCount(sha, found[0])} lines) at ${sha.slice(0, 9)}; fix the line`, + ); + } +} + +function checkPaths(name, sha) { + const s = section(parsed, name, 3); + if (s === undefined || sha === null) return; + const named = namedPaths(s.lines); + for (const ref of pathRefs(s.lines)) checkRef(name, sha, ref, named); +} + +function checkSearches(name) { + const s = section(parsed, name, 3); + if (s !== undefined && !/\bsearch(es|ed)?\b/i.test(text(s))) { + add("B", name, "no `Searches:` line; name the searches that found the list"); + } +} + +function checkModuleCount() { + const s = section(parsed, "Modules touched", 3); + const modules = new Set([...text(s).matchAll(/\bsrc\/([\w-]+)\//g)].map((m) => m[1])); + if (modules.size > 20 && !canSplitReason()) { + add( + "B", + "Modules touched", + `${modules.size} modules; split the task, or say under Scope why it cannot split`, + ); + } +} + +function eventNames() { + const source = readFileSync(join(ROOT, "src", "bus", "index.ts"), "utf8"); + const map = /export interface EventMap \{([\s\S]*?)\n\}/.exec(source)?.[1] ?? ""; + return new Set([...map.matchAll(/^ {2}"([a-z-]+\.[a-z-]+)":/gm)].map((m) => m[1])); +} + +function checkContract() { + const s = section(parsed, "Contract and event changes", 3); + if (s === undefined) return; + const t = text(s); + if (/\bprotocol (version )?\d+|PROTOCOL_VERSION(_RANGE)?\b[^\n]*\d/i.test(t)) { + add( + "B", + "Contract and event changes", + "names a protocol version; say `this PR bumps the protocol by one`", + ); + } + if (/^(none|no contract|no shape)\b/i.test(t)) return; + if (!/bumps the protocol by one/i.test(t)) { + add( + "B", + "Contract and event changes", + "a shape change without `this PR bumps the protocol by one`; add it, or start with `None`", + ); + } + const events = eventNames(); + const named = [...t.matchAll(/`([a-z-]+\.[a-z-]+)`/g)].some((m) => events.has(m[1])); + if (named && !/EVENTS\.md/.test(t)) { + add("B", "Contract and event changes", "names an event but not its `EVENTS.md` entry; add it"); + } +} + +function checkFailureModes() { + const s = section(parsed, "Failure modes", 3); + if (s === undefined) return; + const ok = tables(s.lines).some((t) => FAILURE_HEADER.every((h, i) => t.header[i] === h)); + if (!ok && !/^none\b/i.test(text(s))) { + add( + "B", + "Failure modes", + `no \`${FAILURE_HEADER.join(" | ")}\` table and not \`none\`; add the table`, + ); + } +} + +function existsAt(sha, path) { + return resolvePath(sha ?? "HEAD", path).length > 0; +} + +function checkSeam(s, sha) { + const first = prose(s.lines).find((l) => l.trim() !== "") ?? ""; + if (first === "" || /^[-*] /.test(first)) { + add("B", "Tests", "no seam line before the bullets; name the seam and a test file to model on"); + return; + } + const named = [...first.matchAll(/`([^`]+\.(?:test|spec)\.[a-z]+)(?::[\d-]+)?`/g)].map( + (m) => m[1], + ); + if (!named.some((p) => existsAt(sha, p))) { + add( + "B", + "Tests", + "the seam line names no test file that exists; name one to model the tests on", + ); + } +} + +function checkTests(sha) { + const s = section(parsed, "Tests", 3); + if (s === undefined) return; + checkSeam(s, sha); + const count = bullets(s.lines).length; + if (canSplitReason()) return; + if (count > 30) + add("B", "Tests", `${count} lines; split the task, or say under Scope why it cannot split`); + else if (count > 10) add("N", "Tests", `${count} lines, over about 10`); +} + +function checkHardware(sha) { + const done = section(parsed, "Done when", 2); + if (done === undefined) return; + const modules = text(section(parsed, "Modules touched", 3)); + for (const line of bullets(done.lines).filter((b) => HARDWARE.test(b))) { + const named = [...line.matchAll(SLOW_TEST)].map((m) => m[0]); + const short = line.length > 60 ? `${line.slice(0, 60)}…` : line; + if (named.length === 0) { + add( + "B", + "Done when", + `"${short}" needs the slow lane but names no \`e2e/slow-*.test.ts\`; name it`, + ); + } else if (!named.every((p) => existsAt(sha, p) || modules.includes(p))) { + add( + "B", + "Done when", + `"${short}" names a slow-lane test that does not exist and Modules touched does not add; fix the name`, + ); + } + } +} + +function checkOwnBehaviours() { + if (number === null) return; + const parentBody = + parentFile !== null + ? readFileSync(parentFile, "utf8") + : self.parent === null + ? null + : (issues([self.parent], world).get(self.parent)?.body ?? null); + if (parentBody === null) return; + const rows = behaviourRows(splitBody(parentBody)); + if (rows === null) return; + checkTaskRows({ number, body }, rows); +} + +function lintFeature() { + const first = parsed.preamble.map((l) => l.trim()).find((l) => l !== "") ?? ""; + if (!/^Request: (#\d+|none)\b/.test(first)) { + add("B", "body", "the first line is not `Request: #` or `Request: none`"); + } + checkOrder( + parsed.sections.filter((s) => s.level === 2), + FEATURE_H2, + "body", + "##", + ); + if (lacksMermaid(section(parsed, "How it works for the user", 2))) { + add("B", "How it works for the user", "no ```mermaid block; add the user's flow"); + } + const wordsUsed = section(parsed, "Words used", 2); + if (wordsUsed !== undefined && text(wordsUsed) === "") { + add("B", "Words used", "empty; list the terms, or write `none`"); + } + checkOpenQuestions(); + if (self === null) return; + const subs = [...issues(self.subIssues, world).values()]; + checkBehaviours(subs); + checkGraph(subs); +} + +function checkOpenQuestions() { + const planned = + self !== null && (self.labels.includes("feature:planned") || self.state === "closed"); + const t = stripComments(text(section(parsed, "Open questions", 2))); + if (planned && t.trim() !== "" && !/^none\.?$/i.test(t.trim())) { + add( + "B", + "Open questions", + "not empty on a planned feature; settle them, or move the feature back to feature:spec", + ); + } +} + +function behaviourRows(p) { + const s = section(p, "Behaviours", 2); + if (s === undefined) return null; + const table = tables(s.lines).find((t) => t.header.some((h) => h.startsWith("Task"))); + if (table === undefined) return null; + const at = table.header.findIndex((h) => h.startsWith("Task")); + return table.rows.map((r) => ({ + tasks: [...(r[at] ?? "").matchAll(/#(\d+)/g)].map((m) => Number(m[1])), + behaviour: words(r[0] ?? ""), + words: words(r.filter((_, i) => i !== at).join(" ")), + cell: r[at] ?? "", + })); +} + +function words(s) { + return new Set( + s + .replace(/([a-z])([A-Z])/g, "$1 $2") + .toLowerCase() + .split(/[^a-z0-9]+/) + .filter((w) => w.length > 2 && !STOP.has(w)) + .map((w) => w.replace(/(ing|ed|es|s)$/, "")), + ); +} + +function shared(a, b) { + return [...a].filter((w) => b.has(w)).length; +} + +// why: a line may spell out the cases its row only names ("object rows of Examples"), so it +// passes when it holds most of the row's Behaviour words, or most of the shorter of the two. +function overlap(line, row) { + const byBehaviour = + row.behaviour.size === 0 ? 0 : shared(row.behaviour, line) / row.behaviour.size; + const shorter = Math.min(line.size, row.words.size); + return Math.max(byBehaviour, shorter === 0 ? 0 : shared(line, row.words) / shorter); +} + +function checkTaskRows(task, rows) { + const tests = section(splitBody(task.body), "Tests", 3); + if (tests === undefined) return; + const own = rows.filter((r) => r.tasks.includes(task.number)); + const lines = bullets(tests.lines); + if (own.length !== lines.length) { + add( + "B", + "Behaviours", + `#${task.number} has ${lines.length} Tests lines and ${own.length} Behaviours rows; one line per row`, + task.number, + ); + } + lines.forEach((line, i) => { + const best = Math.max(0, ...own.map((r) => overlap(words(line), r))); + if (best < 0.6) { + add( + "N", + "Tests", + `line ${i + 1} shares ${Math.round(best * 100)}% of its words with its best Behaviours row; copy the row's words`, + task.number, + ); + } + }); +} + +function checkBehaviours(subs) { + const rows = behaviourRows(parsed); + if (rows === null) { + if (subs.length > 0) add("B", "Behaviours", "no Behaviours table with a Task column"); + return; + } + const numbers = new Set(subs.map((t) => t.number)); + for (const row of rows) { + if (row.tasks.length === 0 || !row.tasks.every((n) => numbers.has(n))) { + add("B", "Behaviours", `a row's Task cell "${row.cell}" names no sub-issue; name one`); + } + } + for (const task of subs) { + if (!rows.some((r) => r.tasks.includes(task.number))) { + add("B", "Behaviours", `#${task.number} has no row; add its behaviours`); + } + checkTaskRows(task, rows); + } +} + +function checkGraph(tasks) { + const deps = tasks.filter((t) => t.state === "open").flatMap((t) => dependsOn(t.body)); + const result = graph(tasks, issues(deps, world)); + findings.push(...result.findings); + if (result.chain !== null && tasks.length > 0) { + add("N", "Depends on", `chain ${result.chain} of ${tasks.length} tasks`); + } +} diff --git a/src/runnable.test.ts b/src/runnable.test.ts new file mode 100644 index 000000000..c133086d1 --- /dev/null +++ b/src/runnable.test.ts @@ -0,0 +1,94 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), ".."); +const SCRIPT = join(ROOT, ".agents", "scripts", "runnable.mjs"); + +const depends = (...numbers: number[]) => + `## Scope\n\nx\n\n## Depends on\n\n${numbers.map((n) => `- #${n}`).join("\n")}\n`; +const ready = (body: string, extra: object = {}) => ({ body, labels: ["task:ready"], ...extra }); + +function runnable(world: Record) { + const file = join(mkdtempSync(join(tmpdir(), "runnable-")), "world.json"); + writeFileSync(file, JSON.stringify(world)); + try { + const stdout = execFileSync(process.execPath, [SCRIPT, "10", "--world", file], { + cwd: ROOT, + encoding: "utf8", + }); + return { status: 0, lines: stdout.split("\n").filter((l) => l !== "") }; + } catch (error) { + const failed = error as { status: number; stdout: string }; + return { status: failed.status, lines: failed.stdout.split("\n").filter((l) => l !== "") }; + } +} + +describe("runnable.mjs", () => { + it("lists the open, ready, unassigned tasks whose dependencies are closed or have a ready PR, and says why each other one waits", () => { + const result = runnable({ + 10: { subIssues: [11, 12, 13, 14, 15, 16, 17] }, + 11: ready(depends(), { state: "closed" }), + 12: ready(depends(11)), + 13: ready(depends(12), { prs: [{ number: 90, draft: false }] }), + 14: ready(depends(13)), + 15: ready(depends(16)), + 16: ready(depends(), { prs: [{ number: 91, draft: true }], assignees: ["agent"] }), + 17: { body: depends(), labels: ["task:draft"] }, + }); + + expect(result).toEqual({ + status: 0, + lines: [ + "chain 4 of 7 tasks", + "waiting #13: on #12 (no PR)", + "waiting #15: on #16 (PR in draft)", + "waiting #16: assigned to agent", + "waiting #17: labelled task:draft", + "runnable: #12 #14", + ], + }); + }); + + it("holds a task on an open issue of another feature, prints it as depends-outside, and notes a closed one", () => { + const result = runnable({ + 10: { subIssues: [11] }, + 11: ready(depends(30, 31)), + 30: { state: "closed" }, + 31: { parent: 40 }, + }); + + expect(result).toEqual({ + status: 0, + lines: [ + "N 11 Depends on: #30 is closed, so the line orders nothing; drop it", + "N 11 Depends on: depends-outside #31 (open, feature #40)", + "chain 1 of 1 tasks", + "waiting #11: on #31 (open, outside this feature)", + "runnable: none", + ], + }); + }); + + it("fails on a Depends on number that does not exist and on a cycle", () => { + const missing = runnable({ 10: { subIssues: [11] }, 11: ready(depends(99)) }); + const cycle = runnable({ + 10: { subIssues: [11, 12] }, + 11: ready(depends(12)), + 12: ready(depends(11)), + }); + + expect(missing.status).toBe(1); + expect(missing.lines).toContain("B 11 Depends on: #99 does not exist; fix the line"); + expect(missing.lines.at(-1)).toBe("runnable: none"); + expect(cycle.status).toBe(1); + expect(cycle.lines).toContain( + "B 11 Depends on: a cycle: #11 → #12 → #11; drop one of its lines", + ); + expect(cycle.lines.at(-1)).toBe("runnable: none"); + }); +}); diff --git a/src/spec-lint.test.ts b/src/spec-lint.test.ts new file mode 100644 index 000000000..db07d724f --- /dev/null +++ b/src/spec-lint.test.ts @@ -0,0 +1,582 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), ".."); +const SCRIPT = join(ROOT, ".agents", "scripts", "spec-lint.mjs"); +const SHA = execFileSync("git", ["rev-parse", "HEAD"], { cwd: ROOT, encoding: "utf8" }).trim(); +const FOOTER = "*Written by an agent.*"; +const FENCE = "```"; + +const TASK = `Part of #50. + +## Scope + +An operator's config file is read. + +## In short + +The daemon reads the file. + +${FENCE}mermaid +flowchart LR + A[config.json] --> B[loadConfig] +${FENCE} + +## Technical spec + +Commit ${SHA.slice(0, 9)}. + +### Facts + +| # | Fact | Source | +| -- | ---- | ------ | +| F1 | \`loadConfig\` reads the file. | \`src/core/config.ts:1-2\`, \`:3\` | +| F2 | The package has a name. | \`package.json:2\`; \`node_modules/zod/package.json:9999\` | + +### Modules touched + +Searches: \`loadConfig\` (2 hits). + +- \`src/core/config.ts\`, from \`core/config.ts:1\`. + +### Contract and event changes + +None. + +### Other code on the same state + +Searches: \`loadConfig\` (2 hits). None. + +### Failure modes + +| Call | Outcome | What happens | Limit, counted from | +| ---- | ------- | ------------ | ------------------- | +| file read | ok | the config is returned | at start | + +### Rules in play + +- Architecture rule 10, "one decision in one place": \`v1.2.3:src/far.ts:7\` is read once. + +### Tests + +Seam: \`loadConfig\`, modelled on \`src/diff-kind.test.ts\`. + +- An operator's config is read: a number or an object accepted, a bad value refused. + +## Done when + +- \`pnpm test:changed\` passes. + +## Out of scope + +## Depends on + +${FOOTER} +`; + +const PARENT = `Request: none + +## Behaviours + +| Behaviour (invoker, action, what it sees) | Seam | Cases | Task | +| --- | --- | --- | --- | +| An operator's config is read: number or object accepted, bad value refused | \`loadConfig\` | \`2\`; \`{}\`; \`0\` refused | #100 | + +${FOOTER} +`; + +function write(name: string, text: string): string { + const file = join(mkdtempSync(join(tmpdir(), "spec-lint-")), name); + writeFileSync(file, text); + return file; +} + +function lint(args: string[]) { + try { + const stdout = execFileSync(process.execPath, [SCRIPT, ...args], { + cwd: ROOT, + encoding: "utf8", + }); + return { status: 0, lines: stdout.split("\n").filter((l) => l !== "") }; + } catch (error) { + const failed = error as { status: number; stdout: string }; + return { status: failed.status, lines: failed.stdout.split("\n").filter((l) => l !== "") }; + } +} + +function lintTask(body: string, parent: string | null = PARENT) { + return lint([ + "100", + "--body-file", + write("task.md", body), + ...(parent === null ? [] : ["--parent-file", write("parent.md", parent)]), + "--world", + write("world.json", JSON.stringify({ 100: { parent: 50 } })), + ]); +} + +const swap = (from: string, to: string) => (body: string) => { + expect(body).toContain(from); + return body.replace(from, to); +}; +const LINE = "- An operator's config is read: a number or an object accepted, a bad value refused."; +const tests = (n: number) => + Array.from({ length: n }, (_, i) => `- An operator's config is read, case ${i}.`).join("\n"); + +describe("spec-lint.mjs on a task", () => { + it("passes a body that meets every structural rule, with no line printed but the count", () => { + expect(lintTask(TASK)).toEqual({ status: 0, lines: ["spec-lint: 0 blocking, 0 notes"] }); + }); + + it.each([ + [ + "a missing section", + swap("## Out of scope\n\n", ""), + "B 100 body: `## Out of scope` is missing; add it", + ], + [ + "sections out of order", + (body: string) => + body + .replace("### Contract and event changes", "### Swap") + .replace("### Other code on the same state", "### Contract and event changes") + .replace("### Swap", "### Other code on the same state"), + "B 100 Technical spec: `### Other code on the same state` is out of template order; move it", + ], + [ + "a `#NNN` placeholder", + swap("## Depends on\n", "## Depends on\n\n- #NNN\n"), + "B 100 body: `#NNN` placeholder; name the issue", + ], + [ + "a `- ...` bullet", + swap("## Out of scope\n", "## Out of scope\n\n- ...\n"), + "B 100 body: a `- ...` placeholder bullet; fill it in or delete it", + ], + [ + "a row of `...` cells", + swap("| file read | ok |", "| ... | ... | ... | ... |\n| file read | ok |"), + "B 100 body: a table row of `...` cells; fill it in or delete it", + ], + [ + "a template comment", + swap( + "## Out of scope\n", + "## Out of scope\n\n\n", + ), + 'B 100 body: template comment "One issue per line. Leave empty if none.…" is still there; delete it', + ], + [ + "no footer", + swap(FOOTER, "Thanks."), + "B 100 body: the last line is not `*Written by an agent.*`", + ], + [ + "no Part of line", + swap("Part of #50.", "Belongs to #50."), + "B 100 body: the first line is not `Part of #.`", + ], + [ + "a Part of that is not the GitHub parent", + swap("Part of #50.", "Part of #51."), + "B 100 body: `Part of #51` but GitHub's parent is #50; make them agree", + ], + [ + "no mermaid in In short", + swap(`${FENCE}mermaid`, `${FENCE}text`), + "B 100 In short: no ```mermaid block; add the small diagram the template asks for", + ], + [ + "no commit line", + swap(`Commit ${SHA.slice(0, 9)}.`, ""), + "B 100 Facts: no `Commit .` line, so no path:line can be checked; add the commit read", + ], + [ + "a commit this clone lacks", + swap(`Commit ${SHA.slice(0, 9)}.`, "Commit 0000000000."), + "B 100 Facts: commit 0000000000 is not in this clone; fetch it, or cite one that is", + ], + [ + "an empty Source", + swap("| `package.json:2`; `node_modules/zod/package.json:9999` |", "| |"), + "B 100 Facts: F2 has no Source; cite the line or command it was read from", + ], + [ + "a Source that is memory", + swap("| `package.json:2`; `node_modules/zod/package.json:9999` |", "| memory |"), + 'B 100 Facts: F2\'s Source is "memory", which is no source; cite the line read', + ], + [ + "a fact about code read from docs only", + swap("`src/core/config.ts:1-2`, `:3` |", "`docs/internal/ARCHITECTURE.md:1` |"), + "N 100 Facts: F1 names code but cites only docs; cite the code line", + ], + [ + "a repeated F number", + swap("| F2 |", "| F1 |"), + "B 100 Facts: F1 is used twice; renumber one", + ], + [ + "a line past the end of its file", + swap("`package.json:2`", "`package.json:99999`"), + `B 100 Facts: \`package.json:99999\` is past the end of package.json (`, + ], + [ + "a continuation past the end", + swap("`:3`", "`:99999`"), + "B 100 Facts: `src/core/config.ts:99999` is past the end of src/core/config.ts (", + ], + [ + "a path this repo lacks, in Modules touched", + swap("from `core/config.ts:1`", "from `dist/src/cli.js:12`"), + `B 100 Modules touched: \`dist/src/cli.js:12\` is no file at ${SHA.slice(0, 9)}; fix the path, or cite another tool as \`node_modules//...\` or \`v:\``, + ], + [ + "an ambiguous file name, in Rules in play", + swap("is read once.", "is read once (`index.ts:1`)."), + "N 100 Rules in play: `index.ts:1` matches ", + ], + [ + "Modules touched with no searches", + swap("Searches: `loadConfig` (2 hits).\n\n- `src/core", "- `src/core"), + "B 100 Modules touched: no `Searches:` line; name the searches that found the list", + ], + [ + "Other code with no searches", + swap("Searches: `loadConfig` (2 hits). None.", "None."), + "B 100 Other code on the same state: no `Searches:` line; name the searches that found the list", + ], + [ + "more than 20 modules", + swap( + "- `src/core/config.ts`, from", + `${Array.from({ length: 21 }, (_, i) => `- \`src/m${i}/x.ts\``).join("\n")}\n- \`src/core/config.ts\`, from`, + ), + "B 100 Modules touched: 22 modules; split the task, or say under Scope why it cannot split", + ], + [ + "a shape change without the bump wording", + swap( + "### Contract and event changes\n\nNone.", + "### Contract and event changes\n\n- `lease.cancel` gains `platform`.", + ), + "B 100 Contract and event changes: a shape change without `this PR bumps the protocol by one`; add it, or start with `None`", + ], + [ + "a protocol version number", + swap( + "### Contract and event changes\n\nNone.", + "### Contract and event changes\n\nThis PR bumps the protocol by one, to protocol 36.", + ), + "B 100 Contract and event changes: names a protocol version; say `this PR bumps the protocol by one`", + ], + [ + "an event without its EVENTS.md entry", + swap( + "### Contract and event changes\n\nNone.", + "### Contract and event changes\n\nThis PR bumps the protocol by one. `lease.rejected` gains a reason.", + ), + "B 100 Contract and event changes: names an event but not its `EVENTS.md` entry; add it", + ], + [ + "Failure modes with no table", + swap( + "| Call | Outcome | What happens | Limit, counted from |", + "| Call | Result | What happens | Limit |", + ), + "B 100 Failure modes: no `Call | Outcome | What happens | Limit, counted from` table and not `none`; add the table", + ], + [ + "Tests with no seam line", + swap("Seam: `loadConfig`, modelled on `src/diff-kind.test.ts`.\n\n", ""), + "B 100 Tests: no seam line before the bullets; name the seam and a test file to model on", + ], + [ + "a seam line naming a test file that does not exist", + swap("`src/diff-kind.test.ts`", "`src/no-such.test.ts`"), + "B 100 Tests: the seam line names no test file that exists; name one to model the tests on", + ], + [ + "a hardware line with no slow-lane test", + swap("- `pnpm test:changed` passes.", "- On a Mac with an iOS runtime: the simulator boots."), + 'B 100 Done when: "On a Mac with an iOS runtime: the simulator boots." needs the slow lane but names no `e2e/slow-*.test.ts`; name it', + ], + [ + "a hardware line naming a test that does not exist", + swap( + "- `pnpm test:changed` passes.", + "- In the slow lane, `e2e/slow-nothing.test.ts` passes.", + ), + 'B 100 Done when: "In the slow lane, `e2e/slow-nothing.test.ts` passes." names a slow-lane test that does not exist and Modules touched does not add; fix the name', + ], + [ + "a Tests count that differs from the parent's Behaviours rows", + swap("a bad value refused.", "a bad value refused.\n- A second line nothing backs."), + "B 100 Behaviours: #100 has 2 Tests lines and 1 Behaviours rows; one line per row", + ], + [ + "a Tests line in other words than its row", + swap( + "An operator's config is read: a number or an object accepted, a bad value refused.", + "Startup succeeds.", + ), + "N 100 Tests: line 1 shares 0% of its words with its best Behaviours row; copy the row's words", + ], + ])("reports %s", (_name, mutate, expected) => { + const result = lintTask(mutate(TASK)); + + expect(result.lines.some((line) => line.startsWith(expected))).toBe(true); + expect(result.status).toBe(expected.startsWith("B") ? 1 : 0); + }); + + it("notes an F number that comes after a higher one", () => { + const body = TASK.replace("| F1 |", "| F3 |"); + + expect(lintTask(body).lines).toContain("N 100 Facts: F2 comes after F3"); + }); + + it("passes Failure modes written as none", () => { + const body = TASK.replace( + /\| Call \| Outcome[\s\S]*?\| at start \|/, + "none: the task crosses no boundary.", + ); + + expect(lintTask(body).status).toBe(0); + }); + + it("notes more than 10 Tests lines and blocks more than 30", () => { + expect(lintTask(TASK.replace(LINE, tests(11)), null)).toEqual({ + status: 0, + lines: ["N 100 Tests: 11 lines, over about 10", "spec-lint: 0 blocking, 1 notes"], + }); + expect(lintTask(TASK.replace(LINE, tests(31)), null)).toEqual({ + status: 1, + lines: [ + "B 100 Tests: 31 lines; split the task, or say under Scope why it cannot split", + "spec-lint: 1 blocking, 0 notes", + ], + }); + }); + + it("passes the size rules when Scope says why the task cannot split, and an existing slow-lane test or one Modules touched adds", () => { + const body = TASK.replace( + "An operator's config file is read.", + "An operator's config file is read. One task, not two: the halves share one decision.", + ) + .replace(LINE, tests(31)) + .replace( + "- `pnpm test:changed` passes.", + "- On a Mac with an iOS runtime: `e2e/slow-agent-device.test.ts` passes.\n- In the slow lane, `e2e/slow-new.test.ts` passes.", + ) + .replace( + "- `src/core/config.ts`, from", + "- `e2e/slow-new.test.ts` (new).\n- `src/core/config.ts`, from", + ); + + expect(lintTask(body, null)).toEqual({ status: 0, lines: ["spec-lint: 0 blocking, 0 notes"] }); + }); + + it("checks a draft with no number yet, without a parent to compare", () => { + const result = lint(["--body-file", write("draft.md", TASK)]); + + expect(result).toEqual({ status: 0, lines: ["spec-lint: 0 blocking, 0 notes"] }); + }); +}); + +const FEATURE = `Request: none + +## Problem + +Agents wait. + +## Who it is for + +An operator. + +## Outcome + +They do not wait. + +## How it works for the user + +${FENCE}mermaid +flowchart LR + A --> B +${FENCE} + +## Examples + +## What could go wrong + +## Words used + +- profile: a file, e.g. \`p.json\`. + +## Non-goals + +## Completion conditions + +- The worker keeps the profile. + +## Open questions + +## Decisions + +- ADR 0001 — one: two. + +## Shared values + +none + +## Behaviours + +| Behaviour (invoker, action, what it sees) | Seam | Cases | Task | +| --- | --- | --- | --- | +| A worker restarts and keeps the profile | \`startDaemon\` | same bytes; new port | #101 | +| A caller cancels a waiting request by platform | dispatcher | given; absent | #102 | + +## Tasks + +- #101 Keep the profile. +- #102 Cancel by platform. + +${FOOTER} +`; + +const task = (line: string, depends: string[] = []) => `### Tests + +Seam line. + +- ${line} + +## Depends on + +${depends.map((d) => `- ${d}`).join("\n")} +`; + +const WORLD = { + 200: { labels: ["feature:planned"], subIssues: [101, 102] }, + 101: { + body: task("A worker restarts and keeps the profile: same bytes, new port."), + labels: ["task:ready"], + }, + 102: { + body: task("A caller cancels a waiting request by platform: given, absent.", ["#101"]), + labels: ["task:ready"], + }, +}; + +function lintFeature(body: string, world: Record = WORLD) { + return lint([ + "200", + "--feature", + "--body-file", + write("feature.md", body), + "--world", + write("world.json", JSON.stringify(world)), + ]); +} + +describe("spec-lint.mjs on a feature", () => { + it("passes a feature whose rows match its tasks, and prints the Depends on chain", () => { + expect(lintFeature(FEATURE)).toEqual({ + status: 0, + lines: ["N 200 Depends on: chain 2 of 2 tasks", "spec-lint: 0 blocking, 1 notes"], + }); + }); + + it.each([ + [ + "no Request line", + FEATURE.replace("Request: none", "From the inbox."), + WORLD, + "B 200 body: the first line is not `Request: #` or `Request: none`", + ], + [ + "open questions on a planned feature", + FEATURE.replace("## Open questions\n", "## Open questions\n\n- Which port?\n"), + WORLD, + "B 200 Open questions: not empty on a planned feature; settle them, or move the feature back to feature:spec", + ], + [ + "empty Words used", + FEATURE.replace("- profile: a file, e.g. `p.json`.", ""), + WORLD, + "B 200 Words used: empty; list the terms, or write `none`", + ], + [ + "no mermaid in How it works", + FEATURE.replace(`${FENCE}mermaid`, FENCE), + WORLD, + "B 200 How it works for the user: no ```mermaid block; add the user's flow", + ], + [ + "a task with more Tests lines than rows", + FEATURE, + { + ...WORLD, + 101: { + body: task("A worker restarts and keeps the profile: same bytes.\n- A second line."), + }, + }, + "B 101 Behaviours: #101 has 2 Tests lines and 1 Behaviours rows; one line per row", + ], + [ + "a row naming no sub-issue", + FEATURE.replace("| given; absent | #102 |", "| given; absent | #103 |"), + WORLD, + 'B 200 Behaviours: a row\'s Task cell "#103" names no sub-issue; name one', + ], + [ + "a sub-issue with no row", + FEATURE.replace("| given; absent | #102 |", "| given; absent | #101 |"), + WORLD, + "B 200 Behaviours: #102 has no row; add its behaviours", + ], + [ + "a Depends on number that does not exist", + FEATURE, + { + ...WORLD, + 102: { body: task("A caller cancels a waiting request by platform.", ["#999"]) }, + }, + "B 102 Depends on: #999 does not exist; fix the line", + ], + [ + "a cycle", + FEATURE, + { ...WORLD, 101: { body: task("A worker restarts and keeps the profile.", ["#102"]) } }, + "B 101 Depends on: a cycle: #101 → #102 → #101; drop one of its lines", + ], + [ + "a closed dependency", + FEATURE, + { + ...WORLD, + 102: { body: task("A caller cancels a waiting request by platform.", ["#300"]) }, + 300: { state: "closed" }, + }, + "N 102 Depends on: #300 is closed, so the line orders nothing; drop it", + ], + [ + "an open dependency of another feature", + FEATURE, + { + ...WORLD, + 102: { body: task("A caller cancels a waiting request by platform.", ["#301"]) }, + 301: { parent: 400 }, + }, + "N 102 Depends on: depends-outside #301 (open, feature #400)", + ], + ])("reports %s", (_name, body, world, expected) => { + const result = lintFeature(body, world); + + expect(result.lines).toContain(expected); + expect(result.status).toBe(expected.startsWith("B") ? 1 : 0); + }); +}); From 876f8871e09f25e05c611ea7f225d7a3a4f03aaf Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 09:50:17 +0100 Subject: [PATCH 2/6] chore(agents): the spec session lints each body before a checker, deliver reads runnable.mjs, and task:ready demotes a body the lint fails --- .claude/agents/spec-checker.md | 9 +++++ .claude/skills/deliver/SKILL.md | 7 ++-- .claude/skills/spec-session/SKILL.md | 10 ++++++ .github/workflows/issue-state.yml | 51 +++++++++++++++++++++++++++ docs/internal/agent-rules/delivery.md | 6 +++- 5 files changed, 80 insertions(+), 3 deletions(-) diff --git a/.claude/agents/spec-checker.md b/.claude/agents/spec-checker.md index f4cbd2c6a..2763df1aa 100644 --- a/.claude/agents/spec-checker.md +++ b/.claude/agents/spec-checker.md @@ -62,6 +62,15 @@ spec must agree with: Budget: about 35 file reads or searches. A question you cannot settle within it is a finding, not a reason to read more. +`.agents/scripts/spec-lint.mjs` has passed the body before you start, so +spend the budget on content: do not re-check the structural parts it +covers (`size`; the Tests and Behaviours counts of `untested`; the Failure +modes table of `failure-mode`; the `Searches:` lines of `interaction` and +`derived-list`; the Depends on numbers, cycle, depends-outside and chain of +`overlap`; the slow-lane test name of `hardware`; Words used of +`undefined`; and of `fact`, the commit line, the Source cells and that each +`path:line` exists). + ## 2. Plan it, then break it (`task` scope) Before the categories, two passes. Write both in your scratchpad; neither diff --git a/.claude/skills/deliver/SKILL.md b/.claude/skills/deliver/SKILL.md index a54d8ed70..655f5e197 100644 --- a/.claude/skills/deliver/SKILL.md +++ b/.claude/skills/deliver/SKILL.md @@ -316,7 +316,10 @@ each task's PR in the order its review passed. Runnable: open, `task:ready`, unassigned, and every issue under its Depends on closed or a task of this feature whose PR passed review (open, not a -draft). Check the Depends on yourself; no workflow promotes tasks. +draft). Read it from `node .agents/scripts/runnable.mjs `: its last line +lists the runnable tasks; no workflow promotes tasks. Each `depends-outside +#` line on an open issue is a `Waiting:` line of the run summary (step 4), +and a `B` line (a missing number, a cycle) parks the feature. - **Before the first build**: the feature's ADR branches fork from an old `main`, so run `.agents/scripts/stack-sync.sh ` to merge @@ -343,7 +346,7 @@ origin/`, then a fix check on that PR, then the script again. task to move the other. - Both at the hardware check: the second `hardware-verifier` waits for the lane's lock (rule 16). -- After each `gh pr ready`, list again. +- After each `gh pr ready`, run `runnable.mjs` again. - A task parks: keep delivering the tasks that do not depend on it. The stack stays open and nothing merges; the maintainer decides. diff --git a/.claude/skills/spec-session/SKILL.md b/.claude/skills/spec-session/SKILL.md index d24f13524..d4bb765ef 100644 --- a/.claude/skills/spec-session/SKILL.md +++ b/.claude/skills/spec-session/SKILL.md @@ -267,6 +267,9 @@ the body: - Every sentence in any section that says what code, a tool or a browser does today names its Facts row; every list given as complete names the search that found it. +- `node .agents/scripts/spec-lint.mjs --body-file ` (add `--feature` + for the feature) on each drafted body. Fix every `B` line before + posting; its header lists what it checks. **Post.** Do not ask first: a `task:draft` body is safe to post, and the maintainer approves everything in step 9. Edit the feature in place, @@ -291,6 +294,12 @@ task. ## 8. Check the spec +**Lint before any checker.** Run `node .agents/scripts/spec-lint.mjs ` +on each posted body (`--feature` for the feature, which also lints its +Behaviours rows against each task's Tests lines and its Depends on graph) +and fix every `B` line before a checker starts; a checker never spends a +round on what the lint finds. + **Round 0, the feature first.** Post the feature body (Decisions, Shared values, Behaviours, Tasks as a list) before any task body, and start one `spec-checker` with `# feature`. Its `undefined`, `boundary`, @@ -341,6 +350,7 @@ ADR (PR #): | # | | # or — | Check: ready | open findings, one line each +`, e.g. chain 3 of 4 tasks> ``` The options: approve; approve and accept the open findings listed; or diff --git a/.github/workflows/issue-state.yml b/.github/workflows/issue-state.yml index 03725f666..e3fb3479f 100644 --- a/.github/workflows/issue-state.yml +++ b/.github/workflows/issue-state.yml @@ -5,6 +5,8 @@ name: Issue state # - one : label per issue (adding the next label drops the old); # - a task:ready task whose Technical spec is emptied goes back to task:draft # (the spec session sets task:ready; Depends on is the deliver skill's order); +# - a task that gets task:ready and fails .agents/scripts/spec-lint.mjs goes back to task:draft, +# with a comment listing the blocking lines; # - a reporter's reply moves bug:needs-info back to bug:new, reopening the # issue if the stale job closed it (agents post under a maintainer's # account, so comments carrying the agent footer from rule 12 do not count); @@ -99,6 +101,55 @@ jobs: }); core.info(`#${issue_number}: task:ready -> task:draft`); + task-ready-lint: + name: Demote a task:ready task whose spec fails the lint + if: >- + github.event_name == 'issues' && github.event.action == 'labeled' && + github.event.label.name == 'task:ready' + runs-on: ubuntu-24.04 + permissions: + contents: read + issues: write + pull-requests: read + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + # why: the lint reads each path:line at the commit the body names, which may be any + # commit of main. + fetch-depth: 0 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 22 + - name: Lint the body, and demote it when a line blocks + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + ISSUE: ${{ github.event.issue.number }} + CLAIMED: ${{ join(github.event.issue.assignees.*.login, ',') }} + run: | + status=0 + node .agents/scripts/spec-lint.mjs "$ISSUE" > lint.txt || status=$? + cat lint.txt + # why: exit 2 is a usage error or a missing issue, not a failed spec; it fails the job + # instead of moving the label. + if [ "$status" -ne 1 ]; then exit "$status"; fi + if [ -n "$CLAIMED" ]; then + echo "#$ISSUE fails the spec lint but is claimed by $CLAIMED; leaving it" + exit 0 + fi + { + echo 'Back to `task:draft`: `.agents/scripts/spec-lint.mjs` found blocking lines. Fix them in the body, then set `task:ready` again.' + echo + echo '```' + grep '^B ' lint.txt + echo '```' + } > comment.md + gh issue edit "$ISSUE" --add-label task:draft --remove-label task:ready + gh issue comment "$ISSUE" --body-file comment.md + echo "#$ISSUE: task:ready -> task:draft" + needs-info-reply: name: Reporter replied on bug:needs-info if: >- diff --git a/docs/internal/agent-rules/delivery.md b/docs/internal/agent-rules/delivery.md index ab541548b..9784a987f 100644 --- a/docs/internal/agent-rules/delivery.md +++ b/docs/internal/agent-rules/delivery.md @@ -134,7 +134,11 @@ is open, done means closed as completed. command output it was read from, never a summary. It need not repeat what `always-in-scope.md` lists, and a small gap it leaves is an assumption, not a question (rule 3). A task with an empty technical - section is `task:draft` no matter what anyone says in a comment. Before + section is `task:draft` no matter what anyone says in a comment. The + lint runs before the check: `.agents/scripts/spec-lint.mjs` checks the + body's structure, the spec session fixes its blocking lines before any + checker starts, and the issue-state workflow moves a task whose lint + fails back to `task:draft`. Before a task gets `task:ready`, the spec session has the `check-spec` skill read the body as posted, with a fresh context, and fixes in the body what it reports; the same checker then rechecks only what the fix From 06af67502bfa4ac1072bea58d8e1f98d26668aa1 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 09:56:19 +0100 Subject: [PATCH 3/6] chore(checks): a public surface change needs a website page or a Docs trailer, and the configuration reference names every accepted key Docs: none (configValidators is exported for the reference test; no key, default or message changes) --- .agents/scripts/docs-surface-check.mjs | 139 +++++++++++++++++++++ .github/workflows/ci.yml | 22 ++++ docs/internal/agent-rules/documentation.md | 8 +- lefthook.yml | 4 + src/core/config-reference.test.ts | 88 +++++++++++++ src/core/config.ts | 2 +- src/core/validation.ts | 14 ++- src/docs-surface-check.test.ts | 118 +++++++++++++++++ 8 files changed, 391 insertions(+), 4 deletions(-) create mode 100644 .agents/scripts/docs-surface-check.mjs create mode 100644 src/core/config-reference.test.ts create mode 100644 src/docs-surface-check.test.ts diff --git a/.agents/scripts/docs-surface-check.mjs b/.agents/scripts/docs-surface-check.mjs new file mode 100644 index 000000000..9376a3d93 --- /dev/null +++ b/.agents/scripts/docs-surface-check.mjs @@ -0,0 +1,139 @@ +#!/usr/bin/env node +// `node .agents/scripts/docs-surface-check.mjs [--base ] [--head ]`: the mechanical +// half of documentation rule 9. Since (default HEAD) left (default origin/main), +// a change to the public surface goes with a change to a page under website/src/content/docs/: +// one without the other prints the surfaces it changed and the exit code is 1. The surface is: +// +// cli src/cli/tree.ts, the command tree +// config src/core/config.ts, the config type, defaults and validator +// errors the ERROR_TABLE object in src/contract/errors.ts +// mcp each `registerTool(` name and definition object in src/mcp/server.ts, and the +// tool schemas in src/mcp/contracts.ts +// http each `declareRoute(...)` call in src/http/*.ts +// client src/client/index.ts and src/admin/index.ts, the package's two exports +// events the EventMap interface in src/bus/index.ts, which EventName is the keys of +// +// A part counts as changed when its code differs once comments and whitespace are dropped, so +// a comment or a reflow is no change. A commit in the range with the trailer `Docs: none +// ()` lets a surface change through (a refactor, an internal rename); whether the +// reason holds is the review's question. +import { execFileSync } from "node:child_process"; + +import { option } from "./cli-args.mjs"; + +const TRAILER = /^Docs: none \(.+\)\s*$/m; +const DOCS = "website/src/content/docs/"; + +const git = (...a) => + execFileSync("git", a, { + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); +const head = option("--head") ?? "HEAD"; +const base = git("merge-base", option("--base") ?? "origin/main", head).trim(); + +const whole = (text) => [text]; +const calls = (pattern) => (text) => + [...text.matchAll(pattern)].map((m) => balanced(text, m.index + m[0].length - 1)); +const block = (pattern) => (text) => { + const m = pattern.exec(text); + return m === null ? [] : [balanced(text, m.index + m[0].length - 1)]; +}; +const tools = (text) => + [...text.matchAll(/registerTool\(\s*("[^"]*")/g)].map((m) => { + const open = text.indexOf("{", m.index + m[0].length); + return `${m[1]} ${balanced(text, open)}`; + }); + +const SURFACES = [ + { name: "cli", files: ["src/cli/tree.ts"], parts: whole }, + { name: "config", files: ["src/core/config.ts"], parts: whole }, + { + name: "errors", + files: ["src/contract/errors.ts"], + parts: block(/export const ERROR_TABLE\b[^=]*=\s*\{/), + }, + { name: "mcp", files: ["src/mcp/server.ts"], parts: tools }, + { name: "mcp", files: ["src/mcp/contracts.ts"], parts: whole }, + { name: "http", files: httpFiles(), parts: calls(/declareRoute\(/g) }, + { name: "client", files: ["src/client/index.ts", "src/admin/index.ts"], parts: whole }, + { name: "events", files: ["src/bus/index.ts"], parts: block(/export interface EventMap \{/) }, +]; + +const changed = [...new Set(SURFACES.filter(changes).map((s) => s.name))]; +const documented = git("diff", "--name-only", base, head, "--", DOCS).trim() !== ""; +const waived = TRAILER.test(git("log", "--format=%B", `${base}..${head}`)); +if (changed.length > 0 && !documented && !waived) { + console.log( + `The public surface changed (${changed.join(", ")}), but no page under ${DOCS} did. ` + + `Update the pages that describe it (documentation rule 9), or add the trailer "Docs: none ()" to a commit.`, + ); + process.exit(1); +} + +function httpFiles() { + return [base, head] + .flatMap((rev) => git("ls-tree", "--name-only", `${rev}:src/http/`).split("\n")) + .filter((f) => f.endsWith(".ts") && !f.endsWith(".test.ts")) + .map((f) => `src/http/${f}`) + .filter((f, i, all) => all.indexOf(f) === i); +} + +function show(rev, file) { + try { + return git("show", `${rev}:${file}`); + } catch { + return ""; + } +} + +// why: a string or a comment may hold an unmatched bracket; the scan skips both, so a call +// or a block ends at its own closing bracket. +function balanced(text, open) { + const pair = { "(": ")", "{": "}", "[": "]" }; + const stack = []; + for (let i = open; i < text.length; i++) { + const skip = skipped(text, i); + if (skip > i) { + i = skip - 1; + } else if (pair[text[i]] !== undefined) { + stack.push(pair[text[i]]); + } else if (text[i] === stack.at(-1)) { + stack.pop(); + if (stack.length === 0) return text.slice(open, i + 1); + } + } + return text.slice(open); +} + +function skipped(text, i) { + const two = text.slice(i, i + 2); + if (two === "//") return endOf(text, "\n", i); + if (two === "/*") return endOf(text, "*/", i); + const quote = text[i]; + if (quote !== '"' && quote !== "'" && quote !== "`") return i; + let j = i + 1; + while (j < text.length && text[j] !== quote) j += text[j] === "\\" ? 2 : 1; + return j + 1; +} + +function endOf(text, marker, from) { + const at = text.indexOf(marker, from + 2); + return at === -1 ? text.length : at + marker.length; +} + +function code(text) { + return text + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/(^|[^:"'`\\])\/\/.*$/gm, "$1") + .replace(/\s+/g, ""); +} + +function changes(surface) { + return surface.files.some((file) => { + const before = surface.parts(show(base, file)).map(code); + const after = surface.parts(show(head, file)).map(code); + return before.join("\n") !== after.join("\n"); + }); +} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 28607ae7b..8af586ecb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -281,3 +281,25 @@ jobs: env: BASE_REF: ${{ github.base_ref }} run: node .agents/scripts/protocol-bump-check.mjs --base "origin/$BASE_REF" + + # why: checked on the PR's whole range, so a surface change in one commit and its page in a + # later one pass together. + docs-surface: + name: Docs surface + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + permissions: + contents: read + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + fetch-depth: 0 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 22 + - name: Public surface and website pages change together + env: + BASE_REF: ${{ github.base_ref }} + run: node .agents/scripts/docs-surface-check.mjs --base "origin/$BASE_REF" diff --git a/docs/internal/agent-rules/documentation.md b/docs/internal/agent-rules/documentation.md index efe174347..dc35365f1 100644 --- a/docs/internal/agent-rules/documentation.md +++ b/docs/internal/agent-rules/documentation.md @@ -75,5 +75,9 @@ grounds for rejecting a change even if the prose reads well. prerequisites and install steps. A change that adds, removes or alters any of it updates every website page that describes it — the guide that walks through it and the reference table that lists it — and adds - a page to the sidebar when a new task needs one. Nothing generates or - checks these pages: the author and the reviews keep them true. + a page to the sidebar when a new task needs one. Nothing generates + these pages: the author and the reviews keep them true. The push hook + and CI run `.agents/scripts/docs-surface-check.mjs`, which fails a + change to the surface files its header lists that changes no page under + `website/src/content/docs/`, unless a commit carries the trailer + `Docs: none ()`. diff --git a/lefthook.yml b/lefthook.yml index 70c378357..b754059ef 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -56,6 +56,10 @@ pre-push: priority: 1 run: node .agents/scripts/protocol-bump-check.mjs --base origin/main fail_text: "A contract shape change and a PROTOCOL_VERSION_RANGE bump go together (toolchain.md, Contract shapes)." + docs-surface: + priority: 1 + run: node .agents/scripts/docs-surface-check.mjs --base origin/main + fail_text: "A public surface change and its website pages go together (documentation rule 9)." typecheck-e2e: priority: 2 run: pnpm run typecheck:e2e:types diff --git a/src/core/config-reference.test.ts b/src/core/config-reference.test.ts new file mode 100644 index 000000000..6472d7b9a --- /dev/null +++ b/src/core/config-reference.test.ts @@ -0,0 +1,88 @@ +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; +import { z } from "zod"; + +import { authStrategies } from "../auth/index.js"; +import { optionalModules } from "../module-registry/index.js"; +import { capacityStrategyNames } from "./capacity/index.js"; +import { configValidators, type ModuleConfigEntry } from "./config.js"; +import { type Validator, validatorShape } from "./validation.js"; + +const REFERENCE = join( + dirname(fileURLToPath(import.meta.url)), + "..", + "..", + "website", + "src", + "content", + "docs", + "docs", + "reference", + "configuration.mdx", +); + +const modules: ModuleConfigEntry[] = optionalModules.map(({ config, name }) => ({ + name, + ...config, +})); + +function keyPaths(validator: Validator, path: string): string[] { + const shape = validatorShape(validator); + if (shape === undefined) return [path]; + return Object.entries(shape).flatMap(([key, child]) => keyPaths(child, `${path}.${key}`)); +} + +function moduleKeys(): string[] { + return modules.flatMap(({ name, validator }) => [ + `capabilities.${name}.enabled`, + ...keyPaths(validator, `capabilities.${name}`), + ]); +} + +function authKeys(): string[] { + return authStrategies.flatMap((strategy) => { + const schema = strategy.config?.schema; + if (!(schema instanceof z.ZodObject)) return []; + return Object.keys(schema.shape).map((key) => `http.${strategy.config!.key}.${key}`); + }); +} + +// why: `capabilities` and an auth strategy's block are validated by closures over the module +// and strategy lists, so their keys come from those lists; every other key from the shapes. +function acceptedKeys(): string[] { + const keys = capacityStrategyNames.flatMap((strategy) => + Object.entries(configValidators(strategy, modules, authStrategies)) + .filter(([key]) => key !== "capabilities") + .flatMap(([key, validator]) => keyPaths(validator, key)), + ); + return [...new Set([...keys, ...moduleKeys(), ...authKeys()])].sort(); +} + +// why: a row's first cell names its key, sometimes with a placeholder (`ios.defaultModels.`) +// or as the parent of keys listed in a table of their own (`warmPool.targets`). +function documentedKeys(): string[] { + return readFileSync(REFERENCE, "utf8") + .split("\n") + .filter((line) => line.startsWith("| ")) + .flatMap((line) => [...(line.split("|")[1] ?? "").matchAll(/`([^`]+)`/g)].map((m) => m[1]!)); +} + +// why: the old top-level spelling of `capacity.config.limits` and `.ramBudget`; the page names +// it once and documents each key under `capacity.config`. +const LEGACY = new Set(["limits", "ramBudget"]); + +describe("website reference/configuration.mdx", () => { + it("names every key the config validator accepts in the first cell of a table row", () => { + const documented = documentedKeys(); + const covered = (key: string): boolean => { + const top = key.split(".")[0]!; + if (LEGACY.has(top) && documented.includes(top)) return covered(`capacity.config.${key}`); + return documented.some((d) => d === key || d.startsWith(`${key}.`)); + }; + + expect(acceptedKeys().filter((key) => !covered(key))).toEqual([]); + }); +}); diff --git a/src/core/config.ts b/src/core/config.ts index e6f9b976a..f2486c2e5 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -1063,7 +1063,7 @@ const LEASE_IDENTITIES: readonly LeaseIdentity[] = ["reusable", "fresh"]; * The `capacity.config` validator is the selected strategy's own, so a strategy * owns its options end to end and nothing here has to know their shape. */ -function configValidators( +export function configValidators( strategy: CapacityStrategyName, modules: readonly ModuleConfigEntry[], authStrategies: readonly AuthStrategyDefinition[], diff --git a/src/core/validation.ts b/src/core/validation.ts index e6e88d2e5..6a99853f9 100644 --- a/src/core/validation.ts +++ b/src/core/validation.ts @@ -18,8 +18,18 @@ export function invalidValue(path: string, expected: string): ConfigError { return new ConfigError(`Invalid config value for "${path}": expected ${expected}`); } +// why: a validator is a closure, so the keys it accepts are invisible from outside; the +// configuration reference test walks them from here to hold the website's key list to them. +const SHAPES = new WeakMap>>(); + +export function validatorShape( + validator: Validator, +): Readonly> | undefined { + return SHAPES.get(validator); +} + export function objectValidator(shape: Record): Validator { - return (value, path, warn) => { + const validate: Validator = (value, path, warn) => { const object = requireObject(value, path); const result: Record = {}; @@ -37,6 +47,8 @@ export function objectValidator(shape: Record): Validator { return result; }; + SHAPES.set(validate, shape); + return validate; } export function requireObject(value: unknown, path: string): Record { diff --git a/src/docs-surface-check.test.ts b/src/docs-surface-check.test.ts new file mode 100644 index 000000000..09616abbd --- /dev/null +++ b/src/docs-surface-check.test.ts @@ -0,0 +1,118 @@ +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; + +const SCRIPT = join( + dirname(fileURLToPath(import.meta.url)), + "..", + ".agents", + "scripts", + "docs-surface-check.mjs", +); + +const BASE: Record = { + "src/cli/tree.ts": 'export const tree = { lease: "lease" };\n', + "src/core/config.ts": "export const defaults = { ttl: 1 };\n", + "src/contract/errors.ts": + "export const helper = 1;\nexport const ERROR_TABLE: Table = {\n BAD_REQUEST: { exit: 2 },\n};\n", + "src/mcp/server.ts": + 'server.registerTool(\n "list_devices",\n { title: "List devices" },\n async () => {\n return run();\n },\n);\n', + "src/mcp/contracts.ts": "export const schema = z.object({});\n", + "src/http/route-declarations.ts": + 'const internal = 1;\nexport const healthz = declareRoute({ method: "get", path: "/v1/healthz" });\n', + "src/client/index.ts": "export { connect } from './x.js';\n", + "src/admin/index.ts": "export { admin } from './y.js';\n", + "src/bus/index.ts": + 'export interface EventMap {\n "lease.granted": { readonly id: string };\n}\nexport const bus = 1;\n', + "website/src/content/docs/docs/reference/cli.mdx": "# CLI\n", +}; + +function repo(after: Record, message = "head"): string { + const dir = mkdtempSync(join(tmpdir(), "docs-surface-")); + const git = (...args: string[]) => + execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: dir }); + const write = (files: Record) => { + for (const [path, text] of Object.entries(files)) { + mkdirSync(dirname(join(dir, path)), { recursive: true }); + writeFileSync(join(dir, path), text); + } + }; + git("init", "-q", "-b", "base"); + write(BASE); + git("add", "-A"); + git("commit", "-q", "-m", "base"); + git("switch", "-q", "-c", "work"); + write(after); + git("add", "-A"); + git("commit", "-q", "--allow-empty", "-m", message); + return dir; +} + +function check(dir: string) { + try { + execFileSync("node", [SCRIPT, "--base", "base"], { cwd: dir, encoding: "utf8" }); + return { status: 0, stdout: "" }; + } catch (error) { + const failed = error as { status: number; stdout: string }; + return { status: failed.status, stdout: failed.stdout }; + } +} + +const edit = (file: string, from: string, to: string) => ({ + [file]: BASE[file]!.replace(from, to), +}); + +describe("docs-surface-check.mjs", () => { + it.each([ + ["cli", edit("src/cli/tree.ts", '"lease" }', '"lease", nuke: "nuke" }')], + ["config", edit("src/core/config.ts", "ttl: 1", "ttl: 2")], + ["errors", edit("src/contract/errors.ts", "exit: 2", "exit: 3")], + ["mcp", edit("src/mcp/server.ts", '"List devices"', '"List the devices"')], + ["mcp", edit("src/mcp/contracts.ts", "z.object({})", "z.object({ id: z.string() })")], + [ + "http", + { + "src/http/lease-declarations.ts": + 'export const r = declareRoute({ path: "/v1/leases" });\n', + }, + ], + ["client", edit("src/admin/index.ts", "admin", "admin, nuke")], + ["events", edit("src/bus/index.ts", '"lease.granted"', '"lease.given"')], + ])("fails a %s change with no website page changed, and passes it with one", (name, files) => { + const bare = check(repo(files)); + const documented = check( + repo({ ...files, "website/src/content/docs/docs/reference/cli.mdx": "# CLI\n\nMore.\n" }), + ); + + expect(bare).toEqual({ + status: 1, + stdout: expect.stringContaining( + `The public surface changed (${name}), but no page under website/src/content/docs/ did.`, + ), + }); + expect(documented).toEqual({ status: 0, stdout: "" }); + }); + + it("passes changes outside the surface: comments, a tool's handler, code beside the error table and the routes, and a change waived by a trailer", () => { + const cases = [ + edit("src/cli/tree.ts", "export", "// why: one tree\nexport"), + edit("src/mcp/server.ts", "return run();", "return runAll();"), + edit("src/contract/errors.ts", "helper = 1", "helper = 2"), + edit("src/http/route-declarations.ts", "internal = 1", "internal = 2"), + edit("src/bus/index.ts", "bus = 1", "bus = 2"), + ].map((files) => check(repo(files)).status); + const waived = check( + repo( + edit("src/core/config.ts", "ttl: 1", "ttl: 2"), + "refactor: x\n\nDocs: none (internal rename)", + ), + ); + + expect(cases).toEqual([0, 0, 0, 0, 0]); + expect(waived.status).toBe(0); + }); +}); From 5bd3aa3592c5603e79e65c8b4dddcbfedac6b243 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 09:58:01 +0100 Subject: [PATCH 4/6] chore(agents): flatter helpers in spec-body.mjs and docs-surface-check.mjs --- .agents/scripts/docs-surface-check.mjs | 3 ++- .agents/scripts/spec-body.mjs | 21 +++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/.agents/scripts/docs-surface-check.mjs b/.agents/scripts/docs-surface-check.mjs index 9376a3d93..be725b094 100644 --- a/.agents/scripts/docs-surface-check.mjs +++ b/.agents/scripts/docs-surface-check.mjs @@ -23,6 +23,7 @@ import { option } from "./cli-args.mjs"; const TRAILER = /^Docs: none \(.+\)\s*$/m; const DOCS = "website/src/content/docs/"; +const QUOTES = ["'", '"', "`"]; const git = (...a) => execFileSync("git", a, { @@ -112,7 +113,7 @@ function skipped(text, i) { if (two === "//") return endOf(text, "\n", i); if (two === "/*") return endOf(text, "*/", i); const quote = text[i]; - if (quote !== '"' && quote !== "'" && quote !== "`") return i; + if (!QUOTES.includes(quote)) return i; let j = i + 1; while (j < text.length && text[j] !== quote) j += text[j] === "\\" ? 2 : 1; return j + 1; diff --git a/.agents/scripts/spec-body.mjs b/.agents/scripts/spec-body.mjs index fb621aa40..c893ef0b5 100644 --- a/.agents/scripts/spec-body.mjs +++ b/.agents/scripts/spec-body.mjs @@ -53,17 +53,18 @@ function fromGraphql(node) { }; } +const BLANK = { + state: "open", + body: "", + labels: [], + assignees: [], + parent: null, + subIssues: [], + prs: [], +}; + function normalise(n, raw) { - return { - number: n, - state: raw.state ?? "open", - body: raw.body ?? "", - labels: raw.labels ?? [], - assignees: raw.assignees ?? [], - parent: raw.parent ?? null, - subIssues: raw.subIssues ?? [], - prs: raw.prs ?? [], - }; + return { ...BLANK, ...raw, number: n }; } export function issues(numbers, world) { From 0b417a77b9f6466973f7c76498e46b6e709ffb60 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 10:03:13 +0100 Subject: [PATCH 5/6] chore(agents): pathRefs in spec-lint.mjs splits into one helper per kind of reference --- .agents/scripts/spec-lint.mjs | 52 +++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/.agents/scripts/spec-lint.mjs b/.agents/scripts/spec-lint.mjs index 4d036210f..2c12d7b72 100644 --- a/.agents/scripts/spec-lint.mjs +++ b/.agents/scripts/spec-lint.mjs @@ -354,32 +354,42 @@ function checkFacts() { } } -function pathRefs(lines) { +function ref(path, from, to, tool = false) { + return { tool, path, from: +from, to: +(to ?? from) }; +} + +function backtickedRef(content, last) { + const full = PATH_LINE.exec(content); + if (full !== null) { + const tool = full[1] !== undefined; + return { found: ref(full[2], full[3], full[4], tool), last: tool ? null : full[2] }; + } + const tail = /^:(\d+)(?:-(\d+))?$/.exec(content); + if (tail === null || last === null) return { found: null, last }; + return { found: ref(last, tail[1], tail[2]), last }; +} + +function backtickedRefs(line) { const refs = []; - for (const line of prose(lines)) { - let last = null; - for (const m of line.matchAll(/`([^`]+)`/g)) { - const full = PATH_LINE.exec(m[1]); - const tail = /^:(\d+)(?:-(\d+))?$/.exec(m[1]); - if (full !== null) { - last = full[1] === undefined ? full[2] : null; - refs.push({ - tool: full[1] !== undefined, - path: full[2], - from: +full[3], - to: +(full[4] ?? full[3]), - }); - } else if (tail !== null && last !== null) { - refs.push({ tool: false, path: last, from: +tail[1], to: +(tail[2] ?? tail[1]) }); - } - } - for (const m of line.replace(/`[^`]*`/g, " ").matchAll(PLAIN_PATH_LINE)) { - refs.push({ tool: false, path: m[1], from: +m[2], to: +(m[3] ?? m[2]) }); - } + let last = null; + for (const m of line.matchAll(/`([^`]+)`/g)) { + const next = backtickedRef(m[1], last); + last = next.last; + if (next.found !== null) refs.push(next.found); } return refs; } +function plainRefs(line) { + return [...line.replace(/`[^`]*`/g, " ").matchAll(PLAIN_PATH_LINE)].map((m) => + ref(m[1], m[2], m[3]), + ); +} + +function pathRefs(lines) { + return prose(lines).flatMap((line) => [...backtickedRefs(line), ...plainRefs(line)]); +} + function tree(sha) { if (!trees.has(sha)) { trees.set(sha, new Set(git("ls-tree", "-r", "--name-only", sha).split("\n").filter(Boolean))); From 893e892e89b9895fe19bf6e2e138d406c7776bdd Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 10:21:58 +0100 Subject: [PATCH 6/6] test(core): the configuration reference test says why it reads the validators where they are built --- src/core/config-reference.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/config-reference.test.ts b/src/core/config-reference.test.ts index 6472d7b9a..e0b3019bd 100644 --- a/src/core/config-reference.test.ts +++ b/src/core/config-reference.test.ts @@ -74,6 +74,8 @@ function documentedKeys(): string[] { // it once and documents each key under `capacity.config`. const LEGACY = new Set(["limits", "ramBudget"]); +// why: the keys a validator accepts live in the closures config.ts and validation.ts build, and +// no index exports them; this test reads them where they are made to hold the website to them. describe("website reference/configuration.mdx", () => { it("names every key the config validator accepts in the first cell of a table row", () => { const documented = documentedKeys();