fix(bench): remove unnecessary shell: true from execFileSync

[carlos-alm]

Summary

• Add the missing v2.4.0 native engine benchmark row (measured retroactively using @optave/codegraph@2.4.0 from npm + the v2.4.0 native addon)
• Fill in native column across all benchmark sections (raw totals, 50k-file estimates, incremental rebuilds)
• Add note explaining v2.5.0 build time increase from per-function complexity metrics (cognitive, cyclomatic, Halstead, LOC, MI)
• Add note documenting the retroactive measurement methodology for v2.4.0 native

Test plan

• Verified benchmark numbers by running actual v2.4.0 npm package with native addon
• Cross-checked nodes/edges/DB size match between native and WASM at v2.4.0
• JSON data block updated consistently with markdown tables

[greptile-apps]

Greptile Summary

This PR removes the unnecessary shell: true option from an execFileSync call when installing npm packages. Since execFileSync is called with an array of arguments, the shell option is not needed and removing it is a security improvement that avoids potential shell injection risks.

Note: The PR description appears to describe changes from a different PR (#164 about benchmark updates) and doesn't match the actual code change in this PR.

Confidence Score: 5/5

• This PR is safe to merge - it's a simple one-line fix that improves security
• The change is straightforward and correct: removing shell: true from execFileSync when using array arguments is a best practice that eliminates unnecessary shell spawning and potential security risks. No functional changes to behavior.
• No files require special attention

Important Files Changed

Filename	Overview
scripts/lib/bench-config.js	Removed unnecessary shell: true option from execFileSync call - improves security and follows Node.js best practices

_{Last reviewed commit: 276dffd}

[greptile-apps]

_{16 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

shell: true not needed with execFileSync when passing array args

Suggested change

	execFileSync('npm', ['install', `@optave/codegraph@${version}`, '--no-audit', '--no-fund'], {
	cwd: tmpDir,
	stdio: 'pipe',
	timeout: 120_000,
	shell: true,
	});
	execFileSync('npm', ['install', `@optave/codegraph@${version}`, '--no-audit', '--no-fund'], {
	cwd: tmpDir,
	stdio: 'pipe',
	timeout: 120_000,
	});

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review: Fix shell parameter in execFileSync

Todo List:

• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity check
• Phase 4: Code quality review
• Final assessment and rating

Root Cause Analysis

Problem Being Solved: Removing unnecessary shell: true option from execFileSync() call when using array arguments
Why This Approach: When execFileSync is called with an array of arguments (as on line 69), the shell is not needed. The shell: true option is only required when passing a string command that needs shell interpretation.
Risk Assessment: Very low risk - this is a minor optimization/cleanup with no functional impact.

Configuration/Test Changes

✅ No configuration changes detected - this is a single line removal in application code
✅ No test modifications - no test files changed
✅ No linting/build config changes - purely code cleanup

Code Quality Review

✅ Correct fix: The change is technically correct. execFileSync('npm', ['install', ...], options) doesn't need shell: true
✅ Security improvement: Removing unnecessary shell invocation reduces attack surface
✅ Performance: Minor improvement by avoiding shell spawning
✅ Consistency: Aligns with Node.js best practices for execFileSync with array args

Location: scripts/lib/bench-config.js:73 - within npm package installation retry logic for benchmark infrastructure

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Cleanup of unnecessary `shell: true` parameter in execFileSync call
**Why This Approach**: Technically correct - shell is not needed when using array arguments
**Risk Assessment**: Minimal risk - improves security and performance marginally

## Critical Concerns
None - this is a straightforward cleanup addressing a legitimate code quality issue.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: APPROVE
- Reasoning: Clean, correct fix that improves code quality with no functional risk. The Greptile review correctly identified this issue and the fix is appropriate.

---

[carlos-alm]

@greptileai