From a73cff13a3e1ee003be15b38796a86a69c69fd82 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Wed, 2 Sep 2026 22:06:05 +0200 Subject: [PATCH] fix(test): the docs guard must ask git, not the disk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failed on the guard added in 0c3729b, which passed locally. Both were right: docs/TODO.md sent readers to `.superpowers/sdd/2026-09-02-versioned-allocation-rules/`, a tool-managed ledger whose own .gitignore is `*`. It existed on the machine that wrote the reference and has never been in the repository, so the guard saw a real file locally and nothing at all in a fresh checkout. Two fixes, because there were two faults: - TODO.md now points at the four commit messages that actually record why those surfaces work as they do — de128e3, 1cb91d3, e3023d0, f94e458 — and says plainly that the fuller ledger was never committed. Three of the four brief defects it described are in those messages. - The guard now checks a path is git-TRACKED, not merely present. That is the invariant it was always meant to enforce: a path only the author can follow is exactly the reference a reader cannot. It degrades to a filesystem check when git cannot answer, and `.superpowers/` joins docs/superpowers/specs/ as a declared exception — both are named in the docs precisely because they are not in the repository. A fresh clone is now the standard the check measures against, so it means the same thing on a laptop and in CI. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 17 ++++++---- docs/TODO.md | 15 ++++---- src/docsLinks.test.ts | 79 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 86 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b70aa67..e86876a 100755 --- a/AGENTS.md +++ b/AGENTS.md @@ -104,13 +104,16 @@ ship twice — re-introducing one is a regression, not a style choice. worker's precache list (`src/swPrecache.test.ts`) — which must contain no duplicate, because `cache.addAll` rejects on one, which is why the service worker had never once installed in any environment. -- **Every repository path named in a navigable document must exist** — this - file, `README.md`, and `docs/*.md`. → `src/docsLinks.test.ts`, added when the - docs moved into `docs/`: a restructure that quietly leaves thirty references - pointing at nothing is the failure mode, and nothing else would catch it. It - excludes `docs/specs/` and `docs/plans/` on purpose — those are dated records - of a moment, and a plan naming scaffolding it later deleted is accurate - history, not a broken link. +- **Every repository path named in a navigable document must be git-TRACKED** — + this file, `README.md`, and `docs/*.md`. → `src/docsLinks.test.ts`, added when + the docs moved into `docs/`: a restructure that quietly leaves thirty + references pointing at nothing is the failure mode, and nothing else would + catch it. Tracked rather than merely present, because the first version + checked the disk, passed locally and failed CI — it was pointing readers at a + tool ledger that had never been committed. A path only you can follow is the + reference a reader cannot. It excludes `docs/specs/` and `docs/plans/` on + purpose: those are dated records of a moment, and a plan naming scaffolding it + later deleted is accurate history, not a broken link. Two rules with no test behind them. Only review catches these: diff --git a/docs/TODO.md b/docs/TODO.md index 538f58a..52c7ad9 100755 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -91,12 +91,15 @@ Tasks 3-6 of the versioned-allocation-rules work — `setRuleFrom` / and the month view's "change from here" — were executed by hand rather than dispatched, and no second pair of eyes has been over the diff. -Worth reading first: the task log in -`.superpowers/sdd/2026-09-02-versioned-allocation-rules/` records every deviation -from the original briefs with its reasoning. Four of them fixed real defects in -the brief code — JSX that would not compile, a component that pre-filled the -previous post's rule, a number field that could not be cleared, and missing -validation that let a NaN allocation into the fold. +Worth reading first: the four commit messages, `de128e3`, `1cb91d3`, `e3023d0` +and `f94e458`. They record why each surface works the way it does, including +three of the four places the original briefs were wrong — a number field that +could not be cleared, a component that kept the previous post's rule because it +was not keyed, and missing validation that let a NaN allocation into the fold. + +(The fuller task log lived in a tool-managed ledger under `.superpowers/`, which +ignores its own contents and was never part of the repository. This file used to +send readers there, which worked only on the machine that wrote it.) Both surfaces have since been seen working in a browser. What is missing is review, not verification. diff --git a/src/docsLinks.test.ts b/src/docsLinks.test.ts index 3f239f4..da52733 100755 --- a/src/docsLinks.test.ts +++ b/src/docsLinks.test.ts @@ -1,9 +1,9 @@ import { test, expect } from "bun:test"; -import { Glob } from "bun"; +import { $, Glob } from "bun"; import { dirname, join, normalize } from "node:path"; /** - * Every repository path named in the documentation must exist. + * Every repository path named in the documentation must be in the repository. * * This exists because moving the docs into `docs/` rewrote about thirty * cross-references at once. A reference left pointing at nothing does not fail @@ -11,6 +11,14 @@ import { dirname, join, normalize } from "node:path"; * discovered by the next person who follows it and finds nothing there. The * same silence is why `eventCapture.test.ts` and `cssTokens.test.ts` exist. * + * TRACKED, not merely present. The first version of this test only checked the + * filesystem, passed locally, and failed in CI: TODO.md was sending readers to + * `.superpowers/sdd/...`, a tool-managed ledger that ignores its own contents + * and has never been part of the repository. It existed on the machine that + * wrote the reference and nowhere else, which is precisely the reference a + * reader cannot follow. Asking git rather than the disk is what makes a fresh + * clone the standard, so the check means the same thing everywhere. + * * Two kinds of reference are checked: markdown links, and backticked strings * that look like paths. A candidate passes if it resolves either relative to * the file naming it or from the repository root, because both conventions are @@ -28,12 +36,16 @@ import { dirname, join, normalize } from "node:path"; * import specifiers relative to some source file rather than repository paths. */ -/** Paths named deliberately BECAUSE they do not exist. */ +/** Paths named deliberately BECAUSE they are not in the repository. */ const EXPECTED_ABSENT = new Map([ [ "docs/superpowers/specs/", "AGENTS.md §4 names it as the tooling default to override; it was removed on purpose.", ], + [ + ".superpowers/", + "docs/TODO.md names it to explain that the ledger it held was never committed.", + ], ]); function stripCodeBlocks(markdown: string): string { @@ -67,7 +79,18 @@ function candidatesIn(source: string): string[] { return [...found]; } -async function exists(path: string): Promise { +/** Null when git cannot answer — a tarball, no git binary — so the check degrades. */ +async function trackedFiles(): Promise | null> { + try { + const listing = await $`git ls-files`.quiet().text(); + const files = listing.split("\n").filter(Boolean); + return files.length > 0 ? new Set(files) : null; + } catch { + return null; + } +} + +async function onDisk(path: string): Promise { const clean = path.replace(/\/$/, ""); if (await Bun.file(clean).exists()) return true; // Bun.file().exists() is false for a directory, so probe it as one. @@ -78,7 +101,20 @@ async function exists(path: string): Promise { } } -test("every repository path named in the documentation exists", async () => { +function isTracked(path: string, tracked: Set): boolean { + const clean = path.replace(/\/$/, ""); + if (tracked.has(clean)) return true; + const prefix = `${clean}/`; + for (const file of tracked) if (file.startsWith(prefix)) return true; + return false; +} + +test("every repository path named in the documentation is in the repository", async () => { + const tracked = await trackedFiles(); + const resolves = tracked + ? (path: string) => isTracked(path, tracked) + : (path: string) => onDisk(path); + const files = ["AGENTS.md", "CLAUDE.md", "GEMINI.md", "README.md"]; for await (const file of new Glob("docs/*.md").scan(".")) files.push(file); @@ -89,7 +125,7 @@ test("every repository path named in the documentation exists", async () => { for (const candidate of candidatesIn(source)) { if (EXPECTED_ABSENT.has(candidate)) continue; const fromFile = normalize(join(dirname(file), candidate)); - if ((await exists(fromFile)) || (await exists(candidate))) continue; + if ((await resolves(fromFile)) || (await resolves(candidate))) continue; broken.push(`${file} names ${candidate}`); } } @@ -97,12 +133,31 @@ test("every repository path named in the documentation exists", async () => { expect(broken).toEqual([]); }); -test("the guard resolves both path conventions and rejects a dead one", async () => { - // Root-relative and file-relative both count, so the check cannot be passed - // by accident and cannot be tripped by a convention it does not like. - expect(await exists("docs/specs")).toBe(true); - expect(await exists("src/domain/fold.ts")).toBe(true); - expect(await exists("docs/superpowers/specs")).toBe(false); +test("membership in git's index is the standard, not presence on disk", async () => { + // Asserted against a synthetic index, NOT against whatever happens to be in + // this working copy. The first version of this very test checked that + // `.superpowers/sdd` was on disk — true on the machine that wrote it, false + // in a fresh clone — and so failed in CI for the same reason the guard was + // written: a check that depends on untracked local state proves nothing. + const index = new Set(["docs/specs/a-design.md", "src/domain/fold.ts"]); + expect(isTracked("docs/specs", index)).toBe(true); + expect(isTracked("docs/specs/", index)).toBe(true); + expect(isTracked("src/domain/fold.ts", index)).toBe(true); + expect(isTracked(".superpowers/sdd", index)).toBe(false); + expect(isTracked("docs/spec", index)).toBe(false); // a prefix is not a parent + + const tracked = await trackedFiles(); + if (!tracked) return; // git unavailable: the filesystem fallback is used + expect(isTracked("docs/specs", tracked)).toBe(true); + expect(isTracked("src/domain/fold.ts", tracked)).toBe(true); + expect(isTracked("docs/superpowers/specs", tracked)).toBe(false); + expect(isTracked(".superpowers/sdd", tracked)).toBe(false); +}); + +test("the filesystem fallback answers for paths every checkout has", async () => { + expect(await onDisk("src/domain/fold.ts")).toBe(true); + expect(await onDisk("docs/specs")).toBe(true); + expect(await onDisk("docs/superpowers/specs")).toBe(false); }); test("prose, identifiers and non-repository paths are not mistaken for paths", () => {