Skip to content

fix: clarify_intent non-git commands silently broken via git.run()#170

Closed
TerminalGravity wants to merge 3 commits intomainfrom
fix/clarify-intent-shell-commands
Closed

fix: clarify_intent non-git commands silently broken via git.run()#170
TerminalGravity wants to merge 3 commits intomainfrom
fix/clarify-intent-shell-commands

Conversation

@TerminalGravity
Copy link
Collaborator

Problem

clarify-intent.ts passes two non-git shell commands through git.run():

const typeErrors = run("pnpm tsc --noEmit 2>&1 | grep -c 'error TS' || echo '0'");
const testFiles = run("find tests -name '*.spec.ts' -maxdepth 4 2>/dev/null | head -20");

run() uses execFileSync('git', args), so these become git pnpm tsc ... and git find tests ... — always failing silently. The type error count and test file list in clarify_intent output were always wrong.

Same class of bug as PR #136.

Fix

  • Type checking: use execSync (needs shell for pipe/redirect)
  • Test file discovery: replace shell find with a simple fs walk (no shell needed)

Testing

  • tsc --noEmit clean
  • All 77 tests pass

…ton)

Covers:
- Default config when no .preflight/ dir
- Singleton caching behavior
- config.yml parsing and merging with defaults
- triage.yml parsing
- Graceful handling of invalid/empty YAML
- Environment variable overrides (profile, related projects, embeddings)
- Env vars ignored when .preflight/ dir exists
- getRelatedProjects() backward compat helper
- hasPreflightConfig() detection
- Export helper functions (estimateTokens, extractText, extractToolNames,
  formatTokens, formatCost, formatDuration, analyzeSessionFile) for testability
- Add comprehensive tests covering token estimation, text extraction,
  tool name extraction, formatting, session analysis with corrections,
  tool calls, preflight detection, and timestamp tracking
- First tool-level test file (tests/tools/)
clarify-intent.ts passed shell pipelines ('pnpm tsc ... | grep ...' and
'find tests ...') to git.run(), which uses execFileSync('git', args).
This meant the commands were silently broken — they'd run as
'git pnpm tsc ...' and 'git find tests ...', always failing.

Fix: use execSync for tsc type-checking (needs shell for pipes),
and replace the shell 'find' with a simple fs walk for test files.

Same class of bug as PR #136 (shell operators in run() calls).
TerminalGravity added a commit that referenced this pull request Mar 8, 2026
…, session-health)

run() uses execFileSync which doesn't interpret shell operators (|, ||, &&,
2>/dev/null). These tools were passing shell pipelines/redirects as git args,
causing silent failures or incorrect results.

Fixed:
- what-changed: use getDiffFiles() helper and array args for log
- checkpoint: use array args for add/commit/reset instead of shell chains
- session-health: use array args for diff --stat, split output in JS

Same bug class as PR #170 (clarify-intent). Many more tools still affected —
see forthcoming issue for tracking.
@TerminalGravity
Copy link
Collaborator Author

Superseded by #175 which covers all the shell syntax fixes in one PR. Closing in favor of that.

TerminalGravity added a commit that referenced this pull request Mar 10, 2026
Fixes #172 — remaining tools after #170 and #171.

Tools fixed:
- verify-completion: replaced shell-piped tsc/test/build commands with
  execFileSync, read package.json via fs instead of cat
- token-audit: replaced wc/tail shell commands with fs.readFileSync,
  fs.statSync, and Buffer reads
- session-handoff: replaced 'command -v' with 'which' via execFileSync,
  replaced gh shell pipe with direct execFileSync call
- audit-workspace: replaced shell-piped find|wc with git ls-files + JS filter
- sharpen-followup: replaced shell-string run() calls with array args
- scope-work: replaced git ls-files|head|grep pipe with JS array ops,
  fixed 'git git' double-prefix on status/diff calls
- enrich-agent-task: replaced all shell-piped git/head/grep with
  JS fs.readFileSync and array filtering
- sequence-tasks: replaced git ls-files pipe with array args + JS slice

All changes use Node.js fs/child_process directly or pass proper
array args to run(), eliminating silent failures from shell operators
being passed as literal git arguments.
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