[#74] feat: KB content validation command

[rucka]

Summary

Implements the pair kb-validate command for comprehensive KB content validation before packaging. Covers all 21 acceptance criteria from story #74.

Key changes

• Parser: --layout source|target, --strict, --ignore-config, --skip-registries flags on kb-validate; --layout, --skip-registries, --root on package
• Registry utilities: layout.ts — layout resolution, prefix/flatten, symlink filtering, file collection, source↔target transforms, validateLayoutOption + parseSkipRegistriesOption (extracted from parsers)
• 3 validators: structure (config-based), links (internal + strict external), metadata (SKILL.md frontmatter + placeholder detection)
• Colored report: chalk-colored terminal output with errors (red), warnings (yellow), summary counts, exit codes 0/1/2
• Package integration: layout.ts functions integrated into package handler, link rewriter for absolute→relative, --layout wired through to zip-creator
• Tests: 504 tests (unit + e2e with 5-registry config covering all validation scenarios)
• Smoke tests: kb-validate.sh scenario + infrastructure fixes (run-all.sh, utils.sh)
• Documentation: 10 files updated with new CLI flags

Tasks completed

T-1 through T-11 (all checked in story body)

Tech debt resolved (post-review)

• D3: Extracted duplicated validateLayoutOption + parseSkipRegistriesOption from both parsers into registry/layout.ts with dedicated unit tests
• D1: Wired --layout flag through package handler → createPackageZip → copyRegistrySources (was parsed but ignored)
• D2: Removed silent try-catch in kb-validate/handler.ts loadRegistries — config errors now propagate instead of returning false "0 errors"

Refs: #74

[rucka]

Code Review — PR #137

PR: #137 [#74] feat: KB content validation command
Author: rucka
Reviewer: AI (Claude)
Date: 2026-02-15
Story: #74 Validate KB Content
Type: Feature

---

Review Summary

Overall Assessment: ✅ Approved with Comments

Key Changes: Comprehensive pair kb-validate command with layout-aware validation (structure, links, metadata), colored terminal report, 3 validators, registry filtering, prefix/flatten support. Package command extended with --skip-registries, --root, link rewriter. 41 files, +4798/-122 lines.

Business Value: Enables content creators to validate KB quality before packaging — catches broken links, missing metadata, structural issues. Exit codes integrate into CI.

---

Code Review Checklist

Functionality

• Requirements Met — 19/21 AC implemented (AC17/AC18 package layout partial)
• Business Logic — validators correctly handle all edge cases
• Integration — reuses existing registry/config infrastructure
• Error Handling — appropriate with exit codes 0/1/2
• Performance — no regressions

Code Quality

• Readability — clear separation: parser → handler → validators → report
• Maintainability — layout.ts centralizes layout logic, reused by both commands
• Naming — consistent, descriptive interfaces and functions
• Complexity — reasonable for feature scope

Technical Standards

• Style Guide — lint + prettier pass
• Architecture — follows existing command pattern
• Dependencies — no new deps (chalk already in catalog)

---

Security Review

No concerns — no user input passed to shells, no network calls by default (only with --strict), no file writes outside temp directories.

---

Testing Review

Tests: 492 passed (0 failed)
Quality Gate: 6/6 tasks pass
Coverage: unit (validators) + e2e (14 tests, 5-registry config) + smoke (5 scenarios)

• Unit Tests — structure, links, metadata validators fully tested
• E2E Tests — complex 5-registry config, both layouts, all error types
• Smoke Tests — real dataset validation (source + target)
• Edge Cases — empty dirs, missing registries, broken links, placeholder detection

---

Documentation

• 10/10 doc files updated (cli-contracts, commands, help-examples, workflows, READMEs)
• Smoke tests README updated with new scenario

---

Issues

Major 🔍 (1)

#	File	Issue
1	package/handler.ts	--layout flag parsed but never used. zip-creator hardcodes layout: 'source'. AC17/AC18 (package layout transform) not implemented.

Recommendation: If deferred, remove the flag from package parser or add TODO + follow-up issue to avoid misleading users.

Minor 💡 (2)

#	File	Issue
2	kb-validate/handler.ts:28-30	loadRegistries silently swallows config errors → returns empty {} → misleading "no registries" warning
3	kb-validate/parser.ts + package/parser.ts	validateLayoutOption() and parseSkipRegistriesOption() duplicated — extract to shared utility

---

Positive Feedback

• Excellent architecture: layout.ts centralizes all layout logic (resolve paths, apply prefix/flatten, collect files, transform source↔target) — single source of truth, highly reusable
• Thorough test pyramid: unit → e2e → smoke, each level adds value
• Smoke test infra fixes: multiple root-cause fixes in run-all.sh and utils.sh (symlink guard, TMP_DIR timing, auto-build, empty BINARY_PATH) — these benefit ALL future smoke tests
• Clean separation of concerns: handler orchestrates, validators return data, formatter renders — easy to extend

---

Tech Debt

Item	Severity	Description
Package layout transform	Medium	--layout target for package not implemented (AC17/AC18)
Duplicated parser helpers	Low	2 functions copy-pasted between parsers
Silent config swallowing	Low	Config errors invisible in kb-validate handler

---

Risk Assessment

Risk	Impact	Probability	Mitigation
Users pass --layout target to package	Low	Low	Flag accepted but no effect — not yet documented as working
Config errors silently ignored	Low	Medium	User sees "no registries" warning, may not realize config is broken

---

Decision

TECH-DEBT — Approve current PR, track package layout transform separately.

The kb-validate command is fully functional. The major finding (package --layout) is outside the primary scope of kb-validate and can be addressed in a follow-up.

[rucka]

Decision: TECH-DEBT (APPROVE)

KB validation command is well-implemented with solid test coverage (unit + E2E + smoke). Three tech debt items tracked for follow-up — none block this PR.

See detailed review report in PR comment above.