From 27de8a1b8a4ed83cb9eb95c5b71d2f3b9197789e Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Wed, 2 Sep 2026 22:54:45 -0400 Subject: [PATCH] 16d-review-binding: gate the three rules that failed under attention Three rules this session produced had no gate behind them, and each had already failed once while enforced by attention alone. This commit makes them mechanical. 1. .githooks/pre-commit refuses a commit on `main`. Positive match on the full ref, so a tag named `main` cannot defeat it; detached HEAD is deliberately exempt, because rebase, bisect and the reviewer's index checkout all run detached. A slip-guard, not an authorization control. 2. .githooks/pre-commit greps tracked .md for every path the staged diff deletes or renames, failing on a hit not marked `` on the hit line or the one immediately above. NUL-delimited throughout, and both enumerations check git's exit status: earlier drafts failed OPEN on C-quoted paths, on blobs GNU grep calls binary, and when the enumeration exited 128 -- printing an affirmative verdict over an input never searched. 3. tools/signoff.sh binds the signoff to the tree the reviewers actually read: exactly two reviewer tree files per round, both equal to `git write-tree`, and .githooks/pre-commit re-checks that at commit time. Also: one definition of the reviewable-diff recipe in tools/gate.sh (copies 3 -> 2), with an equality test holding .githooks/commit-msg's remaining copy to it until slice 17 removes it; the two tools/gate.sh comment corrections carried from 16b; and BACKLOG.md section 11 recording the advisory-print `is_uint` duplicate. Four review rounds, two independent reviewers each on index checkouts, plus one owner-bounded record-lane read. Seven blocking findings, seven universal quantifiers, zero mechanism defects after round 1 -- every blocking finding was a claim in a tracked file, not a defect in the code it described. Evidence, probe results and dispositions in internal/slices/16d-review-binding/FINDINGS.md. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VNdY6wVS773UJgrHUeBN3Y Reviewed-diff: sha256:e72d17e808d5f3aac533e64fc0137b4cec601b78c2a5ab2fcf5e1c66950996e2 --- .githooks/pre-commit | 239 +++++++++++++++++- BACKLOG.md | 29 +++ .../test/deleted_path_example_test.exs | 79 ++++++ apps/hacktui_core/test/diff_recipe_test.exs | 143 +++++++++++ tools/gate.sh | 93 ++++++- tools/signoff.sh | 171 +++++++++++++ 6 files changed, 741 insertions(+), 13 deletions(-) create mode 100644 apps/hacktui_core/test/deleted_path_example_test.exs create mode 100644 apps/hacktui_core/test/diff_recipe_test.exs create mode 100755 tools/signoff.sh diff --git a/.githooks/pre-commit b/.githooks/pre-commit index ea44ac8..c27bd11 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -29,6 +29,37 @@ is_uint() { case "${1:-}" in ''|*[!0-9]*) return 1;; *) return 0;; esac; } echo "== CLAUDE.md section 4 commit gates ==" +# ---------- BRANCH ---------- +# Work is sliced (section 2) and lands on main by rebase-merged PR (section 4). A commit +# authored directly on main is a slip, and one already happened: it landed because no hook +# refused it, after a reviewer flagged the branch discrepancy and it was read past. A rule +# enforced by attention failed where the same rule as a gate has held every time. +# +# POSITIVE MATCH ONLY. A detached HEAD is allowed, deliberately: `git rebase` replays commits +# detached, `git bisect` runs detached, and the reviewer's index checkout is a detached +# worktree. Refusing on "cannot determine the branch" would block the rebase this repository +# merges with. The gate refuses when it can see the name `main`, and is silent otherwise -- +# which is the one place in this file where an unmeasurable condition is not a failure, and it +# is named rather than left as a fallthrough. +# +# This is a slip-guard, NOT an authorization control. `--no-verify` bypasses it like any hook, +# and a clone that never ran `mix setup` has no local hook at all. The remote is what actually +# prevents it: ruleset 22066749 requires a pull request, blocks non-fast-forward, and has no +# bypass actors. +# --short is DEFEATED BY REF AMBIGUITY: shorten_unambiguous_ref returns "heads/main" once a +# tag or a refs/main also exists, and "heads/main" != "main". Measured by reviewer 1 -- two +# commands and the refusal is gone. The full ref is unambiguous by construction. +branch_ref=$(git symbolic-ref --quiet HEAD || true) +branch=${branch_ref#refs/heads/} +if [ "$branch_ref" = "refs/heads/main" ]; then + echo + echo "COMMIT REJECTED -- CLAUDE.md sections 2 and 7: this commit is on 'main'." + echo " Work is sliced. Branch first: git switch -c slice/NN-kebab-name" + echo " main is reached by rebase-merged PR, never by a local commit." + exit 1 +fi +note "branch" "${branch:-detached HEAD (allowed: rebase/bisect/review checkout)}" + # ---------- index isolation ---------- # Gates measure the WORKING TREE. If it differs from the index the numbers do not # describe the commit. Section 4 forbids `git stash`, so we refuse rather than guess. @@ -43,6 +74,168 @@ if [ -n "$dirty" ]; then printf '%s\n' "$dirty" | sed 's/^/ /' fi +# ---------- DELETED AND RENAMED PATHS ---------- +# Every commit that moves or deletes a tracked path greps every tracked document for the old +# path before staging. +# +# WHAT THIS CATCHES, stated exactly, because the first version of this comment claimed more: +# a path that is TRACKED, that this commit DELETES OR RENAMES, and that a TRACKED .md cites by +# EXACT STRING, in the INDEX. Everything outside that is not covered, and the boundary is not +# incidental -- see below. +# +# THE THREE INSTANCES THAT MOTIVATED THIS GATE WOULD NOT HAVE BEEN CAUGHT BY IT. Reviewer 2 +# measured all three and the earlier claim here ("it would have caught all three") was false of +# every one: +# - docs/residuals.md's "test_failures is 0" (C2) is a retired baseline JSON KEY, not a path; +# - HANDOFF.md's stale tools/gate.sh citation (C2b) is a stale LINE NUMBER into a file that +# was never deleted; +# - the contracts commit's internal/incoming/ citations are the sharpest case: internal/ has +# never been tracked, so removing a file there produces NO --diff-filter=DR entry at all, +# and the surviving citation in BACKLOG.md names the parent DIRECTORY, which an exact-string +# match does not match. Run against that commit the gate correctly prints "none deleted or +# renamed in this commit" -- it is silent because there is nothing in its domain. +# +# The gate is kept anyway, on a narrower and honest claim: the tracked-path-rename case is real +# and a mechanical check beats attention for it. But it is NOT the general "a commit made a +# tracked document false" detector the first comment implied, and nothing here should be read as +# covering the three defects that produced the rule. +# +# An example that is true of THIS tree, not of a future one. The line below is machine-readable +# and gated: apps/hacktui_core/test/deleted_path_example_test.exs asserts the path is tracked +# and that a tracked .md cites it. The previous example named assets/images/UI.png "cited by +# HANDOFF.md" -- true only after the contracts commit lands, and `git grep` for it in the tree +# this ships in returns rc=1. A justification's example is a claim, so it gets the same gate as +# any other claim. +# EXAMPLE-PATH: docs/not_production_ready.md +# +# Further limits, all measured: an exact-string match flags a DOCUMENT CITING A LONGER PATH that +# merely starts with the deleted one (victim.md flags a citation of victim.md.bak) -- a false +# positive that fails closed; untracked .md are never scanned; and a path containing a newline +# is refused rather than searched. +# +# A hit passes only if the literal marker below is on the hit line or the line IMMEDIATELY +# above it. Two lines above does not count: one marker at the top of a file must not whitewash +# every citation beneath it. +HISTORICAL_MARKER='' +NL=$'\n' + +# EVERYTHING here is NUL-delimited, and that is not fastidiousness. The first version parsed +# `git diff --name-status` line by line and fed the field to `git grep -F`. `core.quotePath` +# defaults to true, so git C-quotes any path holding a non-ASCII byte, a tab, a quote or a +# newline -- and the quoted literal ("caf\303\251.md") cannot match the real bytes in the +# document. The gate then printed "not cited in any tracked .md" AND its affirmative summary +# line for a path it had never really searched for. A fail-OPEN, printed as a verdict, in the +# gate this slice exists to make fail-closed. Measured by reviewer 1: three of four hostile +# path shapes read as clean while their citations sat in the index verbatim. +# +# `-z` removes the quoting entirely, and `git ls-files -z` + `git show :path` removes the +# second parse -- we no longer read `git grep`'s "path:line:text" output at all, so a path +# containing a colon or a newline, or a file git calls binary, cannot confuse the reader. Line +# numbers are counted here, from the blob, which is also the only way to be sure the number +# indexes the same bytes the window is read from. +# THE ENUMERATION'S EXIT STATUS IS CHECKED, not discarded. `done < <(git ...)` throws git's +# status away: one unreadable object -- a partial clone, a failed fetch, a gc race, disk +# corruption -- makes git exit 128 with EMPTY output, so `gone` is empty and this gate prints +# "none deleted or renamed in this commit" over a staged deletion cited verbatim in a tracked +# .md. Measured at rc=128. Same class as the C-quoting and binary fail-opens before it: an +# affirmative verdict over an input that was never enumerated. A temp file is used instead of +# a process substitution precisely so the status survives to be tested. +: > "$LOGDIR/gone.z" +git diff --cached --name-status --find-renames --diff-filter=DR -z > "$LOGDIR/gone.z"; enum_rc=$? +path_enum_failed=0 +if [ "$enum_rc" -ne 0 ]; then + die "deleted paths" "FAIL -- could not enumerate deleted/renamed paths (git exited $enum_rc); refusing to pass unmeasured" + path_enum_failed=1 +fi + +gone=() +while IFS= read -r -d '' _status; do + IFS= read -r -d '' _oldpath || break + case "$_status" in + R*|C*) IFS= read -r -d '' _newpath || break ;; + esac + gone+=("$_oldpath") +done < "$LOGDIR/gone.z" + +if [ "$path_enum_failed" -eq 1 ]; then + : # already reported; no affirmative may follow an unmeasured enumeration +elif [ "${#gone[@]}" -eq 0 ]; then + note "deleted paths" "none deleted or renamed in this commit" +else + path_fail=0 + for old in "${gone[@]}"; do + [ -n "$old" ] || continue + # A newline inside the path makes `grep -F` read it as an ALTERNATION of substrings, and a + # path ending in a newline yields an empty alternative that matches every line of every + # .md. Neither answer is a search for that path, so it is refused rather than approximated. + case "$old" in + *"$NL"*) + die "deleted paths" "FAIL -- '$old' contains a newline; refusing to search for it -- resolve this citation by hand" + path_fail=1; continue ;; + esac + cited=0 + read_failed=0 + # The inner enumeration gets the same treatment. Reviewer 1 could not reach this one with + # stock git -- only with a PATH shim -- but "I could not construct it" is not "it cannot + # happen", and an unchecked status here would falsify the sentence below that says the + # affirmative is printed only when every .md was actually read. + : > "$LOGDIR/mdfiles.z" + git ls-files -z -- ':(icase)*.md' > "$LOGDIR/mdfiles.z"; ls_rc=$? + if [ "$ls_rc" -ne 0 ]; then + die "deleted paths" "FAIL -- could not enumerate tracked .md files (git exited $ls_rc); refusing to pass unmeasured" + path_fail=1; read_failed=1 + fi + while IFS= read -r -d '' mdfile; do + # $(...) on file bytes: safe HERE and unsafe in tools/gate.sh, for a reason worth stating + # rather than leaving as an inconsistency. Command substitution strips trailing newlines; + # that changes a HASH but not whether a substring occurs, and the line numbers below are + # counted from the same stripped bytes the window is read from, so the number and the + # window cannot disagree. gate.sh hashes, so it may not do this. + blob=$(git show ":$mdfile" 2>/dev/null) || { + die "deleted paths" "FAIL -- cannot read $mdfile from the index; refusing to pass unmeasured" + path_fail=1; read_failed=1; continue; } + # grep -n over the blob: exit 0 = matched, 1 = no match, >1 = error. Only 1 is "clean". + # -a is LOAD-BEARING. GNU grep calls input binary on an encoding error, then exits 0, + # writes "binary file matches" to STDERR and emits NOTHING on stdout -- so `matches` came + # back empty, the loop below never ran, and the gate printed "not cited in any tracked + # .md" plus its affirmative summary for a citation sitting in the blob byte-for-byte. + # The same fail-open as the C-quoting defect, reached through blob CONTENT instead of + # path encoding, and measured on a .md holding one invalid UTF-8 byte. + matches=$(printf '%s\n' "$blob" | grep -anF -- "$old"); rc=$? + if [ "$rc" -gt 1 ]; then + die "deleted paths" "FAIL -- grep failed reading $mdfile (rc=$rc); refusing to pass unmeasured" + path_fail=1; read_failed=1; continue + fi + [ "$rc" -eq 0 ] || continue + while IFS= read -r m; do + hl=${m%%:*} + case "$hl" in ''|*[!0-9]*) continue ;; esac + cited=1 + # The hit line and the one immediately above it. Two above must NOT count, or one + # marker at the top of a file would whitewash every citation beneath it. + prev=$((hl - 1)); [ "$prev" -lt 1 ] && prev=1 + window=$(printf '%s\n' "$blob" | sed -n "${prev},${hl}p") + if printf '%s' "$window" | grep -qF -- "$HISTORICAL_MARKER"; then + note "deleted paths" "$mdfile:$hl cites $old -- marked historical, allowed" + else + die "deleted paths" "FAIL -- $mdfile:$hl cites '$old', which this commit deletes or renames" + echo " mark the line (or the one directly above) with $HISTORICAL_MARKER, or fix the citation" + path_fail=1 + fi + done <"). This check is not what closes that window. + # + # What it does catch, each measured: + # 1. a signoff with the right hash and NO Reviewed-tree line -- i.e. one not written by + # tools/signoff.sh, which is what enforces the two-reviewer index-checkout protocol; + # 2. a signoff whose hash was copied in by hand while the tree line says something else; + # 3. any future case where the diff scope and the tree diverge -- the exclusion of + # internal/** above is exactly such a scope, and would become one if internal/ were + # ever tracked. + # It is defence in depth over a hash that is written by the party it certifies, not the + # primary guard. Signoffs written before this gate existed carry no Reviewed-tree line and + # fail closed here; they are historical records, and no commit is expected to match one. + reviewed_tree=$(sed -n 's/^Reviewed-tree:[[:space:]]*\([0-9a-f]\{40\}\).*/\1/p' "$matched" | head -1) + current_tree=$(git write-tree 2>/dev/null || true) + if [ -z "$reviewed_tree" ]; then + die "review signoff" "FAIL -- $matched carries no Reviewed-tree line; write it with tools/signoff.sh" + elif ! printf '%s' "$current_tree" | grep -qE '^[0-9a-f]{40}$'; then + die "review signoff" "FAIL -- git write-tree gave no tree hash; refusing to pass unmeasured" + elif [ "$reviewed_tree" != "$current_tree" ]; then + die "review signoff" "FAIL -- the index has moved since it was reviewed" + printf ' reviewed: %s\n index is: %s\n' "$reviewed_tree" "$current_tree" + else + note "review tree" "index still equals the reviewed tree (${reviewed_tree:0:12}...)" + fi fi fi diff --git a/BACKLOG.md b/BACKLOG.md index 7c13c34..5a2dad3 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -255,3 +255,32 @@ control's limits: the funnel carries others that are **visible in the implementa described in the moduledoc**, so reading the moduledoc alone returns this item and gives a false sense of completeness. Anyone assessing what leaves the MCP boundary should read the bodies of `egress.ex` and `privacy_mask.ex`, not this entry and not the doc comments. + +## 11. `is_uint` is defined twice — advisory print only + +`.githooks/pre-commit` carries a **byte-identical copy** of the `is_uint` helper defined in +`tools/gate.sh`. Found by a reviewer in slice 16b while checking that slice's own +one-invariant-one-implementation criterion. + +**It governs an advisory print only.** `is_uint` has exactly one call site in the hook, inside +the `ADVISORY` block, and neither branch of it sets `fail` — so the commit verdict is +unreachable from the duplicated function. It decides whether a dependency-audit count is +printable, nothing more. **No baseline or ratchet logic is duplicated** — that is the point of +recording it here rather than filing it as a defect. + +*(An earlier draft of this sentence said the hook "delegates every gate verdict to +`tools/gate.sh`". That was false of the tree and was falsified by the very commit that wrote +it: the hook renders several verdicts of its own — the branch refusal, the deleted-path check +and the `Reviewed-tree` check — and `CLAUDE.md` §4 lists all three as gates. The narrow claim +above is the one that was measured.)* + +It is still the exact hazard the surrounding code names: **two byte-identical copies pass a +grep count.** Today they agree; nothing makes them agree tomorrow. A count is not proof of one +implementation — the only proof is a mutation, and a mutation needs one thing to mutate. + +**Assigned to slice 17**, which removes the copy and imports the single definition; the hook +sourcing the function from `tools/gate.sh` is the obvious shape, and 17's PLAN decides it. + +Deliberately cited by **construct, not line number**: both files change under active work, and +a line citation in a file under change is the stale-citation class this repository has already +committed once inside the document recording the fix for it. diff --git a/apps/hacktui_core/test/deleted_path_example_test.exs b/apps/hacktui_core/test/deleted_path_example_test.exs new file mode 100644 index 0000000..2b5657c --- /dev/null +++ b/apps/hacktui_core/test/deleted_path_example_test.exs @@ -0,0 +1,79 @@ +defmodule HacktuiCore.DeletedPathExampleTest do + use ExUnit.Case, async: true + + # The deleted-path gate's justification comment carries one concrete example. An example in a + # justification is a CLAIM, and this repository's standing rule is that a claim in a tracked + # file is true of the tree it ships in — so the example gets the same gate as any other claim. + # + # This exists because it was already wrong once. The comment named `assets/images/UI.png` + # "cited by HANDOFF.md": true only after the contracts commit lands, while `git grep` for it in + # the tree that shipped the comment returned rc=1. A reviewer caught it by running the grep. + # A reviewer catching it is the thing this test replaces. + @root Path.expand("../../..", __DIR__) + @hook Path.join(@root, ".githooks/pre-commit") + + defp example_path do + src = File.read!(@hook) + + case Regex.run(~r/^#\s*EXAMPLE-PATH:\s*(\S+)\s*$/m, src, capture: :all_but_first) do + [path] -> + path + + _ -> + flunk(""" + .githooks/pre-commit carries no `# EXAMPLE-PATH: ` line. + + The deleted-path gate's comment must name its example on a machine-readable line, so + that the example can be checked rather than trusted. If the example was removed, remove + this test in the same commit and say so; do not leave an unchecked example behind. + """) + end + end + + defp tracked_md do + {out, 0} = System.cmd("git", ["ls-files", "-z", "--", "*.md"], cd: @root) + out |> String.split(<<0>>, trim: true) + end + + test "the gate's example path is tracked" do + path = example_path() + + {_, status} = + System.cmd("git", ["ls-files", "--error-unmatch", "--", path], + cd: @root, + stderr_to_stdout: true + ) + + assert status == 0, + "the deleted-path gate's example is `#{path}`, which is not tracked. " <> + "The gate only ever sees tracked paths, so an untracked example illustrates " <> + "nothing it can do." + end + + test "a tracked .md cites the gate's example path, at a line this test can name" do + path = example_path() + + citations = + for file <- tracked_md(), + {line, n} <- Enum.with_index(String.split(File.read!(Path.join(@root, file)), "\n"), 1), + String.contains?(line, path), + do: "#{file}:#{n}" + + refute citations == [], + """ + the deleted-path gate's comment names `#{path}` as its example, but no tracked .md + cites that path in this tree. + + That is the exact defect this test exists to catch: the comment's previous example + named a path whose only citation lived in an unlanded commit, so the justification + was true of a future tree and false of the shipping one. + + Either point EXAMPLE-PATH at a path a tracked .md actually cites, or drop the example. + """ + + # Named, not just counted: the failure message above is only useful if a passing run can say + # where the citation is. A test that proves existence without locating it makes the next + # person re-derive what this one already knew. + assert is_binary(hd(citations)) + end +end diff --git a/apps/hacktui_core/test/diff_recipe_test.exs b/apps/hacktui_core/test/diff_recipe_test.exs new file mode 100644 index 0000000..b6e97c7 --- /dev/null +++ b/apps/hacktui_core/test/diff_recipe_test.exs @@ -0,0 +1,143 @@ +defmodule HacktuiCore.DiffRecipeTest do + use ExUnit.Case, async: true + + # The reviewable-diff recipe decides what a commit's attestation covers. It has ONE + # definition, in tools/gate.sh (`DIFF_RECIPE` and `DIFF_SCOPE`), read by `derive_diff_hash` + # for the attestation gate and by `staged_diff_hash` for .githooks/pre-commit and + # tools/signoff.sh. + # + # .githooks/commit-msg still carries a second copy. It runs in a context where sourcing + # tools/gate.sh is a behaviour change, so removing that copy is slice 17's work. Until then + # the two are held together HERE rather than on faith: if they drift, the trailer a commit + # writes stops matching the hash the gate derives, and `Gate - attestation` goes red on main + # for a reason nobody would connect to an edit in a hook. + # + # One copy is the goal. Two copies with an equality gate is the acceptable interim. Two + # copies on faith is the defect class this repository keeps re-finding -- and a grep COUNT + # is not proof, because two byte-identical copies pass a count. This test compares the token + # sequences, so a change to either side fails it. + @root Path.expand("../../..", __DIR__) + @gate Path.join(@root, "tools/gate.sh") + @commit_msg Path.join(@root, ".githooks/commit-msg") + @pre_commit Path.join(@root, ".githooks/pre-commit") + + defp tokens(s), do: s |> String.split(~r/\s+/, trim: true) + + # `DIFF_RECIPE=(-c diff.noprefix=false ...)` -> the tokens between the parentheses. + # `^` alone was blind to an indented duplicate -- and bash executes an indented assignment + # exactly like a column-0 one, so ` DIFF_RECIPE=(-c diff.context=9)` inserted before + # DIFF_SCOPE changed the hash the gate produces while this test still reported 3 tests, 0 + # failures. A canary that a real duplicate walks past is not a canary. `^[ \t]*` counts them. + defp array(src, name) do + case Regex.run(~r/^[ \t]*#{name}=\((.*)\)\s*$/m, src, capture: :all_but_first) do + [body] -> tokens(body) + _ -> flunk("#{name}=(...) not found in tools/gate.sh, or spans more than one line") + end + end + + # Comment lines are stripped, then assignments are counted. The forms this matcher is known to + # count and known not to count are listed in the test below; it is a matcher, not a bash parser. + defp non_comment(src) do + src + |> String.split("\n") + |> Enum.reject(&Regex.match?(~r/^\s*#/, &1)) + |> Enum.join("\n") + end + + # `NAME=(`, `NAME+=(` and `NAME[i]=` are all assignments bash executes. Counting only `=(` + # was the THIRD form-blindness in this one construct: `^` missed an indented duplicate, + # `^[ \t]*` missed `;`/`then`/`eval`, and this missed `+=` and `[i]=` — each time while the + # duplicate measurably moved the hash the gate produces and the canary reported one + # definition. The forms are now covered by a test rather than by the next reviewer. + # An ASSIGNMENT, not a use: the bracket form must close and be followed by `=`, and `$`/`{` are + # excluded from the lookbehind, so `"${NAME[@]}"` does not count. Measured: without that, this + # test reported 3 definitions of a variable defined once, counting the two array expansions in + # derive_diff_hash and staged_diff_hash. + defp count_defs(src, name) do + length(Regex.scan(~r/(? + "Two byte-identical copies pass a grep count, so the only honest test is a " <> + "mutation, and a mutation needs exactly one thing to mutate." + end + end + + # Nine forms, measured: six that bash executes and the counter counts, three inert ones it does + # not. Regression locks for forms this file has been bitten by. Asserted against synthetic + # source rather than by mutating the real file, so they run in the ordinary suite. + test "nine known assignment forms: six counted, three not" do + base = "DIFF_RECIPE=(-c diff.context=3)\n" + + live = [ + {"plain second definition", "DIFF_RECIPE=(-c diff.context=9)"}, + {"indented", " DIFF_RECIPE=(-c diff.context=9)"}, + {"after a semicolon", "true; DIFF_RECIPE=(-c diff.context=9)"}, + {"after then", "if true; then DIFF_RECIPE=(-c diff.context=9); fi"}, + {"append with +=", "DIFF_RECIPE+=(-c diff.context=9)"}, + {"element assignment", "DIFF_RECIPE[1]=--patience"} + ] + + for {label, form} <- live do + assert count_defs(base <> form <> "\n", "DIFF_RECIPE") == 2, + "the counter does not see a duplicate #{label} (#{inspect(form)}), " <> + "which bash executes." + end + + # Three inert forms. A counter that matched everything would fail these. + assert count_defs(base <> "# DIFF_RECIPE=(-c diff.context=9)\n", "DIFF_RECIPE") == 1, + "a duplicate quoted inside a comment is inert and must not count" + + assert count_defs(base <> "MY_DIFF_RECIPE=(-c diff.context=9)\n", "DIFF_RECIPE") == 1, + "a longer variable name that merely ends in DIFF_RECIPE must not count" + + assert count_defs(base <> ~S| git "${DIFF_RECIPE[@]}" diff --cached| <> "\n", "DIFF_RECIPE") == + 1, + "an array EXPANSION is a use, not a definition, and must not count" + end + + test ".githooks/pre-commit holds no copy of the recipe" do + refute File.read!(@pre_commit) =~ "diff.noprefix", + ".githooks/pre-commit carries its own copy of the diff recipe again. It should call " <> + "`./tools/gate.sh staged-diff-hash` instead." + end + + test ".githooks/commit-msg's recipe is token-identical to the one in tools/gate.sh" do + gate = File.read!(@gate) + recipe = array(gate, "DIFF_RECIPE") + scope = array(gate, "DIFF_SCOPE") + + # The commit-msg invocation runs from `git ` to the pipe into sha256sum, across + # backslash-continued lines. Join continuations, then compare token sequences -- not a + # substring match, which is what fails on line-wrapped text (slice 16b, three times). + src = File.read!(@commit_msg) + + invocation = + case Regex.run(~r/(git\s+-c\s+diff\.noprefix.*?)\|\s*sha256sum/s, src, + capture: :all_but_first + ) do + [inv] -> inv |> String.replace("\\\n", " ") |> tokens() + _ -> flunk(".githooks/commit-msg: could not find the `git ... | sha256sum` invocation") + end + + expected = ["git"] ++ recipe ++ ["diff", "--cached"] ++ scope + + assert invocation == expected, """ + .githooks/commit-msg and tools/gate.sh disagree about the reviewable-diff recipe. + + commit-msg: #{Enum.join(invocation, " ")} + gate.sh: #{Enum.join(expected, " ")} + + These two must produce byte-identical diffs: commit-msg writes the Reviewed-diff trailer + and tools/gate.sh derives the value CI compares it against. A drift here turns every new + commit red on `Gate - attestation`. Change both, or finish removing the copy (slice 17). + """ + end +end diff --git a/tools/gate.sh b/tools/gate.sh index 2334382..42e2ddc 100755 --- a/tools/gate.sh +++ b/tools/gate.sh @@ -263,9 +263,14 @@ baseline_gate() { fi # RULE 1 -- immutable and append-only. Every entry present at the previous ref must still - # be present now. Equality is jq -S canonical over the ENTIRE entry object, every field, - # not a selected subset: a changed `reason` or `slice` is caught as readily as a changed - # `retired_at`, and key order or whitespace cannot disguise an edit. + # be present now. Equality is jq -S canonical NUMERIC equality over the ENTIRE entry object, + # every field, not a selected subset: a changed `reason` or `slice` is caught as readily as a + # changed `retired_at`, and key order or whitespace cannot disguise an edit. + # + # "numeric equality" is the precise word and the earlier "equality over every field" was + # marginally stronger than the mechanism: jq treats 76 and 76.0 as ONE number, so editing an + # existing entry's retired_at from 76 to 76.0 is accepted as unchanged (rc=0), while 76 -> 77 + # is caught (rc=1). Semantically a no-op, and stated rather than left for a reader to discover. if ! jq -e -n --argjson p "$prev_ret" --argjson q "$now_ret" \ '(($p - $q) | length) == 0' >/dev/null 2>&1; then note baseline "FAIL -- a _retired entry present at $ref was removed or altered; the record is append-only and immutable" @@ -304,6 +309,12 @@ baseline_gate() { # A previous version ran `tostring` over any JSON value: 76.0 came through as "76.0", # failed is_uint, and was reported as "carries no numeric retired_at" -- which was false, # since it IS a number. 7.6e1 meanwhile became "76" and passed. Same value, two verdicts. + # + # Not eliminated at the extremes, and said plainly rather than implied: this fails closed on + # inputs no real count can produce; the message is wrong of the input. For 1e20 the reader + # emits "1e+20" rather than "NaN", so the gate says "not a whole number" of a value that IS + # a non-negative integer; -1 produces the same wording. Both are unreachable with real + # baseline counts, and both refuse rather than pass, which is the property that matters. if [ "$rat" = "-" ]; then note baseline "FAIL -- new _retired entry for $rk carries no retired_at" return 1 @@ -456,10 +467,60 @@ baseline_gate() { # against a tracked allowed-signers file. This slice removes the author's ability to hand # the gate its own answer; it does not yet make the attester a different party. # --------------------------------------------------------------------------- +# ONE definition of the reviewable-diff recipe. Every consumer reads these two arrays: +# attestation (derive_diff_hash), the pre-commit signoff check and tools/signoff.sh +# (staged_diff_hash, via the `staged-diff-hash` subcommand). This is the class-(a) rule +# applied to itself -- two byte-identical copies pass a grep count, so the only honest test +# is a mutation, and a mutation needs exactly one thing to mutate. +# +# .githooks/commit-msg still carries its own copy: it runs in a context where sourcing this +# script is a behaviour change, and removing that copy is slice 17's work. It does not sit +# on faith in the meantime -- apps/hacktui_core/test/diff_recipe_test.exs asserts the two +# token sequences are identical, so they cannot drift silently inside that window. +DIFF_RECIPE=(-c diff.noprefix=false -c diff.context=3 -c diff.algorithm=myers -c core.abbrev=40) +DIFF_SCOPE=(--binary --no-ext-diff --no-textconv -- . ':(exclude)internal/**') + derive_diff_hash() { - git -c diff.noprefix=false -c diff.context=3 -c diff.algorithm=myers -c core.abbrev=40 \ - diff "$1" "$2" --binary --no-ext-diff --no-textconv -- . ':(exclude)internal/**' \ - | sha256sum | cut -d' ' -f1 + git "${DIFF_RECIPE[@]}" diff "$1" "$2" "${DIFF_SCOPE[@]}" | sha256sum | cut -d' ' -f1 +} + +# The same recipe against the index. `git diff --cached` takes no ref pair, which is why it +# cannot simply call derive_diff_hash -- but it reads the same two arrays, so a change to the +# recipe moves both or neither. +# +# THREE things this has to get right, all of them found by review rather than by design: +# +# 1. It must FAIL when it cannot measure. The first version piped `git diff` straight into +# sha256sum, so when git errored -- outside a repository, say -- sha256sum hashed empty +# input and produced e3b0c442..., a perfectly well-formed 64-hex string, with rc=0. A +# caller that cannot measure must not receive something that looks like a measurement; +# that is the defect class this whole file was written to remove, committed inside the +# commit that removes it. PIPESTATUS carries git's status out of the pipeline. +# 2. It must not depend on the caller's directory. DIFF_SCOPE ends in `-- .`, which is +# relative, so the same index answered differently from the root and from a subdirectory +# -- returning the empty-diff hash, with rc=0, while a change was staged. It resolves the +# toplevel itself rather than trusting the caller to have cd'd. +# 3. It must not use $(...) on the diff BYTES. Command substitution strips trailing newlines, +# so the hash would silently differ from every other consumer's. Measured: pipeline +# 9fed0f8d..., $(...) 0cad12fe..., and re-appending the one stripped newline restores +# 9fed0f8d... exactly. Only the finished hex string passes through a substitution. +# (An earlier version of this comment also blamed NULs "that --binary hunks contain". That +# is wrong and was corrected by review: --binary hunks are base85 ASCII -- `tr -dc '\000'` +# over such a diff counts zero. Bash does drop NULs from a substitution, but the diff of a +# binary file is not where they come from. The trailing newline alone is the reason.) +staged_diff_hash() { + local top h rc + top=$(git rev-parse --show-toplevel 2>/dev/null) || return 1 + [ -n "$top" ] || return 1 + h=$( + cd "$top" || exit 1 + git "${DIFF_RECIPE[@]}" diff --cached "${DIFF_SCOPE[@]}" | sha256sum | cut -d' ' -f1 + exit "${PIPESTATUS[0]}" + ) + rc=$? + [ "$rc" -eq 0 ] || return 1 + printf '%s' "$h" | grep -qE '^[0-9a-f]{64}$' || return 1 + printf '%s' "$h" } attestation_gate() { @@ -504,7 +565,7 @@ attestation_gate() { return 0 } -case "${1:?usage: tools/gate.sh }" in +case "${1:?usage: tools/gate.sh }" in format) mix format --check-formatted >/dev/null 2>&1 \ @@ -563,5 +624,23 @@ case "${1:?usage: tools/gate.sh &2; exit 2; } + h=$(staged_diff_hash) || { + echo "staged-diff-hash: could not measure the index; refusing to print a hash" >&2; exit 2; } + [ "$h" = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ] && { + echo "staged-diff-hash: the staged reviewable diff is empty; nothing to hash" >&2; exit 2; } + printf '%s\n' "$h" + ;; + *) echo "unknown gate: $1" >&2; exit 2 ;; esac diff --git a/tools/signoff.sh b/tools/signoff.sh new file mode 100755 index 0000000..05839c8 --- /dev/null +++ b/tools/signoff.sh @@ -0,0 +1,171 @@ +#!/usr/bin/env bash +# +# Writes internal/slices//REVIEW.signoff, and refuses unless the index is still the +# tree the reviewers actually read. +# +# Why this exists: the attestation covers INDEX bytes while the drift test hashes WORKTREE +# bytes, so a signoff computed after a mid-review edit attests a diff that no longer exists. +# That happened -- three of six staged files changed on disk during slice 16c's round 2, and +# the reviewer reported "the diff I reviewed and hashed is not the diff on disk". The edits +# were legitimate; doing the right work at the wrong moment silently invalidated a review in +# flight. The protocol said "hold edits during review" and had nothing behind it but the +# author's attention, which is the third rule this session to fail that way. +# +# So the binding is mechanical: reviewers read a checkout of the INDEX, each writes the tree +# hash it read to logs/round.r.tree, and this script refuses to write unless +# `git write-tree` still equals both of them. +# +# EXACTLY TWO tree files are required for the round. "All equal" over a single file is +# vacuous -- it compares a value to itself. +# +# The two-per-round count is THIS SLICE'S rule, recorded in CLAUDE.md section 6. It is not in +# section 8, which this comment used to cite: section 8 says to spawn independent reviewer +# subagents, plural, and names no number. A tracked file that hard-fails a round on the +# authority of a section that does not carry the rule is the founding defect of +# internal/REVIEWER-PROTOCOL.md -- a claim about a governing file's contents that the file does +# not contain -- so the citation is corrected rather than left to be inherited. +# +# NOT in scope: proving a review happened, or who performed it. This records WHICH tree was +# read and WHICH diff is being certified. The hash is still written by the party it +# certifies. See CLAUDE.md section 0. +# +# A single bounded record-lane read is deliberately NOT expressible here. It is not a round, +# and a slice closing on one writes its signoff by hand with that stated in the header. +set -uo pipefail +cd "$(git rev-parse --show-toplevel)" || exit 1 + +die() { echo "SIGNOFF REFUSED -- $*" >&2; exit 1; } + +slice_dir="${1:-}" +[ -n "$slice_dir" ] || die "usage: tools/signoff.sh [--replace]" +slice_dir="${slice_dir%/}" +[ -d "$slice_dir" ] || die "$slice_dir/ does not exist" +replace="${2:-}" + +logs="$slice_dir/logs" +[ -d "$logs" ] || die "$logs/ does not exist; reviewers write their tree hash there" + +# ---------- find the highest round that has tree files ---------- +# Fail closed on every branch: a round that cannot be identified is not round 0. +round="" +for f in "$logs"/round*.r*.tree; do + [ -f "$f" ] || continue + n=${f##*/round}; n=${n%%.r*} + # Three shapes, all found by review, all of which defeated the naive check: + # - non-numeric -> `[ "$n" -gt ... ]` errors and the `if` silently takes the false + # branch, so the file is SKIPPED rather than refused; + # - 99999999999999999999 -> same, because it is numeric but outside intmax; + # - round01 vs round1 -> two globs for ONE logical round, so three reviewer files split + # into 2 + 1 and the "exactly two" invariant passed over a subset. + # A round is 1-4 digits with no leading zero. Anything else is refused, not skipped. + case "$n" in + 0*|''|*[!0-9]*) die "malformed round number in $f (expected round.r.tree, N a 1-4 digit number with no leading zero)" ;; + esac + [ "${#n}" -le 4 ] || die "implausible round number in $f: $n" + if [ -z "$round" ] || [ "$n" -gt "$round" ]; then round="$n"; fi +done +[ -n "$round" ] || die "no logs/round.r.tree files in $logs/ -- nothing to check is not a pass" + +# ---------- exactly two, one per reviewer ---------- +# The suffix must be r and the two must DIFFER. Unvalidated, "exactly two files" was +# not "two reviewers": round1.r1.tree + round1.r1b.tree, or + round1.rZZZ.tree, both satisfied +# the count from one party -- which is the whole property the count exists to enforce. +files=() +suffixes="" +for f in "$logs"/round"$round".r*.tree; do + [ -f "$f" ] || continue + s=${f##*.r}; s=${s%.tree} + case "$s" in ''|*[!0-9]*) die "malformed reviewer suffix in $f (expected round.r.tree, M a number)" ;; esac + case " $suffixes " in *" $s "*) die "two tree files for round $round carry the same reviewer number r$s" ;; esac + suffixes="$suffixes $s" + files+=("$f") +done +found=${#files[@]} +[ "$found" -eq 2 ] || die "round $round has $found tree file(s); exactly 2 are required, one per reviewer" + +# ---------- read them ---------- +# There WAS a `read $read_ok of $found; refusing on a partial read` assertion here, modelled on +# the slice-16b row-count check. It was removed rather than kept, because it could not fail: +# every failure inside this loop calls `die`, which exits, so the counter always equalled the +# total by the time it was compared. A check that can only pass is worse than no check, because +# it reads as coverage -- this file's own CRITERIA say a row that cannot fail is not a +# criterion, and reviewer 1 applied that to the code rather than to the table. +# +# The real guarantee is structural and is stated instead of asserted: the loop is fail-closed +# at every step -- an unreadable file or a non-40-hex body exits non-zero and no signoff is +# written. `found` is separately pinned at exactly 2 above. +trees=() +for f in "${files[@]}"; do + t=$(tr -d '[:space:]' < "$f") || die "cannot read $f" + printf '%s' "$t" | grep -qE '^[0-9a-f]{40}$' \ + || die "$f does not contain a 40-hex tree hash (got: '${t:0:60}')" + trees+=("$t") +done +# There is deliberately NO count assertion here. The first one could not fail; its replacement +# could not fail either, for the identical reason -- `found` is pinned at 2 above and every +# in-loop failure exits -- and it was written three lines under a comment explaining that a +# check which can only pass is worse than none. Twice in one file is a pattern, not a slip, so +# the property is stated instead: this loop is fail-closed at every step, and `found` is the +# one place the count is actually enforced. + +[ "${trees[0]}" = "${trees[1]}" ] || { + echo " reviewer A: ${trees[0]}" >&2 + echo " reviewer B: ${trees[1]}" >&2 + die "the two reviewers of round $round read DIFFERENT trees" +} +reviewed="${trees[0]}" + +# ---------- the index must still be that tree ---------- +# 40-hex assumes a SHA-1 object format. In a SHA-256 repository every hash here is 64 and this +# script dies on all of them -- fail-closed, and recorded rather than handled, because this +# repository is SHA-1 and a widened pattern would accept a 40-hex hash from a 64-hex repo. +current=$(git write-tree) || die "git write-tree failed; refusing to sign an unmeasured index" +printf '%s' "$current" | grep -qE '^[0-9a-f]{40}$' || die "git write-tree did not return a tree hash" + +if [ "$current" != "$reviewed" ]; then + echo " reviewed: $reviewed" >&2 + echo " index is: $current" >&2 + die "the index has moved since round $round was reviewed; re-review the delta, do not sign" +fi + +# ---------- the diff being certified ---------- +# One definition of the recipe, in tools/gate.sh. This script deliberately holds no copy. +# LOGDIR is passed so gate.sh does not mktemp -d a directory it never cleans up; it has no +# EXIT trap, so every unpassed call leaked one. +tmp_logs=$(mktemp -d "${TMPDIR:-/tmp}/hacktui-signoff.XXXXXXXX") || die "cannot create a temp dir" +trap 'rm -rf "$tmp_logs"' EXIT +diff_hash=$(LOGDIR="$tmp_logs" ./tools/gate.sh staged-diff-hash) \ + || die "could not derive the staged-diff hash (unmeasurable, or the reviewable diff is empty)" +if [ "$diff_hash" = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ]; then + die "the staged diff is empty; an empty-diff hash must never read as a signed review" +fi + +out="$slice_dir/REVIEW.signoff" +if [ -f "$out" ] && [ "$replace" != "--replace" ]; then + if grep -qF -- "$diff_hash" "$out" && grep -qF -- "Reviewed-tree: $reviewed" "$out"; then + echo "signoff already records this tree and diff; nothing to do." + exit 0 + fi + die "$out exists and records something else; pass --replace if that is intended" +fi + +{ + echo "# Slice: $slice_dir" + echo "#" + echo "# Round $round, two independent reviewers, both on a checkout of this index." + echo "# Written by tools/signoff.sh, which refused until \`git write-tree\` equalled the tree" + echo "# both reviewers printed. It records WHICH tree was read and WHICH diff is certified." + echo "# It does not prove a review happened -- the hash is written by the party it certifies." + echo "# See CLAUDE.md section 0 and .githooks/pre-commit." + echo "#" + echo "# Reviewer tree files:" + for f in "${files[@]}"; do echo "# $f"; done + echo "" + echo "Reviewed-tree: $reviewed" + echo "" + echo "$diff_hash" +} > "$out" + +echo "signoff written: $out" +echo " reviewed-tree: $reviewed" +echo " staged-diff: $diff_hash"