feat(rule-inventory): fail a PR whose deleted skill prose maps to no rule-inventory row (FAFF-1428) by alechill · Pull Request #1182 · shftwst/faff

alechill · 2026-11-05T04:10:10Z

Summary

faff rule-inventory check [--base REF] [--json] [--summary FILE] fails a PR whose deleted skill prose is neither moved nor accounted for. Every deleted block of skill prose must do one of two things:

reappear word for word in other skill prose (moved); or
be quoted by a ## Rule inventory row in the PR's committed spec that says what happened to it:
- kept, naming a covering test that exists;
- changed or dropped, with a reason.

Anything else is uncovered, and the check exits 1.

What it proves. Coverage is mechanical: every deleted block has a row, and every test a row names exists at HEAD. Whether the row's claim is true stays with review and the holdout. The check never claims a rule was implemented.
Exits.
- 0 for pass or skipped;
- 1 for uncovered blocks or row errors, listing every one, not just the first;
- 2 for usage errors, any failing git call, or an unreadable spec or config. A git error is never a pass.
When it skips. It skips with no-base (local only, with no origin/HEAD), no-prose-deleted, or no-spec-committed (with a notice). A PR that touches .faff/anchors/ but commits no spec fails with footprint_without_spec.
CI. A new validate step, "Rule inventory covers deleted skill prose", sits right after the region step. It uses the same env-only base resolution and appends the covered rows to the job summary, dropped and changed first.
Factory region, no Commissaire surface. It reads only whether the diff adds or modifies a path under .faff/anchors/, never an anchor's content.

Mapping and exemption rules

Blocks.
- Prose is split into blocks: paragraphs, headings, table rows, list items, and a fenced region as one block.
- A block with any deleted line is a deleted block.
- The diff runs with --text, --no-renames, --literal-pathspecs and no textconv or external diff, so a binary or -diff attribute can't hide a deletion.
Moved. The normalised deleted text is a substring of a single block of watched skill prose at HEAD.
Quoted.
- A row's code-span anchor covers a block when it is at least 20 characters and a substring of that block's deleted text, or when it equals the block's whole deleted text.
- Rows from every committed spec are pooled.
- A malformed table or an invalid row is a row error, never "no inventory".
Heading and header-row exemption. It applies only to a group that is wholly and purely deleted (nothing added in its place), that holds at least one block with letters, and none of whose text reappears at HEAD.
Mode-only changes. A mode-only change, or deleting an empty file, deletes nothing. Any other content change with no text hunk exits 2.
Covering tests.
- The path must match ^test/[A-Za-z0-9._/-]+\.test\.mjs$.
- It is read through ls-tree -z and must be a blob.
- A named test must appear in the file as written.
Config. tracking.spec_docs_path is normalised to a repo-relative directory; an absolute path, or one that leaves the repo, exits 2.

Depends on FAFF‑1463: config-location guard removed

This branch is rebased onto FAFF‑1463 (#1181). The shared config lookup's main-checkout probe (git -C <root> rev-parse --git-common-dir) now throws MainWorktreeProbeError on any failure inside a repo, so:

rule-inventory passes that error through to the dispatch arm. A failed probe exits 2 with one line: faff rule-inventory: cannot proceed — git could not resolve the main checkout for <root>: <git message>.
The two calls the every-call test had listed as pending the operator's decision are now ordinary assertions. Every failing git call, with and without --base, must exit 2 with empty stdout, and both probe calls must print exactly that line. The sweep collects every mismatch and asserts once.
verifyConfigLocation is removed. This is an operator decision. It re-ran the probe to catch a silently lost main-checkout fallback, which can no longer happen. Mutation evidence on the 45-case suite:

Guard	Shared lookup	Failing tests
removed	FAFF‑1463 intact	0 (45/45 pass)
kept	reverted to the null-on-failure wrapper	2
removed	reverted to the null-on-failure wrapper	3, including the overlay fallback test

Every failure the guard caught is now caught through FAFF‑1463, and a regression there fails here as well as in FAFF‑1463's own tests. Removing either probe pass-through also makes tests fail. One mutation run that broke two different git calls reported both.

Other changes

Prose baseline: faffter-dark-nlspec/SKILL.md goes from 18443 to 18680 (+237) for the new pointer to the inventory format.
Docs: docs/reference/skill-authoring.md#rule-inventory documents the table format, and docs/guide/cli.md gains the command.
Registration: bin/faff, regions.js (factory region, no selftest), lint-cli-coverage.js, tsconfig.json, gates-report.js, the build manifest, and the count pins in test/check-no-boundary-cast.test.mjs and test/gates-ci-source.test.mjs.
Source: rule-inventory.ts with its committed emit, and 45 cases in test/rule-inventory.test.mjs.

Verification

Code-blind holdout: meets-spec on 21/21 running-stack criteria at head 0f7ea2a5. It ran in a network-isolated git 2.54 container and covered both the absent and the present origin/HEAD cases. This is the sixth run; earlier runs were invalidated by head changes.
Independent review: pass after 5 rounds, 22 findings:
- 14 fixed;
- 6 refuted;
- 1 accepted risk (a main-checkout probe failure is fatal by FAFF‑1463's design);
- 1 open observation (a redundant re-fetch in one test), left as is so the head stays fixed.
Fixture and git versions: git 2.48 and later create origin/HEAD on fetch. The fixtures now remove it with update-ref --no-deref -d, so the no-base cases behave the same on any git version. On git 2.54 the previous head passed 43/45 and this head passes 45/45; on 2.39 both pass 45/45.
Gate ladder: all 10 steps pass, including rule-inventory check on this branch and the full unit run. The quality-gates contract exits 0.
Anchor: the final anchor commit was minted by hand, on operator instruction. faff pr open short-circuited on the anchor directory already in the tree from the first open and did not re-anchor the new head. Follow-up: FAFF‑1470. The recovery bundle from the first open stands, because bundle refs are write-once.

Follow-ups

FAFF‑1458: the "no spec" skip must fail once governance.yml is set to on-missing: fail.
FAFF‑1459: a rewording exemption.
FAFF‑1460: gates discover counts a command inside echo.
FAFF‑1461: whether --if-eligible hypotheticals should write capture records.
FAFF‑1470: faff pr open treats an existing anchor directory as current, so a head pushed after the PR opens keeps a stale anchor and body.
FAFF‑1471: git-version drift in test fixtures, including the redundant re-fetch at test/rule-inventory.test.mjs:442.

Acceptance Criteria

🤖 Generated with Claude Code

Closes FAFF-1428

…ill-prose rule has no rule-inventory entry Signed-off-by: Alec Hill <alec@shftwst.dev>

…rule-inventory row (FAFF-1428) faff rule-inventory check compares the base tree with HEAD. Every block of skill prose the diff deletes must either reappear verbatim inside one block of skill prose at HEAD, or be quoted by a code-span anchor in a ## Rule inventory row of a spec the diff adds under the spec docs path. kept rows name a test that exists; changed and dropped rows give a reason. The check applies when the PR adds a spec, fails a PR that changes an anchor without one, and skips a hand-written edit with a notice. It runs in the validate job beside regions check, with the same env-only base resolution, and writes the row-covered table (dropped and changed first) to the step summary. The format is documented in docs/reference/skill-authoring.md and the nlspec producer points at it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

… pins (FAFF-1428) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

…AFF-1428) - Per-file diffs run with --text and --literal-pathspecs; a watched file whose diff reports "Binary files" or has no hunk is exit 2, so a binary or -diff attribute or a NUL byte can no longer turn a deletion into a skip. - A heading or table header is exempt only when every line of its section or table is deleted by pure-deletion hunks, the group holds a lettered block, and none of that content reappears in skill prose at HEAD. - A heading over an empty section is never exempt. - A covering test must be a blob at HEAD (cat-file -t), not a directory. - tracking.spec_docs_path is normalised (./records/specs/ works); an absolute or out-of-repo path is exit 2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

…anges delete nothing (FAFF-1428) - The shared config lookup reads a failed `rev-parse --git-common-dir` as "no main checkout", which in a linked worktree silently dropped the main checkout's config and skipped the check. rule-inventory now resolves the common dir itself when this checkout lacks its own config or overlay, and exits 2 when the shared lookup found a different file. The shared lookup is unchanged. - Without --base, origin/HEAD is probed with for-each-ref, so a failing git call is exit 2 rather than a no-base skip. - A watched file with no text hunk deletes nothing when its blob is unchanged (a mode-only change) or its base was empty; any other content change with no hunk still exits 2. - Tests pin a heading deleted alone over an edited paragraph, and fail every git call the check makes one at a time to show none ends in exit 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

…ookup is exit 2 (FAFF-1428) - Covering-test and blob-identity lookups use `git ls-tree -z` through the failing-call guard: empty output is absence, a failed call is exit 2, and a covering test must be a blob. - The overlay half of the config-location guard is now pinned by a linked-worktree case whose overlay lives only in the main checkout. - merge-base takes refs/remotes/origin/HEAD, the ref the probe checks, so a local branch named origin/HEAD cannot move the base. - The every-call test starts from a passing run, with and without --base, and lists the one call that still exits 0 when failed: the shared overlay probe in a main checkout, pending the operator's decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

…ispatch line (FAFF-1428) FAFF-1463 makes the shared config lookup's `git -C <root> rev-parse --git-common-dir` probe raise MainWorktreeProbeError instead of reading a failure as "no main checkout". The two calls the every-call test listed as pending the operator's decision now exit 2. - rule-inventory passes the probe error through to the dispatch arm, so the one stderr line names the root and git's message. - The every-call test asserts exit 2 and empty stdout for every failing call, and the exact dispatch line for the probe call. - The linked-worktree scenarios:21 test asserts the exact line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

…kes redundant (FAFF-1428) verifyConfigLocation re-ran `git rev-parse --git-common-dir` and refused when the shared lookup had silently lost its main-checkout fallback. Since FAFF-1463, that lookup's probe throws MainWorktreeProbeError on every failure inside a repo (non-zero exit, spawn error, signal, empty stdout), so the lost fallback can no longer happen. Mutation evidence, on the rule-inventory suite (45 cases): - guard removed, FAFF-1463 intact: 45/45 pass, every failure case still exits 2 through the dispatch arm; - guard removed and the shared lookup reverted to the null-on-failure wrapper: 3 cases fail, including the overlay fallback case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

…smatches before asserting (FAFF-1428) Two delta-review minors, test-only: - The overlay fallback test now names what it pins, the FAFF-1463 shared lookup, since verifyConfigLocation is gone. - The every-call sweep collects status, stdout and probe-stderr mismatches across every failing call and asserts once, so one run reports every bad call. Strictness is unchanged: every call exits 2 with empty stdout, and both probe calls print the one dispatch line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

Signed-off-by: Alec Hill <alec@shftwst.dev>

github-actions · 2026-11-05T04:10:47Z

Ready to land. Run this locally in a real terminal:

faff effects declare --run-dir .faff/anchors/run-20261104-043908-graft-FAFF-1428 --issue FAFF-1428 --step merge <<'EOF'
[{"kind":"merge","target":"pr:1182","reversible":true}]
EOF
faff merge-gate --pr 1182 --issue FAFF-1428 --run-dir .faff/anchors/run-20261104-043908-graft-FAFF-1428 --level L2 \
  --execute --merge-args "--squash --delete-branch"

…git version (FAFF-1428) git 2.48 and later create refs/remotes/origin/HEAD on fetch; older git does not. CI runs git 2.56, so the two no-base cases found origin/HEAD already present and failed there while passing on git 2.39. fixture() and linkedWorktree() now delete the symref after their fetch with `update-ref --no-deref -d`. Without --no-deref, git deletes the ref the symref points at, refs/remotes/origin/main. Cases that need origin/HEAD already set it with `remote set-head`. Under git 2.54 (node:24-alpine): without this change 43/45 pass, and the two failures are exactly the CI failures; with it, 45/45. Under git 2.39: 45/45 both ways. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

Signed-off-by: Alec Hill <alec@shftwst.dev>

alechill and others added 11 commits November 4, 2026 21:42

docs(FAFF-1428): add spec for Fail an extraction PR when a deleted sk…

ef0001f

…ill-prose rule has no rule-inventory entry Signed-off-by: Alec Hill <alec@shftwst.dev>

test: count rule-inventory in the tsconfig include and Linux rung-set…

43ed768

… pins (FAFF-1428) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

test(rule-inventory): an unreadable config exits 2 (FAFF-1428)

00faed8

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Alec Hill <alec@shftwst.dev>

chore(FAFF-1428): anchor run evidence

64243be

Signed-off-by: Alec Hill <alec@shftwst.dev>

alechill requested a review from shftwstdev as a code owner November 5, 2026 04:10

alechill and others added 2 commits November 5, 2026 04:24

chore(FAFF-1428): anchor run evidence

809873f

Signed-off-by: Alec Hill <alec@shftwst.dev>

alechill merged commit 3eb1f6d into main Nov 5, 2026
16 checks passed

alechill deleted the FAFF-1428-rule-inventory-check branch November 5, 2026 04:57

alechill mentioned this pull request Nov 5, 2026

chore(main): release faff 0.45.0 #1159

Open

Navigation Menu

Repository navigation

feat(rule-inventory): fail a PR whose deleted skill prose maps to no rule-inventory row (FAFF-1428) - #1182

feat(rule-inventory): fail a PR whose deleted skill prose maps to no rule-inventory row (FAFF-1428)#1182
alechill merged 13 commits into
mainfrom
FAFF-1428-rule-inventory-check

alechill commented Nov 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

alechill commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Mapping and exemption rules

Depends on FAFF‑1463: config-location guard removed

Other changes

Verification

Follow-ups

Acceptance Criteria

Uh oh!

github-actions Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

alechill commented Nov 5, 2026 •

edited

Loading

github-actions Bot commented Nov 5, 2026 •

edited

Loading