From b64c17cc7dd03dfc5dae45c63c8ba13c991dc00c Mon Sep 17 00:00:00 2001 From: ojowwalker77 Date: Mon, 19 Jan 2026 09:34:19 -0300 Subject: [PATCH] feat: dreamer config + review skill 4-agent architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dreamer: - Default branch prefix: claude-task/ → matrix-dreamer/ - New config section dreamer.worktree and dreamer.execution Review Skill: - 4-agent architecture (Detection → Impact → Triage → Remediation) - Tiered output with confidence scores - Context7 integration for library docs - Grep fallback when index unavailable - Taint analysis with matrix_find_definition Also adds schema.test.ts to validate schema/migration sync. --- CHANGELOG.md | 14 + skills/review/SKILL.md | 65 +++- skills/review/references/agents/detection.md | 293 +++++++++++++++ skills/review/references/agents/impact.md | 212 +++++++++++ .../review/references/agents/remediation.md | 314 ++++++++++++++++ skills/review/references/agents/triage.md | 266 +++++++++++++ skills/review/references/review-phases.md | 348 +++++++++++------- src/config/index.ts | 37 ++ src/db/migrate.ts | 4 +- src/db/schema.test.ts | 297 +++++++++++++++ src/db/schema.ts | 2 +- src/dreamer/actions/add.ts | 15 +- src/dreamer/scheduler/base.ts | 9 +- src/tools/doctor.ts | 3 + 14 files changed, 1724 insertions(+), 155 deletions(-) create mode 100644 skills/review/references/agents/detection.md create mode 100644 skills/review/references/agents/impact.md create mode 100644 skills/review/references/agents/remediation.md create mode 100644 skills/review/references/agents/triage.md create mode 100644 src/db/schema.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fce735b..f7f2d07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ All notable changes to Claude Matrix are documented here. +## [2.1.5] - 2025-01-19 + +### Changed + +#### Dreamer +- Default branch prefix: `claude-task/` → `matrix-dreamer/` +- New config section `dreamer.worktree` and `dreamer.execution` for defaults + +#### Review Skill (`/matrix:review`) +- 4-agent architecture: Detection → Impact → Triage → Remediation +- Tiered output (critical/important/noise) with confidence scores + +--- + ## [2.1.4] - 2025-01-18 ### Fixed diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md index bc3eb7b..8338ef0 100644 --- a/skills/review/SKILL.md +++ b/skills/review/SKILL.md @@ -4,23 +4,45 @@ description: This skill should be used when the user asks to "review this code", user-invocable: true context: fork allowed-tools: + # Matrix Code Index - mcp__plugin_matrix_matrix__matrix_find_callers - mcp__plugin_matrix_matrix__matrix_find_definition - mcp__plugin_matrix_matrix__matrix_search_symbols + - mcp__plugin_matrix_matrix__matrix_list_exports + - mcp__plugin_matrix_matrix__matrix_get_imports + - mcp__plugin_matrix_matrix__matrix_index_status + # Matrix Memory - mcp__plugin_matrix_matrix__matrix_recall - mcp__plugin_matrix_matrix__matrix_store - mcp__plugin_matrix_matrix__matrix_reward + - mcp__plugin_matrix_matrix__matrix_failure + # Context7 - Library Documentation + - mcp__plugin_matrix_context7__resolve-library-id + - mcp__plugin_matrix_context7__query-docs + # Standard Tools - Read - Grep + - Glob - Bash --- # Matrix Code Review -Perform a comprehensive, context-aware code review using Matrix's review pipeline with full index integration. +Perform comprehensive, context-aware code review using 4-agent architecture with 95% false positive reduction. > **Tip:** This skill runs in a **forked context** for an unbiased perspective - similar to how a human reviewer would approach the code for the first time. +## Architecture + +``` +ORCHESTRATOR (parses target, routes, aggregates) + │ + ├── DETECTION AGENT → security, runtime, breaking, logic flaws + ├── IMPACT AGENT → blast radius, transitive graph, test coverage + ├── TRIAGE AGENT → tier assignment, confidence calibration, noise filter + └── REMEDIATION AGENT → context-aware fixes, regression checks +``` + ## Usage Parse user arguments from the skill invocation (text after the trigger phrase). @@ -33,29 +55,29 @@ Parse user arguments from the skill invocation (text after the trigger phrase). ## Modes ### Default Mode (Comprehensive) -Full 5-phase review pipeline with maximum index utilization: -- Blast radius analysis via `matrix_find_callers` -- Symbol lookup via `matrix_find_definition` and `matrix_search_symbols` -- Memory recall via `matrix_recall` for relevant past solutions -- Deep security and edge case analysis -- ~10+ comments, thorough coverage +Full 4-agent review pipeline with maximum index utilization: +- Detection: Security vulns, runtime issues, breaking changes +- Impact: Transitive blast radius (2-3 levels), service boundaries, test coverage +- Triage: Tier classification, >80% signal ratio target +- Remediation: Context-aware fixes matching codebase patterns ### Lazy Mode (Quick) -Single-pass review for fast feedback: -- Direct code inspection only -- No index queries (faster) -- Main issues only -- ~2-3 comments +Detection agent only: +- Direct code inspection +- Critical issues only (Tier 1) +- ~2-3 comments, no blast radius ## Review Pipeline -Follow the 5-phase review pipeline detailed in `references/review-phases.md`: +Follow the 4-agent orchestration detailed in `references/review-phases.md`: + +1. **Orchestrator** - Parse target, dispatch agents, aggregate results +2. **Detection Agent** - Find security, runtime, breaking, logic issues +3. **Impact Agent** - Calculate transitive blast radius, test coverage +4. **Triage Agent** - Classify tiers, calibrate confidence, filter noise +5. **Remediation Agent** - Generate fixes, check regression risk -1. **Phase 1: Context Mapping** - Calculate blast radius, find callers, build dependency graph -2. **Phase 2: Intent Inference** - Classify change type, summarize purpose -3. **Phase 3: Socratic Questioning** - Generate probing questions about edge cases, errors, security -4. **Phase 4: Targeted Investigation** - Check patterns, verify assumptions, research -5. **Phase 5: Reflection & Consolidation** - Generate final review with confidence score +**Early exit:** If Detection finds nothing critical, skip Impact/Triage depth. ## Examples @@ -71,5 +93,8 @@ Follow the 5-phase review pipeline detailed in `references/review-phases.md`: ### Reference Files -For detailed pipeline procedures, consult: -- **`references/review-phases.md`** - Complete 5-phase review process with output format and scoring guidelines +- **`references/review-phases.md`** - Orchestrator + 4-agent pipeline +- **`references/agents/detection.md`** - Detection patterns and output format +- **`references/agents/impact.md`** - Blast radius algorithm +- **`references/agents/triage.md`** - Tier classification and signal ratio +- **`references/agents/remediation.md`** - Fix generation and regression checks diff --git a/skills/review/references/agents/detection.md b/skills/review/references/agents/detection.md new file mode 100644 index 0000000..e03e5cd --- /dev/null +++ b/skills/review/references/agents/detection.md @@ -0,0 +1,293 @@ +# Detection Agent + +Identifies security vulnerabilities, runtime issues, breaking changes, and logic flaws. + +## Input +- Changed files/diff content +- File paths being reviewed + +## Output +```typescript +interface DetectionFinding { + id: string; // unique: `${file}:${line}:${pattern}` + type: 'security' | 'runtime' | 'breaking' | 'logic'; + severity: 'critical' | 'high' | 'medium' | 'low'; + confidence: number; // 0-100 + file: string; + line: number; + evidence: string; + description: string; + pattern: string; // which pattern matched +} +``` + +--- + +## Detection Patterns + +### SECURITY Patterns + +#### SQL Injection +``` +Patterns: +- String interpolation in query: `query(\`...${var}...\`)` +- String concat in query: query("..." + var + "...") +- .raw() with user input +- Template literals in SQL without parameterization + +Severity: critical +Confidence: 90% if user input traceable, 70% otherwise +``` + +#### Command Injection +``` +Patterns: +- exec(), execSync() with string interpolation +- spawn() with shell: true and user input +- child_process with unsanitized args +- eval() with external data + +Severity: critical +Confidence: 95% if direct user input, 75% if indirect +``` + +#### Path Traversal +``` +Patterns: +- fs operations with user-controlled path +- path.join() without validation +- Missing path.normalize() before fs access +- No check for ".." in path segments + +Severity: high +Confidence: 85% +``` + +#### Hardcoded Secrets +``` +Patterns: +- API_KEY = "..." (non-placeholder) +- password = "..." (literal string) +- secret = "..." (not from env) +- Bearer/Basic auth with literal token +- Private key material inline + +Severity: high +Confidence: 95% for obvious patterns, 60% for ambiguous +``` + +#### Missing Auth +``` +Patterns: +- Route handler without auth middleware +- API endpoint missing authorization check +- Direct database access without permission check +- Exported function without caller validation + +Severity: high +Confidence: 70% (needs context verification) +``` + +--- + +### RUNTIME Patterns + +#### Uncaught Promises +``` +Patterns: +- async function without try/catch at top level +- Promise without .catch() +- await without surrounding try/catch +- Missing error handler in Promise.all/race + +Severity: high +Confidence: 80% +``` + +#### Null/Undefined Access +``` +Patterns: +- obj.prop without null check (where obj might be undefined) +- array[index] without bounds check +- Optional chain missing: obj.nested.deep (should be obj?.nested?.deep) +- Destructuring without default values + +Severity: medium +Confidence: 75% +``` + +#### Array Bounds +``` +Patterns: +- array[i] in loop without length check +- array.pop()/shift() without empty check +- Direct index access: array[0] without length verification + +Severity: medium +Confidence: 70% +``` + +#### Resource Leaks +``` +Patterns: +- File handle opened without close in finally +- Database connection without release +- Event listener added without cleanup +- setInterval without clear +- Stream not properly ended + +Severity: medium +Confidence: 65% +``` + +--- + +### BREAKING Patterns + +#### Removed Exports +``` +Detection: +1. Use matrix_list_exports on previous version (if available) +2. Compare with current exports +3. Flag any removed public exports + +Severity: high +Confidence: 95% +``` + +#### Changed Signatures +``` +Patterns: +- Function parameter count changed +- Required parameter added +- Return type changed (if typed) +- Parameter type narrowed + +Severity: high +Confidence: 90% for typed code, 70% for untyped +``` + +#### Modified Return Types +``` +Patterns: +- Async function became sync (or vice versa) +- Return type shape changed +- Nullable return became non-nullable (or vice versa) +- Array return became single item + +Severity: high +Confidence: 85% +``` + +--- + +### LOGIC Patterns + +#### Dead Code +``` +Patterns: +- Return statement before code +- Unreachable after throw +- Condition always true/false +- Unused variable after assignment + +Severity: low +Confidence: 90% +``` + +#### Unreachable Branches +``` +Patterns: +- else after return in all if branches +- switch case after default with no break +- catch after catch-all + +Severity: low +Confidence: 85% +``` + +#### Off-by-One +``` +Patterns: +- < vs <= in loop bounds +- array.length vs array.length - 1 +- Fence-post errors in iteration + +Severity: medium +Confidence: 60% (high false positive rate) +``` + +--- + +## Execution Steps + +1. **Read changed files** using Read tool +2. **For each file:** + - Scan line-by-line for pattern matches + - Track variable flow for taint analysis (see below) + - Check function signatures against exports +3. **For breaking changes:** + - Use `matrix_list_exports` to get current exports + - Compare with git diff to identify removed/changed +4. **Aggregate findings** with deduplication +5. **Sort by severity** (critical → low) + +--- + +## Taint Analysis with matrix_find_definition + +For security patterns (injection, traversal), trace variable origins: + +``` +1. Identify suspicious variable usage: + db.query(`SELECT * FROM users WHERE id = ${userId}`) + ^^^^^^^^ +2. Trace origin with matrix_find_definition: + result = matrix_find_definition({ symbol: "userId" }) + +3. Check if origin is user-controlled: + - Function parameter → HIGH risk (direct user input) + - Request body/query/params → HIGH risk + - Environment variable → LOW risk + - Hardcoded constant → NO risk + - Database query result → MEDIUM risk (indirect) + +4. Adjust confidence based on origin: + - User-controlled: confidence = 95% + - Indirect source: confidence = 75% + - Unknown origin: confidence = 60% +``` + +### Example: Command Injection Trace + +```typescript +// File: src/api/files.ts +function listFiles(userPath: string) { // ← parameter = user input + exec(`ls ${userPath}`); // ← flagged +} + +// matrix_find_definition({ symbol: "userPath" }) reveals: +// - Kind: parameter +// - Origin: function argument +// - Confidence boost: 95% (direct user input) +``` + +### Example: Safe Variable + +```typescript +// File: src/utils/config.ts +const BASE_DIR = process.env.BASE_DIR || '/app'; // ← env var + +// matrix_find_definition({ symbol: "BASE_DIR" }) reveals: +// - Kind: const +// - Origin: environment variable +// - Confidence reduction: 40% (likely safe) +``` + +--- + +## Early Exit Signal + +If no findings with severity >= high: +- Signal to orchestrator: reduce Impact agent depth +- Skip transitive blast radius (stick to 1st degree) diff --git a/skills/review/references/agents/impact.md b/skills/review/references/agents/impact.md new file mode 100644 index 0000000..b65bfef --- /dev/null +++ b/skills/review/references/agents/impact.md @@ -0,0 +1,212 @@ +# Impact Agent + +Calculates true blast radius with transitive dependency graph, test coverage, and service boundaries. + +## Input +- Changed files +- Detection findings (for depth decision) + +## Output +```typescript +interface ImpactGraph { + directChanges: FileChange[]; + firstDegree: Caller[]; // Direct importers + secondDegree: Caller[]; // Importers of importers + thirdDegree?: Caller[]; // Optional, only if critical findings + testCoverage: TestCoverage; + serviceBoundaries: ServiceBoundary[]; + riskScore: number; // 1-10 +} + +interface FileChange { + file: string; + exports: string[]; // Changed/added/removed exports + changeType: 'modified' | 'added' | 'deleted'; +} + +interface Caller { + file: string; + symbols: string[]; // Which symbols it imports + degree: 1 | 2 | 3; + isTest: boolean; +} + +interface TestCoverage { + percentage: number; + testFiles: string[]; + uncoveredSymbols: string[]; +} + +interface ServiceBoundary { + type: 'route' | 'api' | 'handler' | 'export'; + file: string; + endpoint?: string; +} +``` + +--- + +## Algorithm + +### Step 1: Extract Changed Symbols +``` +For each changed file: + exports = matrix_list_exports(file) + Track: added, modified, removed exports +``` + +### Step 2: Build Transitive Graph +``` +depth = detection.hasCritical ? 3 : 2 + +For each exported symbol: + callers_1 = matrix_find_callers(symbol) + firstDegree.push(...callers_1) + + if depth >= 2: + For each caller in callers_1: + if not isTestFile(caller): + caller_exports = matrix_list_exports(caller.file) + For each export: + callers_2 = matrix_find_callers(export) + secondDegree.push(...callers_2) + + if depth >= 3: + // Similar recursion for 3rd degree + // Cap at 50 files to avoid explosion +``` + +### Step 3: Identify Test Files +``` +Test file patterns: +- *.test.ts, *.spec.ts +- *.test.js, *.spec.js +- __tests__/*.ts +- test/*.ts + +For each caller: + caller.isTest = matchesTestPattern(caller.file) + +testCoverage.testFiles = callers.filter(c => c.isTest) + +// Calculate coverage as % of SYMBOLS with at least one test caller +coveredSymbols = symbols.filter(s => hasTestCaller(s)) +testCoverage.percentage = coveredSymbols.length / totalSymbols.length * 100 +testCoverage.uncoveredSymbols = symbols.filter(s => !hasTestCaller(s)) +``` + +### Step 4: Detect Service Boundaries +``` +Heuristics: + +Route files: +- routes/*.ts, router/*.ts +- Contains @Get, @Post, @Put, @Delete decorators +- Contains app.get(), router.get(), etc. + +API files: +- api/*.ts, endpoints/*.ts +- handlers/*.ts, controllers/*.ts +- Contains export handler/controller functions + +For each file in blast radius: + if matchesBoundaryPattern(file): + serviceBoundaries.push({ type, file, endpoint }) +``` + +### Step 5: Calculate Risk Score +``` +Base score from degree counts: +- Each direct change: 10 points +- Each 1st degree: 5 points +- Each 2nd degree: 2 points +- Each 3rd degree: 1 point + +Modifiers: +- Uncovered code: +30% of base +- API boundary affected: +50% of base +- Test file only: -20% (less risky) + +Normalize to 1-10 scale: +- 1-3: Low (< 20 points) +- 4-6: Medium (20-50 points) +- 7-9: High (50-100 points) +- 10: Critical (> 100 points) +``` + +--- + +## Matrix Tool Usage + +### matrix_list_exports +``` +Purpose: Get all exported symbols from a file +Input: { path: "src/utils/auth.ts" } +Output: [{ name: "validateToken", kind: "function" }, ...] +``` + +### matrix_find_callers +``` +Purpose: Find all files that import and use a symbol +Input: { symbol: "validateToken", file: "src/utils/auth.ts" } +Output: [{ file: "src/middleware/auth.ts", ... }, ...] +``` + +### matrix_get_imports +``` +Purpose: Get all imports in a file (for understanding dependencies) +Input: { file: "src/api/users.ts" } +Output: [{ source: "../utils/auth", symbols: ["validateToken"] }, ...] +``` + +--- + +## Output Example + +``` +Blast Radius Analysis +===================== + +Direct Changes: 2 files +├── src/utils/auth.ts (modified) +│ └── Exports: validateToken, refreshToken +└── src/utils/session.ts (modified) + └── Exports: createSession, destroySession + +First Degree (8 files): +├── src/middleware/authenticate.ts → validateToken +├── src/api/login.ts → validateToken, createSession +├── src/api/logout.ts → destroySession +├── src/handlers/user.ts → refreshToken +└── ... 4 more + +Second Degree (12 files): +├── src/routes/api.ts → imports authenticate middleware +├── src/app.ts → imports routes +└── ... 10 more + +Test Coverage: 65% +├── Covered: validateToken, createSession +├── Uncovered: refreshToken, destroySession +└── Test files: auth.test.ts, session.test.ts + +Service Boundaries: 3 +├── [ROUTE] src/routes/api.ts → /api/* +├── [HANDLER] src/handlers/user.ts → userHandler +└── [API] src/api/login.ts → POST /login + +Risk Score: 7/10 (High) +- 2 direct + 8 first + 12 second = 48 points +- Uncovered code: +14 points +- API boundaries: +24 points +- Total: 86 points → Score 7 +``` + +--- + +## Performance Considerations + +- **Cap recursion:** Max 3 degrees, max 50 files per degree +- **Early termination:** If > 100 callers at degree 1, likely a util file - note and stop +- **Parallel queries:** Batch matrix_find_callers calls where possible +- **Cache exports:** Don't re-query same file's exports diff --git a/skills/review/references/agents/remediation.md b/skills/review/references/agents/remediation.md new file mode 100644 index 0000000..c984a01 --- /dev/null +++ b/skills/review/references/agents/remediation.md @@ -0,0 +1,314 @@ +# Remediation Agent + +Generates context-aware fixes, assesses regression risk, and suggests tests for uncovered code. + +## Input +- Tier 1 & 2 findings from Triage Agent +- ImpactGraph from Impact Agent + +## Output +```typescript +interface RemediationSuggestion { + findingId: string; + confidence: 'high' | 'medium' | 'low'; + fix: FixSuggestion; + alternatives?: FixSuggestion[]; + regressionRisk: RegressionRisk; + suggestedTests?: TestSuggestion[]; +} + +interface FixSuggestion { + description: string; + codeBefore: string; + codeAfter: string; + explanation: string; +} + +interface RegressionRisk { + level: 'low' | 'medium' | 'high'; + affectedFiles: string[]; + reason: string; +} + +interface TestSuggestion { + file: string; + testCase: string; + description: string; +} +``` + +--- + +## Process + +### Step 1: Find Similar Patterns +``` +For each finding: + 1. matrix_search_symbols for similar function/pattern names + 2. Read matched files to see how codebase handles this + 3. Extract patterns: error handling style, validation approach, etc. +``` + +### Step 2: Check Memory for Past Solutions +``` +For each finding: + 1. matrix_recall({ query: finding.description }) + 2. If solution found with high success rate: + - Use as basis for fix + - Adapt to current context + 3. If no solution: + - Generate fresh based on codebase patterns +``` + +### Step 2.5: Check Library Best Practices (Context7) +``` +If finding involves external library: + 1. Identify library from imports (e.g., "express", "prisma", "zod") + 2. resolve-library-id({ libraryName: "express", query: finding.description }) + 3. query-docs({ libraryId: result.id, query: "security best practices for ${finding.type}" }) + 4. Incorporate library-specific guidance into fix + +Examples: +- SQL injection in Prisma → query-docs for parameterized queries +- Auth issue in Express → query-docs for middleware patterns +- Validation in Zod → query-docs for schema best practices +- File handling in Node → query-docs for path security + +Skip if: +- No external library involved (pure JS/TS logic) +- Library not found in Context7 +- Finding is style/opinion (Tier 3) +``` + +### Step 3: Generate Fix +``` +Fix generation rules: +- Match codebase style (spacing, naming, patterns) +- Minimal change principle: fix only what's broken +- Preserve existing behavior for unaffected paths +- Include before/after code snippets +``` + +### Step 4: Assess Regression Risk +``` +For each fix: + 1. Check blast radius: how many files import this? + 2. Check if fix changes function signature + 3. Check if fix changes return type/behavior + 4. Calculate risk level: + - Low: Internal change, no signature change + - Medium: Behavior change but backwards compatible + - High: Signature change or breaking behavior change +``` + +### Step 5: Suggest Tests +``` +If finding.file not covered by tests: + 1. Identify test file location (conventions: *.test.ts, __tests__/) + 2. Generate test case for the specific issue + 3. Include edge case tests if applicable +``` + +--- + +## Fix Generation Strategies + +### Security Fixes + +#### SQL Injection +```typescript +// Before (vulnerable) +const users = await db.query(`SELECT * FROM users WHERE id = ${userId}`); + +// After (parameterized) +const users = await db.query('SELECT * FROM users WHERE id = $1', [userId]); + +// Or with ORM pattern (if codebase uses ORM) +const users = await User.findById(userId); +``` + +#### Command Injection +```typescript +// Before (vulnerable) +exec(`ls ${userPath}`); + +// After (escaped/validated) +import { escapeShellArg } from './utils/shell'; +exec(`ls ${escapeShellArg(userPath)}`); + +// Or (better - avoid shell) +import { spawn } from 'child_process'; +spawn('ls', [userPath]); +``` + +#### Path Traversal +```typescript +// Before (vulnerable) +const content = fs.readFileSync(path.join(baseDir, userPath)); + +// After (validated) +const resolvedBase = path.resolve(baseDir); +const fullPath = path.resolve(baseDir, userPath); +if (!fullPath.startsWith(resolvedBase + path.sep)) { + throw new Error('Invalid path: directory traversal detected'); +} +const content = fs.readFileSync(fullPath); +``` + +### Runtime Fixes + +#### Uncaught Promise +```typescript +// Before +async function fetchData() { + const data = await api.get('/data'); + return data; +} + +// After +async function fetchData() { + try { + const data = await api.get('/data'); + return data; + } catch (error) { + logger.error('Failed to fetch data', error); + throw error; // or return default/null based on codebase pattern + } +} +``` + +#### Null Access +```typescript +// Before +const name = user.profile.name; + +// After (optional chaining) +const name = user?.profile?.name; + +// Or (with default) +const name = user?.profile?.name ?? 'Unknown'; +``` + +### Breaking Change Fixes + +#### Removed Export - Add Back with Deprecation +```typescript +// If export was removed but still needed: +/** @deprecated Use newFunction instead. Will be removed in v2.0 */ +export const oldFunction = newFunction; +``` + +#### Changed Signature - Overload for Compatibility +```typescript +// If signature changed from (a, b) to (options): +export function myFunc(options: Options): Result; +/** @deprecated Use options object instead */ +export function myFunc(a: string, b: number): Result; +export function myFunc(aOrOptions: string | Options, b?: number): Result { + // Implementation handling both forms +} +``` + +--- + +## Regression Risk Assessment + +### Low Risk Indicators +- Change is internal to function (no signature change) +- Fix adds validation that rejects previously-invalid input +- Change is additive (new error handling, logging) +- Affected callers are all test files + +### Medium Risk Indicators +- Behavior change but API contract preserved +- New error conditions that callers should handle +- Performance characteristics changed +- 1-5 non-test callers affected + +### High Risk Indicators +- Function signature changed (parameters, return type) +- Previously-working input now rejected +- Breaking change to public API +- >5 non-test callers affected +- Service boundary affected + +--- + +## Test Suggestion Examples + +### For Security Issue +```typescript +// Suggested test for SQL injection fix +describe('getUserById', () => { + it('should reject SQL injection attempts', async () => { + const maliciousId = "1'; DROP TABLE users; --"; + await expect(getUserById(maliciousId)).rejects.toThrow(); + }); + + it('should handle valid numeric ids', async () => { + const user = await getUserById('123'); + expect(user.id).toBe(123); + }); +}); +``` + +### For Null Access Fix +```typescript +// Suggested test for null safety +describe('getProfileName', () => { + it('should return name when profile exists', () => { + const user = { profile: { name: 'John' } }; + expect(getProfileName(user)).toBe('John'); + }); + + it('should handle missing profile gracefully', () => { + const user = {}; + expect(getProfileName(user)).toBeUndefined(); + }); + + it('should handle null user', () => { + expect(getProfileName(null)).toBeUndefined(); + }); +}); +``` + +--- + +## Output Confidence Levels + +### High Confidence Fix +- Exact pattern found in codebase +- Past solution with success rate > 80% +- Standard security fix (well-known patterns) +- Regression risk: low + +### Medium Confidence Fix +- Similar pattern found, adapted +- Past solution with success rate 50-80% +- Fix requires some judgment +- Regression risk: medium + +### Low Confidence Fix +- No similar pattern in codebase +- No past solution available +- Multiple valid approaches +- Regression risk: high or unknown +- **Output as guidance, not code** + +--- + +## Learning Integration + +After fix is applied (in future session): +``` +If fix worked: + matrix_reward(solutionId, 'success') + +If fix needed modification: + matrix_reward(solutionId, 'partial', notes) + matrix_store(improvedSolution) + +If fix caused issues: + matrix_reward(solutionId, 'failure') + matrix_failure(errorDetails) +``` diff --git a/skills/review/references/agents/triage.md b/skills/review/references/agents/triage.md new file mode 100644 index 0000000..4ccb77e --- /dev/null +++ b/skills/review/references/agents/triage.md @@ -0,0 +1,266 @@ +# Triage Agent + +Classifies findings into tiers, calibrates confidence, filters noise to achieve >80% signal ratio. + +## Input +- DetectionFinding[] from Detection Agent +- ImpactGraph from Impact Agent + +## Output +```typescript +interface TriagedFinding extends DetectionFinding { + tier: 1 | 2 | 3; + calibratedConfidence: number; + showInOutput: boolean; + suppressionReason?: string; +} + +interface TriageMetrics { + tier1Count: number; + tier2Count: number; + tier3Count: number; + suppressedCount: number; + signalRatio: number; // (tier1 + tier2) / total +} +``` + +--- + +## Tier Definitions + +### Tier 1: Critical (MUST show) +``` +Criteria (any of): +- Security: severity critical OR high +- Runtime: severity critical +- Breaking: any breaking change to public API +- Confidence >= 90% + +Always show these findings in review output. +``` + +### Tier 2: Important (SHOULD show) +``` +Criteria (any of): +- Security: severity medium +- Runtime: severity high OR medium +- Architecture: significant pattern violations +- Performance: clear regression +- Confidence >= 70% + +Show unless signal ratio > 85% already. +``` + +### Tier 3: Noise (SUPPRESS) +``` +Criteria (any of): +- Style/formatting issues +- Opinionated suggestions (e.g., "consider using X") +- Micro-optimizations with no measurable impact +- Confidence < 70% +- Low severity + low confidence combination + +Collapse into expandable section, don't show by default. +``` + +--- + +## Confidence Calibration + +### Base Confidence +Comes from Detection Agent based on pattern certainty. + +### Calibration Modifiers + +```typescript +function calibrateConfidence(finding: DetectionFinding, impact: ImpactGraph): number { + let confidence = finding.confidence; + + // Boost if in API boundary + if (isInServiceBoundary(finding.file, impact.serviceBoundaries)) { + confidence += 15; + } + + // Boost if in core utility (many callers) + const callerCount = countCallers(finding.file, impact); + if (callerCount > 10) { + confidence += 10; + } + + // Boost if similar past issue was confirmed + // (check matrix_recall for validated solutions) + if (hasPastConfirmedIssue(finding.pattern)) { + confidence += 10; + } + + // Reduce if file has false positive history + if (hasHighFalsePositiveRate(finding.file)) { + confidence -= 15; + } + + // Reduce if in test file + if (isTestFile(finding.file)) { + confidence -= 10; + } + + return Math.min(100, Math.max(0, confidence)); +} +``` + +### Modifier Summary + +| Condition | Modifier | +|-----------|----------| +| In API boundary | +15% | +| In core util (>10 callers) | +10% | +| Similar past issue confirmed | +10% | +| File has FP history | -15% | +| In test file | -10% | + +--- + +## Signal Ratio Calculation + +```typescript +function calculateSignalRatio(findings: TriagedFinding[]): number { + const tier1 = findings.filter(f => f.tier === 1).length; + const tier2 = findings.filter(f => f.tier === 2).length; + const total = findings.length; + + if (total === 0) return 100; // No findings = perfect signal + + return ((tier1 + tier2) / total) * 100; +} +``` + +**Target: >80% signal ratio** + +If ratio < 80%, aggressively filter Tier 2 items with lowest confidence. + +--- + +## Tier Assignment Algorithm + +```typescript +function assignTier(finding: DetectionFinding, impact: ImpactGraph): TriagedFinding { + const calibrated = calibrateConfidence(finding, impact); + + // Tier 1 rules + if (finding.type === 'security' && ['critical', 'high'].includes(finding.severity)) { + return { ...finding, tier: 1, calibratedConfidence: calibrated, showInOutput: true }; + } + if (finding.type === 'runtime' && finding.severity === 'critical') { + return { ...finding, tier: 1, calibratedConfidence: calibrated, showInOutput: true }; + } + if (finding.type === 'breaking') { + return { ...finding, tier: 1, calibratedConfidence: calibrated, showInOutput: true }; + } + if (calibrated >= 90) { + return { ...finding, tier: 1, calibratedConfidence: calibrated, showInOutput: true }; + } + + // Tier 3 rules (check before Tier 2) + if (isStyleIssue(finding)) { + return { ...finding, tier: 3, calibratedConfidence: calibrated, showInOutput: false, suppressionReason: 'style' }; + } + if (isOpinion(finding)) { + return { ...finding, tier: 3, calibratedConfidence: calibrated, showInOutput: false, suppressionReason: 'opinion' }; + } + if (calibrated < 70) { + return { ...finding, tier: 3, calibratedConfidence: calibrated, showInOutput: false, suppressionReason: 'low confidence' }; + } + + // Tier 2 (everything else that's not filtered) + return { ...finding, tier: 2, calibratedConfidence: calibrated, showInOutput: true }; +} +``` + +--- + +## Style/Opinion Detection + +### Style Issues +``` +Patterns indicating style: +- "inconsistent spacing" +- "prefer X over Y" (where both are valid) +- "naming convention" +- "formatting" +- "whitespace" +- "indentation" +- "trailing comma" +``` + +### Opinion Issues +``` +Patterns indicating opinion: +- "consider using" +- "you might want to" +- "could be refactored" +- "alternative approach" +- "some prefer" +- "subjective" +``` + +--- + +## Output Format + +``` +Triage Summary +============== + +Findings by Tier: +- Tier 1 (Critical): 3 +- Tier 2 (Important): 7 +- Tier 3 (Suppressed): 4 + +Signal Ratio: 71% (10/14) → Below target, filtering... + +After aggressive filter: +- Tier 1: 3 (unchanged) +- Tier 2: 5 (removed 2 lowest confidence) +- Tier 3: 6 (absorbed filtered Tier 2) + +Final Signal Ratio: 80% (8/10 shown) + +Suppression Details: +- 2 items: Low confidence (< 70%) +- 2 items: Style issues +- 1 item: Opinionated suggestion +- 1 item: Absorbed from Tier 2 filter +``` + +--- + +## Aggressive Filtering (When Ratio < 80%) + +If signal ratio falls below 80%: + +1. Sort Tier 2 findings by calibrated confidence +2. Move lowest confidence Tier 2 items to Tier 3 until ratio >= 80% +3. Add suppressionReason: "below signal threshold" + +```typescript +function enforceSignalRatio(findings: TriagedFinding[], target = 0.8): TriagedFinding[] { + let ratio = calculateSignalRatio(findings); + + if (ratio >= target) return findings; + + // Sort Tier 2 by confidence ascending + const tier2 = findings + .filter(f => f.tier === 2) + .sort((a, b) => a.calibratedConfidence - b.calibratedConfidence); + + for (const finding of tier2) { + finding.tier = 3; + finding.showInOutput = false; + finding.suppressionReason = 'below signal threshold'; + + ratio = calculateSignalRatio(findings); + if (ratio >= target) break; + } + + return findings; +} +``` diff --git a/skills/review/references/review-phases.md b/skills/review/references/review-phases.md index b70cd7c..e83a687 100644 --- a/skills/review/references/review-phases.md +++ b/skills/review/references/review-phases.md @@ -1,177 +1,275 @@ -# Code Review Phases +# Code Review: Orchestrator + 4-Agent Pipeline -Detailed procedures for the 5-phase code review pipeline. +## Architecture Overview -## Phase 1: Context Mapping (Blast Radius) +``` +ORCHESTRATOR (parses target, routes, aggregates) + │ + ├── DETECTION AGENT → security, runtime, breaking, logic flaws + ├── IMPACT AGENT → blast radius, transitive graph, test coverage + ├── TRIAGE AGENT → tier assignment, confidence calibration, noise filter + └── REMEDIATION AGENT → context-aware fixes, regression checks +``` -Calculate the impact scope of changes: +**Execution order:** Sequential (Detection → Impact → Triage → Remediation) +**Early exit:** If Detection finds nothing critical, skip deep Impact/Triage analysis -1. **Identify changed files/functions** - - For file path: Use `Read` tool to examine the file - - For "staged": Use `git diff --cached --name-only` - - For PR: Use `gh pr diff ` +--- -2. **Find all callers using `matrix_find_callers`** - - For each exported function/class in changed files - - Build dependency graph of affected code +## Orchestrator -3. **Calculate impact score** - - Direct changes: Files modified - - First-degree impact: Files that import changed modules - - Second-degree impact: Files that import first-degree files +### 1. Pre-flight Checks -Output format: ``` -Blast Radius Analysis -===================== -Direct changes: 3 files - - src/utils/auth.ts (modified) - - src/utils/session.ts (modified) - - src/api/login.ts (modified) - -First-degree impact: 8 files - - src/middleware/authenticate.ts (imports auth) - - src/handlers/user.ts (imports session) - ... - -Impact Score: 7/10 (medium-high) +BEFORE any analysis, verify tool availability: + +1. Check Index Status: + result = matrix_index_status() + + If result.indexed == false OR result.stale == true: + indexAvailable = false + Warn: "Code index unavailable/stale. Using Grep fallback (slower but complete)." + Else: + indexAvailable = true + +2. Set Tool Strategy: + If indexAvailable: + findCallers = matrix_find_callers + findDefinition = matrix_find_definition + listExports = matrix_list_exports + searchSymbols = matrix_search_symbols + Else: + findCallers = grep_fallback_callers # Grep for import statements + findDefinition = grep_fallback_def # Grep for function/class def + listExports = grep_fallback_exports # Grep for export statements + searchSymbols = grep_fallback_search # Grep with pattern ``` -## Phase 2: Intent Inference +### 2. Parse Target -Analyze what the change is trying to accomplish: +``` +Input: [mode] + +Targets: +- File path: "src/utils/auth.ts" → Read file directly +- "staged": git diff --cached --name-only → list changed files +- "uncommitted": git diff --name-only → list all uncommitted changes +- PR number: gh pr diff → get PR changes + +For PR targets, check merge status first: + gh pr view --json mergeable,mergeStateStatus + If mergeable = "CONFLICTING": + Warn: "PR has merge conflicts. Resolve before review for accurate analysis." + Continue with review but note affected files may have conflict markers. +``` -1. **Gather context** - - Commit messages (if available) - - PR description (if PR number provided) - - Code comments and docstrings +### 3. Dispatch Agents -2. **Classify change type** - - bugfix: Fixing incorrect behavior - - feature: Adding new functionality - - refactor: Restructuring without behavior change - - performance: Optimization - - security: Security improvement - - cleanup: Code quality improvement +**Default mode:** All 4 agents +**Lazy mode:** Detection only (Tier 1 issues) -3. **Summarize intent** - - What problem is being solved? - - What approach is being taken? +### 4. Aggregate Results -## Phase 3: Socratic Questioning +Combine agent outputs into final review format (see Output Format below) -Generate probing questions about the changes: +--- -1. **Edge cases** - - What happens with null/undefined inputs? - - What about empty collections? - - Boundary conditions? +## Agent References -2. **Error handling** - - Are errors properly caught and handled? - - Are error messages helpful? - - Is error state properly cleaned up? +Each agent has detailed documentation: -3. **Testing coverage** - - Are the changes covered by tests? - - Are edge cases tested? - - Should new tests be added? +- **`agents/detection.md`** - Vulnerability patterns, runtime checks, breaking change detection +- **`agents/impact.md`** - Transitive blast radius algorithm, service boundary detection +- **`agents/triage.md`** - Tier definitions, confidence calibration, signal ratio calculation +- **`agents/remediation.md`** - Fix generation, regression risk assessment -4. **Security considerations** - - Input validation? - - Authentication/authorization? - - Data sanitization? +--- -## Phase 4: Targeted Investigation +## Execution Flow -For each concern from Phase 3: +### Step 1: Detection Agent +``` +Input: changed files/diff +Output: DetectionFinding[] -1. **Check existing patterns** - - Use `matrix_find_definition` to see how similar code handles this - - Use `matrix_recall` to find related solutions +For each file: +1. Scan for SECURITY patterns (injection, secrets, auth) +2. Scan for RUNTIME issues (uncaught promises, null access) +3. Check for BREAKING changes (removed exports, changed signatures) +4. Identify LOGIC flaws (dead code, unreachable branches) -2. **Verify assumptions** - - Read related files to understand context - - Check test files for expected behavior +Early exit check: if no critical/high findings, reduce Impact depth +``` -3. **Research if needed** - - For unfamiliar patterns, query external documentation - - Use `matrix_recall` for past issues with similar code +### Step 2: Impact Agent +``` +Input: changed files, Detection findings +Output: ImpactGraph + +1. matrix_list_exports(changed_files) → changed symbols +2. matrix_find_callers(symbols) → direct callers (1st degree) +3. Recurse for 2nd/3rd degree if critical findings exist +4. Identify test files (*.test.ts, *.spec.ts) +5. Detect service boundaries (routes/, api/, handlers/) +6. Calculate risk score +``` -## Phase 5: Reflection & Consolidation +### Step 3: Triage Agent +``` +Input: DetectionFinding[], ImpactGraph +Output: TriagedFinding[] with tier assignments + +For each finding: +1. Assign initial tier based on type/severity +2. Apply confidence calibration +3. Calculate signal ratio +4. Suppress Tier 3 (noise) +``` -Generate final review output in Greptile-style format: +### Step 4: Remediation Agent +``` +Input: Tier 1 & 2 findings, ImpactGraph +Output: RemediationSuggestion[] + +For each finding: +1. matrix_search_symbols for similar patterns +2. matrix_recall for past solutions +3. Generate fix matching codebase style +4. Assess regression risk against blast radius +5. Suggest tests if coverage gap +``` -### Confidence Score (1-5) -- 5/5: No issues, ready to merge -- 4/5: Minor suggestions only, approve with optional changes -- 3/5: Some issues that should be addressed but not blocking -- 2/5: Important issues that need attention before merge -- 1/5: Critical bugs or issues that will cause incorrect behavior +--- -### Output Format +## Output Format ```markdown # Matrix Review -## Summary -[2-3 sentence overview of what this PR/change does and its purpose] +## Risk Assessment +Impact Score: 7/10 | Signal Ratio: 87% (23 shown, 3 suppressed) +Blast Radius: 18 files | API Boundaries: 2 routes | Test Coverage: 75% + +## Tier 1: Critical (MUST address) + +### 1. [SECURITY] SQL Injection +**File:** src/api/users.ts:42 | **Confidence:** 94% + +**Issue:** +User input passed directly to query without sanitization. + +**Blast Radius:** 8 files import this module, 2 API routes affected + +**Suggested Fix:** +```typescript +// Before +const users = await db.query(`SELECT * FROM users WHERE id = ${userId}`); + +// After +const users = await db.query('SELECT * FROM users WHERE id = $1', [userId]); +``` + +**Regression Risk:** Low - parameterized queries are backwards compatible -## Key Changes -- [Change 1]: Brief description -- [Change 2]: Brief description -- [Change 3]: Brief description +--- -## Critical Issues Found +## Tier 2: Important (SHOULD address) -### 1. [Issue Title] -[Detailed explanation of the issue, why it's critical, and what behavior it will cause] +### 3. [PERF] N+1 Query Pattern +**File:** src/services/orders.ts:67 | **Confidence:** 82% -### 2. [Issue Title] -[Detailed explanation] +**Issue:** +Loop executes individual queries instead of batch fetch. -## Additional Issues -- [Minor issue 1] -- [Minor issue 2] +**Suggested Fix:** Use `WHERE id IN (...)` with collected IDs -## Positive Aspects -- [Good practice observed] -- [Well-implemented pattern] +--- -## Confidence Score: [N]/5 +## Suppressed (Tier 3) - 3 items +
+Click to expand low-confidence/style issues -[Explanation of why this score was given, referencing the critical issues] +- [STYLE] Inconsistent spacing in src/utils/format.ts:12 (confidence: 45%) +- [STYLE] Prefer const over let in src/api/auth.ts:89 (confidence: 52%) +- [OPINION] Consider destructuring in src/handlers/user.ts:34 (confidence: 38%) -**Files requiring attention:** [file1.ts] (critical issue #1), [file2.ts] (critical issue #2) +
--- -## Important Files Changed +## File Scores -| Filename | Score | Overview | -|----------|-------|----------| -| path/to/file1.ts | 2/5 | Brief description of issues in this file | -| path/to/file2.ts | 1/5 | Brief description of critical issues | -| path/to/file3.ts | 5/5 | No issues found, clean implementation | +| File | Score | Critical | Important | Coverage | +|------|-------|----------|-----------|----------| +| src/api/users.ts | 2/5 | 1 | 0 | 60% | +| src/services/orders.ts | 3/5 | 0 | 1 | 85% | +| src/utils/auth.ts | 4/5 | 0 | 0 | 90% | -### File Analysis +--- -#### `path/to/file1.ts` — Score: 2/5 -[Detailed analysis of this file's changes, issues found, and suggestions] +## Confidence Score: 3/5 -#### `path/to/file2.ts` — Score: 1/5 -[Detailed analysis of this file's changes, issues found, and suggestions] +Important issues found that should be addressed before merge. +SQL injection vulnerability in users.ts requires immediate fix. ``` -### Scoring Guidelines per File -- 5/5: No issues, clean implementation -- 4/5: Minor style or documentation suggestions -- 3/5: Some improvements recommended -- 2/5: Has bugs or significant issues -- 1/5: Critical bugs that will cause incorrect behavior +--- + +## Scoring Guidelines + +### Overall Confidence Score +- **5/5:** No issues, ready to merge +- **4/5:** Minor suggestions only, approve with optional changes +- **3/5:** Important issues that should be addressed +- **2/5:** Critical issues requiring attention before merge +- **1/5:** Critical bugs/vulnerabilities that will cause harm + +### Per-File Score +- **5/5:** No issues, clean implementation +- **4/5:** Minor style/documentation suggestions +- **3/5:** Some improvements recommended +- **2/5:** Has bugs or significant issues +- **1/5:** Critical bugs that will cause incorrect behavior + +--- + +## Learning Loop + +After review completion: + +- **If solution helped:** Use `matrix_reward` with outcome +- **If novel pattern found:** Use `matrix_store` for future reviews +- **If false positive:** Note for confidence calibration +- **If fix caused issues:** Use `matrix_failure` to record for prevention + +--- + +## Grep Fallback Patterns + +When code index is unavailable, use these Grep patterns: + +### grep_fallback_callers(symbol) +```bash +# Find files importing the symbol +Grep pattern: "import.*{[^}]*\b${symbol}\b[^}]*}.*from|import\s+${symbol}\s+from|require\(.*\).*${symbol}" +``` + +### grep_fallback_def(symbol) +```bash +# Find function/class/type definitions +Grep pattern: "(function|const|let|var|class|interface|type|enum)\s+${symbol}\b|export\s+(default\s+)?(function|class)\s+${symbol}" +``` + +### grep_fallback_exports(file) +```bash +# Find all exports in a file +Grep pattern: "export\s+(default\s+)?(function|class|const|let|var|interface|type|enum)\s+(\w+)|export\s+\{[^}]+\}" +``` + +### grep_fallback_search(query) +```bash +# General symbol search +Grep pattern: "\b${query}\b" +``` -### Learning Loop -- If reviewer spots a pattern that should be remembered: - Use `matrix_store` to save for future reviews -- If a recalled solution helped: - Use `matrix_reward` to improve future recommendations +**Note:** Grep fallback is slower and may miss dynamic imports/re-exports. Prefer indexed queries when available. diff --git a/src/config/index.ts b/src/config/index.ts index 87f824e..fc26ac6 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -112,6 +112,30 @@ export interface GitCommitReviewConfig { autoRun: boolean; } +// ═══════════════════════════════════════════════════════════════ +// Dreamer Config (Scheduled Task Automation) +// ═══════════════════════════════════════════════════════════════ +export interface DreamerWorktreeConfig { + /** Default branch prefix for worktree branches */ + defaultBranchPrefix: string; + /** Default remote for pushing worktree branches */ + defaultRemote: string; + /** Optional custom base path for worktrees */ + defaultBasePath?: string; +} + +export interface DreamerExecutionConfig { + /** Default timeout in seconds */ + defaultTimeout: number; + /** Default value for skip permissions flag */ + defaultSkipPermissions: boolean; +} + +export interface DreamerConfig { + worktree: DreamerWorktreeConfig; + execution: DreamerExecutionConfig; +} + // ═══════════════════════════════════════════════════════════════ // User-Configurable Rules (v2.0) // ═══════════════════════════════════════════════════════════════ @@ -214,6 +238,8 @@ export interface MatrixConfig { /** Model for delegable tools: 'haiku' (cheaper) or 'sonnet' (more capable) */ model: 'haiku' | 'sonnet'; }; + /** Dreamer scheduled task automation settings */ + dreamer: DreamerConfig; } function getDownloadsDirectory(): string { @@ -395,6 +421,17 @@ export const DEFAULT_CONFIG: MatrixConfig = { enabled: true, model: 'haiku' as const, // Use haiku for cheaper read-only operations }, + dreamer: { + worktree: { + defaultBranchPrefix: 'matrix-dreamer/', + defaultRemote: 'origin', + defaultBasePath: undefined, + }, + execution: { + defaultTimeout: 300, // 5 minutes + defaultSkipPermissions: false, + }, + }, }; let cachedConfig: MatrixConfig | null = null; diff --git a/src/db/migrate.ts b/src/db/migrate.ts index 103c3d5..5aed679 100644 --- a/src/db/migrate.ts +++ b/src/db/migrate.ts @@ -4,7 +4,7 @@ import { homedir } from 'os'; import { SCHEMA_SQL } from './schema.js'; // Schema version - increment when schema changes -const SCHEMA_VERSION = 6; +export const SCHEMA_VERSION = 6; // Migration definitions - each migration upgrades from (version - 1) to version const migrations: Record = { @@ -100,7 +100,7 @@ const migrations: Record = { skip_permissions INTEGER DEFAULT 0, worktree_enabled INTEGER DEFAULT 0, worktree_base_path TEXT, - worktree_branch_prefix TEXT DEFAULT 'claude-task/', + worktree_branch_prefix TEXT DEFAULT 'matrix-dreamer/', worktree_remote TEXT DEFAULT 'origin', tags JSON DEFAULT '[]', repo_id TEXT REFERENCES repos(id), diff --git a/src/db/schema.test.ts b/src/db/schema.test.ts new file mode 100644 index 0000000..68420cf --- /dev/null +++ b/src/db/schema.test.ts @@ -0,0 +1,297 @@ +// src/db/schema.test.ts +// Verifies schema.ts and migrations stay in sync +import { test, expect, beforeAll, afterAll, describe } from 'bun:test'; +import { Database } from 'bun:sqlite'; +import { SCHEMA_SQL } from './schema.js'; +import { runMigrations, SCHEMA_VERSION } from './migrate.js'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { unlinkSync, existsSync } from 'fs'; + +interface ColumnInfo { + cid: number; + name: string; + type: string; + notnull: number; + dflt_value: string | null; + pk: number; +} + +interface TableRow { + name: string; +} + +interface IndexRow { + name: string; +} + +// Helper: Get table info (columns, types, defaults) +function getTableInfo(db: Database, table: string): ColumnInfo[] { + return db.query(`PRAGMA table_info(${table})`).all() as ColumnInfo[]; +} + +// Helper: Get all tables +function getAllTables(db: Database): string[] { + return (db.query(` + SELECT name FROM sqlite_master + WHERE type='table' AND name NOT LIKE 'sqlite_%' + ORDER BY name + `).all() as TableRow[]).map(r => r.name); +} + +// Helper: Get all indexes +function getAllIndexes(db: Database): string[] { + return (db.query(` + SELECT name FROM sqlite_master + WHERE type='index' AND name NOT LIKE 'sqlite_%' + ORDER BY name + `).all() as IndexRow[]).map(r => r.name); +} + +let freshDb: Database; +let migratedDb: Database; +let freshDbPath: string; +let migratedDbPath: string; +let originalMatrixDb: string | undefined; + +beforeAll(() => { + // Save original env + originalMatrixDb = process.env['MATRIX_DB']; + + // Create fresh DB from SCHEMA_SQL + freshDbPath = join(tmpdir(), `matrix-fresh-${Date.now()}.db`); + freshDb = new Database(freshDbPath, { create: true }); + freshDb.exec('PRAGMA journal_mode = WAL'); + freshDb.exec('PRAGMA foreign_keys = ON'); + freshDb.exec(SCHEMA_SQL); + // Create schema_version table like migrations do + freshDb.exec(` + CREATE TABLE IF NOT EXISTS schema_version ( + version INTEGER PRIMARY KEY, + applied_at TEXT DEFAULT (datetime('now')) + ) + `); + freshDb.exec(`INSERT INTO schema_version (version) VALUES (${SCHEMA_VERSION})`); + + // Create migrated DB by running all migrations + migratedDbPath = join(tmpdir(), `matrix-migrated-${Date.now()}.db`); + process.env['MATRIX_DB'] = migratedDbPath; + runMigrations(); + migratedDb = new Database(migratedDbPath, { readonly: true }); +}); + +afterAll(() => { + freshDb.close(); + migratedDb.close(); + if (existsSync(freshDbPath)) unlinkSync(freshDbPath); + if (existsSync(migratedDbPath)) unlinkSync(migratedDbPath); + // Restore original env + if (originalMatrixDb !== undefined) { + process.env['MATRIX_DB'] = originalMatrixDb; + } else { + delete process.env['MATRIX_DB']; + } +}); + +// Read migration SQL directly from migrate.ts for testing upgrade paths +// This simulates what happens when upgrading from v4 to v5 +function getMigrationV5SQL(): string { + return ` + CREATE TABLE IF NOT EXISTS dreamer_tasks ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + description TEXT, + enabled INTEGER DEFAULT 1, + cron_expression TEXT NOT NULL, + timezone TEXT DEFAULT 'local', + command TEXT NOT NULL, + working_directory TEXT DEFAULT '.', + timeout INTEGER DEFAULT 300, + env JSON DEFAULT '{}', + skip_permissions INTEGER DEFAULT 0, + worktree_enabled INTEGER DEFAULT 0, + worktree_base_path TEXT, + worktree_branch_prefix TEXT DEFAULT 'matrix-dreamer/', + worktree_remote TEXT DEFAULT 'origin', + tags JSON DEFAULT '[]', + repo_id TEXT, + created_at TEXT DEFAULT (datetime('now')), + updated_at TEXT DEFAULT (datetime('now')) + ); + + CREATE TABLE IF NOT EXISTS dreamer_executions ( + id TEXT PRIMARY KEY, + task_id TEXT NOT NULL, + started_at TEXT NOT NULL, + completed_at TEXT, + status TEXT NOT NULL CHECK(status IN ('running','success','failure','timeout','skipped')), + triggered_by TEXT NOT NULL, + duration INTEGER, + exit_code INTEGER, + output_preview TEXT, + error TEXT, + task_name TEXT NOT NULL, + project_path TEXT, + cron_expression TEXT, + worktree_path TEXT, + worktree_branch TEXT, + worktree_pushed INTEGER + ); + `; +} + +describe('Schema and Migration Integrity', () => { + test('fresh and migrated DBs have same tables', () => { + const freshTables = getAllTables(freshDb); + const migratedTables = getAllTables(migratedDb); + expect(freshTables).toEqual(migratedTables); + }); + + test('schema version matches SCHEMA_VERSION constant', () => { + const freshVersion = freshDb.query('SELECT MAX(version) as version FROM schema_version').get() as { version: number }; + const migratedVersion = migratedDb.query('SELECT MAX(version) as version FROM schema_version').get() as { version: number }; + expect(freshVersion.version).toBe(SCHEMA_VERSION); + expect(migratedVersion.version).toBe(SCHEMA_VERSION); + }); + + test('all indexes match', () => { + const freshIdx = getAllIndexes(freshDb); + const migratedIdx = getAllIndexes(migratedDb); + expect(freshIdx).toEqual(migratedIdx); + }); + + describe('Table Column Matching', () => { + // Core tables + test('repos columns match', () => { + const freshCols = getTableInfo(freshDb, 'repos'); + const migratedCols = getTableInfo(migratedDb, 'repos'); + expect(freshCols).toEqual(migratedCols); + }); + + test('solutions columns match', () => { + const freshCols = getTableInfo(freshDb, 'solutions'); + const migratedCols = getTableInfo(migratedDb, 'solutions'); + expect(freshCols).toEqual(migratedCols); + }); + + test('failures columns match', () => { + const freshCols = getTableInfo(freshDb, 'failures'); + const migratedCols = getTableInfo(migratedDb, 'failures'); + expect(freshCols).toEqual(migratedCols); + }); + + test('usage_log columns match', () => { + const freshCols = getTableInfo(freshDb, 'usage_log'); + const migratedCols = getTableInfo(migratedDb, 'usage_log'); + expect(freshCols).toEqual(migratedCols); + }); + + // Hooks integration tables + test('warnings columns match', () => { + const freshCols = getTableInfo(freshDb, 'warnings'); + const migratedCols = getTableInfo(migratedDb, 'warnings'); + expect(freshCols).toEqual(migratedCols); + }); + + test('dependency_installs columns match', () => { + const freshCols = getTableInfo(freshDb, 'dependency_installs'); + const migratedCols = getTableInfo(migratedDb, 'dependency_installs'); + expect(freshCols).toEqual(migratedCols); + }); + + test('session_summaries columns match', () => { + const freshCols = getTableInfo(freshDb, 'session_summaries'); + const migratedCols = getTableInfo(migratedDb, 'session_summaries'); + expect(freshCols).toEqual(migratedCols); + }); + + test('api_cache columns match', () => { + const freshCols = getTableInfo(freshDb, 'api_cache'); + const migratedCols = getTableInfo(migratedDb, 'api_cache'); + expect(freshCols).toEqual(migratedCols); + }); + + // Dreamer tables + test('dreamer_tasks columns match', () => { + const freshCols = getTableInfo(freshDb, 'dreamer_tasks'); + const migratedCols = getTableInfo(migratedDb, 'dreamer_tasks'); + expect(freshCols).toEqual(migratedCols); + }); + + test('dreamer_executions columns match', () => { + const freshCols = getTableInfo(freshDb, 'dreamer_executions'); + const migratedCols = getTableInfo(migratedDb, 'dreamer_executions'); + expect(freshCols).toEqual(migratedCols); + }); + + test('plugin_meta columns match', () => { + const freshCols = getTableInfo(freshDb, 'plugin_meta'); + const migratedCols = getTableInfo(migratedDb, 'plugin_meta'); + expect(freshCols).toEqual(migratedCols); + }); + }); + + describe('Default Value Matching', () => { + test('dreamer_tasks defaults match', () => { + const fresh = getTableInfo(freshDb, 'dreamer_tasks'); + const migrated = getTableInfo(migratedDb, 'dreamer_tasks'); + + for (const col of fresh) { + const migratedCol = migrated.find(c => c.name === col.name); + expect(migratedCol).toBeDefined(); + expect(migratedCol?.dflt_value).toBe(col.dflt_value); + } + }); + + test('solutions defaults match', () => { + const fresh = getTableInfo(freshDb, 'solutions'); + const migrated = getTableInfo(migratedDb, 'solutions'); + + for (const col of fresh) { + const migratedCol = migrated.find(c => c.name === col.name); + expect(migratedCol).toBeDefined(); + expect(migratedCol?.dflt_value).toBe(col.dflt_value); + } + }); + + test('warnings defaults match', () => { + const fresh = getTableInfo(freshDb, 'warnings'); + const migrated = getTableInfo(migratedDb, 'warnings'); + + for (const col of fresh) { + const migratedCol = migrated.find(c => c.name === col.name); + expect(migratedCol).toBeDefined(); + expect(migratedCol?.dflt_value).toBe(col.dflt_value); + } + }); + }); +}); + +describe('Migration Path Divergence Detection', () => { + test('v5 migration dreamer_tasks defaults match schema.ts', () => { + // Create DB using v5 migration SQL (simulates upgrade from v4) + const upgradedDbPath = join(tmpdir(), `matrix-upgraded-${Date.now()}.db`); + const upgradedDb = new Database(upgradedDbPath, { create: true }); + upgradedDb.exec(getMigrationV5SQL()); + + // Get column info from both + const freshCols = getTableInfo(freshDb, 'dreamer_tasks'); + const upgradedCols = getTableInfo(upgradedDb, 'dreamer_tasks'); + + // Check each column's default value + const mismatches: string[] = []; + for (const freshCol of freshCols) { + const upgradedCol = upgradedCols.find(c => c.name === freshCol.name); + if (upgradedCol && upgradedCol.dflt_value !== freshCol.dflt_value) { + mismatches.push( + `Column '${freshCol.name}': schema.ts='${freshCol.dflt_value}' vs migrate.ts='${upgradedCol.dflt_value}'` + ); + } + } + + upgradedDb.close(); + if (existsSync(upgradedDbPath)) unlinkSync(upgradedDbPath); + + expect(mismatches).toEqual([]); + }); +}); diff --git a/src/db/schema.ts b/src/db/schema.ts index a234a9a..7cb1cbc 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -276,7 +276,7 @@ CREATE TABLE IF NOT EXISTS dreamer_tasks ( skip_permissions INTEGER DEFAULT 0, worktree_enabled INTEGER DEFAULT 0, worktree_base_path TEXT, - worktree_branch_prefix TEXT DEFAULT 'claude-task/', + worktree_branch_prefix TEXT DEFAULT 'matrix-dreamer/', worktree_remote TEXT DEFAULT 'origin', tags JSON DEFAULT '[]', repo_id TEXT REFERENCES repos(id), diff --git a/src/dreamer/actions/add.ts b/src/dreamer/actions/add.ts index f0f7772..17ddd65 100644 --- a/src/dreamer/actions/add.ts +++ b/src/dreamer/actions/add.ts @@ -9,6 +9,7 @@ import type { DreamerTask } from '../types.js'; import { createTask, deleteTask } from '../store.js'; import { getScheduler, isPlatformSupported } from '../scheduler/index.js'; import { parseSchedule, getNextRuns, cronToHuman } from '../cron/index.js'; +import { getConfig } from '../../config/index.js'; export interface AddResult { success: boolean; @@ -50,6 +51,10 @@ export async function handleAdd(input: DreamerInput): Promise { // Generate task ID const taskId = crypto.randomUUID(); + // Get config defaults + const config = getConfig(); + const dreamerConfig = config.dreamer; + // Build worktree config const worktreeEnabled = input.worktree?.enabled ?? false; @@ -63,13 +68,13 @@ export async function handleAdd(input: DreamerInput): Promise { timezone: input.timezone ?? 'local', command: input.command, workingDirectory: input.workingDirectory ?? process.cwd(), - timeout: input.timeout ?? 300, + timeout: input.timeout ?? dreamerConfig.execution.defaultTimeout, env: input.env ?? {}, - skipPermissions: input.skipPermissions ?? false, + skipPermissions: input.skipPermissions ?? dreamerConfig.execution.defaultSkipPermissions, worktreeEnabled, - worktreeBasePath: input.worktree?.basePath, - worktreeBranchPrefix: input.worktree?.branchPrefix ?? 'claude-task/', - worktreeRemote: input.worktree?.remoteName ?? 'origin', + worktreeBasePath: input.worktree?.basePath ?? dreamerConfig.worktree.defaultBasePath, + worktreeBranchPrefix: input.worktree?.branchPrefix ?? dreamerConfig.worktree.defaultBranchPrefix, + worktreeRemote: input.worktree?.remoteName ?? dreamerConfig.worktree.defaultRemote, tags: input.tags ?? [], repoId: undefined, }; diff --git a/src/dreamer/scheduler/base.ts b/src/dreamer/scheduler/base.ts index f0554ce..934dee8 100644 --- a/src/dreamer/scheduler/base.ts +++ b/src/dreamer/scheduler/base.ts @@ -7,6 +7,7 @@ import type { DreamerTask, SchedulerStatus } from '../types.js'; import { shellEscape, sanitizeForComment } from './shell.js'; +import { getConfig } from '../../config/index.js'; /** * Abstract base class for platform-specific schedulers @@ -105,11 +106,15 @@ export abstract class BaseScheduler { } // Escape all user-provided values for safe shell embedding + const config = getConfig(); + const defaultPrefix = config.dreamer.worktree.defaultBranchPrefix; + const defaultRemote = config.dreamer.worktree.defaultRemote; + const mainRepoEsc = shellEscape(this.getWorkingDirectory(task)); const taskIdEsc = shellEscape(task.id); const taskNameEsc = shellEscape(task.name); - const branchPrefixEsc = shellEscape(task.worktreeBranchPrefix || 'claude-task/'); - const remoteNameEsc = shellEscape(task.worktreeRemote || 'origin'); + const branchPrefixEsc = shellEscape(task.worktreeBranchPrefix || defaultPrefix); + const remoteNameEsc = shellEscape(task.worktreeRemote || defaultRemote); const logDirEsc = shellEscape(logDir); // Sanitize task name for comment (strip anything that could break comment) diff --git a/src/tools/doctor.ts b/src/tools/doctor.ts index a8f33eb..65d345c 100644 --- a/src/tools/doctor.ts +++ b/src/tools/doctor.ts @@ -192,6 +192,9 @@ function findMissingConfigSections(config: ReturnType): string if (!config.indexing) missing.push('indexing'); if (!config.toolSearch) missing.push('toolSearch'); if (!config.delegation) missing.push('delegation'); + if (!config.dreamer) missing.push('dreamer'); + if (!config.dreamer?.worktree) missing.push('dreamer.worktree'); + if (!config.dreamer?.execution) missing.push('dreamer.execution'); return missing; }