Skip to content

fix: pass through -R/--repo flag in gh view commands#328

Merged
pszymkowiak merged 1 commit intortk-ai:masterfrom
rursache:fix/gh-repo-flag-passthrough
Mar 6, 2026
Merged

fix: pass through -R/--repo flag in gh view commands#328
pszymkowiak merged 1 commit intortk-ai:masterfrom
rursache:fix/gh-repo-flag-passthrough

Conversation

@rursache
Copy link
Contributor

@rursache rursache commented Mar 5, 2026

Summary

rtk gh issue view, rtk gh pr view, rtk gh pr checks, and rtk gh run view were ignoring extra flags like -R/--repo, causing failures outside git repositories (where gh needs -R to know which repo to query).

Root cause: These functions took args[0] as the identifier (PR/issue number) and discarded everything else, while constructing hardcoded gh commands without the user's flags.

Fix:

  • Added extract_identifier_and_extra_args() that separates the positional identifier from flags, correctly handling both view 123 -R owner/repo and view -R owner/repo 123 argument orders
  • All four view functions now pass extra args through to gh
  • Also includes the --json passthrough fix from rtk gh pr view --json ... fails #313 (prevents Unknown JSON field errors)

Fixes #223

Test plan

  • Added test_extract_identifier_simple — basic number extraction
  • Added test_extract_identifier_with_repo_flag_after185 -R rtk-ai/rtk
  • Added test_extract_identifier_with_repo_flag_before-R rtk-ai/rtk 185
  • Added test_extract_identifier_with_long_repo_flag--repo owner/repo
  • Added test_extract_identifier_empty — empty args returns None
  • Added test_extract_identifier_only_flags — no positional returns None
  • Added test_extract_identifier_with_web_flag — other flags preserved
  • Added test_has_json_flag_present / test_has_json_flag_absent
  • All 524 tests pass
  • Manual: rtk gh issue view 223 -R rtk-ai/rtk works from /tmp (outside git repo)
  • Manual: rtk gh issue view -R rtk-ai/rtk 223 works (flag before number)
  • Manual: rtk gh pr view 324 -R rtk-ai/rtk works from /tmp
  • Manual: rtk gh issue view 223 -R rtk-ai/rtk --json number,title passes through raw JSON

@FlorianBruniaux
Copy link
Collaborator

Hi @jackfreem, @rursache! We have several PRs tackling gh flag passthrough (#196, #217, #319, #325, #328) — they all touch gh_cmd.rs and will conflict with each other. Before we review, can you coordinate? We're leaning toward consolidating into #328 (most complete). @jackfreem, would your --jq fix (#319) fit as an addition to #328?

gh view/checks commands (pr view, issue view, pr checks, run view)
were ignoring extra flags like -R/--repo, causing failures outside
git repositories where gh needs -R to know which repo to query.

The root cause: these functions took args[0] as the identifier and
discarded everything else, while constructing hardcoded gh commands.

Changes:
- Add extract_identifier_and_extra_args() to separate the positional
  identifier from flags, handling both `view 123 -R owner/repo` and
  `view -R owner/repo 123` argument orders
- Pass extra args through to gh in view_pr, view_issue, pr_checks,
  and view_run
- Add has_json_flag() check at top of run() to passthrough when user
  explicitly requests --json output (related fix from #313)

Fixes #223
@rursache rursache force-pushed the fix/gh-repo-flag-passthrough branch from a2c24e6 to 374ea6c Compare March 5, 2026 17:58
@rursache
Copy link
Contributor Author

rursache commented Mar 5, 2026

@FlorianBruniaux Rebased on latest master and consolidated #325 (now closed) into this PR. All tests pass. Ready for review!

@pszymkowiak
Copy link
Collaborator

Review: LGTM ✅

Verified locally — cargo fmt clean, cargo clippy clean, 618/618 tests pass.

Clean implementation:

Mergeable, no conflicts.

@pszymkowiak pszymkowiak merged commit 0a1bcb0 into rtk-ai:master Mar 6, 2026
2 of 3 checks passed
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.

bug: rtk gh issue view -R fails outside git repo (plain gh works)

3 participants