diff --git a/src/lib/preflight-helpers.ts b/src/lib/preflight-helpers.ts new file mode 100644 index 0000000..8242634 --- /dev/null +++ b/src/lib/preflight-helpers.ts @@ -0,0 +1,102 @@ +/** + * Extracted helpers from preflight-check tool for testability. + */ +import { existsSync, statSync } from "fs"; +import { resolve } from "path"; +import { PROJECT_DIR } from "./files.js"; + +// Common false positives that look like file paths but aren't +const FALSE_POSITIVE_PATHS = new Set([ + "e.g.", "i.e.", "etc.", "vs.", "v1", "v2", "v3", "v4", "v5", + "node.js", "next.js", "vue.js", "react.js", "express.js", "bun.js", +]); + +// Version-like patterns: v1.2.3, 3.2.0, etc. +const VERSION_PATTERN = /^v?\d+\.\d+(\.\d+)?$/; + +// Must contain a slash OR end with a real file extension +const REAL_FILE_EXTENSIONS = new Set([ + "ts", "tsx", "js", "jsx", "mjs", "cjs", + "json", "yaml", "yml", "toml", "md", "mdx", + "css", "scss", "less", "html", "vue", "svelte", + "py", "rb", "rs", "go", "java", "c", "cpp", "h", + "sh", "bash", "zsh", "fish", + "sql", "graphql", "gql", "prisma", + "env", "lock", "log", "txt", "csv", + "png", "jpg", "jpeg", "gif", "svg", "ico", "webp", + "wasm", "map", "d", +]); + +/** Extract file paths from prompt text, filtering out false positives */ +export function extractFilePaths(prompt: string): string[] { + const matches = prompt.match(/[\w\-./\\]+\.\w{1,6}/g) || []; + return [...new Set(matches)].filter((m) => { + // Remove trailing punctuation that got captured + const clean = m.replace(/[.,;:!?)]+$/, ""); + if (!clean.includes(".")) return false; + + // Filter known false positives + const lower = clean.toLowerCase(); + if (FALSE_POSITIVE_PATHS.has(lower)) return false; + + // Filter version strings + if (VERSION_PATTERN.test(clean)) return false; + + // Must have a slash (path separator) or a recognized extension + if (clean.includes("/") || clean.includes("\\")) return true; + const ext = clean.split(".").pop()?.toLowerCase() ?? ""; + return REAL_FILE_EXTENSIONS.has(ext); + }); +} + +/** Verify files exist relative to PROJECT_DIR and return status lines */ +export function verifyFiles(paths: string[]): string[] { + const lines: string[] = []; + for (const p of paths) { + const abs = resolve(PROJECT_DIR, p); + if (!abs.startsWith(resolve(PROJECT_DIR))) continue; // path traversal guard + if (existsSync(abs)) { + const s = statSync(abs); + lines.push(`✅ \`${p}\` — ${s.size} bytes, modified ${s.mtime.toISOString().slice(0, 16)}`); + } else { + lines.push(`❌ \`${p}\` — not found`); + } + } + return lines; +} + +/** Detect ambiguity signals in a prompt */ +export function detectAmbiguity(prompt: string): string[] { + const issues: string[] = []; + if (/\b(it|them|the thing|that|those|this|these)\b/i.test(prompt)) + issues.push("Contains vague pronouns — clarify what 'it'/'them' refers to"); + if (/\b(fix|update|change|refactor|improve)\b/i.test(prompt) && !extractFilePaths(prompt).length) + issues.push("Vague verb without specific file targets"); + if (prompt.trim().length < 40) + issues.push("Very short prompt — likely missing context"); + return issues; +} + +/** Estimate scope complexity from file references */ +export function estimateComplexity(filePaths: string[]): "SMALL" | "MEDIUM" | "LARGE" { + const hasMultipleFiles = filePaths.length > 3; + const hasMultipleDirs = new Set(filePaths.map((f) => f.split("/")[0])).size > 2; + return hasMultipleFiles && hasMultipleDirs ? "LARGE" : filePaths.length > 1 ? "MEDIUM" : "SMALL"; +} + +/** Split prompt into sub-tasks for sequencing */ +export function splitSubtasks(prompt: string): { task: string; risk: string }[] { + const parts = prompt + .split(/\b(?:then|after that|next|finally)\b|(?:,\s*and\s+)|(?:\band\b(?=\s+(?:update|add|remove|create|fix|change|refactor|implement|deploy)))/i) + .map((s) => s.trim()) + .filter((s) => s.length > 5); + + return (parts.length > 1 ? parts : [prompt.slice(0, 100)]).map((part) => { + const risk = /schema|migrat|database|config|env|deploy/i.test(part) + ? "🔴 HIGH" + : /api|route|endpoint/i.test(part) + ? "🟡 MEDIUM" + : "🟢 LOW"; + return { task: part.charAt(0).toUpperCase() + part.slice(1), risk }; + }); +} diff --git a/src/tools/preflight-check.ts b/src/tools/preflight-check.ts index 8c9121a..97ead63 100644 --- a/src/tools/preflight-check.ts +++ b/src/tools/preflight-check.ts @@ -2,8 +2,7 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { triagePrompt, type TriageLevel, type TriageResult } from "../lib/triage.js"; -import { existsSync, statSync } from "fs"; -import { resolve } from "path"; +import { existsSync } from "fs"; import { PROJECT_DIR } from "../lib/files.js"; import { run, getBranch, getStatus, getRecentCommits, getDiffFiles, getStagedFiles } from "../lib/git.js"; import { now } from "../lib/state.js"; @@ -13,32 +12,12 @@ import { searchSemantic } from "../lib/timeline-db.js"; import { basename, join } from "path"; import { loadPatterns, matchPatterns, formatPatternMatches } from "../lib/patterns.js"; +import { extractFilePaths, verifyFiles, detectAmbiguity, estimateComplexity, splitSubtasks } from "../lib/preflight-helpers.js"; + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- -/** Extract file paths from prompt text */ -function extractFilePaths(prompt: string): string[] { - const matches = prompt.match(/[\w\-./\\]+\.\w{1,6}/g) || []; - return [...new Set(matches)]; -} - -/** Verify files exist and return stats */ -function verifyFiles(paths: string[]): string[] { - const lines: string[] = []; - for (const p of paths) { - const abs = resolve(PROJECT_DIR, p); - if (!abs.startsWith(resolve(PROJECT_DIR))) continue; // path traversal guard - if (existsSync(abs)) { - const s = statSync(abs); - lines.push(`✅ \`${p}\` — ${s.size} bytes, modified ${s.mtime.toISOString().slice(0, 16)}`); - } else { - lines.push(`❌ \`${p}\` — not found`); - } - } - return lines; -} - /** Get related project paths from config + env */ function getRelatedProjects(): { alias: string; path: string }[] { // From config @@ -104,10 +83,7 @@ function buildClarifySection(prompt: string): string[] { } // Ambiguity signals - const issues: string[] = []; - if (/\b(it|them|the thing|that|those|this|these)\b/i.test(prompt)) issues.push("Contains vague pronouns — clarify what 'it'/'them' refers to"); - if (/\b(fix|update|change|refactor|improve)\b/i.test(prompt) && !extractFilePaths(prompt).length) issues.push("Vague verb without specific file targets"); - if (prompt.trim().length < 40) issues.push("Very short prompt — likely missing context"); + const issues = detectAmbiguity(prompt); if (issues.length > 0) { sections.push(`### ⚠️ Clarification Needed\n${issues.map(i => `- ${i}`).join("\n")}`); @@ -126,10 +102,7 @@ function buildScopeSection(prompt: string): string[] { sections.push(`### Referenced Files\n${fileVerification.join("\n")}`); } - // Estimate complexity - const hasMultipleFiles = filePaths.length > 3; - const hasMultipleDirs = new Set(filePaths.map(f => f.split("/")[0])).size > 2; - const complexity = hasMultipleFiles && hasMultipleDirs ? "LARGE" : filePaths.length > 1 ? "MEDIUM" : "SMALL"; + const complexity = estimateComplexity(filePaths); sections.push(`### Scope: ${complexity}`); return sections; @@ -137,28 +110,12 @@ function buildScopeSection(prompt: string): string[] { /** Build sequence section for multi-step */ function buildSequenceSection(prompt: string): string[] { - // Split prompt into sub-tasks - const subtasks: string[] = []; - - // Split on "and", "then", numbered lists, bullet points - const parts = prompt - .split(/\b(?:then|after that|next|finally)\b|(?:,\s*and\s+)|(?:\band\b(?=\s+(?:update|add|remove|create|fix|change|refactor|implement|deploy)))/i) - .map(s => s.trim()) - .filter(s => s.length > 5); - - if (parts.length > 1) { - for (let i = 0; i < parts.length; i++) { - const risk = /schema|migrat|database|config|env|deploy/i.test(parts[i]) ? "🔴 HIGH" : - /api|route|endpoint/i.test(parts[i]) ? "🟡 MEDIUM" : "🟢 LOW"; - subtasks.push(`${i + 1}. ${parts[i].charAt(0).toUpperCase() + parts[i].slice(1)} — Risk: ${risk}`); - } - } else { - subtasks.push(`1. ${prompt.slice(0, 100)} — Risk: 🟡 MEDIUM`); - } + const subtasks = splitSubtasks(prompt); + const lines = subtasks.map((s, i) => `${i + 1}. ${s.task} — Risk: ${s.risk}`); return [ `### Execution Plan`, - ...subtasks, + ...lines, "", "### Checkpoints", "- [ ] Verify after each step before proceeding", diff --git a/tests/lib/preflight-helpers.test.ts b/tests/lib/preflight-helpers.test.ts new file mode 100644 index 0000000..bddac0a --- /dev/null +++ b/tests/lib/preflight-helpers.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect } from "vitest"; +import { extractFilePaths, detectAmbiguity, estimateComplexity, splitSubtasks } from "../../src/lib/preflight-helpers.js"; + +describe("extractFilePaths", () => { + it("extracts real file paths", () => { + expect(extractFilePaths("update src/lib/git.ts and tests/foo.test.ts")).toEqual([ + "src/lib/git.ts", + "tests/foo.test.ts", + ]); + }); + + it("filters out version strings", () => { + expect(extractFilePaths("upgrade to v3.2.0")).toEqual([]); + expect(extractFilePaths("version 1.0.0 is out")).toEqual([]); + }); + + it("filters out common abbreviations", () => { + expect(extractFilePaths("e.g. this should work, i.e. correctly")).toEqual([]); + }); + + it("filters out framework names without path context", () => { + // node.js, next.js etc. are not file paths on their own + expect(extractFilePaths("use node.js for the backend")).toEqual([]); + }); + + it("keeps paths with slashes even without known extensions", () => { + expect(extractFilePaths("check config/app.conf")).toEqual(["config/app.conf"]); + }); + + it("deduplicates", () => { + expect(extractFilePaths("edit foo.ts and then foo.ts again")).toEqual(["foo.ts"]); + }); + + it("handles empty/no-match prompts", () => { + expect(extractFilePaths("just fix the bug")).toEqual([]); + expect(extractFilePaths("")).toEqual([]); + }); +}); + +describe("detectAmbiguity", () => { + it("flags vague pronouns", () => { + const issues = detectAmbiguity("fix it and deploy"); + expect(issues.some((i) => i.includes("vague pronouns"))).toBe(true); + }); + + it("flags vague verbs without file targets", () => { + const issues = detectAmbiguity("refactor the code to be cleaner"); + expect(issues.some((i) => i.includes("Vague verb"))).toBe(true); + }); + + it("does not flag vague verbs when files are referenced", () => { + const issues = detectAmbiguity("refactor src/lib/git.ts to use async"); + expect(issues.some((i) => i.includes("Vague verb"))).toBe(false); + }); + + it("flags short prompts", () => { + const issues = detectAmbiguity("fix bug"); + expect(issues.some((i) => i.includes("Very short"))).toBe(true); + }); + + it("returns empty for clear prompts", () => { + const issues = detectAmbiguity("Add a new endpoint in src/routes/users.ts returning paginated user list with offset and limit params"); + expect(issues).toEqual([]); + }); +}); + +describe("estimateComplexity", () => { + it("returns SMALL for 0-1 files", () => { + expect(estimateComplexity([])).toBe("SMALL"); + expect(estimateComplexity(["src/foo.ts"])).toBe("SMALL"); + }); + + it("returns MEDIUM for 2-3 files", () => { + expect(estimateComplexity(["src/a.ts", "src/b.ts"])).toBe("MEDIUM"); + }); + + it("returns LARGE for many files across dirs", () => { + expect(estimateComplexity(["src/a.ts", "tests/b.ts", "lib/c.ts", "docs/d.md"])).toBe("LARGE"); + }); +}); + +describe("splitSubtasks", () => { + it("splits on 'then'", () => { + const tasks = splitSubtasks("update the schema then run migrations then deploy"); + expect(tasks.length).toBe(3); + }); + + it("assigns risk levels", () => { + const tasks = splitSubtasks("update the database schema then add an API endpoint then fix the button"); + expect(tasks[0].risk).toBe("🔴 HIGH"); // database/schema + expect(tasks[1].risk).toBe("🟡 MEDIUM"); // API/endpoint + expect(tasks[2].risk).toBe("🟢 LOW"); + }); + + it("returns single task for simple prompts", () => { + const tasks = splitSubtasks("add a login page"); + expect(tasks.length).toBe(1); + }); +});