From edeb6e685492792284b188db02e4e94b6c0d6dbf Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Fri, 13 Mar 2026 12:17:46 -0700 Subject: [PATCH 1/2] fix: remove all unused imports and dead code (27 lint warnings) - Remove unused imports across 13 source files - Remove dead `gitCmd` function from lib/git.ts - Remove dead `toMarkdown` function from generate-scorecard.ts (superseded by toMarkdownWithBaseline) - Remove dead `CORRECTION_PATTERNS` constant from generate-scorecard.ts - Lint warnings reduced from 74 to 47 (0 errors) - All 43 tests pass, build clean --- package-lock.json | 5 +++-- src/index.ts | 2 +- src/lib/files.ts | 2 +- src/lib/git.ts | 4 ---- src/lib/patterns.ts | 1 - src/lib/timeline-db.ts | 8 ++++---- src/tools/audit-workspace.ts | 2 +- src/tools/checkpoint.ts | 2 +- src/tools/generate-scorecard.ts | 36 +-------------------------------- src/tools/onboard-project.ts | 2 -- src/tools/preflight-check.ts | 8 ++++---- src/tools/scan-sessions.ts | 1 - src/tools/scope-work.ts | 4 ++-- src/tools/sequence-tasks.ts | 3 +-- src/tools/token-audit.ts | 2 +- 15 files changed, 20 insertions(+), 62 deletions(-) diff --git a/package-lock.json b/package-lock.json index 89ef280..44e4450 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,8 @@ "js-yaml": "^4.1.1" }, "bin": { - "preflight-dev": "bin/cli.js" + "preflight-dev": "bin/cli.js", + "preflight-dev-serve": "bin/serve.js" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -29,7 +30,7 @@ "vitest": "^4.0.18" }, "engines": { - "node": ">=18" + "node": ">=20" } }, "node_modules/@esbuild/aix-ppc64": { diff --git a/src/index.ts b/src/index.ts index e7e9d00..9731f96 100644 --- a/src/index.ts +++ b/src/index.ts @@ -73,7 +73,7 @@ function validateRelatedProjects(): void { } // Load config and validate related projects on startup -const config = getConfig(); +getConfig(); validateRelatedProjects(); const profile = getProfile(); diff --git a/src/lib/files.ts b/src/lib/files.ts index 1cca2d4..1275c74 100644 --- a/src/lib/files.ts +++ b/src/lib/files.ts @@ -1,6 +1,6 @@ import { readFileSync, existsSync, readdirSync, statSync } from "fs"; import { join } from "path"; -import type { DocInfo, DocMeta } from "../types.js"; +import type { DocInfo } from "../types.js"; /** Single source of truth for the project directory. */ export const PROJECT_DIR = process.env.CLAUDE_PROJECT_DIR || process.cwd(); diff --git a/src/lib/git.ts b/src/lib/git.ts index a32ee3c..d41a913 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -1,6 +1,5 @@ import { execFileSync } from "child_process"; import { PROJECT_DIR } from "./files.js"; -import type { RunError } from "../types.js"; /** * Run a git command safely using execFileSync (no shell injection). @@ -31,9 +30,6 @@ export function run(argsOrCmd: string | string[], opts: { timeout?: number } = { } /** Convenience: run a raw command string (split on spaces). Only for simple, known-safe commands. */ -function gitCmd(cmdStr: string, opts?: { timeout?: number }): string { - return run(cmdStr.split(/\s+/), opts); -} /** Get the current branch name. */ export function getBranch(): string { diff --git a/src/lib/patterns.ts b/src/lib/patterns.ts index 350e048..b41cc99 100644 --- a/src/lib/patterns.ts +++ b/src/lib/patterns.ts @@ -146,7 +146,6 @@ export function matchPatterns( patterns: CorrectionPattern[], ): CorrectionPattern[] { if (patterns.length === 0) return []; - const promptKeywords = extractKeywords(prompt); const promptLower = prompt.toLowerCase(); return patterns.filter((p) => { diff --git a/src/lib/timeline-db.ts b/src/lib/timeline-db.ts index 49b4f78..42b42f4 100644 --- a/src/lib/timeline-db.ts +++ b/src/lib/timeline-db.ts @@ -1,11 +1,11 @@ import * as lancedb from "@lancedb/lancedb"; import { randomUUID } from "node:crypto"; -import { readFile, writeFile, mkdir, stat } from "node:fs/promises"; +import { readFile, writeFile, mkdir } from "node:fs/promises"; import { createHash } from "node:crypto"; import { homedir } from "node:os"; import { join, basename, resolve } from "node:path"; -import { createEmbeddingProvider, type EmbeddingProvider, type EmbeddingConfig } from "./embeddings.js"; -import type { ProjectMeta, ProjectRegistry, SearchScope } from "../types.js"; +import { createEmbeddingProvider, type EmbeddingProvider } from "./embeddings.js"; +import type { ProjectMeta, ProjectRegistry } from "../types.js"; // --- Types --- @@ -342,7 +342,7 @@ export async function searchSemantic( _score: 1 - (result._distance || 0), }); } - } catch (error) { + } catch { // Skip projects that don't exist or have issues continue; } diff --git a/src/tools/audit-workspace.ts b/src/tools/audit-workspace.ts index d4306bd..1f59fb3 100644 --- a/src/tools/audit-workspace.ts +++ b/src/tools/audit-workspace.ts @@ -1,6 +1,6 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { run } from "../lib/git.js"; -import { readIfExists, findWorkspaceDocs } from "../lib/files.js"; +import { findWorkspaceDocs } from "../lib/files.js"; /** Extract top-level work areas from file paths generically */ function detectWorkAreas(files: string[]): Set { diff --git a/src/tools/checkpoint.ts b/src/tools/checkpoint.ts index e086f01..53d9ace 100644 --- a/src/tools/checkpoint.ts +++ b/src/tools/checkpoint.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { writeFileSync, existsSync, mkdirSync } from "fs"; -import { join, dirname } from "path"; +import { join } from "path"; import { run, getBranch, getStatus, getLastCommit, getStagedFiles } from "../lib/git.js"; import { PROJECT_DIR } from "../lib/files.js"; import { appendLog, now } from "../lib/state.js"; diff --git a/src/tools/generate-scorecard.ts b/src/tools/generate-scorecard.ts index c15576c..3a5f02c 100644 --- a/src/tools/generate-scorecard.ts +++ b/src/tools/generate-scorecard.ts @@ -60,7 +60,7 @@ function clamp(v: number): number { const PATH_RE = /(?:\/[\w./-]+\.\w{1,6}|\b\w+\.\w{2,6}\b)/; const FILE_EXT_RE = /\.\b(?:ts|tsx|js|jsx|py|rs|go|rb|java|c|cpp|h|css|scss|html|json|yaml|yml|toml|md|sql|sh)\b/; -const CORRECTION_PATTERNS = [/\bno\b/i, /\bwrong\b/i, /\bnot that\b/i, /\bi meant\b/i, /\bactually\b/i, /\binstead\b/i, /\bundo\b/i, /\brevert\b/i]; + interface ParsedSession { id: string; @@ -419,40 +419,6 @@ function computeScorecard( }; } -// ── Markdown Output ──────────────────────────────────────────────────────── - -function toMarkdown(sc: Scorecard): string { - const lines: string[] = []; - lines.push(`# 📊 Prompt Discipline Scorecard`); - lines.push(`**Project:** ${sc.project} | **Period:** ${sc.period} (${sc.date}) | **Overall: ${sc.overallGrade} (${sc.overall}/100)**\n`); - - lines.push(`## Category Scores`); - lines.push(`| # | Category | Score | Grade |`); - lines.push(`|---|----------|-------|-------|`); - sc.categories.forEach((c, i) => { - lines.push(`| ${i + 1} | ${c.name} | ${c.score} | ${c.grade} |`); - }); - - lines.push(`\n## Highlights`); - lines.push(`- 🏆 **Best:** ${sc.highlights.best.name} (${sc.highlights.best.grade}) — ${sc.highlights.best.evidence}`); - lines.push(`- ⚠️ **Worst:** ${sc.highlights.worst.name} (${sc.highlights.worst.grade}) — ${sc.highlights.worst.evidence}`); - - lines.push(`\n## Detailed Breakdown`); - sc.categories.forEach((c, i) => { - lines.push(`\n### ${i + 1}. ${c.name} — ${c.grade} (${c.score}/100)`); - lines.push(`Evidence: ${c.evidence}`); - if (c.examples?.bad?.length) { - lines.push(`\nExamples of vague follow-ups:`); - c.examples.bad.forEach((e) => lines.push(`- ❌ "${e}"`)); - } - if (c.examples?.good?.length) { - lines.push(`\nExamples of specific follow-ups:`); - c.examples.good.forEach((e) => lines.push(`- ✅ "${e}"`)); - } - }); - - return lines.join("\n"); -} // ── HTML / PDF Output ────────────────────────────────────────────────────── diff --git a/src/tools/onboard-project.ts b/src/tools/onboard-project.ts index bca91a0..69f7b29 100644 --- a/src/tools/onboard-project.ts +++ b/src/tools/onboard-project.ts @@ -5,9 +5,7 @@ import * as path from "path"; import { insertEvents, getLastIndexedTimestamp, - listIndexedProjects, getEventsTable, - registerProject, loadProjectMeta, saveProjectMeta } from "../lib/timeline-db.js"; diff --git a/src/tools/preflight-check.ts b/src/tools/preflight-check.ts index 8c9121a..1f2d49e 100644 --- a/src/tools/preflight-check.ts +++ b/src/tools/preflight-check.ts @@ -1,17 +1,17 @@ // Unified preflight_check — single entry point that triages and chains tools import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { triagePrompt, type TriageLevel, type TriageResult } from "../lib/triage.js"; +import { triagePrompt, type TriageLevel } from "../lib/triage.js"; import { existsSync, statSync } from "fs"; import { resolve } from "path"; import { PROJECT_DIR } from "../lib/files.js"; -import { run, getBranch, getStatus, getRecentCommits, getDiffFiles, getStagedFiles } from "../lib/git.js"; +import { getBranch, getStatus, getRecentCommits } from "../lib/git.js"; import { now } from "../lib/state.js"; import { findWorkspaceDocs } from "../lib/files.js"; import { getConfig } from "../lib/config.js"; import { searchSemantic } from "../lib/timeline-db.js"; -import { basename, join } from "path"; -import { loadPatterns, matchPatterns, formatPatternMatches } from "../lib/patterns.js"; +import { basename } from "path"; +import { loadPatterns, matchPatterns } from "../lib/patterns.js"; // --------------------------------------------------------------------------- // Helpers diff --git a/src/tools/scan-sessions.ts b/src/tools/scan-sessions.ts index 3d3ecb5..707f7b2 100644 --- a/src/tools/scan-sessions.ts +++ b/src/tools/scan-sessions.ts @@ -1,7 +1,6 @@ import { z } from "zod"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import * as fs from "fs"; -import * as path from "path"; import { findSessionDirs, findSessionFiles } from "../lib/session-parser.js"; interface SessionInfo { diff --git a/src/tools/scope-work.ts b/src/tools/scope-work.ts index 9b5d971..9506afc 100644 --- a/src/tools/scope-work.ts +++ b/src/tools/scope-work.ts @@ -1,13 +1,13 @@ // CATEGORY 1: scope_work — Plans import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run, getBranch, getRecentCommits, getStatus } from "../lib/git.js"; +import { run, getBranch, getRecentCommits } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js"; import { searchSemantic } from "../lib/timeline-db.js"; import { getRelatedProjects } from "../lib/config.js"; import { now } from "../lib/state.js"; import { existsSync } from "fs"; -import { join, normalize, resolve, basename } from "path"; +import { join, resolve, basename } from "path"; import { loadAllContracts, searchContracts, formatContracts } from "../lib/contracts.js"; const STOP_WORDS = new Set([ diff --git a/src/tools/sequence-tasks.ts b/src/tools/sequence-tasks.ts index 22dea23..bc4f36e 100644 --- a/src/tools/sequence-tasks.ts +++ b/src/tools/sequence-tasks.ts @@ -4,8 +4,7 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { run } from "../lib/git.js"; import { now } from "../lib/state.js"; import { PROJECT_DIR } from "../lib/files.js"; -import { existsSync } from "fs"; -import { join, resolve } from "path"; +import { resolve } from "path"; type Cat = "schema" | "config" | "api" | "ui" | "test" | "other"; diff --git a/src/tools/token-audit.ts b/src/tools/token-audit.ts index b7aad2c..bb34277 100644 --- a/src/tools/token-audit.ts +++ b/src/tools/token-audit.ts @@ -3,7 +3,7 @@ import { z } from "zod"; 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 { saveState, now, STATE_DIR } from "../lib/state.js"; import { readFileSync, existsSync, statSync } from "fs"; import { join } from "path"; From 60a5b4997c8806270e692847511d4124fb723477 Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Fri, 13 Mar 2026 13:19:02 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20broken=20shell=20commands=20in=20run?= =?UTF-8?q?()=20=E2=80=94=20strip=20'git'=20prefix,=20shell=20operators,?= =?UTF-8?q?=20add=20shell()=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run() uses execFileSync('git', args) but many callers passed: - 'git diff ...' (doubled to 'git git diff ...') - '2>/dev/null' (passed as literal git arg) - '|| fallback' (pipe/or treated as git args) - Non-git commands like 'find', 'cat', 'wc' (executed as 'git find ...') All of these silently failed and returned error strings. Fix: - run() now strips leading 'git' from string args - run() strips shell operators (2>/dev/null, ||, |, &&) - New shell() function for commands needing real shell features - Migrated 13 tool files to use proper array args or shell() - Added 8 tests covering the new behavior Affects: audit-workspace, checkpoint, clarify-intent, enrich-agent-task, scope-work, sequence-tasks, session-handoff, session-health, sharpen-followup, token-audit, verify-completion, what-changed --- src/lib/git.ts | 53 +++++++++++++++++-- src/tools/audit-workspace.ts | 6 +-- src/tools/checkpoint.ts | 4 +- src/tools/clarify-intent.ts | 6 +-- src/tools/enrich-agent-task.ts | 14 ++--- src/tools/scope-work.ts | 8 +-- src/tools/sequence-tasks.ts | 4 +- src/tools/session-handoff.ts | 6 +-- src/tools/session-health.ts | 4 +- src/tools/sharpen-followup.ts | 16 +++--- src/tools/token-audit.ts | 12 ++--- src/tools/verify-completion.ts | 12 ++--- src/tools/what-changed.ts | 6 ++- tests/lib/git.test.ts | 94 ++++++++++++++++++++++++++++++++++ 14 files changed, 194 insertions(+), 51 deletions(-) create mode 100644 tests/lib/git.test.ts diff --git a/src/lib/git.ts b/src/lib/git.ts index d41a913..ec5c667 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -1,13 +1,39 @@ -import { execFileSync } from "child_process"; +import { execFileSync, execSync } from "child_process"; import { PROJECT_DIR } from "./files.js"; +/** + * Strip shell-isms from a string command that will be passed to execFileSync. + * Removes trailing `2>/dev/null`, `2>&1`, and shell fallback chains (`|| ...`). + */ +function cleanShellArgs(parts: string[]): string[] { + const cleaned: string[] = []; + for (const p of parts) { + // Stop at shell operators + if (p === "||" || p === "&&" || p === "|") break; + // Skip stderr redirections + if (p === "2>/dev/null" || p === "2>&1") continue; + cleaned.push(p); + } + return cleaned; +} + /** * Run a git command safely using execFileSync (no shell injection). * Accepts an array of args (preferred) or a string (split on whitespace for backward compat). + * When given a string starting with "git ", the leading "git" is stripped automatically. + * Shell operators (2>/dev/null, ||, |) in strings are stripped — use shell() if you need them. * Returns stdout on success. On failure, returns a descriptive error string. */ export function run(argsOrCmd: string | string[], opts: { timeout?: number } = {}): string { - const args = typeof argsOrCmd === "string" ? argsOrCmd.split(/\s+/) : argsOrCmd; + let args: string[]; + if (typeof argsOrCmd === "string") { + const parts = argsOrCmd.split(/\s+/); + // Strip leading "git" if present (callers often pass "git diff ...") + if (parts[0] === "git") parts.shift(); + args = cleanShellArgs(parts); + } else { + args = argsOrCmd; + } try { return execFileSync("git", args, { cwd: PROJECT_DIR, @@ -29,7 +55,28 @@ export function run(argsOrCmd: string | string[], opts: { timeout?: number } = { } } -/** Convenience: run a raw command string (split on spaces). Only for simple, known-safe commands. */ +/** + * Run an arbitrary shell command string (with pipes, redirections, etc.). + * Use only for commands that genuinely need shell features. + * Returns stdout on success, or an 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 { diff --git a/src/tools/audit-workspace.ts b/src/tools/audit-workspace.ts index 1f59fb3..f9213a7 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 { 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/checkpoint.ts b/src/tools/checkpoint.ts index 53d9ace..923c6f7 100644 --- a/src/tools/checkpoint.ts +++ b/src/tools/checkpoint.ts @@ -84,11 +84,11 @@ ${dirty || "clean"} if (commitResult === "no uncommitted changes") { // Stage the checkpoint file too - run(`git add "${checkpointFile}"`); + run(["add", checkpointFile]); const result = run(`${addCmd} && git commit -m "${commitMsg.replace(/"/g, '\\"')}" 2>&1`); 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/enrich-agent-task.ts b/src/tools/enrich-agent-task.ts index 236edfa..0ce7d9a 100644 --- a/src/tools/enrich-agent-task.ts +++ b/src/tools/enrich-agent-task.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run, getDiffFiles } from "../lib/git.js"; +import { run, shell, getDiffFiles } from "../lib/git.js"; import { PROJECT_DIR } from "../lib/files.js"; import { getConfig, type RelatedProject } from "../lib/config.js"; import { existsSync, readFileSync } from "fs"; @@ -29,11 +29,11 @@ function findAreaFiles(area: string): string { // If area looks like a path, search directly if (area.includes("/")) { - return run(`git ls-files -- '${safeArea}*' 2>/dev/null | head -20`); + return shell(`git ls-files -- '${safeArea}*' 2>/dev/null | head -20`); } // Search for area keyword in git-tracked file paths - const files = run(`git ls-files 2>/dev/null | grep -i '${safeArea}' | head -20`); + const files = shell(`git ls-files 2>/dev/null | grep -i '${safeArea}' | head -20`); if (files && !files.startsWith("[command failed")) return files; // Fallback to recently changed files @@ -42,18 +42,18 @@ function findAreaFiles(area: string): string { /** 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"); + if (!area) return shell("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10"); 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 tests = shell(`git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | grep -i '${safeArea}' | head -10`); + return tests || shell("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10"); } /** 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'`); + return shell(`head -30 '${shellEscape(firstFile)}' 2>/dev/null || echo 'could not read file'`); } // --------------------------------------------------------------------------- diff --git a/src/tools/scope-work.ts b/src/tools/scope-work.ts index 9506afc..adf34f2 100644 --- a/src/tools/scope-work.ts +++ b/src/tools/scope-work.ts @@ -1,7 +1,7 @@ // CATEGORY 1: scope_work — Plans import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run, getBranch, getRecentCommits } from "../lib/git.js"; +import { run, shell, getBranch, getRecentCommits } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js"; import { searchSemantic } from "../lib/timeline-db.js"; import { getRelatedProjects } from "../lib/config.js"; @@ -93,9 +93,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+/); @@ -128,7 +128,7 @@ export function registerScopeWork(server: McpServer): void { .slice(0, 5); if (grepTerms.length > 0) { const pattern = shellEscape(grepTerms.join("|")); - matchedFiles = run(`git ls-files | head -500 | grep -iE '${pattern}' | head -30`); + matchedFiles = shell(`git ls-files | head -500 | grep -iE '${pattern}' | head -30`); } // 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 bc4f36e..6bde04e 100644 --- a/src/tools/sequence-tasks.ts +++ b/src/tools/sequence-tasks.ts @@ -1,7 +1,7 @@ // CATEGORY 6: sequence_tasks — Sequencing import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run } from "../lib/git.js"; +import { run, shell } from "../lib/git.js"; import { now } from "../lib/state.js"; import { PROJECT_DIR } from "../lib/files.js"; import { resolve } from "path"; @@ -89,7 +89,7 @@ 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 gitFiles = shell("git ls-files 2>/dev/null | head -1000"); 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/session-health.ts b/src/tools/session-health.ts index bd6a819..7f86ba1 100644 --- a/src/tools/session-health.ts +++ b/src/tools/session-health.ts @@ -1,6 +1,6 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; -import { getBranch, getStatus, getLastCommit, getLastCommitTime, run } from "../lib/git.js"; +import { getBranch, getStatus, getLastCommit, getLastCommitTime, run, shell } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs } from "../lib/files.js"; import { loadState, saveState } from "../lib/state.js"; import { getConfig } from "../lib/config.js"; @@ -27,7 +27,7 @@ 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 uncommittedDiff = shell("git diff --stat | tail -1"); // Parse commit time safely const commitDate = parseGitDate(lastCommitTimeStr); diff --git a/src/tools/sharpen-followup.ts b/src/tools/sharpen-followup.ts index db5acaa..32431e6 100644 --- a/src/tools/sharpen-followup.ts +++ b/src/tools/sharpen-followup.ts @@ -27,15 +27,15 @@ 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 argSets = [ + ["diff", "--name-only", "HEAD~1", "HEAD"], + ["diff", "--name-only", "--cached"], + ["diff", "--name-only"], ]; const results = new Set(); - for (const cmd of commands) { - const out = run(cmd); - if (out) out.split("\n").filter(Boolean).forEach((f) => results.add(f)); + for (const args of argSets) { + const out = run(args); + if (out && !out.startsWith("[")) out.split("\n").filter(Boolean).forEach((f) => results.add(f)); if (results.size > 0) break; // first successful source is enough } return [...results]; @@ -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 bb34277..ba38176 100644 --- a/src/tools/token-audit.ts +++ b/src/tools/token-audit.ts @@ -1,7 +1,7 @@ // CATEGORY 5: token_audit — Token Efficiency import { z } from "zod"; 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, PROJECT_DIR } from "../lib/files.js"; import { saveState, now, STATE_DIR } from "../lib/state.js"; import { readFileSync, existsSync, statSync } from "fs"; @@ -39,8 +39,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; @@ -63,7 +63,7 @@ export function registerTokenAudit(server: McpServer): void { 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 wc = shell(`wc -l < '${shellEscape(f)}' 2>/dev/null`); const lines = parseInt(wc) || 0; estimatedContextTokens += lines * AVG_LINE_BYTES * AVG_TOKENS_PER_BYTE; if (lines > 500) { @@ -80,7 +80,7 @@ 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 stat = shell(`wc -c < '${shellEscape("CLAUDE.md")}' 2>/dev/null`); const bytes = parseInt(stat) || 0; if (bytes > 5120) { patterns.push(`CLAUDE.md is ${(bytes / 1024).toFixed(1)}KB — injected every session, burns tokens on paste`); @@ -139,7 +139,7 @@ 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)}'`); + : shell(`tail -c ${MAX_TOOL_LOG_BYTES} '${shellEscape(toolLogPath)}'`); 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..5d9d210 100644 --- a/src/tools/verify-completion.ts +++ b/src/tools/verify-completion.ts @@ -1,6 +1,6 @@ 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 { join } from "path"; @@ -34,7 +34,7 @@ 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 pkg = JSON.parse(shell("cat package.json 2>/dev/null")); return !!pkg?.scripts?.build; } catch { return false; } } @@ -55,7 +55,7 @@ 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 tscOutput = shell(`${pm === "npx" ? "npx" : pm} tsc --noEmit 2>&1 | tail -20`); const errorLines = tscOutput.split("\n").filter(l => /error TS\d+/.test(l)); const typePassed = errorLines.length === 0; checks.push({ @@ -80,7 +80,7 @@ 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") { @@ -112,7 +112,7 @@ export function registerVerifyCompletion(server: McpServer): void { } if (testCmd) { - const testResult = run(testCmd, { timeout: 120000 }); + const testResult = shell(testCmd, { timeout: 120000 }); const testPassed = /pass/i.test(testResult) && !/fail/i.test(testResult); checks.push({ name: "Tests", @@ -130,7 +130,7 @@ 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 buildCheck = shell(`${pm === "npx" ? "npm run" : pm} build 2>&1 | tail -10`, { timeout: 60000 }); const buildPassed = !/\b[Ee]rror\b/.test(buildCheck) || /Successfully compiled/.test(buildCheck); checks.push({ name: "Build", 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); diff --git a/tests/lib/git.test.ts b/tests/lib/git.test.ts new file mode 100644 index 0000000..7b5aafd --- /dev/null +++ b/tests/lib/git.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi } from "vitest"; + +// We test the cleanShellArgs logic indirectly through run() +// by mocking execFileSync to capture the args passed to git. + +vi.mock("child_process", () => ({ + execFileSync: vi.fn(() => "mocked output"), + execSync: vi.fn(() => "mocked shell output"), +})); + +vi.mock("../../src/lib/files.js", () => ({ + PROJECT_DIR: "/tmp/test-project", +})); + +import { run, shell } from "../../src/lib/git.js"; +import { execFileSync, execSync } from "child_process"; + +const mockExecFile = vi.mocked(execFileSync); +const mockExecSync = vi.mocked(execSync); + +describe("run()", () => { + it("accepts array args directly", () => { + run(["diff", "--stat"]); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["diff", "--stat"], + expect.any(Object) + ); + }); + + it("strips leading 'git' from string commands", () => { + run("git diff --name-only"); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["diff", "--name-only"], + expect.any(Object) + ); + }); + + it("strips 2>/dev/null from string commands", () => { + run("git status --porcelain 2>/dev/null"); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["status", "--porcelain"], + expect.any(Object) + ); + }); + + it("strips 2>&1 from string commands", () => { + run("git diff --stat 2>&1"); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["diff", "--stat"], + expect.any(Object) + ); + }); + + it("stops at || shell operator", () => { + run("git diff HEAD~5 --name-only || git diff HEAD~3 --name-only"); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["diff", "HEAD~5", "--name-only"], + expect.any(Object) + ); + }); + + it("stops at | pipe operator", () => { + run("git ls-files | head -20"); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["ls-files"], + expect.any(Object) + ); + }); + + it("works without git prefix", () => { + run("diff --stat --no-color"); + expect(mockExecFile).toHaveBeenCalledWith( + "git", + ["diff", "--stat", "--no-color"], + expect.any(Object) + ); + }); +}); + +describe("shell()", () => { + it("passes full command string to execSync", () => { + shell("find tests -name '*.spec.ts' 2>/dev/null | wc -l"); + expect(mockExecSync).toHaveBeenCalledWith( + "find tests -name '*.spec.ts' 2>/dev/null | wc -l", + expect.any(Object) + ); + }); +});