fix(test): the docs guard must ask git, not the disk
CI failed on the guard added in0c3729b, 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) <noreply@anthropic.com>
This commit is contained in:
1 parent
56180f77ed
commit
a73cff13a3
3 files changed
+86
-25
No files matched your search
@@ -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:
|
||||
|
||||
|
||||
+9
-6
@@ -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.
|
||||
|
||||
+67
-12
@@ -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<boolean> {
|
||||
/** Null when git cannot answer — a tarball, no git binary — so the check degrades. */
|
||||
async function trackedFiles(): Promise<Set<string> | 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<boolean> {
|
||||
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<boolean> {
|
||||
}
|
||||
}
|
||||
|
||||
test("every repository path named in the documentation exists", async () => {
|
||||
function isTracked(path: string, tracked: Set<string>): 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", () => {
|
||||
|
||||
Reference in new issue
Block a user