Skip to content

Commit 3db42ca

Browse files
TerminalGravityTerminalGravity
committed
fix: replace shell syntax in run() calls across 8 tools (#172)
run() uses execFileSync('git', args) — no shell interpretation. Many tools were passing shell operators (pipes, redirects, 2>/dev/null) and non-git commands (cat, wc, find, npm, npx, tsc, gh, tail) through run(), which silently broke. Changes: - Add shell() helper to git.ts for commands needing shell interpretation - verify-completion: use shell() for tsc/npm/build, readFileSync for pkg - token-audit: use Node fs for wc -l/wc -c, shell() for tail, array args for git - session-handoff: use shell() for command -v and gh pr list - audit-workspace: use shell() for find|wc, array args for git diff - sharpen-followup: use array args for git status - scope-work: use array args for git status/diff, JS filtering for ls-files - enrich-agent-task: rewrite findAreaFiles/findRelatedTests/getExamplePattern to use array args + JS filtering instead of shell pipes - sequence-tasks: use array args for git ls-files + JS slicing Closes #172
1 parent 4637eaf commit 3db42ca

10 files changed

Lines changed: 75 additions & 41 deletions

‎package-lock.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/lib/git.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,31 @@
1-
import { execFileSync } from "child_process";
1+
import { execFileSync, execSync } from "child_process";
22
import { PROJECT_DIR } from "./files.js";
33
import type { RunError } from "../types.js";
44

5+
/**
6+
* Run an arbitrary shell command string (with shell interpretation).
7+
* Use this for non-git commands that need pipes, redirects, etc.
8+
* Returns stdout on success, descriptive error string on failure.
9+
*/
10+
export function shell(cmd: string, opts: { timeout?: number; cwd?: string } = {}): string {
11+
try {
12+
return execSync(cmd, {
13+
cwd: opts.cwd || PROJECT_DIR,
14+
encoding: "utf-8",
15+
timeout: opts.timeout || 10000,
16+
maxBuffer: 1024 * 1024,
17+
stdio: ["pipe", "pipe", "pipe"],
18+
}).trim();
19+
} catch (e: any) {
20+
if (e.killed === true || e.signal === "SIGTERM") {
21+
return `[timed out after ${opts.timeout || 10000}ms]`;
22+
}
23+
const output = e.stdout?.trim() || e.stderr?.trim();
24+
if (output) return output;
25+
return `[command failed: ${cmd} (exit ${e.status ?? "?"})]`;
26+
}
27+
}
28+
529
/**
630
* Run a git command safely using execFileSync (no shell injection).
731
* Accepts an array of args (preferred) or a string (split on whitespace for backward compat).

‎src/tools/audit-workspace.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
2-
import { run } from "../lib/git.js";
2+
import { run, shell } from "../lib/git.js";
33
import { readIfExists, findWorkspaceDocs } from "../lib/files.js";
44

55
/** Extract top-level work areas from file paths generically */
@@ -36,7 +36,7 @@ export function registerAuditWorkspace(server: McpServer): void {
3636
{},
3737
async () => {
3838
const docs = findWorkspaceDocs();
39-
const recentFiles = run("git diff --name-only HEAD~10 2>/dev/null || echo ''").split("\n").filter(Boolean);
39+
const recentFiles = run(["diff", "--name-only", "HEAD~10"]).split("\n").filter(Boolean);
4040
const sections: string[] = [];
4141

4242
// Doc freshness
@@ -75,7 +75,7 @@ export function registerAuditWorkspace(server: McpServer): void {
7575
// Check for gap trackers or similar tracking docs
7676
const trackingDocs = Object.entries(docs).filter(([n]) => /gap|track|progress/i.test(n));
7777
if (trackingDocs.length > 0) {
78-
const testFilesCount = parseInt(run("find tests -name '*.spec.ts' -o -name '*.test.ts' 2>/dev/null | wc -l").trim()) || 0;
78+
const testFilesCount = parseInt(shell("find tests -name '*.spec.ts' -o -name '*.test.ts' 2>/dev/null | wc -l").trim()) || 0;
7979
sections.push(`## Tracking Docs\n${trackingDocs.map(([n]) => {
8080
const age = docStatus.find(d => d.name === n)?.ageHours ?? "?";
8181
return `- .claude/${n} — last updated ${age}h ago`;

‎src/tools/enrich-agent-task.ts‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,35 +25,40 @@ function detectPackageManager(): string {
2525
function findAreaFiles(area: string): string {
2626
if (!area) return getDiffFiles("HEAD~3");
2727

28-
const safeArea = shellEscape(area);
28+
const allFiles = run(["ls-files"]).split("\n").filter(Boolean);
2929

30-
// If area looks like a path, search directly
30+
// If area looks like a path, search by prefix
3131
if (area.includes("/")) {
32-
return run(`git ls-files -- '${safeArea}*' 2>/dev/null | head -20`);
32+
const safeArea = shellEscape(area);
33+
return allFiles.filter(f => f.startsWith(safeArea)).slice(0, 20).join("\n") || getDiffFiles("HEAD~3");
3334
}
3435

3536
// Search for area keyword in git-tracked file paths
36-
const files = run(`git ls-files 2>/dev/null | grep -i '${safeArea}' | head -20`);
37-
if (files && !files.startsWith("[command failed")) return files;
38-
39-
// Fallback to recently changed files
40-
return getDiffFiles("HEAD~3");
37+
const safeArea = shellEscape(area).toLowerCase();
38+
const matched = allFiles.filter(f => f.toLowerCase().includes(safeArea)).slice(0, 20);
39+
return matched.length > 0 ? matched.join("\n") : getDiffFiles("HEAD~3");
4140
}
4241

4342
/** Find related test files for an area */
4443
function findRelatedTests(area: string): string {
45-
if (!area) return run("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10");
44+
const allFiles = run(["ls-files"]).split("\n").filter(Boolean);
45+
const testFiles = allFiles.filter(f => /\.(spec|test)\.(ts|tsx|js|jsx)$/.test(f));
46+
47+
if (!area) return testFiles.slice(0, 10).join("\n");
4648

47-
const safeArea = shellEscape(area.split(/\s+/)[0]);
48-
const tests = run(`git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | grep -i '${safeArea}' | head -10`);
49-
return tests || run("git ls-files 2>/dev/null | grep -E '\\.(spec|test)\\.(ts|tsx|js|jsx)$' | head -10");
49+
const safeArea = shellEscape(area.split(/\s+/)[0]).toLowerCase();
50+
const matched = testFiles.filter(f => f.toLowerCase().includes(safeArea)).slice(0, 10);
51+
return matched.length > 0 ? matched.join("\n") : testFiles.slice(0, 10).join("\n");
5052
}
5153

5254
/** Get an example pattern from the first matching file */
5355
function getExamplePattern(files: string): string {
5456
const firstFile = files.split("\n").filter(Boolean)[0];
5557
if (!firstFile) return "no pattern available";
56-
return run(`head -30 '${shellEscape(firstFile)}' 2>/dev/null || echo 'could not read file'`);
58+
try {
59+
const content = readFileSync(join(PROJECT_DIR, firstFile), "utf-8");
60+
return content.split("\n").slice(0, 30).join("\n");
61+
} catch { return "could not read file"; }
5762
}
5863

5964
// ---------------------------------------------------------------------------

‎src/tools/scope-work.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ export function registerScopeWork(server: McpServer): void {
9393
const timestamp = now();
9494
const currentBranch = branch ?? getBranch();
9595
const recentCommits = getRecentCommits(10);
96-
const porcelain = run("git status --porcelain");
96+
const porcelain = run(["status", "--porcelain"]);
9797
const dirtyFiles = parsePortelainFiles(porcelain);
98-
const diffStat = dirtyFiles.length > 0 ? run("git diff --stat") : "(clean working tree)";
98+
const diffStat = dirtyFiles.length > 0 ? run(["diff", "--stat"]) : "(clean working tree)";
9999

100100
// Scan for relevant files based on task keywords
101101
const keywords = task.toLowerCase().split(/\s+/);
@@ -128,7 +128,9 @@ export function registerScopeWork(server: McpServer): void {
128128
.slice(0, 5);
129129
if (grepTerms.length > 0) {
130130
const pattern = shellEscape(grepTerms.join("|"));
131-
matchedFiles = run(`git ls-files | head -500 | grep -iE '${pattern}' | head -30`);
131+
const allFiles = run(["ls-files"]).split("\n").slice(0, 500);
132+
const regex = new RegExp(grepTerms.join("|"), "i");
133+
matchedFiles = allFiles.filter(f => regex.test(f)).slice(0, 30).join("\n");
132134
}
133135

134136
// Check which relevant dirs actually exist (with path traversal protection)

‎src/tools/sequence-tasks.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export function registerSequenceTasks(server: McpServer): void {
9090
// For locality: infer directories from path-like tokens in task text
9191
if (strategy === "locality") {
9292
// Use git ls-files with a depth limit instead of find for performance
93-
const gitFiles = run("git ls-files 2>/dev/null | head -1000");
93+
const gitFiles = run(["ls-files"]).split("\n").slice(0, 1000).join("\n");
9494
const knownDirs = new Set<string>();
9595
for (const f of gitFiles.split("\n").filter(Boolean)) {
9696
const parts = f.split("/");

‎src/tools/session-handoff.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ import { z } from "zod";
22
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
33
import { existsSync, readFileSync } from "fs";
44
import { join } from "path";
5-
import { run, getBranch, getRecentCommits, getStatus } from "../lib/git.js";
5+
import { run, shell, getBranch, getRecentCommits, getStatus } from "../lib/git.js";
66
import { readIfExists, findWorkspaceDocs } from "../lib/files.js";
77
import { STATE_DIR, now } from "../lib/state.js";
88

99
/** Check if a CLI tool is available */
1010
function hasCommand(cmd: string): boolean {
11-
const result = run(`command -v ${cmd} 2>/dev/null`);
11+
const result = shell(`command -v ${cmd} 2>/dev/null`);
1212
return !!result && !result.startsWith("[command failed");
1313
}
1414

@@ -44,7 +44,7 @@ export function registerSessionHandoff(server: McpServer): void {
4444

4545
// Only try gh if it exists
4646
if (hasCommand("gh")) {
47-
const openPRs = run("gh pr list --state open --json number,title,headRefName 2>/dev/null || echo '[]'");
47+
const openPRs = shell("gh pr list --state open --json number,title,headRefName 2>/dev/null || echo '[]'");
4848
if (openPRs && openPRs !== "[]") {
4949
sections.push(`## Open PRs\n\`\`\`json\n${openPRs}\n\`\`\``);
5050
}

‎src/tools/sharpen-followup.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export function registerSharpenFollowup(server: McpServer): void {
8787
// Gather context to resolve ambiguity
8888
const contextFiles: string[] = [...(previous_files ?? [])];
8989
const recentChanged = getRecentChangedFiles();
90-
const porcelainOutput = run("git status --porcelain 2>/dev/null");
90+
const porcelainOutput = run(["status", "--porcelain"]);
9191
const untrackedOrModified = parsePortelainFiles(porcelainOutput);
9292

9393
const allKnownFiles = [...new Set([...contextFiles, ...recentChanged, ...untrackedOrModified])].filter(Boolean);

‎src/tools/token-audit.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// CATEGORY 5: token_audit — Token Efficiency
22
import { z } from "zod";
33
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
4-
import { run } from "../lib/git.js";
4+
import { run, shell } from "../lib/git.js";
55
import { readIfExists, findWorkspaceDocs, PROJECT_DIR } from "../lib/files.js";
66
import { loadState, saveState, now, STATE_DIR } from "../lib/state.js";
77
import { readFileSync, existsSync, statSync } from "fs";
@@ -39,8 +39,8 @@ export function registerTokenAudit(server: McpServer): void {
3939
let wasteScore = 0;
4040

4141
// 1. Git diff size & dirty file count
42-
const diffStat = run("git diff --stat --no-color 2>/dev/null");
43-
const dirtyFiles = run("git diff --name-only 2>/dev/null");
42+
const diffStat = run(["diff", "--stat", "--no-color"]);
43+
const dirtyFiles = run(["diff", "--name-only"]);
4444
const dirtyList = dirtyFiles.split("\n").filter(Boolean);
4545
const dirtyCount = dirtyList.length;
4646

@@ -63,8 +63,11 @@ export function registerTokenAudit(server: McpServer): void {
6363

6464
for (const f of dirtyList.slice(0, 30)) {
6565
// Use shell-safe quoting instead of interpolation
66-
const wc = run(`wc -l < '${shellEscape(f)}' 2>/dev/null`);
67-
const lines = parseInt(wc) || 0;
66+
let lines = 0;
67+
try {
68+
const content = readFileSync(join(PROJECT_DIR, f), "utf-8");
69+
lines = content.split("\n").length;
70+
} catch { /* file unreadable */ }
6871
estimatedContextTokens += lines * AVG_LINE_BYTES * AVG_TOKENS_PER_BYTE;
6972
if (lines > 500) {
7073
largeFiles.push(`${f} (${lines} lines)`);
@@ -80,8 +83,8 @@ export function registerTokenAudit(server: McpServer): void {
8083
// 3. CLAUDE.md bloat check
8184
const claudeMd = readIfExists("CLAUDE.md", 1);
8285
if (claudeMd !== null) {
83-
const stat = run(`wc -c < '${shellEscape("CLAUDE.md")}' 2>/dev/null`);
84-
const bytes = parseInt(stat) || 0;
86+
let bytes = 0;
87+
try { bytes = statSync(join(PROJECT_DIR, "CLAUDE.md")).size; } catch { /* */ }
8588
if (bytes > 5120) {
8689
patterns.push(`CLAUDE.md is ${(bytes / 1024).toFixed(1)}KB — injected every session, burns tokens on paste`);
8790
recommendations.push("Trim CLAUDE.md to essentials (<5KB). Move reference docs to files read on-demand");
@@ -139,7 +142,7 @@ export function registerTokenAudit(server: McpServer): void {
139142
// Read with size cap: take the tail if too large
140143
const raw = stat.size <= MAX_TOOL_LOG_BYTES
141144
? readFileSync(toolLogPath, "utf-8")
142-
: run(`tail -c ${MAX_TOOL_LOG_BYTES} '${shellEscape(toolLogPath)}'`);
145+
: shell(`tail -c ${MAX_TOOL_LOG_BYTES} '${shellEscape(toolLogPath)}'`);
143146

144147
const lines = raw.trim().split("\n").filter(Boolean);
145148
totalToolCalls = lines.length;

‎src/tools/verify-completion.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { z } from "zod";
22
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
3-
import { run, getStatus } from "../lib/git.js";
3+
import { run, shell, getStatus } from "../lib/git.js";
44
import { PROJECT_DIR } from "../lib/files.js";
5-
import { existsSync } from "fs";
5+
import { existsSync, readFileSync } from "fs";
66
import { join } from "path";
77

88
/** Detect package manager from lockfiles */
@@ -34,7 +34,7 @@ function detectTestRunner(): string | null {
3434
/** Check if a build script exists in package.json */
3535
function hasBuildScript(): boolean {
3636
try {
37-
const pkg = JSON.parse(run("cat package.json 2>/dev/null"));
37+
const pkg = JSON.parse(readFileSync(join(PROJECT_DIR, "package.json"), "utf-8"));
3838
return !!pkg?.scripts?.build;
3939
} catch { return false; }
4040
}
@@ -55,7 +55,7 @@ export function registerVerifyCompletion(server: McpServer): void {
5555
const checks: { name: string; passed: boolean; detail: string }[] = [];
5656

5757
// 1. Type check (single invocation, extract both result and count)
58-
const tscOutput = run(`${pm === "npx" ? "npx" : pm} tsc --noEmit 2>&1 | tail -20`);
58+
const tscOutput = shell(`${pm === "npx" ? "npx" : pm} tsc --noEmit 2>&1 | tail -20`);
5959
const errorLines = tscOutput.split("\n").filter(l => /error TS\d+/.test(l));
6060
const typePassed = errorLines.length === 0;
6161
checks.push({
@@ -80,7 +80,7 @@ export function registerVerifyCompletion(server: McpServer): void {
8080
// 3. Tests
8181
if (!skip_tests) {
8282
const runner = detectTestRunner();
83-
const changedFiles = run("git diff --name-only HEAD~1 2>/dev/null").split("\n").filter(Boolean);
83+
const changedFiles = run(["diff", "--name-only", "HEAD~1"]).split("\n").filter(Boolean);
8484
let testCmd = "";
8585

8686
if (runner === "playwright") {
@@ -112,7 +112,7 @@ export function registerVerifyCompletion(server: McpServer): void {
112112
}
113113

114114
if (testCmd) {
115-
const testResult = run(testCmd, { timeout: 120000 });
115+
const testResult = shell(testCmd, { timeout: 120000 });
116116
const testPassed = /pass/i.test(testResult) && !/fail/i.test(testResult);
117117
checks.push({
118118
name: "Tests",
@@ -130,7 +130,7 @@ export function registerVerifyCompletion(server: McpServer): void {
130130

131131
// 4. Build check (only if build script exists and not skipped)
132132
if (!skip_build && hasBuildScript()) {
133-
const buildCheck = run(`${pm === "npx" ? "npm run" : pm} build 2>&1 | tail -10`, { timeout: 60000 });
133+
const buildCheck = shell(`${pm === "npx" ? "npm run" : pm} build 2>&1 | tail -10`, { timeout: 60000 });
134134
const buildPassed = !/\b[Ee]rror\b/.test(buildCheck) || /Successfully compiled/.test(buildCheck);
135135
checks.push({
136136
name: "Build",

0 commit comments

Comments
 (0)