Skip to content

fix: replace broken shell syntax in run() calls with array args#136

Open
TerminalGravity wants to merge 1 commit intomainfrom
fix/what-changed-shell-args
Open

fix: replace broken shell syntax in run() calls with array args#136
TerminalGravity wants to merge 1 commit intomainfrom
fix/what-changed-shell-args

Conversation

@TerminalGravity
Copy link
Collaborator

Problem

run() in lib/git.ts uses execFileSync (no shell), but 5 tools were passing shell operators (2>/dev/null, ||, |, &&) as string args. These got split on whitespace and passed as literal git arguments, silently breaking fallback/pipe logic.

For example, what-changed.ts had:

run(`git diff ${ref} --name-only 2>/dev/null || git diff HEAD~3 --name-only`)

This became execFileSync('git', ['diff', ref, '--name-only', '2>/dev/null', '||', 'git', 'diff', 'HEAD~3', '--name-only']) — git would error or silently ignore the junk args.

Fix

Replaced all broken calls with proper array args and explicit JS fallback logic:

  • what-changed.ts — diff/log fallbacks
  • session-health.tsdiff --stat | tail -1 → array + JS .pop()
  • audit-workspace.ts — diff fallback + replaced find | wc with git ls-files
  • sharpen-followup.ts — 3 diff commands + status
  • checkpoint.ts — add/commit/reset chain

All 43 tests pass. Build clean.

run() uses execFileSync (no shell) but several tools passed shell
operators (2>/dev/null, ||, |, &&) as string args that got split
into literal git arguments, silently breaking fallback logic.

Fixed in: what-changed, session-health, audit-workspace,
sharpen-followup, checkpoint
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.

Clean fix. The startsWith("[") check for error detection is a bit fragile though — if run() ever returns JSON or a file starting with [, it would false-positive. Worth adding a typed error return or throwing from run() so callers don't need to sniff output strings. But that's a follow-up, not a blocker. Ship it.

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.

1 participant