Repository navigation
fix: launch native package bins directly - #1141
Conversation
📝 WalkthroughWalkthroughThis PR adds typed POSIX and Windows bin-shim decoding, centralizes command construction, detects native executables for direct launching, and updates ChangesNative bin execution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExecOrDlx
participant build_bin_command
participant resolve_bin_target
participant NativeExecutable
participant NodeShim
ExecOrDlx->>build_bin_command: pass bin path and arguments
build_bin_command->>resolve_bin_target: resolve target metadata
resolve_bin_target->>NativeExecutable: detect native target
resolve_bin_target->>NodeShim: retain wrapper when required
NativeExecutable-->>build_bin_command: direct executable
NodeShim-->>build_bin_command: Node-backed command
build_bin_command-->>ExecOrDlx: run configured command
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Greptile SummaryThis PR launches recognized native package binaries directly while retaining wrapper behavior for JavaScript and interpreter-backed targets.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (4): Last reviewed commit: "test(linker): use native separators in s..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/aube/src/commands/exec.rs (1)
799-823: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis Windows fixture won't parse — the
@SET PATHEXTline is missing.
parse_cmd_shim_targetrequires the line immediately after) ELSE (to be exactly@setPATHEXT=%PATHEXT:;.JS;=;%(crates/aube-linker/src/sys.rs Line 893). The fixture here jumps straight to thenode ...fallback, soresolve_bin_shimreturnsNoneand all three assertions below will fail. Windows CI hasn't exercised this yet per the PR notes.🐛 Proposed fix for the fixture
`@IF` EXIST \"%~dp0\\node.exe\" (\r\n\ \x20 \"%~dp0\\node.exe\" \"%~dp0\\pkg\\bin.js\" %*\r\n\ ) ELSE (\r\n\ + \x20 `@SET` PATHEXT=%PATHEXT:;.JS;=;%\r\n\ \x20 node \"%~dp0\\pkg\\bin.js\" %*\r\n\ )\r\n",#!/bin/bash # Confirm the generated cmd shim shape the parser expects ast-grep run --pattern 'fn generate_cmd_shim($$$) { $$$ }' --lang rust crates/aube-linker/src/sys.rs🤖 Prompt for 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. In `@crates/aube/src/commands/exec.rs` around lines 799 - 823, Update the Windows cmd shim fixture written in the resolve_bin_target test to include the exact `@SET PATHEXT=%PATHEXT:;.JS;=;%` line immediately after `) ELSE (` and before the fallback node command. Keep the existing assertions and fixture behavior unchanged.
🧹 Nitpick comments (1)
crates/aube-linker/src/sys.rs (1)
931-944: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueReject POSIX-style decoding for Windows
.shshims.
parse_posix_shim_targetaccepts any file containing the marker, but Windows-generated extensionless shims are produced bygenerate_sh_shimand can carryNODE_PATHentries joined with;. Splitting those lines on POSIX:produces a bogus single entry. Gate the POSIX branch with an additional marker/shape check or validate that resolved entries contain no;.🤖 Prompt for 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. In `@crates/aube-linker/src/sys.rs` around lines 931 - 944, Update resolve_shim_node_path so the BinShimStyle::Posix path rejects Windows-generated extensionless shims before splitting NODE_PATH on ':'. Add a marker/shape validation that excludes values containing ';' (or otherwise confirms POSIX format), while preserving valid POSIX resolution and the existing Cmd handling.
🤖 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.
Outside diff comments:
In `@crates/aube/src/commands/exec.rs`:
- Around line 799-823: Update the Windows cmd shim fixture written in the
resolve_bin_target test to include the exact `@SET PATHEXT=%PATHEXT:;.JS;=;%`
line immediately after `) ELSE (` and before the fallback node command. Keep the
existing assertions and fixture behavior unchanged.
---
Nitpick comments:
In `@crates/aube-linker/src/sys.rs`:
- Around line 931-944: Update resolve_shim_node_path so the BinShimStyle::Posix
path rejects Windows-generated extensionless shims before splitting NODE_PATH on
':'. Add a marker/shape validation that excludes values containing ';' (or
otherwise confirms POSIX format), while preserving valid POSIX resolution and
the existing Cmd handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f8244f6-107c-49d5-9260-b3072da9e994
📒 Files selected for processing (6)
crates/aube-linker/src/sys.rscrates/aube/src/commands/dlx.rscrates/aube/src/commands/exec.rsdocs/package-manager/scripts.mdtest/dlx.batstest/exec.bats
Context
I encountered this issue while using aube with OpenCode on my work Windows machine. That machine cannot push to public Git repositories, so I used my personal macOS device to develop and test the fix. The original Windows failure motivated the Windows-specific implementation and tests, but those gated tests were not run locally on macOS.
Summary
Package bins can point directly to compiled executables, but generated wrappers may pass those executables to Node. This change resolves aube-generated wrappers and symlinks before non-shell execution, then launches recognised native targets directly.
This applies to non-shell
aube exec, the local-bin fallback fromaube run, and non-shellaube dlx. Existing JavaScript and interpreter-backed behaviour remains unchanged.What changed
NODE_PATH.dlxnow uses the same command construction asexecandrun.argv[0].exec,run, and scratchdlxlaunches.Deliberately out of scope
.bininvocation continue through their installed wrappers.sysmodule because it currently has one internal consumer.PATHto every non-shell command, but retained the established direct-Node behaviour and limited the change to wrapper and native launch paths.Validation
cargo testpassed.cargo clippy --all-targets -- -D warningspassed.cargo fmt --checkpassed.mise run test:bats test/exec.batspassed.mise run test:bats test/dlx.batspassed.Windows-specific tests are gated by
cfg(windows)and were not run locally on the macOS development host.Summary by CodeRabbit
aube exec,aube run, andaube dlx..cmdshim detection and resolution (including dotted names), with stricter rejection of malformed/oversized wrappers.aube dlxon supported Unix-like systems).