fix(tests): guard validate-content.sh git tag call against non-git env (#233)#234
Merged
Merged
Conversation
#233) `tests/validate-content.sh:654` calls `git tag -l "v2.*"` inside a command substitution. Under `set -euo pipefail`, when run outside a git checkout the call exits 128; `2>/dev/null` suppresses stderr but not the exit code, and pipefail propagates it through the pipeline. The assignment then carries that status and errexit aborts the script — before the intended fallback at line 655-656 can run. The sibling `tests/validate-structure.sh` already guards every git command-substitution with `|| echo ""` (lines 797, 803). validate-content.sh line 654 was the only unguarded call across both validators. Reproduce: non-git tree, exit 128, no `=== Summary ===` block printed. With guard: exit 0, "git tag history unavailable" WARN fires, sub-checks E+F skip cleanly, summary reports `PASS: 267 / FAIL: 0 / WARN: 2 / ALL CHECKS PASSED`. In-repo behavior unchanged: 269 PASS / 0 FAIL / 1 WARN. Tier per ADR-019 line 88: `tests/**` is contributor-doctrine — no version bump, no CHANGELOG entry. Closes #233
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
One-line guard fix at
tests/validate-content.sh:654. Adds|| echo ""after thesort -Vso thegit tag -lexit 128 in non-git environments can't propagate throughpipefailand abort the script before the intended WARN-and-skip fallback at lines 655-656.Closes #233.
Root cause (verbatim from issue)
set -euo pipefail+ command substitution +git tag -lexit 128 +pipefail= script aborts at the assignment, before theif [ -z "$all_tags" ]fallback can fire.2>/dev/nullsuppresses stderr but not the exit code.Defense-in-depth scan (H1)
Grepped both validators for git command-substitutions:
tests/validate-structure.sh:797git describe --tags --abbrev=0 ... || echo ""tests/validate-structure.sh:803git diff --name-only ... || echo ""tests/validate-content.sh:654git tag -l ... | sort -VLine 654 was the only unguarded git command-substitution across both validators. The issue author's claim ("the only git call missing that
\|\| echo ""guard") verified independently.Verification
Reproduce (pre-fix) —
cp -Rfull repo tree to non-git temp dir, then:Post-fix, same non-git tree:
In-repo (tagged checkout, regression check):
tests/validate-content.sh→ 269 PASS / 0 FAIL / 1 WARN (unchanged from v2.18.3)tests/validate-structure.sh→ 358 PASS / 0 FAIL (unchanged)Doctrine
tests/**is contributor-doctrine per ADR-019 line 88 — no version bump, no CHANGELOG, no SKILL-EFFECTIVENESS update. Check 27 confirms.The fix is consumer-affecting in practice (consumers running the shipped self-test see exit 128 instead of a clean WARN). The ADR-019 classification is unchanged — the path stays contributor-doctrine because the script is shipped for verification, not runtime, and the WARN message is unambiguous. If this becomes a recurring "tests/** affects consumers" pattern, a future ADR can refine the classification.
Secondary note (LOW / by-design)
The issue raised a secondary question about
guides/anthropic-engineering-doctrine-audit.mdciting 3 maintainer-local~/.claude/lessons/...paths. This is by-design — the guide self-describes as a "defensive citation surface" for contributors/maintainers, and the lessons it cites are reflection artifacts that don't ship with the plugin. Each contributor's own~/.claude/lessons/accumulates via/reflect. No code change needed; addressed in the issue comment.Test plan
tests/validate-structure.shunchanged (358 PASS / 0 FAIL)