Skip to content

feat(embed): allow runtimes to leave PATH unchanged - #1189

Merged
jdx merged 1 commit into
agent/fix-wrapper-path-precedencefrom
agent/embed-wrapper-without-path
Jul 30, 2026
Merged

jdx merged 1 commit into
agent/fix-wrapper-path-precedencefrom
agent/embed-wrapper-without-path

Conversation

@jdx

@jdx jdx commented Jul 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • add EmbedderRuntime::without_path() to preserve the inherited PATH
  • continue using the configured runtime for direct aube spawns and NODE
  • make path_dir(...) and without_path() follow last-call-wins builder semantics
  • document and test wrapper and selector behavior

Why

Embedding hosts may need to supply the Node program and npm exec path without changing how bare node commands resolve. Previously, wrapper runtimes always derived a PATH entry from node_program.parent(), so that configuration was not expressible.

This is a stacked follow-up to #1188 and targets its branch so this PR contains only the explicit PATH opt-out. It can be retargeted to main after #1188 merges.

Validation

  • cargo fmt --check
  • cargo clippy --all-targets -- -D warnings
  • cargo test

AI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.


Note

Low Risk
Additive embedder builder API with unchanged defaults; only affects hosts that opt in via without_path().

Overview
Adds EmbedderRuntime::without_path() so embedding hosts can keep the inherited PATH while still setting NODE, npm_node_execpath, and using the configured program for direct aube spawns. Wrapper runtimes no longer always prepend the shim’s parent directory to PATH.

path_dir(...) and without_path() use last-call-wins builder semantics ( path_dir clears the opt-out; without_path wins if called last). selector with without_path() still resolves node_program from the bin dir but does not prepend it to PATH.

Docs and unit tests cover wrapper, selector, and builder ordering.

Reviewed by Cursor Bugbot for commit a36c67a. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a1f9dbb-1aa8-4f6f-b371-86fb8d539856

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@greptile-apps

greptile-apps Bot commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds an embedder runtime option that preserves inherited PATH while retaining the configured Node executable.

  • Adds last-call-wins behavior between without_path() and path_dir(...).
  • Adds wrapper and selector coverage for the new runtime behavior.
  • Documents how embedding hosts can opt out of PATH modification.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/aube/src/runtime.rs Adds the PATH opt-out state, resolution behavior, builder precedence, and focused unit tests without an eligible follow-up defect.
docs/embedding/rust.md Documents inherited-PATH behavior and the ordering semantics of the path-related builders.

Reviews (2): Last reviewed commit: "feat(embed): allow runtimes to leave PAT..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 30, 2026 •

Copy link
Copy Markdown

Instruction counts

benchmark trend instructions Δ wall (min) Δ
graph ▁▁█████▇▇▇███▇███████ 16,715,627 → 16,715,398 -0.00% 3.41 → 3.51ms +3.14%
install ▃▁▅▃▄▅▅▄▄▅▃▄▄▄▇▆▆█▇▇█ 103,184,881 → 103,207,294 +0.02% 26.03 → 34.49ms +32.53%
startup ▁▁▇▇▇█▇▆▇▆▇▇▇▇▆█▇▇▇▇▇ 7,667,668 → 7,676,619 +0.12% 2.47 → 3.66ms +48.21%
tree ▁▁▄▄▄▄▄▄▄▄▄▄▄▇▇██████ 17,801,806 → 17,802,569 +0.00% 4.25 → 4.19ms -1.44%

No instruction-count regression above 1%.

Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.

Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.

a36c67a6389a vs 58385562fe49 · measured on this runner, not pushed to the history.

@jdx
jdx force-pushed the agent/embed-wrapper-without-path branch from 710bc00 to a36c67a Compare July 30, 2026 16:49
@jdx
jdx merged commit f5bced5 into main Jul 30, 2026
20 checks passed
@jdx
jdx deleted the agent/embed-wrapper-without-path branch July 30, 2026 17:14
@cursor cursor Bot mentioned this pull request Jul 30, 2026
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