fix: address PR review feedback (issues #134-140)

.gitignore

@@ -2,6 +2,7 @@
 **/*/target/
 .cargo-tmp/
 .home-tmp/
+docs/archive/
 docs/plans/
 .cargo/
 .cargo-home/

Makefile

@@ -56,8 +56,8 @@ define verify_setup
 endef
 
 # Phony targets: core developer flow
-.PHONY: help fmt fmt-check lint lint-md check test test-unit test-integration test-setup test-install \
-	build build-min serve-help install status coverage test-coverage dogfood ci precommit \
+.PHONY: all help version verify fmt fmt-check lint lint-md check test test-unit test-integration test-setup test-install \
+	build build-min serve-help install status coverage test-coverage dogfood dogfood-readme ci precommit \
 	clean clean-demo githooks hooks require-cargo security deny deps-update check-deps \
 	quick watch bench release
 # Phony targets: docs
@@ -74,9 +74,21 @@ endef
 
 $(CARGO_GUARD_TARGETS): require-cargo
 
+all: fmt lint test build
+	@echo "==> Full build complete"
+
+version:
+	@grep '^version' crates/cli/Cargo.toml | head -1 | cut -d'=' -f2 | cut -d'#' -f1 | tr -d " \"'"
+
+verify: fmt-check lint lint-md test test-install
+	@echo "==> All verification checks passed"
+
 help:
 	@printf "Usage: make <target>\n\n"
 	@printf "Core\n"
+	@printf "  %-23s %s\n" "all" "full build (fmt + lint + test + build)"
+	@printf "  %-23s %s\n" "version" "print current version"
+	@printf "  %-23s %s\n" "verify" "run all verification checks"
 	@printf "  %-23s %s\n" "fmt | fmt-check" "format workspace or check only"
 	@printf "  %-23s %s\n" "lint" "clippy with -D warnings"
 	@printf "  %-23s %s\n" "lint-md" "lint markdown files"
@@ -90,7 +102,8 @@ help:
 	@printf "  %-23s %s\n" "serve-help" "binary --help smoke check"
 	@printf "  %-23s %s\n" "status" "show project status and environment"
 	@printf "  %-23s %s\n" "coverage" "generate test coverage report"
-	@printf "  %-23s %s\n" "dogfood" "run skrills on its own codebase"
+	@printf "  %-23s %s\n" "dogfood" "full dogfood (doctor + README validation)"
+	@printf "  %-23s %s\n" "dogfood-readme" "validate README CLI examples only"
 	@printf "  %-23s %s\n" "ci | precommit" "run common pipelines"
 	@printf "  %-23s %s\n" "quick" "fast check (fmt + check, no tests)"
 	@printf "  %-23s %s\n" "watch" "watch mode with cargo-watch"
@@ -112,6 +125,8 @@ help:
 	@printf "  %-23s %s\n" "demo-cli" "test all CLI commands"
 	@printf "  %-23s %s\n" "demo-doctor | demo-empirical" "individual command demos"
 	@printf "  %-23s %s\n" "demo-http" "start HTTP MCP server (127.0.0.1:3000)"
+	@printf "  %-23s %s\n" "demo-cert" "test TLS certificate management"
+	@printf "  %-23s %s\n" "demo-skill-lifecycle" "test skill lifecycle commands"
 	@printf "  %-23s %s\n" "demo-analytics" "test analytics export/import"
 	@printf "  %-23s %s\n" "demo-gateway" "test MCP gateway tools"
 	@printf "  %-23s %s\n" "demo-setup-all" "run all setup flow demos"
@@ -193,11 +208,31 @@ coverage:
 	$(CARGO_CMD) tarpaulin --workspace --all-features --out Html
 	$(call open_file,$(CURDIR)/tarpaulin-report.html)
 
-dogfood: build demo-fixtures
+dogfood: build demo-fixtures dogfood-readme
 	@echo "==> Dogfooding: Running skrills on itself"
 	HOME=$(HOME_DIR) $(BIN_PATH) doctor
 	@echo "==> Dogfood complete"
 
+dogfood-readme: build demo-fixtures
+	@echo "==> Dogfooding README CLI examples"
+	@echo "--- cert status"
+	$(DEMO_RUN) cert status
+	@echo "--- cert renew"
+	$(DEMO_RUN) cert renew
+	@echo "--- skill-catalog"
+	$(DEMO_RUN) skill-catalog
+	@echo "--- skill-profile"
+	$(DEMO_RUN) skill-profile
+	@echo "--- skill-usage-report"
+	$(DEMO_RUN) skill-usage-report
+	@echo "--- skill-score"
+	$(DEMO_RUN) skill-score || echo "    (No skills found - expected on fresh install)"
+	@echo "--- skill-deprecate --help"
+	$(DEMO_RUN) skill-deprecate --help >/dev/null
+	@echo "--- skill-rollback --help"
+	$(DEMO_RUN) skill-rollback --help >/dev/null
+	@echo "==> README examples validated"
+
 docs:
 	RUSTDOCFLAGS="-D warnings" $(CARGO_CMD) doc --workspace --all-features --no-deps
 	$(call open_file,$(CURDIR)/target/doc/skrills/index.html)
@@ -218,6 +253,32 @@ demo-http: build
 	@echo "    Connect to http://127.0.0.1:3000/mcp"
 	$(BIN_PATH) serve --http 127.0.0.1:3000
 
+demo-cert: demo-fixtures build
+	@echo "==> Demo: TLS Certificate Management"
+	@echo "--- cert status"
+	$(DEMO_RUN) cert status
+	@echo "--- cert status (json)"
+	$(DEMO_RUN) cert status --format json | head -5
+	@echo "--- cert renew (skip if valid)"
+	$(DEMO_RUN) cert renew || true
+	@echo "--- cert renew --force"
+	$(DEMO_RUN) cert renew --force
+	@echo "==> Certificate demo complete"
+
+demo-skill-lifecycle: demo-fixtures build
+	@echo "==> Demo: Skill Lifecycle Commands"
+	@echo "--- pre-commit-validate"
+	$(DEMO_RUN) pre-commit-validate || echo "    (No skills to validate)"
+	@echo "--- skill-catalog"
+	$(DEMO_RUN) skill-catalog
+	@echo "--- skill-profile"
+	$(DEMO_RUN) skill-profile
+	@echo "--- skill-usage-report"
+	$(DEMO_RUN) skill-usage-report
+	@echo "--- skill-score"
+	$(DEMO_RUN) skill-score || echo "    (No skills to score)"
+	@echo "==> Skill lifecycle demo complete"
+
 demo-analytics: demo-fixtures build
 	@echo "==> Demo: Analytics Export/Import"
 	@echo "--- export-analytics"

README.md

@@ -68,18 +68,18 @@ skrills serve
 skrills tui
 ```
 
-See [CLI reference](book/src/cli.md) for all 38 commands including skill lifecycle management.
+See [CLI reference](book/src/cli.md) for all 39 commands including skill lifecycle management.
 
 ## Skill Management
 
 Beyond validation and analysis, Skrills provides tools for managing skill lifecycles:
 
 ```bash
 # Deprecate a skill with migration guidance
-skrills skill-deprecate old-skill --replace "new-skill" --reason "Replaced by more efficient version"
+skrills skill-deprecate old-skill --replacement "new-skill" --message "Replaced by more efficient version"
 
-# Rollback a skill to a previous git version
-skrills skill-rollback my-skill --commit abc123
+# Rollback a skill to a previous version
+skrills skill-rollback my-skill --version abc123
 
 # Import skills from external sources
 skrills skill-import https://example.com/skill.md
@@ -89,7 +89,13 @@ skrills skill-import ~/local/skills/
 skrills skill-usage-report --format json > report.json
 
 # Calculate quality scores
-skrills skill-score --min-score 80
+skrills skill-score
+
+# Browse and search available skills
+skrills skill-catalog --filter "python"
+
+# View skill performance metrics
+skrills skill-profile my-skill
 ```
 
 ## Why Skrills
@@ -117,7 +123,7 @@ The sync system uses file hashing to respect manual edits, ensuring user changes
 | `discovery` | Skill discovery and ranking |
 | `state` | Environment config, manifest settings, runtime overrides |
 | `subagents` | Shared subagent runtime and backends |
-| `test-utils` | Shared test infrastructure |
+| `test-utils` | Shared test infrastructure (fixtures, RAII guards, temp dirs) |
 
 See [architecture docs](docs/architecture.md) for details.
 
@@ -136,6 +142,24 @@ Precedence: CLI flags > environment variables > config file.
 
 See [security docs](docs/security.md) for TLS setup and [FAQ](docs/FAQ.md) for environment variables.
 
+## TLS Certificate Management
+
+For secure HTTPS transport, Skrills includes certificate management:
+
+```bash
+# Check certificate status
+skrills cert status
+
+# Auto-generate self-signed certificates for development
+skrills serve --tls-auto
+
+# Renew expiring certificates
+skrills cert renew
+
+# Install custom certificates
+skrills cert install /path/to/cert.pem --key /path/to/key.pem
+```
+
 ## Documentation
 
 | Resource | Description |

book/src/changelog.md

@@ -1,5 +1,9 @@
 # Changelog Highlights
 
+## 0.5.6 (2026-01-28)
+
+- **Testing**: Added BDD-style unit tests for skill management modules (deprecation, pre-commit, profiling, rollback, usage-report) covering serialization, parsing, and validation logic.
+
 ## 0.5.5 (2026-01-26)
 
 - **NEW: TLS Certificate Management**: Added `skrills cert` subcommand with `status`, `renew`, and `install` operations. Certificate validity is shown on server startup.
@@ -8,7 +12,7 @@
 
 ## 0.5.4 (2026-01-25)
 
-- **Testing**: Added comprehensive BDD-style tests for configuration loading and directory validation
+- **Testing**: Added BDD-style tests for configuration loading and directory validation
 - **Documentation**: Updated README command count to 37 and added skill management section
 
 ## 0.5.3 (2026-01-23)
@@ -84,7 +88,7 @@
 
 ## 0.4.5 (2026-01-03)
 
-- **Testing**: Added comprehensive test coverage for tool handler functions.
+- **Testing**: Added tests for tool handler functions.
 
 ## 0.4.4 (2026-01-02)
 
@@ -168,7 +172,7 @@
 - **NEW: Subagents Module**: Comprehensive subagent functionality with MCP server support via `list-subagents`, `run-subagent`, and `get-run-status` tools.
 - **NEW: Backend Support**: Dual backend support for both Claude-style and Codex-style subagent execution.
 - **NEW: Sync Infrastructure**: Cross-agent sync orchestration with `SyncOrchestrator` and adapters for Claude/Codex.
-- **Documentation**: Added comprehensive AGENTS.md with subagent usage examples.
+- **Documentation**: Added AGENTS.md with subagent usage examples.
 - **BREAKING**: Removed the gateway crate and related functionality. Replaced with simpler MCP server integration.
 - **Security Fix**: Updated `rmcp` from 0.9.1 to 0.10.0, replacing unmaintained `paste` with `pastey`.
 

crates/analyze/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "skrills-analyze"
-version = "0.5.5"
+version = "0.5.6"
 edition.workspace = true
 description = "Skill analysis: token counting, dependencies, and optimization"
 license = "MIT"

crates/cli/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "skrills"
-version = "0.5.5"
+version = "0.5.6"
 edition = "2021"
 description = "A command-line interface and MCP server for managing local SKILL.md files."
 license = "MIT"

crates/discovery/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "skrills-discovery"
-version = "0.5.5"
+version = "0.5.6"
 edition = "2021"
 description = "Filesystem discovery and hashing utilities used by the skrills MCP server."
 license = "MIT"

crates/discovery/src/scanner.rs

@@ -230,6 +230,12 @@ fn collect_skills_from(
             .max_depth(max_depth)
             .into_iter()
             .filter_entry(|e| {
+                // SAFETY: Known TOCTOU limitation - a file could become a symlink between
+                // this check and subsequent fs::read(). Mitigated by:
+                // 1. WalkDir's follow_links(false) default behavior
+                // 2. Skills directories are typically user-controlled, not attacker-writable
+                // 3. The sanitize_name() function prevents path traversal in output paths
+                // See: https://github.com/athola/skrills/issues/135
                 if e.file_type().is_symlink() {
                     return false;
                 }
@@ -346,6 +352,12 @@ pub fn discover_agents(roots: &[SkillRoot]) -> Result<Vec<crate::types::AgentMet
             .max_depth(20)
             .into_iter()
             .filter_entry(|e| {
+                // SAFETY: Known TOCTOU limitation - a file could become a symlink between
+                // this check and subsequent fs::read(). Mitigated by:
+                // 1. WalkDir's follow_links(false) default behavior
+                // 2. Skills directories are typically user-controlled, not attacker-writable
+                // 3. The sanitize_name() function prevents path traversal in output paths
+                // See: https://github.com/athola/skrills/issues/135
                 if e.file_type().is_symlink() {
                     return false;
                 }

crates/intelligence/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "skrills-intelligence"
-version = "0.5.5"
+version = "0.5.6"
 edition = "2021"
 description = "Intelligent skill recommendations based on usage patterns and project context."
 license = "MIT"