diff --git a/.claude/hooks/README.md b/.claude/hooks/README.md new file mode 100644 index 0000000..19d94ed --- /dev/null +++ b/.claude/hooks/README.md @@ -0,0 +1,46 @@ +# Claude Code hooks + +Repo-committed hooks so they travel to every checkout. Registered in [`.claude/settings.json`](../settings.json) once a human has reviewed them (harness-safety: a hook lands in `settings.json` only after review, because a bad hook can lock out the tools that would fix it). + +They **fail open**: any missing tool, unparseable input, or internal error is a no-op, never a false block. The only actions any hook takes are the narrow, high-confidence cases, each with an announced environment-variable override. Every gate is provable without a running app by a committed `*.tests.sh` matrix that drives it against a throwaway `CLAUDE_PROJECT_DIR`; run the matrix after any edit to the gate. + +NetPace keeps both production and `*.Tests` projects under `src/`, so the filesystem-reading gates scan `src/` (there is no top-level `tests/`). + +## `no-skipped-tests.sh` — skip-family ban (Constitution §X) + +PreToolUse(Bash) gate that blocks a `git commit` while any banned skipped-test construct exists under `src/`: `Skip.If/IfNot/Always/Unless`, `Assert.Skip`, `[Fact/Theory(Skip=…)]`, `[SkippableFact/Theory]`, `SkipException`, and xUnit v3's `SkipUnless=`/`SkipWhen=`. Also exposes `--check` for CI/manual scans (the audit ignores the override, so a leaked env var can never silence it). Fails **closed** once a command is classified as a commit — a gate that can't scan must not read as clean — and waves every non-commit Bash call straight through. Override: `NETPACE_ALLOW_SKIPS=1` (announced on stderr). Promotes Constitution §X into a gate rather than a rule the agent must remember. + +```bash +.claude/hooks/no-skipped-tests.sh --check # scan src/, exit 1 on any banned construct +.claude/hooks/no-skipped-tests.tests.sh # synthetic-sandbox matrix — non-zero on failure +``` + +## `green-gate.sh` — `dotnet test --no-build` staleness guard + +PreToolUse(Bash) gate that denies `dotnet test --no-build` when it would report misleading results: no test assembly has been built yet, or a `*.cs` under `src/` is newer than the newest built `*.Tests.dll`. Either way `--no-build` would run a stale or absent assembly. Promotes [`feedback_dotnet_test_no_build`](../memory/feedback_dotnet_test_no_build.md). + +**Command detection:** the `dotnet test` matcher fires only when it is the actual command — after stripping benign `cd …&&` / `export …&&` / env-assignment / `rtk` prefixes — not when the string merely appears inside a commit message, an `echo`, or quoted data. `--no-build` must be a flag of the `dotnet test` invocation itself, not a substring in a chained command. The staleness scan ignores generated `obj/`/`bin/` `.cs` so an unrelated restore or build can't falsely fire. + +**Fail open.** Any missing tool, unparseable input, or non-`dotnet test` command is a no-op. It is wired without an `if`, so the script's own prefix-stripping does the filtering and a prefix-wrapped `cd repo && dotnet test --no-build` is still caught. Override: `NETPACE_SKIP_GREEN_GATE=1` (announced on stderr). + +```bash +.claude/hooks/green-gate.tests.sh # synthetic-JSON matrix — non-zero on failure +``` + +## `traceability-gate.sh` — AC↔marker traceability gate (Constitution §VIII) + +Stop hook enforcing the two exact-match edges of the §VIII traceability chain — spec.md `**Scenario: X**` label → test-plan.md `#### Scenario: X` header → test `// SCENARIO: X` marker under `src/`. It checks the edges a machine can decide; the judgment checks (fuzzy match, mock self-satisfaction, trivially-passing bodies, undocumented-test detection) stay in `/speckit.testchecklist`. + +| Edge | Rule | Direction | +|------|------|-----------| +| **spec ⟷ test-plan** | every `**Scenario: X**` label has exactly one matching `#### Scenario: X` header, and vice versa — a repeated name on either side is flagged (the label is a unique §VIII key) | bijection | +| **test-plan → code** | every `#### Scenario: X` header has ≥1 matching `// SCENARIO: X` marker under `src/` (generated `obj/`/`bin/` copies excluded) | coverage only | + +**Scope — active specs only.** The gate reads `specs/*/spec.md`. Merged features have their specs deleted (leaving only drifted markers behind), so a repo with no in-flight feature — the steady state — is a clean no-op. The test-plan→code edge is deliberately **directional**: a marker with no plan scenario is not flagged, because `src/` accumulates markers from already-merged features whose specs are gone. "Undocumented test" is a judgment left to `/speckit.testchecklist`. + +**Staged fail-open:** a spec still being authored never blocks. No `test-plan.md`, or a plan with no scenarios yet → no-op. Plan scenarios present but zero have a marker → pre-implementation, so the coverage edge is skipped (only the spec⟷plan edge runs). Once any scenario has a marker, all must. It is loop-guarded (`stop_hook_active`), so it nudges at most once per turn and can never hard-lock. Override: `NETPACE_SKIP_TRACEABILITY_GATE=1` (announced on stderr). + +```bash +.claude/hooks/traceability-gate.sh --check [specdir] # report + exit 1 on any mismatch (CI/manual) +.claude/hooks/traceability-gate.tests.sh # synthetic-fixture matrix — non-zero on failure +``` diff --git a/.claude/hooks/green-gate.sh b/.claude/hooks/green-gate.sh new file mode 100755 index 0000000..9eb920f --- /dev/null +++ b/.claude/hooks/green-gate.sh @@ -0,0 +1,119 @@ +#!/usr/bin/env bash +# +# green-gate.sh — `dotnet test --no-build` staleness guard. +# +# A single PreToolUse(Bash) gate: deny `dotnet test --no-build` when no test assembly has +# been built yet, or when a source is newer than the built assembly — either way a +# --no-build run would execute a stale/absent DLL and report misleading results. Promotes +# feedback_dotnet_test_no_build. +# +# SCOPE. This gate does exactly one thing: deny a `dotnet test --no-build` that would run a +# stale or absent test assembly. The "tests are green before a PR" guarantee lives elsewhere — +# a real whole-suite run inside the human-invoked `/ship` command, not in this hook. +# +# DESIGN RULE: fail OPEN. Any missing tool, unparseable input, or internal error exits +# 0 (no objection). The one action taken (the --no-build deny) is the narrow, high-confidence case +# only; every uncertain path defaults to allow, because a guard that falsely blocks is +# worse than no guard — and, for a harness we edit with itself, a false block can lock out +# the very tools that would fix it. +# +# Escape hatch (harness-safety: override-first, then tighten): set NETPACE_SKIP_GREEN_GATE=1 +# to make the whole script a no-op. It announces itself on stderr so it can never be +# silently in effect. + +set -uo pipefail + +# --- override-first escape hatch — announced, never silent ------------------------ +if [ "${NETPACE_SKIP_GREEN_GATE:-}" = "1" ]; then + echo "green-gate: WARNING — gate BYPASSED via NETPACE_SKIP_GREEN_GATE=1 (--no-build staleness deny NOT enforced)." >&2 + exit 0 +fi + +# --- fail-open preconditions ------------------------------------------------------ +command -v jq >/dev/null 2>&1 || exit 0 +INPUT=$(cat 2>/dev/null) || exit 0 +[ -n "$INPUT" ] || exit 0 + +jget() { printf '%s' "$INPUT" | jq -r "$1" 2>/dev/null; } + +EVENT=$(jget '.hook_event_name // empty') +TOOL=$(jget '.tool_name // empty') + +ROOT="${CLAUDE_PROJECT_DIR:-$(pwd)}" + +# Print the first source file newer than the reference file $1, or nothing. The source root +# whose edits invalidate a build is the .NET tree under src/ (*.cs) — where NetPace keeps both +# production and *.Tests projects; obj/ and bin/ (generated .cs) are excluded so an unrelated +# restore/build can't read as "our code changed". Only roots that exist are scanned; find needs +# the roots BEFORE the expression, and -quit stops at the first hit (cheap). +first_newer_than() { + local ref="$1" d roots=() + for d in src; do [ -d "$ROOT/$d" ] && roots+=("$ROOT/$d"); done + [ ${#roots[@]} -eq 0 ] && return 0 + find "${roots[@]}" -name '*.cs' \ + -not -path '*/obj/*' -not -path '*/bin/*' \ + -newer "$ref" -print -quit 2>/dev/null +} + +# Strip the known-benign LEADING prefixes so what remains begins with the real command: +# chained `cd …&&` / `export …&&`, env-assignments, an optional `rtk` wrapper. Regex can't +# parse shell, so we only ever strip these safe leaders — never anything that could hide a +# different command. +strip_cmd_prefixes() { + printf '%s' "$1" | sed -E ' + s/^[[:space:]]+// + :a; s/^(cd|export)[[:space:]]+[^&]*&&[[:space:]]*//; ta + :b; s/^[A-Za-z_][A-Za-z0-9_]*=[^[:space:]]*[[:space:]]+//; tb + s/^rtk[[:space:]]+// + ' +} + +# True only when `dotnet test` is the actual command being run — not text inside an +# argument (a commit message, an echo string, quoted data that merely mentions it). After +# prefix-stripping, require what remains to START with `dotnet test` at a word boundary +# (so `dotnet testfoo` and `git commit -m "…dotnet test…"` do not match). +is_dotnet_test() { + strip_cmd_prefixes "$1" | grep -Eq '^dotnet[[:space:]]+test([[:space:]]|$)' +} + +# Echo only the leading `dotnet test …` invocation — the segment up to the first chained +# command (&&, ||, ;, |, &) — so a `--no-build` sitting in a trailing `echo` or a quoted +# argument can't be mistaken for a flag belonging to `dotnet test`. Empty if not a real +# `dotnet test` run. +dotnet_test_invocation() { + is_dotnet_test "$1" || return 0 + strip_cmd_prefixes "$1" | sed -E 's/[[:space:]]*(&&|\|\||[;&|]).*$//' +} + +emit_pre_deny() { # reason + jq -n --arg r "$1" '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"deny",permissionDecisionReason:$r}}' + exit 0 +} + +case "$EVENT" in +# --------------------------------------------------------------------------------- +# deny `dotnet test --no-build` against a stale build. +PreToolUse) + [ "$TOOL" = "Bash" ] || exit 0 + CMD=$(jget '.tool_input.command // empty') + INVOKE=$(dotnet_test_invocation "$CMD") + [ -n "$INVOKE" ] || exit 0 + # --no-build must be a flag of the `dotnet test` invocation itself, not a substring in a + # chained command or a quoted argument — check the extracted invocation, not raw $CMD. + printf '%s' "$INVOKE" | grep -Eq -- '--no-build' || exit 0 + + # Newest built test assembly is the reference "last build". + newest_dll=$(find "$ROOT/src" -path '*/bin/*' -name '*.Tests.dll' -printf '%T@ %p\n' 2>/dev/null \ + | sort -n | tail -1 | cut -d' ' -f2-) + if [ -z "$newest_dll" ]; then + emit_pre_deny "No built test assemblies found, but --no-build was requested. Build first (drop --no-build), then re-run. (feedback_dotnet_test_no_build)" + fi + newer=$(first_newer_than "$newest_dll") + if [ -n "$newer" ]; then + emit_pre_deny "Source changed since the last build (e.g. ${newer#"$ROOT"/}). 'dotnet test --no-build' would run a STALE assembly and report misleading results. Rebuild first (drop --no-build). (feedback_dotnet_test_no_build)" + fi + exit 0 + ;; +esac + +exit 0 diff --git a/.claude/hooks/green-gate.tests.sh b/.claude/hooks/green-gate.tests.sh new file mode 100755 index 0000000..b877340 --- /dev/null +++ b/.claude/hooks/green-gate.tests.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# +# green-gate.tests.sh — standalone synthetic-JSON test matrix for green-gate.sh. +# +# The hook is a single PreToolUse(Bash) gate, so every branch is provable by piping +# synthetic hook JSON — no dev stack required. This script drives all branches against a +# throwaway CLAUDE_PROJECT_DIR so the real repo is never touched, and exits non-zero on any +# failure. Run it after any edit to the hook. +# +# (History: the hook once also carried PostToolUse ledger-stamping and a Stop completion +# gate; both were retired in issue #122 when test-green enforcement moved to `/ship`. Those +# cases are gone from this matrix; only the marker-independent B7 --no-build deny remains.) +# +# Usage: .claude/hooks/green-gate.tests.sh + +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HERE/green-gate.sh" + +SB="$(mktemp -d)" +trap 'rm -rf "$SB"' EXIT +export CLAUDE_PROJECT_DIR="$SB" +mkdir -p "$SB/src/Svc" "$SB/src/Svc.Tests/bin/Debug" +DLL="$SB/src/Svc.Tests/bin/Debug/Svc.Tests.dll" + +pass=0; fail=0 +run() { OUTPUT="$(printf '%s' "$1" | "$HOOK" 2>/dev/null)"; RC=$?; } +ok() { if eval "$2"; then echo " ok $1"; pass=$((pass+1)); else echo " FAIL $1 -- got:[$OUTPUT] rc=$RC"; fail=$((fail+1)); fi; } + +# Synthetic-payload builder (keep the real hook schema in one place). +pre() { printf '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":%s}}' "$(jq -Rn --arg c "$1" '$c')"; } + +echo "PreToolUse / B7 (--no-build staleness):" +run "$(printf '{"hook_event_name":"PreToolUse","tool_name":"Edit","tool_input":{"file_path":"x.cs"}}')"; ok "non-Bash tool → allow" '[ -z "$OUTPUT" ] && [ "$RC" = 0 ]' +run "$(pre 'echo dotnet test --no-build')"; ok "echo mention → allow" '[ -z "$OUTPUT" ]' +run "$(pre 'dotnet test')"; ok "dotnet test (build) → allow" '[ -z "$OUTPUT" ]' +run "$(pre 'dotnet test --no-build')"; ok "no dll → deny" 'echo "$OUTPUT"|jq -e ".hookSpecificOutput.permissionDecision==\"deny\"">/dev/null' +touch "$SB/src/Svc/A.cs"; sleep 0.05; : > "$DLL" +run "$(pre 'dotnet test --no-build')"; ok "fresh build → allow" '[ -z "$OUTPUT" ]' +sleep 0.05; touch "$SB/src/Svc/A.cs" +run "$(pre 'dotnet test --no-build')"; ok "stale source → deny" 'echo "$OUTPUT"|jq -e ".hookSpecificOutput.permissionDecision==\"deny\"">/dev/null' +run "$(pre 'cd /repo && dotnet test --no-build')"; ok "prefix cd&& stale → deny" 'echo "$OUTPUT"|jq -e ".hookSpecificOutput.permissionDecision==\"deny\"">/dev/null' +run "$(pre 'git commit -m "run dotnet test --no-build later"')"; ok "commit msg mention → allow" '[ -z "$OUTPUT" ]' +# regression: --no-build in a chained command must NOT deny a plain `dotnet test` (fresh build) +sleep 0.05; : > "$DLL" +run "$(pre 'dotnet test --filter X && echo try --no-build next')"; ok "--no-build in chained echo → allow" '[ -z "$OUTPUT" ]' +# a generated obj/ source newer than the DLL must NOT deny (only real *.cs under src/ count) +sleep 0.05; mkdir -p "$SB/src/Svc/obj"; touch "$SB/src/Svc/obj/Gen.cs" +run "$(pre 'dotnet test --no-build')"; ok "newer obj/*.cs ignored → allow" '[ -z "$OUTPUT" ]' + +echo "fail-open / override:" +run ''; ok "empty stdin → allow" '[ "$RC" = 0 ] && [ -z "$OUTPUT" ]' +OUTPUT="$(printf '%s' "$(pre 'dotnet test --no-build')" | NETPACE_SKIP_GREEN_GATE=1 "$HOOK" 2>"$SB/err")"; RC=$? +ok "NETPACE_SKIP_GREEN_GATE=1 → no-op + warns" '[ -z "$OUTPUT" ] && grep -q BYPASSED "$SB/err"' + +echo "" +echo "RESULT: $pass passed, $fail failed" +[ "$fail" = 0 ] diff --git a/.claude/hooks/no-skipped-tests.sh b/.claude/hooks/no-skipped-tests.sh index 3df405f..c3f3f4d 100755 --- a/.claude/hooks/no-skipped-tests.sh +++ b/.claude/hooks/no-skipped-tests.sh @@ -1,12 +1,16 @@ #!/usr/bin/env bash # -# no-skipped-tests.sh — static gate banning the entire skipped-test family (Constitution Principle X). +# no-skipped-tests.sh — static gate banning the entire skipped-test family. # # A skipped test reports "green" while checking nothing, so it is silent non-coverage. # This gate blocks any `git commit` while a banned construct exists anywhere under src/. # Legitimate needs are met without skips: fail loudly on a missing dependency, exclude # destructive opt-in suites by [Trait("Category", …)], or document a genuinely untestable -# branch with a comment at the site. +# branch with an explanatory comment at the site. +# +# This gate covers the skip FAMILY only (the constructs in BANNED_RE below). The adjacent +# non-coverage patterns it does NOT catch — NotImplementedException test stubs and +# Decision=Pending placeholder traits — are out of scope for this gate. # # Two modes: # --check Scan the tree and exit 1 if any banned construct is found (for CI / manual). @@ -14,58 +18,86 @@ # that would carry a banned construct. # # For a commit it is gating, the gate fails CLOSED: if it cannot locate src/, cannot scan, or -# cannot parse its hook payload (e.g. jq missing), it blocks rather than silently allowing — a +# cannot parse its hook payload (jq/grep missing), it blocks rather than silently allowing — a # gate that fails open is the same silent non-coverage it exists to prevent. These fail-closed # guards apply ONLY after the command is classified as a `git commit`; every non-commit Bash call -# is waved straight through first, so a missing dependency never blocks unrelated commands. +# is waved straight through first, so a missing dependency never blocks unrelated commands. The +# classification itself therefore uses shell builtins only — see the pre-filter below, where +# shelling out would reintroduce exactly the fail-open hole these guards exist to close. # # Escape hatch (harness-safety rule: override-first, then tighten): set NETPACE_ALLOW_SKIPS=1 # to bypass. It announces itself on every invocation so it can never be silently in effect. -# Delete this block once the gate is trusted to make the ban absolute. +# It applies to HOOK MODE ONLY — `--check` ignores it, so an audit can never be silenced by a +# stray env var. Delete this block once the gate is trusted to make the ban absolute. # -# Wired into .claude/settings.json as a PreToolUse(git commit) hook, so the gate arms -# automatically on every `git commit`. Constitution Principle X makes the ban itself binding. +# Wired into .claude/settings.json as a PreToolUse(Bash) hook, so the gate arms +# automatically on every `git commit`. Constitution §X makes the ban itself binding. set -uo pipefail -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +# Root to scan. Honours CLAUDE_PROJECT_DIR (as the harness always sets it, and as the other +# filesystem-reading gates green-gate.sh / traceability-gate.sh already do), falling back to the +# script's own location. Matching traceability-gate.sh here keeps the FS-reading gates consistent +# and lets a test matrix point this one at a sandbox by env var instead of relocating it. +REPO_ROOT="${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}" # Banned constructs: xUnit runtime skips (Skip.If/IfNot/Always/Unless, Assert.Skip), the -# SkippableFact/Theory family, the static Skip= attribute arg (string OR identifier, e.g. -# [Fact(Skip = GateReason)]), and SkipException. -BANNED_RE='Skip\.(If|IfNot|Always|Unless)|Assert\.Skip|Skippable(Fact|Theory)|Skip[[:space:]]*=|SkipException' +# SkippableFact/Theory family, SkipException, and the attribute-argument family — Skip=, +# plus xUnit v3's conditional SkipUnless=/SkipWhen=. The argument alternative deliberately +# matches a string OR an identifier ([Fact(Skip = GateReason)]), which is why it is a bare +# `Skip…=` rather than something anchored to a quote: breadth here over-blocks (loud, fixable +# by a rename) where narrowness would under-block (silent non-coverage — the thing §X exists +# to prevent). Note `If` is a prefix of `IfNot`, so the IfNot alternative never decides a match; +# it is kept for readability of intent. +BANNED_RE='Skip\.(If|IfNot|Always|Unless)|Assert\.Skip|Skippable(Fact|Theory)|Skip(Unless|When)?[[:space:]]*=|SkipException' # Scan src/ for banned constructs. Returns 0 with matching lines on stdout, 0 with no # output when clean, and 2 when the scan itself errored (permission, unreadable tree) so -# the caller can fail closed instead of reading an error as "clean". +# the caller can fail closed instead of reading an error as "clean". On error, grep's own +# stderr is echoed so the caller can name the offending path rather than reporting a bare +# "could not scan" — one unreadable file blocks every commit, so the diagnostic must point +# at it. +# +# bin/ and obj/ are excluded by grep during traversal (--exclude-dir), NOT by filtering its +# output afterwards. An output filter tests the whole `path:lineno:content` line, so a banned +# construct whose SOURCE TEXT mentions /bin/ or /obj/ would filter itself out and commit clean +# — e.g. `Skip.If(!File.Exists("/bin/bash"), …)`, which is exactly the shape of skip §X most +# often has to stop (a runtime skip guarding a missing dependency, and dependency probes name +# absolute paths). scan() { local raw status - raw="$(grep -rnE --include='*.cs' "$BANNED_RE" "$REPO_ROOT/src")" + raw="$(grep -rnE --include='*.cs' --exclude-dir=bin --exclude-dir=obj "$BANNED_RE" "$REPO_ROOT/src" 2>"$SCAN_ERR")" status=$? # grep: 0 = match, 1 = no match, >=2 = error. if [ "$status" -ge 2 ]; then return 2 fi [ -z "$raw" ] && return 0 - printf '%s\n' "$raw" | grep -vE '/(bin|obj)/' + printf '%s\n' "$raw" return 0 } -# Override-first escape hatch — announced, never silent. -if [ "${NETPACE_ALLOW_SKIPS:-}" = "1" ]; then - echo "no-skipped-tests: WARNING — gate BYPASSED via NETPACE_ALLOW_SKIPS=1 (skip ban NOT enforced)." >&2 - exit 0 -fi +# Where scan() parks grep's stderr, so an error can be reported with the path that caused it. +SCAN_ERR="$(mktemp 2>/dev/null)" || SCAN_ERR=/dev/null +trap 'rm -f "$SCAN_ERR"' EXIT +# --check is deliberately ABOVE the override: an audit must never be silenceable by an env var. +# NETPACE_ALLOW_SKIPS=1 is a local, per-commit emergency hatch for the interactive agent loop; if it +# leaked into a CI environment it would turn this audit into a no-op that still reports green — +# silent non-coverage dressed as a pass, which is precisely what §X exists to stop. The +# Constitution scopes the override to "genuine emergencies, not routine bypass"; an unattended +# runner is neither. if [ "${1:-}" = "--check" ]; then # Fail closed if we cannot locate the tree to scan (a wrong REPO_ROOT must not read as clean). if [ ! -d "$REPO_ROOT/src" ]; then echo "no-skipped-tests: cannot locate src/ under '$REPO_ROOT' — failing closed (exit 2)." >&2 exit 2 fi + command -v grep >/dev/null 2>&1 || { echo "no-skipped-tests: grep unavailable — failing closed (exit 2)." >&2; exit 2; } hits="$(scan)"; rc=$? if [ "$rc" -ge 2 ]; then echo "no-skipped-tests: scan of '$REPO_ROOT/src' failed — failing closed (exit 2)." >&2 + [ -s "$SCAN_ERR" ] && sed 's/^/ /' "$SCAN_ERR" >&2 exit 2 fi if [ -n "$hits" ]; then @@ -76,6 +108,12 @@ if [ "${1:-}" = "--check" ]; then exit 0 fi +# Override-first escape hatch — announced, never silent. Hook mode only (see --check above). +if [ "${NETPACE_ALLOW_SKIPS:-}" = "1" ]; then + echo "no-skipped-tests: WARNING — gate BYPASSED via NETPACE_ALLOW_SKIPS=1 (skip ban NOT enforced)." >&2 + exit 0 +fi + # Hook mode: read the PreToolUse JSON on stdin and gate ONLY `git commit`. # # Classify the command as a commit BEFORE requiring jq or the src/ tree. This hook fires on @@ -85,18 +123,30 @@ fi # we know this is a commit; a non-commit Bash call is always waved straight through. input="$(cat)" -# Cheap, dependency-free pre-filter: if the raw payload cannot even contain a `git commit`, there -# is nothing to gate — return before touching jq. This deliberately over-matches (any payload -# mentioning git…commit, including a non-Bash tool editing such text); the precise jq parse below +# Cheap pre-filter: if the raw payload cannot even contain a `git commit`, there is nothing to +# gate — return before touching any external tool. This deliberately over-matches (any payload +# mentioning git…commit, including a non-Bash tool editing such text); the precise parse below # narrows it. It only ever WIDENS what reaches the real check, so no commit can slip past here. -printf '%s' "$input" | grep -qE '\bgit\b.*\bcommit\b' || exit 0 +# +# It uses BASH PATTERN MATCHING, not grep, and that is load-bearing. This test is the one thing +# that must run before any dependency check — so if it shelled out, a `grep` that was missing or +# erroring would make `… || exit 0` wave a real commit straight through, fail-OPEN, silently. +# `[[ ]]` is a shell builtin: it cannot be absent and cannot fail. (grep is still needed for the +# scan, but only AFTER we know this is a commit — where a hard fail-closed check is safe and does +# not deadlock unrelated commands on a box without it.) +[[ "$input" == *git* && "$input" == *commit* ]] || exit 0 -# jq is required to parse the payload precisely; for a would-be commit a missing jq must fail -# closed, not wave it through. +# From here we know the payload mentions git+commit, so it is a would-be commit and every +# uncertainty below must fail CLOSED. jq parses the payload; grep performs the scan. A missing +# either one must block rather than wave through. if ! command -v jq >/dev/null 2>&1; then echo "BLOCKED: no-skipped-tests requires jq to parse the hook payload, but jq is not installed — failing closed." >&2 exit 2 fi +if ! command -v grep >/dev/null 2>&1; then + echo "BLOCKED: no-skipped-tests requires grep to scan src/, but grep is not available — failing closed." >&2 + exit 2 +fi tool="$(printf '%s' "$input" | jq -r '.tool_name // empty')" || { echo "BLOCKED: no-skipped-tests could not parse the hook payload as JSON — failing closed." >&2 @@ -104,7 +154,21 @@ tool="$(printf '%s' "$input" | jq -r '.tool_name // empty')" || { } cmd="$(printf '%s' "$input" | jq -r '.tool_input.command // empty')" -if [ "$tool" != "Bash" ] || ! printf '%s' "$cmd" | grep -qE '\bgit\b.*\bcommit\b'; then +# Flatten line continuations before matching. `grep -qE` is line-oriented, so a command split +# across lines — `git \ commit -m x` — has no single line carrying both tokens, and the +# check below would wave a real commit through. (A bare newline is a genuine command separator +# and is left alone: `git` on one line and `commit` on the next are two unrelated commands, and +# grep -q already matches a `git commit` sitting on any single line of a multi-line script.) +cmd_flat="${cmd//\\$'\n'/ }" + +# Deliberately NOT narrowed to command-word position (issue #91 H7). A read-only command that +# merely mentions the words — `grep -r "git commit" docs/` — is gated too, and that over-block is +# accepted: it can only bite when src/ ALREADY holds a banned construct, i.e. when §X is +# already violated and must be fixed regardless; in a clean tree it never fires. Parsing shell to +# find the command-word (as stack-guard.sh does for its own, narrower job) would trade this +# harmless over-block for the failure that actually matters — a parser gap missing a real commit +# and letting a skip land silently. +if [ "$tool" != "Bash" ] || ! printf '%s' "$cmd_flat" | grep -qE '\bgit\b.*\bcommit\b'; then exit 0 fi @@ -116,14 +180,20 @@ fi hits="$(scan)"; rc=$? if [ "$rc" -ge 2 ]; then - echo "BLOCKED: no-skipped-tests could not scan '$REPO_ROOT/src' — failing closed (exit 2)." >&2 + { + echo "BLOCKED: no-skipped-tests could not scan '$REPO_ROOT/src' — failing closed (exit 2)." + # Name the path that failed. One unreadable file blocks EVERY commit, so a bare "could not + # scan" leaves the operator with a repo-wide lockout and nothing to act on. + [ -s "$SCAN_ERR" ] && sed 's/^/ /' "$SCAN_ERR" + echo "Fix the unreadable path above (a stray mode/owner is the usual cause), then retry." + } >&2 exit 2 fi if [ -n "$hits" ]; then { echo "BLOCKED: commit would introduce/retain banned skipped-test constructs." - echo "Prohibited: the skip family (Skip.If/IfNot/Always/Unless, Assert.Skip, [Fact/Theory(Skip=…)], [SkippableFact/Theory], SkipException) — see constitution Principle X." - echo "Fix: make the test fail loudly, exclude destructive suites by [Trait(\"Category\", …)], or document it with a comment at the site." + echo "Prohibited: the skip family (Skip.If/IfNot/Always/Unless, Assert.Skip, [Fact/Theory(Skip=…)], [SkippableFact/Theory], SkipException) — see Constitution §X." + echo "Fix: make the test fail loudly, exclude destructive suites by [Trait(\"Category\", …)], or document the untestable branch with a comment at the site." echo "Offending lines:" echo "$hits" echo "(Emergency override only: NETPACE_ALLOW_SKIPS=1)" diff --git a/.claude/hooks/no-skipped-tests.tests.sh b/.claude/hooks/no-skipped-tests.tests.sh new file mode 100755 index 0000000..0cfc1ed --- /dev/null +++ b/.claude/hooks/no-skipped-tests.tests.sh @@ -0,0 +1,228 @@ +#!/usr/bin/env bash +# +# no-skipped-tests.tests.sh — standalone matrix for no-skipped-tests.sh (issue #91 B12). +# +# The gate reads the filesystem (it greps `$REPO_ROOT/src`), so a synthetic payload alone cannot +# drive it. Like its fellow filesystem-reading gates green-gate.sh and traceability-gate.sh, it +# honours `CLAUDE_PROJECT_DIR`, so every case points it at a throwaway sandbox tree and asserts on +# the real, unmodified gate. No fixture ever touches the repo's own src/. +# +# TWO THINGS THIS FILE IS CAREFUL ABOUT, both learned the hard way: +# +# 1. A case must fail when the gate breaks. Assertions therefore distinguish DETECTION +# (blocked_detect) from FAIL-CLOSED (blocked_closed) by inspecting the message, not just the +# exit code — both exit 2, so a bare `rc == 2` check passes against a gate stubbed to +# `exit 2` on line 1, and a regression turning real detection into a spurious fail-closed +# would go unseen. +# 2. The gate's exit code must be the one measured. The payload goes in by HERESTRING, never +# `jq … | hook`: under `pipefail` a pipeline reports the rightmost non-zero status, so a jq +# hiccup would be read as the gate's verdict. +# +# Exits non-zero on any failure. Run after any edit to the gate. +# +# Usage: .claude/hooks/no-skipped-tests.tests.sh + +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HERE/no-skipped-tests.sh" + +# One parent-scoped root holds every sandbox, so cleanup cannot be defeated by subshells (a +# `TMPDIRS+=(…)` inside `$(sandbox …)` mutates a subshell's copy and leaks every fixture). +# u+rwX first: the scan-error case deliberately leaves an unreadable file behind. +ROOT="$(mktemp -d)" +cleanup() { chmod -R u+rwX "$ROOT" 2>/dev/null; rm -rf "$ROOT"; } +trap cleanup EXIT + +pass=0; fail=0 +ok() { if eval "$2"; then echo " ok $1"; pass=$((pass+1)); else echo " FAIL $1 -- rc=$RC out:[$(printf '%s' "$OUTPUT" | head -1)]"; fail=$((fail+1)); fi; } + +# sandbox CONTENT [RELPATH] — build a throwaway project root whose src/ holds CONTENT; echo it. +# Returns empty on any setup failure; run_* below turn that into a loud FAIL rather than letting +# an empty CLAUDE_PROJECT_DIR silently send the gate at the real repo. +sandbox() { + local t rel; t="$(mktemp -d -p "$ROOT")" || return 1 + rel="${2:-src/Sample.Tests/SampleTests.cs}" + mkdir -p "$t/$(dirname "$rel")" || return 1 + printf '%s\n' "$1" > "$t/$rel" || return 1 + [ -s "$t/$rel" ] || return 1 # a partially-failed setup (ENOSPC/quota) must not read as "allow" + printf '%s' "$t" +} + +# empty_sandbox — a project root with NO src/ tree (fail-closed case). +empty_sandbox() { local t; t="$(mktemp -d -p "$ROOT")" || return 1; printf '%s' "$t"; } + +# _drive SANDBOX PAYLOAD [ENV…] — run the gate on PAYLOAD with CLAUDE_PROJECT_DIR=SANDBOX. +_drive() { + local sb="$1" payload="$2"; shift 2 + [ -n "$sb" ] && [ -d "$sb" ] || { OUTPUT="SANDBOX SETUP FAILED"; RC=99; return; } + OUTPUT="$(env CLAUDE_PROJECT_DIR="$sb" "$@" "$HOOK" <<<"$payload" 2>&1)"; RC=$? +} +# run_hook SANDBOX [CMD] — hook mode with a Bash payload (default: a git commit). +run_hook() { + local p; p="$(jq -n --arg c "${2:-git commit -m msg}" '{tool_name:"Bash",tool_input:{command:$c}}')" \ + || { OUTPUT="JQ FAILED TO BUILD PAYLOAD"; RC=98; return; } + _drive "$1" "$p" +} +# run_raw SANDBOX RAWPAYLOAD — hook mode with a caller-supplied payload (malformed, other tools). +run_raw() { _drive "$1" "$2"; } +# run_check SANDBOX [ENV…] — --check (audit) mode. +run_check() { + local sb="$1"; shift + [ -n "$sb" ] && [ -d "$sb" ] || { OUTPUT="SANDBOX SETUP FAILED"; RC=99; return; } + OUTPUT="$(env CLAUDE_PROJECT_DIR="$sb" "$@" "$HOOK" --check 2>&1)"; RC=$? +} + +allowed() { [ "$RC" = "0" ]; } +# Exit 2 covers BOTH detection and every fail-closed path, so assert on which one fired. +blocked_detect() { [ "$RC" = "2" ] && printf '%s' "$OUTPUT" | grep -q 'banned skipped-test constructs'; } +blocked_closed() { [ "$RC" = "2" ] && printf '%s' "$OUTPUT" | grep -q 'failing closed'; } + +CLEAN='public class SampleTests { [Fact] public void Works() { Assert.True(true); } }' + +echo "The skip family — a commit carrying one must be DETECTED and blocked:" +run_hook "$(sandbox 'class T { void M(){ Skip.If(cond, "why"); } }')"; ok "Skip.If" 'blocked_detect' +# `If` is a prefix of `IfNot` in the regex, so this asserts the OUTCOME; it cannot isolate the +# IfNot alternative (removing it from BANNED_RE leaves this green — by design, not by oversight). +run_hook "$(sandbox 'class T { void M(){ Skip.IfNot(cond, "why"); } }')"; ok "Skip.IfNot (via the If prefix)" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ Skip.Always("why"); } }')"; ok "Skip.Always" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ Skip.Unless(cond, "why"); } }')"; ok "Skip.Unless" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ Assert.Skip("why"); } }')"; ok "Assert.Skip" 'blocked_detect' +run_hook "$(sandbox 'class T { [SkippableFact] void M(){} }')"; ok "[SkippableFact]" 'blocked_detect' +run_hook "$(sandbox 'class T { [SkippableTheory] void M(){} }')"; ok "[SkippableTheory]" 'blocked_detect' +run_hook "$(sandbox 'class T { [Fact(Skip = "not ready")] void M(){} }')"; ok "[Fact(Skip=\"str\")]" 'blocked_detect' +run_hook "$(sandbox 'class T { [Theory(Skip="s")] void M(){} }')"; ok "[Theory(Skip=\"s\")] no-space" 'blocked_detect' +run_hook "$(sandbox 'class T { [Fact(Skip = GateReason)] void M(){} }')"; ok "[Fact(Skip=identifier)]" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ throw new SkipException("x"); } }')"; ok "SkipException" 'blocked_detect' +# xUnit v3's conditional-skip properties. The repo is on xunit 2.x, so these are latent — pinned +# now so a v3 migration cannot silently un-arm the gate against v3's most ergonomic skip. +run_hook "$(sandbox 'class T { [Fact(SkipUnless = nameof(StackIsUp))] void M(){} }')"; ok "[Fact(SkipUnless=…)] (xUnit v3)" 'blocked_detect' +run_hook "$(sandbox 'class T { [Fact(SkipWhen = nameof(IsCi))] void M(){} }')"; ok "[Fact(SkipWhen=…)] (xUnit v3)" 'blocked_detect' + +echo "" +echo "Exclusion by PATH, not by line content (#91 H1) — the escape that let skips commit clean:" +# The bin/obj exclusion must filter on the file's PATH. Filtering grep's output instead tests the +# whole `path:lineno:content` line, so a construct whose SOURCE TEXT names /bin/ or /obj/ filters +# itself out — and a dependency-probe skip naming an absolute path is the likeliest real skip +# there is, which made the hole line up exactly with what §X most needs to catch. +run_hook "$(sandbox 'class T { void M(){ Skip.If(!File.Exists("/bin/bash"), "no bash"); } }')"; ok "Skip.If with \"/bin/…\" in the string" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ Assert.Skip("see /obj/notes"); } }')"; ok "Assert.Skip with \"/obj/…\" in the string" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ Skip.If(x); } }' 'src/Sample.Tests/bin/Debug/G.cs')"; ok "real bin/ dir still excluded" 'allowed' +run_hook "$(sandbox 'class T { void M(){ Skip.If(x); } }' 'src/Sample.Tests/obj/Debug/G.cs')"; ok "real obj/ dir still excluded" 'allowed' +run_hook "$(sandbox 'class T { void M(){ Skip.If(x); } }' 'src/robin/objects/T.cs')"; ok "'robin'/'objects' NOT over-excluded" 'blocked_detect' + +echo "" +echo "Clean tree — a commit must be ALLOWED:" +run_hook "$(sandbox "$CLEAN")"; ok "ordinary test file" 'allowed' +run_hook "$(sandbox 'class T { void M(){ var p = list.Skip(2).Take(3); } }')"; ok "LINQ .Skip(2) (no '=')" 'allowed' +# Differs from a banned construct ONLY by case, so it fails if the scan ever loses -E for -iE. +run_hook "$(sandbox 'class T { void M(){ assert.skip("x"); } }')"; ok "lowercase assert.skip (regex is case-sensitive)" 'allowed' +run_hook "$(sandbox 'Do not use Assert.Skip in tests.' 'src/Sample.Tests/README.md')"; ok "banned construct in a .md (only .cs is scanned)" 'allowed' + +echo "" +echo "Non-commit Bash — waved through BEFORE any dependency or tree check:" +DIRTY='class T { void M(){ Skip.If(x); } }' +run_hook "$(sandbox "$DIRTY")" 'ls -la'; ok "ls with a dirty tree" 'allowed' +run_hook "$(sandbox "$DIRTY")" 'git status'; ok "git status (no 'commit')" 'allowed' +run_hook "$(sandbox "$DIRTY")" 'dotnet build'; ok "dotnet build" 'allowed' +run_hook "$(empty_sandbox)" 'ls'; ok "non-commit + no src/ (no deadlock)" 'allowed' + +echo "" +echo "Multi-line commands (#91 H2) — a line-oriented match must not be dodged by a continuation:" +run_hook "$(sandbox "$DIRTY")" 'git \ + commit -m x'; ok "git \\ commit" 'blocked_detect' +run_hook "$(sandbox "$DIRTY")" 'set -e +git commit -m x'; ok "commit on a later line" 'blocked_detect' +run_hook "$(sandbox "$DIRTY")" 'echo git +echo commit'; ok "'git' and 'commit' on separate lines, no commit" 'allowed' + +echo "" +echo "Non-Bash tools — the tool_name check must be what saves them:" +# Each carries a real `command` field, so ONLY tool_name can allow it. Without the command field +# these pass via the pre-filter instead, and deleting the tool_name check outright goes unnoticed. +run_raw "$(sandbox "$DIRTY")" '{"tool_name":"Edit","tool_input":{"file_path":"a.md","command":"git commit -m x","new_string":"x"}}' +ok "Edit carrying a git-commit command field" 'allowed' +run_raw "$(sandbox "$DIRTY")" '{"tool_name":"Write","tool_input":{"file_path":"a.md","command":"git commit -m x","content":"x"}}' +ok "Write carrying a git-commit command field" 'allowed' + +echo "" +echo "Fail-closed — for a would-be COMMIT, uncertainty must block, never wave through:" +run_hook "$(empty_sandbox)"; ok "commit + no src/ tree" 'blocked_closed' +run_raw "$(sandbox "$CLEAN")" 'git commit -m "not json at all"'; ok "commit-ish, unparseable payload" 'blocked_closed' +run_raw "$(sandbox "$CLEAN")" '{"tool_name":"Bash","tool_input":{"command":"git commit -m x"'; ok "commit-ish, truncated JSON" 'blocked_closed' +# An unreadable file makes grep error. Reading that as "clean" would be silent non-coverage, so it +# must block — and must name the path, since one bad file mode blocks EVERY commit repo-wide. +# NOTE: this case requires a non-root runner; root reads through mode 000 and it will fail. That +# is deliberate — conditioning a test on the environment is the skip pattern this very gate bans. +_sb="$(sandbox "$CLEAN")"; printf 'class L {}\n' > "$_sb/src/Sample.Tests/Locked.cs"; chmod 000 "$_sb/src/Sample.Tests/Locked.cs" +run_hook "$_sb"; ok "commit + unreadable file → block" 'blocked_closed' +ok "…and the message names the offending path" 'printf "%s" "$OUTPUT" | grep -q "Locked.cs"' + +echo "" +echo "Dependency bootstrap (#91 H3) — a missing tool must block a commit, never wave it through:" +# The gate's own contract says it fails closed for a commit. That held for jq but NOT for grep: +# the classifier used `grep … || exit 0`, which cannot tell "no match" from "grep is missing", so +# a broken grep allowed every commit. The classifier is now a shell builtin and grep is checked. +MINBIN="$(mktemp -d -p "$ROOT")" +for b in bash dirname cat jq mktemp sed rm; do ln -s "$(env -i bash -c "command -v $b")" "$MINBIN/$b" 2>/dev/null; done +[ -x "$MINBIN/bash" ] || { echo " FAIL MINBIN setup (bash not linked)"; fail=$((fail+1)); } +_sb="$(sandbox "$DIRTY")" +run_raw_env() { OUTPUT="$(env CLAUDE_PROJECT_DIR="$1" PATH="$2" "$HOOK" <<<"$3" 2>&1)"; RC=$?; } +run_raw_env "$_sb" "$MINBIN" '{"tool_name":"Bash","tool_input":{"command":"git commit -m x"}}' +ok "commit + no grep → block (was fail-OPEN)" 'blocked_closed' +run_raw_env "$_sb" "$MINBIN" '{"tool_name":"Bash","tool_input":{"command":"ls -la"}}' +ok "non-commit + no grep → allow (no deadlock)" 'allowed' +NOJQ="$(mktemp -d -p "$ROOT")" +for b in bash dirname cat grep mktemp sed rm; do ln -s "$(env -i bash -c "command -v $b")" "$NOJQ/$b" 2>/dev/null; done +run_raw_env "$_sb" "$NOJQ" '{"tool_name":"Bash","tool_input":{"command":"git commit -m x"}}' +ok "commit + no jq → block" 'blocked_closed' +run_raw_env "$_sb" "$NOJQ" '{"tool_name":"Bash","tool_input":{"command":"ls"}}' +ok "non-commit + no jq → allow (no deadlock)" 'allowed' + +echo "" +echo "The override — hook mode only, and never silent:" +_sb="$(sandbox "$DIRTY")" +run_hook_env() { OUTPUT="$(env CLAUDE_PROJECT_DIR="$1" NETPACE_ALLOW_SKIPS=1 "$HOOK" <<<'{"tool_name":"Bash","tool_input":{"command":"git commit -m x"}}' 2>&1)"; RC=$?; } +run_hook_env "$_sb"; ok "NETPACE_ALLOW_SKIPS=1 + dirty tree → allow" 'allowed' +ok "…and announces itself on stderr" 'printf "%s" "$OUTPUT" | grep -q "BYPASSED"' +# An audit must not be silenceable by a stray env var, or a leaked override turns CI into a +# green-reporting no-op (#91 H6). +run_check "$_sb" NETPACE_ALLOW_SKIPS=1; ok "--check IGNORES the override → still 1" '[ "$RC" = "1" ]' + +echo "" +echo "--check mode — distinct exit codes from hook mode (1 = found, not 2):" +run_check "$(sandbox "$CLEAN")"; ok "--check clean → 0" 'allowed' +run_check "$(sandbox 'class T { void M(){ Assert.Skip("x"); } }')"; ok "--check finds a skip → 1" '[ "$RC" = "1" ]' +run_check "$(empty_sandbox)"; ok "--check no src/ → 2 (fail closed)" 'blocked_closed' + +echo "" +echo "Root resolution (#91 H8) — CLAUDE_PROJECT_DIR must win over the script's own location:" +# Every case above depends on this. If the gate ever reverts to a purely location-derived root, +# each sandbox would silently scan the REAL repo instead — green while verifying nothing. +run_hook "$(sandbox "$DIRTY")"; ok "sandbox skip is seen (env root honoured, not \$HERE/../..)" 'blocked_detect' +run_hook "$(sandbox "$CLEAN")"; ok "clean sandbox is clean (real repo's src/ not scanned)" 'allowed' + +echo "" +echo "KNOWN OVER-BLOCKS — deliberate, pinned so a future 'tighten this' sees the trade:" +# BANNED_RE carries a bare `Skip…=` to catch [Fact(Skip = GateReason)], where the reason is an +# identifier. That breadth also matches ordinary C# assigning to something named Skip. Over-blocking +# is the safe failure — loud, and fixed by a rename — where a false pass is silent non-coverage, +# which is what §X exists to prevent. +run_hook "$(sandbox 'class T { void M(){ var q = new Page { Skip = 10, Take = 5 }; } }')" +ok "pagination initializer { Skip = 10 }" 'blocked_detect' +run_hook "$(sandbox 'class T { void M(){ int Skip = 0; } }')" +ok "local variable named Skip" 'blocked_detect' +# #91 H7, assessed and accepted: classification is not narrowed to command-word position. A +# read-only command merely MENTIONING the words is gated — but only when src/ already holds a +# banned construct, i.e. when §X is already violated and must be fixed anyway; in a clean tree +# it never fires. Parsing shell to find the command-word would trade this harmless over-block for +# the failure that matters: a parser gap missing a real commit and letting a skip land silently. +run_hook "$(sandbox "$DIRTY")" 'grep -r "git commit" docs/' +ok "read-only grep mentioning 'git commit'" 'blocked_detect' +run_hook "$(sandbox "$CLEAN")" 'grep -r "git commit" docs/' +ok "…but never on a clean tree" 'allowed' + +echo "" +echo "----------------------------------------" +echo "passed: $pass failed: $fail" +[ "$fail" -eq 0 ] || exit 1 diff --git a/.claude/hooks/traceability-gate.sh b/.claude/hooks/traceability-gate.sh new file mode 100755 index 0000000..ac46460 --- /dev/null +++ b/.claude/hooks/traceability-gate.sh @@ -0,0 +1,228 @@ +#!/usr/bin/env bash +# +# traceability-gate.sh — deterministic AC↔marker traceability gate. +# +# Constitution §VIII makes the scenario LABEL the traceability key that links +# acceptance criteria → test scenarios → test code: +# +# spec.md **Scenario: [name]** (the acceptance-criteria label) +# ↓ exact bijection +# test-plan.md #### Scenario: [name] (the planned test scenario) +# ↓ exact coverage +# test code // SCENARIO: [name] (the implemented test marker) +# +# This gate enforces the two EXACT-MATCH edges of that chain deterministically. The +# JUDGMENT checks (fuzzy name match, mock self-satisfaction, trivially-passing bodies, +# "undocumented test" detection) stay in /speckit.testchecklist — this gate only does +# what a machine can decide without inference. Names match character-for-character after +# trimming leading/trailing whitespace: case, punctuation and internal spacing all count +# (.claude/commands/speckit.testplan.md). +# +# Edge 1 spec ⟷ test-plan (bijection): every **Scenario: X** label in spec.md has +# exactly one matching #### Scenario: X header in test-plan.md, and vice versa. +# Both are authored docs in the SAME active spec dir, so a mismatch is always a +# real §VIII violation — there is no false-positive path. +# Edge 2 test-plan → code (coverage, DIRECTIONAL): every #### Scenario: X header has +# at least one matching // SCENARIO: X marker somewhere under src/. The reverse +# (a marker with no plan scenario) is NOT enforced here — the src/ tree +# accumulates markers from already-merged features whose specs are deleted, so a +# global marker→plan bijection would false-block on a clean repo. "Undocumented +# test" is therefore a judgment call left to /speckit.testchecklist. +# +# SCOPE — active specs only. The gate reads `specs/*/spec.md`. Merged features have their +# specs deleted (they leave only drifted markers behind), so a repo with no active feature — +# the steady state — is a clean no-op. Nothing outside an in-flight spec dir is ever judged. +# +# "No skip markers" is NOT re-implemented here: the whole skip +# family is already gated at commit time by no-skipped-tests.sh (Constitution §X), +# which is strictly stronger (it bans every skip anywhere under src/, traced or not). +# Duplicating it would only create two regexes to drift apart. +# +# Two modes: +# --check [specdir] Scan and print a report; exit 1 on any violation, 0 when clean or +# when there is no active spec. With a specdir argument, check only +# that dir; otherwise every specs/*/ that has a spec.md. For CI / the +# human's pre-wire verification. +# (default) Stop hook: read hook JSON on stdin and BLOCK ending the turn (once — +# loop-guarded via stop_hook_active, exactly like the green completion +# gate) when an active spec's chain is broken. +# +# DESIGN RULE: fail OPEN. Missing tool, unparseable input, absent specs/ dir, a spec still +# being authored (no test-plan scenarios yet), or any internal error → exit 0 (no objection). +# The gate acts only on the narrow, high-confidence broken-chain case. As with the other +# harness hooks, a guard that falsely blocks is worse than no guard — doubly so for a harness +# we edit with itself, where a false block can lock out the tools that would fix it. +# +# Escape hatch (harness-safety: override-first, then tighten): NETPACE_SKIP_TRACEABILITY_GATE=1 +# makes the whole script a no-op, announced on stderr so it can never be silently in effect. +# +# Wired into .claude/settings.json as the Stop hook. (It once ran alongside the green-gate +# completion gate; that gate was retired in issue #122 when test-green enforcement moved to +# /ship, so this is now the sole Stop hook.) Loop-guarded, so it nudges at most once per turn +# and can never hard-lock. Verify any edit with traceability-gate.tests.sh and --check first; +# see .claude/hooks/README.md. + +set -uo pipefail + +# --- override-first escape hatch — announced, never silent ------------------------ +if [ "${NETPACE_SKIP_TRACEABILITY_GATE:-}" = "1" ]; then + echo "traceability-gate: WARNING — gate BYPASSED via NETPACE_SKIP_TRACEABILITY_GATE=1 (traceability NOT enforced)." >&2 + exit 0 +fi + +ROOT="${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}" + +# Trim leading/trailing whitespace only; internal spacing is load-bearing (part of the key). +trim() { sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//'; } + +# Extract the scenario NAMES from each artifact, one per line, in file order. +# A name never contains an asterisk (spec) or a newline, so the [^*] / line-based captures +# are exact. grep exits 1 on no match under `set -o pipefail`; `|| true` keeps that benign. +spec_labels() { grep -oE '\*\*Scenario:[^*]+\*\*' "$1" 2>/dev/null | sed -E 's/^\*\*Scenario:[[:space:]]*//; s/\*\*$//' | trim || true; } +plan_scenarios() { grep -E '^####[[:space:]]+Scenario:' "$1" 2>/dev/null | sed -E 's/^####[[:space:]]+Scenario:[[:space:]]*//' | trim || true; } + +# Every // SCENARIO: marker anywhere under src/, generated obj/bin excluded so a built copy +# of a test source can't double-count. The exclusion MUST prune during the directory walk +# (--exclude-dir): -h -o emit only the marker text with no path, so a post-hoc `grep -v obj` +# on that output would silently match nothing — a dead filter (the bug this replaced). +code_markers() { + [ -d "$ROOT/src" ] || return 0 + grep -rhoE '//[[:space:]]*SCENARIO:.*$' "$ROOT/src" --include='*.cs' \ + --exclude-dir=obj --exclude-dir=bin 2>/dev/null \ + | sed -E 's|^//[[:space:]]*SCENARIO:[[:space:]]*||' | trim || true +} + +# Print lines present in $1 but absent from $2 (exact whole-line set difference). Empty +# inputs are handled (comm needs sorted streams; process substitution keeps it in-memory). +minus() { comm -23 <(printf '%s\n' "$1" | sort -u) <(printf '%s\n' "$2" | sort -u) | sed '/^$/d'; } +# Count non-empty lines in a newline list. +count() { printf '%s\n' "$1" | sed '/^$/d' | grep -c '' ; } +# Names appearing more than once in a newline list. §VIII requires the label to be a UNIQUE +# traceability key, so a repeat is a violation even when both sides are set-equal — and the +# set-difference in minus() dedups, so duplicates must be caught explicitly here. +dupes() { printf '%s\n' "$1" | sed '/^$/d' | sort | uniq -d; } + +# Check one active spec dir. Echoes a human-readable violation report to stdout and returns +# 1 if the chain is broken, 0 if clean or not-yet-enforceable (pre-plan / pre-implementation). +check_spec() { + local dir="$1" rel="${1#"$ROOT"/}" spec plan labels scenarios markers + spec="$dir/spec.md"; plan="$dir/test-plan.md" + [ -f "$spec" ] || return 0 # not a spec dir → nothing to judge + + labels="$(spec_labels "$spec")" + # No test-plan, or a test-plan with no scenarios yet → the contract does not exist yet. + # Fail open: the spec is still being authored/planned (avoids nagging before the plan lands). + [ -f "$plan" ] || return 0 + scenarios="$(plan_scenarios "$plan")" + [ "$(count "$scenarios")" -eq 0 ] && return 0 + + local rc=0 out="" miss_plan miss_spec dup_labels dup_scen + + # Edge 1: spec ⟷ test-plan bijection. A repeated name is an ambiguous key, so duplicates on + # either side are flagged first (minus() below dedups and would miss them). + dup_labels="$(dupes "$labels")" + dup_scen="$(dupes "$scenarios")" + if [ -n "$dup_labels" ]; then + out+=" duplicate '**Scenario:**' labels in spec.md (each name must be unique — §VIII key):"$'\n' + out+="$(printf '%s\n' "$dup_labels" | sed 's/^/ - /')"$'\n'; rc=1 + fi + if [ -n "$dup_scen" ]; then + out+=" duplicate '#### Scenario:' headers in test-plan.md (each name must be unique — §VIII key):"$'\n' + out+="$(printf '%s\n' "$dup_scen" | sed 's/^/ - /')"$'\n'; rc=1 + fi + miss_plan="$(minus "$labels" "$scenarios")" # spec label with no plan scenario + miss_spec="$(minus "$scenarios" "$labels")" # plan scenario with no spec label + if [ -n "$miss_plan" ]; then + out+=" spec.md labels with no matching test-plan.md '#### Scenario:' header:"$'\n' + out+="$(printf '%s\n' "$miss_plan" | sed 's/^/ - /')"$'\n'; rc=1 + fi + if [ -n "$miss_spec" ]; then + out+=" test-plan.md scenarios with no matching spec.md '**Scenario:**' label:"$'\n' + out+="$(printf '%s\n' "$miss_spec" | sed 's/^/ - /')"$'\n'; rc=1 + fi + + # Edge 2: test-plan → code coverage. Enforced only once implementation has STARTED for this + # spec — i.e. at least one of its scenarios already has a marker. Zero matches → treated as + # pre-implementation (tests not written) → skip so authoring isn't nagged. + # + # The "started" signal is a heuristic over the GLOBAL marker set (markers cannot be scoped to + # a spec — see the header). Two BOUNDED consequences follow, both accepted: + # - false NEGATIVE: if EVERY marker is mistyped (total drift, zero exact matches) the spec + # reads as pre-implementation and the gap is missed. Partial drift is still caught (one + # correct marker arms the check); /speckit.testchecklist's fuzzy pass backstops total drift. + # - false BLOCK: a leftover marker from a merged feature whose name equals one of this spec's + # scenarios can arm the check early and nag about the spec's other unwritten scenarios. + # Bounded to a single loop-guarded Stop nudge (never a lock-out) plus the override — the + # accepted cost of an unscoped, deterministic scan. + markers="$(code_markers)" + local covered + covered="$(comm -12 <(printf '%s\n' "$scenarios" | sort -u) <(printf '%s\n' "$markers" | sort -u) | sed '/^$/d')" + if [ "$(count "$covered")" -gt 0 ]; then + local uncovered + uncovered="$(minus "$scenarios" "$markers")" + if [ -n "$uncovered" ]; then + out+=" test-plan.md scenarios with no matching '// SCENARIO:' marker in src/:"$'\n' + out+="$(printf '%s\n' "$uncovered" | sed 's/^/ - /')"$'\n'; rc=1 + fi + fi + + if [ "$rc" -ne 0 ]; then + printf 'Traceability chain broken in %s:\n%s' "$rel" "$out" + fi + return "$rc" +} + +# Enumerate active spec dirs (those containing spec.md). Steady state: none. +active_specs() { + local d + for d in "$ROOT"/specs/*/; do + [ -f "${d}spec.md" ] && printf '%s\n' "${d%/}" + done 2>/dev/null +} + +# --- --check CLI mode ------------------------------------------------------------- +if [ "${1:-}" = "--check" ]; then + target="${2:-}" + report=""; violations=0 + if [ -n "$target" ]; then + dirs="${target%/}" + else + dirs="$(active_specs)" + fi + if [ -z "$dirs" ]; then + echo "traceability-gate: no active spec dir (specs/*/spec.md) — nothing to check." + exit 0 + fi + while IFS= read -r d; do + [ -n "$d" ] || continue + if r="$(check_spec "$d")"; then :; else violations=1; report+="$r"$'\n'; fi + done <<< "$dirs" + if [ "$violations" -ne 0 ]; then + printf '%s' "$report" >&2 + echo "traceability-gate: FAIL — fix the mismatches above (Constitution §VIII)." >&2 + exit 1 + fi + echo "traceability-gate: OK — every active spec's scenario chain matches." + exit 0 +fi + +# --- Stop hook mode --------------------------------------------------------------- +command -v jq >/dev/null 2>&1 || exit 0 # fail open: cannot parse payload +INPUT="$(cat 2>/dev/null)" || exit 0 +[ -n "$INPUT" ] || exit 0 +EVENT="$(printf '%s' "$INPUT" | jq -r '.hook_event_name // empty' 2>/dev/null)" +[ "$EVENT" = "Stop" ] || exit 0 +# Loop guard: block at most once per turn so the gate can nudge but never hard-lock. +[ "$(printf '%s' "$INPUT" | jq -r '.stop_hook_active // false' 2>/dev/null)" = "true" ] && exit 0 + +report=""; violations=0 +while IFS= read -r d; do + [ -n "$d" ] || continue + if r="$(check_spec "$d")"; then :; else violations=1; report+="$r"$'\n'; fi +done <<< "$(active_specs)" +[ "$violations" -eq 0 ] && exit 0 + +reason="$report +Fix the traceability chain before finishing: every spec.md '**Scenario:**' label, its test-plan.md '#### Scenario:' header, and its test '// SCENARIO:' marker must match character-for-character (Constitution §VIII). Run '.claude/hooks/traceability-gate.sh --check' to re-verify, and '/speckit.testchecklist' for the judgment checks. If you are stopping deliberately mid-implementation, say so plainly rather than reporting the chain as complete. (Emergency override only: NETPACE_SKIP_TRACEABILITY_GATE=1)" +jq -n --arg r "$reason" '{decision:"block",reason:$r}' +exit 0 diff --git a/.claude/hooks/traceability-gate.tests.sh b/.claude/hooks/traceability-gate.tests.sh new file mode 100755 index 0000000..e8525d8 --- /dev/null +++ b/.claude/hooks/traceability-gate.tests.sh @@ -0,0 +1,172 @@ +#!/usr/bin/env bash +# +# traceability-gate.tests.sh — standalone synthetic-fixture matrix for traceability-gate.sh. +# +# The gate reads spec.md / test-plan.md / test *.cs off disk and (in hook mode) dispatches on +# hook_event_name, so every branch is provable by building throwaway spec dirs under a +# throwaway CLAUDE_PROJECT_DIR and piping synthetic hook JSON — no dev stack, no real spec +# required. Exits non-zero on any failure. Run after any edit to the gate. +# +# Usage: .claude/hooks/traceability-gate.tests.sh + +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HERE/traceability-gate.sh" + +SB="$(mktemp -d)" +trap 'rm -rf "$SB"' EXIT +export CLAUDE_PROJECT_DIR="$SB" + +pass=0; fail=0 +ok() { if eval "$2"; then echo " ok $1"; pass=$((pass+1)); else echo " FAIL $1 -- got:[$OUTPUT] rc=$RC"; fail=$((fail+1)); fi; } + +# --- fixture builders ------------------------------------------------------------- +reset_repo() { rm -rf "$SB/specs" "$SB/src"; mkdir -p "$SB/src"; } +# make_spec DIR "Label A" "Label B" ... — writes specs/DIR/spec.md with **Scenario:** labels. +make_spec() { + local d="$SB/specs/$1"; shift; mkdir -p "$d" + { echo "**Tier**: Production"; echo; echo "### User Story 1"; echo "**Acceptance Scenarios**:"; echo + local i=1; for s in "$@"; do echo "$i. **Scenario: $s**"; echo " **Given** x, **When** y, **Then** z"; i=$((i+1)); done + } > "$d/spec.md" +} +# make_plan DIR "Scenario A" ... — writes test-plan.md with #### Scenario: headers. +make_plan() { + local d="$SB/specs/$1"; shift; mkdir -p "$d" + { echo "### User Story 1"; echo; for s in "$@"; do echo "#### Scenario: $s"; echo "AAA ..."; echo; done; } > "$d/test-plan.md" +} +# make_test FILE "Scenario A" ... — writes src/FILE with // SCENARIO: markers. +make_test() { + local f="$SB/src/$1"; mkdir -p "$(dirname "$f")" + { for s in "$@"; do [ "$s" = "$1" ] && continue; echo " // SCENARIO: $s"; echo " [Fact] public void T() { Assert.True(cond); }"; done; } > "$f" +} +run_check() { OUTPUT="$("$HOOK" --check "$@" 2>&1)"; RC=$?; } +stop_json() { printf '{"hook_event_name":"Stop","stop_hook_active":%s}' "${1:-false}"; } +run_stop() { OUTPUT="$(printf '%s' "$(stop_json "${1:-false}")" | "$HOOK" 2>/dev/null)"; RC=$?; } +blocked() { printf '%s' "$OUTPUT" | jq -e '.decision=="block"' >/dev/null 2>&1; } + +# --------------------------------------------------------------------------------- +echo "no active spec (steady state):" +reset_repo +run_check; ok "--check, no specs/ → OK exit 0" '[ "$RC" = 0 ] && printf "%s" "$OUTPUT" | grep -q "nothing to check"' +run_stop false; ok "Stop, no specs/ → allow" '[ -z "$OUTPUT" ] && [ "$RC" = 0 ]' +mkdir -p "$SB/specs" +run_stop false; ok "Stop, empty specs/ → allow" '[ -z "$OUTPUT" ]' + +echo "" +echo "edge 1 — spec ⟷ test-plan bijection:" +reset_repo +make_spec F1 "Reports download speed" "Retries on timeout" +make_plan F1 "Reports download speed" "Retries on timeout" +run_check; ok "matching labels, no markers yet → OK (pre-impl)" '[ "$RC" = 0 ]' +run_stop false; ok "matching labels, no markers → allow (pre-impl)" '[ -z "$OUTPUT" ]' + +make_plan F1 "Reports download speed" +run_check; ok "spec label missing from plan → FAIL exit 1" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "no matching test-plan"' +run_stop false; ok "spec label missing from plan → block" 'blocked' + +make_spec F1 "Reports download speed" +make_plan F1 "Reports download speed" "Retries on timeout" +run_check; ok "plan scenario missing from spec → FAIL" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "no matching spec.md"' +run_stop false; ok "plan scenario missing from spec → block (Stop)" 'blocked' + +echo "" +echo "duplicate scenario name (unique-key rule):" +reset_repo +make_spec F1 "Dup" "Dup" "Other" # duplicated label; sets still equal, so only dup fires +make_plan F1 "Dup" "Other" +run_check; ok "duplicate spec.md label → FAIL" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "duplicate .\*\*Scenario"' +make_spec F1 "Dup" "Other" +make_plan F1 "Dup" "Dup" "Other" # duplicated header +run_check; ok "duplicate test-plan.md header → FAIL" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "duplicate .#### Scenario"' + +echo "" +echo "exact-match semantics (case / whitespace):" +reset_repo +make_spec F1 "Reports download speed" +make_plan F1 "adult patron proceeds" # case drift +run_check; ok "case drift → FAIL (case-sensitive)" '[ "$RC" = 1 ]' +make_spec F1 " Reports download speed " # leading/trailing space trimmed +make_plan F1 "Reports download speed" +run_check; ok "leading/trailing space trimmed → OK" '[ "$RC" = 0 ]' + +echo "" +echo "edge 2 — test-plan → code coverage:" +reset_repo +make_spec F1 "Reports download speed" "Retries on timeout" +make_plan F1 "Reports download speed" "Retries on timeout" +make_test Speedtest.Tests/Ingest.cs "Reports download speed" # only one of two covered +run_check; ok "partial coverage → FAIL" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "no matching .// SCENARIO"' +run_stop false; ok "partial coverage → block" 'blocked' +make_test Speedtest.Tests/Ingest.cs "Reports download speed" "Retries on timeout" +run_check; ok "full coverage + bijection → OK" '[ "$RC" = 0 ]' +run_stop false; ok "full coverage + bijection → allow" '[ -z "$OUTPUT" ]' +# near-miss marker does not count as coverage +make_test Speedtest.Tests/Ingest.cs "Reports download speed" "Retries on timeout" # double space +run_check; ok "near-miss marker (double space) → FAIL" '[ "$RC" = 1 ]' +# a longer marker must not satisfy a shorter (substring) scenario — coverage is whole-line exact +reset_repo +make_spec F1 "Reports download speed" "Reports download speed in Mbps" +make_plan F1 "Reports download speed" "Reports download speed in Mbps" +make_test Speedtest.Tests/Ingest.cs "Reports download speed in Mbps" +run_check; ok "substring scenario not covered by longer marker → FAIL" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -qE "^ +- Reports download speed$"' +# names carrying regex/shell metacharacters are compared literally (not as patterns) +reset_repo +make_spec F1 "Rejects a.b [edge] c+d" +make_plan F1 "Rejects a.b [edge] c+d" +make_test Speedtest.Tests/Ingest.cs "Rejects a.b [edge] c+d" +run_check; ok "metacharacter name round-trips exact → OK" '[ "$RC" = 0 ]' + +echo "" +echo "generated obj/bin markers excluded:" +# Two-scenario fixture that DISTINGUISHES exclusion from counting: 'Real covered' has a real +# marker (so edge 2 is armed), 'Obj only' has a marker solely under obj/. If obj/ is excluded +# (correct) 'Obj only' is uncovered → FAIL naming it; if obj/ were counted (the old bug) it +# would read covered → OK. Asserting FAIL pins the exclusion. +reset_repo +make_spec F1 "Real covered" "Obj only" +make_plan F1 "Real covered" "Obj only" +make_test Speedtest.Tests/Ingest.cs "Real covered" +make_test Speedtest.Tests/obj/Debug/Ingest.g.cs "Obj only" +run_check; ok "obj-only marker excluded → its scenario uncovered → FAIL" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "Obj only"' +make_test Speedtest.Tests/Ingest.cs "Real covered" "Obj only" # add the real marker +run_check; ok "real marker added → OK" '[ "$RC" = 0 ]' + +echo "" +echo "pre-plan / authoring stages fail open:" +reset_repo +make_spec F1 "Reports download speed" # spec only, no test-plan +run_stop false; ok "spec.md but no test-plan.md → allow" '[ -z "$OUTPUT" ]' +make_plan F1 # test-plan exists but no scenarios yet +run_stop false; ok "test-plan.md with no scenarios → allow" '[ -z "$OUTPUT" ]' + +echo "" +echo "multiple active specs — one broken:" +reset_repo +make_spec Good "A"; make_plan Good "A"; make_test G.cs "A" +make_spec Bad "B"; make_plan Bad "C" +run_check; ok "one good, one broken → FAIL naming the broken dir" '[ "$RC" = 1 ] && printf "%s" "$OUTPUT" | grep -q "specs/Bad"' +run_check "$SB/specs/Good"; ok "--check scoped to good dir → OK" '[ "$RC" = 0 ]' +# --check on a target that isn't a spec dir must fail open, not error (typo'd argument path). +mkdir -p "$SB/specs/NoSpec" +run_check "$SB/specs/NoSpec"; ok "--check dir lacking spec.md → OK" '[ "$RC" = 0 ]' +run_check "$SB/specs/DoesNotExist"; ok "--check nonexistent dir → OK" '[ "$RC" = 0 ]' + +echo "" +echo "loop guard / override / fail-open:" +reset_repo +make_spec F1 "A"; make_plan F1 "B" # broken, would block +run_stop false; ok "broken chain → block" 'blocked' +run_stop true; ok "stop_hook_active=true → allow (loop guard)" '[ -z "$OUTPUT" ]' +OUTPUT="$(printf '%s' "$(stop_json false)" | NETPACE_SKIP_TRACEABILITY_GATE=1 "$HOOK" 2>"$SB/err")"; RC=$? +ok "NETPACE_SKIP_TRACEABILITY_GATE=1 → no-op + warns" '[ -z "$OUTPUT" ] && grep -q BYPASSED "$SB/err"' +OUTPUT="$(printf '' | "$HOOK" 2>/dev/null)"; RC=$? +ok "empty stdin → allow (fail open)" '[ "$RC" = 0 ] && [ -z "$OUTPUT" ]' +OUTPUT="$(printf '{not json' | "$HOOK" 2>/dev/null)"; RC=$? +ok "malformed JSON stdin → allow (fail open)" '[ "$RC" = 0 ] && [ -z "$OUTPUT" ]' +OUTPUT="$(printf '{"hook_event_name":"PreToolUse"}' | "$HOOK" 2>/dev/null)"; RC=$? +ok "non-Stop event → allow" '[ "$RC" = 0 ] && [ -z "$OUTPUT" ]' + +echo "" +echo "RESULT: $pass passed, $fail failed" +[ "$fail" = 0 ] diff --git a/.claude/memory/MEMORY.md b/.claude/memory/MEMORY.md index 4ed7f9a..b431038 100644 --- a/.claude/memory/MEMORY.md +++ b/.claude/memory/MEMORY.md @@ -16,3 +16,8 @@ - [Verify-snapshot tests count as coverage in NetPace.Console.Tests](feedback_console_output_snapshot_coverage.md) — check Expectations/*.verified.txt before reporting an output mode as untested - [Spec-kit upgrade procedure](speckit_upgrade_procedure.md) — stock github/spec-kit via `specify` CLI; `init --here --force --integration claude --script sh` is additive/hash-guarded - [feedback_read_source_before_designing.md](feedback_read_source_before_designing.md) — read the source before designing a fix; verify a handed-down issue/spec diagnosis against HEAD before implementing +- [dotnet test --no-build runs the stale test DLL](feedback_dotnet_test_no_build.md) — rebuild before trusting a test run; green-gate.sh denies a stale --no-build +- [Re-run tests before declaring done](feedback_rerun_tests_before_done.md) — after any post-implementation edit, re-run `dotnet test ./src`; don't extrapolate an earlier green +- [Soft-wrap markdown — one line per paragraph](feedback_markdown_soft_wrap.md) — no manual ~80/100-col hard breaks; they reflow whole blocks and bury one-word edits in noisy diffs +- [Explain tradeoffs in plain language before asking the user to decide](feedback_plain_language_decisions.md) — frame AskUserQuestion options by consequence/cost, not mechanism +- [Audit the failure class after the second occurrence](feedback_audit_class_after_two_failures.md) — on the 2nd same-shape failure, enumerate every site and fix the class, not the instance diff --git a/.claude/memory/feedback_audit_class_after_two_failures.md b/.claude/memory/feedback_audit_class_after_two_failures.md new file mode 100644 index 0000000..eca71b4 --- /dev/null +++ b/.claude/memory/feedback_audit_class_after_two_failures.md @@ -0,0 +1,11 @@ +--- +name: Audit the failure class after the second occurrence +description: When two failures share the same root-cause shape, stop patching individual instances and enumerate every site exhibiting the pattern. The third patch concedes to whack-a-mole. +type: feedback +--- + +When two failures share the same root-cause shape (shared resource + per-caller assumption, timing race against the same subsystem, cross-project state pollution of the same kind), **stop patching individual instances**. Audit every site that exhibits the pattern, build a small matrix of (resource, user, assumption), and fix the whole class in one pass. Patching the third instance without auditing concedes to whack-a-mole — and the user loses faith faster than the fixes accumulate. + +**Why:** Fixing look-alike failures one at a time feels like progress but hides that they are one bug wearing several hats. Each individual patch is cheap; the pattern behind them is what actually needs deciding on. By the second occurrence you already have enough signal to name the shape — so the economical move is to enumerate the class then, not after the fifth "why is this still happening?". + +**How to apply:** On the second failure of a shape you've seen before, before writing any fix code: enumerate every call site in the codebase that exhibits the same pattern. Produce the (resource, user, assumption) matrix and identify every conflict cell. Propose a single fix that resolves the class, not the instance. If the matrix reveals an architectural gap — the system permits violations that conventions merely ask callers to avoid — the fix may need to move into production code rather than be repeated at each call site. diff --git a/.claude/memory/feedback_dotnet_test_no_build.md b/.claude/memory/feedback_dotnet_test_no_build.md new file mode 100644 index 0000000..6ba9cb8 --- /dev/null +++ b/.claude/memory/feedback_dotnet_test_no_build.md @@ -0,0 +1,15 @@ +--- +name: dotnet test --no-build runs the stale test DLL +description: After editing test-project sources, `dotnet test --no-build` runs the stale DLL — always rebuild first. green-gate.sh now enforces this. +type: feedback +--- + +`dotnet test --no-build` skips compilation across the **entire solution**, including test projects. After editing any `.cs` under `src/` (production or a `*.Tests` project), run `dotnet build` first (or drop the `--no-build` flag) before trusting the run output — otherwise you are asserting against the DLL compiled *before* your edit. + +**Why:** `--no-build` is a genuine footgun. Edit a source, run `dotnet test --no-build`, and you get the OLD compiled behaviour — a failure (or a pass) that no longer matches the code on disk — followed by a confused investigation whose real fix is just `dotnet build` and re-run. Compile-current-on-disk is not implied by "I just edited the source". + +**How to apply:** +- After any source edit, **always** rebuild before re-running tests. +- Only use `--no-build` to re-run an already-current build — i.e. no source has changed since the last build. +- This is now gate-enforced: [`green-gate.sh`](../hooks/green-gate.sh) denies a `dotnet test --no-build` when no test assembly is built yet, or when a `*.cs` under `src/` is newer than the newest `*.Tests.dll`. Emergency override: `NETPACE_SKIP_GREEN_GATE=1` (announced on stderr). +- Pairs with [[feedback_rerun_tests_before_done]]: compile-clean is not test-clean, and DLL-current-on-disk is not implied by the most recent build either. diff --git a/.claude/memory/feedback_markdown_soft_wrap.md b/.claude/memory/feedback_markdown_soft_wrap.md new file mode 100644 index 0000000..0705870 --- /dev/null +++ b/.claude/memory/feedback_markdown_soft_wrap.md @@ -0,0 +1,15 @@ +--- +name: Soft-wrap markdown — one line per paragraph, no hard breaks +description: Author markdown paragraphs as a single unwrapped line each; don't insert manual ~80/100-col hard line breaks. Reflowing wrapped prose rewrites whole blocks and buries a one-word edit in a noisy diff. +type: feedback +--- + +When writing or editing markdown in NetPace (docs, READMEs, memory files, PR bodies, issue comments), author each paragraph, list item, and table row as a **single unwrapped line**. Let the editor/viewer soft-wrap it. Do NOT insert manual hard line breaks to keep lines near 80 or 100 columns. + +**Why:** Hard-wrapped paragraphs produce hostile diffs: changing one word near the start of a paragraph reflows every following line, so a one-word edit shows up as a whole rewritten block and the real change is impossible to spot in review. Soft-wrapped (one-line-per-paragraph) prose makes each edit a minimal, reviewable line-level change — the whole point of a text-based, diff-reviewed workflow. + +**How to apply:** +- One physical line per paragraph. One physical line per bullet / numbered-list item. One physical line per table row. Blank line between paragraphs as usual. +- Do not "tidy" existing files by re-wrapping them at a column width, and do not add a hard-wrap step to any formatting hook or tooling. +- Genuine line breaks that are semantically meaningful (fenced code blocks, a deliberate two-space or backslash line break inside a paragraph) are unaffected — this is about not *reflowing prose to a column*. +- Adjacent guidance on keeping docs clean and diff-friendly: [[feedback_docs_no_forward_references]], [[feedback_no_spec_references]], [[feedback_no_column_alignment]]. diff --git a/.claude/memory/feedback_plain_language_decisions.md b/.claude/memory/feedback_plain_language_decisions.md new file mode 100644 index 0000000..dc4997f --- /dev/null +++ b/.claude/memory/feedback_plain_language_decisions.md @@ -0,0 +1,14 @@ +--- +name: Explain tradeoffs in plain language before asking the user to decide +description: When surfacing a technical decision, explain each option in plain, jargon-free language — consequences and analogies over internals — before asking the user to choose. +type: feedback +--- + +When NetPace work reaches a genuine decision point and you put it to the user (an `AskUserQuestion`, a "which approach?" prompt, a spec tradeoff), explain each option in **plain, jargon-free language first**: what it means for them, what it costs, what it unlocks — using analogies over internals. Lead with the consequence, not the mechanism. The user should be able to choose well without first having to decode framework names, protocol details, or implementation jargon. + +**Why:** Frank's standing preference is to decide on the basis of *consequences*, not internals. Decisions framed in implementation vocabulary ("System.CommandLine vs a hand-rolled parser", "PreToolUse vs Stop hook", "bundle the amendment vs split the PR") push the translation work onto the reader and make the tradeoff harder to weigh, not easier. A one-line plain-language framing per option — *"this keeps one clean history but mixes a rule change with tooling; that keeps them separate at the cost of a second PR"* — lets the user exercise judgment on the thing that actually matters to them. + +**How to apply:** +- Before an `AskUserQuestion`, write each option's `description` as a plain-language consequence ("what you get / what it costs"), not a restatement of the mechanism. Keep the jargon term available for those who want it, but after the plain framing, not instead of it. +- Prefer a concrete analogy or a before/after over an internals walkthrough when the internals aren't the point of the decision. +- This is the *how you present* companion to surfacing tool/architecture tradeoffs as explicit user choices rather than burying them — surface the choice, and frame it in plain language. diff --git a/.claude/memory/feedback_rerun_tests_before_done.md b/.claude/memory/feedback_rerun_tests_before_done.md new file mode 100644 index 0000000..4dcc716 --- /dev/null +++ b/.claude/memory/feedback_rerun_tests_before_done.md @@ -0,0 +1,14 @@ +--- +name: Re-run tests before declaring done +description: After any post-implementation edit, re-run `dotnet test ./src` before reporting work as complete — don't extrapolate from an earlier green run. +type: feedback +--- + +After any post-implementation edit (review fixes, refactors, late-stage tweaks), re-run `dotnet test ./src` before reporting the work as complete. Don't extrapolate from an earlier green run — compile-clean is not test-clean, and the earlier count was taken against earlier code. + +**Where the hard guarantee lives:** the "suite is green before a PR" check is a **real whole-suite run inside `/ship`** ([.claude/commands/ship.md](../commands/ship.md)), which gates the review/PR on `dotnet build ./src && dotnet test ./src`. During implementation, keeping the suite green is a **soft** standard — run it at your own discretion; the hard gate is `/ship`. + +**Why:** Relaying an earlier "N/N passed" after subsequent edits is how a regression reaches review unseen. The fix is cheap — one more run — and NetPace's suite is fast and needs no external stack (unlike a services-backed project, there is nothing to "bring up" first, so "deferred — needs a running stack" is never the right answer here). + +**How to apply:** When ending a session, capping a feature, or reporting after fixes, run `dotnet test ./src` one more time and state the actual fresh count, not the most recent earlier count. +- Pairs with [[feedback_dotnet_test_no_build]]: when you re-run, rebuild first — a `--no-build` re-run can report the stale DLL and undo the point of re-running. diff --git a/.claude/settings.json b/.claude/settings.json index cc73ee9..6ad4fc5 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -115,6 +115,11 @@ "if": "Bash(git commit:*)", "statusMessage": "Checking for banned skipped-test constructs..." }, + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/green-gate.sh\"", + "statusMessage": "Guarding against dotnet test --no-build staleness..." + }, { "type": "command", "command": "STAGED=$(git diff --cached --name-only | grep \"[.]cs$\"); [ -z \"$STAGED\" ] && exit 0; INCLUDE=$(echo \"$STAGED\" | tr '\\n' ' '); dotnet format style --include $INCLUDE && dotnet format whitespace --include $INCLUDE && echo \"$STAGED\" | xargs -d '\\n' git add", @@ -129,10 +134,26 @@ } ] } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/traceability-gate.sh\"" + } + ] + } ] }, "enabledPlugins": { "pr-review-toolkit@claude-plugins-official": true, "context-mode@context-mode": true + }, + "preferredNotifChannel": "terminal_bell", + "statusLine": { + "type": "command", + "command": "sh \"$CLAUDE_PROJECT_DIR/.claude/statusline-command.sh\"", + "padding": 0 } } diff --git a/.claude/statusline-command.sh b/.claude/statusline-command.sh new file mode 100755 index 0000000..ea5f715 --- /dev/null +++ b/.claude/statusline-command.sh @@ -0,0 +1,89 @@ +#!/bin/sh +# Claude Code status line — two-line, icon-segmented. +# Adapted from danielmackay/claude-code-statusline (dandoescode.com), fixed for Linux/WSL: +# - every segment renders only when its field is present (no blank / stray bars) +# - reset time parses both epoch and ISO-8601 via GNU `date -d` +# - git runs against the session cwd +# +# Line 1 (session): 🤖 model | 💪 effort | 🧠 context% | 💰 cost | ⏱️ 5h-limit | 📅 7d-limit +# Line 2 (place): 📁 repo | 🌳 worktree | 🌿 branch +added -removed (lines vs HEAD) + +input=$(cat) +j() { printf '%s' "$input" | jq -r "$1 // empty" 2>/dev/null; } + +model=$(j '.model.display_name'); [ -z "$model" ] && model="Claude" +effort=$(j '.effort.level') +used=$(j '.context_window.used_percentage') +total_cost=$(j '.cost.total_cost_usd') +worktree=$(j '.worktree.name') +current_dir=$(j '.workspace.current_dir') +[ -z "$current_dir" ] && current_dir=$(j '.worktree.original_cwd') +[ -z "$current_dir" ] && current_dir=$(j '.cwd') +[ -z "$current_dir" ] && current_dir="$PWD" +rl5_pct=$(j '.rate_limits.five_hour.used_percentage') +rl5_reset=$(j '.rate_limits.five_hour.resets_at') +rl7_pct=$(j '.rate_limits.seven_day.used_percentage') +rl7_reset=$(j '.rate_limits.seven_day.resets_at') + +GREEN='\033[32m'; YELLOW='\033[33m'; RED='\033[31m' +CYAN='\033[36m'; MAGENTA='\033[35m'; DIM='\033[2m'; RESET='\033[0m' +SEP="${DIM}|${RESET}" + +# --- git: branch + line insertions/deletions vs HEAD --- +if git -C "$current_dir" rev-parse --git-dir >/dev/null 2>&1; then + branch=$(git -C "$current_dir" branch --show-current 2>/dev/null) + [ -z "$branch" ] && branch=$(git -C "$current_dir" rev-parse --abbrev-ref HEAD 2>/dev/null) + diffstat=$(git -C "$current_dir" diff HEAD --numstat 2>/dev/null | awk '{a+=$1; d+=$2} END {printf "%d %d", a, d}') + add=${diffstat% *}; del=${diffstat#* } + git_str="${GREEN}${branch}${RESET}" + [ "${add:-0}" -gt 0 ] 2>/dev/null && git_str="${git_str} ${GREEN}+${add}${RESET}" + [ "${del:-0}" -gt 0 ] 2>/dev/null && git_str="${git_str} ${RED}-${del}${RESET}" +else + git_str="${DIM}no branch${RESET}" +fi + +dir_display=$(basename "$(cd "$current_dir" 2>/dev/null && git rev-parse --show-toplevel 2>/dev/null || printf '%s' "$current_dir")") + +# --- rate-limit segments (each renders only when its data is present) --- +make_bar() { + pct=$1; width=10 + filled=$(( pct * width / 100 )); [ "$filled" -gt "$width" ] && filled=$width + i=0; bar="" + while [ $i -lt $filled ]; do bar="${bar}█"; i=$((i+1)); done + while [ $i -lt $width ]; do bar="${bar}░"; i=$((i+1)); done + printf '%s' "$bar" +} +fmt_reset() { # $1 = timestamp, $2 = strftime format + case "$1" in + '') : ;; + *[!0-9]*) date -d "$1" "$2" 2>/dev/null ;; # ISO-8601 + *) date -d "@$1" "$2" 2>/dev/null ;; # epoch seconds + esac +} +rl_segment() { # $1 pct, $2 reset_ts, $3 label, $4 reset_fmt + [ -z "$1" ] && return + p=$(printf '%.0f' "$1" 2>/dev/null) + if [ "$p" -ge 90 ] 2>/dev/null; then c=$RED + elif [ "$p" -ge 70 ] 2>/dev/null; then c=$YELLOW + else c=$GREEN; fi + rt=$(fmt_reset "$2" "$4") + seg="${c}$3 $(make_bar "$p") ${p}%" + [ -n "$rt" ] && seg="${seg} resets ${rt}" + printf '%s%s' "$seg" "$RESET" +} +rate5=$(rl_segment "$rl5_pct" "$rl5_reset" "5h" '+%-I:%M%p') +rate7=$(rl_segment "$rl7_pct" "$rl7_reset" "7d" '+%a %-I%p') + +# --- assemble --- +line1="🤖 ${MAGENTA}${model}${RESET}" +[ -n "$effort" ] && line1="${line1} ${SEP} 💪 ${effort}" +[ -n "$used" ] && line1="${line1} ${SEP} 🧠 $(printf '%.0f' "$used" 2>/dev/null)%" +[ -n "$total_cost" ] && line1="${line1} ${SEP} 💰 \$$(awk "BEGIN{printf \"%.2f\", $total_cost}" 2>/dev/null)" +[ -n "$rate5" ] && line1="${line1} ${SEP} ⏱️ ${rate5}" +[ -n "$rate7" ] && line1="${line1} ${SEP} 📅 ${rate7}" + +line2="📁 ${CYAN}${dir_display}${RESET}" +[ -n "$worktree" ] && line2="${line2} ${SEP} 🌳 ${worktree}" +line2="${line2} ${SEP} 🌿 ${git_str}" + +printf '%b\n%b' "$line1" "$line2" diff --git a/.specify/memory/constitution.md b/.specify/memory/constitution.md index 3b5e65e..b5e5243 100644 --- a/.specify/memory/constitution.md +++ b/.specify/memory/constitution.md @@ -1,23 +1,20 @@ @@ -101,8 +98,9 @@ NetPace.Core MUST keep dependencies minimal: - Prefer .NET BCL over third-party libraries when possible - Document all dependencies and their purpose - Review dependency security regularly +- Runtime dependencies MUST use a permissive licence (MIT, Apache 2.0, or BSD). Copyleft licences (GPL, LGPL, AGPL) are prohibited without documented justification and maintainer sign-off. -**Rationale**: As a NuGet package, NetPace.Core's dependencies become consumers' dependencies. Minimal dependencies reduce version conflicts and security surface area. +**Rationale**: As a NuGet package, NetPace.Core's dependencies become consumers' dependencies — a copyleft runtime dependency propagates its licence obligations to every consumer, so a permissive-only runtime baseline is a compatibility guarantee, not just hygiene. Minimal dependencies reduce version conflicts and security surface area; the permissive-licence constraint keeps NetPace freely embeddable in closed and commercial software. ### VII. Semantic Versioning @@ -246,4 +244,4 @@ This constitution supersedes all other development practices and guides. All dev - Complexity MUST be justified against simplicity principles - For runtime development guidance, refer to `CLAUDE.md` -**Version**: 1.5.0 | **Ratified**: 2026-04-10 | **Last Amended**: 2026-07-11 +**Version**: 1.6.0 | **Ratified**: 2026-04-10 | **Last Amended**: 2026-07-29