[danieldutu]

Summary

Refs #4753. This covers the non-Python commands. pytest, mypy, pip, uv and sqlfluff are out of scope here; Protocol-zero-0 posted a patch for them in the issue. So I used Refs, not Fixes.

Defect

rtk <tool> -- zzz dropped the separator for 14 commands. I recorded the argv with a stub on PATH that writes its arguments to a file:

tool	before (develop 32b2ec4)	after
tsc, prettier, phpunit, phpstan, mvn	zzz	-- zzz
go, cargo, docker, kubectl (top level)	zzz	-- zzz
rspec, rubocop	--format json zzz	--format json -- zzz
vitest	run --reporter=json zzz	run --reporter=json -- zzz
jest	--no-watch --json zzz	--no-watch --json -- zzz
pest	--no-progress zzz	--no-progress -- zzz
rtk vitest -- run	run --reporter=json	run --reporter=json -- run
rtk jest -- run	--no-watch --json	--no-watch --json -- run

Root cause

clap's trailing_var_arg strips a -- that leads a variant's arguments, and an external_subcommand loses its leading -- the same way. The 10 args arms and the 4 Other arms above passed clap's vector to their module without calling core::args_utils::restore_double_dash.

jest and vitest had a second problem. should_skip_jest_arg and should_skip_vitest_arg (src/cmds/js/vitest_cmd.rs) dropped run, --json* and --watch* wherever they appeared, including after a --.

Fix

• src/lib.rs: run_cli calls a new restore_leading_double_dash(&mut cli.command) once, right after parsing. One or-pattern lists the 10 Vec<String> arms, and a second lists the 4 Other(Vec<OsString>) arms. Every module then receives the user's arguments as typed. Each module already appends those arguments after the options it injects, so the restored -- lands after --format json, --no-progress and the others without any per-module change.
• src/core/args_utils.rs: restore_double_dash_with_raw is now generic over AsRef<OsStr> + Clone. The new restore_double_dash_os reads std::env::args_os(), because std::env::args() panics on a non-UTF-8 argument. Existing callers compile unchanged.
• src/cmds/js/vitest_cmd.rs: the skip filters apply only before the -- that arg_tokenizer::injection_point reports.

I chose one shared point in the dispatch over 14 per-arm calls, as the issue proposed. It costs one function instead of 14 three-line blocks. The restore is idempotent, so modules that already call restore_double_dash internally are not affected. The Python arms can join by adding 5 names to the first or-pattern.

Evidence

• cargo test --test passthrough_double_dash_test: before the fix, test result: FAILED. 1 passed; 2 failed (14 of 14 tools listed). After the fix, test result: ok. 3 passed; 0 failed.
• cargo test: 4914 passed, 0 failed, 8 ignored across 48 suites. A unit test in vitest_cmd.rs, test_invocation_keeps_skipped_words_after_double_dash, pins the skip-filter boundary.
• cargo fmt --all -- --check and cargo clippy --all-targets -- -D warnings pass.
• I probed protected words with stubs. rtk go -- test, rtk cargo -- build, rtk docker -- ps and rtk kubectl -- get reach the Other arm and forward -- <word> with no injected flags. rtk mvn -- test forwards mvn -- test; the mvn router already reads goals after --, as its unit test everything_after_double_dash_is_a_goal pins.

Boundaries

• The new test file is tests/passthrough_double_dash_test.rs. It follows the existing *_double_dash_test.rs layout and is cfg(unix). I ran it on macOS only; Linux and Windows are left to CI.
• I did not change the mvnd, oc, sbt, gt, bun and deno Other arms, docker compose, or per-subcommand arms such as kubectl get, because the issue does not list them.
• rspec's -f detection and vitest's --reporter detection still scan past --. I did not change that pre-existing behavior.
• A non-UTF-8 argument still panics in a std::env::args() call elsewhere in rtk, both before and after this change (fix(cli): stop aborting on non-UTF-8 argv bytes #3224). The new OsString path adds no panic.

Question

Protocol-zero-0's patch for the Python half uses per-arm restore_double_dash blocks in the same match, near src/lib.rs:3574-3606. Do you want those 5 arms folded into restore_leading_double_dash as well, so that one place owns the rule? I can do that here, or split the jest/vitest skip-filter change into its own PR if you prefer.

Test plan

• cargo fmt --all && cargo clippy --all-targets && cargo test
• Manual testing: rtk <command> output inspected (see Evidence)

[CLAassistant]

All committers have signed the CLA.

[rtk-wshm-sync-bot]

wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type	bug-fix
🟡 Risk	medium

Summary

Fixes clap's trailing_var_arg and external_subcommand stripping the leading -- separator for 14 non-Python passthrough commands (tsc, prettier, phpunit, phpstan, mvn, pest, rspec, rubocop, vitest, jest, go, cargo, docker, kubectl), via a single restore_leading_double_dash call in run_cli plus an OsString-aware restore path for external subcommands. Also fixes jest/vitest argument skip-filters that were incorrectly stripping words like run/--json even after a user-supplied --.

Review Checklist

• Tests present
• Breaking change
• Docs updated

---

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

[Protocol-zero-0]

Reviewed 88e1f304 against 32b2ec43 on Linux. I found no blocker in the 14-command fix.

Independent argv checks used NUL-delimited recording stubs: 14 tools × 12 argument forms × two verbosity settings × exit 0/7, on both base and head (1,344 processes). Head preserves exact child argv and exit status in 664/672 combinations, fixing 544 base combinations. The remaining eight are the existing bare-separator fallback for rtk -v {go,cargo,docker,kubectl} --: both revisions exit 127 before invoking the child, with identical diagnostics. This PR does not introduce that behavior. Repeated separators, empty/space/Unicode/newline arguments, and run/--json/--watch after the boundary were included.

For your Python question: yes, please fold the five arms into this helper, keeping one dispatch entry point as #4753 requests. I tried adding Pytest | Mypy | Pip | Uv | Sqlfluff to the String-argument pattern on this exact head, without the earlier per-arm blocks. The existing five Python process regressions go from 5 failed to 5 passed; the three non-Python process tests still pass. Real pytest 9.0.3/9.1.1 also passes 24 scenarios / 72 native, unmodified-head and folded-head executions, including the wrong-test-selection reproduction and exit-code checks. This is a local integration trial, not a change already present in this PR; #4753 remains open until the Python portion lands.

On the unmodified PR, 17 argument-helper tests, 17 Jest/Vitest tests, the three process tests, formatting and warnings-denied all-targets clippy pass. Formatting/clippy also pass for the five-arm trial. No full-workspace or cross-platform run; tools other than pytest use stubs, not real installs or services.

[KuSh]

Round 1 — CHANGES REQUESTED

Claim: restore the -- clap strips from the front of 14 passthrough commands, once in dispatch, so it reaches the tool after the options rtk injects, and keep the jest/vitest skip filters off the words after it.
Scope: accept (round 1, frozen); maintainer PRs: checked against #4010 (cargo subcommand arms, not Other): no shared decision.
Ran: stub argv recorder × 19 tools (14 + the 5 Python ones from danieldutu#1) × 11 forms (-- zzz, bare --, -- -- zzz, x -- zzz, -- run --json --watch -k.py, -v/-vv before the tool, empty and spaced args, non-UTF-8, -- --help, -- --version) on merge-base, head, head + #1 and head merged into develop; real jest 29, vitest 3, prettier 3, tsc 5.9, go 1.24, cargo 1.98, docker 29 and pytest, native vs base vs head; rtk rewrite on every <tool> -- zzz form plus the npx, bundle exec, python -m and vendor/bin spellings. Invariants: under the hook, every decision, non-default targets (-- -sel, -- -weird.js, -- -k.py). The PR leaves the filters unchanged; on the real runs jest saves 91% and vitest 26%, against the 20% floor (CONTRIBUTING.md).
Pins: 24 decisions (19 dispatch arms including the 5 Python ones, the call site, the args_os read, the two skip-filter boundaries, user_boundary), 24 mutated, 1 survived (restore_double_dash_os reading args()).
Matrix: executed this round; pin check: executed this round
CI @88e1f3044: all green

Blocks merge (3, frozen at round 1)

1. src/lib.rs:2509 — an injected option after the user's arguments (1 site among the 19 modules: phpstan). The PR body says each module appends the user's arguments after the options it injects. phpstan does the opposite (phpstan_cmd.rs:76 adds --error-format json after them). So rtk phpstan -- analyse src now runs phpstan -- analyse src --error-format json; base ran phpstan analyse src --error-format json. With real phpstan 2.3.4 (official image), base reports the one finding through the JSON filter, while head prints Path /app/--error-format does not exist and no finding, exit 1. rtk phpstan analyse -- src already fails the same way on develop. The fix must satisfy: rtk phpstan -- analyse src and rtk phpstan analyse -- src report the same findings as native, through the JSON filter; analyse src, --error-format=table and --error-format json stay as they are. One constraint from the real tool: placed in front of the command name, a two-token --error-format json fails (Command "json" is not defined), while --error-format=json works. If you would rather not touch phpstan here, dropping it from the list also clears this, and it stays with #4753.
2. src/lib.rs:2515 — a new panic on these 10 arms. restore_double_dash reads std::env::args(), and the 10 Vec<String> arms now call it on every run. When argv[0] is not UTF-8 (rtk run by absolute path from such a directory), rtk tsc a and rtk jest a exit 101 with a panic on head; on base they run the tool. This is #3025's hazard reaching 10 more commands. The fix must satisfy: the same two runs reach the tool with argv unchanged. Reading args_os() and converting lossily is safe here, because clap has already required every user argument on these arms to be UTF-8, and argv[0] lies outside the region that is returned.
3. src/core/args_utils.rs:19 — unpinned behaviour (restore_double_dash_os reads args_os). Evidence: mutation std::env::args_os().collect() → std::env::args().map(OsString::from).collect() passes 5220/5220 tests. The test must run an Other arm with a non-UTF-8 argument (rtk go foo $'\xff.py', which works on head) and assert the child's argv bytes and exit code; that mutation must turn it red. The same shape with argv[0] pins item 2.

Optional — will not hold merge

• vitest_cmd.rs:219, same file: has_explicit_vitest_reporter still scans past the boundary you added for the skip filters. So rtk vitest -- --reporter=dot switches to passthrough and drops --reporter=json, though vitest now receives --reporter=dot as a filter. Giving it the same user_boundary would fix that.
• args_utils.rs:24: the precondition comment ("all callers use trailing_var_arg = true") now also covers external-subcommand callers, which rely on clap passing everything after the subcommand name through unchanged. One line there would help the next reader.

Filed as follow-ups

• #4919 A leading -- is still dropped for mvnd, oc, sbt, gt, bun, deno, docker compose, go test/build/vet and kubectl get — outside this PR's list; filed.
• #4920 Once this lands, pest, rspec and rubocop still look for their own options past the restored --, so a word after it switches off rtk's injected format, and rtk go -- tool golangci-lint run skips the golangci interception. The tool still gets what the user typed; the output is no longer filtered. Outside this PR's hunks; filed.

Checked and correct — no need to re-verify

• The 14 tools forward the user's arguments verbatim after rtk's injected options in 10 of 11 forms. The 11th is non-UTF-8, which panics in run_fallback on base too (#3025).
• Real tools on head match native: jest -- -sel (base runs the failing test, exit 1), vitest -- -sel and -- run, prettier -- -weird.js (base: exit 2 with unknown-option warnings), tsc -- --version (TS5023 like native), go -- version, cargo -- version, docker -- --version.
• The hook rewrites tsc|prettier|phpunit|pest|rspec|rubocop|vitest|jest -- x and their npx / bundle exec / vendor/bin spellings to rtk <tool> -- x. npx jest -- -sel matches native.
• -v/-vv before the tool, a -- after an argument, repeated separators and idempotence with module-level restores.
• cargo fmt, clippy --all-targets -D warnings and cargo test --all (5215 passed) on head, and on head merged into develop 4b4bc40 (5419 passed).

Your questions

• "Do you want those 5 arms folded into restore_leading_double_dash as well, so that one place owns the rule?" → Yes, please fold them in, though this PR does not wait on it. Protocol-zero-0's danieldutu#1 (42bb2ec) targets this branch and is exactly that change; merging it here and switching the body to Fixes #4753 closes the issue in one go. Otherwise #4753 stays open for the five Python commands. I ran it: real pytest -- -k.py now runs only the selected file (exit 0, 1 passed, as native), all 5 arm mutations are caught by its tests, and fmt/clippy/tests are green (5220 passed).
• "…or split the jest/vitest skip-filter change into its own PR" → Keep it here. Without it, -- run never reaches jest or vitest, so it belongs to the same claim.

Next

Fix items 1–3, with a test for each; then this merges once CI is green. Merging danieldutu#1 too is welcome, not required.
Rounds: 1. Threads: 3 open (waiting on the author), 0 resolved this round.

[KuSh]

Blocks merge (item 1): phpstan appends --error-format json after the user's arguments (phpstan_cmd.rs:76), so with the restored separator rtk phpstan -- analyse src runs phpstan -- analyse src --error-format json. Real phpstan 2.3.4: Path /app/--error-format does not exist, no finding, where develop reported it through the JSON filter. Either insert the format before the user's first -- (as a single --error-format=json token: two tokens in front of the command name make phpstan read json as the command), or leave phpstan out of this list.

[KuSh]

Blocks merge (item 2): these 10 arms now call restore_double_dash on every run, and it reads std::env::args(), which panics on a non-UTF-8 argv[0]. Run by absolute path from such a directory, rtk tsc a and rtk jest a exit 101 here and run the tool on develop. Reading args_os() lossily is safe: clap already required every user argument on these arms to be UTF-8, and argv[0] is outside the returned region.

[KuSh]

Blocks merge (item 3): nothing pins this args_os() read. With std::env::args().map(OsString::from).collect() instead, all 5220 tests pass. A test that runs an Other arm with a non-UTF-8 argument (rtk go foo $'\xff.py', which works on head) and asserts the child's argv bytes and exit code would turn that mutation red.