Skip to content

Widespread shell syntax in run() calls after execFileSync migration #110

Closed

Description

@TerminalGravity

Problem

run() in src/lib/git.ts uses execFileSync (no shell), but many tool files still pass shell syntax that silently breaks:

  • Shell operators: ||, &&, |
  • Redirects: 2>/dev/null, 2>&1
  • Prefixing git in strings (becomes a literal arg since execFileSync('git', ['git', ...]))
  • Shell quoting/escaping

Affected files

PR #109 fixes 3 files. These still need fixing:

  • verify-completion.ts — cat, tsc --noEmit 2>&1 | tail, npm run build 2>&1 | tail
  • token-audit.ts — wc -l, wc -c, tail -c, 2>/dev/null
  • clarify-intent.ts — tsc --noEmit 2>&1 | grep, find ... | head
  • session-handoff.ts — command -v, gh pr list ... || echo
  • audit-workspace.ts — find ... | wc -l, 2>/dev/null
  • enrich-agent-task.ts — git ls-files | grep, head -30
  • scope-work.ts — git ls-files | head | grep
  • sequence-tasks.ts — git ls-files | head

Options

  1. Fix each caller to use array args + JS logic (like PR fix: shell syntax bugs in what-changed, checkpoint, session-health #109)
  2. Add a shell() helper that uses execSync for commands that genuinely need shell features
  3. Both — array args where simple, shell() for complex piping

Option 3 is probably best. Some commands (find | grep | head) are genuinely easier with shell.

Activity

  1. added 3 commits that reference this issue on Mar 5, 2026
    690a220
    a9575b5
    c5e4e8b
  2. TerminalGravity commented on Mar 5, 2026

    @TerminalGravity
    CollaboratorAuthor

    PR #113 covers all the files listed here with a clean three-tier approach: array args for simple git commands, shell() for genuine pipe/redirect needs, and exec() for non-git binaries. Once that lands, the older fix PRs (#109, #111, #112, #101, #100) can be closed.

    One thing to watch: the shell() helper defaults to a 10s timeout which might be tight for type-checking larger projects. Easy fix if it comes up.

  3. added 5 commits that reference this issue on Mar 5, 2026
    498aa1b
    04c5b4b
    cd113cf
    d57447c
    01a62d4
  4. TerminalGravity commented on Mar 5, 2026

    @TerminalGravity
    CollaboratorAuthor

    Quick status check on the competing fix PRs:

    Recommendation: merge #120 (once CI passes) since it builds on #109's approach and the JS replacements are solid. Then close #116 and #119 as superseded. The Option 3 approach (array args where simple, shell helper for complex piping) is the right call — some of those find | grep | head chains really are easier with shell.

  5. added 6 commits that reference this issue on Mar 5, 2026
    5594e3d
    4085690
    bbda30a
    8901479
    350b566
    b0c30ea
  6. TerminalGravity commented on Mar 5, 2026

    @TerminalGravity
    CollaboratorAuthor

    All files listed here are already using the shell() helper from src/lib/shell.ts. No remaining run() calls with shell syntax. This was resolved alongside PR #109.

  7. added a commit that references this issue on Mar 5, 2026
    c7297f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions