Skip to content

fix: resolve shell syntax in run() calls across all tools#116

Closed
TerminalGravity wants to merge 2 commits intomainfrom
fix/shell-syntax-110-v3
Closed

fix: resolve shell syntax in run() calls across all tools#116
TerminalGravity wants to merge 2 commits intomainfrom
fix/shell-syntax-110-v3

Conversation

@TerminalGravity
Copy link
Collaborator

Fixes #110

What

Adds a shell() helper to src/lib/git.ts for commands that genuinely need shell features (pipes, redirects, ||), and fixes all 13 affected tool files to use the correct execution method:

  • Simple git commandsrun() with array args (no shell, safe from injection)
  • Commands with pipes/redirectsshell() (explicit shell use)
  • File operations (cat, wc -l, wc -c) → native Node.js fs calls

Files changed (13)

git.ts, verify-completion.ts, token-audit.ts, audit-workspace.ts, enrich-agent-task.ts, clarify-intent.ts, session-handoff.ts, scope-work.ts, sequence-tasks.ts, session-health.ts, sharpen-followup.ts, checkpoint.ts, what-changed.ts

Testing

  • tsc --noEmit passes clean
  • Follows Option 3 from the issue (array args where simple, shell() for complex piping)

Covers LanceDB native binary failures, CLAUDE_PROJECT_DIR config,
vector search not returning results, MCP handshake debugging,
and performance tips. Links from README nav bar.
Add shell() helper to git.ts for commands that genuinely need shell
features (pipes, redirects, ||). Convert simple git commands to use
run() with array args. Replace shell-based file operations (cat, wc)
with native Node.js fs calls where possible.

Files fixed: verify-completion, token-audit, audit-workspace,
enrich-agent-task, clarify-intent, session-handoff, scope-work,
sequence-tasks, session-health, sharpen-followup, checkpoint,
what-changed (13 files total).

Closes #110
@TerminalGravity
Copy link
Collaborator Author

Reviewed — this is the cleanest version of the #110 fix. The three-tier approach (array args for simple git, shell() for pipes/redirects, fs for file reads) is solid.

Two things to watch:

  1. Timeout: shell() defaults to 10s — might be tight for tsc --noEmit on larger projects. The verify-completion calls should probably pass { timeout: 30000 } explicitly.
  2. PR cleanup: This supersedes fix: shell syntax bugs in what-changed, checkpoint, session-health #109, fix: resolve all remaining shell syntax in run() calls #111, fix: shell syntax in run() calls (fixes #110) #112, fix(tools): replace shell syntax in run() calls with shell()/exec()/array args #113, fix: resolve shell syntax in run() calls after execFileSync migration #115, fix: replace shell syntax in run() calls with shellRun() and Node APIs #100, fix: replace remaining run() calls using shell syntax with shellRun() #101 — let's close those once this lands.

Ready to merge.

@TerminalGravity
Copy link
Collaborator Author

Superseded by #119 and #120 which both have broader coverage. Closing in favor of those — #120 in particular covers the most files. Thanks for the initial pass though, the array-args approach here informed the later PRs.

@TerminalGravity
Copy link
Collaborator Author

Closing — superseded by #120 which covers more files with the same approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Widespread shell syntax in run() calls after execFileSync migration

1 participant