git describe --match is a shell glob, not a regex: the trailing `*`s
in 'v[0-9]*.[0-9]*.[0-9]*' match any suffix, so a tag like
v1.2.3-checkpoint or v1.2.3-rc1 satisfied the pattern and could be
picked as LAST_TAG instead of the true last release. That silently
shifts the diff baseline and can let a push skip a required release.
--exclude '*-*' rules out any tag carrying a hyphenated suffix.
Added a regression test that tags a release-relevant change with a
v1.0.1-checkpoint tag right after v1.0.0 and asserts the gate still
fires — confirmed it fails against the pre-fix script and passes
against the fix.
run_check set PRE_COMMIT_REMOTE_BRANCH and, when asked, PRE_COMMIT_TO_REF, but
never cleared what was already in the environment. Standalone that is invisible
— nothing sets those vars. Under the pre-push hook this suite exists to guard,
pre-commit exports PRE_COMMIT_TO_REF and PRE_COMMIT_FROM_REF as shas of the
real repo; the fixtures inherited them, the script resolved a rev that does not
exist in the fixture, and 13 of 20 cases failed. The suite passed in every
context except the only one that matters.
The variables are now cleared in both branches, so a standalone run and a
pre-push run are the same test. Verified 20/20 with the vars unset and with
them set to real shas of this repo.
Found by the pre-push hook rejecting the push, not by any test — the same shape
as the --config regression: the local invocation exercised a different thing
than the shipped one, and the two were indistinguishable by reading the file.
Refs: #85
The gate hardcoded HEAD as its diff tip, but pre-commit exports
PRE_COMMIT_TO_REF for exactly this. Pushing "somebranch:main" from another
checkout diffed the wrong tip — a false negative when HEAD is older, a false
positive when newer. Fixing only the diff tip leaves a second bug: git describe
took the tag baseline from HEAD too, so a tag reachable only from HEAD becomes
a baseline the pushed ref never saw. Both now resolve from the pushed ref, and
an all-zeros ref (branch deletion) short-circuits before any rev resolution
rather than surfacing as "could not diff".
PRE_COMMIT_FROM_REF is deliberately not used: it is the remote's current tip,
so diffing from it would let an untagged release-relevant commit already on
main excuse the next push from cutting a tag — the drift this gate exists to
catch. The baseline must stay the last release tag.
collect_release_paths took tokens[0] as a path unconditionally. ADR-0014 makes
bare single-path entries a binding constraint, but nothing enforced it, and the
sibling .pre-commit-config.yaml already ships "entry: bash <script>". Under
that shape add_release_path takes "bash", git diff accepts the non-matching
pathspec silently, bundle_root becomes "." and is skipped — the hook's whole
surface leaves the gate with no error, the same shape as the --config
regression in LESSONS.md. Multi-token entries now fail loudly naming the hook
and the ADR, and tokens[0] must resolve at HEAD or at the tag (the union is
load-bearing: a per-scope check would reject the deletion cases).
Six mutations verified, each restored. One correction worth recording: the
first multi-token test passed with its guard removed, because the existence
guard caught "bash" and printed a similar message. It now requires the verbatim
entry text that only the multi-token diagnostic emits.
Refs: #85
ADR: 0014
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
The gate derived release-relevant paths from the dirname of each
entry's --config target. Dropping --config from .pre-commit-hooks.yaml
left that loop dead, silently removing both assets/vale/ trees from
coverage — a Vale rule change could land on main without demanding a
release tag, leaving consumers pinned to an old rev: with stale rules.
Coverage now derives from tokens[0] instead: double-dirname for the ..
normalization, guarded on the tree existing and on the bundle root not
resolving to "." so skill-size-check.sh cannot invent a bogus path.
The --config branch is removed rather than kept as dead code. Since
pre-commit rewrites only entry[0], no argument in any entry can ever
name a file this repo ships, so that shape is broken by design.
Known gap: deleting a hook's entire assets/ tree is not flagged, as the
candidate path stops existing. Deletions within a surviving tree are.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
A review of PR #85's last two commits (1164f3a, 4d018af) found the new
release-gate script fails open in four separate ways, and the new drift
check for the duplicated Vale styles only ever detects drift after a
human already hand-edited both copies out of sync.
check-release-needed.sh:
- The `-e` existence filter dropped a RELEASE_PATHS entry from the diff
pathspec once it was deleted from the tree, so deleting a path exposed
via .pre-commit-hooks.yaml since the last tag passed the gate clean —
exactly the breakage the gate exists to catch. git diff reports
deletions fine without an existence check; the filter is gone.
- `git diff ... 2>/dev/null || true` turned any git failure (a shallow
clone missing the tag's objects, a corrupted ref) into an empty,
falsely-clean diff. The diff result is no longer swallowed: a failure
now hard-fails with the underlying git error visible.
- RELEASE_PATHS was a hand-maintained array duplicating
.pre-commit-hooks.yaml's entry: paths with only a comment holding them
in sync, and was already over-broad (it swept in validate.sh /
validate-provenance.sh, which no hook entry references). It's now
parsed straight from .pre-commit-hooks.yaml's entry: lines at
runtime, so it can't drift from the manifest and only tracks what a
hook actually exposes.
- `git describe --tags --abbrev=0` accepted any tag reachable from HEAD
as the diff baseline, not just release tags. Added
`--match 'v[0-9]*.[0-9]*.[0-9]*'` so an incidental checkpoint tag
can't shift the baseline and mask a real release-relevant change.
check-vale-style-sync.sh still only detects drift between skill-audit's
and agent-audit's duplicated vale-wrap.sh/styles/Kyberforge copies
(both copies must exist independently per the plugin's no-cross-skill-
path packaging rule — a symlink would break at install time). Added
scripts/sync-vale-styles.sh to regenerate skill-audit's copy from
agent-audit's canonical one on demand, and pointed the sync check's
failure message at it, so fixing drift is one command instead of a
hand diff across two files.
Also recorded, rather than silently left unfixed: check-release-needed.sh
only fires on a local `git push` through pre-commit's pre-push hook — a
PR merged via Gitea's merge button, or CI invoking
`pre-commit run --hook-stage pre-push` directly, never sets
PRE_COMMIT_REMOTE_BRANCH and skips the gate entirely. Closing that needs
a server-side CI job this repo doesn't have yet; documented as a known
limitation in ADR-0014 rather than papered over.
Separately, LESSONS.md's "a clean check can mean nothing ran" entry was
marked **Graduated** without ever being promoted per the repo's own
graduation rule (3+ instances → a standing doc, marked
`[graduated → target file]`). Actually promoted it into
core/instructions/testing.md and fixed the marker.
tests/test-check-release-needed.sh gained 4 regression tests, one per
check-release-needed.sh fix above, each verified to fail against the
pre-fix script and pass against the current one.
Verification: bash tests/run-tests.sh (11 scripts + 125 bats, all
passing), pre-commit run --all-files, and
pre-commit run --all-files --hook-stage pre-push all clean.
ADR: 0014
.pre-commit-hooks.yaml now exposes hooks to external consumers pinning
rev: <tag>, but nothing enforced that a tag actually gets cut when the
files it references change — relying on memory is exactly what this
repo's governance rules say to avoid for a repeatable, deterministic
check.
scripts/check-release-needed.sh hard-fails at pre-push, but only when
PRE_COMMIT_REMOTE_BRANCH (set by pre-commit's hook-impl) is
refs/heads/main: it diffs .pre-commit-hooks.yaml's referenced paths
against the last tag reachable from HEAD, and fails if either no tag
exists yet or something changed since. It's a silent no-op on every
other branch — hard-failing on feature-branch pushes mid-review would
force a premature tag on a commit that might not survive a
squash-merge, the exact risk the repo: local (vs. pinned self-
reference) decision in ADR-0014 already avoids for this repo's own
dev-time gate.
Verified against the real git pre-push hook path (not just the script
in isolation): simulated stdin matching git's pre-push protocol through
.git/hooks/pre-push, confirmed it correctly fires and fails when
targeting main with no tag, and is silent otherwise.
ADR: 0014
Refs: #87
