Skip to content

fix(tools): replace shell syntax in run() calls with shell()/exec()/array args#113

Closed
TerminalGravity wants to merge 1 commit intomainfrom
fix/shell-syntax-issue-110-batch2
Closed

fix(tools): replace shell syntax in run() calls with shell()/exec()/array args#113
TerminalGravity wants to merge 1 commit intomainfrom
fix/shell-syntax-issue-110-batch2

Conversation

@TerminalGravity
Copy link
Collaborator

Fixes #110. Converts all tool files that were passing shell syntax (pipes, redirects, ||, &&) to run() (which uses execFileSync without a shell) to use the appropriate helper:

  • shell() for commands needing pipes, redirects, || fallbacks
  • run([...args]) array syntax for simple git commands (removing unnecessary 2>/dev/null since run() already captures stderr)
  • readFileSync where we were shelling out to cat

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

Copy link
Collaborator Author

@TerminalGravity TerminalGravity left a comment

Choose a reason for hiding this comment

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

Solid approach — the three-tier strategy (array args for simple git, shell() for pipes/redirects, exec() for non-git binaries) is clean. The security comment on shell() is appreciated.

A couple notes:

  • The shellEscape() mention in the JSDoc — is that implemented somewhere? Didn't see it in the diff. Might want to either add it or remove the reference to avoid confusion.
  • Timeout default of 10s in shell() seems tight for pnpm tsc --noEmit on larger projects. Worth bumping or making configurable per-call.

This supersedes #109, #111, #112, #101, #100 — those should be closed once this lands. 🚀

@TerminalGravity
Copy link
Collaborator Author

Closing — superseded by #116 which covers all files with the cleaner three-tier 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