From 9b69150bcd2bdc82c6de02e327c1d6eb107e7846 Mon Sep 17 00:00:00 2001 From: Jack Felke Date: Fri, 20 Mar 2026 07:53:49 -0700 Subject: [PATCH] fix: remove shell syntax from run() calls in 4 tools (#302) Convert shell strings to proper array args for execFileSync: - checkpoint.ts: inline staging logic, use array args for add/commit/reset - sharpen-followup.ts: convert diff commands to arrays, remove 2>/dev/null - sequence-tasks.ts: replace piped git ls-files|head with array + slice - session-handoff.ts: replace command -v with which, gh pr list with execFileSync --- src/tools/checkpoint.ts | 35 +++++++++++++++-------------------- src/tools/sequence-tasks.ts | 3 ++- src/tools/session-handoff.ts | 20 +++++++++++++++++--- src/tools/sharpen-followup.ts | 14 +++++++------- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/tools/checkpoint.ts b/src/tools/checkpoint.ts index e086f01..ebc5a0c 100644 --- a/src/tools/checkpoint.ts +++ b/src/tools/checkpoint.ts @@ -63,32 +63,27 @@ ${dirty || "clean"} const shortSummary = summary.split("\n")[0].slice(0, 72); const commitMsg = `checkpoint: ${shortSummary}`; - let addCmd: string; - switch (mode) { - case "staged": { - const staged = getStagedFiles(); - if (!staged) { - commitResult = "nothing staged — skipped commit (use 'tracked' or 'all' mode, or stage files first)"; - } - addCmd = "true"; // noop, already staged - break; + if (mode === "staged") { + const staged = getStagedFiles(); + if (!staged) { + commitResult = "nothing staged — skipped commit (use 'tracked' or 'all' mode, or stage files first)"; } - case "all": - addCmd = "git add -A"; - break; - case "tracked": - default: - addCmd = "git add -u"; - break; } 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 based on mode + if (mode === "all") { + run(["add", "-A"]); + } else if (mode === "tracked") { + run(["add", "-u"]); + } + // staged mode: nothing extra to add + const result = run(["commit", "-m", commitMsg]); + if (result.includes("command 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/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..507cbd3 100644 --- a/src/tools/session-handoff.ts +++ b/src/tools/session-handoff.ts @@ -2,14 +2,19 @@ import { z } from "zod"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { existsSync, readFileSync } from "fs"; import { join } from "path"; +import { execFileSync } from "child_process"; import { run, 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`); - 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,7 +49,16 @@ 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 '[]'"); + let openPRs = "[]"; + try { + openPRs = execFileSync("gh", ["pr", "list", "--state", "open", "--json", "number,title,headRefName"], { + encoding: "utf-8", + timeout: 10000, + stdio: ["pipe", "pipe", "pipe"], + }).trim(); + } catch { + // gh not authenticated or no remote — skip + } 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..9729719 100644 --- a/src/tools/sharpen-followup.ts +++ b/src/tools/sharpen-followup.ts @@ -27,14 +27,14 @@ 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) { - const out = run(cmd); + for (const args of commands) { + const out = run(args); if (out) out.split("\n").filter(Boolean).forEach((f) => results.add(f)); if (results.size > 0) break; // first successful source is enough } @@ -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);