From 3b5a61fdc8298e033ea785388e4fd5fa842bf09c Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Thu, 19 Mar 2026 20:14:44 -0700 Subject: [PATCH 1/2] fix(what-changed): use array args instead of shell syntax in run() calls run() uses execFileSync without a shell, so shell operators like 2>/dev/null and || were being passed as literal git arguments, causing silent failures. Replace with proper array args and existing git.ts helpers (getDiffFiles) with built-in fallback logic. --- src/tools/what-changed.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tools/what-changed.ts b/src/tools/what-changed.ts index 913dfa2..27eea20 100644 --- a/src/tools/what-changed.ts +++ b/src/tools/what-changed.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run, getBranch, getDiffStat } from "../lib/git.js"; +import { run, getBranch, getDiffFiles, getDiffStat } from "../lib/git.js"; export function registerWhatChanged(server: McpServer): void { server.tool( @@ -12,8 +12,9 @@ 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`); + const diffFiles = getDiffFiles(ref); + const logResult = run(["log", `${ref}..HEAD`, "--oneline"]); + const log = logResult.startsWith("[") ? run(["log", "-5", "--oneline"]) : logResult; const branch = getBranch(); const fileList = diffFiles.split("\n").filter(Boolean); From 69abfe412fbe45ad1dd37e2b160ca56e9c6d802d Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Thu, 19 Mar 2026 20:44:52 -0700 Subject: [PATCH 2/2] fix(#302): remove shell syntax from run() calls in 4 tools Convert sharpen-followup, checkpoint, session-handoff, and sequence-tasks to use array args with run() or execFileSync directly for non-git commands. Eliminates silent failures from shell operators (2>/dev/null, pipes, ||) being passed as literal args to execFileSync. Part of #302. --- src/tools/checkpoint.ts | 14 ++++++++++---- src/tools/sequence-tasks.ts | 3 ++- src/tools/session-handoff.ts | 23 ++++++++++++++++++----- src/tools/sharpen-followup.ts | 33 +++++++++++++++++++-------------- 4 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/tools/checkpoint.ts b/src/tools/checkpoint.ts index e086f01..e3b380b 100644 --- a/src/tools/checkpoint.ts +++ b/src/tools/checkpoint.ts @@ -84,11 +84,17 @@ ${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`); - if (result.includes("commit failed") || result.includes("nothing to commit")) { + run(["add", checkpointFile]); + // Stage files based on mode + if (mode === "tracked") { + run(["add", "-u"]); + } else if (mode === "all") { + run(["add", "-A"]); + } + const result = run(["commit", "-m", commitMsg]); + if (result.includes("nothing to commit") || result.startsWith("[command failed")) { // 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/sequence-tasks.ts b/src/tools/sequence-tasks.ts index 22dea23..5f76840 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 allGitFiles = run(["ls-files"]); + const gitFiles = allGitFiles.startsWith("[") ? "" : allGitFiles.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..789ae45 100644 --- a/src/tools/session-handoff.ts +++ b/src/tools/session-handoff.ts @@ -5,11 +5,16 @@ import { join } from "path"; import { run, getBranch, getRecentCommits, getStatus } from "../lib/git.js"; import { readIfExists, findWorkspaceDocs } from "../lib/files.js"; import { STATE_DIR, now } from "../lib/state.js"; +import { execFileSync } from "child_process"; /** Check if a CLI tool is available */ function hasCommand(cmd: string): boolean { - const result = run(`command -v ${cmd} 2>/dev/null`); - return !!result && !result.startsWith("[command failed"); + try { + execFileSync("which", [cmd], { encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }); + return true; + } catch { + return false; + } } export function registerSessionHandoff(server: McpServer): void { @@ -44,9 +49,17 @@ 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 '[]'"); - if (openPRs && openPRs !== "[]") { - sections.push(`## Open PRs\n\`\`\`json\n${openPRs}\n\`\`\``); + try { + const openPRs = execFileSync("gh", ["pr", "list", "--state", "open", "--json", "number,title,headRefName"], { + encoding: "utf-8", + timeout: 10000, + stdio: ["pipe", "pipe", "pipe"], + }).trim(); + if (openPRs && openPRs !== "[]") { + sections.push(`## Open PRs\n\`\`\`json\n${openPRs}\n\`\`\``); + } + } catch { + // gh pr list failed, skip } } diff --git a/src/tools/sharpen-followup.ts b/src/tools/sharpen-followup.ts index db5acaa..de803f9 100644 --- a/src/tools/sharpen-followup.ts +++ b/src/tools/sharpen-followup.ts @@ -1,7 +1,7 @@ // CATEGORY 4: sharpen_followup — Follow-up Specificity import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { run } from "../lib/git.js"; +import { run, getDiffFiles, getStagedFiles } from "../lib/git.js"; import { now } from "../lib/state.js"; /** Parse git porcelain output into deduplicated file paths, handling renames (R/C) */ @@ -26,19 +26,24 @@ 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 results = new Set(); - for (const cmd of commands) { - const out = run(cmd); - if (out) out.split("\n").filter(Boolean).forEach((f) => results.add(f)); - if (results.size > 0) break; // first successful source is enough + // Try HEAD~1..HEAD first + const committed = run(["diff", "--name-only", "HEAD~1", "HEAD"]); + if (committed && !committed.startsWith("[")) { + const files = committed.split("\n").filter(Boolean); + if (files.length > 0) return files; } - return [...results]; + // Fall back to staged + const staged = getStagedFiles(); + if (staged && !staged.startsWith("[")) { + const files = staged.split("\n").filter(Boolean); + if (files.length > 0) return files; + } + // Fall back to unstaged + const unstaged = run(["diff", "--name-only"]); + if (unstaged && !unstaged.startsWith("[")) { + return unstaged.split("\n").filter(Boolean); + } + return []; } export function registerSharpenFollowup(server: McpServer): void { @@ -87,7 +92,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);