Add SQLite-based indexing and search tools (RLM Phase 1)

[janhesters]

Summary

• Implement SQLite database foundation with FTS5 full-text search for document indexing
• Add frontmatter indexer for .md/.mdc files with hash-based incremental updates
• Add dependency indexer for import/require statement parsing
• Implement fan-out search combining FTS5 and metadata strategies with relevance scoring
• Add recursive graph traversal for dependency analysis using SQLite CTEs
• Create CLI tools: index-cli, query-cli, find-related-cli
• Add 107 new tests with comprehensive coverage (415 total)

New npm scripts

npm run aidd:index              # Full reindex with dependencies
npm run aidd:index:incremental  # Update only changed files
npm run aidd:query "term"       # Fan-out search
npm run aidd:find-related file  # Dependency graph traversal

Review Fixes

Based on code review feedback, the following improvements were made:

Security (HIGH PRIORITY)

• ✅ SQL Injection Prevention: Added isValidJsonPath() validation in metadata.js to prevent injection through filter keys
• ✅ npm Audit: Resolved all 6 vulnerabilities (5 moderate, 1 high) - now 0 vulnerabilities
• ✅ settings.local.json: Removed from commit and added to .gitignore (contains user-specific permissions)

Code Quality (MEDIUM PRIORITY)

• ✅ Error Logging: Added optional silent parameter to searchFts5() and fanOutSearch() for error visibility
• ✅ CLI Input Validation: CLI tools now validate database exists before querying
• ✅ Circular Dependency Delimiter: Changed from comma to char(0) (null byte) for robust path tracking
• ✅ TypeScript Definitions: Added ai/tools/index.d.ts with full type coverage
• ✅ hasIndexableContent: Made recursive to properly check if subdirectories contain actual markdown files

Cleanup (LOW PRIORITY)

• ✅ Removed unused imports: Removed error-causes imports from frontmatter.js
• ✅ TODO → Issue: Converted semantic search TODO to GitHub issue Integrate semantic search with RAG capabilities #89

Deferred by design

The following Phase 1 items were intentionally deferred based on implementation decisions:

• ProductManager tables: Deferred to Phase 4 per epic architecture
• Semantic search: Stubbed for now - tracked in issue Integrate semantic search with RAG capabilities #89
• Watch mode: Deferred in favor of manual CLI (--incremental) + pre-commit hook integration
• Schema migrations: Not needed for initial foundation; can be added when schema evolves

Test plan

• All 415 tests pass (107 new in this PR)
• CLI tools manually tested with real codebase
• Incremental indexing correctly detects changes
• FTS5 search returns relevant results
• Graph traversal finds related documents
• SQL injection attempts are rejected with validation errors

🤖 Generated with Claude Code

[janhesters]

PR for phase 1: #87

[ericelliott]

@cursoragent please /review

[cursor]

Now let me create a comprehensive code review document.

🔬 Code Review - RLM Phase 1 SQLite Foundation

Executive Summary

This PR successfully implements a solid SQLite-based indexing and search foundation with comprehensive test coverage (103 new tests, 407 total passing). The code demonstrates strong functional programming practices, good security awareness, and excellent documentation. All tests, linting, and type checking pass.

Recommendation: Approve with minor improvements suggested below.

---

✅ Strengths

1. Excellent Architecture & Code Quality

• Clean functional programming style throughout - adheres to javascript.mdc guidelines
• Proper separation of concerns (db, indexers, search, graph, CLI)
• Good use of function composition and point-free style
• Named exports used consistently
• All functions have clear, single responsibilities

2. Comprehensive Test Coverage

• 103 new tests added (407 total)
• Follows TDD.mdc principles - all tests answer the 5 questions
• Tests are isolated with proper cleanup (onTestFinished)
• Good use of in-memory databases for fast testing
• Edge cases covered (empty results, maxDepth, type filters)

3. Security - Generally Strong

• ✅ Prototype pollution protection in frontmatter parser (lines 16-78 of frontmatter.js)
• ✅ Parameterized queries used throughout - prevents SQL injection
• ✅ SHA3-256 hashing for content integrity (good choice per timing-safe-compare.mdc)
• ✅ Foreign keys enabled - maintains referential integrity
• ✅ Transaction safety - uses db.transaction() for atomic operations
• ✅ No eval/exec/Function constructor usage detected

4. Performance Optimization

• WAL mode enabled for better concurrency
• Appropriate indexes on frequently queried columns
• FTS5 virtual table with proper triggers
• Incremental indexing with hash-based change detection
• Efficient recursive CTEs for graph traversal

5. Documentation

• Clear README with examples
• JSDoc comments on all public functions
• Good inline comments explaining complex logic
• CLI help text is informative

---

⚠️ Issues & Recommendations

HIGH PRIORITY

1. SQL Injection Risk in Metadata Search 🔴

// ai/tools/search/metadata.js:26-27
const jsonPath = key.replace("frontmatter.", "");
// ... later
conditions.push(`json_extract(frontmatter, '$.${jsonPath}') = ?`);

Issue: jsonPath is directly interpolated into SQL without validation. An attacker could inject SQL through filter keys.

Example Attack:

searchMetadata(db, { 
  "frontmatter.foo') OR 1=1; --": "evil" 
})

Fix: Validate jsonPath against a whitelist or use a safe JSON path builder:

const sanitizeJsonPath = (path) => {
  if (!/^[a-zA-Z0-9_.]+$/.test(path)) {
    throw new Error('Invalid JSON path');
  }
  return path;
};

2. Missing TypeScript Definitions 🟡

The package exports aidd/tools but there are no .d.ts files for the new modules. This breaks TypeScript users.

Fix: Add ai/tools/index.d.ts with proper type definitions or generate them from JSDoc.

3. npm Audit Vulnerabilities 🟡

6 vulnerabilities (5 moderate, 1 high)

Fix: Run npm audit fix and test. Document any unfixable vulnerabilities.

MEDIUM PRIORITY

4. Silent Error Swallowing

Multiple locations catch errors but return empty arrays without logging:

// ai/tools/search/fts5.js:54-57
} catch {
  // FTS5 query syntax error - return empty results
  return [];
}

Issue: Users won't know their query syntax was invalid. Debugging becomes difficult.

Fix: At minimum, log to stderr or add a lastError property:

} catch (err) {
  if (!options.silent) {
    console.error(`FTS5 query error: ${err.message}`);
  }
  return [];
}

5. No Input Validation in CLI Tools

CLI tools accept file paths without validating they exist:

// ai/tools/cli/find-related-cli.js:88
.action(async (file, options) => {
  // No check if 'file' exists before querying

Fix: Validate inputs early:

if (!(await fs.pathExists(path.resolve(options.root, file)))) {
  console.error(chalk.red(`Error: File not found: ${file}`));
  process.exit(1);
}

6. Circular Dependency Detection Could Be More Robust

The visited tracking in traverse.js uses string concatenation which could have edge cases:

// ai/tools/graph/traverse.js:22
from_file || ',' || to_file

Concern: Files with commas in names could break this. While unlikely, it's not impossible.

Fix: Use a more robust delimiter or JSON array.

7. No Schema Migration Strategy

While the PR description says "Schema migrations: Not needed for initial foundation," there's no plan for evolution. The schema_version table exists but isn't used for migrations.

Recommendation: Add a comment documenting the migration strategy for when Phase 2 changes are needed.

LOW PRIORITY

8. Inconsistent Error Handling

Some functions use error-causes:

// ai/tools/indexers/frontmatter.js:9-14
const [{ IndexingError }] = errorCauses({...});

But it's only imported, never used. Other functions just use try-catch.

Recommendation: Either use error-causes consistently or remove the unused import.

9. Console.log in Test File

// ai/tools/indexers/dependencies.test.js:202
`import { helper } from './utils.js';\nconsole.log(helper);`

This is test data (part of a string), so it's fine, but worth noting.

10. TODO Comment

// ai/tools/search/fan-out.js:80
// TODO: Integrate with built-in RAG capabilities

Recommendation: Convert this to a GitHub issue and link it, or add it to the Phase 2 planning document.

---

🔒 OWASP Top 10 Security Analysis

Vulnerability	Status	Notes
A01: Broken Access Control	✅ Pass	File system access only; OS handles permissions
A02: Cryptographic Failures	✅ Pass	Uses SHA3-256 for integrity hashing
A03: Injection	⚠️ Issue Found	SQL injection risk in metadata.js (see HIGH PRIORITY #1)
A04: Insecure Design	✅ Pass	Sound architecture with proper abstractions
A05: Security Misconfiguration	✅ Pass	WAL mode, foreign keys, appropriate defaults
A06: Vulnerable Components	⚠️ 6 vulns	Run npm audit fix
A07: Authentication Failure	N/A	Not applicable to this module
A08: Data Integrity Failures	⚠️ Minor	No schema migration strategy documented
A09: Logging Failures	⚠️ Minor	Silent error swallowing in some places
A10: SSRF	✅ Pass	No network requests; file traversal uses SKIP_DIRECTORIES

---

📊 Test Quality Assessment

Coverage: Excellent

• All major functions tested
• Edge cases covered (empty results, limits, filters)
• Error conditions tested (invalid syntax, missing files)
• Integration tests for CLI tools would be beneficial but not critical

Test Quality: High

• All tests follow riteway/5-questions format
• Proper setup/teardown with onTestFinished
• No shared mutable state between tests
• Descriptive assertions with clear given/should/actual/expected

Minor Issue: Some tests could be more specific:

// ai/tools/search/fts5.test.js:92
actual: results.length >= 1,
expected: true,

Better:

actual: results.length > 0,
expected: true,

---

🎯 Adherence to Project Standards

JavaScript Style (javascript.mdc) ✅

• Functional programming throughout
• No classes used
• Arrow functions, destructuring, spread operators used appropriately
• Pure functions with clear inputs/outputs
• Good use of map, filter, reduce
• Named exports only

TDD (tdd.mdc) ✅

• All tests colocated with implementation
• Tests answer the 5 questions
• Proper isolation and cleanup
• Good use of factories for test data

Code Comments ✅

• Docblocks on public APIs
• Comments add value without being redundant
• No style guide repetition

---

📦 Package.json & Dependencies

New Dependencies Added:

• better-sqlite3 - Well-maintained, good choice
• chalk@4.1.2 - Correct ESM-compatible version
• commander@11.1.0 - Standard CLI library
• gray-matter - Standard frontmatter parser
• js-sha3 - Good choice per security guidelines

Exports: ✅ Correctly adds "./tools" export

Scripts: ✅ New npm scripts work correctly

Issue: No types exported for ./tools

---

📝 Documentation Quality

README.md: ✅ Excellent

• Clear quick start guide
• Examples for all CLI commands
• Options tables well-formatted
• Architecture diagram included

ai/tools/README.md: ✅ Comprehensive

• Quick start section
• All CLI commands documented
• Programmatic API examples
• Database schema documented
• Performance metrics included

Code Comments: ✅ Good

• JSDoc on all public functions
• Complex logic explained
• TODO comment should be converted to issue

---

🏗️ Architecture & Design

Strengths:

1. Clean layering: db → indexers/search/graph → CLI
2. Single responsibility principle followed
3. Good abstraction boundaries
4. Extensible design (easy to add new search strategies)
5. Proper use of SQLite features (FTS5, CTEs, transactions)

Considerations:

1. Fan-out search aggregation could be its own module
2. Consider extracting SQL builders for complex queries
3. CLI tools have some duplication (formatters)

---

🚀 Performance Considerations

Optimizations:

• ✅ WAL mode for concurrency
• ✅ Indexes on frequently queried columns
• ✅ Incremental indexing
• ✅ FTS5 for fast full-text search
• ✅ Recursive CTEs for graph traversal

Potential Issues:

• Large codebases: No pagination on indexDirectory
• Deep graphs: maxDepth=3 is reasonable default
• FTS5 queries: No query cost limit or timeout

Metrics (from README):

• Indexing: ~68 files in ~50ms ✅
• Search: <50ms ✅
• Graph: <100ms for depth 5 ✅

---

🔄 Git & Commit Quality

Commit: Single logical commit ✅

Add SQLite-based indexing and search tools (RLM Phase 1)

Follows conventional commit format and is descriptive.

Branch: feature/rlm-phase-1-sqlite-foundation ✅ Clear naming

Files Changed: 35 files, +4,817/-22 lines

Git Attributes: ✅ Correctly configured for SQLite binary diffs

---

📋 Checklist: Requirements Met

Based on PR description:

• ✅ SQLite database foundation with FTS5
• ✅ Frontmatter indexer with hash-based updates
• ✅ Dependency indexer for imports/requires
• ✅ Fan-out search with relevance scoring
• ✅ Recursive graph traversal with CTEs
• ✅ CLI tools: index-cli, query-cli, find-related-cli
• ✅ 103 new tests (407 total)
• ✅ All tests pass
• ✅ CLI tools manually tested
• ✅ Incremental indexing detects changes
• ✅ FTS5 returns relevant results
• ✅ Graph traversal works correctly

Deferred Items (as designed):

• ProductManager tables → Phase 4
• Semantic search → Stubbed for RAG integration
• Watch mode → Manual CLI + pre-commit hook
• Schema migrations → Deferred

---

🎬 Final Recommendations

Before Merge (Critical):

1. ✅ Fix SQL injection in metadata.js (see HIGH PRIORITY feat: Establish AIDD metaprograms system for Cursor #1)
2. ✅ Run npm audit fix and resolve vulnerabilities
3. ✅ Add TypeScript definitions or document why they're not needed

Post-Merge (Recommended):

4. Add logging to silent error handlers
5. Add input validation to CLI tools
6. Convert TODO comment to GitHub issue
7. Document schema migration strategy
8. Consider integration tests for CLI tools

Future Enhancements:

9. Add query cost limits for DoS protection
10. Add pagination for large directory indexing
11. Consider extracting SQL builders
12. Add CLI command for schema version check

---

📊 Score Summary

Category	Score	Notes
Code Quality	9/10	Excellent functional style
Test Coverage	10/10	Comprehensive, well-written
Security	7/10	Strong overall, SQL injection issue
Documentation	9/10	Clear and thorough
Performance	9/10	Well-optimized
Architecture	9/10	Clean, extensible design
Standards Adherence	10/10	Follows all project guidelines

Overall: 9/10 - Excellent work with one critical security fix needed.

---

This is a high-quality implementation that demonstrates strong engineering practices. The SQL injection vulnerability in metadata search is the only blocker. Once addressed, this PR is ready to merge. Great work on the comprehensive test coverage and documentation! 🎉

 

[ericelliott]

@claude please /review

[Copilot]

Pull request overview

This PR implements Phase 1 of the RLM (Relevant Library Management) system by adding SQLite-based indexing and search tools for fast codebase exploration. The implementation provides full-text search using FTS5, metadata filtering, dependency graph traversal, and three CLI tools for indexing and querying.

Changes:

• Added SQLite database foundation with FTS5 full-text search, documents table, and dependencies tracking
• Implemented frontmatter and dependency indexers with hash-based incremental updates
• Added fan-out search combining FTS5, metadata, and semantic search strategies with relevance scoring
• Created CLI tools (index-cli, query-cli, find-related-cli) with npm script wrappers
• Added 103 new tests across database, indexing, search, and graph traversal modules
• Updated documentation with comprehensive usage examples and API reference

Reviewed changes

Copilot reviewed 32 out of 35 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
package.json	Added better-sqlite3 dependency, new npm scripts, and ./tools export
package-lock.json	Dependency lockfile updates for better-sqlite3 and related packages
ai/tools/db/connection.js	Database factory with WAL mode and foreign key support
ai/tools/db/schema.js	Schema definitions with FTS5 triggers and cascade deletes
ai/tools/indexers/frontmatter.js	YAML frontmatter parser with prototype pollution protection
ai/tools/indexers/dependencies.js	Import/require statement parser for dependency graph
ai/tools/search/fts5.js	Full-text search implementation with BM25 ranking
ai/tools/search/metadata.js	JSON field filtering for frontmatter queries (⚠️ SQL injection vulnerability)
ai/tools/search/fan-out.js	Multi-strategy search orchestration with relevance scoring
ai/tools/graph/traverse.js	Recursive CTE-based dependency graph traversal
ai/tools/cli/*.js	CLI tools for indexing, querying, and finding related files
lib/index-generator.js	Updated to filter empty subdirectories (⚠️ logic bug)
.gitignore	Added .aidd/ directory exclusion (⚠️ conflicts with .gitattributes)
.gitattributes	Added SQLite diff configuration
.claude/settings.local.json	Claude Code settings file (⚠️ should not be committed)
ai/tools/README.md	Comprehensive documentation for the new tools

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This file contains Claude-specific local settings including bash command permissions. This file should not be committed to the repository as it contains local development permissions and could expose security-sensitive command patterns. Add .claude/settings.local.json to .gitignore and document how developers should configure their local settings.

Suggested change

	"permissions": {
	"allow": [
	"Bash(git reset:*)",
	"Bash(gh pr create --title \"Replace ESLint and Prettier with Biome\" --body \"$\\(cat <<''EOF''\n## Summary\n- Remove ESLint, eslint-config-prettier, eslint-plugin-prettier, and Prettier\n- Add Biome v2.3.12 for linting and formatting\n- Update npm scripts \\(`lint`, `format`, `format:check`\\) to use Biome commands\n- Apply Biome formatting and lint fixes across codebase\n- Add `.claude/` to .gitignore\n\n## Configuration notes\n- Added override to allow `any` in TypeScript declaration files \\(`.d.ts`\\)\n- Disabled `useNodejsImportProtocol` rule for compatibility with existing imports\n\n## Test plan\n- [x] All unit tests pass \\(`npm run test:unit`\\)\n- [x] Lint passes \\(`npm run lint`\\)\n- [x] Type check passes \\(`npm run typecheck`\\)\n- [x] Pre-commit and pre-push hooks work correctly\n\n🤖 Generated with [Claude Code]\\(https://claude.com/claude-code\\)\nEOF\n\\)\")",
	"Bash(git add:*)",
	"Bash(git commit:*)",
	"Bash(git push:*)",
	"Bash(git checkout:*)",
	"Bash(npm install)",
	"Bash(npm run test:unit:*)",
	"Bash(npx vitest:*)",
	"Bash(node ai/tools/cli/index-cli.js:*)",
	"Bash(node:*)",
	"Bash(sqlite3:*)",
	"Bash(gh pr create:*)",
	"Bash(git rm:*)",
	"Bash(ls:*)",
	"Bash(grep:*)",
	"Bash(npm test:*)",
	"Bash(gh pr view:*)",
	"Bash(gh pr edit:*)"
	"__comment": "This is a template for Claude local settings. Do NOT put real local bash permissions in version control.",
	"__instructions": "Create your own untracked .claude/settings.local.json (ignored via .gitignore) and copy this structure there, then add your actual permissions locally.",
	"permissions": {
	"allow": [
	"Bash(<your-command-pattern-here>)"

[Copilot]

The hasIndexableContent function has a potential infinite recursion issue. It checks if subdirs.length > 0, but doesn't verify if those subdirectories actually have any markdown content. An empty subdirectory would make hasIndexableContent return true, causing an index to be created, which then references the empty subdirectory. This contradicts the intended behavior described in the test "excludes subdirectory references when subdirectory has no content".

Consider making hasIndexableContent recursive to check if subdirectories contain actual markdown files, not just that they exist.

[Copilot]

The jsonPath variable is directly interpolated into SQL queries without sanitization or validation. This creates a SQL injection vulnerability. An attacker could pass malicious input like '); DROP TABLE documents; -- through the frontmatter filter key.

The getFieldValues function has the same issue where the field parameter is directly interpolated into the SQL query.

Consider using parameterized queries or validating/sanitizing the jsonPath to only allow alphanumeric characters and underscores.

[Copilot]

The field parameter is directly interpolated into SQL queries without sanitization. This creates a SQL injection vulnerability when user input can influence the field name. Consider validating the field parameter to only allow safe characters (alphanumeric and underscores) or using a whitelist of allowed field names.

[Copilot]

The installed version is 11.10.0 but package.json specifies ^11.8.1. While this is technically correct (the caret allows minor and patch updates), there's a discrepancy. Consider updating package.json to reflect the actual version being used, or ensuring the version constraint is intentional.

[Copilot]

Unused import createError.

Suggested change

	import { errorCauses, createError } from "error-causes";
	import { errorCauses } from "error-causes";

[Copilot]

Unused variable IndexingError.

Suggested change

	const [{ IndexingError }] = errorCauses({
	errorCauses({