diff --git a/.claude/commands/stackshift.diff.md b/.claude/commands/stackshift.diff.md new file mode 100644 index 0000000..5e33f2f --- /dev/null +++ b/.claude/commands/stackshift.diff.md @@ -0,0 +1,139 @@ +--- +name: stackshift.diff +description: Compare specifications between directories or git commits to visualize what changed. Useful for PR reviews and tracking spec evolution. +--- + +# Spec Diff + +Compare specifications between two locations to see what changed. + +## What This Command Does + +This command compares specifications and shows: + +1. **Added specs** - New specifications that didn't exist before +2. **Removed specs** - Specifications that were deleted +3. **Modified specs** - Specifications with changed content +4. **Section-level changes** - Which sections were added, removed, or modified + +## Usage Patterns + +### Compare with Previous Git Commit + +Compare current specs with the previous commit: + +```bash +# Create temp directory with previous commit's specs +git show HEAD~1:.specify/memory/specifications/ > /tmp/old-specs/ + +# Then compare +/stackshift.diff /tmp/old-specs .specify/memory/specifications +``` + +### Compare Two Branches + +```bash +# Checkout base branch specs to temp +git show main:.specify/memory/specifications/ > /tmp/main-specs/ + +# Compare with current branch +/stackshift.diff /tmp/main-specs .specify/memory/specifications +``` + +### Compare Two Repositories + +```bash +/stackshift.diff ~/git/legacy-app ~/git/new-app +``` + +## Output Format + +```markdown +# Specification Diff Report + +**Generated:** 2024-11-24 10:30:00 +**Base:** /path/to/base +**Compare:** /path/to/compare + +## Summary + +| Status | Count | +|--------|-------| +| βž• Added | 2 | +| βž– Removed | 1 | +| πŸ“ Modified | 3 | +| βœ… Unchanged | 10 | +| **Total** | **16** | + +## Changes + +### βž• user-mfa +**File:** `user-mfa.md` +**Status:** added + +New specification added: Multi-factor Authentication + +### πŸ“ user-authentication +**File:** `user-authentication.md` +**Status:** modified + +**Section Changes:** + +- `+` **Session Management** + - + Session timeout increased to 60 minutes + - + Added refresh token support + +- `~` **Acceptance Criteria** + - + Must support MFA verification + - - Legacy OAuth 1.0 support removed + +### βž– legacy-login +**File:** `legacy-login.md` +**Status:** removed + +Specification removed: Legacy Login System +``` + +## When to Use + +1. **PR Reviews** - See what specs changed in a pull request +2. **Migration Tracking** - Compare legacy and new app specs +3. **Audit Trail** - Track how specs evolved over time +4. **Sync Verification** - Ensure specs are in sync across repos + +## Integration with CI/CD + +The GitHub Actions workflow (`.github/workflows/spec-alignment.yml`) automatically runs spec diff on PRs and posts a comment with the changes. + +--- + +## Execute Diff + +To compare specifications, I need two directories: + +1. **Base directory** - The "before" state (e.g., main branch, legacy app) +2. **Compare directory** - The "after" state (e.g., feature branch, new app) + +**Please provide the two directories to compare:** + +``` +Base: [path to base specs] +Compare: [path to compare specs] +``` + +Or specify a git ref to compare against: + +``` +Compare current specs with: [git ref like HEAD~1, main, v1.0.0] +``` + +**For git comparison, I'll:** +1. Create a temporary directory +2. Extract specs from the git ref +3. Compare with current `.specify/` directory +4. Show the diff report + +**Example:** +``` +Compare current specs with HEAD~1 +``` diff --git a/.claude/commands/stackshift.quality.md b/.claude/commands/stackshift.quality.md new file mode 100644 index 0000000..22d67b9 --- /dev/null +++ b/.claude/commands/stackshift.quality.md @@ -0,0 +1,155 @@ +--- +name: stackshift.quality +description: Analyze specification quality and get scores on completeness, testability, and clarity. Provides actionable feedback for improving specs. +--- + +# Spec Quality Analysis + +Analyze the quality of specifications in this project and provide actionable feedback. + +## What This Command Does + +This command analyzes all specifications in the `.specify/` directory and scores them on: + +1. **Completeness** (35% weight) + - Are all required sections present? (Feature, User Stories, Acceptance Criteria, Technical Requirements) + - Are recommended sections included? (Dependencies, Out of Scope, Implementation Notes) + - Are there unresolved `[NEEDS CLARIFICATION]` or `TODO` markers? + +2. **Testability** (35% weight) + - Are acceptance criteria measurable? (specific numbers, timeouts, status codes) + - Do criteria use definitive language? (must, shall, will) + - Are there vague criteria? (should be good, performant, seamless) + +3. **Clarity** (30% weight) + - Is the language unambiguous? (avoiding: appropriate, reasonable, some, various) + - Are sentences concise? (< 40 words) + - Are examples provided? (code blocks, Given/When/Then) + +## Process + +1. **Find specifications** in `.specify/memory/specifications/` or alternative locations +2. **Analyze each spec** for completeness, testability, and clarity +3. **Calculate scores** (0-100) for each dimension +4. **Identify issues** (errors, warnings, info) +5. **Generate suggestions** for improvement +6. **Output report** with visual score bars + +## Output Format + +``` +# Specification Quality Report + +**Generated:** 2024-11-24 10:30:00 +**Project:** /path/to/project + +## Summary + +Overall Score: β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘ 80/100 +Completeness: β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘ 90/100 +Testability: β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘β–‘ 70/100 +Clarity: β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–‘β–‘ 80/100 + +**Specs Analyzed:** 5 +**Issues:** 2 errors, 5 warnings, 3 info + +## Specifications + +### βœ… user-authentication (92/100) +| Metric | Score | +|--------|-------| +| Completeness | 95/100 | +| Testability | 90/100 | +| Clarity | 90/100 | + +### ⚠️ payment-processing (65/100) +| Metric | Score | +|--------|-------| +| Completeness | 70/100 | +| Testability | 55/100 | +| Clarity | 70/100 | + +**Issues:** +- ⚠️ Found 3 "[NEEDS CLARIFICATION]" markers +- ⚠️ Vague or untestable criteria: "should be fast and responsive..." + +**Suggestions:** +- πŸ’‘ Make 3 criteria more specific with measurable values +- πŸ’‘ Consider adding "Testing Strategy" section +``` + +## Scoring Thresholds + +| Score | Rating | Action | +|-------|--------|--------| +| 80-100 | βœ… Good | Ready for implementation | +| 60-79 | ⚠️ Needs Work | Address warnings before implementing | +| 0-59 | ❌ Poor | Significant revision needed | + +## Example Usage + +**After running StackShift Gear 3 (Create Specs):** +``` +/stackshift.quality +``` + +**Before implementing a feature:** +``` +/stackshift.quality +# Review suggestions +# Fix issues in specs +# Re-run to verify improvement +``` + +## Integration with CI/CD + +Copy `.github/workflows/spec-alignment.yml` to your repository to automatically check spec quality on PRs: + +```yaml +- Runs spec quality analysis +- Posts report as PR comment +- Fails PR if score < 60 or errors > 0 +``` + +--- + +## Execute Analysis + +Now analyzing specification quality in the current directory... + +```bash +# Find specs directory +SPEC_DIR="" +if [ -d ".specify/memory/specifications" ]; then + SPEC_DIR=".specify/memory/specifications" +elif [ -d ".specify/specifications" ]; then + SPEC_DIR=".specify/specifications" +elif [ -d "specs" ]; then + SPEC_DIR="specs" +fi + +if [ -z "$SPEC_DIR" ]; then + echo "❌ No specifications directory found!" + echo "Expected: .specify/memory/specifications/, .specify/specifications/, or specs/" + exit 0 +fi + +echo "πŸ“‚ Found specs in: $SPEC_DIR" +ls -la "$SPEC_DIR"/*.md 2>/dev/null || echo "No .md files found" +``` + +**Analyze each specification file for:** + +1. **Required sections** - Check for `## Feature`, `## User Stories`, `## Acceptance Criteria`, `## Technical Requirements` + +2. **Incomplete markers** - Search for `[NEEDS CLARIFICATION]`, `[TODO]`, `[TBD]`, `[PLACEHOLDER]` + +3. **Testable criteria** - Look for specific numbers, timeouts, status codes in acceptance criteria + +4. **Ambiguous language** - Flag words like: appropriate, reasonable, adequate, sufficient, good, fast, various, some + +5. **Long sentences** - Flag sentences > 40 words + +6. **Code examples** - Bonus for having code blocks or Given/When/Then format + +Generate a comprehensive quality report with scores and actionable suggestions for each specification. diff --git a/.github/workflows/spec-alignment.yml b/.github/workflows/spec-alignment.yml new file mode 100644 index 0000000..0b884a5 --- /dev/null +++ b/.github/workflows/spec-alignment.yml @@ -0,0 +1,201 @@ +# StackShift Specification Alignment Check +# +# This workflow validates that specifications stay aligned with implementation. +# Run on PRs to catch spec drift before merging. +# +# Usage: +# 1. Copy this file to your repository's .github/workflows/ directory +# 2. Customize the configuration below +# 3. PRs will automatically be checked for spec alignment + +name: Spec Alignment + +on: + pull_request: + branches: [main, master, develop] + paths: + - 'src/**' + - 'lib/**' + - 'app/**' + - '.specify/**' + - 'specs/**' + + # Allow manual trigger + workflow_dispatch: + inputs: + fail_on_drift: + description: 'Fail if spec drift detected' + required: false + default: 'true' + +jobs: + spec-quality: + name: Spec Quality Check + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install StackShift MCP + run: npm install -g stackshift-mcp + + - name: Check spec quality + id: quality + run: | + # Run spec quality analysis + stackshift-mcp spec-quality --directory . --format json > quality-report.json + + # Extract overall score + SCORE=$(jq '.overallScore' quality-report.json) + echo "score=$SCORE" >> $GITHUB_OUTPUT + + # Check for errors + ERRORS=$(jq '.issueSummary.errors' quality-report.json) + echo "errors=$ERRORS" >> $GITHUB_OUTPUT + + # Generate markdown summary + stackshift-mcp spec-quality --directory . --format markdown > quality-report.md + + - name: Post quality report + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const report = fs.readFileSync('quality-report.md', 'utf8'); + const score = ${{ steps.quality.outputs.score }}; + + const icon = score >= 80 ? 'βœ…' : score >= 60 ? '⚠️' : '❌'; + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: `## ${icon} Spec Quality Report\n\n**Score: ${score}/100**\n\n
\nView Full Report\n\n${report}\n\n
` + }); + + - name: Check quality threshold + if: steps.quality.outputs.score < 60 || steps.quality.outputs.errors > 0 + run: | + echo "❌ Spec quality below threshold!" + echo "Score: ${{ steps.quality.outputs.score }}/100 (minimum: 60)" + echo "Errors: ${{ steps.quality.outputs.errors }}" + exit 1 + + spec-drift: + name: Spec Drift Detection + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + + steps: + - name: Checkout PR branch + uses: actions/checkout@v4 + with: + ref: ${{ github.head_ref }} + path: pr-branch + + - name: Checkout base branch + uses: actions/checkout@v4 + with: + ref: ${{ github.base_ref }} + path: base-branch + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install StackShift MCP + run: npm install -g stackshift-mcp + + - name: Compare specs + id: diff + run: | + # Run spec diff + stackshift-mcp spec-diff \ + --base-directory base-branch \ + --compare-directory pr-branch \ + --format json > diff-report.json + + # Extract counts + ADDED=$(jq '.added' diff-report.json) + REMOVED=$(jq '.removed' diff-report.json) + MODIFIED=$(jq '.modified' diff-report.json) + + echo "added=$ADDED" >> $GITHUB_OUTPUT + echo "removed=$REMOVED" >> $GITHUB_OUTPUT + echo "modified=$MODIFIED" >> $GITHUB_OUTPUT + + # Generate markdown + stackshift-mcp spec-diff \ + --base-directory base-branch \ + --compare-directory pr-branch \ + --format markdown > diff-report.md + + - name: Post diff report + if: steps.diff.outputs.added > 0 || steps.diff.outputs.removed > 0 || steps.diff.outputs.modified > 0 + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const report = fs.readFileSync('diff-report.md', 'utf8'); + + const added = ${{ steps.diff.outputs.added }}; + const removed = ${{ steps.diff.outputs.removed }}; + const modified = ${{ steps.diff.outputs.modified }}; + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: `## πŸ“‹ Spec Changes Detected\n\n| Change | Count |\n|--------|-------|\n| βž• Added | ${added} |\n| βž– Removed | ${removed} |\n| πŸ“ Modified | ${modified} |\n\n
\nView Full Diff\n\n${report}\n\n
` + }); + + - name: Check for removed specs + if: steps.diff.outputs.removed > 0 && github.event.inputs.fail_on_drift != 'false' + run: | + echo "⚠️ Specifications were removed in this PR!" + echo "Removed: ${{ steps.diff.outputs.removed }}" + echo "Please ensure this is intentional and update related code." + # Uncomment to fail on removed specs: + # exit 1 + + validate-implementation: + name: Validate Spec-Code Alignment + runs-on: ubuntu-latest + needs: [spec-quality] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install dependencies + run: npm ci + + - name: Check for spec markers in code + run: | + # Find any [NEEDS CLARIFICATION] or TODO markers that should be resolved + MARKERS=$(grep -r "\[NEEDS CLARIFICATION\]\|TODO.*spec\|FIXME.*spec" --include="*.ts" --include="*.js" --include="*.md" . || true) + + if [ -n "$MARKERS" ]; then + echo "⚠️ Found unresolved spec markers:" + echo "$MARKERS" + echo "" + echo "Consider resolving these before merging." + fi + + - name: Summary + run: | + echo "## βœ… Spec Alignment Check Complete" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "All specification checks passed!" >> $GITHUB_STEP_SUMMARY diff --git a/mcp-server/src/analyzers/feature-analyzer.ts b/mcp-server/src/analyzers/feature-analyzer.ts index d0d9767..c3d65bb 100644 --- a/mcp-server/src/analyzers/feature-analyzer.ts +++ b/mcp-server/src/analyzers/feature-analyzer.ts @@ -12,6 +12,8 @@ import { FileSearcher } from './utils/file-searcher.js'; import { ASTParser } from './utils/ast-parser.js'; import { ConfidenceScorer } from './utils/confidence-scorer.js'; import { GapDetectionError } from '../types/errors.js'; +import { readFileSafe } from '../utils/file-utils.js'; +import { createLogger } from '../utils/logger.js'; import type { FeatureGap, DocumentationClaim, @@ -22,6 +24,8 @@ import type { } from '../types/roadmap.js'; import { createEvidence } from '../types/roadmap.js'; +const logger = createLogger('feature-analyzer'); + const md = new MarkdownIt(); /** @@ -320,21 +324,33 @@ export class FeatureAnalyzer { // docs/ doesn't exist } - // Parse each file + // Parse each file (with size limit to prevent DoS) + const MAX_DOC_SIZE = 5 * 1024 * 1024; // 5MB max per doc file for (const filePath of files) { - const content = await fs.readFile(filePath, 'utf-8'); - const type = this.classifyDocType(path.basename(filePath)); - const claims = this.parseDocumentation(content); - - docFiles.push({ - path: filePath, - type, - content, - claims, - }); + try { + const content = await readFileSafe(filePath, MAX_DOC_SIZE); + const type = this.classifyDocType(path.basename(filePath)); + const claims = this.parseDocumentation(content); + + docFiles.push({ + path: filePath, + type, + content, + claims, + }); + } catch (error) { + // Log but continue with other files + logger.warn(`Could not read documentation file`, { + file: filePath, + error: error instanceof Error ? error.message : String(error), + }); + } } } catch (error) { - this.log(`Warning: Could not read documentation directory ${docsDir}`); + logger.warn(`Could not read documentation directory`, { + directory: docsDir, + error: error instanceof Error ? error.message : String(error), + }); } return docFiles; diff --git a/mcp-server/src/analyzers/gap-analyzer.ts b/mcp-server/src/analyzers/gap-analyzer.ts index 7336fa4..dfc13f5 100644 --- a/mcp-server/src/analyzers/gap-analyzer.ts +++ b/mcp-server/src/analyzers/gap-analyzer.ts @@ -12,6 +12,7 @@ import { FileSearcher } from './utils/file-searcher.js'; import { ASTParser, FunctionSignature } from './utils/ast-parser.js'; import { ConfidenceScorer } from './utils/confidence-scorer.js'; import { GapDetectionError } from '../types/errors.js'; +import { createLogger } from '../utils/logger.js'; import type { SpecGap, ParsedSpec, @@ -26,6 +27,40 @@ import { combineEvidence, } from '../types/roadmap.js'; +// ============================================================================ +// Module-level logger +// ============================================================================ +const logger = createLogger('gap-analyzer'); + +// ============================================================================ +// PRE-COMPILED REGEX PATTERNS (Performance optimization) +// Compiling regex once at module load instead of per-call +// ============================================================================ + +/** Pattern to extract dependency references from text (e.g., "depends on FR001") */ +const DEPENDENCY_PATTERN = /depends on ([A-Z]+\d+)/gi; + +/** Pattern to extract camelCase to kebab-case conversion points */ +const CAMEL_TO_KEBAB_PATTERN = /([a-z])([A-Z])/g; + +/** Pattern to remove non-alphanumeric characters for keyword extraction */ +const NON_ALPHANUM_PATTERN = /[^a-z0-9\s]/g; + +/** Pattern to split on whitespace */ +const WHITESPACE_PATTERN = /\s+/; + +// ============================================================================ +// STATIC DATA (Moved from function scope for performance) +// ============================================================================ + +/** Common words to exclude from keyword extraction (using Set for O(1) lookup) */ +const COMMON_WORDS = new Set([ + 'the', 'a', 'an', 'and', 'or', 'but', 'in', 'on', 'at', 'to', 'for', 'of', + 'with', 'by', 'from', 'as', 'is', 'was', 'are', 'were', 'be', 'been', 'being', + 'have', 'has', 'had', 'do', 'does', 'did', 'will', 'would', 'should', 'could', + 'may', 'might', 'must', 'can', 'system', +]); + /** * Gap Analyzer Configuration */ @@ -326,7 +361,10 @@ export class SpecGapAnalyzer { } } catch (error) { // Directory might not exist or not be readable - this.log(`Warning: Could not read directory ${specsDir}`); + logger.warn('Could not read specs directory', { + directory: specsDir, + error: error instanceof Error ? error.message : String(error), + }); } return specFiles; @@ -550,6 +588,8 @@ export class SpecGapAnalyzer { /** * Generate expected file locations for a requirement + * Uses pre-compiled CAMEL_TO_KEBAB_PATTERN for performance + * * @param requirement - Requirement * @param spec - Spec * @returns Array of expected locations @@ -561,7 +601,8 @@ export class SpecGapAnalyzer { const keywords = this.extractKeywords(requirement.title); for (const keyword of keywords) { - const kebab = keyword.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); + // Use pre-compiled pattern for camelCase to kebab-case conversion + const kebab = keyword.replace(CAMEL_TO_KEBAB_PATTERN, '$1-$2').toLowerCase(); locations.push(`src/${kebab}.ts`); locations.push(`src/${spec.id.toLowerCase()}/${kebab}.ts`); } @@ -679,14 +720,19 @@ export class SpecGapAnalyzer { /** * Extract dependencies from requirement + * Uses pre-compiled DEPENDENCY_PATTERN for performance + * * @param requirement - Requirement * @returns Array of dependency IDs */ private extractDependencies(requirement: Requirement): string[] { const deps: string[] = []; - // Look for "depends on" in description - const depMatch = requirement.description.match(/depends on ([A-Z]+\d+)/gi); + // Reset regex lastIndex for global pattern (required for reuse) + DEPENDENCY_PATTERN.lastIndex = 0; + + // Look for "depends on" in description using pre-compiled pattern + const depMatch = requirement.description.match(DEPENDENCY_PATTERN); if (depMatch) { deps.push(...depMatch.map(m => m.replace(/depends on /i, ''))); } @@ -696,70 +742,32 @@ export class SpecGapAnalyzer { /** * Extract keywords from text + * Uses pre-compiled patterns and Set-based lookup for performance + * * @param text - Text to extract keywords from * @returns Array of keywords */ private extractKeywords(text: string): string[] { - // Remove common words and split into keywords - const commonWords = [ - 'the', - 'a', - 'an', - 'and', - 'or', - 'but', - 'in', - 'on', - 'at', - 'to', - 'for', - 'of', - 'with', - 'by', - 'from', - 'as', - 'is', - 'was', - 'are', - 'were', - 'be', - 'been', - 'being', - 'have', - 'has', - 'had', - 'do', - 'does', - 'did', - 'will', - 'would', - 'should', - 'could', - 'may', - 'might', - 'must', - 'can', - 'system', - 'must', - ]; - + // Use pre-compiled patterns for better performance const words = text .toLowerCase() - .replace(/[^a-z0-9\s]/g, ' ') - .split(/\s+/) - .filter(w => w.length > 3 && !commonWords.includes(w)); + .replace(NON_ALPHANUM_PATTERN, ' ') + .split(WHITESPACE_PATTERN) + .filter(w => w.length > 3 && !COMMON_WORDS.has(w)); // Return unique words, sorted by length (longer = more specific) return [...new Set(words)].sort((a, b) => b.length - a.length); } /** - * Log message if verbose + * Log message if verbose mode is enabled + * Uses structured logger for better observability + * * @param message - Message to log */ private log(message: string): void { if (this.config.verbose) { - console.log(`[SpecGapAnalyzer] ${message}`); + logger.debug(message); } } } diff --git a/mcp-server/src/analyzers/utils/file-searcher.ts b/mcp-server/src/analyzers/utils/file-searcher.ts index b665ad6..9da2a47 100644 --- a/mcp-server/src/analyzers/utils/file-searcher.ts +++ b/mcp-server/src/analyzers/utils/file-searcher.ts @@ -253,16 +253,33 @@ export class FileSearcher { /** * Convert a glob pattern to a RegExp + * Includes ReDoS protection to prevent exponential backtracking + * * @param glob - Glob pattern * @returns Regular expression + * @throws Error if glob pattern is invalid or potentially dangerous */ private globToRegex(glob: string): RegExp { + // ReDoS Protection: Limit pattern length + const MAX_GLOB_LENGTH = 500; + if (glob.length > MAX_GLOB_LENGTH) { + throw new Error(`Glob pattern too long: ${glob.length} chars (max ${MAX_GLOB_LENGTH})`); + } + + // ReDoS Protection: Detect pathological patterns that could cause exponential backtracking + // Examples: a*a*a*a*b, .*.*.*.*x, patterns with many overlapping wildcards + const consecutiveWildcards = glob.match(/\*[^/*]*\*/g) || []; + if (consecutiveWildcards.length > 3) { + throw new Error(`Potentially dangerous glob pattern detected (ReDoS risk): too many wildcards`); + } + + // Escape special regex characters except our glob wildcards (* and ?) let regex = glob - .replace(/\./g, '\\.') - .replace(/\*\*/g, 'Β§DOUBLESTARΒ§') - .replace(/\*/g, '[^/]*') - .replace(/Β§DOUBLESTARΒ§/g, '.*') - .replace(/\?/g, '.'); + .replace(/[.+^${}()|[\]\\]/g, '\\$&') // Escape regex special chars + .replace(/\*\*/g, 'Β§DOUBLESTARΒ§') // Temporarily mark ** + .replace(/\*/g, '[^/]*?') // * matches anything except / (non-greedy) + .replace(/Β§DOUBLESTARΒ§/g, '.*?') // ** matches anything including / (non-greedy) + .replace(/\?/g, '.'); // ? matches single character return new RegExp(`^${regex}$`); } diff --git a/mcp-server/src/index.ts b/mcp-server/src/index.ts index d74dc9f..b38a1f1 100644 --- a/mcp-server/src/index.ts +++ b/mcp-server/src/index.ts @@ -28,6 +28,8 @@ import { generateAllSpecsToolHandler } from './tools/generate-all-specs.js'; import { createConstitutionToolHandler } from './tools/create-constitution.js'; import { createFeatureSpecsToolHandler } from './tools/create-feature-specs.js'; import { createImplPlansToolHandler } from './tools/create-impl-plans.js'; +import { specQualityTool, executeSpecQuality } from './tools/spec-quality.js'; +import { specDiffTool, executeSpecDiff } from './tools/spec-diff.js'; import { getStateResource, getProgressResource, getRouteResource } from './resources/index.js'; const server = new Server( @@ -295,6 +297,50 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { }, }, }, + { + name: 'stackshift_spec_quality', + description: + 'Analyze specification quality and get scores on completeness, testability, and clarity. Provides actionable feedback for improving specs.', + inputSchema: { + type: 'object', + properties: { + directory: { + type: 'string', + description: 'Project directory containing .specify/ folder', + }, + format: { + type: 'string', + enum: ['json', 'markdown'], + description: 'Output format (default: markdown)', + }, + }, + required: ['directory'], + }, + }, + { + name: 'stackshift_spec_diff', + description: + 'Compare specifications between two directories to visualize what changed. Useful for PR reviews and tracking spec evolution.', + inputSchema: { + type: 'object', + properties: { + base_directory: { + type: 'string', + description: 'Base directory for comparison', + }, + compare_directory: { + type: 'string', + description: 'Directory to compare against base', + }, + format: { + type: 'string', + enum: ['json', 'markdown'], + description: 'Output format (default: markdown)', + }, + }, + required: ['base_directory', 'compare_directory'], + }, + }, ], }; }); @@ -371,6 +417,12 @@ server.setRequestHandler(CallToolRequestSchema, async request => { case 'stackshift_create_impl_plans': return await createImplPlansToolHandler(args || {}); + case 'stackshift_spec_quality': + return await executeSpecQuality(args as { directory: string; format?: 'json' | 'markdown' }); + + case 'stackshift_spec_diff': + return await executeSpecDiff(args as { base_directory: string; compare_directory: string; format?: 'json' | 'markdown' }); + default: throw new Error(`Unknown tool: ${name}`); } diff --git a/mcp-server/src/tools/analyze.ts b/mcp-server/src/tools/analyze.ts index 5dfbe92..d3d4526 100644 --- a/mcp-server/src/tools/analyze.ts +++ b/mcp-server/src/tools/analyze.ts @@ -15,6 +15,10 @@ import * as path from 'path'; import { createDefaultValidator, validateRoute } from '../utils/security.js'; import { StateManager } from '../utils/state-manager.js'; import { countFiles, fileExists, readJsonSafe } from '../utils/file-utils.js'; +import { createToolErrorHandler } from '../utils/error-handler.js'; + +// Create error handler for this tool +const errorHandler = createToolErrorHandler('analyze'); interface AnalyzeArgs { directory?: string; @@ -116,7 +120,8 @@ Access state via: \`stackshift://state\` resource ], }; } catch (error) { - throw new Error(`Analysis failed: ${error instanceof Error ? error.message : String(error)}`); + // Use error handler for consistent error wrapping and logging + throw errorHandler.wrap('execute', error, { directory: args.directory }); } } @@ -166,7 +171,11 @@ async function detectTechStack(directory: string) { result.buildSystem = 'Cargo'; } } catch (error) { - // Return defaults if detection fails + // Log but return defaults - tech stack detection is not critical + errorHandler.logWarning('detectTechStack', 'Tech stack detection failed, using defaults', { + directory, + error: error instanceof Error ? error.message : String(error), + }); } return result; @@ -205,6 +214,12 @@ async function assessCompleteness(directory: string) { (result.backend + result.frontend + result.tests + result.documentation) / 4 ); } catch (error) { + // Log but return defaults - completeness assessment is not critical + errorHandler.logWarning('assessCompleteness', 'Completeness assessment failed, using defaults', { + directory, + error: error instanceof Error ? error.message : String(error), + }); + // Return defaults result.overall = 50; result.backend = 50; diff --git a/mcp-server/src/tools/spec-diff.ts b/mcp-server/src/tools/spec-diff.ts new file mode 100644 index 0000000..c8035c5 --- /dev/null +++ b/mcp-server/src/tools/spec-diff.ts @@ -0,0 +1,478 @@ +/** + * Spec Diff Tool + * + * Visualizes differences between specifications: + * - Compare specs between git commits + * - Compare specs between directories + * - Generate human-readable diff reports + */ + +import * as fs from 'fs/promises'; +import * as path from 'path'; +import { createLogger } from '../utils/logger.js'; +import { readFileSafe } from '../utils/file-utils.js'; +import { createDefaultValidator } from '../utils/security.js'; + +const logger = createLogger('spec-diff'); + +/** + * Change type for a specification + */ +export type ChangeType = 'added' | 'removed' | 'modified' | 'unchanged'; + +/** + * Section change within a specification + */ +export interface SectionChange { + section: string; + changeType: ChangeType; + oldContent?: string; + newContent?: string; + addedLines: string[]; + removedLines: string[]; +} + +/** + * Diff result for a single specification + */ +export interface SpecDiff { + file: string; + featureName: string; + changeType: ChangeType; + sections: SectionChange[]; + summary: string; +} + +/** + * Overall diff report + */ +export interface DiffReport { + timestamp: string; + baseRef: string; + compareRef: string; + totalSpecs: number; + added: number; + removed: number; + modified: number; + unchanged: number; + diffs: SpecDiff[]; +} + +/** + * Compare specifications between two directories + */ +export async function compareSpecs( + baseDir: string, + compareDir: string +): Promise { + const validator = createDefaultValidator(); + const validBase = validator.validateDirectory(baseDir); + const validCompare = validator.validateDirectory(compareDir); + + logger.info('Comparing specifications', { baseDir: validBase, compareDir: validCompare }); + + // Find spec directories + const baseSpecDir = await findSpecDir(validBase); + const compareSpecDir = await findSpecDir(validCompare); + + // Get spec files from both directories + const baseFiles = await getSpecFiles(baseSpecDir); + const compareFiles = await getSpecFiles(compareSpecDir); + + // Create sets for comparison + const baseSet = new Set(baseFiles.map(f => path.basename(f))); + const compareSet = new Set(compareFiles.map(f => path.basename(f))); + + const diffs: SpecDiff[] = []; + let added = 0, removed = 0, modified = 0, unchanged = 0; + + // Find added files (in compare but not in base) + for (const file of compareFiles) { + const basename = path.basename(file); + if (!baseSet.has(basename)) { + const content = await readFileSafe(file, 2 * 1024 * 1024); + const featureName = extractFeatureName(content, basename); + + diffs.push({ + file: basename, + featureName, + changeType: 'added', + sections: [], + summary: `New specification added: ${featureName}`, + }); + added++; + } + } + + // Find removed files (in base but not in compare) + for (const file of baseFiles) { + const basename = path.basename(file); + if (!compareSet.has(basename)) { + const content = await readFileSafe(file, 2 * 1024 * 1024); + const featureName = extractFeatureName(content, basename); + + diffs.push({ + file: basename, + featureName, + changeType: 'removed', + sections: [], + summary: `Specification removed: ${featureName}`, + }); + removed++; + } + } + + // Compare files that exist in both + for (const file of baseFiles) { + const basename = path.basename(file); + if (compareSet.has(basename)) { + const compareFile = compareFiles.find(f => path.basename(f) === basename)!; + + const baseContent = await readFileSafe(file, 2 * 1024 * 1024); + const compareContent = await readFileSafe(compareFile, 2 * 1024 * 1024); + + if (baseContent === compareContent) { + unchanged++; + continue; + } + + const diff = await compareSpecContent(basename, baseContent, compareContent); + diffs.push(diff); + modified++; + } + } + + const totalSpecs = baseSet.size + (compareSet.size - baseSet.size); + + logger.info('Comparison complete', { added, removed, modified, unchanged }); + + return { + timestamp: new Date().toISOString(), + baseRef: validBase, + compareRef: validCompare, + totalSpecs, + added, + removed, + modified, + unchanged, + diffs, + }; +} + +/** + * Find the specifications directory + */ +async function findSpecDir(baseDir: string): Promise { + const candidates = [ + path.join(baseDir, '.specify', 'memory', 'specifications'), + path.join(baseDir, '.specify', 'specifications'), + path.join(baseDir, 'specs'), + path.join(baseDir, 'specifications'), + ]; + + for (const candidate of candidates) { + try { + const stat = await fs.stat(candidate); + if (stat.isDirectory()) { + return candidate; + } + } catch { + continue; + } + } + + return path.join(baseDir, '.specify', 'memory', 'specifications'); +} + +/** + * Get all spec files in a directory + */ +async function getSpecFiles(specDir: string): Promise { + try { + const files = await fs.readdir(specDir); + return files + .filter(f => f.endsWith('.md')) + .map(f => path.join(specDir, f)); + } catch { + return []; + } +} + +/** + * Extract feature name from spec content + */ +function extractFeatureName(content: string, filename: string): string { + const match = content.match(/^#\s+(?:Feature:\s*)?(.+)$/m); + return match ? match[1].trim() : path.basename(filename, '.md'); +} + +/** + * Compare content of two specifications + */ +async function compareSpecContent( + filename: string, + baseContent: string, + compareContent: string +): Promise { + const featureName = extractFeatureName(compareContent, filename); + const sections: SectionChange[] = []; + const changes: string[] = []; + + // Parse sections from both contents + const baseSections = parseSections(baseContent); + const compareSections = parseSections(compareContent); + + // Find all unique section names + const allSections = new Set([...baseSections.keys(), ...compareSections.keys()]); + + for (const sectionName of allSections) { + const baseSection = baseSections.get(sectionName); + const compareSection = compareSections.get(sectionName); + + if (!baseSection && compareSection) { + // Section added + sections.push({ + section: sectionName, + changeType: 'added', + newContent: compareSection, + addedLines: compareSection.split('\n'), + removedLines: [], + }); + changes.push(`+ Added section: ${sectionName}`); + } else if (baseSection && !compareSection) { + // Section removed + sections.push({ + section: sectionName, + changeType: 'removed', + oldContent: baseSection, + addedLines: [], + removedLines: baseSection.split('\n'), + }); + changes.push(`- Removed section: ${sectionName}`); + } else if (baseSection && compareSection && baseSection !== compareSection) { + // Section modified + const { addedLines, removedLines } = diffLines(baseSection, compareSection); + sections.push({ + section: sectionName, + changeType: 'modified', + oldContent: baseSection, + newContent: compareSection, + addedLines, + removedLines, + }); + + if (addedLines.length > 0 || removedLines.length > 0) { + changes.push(`~ Modified section: ${sectionName} (+${addedLines.length}/-${removedLines.length})`); + } + } + } + + return { + file: filename, + featureName, + changeType: 'modified', + sections, + summary: changes.length > 0 ? changes.join(', ') : 'Minor changes', + }; +} + +/** + * Parse sections from markdown content + */ +function parseSections(content: string): Map { + const sections = new Map(); + const lines = content.split('\n'); + + let currentSection = 'Header'; + let currentContent: string[] = []; + + for (const line of lines) { + const headerMatch = line.match(/^(#{1,3})\s+(.+)$/); + + if (headerMatch) { + // Save previous section + if (currentContent.length > 0) { + sections.set(currentSection, currentContent.join('\n').trim()); + } + + currentSection = headerMatch[2].trim(); + currentContent = []; + } else { + currentContent.push(line); + } + } + + // Save last section + if (currentContent.length > 0) { + sections.set(currentSection, currentContent.join('\n').trim()); + } + + return sections; +} + +/** + * Simple line-by-line diff + */ +function diffLines( + oldContent: string, + newContent: string +): { addedLines: string[]; removedLines: string[] } { + const oldLines = new Set(oldContent.split('\n').map(l => l.trim()).filter(l => l)); + const newLines = new Set(newContent.split('\n').map(l => l.trim()).filter(l => l)); + + const addedLines: string[] = []; + const removedLines: string[] = []; + + for (const line of newLines) { + if (!oldLines.has(line)) { + addedLines.push(line); + } + } + + for (const line of oldLines) { + if (!newLines.has(line)) { + removedLines.push(line); + } + } + + return { addedLines, removedLines }; +} + +/** + * Format diff report as markdown + */ +export function formatDiffReport(report: DiffReport): string { + const lines: string[] = []; + + lines.push('# Specification Diff Report'); + lines.push(''); + lines.push(`**Generated:** ${new Date(report.timestamp).toLocaleString()}`); + lines.push(`**Base:** ${report.baseRef}`); + lines.push(`**Compare:** ${report.compareRef}`); + lines.push(''); + + // Summary + lines.push('## Summary'); + lines.push(''); + lines.push(`| Status | Count |`); + lines.push(`|--------|-------|`); + lines.push(`| βž• Added | ${report.added} |`); + lines.push(`| βž– Removed | ${report.removed} |`); + lines.push(`| πŸ“ Modified | ${report.modified} |`); + lines.push(`| βœ… Unchanged | ${report.unchanged} |`); + lines.push(`| **Total** | **${report.totalSpecs}** |`); + lines.push(''); + + if (report.diffs.length === 0) { + lines.push('_No changes detected._'); + return lines.join('\n'); + } + + // Details + lines.push('## Changes'); + lines.push(''); + + for (const diff of report.diffs) { + const icon = diff.changeType === 'added' ? 'βž•' : + diff.changeType === 'removed' ? 'βž–' : 'πŸ“'; + + lines.push(`### ${icon} ${diff.featureName}`); + lines.push(''); + lines.push(`**File:** \`${diff.file}\``); + lines.push(`**Status:** ${diff.changeType}`); + lines.push(''); + + if (diff.sections.length > 0) { + lines.push('**Section Changes:**'); + lines.push(''); + + for (const section of diff.sections) { + const sectionIcon = section.changeType === 'added' ? '+' : + section.changeType === 'removed' ? '-' : '~'; + + lines.push(`- \`${sectionIcon}\` **${section.section}**`); + + if (section.addedLines.length > 0 && section.addedLines.length <= 5) { + for (const line of section.addedLines.slice(0, 3)) { + const truncated = line.length > 60 ? line.substring(0, 60) + '...' : line; + lines.push(` - + ${truncated}`); + } + if (section.addedLines.length > 3) { + lines.push(` - _...and ${section.addedLines.length - 3} more additions_`); + } + } + + if (section.removedLines.length > 0 && section.removedLines.length <= 5) { + for (const line of section.removedLines.slice(0, 3)) { + const truncated = line.length > 60 ? line.substring(0, 60) + '...' : line; + lines.push(` - - ${truncated}`); + } + if (section.removedLines.length > 3) { + lines.push(` - _...and ${section.removedLines.length - 3} more removals_`); + } + } + } + lines.push(''); + } + } + + return lines.join('\n'); +} + +/** + * MCP Tool definition for spec diff + */ +export const specDiffTool = { + name: 'stackshift_spec_diff', + description: 'Compare specifications between two directories or git commits', + inputSchema: { + type: 'object', + properties: { + base_directory: { + type: 'string', + description: 'Base directory or git ref for comparison', + }, + compare_directory: { + type: 'string', + description: 'Directory or git ref to compare against base', + }, + format: { + type: 'string', + enum: ['json', 'markdown'], + description: 'Output format (default: markdown)', + }, + }, + required: ['base_directory', 'compare_directory'], + }, +}; + +/** + * Execute spec diff + */ +export async function executeSpecDiff(args: { + base_directory: string; + compare_directory: string; + format?: 'json' | 'markdown'; +}): Promise<{ content: Array<{ type: 'text'; text: string }> }> { + const report = await compareSpecs(args.base_directory, args.compare_directory); + + if (args.format === 'json') { + return { + content: [ + { + type: 'text', + text: JSON.stringify(report, null, 2), + }, + ], + }; + } + + return { + content: [ + { + type: 'text', + text: formatDiffReport(report), + }, + ], + }; +} diff --git a/mcp-server/src/tools/spec-quality.ts b/mcp-server/src/tools/spec-quality.ts new file mode 100644 index 0000000..fcf2bf1 --- /dev/null +++ b/mcp-server/src/tools/spec-quality.ts @@ -0,0 +1,664 @@ +/** + * Spec Quality Tool + * + * Analyzes specification quality and provides scores on: + * - Completeness: Are all required sections present? + * - Testability: Are acceptance criteria measurable? + * - Clarity: Is the language unambiguous? + * - Coverage: Are all features documented? + */ + +import * as fs from 'fs/promises'; +import * as path from 'path'; +import { createLogger } from '../utils/logger.js'; +import { readFileSafe } from '../utils/file-utils.js'; +import { createDefaultValidator } from '../utils/security.js'; + +const logger = createLogger('spec-quality'); + +/** + * Quality scores for a single specification + */ +export interface SpecQualityScore { + /** Specification file path */ + file: string; + /** Feature name */ + featureName: string; + /** Overall score (0-100) */ + overallScore: number; + /** Completeness score (0-100) - Are all sections present? */ + completeness: number; + /** Testability score (0-100) - Are criteria measurable? */ + testability: number; + /** Clarity score (0-100) - Is language unambiguous? */ + clarity: number; + /** Issues found */ + issues: QualityIssue[]; + /** Suggestions for improvement */ + suggestions: string[]; +} + +/** + * Quality issue found in a specification + */ +export interface QualityIssue { + type: 'missing-section' | 'vague-criteria' | 'ambiguous-language' | 'incomplete' | 'untestable'; + severity: 'error' | 'warning' | 'info'; + message: string; + line?: number; +} + +/** + * Overall quality report for a project + */ +export interface QualityReport { + /** Project directory */ + projectPath: string; + /** Timestamp of analysis */ + timestamp: string; + /** Number of specs analyzed */ + totalSpecs: number; + /** Overall project score (0-100) */ + overallScore: number; + /** Average completeness across all specs */ + averageCompleteness: number; + /** Average testability across all specs */ + averageTestability: number; + /** Average clarity across all specs */ + averageClarity: number; + /** Individual spec scores */ + specs: SpecQualityScore[]; + /** Summary of all issues */ + issueSummary: { + errors: number; + warnings: number; + info: number; + }; +} + +// Required sections in a GitHub Spec Kit specification +const REQUIRED_SECTIONS = [ + 'Feature', + 'User Stories', + 'Acceptance Criteria', + 'Technical Requirements', +]; + +const RECOMMENDED_SECTIONS = [ + 'Dependencies', + 'Out of Scope', + 'Implementation Notes', + 'Testing Strategy', +]; + +// Ambiguous words that reduce clarity +const AMBIGUOUS_WORDS = [ + 'appropriate', + 'reasonable', + 'adequate', + 'sufficient', + 'proper', + 'good', + 'nice', + 'fast', + 'slow', + 'many', + 'few', + 'some', + 'most', + 'quickly', + 'easily', + 'simply', + 'obviously', + 'clearly', + 'basically', + 'etc', + 'and so on', + 'and more', + 'similar', + 'various', + 'certain', + 'specific', + 'generally', + 'usually', + 'often', + 'sometimes', + 'maybe', + 'might', + 'could', + 'possibly', + 'perhaps', +]; + +// Markers that indicate incomplete specs +const INCOMPLETE_MARKERS = [ + '[NEEDS CLARIFICATION]', + '[TODO]', + '[TBD]', + '[PLACEHOLDER]', + '[WIP]', + '[PENDING]', + 'TODO:', + 'FIXME:', + '???', + 'XXX', +]; + +// Testable criteria patterns (good) +const TESTABLE_PATTERNS = [ + /\b\d+(\.\d+)?\s*(ms|milliseconds?|s|seconds?|minutes?|hours?)\b/i, // Time measurements + /\b\d+(\.\d+)?\s*%\b/, // Percentages + /\b(at least|at most|exactly|no more than|no less than)\s+\d+/i, // Quantity bounds + /\b(returns?|responds?|displays?|shows?)\s+\d+/i, // Specific counts + /\b(status code|http)\s*\d{3}\b/i, // HTTP status codes + /\b(within|under|over|below|above)\s+\d+/i, // Thresholds + /\bmust\s+(be|have|return|display|show|contain|include)/i, // Definitive requirements + /\bshall\s+(be|have|return|display|show|contain|include)/i, // Definitive requirements + /\bwill\s+(be|have|return|display|show|contain|include)/i, // Definitive requirements +]; + +// Untestable criteria patterns (bad) +const UNTESTABLE_PATTERNS = [ + /\bshould\s+be\s+(good|nice|fast|easy|user-friendly|intuitive)/i, + /\bperformant\b/i, + /\bscalable\b/i, + /\brobust\b/i, + /\bseamless(ly)?\b/i, + /\bintuitive(ly)?\b/i, + /\buser-friendly\b/i, + /\belegant(ly)?\b/i, + /\bwhen\s+appropriate\b/i, + /\bas\s+needed\b/i, +]; + +/** + * Analyze specification quality + */ +export async function analyzeSpecQuality( + directory: string +): Promise { + const validator = createDefaultValidator(); + const validatedDir = validator.validateDirectory(directory); + + logger.info('Analyzing spec quality', { directory: validatedDir }); + + // Find .specify directory + const specifyDir = path.join(validatedDir, '.specify', 'memory', 'specifications'); + const specs: SpecQualityScore[] = []; + + try { + const files = await fs.readdir(specifyDir); + const mdFiles = files.filter(f => f.endsWith('.md')); + + for (const file of mdFiles) { + const filePath = path.join(specifyDir, file); + const score = await analyzeSpecFile(filePath); + specs.push(score); + } + } catch (error) { + // Try alternative spec locations + const altLocations = [ + path.join(validatedDir, '.specify', 'specifications'), + path.join(validatedDir, 'specs'), + path.join(validatedDir, 'specifications'), + ]; + + for (const altDir of altLocations) { + try { + const files = await fs.readdir(altDir); + const mdFiles = files.filter(f => f.endsWith('.md')); + + for (const file of mdFiles) { + const filePath = path.join(altDir, file); + const score = await analyzeSpecFile(filePath); + specs.push(score); + } + break; // Found specs in this location + } catch { + // Continue to next location + } + } + } + + // Calculate aggregates + const totalSpecs = specs.length; + const overallScore = totalSpecs > 0 + ? Math.round(specs.reduce((sum, s) => sum + s.overallScore, 0) / totalSpecs) + : 0; + const averageCompleteness = totalSpecs > 0 + ? Math.round(specs.reduce((sum, s) => sum + s.completeness, 0) / totalSpecs) + : 0; + const averageTestability = totalSpecs > 0 + ? Math.round(specs.reduce((sum, s) => sum + s.testability, 0) / totalSpecs) + : 0; + const averageClarity = totalSpecs > 0 + ? Math.round(specs.reduce((sum, s) => sum + s.clarity, 0) / totalSpecs) + : 0; + + const issueSummary = { + errors: specs.reduce((sum, s) => sum + s.issues.filter(i => i.severity === 'error').length, 0), + warnings: specs.reduce((sum, s) => sum + s.issues.filter(i => i.severity === 'warning').length, 0), + info: specs.reduce((sum, s) => sum + s.issues.filter(i => i.severity === 'info').length, 0), + }; + + logger.info('Quality analysis complete', { + totalSpecs, + overallScore, + errors: issueSummary.errors, + warnings: issueSummary.warnings, + }); + + return { + projectPath: validatedDir, + timestamp: new Date().toISOString(), + totalSpecs, + overallScore, + averageCompleteness, + averageTestability, + averageClarity, + specs, + issueSummary, + }; +} + +/** + * Analyze a single specification file + */ +async function analyzeSpecFile(filePath: string): Promise { + const content = await readFileSafe(filePath, 2 * 1024 * 1024); // 2MB max + const lines = content.split('\n'); + const issues: QualityIssue[] = []; + const suggestions: string[] = []; + + // Extract feature name from first heading + const featureMatch = content.match(/^#\s+(?:Feature:\s*)?(.+)$/m); + const featureName = featureMatch ? featureMatch[1].trim() : path.basename(filePath, '.md'); + + // 1. Check Completeness + const completenessScore = calculateCompletenessScore(content, issues, suggestions); + + // 2. Check Testability + const testabilityScore = calculateTestabilityScore(content, lines, issues, suggestions); + + // 3. Check Clarity + const clarityScore = calculateClarityScore(content, lines, issues, suggestions); + + // Calculate overall score (weighted average) + const overallScore = Math.round( + completenessScore * 0.35 + + testabilityScore * 0.35 + + clarityScore * 0.30 + ); + + return { + file: filePath, + featureName, + overallScore, + completeness: completenessScore, + testability: testabilityScore, + clarity: clarityScore, + issues, + suggestions, + }; +} + +/** + * Calculate completeness score + */ +function calculateCompletenessScore( + content: string, + issues: QualityIssue[], + suggestions: string[] +): number { + let score = 100; + const contentLower = content.toLowerCase(); + + // Check required sections + for (const section of REQUIRED_SECTIONS) { + const sectionLower = section.toLowerCase(); + const hasSection = contentLower.includes(`## ${sectionLower}`) || + contentLower.includes(`### ${sectionLower}`) || + contentLower.includes(`# ${sectionLower}`); + + if (!hasSection) { + score -= 15; + issues.push({ + type: 'missing-section', + severity: 'error', + message: `Missing required section: "${section}"`, + }); + } + } + + // Check recommended sections (less penalty) + for (const section of RECOMMENDED_SECTIONS) { + const sectionLower = section.toLowerCase(); + const hasSection = contentLower.includes(`## ${sectionLower}`) || + contentLower.includes(`### ${sectionLower}`); + + if (!hasSection) { + score -= 5; + suggestions.push(`Consider adding "${section}" section`); + } + } + + // Check for incomplete markers + for (const marker of INCOMPLETE_MARKERS) { + const count = (content.match(new RegExp(escapeRegex(marker), 'gi')) || []).length; + if (count > 0) { + score -= count * 5; + issues.push({ + type: 'incomplete', + severity: 'warning', + message: `Found ${count} "${marker}" marker(s)`, + }); + } + } + + // Check minimum content length + if (content.length < 500) { + score -= 20; + issues.push({ + type: 'incomplete', + severity: 'warning', + message: 'Specification is very short (< 500 characters)', + }); + suggestions.push('Add more detail to the specification'); + } + + return Math.max(0, Math.min(100, score)); +} + +/** + * Calculate testability score + */ +function calculateTestabilityScore( + content: string, + lines: string[], + issues: QualityIssue[], + suggestions: string[] +): number { + let score = 100; + + // Find acceptance criteria section + const acStartIndex = lines.findIndex(l => + l.toLowerCase().includes('acceptance criteria') || + l.toLowerCase().includes('## criteria') + ); + + if (acStartIndex === -1) { + return 50; // No criteria section found + } + + // Find the end of the acceptance criteria section + let acEndIndex = lines.length; + for (let i = acStartIndex + 1; i < lines.length; i++) { + if (lines[i].match(/^#{1,3}\s+/)) { + acEndIndex = i; + break; + } + } + + const acLines = lines.slice(acStartIndex, acEndIndex); + const criteriaLines = acLines.filter(l => l.match(/^[-*]\s+/) || l.match(/^\d+\.\s+/)); + + if (criteriaLines.length === 0) { + score -= 30; + issues.push({ + type: 'untestable', + severity: 'warning', + message: 'No acceptance criteria bullet points found', + }); + suggestions.push('Add specific acceptance criteria as bullet points'); + return Math.max(0, score); + } + + let testableCount = 0; + let untestableCount = 0; + + for (let i = 0; i < criteriaLines.length; i++) { + const line = criteriaLines[i]; + const lineNumber = acStartIndex + acLines.indexOf(line) + 1; + + // Check for testable patterns + const isTestable = TESTABLE_PATTERNS.some(p => p.test(line)); + const isUntestable = UNTESTABLE_PATTERNS.some(p => p.test(line)); + + if (isTestable) { + testableCount++; + } else if (isUntestable) { + untestableCount++; + issues.push({ + type: 'untestable', + severity: 'warning', + message: `Vague or untestable criteria: "${line.substring(0, 60)}..."`, + line: lineNumber, + }); + } + } + + // Calculate score based on ratio of testable criteria + const totalCriteria = criteriaLines.length; + const testableRatio = totalCriteria > 0 ? testableCount / totalCriteria : 0; + const untestableRatio = totalCriteria > 0 ? untestableCount / totalCriteria : 0; + + score = Math.round(100 * testableRatio - 50 * untestableRatio); + + if (untestableCount > 0) { + suggestions.push( + `Make ${untestableCount} criteria more specific with measurable values` + ); + } + + return Math.max(0, Math.min(100, score)); +} + +/** + * Calculate clarity score + */ +function calculateClarityScore( + content: string, + lines: string[], + issues: QualityIssue[], + suggestions: string[] +): number { + let score = 100; + const contentLower = content.toLowerCase(); + + // Check for ambiguous words + let ambiguousCount = 0; + for (const word of AMBIGUOUS_WORDS) { + const regex = new RegExp(`\\b${escapeRegex(word)}\\b`, 'gi'); + const matches = content.match(regex) || []; + ambiguousCount += matches.length; + } + + if (ambiguousCount > 0) { + const penalty = Math.min(30, ambiguousCount * 2); + score -= penalty; + + if (ambiguousCount > 5) { + issues.push({ + type: 'ambiguous-language', + severity: 'warning', + message: `Found ${ambiguousCount} ambiguous words/phrases`, + }); + suggestions.push('Replace vague terms with specific, measurable language'); + } + } + + // Check for passive voice (simple heuristic) + const passiveMatches = content.match(/\b(is|are|was|were|be|been|being)\s+\w+ed\b/gi) || []; + if (passiveMatches.length > 5) { + score -= 10; + suggestions.push('Consider using active voice for clearer requirements'); + } + + // Check for very long sentences (> 40 words) + const sentences = content.split(/[.!?]+/).filter(s => s.trim().length > 0); + const longSentences = sentences.filter(s => s.split(/\s+/).length > 40); + if (longSentences.length > 0) { + score -= longSentences.length * 3; + issues.push({ + type: 'ambiguous-language', + severity: 'info', + message: `${longSentences.length} sentences are very long (>40 words)`, + }); + suggestions.push('Break long sentences into shorter, clearer statements'); + } + + // Bonus for having examples or code blocks + const codeBlocks = (content.match(/```[\s\S]*?```/g) || []).length; + if (codeBlocks > 0) { + score += Math.min(10, codeBlocks * 3); + } + + // Bonus for having "Given/When/Then" or BDD-style criteria + if (contentLower.includes('given') && contentLower.includes('when') && contentLower.includes('then')) { + score += 10; + } + + return Math.max(0, Math.min(100, score)); +} + +/** + * Escape special regex characters + */ +function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Format quality report as markdown + */ +export function formatQualityReport(report: QualityReport): string { + const lines: string[] = []; + + lines.push('# Specification Quality Report'); + lines.push(''); + lines.push(`**Generated:** ${new Date(report.timestamp).toLocaleString()}`); + lines.push(`**Project:** ${report.projectPath}`); + lines.push(''); + + // Overall summary + lines.push('## Summary'); + lines.push(''); + lines.push('```'); + lines.push(`Overall Score: ${formatScoreBar(report.overallScore)} ${report.overallScore}/100`); + lines.push(`Completeness: ${formatScoreBar(report.averageCompleteness)} ${report.averageCompleteness}/100`); + lines.push(`Testability: ${formatScoreBar(report.averageTestability)} ${report.averageTestability}/100`); + lines.push(`Clarity: ${formatScoreBar(report.averageClarity)} ${report.averageClarity}/100`); + lines.push('```'); + lines.push(''); + lines.push(`**Specs Analyzed:** ${report.totalSpecs}`); + lines.push(`**Issues:** ${report.issueSummary.errors} errors, ${report.issueSummary.warnings} warnings, ${report.issueSummary.info} info`); + lines.push(''); + + // Individual specs + if (report.specs.length > 0) { + lines.push('## Specifications'); + lines.push(''); + + // Sort by score (lowest first to highlight problem specs) + const sortedSpecs = [...report.specs].sort((a, b) => a.overallScore - b.overallScore); + + for (const spec of sortedSpecs) { + const icon = spec.overallScore >= 80 ? 'βœ…' : + spec.overallScore >= 60 ? '⚠️' : '❌'; + + lines.push(`### ${icon} ${spec.featureName}`); + lines.push(''); + lines.push(`**Score:** ${spec.overallScore}/100`); + lines.push(''); + lines.push('| Metric | Score |'); + lines.push('|--------|-------|'); + lines.push(`| Completeness | ${spec.completeness}/100 |`); + lines.push(`| Testability | ${spec.testability}/100 |`); + lines.push(`| Clarity | ${spec.clarity}/100 |`); + lines.push(''); + + if (spec.issues.length > 0) { + lines.push('**Issues:**'); + for (const issue of spec.issues) { + const icon = issue.severity === 'error' ? '❌' : + issue.severity === 'warning' ? '⚠️' : 'ℹ️'; + const lineRef = issue.line ? ` (line ${issue.line})` : ''; + lines.push(`- ${icon} ${issue.message}${lineRef}`); + } + lines.push(''); + } + + if (spec.suggestions.length > 0) { + lines.push('**Suggestions:**'); + for (const suggestion of spec.suggestions) { + lines.push(`- πŸ’‘ ${suggestion}`); + } + lines.push(''); + } + } + } + + return lines.join('\n'); +} + +/** + * Format a score as a visual bar + */ +function formatScoreBar(score: number): string { + const filled = Math.round(score / 10); + const empty = 10 - filled; + return 'β–ˆ'.repeat(filled) + 'β–‘'.repeat(empty); +} + +/** + * MCP Tool definition for spec quality analysis + */ +export const specQualityTool = { + name: 'stackshift_spec_quality', + description: 'Analyze specification quality and get scores on completeness, testability, and clarity', + inputSchema: { + type: 'object', + properties: { + directory: { + type: 'string', + description: 'Project directory containing .specify/ folder', + }, + format: { + type: 'string', + enum: ['json', 'markdown'], + description: 'Output format (default: markdown)', + }, + }, + required: ['directory'], + }, +}; + +/** + * Execute spec quality analysis + */ +export async function executeSpecQuality(args: { + directory: string; + format?: 'json' | 'markdown'; +}): Promise<{ content: Array<{ type: 'text'; text: string }> }> { + const report = await analyzeSpecQuality(args.directory); + + if (args.format === 'json') { + return { + content: [ + { + type: 'text', + text: JSON.stringify(report, null, 2), + }, + ], + }; + } + + return { + content: [ + { + type: 'text', + text: formatQualityReport(report), + }, + ], + }; +} diff --git a/mcp-server/src/utils/__tests__/state-manager.test.ts b/mcp-server/src/utils/__tests__/state-manager.test.ts index 3e30552..8dd3289 100644 --- a/mcp-server/src/utils/__tests__/state-manager.test.ts +++ b/mcp-server/src/utils/__tests__/state-manager.test.ts @@ -61,7 +61,7 @@ describe('StateManager', () => { }); describe('State Validation', () => { - it('should reject state with dangerous properties', async () => { + it('should sanitize dangerous properties instead of rejecting', async () => { // Write malicious state with __proto__ directly in JSON string // (creating it as an object doesn't work because JS handles __proto__ specially) const stateFile = path.join(testDir, '.stackshift-state.json'); @@ -78,8 +78,21 @@ describe('StateManager', () => { }`; await fs.writeFile(stateFile, maliciousJson); - // Should throw when loading - await expect(stateManager.load()).rejects.toThrow(/dangerous properties/); + // SECURITY: Should sanitize dangerous properties BEFORE validation + // This is more secure than rejecting because: + // 1. Dangerous properties are removed immediately after parse + // 2. No window for exploitation during validation + const state = await stateManager.load(); + + // Verify state loaded successfully (dangerous props were sanitized) + expect(state.version).toBe('1.0.0'); + expect(state.path).toBe('greenfield'); + + // Verify dangerous property was removed as an own property + // Note: __proto__ accessor always returns Object.prototype, so we check hasOwnProperty + expect(Object.prototype.hasOwnProperty.call(state, '__proto__')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(state, 'constructor')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(state, 'prototype')).toBe(false); }); it('should reject state with invalid route', async () => { diff --git a/mcp-server/src/utils/error-handler.ts b/mcp-server/src/utils/error-handler.ts new file mode 100644 index 0000000..9da6a79 --- /dev/null +++ b/mcp-server/src/utils/error-handler.ts @@ -0,0 +1,219 @@ +/** + * Error Handling Utilities + * + * Provides consistent error handling patterns across all tools: + * - Standardized error wrapping + * - Safe async execution with context + * - Error type detection + */ + +import { Logger, createLogger } from './logger.js'; + +/** + * StackShift-specific error with operation context + */ +export class StackShiftError extends Error { + public readonly operation: string; + public readonly cause?: Error; + public readonly context?: Record; + + constructor( + operation: string, + message: string, + options?: { cause?: Error; context?: Record } + ) { + super(`${operation} failed: ${message}`); + this.name = 'StackShiftError'; + this.operation = operation; + this.cause = options?.cause; + this.context = options?.context; + + // Maintain proper stack trace + if (Error.captureStackTrace) { + Error.captureStackTrace(this, StackShiftError); + } + } +} + +/** + * Extract error message from unknown error type + */ +export function getErrorMessage(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + if (typeof error === 'string') { + return error; + } + return String(error); +} + +/** + * Wrap an error with operation context + * + * @param operation Name of the operation that failed + * @param error The original error + * @param context Additional context for debugging + * @returns StackShiftError with full context + */ +export function wrapError( + operation: string, + error: unknown, + context?: Record +): StackShiftError { + const message = getErrorMessage(error); + const cause = error instanceof Error ? error : undefined; + return new StackShiftError(operation, message, { cause, context }); +} + +/** + * Execute an async function with error wrapping and optional logging + * + * @param operation Name of the operation + * @param fn The async function to execute + * @param options Execution options + * @returns Result of the function + * @throws StackShiftError if the function throws + */ +export async function safeExecute( + operation: string, + fn: () => Promise, + options?: { + logger?: Logger; + context?: Record; + rethrow?: boolean; + } +): Promise { + const logger = options?.logger ?? createLogger('safeExecute'); + + try { + return await fn(); + } catch (error) { + const wrappedError = wrapError(operation, error, options?.context); + + logger.error(operation, { + error: wrappedError.message, + cause: wrappedError.cause?.message, + ...options?.context, + }); + + if (options?.rethrow !== false) { + throw wrappedError; + } + + // This path is only taken when rethrow is explicitly false + throw wrappedError; + } +} + +/** + * Execute an async function, returning a result object instead of throwing + * + * @param operation Name of the operation + * @param fn The async function to execute + * @returns Success result with data, or error result + */ +export async function tryExecute( + operation: string, + fn: () => Promise, + logger?: Logger +): Promise<{ success: true; data: T } | { success: false; error: StackShiftError }> { + try { + const data = await fn(); + return { success: true, data }; + } catch (error) { + const wrappedError = wrapError(operation, error); + + if (logger) { + logger.error(operation, { error: wrappedError.message }); + } + + return { success: false, error: wrappedError }; + } +} + +/** + * Create an error handler for a specific tool + * + * @param toolName Name of the tool + * @returns Object with bound error handling methods + */ +export function createToolErrorHandler(toolName: string) { + const logger = createLogger(toolName); + + return { + /** + * Wrap an error with tool context + */ + wrap: (operation: string, error: unknown, context?: Record) => + wrapError(`${toolName}.${operation}`, error, context), + + /** + * Execute with error handling + */ + execute: ( + operation: string, + fn: () => Promise, + context?: Record + ) => safeExecute(`${toolName}.${operation}`, fn, { logger, context }), + + /** + * Try to execute, returning result object + */ + try: (operation: string, fn: () => Promise) => + tryExecute(`${toolName}.${operation}`, fn, logger), + + /** + * Log an error without throwing + */ + logError: (operation: string, error: unknown, context?: Record) => { + logger.error(`${toolName}.${operation}`, { + error: getErrorMessage(error), + ...context, + }); + }, + + /** + * Log a warning + */ + logWarning: (operation: string, message: string, context?: Record) => { + logger.warn(`${toolName}.${operation}`, { message, ...context }); + }, + + logger, + }; +} + +/** + * Check if an error is a specific type of filesystem error + */ +export function isFileNotFoundError(error: unknown): boolean { + return ( + error instanceof Error && + 'code' in error && + (error as NodeJS.ErrnoException).code === 'ENOENT' + ); +} + +/** + * Check if an error is a permission error + */ +export function isPermissionError(error: unknown): boolean { + return ( + error instanceof Error && + 'code' in error && + ((error as NodeJS.ErrnoException).code === 'EACCES' || + (error as NodeJS.ErrnoException).code === 'EPERM') + ); +} + +/** + * Check if an error indicates the path is a directory + */ +export function isDirectoryError(error: unknown): boolean { + return ( + error instanceof Error && + 'code' in error && + (error as NodeJS.ErrnoException).code === 'EISDIR' + ); +} diff --git a/mcp-server/src/utils/logger.ts b/mcp-server/src/utils/logger.ts new file mode 100644 index 0000000..e2de389 --- /dev/null +++ b/mcp-server/src/utils/logger.ts @@ -0,0 +1,220 @@ +/** + * Structured Logging Module + * + * Provides consistent logging across all StackShift components: + * - Log levels (debug, info, warn, error) + * - Structured context for each log entry + * - Environment-aware output (JSON in production, pretty in dev) + * - Silent mode for testing + */ + +export type LogLevel = 'debug' | 'info' | 'warn' | 'error'; + +export interface LogEntry { + timestamp: string; + level: LogLevel; + component: string; + message: string; + context?: Record; +} + +export interface Logger { + debug(message: string, context?: Record): void; + info(message: string, context?: Record): void; + warn(message: string, context?: Record): void; + error(message: string, context?: Record): void; + child(component: string): Logger; +} + +/** + * Log level priority (higher = more important) + */ +const LOG_LEVEL_PRIORITY: Record = { + debug: 0, + info: 1, + warn: 2, + error: 3, +}; + +/** + * ANSI color codes for pretty printing + */ +const COLORS = { + reset: '\x1b[0m', + dim: '\x1b[2m', + red: '\x1b[31m', + yellow: '\x1b[33m', + blue: '\x1b[34m', + cyan: '\x1b[36m', + gray: '\x1b[90m', +}; + +/** + * Get the current log level from environment + */ +function getMinLogLevel(): LogLevel { + const envLevel = process.env.STACKSHIFT_LOG_LEVEL?.toLowerCase(); + if (envLevel && envLevel in LOG_LEVEL_PRIORITY) { + return envLevel as LogLevel; + } + // Default to 'info' in production, 'debug' in development + return process.env.NODE_ENV === 'production' ? 'info' : 'debug'; +} + +/** + * Check if we should output JSON (for production/CI) + */ +function useJsonOutput(): boolean { + return ( + process.env.STACKSHIFT_LOG_FORMAT === 'json' || + process.env.NODE_ENV === 'production' || + process.env.CI === 'true' + ); +} + +/** + * Check if logging is disabled (for tests) + */ +function isLoggingDisabled(): boolean { + return ( + process.env.STACKSHIFT_LOG_SILENT === 'true' || + (process.env.NODE_ENV === 'test' && process.env.STACKSHIFT_LOG_LEVEL === undefined) + ); +} + +/** + * Format a log entry as JSON + */ +function formatJson(entry: LogEntry): string { + return JSON.stringify(entry); +} + +/** + * Format a log entry for human-readable output + */ +function formatPretty(entry: LogEntry): string { + const { timestamp, level, component, message, context } = entry; + + // Level colors + const levelColors: Record = { + debug: COLORS.gray, + info: COLORS.blue, + warn: COLORS.yellow, + error: COLORS.red, + }; + + const levelColor = levelColors[level]; + const levelStr = level.toUpperCase().padEnd(5); + + // Format timestamp (just time portion for readability) + const time = timestamp.split('T')[1]?.split('.')[0] || timestamp; + + let output = `${COLORS.dim}${time}${COLORS.reset} ${levelColor}${levelStr}${COLORS.reset} ${COLORS.cyan}[${component}]${COLORS.reset} ${message}`; + + // Add context if present + if (context && Object.keys(context).length > 0) { + const contextStr = Object.entries(context) + .map(([key, value]) => { + const valueStr = typeof value === 'object' ? JSON.stringify(value) : String(value); + return `${COLORS.dim}${key}=${COLORS.reset}${valueStr}`; + }) + .join(' '); + output += ` ${contextStr}`; + } + + return output; +} + +/** + * Create a logger instance for a component + * + * @param component Name of the component (e.g., 'analyze', 'gap-analyzer') + * @returns Logger instance + */ +export function createLogger(component: string): Logger { + const minLevel = getMinLogLevel(); + const useJson = useJsonOutput(); + const silent = isLoggingDisabled(); + + function log(level: LogLevel, message: string, context?: Record): void { + // Skip if silent or below minimum level + if (silent || LOG_LEVEL_PRIORITY[level] < LOG_LEVEL_PRIORITY[minLevel]) { + return; + } + + const entry: LogEntry = { + timestamp: new Date().toISOString(), + level, + component, + message, + context, + }; + + const formatted = useJson ? formatJson(entry) : formatPretty(entry); + + // Use appropriate console method + switch (level) { + case 'error': + console.error(formatted); + break; + case 'warn': + console.warn(formatted); + break; + default: + console.log(formatted); + } + } + + return { + debug: (message, context) => log('debug', message, context), + info: (message, context) => log('info', message, context), + warn: (message, context) => log('warn', message, context), + error: (message, context) => log('error', message, context), + child: (childComponent: string) => createLogger(`${component}:${childComponent}`), + }; +} + +/** + * Create a silent logger (useful for testing) + */ +export function createSilentLogger(): Logger { + const noop = () => {}; + return { + debug: noop, + info: noop, + warn: noop, + error: noop, + child: () => createSilentLogger(), + }; +} + +/** + * Create a logger that collects entries (useful for testing) + */ +export function createCollectingLogger(): Logger & { entries: LogEntry[] } { + const entries: LogEntry[] = []; + + function log(level: LogLevel, message: string, context?: Record): void { + entries.push({ + timestamp: new Date().toISOString(), + level, + component: 'test', + message, + context, + }); + } + + return { + entries, + debug: (message, context) => log('debug', message, context), + info: (message, context) => log('info', message, context), + warn: (message, context) => log('warn', message, context), + error: (message, context) => log('error', message, context), + child: () => createCollectingLogger(), + }; +} + +/** + * Default logger for quick access + */ +export const logger = createLogger('stackshift'); diff --git a/mcp-server/src/utils/state-manager.ts b/mcp-server/src/utils/state-manager.ts index 99a76aa..173289d 100644 --- a/mcp-server/src/utils/state-manager.ts +++ b/mcp-server/src/utils/state-manager.ts @@ -202,6 +202,9 @@ export class StateManager { */ async load(): Promise { try { + // Clean up any orphaned temp files first + await this.cleanupOrphanedTempFiles(); + const data = await fs.readFile(this.stateFile, 'utf-8'); // Limit file size to 10MB to prevent DoS @@ -209,21 +212,17 @@ export class StateManager { throw new Error('State file too large (>10MB)'); } - // Parse JSON first (without sanitizing yet) - const parsed = JSON.parse(data); + // SECURITY: Sanitize IMMEDIATELY after parsing to prevent prototype pollution + // This must happen BEFORE any validation or other operations on the object + const parsed = this.safeJsonParse(data); - // Validate BEFORE sanitizing, so we can detect dangerous properties + // Now validate the sanitized object const validation = this.validateState(parsed); if (!validation.valid) { throw new Error(`Invalid state file structure:\n${validation.errors.join('\n')}`); } - // Now sanitize the validated object - delete parsed.__proto__; - delete parsed.constructor; - delete parsed.prototype; - return parsed as State; } catch (error: any) { if (error.code === 'ENOENT') { @@ -234,6 +233,39 @@ export class StateManager { } } + /** + * Clean up orphaned temporary files from failed writes + * Removes temp files older than 1 hour + */ + private async cleanupOrphanedTempFiles(): Promise { + try { + const dir = path.dirname(this.stateFile); + const basename = path.basename(this.stateFile); + const files = await fs.readdir(dir); + + const tempFiles = files.filter( + f => f.startsWith(basename) && f.endsWith('.tmp') + ); + + const ONE_HOUR = 3600000; + const now = Date.now(); + + for (const temp of tempFiles) { + try { + const tempPath = path.join(dir, temp); + const stats = await fs.stat(tempPath); + if (now - stats.mtimeMs > ONE_HOUR) { + await fs.unlink(tempPath); + } + } catch { + // Ignore errors for individual file cleanup + } + } + } catch { + // Ignore cleanup errors - this is best-effort + } + } + /** * Create initial state * diff --git a/mcp-server/src/utils/validation.ts b/mcp-server/src/utils/validation.ts new file mode 100644 index 0000000..8bb286d --- /dev/null +++ b/mcp-server/src/utils/validation.ts @@ -0,0 +1,300 @@ +/** + * Input Validation Module + * + * Provides centralized validation for all tool inputs: + * - String length limits + * - Safe patterns + * - Type checking + * - Custom validators + */ + +import { createLogger } from './logger.js'; + +const logger = createLogger('validation'); + +/** + * Validation result + */ +export interface ValidationResult { + valid: boolean; + errors: string[]; + sanitized?: unknown; +} + +/** + * Validator function type + */ +export type Validator = (value: T) => string | true; + +/** + * Validation error + */ +export class ValidationError extends Error { + public readonly field: string; + public readonly errors: string[]; + + constructor(field: string, errors: string[]) { + super(`Validation failed for ${field}: ${errors.join(', ')}`); + this.name = 'ValidationError'; + this.field = field; + this.errors = errors; + } +} + +// ============================================================================ +// Common validators +// ============================================================================ + +/** + * Create a max length validator + */ +export function maxLength(max: number): Validator { + return (value: string) => + value.length <= max ? true : `exceeds maximum length of ${max} characters`; +} + +/** + * Create a min length validator + */ +export function minLength(min: number): Validator { + return (value: string) => + value.length >= min ? true : `must be at least ${min} characters`; +} + +/** + * Create a range validator for numbers + */ +export function range(min: number, max: number): Validator { + return (value: number) => + value >= min && value <= max ? true : `must be between ${min} and ${max}`; +} + +/** + * Create a pattern validator + */ +export function pattern(regex: RegExp, description: string): Validator { + return (value: string) => + regex.test(value) ? true : `must match pattern: ${description}`; +} + +/** + * Create an enum validator + */ +export function oneOf(allowed: T[], name: string = 'value'): Validator { + return (value: T) => + allowed.includes(value) ? true : `${name} must be one of: ${allowed.join(', ')}`; +} + +/** + * Create a safe string validator (no dangerous characters) + */ +export function safeString(): Validator { + const dangerousChars = /[<>&'";\x00-\x1f]/; + return (value: string) => + !dangerousChars.test(value) ? true : 'contains potentially dangerous characters'; +} + +/** + * Create a safe glob pattern validator + */ +export function safeGlobPattern(): Validator { + return (value: string) => { + // Check for pathological patterns + const consecutiveWildcards = (value.match(/\*[^/*]*\*/g) || []).length; + if (consecutiveWildcards > 3) { + return 'pattern has too many consecutive wildcards (potential ReDoS)'; + } + if (value.length > 500) { + return 'pattern is too long (max 500 characters)'; + } + return true; + }; +} + +/** + * Create a file path validator + */ +export function safePath(): Validator { + return (value: string) => { + // Check for path traversal + if (value.includes('..')) { + return 'path contains directory traversal (..)'; + } + // Check for null bytes + if (value.includes('\x00')) { + return 'path contains null byte'; + } + return true; + }; +} + +// ============================================================================ +// Validation utilities +// ============================================================================ + +/** + * Validate a value with multiple validators + */ +export function validate( + value: T, + validators: Validator[], + fieldName: string = 'value' +): ValidationResult { + const errors: string[] = []; + + for (const validator of validators) { + const result = validator(value); + if (result !== true) { + errors.push(result); + } + } + + return { + valid: errors.length === 0, + errors, + }; +} + +/** + * Validate and throw if invalid + */ +export function validateOrThrow( + value: T, + validators: Validator[], + fieldName: string = 'value' +): T { + const result = validate(value, validators, fieldName); + + if (!result.valid) { + logger.warn('Validation failed', { field: fieldName, errors: result.errors }); + throw new ValidationError(fieldName, result.errors); + } + + return value; +} + +/** + * Create a schema-based validator for objects + */ +export function createObjectValidator>( + schema: Record[]> +): (obj: T) => ValidationResult { + return (obj: T) => { + const errors: string[] = []; + + for (const [field, validators] of Object.entries(schema)) { + const value = obj[field as keyof T]; + + for (const validator of validators as Validator[]) { + const result = validator(value); + if (result !== true) { + errors.push(`${field}: ${result}`); + } + } + } + + return { + valid: errors.length === 0, + errors, + }; + }; +} + +// ============================================================================ +// Tool-specific validators +// ============================================================================ + +/** + * Validate directory parameter + */ +export function validateDirectory(directory: string | undefined): string { + if (!directory) { + return process.cwd(); + } + + return validateOrThrow(directory, [maxLength(4096), safePath()], 'directory'); +} + +/** + * Validate feature name + */ +export function validateFeatureName(name: string | undefined): string | undefined { + if (!name) return undefined; + + return validateOrThrow(name, [maxLength(200), safeString()], 'featureName'); +} + +/** + * Validate format parameter + */ +export function validateFormat( + format: string | undefined, + allowed: string[] = ['json', 'markdown'] +): string { + if (!format) return allowed[0]; + + return validateOrThrow(format, [oneOf(allowed, 'format')], 'format'); +} + +/** + * Validate confidence threshold + */ +export function validateConfidence(confidence: number | undefined): number { + if (confidence === undefined) return 50; + + return validateOrThrow(confidence, [range(0, 100)], 'confidence'); +} + +/** + * Validate array parameter + */ +export function validateArray( + arr: T[] | undefined, + maxItems: number, + itemValidator?: Validator, + fieldName: string = 'array' +): T[] { + if (!arr) return []; + + if (arr.length > maxItems) { + throw new ValidationError(fieldName, [`exceeds maximum of ${maxItems} items`]); + } + + if (itemValidator) { + const errors: string[] = []; + for (let i = 0; i < arr.length; i++) { + const result = itemValidator(arr[i]); + if (result !== true) { + errors.push(`[${i}]: ${result}`); + } + } + + if (errors.length > 0) { + throw new ValidationError(fieldName, errors); + } + } + + return arr; +} + +/** + * Sanitize string by removing dangerous characters + */ +export function sanitizeString(value: string): string { + return value + .replace(/[<>&'"]/g, '') // Remove HTML-dangerous chars + .replace(/[\x00-\x1f]/g, '') // Remove control characters + .trim(); +} + +/** + * Sanitize HTML entities + */ +export function escapeHtml(value: string): string { + return value + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +}