fix: replace shell syntax in run() calls across 8 tools#175
fix: replace shell syntax in run() calls across 8 tools#175TerminalGravity wants to merge 1 commit intomainfrom
Conversation
|
Reviewed — this looks good to merge. The Two things to check before merging:
This supersedes #173, #171, #170, and #165 — close those once this lands. |
TerminalGravity
force-pushed
the
fix/shell-syntax-remaining-tools
branch
from
3db42ca to
0cec6fe
Compare
TerminalGravity
left a comment
TerminalGravity
left a comment
There was a problem hiding this comment.
Solid fix — the approach is clean. Array args for run(), Node.js fs for file ops, execFileSync for the stuff that genuinely needs a shell. A few things:
-
This branch includes the examples/.preflight/ files from #174. Looks like the branch wasn't rebased cleanly — the config example commits ended up here too. Should rebase off main (after #174 merges) or cherry-pick just the fix commits.
-
The shellRun helper in verify-completion.ts is good, but it's local to that file. Since session-handoff.ts also does inline execFileSync('gh', ...), might be worth pulling shellRun into lib/git.ts (or a new lib/shell.ts) so there's one pattern for "needs a real shell" calls. Not blocking, but worth considering before this grows.
-
In audit-workspace.ts, the recursive countTests function is fine for now, but consider globSync from node:fs if you're on Node 22+ — cleaner than manual recursion.
The actual fixes look correct. The run([...]) array args pattern is right, the error handling is sensible. Just clean up the branch overlap with #174 and this is good to merge.
Replace shell pipes, redirects, and non-git commands that were passed
as literal args to execFileSync('git', args):
- verify-completion: use fs.readFileSync for package.json, execFileSync
for tsc/test/build commands instead of run() with pipes
- token-audit: use fs for line counting and file size instead of wc/tail
- session-handoff: use 'which' + execFileSync('gh') instead of shell
- audit-workspace: use fs.readdirSync for test file counting
- sharpen-followup: use array args for git diff/status
- scope-work: use JS regex filter instead of shell grep pipes
- enrich-agent-task: use JS filter/slice instead of grep/head pipes
- sequence-tasks: use array args + JS slice instead of shell pipes
All 43 tests pass, clean build.
TerminalGravity
force-pushed
the
fix/shell-syntax-remaining-tools
branch
from
0cec6fe to
ae5ae50
Compare
|
Closing in favor of #189 which is the latest iteration of this fix. Thanks! |
1 participant
Fixes #172
run()usesexecFileSync('git', args)— no shell. Many tools were passing shell operators and non-git commands through it, silently breaking.Changes across 8 tools:
shell()helper togit.tsfor commands needing shell interpretationrun('git status --porcelain 2>/dev/null')→run(['status', '--porcelain'])run('cat package.json')→readFileSync()run('wc -l < file')→ Node.js fsrun('git ls-files | grep | head')→ array args + JS filteringrun('npm build | tail')→shell()Build passes clean. All 10 files touched, net +34 lines.