Skip to content

fix: passthrough --json/--jq/--template flags in all gh subcommands#196

Closed
shadowofdoom wants to merge 1 commit intortk-ai:masterfrom
shadowofdoom:fix/gh-view-passthrough-json
Closed

fix: passthrough --json/--jq/--template flags in all gh subcommands#196
shadowofdoom wants to merge 1 commit intortk-ai:masterfrom
shadowofdoom:fix/gh-view-passthrough-json

Conversation

@shadowofdoom
Copy link
Contributor

Summary

  • gh pr view --json fields failed with Unknown JSON field: "--json" because view_pr() treated --json as the PR identifier
  • The v0.21.1 fix (fix: gh run view drops --log-failed, --log, --json flags #159) only covered gh run view via should_passthrough_run_view()
  • This generalizes the fix with has_output_format_flags() and applies it to all handlers that hardcode --json: list_prs, view_pr, pr_status, list_issues, view_issue, list_runs, and run_repo
  • Also fixes view_pr to handle omitted PR number (gh defaults to current branch)

Reproduction

rtk gh pr view --json number,title,state
# Before: Unknown JSON field: "--json"
# After: passes through to gh correctly

Test plan

  • All 383 tests pass (5 new tests for has_output_format_flags)
  • rtk gh pr view --json number,title,state passes through correctly
  • rtk gh pr list still uses RTK's compressed output
  • rtk gh pr view <number> still uses RTK's compressed output

🤖 Generated with Claude Code

view_pr() assumed args[0] was always a PR number, so
`gh pr view --json fields` treated "--json" as the PR identifier
and appended its own --json, causing `Unknown JSON field: "--json"`.

The v0.21.1 fix (should_passthrough_run_view) only covered
`gh run view`. This generalizes it with has_output_format_flags()
and applies it to all handlers that hardcode --json fields:
list_prs, view_pr, pr_status, list_issues, view_issue,
list_runs, and run_repo.

Also fixes view_pr to handle omitted PR number (gh defaults to
current branch).

Closes rtk-ai#159 (broader fix)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pszymkowiak
Copy link
Collaborator

Tested locally on the branch — LGTM, approve.

All 383 tests pass. Manual testing confirms:

  • gh pr view --json, gh pr list --json --jq, gh issue list --json --jq, gh repo view --json, gh pr status --json, gh run list --json --jq — all passthrough correctly with raw JSON output
  • gh pr list, gh pr view 196, gh issue view 228 — RTK filtering still works, no regression

Minor nits (non-blocking):

  • cargo fmt flags 2 formatting issues in tests
  • Consider dropping -t/-q short flags from has_output_format_flags() — they could cause false positives if other gh subcommands reuse those short flags for something else. --json, --jq,
    --template are sufficient and unambiguous.

@pszymkowiak
Copy link
Collaborator

please review conflict to merge

heAdz0r added a commit to heAdz0r/rtk that referenced this pull request Feb 28, 2026
…tk-ai#217, rtk-ai#196, rtk-ai#248, rtk-ai#211, rtk-ai#200, rtk-ai#192, rtk-ai#268)

Wave 1 (critical bugs):
- fix(registry): fi/done moved to IGNORED_EXACT — find no longer shadowed (rtk-ai#246)
- fix(playwright): f64 duration, specs[] structure, --reporter=json after subcmd (rtk-ai#193)
- fix(gh): should_passthrough_gh_view for --json/--jq/--template/--web in view_pr/issue/run (rtk-ai#217+196)

Wave 2 (reliability):
- fix(git): is_blob_show_arg — blob show passthrough without trailing-newline trim (rtk-ai#248)
- fix(find): parse_find_args with native -name/-type/-maxdepth/-iname support (rtk-ai#211)
- fix(main): graceful Clap fallback + parse_failures SQLite table + rtk gain --failures (rtk-ai#200)

Wave 3 (UX):
- feat(git): global options -C/-c/--git-dir/--work-tree/--no-pager/--no-optional-locks/--bare/--literal-pathspecs (rtk-ai#192)
- feat(proxy): streaming output via spawn()+threads instead of buffered output() (rtk-ai#268)

Tests: 1091 → 1117 (+26), 0 regressions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@MickaelV0
Copy link

Ran into this exact issue while auditing RTK compatibility with a plugin-heavy Claude Code setup. Sharing some findings that might help justify prioritizing this merge.

What breaks in practice

We use a dev-core plugin (25+ skills) where Claude Code directly runs gh commands with structured output flags and then parses the result — either via inline --jq filters or by passing JSON to downstream programs. With RTK installed, these all silently break:

gh issue list --search "..." --json number,title,state --jq '.[:3]'
gh pr list --head "$BRANCH" --json number,state,reviewDecision --jq '.[]'
gh pr view --json comments --jq '.comments[].body'
gh repo view --json name --jq '.name'

The failure mode is subtle: the hook rewrites the command, RTK runs gh internally with its own hardcoded --json fields (prepended), then reformats the output as compact text. By the time --jq runs (as a gh flag, not a shell pipe), the fields requested by the user may not even be present. Then RTK reformats the final result anyway — Claude gets compact text where it expected a JSON array.

What we verified in the source

  • The hook pattern '^gh[[:space:]]+(pr|issue|run|api|release)' matches unconditionally on command name, with no flag inspection
  • list_issues() and view_issue() in gh_cmd.rs prepend their own --json fields before appending user args, then always reformat via serde_json
  • gh project subcommands correctly fall through to run_passthrough — that part works great

Workaround we're using in the meantime

From the thread in #188, @mariusvniekerk's stopgap (skip rewrite when piping to jq) handles shell-pipe cases. But it doesn't cover gh's built-in --jq flag or bare --json usage without a pipe. PR #196's has_output_format_flags() approach is the right fix.

Is there anything blocking this beyond the merge conflict? Happy to help test if a rebase lands.

@MickaelV0
Copy link

Architectural position needed: structured-output flags should always passthrough

Beyond this specific fix, I think RTK needs a clear architectural stance on passthrough behavior for any CLI flag that signals structured/machine-readable output.

The issue I hit with `gh --json` / `--jq` is not a one-off — it's a fundamental pattern that will repeat for every proxied CLI that supports structured output conventions:

  • `gh` → `--json`, `--jq`, `--template`
  • `git` → `--porcelain`, `--format`, `--name-only`
  • `docker` → `--format`, `--quiet`
  • `kubectl` → `-o json`, `-o yaml`, `-o jsonpath`
  • etc.

When a Claude Code skill (or any plugin) runs a CLI command and parses the output programmatically — via inline `--jq`, downstream `jq` pipes, or direct JSON deserialization — RTK's reformatting silently breaks the contract.

Proposed convention: any CLI that has established object-passing or post-processing conventions (structured output flags, machine-readable modes) should be treated as passthrough when those flags are present, to guarantee correct integration with skill and plugin ecosystems.

PR #196's `has_output_format_flags()` is exactly the right pattern. Elevating it to a documented design principle — and applying it systematically to every future CLI proxy RTK adds — would prevent this entire class of breakage upfront, rather than fixing it case by case.

@FlorianBruniaux
Copy link
Collaborator

Hi @shadowofdoom! We have multiple gh flag passthrough PRs open (#196, #217, #319, #325, #328). They all conflict with each other. Could you rebase both PRs on latest master first to fix the CI failures, then we'll coordinate which approach to take? Alternatively, feel free to consolidate your two PRs into one — that would help.

pszymkowiak added a commit that referenced this pull request Mar 5, 2026
gh commands with --json, --jq, or --template produce structured output
that rtk gh filtering would corrupt. Skip rewrite so callers get raw JSON.
pszymkowiak added a commit that referenced this pull request Mar 5, 2026
@pszymkowiak
Copy link
Collaborator

Fixed in v0.27.0 — gh with --json/--jq/--template now skips rewrite.

@pszymkowiak pszymkowiak closed this Mar 5, 2026
dragonGR pushed a commit to dragonGR/rtk that referenced this pull request Mar 5, 2026
gh commands with --json, --jq, or --template produce structured output
that rtk gh filtering would corrupt. Skip rewrite so callers get raw JSON.
ahundt added a commit to ahundt/rtk that referenced this pull request Mar 6, 2026
Conflict resolution (1 conflict, additive):
- src/discover/registry.rs: keep all tests from both sides
  - ours: test_classify_wc, test_classify_wc_bare, test_route_wc
  - upstream: test_rewrite_gh_json_skipped, test_rewrite_gh_jq_skipped,
    test_rewrite_gh_template_skipped, test_rewrite_gh_api_json_skipped,
    test_rewrite_gh_without_json_still_works (rtk-ai#196)

Upstream v0.27.0 changes absorbed:
- fix(registry): RTK_DISABLED=1 env prefix skips rewrite entirely (rtk-ai#345)
- fix(registry): gh --json/--jq/--template skips rewrite to avoid
  corrupting structured output (rtk-ai#196)
- fix: RTK_DISABLED ignored, 2>&1 broken, json TOML error (rtk-ai#345,rtk-ai#346,rtk-ai#347)
- docs: version refs, module count, CHANGELOG, ARCHITECTURE

Also fixed pre-existing test race exposed by new parallel tests:
- fix(test): add EnvGuard to test_shared_is_hook_disabled_* in claude.rs
  to prevent RTK_ACTIVE race with test_raii_guard_clears_on_panic
  (both tests set RTK_ACTIVE without holding ENV_LOCK)

Tests: 1029 pass, 0 fail (parallel), 5 ignored
pszymkowiak added a commit that referenced this pull request Mar 6, 2026
… smart, docker, json edge cases

Covers bug fixes #196, #344, #345, #346, #347 and previously untested
commands. Adds assert_fails helper. 118 assertions total (was 69).
ahundt added a commit to ahundt/rtk that referenced this pull request Mar 6, 2026
…nged

Bug: rtk hook claude was routing gh pr list --json ... to rtk gh pr list
--json ... which corrupts structured JSON output. Mirrors upstream fix
registry::rewrite_segment rtk-ai#196 to the binary hook path.

Fix: should_passthrough() returns true for gh commands containing
--json, --jq, or --template flags — hook emits no output (NoOpinion)
so Claude Code runs the original gh command unchanged.

Tests: 2 new (test_gh_json_flag_passes_through,
test_gh_without_json_not_passthrough); 1031 pass total
ahundt added a commit to ahundt/rtk that referenced this pull request Mar 6, 2026
…at/gemini-support-v2

Conflict resolution (1 conflict, additive):
- src/main.rs Init command: merge --claude/--gemini flags (gemini branch)
  with --hook-type flag (rust-hooks-v2); keep all three flags
  - Dispatch: multi-platform logic calls init::run(..., hook_type, ...)
    for Claude and init::run_gemini() for Gemini; hook_type threads through

Result: rtk init now supports --claude, --gemini, and --hook-type together
  rtk init               → Claude + Gemini, script hook (default)
  rtk init --claude      → Claude only
  rtk init --gemini      → Gemini only
  rtk init --hook-type binary → Claude + Gemini with binary hook

Inherits from rust-hooks-v2 (dbc46c7):
- fix(hook): binary hook passes gh --json/--jq/--template through unchanged
  (mirrors upstream registry::rewrite_segment fix rtk-ai#196)
- 2 new tests: test_gh_json_flag_passes_through, test_gh_without_json_not_passthrough

Tests: 1050 pass, 0 fail (parallel), 5 ignored
pszymkowiak added a commit that referenced this pull request Mar 6, 2026
* fix: prettier reports "All OK" when not installed (#221)

Empty or failed prettier output was incorrectly treated as "all files
formatted". Now detects empty output and non-zero exit code, shows the
actual error message instead of a false positive.

* test: add smoke tests for rewrite, verify, proxy, discover, diff, wc, smart, docker, json edge cases

Covers bug fixes #196, #344, #345, #346, #347 and previously untested
commands. Adds assert_fails helper. 118 assertions total (was 69).

* chore: update benchmark.sh with missing commands and fix paths

- Add cargo (build/test/clippy/check), diff, smart, wc, curl, wget sections
- Fix Python commands: use dedicated rtk ruff/pytest instead of rtk test
- Fix Go commands: use dedicated rtk go/golangci-lint, add go build/vet
- Make BENCH_DIR absolute so debug files work from temp fixture dirs
- Fallback to installed rtk if target/release/rtk not found
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants