test: comprehensive validation layer for skills/agents/commands#58
test: comprehensive validation layer for skills/agents/commands#58alirezarezvani wants to merge 1 commit into
Conversation
Code Review — PR #58: Comprehensive Validation LayerOverall: Strong, well-scoped testing PR that closes real coverage gaps. The Python scripts are clean, the baseline-file pattern for known-unresolved refs is pragmatic, and the BACKUP_DIR bug fix in Critical Bugs1.
BACKUP_DIR="${CLAUDE_CODE_DIR}.backup-$(date +%Y%m%d-%H%M%S)" # sibling ✓But BACKUP_DIR="${CLAUDE_CODE_DIR}/backup-update-$(date +%Y%m%d-%H%M%S)" # child ✗The same infinite-recurse-on- BACKUP_DIR="${CLAUDE_CODE_DIR}.backup-update-$(date +%Y%m%d-%H%M%S)"2.
Fix: add a dedicated counter and use it: def count_extended_agents() -> int:
return count_agents() - count_core_agents()
COUNTERS = {
...
"extended-agents": count_extended_agents,
}
# in CHECKS:
("CLAUDE.md", "extended-agents", r"\*\*(\d+)\s+Extended\s+Agents\*\*", "Extended agents count"),Schema Issues3. "$id": "https://github.com/alirezarezvani/coderezz/claude-code-tresor/..."Has a spurious "$id": "https://github.com/alirezarezvani/claude-code-tresor/..."Fix the 4. Per CLAUDE.md: "Skills have limited tool access (Read, Write, Edit, Grep, Glob)". The current schema locks tools to a comma-separated string, which matches existing skill files. However, adding Code Quality Issues5. The - Example:
```bash
@some-agent-name do something
```Agent names in those blocks will be scanned as prose. Low risk today, but worth documenting as a known limitation in the module docstring. 6. # update.sh exits non-zero when there are no updates to pull
if [ "$status" -ne 0 ] && [ "$status" -ne 1 ]; thenExit code 1 is the generic failure code for shell scripts. Silently accepting it means a real Documentation Inconsistency7. CLAUDE.md still uses old command names while GETTING-STARTED.md was updated
Users who read CLAUDE.md first (the majority) will invoke broken commands. Either update CLAUDE.md to match, or document in NAVIGATION.md that the short names are command aliases. Minor / Suggestions8. return max(validate_target(t) for t in TARGETS)If 9. { "pattern": "^#" }This disables all fragment-link checking. Broken cross-section anchors (e.g., 10. Known policy inconsistency should be tracked The PR description calls out that What's Working Well
Summary
Fix the two red bugs and the schema Review generated by Claude Code on 2026-05-19 |
🔒 Security Audit (Claude)Severity Summary: Critical: 0 | High: 0 | Medium: 0 | Low: 2 | Info: 1 Overall this PR is well-constructed from a security standpoint: Findings1. [LOW]
|
97f986a to
b0db3e0
Compare
PR #58 was opened before PR #59 was created. PR #59 bundled the same 3 test-infrastructure commits along with CI/CD overhaul work, and was merged first. The actual content additions (validate_frontmatter.py, schemas, smoke test, fixes) are now on dev. This empty commit aligns the PR branch with dev so the PR can close cleanly as merged.
b0db3e0 to
4b64232
Compare
Replaces #57 (closed because the original branch name
claude/analyze-test-coverage-B1AOQdidn't match the repo's requiredfeat|fix|test|...prefix pattern). Same commits, same diff.Why
The old CI quality gate only ran
yaml.safe_loadagainstskills/(8 files) andagents/(8 symlinks). It silently skippedsubagents/(133 agents),commands/(24 files), and the 1,142 lines of bash inscripts/install.sh+scripts/update.sh. This PR closes those gaps and fixes the bugs the new checks immediately surfaced.What's new
scripts/validate_frontmatter.py+validation/schemas/*.jsonscripts/check_agent_references.py+ baseline file@namementions in commands, skills, root docsscripts/check_doc_counts.pyscripts/smoke_install.sh.markdown-link-check.jsonBugs fixed (caught by the new checks)
argument-hint:values starting with[, which YAML interpreted as broken arrays. Old CI never validated commands/.install.shplacedBACKUP_DIRinsideCLAUDE_CODE_DIR, causingcp -rto recurse into itself on every re-run. The smoke test caught this on the first try.docs/VALIDATION-REPORT-CONTENT.md— the file had moved todocs/archive/.TRESOR_DIRor the repo URL, so they couldn't be tested in isolation. Added env-overridable defaults (no behavior change for real users).Known baseline
validation/known-unresolved-refs.txtsnapshots 26 referenced agent names in orchestration command docs (@auth-security-specialist,@anomaly-detection-specialist, etc.) that don't exist in the 133-agent catalog. These look like aspirational names from when those commands were designed. CI now fails on any new unresolved name; this list can be drained incrementally.What I deliberately did NOT do
Read/Write/Edit/Grep/Glob-only tool policy on skills, despitesecurity-auditordeclaringBash. That's a real policy/doc inconsistency — flagged here but out of scope for a testing PR.erroronly; theSC2155/SC2115cleanup is a separate task.Test plan
python3 scripts/validate_frontmatter.py all→ 165 files, 0 errorspython3 scripts/check_agent_references.py→ 26 baselined, 0 newpython3 scripts/check_doc_counts.py→ 5 checks pass, 0 driftshellcheck --severity=error scripts/*.sh→ passesbash scripts/smoke_install.sh→ installs 8 skills, 8 agents, 24 commands; idempotent re-install; backup createdmarkdown-link-checkagainst README.md, CLAUDE.md, NAVIGATION.md → no dead internal linksLint, Tests, Docs, SecurityCI check on previous PR (test: comprehensive validation layer for skills/agents/commands #57) → successGenerated by Claude Code