Skip to content

fix: replace run() with shell() across 9 tools — closes #215 - #220

Open
TerminalGravity wants to merge 1 commit into
mainfrom
fix/run-misuse-bulk
Open

fix: replace run() with shell() across 9 tools — closes #215#220
TerminalGravity wants to merge 1 commit into
mainfrom
fix/run-misuse-bulk

Conversation

@TerminalGravity

Copy link
Copy Markdown
Collaborator

Fixes #215

All 9 affected tools now use the correct execution method:

  • shell() for commands needing pipes, redirects, or non-git binaries
  • run([...args]) with proper array args for git commands (no double-git, no shell redirects)
  • Node fs APIs where appropriate (e.g. reading package.json)

Zero new dependencies. Types clean, all 43 tests pass.

…failures

Fixes #215. The run() helper uses execFileSync('git', args) which breaks
for non-git commands, pipes, redirects, and shell operators. This commit:

- Adds shell() helper to lib/git.ts using execSync for commands needing
  shell features (pipes, redirects, non-git binaries)
- Converts git commands with 2>/dev/null to proper run() array args
- Replaces fs-readable operations (cat package.json) with Node fs APIs

Affected tools: verify-completion, clarify-intent, session-handoff,
audit-workspace, sharpen-followup, enrich-agent-task, sequence-tasks,
checkpoint, scope-work

@TerminalGravity TerminalGravity left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — this is the right fix. The shell() helper with timeout protection and graceful error returns is clean. All 9 tool migrations look correct: run() kept for pure git commands with array args, shell() for pipes/redirects.

Two small things for follow-up (not blocking):

  1. scope-work.ts input escapinggrepTerms interpolated into shell() uses shellEscape, but double-check edge cases (terms with | in them would break the grep pattern vs shell pipe ambiguity).

  2. checkpoint.ts commit message — the commitMsg.replace(/"/g, '\\"') in a shell() call is fragile for summaries with $ or backticks. Since it's a pure git command, run(['commit', '-m', commitMsg]) would be safer.

This supersedes #217 and #219 — close those once this merges. Ready to go. 🚀

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.

bug: run() misuse across 8+ tools — shell commands silently failing

1 participant