Skip to content

fix: broken shell commands in token_audit and what_changed tools - #214

Open
TerminalGravity wants to merge 3 commits into
mainfrom
fix/token-audit-shell-commands
Open

fix: broken shell commands in token_audit and what_changed tools#214
TerminalGravity wants to merge 3 commits into
mainfrom
fix/token-audit-shell-commands

Conversation

@TerminalGravity

Copy link
Copy Markdown
Collaborator

Problem

run() from lib/git.ts uses execFileSync('git', args) — it always prepends git and doesn't support shell features. Multiple tools were calling it with:

  1. Double git prefix: run('git diff ...') → executes git git diff ...
  2. Shell redirects as args: 2>/dev/null passed as a literal argument to git
  3. Non-git commands: run('wc -l ...') → executes git wc -l ...

This means token_audit was completely non-functional — every command it ran would fail silently and return error strings.

Fix

  • token-audit.ts: Replace shell-dependent run() calls with Node fs APIs (countLines, fileBytes) and proper run() array args
  • what-changed.ts: Fix double-git prefix and shell fallback logic
  • lib/git.ts: Add shell() export for cases that genuinely need shell features (pipes, redirects, || fallbacks)

Scope

This PR fixes the two worst-affected tools. ~8 other files have the same bug pattern — filing a separate issue for those.

@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. The root cause analysis is spot-on — run() wrapping everything with git prefix makes all those shell-string calls silently fail.

The approach is solid:

  1. shell() helper — clean, timeout-protected, appropriate escape hatch for genuine shell needs
  2. Node fs replacementscountLines() and fileBytes() are better than shelling out anyway. No parsing, no injection risk.
  3. what-changed.ts — removing the double-git prefix and fixing fallback logic

One thing to watch: shell() returns empty string on failure, same as run(). Fine for now, but if downstream code ever needs to distinguish 'empty output' from 'command failed', we might want an option to throw. Not blocking — can address in the follow-up for the remaining ~8 affected files listed in #215.

README and troubleshooting additions are a nice bonus. LGTM — ready to merge.

Shows 4 concrete scenarios: vague prompt clarification, multi-step
scoping, correction pattern matching, and cross-service awareness.
Each example includes the prompt, triage classification, and the
actual output users can expect.
The README and examples/README.md referenced examples/.preflight/
but the actual config files didn't exist. Added:

- config.yml — profile, related projects, thresholds, embeddings
- triage.yml — keyword rules and strictness settings
- contracts/api.yml — manual contract definition examples

All files are heavily commented so users can copy them into their
project root and customize without needing to reference the docs.
The tool used run() (which calls execFileSync('git', ...)) for non-git
commands like wc and tail. These would silently fail since 'wc' and 'tail'
were passed as arguments to git, not as separate commands.

Replaced with Node.js APIs:
- countFileLines() using readFileSync for line counting
- getFileSize() using statSync for byte size
- readFileTail() using fs read with offset for large file tailing
- Git calls now use array syntax instead of string with shell redirects

Also removed the now-unused shellEscape helper.
@TerminalGravity
TerminalGravity force-pushed the fix/token-audit-shell-commands branch from 1c7aac9 to acdfb8b Compare March 16, 2026 16:16
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