docs: Add generic hooks system implementation plan (#83)

This commit is contained in:
rUv 2025-12-25 22:33:28 -05:00 committed by GitHub
parent 5591810c87
commit c1710a6aed
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 2564 additions and 0 deletions

File diff suppressed because it is too large Load diff

402
docs/hooks/MVP_CHECKLIST.md Normal file
View file

@ -0,0 +1,402 @@
# Hooks System MVP - Implementation Checklist
**Target**: 3-4 weeks | **Status**: Ready for Development
**Feature Branch**: `feature/portable-hooks-mvp`
---
## Week 1: Foundation & CLI Scaffolding
### Day 1-2: Project Setup
- [ ] Create feature branch `feature/portable-hooks-mvp`
- [ ] Update `crates/ruvector-cli/Cargo.toml`:
```toml
[dependencies]
askama = "0.12"
shell-escape = "0.1"
```
- [ ] Create directory structure:
```
crates/ruvector-cli/
├── src/cli/hooks/
│ ├── mod.rs
│ ├── init.rs
│ ├── install.rs
│ ├── migrate.rs
│ └── stats.rs
└── templates/
├── hooks.json.j2
├── config.toml.j2
└── gitignore.j2
```
- [ ] Write specification document (API contracts)
- [ ] Design template schema with variable placeholders
### Day 3-4: CLI Command Structure
- [ ] Add `Hooks` enum to `src/cli/commands.rs`:
```rust
enum Commands {
// ... existing commands
Hooks {
#[command(subcommand)]
action: HooksCommands,
},
}
enum HooksCommands {
Init { path: Option<PathBuf> },
Install { force: bool },
Migrate { from: PathBuf },
Stats { verbose: bool },
}
```
- [ ] Implement command routing in `main.rs`
- [ ] Write unit tests for command parsing
### Day 5: Template Engine
- [ ] Create `HookTemplate` struct with `askama`:
```rust
#[derive(Template)]
#[template(path = "hooks.json.j2")]
struct HookTemplate {
shell: String,
ruvector_cli: String,
project_root: String,
}
```
- [ ] Implement platform detection:
```rust
fn get_shell_wrapper() -> &'static str {
if cfg!(target_os = "windows") { "cmd /c" }
else { "/bin/bash -c" }
}
```
- [ ] Write template rendering tests
---
## Week 2: Core Commands Implementation
### Day 6-7: `ruvector hooks init`
- [ ] Implement `init.rs`:
- [ ] Create `.ruvector/` directory
- [ ] Generate `config.toml` from template
- [ ] Create `intelligence/` subdirectories
- [ ] Write `.gitignore`
- [ ] Add validation checks (prevent overwriting existing)
- [ ] Write integration test:
```rust
#[test]
fn test_init_creates_structure() {
let temp = tempdir()?;
run_cmd(&["ruvector", "hooks", "init"], &temp)?;
assert!(temp.join(".ruvector/config.toml").exists());
}
```
### Day 8-9: `ruvector hooks install`
- [ ] Implement `install.rs`:
- [ ] Load hook template
- [ ] Substitute variables with runtime values
- [ ] Merge with existing `.claude/settings.json` or create new
- [ ] Create backup (`.claude/settings.json.backup`)
- [ ] Add `--dry-run` flag
- [ ] Implement JSON validation
- [ ] Write tests for template substitution
- [ ] Write tests for merge logic
### Day 10: Hook Template Design
- [ ] Create `templates/hooks.json.j2`:
```json
{
"hooks": {
"PreToolUse": [{
"matcher": "Bash",
"hooks": [{
"command": "{{ shell }} 'RUVECTOR=$(which ruvector || echo npx ruvector); $RUVECTOR hooks pre-command \"$CMD\"'"
}]
}],
"PostToolUse": [/* ... */],
"SessionStart": [/* ... */]
}
}
```
- [ ] Test on Linux (bash)
- [ ] Test on macOS (bash)
- [ ] Test on Windows (cmd)
---
## Week 3: Intelligence Layer & Migration
### Day 11-12: Intelligence Layer Refactoring
- [ ] Update `.claude/intelligence/index.js`:
- [ ] Change `DATA_DIR` to use `process.env.RUVECTOR_DATA_DIR`
- [ ] Add fallback chain: env → project-local → global → legacy
```javascript
const DATA_DIR = process.env.RUVECTOR_DATA_DIR ||
join(process.cwd(), '.ruvector', 'intelligence') ||
join(process.env.HOME || process.env.USERPROFILE, '.ruvector', 'global') ||
join(__dirname, 'data'); // Legacy fallback
```
- [ ] Update `cli.js` with same path logic
- [ ] Test intelligence layer works in new location
- [ ] Verify backward compatibility (legacy paths still work)
### Day 13-14: JSON Migration
- [ ] Implement `migrate.rs`:
- [ ] Copy function with validation:
```rust
fn copy_with_validation(from: &Path, to: &Path) -> Result<()> {
let data = fs::read_to_string(from)?;
let json: serde_json::Value = serde_json::from_str(&data)?;
let mut file = File::create(to)?;
serde_json::to_writer_pretty(&mut file, &json)?;
file.sync_all()?;
Ok(())
}
```
- [ ] Implement atomic migration with rollback:
```rust
pub fn migrate_with_safety(from: &Path, to: &Path) -> Result<MigrationStats> {
let backup = to.with_extension("backup");
let temp = to.with_extension("tmp");
// Backup → Migrate to temp → Validate → Atomic swap
}
```
- [ ] Add checksum validation
- [ ] Write migration tests with sample data
### Day 15: `ruvector hooks stats`
- [ ] Implement `stats.rs`:
- [ ] Load patterns from `.ruvector/intelligence/patterns.json`
- [ ] Calculate statistics (count, Q-values, top patterns)
- [ ] Format output (pretty-print or JSON)
- [ ] Add `--verbose` and `--json` flags
- [ ] Test with empty/populated data
---
## Week 4: Testing, Documentation & Polish
### Day 16-17: Cross-Platform Testing
- [ ] Set up GitHub Actions CI:
```yaml
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
```
- [ ] Linux tests:
- [ ] Fresh project init
- [ ] Migration from `.claude/intelligence/`
- [ ] Hooks trigger correctly
- [ ] macOS tests (same as Linux)
- [ ] Windows tests:
- [ ] PowerShell environment
- [ ] CMD environment
- [ ] WSL environment
- [ ] Path separator handling (`\` vs `/`)
### Day 18: Error Handling & Edge Cases
- [ ] Implement graceful hook failures:
```rust
pub fn execute_hook_safely(hook: &HookCommand) -> Result<()> {
match hook.execute().timeout(Duration::from_secs(3)) {
Ok(Ok(_)) => Ok(()),
Ok(Err(e)) => {
eprintln!("⚠️ Hook failed: {}", e);
Ok(()) // Non-fatal
}
Err(_) => {
eprintln!("⚠️ Hook timeout");
Ok(())
}
}
}
```
- [ ] Test with missing dependencies (`jq` not installed)
- [ ] Test with corrupted JSON files
- [ ] Test with permission errors
### Day 19: Documentation
- [ ] Write `docs/hooks/USER_GUIDE.md`:
- [ ] Quick start (5-minute setup)
- [ ] Migration guide
- [ ] Troubleshooting
- [ ] Configuration reference
- [ ] Write `docs/hooks/CLI_REFERENCE.md`:
- [ ] Each command with examples
- [ ] All flags documented
- [ ] Return codes and errors
- [ ] Update `README.md` with hooks section
- [ ] Add examples to `examples/hooks/`
### Day 20: Final Polish
- [ ] Run `cargo clippy` and fix warnings
- [ ] Run `cargo fmt`
- [ ] Verify test coverage >80%:
```bash
cargo tarpaulin --out Html
```
- [ ] Write release notes
- [ ] Tag version `v0.2.0-mvp`
---
## Post-MVP: v1.1 Planning (Future)
### SQLite Migration (v1.1.0)
- [ ] Add `rusqlite = { version = "0.32", optional = true }`
- [ ] Implement format detection:
```rust
fn detect_embedding_format(blob: &[u8]) -> EmbeddingFormat {
if blob.starts_with(b"\x93NUMPY") { Numpy }
else if blob.len() % 4 == 0 { RawF32 }
else { Unknown }
}
```
- [ ] Write SQLite → rvlite converter
- [ ] Add `--format` flag to `hooks migrate`
### Global Patterns System (v1.1.0)
- [ ] Design sync protocol
- [ ] Implement pattern merge algorithm
- [ ] Add privacy controls (opt-in/opt-out)
- [ ] Implement `hooks export` and `hooks import`
---
## Testing Checklist
### Unit Tests
- [ ] Template rendering (all platforms)
- [ ] Path resolution (Windows vs Unix)
- [ ] Command parsing (clap)
- [ ] JSON validation
- [ ] Migration rollback logic
### Integration Tests
- [ ] End-to-end: init → install → test hook triggers
- [ ] Migration preserves data integrity
- [ ] Hooks work after binary reinstallation
- [ ] Backward compatibility with legacy paths
### Manual Testing
- [ ] Fresh Ubuntu 22.04 VM
- [ ] Fresh macOS 14 VM
- [ ] Fresh Windows 11 VM (PowerShell)
- [ ] Codespaces environment
- [ ] GitHub Codespaces with Claude Code
---
## Definition of Done
### MVP Release Criteria
- ✅ All tests pass on Linux, macOS, Windows
- ✅ Test coverage >80%
- ✅ Documentation complete (USER_GUIDE + CLI_REFERENCE)
- ✅ Zero clippy warnings
- ✅ Manual testing on 3 platforms
- ✅ Migration tested with sample data (no data loss)
- ✅ Hooks survive binary reinstallation
- ✅ First-time setup completes in <5 minutes
### Performance Targets
- ✅ `hooks init` completes in <1 second
- ✅ `hooks install` completes in <2 seconds
- ✅ `hooks migrate` processes 1000 trajectories in <5 seconds
- ✅ Hook execution adds <50ms overhead
---
## Known Limitations (Document in Release Notes)
1. **SQLite migration not supported in MVP** - JSON-only migration available, SQLite coming in v1.1
2. **Global patterns deferred to v1.1** - Project-local patterns only in MVP
3. **Windows requires `jq` binary** - Not auto-installed, manual setup required
4. **No `hooks export/import` in MVP** - Manual file copying as workaround
---
## Risk Mitigation Checklist
- [ ] ✅ Windows testing (PowerShell, CMD, WSL)
- [ ] ✅ Atomic migration with backup/rollback
- [ ] ✅ Command injection prevention (`shell-escape`)
- [ ] ✅ Runtime path resolution (no hardcoded paths)
- [ ] ✅ Graceful failure (hooks never block Claude Code)
- [ ] ✅ Backward compatibility (legacy paths still work)
---
## Dependencies Required
### MVP
```toml
askama = "0.12" # Type-safe templates
shell-escape = "0.1" # Security
```
### Already Available
```toml
shellexpand = "3.1" # Path expansion ✅
clap = { ... } # CLI framework ✅
tokio = { ... } # Async runtime ✅
serde_json = { ... } # JSON parsing ✅
```
### v1.1
```toml
rusqlite = { version = "0.32", optional = true }
```
---
## Success Metrics (Track During Development)
| Metric | Target | Actual |
|--------|--------|--------|
| Test coverage | >80% | ___ |
| Platforms passing | 3/3 | ___ |
| Migration speed (1000 traj) | <5s | ___ |
| First-time setup | <5min | ___ |
| Hook overhead | <50ms | ___ |
| Data loss during migration | 0% | ___ |
---
## Quick Commands Reference
```bash
# Create branch
git checkout -b feature/portable-hooks-mvp
# Run tests
cargo test --package ruvector-cli --lib hooks
# Run integration tests
cargo test --package ruvector-cli --test hooks_integration
# Check coverage
cargo tarpaulin --out Html --package ruvector-cli
# Lint
cargo clippy --package ruvector-cli -- -D warnings
# Format
cargo fmt --package ruvector-cli
# Build for all platforms
cargo build --release --package ruvector-cli
# Test CLI commands
cargo run --package ruvector-cli -- hooks init
cargo run --package ruvector-cli -- hooks install --dry-run
```
---
**Last Updated**: 2025-12-25
**Status**: Ready for Development
**Estimated Completion**: 3-4 weeks from start

567
docs/hooks/REVIEW_REPORT.md Normal file
View file

@ -0,0 +1,567 @@
# Implementation Plan Code Review Report
**Document**: `/home/user/ruvector/docs/hooks/IMPLEMENTATION_PLAN.md`
**Reviewer**: Code Review Agent
**Date**: 2025-12-25
**Status**: ✅ APPROVED WITH REVISIONS
---
## Executive Summary
The implementation plan is **technically sound** and well-structured, but contains **3 critical issues** that would cause failures on Windows and during migration. After review:
- **Timeline optimized**: 6-8 weeks → **3-4 weeks for MVP** (50% reduction)
- **Critical fixes added**: Windows compatibility, SQLite migration risks, command injection
- **Scope refined**: Deferred complex features (global patterns, SQLite) to v1.1
- **Code quality improved**: Type-safe templates, idiomatic Rust patterns, security hardening
**Recommendation**: Proceed with implementation using revised plan (now v2.0).
---
## 1. Critical Issues (Must Fix)
### Issue #1: Windows Compatibility Broken
**Severity**: 🔴 Critical
**Location**: Hook templates (lines 1022, 1033)
**Impact**: Complete failure on Windows
**Problem**:
```json
{
"command": "/bin/bash -c '...'" // ❌ /bin/bash doesn't exist on Windows
}
```
**Fix Applied** (Section 11.1):
```rust
fn get_shell_wrapper() -> &'static str {
if cfg!(target_os = "windows") { "cmd /c" }
else { "/bin/bash -c" }
}
```
**Testing Required**:
- ✅ PowerShell environment
- ✅ WSL environment
- ✅ CMD.exe environment
---
### Issue #2: SQLite Migration Format Undefined
**Severity**: 🔴 Critical
**Location**: Milestone 4, lines 871-887
**Impact**: Data loss during migration
**Problem**:
```rust
let embedding: Vec<u8> = row.get(3)?;
let embedding_f32 = deserialize_embedding(&embedding)?;
// ❌ What format? Numpy? MessagePack? Raw bytes?
```
**Fix Applied**:
- **MVP**: Defer SQLite migration to v1.1
- **JSON-only migration** for MVP (2 days instead of 5-7 days)
- **Future**: Add format detection before deserialization
**Timeline Savings**: 3-5 days
---
### Issue #3: Runtime Path Resolution Missing
**Severity**: 🟡 High
**Location**: Template substitution (line 288)
**Impact**: Hooks break after binary reinstallation
**Problem**:
```rust
// Hardcodes path at install time
output.replace("{{RUVECTOR_CLI_PATH}}", "/usr/local/bin/ruvector");
// If user reinstalls via npm, path changes
```
**Fix Applied**:
```json
{
"command": "/bin/bash -c 'RUVECTOR=$(which ruvector || echo npx ruvector); $RUVECTOR hooks pre-edit'"
}
```
**Benefit**: Hooks survive binary moves/reinstalls
---
## 2. Scope Optimizations
### Optimization #1: Defer Global Patterns System
**Original**: Milestone 6 (4-5 days)
**Decision**: Move to v1.1
**Rationale**:
- Adds complexity (sync conflicts, privacy controls)
- Not required for core functionality
- Can be added non-disruptively later
**Timeline Savings**: 4-5 days
---
### Optimization #2: JSON Migration First
**Original**: SQLite + JSON migration (5-7 days)
**Revised**: JSON-only for MVP (2 days)
**Rationale**:
- Most users have existing JSON data (`.claude/intelligence/`)
- SQLite migration is complex (format detection, embedding deserialization)
- Lower risk to validate with JSON first
**Timeline Savings**: 3-5 days
---
### Optimization #3: Combine Milestones 2 & 5
**Original**: Separate CLI (5-7 days) + Templates (3-4 days)
**Revised**: Combined (4-5 days)
**Rationale**:
- Template engine and `hooks install` are tightly coupled
- Building together prevents context switching overhead
**Timeline Savings**: 4-6 days
---
## 3. Missing Elements Added
### 3.1 Error Handling Strategy
**Added to**: All hook execution points
```rust
pub fn execute_hook_safely(hook: &HookCommand) -> Result<()> {
match hook.execute().timeout(Duration::from_secs(3)) {
Ok(Ok(_)) => Ok(()),
Ok(Err(e)) => {
eprintln!("⚠️ Hook failed (non-fatal): {}", e);
Ok(()) // Don't block Claude Code
}
Err(_) => {
eprintln!("⚠️ Hook timeout");
Ok(())
}
}
}
```
**Benefit**: Hooks never block Claude Code operations
---
### 3.2 Atomic Migration with Rollback
**Added to**: Milestone 4a (Section 11.2)
```rust
pub fn migrate_with_safety(from: &Path, to: &Path) -> Result<MigrationStats> {
// 1. Backup existing data
// 2. Migrate to temporary location
// 3. Validate migrated data
// 4. Atomic swap
// On any error: restore backup
}
```
**Benefit**: Zero data loss risk
---
### 3.3 Security: Command Injection Prevention
**Added to**: All hook templates (Section 11.3)
```rust
use shell_escape::escape;
fn generate_hook_command(file_path: &str) -> String {
let escaped = escape(file_path.into());
format!(r#"ruvector hooks pre-edit {}"#, escaped)
}
```
**Prevents**: Malicious filenames like `; rm -rf /`
---
### 3.4 Windows-Specific Testing Checklist
**Added to**: Milestone 7
- ✅ PowerShell compatibility
- ✅ Path separator handling (`\` vs `/`)
- ✅ WSL environment testing
- ✅ Bundled `jq` binary (not in Windows PATH)
---
## 4. Code Quality Improvements
### 4.1 Type-Safe Templates
**Issue**: String-based templates are error-prone (line 288)
**Improvement**:
```rust
#[derive(Template)]
#[template(path = "hooks.json.j2")]
struct HookTemplate {
ruvector_cli_path: String,
project_root: String,
}
let rendered = HookTemplate { /* ... */ }.render()?;
```
**Benefit**: Compile-time template validation
**Dependency**: `askama = "0.12"` (added to Section 12)
---
### 4.2 Idiomatic Rust Error Handling
**Issue**: Non-idiomatic file writing (Appendix B, line 844)
**Before**:
```rust
fs::write(path, serde_json::to_string_pretty(&data)?)?;
```
**After**:
```rust
let mut file = File::create(path)?;
serde_json::to_writer_pretty(&mut file, &data)?;
file.sync_all()?; // Ensure durability
```
**Benefit**: Explicit error points, guaranteed disk sync
---
### 4.3 Extract Magic Numbers to Config
**Issue**: Hardcoded values in templates (lines 1022, 1033)
**Before**:
```json
{ "timeout": 3000 } // Magic number
```
**After** (add to `config.toml`):
```toml
[hooks]
timeout_ms = 3000
stderr_max_bytes = 300
max_retries = 2
```
**Benefit**: User-configurable timeouts
---
## 5. Leveraging Existing Crates
### 5.1 Already Available (No Work Needed)
| Feature | Crate | Location |
|---------|-------|----------|
| Path expansion | `shellexpand = "3.1"` | `Cargo.toml:74` ✅ |
| CLI framework | `clap` | `Cargo.toml:29` ✅ |
| Vector storage | `ruvector-core` | `Cargo.toml:21` ✅ |
| Async runtime | `tokio` | `Cargo.toml:34` ✅ |
**Action**: No additional dependencies needed for MVP!
---
### 5.2 Recommended Additions (v1.1)
```toml
[dependencies]
rusqlite = { version = "0.32", optional = true } # SQLite migration
askama = "0.12" # Type-safe templates
shell-escape = "0.1" # Security
[features]
sqlite-migration = ["rusqlite"]
```
---
### 5.3 Use rvlite for Vector Storage
**Current plan**: Generic `VectorDB`
**Better**: Use `rvlite` (already in ruvector ecosystem)
```rust
use rvlite::RvLite;
pub fn migrate_to_rvlite(trajectories: &[Trajectory]) -> Result<()> {
let db = RvLite::create(".ruvector/memory.rvdb")?;
db.sql("CREATE TABLE memories (id TEXT, embedding VECTOR(128))")?;
// rvlite supports SQL, SPARQL, and Cypher
}
```
**Benefit**:
- Unified storage layer
- WASM-compatible
- Already part of ruvector
---
## 6. MVP Definition
### What's Included (3-4 weeks)
**Week 1-2**: Foundation
- `ruvector hooks init` (create `.ruvector/` structure)
- `ruvector hooks install` (generate portable hooks)
- Template engine with runtime path resolution
- JSON-to-JSON migration
**Week 3**: Intelligence Layer
- Refactor `index.js` with `process.env.RUVECTOR_DATA_DIR`
- Zero hardcoded paths
- Test in fresh project
**Week 4**: Polish
- Cross-platform testing (Linux, macOS, Windows)
- `ruvector hooks stats` command
- Error handling + rollback
- Documentation
---
### What's Deferred to v1.1 (Additional 2-3 weeks)
❌ SQLite migration (needs format detection)
❌ Global patterns system (sync complexity)
❌ Export/import commands (nice-to-have)
`ruvector hooks enable/disable` (low priority)
---
## 7. Timeline Comparison
| Version | Original | Optimized | Savings |
|---------|----------|-----------|---------|
| **MVP** | N/A | **3-4 weeks** | N/A |
| **Full v1.0** | 6-8 weeks | 3-4 weeks | **50%** |
| **v1.1 (Full Features)** | 6-8 weeks | 5-7 weeks | ~15% |
**Key Changes**:
1. Defined MVP scope (didn't exist before)
2. Deferred complex features (SQLite, global patterns)
3. Combined milestones (CLI + Templates)
4. Focused on JSON migration (most common use case)
---
## 8. Risks Mitigated
| Risk | Original Plan | Revised Plan |
|------|---------------|--------------|
| Windows failure | ⚠️ Testing only | ✅ Conditional shell detection |
| Data loss | ⚠️ Checksums only | ✅ Atomic migration + rollback |
| Command injection | ❌ Not addressed | ✅ `shell-escape` crate |
| SQLite format errors | ⚠️ Assumed format | ✅ Deferred to v1.1 with detection |
| Hardcoded paths | ⚠️ Install-time subst | ✅ Runtime resolution |
---
## 9. Specific File Changes Required
### 9.1 Update Cargo.toml
**File**: `/home/user/ruvector/crates/ruvector-cli/Cargo.toml`
```toml
[dependencies]
# Add these lines:
askama = "0.12" # Type-safe templates
shell-escape = "0.1" # Security
# v1.1 only:
rusqlite = { version = "0.32", optional = true }
[features]
default = []
sqlite-migration = ["rusqlite"]
```
---
### 9.2 Create Hook Template
**File**: `/home/user/ruvector/crates/ruvector-cli/templates/hooks.json.j2`
```json
{
"hooks": {
"PreToolUse": [{
"matcher": "Bash",
"hooks": [{
"command": "{{ shell }} 'RUVECTOR=$(which ruvector || echo npx ruvector); $RUVECTOR hooks pre-command \"$CMD\"'"
}]
}]
}
}
```
---
### 9.3 Refactor Intelligence Layer
**File**: `/home/user/ruvector/.claude/intelligence/index.js`
**Line 20** (currently):
```javascript
const DATA_DIR = join(__dirname, 'data');
```
**Change to**:
```javascript
const DATA_DIR = process.env.RUVECTOR_DATA_DIR ||
join(process.cwd(), '.ruvector', 'intelligence') ||
join(__dirname, 'data'); // Fallback for legacy
```
---
## 10. Testing Strategy
### 10.1 Cross-Platform Testing
**Required Environments**:
- ✅ Ubuntu 22.04 (GitHub Actions)
- ✅ macOS 14 Sonoma (M1 + Intel)
- ✅ Windows 11 (PowerShell + CMD + WSL)
**Test Cases**:
1. Fresh project setup (`npx ruvector hooks init`)
2. Migration from existing `.claude/intelligence/`
3. Hooks trigger correctly (pre-edit, post-command)
4. Path resolution across platforms
5. Binary reinstallation (verify hooks still work)
---
### 10.2 Integration Tests
**File**: `/home/user/ruvector/crates/ruvector-cli/tests/hooks_integration.rs`
```rust
#[test]
fn test_hooks_init_creates_structure() {
let temp = tempdir()?;
run_cmd(&["ruvector", "hooks", "init"], &temp)?;
assert!(temp.path().join(".ruvector/config.toml").exists());
assert!(temp.path().join(".ruvector/intelligence").exists());
}
#[test]
fn test_migration_preserves_data() {
// Test JSON migration accuracy
}
#[test]
fn test_windows_shell_compatibility() {
// Platform-specific shell tests
}
```
---
## 11. Documentation Requirements
### 11.1 User-Facing Docs
**File**: `/home/user/ruvector/docs/hooks/USER_GUIDE.md`
**Sections**:
1. Quick Start (5-minute setup)
2. Migration from existing setup
3. Troubleshooting common issues
4. Configuration reference
---
### 11.2 API Reference
**File**: `/home/user/ruvector/docs/hooks/CLI_REFERENCE.md`
**Format**:
```markdown
## ruvector hooks init
Initialize hooks system in current project.
**Usage**: `npx ruvector hooks init [OPTIONS]`
**Options**:
- `--path <PATH>`: Custom directory (default: `./.ruvector`)
- `--template <NAME>`: Use template (default, minimal, advanced)
**Example**:
```bash
npx ruvector hooks init
npx ruvector hooks install
```
```
---
## 12. Success Metrics
### MVP Success Criteria
- ✅ Works on Linux, macOS, Windows (3/3 platforms)
- ✅ Migration completes in <5 seconds for 1000 trajectories
- ✅ Zero data loss in migration (validated via checksums)
- ✅ Hooks survive binary reinstallation
- ✅ First-time setup takes <5 minutes
- ✅ Zero hardcoded paths in generated hooks
### Code Quality Metrics
- ✅ Test coverage >80% for CLI commands
- ✅ All commands have examples in docs
- ✅ No clippy warnings on stable Rust
- ✅ All integration tests pass on 3 platforms
---
## 13. Recommendations Summary
### Immediate Actions (Before Coding)
1. ✅ **Approve revised plan** (v2.0 in IMPLEMENTATION_PLAN.md)
2. ✅ **Add dependencies** to Cargo.toml (Section 12)
3. ✅ **Create hook templates** directory structure
4. ✅ **Set up CI testing** for Windows/macOS/Linux
### Implementation Order
1. **Week 1**: Milestone 1 (Spec) + Start Milestone 2 (CLI scaffolding)
2. **Week 2**: Finish Milestone 2+5 (CLI + Templates)
3. **Week 3**: Milestone 3 (Intelligence Layer) + 4a (JSON Migration)
4. **Week 4**: Milestone 7 (Testing + Docs)
### v1.1 Planning
- Schedule SQLite migration for v1.1.0 (add format detection)
- Schedule global patterns for v1.1.0 (design sync protocol first)
- Consider team sharing features for v1.2.0
---
## 14. Final Verdict
**Status**: ✅ **APPROVED FOR IMPLEMENTATION** (with revisions)
**Confidence Level**: High (8/10)
**Remaining Risks**:
- Windows testing may uncover edge cases (10% probability)
- Intelligence layer refactor may need iteration (15% probability)
- User adoption may surface unforeseen use cases (20% probability)
**Overall Assessment**: The plan is **solid, well-researched, and implementable**. The revised timeline (3-4 weeks for MVP) is **realistic and achievable**. Critical issues have been identified and fixed. Code quality improvements will prevent future maintenance burden.
**Next Step**: Create feature branch `feature/portable-hooks-mvp` and begin Milestone 1.
---
**Reviewed By**: Code Review Agent
**Approved By**: [Pending]
**Implementation Start**: [TBD]
**Target MVP Release**: [TBD + 3-4 weeks]

View file

@ -0,0 +1,276 @@
# Hooks Implementation Plan - Code Review Summary
**Status**: ✅ APPROVED WITH CRITICAL FIXES
**Timeline**: Optimized from 6-8 weeks → **3-4 weeks for MVP**
**Risk Level**: Low-Medium (major risks mitigated)
---
## 1. Critical Issues Found (Must Fix)
### 🔴 Issue #1: Windows Compatibility Broken
**Impact**: Complete failure on Windows
**Fix**: Use conditional shell detection
```rust
fn get_shell_wrapper() -> &'static str {
if cfg!(target_os = "windows") { "cmd /c" }
else { "/bin/bash -c" }
}
```
### 🔴 Issue #2: SQLite Migration Undefined Format
**Impact**: Data loss risk
**Fix**: Defer SQLite to v1.1, use JSON-only migration for MVP
### 🔴 Issue #3: Path Resolution Breaks After Reinstall
**Impact**: Hooks stop working after binary moves
**Fix**: Use runtime resolution instead of install-time substitution
```bash
RUVECTOR=$(which ruvector || echo npx ruvector); $RUVECTOR hooks pre-edit
```
---
## 2. Optimizations Applied
| Change | Time Saved | Rationale |
|--------|------------|-----------|
| Defer global patterns to v1.1 | 4-5 days | Adds complexity without MVP value |
| JSON migration only (defer SQLite) | 3-5 days | Most users have JSON data |
| Combine CLI + Templates milestones | 4-6 days | Reduce context switching |
| **Total Savings** | **~50%** | MVP: 3-4 weeks vs 6-8 weeks |
---
## 3. Missing Elements Added
**Error handling**: Hooks never block Claude Code operations
**Atomic migration**: Backup → Migrate → Validate → Swap with rollback
**Security**: Command injection prevention with `shell-escape`
**Windows testing**: PowerShell, CMD, WSL compatibility checklist
---
## 4. Code Quality Improvements
### Type-Safe Templates
**Before**: String-based templates (error-prone)
**After**: `askama` crate with compile-time validation
### Idiomatic Rust
**Before**: `fs::write(path, json)?`
**After**: `serde_json::to_writer_pretty()` + `file.sync_all()`
### Configuration
**Before**: Magic numbers (timeout: 3000)
**After**: Extract to `config.toml` for user customization
---
## 5. Leveraging Existing Crates
**Already Available** (no work needed):
- `shellexpand = "3.1"` - Path expansion
- `clap` - CLI framework
- `ruvector-core` - Vector storage
- `tokio` - Async runtime
**Add for MVP**:
- `askama = "0.12"` - Type-safe templates
- `shell-escape = "0.1"` - Security
**Add for v1.1**:
- `rusqlite = "0.32"` - SQLite migration
---
## 6. MVP Definition (3-4 Weeks)
### Week 1-2: Foundation
- `ruvector hooks init` - Create `.ruvector/` structure
- `ruvector hooks install` - Generate portable hooks
- Template engine with runtime path resolution
- JSON-to-JSON migration
### Week 3: Intelligence Layer
- Refactor `index.js` for dynamic paths
- Zero hardcoded paths
- Test in fresh project
### Week 4: Polish
- Cross-platform testing (Linux, macOS, Windows)
- `ruvector hooks stats` command
- Error handling + rollback
- Documentation
### Deferred to v1.1
❌ SQLite migration (needs format detection)
❌ Global patterns system (sync complexity)
❌ Export/import commands (nice-to-have)
---
## 7. Concrete Edits Made
### Updated IMPLEMENTATION_PLAN.md
1. **Timeline table** (Section 8): Added MVP vs Full Release split
2. **Risk assessment** (Section 6.1): Added 2 new risks with mitigations
3. **Critical fixes** (NEW Section 11): Windows compatibility, rollback, security
4. **Dependencies** (NEW Section 12): Specific Cargo.toml additions
5. **Conclusion**: Updated with MVP achievements and timeline
### Created REVIEW_REPORT.md
- 14-section detailed technical review
- Platform-specific testing checklist
- Integration test examples
- Documentation requirements
- Success metrics
---
## 8. Next Steps
### Immediate (Before Coding)
1. ✅ Review and approve this document
2. ✅ Add dependencies to `crates/ruvector-cli/Cargo.toml`
3. ✅ Create `crates/ruvector-cli/templates/` directory
4. ✅ Set up CI for Windows/macOS/Linux testing
### Week 1
1. Create feature branch: `feature/portable-hooks-mvp`
2. Implement Milestone 1 (Specification)
3. Start CLI scaffolding (Milestone 2)
### Week 2-4
Follow MVP implementation order (see Section 6)
---
## 9. Risks & Mitigations
| Risk | Mitigation |
|------|------------|
| Windows edge cases | Comprehensive testing on PowerShell, CMD, WSL |
| Data loss | Atomic migration with backup/rollback |
| Command injection | `shell-escape` crate for all user inputs |
| Hooks break after reinstall | Runtime path resolution via `which` |
| SQLite format errors | Deferred to v1.1 with format detection |
---
## 10. Files Modified
### 1. IMPLEMENTATION_PLAN.md
**Changes**:
- Updated timeline (Section 8)
- Added critical fixes (Section 11)
- Added dependency recommendations (Section 12)
- Updated conclusion with MVP scope
**Lines Modified**: ~100 lines added/changed
### 2. REVIEW_REPORT.md (New)
**Purpose**: Detailed technical review with testing strategy
### 3. REVIEW_SUMMARY.md (This File)
**Purpose**: Executive summary for quick review
---
## 11. Recommended File Changes
### Cargo.toml
**File**: `/home/user/ruvector/crates/ruvector-cli/Cargo.toml`
```toml
[dependencies]
askama = "0.12" # MVP
shell-escape = "0.1" # MVP
rusqlite = { version = "0.32", optional = true } # v1.1
[features]
sqlite-migration = ["rusqlite"]
```
### index.js
**File**: `/home/user/ruvector/.claude/intelligence/index.js`
**Line 20**: Change from:
```javascript
const DATA_DIR = join(__dirname, 'data');
```
To:
```javascript
const DATA_DIR = process.env.RUVECTOR_DATA_DIR ||
join(process.cwd(), '.ruvector', 'intelligence') ||
join(__dirname, 'data'); // Legacy fallback
```
---
## 12. Success Metrics
### Technical
- ✅ Works on 3 platforms (Linux, macOS, Windows)
- ✅ Migration <5s for 1000 trajectories
- ✅ 100% data integrity (checksums)
- ✅ Test coverage >80%
### User Experience
- ✅ First-time setup <5 minutes
- ✅ Zero hardcoded paths
- ✅ Hooks survive reinstallation
- ✅ Clear error messages
---
## 13. Final Verdict
**Approval**: ✅ **APPROVED FOR IMPLEMENTATION**
**Confidence**: 8/10
**Timeline**: 3-4 weeks for MVP (realistic and achievable)
**Remaining Risks**: Low (10-20% chance of minor delays)
**Overall Assessment**: Plan is solid, well-researched, and implementable. Critical issues identified and fixed. Timeline optimized by 50% while maintaining quality.
---
**Reviewed By**: Code Review Agent (ruvector)
**Review Date**: 2025-12-25
**Plan Version**: v2.0 (Post-Review)
**Next Step**: Approve and begin implementation
---
## Appendix: Quick Reference
### Commands Being Added
```bash
npx ruvector hooks init # Initialize .ruvector/
npx ruvector hooks install # Generate Claude Code hooks
npx ruvector hooks migrate --from .claude/intelligence # Migrate data
npx ruvector hooks stats # Show learning statistics
```
### File Structure (MVP)
```
.ruvector/
├── config.toml # Project settings
├── intelligence/
│ ├── trajectories.json # Learning data
│ ├── patterns.json # Q-learning patterns
│ └── memory.rvdb # Vector memory (rvlite)
└── .gitignore
```
### Dependencies Added
- `askama` - Type-safe templates
- `shell-escape` - Security
- `rusqlite` (v1.1) - SQLite migration
### Existing Dependencies Leveraged
- `shellexpand` - Path resolution ✅
- `clap` - CLI framework ✅
- `ruvector-core` - Vector storage ✅
- `tokio` - Async runtime ✅