[pashifika]

Summary

PR A of the three-way split of #3911, per @KuSh's round-1 review. Nothing here touches fish or the rewrite path: this is the CLI execution boundary on its own, against current develop.

rtk run, rtk err, rtk test and rtk summary rebuilt their positional arguments into one string and executed it through sh -c (cmd /C on Windows). Argument boundaries were lost and globs, variables and operators were expanded by a shell the caller never asked for:

rtk run printf '[%s]\n' 'a b' '*' '$HOME'
# develop:  [a]/[b]/[a.rs]/[b.rs]/[/home/you]     ← sh -c re-split and expanded
# here:     [a b]/[*]/[$HOME]

Positional forms now execute the program directly. An intentional script is explicit:

rtk run -c '<script>'                 # platform default: sh, or cmd on Windows
rtk run --shell fish -c '<script>'    # named shell; pwsh/PowerShell get -Command
rtk err|test|summary --shell sh '<script>'

Shell mode takes the complete script as one argument, so RTK never reconstructs quoting after Clap has already split argv.

What the interposed shell used to provide, and now does not have to

Blocker 3 of the review, plus the two neighbouring contracts the same switch touched:

1. Exit 127 for an unresolvable program. Direct execution resolves the program itself (resolve_binary), and a miss is an execution outcome rather than an RTK error — it renders through each runner's own failure path:

$ rtk err nosuchcmd123
[FAIL] Command failed (exit code: 127)
  rtk: nosuchcmd123: command not found          exit 127

$ rtk run nosuchcmd123
rtk: nosuchcmd123: command not found            exit 127   (stderr, like a shell)

rtk test and rtk summary do the same through their own summarizers. 127 is the POSIX "command not found" code that CI steps and [FAIL]-line parsers key on; before this commit the head of #3911 replaced both with an anyhow chain and exit 1.

2. rtk test ! <cmd>. ! is both test's negation and the shell's. A command behind it is not a native test expression, so it goes to the test filter — and the negation the joined sh -c string used to get from the shell is now applied to the child's exit status, with argument boundaries intact. tests/native_test_expression_test.rs::bang_before_a_command_still_runs_the_command pins it.

3. --shell arity. rtk run spells its rule with clap (-c requires/conflicts_with); err/test/summary take a positional, and clap cannot express "exactly one value for this positional when that flag is present". The rule is enforced at the same place clap would answer, so it is a usage error (exit 2) instead of a mid-execution failure:

$ rtk err --shell sh printf hello
error: --shell takes the complete command as one quoted argument
Usage: rtk [OPTIONS] <COMMAND>

Also from the review's optional list: tracking labels re-quote arguments that carry whitespace, so rtk test cargo test --filter "a b" is recorded as the command that ran rather than as cargo test --filter a b.

Breaking change

Callers that relied on implicit shell expansion in positional arguments must pass the script explicitly:

rtk test cargo test                              # unchanged — direct argv
rtk test --shell sh 'cargo test && cargo clippy' # was: implicit sh -c
rtk run -c 'ls *.rs | wc -l'                     # was: rtk run ls *.rs | wc -l

No registry rule emits rtk run|err|test|summary, so the hook path is unaffected; the blast radius is direct CLI callers who passed a quoted shell string. The awareness body shipped as RTK.md (RTK_INSTRUCTIONS in src/hooks/init.rs) is updated to teach the argv form and --shell, which is the other half of blocker 3.

Key files

File	Purpose
src/core/shell.rs	Direct-argv and explicit-shell command construction, per-shell flag selection, Launch::NotFound, display quoting
src/core/runner.rs	Renders an unresolvable program through the err/test filters, exit 127
src/cmds/rust/runner.rs, src/cmds/system/summary.rs	argv/--shell entry points
src/main.rs	--shell surface, ! negation, --shell arity usage error
src/hooks/init.rs	RTK_INSTRUCTIONS documents argv execution and --shell
tests/run_shell_test.rs	argv preservation, exit codes, default and explicit shells, missing shell, split-script rejection, the 127 contract

Test plan

• cargo fmt --all --check — clean
• cargo clippy --all-targets -- -D warnings — clean
• cargo test --all — 3,864 passed, 0 failed, 8 ignored
• Manual, isolated HOME/XDG_CONFIG_HOME: argv boundaries ('a b', *, $HOME unexpanded), -c POSIX default, --shell fish -c fish syntax, exit 42/7 propagation, err|test|summary|run on a missing program → 127, --shell with split arguments → usage error
• CI for Linux and Windows (cmd /C, -Command flag selection)

Tested on macOS 26.6.2 arm64, rustc 1.97.1, fish 4.9.0.

---

Follow-ups from the same split, opened separately: B — fish -c wrapper rewriting on top of #3828, with the (…)/control-keyword deferral scoped to the wrapper's own script; C — fish-script wrapping at the hook gate, where the premise can be argued on its own. #3911 is closed in favour of the three.

[rtk-wshm-sync-bot]

wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type	bug-fix
🔴 Risk	high

Summary

This PR changes rtk run/err/test/summary to execute arguments directly instead of joining them into a string and running through sh -c/cmd /C, preserving argv boundaries (globs, variables, whitespace) that were previously expanded by an implicit shell. It adds an explicit --shell flag (or -c for run) for callers who intentionally need shell syntax, restores the POSIX exit 127 contract for unresolvable programs, and updates docs/tests accordingly.

Review Checklist

• Tests present
• Breaking change
• Docs updated

---

Analyzed automatically by wshm · This is an automated analysis, not a human review.

[KuSh]

Round 1 of 3 — CHANGES REQUESTED

Claim: rtk run|err|test|summary stop joining positional argv into a string for an implicit sh -c/cmd /C and execute the program directly; a script becomes explicit via rtk run -c or --shell <shell>; exit 127 and the [FAIL] contract survive the switch.

Scope: accept (round 1, frozen). This is PR A of the split, it is the piece closest to landing, and the argv work is correct as claimed on Unix. The three blockers below are completion items on the new code, not a reason to narrow. It also supersedes #3002, which addresses the same defect by adding quote_shell_arg/shell_quote_join to re-quote argv before joining it for the shell, and covers err/test only. Removing the reconstruction is the better answer than making it correct, so I have closed #3002 pointing here.

Ran: merge-base 0924356b vs head 790a747a, release-built with one CARGO_TARGET_DIR each (md5 ec117312… / 32773c71…, confirmed distinct; the merge onto current develop b748a5f7 got a third). ~60 rows across the four commands × {argv boundaries, exit codes, -c, --shell default/named/missing, --shell arity, ! negation, native test expressions, empty argv, path spellings, builtins, piped stdin, /dev/null stdin, signals}. Invariants: every branch of the new decision (Ready / NotFound / shell / native-test / negation); split streams on a failing cargo build; under the hook (rtk rewrite on all four, identical base vs head, and no rules.rs rule emits them). Savings 60.4% vs the 20% floor (CONTRIBUTING.md) — and the filtered bytes are byte-identical base vs head, so the diff forfeits nothing. cargo fmt --all --check clean, clippy --all-targets 0 warnings, cargo test --all 3864 passed / 0 failed on the head and 3940 / 0 on the head merged onto current develop (real cargo exit status, not a tail-masked pipeline).

Matrix: never executed on this PR before this round — executed now, in full.

CI @790a747a: the run had never been created (action_required); I approved it. Now 11 green, 1 failing: semgrep security scan — the blocking dynamic-command-execution rule on src/core/shell.rs:57, caused by this PR, and src/core/shell.rs is a new file so --baseline-commit cannot absorb it. My prepared fixup fixes it (see Next). Windows, macOS and Ubuntu test jobs all pass, which is the first real signal this branch has had.

---

Blocks merge (3, frozen at round 1)

1. src/core/shell.rs:40 — the "program cannot be run" path was fixed at resolution and left unfixed at spawn, so blocker 3 of #3911 survives verbatim one step later. This is my miss as much as yours: I named the ENOENT-at-resolution instance and you fixed exactly that, and I should have named the class.

resolve_binary answers only "is this resolvable". Once Launch::Ready is returned, every execve failure is a Rust Err that ? turns back into the shape the fix removed. A script checked out with CRLF line endings — not an exotic input — is enough:

$ rtk err ./crlf.sh                       # '#!/bin/sh\r\n'
merge-base:  [FAIL] Command failed (exit code: 127)
             sh: 1: ./crlf.sh: not found                                  exit 127
head      :  rtk: Failed to run err command: Failed to run err:
             Failed to spawn process: No such file or directory (os error 2)  exit 1
dash reference:                                                           exit 127

Same errno, same three-level Context chain, same lost [FAIL] line as rtk err nosuchcmd123 had before this PR. Reproduces on run, err, test and summary, and on ENOEXEC (+x file that is not a valid executable) as well. An unresolvable --shell name is the third arm of it (exit 1 + chain, where an unresolvable program is a rendered [FAIL] + 127).

The second arm is pre-spawn: which::which returns one error for "nothing by that name" and for "it is there but you cannot run it", so a path-y spelling that exists reports the wrong code and an actively false message:

$ rtk run ./bindir/noexec-prog        # exists, mode 644
merge-base:  sh: 1: ./bindir/noexec-prog: Permission denied               exit 126
head      :  rtk: ./bindir/noexec-prog: command not found                 exit 127
$ rtk run ./adir2                     # a directory
merge-base:  exit 126      head: exit 127

dash and bash agree: 126 for non-executable, directory and ENOEXEC; 127 only for genuinely missing. A bare PATH name that exists without +x correctly stays 127 on both — the divergence is confined to spellings that address the filesystem.

Class: 2 causes (pre-spawn classification, post-spawn io::Error) × 4 surfaces, one mechanism each. The argument this PR makes for restoring 127 — "CI steps and [FAIL]-line parsers key on it" — is exactly the argument for 126 and for not replacing either with an anyhow chain.

Prototype for the pre-spawn arm, so this is a measurement and not a request to go write code: Launch::NotFound(String) → Launch::Unrunnable { program, code, reason }, EXIT_COMMAND_NOT_EXECUTABLE = 126, and one names_existing_path check that only fires for path-y spellings. 123 insertions across the five files, fmt clean, clippy 0, cargo test --all 3866 passed, and it restores 126 on all four surfaces while leaving every 127 case — including the bare-name one — untouched. Say the word and I will push it as a fixup, or paste the patch here if you would rather read it first. The post-spawn arm I deliberately did not prototype: the spawn sites are in src/core/stream.rs, shared by every rtk command, so mapping errno there has a blast radius well beyond this PR and is your call whether it belongs here or in a follow-up.

Test that closes it: extend tests/run_shell_test.rs with a 126 case and a CRLF/ENOEXEC case across the four surfaces, mirroring test_and_summary_report_an_unresolvable_program_as_exit_127.

2. src/core/shell.rs:45, tests/run_shell_test.rs — on Windows the new path neither uses the repo's argv-quoting helper nor replaces what cmd /C provided, and nothing tests it.

direct_command is now the widest caller-supplied-argv carrier in the binary, and it is the one place that calls command.args(program_args). ChildArgExt's own doc (src/core/utils.rs:524) says "Use instead of Command::arg/args for anything that arrived on rtk's own command line"; 26 call sites obey it, including run_passthrough. That helper is #3728 — merged as this PR's own merge-base commit. So for an argument containing ", MSYS/Node children on Windows get std's encoding instead of libuv's, and the PR's headline claim is the one thing not delivered there.

Separately, cmd /C resolved from the current directory and carried ~30 builtins (echo, dir, type, set, copy, del…). Direct execution does neither — which walks %PATH% only — so rtk err dir, rtk summary echo hi, and rtk err mytool with mytool.exe in the cwd go from working to exit 127 on Windows. Unix loses nothing equivalent (sh does not search ., and echo/test/pwd all exist as binaries), which is why the Unix breaking-change section reads complete while the larger Windows delta is undocumented. All 10 new integration tests are #[cfg(unix)], so no runner sees any of this.

I could not execute this arm — I have no Windows machine, and the green test (windows-latest) job proves only that nothing covers it. The narrow ask is: child_args instead of args, one #[cfg(windows)] test, and a line in the breaking-change section. I have an unmerged branch making that encoding unforgettable at the type level (the ChildCommand newtype, #4103) — that is not a reason to hold this PR, and I am not asking you to wait for it; the one-line swap now is the right move and I will rebase mine onto it.

3. src/main.rs:245, :255, :336 — --shell is the first flag these three surfaces have ever had, and they are wired to swallow flag errors.

err/test/summary are PASSTHROUGH, not RTK_META_COMMANDS (src/core/constants.rs:11-33), so a clap parse error under them falls to run_fallback, which tries to exec a binary called err/test/summary. Forgetting the value after the new flag — the likeliest mistake with it — reports a file that does not exist:

$ rtk err --shell
merge-base:  [FAIL] Command failed (exit code: 2)                    exit 2
head      :  [rtk: No such file or directory (os error 2)]           exit 127

$ rtk summary --shell        head: [rtk: No such file or directory (os error 2)]   exit 127
$ rtk test --shell           head: <no output>                                     exit 0
$ rtk run --shell            head: error: a value is required for '--shell <SHELL>' but none was supplied   exit 2

run is right because it is in RTK_META_COMMANDS. The test arm is the one that worries me: asking for a shell and getting silent success. This is reach widening — the swallowing path is older than the PR, but nothing reached it on these commands until the PR gave them a flag. Moving err and summary into RTK_META_COMMANDS (plus the trailing_var_arg skip list in test_meta_commands_reject_bad_flags) makes both print the clap error at exit 2 with the suite green; test cannot move, since it deliberately shadows the POSIX test binary, so it needs --shell validated by hand the way require_single_script already validates arity.

---

Optional — will not hold merge; I will resolve these threads

• quote_for_display under-quotes, so the labels this PR adds are not re-runnable — which is most of what I asked for in #3911's optional list. It quotes on whitespace and quote characters only, leaving bare exactly the metacharacters the PR exists to stop interpreting: rtk err /bin/echo '*' '$HOME' 'a;b' 'x&&y' is recorded and printed as /bin/echo * $HOME a;b x&&y, while its doc comment says a label "reads back as the command that ran". Same string feeds -v "Running:", summary's [FAIL] Command: header and TestEcosystem::detect. Wants a shell-safe metacharacter set, not a whitespace set.
• rtk test '(' <cmd> ')' loses its subshell: merge-base exit 1, head exit 127 rtk: (: command not found. is_native_test_expression deliberately treats ( as a possible native prefix, but split_leading_negations only handles !, so the two functions now disagree about (. Either handle (/) beside !, or drop ( from is_native_test_expression so they agree.
• report_not_found drops the -v "Running:" line the normal path prints (rtk -v err rtk-no-such shows it on the merge-base, not on the head).
• Both bail! arms of command_from_args are unreachable. All three production callers run require_single_script first, which .exit()s on len != 1, and run never calls it — so "pass the shell command as one quoted argument after --shell" can only be seen from a unit test, while users always get require_single_script's differently-worded message. One rule, two texts, one reachable.
• require_single_script prints the top-level usage (Usage: rtk [OPTIONS] <COMMAND>) where the run surface prints Usage: rtk run …. Cosmetic, but it is the error people hit while learning the flag.
• argv[0] changes for path-y spellings: the child of rtk run ./pb/tool sees $0 = /abs/path/pb/tool where the merge-base's sh gave it ./pb/tool. Only affects multi-call binaries and tools that print usage from argv[0]; bare PATH names are unchanged.
• report_not_found books a second negative-savings row in rtk gain (rtk test … 0.0% → −60%). Pre-existing shape, mildly widened — the merge-base already books −125% for rtk err <missing> — so evidence went to #3702, which already tracks it, rather than a new issue. The obvious fix is wrong, and worth recording so nobody tries it: wrapping rendered in never_worse (as summary.rs does) makes it print the raw line and drops the [FAIL] line your own err_reports_an_unresolvable_program_as_exit_127 asserts — I applied it and watched that test fail. It needs a decision, not a one-liner.
• Doc residue from a fix(cli)!: src/hooks/init.rs:277 still reads rtk test <cmd> in the Test section, 60 lines above the new caveat under "Analysis & Debug"; INSTALL.md:323 and README_{fr,es,pt,ja,ko,zh}.md still show the old form. None carries shell syntax, so nothing breaks — they are just stale. Nothing documents the !-only negation restoration, or that --shell disables the native test passthrough.

Filed as follow-ups

• #3702 (rtk gain cannot represent a regression) — absorbs the negative-savings row above; I added the base-vs-head transcript there rather than opening a duplicate.

Checked and correct — no need to re-verify

• Argv preservation itself, which is the point of the PR: 'a b', *, $HOME, ;, &&, |, $(…), backticks and an embedded newline all arrive at the child intact, where the merge-base re-split and expanded every one of them. One win the summary undersells: an empty argument used to vanish entirely — rtk run printf '[%s]' "" x -- y gives [x][--][y] on the merge-base and [][x][--][y] here. 5000 arguments round-trip cleanly on both.
• The 127 + [FAIL] contract on all four surfaces. I broke EXIT_COMMAND_NOT_FOUND to 1 and watched err_reports_an_unresolvable_program_as_exit_127, test_and_summary_… and run_reports_… all fail — the claim is genuinely pinned, not incidentally true.
• rtk test ! <cmd>. I removed the negation and bang_before_a_command_still_runs_the_command failed, so that test pins it too. The is_native_test_expression / --shell / negation ordering in the Test arm is right: ! -d adir still reaches the real test, ! false negates, ! ! cancels.
• --shell arity → clap usage error, exit 2, on all three positional surfaces; clap's own requires/conflicts_with on run. The #3911 asymmetry is gone.
• The hook path is untouched: rtk rewrite output is identical base vs head for all four commands, and no rules.rs rule emits them. The blast radius really is direct CLI callers.
• The filter forfeits nothing: on a failing cargo build, base and head produce byte-identical filtered output, 60.4% savings, streams captured separately.
• display_args re-quoting does not disturb TestEcosystem::detect (it substring-matches, and the quotes sit outside the token), and it fixes the tracking label from #3911's optional list.
• The awareness body now teaches the argv form and --shell in RTK_INSTRUCTIONS, which was the other half of #3911's blocker 3.
• Startup got faster, not slower: rtk run /bin/true is 1.7 ms ± 0.2 against the merge-base's 2.1 ms ± 0.2 (hyperfine, 50 runs, 1.28× ± 0.16) — one less sh in the middle. Both well inside the 10 ms target.

Hypotheses built and dropped

• "Direct execution changes what the child sees on stdin." Piped input, /dev/null and no-TTY are identical base vs head on all four commands (printf 'a\nb' | rtk err /bin/cat, rtk run /bin/cat </dev/null). Dropped.
• "Signal exit codes regress." They improve: kill -TERM $$ gives 143 on the head where the merge-base's joined string produced a kill usage error and exit 2.
• "display_args quoting breaks ecosystem detection." rtk test cargo test --filter "a b" → cargo test --filter 'a b' → still Cargo.
• "resolved_command's PATHEXT/.CMD/.BAT fallback is lost on Windows." It is not: that resolution lives in which::which, which direct_command calls first, and which-8.0.1 reads PATHEXT. What the short-circuit does drop is the last-ditch Command::new(name) that searched rtk.exe's own directory and System32 — minor, and folded into blocker 2.
• "A bare PATH name that exists without +x should be 126." dash itself returns 127 there, and so does the head. Parity, not a gap — which is why the prototype's check is restricted to path-y spellings.
• "The --shell flag collides with a wrapped tool's own --shell." trailing_var_arg keeps everything after the first positional literal, and clap has no infer_long_args. No collision.

Your questions

• "[ ] CI for Linux and Windows (cmd /C, -Command flag selection)" → Answered, partly. No CI run had ever been created for this branch; I approved it and all three test jobs are green, so this is the first branch signal you have had. But it is weaker than it looks: every one of the 10 new integration tests is #[cfg(unix)], so the Windows job compiles the new code and exercises none of it. That gap is blocker 2, and one #[cfg(windows)] test closes the part of it I can ask for.

Next

Three things, then this merges: map the spawn-side failures (and 126) through the runners' own failure path the way the resolution-side ones now are; on Windows use child_args plus one #[cfg(windows)] test and a breaking-change line about cmd's builtins and cwd resolution; and make --shell report its own misuse on err/test/summary.

Fixup 4c2da904 prepared, not pushed — yours whenever you want it. It does the two low-hanging pieces: resolves an explicit shell without a variable Command::new, which clears the semgrep job without suppressing a security rule (verified with the repo's own .semgrep.yml in the semgrep Docker image: 1 blocking finding → 0); and adds explicit_shell_by_name_runs_the_named_shell, because the two tests that exercise a named shell both start with let Ok(fish) = which::which("fish") else { return; } and pass vacuously wherever fish is absent — which is every CI runner. I proved that gap rather than assumed it: with the named-shell branch replaced by one that resolves the shell and then ignores it, all eleven existing tests still passed; the new test is the only one that fails. fmt clean, clippy 0, 3865 passed, and I diffed the fixup binary against yours over 11 rows covering every --shell form and the missing-shell error — identical output and exit code on all of them.

The 126 prototype (f5cc836f, 123 insertions, 3866 passed) is separate and yours to take or leave.

Rounds: 1/3. Threads: 3 open (all waiting on you), 0 resolved this round.

[KuSh]

One addendum to the "Optional" list above, now that I have closed #3002 in favour of this PR.

#3002 attacked the same defect by re-quoting argv before joining it for the shell, which this PR obsoletes — but its quoting helper is better than quote_for_display for the job quote_for_display still has, and it is worth lifting rather than rewriting:

let is_safe = !arg.is_empty()
    && arg.chars().all(|c| {
        c.is_ascii_alphanumeric() || matches!(c, '_' | '-' | '.' | '/' | ':' | '=' | '@')
    });

An allowlist is the right shape here. quote_for_display currently decides with a denylist — [' ', '\t', '\n', '\'', '"', '\\'] — so every other metacharacter stays bare in the label, which is why rtk err /bin/echo '*' '$HOME' 'a;b' 'x&&y' is recorded and printed as /bin/echo * $HOME a;b x&&y. A denylist has to enumerate the dangerous set correctly forever; an allowlist only has to enumerate the boring set once.

The escaping below it in #3002 splits on cfg!(target_os = "windows"), which you will not want as-is — that is a question about a child process's encoding, not about a label, and it belongs with ChildArgExt rather than here. It is the predicate that is worth taking.

[pashifika]

@KuSh — all three blockers addressed on 482d67d. Round-2 head is rebased on develop @ 0924356 still; say the word if you want a fresh rebase before you re-run.

Blocker 1 — fixed as a class, both arms. Launch::NotFound(String) became Launch::Unrunnable { message, code }, so the answer carries the code rather than assuming 127:

• pre-spawn: which is still asked first, and a spelling that addresses the filesystem is then stat'd, so a directory and a file without +x report 126. A bare PATH name stays 127 even when a non-executable file of that name exists — the parity you measured against dash.
• post-spawn: spawn_failure maps the io::Error out of the runner's own Err — ENOENT → 127, EACCES/EISDIR → 126, ENOEXEC → 126 — and renders it through the same failure path. No anyhow chain reaches stderr for any of them.
• third arm: an unresolvable --shell name now answers like an unrunnable program (127 + its own line) instead of Failed to prepare.

$ rtk err ./crlf.sh                 # '#!/bin/sh\r\n'
[FAIL] Command failed (exit code: 127)
  rtk: /tmp/…/crlf.sh: command not found                        exit 127
$ rtk run ./bindir/noexec-prog      # mode 644
rtk: /tmp/…/bindir/noexec-prog: Permission denied               exit 126
$ rtk run ./adir2                   # a directory
rtk: /tmp/…/adir2: Is a directory                               exit 126

tests/run_shell_test.rs gained an_unexecutable_path_reports_exit_126 (all four surfaces), a_directory_reports_exit_126, spawn_failures_keep_the_shell_contract (CRLF shebang + a file the kernel refuses; the second asserts 126-or-127 and never a spawn error, because Linux reports ENOEXEC where macOS falls back to sh and gets 127 itself) and an_unresolvable_shell_reports_the_shell_contract. I did not take the prototype — thank you for offering it; writing it against the same shape was faster than reconciling two diffs, and the errno mapping stayed out of src/core/stream.rs exactly as you suggested: it reads the error the four surfaces already receive.

Blocker 2 — child_args, plus the Windows delta documented. direct_command passes caller argv through ChildArgExt::child_args, and shell_command uses child_arg for the script. tests/run_shell_test.rs grew a #[cfg(windows)] mod windows: PATH resolution, an argument carrying a space and a " reaching the child intact, and the 127 contract. README now states what cmd /C carried and direct execution does not — the working-directory search and cmd's ~30 builtins (rtk run -c 'dir' or rtk err --shell cmd 'echo hi' is the replacement) — and the argv[0] spelling change from your optional list is in the same paragraph.

Blocker 3 — the flag reports its own misuse. err and summary joined RTK_META_COMMANDS (with the trailing_var_arg skip in test_meta_commands_reject_bad_flags). test cannot, as you said, so run_fallback checks for RTK's own --shell in argv before falling through — a command that asked for a shell never silently execs the POSIX test:

$ rtk err --shell        error: a value is required for '--shell <SHELL>' but none was supplied   exit 2
$ rtk test --shell       (same)                                                                   exit 2
$ rtk summary --shell    (same)                                                                   exit 2

a_missing_shell_value_reports_a_flag_error covers all four surfaces.

semgrep. src/core/shell.rs no longer constructs a Command at all: the explicit shell is resolved and then built through core::utils::resolved_command, which is where RTK's PATHEXT/.CMD handling lives anyway. That leaves dynamic-command-execution nothing to flag in the new file, without a suppression. Your fixup's second half is in too — explicit_shell_by_name_runs_the_named_shell names sh, so the explicit-shell branch is exercised on every runner rather than skipping wherever fish is absent.

From the optional list: display_args quotes the full shell metacharacter set (*, $, ;, &, |, <, >, backtick, #, …), so rtk err /bin/echo '*' 'a;b' records and prints /bin/echo '*' 'a;b'; rtk test '(' cmd ')' keeps its grouping (split_leading_negations handles (/) beside !, so it and is_native_test_expression agree again); report_unrunnable prints the -v "Running:" line; the arity rule has one wording, core::shell::SHELL_ARITY_MESSAGE, quoted by both the clap error and command_from_args; the usage line now comes from the subcommand that was asked; and the stale docs are refreshed — RTK_INSTRUCTIONS' Test line, INSTALL.md, and all six translated READMEs.

Left alone: the negative-savings row. You are right that never_worse here drops the [FAIL] line the 127 test asserts — I reproduced that — so it stays with #3702 where the decision belongs.

cargo fmt --all --check                  clean
cargo clippy --all-targets -D warnings   clean
cargo test --all                         3,875 passed, 0 failed, 8 ignored

(macOS 26.6.2 arm64, rustc 1.97.1.) CI needs approving again on the new head — the Windows job now has three tests that actually exercise the new path.

[pashifika]

CI caught the one thing I could not: windows::quoted_arguments_reach_the_child_intact failed on 482d67d, and the failure is the test, not the encoding.

thread 'windows::quoted_arguments_reach_the_child_intact' panicked at tests\run_shell_test.rs:385:
"a b" "c\"d"

That is cmd /C echo printing back the MSVC/libuv encoding child_args applies — cmd.exe does not parse backslash escapes, so it echoes "c\"d" verbatim. I picked the wrong witness: the argument reached the child exactly as intended, and the only thing that misreported it was cmd's own parsing.

Fixed on 2a5a793 by making rtk the child instead. rtk rewrite prints the command string it was handed, so the argument comes back unchanged only if the quote survived encoding on the way out and parsing on the way in — which is the child_args claim (#3728):

rtk run <rtk.exe> rewrite 'git status "a b"'   →   rtk git status "a b"

HOME/USERPROFILE/XDG_CONFIG_HOME are isolated so the verdict does not depend on the runner's settings files, and the exit code is deliberately not asserted for the same reason.

Also ran cargo check --target x86_64-pc-windows-gnu --all-targets locally this time (clean), which is the part of your round-1 note about the Windows gap I could reproduce without a Windows machine. The other ten jobs were green on 482d67d, including semgrep security scan, so this was the only outstanding failure.

#4127 carries the same commit, since it is stacked on this branch.

[KuSh]

Round 2 of 3 — CHANGES REQUESTED

Claim: rtk run|err|test|summary stop joining positional argv into a string for an implicit sh -c/cmd /C and execute the program directly; a script becomes explicit via rtk run -c or --shell <shell>; exit 127 and the [FAIL] contract survive the switch.

Scope: accept (round 1, frozen).

Ran: merge-base 0924356b vs head 2a5a7933, release-built with one CARGO_TARGET_DIR each (md5 32ae4fed… / b04b3483…, distinct; the merge onto current develop feac25d1 got a third). You pushed 2a5a7933 while I was running, so I re-fetched and rebuilt it in a fourth target dir: it is md5-identical to 482d67db's binary, which is what lets every behaviour transcript below stand for the current head — the commit is tests-only. ~70 rows: the 126/127 classifier against dash on nine path shapes × four surfaces, all three arms of round 1's blocker 1, --shell misuse × four surfaces, the !/(/) spellings, argv boundaries incl. empty-string args, exit 42/7, signal 143, piped and /dev/null stdin, empty argv, -v, tee/recovery, and the passthrough tools that carry their own --shell. Invariants: every branch of the new decision (Ready / Unrunnable pre-spawn / Unrunnable post-spawn / shell / native-test / negation); split streams on a failing cargo build; under the hook (rtk rewrite on all four commands, identical base vs head). Savings 65.5% vs the 20% floor (CONTRIBUTING.md), filtered output byte-identical base vs head. fmt clean, clippy 0, cargo test --all 3875 / 0 on the head and 3985 / 0 merged onto current develop (real cargo exit status).

Matrix: executed round 1 @790a747a; rerun in full this round.

CI @2a5a7933: had never been created — I approved it, as on the previous head. Now 11/11 green, Windows included. 482d67db failed test (windows-latest) on quoted_arguments_reach_the_child_intact, and 2a5a7933 is your fix for exactly that — the right one, and it landed before I could ask. So CI is not what holds this round; the two items below are.

---

Round 1's blocking set — all three closed

The work here is good, and the two items below are consequences of these fixes rather than things missed in them.

1. The exit-code class is closed, both arms. The head matches dash exactly on: no-+x file (126 "Permission denied"), directory (126 "Is a directory"), CRLF shebang (127), ENOEXEC binary garbage (126 "cannot execute binary file"), missing interpreter (127), missing program (127), bare name with a non-executable file of that name in the cwd (127 — the parity that is easy to lose), relative nonexistent (127), unresolvable --shell (127 with its own line). Four surfaces, no anyhow chain anywhere. Mutation-checked: forcing EXIT_COMMAND_NOT_EXECUTABLE to 127 fails a_directory_reports_exit_126 and an_unexecutable_path_reports_exit_126; making spawn_failure always return None fails spawn_failures_keep_the_shell_contract. Both groups genuinely pin the contract. One residual arm is in Optional.
2. child_args is in on both direct_command and shell_command, the README documents the cwd search, the cmd builtins and the argv[0] spelling, and 2a5a7933 replaces the witness that could not work. See the Windows group in Optional for what is left.
3. --shell misuse reports itself on all four surfaces, test included, where the merge-base silently exited 0. The mechanism is blocker 1 below.

Blocks merge (2 — round 1's set is closed; both of these are newly introduced)

1. src/main.rs:1582 — the --shell scan in run_fallback is unscoped, so it breaks every passthrough tool with its own --shell, and the hook emits that spelling.

It runs over all of argv for any command that failed clap parsing, including unknown subcommands that used to fall through to raw execution:

$ rtk just --shell bash --version
merge-base:  just 1.49.0                                   exit 0
head      :  error: unrecognized subcommand 'just'         exit 2

$ rtk hyperfine --shell=none -w1 -r2 /bin/true
merge-base:  Benchmark 1: /bin/true …                      exit 0
head      :  error: unrecognized subcommand 'hyperfine'    exit 2

$ rtk rewrite 'just --shell bash --list'
head      :  rtk just --shell bash --list

Both tools are installed here, and the last line is the point: the hook produces the broken spelling, so this fires in the normal agent path, not only by hand. hyperfine -S none is unaffected — it is the literal --shell/--shell= spelling. The check also sits before the TOML-filter lookup, so a project filter for such a tool never gets a chance.

Class: any passthrough tool with a --shell flag — just, hyperfine (-S/--shell), mise … --shell. One site, and only test needs the guard.

Fix, verified rather than suggested. err and summary need nothing here: the RTK_META_COMMANDS check eight lines above (src/main.rs:1574) already runs first and they are in that list now, so the two changes overlap — and the overlap is what costs the passthrough.

if args[0] == "test"
    && args.iter().any(|arg| arg == "--shell" || arg.starts_with("--shell="))
{
    parse_error.exit();
}

Built from a tree I asserted carried the patch, own CARGO_TARGET_DIR, own binary:

                                          head   scoped
just --shell bash --version               2      0      ← restored
hyperfine --shell=none -w1 -r2 /bin/true  2      0      ← restored
rtk test --shell                          2      2
rtk err --shell                           2      2
rtk summary --shell                       2      2
rtk test --shell sh a b                   2      2

cargo test --all on it: 3876 passed, 0 failed, including your test_meta_commands_reject_bad_flags and a_missing_shell_value_reports_a_flag_error.

2. src/main.rs:1972 — the new ( … ) strip does not compose with the negation strip, so rtk test ! ( cmd ) reports success for a command that never ran.

The group strip runs once, at :1972, before the take_while(|t| *t == "!") at :1979. A leading ! therefore hides the parens from it; the group becomes the program (, which is not found (127), and negations % 2 == 1 turns that 127 into i32::from(false) = 0.

$ rtk test ! ( /bin/true )
merge-base:  exit 1
head      :  OUTPUT (last 5 lines):
             rtk: (: command not found                       exit 0    ← reported as a pass
dash  ! ( /bin/true ):                                       exit 1

$ rtk test ! ( /bin/false )    head: exit 0 — right code, wrong reason: nothing ran
$ rtk test ( ( /bin/false ) )  merge-base: exit 1   head: exit 127   (dash: 1)

A test wrapper reporting pass for a command it could not launch is the one shape worth blocking on, and it is the shape the rest of this PR removes everywhere else. Class: of the five spellings the new code claims, 3 are unhandled — ! ( … ), ( ( … ) ), ! ! ( … ). The two that work, ( cmd ) and ( ! cmd ), are exactly the two in grouped_and_negated_commands_keep_their_exit_codes. The shape that covers all five is to strip groups and negations in one loop until neither applies, instead of one pass of each; I have not prototyped that, so treat it as the shape and not a verified patch. Test that closes it: those five spellings, asserted against dash.

Optional — will not hold merge; threads resolved by me

A Windows group — three things that want one pass on a Windows box, none of which I can execute. I am not holding the PR on any of them.

• Whether is_batch_program (src/core/utils.rs:572) should recognise cmd.exe as a program. You read the 482d67db failure as the witness being wrong rather than the encoding, and I think that reading is defensible: direct_command cannot know the child's parser, child_args targets the MSYS/libuv convention on purpose, and the .bat/.cmd exclusion exists for shims that std invokes through cmd.exe. The open question is only whether cmd.exe named directly belongs in that same exclusion — today it does not, because the predicate decides from the file extension and which("cmd") yields …\cmd.exe, so rtk run cmd /C 'echo a"b' emits a literal backslash on Windows. It is not clearly worse than the merge-base, which mangled the same input differently, so it is an issue at most rather than a blocker. powershell.exe/pwsh.exe are a third parser and also unrecognised. Say the word and I will file it with this transcript; I am not asking you to decide it inside this PR.
• src/core/shell.rs:105 matches ENOEXEC = 8 on raw_os_error() with no cfg(unix) gate. 8 is ENOEXEC on Linux/macOS/BSD, but raw_os_error is a Win32 code on Windows, where 8 is ERROR_NOT_ENOUGH_MEMORY — so a spawn that fails for want of memory would be reported as rtk: <prog>: cannot execute binary file, exit 126. Windows' bad-format code is 193 and nothing maps it. A one-line cfg(unix) gate is the whole fix. (ErrorKind::IsADirectory is fine — stable 1.83, MSRV 1.91.)
• The new witness may still not discriminate child_args from args. Using rtk as the child tests a round-trip through CommandLineToArgvW, which is the convention std's own escaping already targets, so I would expect the assertion to hold with plain .args too. I could not execute this — it needs Windows, and the divergence child_args exists for is against MSYS/Cygwin/libuv children, not Rust ones. If that reasoning is right, a real discriminator needs an MSYS, Cygwin or Node child. Worth one test if you can run Windows locally; not a finding I will stand behind without having run it.

And four Unix-side notes:

• classify_unrunnable's Err(_) arm reads an unsearchable parent as a missing program. metadata answers EACCES rather than yes-or-no when a directory on the way is mode 000, so existence is unknown from there and 127 asserts what the call never established: rtk run ./nosearch/prog gives 127 where the merge-base and dash both give 126. Last arm of blocker 1; it is in the fixup below.
• require_single_script's usage line names the POSIX utility: Usage: test [OPTIONS] [COMMAND]..., where run prints Usage: rtk run …. This exists because round 1 asked for the subcommand's usage, so it is mine to own — also in the fixup.
• program_name(&[], shell) yields the literal "command" for rtk run -c with no --shell: PATH=/nonexistent rtk run -c 'echo hi' → rtk: command: command not found. The code is right, the message names a program that does not exist; exposing default_shell() through program_name fixes it. The exit code did improve — 127 here, 1 on the merge-base.
• A bare rtk err / rtk summary still answers with a doubled anyhow chain and exit 1 where the merge-base exited 0 — unchanged from round 1. Both take a trailing var-arg, so clap never errors, the RTK_META_COMMANDS entry cannot reach it, and command_from_args's bail!("command is required") surfaces raw. The consistent answer is the clap usage error you already produce for --shell with no value.

Checked and correct — no need to re-verify

• The 126/127 classifier against dash, nine path shapes × four surfaces; both test groups mutation-checked.
• --shell misuse on all four surfaces at exit 2, one wording via SHELL_ARITY_MESSAGE shared by the clap error and command_from_args.
• (/) and ! in the spellings your test covers: ( cmd ), ( ! cmd ), ! cmd, ! ! cmd, native test -d/-f/-e.
• -v "Running:" is back on the unrunnable path.
• Argv boundaries including the empty-string argument the merge-base dropped; 5000 args; exit 42/7; signal 143; piped and /dev/null stdin all identical to the merge-base.
• The hook path is untouched: rtk rewrite identical base vs head for all four commands.
• The filter forfeits nothing: 65.5% savings, filtered stderr byte-identical base vs head on a failing cargo build.
• Merges clean into current develop (feac25d1); 3985 tests pass there.

Hypotheses built and dropped

• "spawn_failure can misattribute an internal I/O error as command not found." The shape I most wanted to find, since it is applied to the whole Err from the runner rather than to the spawn call alone. Not reachable: an unwritable RTK_DB_PATH (missing dir, and mode-500 dir), a nonexistent and a mode-500 RTK_TEE_DIR with 4000 lines of failing output, and a closed stdout all exit with the child's own code and never enter spawn_failure — tracking and tee failures are swallowed before they propagate. _ => None covers the rest.
• "The widened display_args breaks a consumer." TestEcosystem::detect is a pure contains and quoting only wraps, so cargo test --features=foo still detects Cargo, and a real failing cargo test gives identical filtered output base vs head. categorize_command reads parts[1], the tool name, never quoted.
• "err/summary joining RTK_META_COMMANDS changes something else." Readers enumerated: one live (run_fallback), one with no production caller (toml_filter::is_rtk_reserved_command), two test-only. No hook, filter or docs path affected.
• "A bare PATH name whose non-executable file exists should be 126." dash answers 127 there and so does the head; only the message differs.

Your questions

• "Round-2 head is rebased on develop @ 0924356 still; say the word if you want a fresh rebase before you re-run." → No rebase needed. It merges cleanly into current develop (feac25d1) and the merged tree passes 3985 tests. Rebase only if you want the history tidy.
• "CI needs approving again on the new head." → Done, on both heads. It is how the Windows test surfaced, and how you came to fix it before I could ask.
• "I did not take the prototype … writing it against the same shape was faster than reconciling two diffs." → Right call, and what you wrote is better than what I offered: carrying message beside code is cleaner than my separate reason, and keeping the errno mapping out of src/core/stream.rs is exactly right.
• "the failure is the test, not the encoding … the argument reached the child exactly as intended, and the only thing that misreported it was cmd's own parsing." → Agreed on the diagnosis, and I reached the same one independently from the job log and the pinned quote_arg_for_child(r#"a"b"#) == r#""a\"b""# unit test. Where I would leave a question open rather than close it is above, in the Windows group: the encoding is right for the child child_args is aimed at, and the remaining question is only whether a directly-named cmd.exe should join the .bat/.cmd exclusion. That is not this PR's job unless you want it to be.
• "cargo check --target x86_64-pc-windows-gnu --all-targets locally this time (clean)" → Good, and that is the right substitute for a Windows box. It is also why the remaining Windows items in this round are framed as questions rather than findings: a compile check cannot settle any of them, and neither can I.
• "#4127 carries the same commit, since it is stacked on this branch." → Noted, and it cuts both ways: whatever you push for the two blockers below will need to reach #4127's base too, so it is worth doing here first and rebasing that one after, rather than fixing it twice.

Next

Two things: scope the --shell scan to test, and make the group/negation strip compose so ! ( cmd ) cannot report a pass.

Fixup e3f3ef5d prepared, not pushed — the EACCES arm and the usage-line prefix, 47 lines with a test that fails without it. fmt clean, clippy 0, 3876 passed, and diffed against your binary over 12 rows: only the two intended rows move. Yours whenever you want it, before or after your next push; I am holding it so it does not land on top of a change you are already making.

Rounds: 2/3. Threads: 2 open (both waiting on you), 3 resolved this round (round 1's blocking set).

[pashifika]

@KuSh — both blockers fixed on 309c4f0, along with the four Unix-side items from Optional.

Blocker 1 — the --shell scan is scoped to test. Your patch, unchanged in substance: err and summary are covered by the RTK_META_COMMANDS check eight lines above, so only test — which cannot join that list — needs the guard. The overlap was the whole cost.

rtk just --shell bash --version       runs again
rtk hyperfine --shell=none …          runs again
rtk test|err|summary|run --shell      error: a value is required …   exit 2

Blocker 2 — groups and negations compose. One loop that peels !s and a wrapping ( … ) until neither applies, and a group is only peeled when its parentheses balance across the whole command, so ( a ) b ( c ) keeps both. Against sh on all five spellings the code claims:

                 rtk  sh
! ( true )        1    1
! ( false )       0    0
( ( false ) )     1    1
! ! ( true )      0    —   (sh rejects `! !`; `test`'s own `!` cancels, as in round 1)
( ! false )       0    0

grouped_and_negated_commands_keep_their_exit_codes now covers all eight spellings and also asserts the command actually ran — !String::from_utf8_lossy(&output.stdout).contains("command not found") — so a 0 that came from a 127 fails the test rather than passing it.

From Optional, all four Unix-side items:

• classify_unrunnable no longer reads an unsearchable parent as missing: metadata answering EACCES never established existence, so it reports 126 like dash instead of asserting 127.
• The arity usage line reads Usage: rtk test [OPTIONS] [COMMAND]... — the subcommand is cloned with bin_name("rtk <sub>"), which is what was missing when round 1 asked for the subcommand's own usage.
• program_name names the platform default shell, so PATH=/nonexistent rtk run -c 'echo hi' says rtk: sh: command not found rather than naming a program called command.
• Bare rtk err / rtk summary answer clap's usage error at exit 2 (required = true on the positional), replacing the doubled anyhow chain at exit 1. rtk test keeps its POSIX behaviour and is untouched.

And one from the Windows group I could act on: the bad-format code is cfg-gated — 8 is ENOEXEC on Unix, but on Windows raw_os_error is a Win32 code where 8 is ERROR_NOT_ENOUGH_MEMORY, so that arm would have mislabelled an out-of-memory spawn; 193 is mapped there instead. The other two Windows questions I am leaving as you framed them: whether a directly-named cmd.exe belongs in the .bat/.cmd exclusion is is_batch_program's question, not this PR's, and a real child_args-vs-args discriminator needs an MSYS/Cygwin/Node child, which I cannot run either.

I did not take e3f3ef5d — the EACCES arm and the usage-line prefix are both in 309c4f0, written against the same shape, so the fixup would now conflict rather than apply. Thank you for holding it.

cargo fmt --all --check                          clean
cargo clippy --all-targets -D warnings           clean
cargo test --all                                 3,875 passed, 0 failed, 8 ignored
cargo check --target x86_64-pc-windows-gnu       clean

(macOS 26.6.2 arm64, rustc 1.97.1.) CI needs approving on the new head; 2a5a7933 was the last one you approved and it went 11/11.

[ryanduguid]

Native Windows validation at 309c4f0 confirms that run, err, test and summary preserve spaced, empty, quoted and literal metacharacter arguments. A child exiting 7 returns 7 through all four runners. Calling err from PowerShell, cmd and Git Bash also preserves the tested arguments and exit code. The missing-program result is 127 and a split explicit-shell script returns usage error 2.

One explicit-shell case still fails: rtk run -c '"C:\path with spaces\rtk.exe" --version'. The child is never started, and cmd reports the escaped quoted path as an unknown command. The same command passed directly to cmd /d /s /c with its outer command quoting works. src/core/shell.rs:163 passes the script through child_arg; cmd needs its own command-string quoting rules.

I have prepared a Windows-only regression that copies the test RTK binary into a directory containing spaces, then invokes --version through run -c. It fails at this head with the error above. The existing three Windows run_shell_test tests pass.

The regression belongs in tests/windows_shell_quoting_test.rs:

#![cfg(windows)]

use std::process::Command;

#[test]
fn explicit_cmd_script_accepts_a_quoted_executable_path() {
    let directory = tempfile::tempdir().expect("create fixture directory");
    let spaced = directory.path().join("directory with spaces");
    std::fs::create_dir(&spaced).expect("create spaced directory");
    let executable = spaced.join("rtk.exe");
    std::fs::copy(env!("CARGO_BIN_EXE_rtk"), &executable).expect("copy fixture executable");
    let script = format!("\"{}\" --version", executable.display());
    let output = Command::new(env!("CARGO_BIN_EXE_rtk"))
        .args(["run", "-c", &script])
        .env("RTK_TELEMETRY_DISABLED", "1")
        .output()
        .expect("run explicit cmd script");

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    assert!(String::from_utf8_lossy(&output.stdout).starts_with("rtk "));
}

The quoted-path failure also reproduces at #3505 3f285de and #4206 c72f693. #3505 preserves direct argv across all four runners; #4206 preserves it for err, within that PR's stated scope. Their single-string shell compatibility differs from #4125's explicit-shell contract.

This review did not run the complete RTK suite for each proposal or test Linux/macOS. It leaves the choice of compatibility contract with the maintainers.

[ryanduguid]

Following up on the quoted-path failure: the patch below fixes it on Windows at 309c4f0b5007efd768e2ae0f3e9b4dc0716e709b and is ready to fold into this PR.

At src/core/shell.rs:163, pass cmd /S /C followed by the script in one extra pair of outer quotes through CommandExt::raw_arg. This preserves the script's own quotes through cmd's command-string parsing. It follows Rust's cmd example and cmd's documented /S behaviour. The existing shell classification, program resolution and single-script validation remain in place. Direct argv and other shells keep their existing encoding.

Both new tests fail on the unchanged head and pass with the patch. They cover paths containing spaces, & and parentheses, embedded quotes, trailing backslashes, redirection and exit code 7 across run, err, test and summary, including default cmd and upper-case CMD.EXE.

Native Windows, Rust 1.91.0 MSVC:

• cargo fmt --all: passed.
• cargo clippy --all-targets: passed without warnings.
• cargo test --all: 3,759 passed, 8 ignored.
• A separate comparison of 16 cases before and after confirms that the quoted cmd script now delivers its arguments and exit code 7. Direct argv, PowerShell and Git Bash cases retain their earlier results.

Linux/macOS, upstream CI, the 8 ignored tests, release smoke tests and performance benchmarks remain unverified. This patch only changes explicit cmd script execution on Windows.

Patch against 309c4f0

diff --git a/src/core/shell.rs b/src/core/shell.rs
index 0cc5c2f..c7e22f0 100644
--- a/src/core/shell.rs
+++ b/src/core/shell.rs
@@ -160,7 +160,16 @@ pub fn shell_command(script: &str, shell: Option<&str>) -> Result<Launch> {
     }
 
     let mut command = resolved_command(program);
-    command.arg(command_flag(program)).child_arg(script);
+    let flag = command_flag(program);
+    #[cfg(windows)]
+    if flag == "/C" {
+        use std::os::windows::process::CommandExt;
+
+        // /S strips the extra outer quotes; C-runtime escaping would alter the script.
+        command.args(["/S", flag]).raw_arg(format!("\"{script}\""));
+        return Ok(Launch::Ready(command));
+    }
+    command.arg(flag).child_arg(script);
     Ok(Launch::Ready(command))
 }
 
diff --git a/tests/windows_shell_quoting_test.rs b/tests/windows_shell_quoting_test.rs
new file mode 100644
index 0000000..7337cba
--- /dev/null
+++ b/tests/windows_shell_quoting_test.rs
@@ -0,0 +1,83 @@
+#![cfg(windows)]
+
+use std::path::Path;
+use std::process::Command;
+
+const RUNNERS: &[&[&str]] = &[
+    &["run", "-c"],
+    &["run", "--shell", "cmd", "-c"],
+    &["run", "--shell", "CMD.EXE", "-c"],
+    &["err", "--shell", "cmd"],
+    &["test", "--shell", "cmd"],
+    &["summary", "--shell", "cmd"],
+];
+
+fn rtk(home: &Path) -> Command {
+    let mut command = Command::new(env!("CARGO_BIN_EXE_rtk"));
+    command
+        .current_dir(home)
+        .env("HOME", home)
+        .env("USERPROFILE", home)
+        .env("APPDATA", home)
+        .env("LOCALAPPDATA", home)
+        .env("XDG_CONFIG_HOME", home)
+        .env("RTK_TELEMETRY_DISABLED", "1");
+    command
+}
+
+#[test]
+fn explicit_cmd_script_accepts_a_quoted_executable_path() {
+    let directory = tempfile::tempdir().expect("create fixture directory");
+    let spaced = directory
+        .path()
+        .join("directory with spaces & (parentheses)");
+    std::fs::create_dir(&spaced).expect("create spaced directory");
+    let executable = spaced.join("rtk.exe");
+    std::fs::copy(env!("CARGO_BIN_EXE_rtk"), &executable).expect("copy fixture executable");
+    let script = format!(
+        "\"{}\" --version > \"version output.txt\"",
+        executable.display()
+    );
+    for prefix in RUNNERS {
+        let output = rtk(directory.path())
+            .args(*prefix)
+            .arg(&script)
+            .output()
+            .expect("run explicit cmd script");
+
+        assert!(
+            output.status.success(),
+            "{prefix:?}: {}",
+            String::from_utf8_lossy(&output.stderr)
+        );
+        let result = directory.path().join("version output.txt");
+        let contents = std::fs::read_to_string(&result).expect("read child version");
+        assert!(contents.starts_with("rtk "), "{prefix:?}");
+        std::fs::remove_file(result).expect("remove checked output");
+    }
+}
+
+#[test]
+fn explicit_cmd_script_preserves_quotes_redirection_and_exit_code() {
+    let directory = tempfile::tempdir().expect("create fixture directory");
+    let script =
+        r#"echo "a&b">"result file.txt" & echo "C:\folder\">>"result file.txt" & exit /b 7"#;
+
+    for prefix in RUNNERS {
+        let output = rtk(directory.path())
+            .args(*prefix)
+            .arg(script)
+            .output()
+            .expect("run explicit cmd script");
+
+        assert_eq!(output.status.code(), Some(7), "{prefix:?}");
+        let result = directory.path().join("result file.txt");
+        let contents = std::fs::read_to_string(&result).expect("read redirected output");
+        assert_eq!(
+            contents.lines().collect::<Vec<_>>(),
+            ["\"a&b\" ", "\"C:\\folder\\\" "],
+            "{prefix:?}"
+        );
+        std::fs::remove_file(result).expect("remove checked output");
+    }
+}

[bharvey42]

Independent confirmation of @ryanduguid's cmd patch, on a different setup: Git Bash, GNU toolchain, and a node child that writes the argv it receives to a file.

• 309c4f0 plus an equivalent change (the same cmd.exe /S /C "<script>" command line). node argv.js "a b" c now arrives as ["a b","c"] through run -c and err/test/summary --shell cmd; unpatched it arrives as ["\"a","b\"","c"].
• Through run -c, a script that starts with "C:/Program Files/…/node.exe" followed by quoted args runs, and echo "x > y" & echo done keeps the redirect as data.
• The argv matrix, composition and exit codes are unchanged.
• /S and the outer quote pair both matter. Plain raw_arg(script) fixes the embedded quotes, but a script that starts with a quoted path and carries further quoted args still breaks, because cmd strips the first and last quote ('C:/Program' is not recognized).

The before-state on stable v0.50.0 (which already includes #3728), through all four runners:

Command	v0.50.0 result
'a b' c	["a","b","c"]
"x > stray"	["x"], plus an empty file stray
sh -c 'exit 7'	rc 0
nosuchcmd123	rc 1
"C:/Program Files/Git/cmd/git.exe" --version	'C:/Program' is not recognized

309c4f0 fixes each of them. The exit masking isn't Windows-only: on Linux, v0.50.0 returns 0 for sh -c 'exit 7' through all four.

[pashifika]

Rebased onto develop @ c75f159 — 09cc268. No behaviour change; the conflict was structural.

src/hooks/init.rs is gone on develop: it was split into src/hooks/init/, and RTK_INSTRUCTIONS now lives in src/hooks/init/mod.rs. Both of this branch's awareness-body edits moved there unchanged — the rtk err|summary <cmd> [args...] lines with the "argv runs directly, no shell" caveat, and the rtk test <cmd> [args...] line in the Test section. Everything else merged clean.

cargo fmt --all --check                  clean
cargo clippy --all-targets -D warnings   clean
cargo test --all                         4,009 passed, 0 failed, 8 ignored

Re-verified on the rebased binary: CRLF shebang → 127 with the [FAIL] line, non-executable path → 126 "Permission denied", directory → 126 "Is a directory", --shell with no value → exit 2 on all four surfaces, rtk test --shell sh a b → Usage: rtk test …, rtk test ! ( true ) → 1, bare rtk err → clap usage error.

#4127 is stacked on this branch and was rebased the same way (813d8fd).

[KuSh]

Round 3 of 3 — APPROVED, follow-ups filed

Claim: rtk run|err|test|summary stop joining positional argv into a string for an implicit sh -c/cmd /C and execute the program directly; a script becomes explicit via rtk run -c or --shell <shell>; exit 127 and the [FAIL] contract survive the switch.

Scope: accept (round 1, frozen).

Ran: merge-base c75f1590 vs head 09cc268a, release-built with one CARGO_TARGET_DIR each (md5 df4c0a87… / 13b74b1b…, distinct; the merge onto current develop a89a3149 got a third). ~55 rows: the 126/127 classifier against dash on the path shapes from rounds 1–2, both round-2 blockers and all five ( … )/! spellings the new code claims, --shell misuse × four surfaces, the four Optional items, argv boundaries including empty-string arguments, exit 42/7, signal 143, piped and /dev/null stdin, -v, and the passthrough tools that carry their own --shell. Invariants: every branch of the decision (Ready / Unrunnable pre-spawn / post-spawn / shell / native-test / negation); split streams on a failing cargo build; under the hook (rtk rewrite on all four commands, identical base vs head). Savings 65.2% vs the 20% floor (CONTRIBUTING.md), filtered output byte-identical base vs head. fmt clean, clippy 0, cargo test --all 4009 / 0 on the head and 4029 / 0 merged onto current develop (real cargo exit status).

Matrix: executed rounds 1 and 2; rerun in full this round.

CI @09cc268a: 11/11 green, Windows included.

---

Blocks merge (0 — round 2's set is closed)

Blocker 1 — the --shell guard. Scoped to args[0] == "test", with err/summary left to the RTK_META_COMMANDS check above it. rtk just --shell bash --version and rtk hyperfine --shell=none -w1 -r2 /bin/true run again at exit 0, while all four surfaces still answer a valueless --shell with clap's error at exit 2 and rtk test --shell sh a b reports the arity rule under Usage: rtk test ….

Blocker 2 — group/negation composition. One loop until neither applies, and strip_outer_group requires the parentheses to balance across the command. That is more than I asked for and the right call: ( a ) b ( c ) keeps both pairs. Against dash: ! ( true ) → 1, ! ( false ) → 0, ( ( false ) ) → 1, ( ! false ) → 0, ! ! ( true ) → 0. The reported pass that came from a 127 is gone, and the test now asserts the command actually ran rather than only its exit code — I reverted the loop to a single pass of each and grouped_and_negated_commands_keep_their_exit_codes caught it.

All four Optional items from round 2 landed as well, and the cfg-gated bad-format code (8 on unix, 193 on Windows) closes the one Windows question that did not need a Windows machine.

Filed as follow-ups

• #4288 — rtk run -c and --shell cmd lose a script's own quotes to cmd.exe. Pre-existing, not from this PR. Found on native Windows by @ryanduguid and independently confirmed by @bharvey42 on a different toolchain, with a patch and tests. The merge-base did Command::new("cmd").arg("/C").arg(&raw) — plain std escaping, no /S — so the quoted-path case fails on stable too, which @bharvey42's v0.50.0 before-state table confirms. One caveat is recorded there: for a script with embedded quotes the two are differently wrong and arguably worse now (stable dropped the quotes, the current code injects backslashes), which is why it is worth doing promptly. Same family as #3185, #3691 and #4102, distinct site.
• #4289 — an even, non-zero negation count returns the child's status instead of normalising it. rtk test ! ( ! sh -c 'exit 2' ) gives 2 where bash gives 1. Not a regression: the merge-base answered 0 there, a reported pass for a command that failed, so this moved into the right direction and stopped short of the right value. The new test cannot see it because its ! ! ( true ) case uses a child whose status is already 0.
• #4290 — two misuses still answer with an anyhow chain at exit 1 where SHELL_ARITY_MESSAGE's own doc promises a clap usage error: an empty --shell= value on the three positional surfaces (the merge-base exited 2), and a bare rtk test. Grouped because they are one question — which layer answers a misuse. On the second the exit code is now correct and the merge-base's was not: /usr/bin/test and dash's builtin both exit 1 with no arguments where the merge-base exited 0, so only the message is wrong.
• #3426 absorbed the last one, with a comment rather than a new issue: a global flag before the subcommand defeats run_fallback, which reads args[0] while args is env::args().skip(1). rtk -v test --shell execs -v and exits 127 instead of reporting the flag, and rtk -v gain --badtypo does the same on develop — so the assumption predates this PR, which inherits it at a second site. That issue already owns the root ("an unknown leading flag is treated as a program name"); this is its behavioural half.

Checked and correct — no need to re-verify

• The 126/127 contract across the path shapes and all four surfaces: no-+x file 126, directory 126, CRLF shebang 127, ENOEXEC garbage 126, missing program 127, bare name whose non-executable file sits in the cwd 127, unsearchable parent 126 — each matching dash, none surfacing an anyhow chain.
• Argv boundaries, including the empty-string argument the merge-base dropped; exit 42/7; signal 143; piped and /dev/null stdin identical to the merge-base.
• The hook path is untouched: rtk rewrite identical base vs head for all four commands.
• The filter forfeits nothing: 65.2% savings, filtered stderr byte-identical base vs head on a failing cargo build.
• The rebase is clean. This is the part I most wanted to check, because a rebase across a file split is where an edit goes missing: src/hooks/init.rs became src/hooks/init/ on develop, and the three awareness-body edits are byte-identical in src/hooks/init/mod.rs to what round 2 reviewed. The PR's delta to that file is exactly those three edits and nothing else, and no develop change to any file this PR touches was reverted.
• Merges clean into current develop (a89a3149); 4029 tests pass there.
• #4127 is in sync: its a9ca4942 is this branch's 09cc268a, so the stack carries these fixes rather than a stale copy.

Your questions

• "CI needs approving on the new head; 2a5a7933 was the last one you approved and it went 11/11." → Approved, and 11/11 again on 09cc268a.
• "Rebased onto develop @ c75f159 … Both of this branch's awareness-body edits moved there unchanged." → Verified, byte-identical, and nothing else in that file or the others moved. See "Checked and correct".
• "I did not take e3f3ef5d — the EACCES arm and the usage-line prefix are both in 309c4f0, written against the same shape." → Right call, and nothing of mine was better: your require_single_script is the same cloned-subcommand bin_name("rtk <sub>"), and your EACCES arm is the same condition. Dropped on my side.
• @ryanduguid and @bharvey42's Windows findings → not blocking this PR, because the defect predates it; filed as the follow-up above with their transcripts and patch credited. Thank you both — that is a platform neither the author nor I can run, and the before/after tables are what settled the severity question rather than leaving it to argument.

Next

Nothing — approving. Fixup 3598afc4 pushed: three tests, no behaviour change. Re-widening the --shell guard over all of argv, dropping the balance requirement from strip_outer_group, and dropping the EACCES arm each passed the whole suite unchanged, so none of the three fixes this round settled was pinned. a_passthrough_keeps_its_own_shell_flag runs a passthrough carrying its own --shell; a_group_that_closes_early_is_not_peeled uses the ( a ) b ( c ) shape the balance check exists for; an_unsearchable_parent_reports_exit_126 puts the program behind an unsearchable directory and leaves early under a uid that ignores the search bit, so a root test run does not fail spuriously. Each fails on exactly its own mutation and on no other, and a fresh reviewer who had not seen this round's reasoning confirmed that independently.

Rounds: 3/3. Threads: 0 open, 2 resolved this round.