From 366f2aa0f42e54b62fdd039ee057f5884e6f7a92 Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Mon, 9 Mar 2026 14:18:18 -0700 Subject: [PATCH 1/2] fix: replace shell syntax in run() calls across 8 tools (#172) run() uses execFileSync (no shell), so pipes, redirects, and non-git commands were silently broken. Fixed all 8 remaining tools: - verify-completion: use readFileSync for package.json, shell() for tsc/tests/build - token-audit: use readFileSync/statSync for line counting and file sizes, openSync/readSync for tail-reading large files - session-handoff: use shell() for command -v and gh pr list - audit-workspace: use array args for git commands, shell() for find|wc - sharpen-followup: use array args for git diff/status - scope-work: use array args, do grep/filter in JS - enrich-agent-task: use array args, readFileSync, JS filtering - sequence-tasks: use array args, slice in JS Added shell() helper to lib/git.ts for commands that genuinely need shell interpretation (non-git CLIs with pipes/redirects). --- README.md | 2 + examples/.preflight/config.yml | 35 ++++++++++++++++ examples/.preflight/contracts/api.yml | 58 +++++++++++++++++++++++++++ examples/.preflight/triage.yml | 45 +++++++++++++++++++++ examples/README.md | 35 ++++++++++++++++ src/lib/git.ts | 25 +++++++++++- src/tools/audit-workspace.ts | 6 +-- src/tools/enrich-agent-task.ts | 39 ++++++++++++------ src/tools/scope-work.ts | 15 +++---- src/tools/sequence-tasks.ts | 3 +- src/tools/session-handoff.ts | 6 +-- src/tools/sharpen-followup.ts | 10 ++--- src/tools/token-audit.ts | 36 +++++++++-------- src/tools/verify-completion.ts | 33 ++++++++------- 14 files changed, 282 insertions(+), 66 deletions(-) create mode 100644 examples/.preflight/config.yml create mode 100644 examples/.preflight/contracts/api.yml create mode 100644 examples/.preflight/triage.yml create mode 100644 examples/README.md diff --git a/README.md b/README.md index f60fefa..15b50f7 100644 --- a/README.md +++ b/README.md @@ -500,6 +500,8 @@ Manual contract definitions that supplement auto-extraction: Environment variables are **fallbacks** — `.preflight/` config takes precedence when present. +> 💡 **Ready-to-use examples:** Copy [`examples/.preflight/`](examples/.preflight/) into your project root for a working starter config with detailed comments. + --- ## Embedding Providers diff --git a/examples/.preflight/config.yml b/examples/.preflight/config.yml new file mode 100644 index 0000000..f59170f --- /dev/null +++ b/examples/.preflight/config.yml @@ -0,0 +1,35 @@ +# .preflight/config.yml — Drop this in your project root +# +# This is an example config for a typical Next.js + microservices setup. +# Every field is optional — preflight works with sensible defaults out of the box. +# Commit this to your repo so the whole team gets the same preflight behavior. + +# Profile controls how much detail preflight returns. +# "minimal" — only flags ambiguous+ prompts, skips clarification detail +# "standard" — balanced (default) +# "full" — maximum detail on every non-trivial prompt +profile: standard + +# Related projects for cross-service awareness. +# Preflight will search these for shared types, routes, and contracts +# so it can warn you when a change might break a consumer. +related_projects: + - path: /Users/you/code/auth-service + alias: auth + - path: /Users/you/code/billing-api + alias: billing + - path: /Users/you/code/shared-types + alias: types + +# Behavioral thresholds — tune these to your workflow +thresholds: + session_stale_minutes: 30 # Warn if no activity for this long + max_tool_calls_before_checkpoint: 100 # Suggest a checkpoint after N tool calls + correction_pattern_threshold: 3 # Min corrections before flagging a pattern + +# Embedding provider for semantic search over session history. +# "local" uses Xenova transformers (no API key needed, runs on CPU). +# "openai" uses text-embedding-3-small (faster, needs OPENAI_API_KEY). +embeddings: + provider: local + # openai_api_key: sk-... # Uncomment if using openai provider diff --git a/examples/.preflight/contracts/api.yml b/examples/.preflight/contracts/api.yml new file mode 100644 index 0000000..512543f --- /dev/null +++ b/examples/.preflight/contracts/api.yml @@ -0,0 +1,58 @@ +# .preflight/contracts/api.yml — Manual contract definitions +# +# Define shared types and interfaces that preflight should know about. +# These supplement auto-extracted contracts from your codebase. +# Manual definitions win on name conflicts with auto-extracted ones. +# +# Why manual contracts? +# - Document cross-service interfaces that live in docs, not code +# - Define contracts for external APIs your services consume +# - Pin down types that are implicit (e.g., event payloads) + +- name: User + kind: interface + description: Core user model shared across all services + fields: + - name: id + type: string + required: true + - name: email + type: string + required: true + - name: tier + type: "'free' | 'pro' | 'enterprise'" + required: true + - name: createdAt + type: Date + required: true + +- name: AuthToken + kind: interface + description: JWT payload structure from auth-service + fields: + - name: userId + type: string + required: true + - name: permissions + type: string[] + required: true + - name: expiresAt + type: number + required: true + +- name: WebhookPayload + kind: interface + description: Standard webhook envelope for inter-service events + fields: + - name: event + type: string + required: true + - name: timestamp + type: string + required: true + - name: data + type: Record + required: true + - name: source + type: string + required: true diff --git a/examples/.preflight/triage.yml b/examples/.preflight/triage.yml new file mode 100644 index 0000000..b3d394e --- /dev/null +++ b/examples/.preflight/triage.yml @@ -0,0 +1,45 @@ +# .preflight/triage.yml — Controls how preflight classifies your prompts +# +# The triage engine routes prompts into categories: +# TRIVIAL → pass through (commit, format, lint) +# CLEAR → well-specified, no intervention needed +# AMBIGUOUS → needs clarification before proceeding +# MULTI-STEP → complex task, preflight suggests a plan +# CROSS-SERVICE → touches multiple projects, pulls in contracts +# +# Customize the keywords below to match your domain. + +rules: + # Prompts containing these words are always flagged as AMBIGUOUS. + # Add domain-specific terms that tend to produce vague prompts. + always_check: + - rewards + - permissions + - migration + - schema + - pricing # example: your billing domain + - onboarding # example: multi-step user flows + + # Prompts containing these words skip checks entirely (TRIVIAL). + # These are safe, mechanical tasks that don't need guardrails. + skip: + - commit + - format + - lint + - prettier + - "git push" + + # Prompts containing these words trigger CROSS-SERVICE classification. + # Preflight will search related_projects for relevant types and routes. + cross_service_keywords: + - auth + - notification + - event + - webhook + - billing # matches the related_project alias + +# How aggressively to classify prompts. +# "relaxed" — more prompts pass as clear (experienced users) +# "standard" — balanced (default) +# "strict" — more prompts flagged as ambiguous (new teams, complex codebases) +strictness: standard diff --git a/examples/README.md b/examples/README.md new file mode 100644 index 0000000..778f15d --- /dev/null +++ b/examples/README.md @@ -0,0 +1,35 @@ +# Examples + +## `.preflight/` Config Directory + +The `.preflight/` directory contains example configuration files you can copy into your project root: + +``` +.preflight/ +├── config.yml # Main config — profile, related projects, thresholds +├── triage.yml # Triage rules — keywords, strictness +└── contracts/ + └── api.yml # Manual contract definitions for cross-service types +``` + +### Quick setup + +```bash +# From your project root: +cp -r /path/to/preflight/examples/.preflight .preflight + +# Edit paths in config.yml to match your setup: +$EDITOR .preflight/config.yml +``` + +Then commit `.preflight/` to your repo — your whole team gets the same preflight behavior. + +### What each file does + +| File | Purpose | Required? | +|------|---------|-----------| +| `config.yml` | Profile, related projects, thresholds, embedding config | No — sensible defaults | +| `triage.yml` | Keyword rules for prompt classification | No — sensible defaults | +| `contracts/*.yml` | Manual type/interface definitions for cross-service awareness | No — auto-extraction works without it | + +All files are optional. Preflight works out of the box with zero config — these files let you tune it to your codebase. diff --git a/src/lib/git.ts b/src/lib/git.ts index a32ee3c..be903dd 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -1,4 +1,4 @@ -import { execFileSync } from "child_process"; +import { execFileSync, execSync } from "child_process"; import { PROJECT_DIR } from "./files.js"; import type { RunError } from "../types.js"; @@ -35,6 +35,29 @@ function gitCmd(cmdStr: string, opts?: { timeout?: number }): string { return run(cmdStr.split(/\s+/), opts); } +/** + * Run an arbitrary shell command (with pipes, redirects, etc.). + * Use sparingly — only for non-git commands that genuinely need shell features. + * Returns stdout on success, descriptive error string on failure. + */ +export function shell(cmd: string, opts: { timeout?: number } = {}): string { + try { + return execSync(cmd, { + cwd: PROJECT_DIR, + encoding: "utf-8", + timeout: opts.timeout || 10000, + maxBuffer: 1024 * 1024, + stdio: ["pipe", "pipe", "pipe"], + }).trim(); + } catch (e: any) { + const timedOut = e.killed === true || e.signal === "SIGTERM"; + if (timedOut) return `[timed out after ${opts.timeout || 10000}ms]`; + const output = e.stdout?.trim() || e.stderr?.trim(); + if (output) return output; + return `[command failed: ${cmd} (exit ${e.status ?? "?"})]`; + } +} + /** Get the current branch name. */ export function getBranch(): string { return run(["branch", "--show-current"]); diff --git a/src/tools/audit-workspace.ts b/src/tools/audit-workspace.ts index d4306bd..6f49a46 100644 --- a/src/tools/audit-workspace.ts +++ b/src/tools/audit-workspace.ts @@ -1,5 +1,5 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run } from "../lib/git.js"; +import { run, shell } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs } from "../lib/files.js"; /** Extract top-level work areas from file paths generically */ @@ -36,7 +36,7 @@ export function registerAuditWorkspace(server: McpServer): void { {}, async () => { const docs = findWorkspaceDocs(); - const recentFiles = run("git diff --name-only HEAD~10 2>/dev/null || echo ''").split("\n").filter(Boolean); + const recentFiles = run(["diff", "--name-only", "HEAD~10"]).split("\n").filter(Boolean); const sections: string[] = []; // Doc freshness @@ -75,7 +75,7 @@ export function registerAuditWorkspace(server: McpServer): void { // Check for gap trackers or similar tracking docs const trackingDocs = Object.entries(docs).filter(([n]) => /gap|track|progress/i.test(n)); if (trackingDocs.length > 0) { - const testFilesCount = parseInt(run("find tests -name '*.spec.ts' -o -name '*.test.ts' 2>/dev/null | wc -l").trim()) || 0; + const testFilesCount = parseInt(shell("find tests -name '*.spec.ts' -o -name '*.test.ts' 2>/dev/null | wc -l").trim()) || 0; sections.push(`## Tracking Docs\n${trackingDocs.map(([n]) => { const age = docStatus.find(d => d.name === n)?.ageHours ?? "?"; return `- .claude/${n} — last updated ${age}h ago`; diff --git a/src/tools/enrich-agent-task.ts b/src/tools/enrich-agent-task.ts index 236edfa..7f5aac9 100644 --- a/src/tools/enrich-agent-task.ts +++ b/src/tools/enrich-agent-task.ts @@ -21,39 +21,52 @@ function detectPackageManager(): string { return "npm"; } +/** Get all git-tracked files (cached for multiple calls) */ +function getGitFiles(): string[] { + return run(["ls-files"]).split("\n").filter(Boolean); +} + +const TEST_RE = /\.(spec|test)\.(ts|tsx|js|jsx)$/; + /** Find files in a target area using git-tracked files (project-agnostic) */ function findAreaFiles(area: string): string { if (!area) return getDiffFiles("HEAD~3"); - const safeArea = shellEscape(area); + const allFiles = getGitFiles(); - // If area looks like a path, search directly + // If area looks like a path, search by prefix if (area.includes("/")) { - return run(`git ls-files -- '${safeArea}*' 2>/dev/null | head -20`); + const safeArea = shellEscape(area); + return allFiles.filter(f => f.startsWith(safeArea)).slice(0, 20).join("\n") || getDiffFiles("HEAD~3"); } - // Search for area keyword in git-tracked file paths - const files = run(`git ls-files 2>/dev/null | grep -i '${safeArea}' | head -20`); - if (files && !files.startsWith("[command failed")) return files; - - // Fallback to recently changed files - return getDiffFiles("HEAD~3"); + // Search for area keyword in file paths + const pattern = new RegExp(shellEscape(area), "i"); + const matched = allFiles.filter(f => pattern.test(f)).slice(0, 20).join("\n"); + return matched || getDiffFiles("HEAD~3"); } /** Find related test files for an area */ function findRelatedTests(area: string): string { - if (!area) return run("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10"); + const allFiles = getGitFiles(); + const testFiles = allFiles.filter(f => TEST_RE.test(f)); + + if (!area) return testFiles.slice(0, 10).join("\n"); const safeArea = shellEscape(area.split(/\s+/)[0]); - const tests = run(`git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | grep -i '${safeArea}' | head -10`); - return tests || run("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10"); + const pattern = new RegExp(safeArea, "i"); + const matched = testFiles.filter(f => pattern.test(f)).slice(0, 10).join("\n"); + return matched || testFiles.slice(0, 10).join("\n"); } /** Get an example pattern from the first matching file */ function getExamplePattern(files: string): string { const firstFile = files.split("\n").filter(Boolean)[0]; if (!firstFile) return "no pattern available"; - return run(`head -30 '${shellEscape(firstFile)}' 2>/dev/null || echo 'could not read file'`); + try { + const content = readFileSync(join(PROJECT_DIR, firstFile), "utf-8"); + return content.split("\n").slice(0, 30).join("\n"); + } catch { return "could not read file"; } } // --------------------------------------------------------------------------- diff --git a/src/tools/scope-work.ts b/src/tools/scope-work.ts index 9b5d971..ac01006 100644 --- a/src/tools/scope-work.ts +++ b/src/tools/scope-work.ts @@ -17,11 +17,6 @@ const STOP_WORDS = new Set([ "like", "some", "each", "only", "need", "want", "please", "update", "change", ]); -/** Shell-escape a string for use inside single quotes */ -function shellEscape(s: string): string { - return s.replace(/'/g, "'\\''"); -} - /** Safely parse git porcelain status lines */ function parsePortelainFiles(porcelain: string): string[] { if (!porcelain.trim()) return []; @@ -93,9 +88,9 @@ export function registerScopeWork(server: McpServer): void { const timestamp = now(); const currentBranch = branch ?? getBranch(); const recentCommits = getRecentCommits(10); - const porcelain = run("git status --porcelain"); + const porcelain = run(["status", "--porcelain"]); const dirtyFiles = parsePortelainFiles(porcelain); - const diffStat = dirtyFiles.length > 0 ? run("git diff --stat") : "(clean working tree)"; + const diffStat = dirtyFiles.length > 0 ? run(["diff", "--stat"]) : "(clean working tree)"; // Scan for relevant files based on task keywords const keywords = task.toLowerCase().split(/\s+/); @@ -127,8 +122,10 @@ export function registerScopeWork(server: McpServer): void { .filter((k) => k.length > 2) .slice(0, 5); if (grepTerms.length > 0) { - const pattern = shellEscape(grepTerms.join("|")); - matchedFiles = run(`git ls-files | head -500 | grep -iE '${pattern}' | head -30`); + const allFiles = run(["ls-files"]); + const pattern = new RegExp(grepTerms.join("|"), "i"); + matchedFiles = allFiles.split("\n").filter(Boolean).slice(0, 500) + .filter(f => pattern.test(f)).slice(0, 30).join("\n"); } // Check which relevant dirs actually exist (with path traversal protection) diff --git a/src/tools/sequence-tasks.ts b/src/tools/sequence-tasks.ts index 22dea23..b6fbdc0 100644 --- a/src/tools/sequence-tasks.ts +++ b/src/tools/sequence-tasks.ts @@ -90,7 +90,8 @@ export function registerSequenceTasks(server: McpServer): void { // For locality: infer directories from path-like tokens in task text if (strategy === "locality") { // Use git ls-files with a depth limit instead of find for performance - const gitFiles = run("git ls-files 2>/dev/null | head -1000"); + const gitFilesRaw = run(["ls-files"]); + const gitFiles = gitFilesRaw.split("\n").slice(0, 1000).join("\n"); const knownDirs = new Set(); for (const f of gitFiles.split("\n").filter(Boolean)) { const parts = f.split("/"); diff --git a/src/tools/session-handoff.ts b/src/tools/session-handoff.ts index d199462..6d01428 100644 --- a/src/tools/session-handoff.ts +++ b/src/tools/session-handoff.ts @@ -2,13 +2,13 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { existsSync, readFileSync } from "fs"; import { join } from "path"; -import { run, getBranch, getRecentCommits, getStatus } from "../lib/git.js"; +import { run, shell, getBranch, getRecentCommits, getStatus } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs } from "../lib/files.js"; import { STATE_DIR, now } from "../lib/state.js"; /** Check if a CLI tool is available */ function hasCommand(cmd: string): boolean { - const result = run(`command -v ${cmd} 2>/dev/null`); + const result = shell(`command -v ${cmd} 2>/dev/null`); return !!result && !result.startsWith("[command failed"); } @@ -44,7 +44,7 @@ export function registerSessionHandoff(server: McpServer): void { // Only try gh if it exists if (hasCommand("gh")) { - const openPRs = run("gh pr list --state open --json number,title,headRefName 2>/dev/null || echo '[]'"); + const openPRs = shell("gh pr list --state open --json number,title,headRefName 2>/dev/null || echo '[]'"); if (openPRs && openPRs !== "[]") { sections.push(`## Open PRs\n\`\`\`json\n${openPRs}\n\`\`\``); } diff --git a/src/tools/sharpen-followup.ts b/src/tools/sharpen-followup.ts index db5acaa..e95bdaf 100644 --- a/src/tools/sharpen-followup.ts +++ b/src/tools/sharpen-followup.ts @@ -27,10 +27,10 @@ function parsePortelainFiles(output: string): string[] { /** Get recently changed files, safe for first commit / shallow clones */ function getRecentChangedFiles(): string[] { // Try HEAD~1..HEAD, fall back to just staged, then unstaged - const commands = [ - "git diff --name-only HEAD~1 HEAD 2>/dev/null", - "git diff --name-only --cached 2>/dev/null", - "git diff --name-only 2>/dev/null", + const commands: string[][] = [ + ["diff", "--name-only", "HEAD~1", "HEAD"], + ["diff", "--name-only", "--cached"], + ["diff", "--name-only"], ]; const results = new Set(); for (const cmd of commands) { @@ -87,7 +87,7 @@ export function registerSharpenFollowup(server: McpServer): void { // Gather context to resolve ambiguity const contextFiles: string[] = [...(previous_files ?? [])]; const recentChanged = getRecentChangedFiles(); - const porcelainOutput = run("git status --porcelain 2>/dev/null"); + const porcelainOutput = run(["status", "--porcelain"]); const untrackedOrModified = parsePortelainFiles(porcelainOutput); const allKnownFiles = [...new Set([...contextFiles, ...recentChanged, ...untrackedOrModified])].filter(Boolean); diff --git a/src/tools/token-audit.ts b/src/tools/token-audit.ts index b7aad2c..35dd5a7 100644 --- a/src/tools/token-audit.ts +++ b/src/tools/token-audit.ts @@ -4,14 +4,9 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { run } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js"; import { loadState, saveState, now, STATE_DIR } from "../lib/state.js"; -import { readFileSync, existsSync, statSync } from "fs"; +import { readFileSync, existsSync, statSync, openSync, readSync, closeSync } from "fs"; import { join } from "path"; -/** Shell-escape a filename for safe interpolation */ -function shellEscape(s: string): string { - return s.replace(/'/g, "'\\''"); -} - /** * Grade thresholds rationale: * - A (0-10): Minimal waste — small diffs, targeted reads, lean context @@ -39,8 +34,8 @@ export function registerTokenAudit(server: McpServer): void { let wasteScore = 0; // 1. Git diff size & dirty file count - const diffStat = run("git diff --stat --no-color 2>/dev/null"); - const dirtyFiles = run("git diff --name-only 2>/dev/null"); + const diffStat = run(["diff", "--stat", "--no-color"]); + const dirtyFiles = run(["diff", "--name-only"]); const dirtyList = dirtyFiles.split("\n").filter(Boolean); const dirtyCount = dirtyList.length; @@ -62,9 +57,11 @@ export function registerTokenAudit(server: McpServer): void { const largeFiles: string[] = []; for (const f of dirtyList.slice(0, 30)) { - // Use shell-safe quoting instead of interpolation - const wc = run(`wc -l < '${shellEscape(f)}' 2>/dev/null`); - const lines = parseInt(wc) || 0; + let lines = 0; + try { + const content = readFileSync(join(PROJECT_DIR, f), "utf-8"); + lines = content.split("\n").length; + } catch { /* skip unreadable files */ } estimatedContextTokens += lines * AVG_LINE_BYTES * AVG_TOKENS_PER_BYTE; if (lines > 500) { largeFiles.push(`${f} (${lines} lines)`); @@ -80,8 +77,8 @@ export function registerTokenAudit(server: McpServer): void { // 3. CLAUDE.md bloat check const claudeMd = readIfExists("CLAUDE.md", 1); if (claudeMd !== null) { - const stat = run(`wc -c < '${shellEscape("CLAUDE.md")}' 2>/dev/null`); - const bytes = parseInt(stat) || 0; + let bytes = 0; + try { bytes = statSync(join(PROJECT_DIR, "CLAUDE.md")).size; } catch { /* ignore */ } if (bytes > 5120) { patterns.push(`CLAUDE.md is ${(bytes / 1024).toFixed(1)}KB — injected every session, burns tokens on paste`); recommendations.push("Trim CLAUDE.md to essentials (<5KB). Move reference docs to files read on-demand"); @@ -137,9 +134,16 @@ export function registerTokenAudit(server: McpServer): void { } // Read with size cap: take the tail if too large - const raw = stat.size <= MAX_TOOL_LOG_BYTES - ? readFileSync(toolLogPath, "utf-8") - : run(`tail -c ${MAX_TOOL_LOG_BYTES} '${shellEscape(toolLogPath)}'`); + let raw: string; + if (stat.size <= MAX_TOOL_LOG_BYTES) { + raw = readFileSync(toolLogPath, "utf-8"); + } else { + const fd = openSync(toolLogPath, "r"); + const buf = Buffer.alloc(MAX_TOOL_LOG_BYTES); + readSync(fd, buf, 0, MAX_TOOL_LOG_BYTES, stat.size - MAX_TOOL_LOG_BYTES); + closeSync(fd); + raw = buf.toString("utf-8"); + } const lines = raw.trim().split("\n").filter(Boolean); totalToolCalls = lines.length; diff --git a/src/tools/verify-completion.ts b/src/tools/verify-completion.ts index 732532f..5278d2f 100644 --- a/src/tools/verify-completion.ts +++ b/src/tools/verify-completion.ts @@ -1,8 +1,8 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run, getStatus } from "../lib/git.js"; +import { run, shell, getStatus } from "../lib/git.js"; import { PROJECT_DIR } from "../lib/files.js"; -import { existsSync } from "fs"; +import { existsSync, readFileSync } from "fs"; import { join } from "path"; /** Detect package manager from lockfiles */ @@ -34,7 +34,9 @@ function detectTestRunner(): string | null { /** Check if a build script exists in package.json */ function hasBuildScript(): boolean { try { - const pkg = JSON.parse(run("cat package.json 2>/dev/null")); + const pkgPath = join(PROJECT_DIR, "package.json"); + if (!existsSync(pkgPath)) return false; + const pkg = JSON.parse(readFileSync(pkgPath, "utf-8")); return !!pkg?.scripts?.build; } catch { return false; } } @@ -55,7 +57,8 @@ export function registerVerifyCompletion(server: McpServer): void { const checks: { name: string; passed: boolean; detail: string }[] = []; // 1. Type check (single invocation, extract both result and count) - const tscOutput = run(`${pm === "npx" ? "npx" : pm} tsc --noEmit 2>&1 | tail -20`); + const tscRaw = shell(`${pm === "npx" ? "npx" : pm} tsc --noEmit 2>&1`, { timeout: 60000 }); + const tscOutput = tscRaw.split("\n").slice(-20).join("\n"); const errorLines = tscOutput.split("\n").filter(l => /error TS\d+/.test(l)); const typePassed = errorLines.length === 0; checks.push({ @@ -80,39 +83,38 @@ export function registerVerifyCompletion(server: McpServer): void { // 3. Tests if (!skip_tests) { const runner = detectTestRunner(); - const changedFiles = run("git diff --name-only HEAD~1 2>/dev/null").split("\n").filter(Boolean); + const changedFiles = run(["diff", "--name-only", "HEAD~1"]).split("\n").filter(Boolean); let testCmd = ""; if (runner === "playwright") { const runnerCmd = `${pm === "npx" ? "npx" : `${pm} exec`} playwright test`; if (test_scope && test_scope !== "all") { testCmd = test_scope.endsWith(".spec.ts") || test_scope.endsWith(".test.ts") - ? `${runnerCmd} ${test_scope} --reporter=line 2>&1 | tail -20` - : `${runnerCmd} --grep "${test_scope}" --reporter=line 2>&1 | tail -20`; + ? `${runnerCmd} ${test_scope} --reporter=line 2>&1` + : `${runnerCmd} --grep "${test_scope}" --reporter=line 2>&1`; } else { - // Auto-detect from changed files const changedTests = changedFiles.filter(f => /\.(spec|test)\.(ts|tsx|js)$/.test(f)).slice(0, 5); if (changedTests.length > 0) { - testCmd = `${runnerCmd} ${changedTests.join(" ")} --reporter=line 2>&1 | tail -20`; + testCmd = `${runnerCmd} ${changedTests.join(" ")} --reporter=line 2>&1`; } } } else if (runner === "vitest" || runner === "jest") { const runnerCmd = `${pm === "npx" ? "npx" : `${pm} exec`} ${runner}`; if (test_scope && test_scope !== "all") { - testCmd = `${runnerCmd} --run ${test_scope} 2>&1 | tail -20`; + testCmd = `${runnerCmd} --run ${test_scope} 2>&1`; } else { const changedTests = changedFiles.filter(f => /\.(spec|test)\.(ts|tsx|js)$/.test(f)).slice(0, 5); if (changedTests.length > 0) { - testCmd = `${runnerCmd} --run ${changedTests.join(" ")} 2>&1 | tail -20`; + testCmd = `${runnerCmd} --run ${changedTests.join(" ")} 2>&1`; } } } else if (test_scope) { - // No recognized runner but scope given — try npm test - testCmd = `${pm} test 2>&1 | tail -20`; + testCmd = `${pm} test 2>&1`; } if (testCmd) { - const testResult = run(testCmd, { timeout: 120000 }); + const testRaw = shell(testCmd, { timeout: 120000 }); + const testResult = testRaw.split("\n").slice(-20).join("\n"); const testPassed = /pass/i.test(testResult) && !/fail/i.test(testResult); checks.push({ name: "Tests", @@ -130,7 +132,8 @@ export function registerVerifyCompletion(server: McpServer): void { // 4. Build check (only if build script exists and not skipped) if (!skip_build && hasBuildScript()) { - const buildCheck = run(`${pm === "npx" ? "npm run" : pm} build 2>&1 | tail -10`, { timeout: 60000 }); + const buildRaw = shell(`${pm === "npx" ? "npm run" : pm} build 2>&1`, { timeout: 60000 }); + const buildCheck = buildRaw.split("\n").slice(-10).join("\n"); const buildPassed = !/\b[Ee]rror\b/.test(buildCheck) || /Successfully compiled/.test(buildCheck); checks.push({ name: "Build", From c3100caa64fa6139bd7b5d8c1c5d1f6648aa5c21 Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Tue, 10 Mar 2026 14:15:45 -0700 Subject: [PATCH 2/2] fix: replace shell syntax in run() calls with array args or shell() Fixes remaining broken run() calls in 4 tools: - clarify-intent: use shell() for pnpm/find commands with pipes - session-health: use array args + JS .pop() instead of pipe to tail - what-changed: use array args with JS fallback instead of || operator - checkpoint: use array args for add/commit/reset instead of string interpolation Part of #172 --- src/tools/checkpoint.ts | 9 ++++++--- src/tools/clarify-intent.ts | 6 +++--- src/tools/session-health.ts | 3 ++- src/tools/what-changed.ts | 6 ++++-- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/tools/checkpoint.ts b/src/tools/checkpoint.ts index e086f01..45b90df 100644 --- a/src/tools/checkpoint.ts +++ b/src/tools/checkpoint.ts @@ -84,11 +84,14 @@ ${dirty || "clean"} if (commitResult === "no uncommitted changes") { // Stage the checkpoint file too - run(`git add "${checkpointFile}"`); - const result = run(`${addCmd} && git commit -m "${commitMsg.replace(/"/g, '\\"')}" 2>&1`); + run(["add", checkpointFile]); + // Stage files based on scope + if (addCmd === "git add -A") run(["add", "-A"]); + else if (addCmd === "git add -u") run(["add", "-u"]); + const result = run(["commit", "-m", commitMsg]); if (result.includes("commit failed") || result.includes("nothing to commit")) { // Rollback: unstage if commit failed - run("git reset HEAD 2>/dev/null"); + run(["reset", "HEAD"]); commitResult = `commit failed: ${result}`; } else { commitResult = result; diff --git a/src/tools/clarify-intent.ts b/src/tools/clarify-intent.ts index 32efa3a..f141269 100644 --- a/src/tools/clarify-intent.ts +++ b/src/tools/clarify-intent.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run, getBranch, getStatus, getRecentCommits, getDiffFiles, getStagedFiles } from "../lib/git.js"; +import { run, shell, getBranch, getStatus, getRecentCommits, getDiffFiles, getStagedFiles } from "../lib/git.js"; import { findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js"; import { searchSemantic } from "../lib/timeline-db.js"; import { getRelatedProjects } from "../lib/config.js"; @@ -152,10 +152,10 @@ export function registerClarifyIntent(server: McpServer): void { let hasTestFailures = false; if (!area || area.includes("test") || area.includes("fix") || area.includes("ui") || area.includes("api")) { - const typeErrors = run("pnpm tsc --noEmit 2>&1 | grep -c 'error TS' || echo '0'"); + const typeErrors = shell("pnpm tsc --noEmit 2>&1 | grep -c 'error TS' || echo '0'"); hasTypeErrors = parseInt(typeErrors, 10) > 0; - const testFiles = run("find tests -name '*.spec.ts' -maxdepth 4 2>/dev/null | head -20"); + const testFiles = shell("find tests -name '*.spec.ts' -maxdepth 4 2>/dev/null | head -20"); const failingTests = getTestFailures(); hasTestFailures = failingTests !== "all passing" && failingTests !== "no test report found"; diff --git a/src/tools/session-health.ts b/src/tools/session-health.ts index bd6a819..343b882 100644 --- a/src/tools/session-health.ts +++ b/src/tools/session-health.ts @@ -27,7 +27,8 @@ export function registerSessionHealth(server: McpServer): void { const dirtyCount = dirty ? dirty.split("\n").filter(Boolean).length : 0; const lastCommit = getLastCommit(); const lastCommitTimeStr = getLastCommitTime(); - const uncommittedDiff = run("git diff --stat | tail -1"); + const uncommittedDiffRaw = run(["diff", "--stat"]); + const uncommittedDiff = uncommittedDiffRaw.split("\n").filter(Boolean).pop() || ""; // Parse commit time safely const commitDate = parseGitDate(lastCommitTimeStr); diff --git a/src/tools/what-changed.ts b/src/tools/what-changed.ts index 913dfa2..bb4ea39 100644 --- a/src/tools/what-changed.ts +++ b/src/tools/what-changed.ts @@ -12,8 +12,10 @@ export function registerWhatChanged(server: McpServer): void { async ({ since }) => { const ref = since || "HEAD~5"; const diffStat = getDiffStat(ref); - const diffFiles = run(`git diff ${ref} --name-only 2>/dev/null || git diff HEAD~3 --name-only`); - const log = run(`git log ${ref}..HEAD --oneline 2>/dev/null || git log -5 --oneline`); + let diffFiles = run(["diff", ref, "--name-only"]); + if (diffFiles.startsWith("[")) diffFiles = run(["diff", "HEAD~3", "--name-only"]); + let log = run(["log", `${ref}..HEAD`, "--oneline"]); + if (log.startsWith("[")) log = run(["log", "-5", "--oneline"]); const branch = getBranch(); const fileList = diffFiles.split("\n").filter(Boolean);