diff --git a/scripts/check-release-needed.sh b/scripts/check-release-needed.sh index cc1927f..63b1d3f 100755 --- a/scripts/check-release-needed.sh +++ b/scripts/check-release-needed.sh @@ -30,6 +30,16 @@ if [[ ! -f "$HOOKS_MANIFEST" ]]; then exit 0 fi +# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or +# experiment tag reachable from HEAD must not shift the diff baseline. +LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null || true)" + +if [[ -z "$LAST_TAG" ]]; then + echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2 + echo " Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2 + exit 1 +fi + # Derive release-relevant paths from .pre-commit-hooks.yaml's own entry: lines # instead of hand-maintaining a parallel list — the manifest is the single # source of truth for what external consumers actually pull at a pinned rev, @@ -46,26 +56,64 @@ fi # from inventing paths: a bundle root of "." is skipped, because a script in a # top-level directory (scripts/skill-size-check.sh) would derive the repo's own # shared assets/, which no hook owns and whose churn must not demand a release; -# and the directory is added only when it exists, since a hook that bundles -# nothing must not contribute a pathspec matching nothing. +# and the assets/ directory is added only where it is known to exist, since a +# hook that bundles nothing must not contribute a pathspec matching nothing. RELEASE_PATHS=("$HOOKS_MANIFEST") -while IFS= read -r entry; do - read -ra tokens <<< "$entry" - [[ ${#tokens[@]} -eq 0 ]] && continue - RELEASE_PATHS+=("${tokens[0]}") - bundle_root="$(dirname "$(dirname "${tokens[0]}")")" - if [[ "$bundle_root" != "." && -d "$bundle_root/assets" ]]; then - RELEASE_PATHS+=("$bundle_root/assets") - fi -done < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$HOOKS_MANIFEST") -# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or -# experiment tag reachable from HEAD must not shift the diff baseline. -LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null || true)" +add_release_path() { + local candidate="$1" existing + for existing in "${RELEASE_PATHS[@]}"; do + [[ "$existing" == "$candidate" ]] && return 0 + done + RELEASE_PATHS+=("$candidate") +} -if [[ -z "$LAST_TAG" ]]; then - echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2 - echo " Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2 +# $1 selects where the "does this hook bundle an assets/ tree?" guard looks: +# "worktree" probes the filesystem, anything else is a rev whose tree is probed +# with git plumbing. Reading entry lines from stdin keeps one derivation for +# both the tagged manifest and the current one. +collect_release_paths() { + local scope="$1" entry bundle_root + local -a tokens + while IFS= read -r entry; do + read -ra tokens <<< "$entry" + [[ ${#tokens[@]} -eq 0 ]] && continue + add_release_path "${tokens[0]}" + bundle_root="$(dirname "$(dirname "${tokens[0]}")")" + [[ "$bundle_root" == "." ]] && continue + if [[ "$scope" == "worktree" ]]; then + [[ -d "$bundle_root/assets" ]] && add_release_path "$bundle_root/assets" + else + git cat-file -e "$scope:$bundle_root/assets" 2>/dev/null && add_release_path "$bundle_root/assets" + fi + done + return 0 +} + +# The worktree alone is not enough: a path is release-relevant if it was part of +# the contract at $LAST_TAG *or* is part of it at HEAD, so both trees have to be +# derived and unioned. Deriving only from the worktree meant that deleting a +# hook's entire assets/ tree made the `-d` guard drop the path from the pathspec +# altogether, and the deletion — which breaks every consumer at the next rev — +# diffed clean. The two manifests can genuinely disagree (an entry added, +# removed, or renamed since the tag), and the union is the conservative side of +# that disagreement: a path the tag exposed and HEAD no longer does is a removal +# consumers must be told about, and a path only HEAD exposes is new contract +# surface they cannot reach without a new tag. The union never over-fires on its +# own, either — any manifest edit that makes the two disagree already changes +# $HOOKS_MANIFEST, which is itself a release-relevant path. +collect_release_paths worktree < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$HOOKS_MANIFEST") + +# A missing manifest at the tag is legitimate (the manifest was added since) but +# is indistinguishable from an unreadable tagged tree by its exit status alone, +# so the tag's root tree is verified separately. An absent tree object — a +# shallow clone, a truncated fetch — fails closed exactly like a `git diff` +# failure does, rather than silently degrading to worktree-only derivation. +if MANIFEST_AT_TAG="$(git cat-file -p "$LAST_TAG:$HOOKS_MANIFEST" 2>/dev/null)"; then + collect_release_paths "$LAST_TAG" < <(printf '%s\n' "$MANIFEST_AT_TAG" | sed -n 's/^[[:space:]]*entry:[[:space:]]*//p') +elif ! git cat-file -e "$LAST_TAG^{tree}" 2>/dev/null; then + echo "FAIL: could not read the tree at $LAST_TAG to determine which paths that release exposed." >&2 + echo " Fix: ensure full tag history is available (e.g. git fetch --unshallow) and retry." >&2 exit 1 fi diff --git a/tests/test-check-release-needed.sh b/tests/test-check-release-needed.sh index ba473bf..c1b1d2f 100755 --- a/tests/test-check-release-needed.sh +++ b/tests/test-check-release-needed.sh @@ -211,6 +211,63 @@ else fail "invented a bogus assets/ path for a hook script with no bundled tree" fi +# --- 12. Deleting a hook's entire bundled assets/ tree is release-relevant --- +# The worktree-only derivation guarded the assets/ path on the directory still +# existing, so wiping the whole tree removed the path from the pathspec instead +# of diffing it: the single most consumer-breaking change possible diffed clean. +# The path list therefore has to be unioned with what $LAST_TAG exposed. +echo "" +echo "--- exits 1 when a hook's entire bundled assets/ tree was deleted since the tag ---" +FIXTURE12="$(make_tagged_fixture)"; track "$FIXTURE12" +rm -rf "${FIXTURE12:?}/$HOOK_DIR/assets" +(cd "$FIXTURE12" && git add -A && git commit -q -m "delete the whole bundled assets tree") +OUT12=$(run_check "$FIXTURE12" "refs/heads/main" || true) +if echo "$OUT12" | grep -q "assets/vale/.vale.ini"; then + pass "flags a wholesale deletion of a hook's bundled assets/ tree" +else + fail "a hook's entire bundled assets/ tree vanished since the tag without demanding a release" +fi + +# --- 13. A hook script deleted while its manifest entry survives is flagged --- +# Characterisation test, not a bug fix: tokens[0] is added to the pathspec +# unconditionally (no existence guard), so this case was already covered. It is +# pinned here so the tagged-tree union can't accidentally introduce an existence +# guard on tokens[0] and reopen the hole its assets/ sibling had. +echo "" +echo "--- exits 1 when a hook script was deleted but its manifest entry remains ---" +FIXTURE13="$(make_tagged_fixture)"; track "$FIXTURE13" +rm -f "$FIXTURE13/$HOOK_DIR/scripts/vale-wrap.sh" +(cd "$FIXTURE13" && git add -A && git commit -q -m "delete a hook script, keep its manifest entry") +OUT13=$(run_check "$FIXTURE13" "refs/heads/main" || true) +if echo "$OUT13" | grep -q "vale-wrap.sh"; then + pass "flags a hook script deleted out from under a surviving manifest entry" +else + fail "a manifest entry's script vanished since the tag without demanding a release" +fi + +# --- 14. Retiring a whole hook names what the tag exposed, not just the manifest --- +# Removing the entry and everything it shipped changes $HOOKS_MANIFEST, so the +# gate fires either way — but a derivation that only reads the current manifest +# can no longer name the retired script or its assets, and the failure message +# understates the breakage to consumers pinned at the old rev. The tagged +# manifest is what makes those paths reportable. +echo "" +echo "--- names the retired hook's own paths when an entry and its files are removed together ---" +FIXTURE14="$(make_tagged_fixture)"; track "$FIXTURE14" +cat > "$FIXTURE14/.pre-commit-hooks.yaml" <<'EOF' +- id: fake-size-check + entry: scripts/skill-size-check.sh + language: script +EOF +rm -rf "${FIXTURE14:?}/$HOOK_DIR" +(cd "$FIXTURE14" && git add -A && git commit -q -m "retire the vale hook entirely") +OUT14=$(run_check "$FIXTURE14" "refs/heads/main" || true) +if echo "$OUT14" | grep -q "vale-wrap.sh" && echo "$OUT14" | grep -q "assets/vale/.vale.ini"; then + pass "names the retired hook's script and bundled assets, not just the manifest edit" +else + fail "reported only the manifest change and hid which shipped paths the retirement removed" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]