fix(lint): flag release-relevant paths retired since the last tag
Coverage was derived from the worktree alone, so the -d guard on a hook's bundled assets/ tree meant deleting the whole tree removed it from the pathspec instead of flagging it — the gate stayed silent about a change that breaks every consumer at the next rev:. The path set is now derived twice, from the worktree manifest and from the manifest at $LAST_TAG, then unioned. A path the tag exposed but HEAD no longer does is a removal pinned consumers must be told about; a path only HEAD exposes is new contract surface. Both need flagging. Fails closed on an unreadable tagged tree (shallow clone), and treats a readable root tree with no manifest as "added since the tag". tokens[0] needed no exit-code fix — it carries no existence guard, so both deletion cases already exited non-zero. What was wrong was the reporting: a fully retired hook could no longer be named in the failure message. The tagged manifest fixes that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
This commit is contained in:
@@ -30,6 +30,16 @@ if [[ ! -f "$HOOKS_MANIFEST" ]]; then
|
|||||||
exit 0
|
exit 0
|
||||||
fi
|
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
|
# 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
|
# 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,
|
# 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
|
# 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
|
# 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;
|
# 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
|
# and the assets/ directory is added only where it is known to exist, since a
|
||||||
# nothing must not contribute a pathspec matching nothing.
|
# hook that bundles nothing must not contribute a pathspec matching nothing.
|
||||||
RELEASE_PATHS=("$HOOKS_MANIFEST")
|
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
|
add_release_path() {
|
||||||
# experiment tag reachable from HEAD must not shift the diff baseline.
|
local candidate="$1" existing
|
||||||
LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null || true)"
|
for existing in "${RELEASE_PATHS[@]}"; do
|
||||||
|
[[ "$existing" == "$candidate" ]] && return 0
|
||||||
|
done
|
||||||
|
RELEASE_PATHS+=("$candidate")
|
||||||
|
}
|
||||||
|
|
||||||
if [[ -z "$LAST_TAG" ]]; then
|
# $1 selects where the "does this hook bundle an assets/ tree?" guard looks:
|
||||||
echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2
|
# "worktree" probes the filesystem, anything else is a rev whose tree is probed
|
||||||
echo " Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2
|
# 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
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|||||||
@@ -211,6 +211,63 @@ else
|
|||||||
fail "invented a bogus assets/ path for a hook script with no bundled tree"
|
fail "invented a bogus assets/ path for a hook script with no bundled tree"
|
||||||
fi
|
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 ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]]
|
[[ $FAIL -eq 0 ]]
|
||||||
|
|||||||
Reference in New Issue
Block a user