Skip to content

Codify the review battery: scripts/ci hygiene lane + doctrine - #51

Merged
XVVH merged 1 commit into
mainfrom
tooling/review-battery
Jul 15, 2026
Merged

XVVH merged 1 commit into
mainfrom
tooling/review-battery

Conversation

@XVVH

@XVVH XVVH commented Jul 15, 2026

Copy link
Copy Markdown
Owner

What & why

Codifies the author-side review battery so it runs consistently instead of living in my head. Founding case: PR #50 (the SI-32 candidate), where a self-administered battery — collapsed into one adversarial subagent, no hygiene tier, checklist tasks handed off as prose — let an external review find 5 gaps it should have killed. Same failure G9 fixed for test coverage ("green lanes missed three PR #33 findings"); same remedy: a required artifact plus a mechanical lane, not author memory.

The structural rule the whole thing turns on: enumeration tasks produce row-per-item tables; adversarial tasks produce findings; never bundle them — a missing table row is a visible miss, prose hides omissions.

Deliverables (this is "full codification now")

Tier Deliverable Where Status
1 mechanical scripts/ci hygiene — doc/ADR ref resolution, ordered-list numbering, named-appendix section existence scripts/check-doc-hygiene, wired into run_test ✅ green; two-sided via --self-test
1 PR-context declared-scope-matches-diff, branch-off-main, PR-not-SHA documented author+reviewer steps ✅ documented in docs/review-battery.md
2 artifacts completeness table + code-fact table required in the PR body docs/review-battery.md, like the G9 sweep ✅ doctrine written (and applied below)
3 adversarial internal pre-review (candidates) scoped to soundness/shape, never the exhaustive checker docs/review-battery.md ✅ doctrine written
doctrine binding paragraph next to G9 CLAUDE.md + AGENTS.md ✅

Code-fact / behavior table (dogfooding Tier 2 on this PR)

Claim Verified
hygiene lane is two-sided sh scripts/check-doc-hygiene --self-test — 3 checks fire on broken fixtures, pass on good + a meta-discussion false-positive guard
lane runs in the pre-push gate run_hygiene called from run_test (scripts/ci:51); CI test step already invokes it
check 3 catches finding-7 without false-positives narrowed to a quoted-name appendix ("..." appendix); self-test fixture is the exact finding-7 shape; review-battery.md (which describes the check) passes
roadmap.md exemption is principled it names unmerged-branch artifacts by its own boundary rule; the lane surfaced exactly one such ref (workboard-dogfooding.md) and it is legitimate
no Rust touched git diff --cached main = 5 files, 0 .rs/Cargo; contract registry unchanged (scripts/ci contracts green)
declared scope matches diff git diff --cached main --stat = exactly the 5 files below
branch off current main git merge-base --is-ancestor origin/main HEAD → true

Scope (5 files)

scripts/check-doc-hygiene (new), scripts/ci (hygiene lane), docs/review-battery.md (new doctrine), CLAUDE.md + AGENTS.md (binding paragraph).

Evidence scoping

Tier-1 shell lane; two-sided evidence is the --self-test. Mutation testing does not apply — this is shell enforcement, not Rust (the two-sided contract discipline's explicit escape hatch), stated in the doctrine and the script header. No Rust changed, so the workspace test/clippy lanes are unaffected; scripts/ci contracts and scripts/ci hygiene both green.

Not a spec/authority-surface change — process tooling — so no independent-context ratification review is required, but a second read is welcome.

🤖 Generated with Claude Code

Founding case: the PR #50 SI-32 candidate, where a self-administered
battery collapsed into one subagent let an external review find 5 gaps
it should have killed (a code-fact claim contradicting source, three
filing-named deliverables dropped, a phantom appendix, a wrong-base
branch). Fix, per the repo's own G9 remedy: make the battery a required
artifact plus a mechanical lane, not a thing the author remembers.

- scripts/check-doc-hygiene: dependency-free POSIX shell, three
  repo-state checks (doc/ADR ref resolution; ordered-list numbering;
  named-"..."-appendix section existence). Two-sided via --self-test
  (checks fire on broken fixtures, pass on good + a false-positive
  guard). roadmap.md exempt from file-existence refs (forward-looking).
- scripts/ci: new `hygiene` lane, wired into run_test so it runs in
  contracts/test/required/full/all and in GitHub Actions' `test` step.
- docs/review-battery.md: the doctrine — the three tiers, the structural
  rule (enumeration -> tables, adversary -> findings, never bundled),
  the PR-context checks, and the running order.
- CLAUDE.md / AGENTS.md: binding paragraph next to G9.

Mutation testing N/A (shell enforcement, not Rust — the two-sided
contract discipline's stated escape hatch); the --self-test is the
two-sided evidence. No Rust touched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@XVVH
XVVH merged commit f5ae86b into main Jul 15, 2026
6 checks passed
@XVVH
XVVH deleted the tooling/review-battery branch July 15, 2026 13:37
XVVH pushed a commit that referenced this pull request Jul 15, 2026
Applied via the codified review battery (PR #51): completeness table
from the SI-32 filing deliverables drove the fold, every code-fact
verified at source, Tier-1 hygiene green, Tier-3 internal adversarial
pre-review (2 LOW nits, no new soundness hole, ready for challenge pass).

- F1 T3 active-publication-window edit: T3 is orthogonal not cumulative;
  a human write during a live promotion/revert is overwritten with no
  capture/drift (the item ADR 0007 R14 deferred here) -> decision point,
  protocol-class, not designed
- F2 T1 rollback precise domain: layer 1 detects only rollback
  inconsistent vs a surviving expected terminal; coherent whole-DB
  suffix regression is layer 2's (was imprecise "partial rollback")
- F3 trust-root unsound: keys/KEK/credentials split; substitution is an
  SI-27/RF-14 residual (self-consistent forged home; auth-as-other-
  account), not "degrades to signature failure"; out-of-scope splits
  root (out) from unprivileged different-uid (defended by perms)
- F4 capture_sqlite follows symlinks for registered stores (only
  fabric.db is checked) -> RF-42
- F5 directory/kind semantics carried from A24/R25 + R24, per operation
- F6 G-PUBLISH conditional-publication decision point (the filing's
  explicit ask, previously dropped)
- F7 real inventory appendix with file:line anchors; R3 no-follow recast
  (capture_fs rejects symlinks via a check, not O_NOFOLLOW)
- F8 SHA cite -> PR #48; wrong-base scope fixed by rebase onto main

Decision points now 10; SI-32 stays open, no implementation authorized.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
XVVH added a commit that referenced this pull request Jul 15, 2026
…008 ACCEPTED, spec v0.10) (#50)

* W-20 item (3): SI-32 filesystem-attacker-model candidate (ADR 0008, PROPOSED)

Three-tier attacker model + candidate rules A27.1-A27.4, grounded in a
complete publication-site inventory. Spec untouched; SI-32 stays open;
authorizes no implementation. Queued for operator ratification.

- T1 offline tampering: content-address verification for
  substitution/corruption/truncation; A23 (global_seq + anchor) for
  rollback; keys and the R7 unsigned-index gap are the carve-outs
- T2 same-uid active writer: no pathname check wins — W-4 containment;
  every T2 claim labeled "COOP now; W-4 at G-ADVERSARIAL". Super-user
  out of scope (trust boundary = the Unix account, P7)
- T3 human edit: not an attack; M8 + A24/R14 attribution
- A27.1 staged-bytes (ratifies RF-20); A27.2 verify-on-read-back,
  tier-labeled, three read classes; A27.3 per-kind incl. hardlinks;
  A27.4 containment sentence
- Surfaces one genuine T1 gap (R7: unsigned fabric.db meta store-paths
  trusted while sibling events signed; feeds broker authority via
  substrate_span) -> proposed RF-41, posture-bounded under SU
- One uniformity correction (R3: sync_store fs walk omits .git
  exclusion + no-follow)
- Seven ratification choices enumerated

Passed the four-audit battery + an internal adversarial pre-review
(7 findings, one blocking: the first draft wrongly folded T1-rollback
into content-addressing when A23 owns it -- fixed, promoted to a
decision point). Ledger mirrors: spec-issues SI-32 candidate note,
roadmap W-20 progress.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fold external review round 1 (8 findings) into the SI-32 candidate

Applied via the codified review battery (PR #51): completeness table
from the SI-32 filing deliverables drove the fold, every code-fact
verified at source, Tier-1 hygiene green, Tier-3 internal adversarial
pre-review (2 LOW nits, no new soundness hole, ready for challenge pass).

- F1 T3 active-publication-window edit: T3 is orthogonal not cumulative;
  a human write during a live promotion/revert is overwritten with no
  capture/drift (the item ADR 0007 R14 deferred here) -> decision point,
  protocol-class, not designed
- F2 T1 rollback precise domain: layer 1 detects only rollback
  inconsistent vs a surviving expected terminal; coherent whole-DB
  suffix regression is layer 2's (was imprecise "partial rollback")
- F3 trust-root unsound: keys/KEK/credentials split; substitution is an
  SI-27/RF-14 residual (self-consistent forged home; auth-as-other-
  account), not "degrades to signature failure"; out-of-scope splits
  root (out) from unprivileged different-uid (defended by perms)
- F4 capture_sqlite follows symlinks for registered stores (only
  fabric.db is checked) -> RF-42
- F5 directory/kind semantics carried from A24/R25 + R24, per operation
- F6 G-PUBLISH conditional-publication decision point (the filing's
  explicit ask, previously dropped)
- F7 real inventory appendix with file:line anchors; R3 no-follow recast
  (capture_fs rejects symlinks via a check, not O_NOFOLLOW)
- F8 SHA cite -> PR #48; wrong-base scope fixed by rebase onto main

Decision points now 10; SI-32 stays open, no implementation authorized.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Ratify SI-32 as A27 (spec v0.10): storage adversary model, determinations D32-1..D32-10

ADR 0008 ACCEPTED with the ratification addendum. Spec v0.10 gains the
A27 bullet family in section 5.3 (three adversary tiers with the
rollback/trust-root/unsigned-index carve-outs, staged-bytes and
verify-on-read-back normative, per-kind entry rules, the standing T2
containment sentence), cross-refs in sections 1 and 9, and the
changelog. Filings: SI-40 (active-publication-window edit; topology arm
foreclosed by D32-4, preservation protocol leading candidate,
substrate-assisted CoW-snapshot direction recorded from the operator),
RF-41/RF-42/RF-43 with carriers, P29 and the G-PUBLISH gate entry, G14
negatives. Internal adversarial pre-review before the external round:
14 findings (4 medium), all applied and recorded in the addendum. Per
the stopping rule (PR #52): one delta-scoped external round next.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fold the delta round (5 findings): A27.1 T1 label, D32-2 caller-pin correction, SI-40 snapshot-timing fix

External delta round returned REQUEST CHANGES; all five findings folded.
Blocking: A27.1 regains its ratified holds-against-T1 label (D32-5); the
candidate body's "layer 2's job (external anchor / caller-pinned
expected head)" is corrected in the D32-2 addendum - caller pins are
layer 1's completeness tier, only the external anchor is layer 2.
Non-blocking: the SI-40 substrate-assisted paragraph no longer claims a
gate-acquisition snapshot closes the window (the in-window edit
postdates it); the preserving variant is outgoing-state retention at
swap plus diff/CAS-ingest/attribute. Lows: A27.4 label variants in
preserved candidate text normalized; v0.9 stamps in CLAUDE.md and the
spec status line bumped to v0.10. Round record in ADR 0008's addendum.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Record the operator gate decision: confirming delta round on the fold commit

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Close the review cycle: confirming round clean, two non-blocking findings folded

No blocking findings from the confirming delta round - the cycle closes
under the stopping rule. Folded: SI-40's per-file clone variant now
requires atomic clone-and-swap semantics (separate clone->rename
re-opens the window in miniature; dataset-level retain-and-swap is the
sound variant), and the roadmap's branch-local SHA citation replaced
per the citation rule. Cycle records in ADR 0008's addendum and the
roadmap: one internal pre-review (14 findings) + two external delta
rounds (5 + 2), against PR #48's seven rounds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: XVVH <admin@fent.quest>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant