From 05e2dd08ba9f81796441e4c2ce77fe33eeabb5bf Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:36:09 -0400 Subject: [PATCH 1/3] feat(ci): wire the contract-slice prune gate the topic-docs convention specifies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1417 ## Summary `docs/conventions/topic-docs/README.md` specifies a required check that a merged PR carries no path under the contract-slice dir. The check was never built, so the convention has been unenforced for its entire life and 17 slices reached `main` — 6 of them on a single day. The only place `docs/topics/` reached CI at all was `scripts/docs-only-paths.txt`, as a docs-only ALLOWLIST entry, which makes such a PR cheaper to merge rather than blocking it. Evidence the rule is real and was being enforced by hand: PR #1286 was closed rather than merged, explicitly because its content was contract tier under `docs/topics/`. ## The deletion exemption The convention's own step 4 is a final commit that PRUNES the slice, so a literal "no path under the contract dir appears in the diff" reading would red-line the very commit that satisfies it. This gate keys on where a path LANDS: removals pass, a history-preserving `git mv` out of the contract dir (step 3's graduation) passes, and only an add, edit, or rename-into is red-lined. That requires knowing a path's status, which `--name-only` cannot express, so the gate reads `--name-status`. Deliberate deviation from the letter of the convention in service of its intent; the three-dot `base...HEAD` range is unchanged. ## Existing debt The 17 pre-existing slices are grandfathered by slug in `scripts/contract-slice-baseline.txt`, using the same stale-guarded idiom as `changelog-parity-baseline.txt` and `orphaned-fixtures-baseline.txt`: `--check` fails on an entry whose slice no longer exists, so an exemption cannot outlive its debt and a future slice cannot inherit a grandfathered slug. Graduating and pruning them is tracked separately. This is why the gate can land now instead of after a 71-file cleanup: it stops the bleed immediately while each slice graduates on its own PR, by whoever owns it. ## Verification The 11-case suite covers the add, pure-deletion, untouched, grandfathered, new-slug-despite-baseline, graduation-out, rename-into, unresolvable-base, live-baseline, stale-baseline, and usage paths. Measured against the four open PRs that carry `docs/topics/` paths, rather than asserted: #1318, #1252, and #1096 pass on their baseline exemptions; #1400 fails, correctly, because it adds two slices that are not pre-existing debt. Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 31 ++++ scripts/check-contract-slice-prune.sh | 170 +++++++++++++++++++++ scripts/check-contract-slice-prune.test.sh | 139 +++++++++++++++++ scripts/contract-slice-baseline.txt | 38 +++++ 4 files changed, 378 insertions(+) create mode 100755 scripts/check-contract-slice-prune.sh create mode 100755 scripts/check-contract-slice-prune.test.sh create mode 100644 scripts/contract-slice-baseline.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9e367730..574fc7e22 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -422,6 +422,36 @@ jobs: BASE_REF: ${{ github.base_ref }} run: scripts/check-changelog-parity.sh --check-bump "origin/$BASE_REF" + # Contract-slice prune: docs/topics// is Contract tier per + # docs/conventions/topic-docs/README.md — committed on a task branch only, + # pruned before merge. The convention specified this required check but it was + # never wired, so 17 slices reached main (#1417). The gate keys on where a path + # LANDS, so the prune commit itself and a history-preserving graduation out of + # docs/topics/ both pass; only an add, edit, or rename-into is red-lined. The + # pre-existing 17 are grandfathered by slug in + # scripts/contract-slice-baseline.txt with a stale guard, and burn down under + # #1419. The self-test runs unconditionally so a broken gate cannot mask a + # regression, and the PR-diff step is event-gated. + contract-slice-prune-gate: + runs-on: ubuntu-24.04 + timeout-minutes: 15 + steps: + - name: Check out + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + # Full history so the PR base ref is resolvable for the diff gate. + fetch-depth: 0 + - name: Test the contract-slice prune gate + run: bash scripts/check-contract-slice-prune.test.sh + - name: Verify no baseline entry outlives its slice + run: scripts/check-contract-slice-prune.sh --check + - name: Verify the change set prunes its contract slice + if: github.event_name == 'pull_request' + env: + BASE_REF: ${{ github.base_ref }} + run: scripts/check-contract-slice-prune.sh --check-diff "origin/$BASE_REF" + # Deep plugin-contract lane and the heaviest suite (Node + Python installs, # every plugins/**/*.test.sh, manifest + catalog validation). A PR whose diff # is confined to the docs-only allowlist (scripts/docs-only-paths.txt) cannot @@ -714,6 +744,7 @@ jobs: - silent-skip-gate - orphaned-fixture-gate - changelog-parity-gate + - contract-slice-prune-gate - plugin-gate - miro-plugin - youtube-extraction diff --git a/scripts/check-contract-slice-prune.sh b/scripts/check-contract-slice-prune.sh new file mode 100755 index 000000000..549bdf046 --- /dev/null +++ b/scripts/check-contract-slice-prune.sh @@ -0,0 +1,170 @@ +#!/usr/bin/env bash +# Enforce the topic-docs contract-slice prune step. +# +# scripts/check-contract-slice-prune.sh --check fail if a baseline +# entry no longer +# names a slice that +# exists +# scripts/check-contract-slice-prune.sh --check-diff fail if the change +# set leaves any path +# under the contract +# dir added, edited, +# renamed-into, or +# copied-into +# +# docs/conventions/topic-docs/README.md specifies a required check that the net +# PR diff carries no path under the resolved contract dir (default docs/topics/). +# The convention was written but never wired, and 17 slices reached main as a +# result (#1417). +# +# Deletion is the exemption, not an oversight. The convention's own step 4 is a +# final commit that PRUNES the slice, so a literal "no path under docs/topics/ +# appears in the diff" reading would red-line the very commit that satisfies the +# convention. This gate therefore keys on where a path LANDS, not on whether it +# appears: a diff may remove paths under the contract dir freely, and may +# `git mv` one OUT of it (that is step 3's history-preserving graduation), but it +# may not leave one behind. +# +# The convention formulates the check as `git diff --name-only base...head`. +# --name-only cannot distinguish a deletion from an addition, so this gate reads +# --name-status instead. That is a deliberate deviation from the letter of the +# convention in service of its intent; the three-dot base...HEAD range is +# unchanged. +# +# Existing debt is grandfathered by SLUG in scripts/contract-slice-baseline.txt +# (the same stale-guarded idiom as scripts/changelog-parity-baseline.txt and +# scripts/orphaned-fixtures-baseline.txt): --check-diff exempts a listed slug so +# the gate can land without red-lining the open PRs that already carry those +# paths, and --check fails on a STALE entry — one whose slice no longer exists — +# so an exemption cannot outlive the debt it covers. Graduating a slice is +# tracked as #1419; each slice's prune PR drops its own baseline line. +# +# Fail-closed: an unresolvable base ref, or a diff that cannot be computed, exits +# non-zero rather than passing unchecked. CONTRACT_SLICE_DIR and +# CONTRACT_SLICE_BASELINE override the contract dir and baseline path (test +# injection). +set -uo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 2 + +CONTRACT_DIR="${CONTRACT_SLICE_DIR:-docs/topics}" +CONTRACT_DIR="${CONTRACT_DIR%/}" +BASELINE="${CONTRACT_SLICE_BASELINE:-scripts/contract-slice-baseline.txt}" + +mode="${1:-}" +case "$mode" in +--check | --check-diff) ;; +*) + echo "usage: $(basename "$0") [--check | --check-diff ]" >&2 + exit 2 + ;; +esac + +# Grandfathered slice slugs (directory names directly under the contract dir). +declare -A grandfathered +if [[ -f "$BASELINE" ]]; then + while IFS= read -r line; do + line="${line%%#*}" + line="${line#"${line%%[![:space:]]*}"}" + line="${line%"${line##*[![:space:]]}"}" + [[ -z "$line" ]] && continue + grandfathered["$line"]=1 + done <"$BASELINE" +fi + +# Map a repo path to the slice slug that owns it, or empty if the path is not +# under the contract dir. "docs/topics/foo/design/x.md" -> "foo". +slug_of() { + local path="$1" + case "$path" in + "$CONTRACT_DIR"/*) + local rest="${path#"$CONTRACT_DIR"/}" + printf '%s' "${rest%%/*}" + ;; + *) printf '' ;; + esac +} + +if [[ "$mode" == "--check" ]]; then + stale=0 + for slug in "${!grandfathered[@]}"; do + if [[ ! -d "$CONTRACT_DIR/$slug" ]]; then + echo "STALE BASELINE: '$slug' in $BASELINE no longer names a slice under $CONTRACT_DIR/ — remove the line." >&2 + stale=1 + fi + done + if ((stale)); then + echo "" >&2 + echo "A baseline entry outliving its slice would silently re-open the exemption for a future slice reusing that slug." >&2 + exit 1 + fi + echo "Every $BASELINE entry still names an existing slice under $CONTRACT_DIR/ ($(("${#grandfathered[@]}")) grandfathered)." + exit 0 +fi + +# --check-diff mode +if [[ -z "${2:-}" ]]; then + echo "usage: $(basename "$0") --check-diff " >&2 + exit 2 +fi +base="$2" +if ! git rev-parse --verify --quiet "${base}^{commit}" >/dev/null; then + echo "check-contract-slice-prune: base ref '$base' is not a resolvable commit." >&2 + exit 2 +fi + +# Three-dot base...HEAD is diff(merge-base(base,HEAD), HEAD) — only the commits +# unique to this branch. A slice that main gained after this branch forked is +# therefore out of scope, so an untouched stale branch is never forced to +# merge-from-main over someone else's violation. +# +# Read via COMMAND substitution, not process substitution: this is a required +# merge check and a git failure must fail loud. Process substitution swallows +# git's exit status, so a diff that genuinely cannot be computed (no common +# ancestor -> "fatal: no merge base", exit 128) would yield empty output and let +# the gate exit 0 without checking anything. A legitimate empty diff succeeds +# with empty output and correctly finds no violations. +if ! diff_status="$(git diff --name-status --find-renames "$base...HEAD")"; then + echo "check-contract-slice-prune: 'git diff --name-status $base...HEAD' failed (no common ancestor between '$base' and HEAD, or history not fetched deeply enough); refusing to pass without checking." >&2 + exit 2 +fi + +violations=() +exempted=() +while IFS=$'\t' read -r status path dest; do + [[ -z "$status" ]] && continue + # Rename and copy carry two paths; what matters is where the content LANDS, so + # the destination is the path under test. The source side of a rename out of + # the contract dir is a graduation (`git mv` to docs/adr/) and must pass. + case "$status" in + R* | C*) landed="$dest" ;; + D) continue ;; + *) landed="$path" ;; + esac + [[ -z "$landed" ]] && continue + + slug="$(slug_of "$landed")" + [[ -z "$slug" ]] && continue + + if [[ -n "${grandfathered[$slug]:-}" ]]; then + exempted+=("$landed") + else + violations+=("$status $landed") + fi +done <<<"$diff_status" + +if ((${#violations[@]})); then + echo "Contract-slice prune gate FAILED — this change set leaves ${#violations[@]} path(s) under $CONTRACT_DIR/:" >&2 + printf ' %s\n' "${violations[@]}" >&2 + echo "" >&2 + echo "$CONTRACT_DIR// is Contract tier per docs/conventions/topic-docs/README.md: committed on a task branch only, pruned before merge." >&2 + echo "Before merging, graduate the durable outcomes (ADR / spec / tracker item) and delete the slice — the deletion itself passes this gate." >&2 + exit 1 +fi + +if ((${#exempted[@]})); then + echo "Contract-slice prune gate passed — ${#exempted[@]} path(s) under $CONTRACT_DIR/ exempted by $BASELINE (see #1419)." +else + echo "Contract-slice prune gate passed — this change set leaves no path under $CONTRACT_DIR/." +fi +exit 0 diff --git a/scripts/check-contract-slice-prune.test.sh b/scripts/check-contract-slice-prune.test.sh new file mode 100755 index 000000000..c504c671f --- /dev/null +++ b/scripts/check-contract-slice-prune.test.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# Unit tests for check-contract-slice-prune.sh. Each scenario builds a throwaway +# git repo, commits a base, applies a branch change, and asserts the gate's +# verdict. The deletion and graduation cases are the ones that matter most: a +# gate that red-lined the prune commit would forbid the very step the topic-docs +# convention requires. +set -uo pipefail + +SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SELF_DIR/check-contract-slice-prune.sh" + +PASS=0 +FAIL=0 +fail() { + echo "FAIL: $*" >&2 + FAIL=$((FAIL + 1)) +} +ok() { + echo "ok: $*" + PASS=$((PASS + 1)) +} + +git_q() { git -c user.email=t@t -c user.name=t -c commit.gpgsign=false "$@" >/dev/null 2>&1; } + +# mk_repo : throwaway git repo with the gate installed, one +# base commit on a `base` branch, and a checked-out `work` branch. +mk_repo() { + local dir + dir="$(mktemp -d)" + mkdir -p "$dir/scripts" "$dir/docs/topics" + cp "$SCRIPT" "$dir/scripts/check-contract-slice-prune.sh" + printf '%s' "${1:-}" >"$dir/scripts/contract-slice-baseline.txt" + printf 'seed\n' >"$dir/README.md" + ( + cd "$dir" || exit 1 + git_q init -b base + git_q add -A + git_q commit -m base + git_q checkout -b work + ) + printf '%s' "$dir" +} + +# commit everything currently in the tree onto the work branch +commit_work() { (cd "$1" && git_q add -A && git_q commit -m work); } + +run_diff() { (cd "$1" && bash scripts/check-contract-slice-prune.sh --check-diff base 2>&1); } +run_check() { (cd "$1" && bash scripts/check-contract-slice-prune.sh --check 2>&1); } + +# --- adding a new slice is the core violation ------------------------------ +repo="$(mk_repo)" +mkdir -p "$repo/docs/topics/newslug" +printf 'plan\n' >"$repo/docs/topics/newslug/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then fail "an added slice must be red-lined"; else ok "added slice is red-lined"; fi +rm -rf "$repo" + +# --- deleting a slice is the prune step and must pass ---------------------- +repo="$(mk_repo)" +mkdir -p "$repo/docs/topics/oldslug" +printf 'plan\n' >"$repo/docs/topics/oldslug/PLAN.md" +(cd "$repo" && git_q add -A && git_q commit -m seed-slice && git_q checkout base && git_q merge work && git_q checkout work) +rm -rf "${repo:?}/docs/topics/oldslug" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "pure deletion passes (the prune commit)"; else fail "the prune commit must not be red-lined"; fi +rm -rf "$repo" + +# --- a change set that never touches the contract dir passes --------------- +repo="$(mk_repo)" +printf 'edit\n' >>"$repo/README.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "untouched contract dir passes"; else fail "unrelated change wrongly red-lined"; fi +rm -rf "$repo" + +# --- a grandfathered slug is exempt from edits AND additions --------------- +repo="$(mk_repo $'# c\nlegacy\n')" +mkdir -p "$repo/docs/topics/legacy" +printf 'plan\n' >"$repo/docs/topics/legacy/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "grandfathered slug is exempt"; else fail "baseline entry failed to exempt its slug"; fi +rm -rf "$repo" + +# --- a NEW slug is still red-lined while a baseline exists ----------------- +repo="$(mk_repo $'# c\nlegacy\n')" +mkdir -p "$repo/docs/topics/brandnew" +printf 'plan\n' >"$repo/docs/topics/brandnew/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then fail "a non-baselined slug must still be red-lined"; else ok "baseline does not blanket-exempt new slugs"; fi +rm -rf "$repo" + +# --- graduation: git mv OUT of the contract dir must pass ------------------ +repo="$(mk_repo)" +mkdir -p "$repo/docs/topics/grad" "$repo/docs/adr" +printf 'a durable decision worth graduating, long enough to score as a rename\n' >"$repo/docs/topics/grad/PLAN.md" +(cd "$repo" && git_q add -A && git_q commit -m seed-slice && git_q checkout base && git_q merge work && git_q checkout work) +(cd "$repo" && git_q mv docs/topics/grad/PLAN.md docs/adr/0005-decision.md) +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "graduation out of the contract dir passes"; else fail "history-preserving graduation must not be red-lined"; fi +rm -rf "$repo" + +# --- a rename INTO the contract dir is still a violation ------------------- +repo="$(mk_repo)" +printf 'content that will be moved into the contract dir, long enough to rename-score\n' >"$repo/docs/stray.md" +(cd "$repo" && git_q add -A && git_q commit -m seed && git_q checkout base && git_q merge work && git_q checkout work) +mkdir -p "$repo/docs/topics/moved" +(cd "$repo" && git_q mv docs/stray.md docs/topics/moved/PLAN.md) +commit_work "$repo" +if run_diff "$repo" >/dev/null; then fail "a rename INTO the contract dir must be red-lined"; else ok "rename into the contract dir is red-lined"; fi +rm -rf "$repo" + +# --- fail-closed on an unresolvable base ref ------------------------------- +repo="$(mk_repo)" +out="$(cd "$repo" && bash scripts/check-contract-slice-prune.sh --check-diff no/such/ref 2>&1)" +rc=$? +if ((rc == 2)) && [[ "$out" == *"not a resolvable commit"* ]]; then ok "unresolvable base ref exits 2"; else fail "unresolvable base ref must exit 2, got rc=$rc"; fi +rm -rf "$repo" + +# --- --check: a live baseline entry passes --------------------------------- +repo="$(mk_repo $'legacy\n')" +mkdir -p "$repo/docs/topics/legacy" +printf 'plan\n' >"$repo/docs/topics/legacy/PLAN.md" +if run_check "$repo" >/dev/null; then ok "--check passes while the slice exists"; else fail "--check wrongly flagged a live baseline entry"; fi +rm -rf "$repo" + +# --- --check: a stale baseline entry fails --------------------------------- +repo="$(mk_repo $'ghost\n')" +out="$(run_check "$repo")" +if [[ "$out" == *"STALE BASELINE"* ]]; then ok "--check fails on a stale baseline entry"; else fail "a baseline entry outliving its slice must fail --check"; fi +rm -rf "$repo" + +# --- usage ------------------------------------------------------------------ +repo="$(mk_repo)" +(cd "$repo" && bash scripts/check-contract-slice-prune.sh --bogus >/dev/null 2>&1) +if (($? == 2)); then ok "unknown mode exits 2"; else fail "unknown mode must exit 2"; fi +rm -rf "$repo" + +echo "" +echo "check-contract-slice-prune.test.sh: $PASS passed, $FAIL failed" +((FAIL == 0)) diff --git a/scripts/contract-slice-baseline.txt b/scripts/contract-slice-baseline.txt new file mode 100644 index 000000000..c2c6425ec --- /dev/null +++ b/scripts/contract-slice-baseline.txt @@ -0,0 +1,38 @@ +# Grandfathered contract slices — the docs/topics// directories that +# reached main before check-contract-slice-prune.sh existed, and which this gate +# must not red-line. One slice SLUG per line, matched as the first path segment +# under the contract dir (comments and blank lines ignored). A listed slug +# exempts every path beneath it from --check-diff. +# +# This is a snapshot of the debt at the moment the gate landed (#1417), so a NEW +# slice matches no line and is red-lined rather than grandfathered. That is the +# whole point: the gate stops the bleed immediately, while the existing 17 are +# graduated and pruned one at a time under #1419. +# +# Stale-guarded: --check fails on an entry whose slice no longer exists, so an +# exemption cannot outlive its debt, and a future slice cannot silently inherit +# a grandfathered slug. Delete the line in the same PR that prunes the slice. +# +# Do NOT add a slug here to get a slice past the gate. The exemption exists for +# work that predates enforcement; new work prunes before merge, as the +# convention has always required. +# +# Done when this file lists no slugs and docs/topics/ is empty on main (#1419). + +ai-adoption-ladder +autonomy-ignition +babysit-prs-migration +boris-video-absorption +commit-convention-well-known-path +context-engineering-claude-5 +dometrain-mcp +fable-field-guide-audit +fresh-eyes-checkpoint-audit +github-plugin-candidates +handoff-bg-cutover +ladder-climb-roadmap +loop-engineering-codification +plugin-audit-port +plugin-fleet-sync-skill +plugin-organization +underspecification From 5ec5aeeacb8d88e25926ec1735a58f89e2631c20 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:28:01 -0400 Subject: [PATCH 2/3] fix(ci): read prune-gate exemptions from the base revision, resolve contract_dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both reproduced before fixing. ## P1 — the gate was bypassable by self-grandfathering The baseline was loaded from the working tree, so ONE change set could add `docs/topics//` and add `` to the baseline together: `--check` accepted it because the directory existed, `--check-diff` exempted the path because the line existed, and the required gate passed. That is the exact edit the baseline's own header prohibits, and prose cannot enforce it. Exemptions now resolve from the BASE revision, so only debt already grandfathered before the change set can exempt anything. A baseline line grants nothing to the PR that adds it. The failure message says so, since adding the slug is the obvious next thing a blocked author would try. ## P2 — contract_dir ignored the tracked concern file `docs/conventions/topic-docs/README.md` makes `.claude/topic-docs.yaml` the runtime authority for `contract_dir`, resolved before the documented default. The gate hardcoded `docs/topics`, so a repo that relocated its contract root would have had the real root silently unpoliced while the gate reported green against a directory nothing writes to — a fail-open on the check's whole subject. It now resolves from the concern file, also read from the base revision (for the same reason as the baseline: relocating the root in the same PR that adds a slice under the old one would otherwise dodge the gate). A root-equivalent value exits 2 rather than policing the entire repo. ## Baseline correction #1400 merged while this PR was open, landing `interview-batch-rounds` and `shadowed-skill-renames` on main. They are now pre-existing debt rather than incoming work, so they join the baseline: 17 -> 19. Without this the gate would red-line unrelated PRs for slices they did not introduce. Five cases added (16 total): self-grandfathering rejected, a pre-existing entry still exempting, contract_dir resolved from the concern file, a relocated root moving the gate's scope, and a root-equivalent value exiting 2. Co-authored-by: Claude Opus 5 (1M context) --- scripts/check-contract-slice-prune.sh | 98 ++++++++++++++++++---- scripts/check-contract-slice-prune.test.sh | 55 ++++++++++++ scripts/contract-slice-baseline.txt | 8 +- 3 files changed, 143 insertions(+), 18 deletions(-) diff --git a/scripts/check-contract-slice-prune.sh b/scripts/check-contract-slice-prune.sh index 549bdf046..5c5c7ef5e 100755 --- a/scripts/check-contract-slice-prune.sh +++ b/scripts/check-contract-slice-prune.sh @@ -39,18 +39,64 @@ # so an exemption cannot outlive the debt it covers. Graduating a slice is # tracked as #1419; each slice's prune PR drops its own baseline line. # -# Fail-closed: an unresolvable base ref, or a diff that cannot be computed, exits -# non-zero rather than passing unchecked. CONTRACT_SLICE_DIR and -# CONTRACT_SLICE_BASELINE override the contract dir and baseline path (test -# injection). +# The exemption set is read from the BASE revision, never from the working tree. +# A baseline read at head would let one PR add docs/topics// AND add +# to the baseline in the same diff, exempting itself — the gate would pass and +# the enforcement would be decorative. Reading the base revision means only debt +# that was already grandfathered before the change set can exempt anything, so a +# baseline addition grants nothing to the PR that makes it. +# +# contract_dir resolves from the tracked concern file .claude/topic-docs.yaml +# when present, per the resolution order in +# docs/conventions/topic-docs/README.md — that file is the runtime authority, so +# a repo that relocates its contract root must not silently fall back to +# docs/topics and leave the real root unchecked. Also read from the base +# revision, for the same self-exemption reason: relocating contract_dir in the +# same PR that adds a slice under the old root would otherwise dodge the gate. +# +# Fail-closed: an unresolvable base ref, a diff that cannot be computed, or a +# concern file present but unreadable exits non-zero rather than passing +# unchecked. CONTRACT_SLICE_DIR and CONTRACT_SLICE_BASELINE override the +# resolved contract dir and baseline path (test injection). set -uo pipefail cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 2 -CONTRACT_DIR="${CONTRACT_SLICE_DIR:-docs/topics}" -CONTRACT_DIR="${CONTRACT_DIR%/}" +CONCERN_FILE=".claude/topic-docs.yaml" BASELINE="${CONTRACT_SLICE_BASELINE:-scripts/contract-slice-baseline.txt}" +# Read a repo path at , or from the working tree when rev is empty. +# Prints nothing and returns 1 when the path does not exist there. +read_at_rev() { + local rev="$1" path="$2" + if [[ -z "$rev" ]]; then + [[ -f "$path" ]] || return 1 + cat -- "$path" + return 0 + fi + git show "$rev:$path" 2>/dev/null +} + +# contract_dir from the concern file at , falling back to the documented +# default. Minimal scalar parse: the key is documented as a plain unquoted +# scalar with an optional trailing comment. +resolve_contract_dir() { + local rev="$1" content value + if ! content="$(read_at_rev "$rev" "$CONCERN_FILE")"; then + printf 'docs/topics' + return 0 + fi + value="$(printf '%s\n' "$content" | + sed -n 's/^[[:space:]]*contract_dir[[:space:]]*:[[:space:]]*//p' | + head -n1 | + sed 's/[[:space:]]*#.*$//; s/^["'"'"']//; s/["'"'"']$//; s/[[:space:]]*$//')" + if [[ -z "$value" ]]; then + printf 'docs/topics' + return 0 + fi + printf '%s' "$value" +} + mode="${1:-}" case "$mode" in --check | --check-diff) ;; @@ -60,16 +106,41 @@ case "$mode" in ;; esac +# --check inspects the working tree (is any entry stale right now?), so it reads +# both the concern file and the baseline from the working tree. --check-diff +# judges a change set and therefore resolves both from the BASE revision, so the +# change set cannot widen its own exemptions. The base ref is validated below, +# before either is read. +baseline_rev="" +if [[ "$mode" == "--check-diff" ]]; then + if [[ -z "${2:-}" ]]; then + echo "usage: $(basename "$0") --check-diff " >&2 + exit 2 + fi + if ! git rev-parse --verify --quiet "${2}^{commit}" >/dev/null; then + echo "check-contract-slice-prune: base ref '$2' is not a resolvable commit." >&2 + exit 2 + fi + baseline_rev="$2" +fi + +CONTRACT_DIR="${CONTRACT_SLICE_DIR:-$(resolve_contract_dir "$baseline_rev")}" +CONTRACT_DIR="${CONTRACT_DIR%/}" +if [[ -z "$CONTRACT_DIR" || "$CONTRACT_DIR" == "." || "$CONTRACT_DIR" == "/" ]]; then + echo "check-contract-slice-prune: resolved contract_dir is root-equivalent ('$CONTRACT_DIR'); refusing to run." >&2 + exit 2 +fi + # Grandfathered slice slugs (directory names directly under the contract dir). declare -A grandfathered -if [[ -f "$BASELINE" ]]; then +if baseline_content="$(read_at_rev "$baseline_rev" "$BASELINE")"; then while IFS= read -r line; do line="${line%%#*}" line="${line#"${line%%[![:space:]]*}"}" line="${line%"${line##*[![:space:]]}"}" [[ -z "$line" ]] && continue grandfathered["$line"]=1 - done <"$BASELINE" + done <<<"$baseline_content" fi # Map a repo path to the slice slug that owns it, or empty if the path is not @@ -102,16 +173,8 @@ if [[ "$mode" == "--check" ]]; then exit 0 fi -# --check-diff mode -if [[ -z "${2:-}" ]]; then - echo "usage: $(basename "$0") --check-diff " >&2 - exit 2 -fi +# --check-diff mode (argument and base ref already validated above) base="$2" -if ! git rev-parse --verify --quiet "${base}^{commit}" >/dev/null; then - echo "check-contract-slice-prune: base ref '$base' is not a resolvable commit." >&2 - exit 2 -fi # Three-dot base...HEAD is diff(merge-base(base,HEAD), HEAD) — only the commits # unique to this branch. A slice that main gained after this branch forked is @@ -159,6 +222,7 @@ if ((${#violations[@]})); then echo "" >&2 echo "$CONTRACT_DIR// is Contract tier per docs/conventions/topic-docs/README.md: committed on a task branch only, pruned before merge." >&2 echo "Before merging, graduate the durable outcomes (ADR / spec / tracker item) and delete the slice — the deletion itself passes this gate." >&2 + echo "Adding the slug to $BASELINE will NOT help: exemptions are read from the base revision, so a baseline line added by this change set grants it nothing." >&2 exit 1 fi diff --git a/scripts/check-contract-slice-prune.test.sh b/scripts/check-contract-slice-prune.test.sh index c504c671f..ca3ebcedc 100755 --- a/scripts/check-contract-slice-prune.test.sh +++ b/scripts/check-contract-slice-prune.test.sh @@ -128,6 +128,61 @@ out="$(run_check "$repo")" if [[ "$out" == *"STALE BASELINE"* ]]; then ok "--check fails on a stale baseline entry"; else fail "a baseline entry outliving its slice must fail --check"; fi rm -rf "$repo" +# --- a PR cannot self-grandfather: baseline is read from the BASE revision --- +# Without this, one diff adding docs/topics// AND the matching baseline +# line would exempt itself and the gate would be decorative. +repo="$(mk_repo)" +mkdir -p "$repo/docs/topics/sneaky" +printf 'plan\n' >"$repo/docs/topics/sneaky/PLAN.md" +printf 'sneaky\n' >"$repo/scripts/contract-slice-baseline.txt" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then fail "a PR adding its own baseline line must NOT exempt itself"; else ok "baseline read from base revision: self-grandfathering is rejected"; fi +rm -rf "$repo" + +# --- a baseline entry that already existed at base still exempts ------------- +repo="$(mk_repo $'legacy\n')" +mkdir -p "$repo/docs/topics/legacy" +printf 'plan\n' >"$repo/docs/topics/legacy/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "a pre-existing baseline entry still exempts"; else fail "base-revision read must not break legitimate grandfathering"; fi +rm -rf "$repo" + +# --- contract_dir resolves from the tracked concern file -------------------- +# A repo that relocates its contract root must not have the gate silently keep +# checking docs/topics and leave the real root unguarded. +repo="$(mk_repo)" +mkdir -p "$repo/.claude" +printf 'contract_dir: docs/slices # relocated\n' >"$repo/.claude/topic-docs.yaml" +(cd "$repo" && git_q add -A && git_q commit -m concern && git_q checkout base && git_q merge work && git_q checkout work) +mkdir -p "$repo/docs/slices/relocated" +printf 'plan\n' >"$repo/docs/slices/relocated/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then fail "a slice under the configured contract_dir must be red-lined"; else ok "contract_dir resolves from .claude/topic-docs.yaml"; fi +rm -rf "$repo" + +# --- with a relocated contract_dir, the OLD default is no longer policed ----- +repo="$(mk_repo)" +mkdir -p "$repo/.claude" +printf 'contract_dir: docs/slices\n' >"$repo/.claude/topic-docs.yaml" +(cd "$repo" && git_q add -A && git_q commit -m concern && git_q checkout base && git_q merge work && git_q checkout work) +mkdir -p "$repo/docs/topics/stale" +printf 'plan\n' >"$repo/docs/topics/stale/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "a relocated contract_dir moves the gate's scope"; else fail "docs/topics must not stay policed once contract_dir moves"; fi +rm -rf "$repo" + +# --- a root-equivalent contract_dir is refused, not silently honoured -------- +repo="$(mk_repo)" +mkdir -p "$repo/.claude" +printf 'contract_dir: .\n' >"$repo/.claude/topic-docs.yaml" +(cd "$repo" && git_q add -A && git_q commit -m concern && git_q checkout base && git_q merge work && git_q checkout work) +printf 'edit\n' >>"$repo/README.md" +commit_work "$repo" +out="$(run_diff "$repo")" +rc=$? +if ((rc == 2)) && [[ "$out" == *"root-equivalent"* ]]; then ok "root-equivalent contract_dir exits 2"; else fail "root-equivalent contract_dir must exit 2, got rc=$rc"; fi +rm -rf "$repo" + # --- usage ------------------------------------------------------------------ repo="$(mk_repo)" (cd "$repo" && bash scripts/check-contract-slice-prune.sh --bogus >/dev/null 2>&1) diff --git a/scripts/contract-slice-baseline.txt b/scripts/contract-slice-baseline.txt index c2c6425ec..ed2ff9945 100644 --- a/scripts/contract-slice-baseline.txt +++ b/scripts/contract-slice-baseline.txt @@ -6,9 +6,13 @@ # # This is a snapshot of the debt at the moment the gate landed (#1417), so a NEW # slice matches no line and is red-lined rather than grandfathered. That is the -# whole point: the gate stops the bleed immediately, while the existing 17 are +# whole point: the gate stops the bleed immediately, while the existing 19 are # graduated and pruned one at a time under #1419. # +# --check-diff reads this file from the BASE revision, so adding a line here in +# the same PR that adds the slice exempts nothing. An entry only takes effect +# once it is already on the base branch. +# # Stale-guarded: --check fails on an entry whose slice no longer exists, so an # exemption cannot outlive its debt, and a future slice cannot silently inherit # a grandfathered slug. Delete the line in the same PR that prunes the slice. @@ -30,9 +34,11 @@ fable-field-guide-audit fresh-eyes-checkpoint-audit github-plugin-candidates handoff-bg-cutover +interview-batch-rounds ladder-climb-roadmap loop-engineering-codification plugin-audit-port plugin-fleet-sync-skill plugin-organization +shadowed-skill-renames underspecification From 7d9a91309e6878929409921780d59e8627effa7b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:38:15 -0400 Subject: [PATCH 3/3] fix(ci): police both contract roots, survive a slug-less baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more review findings, both reproduced before fixing. ## P1 — a relocation left the root it selected uninspected Resolving contract_dir from the base revision stopped a PR narrowing its own scope, but introduced the mirror gap: a PR that RELOCATES contract_dir could migrate the grandfathered slices to the new root and add an unpruned slice alongside them, and --check-diff would still be reading the old root. Reproduced: relocating docs/topics to docs/slices while adding docs/slices/newslug passed both workflow steps. --check saw the head root and reported healthy; --check-diff saw the base root and found nothing. The diff is now checked against the UNION of the base root and the head root, so neither side of a relocation can carry an unpruned slice. Identical values collapse to one, and messages name every root actually policed rather than implying a single one. A legitimate relocation that carries its debt across still passes. ## P2 — the intended end state crashed the gate `declare -A grandfathered` leaves the variable unset under `set -u`, so `${#grandfathered[@]}` aborted with "unbound variable" whenever the baseline held no slugs. That is exactly the state the debt burn-down is driving toward: the gate would have broken the moment the last slice was pruned. Now explicitly initialized empty, with the key expansion guarded the same way. Three cases added (19 total): a slug-less baseline surviving --check, a slice under a same-PR-relocated root being red-lined, and a grandfathered slice migrating to a relocated root still passing. Co-authored-by: Claude Opus 5 (1M context) --- scripts/check-contract-slice-prune.sh | 79 ++++++++++++++++------ scripts/check-contract-slice-prune.test.sh | 38 +++++++++++ 2 files changed, 96 insertions(+), 21 deletions(-) diff --git a/scripts/check-contract-slice-prune.sh b/scripts/check-contract-slice-prune.sh index 5c5c7ef5e..7a571742c 100755 --- a/scripts/check-contract-slice-prune.sh +++ b/scripts/check-contract-slice-prune.sh @@ -124,15 +124,43 @@ if [[ "$mode" == "--check-diff" ]]; then baseline_rev="$2" fi -CONTRACT_DIR="${CONTRACT_SLICE_DIR:-$(resolve_contract_dir "$baseline_rev")}" -CONTRACT_DIR="${CONTRACT_DIR%/}" -if [[ -z "$CONTRACT_DIR" || "$CONTRACT_DIR" == "." || "$CONTRACT_DIR" == "/" ]]; then - echo "check-contract-slice-prune: resolved contract_dir is root-equivalent ('$CONTRACT_DIR'); refusing to run." >&2 - exit 2 +# Both roots are policed, not just one. --check-diff resolves contract_dir from +# the base revision so a change set cannot narrow its own scope, but a PR that +# RELOCATES contract_dir would then leave the root it selected uninspected — it +# could migrate the grandfathered slices to the new root and add an unpruned one +# alongside them, and the diff check would still be looking at the old root. So +# the diff is checked against the union of the base root and the head root. +# Identical values collapse to one. +declare -a CONTRACT_DIRS=() +add_contract_dir() { + local dir="${1%/}" + [[ -z "$dir" || "$dir" == "." || "$dir" == "/" ]] && { + echo "check-contract-slice-prune: resolved contract_dir is root-equivalent ('$1'); refusing to run." >&2 + exit 2 + } + local existing + for existing in ${CONTRACT_DIRS[@]+"${CONTRACT_DIRS[@]}"}; do + [[ "$existing" == "$dir" ]] && return 0 + done + CONTRACT_DIRS+=("$dir") +} + +if [[ -n "${CONTRACT_SLICE_DIR:-}" ]]; then + add_contract_dir "$CONTRACT_SLICE_DIR" +else + add_contract_dir "$(resolve_contract_dir "$baseline_rev")" + # The head root only matters when judging a change set; --check inspects the + # working tree, which resolve_contract_dir already reads with an empty rev. + [[ "$mode" == "--check-diff" ]] && add_contract_dir "$(resolve_contract_dir "")" fi +# The primary root — the one messages name, and the only one --check walks. +CONTRACT_DIR="${CONTRACT_DIRS[0]}" -# Grandfathered slice slugs (directory names directly under the contract dir). -declare -A grandfathered +# Grandfathered slice slugs (directory names directly under a contract dir). +# Explicitly emptied: under `set -u`, a bare `declare -A` leaves the variable +# unset, so `${#grandfathered[@]}` aborts when the baseline holds no slugs — the +# exact END state this gate's debt burn-down is driving toward (#1419). +declare -A grandfathered=() if baseline_content="$(read_at_rev "$baseline_rev" "$BASELINE")"; then while IFS= read -r line; do line="${line%%#*}" @@ -143,22 +171,26 @@ if baseline_content="$(read_at_rev "$baseline_rev" "$BASELINE")"; then done <<<"$baseline_content" fi -# Map a repo path to the slice slug that owns it, or empty if the path is not -# under the contract dir. "docs/topics/foo/design/x.md" -> "foo". +# Map a repo path to the slice slug that owns it, or empty if the path is under +# no contract root. "docs/topics/foo/design/x.md" -> "foo". slug_of() { - local path="$1" - case "$path" in - "$CONTRACT_DIR"/*) - local rest="${path#"$CONTRACT_DIR"/}" - printf '%s' "${rest%%/*}" - ;; - *) printf '' ;; - esac + local path="$1" dir rest + for dir in "${CONTRACT_DIRS[@]}"; do + case "$path" in + "$dir"/*) + rest="${path#"$dir"/}" + printf '%s' "${rest%%/*}" + return 0 + ;; + *) ;; # not under this root; try the next + esac + done + printf '' } if [[ "$mode" == "--check" ]]; then stale=0 - for slug in "${!grandfathered[@]}"; do + for slug in ${grandfathered[@]+"${!grandfathered[@]}"}; do if [[ ! -d "$CONTRACT_DIR/$slug" ]]; then echo "STALE BASELINE: '$slug' in $BASELINE no longer names a slice under $CONTRACT_DIR/ — remove the line." >&2 stale=1 @@ -216,8 +248,13 @@ while IFS=$'\t' read -r status path dest; do fi done <<<"$diff_status" +# Name every root actually policed, so a relocation's second root is visible in +# the log rather than implied. +roots_label="$(printf '%s/, ' "${CONTRACT_DIRS[@]}")" +roots_label="${roots_label%, }" + if ((${#violations[@]})); then - echo "Contract-slice prune gate FAILED — this change set leaves ${#violations[@]} path(s) under $CONTRACT_DIR/:" >&2 + echo "Contract-slice prune gate FAILED — this change set leaves ${#violations[@]} path(s) under $roots_label:" >&2 printf ' %s\n' "${violations[@]}" >&2 echo "" >&2 echo "$CONTRACT_DIR// is Contract tier per docs/conventions/topic-docs/README.md: committed on a task branch only, pruned before merge." >&2 @@ -227,8 +264,8 @@ if ((${#violations[@]})); then fi if ((${#exempted[@]})); then - echo "Contract-slice prune gate passed — ${#exempted[@]} path(s) under $CONTRACT_DIR/ exempted by $BASELINE (see #1419)." + echo "Contract-slice prune gate passed — ${#exempted[@]} path(s) under $roots_label exempted by $BASELINE (see #1419)." else - echo "Contract-slice prune gate passed — this change set leaves no path under $CONTRACT_DIR/." + echo "Contract-slice prune gate passed — this change set leaves no path under $roots_label." fi exit 0 diff --git a/scripts/check-contract-slice-prune.test.sh b/scripts/check-contract-slice-prune.test.sh index ca3ebcedc..993f7b72c 100755 --- a/scripts/check-contract-slice-prune.test.sh +++ b/scripts/check-contract-slice-prune.test.sh @@ -183,6 +183,44 @@ rc=$? if ((rc == 2)) && [[ "$out" == *"root-equivalent"* ]]; then ok "root-equivalent contract_dir exits 2"; else fail "root-equivalent contract_dir must exit 2, got rc=$rc"; fi rm -rf "$repo" +# --- a slug-less baseline must not abort under set -u ----------------------- +# This is the END state #1419 drives toward, so a crash here would block the +# very cleanup the gate exists to enable. +repo="$(mk_repo $'# only a comment, no slugs\n')" +out="$(run_check "$repo")" +rc=$? +if ((rc == 0)) && [[ "$out" != *"unbound variable"* ]]; then ok "--check survives a slug-less baseline"; else fail "empty baseline must not abort: rc=$rc out=$out"; fi +rm -rf "$repo" + +# --- relocating contract_dir cannot smuggle a slice under the NEW root ------- +# --check-diff resolves the base root so a PR cannot narrow its own scope; it +# must ALSO police the head root, or a relocation leaves the root it selected +# uninspected. +repo="$(mk_repo $'legacy\n')" +mkdir -p "$repo/docs/topics/legacy" +printf 'plan\n' >"$repo/docs/topics/legacy/PLAN.md" +(cd "$repo" && git_q add -A && git_q commit -m seed && git_q checkout base && git_q merge work && git_q checkout work) +mkdir -p "$repo/.claude" "$repo/docs/slices/legacy" "$repo/docs/slices/newslug" +printf 'contract_dir: docs/slices\n' >"$repo/.claude/topic-docs.yaml" +(cd "$repo" && git_q mv docs/topics/legacy/PLAN.md docs/slices/legacy/PLAN.md) +printf 'plan\n' >"$repo/docs/slices/newslug/PLAN.md" +commit_work "$repo" +if run_diff "$repo" >/dev/null; then fail "a slice under a same-PR-relocated root must be red-lined"; else ok "both base and head contract roots are policed"; fi +rm -rf "$repo" + +# --- migrating a grandfathered slice to a relocated root still passes ------- +# The union must not punish a legitimate relocation that carries its debt over. +repo="$(mk_repo $'legacy\n')" +mkdir -p "$repo/docs/topics/legacy" +printf 'plan\n' >"$repo/docs/topics/legacy/PLAN.md" +(cd "$repo" && git_q add -A && git_q commit -m seed && git_q checkout base && git_q merge work && git_q checkout work) +mkdir -p "$repo/.claude" "$repo/docs/slices/legacy" +printf 'contract_dir: docs/slices\n' >"$repo/.claude/topic-docs.yaml" +(cd "$repo" && git_q mv docs/topics/legacy/PLAN.md docs/slices/legacy/PLAN.md) +commit_work "$repo" +if run_diff "$repo" >/dev/null; then ok "a grandfathered slice may migrate to a relocated root"; else fail "carrying existing debt to a new root must not be red-lined"; fi +rm -rf "$repo" + # --- usage ------------------------------------------------------------------ repo="$(mk_repo)" (cd "$repo" && bash scripts/check-contract-slice-prune.sh --bogus >/dev/null 2>&1)