Skip to content

fix(cli): use forward slashes in transparent hook executable path#320

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
nanzhijin:main
Jun 28, 2026
Merged

fix(cli): use forward slashes in transparent hook executable path#320
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
nanzhijin:main

Conversation

@nanzhijin

@nanzhijin nanzhijin commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

On Windows, std::env::current_exe() returns a path with backslashes
(e.g. C:\Users....cargo\bin\nemo-relay.exe). This path is written into
the generated hooks.json and later executed by the agent's hook runner via
bash, which interprets backslashes as escape sequences, corrupting the path.

Replace backslashes with forward slashes — valid on both Windows cmd and
Git Bash, and already the convention on Linux/macOS.


Overview

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

One-line change in crates/cli/src/launcher.rs:826:

- .and_then(|path| path.to_str().map(str::to_owned))
+ .and_then(|path| path.to_str().map(|s| s.replace('\\', "/")))

std::env::current_exe() on Windows returns paths with backslashes. When
this path is embedded into the hook command and executed by bash (Claude
Code's hook runner), bash interprets the backslashes as escape sequences
(\U, \1, \c, \b, \n), mangling the path into something like
C:Users16611.cargobinnemo-relay.exe.

Forward slashes (C:/Users/.../.cargo/bin/nemo-relay.exe) work correctly
on both Windows cmd and Git Bash.

Where should the reviewer start?

crates/cli/src/launcher.rs:826transparent_hook_executable()

Related Issues

Summary by CodeRabbit

  • Bug Fixes
    • Improved hook executable path formatting on Windows by converting backslashes to forward slashes for consistent injection.
    • Kept existing fallback behavior unchanged when the current executable path can’t be resolved.

@nanzhijin nanzhijin requested a review from a team as a code owner June 27, 2026 16:05
@copy-pr-bot

copy-pr-bot Bot commented Jun 27, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added size:XS PR is extra small Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code labels Jun 27, 2026
@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 852d92f1-c8cf-4925-bfdc-440289798870

📥 Commits

Reviewing files that changed from the base of the PR and between 2be4bab and 1670da8.

📒 Files selected for processing (1)
  • crates/cli/src/launcher.rs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/cli/src/launcher.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/cli/src/launcher.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/cli/src/launcher.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/cli/src/launcher.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/cli/src/launcher.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/cli/src/launcher.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/cli/src/launcher.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/cli/src/launcher.rs
🔇 Additional comments (1)
crates/cli/src/launcher.rs (1)

826-837: LGTM!


Walkthrough

transparent_hook_executable() now normalizes the resolved executable path string on Windows by replacing backslashes with forward slashes before returning it. Non-Windows behavior and the fallback path remain unchanged.

Changes

Hook Executable Path Normalization

Layer / File(s) Summary
Backslash-to-forward-slash normalization in transparent_hook_executable
crates/cli/src/launcher.rs
transparent_hook_executable() returns a Windows-normalized path string by replacing \ with /, while other platforms keep the raw to_str() output.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the Windows path normalization fix.
Description check ✅ Passed The description matches the template and includes overview, details, reviewer guidance, and related issue.
Linked Issues check ✅ Passed The change directly addresses issue #319 by converting Windows backslashes to forward slashes in the hook executable path.
Out of Scope Changes check ✅ Passed The PR appears scoped to the reported Windows Git Bash path bug with no unrelated code changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/cli/src/launcher.rs`:
- Line 826: Shell-quote the executable path before it is inserted into the hook
command, because the current `path.to_str().map(|s| s.replace('\\', "/"))` only
normalizes separators and still leaves spaces in absolute Windows paths
unescaped. Update `hook_forward_command()` in `crates/cli/src/launcher.rs` so
the returned executable value is already quoted, which will automatically
protect all downstream uses including the command formatting in
`crates/cli/src/installer.rs`.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0779d5cb-4f45-42d1-93b5-3a54138ea731

📥 Commits

Reviewing files that changed from the base of the PR and between 18e926b and 2be4bab.

📒 Files selected for processing (1)
  • crates/cli/src/launcher.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/cli/src/launcher.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/cli/src/launcher.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/cli/src/launcher.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/cli/src/launcher.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/cli/src/launcher.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/cli/src/launcher.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/cli/src/launcher.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/cli/src/launcher.rs
🔇 Additional comments (1)
crates/cli/src/launcher.rs (1)

823-828: 📐 Maintainability & Code Quality

Please confirm the required Rust validation ran.

I don't see evidence in the provided context that the mandatory Rust checks were run for this change. As per coding guidelines, "If any Rust code changed, always run just test-rust", "also run cargo fmt --all", and "also run cargo clippy --workspace --all-targets -- -D warnings."

Source: Coding guidelines

Comment thread crates/cli/src/launcher.rs Outdated
…Windows, std::env::current_exe() returns a path with backslashes (e.g. C:\Users\...\.cargo\bin\nemo-relay.exe). This path is written into the generated hooks.json and later executed by the agent's hook runner via bash, which interprets backslashes as escape sequences, corrupting the path. Replace backslashes with forward slashes — valid on both Windows cmd and Git Bash, and already the convention on Linux/macOS.

Signed-off-by: nanzhijin <N19931465818@outlook.com>
@github-actions github-actions Bot added size:S PR is small and removed size:XS PR is extra small labels Jun 27, 2026
@nanzhijin

nanzhijin commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Apologies for pushing a few extra lines — wanted to avoid any risk of the change leaking into non-Windows paths.
Reasoning below:
Tested on Windows 11 x64 with Git Bash — all hook events (PreToolUse, PostToolUse, Stop, etc.) now execute successfully with no command not found errors.
On non-Windows platforms the #[cfg(not(windows))] path passes through to_owned() unchanged, so Linux/macOS behavior is untouched.

@willkill07

Copy link
Copy Markdown
Member

/ok to test 1670da8

@willkill07

Copy link
Copy Markdown
Member

/merge

@nanzhijin

Copy link
Copy Markdown
Contributor Author

@willkill07 Thanks for the review! Let me know if anything else is needed on my end before this can go in.

@willkill07

Copy link
Copy Markdown
Member

/merge

@rapids-bot rapids-bot Bot merged commit 19a537d into NVIDIA:main Jun 28, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug issue describes bug; PR fixes bug lang:rust PR changes/introduces Rust code size:S PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: fix: Windows Git Bash: transparent hook executable path uses backslashes

2 participants