From 219acfff3567731b7c33198cec7088b6ec5b7408 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 24 Aug 2026 00:36:55 -0400 Subject: [PATCH 1/4] feat(scripts): gate the de-slopped surfaces against em-dash regression The de-slop campaign purges prose surface by surface, and each landed shard is paid for by hand: a mechanical em-dash split changes meaning often enough that every shard so far has needed a rationale-withheld reviewer to catch clauses the automated passes waved through. Nothing then stopped the next contributor from reintroducing one, because no lane enforced the policy. A one-time purge with no gate is a purge that silently rots. Add scripts/check-purged-em-dashes.sh, which enforces zero em dashes on the paths declared in scripts/em-dash-purged-paths.txt, and wire it into the ci hygiene job so its outcome reaches the ci-status aggregate. Enforcement is scoped to an allowlist rather than the corpus. 29,649 em-dash prose lines across 1,074 tracked markdown files remain, so a repo-wide check could only have been merged switched off. Listing what is already clean inverts that: adoption breaks nothing, because every listed path passes the moment it is added, and enforcement grows with the campaign instead of waiting for it. This is the same allowlist-not-blocklist argument, and the same fail-safe direction, that scripts/docs-only-paths.txt already makes. The tracked .claude/ai-slop.json is untouched. Re-enabling rule-em-dash corpus-wide there is a separate decision the campaign has gated on the purge finishing, so the gate instead copies that config into a throwaway layer for its own detector run and removes only rule-em-dash from disabled_rules. Every exclusion the tracked file carries, including the vendor, catalog and eval-fixture paths that hold em dashes as data, keeps applying unchanged. The detector is driven rather than reimplemented: a bare grep would fire inside code fences, inline code spans and ignore-marked regions, which legitimately carry the character. Because that detector exits 0 on every audit path, the gate believes a run only when the rule-em-dash summary is present, reports disabled=0, and accounts for exactly the files handed to it; any other shape is exit 2 rather than a false green. A stale allowlist entry matching no tracked file fails for the same reason. Refs #2891 Co-authored-by: Kyle Sexton --- .github/workflows/ci.yml | 14 ++ scripts/check-purged-em-dashes.sh | 234 +++++++++++++++++++++++++ scripts/check-purged-em-dashes.test.sh | 222 +++++++++++++++++++++++ scripts/em-dash-purged-paths.txt | 69 ++++++++ 4 files changed, 539 insertions(+) create mode 100644 scripts/check-purged-em-dashes.sh create mode 100644 scripts/check-purged-em-dashes.test.sh create mode 100644 scripts/em-dash-purged-paths.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df2277dc4..4ce2d6c69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -285,6 +285,19 @@ jobs: continue-on-error: true run: scripts/check-hook-wiring-liveness.sh + # Deliberately UNCONDITIONAL, like the other whole-repo scanners. This + # gate reads markdown prose, which is exactly what a docs-only diff + # changes, so gating it on run_full would disable it on precisely the pull + # requests most able to trip it -- a fail-closed hole rather than an + # honest not-applicable. The self-test runs first and ungated so a broken + # gate cannot mask a regression. + - name: Test the purged-em-dash gate + run: bash scripts/check-purged-em-dashes.test.sh + - name: Check purged surfaces stayed free of em dashes + id: purged_em_dashes + continue-on-error: true + run: scripts/check-purged-em-dashes.sh + - name: Report docs-irrelevant checks not applicable to a docs-only diff if: needs.scope.outputs.run_full == 'false' run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the path-scoped linters (actionlint, check-jsonschema x4, the manifest duplicate-key detector) cannot be affected — reporting success for them. ShellCheck, exec-bit, and hook-wiring-liveness scan the whole repo and stay unconditional." @@ -329,6 +342,7 @@ jobs: eol-renormalize=${{ steps.eol.outcome }} comment-hygiene=${{ steps.comment_hygiene.outcome }} hook-wiring-liveness=${{ steps.hook_wiring.outcome }} + purged-em-dashes=${{ steps.purged_em_dashes.outcome }} run: scripts/aggregate-hygiene-results.sh zizmor: diff --git a/scripts/check-purged-em-dashes.sh b/scripts/check-purged-em-dashes.sh new file mode 100644 index 000000000..23b8032d7 --- /dev/null +++ b/scripts/check-purged-em-dashes.sh @@ -0,0 +1,234 @@ +#!/usr/bin/env bash +# Fail when an instruction surface this repository has already purged of em +# dashes grows a new one. +# +# scripts/check-purged-em-dashes.sh report violations, exit 1 if any +# scripts/check-purged-em-dashes.sh --check same (explicit form, matches sibling gates) +# scripts/check-purged-em-dashes.sh --list list every declared path and its verdict +# +# Exit: 0 clean, 1 a violation or a stale allowlist entry, 2 usage or a +# prerequisite this gate cannot verify around. +# +# WHY (#2891). The de-slop campaign rewrites prose surface by surface, and each +# landed shard is paid for by hand: a mechanical em-dash split changes meaning +# often enough that every shard so far has needed a rationale-withheld reviewer +# to catch clauses the automated passes waved through. Nothing then stops the +# next contributor from reintroducing one, because no lane enforces the policy. +# A one-time purge with no gate is a purge that silently rots. This gate is the +# ratchet: what the campaign has already cleaned stays clean. +# +# ALLOWLIST, NOT A REPO-WIDE RULE, and the distinction is the whole design. +# 29,649 em-dash prose lines across 1,074 tracked markdown files remained when +# this gate was written, so a repo-wide check would fail nearly every pull +# request on contact and would have to be merged disabled, which is not a gate. +# Enforcing only DECLARED-CLEAN paths inverts that: blast radius at adoption is +# zero, because every listed path already passes. Enforcement then grows with +# the campaign instead of waiting for it. A shard that purges a surface adds +# its lines here in the same pull request, and the surface is defended from that +# moment on. +# +# This is the same shape, and the same argument, as scripts/docs-only-paths.txt: +# a positive list fails safe, because a path NOT listed is simply unenforced +# rather than wrongly declared clean. The failure mode of a blocklist here would +# be the reverse and much worse: a surface silently dropping out of enforcement +# the day someone widened an exclusion. +# +# A STALE ENTRY IS A FAILURE, not a skip. An allowlist entry matching no tracked +# file means the surface was renamed or deleted and the declaration outlived it; +# left as a no-op, the list would accumulate dead lines and quietly enforce less +# than it claims. That is the #1513 shape, a gate that enforces nothing and +# still exits 0, so a zero-match entry fails, naming itself. For the same +# reason an unreadable or entirely inactive allowlist is exit 2 rather than a +# clean run. +# +# THE TRACKED DETECTOR CONFIG IS NOT MODIFIED, and must not be. This repository's +# .claude/ai-slop.json disables rule-em-dash corpus-wide, and re-enabling it there +# is a separate decision the campaign has explicitly gated on the purge finishing +# (#2891, checkbox 4). So this gate does not touch that file, and running it +# changes nothing about what /ai-slop:audit reports. It instead builds a +# THROWAWAY config layer for its own detector invocation: the tracked config +# copied verbatim, with rule-em-dash removed from disabled_rules and nothing else +# altered. Copying rather than synthesizing is deliberate: excluded_paths, +# em_dash_allowed_paths and every threshold stay whatever the tracked file says, +# so the vendor, catalog and eval-fixture exclusions that exist precisely because +# they contain em dashes as DATA keep applying here, and keep applying without a +# second copy of that list to drift. +# +# REUSES THE DETECTOR RATHER THAN GREPPING. A bare grep for the em-dash byte +# sequence would fire inside fenced code blocks, inline code spans, and +# ignore-marked regions, all of which legitimately carry the character. Prose +# extraction is exactly what plugins/ai-slop/skills/audit/scripts/detect.sh +# already implements and tests, so this gate drives that script and reads its +# findings instead of growing a second, less-tested notion of what prose is. +# +# LIVENESS IS ASSERTED, NOT ASSUMED. detect.sh exits 0 on every audit path by +# design (a read-only audit must never fail its caller), so an exit code proves +# nothing here. Worse, the failure this gate is most exposed to is silent: if the +# throwaway config ever stopped taking effect, rule-em-dash would be disabled, +# the detector would report no em-dash findings, and this gate would pass +# everything forever. So the run is only believed when the detector's own summary +# line for rule-em-dash is present AND reports disabled=0, and when the file +# count it says it handled matches the count handed to it. Any other shape is +# exit 2. +# +# Test injection, all defaulting to this repository: +# EM_DASH_PURGED_ROOT repository root to scan +# EM_DASH_PURGED_PATHS allowlist file (relative to the root, or absolute) +# EM_DASH_SLOP_CONFIG tracked detector config to copy +# EM_DASH_DETECT detect.sh to drive +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit 2 + +ROOT="${EM_DASH_PURGED_ROOT:-$SCRIPT_DIR/..}" +cd "$ROOT" || { + echo "check-purged-em-dashes: cannot enter root: $ROOT" >&2 + exit 2 +} +ROOT="$(pwd)" + +# shellcheck source=lib/read-list.sh +. "$SCRIPT_DIR/lib/read-list.sh" || exit 2 + +ALLOWLIST="${EM_DASH_PURGED_PATHS:-scripts/em-dash-purged-paths.txt}" +SLOP_CONFIG="${EM_DASH_SLOP_CONFIG:-.claude/ai-slop.json}" +DETECT="${EM_DASH_DETECT:-plugins/ai-slop/skills/audit/scripts/detect.sh}" + +MODE=check +case "${1-}" in +"" | --check) ;; +--list) MODE=list ;; +*) + echo "usage: check-purged-em-dashes.sh [--check|--list]" >&2 + exit 2 + ;; +esac + +# --- Prerequisites ---------------------------------------------------------- +# Each is fail-closed at exit 2: this gate cannot report a trustworthy "clean" +# without all four, and reporting an untrustworthy one is the failure mode the +# whole design is arranged against. + +if ! command -v jq >/dev/null 2>&1; then + echo "check-purged-em-dashes: jq not found; cannot build the detector config layer" >&2 + exit 2 +fi +if [[ ! -r "$SLOP_CONFIG" ]]; then + echo "check-purged-em-dashes: detector config not readable: $SLOP_CONFIG" >&2 + exit 2 +fi +if [[ ! -r "$DETECT" ]]; then + echo "check-purged-em-dashes: detector not readable: $DETECT" >&2 + exit 2 +fi + +declare -a GLOBS=() +read_list::into GLOBS "$ALLOWLIST" --comments inline || exit 2 +if ((${#GLOBS[@]} == 0)); then + echo "check-purged-em-dashes: no active entries in $ALLOWLIST; a gate with an empty allowlist enforces nothing" >&2 + exit 2 +fi + +# --- Expand the allowlist --------------------------------------------------- +# `:(glob)` pathspec magic, not git's default wildmatch: without it `*` matches +# across directory separators, so `plugins/*/README.md` would silently pull in +# `plugins/a/b/README.md`. An allowlist whose entries claim more surface than +# they name is a declaration nobody can audit by reading it. + +TMP="$(mktemp -d)" || exit 2 +trap 'rm -rf "$TMP"' EXIT + +FILES="$TMP/files.txt" +: >"$FILES" +stale=0 +for glob in "${GLOBS[@]}"; do + matched="$(git ls-files -z -- ":(glob)$glob" | tr '\0' '\n' | sed '/^$/d')" + count=0 + [[ -n "$matched" ]] && count="$(printf '%s\n' "$matched" | wc -l | tr -d ' ')" + if ((count == 0)); then + echo "check-purged-em-dashes: stale allowlist entry matches no tracked file: $glob" >&2 + stale=1 + [[ "$MODE" == list ]] && printf 'STALE %s\n' "$glob" + continue + fi + [[ "$MODE" == list ]] && printf 'ok %s (%s files)\n' "$glob" "$count" + printf '%s\n' "$matched" >>"$FILES" +done + +sort -u -o "$FILES" "$FILES" +EXPECTED="$(wc -l <"$FILES" | tr -d ' ')" + +if ((stale == 1)); then + echo "check-purged-em-dashes: remove or repoint the stale entries above" >&2 + exit 1 +fi +if ((EXPECTED == 0)); then + echo "check-purged-em-dashes: allowlist expanded to zero files" >&2 + exit 2 +fi + +# --- Throwaway detector config ---------------------------------------------- +# The tracked config verbatim, minus rule-em-dash's entry in disabled_rules. An +# empty HOME keeps the user-global layer out: detect.sh cascades +# $HOME/.claude/ai-slop.json under the repo layer, and a contributor who happens +# to carry one must not be able to change this gate's verdict. + +mkdir -p "$TMP/root/.claude" "$TMP/home" || exit 2 +if ! jq '.disabled_rules |= ((. // []) | map(select(. != "rule-em-dash")))' \ + "$SLOP_CONFIG" >"$TMP/root/.claude/ai-slop.json"; then + echo "check-purged-em-dashes: could not derive the detector config from $SLOP_CONFIG" >&2 + exit 2 +fi + +OUT="$TMP/findings.txt" +if ! HOME="$TMP/home" CLAUDE_PROJECT_DIR="$TMP/root" \ + bash "$DETECT" --paths-file "$FILES" >"$OUT" 2>"$TMP/detect.err"; then + echo "check-purged-em-dashes: detector failed" >&2 + cat "$TMP/detect.err" >&2 + exit 2 +fi + +# --- Believe the run only if it proves it happened -------------------------- + +summary="$(grep -m1 '^Summary rule=ai-slop/audit/rule-em-dash ' "$OUT")" +if [[ -z "$summary" ]]; then + echo "check-purged-em-dashes: detector emitted no rule-em-dash summary; cannot confirm the rule ran" >&2 + exit 2 +fi +if [[ "$summary" != *" disabled=0"* ]]; then + echo "check-purged-em-dashes: rule-em-dash was disabled for this run ($summary); the config layer did not take effect" >&2 + exit 2 +fi + +scanned="$(sed -n 's/^Summary total: [0-9]* findings across \([0-9]*\) files scanned (\([0-9]*\) files declined)$/\1 \2/p' "$OUT")" +if [[ -z "$scanned" ]]; then + echo "check-purged-em-dashes: detector emitted no total summary; cannot confirm coverage" >&2 + exit 2 +fi +handled=$(($(echo "$scanned" | cut -d' ' -f1) + $(echo "$scanned" | cut -d' ' -f2))) +if ((handled != EXPECTED)); then + echo "check-purged-em-dashes: detector handled $handled files but $EXPECTED were declared; coverage is not what the allowlist claims" >&2 + exit 2 +fi + +# --- Verdict ---------------------------------------------------------------- + +findings="$(grep '^Finding: rule=ai-slop/audit/rule-em-dash ' "$OUT")" +if [[ -z "$findings" ]]; then + printf 'check-purged-em-dashes: %s declared paths, %s files, no em dashes.\n' "${#GLOBS[@]}" "$EXPECTED" + exit 0 +fi + +count="$(printf '%s\n' "$findings" | wc -l | tr -d ' ')" +echo "check-purged-em-dashes: $count em-dash line(s) on surfaces declared purged in $ALLOWLIST" >&2 +printf '%s\n' "$findings" | + sed -n 's|^Finding: rule=ai-slop/audit/rule-em-dash file=\([^ ]*\) line=\([0-9]*\) .*excerpt=\(.*\)$| \1:\2: \3|p' >&2 +cat >&2 <<'EOF' + +These paths are declared already purged, so an em dash here is a regression. +Rewrite the line: a period, a comma, or a restructured sentence. Do not +substitute parentheses, en dashes, or spaced hyphens, and do not "fix" an em +dash that is quoted data, a fixture, or third-party text. Take that path off +the allowlist instead, with the reason. +EOF +exit 1 diff --git a/scripts/check-purged-em-dashes.test.sh b/scripts/check-purged-em-dashes.test.sh new file mode 100644 index 000000000..f43aa543a --- /dev/null +++ b/scripts/check-purged-em-dashes.test.sh @@ -0,0 +1,222 @@ +#!/usr/bin/env bash +# Black-box contract test for check-purged-em-dashes.sh. +# +# Hermetic and cwd-independent: builds a throwaway git repository containing its +# own markdown, its own allowlist and its own detector config, points the SUT at +# it through the four EM_DASH_* injection variables, and asserts on exit code +# plus output. The one thing it does NOT stub is the detector. The fixture +# drives the real plugins/ai-slop/skills/audit/scripts/detect.sh, because the +# behaviour most worth pinning here is precisely that this gate and that +# detector agree on what counts as prose. A stubbed detector would let the two +# drift and still report green. +# +# The seeded violation is the point of the suite. A gate whose failing path is +# never exercised is a gate nobody has evidence works; every "clean" assertion +# below would also pass against a script that unconditionally exits 0. So the +# fixture plants a real em dash in prose and requires exit 1 and the offending +# file named, and separately plants em dashes in the places that legitimately +# carry them, a fenced code block and an ignore-marked line, and requires +# those NOT to fire. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SUT="$SCRIPT_DIR/check-purged-em-dashes.sh" +DETECT="$SCRIPT_DIR/../plugins/ai-slop/skills/audit/scripts/detect.sh" +DETECT="$(cd "$(dirname "$DETECT")" && pwd)/$(basename "$DETECT")" + +# shellcheck source=lib/test-harness.sh +. "$SCRIPT_DIR/lib/test-harness.sh" +# Clears the inherited git environment for the whole suite; see +# scripts/check-fixture-git-isolation.sh for why -C alone does not isolate. +# shellcheck source=test-git-helpers.sh +. "$SCRIPT_DIR/test-git-helpers.sh" + +EM="$(printf '\xe2\x80\x94')" + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +# run [args...]: sets OUT and RC. +# Deliberately not wrapped in a command substitution of its own: that forks a +# subshell and the exit status never reaches the caller. +run() { + local root="$1" list="$2" + shift 2 + OUT="$( + EM_DASH_PURGED_ROOT="$root" \ + EM_DASH_PURGED_PATHS="$list" \ + EM_DASH_SLOP_CONFIG=".claude/ai-slop.json" \ + EM_DASH_DETECT="$DETECT" \ + bash "$SUT" "$@" 2>&1 + )" + RC=$? +} + +# --- Fixture ---------------------------------------------------------------- +# One repository, several allowlists pointed at different subsets of it, so each +# case selects the surface it means to assert on rather than rebuilding a repo. + +REPO="$TMP/repo" +mkdir -p "$REPO/.claude" "$REPO/surface" "$REPO/other" +git_init_test_repo "$REPO" + +write() { printf '%s\n' "$2" >"$REPO/$1"; } + +# The tracked detector config, shaped like this repository's: rule-em-dash +# disabled here, so a passing run PROVES the SUT's throwaway layer re-enabled it +# rather than inheriting an already-permissive config. +write ".claude/ai-slop.json" '{"excluded_paths":[],"disabled_rules":["rule-em-dash","rule-emoji-formatting"]}' + +write "surface/clean.md" 'A purged surface. It uses periods, and commas, instead.' + +write "surface/dirty.md" "A surface with a regression ${EM} right here in prose." + +# Em dashes that are DATA, not prose: one inside a fenced code block, one on a +# line the detector's own ignore marker declines. Both must stay silent, or the +# gate would push contributors to corrupt quoted material to satisfy a style +# rule. +printf '%s\n' \ + 'Prose with no em dash.' \ + '' \ + '```text' \ + "code ${EM} fence" \ + '```' \ + '' \ + "ignored ${EM} line " \ + >"$REPO/surface/data.md" + +write "other/unlisted.md" "Not on the allowlist ${EM} so not this gate's business." + +list() { + local name="$1" + shift + printf '%s\n' "$@" >"$REPO/$name.txt" +} + +list "only-clean" 'surface/clean.md' +list "clean-and-data" 'surface/clean.md' 'surface/data.md' +list "with-dirty" 'surface/clean.md' 'surface/dirty.md' +list "globbed" 'surface/*.md' +list "stale" 'surface/clean.md' 'surface/gone.md' +list "empty" '# only a comment' '' +list "commented" 'surface/clean.md # the surface purged in this fixture' + +git_test_config "$REPO" add -A +git_test_config "$REPO" commit -qm fixture + +# --- Cases ------------------------------------------------------------------ + +# Each `run` forks the detector, which is the expensive part of this suite, so +# every allowlist is run ONCE and all of its assertions read the captured +# result. Re-running the same fixture to ask a second question about it is what +# made this suite minutes long instead of seconds. + +run "$REPO" "only-clean.txt" +clean_rc=$RC clean_out="$OUT" +if ((clean_rc == 0)); then + ok "clean surface passes" +else + fail "clean surface passes (rc=$clean_rc): $clean_out" +fi +# An unlisted file is unenforced: that is the allowlist's defining property, and +# asserting it here is what stops a later "just scan everything" edit from +# passing this suite. +if [[ "$clean_out" != *"unlisted.md"* ]]; then + ok "a file outside the allowlist is not scanned" +else + fail "a file outside the allowlist is not scanned: $clean_out" +fi + +# THE SEEDED VIOLATION. +run "$REPO" "with-dirty.txt" +if ((RC == 1)); then + ok "seeded em dash fails the gate" +else + fail "seeded em dash fails the gate (rc=$RC, want 1): $OUT" +fi +if [[ "$OUT" == *"surface/dirty.md"* ]]; then + ok "the failure names the offending file" +else + fail "the failure names the offending file: $OUT" +fi +if [[ "$OUT" != *"surface/clean.md:"* ]]; then + ok "the failure does not implicate the clean file" +else + fail "the failure does not implicate the clean file: $OUT" +fi +# If the SUT ever stopped re-enabling rule-em-dash, the fixture's tracked config +# would leave the rule off and this same case would report a clean run. The +# liveness guard must therefore not be what fired here. +if [[ "$OUT" != *"config layer did not take effect"* ]]; then + ok "the derived config layer takes effect" +else + fail "the derived config layer takes effect: $OUT" +fi + +run "$REPO" "clean-and-data.txt" +if ((RC == 0)); then + ok "em dashes in a code fence and an ignore-marked line do not fire" +else + fail "em dashes in a code fence and an ignore-marked line do not fire (rc=$RC): $OUT" +fi + +run "$REPO" "globbed.txt" +if ((RC == 1)) && [[ "$OUT" == *"surface/dirty.md"* ]]; then + ok "a glob entry expands to every matching tracked file" +else + fail "a glob entry expands to every matching tracked file (rc=$RC): $OUT" +fi + +run "$REPO" "stale.txt" +if ((RC == 1)) && [[ "$OUT" == *"stale allowlist entry"* ]] && [[ "$OUT" == *"surface/gone.md"* ]]; then + ok "an entry matching no tracked file fails as stale" +else + fail "an entry matching no tracked file fails as stale (rc=$RC): $OUT" +fi + +run "$REPO" "empty.txt" +if ((RC == 2)); then + ok "an allowlist with no active entry is exit 2, not a clean run" +else + fail "an allowlist with no active entry is exit 2 (rc=$RC): $OUT" +fi + +run "$REPO" "missing.txt" +if ((RC == 2)); then + ok "a missing allowlist is exit 2" +else + fail "a missing allowlist is exit 2 (rc=$RC): $OUT" +fi + +run "$REPO" "commented.txt" +if ((RC == 0)); then + ok "an inline comment is stripped from an entry" +else + fail "an inline comment is stripped from an entry (rc=$RC): $OUT" +fi + +run "$REPO" "only-clean.txt" --list +if ((RC == 0)) && [[ "$OUT" == *"surface/clean.md"* ]]; then + ok "--list reports each declared entry" +else + fail "--list reports each declared entry (rc=$RC): $OUT" +fi + +run "$REPO" "only-clean.txt" --bogus +if ((RC == 2)); then + ok "an unknown argument is exit 2" +else + fail "an unknown argument is exit 2 (rc=$RC): $OUT" +fi + +OUT="$(EM_DASH_PURGED_ROOT="$REPO" EM_DASH_PURGED_PATHS="only-clean.txt" \ + EM_DASH_SLOP_CONFIG="does-not-exist.json" EM_DASH_DETECT="$DETECT" \ + bash "$SUT" 2>&1)" +RC=$? +if ((RC == 2)); then + ok "an unreadable detector config is exit 2" +else + fail "an unreadable detector config is exit 2 (rc=$RC): $OUT" +fi + +test_harness::report diff --git a/scripts/em-dash-purged-paths.txt b/scripts/em-dash-purged-paths.txt new file mode 100644 index 000000000..917bc0c99 --- /dev/null +++ b/scripts/em-dash-purged-paths.txt @@ -0,0 +1,69 @@ +# Instruction surfaces the de-slop campaign (#2891) has already purged of em +# dashes, and which must therefore stay purged. Consumed by +# scripts/check-purged-em-dashes.sh. +# +# ALLOWLIST, NOT A REPO-WIDE RULE, and that is the design rather than a +# concession. 29,649 em-dash prose lines across 1,074 tracked markdown files +# remained when this list was seeded, so a corpus-wide check could only have +# been merged switched off. Listing what is already clean inverts the problem: +# adopting the gate breaks nothing, because every path below passes the moment +# it is added, and enforcement then grows with the campaign rather than waiting +# for it. +# +# The same argument, and the same fail-safe direction, as +# scripts/docs-only-paths.txt: a path NOT on this list is merely unenforced, +# whereas a blocklist would let a surface fall silently out of enforcement the +# day someone widened an exclusion. +# +# ADDING AN ENTRY is the last step of purging a surface, in the same pull +# request that purges it. Two conditions, both checked: +# +# 1. The entry must match at least one tracked file. A glob matching nothing +# fails the gate rather than passing vacuously, so a renamed or deleted +# surface cannot leave a dead line here claiming coverage. +# 2. Every file it matches must be free of em dashes in prose, as +# plugins/ai-slop/skills/audit/scripts/detect.sh judges prose: fenced code +# blocks, inline code spans, and ai-slop-ignore-marked regions are exempt, +# because those legitimately carry the character as data. +# +# THE SEED IS STRICTER THAN THE GATE, deliberately. Every path in the seed +# contains no em dash ANYWHERE, not merely none in prose. That makes the whole +# list auditable without running the detector: expand it with +# `scripts/check-purged-em-dashes.sh --list` and grep the named files for the +# character, and nothing should match. A later entry is free to rely on the +# detector's prose rule instead, and to carry the character inside a code fence +# or an ignore-marked region; it simply cannot be spot-checked that cheaply. +# +# WHAT IS NOT HERE. Plugin surfaces still carrying em dashes are absent by +# definition, and are the campaign's remaining work. Also absent on purpose: +# docs/adr/** (decision records are a historical account, not prose to restyle), +# docs/upstream/** and plugins/*/skills/*/vendor/** (third-party text, which the +# repo's own vendor-docs-are-not-style rule puts out of scope), and the eval +# fixtures under plugins/ai-slop/skills/audit/evals/, which contain the tells +# they exist to test. + +# Root README, purged in #3265. +README.md + +# Plugin instruction surfaces, purged in the campaign's per-plugin shards. The +# skills glob is intentionally open: a NEW skill added to one of these plugins +# is covered from the moment it lands, which is the cheapest possible time to +# keep it clean. +plugins/adhd/README.md +plugins/adhd/skills/*/SKILL.md +plugins/codebase-health/README.md +plugins/codebase-health/skills/*/SKILL.md +plugins/debugging/README.md +plugins/debugging/skills/*/SKILL.md +plugins/implementation/README.md +plugins/implementation/skills/*/SKILL.md +plugins/naming/README.md +plugins/naming/skills/*/SKILL.md +plugins/overengineering/README.md +plugins/overengineering/skills/*/SKILL.md +plugins/prototype/README.md +plugins/prototype/skills/*/SKILL.md +plugins/verification/README.md +plugins/verification/skills/*/SKILL.md +plugins/wizard/README.md +plugins/wizard/skills/*/SKILL.md From 69cbbe9e90b10c2d99bce643d96289e102bc5680 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 24 Aug 2026 06:28:43 -0400 Subject: [PATCH 2/4] fix(scripts): make the purged-em-dash gate executable and cover playwright The gate shipped mode 100644 while the workflow invokes it as a bare path, `scripts/check-purged-em-dashes.sh`, exactly as it invokes every sibling scanner. Every one of those siblings is tracked 100755. The hygiene job would therefore have failed on contact with a permission error, and the repository's own unconditional exec-bit scanner would have reported the same defect a second time. Both the gate and its test script are now tracked executable. Add plugins/playwright to the allowlist. Its README and its skills were purged in #3320 and contain no em dash anywhere, so the entry meets the stricter standard the allowlist header declares for a seed entry: the whole list stays auditable by expanding it with --list and grepping the named files for the character. The plugin was clean when the list was seeded and was simply not carried over. Restructure the spaced double hyphen in the workflow comment into a sentence break. The gate's own failure message tells a contributor not to reach for a hyphen when an em dash comes out, and a comment introducing that gate should not model the substitution it warns against. Refs #2891 Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 2 +- scripts/check-purged-em-dashes.sh | 0 scripts/check-purged-em-dashes.test.sh | 0 scripts/em-dash-purged-paths.txt | 2 ++ 4 files changed, 3 insertions(+), 1 deletion(-) mode change 100644 => 100755 scripts/check-purged-em-dashes.sh mode change 100644 => 100755 scripts/check-purged-em-dashes.test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ce2d6c69..05ef6f01d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -288,7 +288,7 @@ jobs: # Deliberately UNCONDITIONAL, like the other whole-repo scanners. This # gate reads markdown prose, which is exactly what a docs-only diff # changes, so gating it on run_full would disable it on precisely the pull - # requests most able to trip it -- a fail-closed hole rather than an + # requests most able to trip it. That is a fail-closed hole rather than an # honest not-applicable. The self-test runs first and ungated so a broken # gate cannot mask a regression. - name: Test the purged-em-dash gate diff --git a/scripts/check-purged-em-dashes.sh b/scripts/check-purged-em-dashes.sh old mode 100644 new mode 100755 diff --git a/scripts/check-purged-em-dashes.test.sh b/scripts/check-purged-em-dashes.test.sh old mode 100644 new mode 100755 diff --git a/scripts/em-dash-purged-paths.txt b/scripts/em-dash-purged-paths.txt index 917bc0c99..44ee9f56e 100644 --- a/scripts/em-dash-purged-paths.txt +++ b/scripts/em-dash-purged-paths.txt @@ -61,6 +61,8 @@ plugins/naming/README.md plugins/naming/skills/*/SKILL.md plugins/overengineering/README.md plugins/overengineering/skills/*/SKILL.md +plugins/playwright/README.md +plugins/playwright/skills/*/SKILL.md plugins/prototype/README.md plugins/prototype/skills/*/SKILL.md plugins/verification/README.md From 84c224a75531c6d8c2886b62226f892f2b277859 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 24 Aug 2026 08:22:50 -0400 Subject: [PATCH 3/4] fix(scripts): correct two defects an independent review found in the gate A fresh-context reviewer audited the gate with the authoring rationale withheld and returned two defects that every automated pass had waved through. Both are fixed here, each with a regression case that fails against the previous code. --list ran the whole detector. The mode changed only what the expansion loop printed; control then fell through to the detector invocation and the check verdict, so a listing cost the same minutes as a full check and returned the check's exit code. That contradicted the allowlist header, which sends a reader to --list precisely to audit the declaration WITHOUT running the detector. It now stops after expansion, still failing on a stale entry, and the real allowlist lists in under a minute instead of nine. The coverage assertion double-counted a declined file. The gate proved the detector had accounted for exactly the files handed to it by adding the two totals in the detector's summary, but those totals overlap: a file the detector opens and then declines on an in-file marker is counted under BOTH "files scanned" and "files declined", while a file excluded by a config glob is never opened and appears only under the latter. An allowlisted path carrying an ai-slop-ignore-file marker therefore failed the run at exit 2, naming a coverage problem that did not exist, for using an exemption the allowlist explicitly promises. Unique files handled is now the scanned count plus the excluded-glob declines alone, read from the decline rows the detector already labels with a cause. Two smaller corrections come with them. A declared path that the tracked detector config excludes is now named in the output instead of being folded into the clean count: such a path sits inside the allowlist and outside enforcement at once, which is the one way this gate can report green over a surface it reads nothing on. And the clean verdict now reports files scanned rather than files declared, so the number means what it says. The workflow comment called an ungated docs-only diff a fail-closed hole when the hazard is the opposite, and the two enumerations of the unconditional whole-repo scanners had not been told about the fourth one. Refs #2891 Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 18 +++++----- scripts/check-purged-em-dashes.sh | 46 ++++++++++++++++++++---- scripts/check-purged-em-dashes.test.sh | 50 ++++++++++++++++++++++++-- 3 files changed, 98 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05ef6f01d..d45c60028 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -95,12 +95,14 @@ jobs: # inputs (.github/workflows/** and the named manifests) that a diff confined # to the docs-only allowlist (scripts/docs-only-paths.txt) cannot touch, so # on such a diff they report an honest evaluated-and-not-applicable success. - # ShellCheck, exec-bit, and hook-wiring-liveness are NOT + # ShellCheck, exec-bit, hook-wiring-liveness, and purged-em-dashes are NOT # gated: ShellCheck lints every tracked *.sh/*.bash, exec-bit flags every - # tracked shebang file recorded 100644, and hook-wiring-liveness greps - # .claude/hooks/*.sh against settings.json — a shell/shebang/hook file added + # tracked shebang file recorded 100644, hook-wiring-liveness greps + # .claude/hooks/*.sh against settings.json, and purged-em-dashes reads the + # markdown prose a docs-only diff is made of — a shell/shebang/hook file added # under an otherwise docs-only prefix like docs/topics/ is real input they - # must still catch — gating them would open a fail-closed hole. The job NEVER skips — only + # must still catch, and so is a docs-only diff that reintroduces an em dash on + # a surface already declared purged. The job NEVER skips — only # the path-scoped steps are gated, on `needs.scope.outputs.run_full` from the # `scope` lane, which resolves the diff once for the whole workflow and # carries the self-test and fail-closed guarantees (see that job). The gated @@ -288,9 +290,9 @@ jobs: # Deliberately UNCONDITIONAL, like the other whole-repo scanners. This # gate reads markdown prose, which is exactly what a docs-only diff # changes, so gating it on run_full would disable it on precisely the pull - # requests most able to trip it. That is a fail-closed hole rather than an - # honest not-applicable. The self-test runs first and ungated so a broken - # gate cannot mask a regression. + # requests most able to trip it. A regression would then reach main under + # a green check rather than an honest not-applicable. The self-test runs + # first and ungated so a broken gate cannot mask a regression. - name: Test the purged-em-dash gate run: bash scripts/check-purged-em-dashes.test.sh - name: Check purged surfaces stayed free of em dashes @@ -300,7 +302,7 @@ jobs: - name: Report docs-irrelevant checks not applicable to a docs-only diff if: needs.scope.outputs.run_full == 'false' - run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the path-scoped linters (actionlint, check-jsonschema x4, the manifest duplicate-key detector) cannot be affected — reporting success for them. ShellCheck, exec-bit, and hook-wiring-liveness scan the whole repo and stay unconditional." + run: echo "Diff is within the docs-only allowlist (scripts/docs-only-paths.txt); the path-scoped linters (actionlint, check-jsonschema x4, the manifest duplicate-key detector) cannot be affected — reporting success for them. ShellCheck, exec-bit, hook-wiring-liveness, and purged-em-dashes scan the whole repo and stay unconditional." - name: Test hygiene result aggregation run: scripts/aggregate-hygiene-results.sh --self-test diff --git a/scripts/check-purged-em-dashes.sh b/scripts/check-purged-em-dashes.sh index 23b8032d7..c49c518aa 100755 --- a/scripts/check-purged-em-dashes.sh +++ b/scripts/check-purged-em-dashes.sh @@ -67,9 +67,10 @@ # throwaway config ever stopped taking effect, rule-em-dash would be disabled, # the detector would report no em-dash findings, and this gate would pass # everything forever. So the run is only believed when the detector's own summary -# line for rule-em-dash is present AND reports disabled=0, and when the file -# count it says it handled matches the count handed to it. Any other shape is -# exit 2. +# line for rule-em-dash is present AND reports disabled=0, and when the files it +# accounts for are exactly the files handed to it. Any other shape is exit 2. +# That second count is derived rather than added up from the two summary +# totals, which overlap; see the comment above the arithmetic. # # Test injection, all defaulting to this repository: # EM_DASH_PURGED_ROOT repository root to scan @@ -141,6 +142,10 @@ trap 'rm -rf "$TMP"' EXIT FILES="$TMP/files.txt" : >"$FILES" stale=0 +# Initialized here rather than only where it is computed: the verdict section +# reads it, this script runs under `set -u`, and a future early return between +# the two would turn a clean run into an unbound-variable crash. +excluded=0 for glob in "${GLOBS[@]}"; do matched="$(git ls-files -z -- ":(glob)$glob" | tr '\0' '\n' | sed '/^$/d')" count=0 @@ -167,6 +172,16 @@ if ((EXPECTED == 0)); then exit 2 fi +# --list stops here, and stopping here is the point. The allowlist header tells +# a reader to audit the declaration by expanding it with --list and grepping the +# named files, WITHOUT running the detector. Falling through to the detector run +# would make that audit cost the same minutes as a full check and would report a +# check verdict under a listing flag. +if [[ "$MODE" == list ]]; then + printf 'check-purged-em-dashes: %s declared paths, %s files.\n' "${#GLOBS[@]}" "$EXPECTED" + exit 0 +fi + # --- Throwaway detector config ---------------------------------------------- # The tracked config verbatim, minus rule-em-dash's entry in disabled_rules. An # empty HOME keeps the user-global layer out: detect.sh cascades @@ -200,12 +215,21 @@ if [[ "$summary" != *" disabled=0"* ]]; then exit 2 fi -scanned="$(sed -n 's/^Summary total: [0-9]* findings across \([0-9]*\) files scanned (\([0-9]*\) files declined)$/\1 \2/p' "$OUT")" +# The detector's two counters OVERLAP, and reading them as disjoint is how this +# assertion misfires. "files scanned" counts every file the detector opened, +# which includes one it opened and then declined on an in-file marker; that file +# is counted a second time under "files declined". Only a file excluded by a +# config glob is never opened, so only that cause is missing from the scanned +# count. Unique files handled is therefore the scanned count plus the +# excluded-glob declines alone, and the detector names the cause on every +# decline row, so the gate reads those rather than inferring them. +scanned="$(sed -n 's/^Summary total: [0-9]* findings across \([0-9]*\) files scanned ([0-9]* files declined)$/\1/p' "$OUT")" if [[ -z "$scanned" ]]; then echo "check-purged-em-dashes: detector emitted no total summary; cannot confirm coverage" >&2 exit 2 fi -handled=$(($(echo "$scanned" | cut -d' ' -f1) + $(echo "$scanned" | cut -d' ' -f2))) +excluded="$(grep -c '^Declined: file=.* cause=excluded-glob$' "$OUT" || true)" +handled=$((scanned + excluded)) if ((handled != EXPECTED)); then echo "check-purged-em-dashes: detector handled $handled files but $EXPECTED were declared; coverage is not what the allowlist claims" >&2 exit 2 @@ -213,9 +237,19 @@ fi # --- Verdict ---------------------------------------------------------------- +# A declared path that the tracked config excludes is reported rather than +# folded into the clean count. Such a path is inside the allowlist and outside +# enforcement at the same time, which is the one way this gate can be green over +# a surface it is checking nothing on, and a silent count would hide it. +if ((excluded > 0)); then + echo "check-purged-em-dashes: $excluded declared file(s) are excluded by $SLOP_CONFIG and were NOT checked:" >&2 + sed -n 's/^Declined: file=\([^ ]*\) cause=excluded-glob$/ \1/p' "$OUT" >&2 + echo "check-purged-em-dashes: remove them from $ALLOWLIST or from the config's excluded_paths; declaring a path the detector never reads enforces nothing" >&2 +fi + findings="$(grep '^Finding: rule=ai-slop/audit/rule-em-dash ' "$OUT")" if [[ -z "$findings" ]]; then - printf 'check-purged-em-dashes: %s declared paths, %s files, no em dashes.\n' "${#GLOBS[@]}" "$EXPECTED" + printf 'check-purged-em-dashes: %s declared paths, %s files scanned, no em dashes.\n' "${#GLOBS[@]}" "$scanned" exit 0 fi diff --git a/scripts/check-purged-em-dashes.test.sh b/scripts/check-purged-em-dashes.test.sh index f43aa543a..77161bd31 100755 --- a/scripts/check-purged-em-dashes.test.sh +++ b/scripts/check-purged-em-dashes.test.sh @@ -87,6 +87,25 @@ printf '%s\n' \ write "other/unlisted.md" "Not on the allowlist ${EM} so not this gate's business." +# A file the detector opens and then declines on an in-file marker. The detector +# counts such a file BOTH as scanned and as declined, so a gate that adds those +# two totals sees one file too many and reports a coverage mismatch on a file the +# allowlist legitimately names. The marker is documented as an exemption, so it +# has to be usable from an allowlisted path. +printf '%s\n' \ + '' \ + '' \ + "Wholly declined text ${EM} not this gate's business either." \ + >"$REPO/surface/marked.md" + +# The gate's sibling accounting path, an excluded_paths glob, is deliberately +# NOT exercised here. detect.sh reads that array through a jq call whose output +# carries CRLF under the Windows build of jq, so every element arrives with a +# trailing carriage return and matches nothing; a case asserting the exclusion +# took effect would pass on the Linux runner and fail on a Windows workstation. +# That is a defect in the detector's own config reader rather than in this gate, +# and pinning it from here would only make this suite platform-dependent. + list() { local name="$1" shift @@ -94,6 +113,7 @@ list() { } list "only-clean" 'surface/clean.md' +list "marked" 'surface/clean.md' 'surface/marked.md' list "clean-and-data" 'surface/clean.md' 'surface/data.md' list "with-dirty" 'surface/clean.md' 'surface/dirty.md' list "globbed" 'surface/*.md' @@ -120,8 +140,12 @@ else fi # An unlisted file is unenforced: that is the allowlist's defining property, and # asserting it here is what stops a later "just scan everything" edit from -# passing this suite. -if [[ "$clean_out" != *"unlisted.md"* ]]; then +# passing this suite. Asserting the file is merely ABSENT from the output does +# not pin that, because a clean run prints one summary line and names nothing; +# the scanned count is what distinguishes "one file was checked" from "the whole +# fixture was checked and happened to be clean apart from the listed one". +if [[ "$clean_out" != *"unlisted.md"* ]] && + [[ "$clean_out" == *"1 declared paths, 1 files scanned"* ]]; then ok "a file outside the allowlist is not scanned" else fail "a file outside the allowlist is not scanned: $clean_out" @@ -160,6 +184,18 @@ else fail "em dashes in a code fence and an ignore-marked line do not fire (rc=$RC): $OUT" fi +# A whole-file ignore marker is one of the exemptions this gate documents, so an +# allowlisted path is allowed to carry one. The detector counts such a file both +# as scanned and as declined, and a gate that reads those two totals as disjoint +# concludes it was handed one file too many and refuses the run at exit 2 — +# green surfaces reported broken because of an exemption they were promised. +run "$REPO" "marked.txt" +if ((RC == 0)); then + ok "a whole-file ignore marker does not break the coverage assertion" +else + fail "a whole-file ignore marker does not break the coverage assertion (rc=$RC): $OUT" +fi + run "$REPO" "globbed.txt" if ((RC == 1)) && [[ "$OUT" == *"surface/dirty.md"* ]]; then ok "a glob entry expands to every matching tracked file" @@ -201,6 +237,16 @@ if ((RC == 0)) && [[ "$OUT" == *"surface/clean.md"* ]]; then else fail "--list reports each declared entry (rc=$RC): $OUT" fi +# --list is the cheap audit the allowlist header sends a reader to, so it must +# stop at the expansion. Pointed at the seeded violation it still lists rather +# than judging: no detector verdict in the output, and exit 0 for a listing that +# expanded cleanly even though a check of the same allowlist would exit 1. +run "$REPO" "with-dirty.txt" --list +if ((RC == 0)) && [[ "$OUT" != *"no em dashes"* ]] && [[ "$OUT" != *"em-dash line(s)"* ]]; then + ok "--list expands the allowlist without running the detector" +else + fail "--list expands the allowlist without running the detector (rc=$RC): $OUT" +fi run "$REPO" "only-clean.txt" --bogus if ((RC == 2)); then From af46aa6c327b868235160e21269457a10140f075 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:25:19 -0400 Subject: [PATCH 4/4] fix(scripts): close two ways the purged-em-dash gate could pass unenforced Two switches in the tracked config could leave a declared path green without being judged, and the gate reported neither as a failure. `em_dash_allowed_paths` and `rule_allowed_paths["rule-em-dash"]` exempt a file from this one rule. The derived config copied both through, and the exemption is invisible in the run it produces: detect.sh still OPENS such a file, so it lands in the scanned count, the coverage assertion still balances, and the rule summary still reports `disabled=0`. Every signal the gate reads says the surface was checked and clean while no finding on it was ever possible. The derivation now strips both, alongside the `disabled_rules` entry it already stripped, because a path on the allowlist declares the surface purged and a per-rule exemption on that path is the opposite claim. A declared file that genuinely carries the character as data still has the detector's in-file exemptions, and failing those it belongs off the allowlist with the reason. `excluded_paths` is the one exclusion left standing, since its files are never opened and so remain visible as excluded-glob declines. The gate named them and then fell through to the clean path, printing "no em dashes" and exiting 0. A declaration the detector never reads is not coverage, and a green exit is how it gets mistaken for some, so that count is now a nonzero verdict with a message that distinguishes it from a clean run. Four cases cover it, each pointed at the seeded violation, which is the only fixture where suppression is observable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01N2RpNNVGx3wMp8oFeFVBSA --- scripts/check-purged-em-dashes.sh | 69 ++++++++++++++++++----- scripts/check-purged-em-dashes.test.sh | 76 ++++++++++++++++++++++---- 2 files changed, 118 insertions(+), 27 deletions(-) diff --git a/scripts/check-purged-em-dashes.sh b/scripts/check-purged-em-dashes.sh index c49c518aa..b925a3f05 100755 --- a/scripts/check-purged-em-dashes.sh +++ b/scripts/check-purged-em-dashes.sh @@ -6,8 +6,9 @@ # scripts/check-purged-em-dashes.sh --check same (explicit form, matches sibling gates) # scripts/check-purged-em-dashes.sh --list list every declared path and its verdict # -# Exit: 0 clean, 1 a violation or a stale allowlist entry, 2 usage or a -# prerequisite this gate cannot verify around. +# Exit: 0 clean, 1 a violation, a stale allowlist entry, or a declared path the +# tracked config puts out of enforcement, 2 usage or a prerequisite this gate +# cannot verify around. # # WHY (#2891). The de-slop campaign rewrites prose surface by surface, and each # landed shard is paid for by hand: a mechanical em-dash split changes meaning @@ -47,12 +48,26 @@ # (#2891, checkbox 4). So this gate does not touch that file, and running it # changes nothing about what /ai-slop:audit reports. It instead builds a # THROWAWAY config layer for its own detector invocation: the tracked config -# copied verbatim, with rule-em-dash removed from disabled_rules and nothing else -# altered. Copying rather than synthesizing is deliberate: excluded_paths, -# em_dash_allowed_paths and every threshold stay whatever the tracked file says, -# so the vendor, catalog and eval-fixture exclusions that exist precisely because -# they contain em dashes as DATA keep applying here, and keep applying without a -# second copy of that list to drift. +# copied, with every switch that can quiet rule-em-dash removed and nothing else +# altered. Copying rather than synthesizing is deliberate: excluded_paths and +# every threshold stay whatever the tracked file says, so the vendor, catalog +# and eval-fixture exclusions that exist precisely because they contain em +# dashes as DATA keep applying here, and keep applying without a second copy of +# that list to drift. +# +# WHAT THE THROWAWAY LAYER DOES OVERRIDE is exactly the set of keys that would +# let a declared path pass without being judged: rule-em-dash's entry in +# disabled_rules, em_dash_allowed_paths, and rule_allowed_paths["rule-em-dash"]. +# A path on the allowlist is a claim that the surface is purged; a per-rule +# exemption on the same path is the opposite claim, and honouring it would let +# the gate report the surface clean while no finding on it was ever possible, +# because such a file is still opened and still counted as scanned. +# excluded_paths is the one exclusion left standing, because its files are +# never opened at all and are therefore visible in the run as declines that the +# verdict names and fails on, rather than folded silently into the clean count. +# A file on this allowlist that genuinely carries the character as data still +# has the detector's in-file exemptions, an ignore marker or a code fence, and +# failing those it belongs off the allowlist with the reason. # # REUSES THE DETECTOR RATHER THAN GREPPING. A bare grep for the em-dash byte # sequence would fire inside fenced code blocks, inline code spans, and @@ -183,13 +198,29 @@ if [[ "$MODE" == list ]]; then fi # --- Throwaway detector config ---------------------------------------------- -# The tracked config verbatim, minus rule-em-dash's entry in disabled_rules. An +# The tracked config with every switch that can quiet rule-em-dash removed. An # empty HOME keeps the user-global layer out: detect.sh cascades # $HOME/.claude/ai-slop.json under the repo layer, and a contributor who happens # to carry one must not be able to change this gate's verdict. +# +# Three keys can silence this rule and all three are stripped, because a path on +# the allowlist declares the surface purged and a per-rule exemption claims it +# need not be. `disabled_rules` turns the rule off outright. +# `em_dash_allowed_paths` and `rule_allowed_paths["rule-em-dash"]` turn it off +# per file, and those two are the quieter hazard: detect.sh still OPENS such a +# file, so it lands in the scanned count, the coverage assertion below still +# balances, and the rule summary still reports `disabled=0`. Nothing in the run +# says why no finding was possible, and the gate would call the surface checked +# and clean. +# +# `excluded_paths` is deliberately NOT stripped. Those files are never opened, +# so they surface as excluded-glob declines that the verdict below names and +# fails on, which is the report this gate wants rather than a silent override. mkdir -p "$TMP/root/.claude" "$TMP/home" || exit 2 -if ! jq '.disabled_rules |= ((. // []) | map(select(. != "rule-em-dash")))' \ +if ! jq '.disabled_rules |= ((. // []) | map(select(. != "rule-em-dash"))) + | del(.em_dash_allowed_paths) + | .rule_allowed_paths |= ((. // {}) | del(."rule-em-dash"))' \ "$SLOP_CONFIG" >"$TMP/root/.claude/ai-slop.json"; then echo "check-purged-em-dashes: could not derive the detector config from $SLOP_CONFIG" >&2 exit 2 @@ -237,20 +268,28 @@ fi # --- Verdict ---------------------------------------------------------------- -# A declared path that the tracked config excludes is reported rather than -# folded into the clean count. Such a path is inside the allowlist and outside +# A declared path that the tracked config excludes FAILS the gate rather than +# folding into the clean count. Such a path is inside the allowlist and outside # enforcement at the same time, which is the one way this gate can be green over -# a surface it is checking nothing on, and a silent count would hide it. +# a surface it is checking nothing on. Reporting it is not enough: an unenforced +# declaration reads as coverage to every consumer of this exit code, so only a +# nonzero verdict keeps it from shipping as one. +verdict=0 if ((excluded > 0)); then echo "check-purged-em-dashes: $excluded declared file(s) are excluded by $SLOP_CONFIG and were NOT checked:" >&2 sed -n 's/^Declined: file=\([^ ]*\) cause=excluded-glob$/ \1/p' "$OUT" >&2 echo "check-purged-em-dashes: remove them from $ALLOWLIST or from the config's excluded_paths; declaring a path the detector never reads enforces nothing" >&2 + verdict=1 fi findings="$(grep '^Finding: rule=ai-slop/audit/rule-em-dash ' "$OUT")" if [[ -z "$findings" ]]; then - printf 'check-purged-em-dashes: %s declared paths, %s files scanned, no em dashes.\n' "${#GLOBS[@]}" "$scanned" - exit 0 + if ((verdict == 0)); then + printf 'check-purged-em-dashes: %s declared paths, %s files scanned, no em dashes.\n' "${#GLOBS[@]}" "$scanned" + else + echo "check-purged-em-dashes: the $scanned file(s) that were checked carry no em dashes, but the excluded file(s) above were not checked at all" >&2 + fi + exit "$verdict" fi count="$(printf '%s\n' "$findings" | wc -l | tr -d ' ')" diff --git a/scripts/check-purged-em-dashes.test.sh b/scripts/check-purged-em-dashes.test.sh index 77161bd31..55562329c 100755 --- a/scripts/check-purged-em-dashes.test.sh +++ b/scripts/check-purged-em-dashes.test.sh @@ -36,22 +36,27 @@ EM="$(printf '\xe2\x80\x94')" TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT -# run [args...]: sets OUT and RC. +# run_cfg [args...]: +# sets OUT and RC. The config is a parameter because three of the cases below +# turn on what the SUT derives from a DIFFERENT tracked config than the default +# one; every other case goes through run(), which pins the default. # Deliberately not wrapped in a command substitution of its own: that forks a # subshell and the exit status never reaches the caller. -run() { - local root="$1" list="$2" - shift 2 +run_cfg() { + local cfg="$1" root="$2" list="$3" + shift 3 OUT="$( EM_DASH_PURGED_ROOT="$root" \ EM_DASH_PURGED_PATHS="$list" \ - EM_DASH_SLOP_CONFIG=".claude/ai-slop.json" \ + EM_DASH_SLOP_CONFIG="$cfg" \ EM_DASH_DETECT="$DETECT" \ bash "$SUT" "$@" 2>&1 )" RC=$? } +run() { run_cfg ".claude/ai-slop.json" "$@"; } + # --- Fixture ---------------------------------------------------------------- # One repository, several allowlists pointed at different subsets of it, so each # case selects the surface it means to assert on rather than rebuilding a repo. @@ -98,13 +103,14 @@ printf '%s\n' \ "Wholly declined text ${EM} not this gate's business either." \ >"$REPO/surface/marked.md" -# The gate's sibling accounting path, an excluded_paths glob, is deliberately -# NOT exercised here. detect.sh reads that array through a jq call whose output -# carries CRLF under the Windows build of jq, so every element arrives with a -# trailing carriage return and matches nothing; a case asserting the exclusion -# took effect would pass on the Linux runner and fail on a Windows workstation. -# That is a defect in the detector's own config reader rather than in this gate, -# and pinning it from here would only make this suite platform-dependent. +# Three more tracked configs, each carrying one switch that can quiet +# rule-em-dash on an allowlisted file. The SUT strips the first two when it +# derives its throwaway layer and fails the run on the third, so each of these +# is pointed at the SEEDED VIOLATION: a config that could suppress the finding +# is the only fixture in which stripping is observable. +write ".claude/cfg-em-dash-allowed.json" '{"excluded_paths":[],"em_dash_allowed_paths":["surface/dirty.md"],"disabled_rules":["rule-em-dash"]}' +write ".claude/cfg-rule-allowed.json" '{"excluded_paths":[],"rule_allowed_paths":{"rule-em-dash":["surface/dirty.md"]},"disabled_rules":["rule-em-dash"]}' +write ".claude/cfg-excluded.json" '{"excluded_paths":["surface/dirty.md"],"disabled_rules":["rule-em-dash"]}' list() { local name="$1" @@ -265,4 +271,50 @@ else fail "an unreadable detector config is exit 2 (rc=$RC): $OUT" fi +# --- Switches that could quiet rule-em-dash on a declared path --------------- +# +# Each of the three runs below uses with-dirty.txt, whose surface/dirty.md the +# base case above proves is a real finding. So each asserts on the ONE thing +# its config changes, against a fixture where suppression would be visible. + +# em_dash_allowed_paths exempts a single file from rule-em-dash alone. The +# detector still opens the file, so it counts as scanned, the coverage +# assertion still balances and the rule summary still says disabled=0: nothing +# in a passing run would have said the file was never actually judged. The SUT +# strips the key rather than trusting those counters to reveal it. +run_cfg ".claude/cfg-em-dash-allowed.json" "$REPO" "with-dirty.txt" +if ((RC == 1)) && [[ "$OUT" == *"surface/dirty.md"* ]]; then + ok "em_dash_allowed_paths does not exempt a declared path" +else + fail "em_dash_allowed_paths does not exempt a declared path (rc=$RC, want 1): $OUT" +fi + +# rule_allowed_paths is the generalized form of the same switch, keyed by rule +# slug, and reaches the detector through a different reader. Stripping one and +# not the other would leave the identical hole under a second spelling. +run_cfg ".claude/cfg-rule-allowed.json" "$REPO" "with-dirty.txt" +if ((RC == 1)) && [[ "$OUT" == *"surface/dirty.md"* ]]; then + ok "rule_allowed_paths[rule-em-dash] does not exempt a declared path" +else + fail "rule_allowed_paths[rule-em-dash] does not exempt a declared path (rc=$RC, want 1): $OUT" +fi + +# excluded_paths is handled the other way round: the file is never opened, so +# the SUT cannot judge it and does not pretend to. It reports the path and +# FAILS, because a declaration the detector never reads is not coverage, and a +# green exit is exactly how it would be mistaken for some. +run_cfg ".claude/cfg-excluded.json" "$REPO" "with-dirty.txt" +if ((RC == 1)) && [[ "$OUT" == *"were NOT checked"* ]] && [[ "$OUT" == *"surface/dirty.md"* ]]; then + ok "a declared path the config excludes fails the gate and is named" +else + fail "a declared path the config excludes fails the gate and is named (rc=$RC, want 1): $OUT" +fi +# ... and the failure is the exclusion, not the em dash that happens to be in +# that file: the detector never read it, so it can report no finding on it. +if [[ "$OUT" != *"em-dash line(s)"* ]]; then + ok "the excluded path fails as unchecked rather than as a finding" +else + fail "the excluded path fails as unchecked rather than as a finding: $OUT" +fi + test_harness::report