fix: passthrough --json/--jq/--template flags in all gh subcommands#196
fix: passthrough --json/--jq/--template flags in all gh subcommands#196shadowofdoom wants to merge 1 commit intortk-ai:masterfrom
Conversation
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>
|
Tested locally on the branch — LGTM, approve. All 383 tests pass. Manual testing confirms:
Minor nits (non-blocking):
|
|
please review conflict to merge |
…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>
|
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 practiceWe use a 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 What we verified in the source
Workaround we're using in the meantimeFrom the thread in #188, Is there anything blocking this beyond the merge conflict? Happy to help test if a rebase lands. |
|
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:
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. |
|
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. |
gh commands with --json, --jq, or --template produce structured output that rtk gh filtering would corrupt. Skip rewrite so callers get raw JSON.
|
Fixed in v0.27.0 — gh with --json/--jq/--template now skips rewrite. |
gh commands with --json, --jq, or --template produce structured output that rtk gh filtering would corrupt. Skip rewrite so callers get raw JSON.
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
…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
…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
* 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
Summary
gh pr view --json fieldsfailed withUnknown JSON field: "--json"becauseview_pr()treated--jsonas the PR identifiergh run viewviashould_passthrough_run_view()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, andrun_repoview_prto handle omitted PR number (gh defaults to current branch)Reproduction
Test plan
has_output_format_flags)rtk gh pr view --json number,title,statepasses through correctlyrtk gh pr liststill uses RTK's compressed outputrtk gh pr view <number>still uses RTK's compressed output🤖 Generated with Claude Code