fix(run): execute local files instead of mis-routing to package-exec#75
Merged
Conversation
`run <path>` / `runner run <path>` had no local-file detection: a path token fell through to the PM-exec fallback, where a node PM turned it into `<pm> x <path>` (resolving the local path as a remote package -> registry 404 / `git clone` failure) and a non-node project spawned it directly, so the executable bit and shebang were never consulted. Add a local-file dispatch path (`cmd::run::local_file`) wired into `resolve_dispatch` in two places: - An explicit path-like token (separator or `./`/`/`/`~` prefix) is checked at the top, before task lookup, so an explicit path outranks a same-named task. A missing explicit path reports a clear error rather than a 404. - A bare filename is checked after task lookup misses but before the PM-exec fallback, so a task always wins for a bare name yet a local script such as `main.ts` still runs. Classification: executable bit (Unix) or native extension (Windows) -> spawn directly; `#!` shebang (including `#!/usr/bin/env -S <interp> <args>`) -> parse interpreter and invoke; recognized source extension -> detected runtime (`.ts`/`.tsx`/`.js`/`.mjs`/`.cjs` via bun / `deno run` / node; `.py` via `uv run` / python; `.go` via `go run`); otherwise a clear error. A token resolving to an existing local file never reaches `bunx`/`npx`/`pnpm dlx`/`deno x`/`uvx`. Reuses `configure_command` for cwd + `node_modules/.bin` PATH and new `run_file_cmd` helpers on the runtime tool modules. Verification: `cargo build`, `cargo test --lib` (727 pass), `cargo clippy --all-targets --all-features` (0 warnings), and `dprint check` all green. Exercised the built `run` binary end to end: subdir `.ts` runs via bun (not bunx), bare `main.ts` runs via `deno run` (not `deno x`), `.mjs` via node (not npx), `.go` via `go run`, an exec script and a no-`+x` shebang script both run, a bare task name still beats a same-named file, an explicit `./deploy` beats a same-named task, a missing `./nope.sh` errors cleanly, and a remote `@scope/pkg` spec still reaches package-exec. Closes #69
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
🚫 Excluded labels (none allowed) (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds local-file execution for Sequence Diagram(s)sequenceDiagram
participant CLI as resolve_dispatch
participant Qualify as precheck_task
participant LocalFile as local_file.rs
participant Tool as runtime helpers
participant Spawn as Command
CLI->>LocalFile: try_path_token / try_bare_file
Qualify->>Qualify: accept explicit-prefix local paths
LocalFile->>Tool: build runtime or interpreter command
Tool->>Spawn: return configured Command
CLI->>Spawn: dispatch local file
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: Comment |
…tive paths build_command read the exec bit before the shebang/runtime check, so a +x shebang-less .ts/.js/.py/.go (common on vfat/exfat/ntfs-3g mounts that report mode 0777 for every file) was spawned via raw execve and failed ENOEXEC instead of reaching bun/deno/node/uv/python/go. Read the shebang up front and take the direct-exec branch only for a real shebang or a non-source binary; a recognized source file with no shebang now runs via its runtime. The pre-task local-file short-circuit fired for any separator-bearing token, so once a make target like bin/tool produced its output file, run bin/tool spawned the stale artifact (or errored) instead of running make bin/tool. Gate the pre-task branch on an explicit local prefix (./ ../ / \ ~ or a Windows drive root); route prefix-less relative paths through the after-task-miss fallback so a matching task wins first.
- try_bare_file + resolve_path now join relative/bare tokens onto ctx.root (the --dir/RUNNER_DIR target task detection scans and the child runs in), not the live cwd; fixes --dir mis-routing a relative token into the bunx 404 fallback and running a wrong same-named cwd file. - precheck_task gains the same explicit-prefix escape hatch as dispatch (has_local_prefix, now pub(super)) so a chain / install --parallel under --runner / [task_runner].prefer no longer rejects ./gen.sh. - fix module doc: dead [`try_build`] link -> try_path_token / try_bare_file.
read_shebang no longer hard-fails when a file cannot be opened for read: an execute-only binary (Unix mode 0111) can be execve'd but returns EACCES on open(O_RDONLY). Treat that as 'no shebang' and defer to the exec bit / extension, so a 0111 native binary spawns directly and a 0111 recognized source routes to its runtime instead of bunx. Signature simplifies to Option<Shebang>. .jsx/.tsx no longer route to bare node, which has no JSX transform and type-strips only .ts/.mts/.cts -> 'node app.tsx' is categorically broken. They now route via bun/deno when detected, else surface a clear 'node cannot run' error (outcome 4) rather than an unrunnable command.
bare_file_in did build_command(...).ok(), swallowing the classification error for a recognized source file the runtime cannot run (e.g. .tsx in a node-only project). The None then fell through to build_pm_exec_command, producing pnpm exec app.tsx / npx app.tsx -> registry 404 on a file that exists on disk — the exact #69 mis-route, and an explicit-vs-bare split (run ./app.tsx errored cleanly, run app.tsx 404'd). try_bare_file/bare_file_in now return Result<Option<LocalDispatch>>, mirroring the explicit ./ path: propagate build_command's error unless the extension is genuinely Unrecognized (data.bin still falls through to PM-exec; EACCES execute-only source still routes to its runtime). Add a regression test: a bare app.tsx in a Pnpm project must Err 'node cannot run', never build a pnpm/npx command.
kjanat
added
bug
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/cmd/run/dispatch.rs`:
- Around line 243-257: The bare-file fallback in dispatch is being checked too
late, after runner_constraint_error and resolve_node_pm can already fail. Move
the try_bare_file probe in dispatch::Dispatch around the local-file path so it
runs before the PM/task fallback gates, preserving local runnable files like
main.ts even in constrained projects and before any unrelated PM resolution
error can abort dispatch.
In `@src/cmd/run/local_file.rs`:
- Around line 138-143: The error handling in build_command/routing_for_extension
is letting existing bare local files fall back to Ok(None), which can route them
into PM-exec instead of failing as unsupported local files. Update the match in
local_file.rs so that when the path already resolves to a real local file,
unsupported cases return the local-file error from build_command rather than
treating SourceRouting::Unrecognized as None; keep the fallback to Ok(None) only
for non-local inputs. Use build_command and routing_for_extension as the key
symbols when adjusting this branch.
- Around line 378-391: The shebang parsing in local_file::Shebang handling is
re-splitting arguments with split_whitespace, which breaks env -S quoting and
incorrectly splits non--S shebang arguments. Update the parsing logic in the
is_env branch and the fallback args collection to preserve kernel-style shebang
semantics: treat non--S env arguments as a single argument string, and only
apply env -S parsing rules where appropriate without losing quoted substrings.
Use the existing Shebang construction path in local_file.rs to keep program and
args aligned with what env and the kernel would receive.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cfa317cb-a5d8-4d7e-bd35-ca8ec0f4c455
📒 Files selected for processing (12)
CHANGELOG.mdsrc/cmd/run.rssrc/cmd/run/dispatch.rssrc/cmd/run/local_file.rssrc/cmd/run/qualify.rssrc/lib.rssrc/tool/bun.rssrc/tool/deno.rssrc/tool/go_pm.rssrc/tool/node.rssrc/tool/python.rssrc/tool/uv.rs
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: verify PR #75 / 9_verify.txt: verify PR #75
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: PR #75 / 0_Analyze (rust).txt: PR #75
Conclusion: failure
failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:954:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:955:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:956:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:963:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:964:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:965:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:966:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:967:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:972:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:973:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:978:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:983:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:984:9: macro expansion failed for 'assert'
[] [build-stdout] [] [build-stdout] �[33m WARN�[0m /home/runner/work/runner/runner/src/types.rs:985:9: macro expansion failed for 'assert'
[] [build-stdout...
🧰 Additional context used
📓 Path-based instructions (1)
**/CHANGELOG.md
📄 CodeRabbit inference engine (Custom checks)
**/CHANGELOG.md: If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, CHANGELOG.md MUST also be modified in the same PR.
If a version bump is detected, CHANGELOG.md MUST contain a new section header matching the exact new version number in the format: '## [X.Y.Z] - YYYY-MM-DD'.
If NO version bump is detected, the changes in the PR MUST be added under the existing '## [Unreleased]' section in CHANGELOG.md. The entry MUST describe the changes (e.g., Added, Changed, Fixed, Removed).
Files:
CHANGELOG.md
🧠 Learnings (7)
📚 Learning: 2026-03-26T20:05:44.851Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/cmd/mod.rs:67-75
Timestamp: 2026-03-26T20:05:44.851Z
Learning: In Rust, `std::process::Command` does not provide getters for stdio configuration, so whether `Stdio::inherit()` (or other `Stdio::*` settings) was applied cannot be asserted via pure unit tests without spawning a process and inspecting OS-level fds. When reviewing Rust code, do not flag “missing unit test coverage” for stdio configuration on `std::process::Command` as a code issue—treat the explicit `Command::stdin/stdout/stderr` setter calls in the source as the meaningful guarantee.
Applied to files:
src/tool/bun.rssrc/lib.rssrc/tool/deno.rssrc/cmd/run.rssrc/tool/go_pm.rssrc/cmd/run/dispatch.rssrc/tool/python.rssrc/cmd/run/qualify.rssrc/tool/uv.rssrc/tool/node.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-04-21T15:16:40.277Z
Learnt from: kjanat
Repo: kjanat/runner PR: 4
File: src/tool/just.rs:132-133
Timestamp: 2026-04-21T15:16:40.277Z
Learning: In Rust, if a method like `ExtractedTask::name() -> &str` returns a borrowed `&str` tied to `&self`, then using `sort_unstable_by_key(|t| t.name())` should be avoided because `by_key` requires the key to not borrow from the element being sorted (it will fail to compile due to the key’s lifetime). Do not recommend `sort_unstable_by_key` as a simplification in this situation. If you need an allocation-free and idiomatic comparison, use `sort_unstable_by(|a, b| a.name().cmp(b.name()))` instead. Using `.to_owned()` inside `by_key` is an alternative but allocates a `String` per element.
Applied to files:
src/tool/bun.rssrc/lib.rssrc/tool/deno.rssrc/cmd/run.rssrc/tool/go_pm.rssrc/cmd/run/dispatch.rssrc/tool/python.rssrc/cmd/run/qualify.rssrc/tool/uv.rssrc/tool/node.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-05-04T23:28:17.947Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: src/lib.rs:222-223
Timestamp: 2026-05-04T23:28:17.947Z
Learning: When reviewing Rust `rustdoc` comments (`/// ...`), treat a trailing backslash (`\`) at the end of a comment line as valid CommonMark syntax for a hard line break (rendered as `<br>`). Do not flag such trailing backslashes as papercuts or recommend removing them unless there is clear evidence they are unintended (e.g., they are not in the `rustdoc` comment context or the surrounding formatting contradicts the intended CommonMark hard-break usage).
Applied to files:
src/tool/bun.rssrc/lib.rssrc/tool/deno.rssrc/cmd/run.rssrc/tool/go_pm.rssrc/cmd/run/dispatch.rssrc/tool/python.rssrc/cmd/run/qualify.rssrc/tool/uv.rssrc/tool/node.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-06-01T17:42:48.461Z
Learnt from: kjanat
Repo: kjanat/runner PR: 34
File: Cargo.toml:61-61
Timestamp: 2026-06-01T17:42:48.461Z
Learning: In this repo, do not flag `actions_rs::log::GroupGuard` or `actions_rs::log::group_guard` as missing/non-existent when they are referenced in Rust code, because the `actions-rs` crate (dependency `actions-rs = "0.1"` / published v0.1.x) exports `pub struct GroupGuard` and `pub fn group_guard` from `actions_rs::log` (and also exports `actions_rs::env::is_github_actions`). This avoids false positives from outdated/incorrect web search results. If the repo does not depend on `actions-rs = "0.1"` in `Cargo.toml`, then normal missing-import/item checks can apply.
Applied to files:
src/tool/bun.rssrc/lib.rssrc/tool/deno.rssrc/cmd/run.rssrc/tool/go_pm.rssrc/cmd/run/dispatch.rssrc/tool/python.rssrc/cmd/run/qualify.rssrc/tool/uv.rssrc/tool/node.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-06-11T18:52:28.233Z
Learnt from: kjanat
Repo: kjanat/runner PR: 45
File: src/lib.rs:659-661
Timestamp: 2026-06-11T18:52:28.233Z
Learning: In Rust, it’s acceptable to use `matches!(cli.command, Some(cli::Command::Doctor { .. }))` (or similar) when the scrutinee comes from a field accessed through a shared reference (e.g., `cli: &Cli`). If the match pattern uses `..` and binds zero fields (so only the enum discriminant is matched), Rust match ergonomics will avoid moving out of the referenced value. Reviewers should not flag such code as a compile-blocking move; confirm with `cargo check` rather than forcing changes like `.as_ref()` when compilation succeeds.
Applied to files:
src/tool/bun.rssrc/lib.rssrc/tool/deno.rssrc/cmd/run.rssrc/tool/go_pm.rssrc/cmd/run/dispatch.rssrc/tool/python.rssrc/cmd/run/qualify.rssrc/tool/uv.rssrc/tool/node.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-05-15T01:31:48.037Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/types.rs:584-585
Timestamp: 2026-05-15T01:31:48.037Z
Learning: In the `kjanat/runner` Rust codebase, remember that `just` treats its `justfile` name as officially case-insensitive and allows the hidden variant: `justfile`, `Justfile`, `JUSTFILE`, etc., and `.justfile`. `TaskSource::Justfile` detection should recognize these filename variants. Also note that `from_label` in `src/types.rs` is for parsing user-supplied qualifier prefixes (e.g. `justfile:build`), not for detecting the on-disk justfile. If you add/adjust qualifier label emission for `TaskSource::Justfile`, it’s not strictly required for backward compatibility because `TaskSource::label()` previously only emitted lowercase `"justfile"`, but adding additional casing/hidden variants as a defensive UX improvement is consistent with `just`’s official support.
Applied to files:
src/tool/bun.rssrc/lib.rssrc/tool/deno.rssrc/cmd/run.rssrc/tool/go_pm.rssrc/cmd/run/dispatch.rssrc/tool/python.rssrc/cmd/run/qualify.rssrc/tool/uv.rssrc/tool/node.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-05-14T15:35:39.922Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/cmd/run.rs:206-212
Timestamp: 2026-05-14T15:35:39.922Z
Learning: For Go projects using the canonical `cmd/` layout, when `runner` dispatches a local Go tool by its bare name (e.g., `some-cli`) under `PackageManager::Go`, resolve it relative to `ctx.root` by probing `ctx.root/cmd/<task_name>/`. If it exists, run the tool with the filesystem-path form: `go run ./cmd/<task_name>` (note the leading `./`). Do NOT generate `go run <task_name>` because bare names are treated as Go import paths, not local directories. While `go run <module>/cmd/<task_name>` can work, it’s unnecessary for local development—prefer `./cmd/<task_name>`.
Applied to files:
src/cmd/run.rssrc/cmd/run/dispatch.rssrc/cmd/run/qualify.rssrc/cmd/run/local_file.rs
🔇 Additional comments (9)
CHANGELOG.md (1)
20-48: LGTM!src/lib.rs (1)
602-602: LGTM!src/cmd/run.rs (1)
28-28: LGTM!src/tool/bun.rs (1)
42-55: LGTM!Also applies to: 77-89
src/tool/deno.rs (1)
219-229: LGTM!Also applies to: 263-283
src/tool/go_pm.rs (1)
161-189: LGTM!src/tool/node.rs (1)
5-5: LGTM!Also applies to: 33-41, 597-617
src/tool/python.rs (1)
5-24: LGTM!Also applies to: 156-171
src/tool/uv.rs (1)
49-63: LGTM!Also applies to: 95-107
kjanat
added a commit
that referenced
this pull request
Jun 28, 2026
CodeRabbit review on #75 — two valid findings fixed, one skipped. dispatch: move the bare local-file probe ahead of node-PM resolution (but still after the `--runner` constraint hard error), so a local file runs when PM resolution would hard-error for reasons unrelated to it (a strict devEngines/packageManager mismatch, an incompatible `--pm`) — running `main.ts` via its runtime never needed the package.json PM. The explicit `--runner` constraint still hard-errors, never silently downgrading to a coincidental file. shebang: parse the `env -S` / `--split-string` form with a quote-aware splitter instead of `split_whitespace`, so quoted arguments (`-S deno run --allow-read="a b"`) stay a single token. Skipped: routing an *unrecognized* existing bare file to an error instead of PM-exec. That fallthrough is deliberate and pinned by `bare_file_ignores_missing_and_non_script_tokens`; #69's protection already errors on recognized-but-unrunnable files (e.g. a bare `.tsx` on node).
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/cmd/run/local_file.rs (2)
1266-1286: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExisting bare local files still leak into PM-exec.
Arrr, this test locks in the exact behaviour the PR says to kill: once
data.binhas resolved to a real local file, returningNoneletsrunner run data.bindrift back intobunx/npx/pnpm dlxinstead of raising the clear unsupported-local-file error. Flip this toexpect_err, and keepbare_file_inon the local-file path for already-existing files.🤖 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 `@src/cmd/run/local_file.rs` around lines 1266 - 1286, The bare-file resolution test is asserting the old behavior where existing local files fall through to PM-exec, which hides unsupported-file errors. Update the `bare_file_ignores_missing_and_non_script_tokens` case to expect an error for already-existing local files like `data.bin`, while keeping `bare_file_in` coverage for the local-file path so it still distinguishes missing tokens from real files.
379-396: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPlain
/usr/bin/envshebangs are still being re-split, matey.Arrr, without
-Sthe shebang tail is one opaque argument toenv, not a free buffet of argv tokens. This still makes#!/usr/bin/env deno runor#!/usr/bin/env python3 -Owork on the non-exec path even though the executable form would fail, so the same script changes behaviour based on its exec bit.🏴☠️ Minimal fix
- let words = split_form.map_or_else( - || { - rest.split_whitespace() - .map(ToOwned::to_owned) - .collect::<Vec<_>>() - }, - split_env_string, - ); + let words = match split_form { + Some(split) => split_env_string(split), + None if rest.is_empty() => Vec::new(), + None => vec![rest.to_owned()], + };Does `/usr/bin/env` split multiple shebang arguments without `-S`, and what do GNU coreutils docs say about `#!/usr/bin/env python3 -O` or `#!/usr/bin/env deno run` versus `#!/usr/bin/env -S ...`?🤖 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 `@src/cmd/run/local_file.rs` around lines 379 - 396, The `/usr/bin/env` parsing in the shebang handling still splits plain `env` tails with `split_whitespace`, which incorrectly treats `#!/usr/bin/env deno run` and similar as multiple argv tokens even without `-S`. Update the logic around the `split_form`/`words` handling in `src/cmd/run/local_file.rs` so only `-S`/`--split-string` uses `split_env_string`, while plain `env` preserves the entire remainder as a single argument rather than re-splitting it. Keep the existing `split_form` detection and adjust the fallback path to match `env`’s non-`-S` behavior consistently.Source: MCP tools
🤖 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.
Duplicate comments:
In `@src/cmd/run/local_file.rs`:
- Around line 1266-1286: The bare-file resolution test is asserting the old
behavior where existing local files fall through to PM-exec, which hides
unsupported-file errors. Update the
`bare_file_ignores_missing_and_non_script_tokens` case to expect an error for
already-existing local files like `data.bin`, while keeping `bare_file_in`
coverage for the local-file path so it still distinguishes missing tokens from
real files.
- Around line 379-396: The `/usr/bin/env` parsing in the shebang handling still
splits plain `env` tails with `split_whitespace`, which incorrectly treats
`#!/usr/bin/env deno run` and similar as multiple argv tokens even without `-S`.
Update the logic around the `split_form`/`words` handling in
`src/cmd/run/local_file.rs` so only `-S`/`--split-string` uses
`split_env_string`, while plain `env` preserves the entire remainder as a single
argument rather than re-splitting it. Keep the existing `split_form` detection
and adjust the fallback path to match `env`’s non-`-S` behavior consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dcb1ebb6-b3de-4331-87bc-fe258a1bc33b
📒 Files selected for processing (3)
CHANGELOG.mdsrc/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (actions)
🧰 Additional context used
📓 Path-based instructions (1)
**/CHANGELOG.md
📄 CodeRabbit inference engine (Custom checks)
**/CHANGELOG.md: If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, CHANGELOG.md MUST also be modified in the same PR.
If a version bump is detected, CHANGELOG.md MUST contain a new section header matching the exact new version number in the format: '## [X.Y.Z] - YYYY-MM-DD'.
If NO version bump is detected, the changes in the PR MUST be added under the existing '## [Unreleased]' section in CHANGELOG.md. The entry MUST describe the changes (e.g., Added, Changed, Fixed, Removed).
Files:
CHANGELOG.md
🧠 Learnings (7)
📚 Learning: 2026-03-26T20:05:44.851Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/cmd/mod.rs:67-75
Timestamp: 2026-03-26T20:05:44.851Z
Learning: In Rust, `std::process::Command` does not provide getters for stdio configuration, so whether `Stdio::inherit()` (or other `Stdio::*` settings) was applied cannot be asserted via pure unit tests without spawning a process and inspecting OS-level fds. When reviewing Rust code, do not flag “missing unit test coverage” for stdio configuration on `std::process::Command` as a code issue—treat the explicit `Command::stdin/stdout/stderr` setter calls in the source as the meaningful guarantee.
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-04-21T15:16:40.277Z
Learnt from: kjanat
Repo: kjanat/runner PR: 4
File: src/tool/just.rs:132-133
Timestamp: 2026-04-21T15:16:40.277Z
Learning: In Rust, if a method like `ExtractedTask::name() -> &str` returns a borrowed `&str` tied to `&self`, then using `sort_unstable_by_key(|t| t.name())` should be avoided because `by_key` requires the key to not borrow from the element being sorted (it will fail to compile due to the key’s lifetime). Do not recommend `sort_unstable_by_key` as a simplification in this situation. If you need an allocation-free and idiomatic comparison, use `sort_unstable_by(|a, b| a.name().cmp(b.name()))` instead. Using `.to_owned()` inside `by_key` is an alternative but allocates a `String` per element.
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-05-04T23:28:17.947Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: src/lib.rs:222-223
Timestamp: 2026-05-04T23:28:17.947Z
Learning: When reviewing Rust `rustdoc` comments (`/// ...`), treat a trailing backslash (`\`) at the end of a comment line as valid CommonMark syntax for a hard line break (rendered as `<br>`). Do not flag such trailing backslashes as papercuts or recommend removing them unless there is clear evidence they are unintended (e.g., they are not in the `rustdoc` comment context or the surrounding formatting contradicts the intended CommonMark hard-break usage).
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-06-01T17:42:48.461Z
Learnt from: kjanat
Repo: kjanat/runner PR: 34
File: Cargo.toml:61-61
Timestamp: 2026-06-01T17:42:48.461Z
Learning: In this repo, do not flag `actions_rs::log::GroupGuard` or `actions_rs::log::group_guard` as missing/non-existent when they are referenced in Rust code, because the `actions-rs` crate (dependency `actions-rs = "0.1"` / published v0.1.x) exports `pub struct GroupGuard` and `pub fn group_guard` from `actions_rs::log` (and also exports `actions_rs::env::is_github_actions`). This avoids false positives from outdated/incorrect web search results. If the repo does not depend on `actions-rs = "0.1"` in `Cargo.toml`, then normal missing-import/item checks can apply.
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-06-11T18:52:28.233Z
Learnt from: kjanat
Repo: kjanat/runner PR: 45
File: src/lib.rs:659-661
Timestamp: 2026-06-11T18:52:28.233Z
Learning: In Rust, it’s acceptable to use `matches!(cli.command, Some(cli::Command::Doctor { .. }))` (or similar) when the scrutinee comes from a field accessed through a shared reference (e.g., `cli: &Cli`). If the match pattern uses `..` and binds zero fields (so only the enum discriminant is matched), Rust match ergonomics will avoid moving out of the referenced value. Reviewers should not flag such code as a compile-blocking move; confirm with `cargo check` rather than forcing changes like `.as_ref()` when compilation succeeds.
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-05-14T15:35:39.922Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/cmd/run.rs:206-212
Timestamp: 2026-05-14T15:35:39.922Z
Learning: For Go projects using the canonical `cmd/` layout, when `runner` dispatches a local Go tool by its bare name (e.g., `some-cli`) under `PackageManager::Go`, resolve it relative to `ctx.root` by probing `ctx.root/cmd/<task_name>/`. If it exists, run the tool with the filesystem-path form: `go run ./cmd/<task_name>` (note the leading `./`). Do NOT generate `go run <task_name>` because bare names are treated as Go import paths, not local directories. While `go run <module>/cmd/<task_name>` can work, it’s unnecessary for local development—prefer `./cmd/<task_name>`.
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
📚 Learning: 2026-05-15T01:31:48.037Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/types.rs:584-585
Timestamp: 2026-05-15T01:31:48.037Z
Learning: In the `kjanat/runner` Rust codebase, remember that `just` treats its `justfile` name as officially case-insensitive and allows the hidden variant: `justfile`, `Justfile`, `JUSTFILE`, etc., and `.justfile`. `TaskSource::Justfile` detection should recognize these filename variants. Also note that `from_label` in `src/types.rs` is for parsing user-supplied qualifier prefixes (e.g. `justfile:build`), not for detecting the on-disk justfile. If you add/adjust qualifier label emission for `TaskSource::Justfile`, it’s not strictly required for backward compatibility because `TaskSource::label()` previously only emitted lowercase `"justfile"`, but adding additional casing/hidden variants as a defensive UX improvement is consistent with `just`’s official support.
Applied to files:
src/cmd/run/dispatch.rssrc/cmd/run/local_file.rs
🔍 Remote MCP GitHub Grep
Relevant context from related source code:
- Deno’s task shell has an integration test where
./file.tswith#!/usr/bin/env -S deno runexecutes successfully, while#!/usr/bin/env deno run(no-S) fails withdeno run: command not found. Its command layer also has explicitCommandPathResolutionErrorandFailedShebangErrorpaths. - Deno’s CLI code documents parsing shebangs like
#!/usr/bin/env -S deno run --allow-readwith its own CLI parser, and its arg handling strips trailing\rfrom shebang arguments. - uv’s script code warns that directly executing a script may ignore inline metadata and recommends
#!/usr/bin/env -S uv run --script; uv’s init tests preserve that shebang when generating scripts. - Bun’s
bun runhelp/examples explicitly show local file execution (bun run ./index.js,bun run ./index.tsx) alongside package scripts. - Go’s
go rundocs say it can run a filesystem path or a list of.gosource files, not only import paths.
🔇 Additional comments (1)
src/cmd/run/dispatch.rs (1)
158-171: LGTM!Also applies to: 221-239
Review on #75 — two valid findings fixed, one skipped. dispatch: move the bare local-file probe ahead of node-PM resolution (but still after the `--runner` constraint hard error), so a local file runs when PM resolution would hard-error for reasons unrelated to it (a strict devEngines/packageManager mismatch, an incompatible `--pm`) — running `main.ts` via its runtime never needed the package.json PM. The explicit `--runner` constraint still hard-errors, never silently downgrading to a coincidental file. shebang: parse the `env -S` / `--split-string` form with a quote-aware splitter instead of `split_whitespace`, so quoted arguments (`-S deno run --allow-read="a b"`) stay a single token. Skipped: routing an *unrecognized* existing bare file to an error instead of PM-exec. That fallthrough is deliberate and pinned by `bare_file_ignores_missing_and_non_script_tokens`; #69's protection already errors on recognized-but-unrunnable files (e.g. a bare `.tsx` on node).
kjanat
force-pushed
the
fix/run-path-local-file-execution
branch
from
c56055d to
300e297
Compare
…hebang args Review on #75 — two findings I previously skipped; on reflection both valid. local file: once a bare token names an existing regular file, treat it as a local file. An unrunnable one (unsupported type, no shebang and not executable, a `.tsx` on node) now surfaces the clear `don't know how to run` error instead of falling through to `bunx`/`npx <file>` and a registry 404 — the #69 bug, just via the bare form rather than `./file`. A token that names nothing on disk (a real package spec) still falls through to PM-exec. `bare_file_in` collapses to `build_command?`, matching the explicit `./file` path. shebang: the kernel hands the interpreter exactly one argument — the whole remainder, internal spaces intact — never whitespace-split. Plain `env` and direct interpreters now keep the tail as a single argument (`#!/usr/bin/env python3 -O` resolves to program "python3 -O", as the kernel runs it); only `env -S` / `--split-string` re-splits, via the quote-aware `split_env_string`.
kjanat
added a commit
that referenced
this pull request
Jun 28, 2026
The #75 branch forked before #71 and #73 merged, so its CHANGELOG was missing their Unreleased entries. Three-way merged master's CHANGELOG.md (per-task chain timing in Added, forced-PM source bias in Fixed) on top of the branch's `run <path>` entry. The changelog now diverges from master by only this PR's entry, so it merges cleanly when #75 lands.
# Conflicts: # CHANGELOG.md
kjanat
force-pushed
the
fix/run-path-local-file-execution
branch
from
4856069 to
59b5b7e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
run <path>/runner run <path>had no local-file detection. A pathtoken was looked up as a task name and, on no match, hit the PM-exec
fallback (
build_pm_exec_command). What happened then was an accident ofpackage-manager detection:
<pm> x <path>, which resolvesthe local path as a remote package:
run bin/recipe-tmlang.ts->bun bin/recipe-tmlang.ts-> registry 404run ./bin/recipe-tmlang.ts->bun ./bin/recipe-tmlang.ts->git clonefailurewith a shebang died with
Permission denied.The executable bit and shebang were never consulted. Adding
./changednothing.
Change
New module
src/cmd/run/local_file.rs, wired intoresolve_dispatchin twoplaces:
.//..////~prefix) is handled at the top, before task lookup and PM resolution,
so an explicit path outranks a same-named task. A directory falls through
(e.g. so
go run ./cmd/foois unaffected); a missing explicit pathreports a clear
no such fileerror instead of a 404; a separator-bearingremote spec like
@scope/pkgorgithub.com/owner/tool(no local prefix,not on disk) still falls through to package-exec.
PM-exec fallback, so a bare name resolves to a task first (no collisions)
yet a local script such as
main.tsstill runs rather than being treatedas a remote package.
Classification of an existing regular file:
.exe/.com) -> spawn directly.#!shebang -> parse the interpreter, including#!/usr/bin/env -S <interp> <args>, and invoke<interp> [args] <file> <forwarded>..ts/.tsx/.mts/.cts/.js/.jsx/.mjs/.cjsvia bun,deno run,or node (per detected project /
--pm);.pyviauv runor python;.goviago run; Windows.ps1/.bat/.cmdvia PowerShell /cmd /c.Hard rule honored: a token that resolves to an existing local file never
reaches
bunx/npx/pnpm dlx/deno x/uvx. Reusesconfigure_commandfor cwd +
node_modules/.binPATH, and addsrun_file_cmdhelpers to thebun/deno/node/uv/python/go tool modules.
expand_tildeis promoted topub(crate)for reuse.Verification
Gates (all green):
cargo buildcargo test --lib-> 727 passedcargo clippy --all-targets --all-features-> 0 warningsdprint check-> exit 0End-to-end with the built
runbinary across fixture projects:sub/hello.ts arg1->bun sub/hello.ts arg1(not bunx), ran./build.sh foo(exec) ->exec, ran./noexec.py x y(shebang, no+x) ->python3 ./noexec.py x y, ranmain.ts a b->deno run main.ts a b(notdeno x), ran./scr.tswith#!/usr/bin/env -S deno run -A->deno run -A ./scr.ts, ranx.mjs hi->node x.mjs hi(not npx), ran./tool.go zzz->go run ./tool.go zzz, randeploy(task) runs the task; explicit./deployruns the file./nope.sh->Error: no such file: ./nope.sh(not Permission denied / 404)@scope/...(remote spec) still routes to package-execCloses #69