Skip to content

fix(codegraph): add injectable shell-free runner - #163

Open
MarsSall wants to merge 1 commit into
Gentleman-Programming:mainfrom
MarsSall:fix/codegraph-runner-foundation
Open

fix(codegraph): add injectable shell-free runner#163
MarsSall wants to merge 1 commit into
Gentleman-Programming:mainfrom
MarsSall:fix/codegraph-runner-foundation

Conversation

@MarsSall

@MarsSall MarsSall commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduce an injectable, shell-free CodeGraph runner without changing existing Unix execution behavior.
  • Preserve argument boundaries and forward cwd, cancellation signals, and output-buffer limits.
  • Include structured command arguments in fallback diagnostics.

Part of #160. The follow-up PR will add the Windows npm-entry resolution and close the issue.

Changes

File Change
extensions/codegraph-tools.ts Adds the injectable runner and structured fallback arguments.
tests/codegraph-tools.test.ts Covers direct shell-free execution and forwarded process options.

Test plan

  • node --experimental-strip-types --test tests/codegraph-tools.test.ts — 14/14 passing on the completed stack.
  • PR1 focused snapshot passed 10/10 tests.
  • git diff --check passed.
  • Independent bounded reliability review approved.

Chain Context

Field Value
Chain Windows CodeGraph runner
Tracker PR Not needed
Position 1 of 2
Base main
Depends on None
Follow-up fix/windows-codegraph-npm-entry after this PR merges
Review budget 286 / 400 changed lines
Starts at main at 1fbaf829
Ends with Injectable shell-free runner foundation

Chain Overview

main
 └── 📍 PR 1: runner foundation
      └── PR 2: safe Windows npm entry resolution

Scope

  • Includes: runner injection, Unix behavior preservation, process-option forwarding, fallback argv diagnostics.
  • Excludes: Windows npm package-entry resolution and Windows-specific security checks.

Autonomy

  • CI is expected to pass for this PR branch.
  • This PR has one deliverable scope.
  • This PR can be rolled back without unrelated changes.
  • Tests cover this unit.

Contributor checklist

  • Links the implementation issue.
  • Uses a conventional commit.
  • Contains no Co-Authored-By trailer.
  • Keeps implementation and tests in the same review unit.
  • Adds exactly one type:* label after PR creation.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Code Graph command execution reliability and error reporting.
    • Failure details now include the command arguments used.
    • Output truncation behavior remains supported for large command results.
  • Tests

    • Added coverage for command execution options, cancellation handling, working directories, and output limits.
    • Enhanced validation of truncated output and error responses.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9d640855-7ce6-461d-a72e-4c8fa00c4399

📥 Commits

Reviewing files that changed from the base of the PR and between 1fbaf82 and 1b6aea1.

📒 Files selected for processing (2)
  • extensions/codegraph-tools.ts
  • tests/codegraph-tools.test.ts

📝 Walkthrough

Walkthrough

The CodeGraph runner is now created through an injectable factory, forwards execution options to execFile, and includes resolved command arguments in failure details. Tests cover direct execution, fallback responses, validation, command arguments, and output truncation.

Changes

CodeGraph runner execution

Layer / File(s) Summary
Injectable runner factory
extensions/codegraph-tools.ts
Adds the exported runner factory and dependency types while preserving workspace and command argument resolution.
Tool execution and failure reporting
extensions/codegraph-tools.ts
Uses the factory-created runner, forwards cwd, signal, and maxBuffer, truncates output, and includes command arguments in failure details.
Runner and tool tests
tests/codegraph-tools.test.ts
Tests direct execution option forwarding, workspace validation, fallback details, command arguments, output truncation, and request validation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant createCodeGraphTool
  participant runCodeGraphCommand
  participant execFile
  createCodeGraphTool->>runCodeGraphCommand: execute with cwd, signal, maxBuffer
  runCodeGraphCommand->>execFile: run command without shell wrapping
  execFile-->>runCodeGraphCommand: stdout and stderr or error
  runCodeGraphCommand-->>createCodeGraphTool: output or failure details with args
Loading

Possibly related PRs

Suggested labels: type:feature

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: an injectable, shell-free CodeGraph runner.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MarsSall

Copy link
Copy Markdown
Contributor Author

The dependent Windows slice is now available as a clean fork-local Draft: MarsSall#1

It targets this PR's branch, so reviewers can inspect only the 234-line follow-up. After #163 merges, the same reviewed branch will be proposed to main as the official upstream PR that closes #160.

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