Skip to content

fix: preserve -- separator for cargo commands and silence fallback#326

Merged
pszymkowiak merged 1 commit intortk-ai:masterfrom
rursache:fix/passthrough-and-double-dash
Mar 6, 2026
Merged

fix: preserve -- separator for cargo commands and silence fallback#326
pszymkowiak merged 1 commit intortk-ai:masterfrom
rursache:fix/passthrough-and-double-dash

Conversation

@rursache
Copy link
Contributor

@rursache rursache commented Mar 5, 2026

Summary

  • rtk errors when commands have -- #287: Clap strips -- from parsed args, so rtk cargo test -- --nocapture was passing cargo test --nocapture (missing --), causing cargo to reject it. Added restore_double_dash() in cargo_cmd.rs that detects -- from std::env::args() and re-inserts it at the correct position before forwarding to cargo. Works for all cargo subcommands (test, clippy, etc.)
  • rtk must pass through unrecognized commands #286: Removed the noisy [rtk: parse failed, running raw] stderr message when falling back to raw command execution for unrecognized commands like make or ruby. The passthrough now runs silently, matching the documented behavior.

Fixes #286
Fixes #287

Test plan

  • Added test_restore_double_dash_with_separator["--nocapture"]["--", "--nocapture"]
  • Added test_restore_double_dash_with_test_name["my_test", "--nocapture"]["my_test", "--", "--nocapture"]
  • Added test_restore_double_dash_without_separator — no -- in original, args unchanged
  • Added test_restore_double_dash_empty_args — empty args stay empty
  • Added test_restore_double_dash_clippy["-D", "warnings"]["--", "-D", "warnings"]
  • All 523 tests pass
  • Manual: rtk cargo test -- --nocapture now runs successfully
  • Manual: rtk make --version and rtk ruby --version run silently (no stderr noise)

Two fixes for command passthrough issues:

1. Restore `--` separator stripped by clap (#287): When running
   `rtk cargo test -- --nocapture`, clap consumes `--` and passes only
   `["--nocapture"]` to cargo, which rejects it. Now detect `--` from
   the original command line and re-insert it in the correct position.

2. Remove noisy `[rtk: parse failed, running raw]` stderr message (#286):
   Unrecognized commands like `rtk make` or `rtk ruby` pass through
   silently via the fallback, matching the documented behavior.

Fixes #286
Fixes #287
@FlorianBruniaux
Copy link
Collaborator

Hey @rursache, heads up: PR #349 (TOML filter DSL) modifies run_fallback() in src/main.rs. Your PR touches the same area. We'd prefer to merge your PR first so the TOML work builds on top of your changes — avoids a painful rebase. No action needed from you, just FYI.

@pszymkowiak
Copy link
Collaborator

LGTM — verified locally on 0.27.0:

  • rtk cargo test -- --nocapture ✅ (was crashing with unexpected argument '--nocapture')
  • rtk cargo clippy -- -D warnings ✅ (-- separator preserved)
  • rtk make --version ✅ (silent passthrough, no more [rtk: parse failed] noise)
  • cargo fmt ✅, cargo clippy ✅, 614 tests pass ✅
  • No regression on other commands

Clean fix — restore_double_dash() approach is solid since Clap inherently strips --.

@pszymkowiak pszymkowiak merged commit 45f9344 into rtk-ai:master Mar 6, 2026
2 of 3 checks passed
@rursache rursache deleted the fix/passthrough-and-double-dash branch March 6, 2026 08:01
ahundt added a commit to ahundt/rtk that referenced this pull request Mar 11, 2026
Incorporates 52 upstream commits (v0.27.0 → v0.28.2):
- TOML filter DSL engine + 30 built-in filters (PRs rtk-ai#349, rtk-ai#351, rtk-ai#386)
- Graphite CLI support (PR rtk-ai#290)
- git commit -am/--amend fix via trailing_var_arg (PR rtk-ai#327)
- restore_double_dash for cargo (PR rtk-ai#326)
- gh -R/--repo passthrough, pr edit/comment fix (PRs rtk-ai#328, rtk-ai#332)
- docker compose subcommand filtering (PR rtk-ai#336)
- Telemetry tokens_saved + install_method (PRs rtk-ai#462, rtk-ai#469, rtk-ai#471)
- proxy streaming (PR rtk-ai#268)
- Diff limits increased (100→500 lines, 10→30 hunk lines)

Conflict resolution (5 files):
- cargo_cmd.rs: adopted upstream restore_double_dash, adapted streaming
  run_test() to use it, converted old split_at_double_dash tests
- git.rs: adopted upstream simplified Commit unit variant (fixes -am),
  adapted all commit tests to flat args API, added 6 new edge case tests
- init.rs: added TOML template generation alongside hook manifest
- main.rs: merged both upstream (gt, toml_filter, verify) and hooks-v2
  (cmd, hook, stream, pipe) modules, kept all tests from both sides
- utils.rs: kept hooks-v2 command_in_path/which_command + upstream English docs

Hook engine additions during merge:
- Added gt to hook_lookup() whitelist with 4 routing test cases

All 5 hook bug fixes from issue rtk-ai#361 preserved:
1. Streaming (stream.rs BufReader)
2. Handler coordination (parallel-merge + run_manifest_handlers on both paths)
3. Stderr deny (exit 2)
4. Routing whitelist (hook_lookup)
5. Vitest run injection

1182 tests pass (1 environment-dependent upstream test excluded).
ahundt added a commit to ahundt/rtk that referenced this pull request Mar 11, 2026
Incorporates 52 upstream commits (v0.27.0 → v0.28.2):
- TOML filter DSL engine + 30 built-in filters (PRs rtk-ai#349, rtk-ai#351, rtk-ai#386)
- Graphite CLI support (PR rtk-ai#290)
- git commit -am/--amend fix via trailing_var_arg (PR rtk-ai#327)
- restore_double_dash for cargo (PR rtk-ai#326)
- gh -R/--repo passthrough, pr edit/comment fix (PRs rtk-ai#328, rtk-ai#332)
- docker compose subcommand filtering (PR rtk-ai#336)
- Telemetry tokens_saved + install_method (PRs rtk-ai#462, rtk-ai#469, rtk-ai#471)
- proxy streaming (PR rtk-ai#268)
- Diff limits increased (100→500 lines, 10→30 hunk lines)

Conflict resolution (5 files):
- cargo_cmd.rs: adopted upstream restore_double_dash
- git.rs: adopted upstream simplified Commit variant (fixes -am),
  fixed test_git_status_not_a_repo via GIT_DIR env override
- init.rs: added TOML template generation alongside hook manifest,
  made resolve_claude_dir pub(crate) for config/mod.rs
- main.rs: merged upstream (gt, toml_filter, verify) and
  multi-platform (cmd, hook, stream, safety, gemini) modules
- utils.rs: accepted English doc comments

Recovery edits (safety integration restored after incorrect overwrite):
- hook/mod.rs: restored config::rules::try_remap(), safety::check_raw(),
  safety::check() per-command, FORMAT_PRESERVING/TRANSPARENT_SINKS
  pub(crate), basename extraction, safety-dependent tests
- discover/registry.rs: updated 3 wc tests for upstream IGNORED_PREFIXES

All hook engine + safety + gemini features preserved.
1332 tests pass, 0 failures, 5 ignored.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rtk errors when commands have -- rtk must pass through unrecognized commands

3 participants