A leading -- is dropped before the tool runs for 19 passthrough commands (rtk pytest -- -k.py runs the wrong tests) #4753

Activity

1. Protocol-zero-0 commented on Oct 23, 2026

   

   Protocol-zero-0

   on Oct 23, 2026

   Last edited by Protocol-zero-0

   Contributor

   More actions

   The five Python commands are now folded into the shared helper in a reviewable supplement against #4768's source branch.

   I reproduced the pytest case and prepared the Python portion of this fix on develop 8554743d (which includes #4727).

   The patch restores clap's consumed separator in the dispatch for pytest, mypy, pip, uv and sqlfluff, using the existing core::args_utils::restore_double_dash. Each module receives the restored user arguments before routing or injecting options. In particular, pytest forwards --tb=short -q -rxX -- -k.py, and protected words such as pip -- list, uv -- run and sqlfluff -- lint do not enter their subcommand-specific paths.

   Validation on Linux:

   • Five new process-level regression tests all fail on the unmodified base and pass with the patch. They exercise 90 stub processes per successful suite: nine argument forms for each of the five tools, at exit 0 and exit 7, with rtk -v before the tool. Exact NUL-delimited child argv is checked, including a bare separator, repeated separators, protected subcommand words and already-preserved separators.
   • Real pytest 9.0.3 and 9.1.1, 12 scenarios each, 72 native/base/patched executions: dash-prefixed files, node IDs, a selected failing file, multiple paths, missing files, protected --help, repeated separators and quiet/verbose/traceback controls. Patched exit codes match native throughout. The original -- -k.py case runs the unrelated failing test on base; patched runs only the selected passing test.
   • Root workspace tests: 5067 passed, 8 ignored. Root/proxy formatting checks and warnings-denied all-targets clippy pass.

   The other four Python tools use argument-recording stubs; no real package installation or SQL linting is claimed. Other platforms and the remaining commands in #4753 are outside this patch, so the overall issue remains open.

   Apply-ready patch against 8554743d:

   Python dispatch and process regressions

   diff --git a/src/lib.rs b/src/lib.rs
   index 93e9d399..e9f787c5 100644
   --- a/src/lib.rs
   +++ b/src/lib.rs
   @@ -3564,11 +3564,20 @@ fn run_cli() -> Result<i32> {
    
            Commands::Ruff { args } => ruff_cmd::run(&args, cli.verbose)?,
    
   -        Commands::Sqlfluff { args } => sqlfluff_cmd::run(&args, cli.verbose)?,
   +        Commands::Sqlfluff { args } => {
   +            let args = core::args_utils::restore_double_dash(&args);
   +            sqlfluff_cmd::run(&args, cli.verbose)?
   +        }
    
   -        Commands::Pytest { args } => pytest_cmd::run(&args, cli.verbose)?,
   +        Commands::Pytest { args } => {
   +            let args = core::args_utils::restore_double_dash(&args);
   +            pytest_cmd::run(&args, cli.verbose)?
   +        }
    
   -        Commands::Mypy { args } => mypy_cmd::run(&args, cli.verbose)?,
   +        Commands::Mypy { args } => {
   +            let args = core::args_utils::restore_double_dash(&args);
   +            mypy_cmd::run(&args, cli.verbose)?
   +        }
    
            Commands::Php { args } => php_cmd::run(&args, cli.verbose)?,
    
   @@ -3592,9 +3601,15 @@ fn run_cli() -> Result<i32> {
    
            Commands::Rspec { args } => rspec_cmd::run(&args, cli.verbose)?,
    
   -        Commands::Pip { args } => pip_cmd::run(&args, cli.verbose)?,
   +        Commands::Pip { args } => {
   +            let args = core::args_utils::restore_double_dash(&args);
   +            pip_cmd::run(&args, cli.verbose)?
   +        }
    
   -        Commands::Uv { args } => uv_cmd::run(&args, cli.verbose)?,
   +        Commands::Uv { args } => {
   +            let args = core::args_utils::restore_double_dash(&args);
   +            uv_cmd::run(&args, cli.verbose)?
   +        }
    
            Commands::Go { command } => match command {
                GoCommands::Test { args } => go_cmd::run_test(&args, cli.verbose)?,
   diff --git a/tests/python_double_dash_test.rs b/tests/python_double_dash_test.rs
   new file mode 100644
   index 00000000..92463cbc
   --- /dev/null
   +++ b/tests/python_double_dash_test.rs
   @@ -0,0 +1,94 @@
   +//! Exercise clap, dispatch and the child argv together: a leading separator
   +//! must reach the tool, after pytest's injected options, without routing a
   +//! protected word such as `list`, `run` or `lint` as a subcommand.
   +#![cfg(unix)]
   +
   +mod common;
   +
   +use std::os::unix::fs::PermissionsExt;
   +
   +fn check_tool(tool: &str, injected: &[&str]) {
   +    let dir = common::tempdir();
   +    let capture = dir.path().join("argv");
   +    let stub = dir.path().join(tool);
   +    std::fs::write(
   +        &stub,
   +        "#!/bin/sh\nprintf '%s\\0' \"$@\" > \"$ARGV_CAPTURE\"\nexit \"$STUB_EXIT\"\n",
   +    )
   +    .expect("write stub");
   +    std::fs::set_permissions(&stub, std::fs::Permissions::from_mode(0o755)).expect("chmod stub");
   +    let path = std::env::join_paths(std::iter::once(dir.path().to_path_buf()).chain(
   +        std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default()),
   +    ))
   +    .expect("PATH");
   +
   +    let cases: &[&[&str]] = &[
   +        &["--", "-k.py"],
   +        &["--"],
   +        &["--", "--"],
   +        &["--", "--", "-k.py"],
   +        &["--", "list"],
   +        &["--", "run"],
   +        &["--", "lint"],
   +        &["zzz", "--", "-k.py"],
   +        &["zzz"],
   +    ];
   +    for args in cases {
   +        for code in [0, 7] {
   +            // Each process has fresh argv evidence, even if dispatch fails.
   +            let _ = std::fs::remove_file(&capture);
   +            let output = common::rtk_command()
   +                .current_dir(dir.path())
   +                .env("PATH", &path)
   +                .env("ARGV_CAPTURE", &capture)
   +                .env("STUB_EXIT", code.to_string())
   +                .args(["-v", tool])
   +                .args(*args)
   +                .output()
   +                .expect("run rtk");
   +            assert_eq!(
   +                output.status.code(),
   +                Some(code),
   +                "{tool} {args:?}: {}",
   +                String::from_utf8_lossy(&output.stderr)
   +            );
   +            let bytes = std::fs::read(&capture).expect("tool must execute");
   +            let actual: Vec<&[u8]> = bytes
   +                .strip_suffix(&[0])
   +                .expect("NUL-terminated argv")
   +                .split(|b| *b == 0)
   +                .collect();
   +            let expected: Vec<&[u8]> = injected
   +                .iter()
   +                .chain(args.iter())
   +                .map(|s| s.as_bytes())
   +                .collect();
   +            assert_eq!(actual, expected, "{tool} {args:?}");
   +        }
   +    }
   +}
   +
   +#[test]
   +fn pytest_keeps_separator_after_injected_options() {
   +    check_tool("pytest", &["--tb=short", "-q", "-rxX"]);
   +}
   +
   +#[test]
   +fn mypy_keeps_separator() {
   +    check_tool("mypy", &[]);
   +}
   +
   +#[test]
   +fn pip_keeps_separator_before_subcommand_like_paths() {
   +    check_tool("pip", &[]);
   +}
   +
   +#[test]
   +fn uv_keeps_separator_before_subcommand_like_paths() {
   +    check_tool("uv", &[]);
   +}
   +
   +#[test]
   +fn sqlfluff_keeps_separator_before_subcommand_like_paths() {
   +    check_tool("sqlfluff", &[]);
   +}
2. rtk-wshm-sync-bot

   added

   area:cli

   bugSomething isn't workingSomething isn't working

   good first issueGood for newcomersGood for newcomers

   priority:high

   on Oct 23, 2026
3. rtk-wshm-sync-bot commented on Oct 23, 2026

   

   rtk-wshm-sync-botBot

   on Oct 23, 2026 – with rtk-wshm-sync-bot

   More actions

   wshm · Automated triage by AI

   🔍 Automated Triage

   🐛 Category	bug
   🟠 Priority	high
   🎯 Confidence	93%

   Summary

   trailing_var_arg strips a leading -- before dispatch for 19 passthrough tools, causing arguments after -- to be misread (e.g. rtk pytest -- -k.py runs wrong tests); fix should restore -- once centrally in dispatch rather than per-module like the existing git/gh/glab/cargo/ls/grep/golangci-lint/dotnet/ruff fixes.

   📁 Relevant files

   • core/args_utils.rs

   Suggested Actions

   1. Centralize restore_double_dash in dispatch (core/args_utils.rs) instead of per-module calls
   2. Ensure restored -- is placed after options rtk injects, per the table of forwarded argv in the issue
   3. Add regression tests covering all 19 affected tools (pytest, jest, vitest, rspec, rubocop, etc.)
   4. Cross-check against fix(ruff): restore a leading -- before routing #4727 (ruff fix) and rtk ruff check appends . when every path is dash-prefixed or contains =, so rtk ruff check a=b.py lints the whole tree #4752 (ruff path handling) for consistent dispatch-layer conventions

   ---

   Triaged automatically by wshm · This is an automated analysis, not a human review.
4. TechWizard9999 commented on Oct 30, 2026

   

   TechWizard9999

   on Oct 30, 2026

   Contributor

   More actions

   Hi @KuSh , If this is still open I can take this
5. Protocol-zero-0 commented on Oct 30, 2026

   

   Protocol-zero-0

   on Oct 30, 2026

   Contributor

   More actions

   @TechWizard9999 For coordination: #4768 already has a shared-entry fix in progress, and I submitted the five Python entry points as a supplement to its source branch: danieldutu#1. Both PRs are still open; the supplement has not been incorporated into #4768 yet. Please coordinate with the #4768 author before starting a parallel per-command fix.
6. TechWizard9999 commented on Oct 30, 2026

   

   TechWizard9999

   on Oct 30, 2026

   Contributor

   More actions

   Ohh!! Sure thanks @Protocol-zero-0