feat: add --jq flag for filtering JSON output#211
Conversation
Add jq expression filtering (--jq / -q) to api, service, and shortcut commands using gojq. Includes early expression validation, mutual exclusion checks with --output and non-json --format, pagination+jq aggregation path, and comprehensive test coverage. Change-Id: I52e7d158a6264cc51f24a267b60674330e223450 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a global Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as Command (API/Service/Shortcut)
participant Client as HTTP Client / Paginator
participant JQ as JQ Engine
participant Out as Output Writer
User->>Cmd: Invoke with --jq "<expr>"
Cmd->>Cmd: Validate flags (ValidateJqFlags / ValidateJqExpression)
Cmd->>Client: Execute request (pass JqExpr)
alt Pagination requested & JqExpr set
Client->>Client: PaginateAll -> aggregate pages
Client-->>Cmd: Return aggregated JSON
else Single response
Client-->>Cmd: Return single JSON response
end
Cmd->>JQ: Call JqFilter(data, expr)
JQ->>JQ: Parse/compile expr, normalize data, execute
alt JQ success
JQ->>Out: Write scalar/raw or JSON outputs (each result newline)
else JQ error
JQ->>Out: Write jq error
end
alt No JqExpr
Cmd->>Out: Format with standard JSON formatter
end
Out->>User: Display result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/efficiency suggestions with no impact on correctness. The core jq filtering logic, flag validation, pagination integration, and error propagation are all correct. The three P2 findings (double compilation, in-place map mutation, %g scientific notation) do not affect current behavior and can be addressed in follow-up work. The previously-flagged exit-code and truncation issues have been handled or are pre-existing platform limitations. internal/output/jq.go – double compilation and in-place map mutation are both confined here and are straightforward to fix. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as CLI (api/service/shortcut)
participant Val as ValidateJqFlags
participant HC as HandleResponse / PaginateWithJq
participant JQ as JqFilter
participant Out as stdout
CLI->>Val: ValidateJqFlags(jqExpr, output, format)
Val-->>CLI: error or nil (parse+compile)
alt Single request
CLI->>HC: HandleResponse(resp, opts{JqExpr})
HC->>HC: parseJSONResponse → result
HC->>JQ: JqFilter(out, result, jqExpr)
else --page-all + --jq
CLI->>HC: PaginateWithJq(ctx, ac, req, jqExpr, out, opts, checkErr)
HC->>HC: ac.PaginateAll → merged result
HC->>HC: checkErr(result)
HC->>JQ: JqFilter(out, result, jqExpr)
end
JQ->>JQ: gojq.Parse + Compile (again)
JQ->>JQ: toGeneric → convertNumbers
JQ->>JQ: code.Run(normalized)
loop each result value
JQ->>Out: writeJqValue (raw scalar or indented JSON)
end
Reviews (4): Last reviewed commit: "fix: reject --jq for non-JSON responses ..." | Re-trigger Greptile |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@bf95808c4cefe4edad19ab89d2a711fbe2668bfd🧩 Skill updatenpx skills add larksuite/cli#feat/jq -y -g |
Change-Id: Iad365fdb387ff808a6fca0746d85d2a3b4c2a911 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
shortcuts/common/runner_jq_test.go (1)
22-35: Make test flag setup fail-fast on flag API errors.These
ParseFlags/Setcalls ignore errors. If a flag name changes, tests may pass with unintended defaults instead of failing clearly.Proposed refactor
-func newJqTestContext(jqExpr, format string) (*RuntimeContext, *bytes.Buffer, *bytes.Buffer) { +func newJqTestContext(t *testing.T, jqExpr, format string) (*RuntimeContext, *bytes.Buffer, *bytes.Buffer) { + t.Helper() stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} cmd := &cobra.Command{Use: "test"} cmd.Flags().String("jq", "", "") cmd.Flags().String("format", "json", "") cmd.Flags().String("as", "bot", "") - cmd.ParseFlags(nil) + if err := cmd.ParseFlags(nil); err != nil { + t.Fatalf("parse flags: %v", err) + } if jqExpr != "" { - cmd.Flags().Set("jq", jqExpr) + if err := cmd.Flags().Set("jq", jqExpr); err != nil { t.Fatalf("set jq: %v", err) } } if format != "" { - cmd.Flags().Set("format", format) + if err := cmd.Flags().Set("format", format); err != nil { t.Fatalf("set format: %v", err) } } @@ - rctx, stdout, _ := newJqTestContext(".data.name", "") + rctx, stdout, _ := newJqTestContext(t, ".data.name", "")Also applies to: 137-139, 160-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/common/runner_jq_test.go` around lines 22 - 35, The helper newJqTestContext currently ignores errors from cmd.ParseFlags and cmd.Flags().Set so flag API changes silently produce wrong defaults; update newJqTestContext to check the error returned by cmd.ParseFlags(nil) and each cmd.Flags().Set(...) and fail-fast (e.g., panic with a clear message) if any error occurs, and apply the same pattern to the other test helpers/uses referenced (the calls around lines 137-139 and 160-161) so flag parsing and setting never silently fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/service/service.go`:
- Around line 424-426: The branch that handles API errors calls checkErr and
then prints the raw result to stdout via output.FormatValue(out, result,
output.FormatJSON) before returning apiErr, which mixes stdout/stderr; change it
so that when checkErr(result) returns a non-nil apiErr you do NOT write to the
stdout writer variable out but instead write the raw result to stderr (e.g., use
os.Stderr or the existing stderr writer if one exists) via
output.FormatValue(<stderr writer>, result, output.FormatJSON) and then return
apiErr.
In `@internal/client/response.go`:
- Around line 29-30: The code currently ignores JqExpr when the response isn't
JSON, silently saving binary output; update the response handling in
HandleResponse (and the other branch referenced around lines 66-68) to validate
that JqExpr is only accepted for JSON responses: if reqOpts.JqExpr (or the
struct field JqExpr) is non-empty and the response Content-Type is not
application/json (or does not parse as JSON), return an explicit error (or print
a clear message) instead of falling through to the binary-save path. Locate the
JSON-path logic and the binary-save logic in HandleResponse, add the
Content-Type check before applying jq, and ensure the binary-save branch rejects
requests that incorrectly supplied --jq.
In `@internal/output/jq.go`:
- Around line 67-87: The writeJqValue function currently ignores errors from
fmt.Fprintln/Fprintf calls, so update each write (the fmt.Fprintln/Fprintf
invocations in the bool/int/float64/*big.Int/string/default branches inside
writeJqValue) to capture the (int, error) return, check the error, and on error
return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err)
(or a similar descriptive message), preserving the existing json.MarshalIndent
error style; ensure the function returns nil only after successful writes.
- Around line 95-103: In convertNumbers' json.Number case, don't cast to int or
float64 (which overflow/lose precision); instead detect if val contains an
integer and parse it into a *big.Int (e.g., using
big.Int.SetString(val.String(), 10)) and return that so writeJqValue (which
already supports *big.Int) can handle it; only if the number contains
fractional/exponent parts or SetString fails, fall back to parsing as float64 or
returning the original string as before.
In `@shortcuts/common/runner.go`:
- Around line 423-426: The jq evaluation error is currently only printed to
stderr in the shortcut runner (when ctx.JqExpr != "" and output.JqFilter is
called), causing the command to exit 0; change this to propagate the error so
callers observe a non-zero exit. Replace the fmt.Fprintf-only handling by
returning the error from the surrounding function (or wrapping it with context)
instead of swallowing it; update the branch that calls output.JqFilter
(referencing ctx.JqExpr, output.JqFilter, and ctx.IO().ErrOut) to return the
error when output.JqFilter fails so shortcut commands behave like the
api/service flows.
---
Nitpick comments:
In `@shortcuts/common/runner_jq_test.go`:
- Around line 22-35: The helper newJqTestContext currently ignores errors from
cmd.ParseFlags and cmd.Flags().Set so flag API changes silently produce wrong
defaults; update newJqTestContext to check the error returned by
cmd.ParseFlags(nil) and each cmd.Flags().Set(...) and fail-fast (e.g., panic
with a clear message) if any error occurs, and apply the same pattern to the
other test helpers/uses referenced (the calls around lines 137-139 and 160-161)
so flag parsing and setting never silently fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67bc4f45-69a8-443f-b1f8-680c262486b7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
cmd/api/api.gocmd/api/api_test.gocmd/root.gocmd/service/service.gocmd/service/service_test.gogo.modinternal/client/response.gointernal/output/jq.gointernal/output/jq_test.goshortcuts/common/runner.goshortcuts/common/runner_jq_test.go
gojq v0.12.18 requires Go 1.24, which unnecessarily bumped the project minimum version. v0.12.17 requires only Go 1.21 and provides the same jq functionality needed. Change-Id: Ic881b8efc922a5d33ece5459a37b74df73bb554d Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract ValidateJqFlags() and PaginateWithJq() shared functions to eliminate duplicated jq logic across api, service, and shortcut commands. Change-Id: I5fd46daa49eab1290a8bcdcacc80efae686d92b7 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (3)
internal/client/pagination.go (1)
22-33:⚠️ Potential issue | 🟠 MajorDon’t bake API-error stdout behavior into
PaginateWithJq.This helper is now shared by both
apiandservice, but Lines 29-31 always dump failing responses toout. That matchesapi, yet it changesservice --page-all --jqfrom “no stdout on API error” to mixed stdout/stderr, which is a breaking change for scripts that treat stdout as success payload only.Suggested fix
-func PaginateWithJq(ctx context.Context, ac *APIClient, request RawApiRequest, - jqExpr string, out io.Writer, pagOpts PaginationOptions, +func PaginateWithJq(ctx context.Context, ac *APIClient, request RawApiRequest, + jqExpr string, out, apiErrOut io.Writer, pagOpts PaginationOptions, checkErr func(interface{}) error) error { result, err := ac.PaginateAll(ctx, request, pagOpts) if err != nil { return output.ErrNetwork("API call failed: %v", err) } if apiErr := checkErr(result); apiErr != nil { - output.FormatValue(out, result, output.FormatJSON) + if apiErrOut != nil { + output.FormatValue(apiErrOut, result, output.FormatJSON) + } return apiErr } return output.JqFilter(out, result, jqExpr) }Then let callers choose the channel:
cmd/api/api.gocan keep passingout, whilecmd/service/service.goshould passerrOutornilto preserve its current contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/client/pagination.go` around lines 22 - 33, PaginateWithJq currently always writes failing API responses to the provided out writer (via output.FormatValue) which forces stdout behavior on all callers; change PaginateWithJq to accept a separate error-output writer (e.g., add errOut io.Writer parameter) or allow nil to skip dumping, then use errOut for output.FormatValue when checkErr returns an error instead of out; update callers (cmd/api/api.go should pass its stdout writer, cmd/service/service.go should pass its stderr writer or nil to preserve previous no-stdout-on-error contract) and adjust all call sites to the new signature.internal/output/jq.go (2)
79-103:⚠️ Potential issue | 🟠 MajorPropagate write failures from
writeJqValue.
writeJqValuepromises error propagation, but Lines 82-101 ignore everyfmt.Fprintln/fmt.Fprintfresult. Broken pipes or short writes will currently look like success, so jq output can be truncated without failing the command.Suggested fix
func writeJqValue(w io.Writer, v interface{}) error { switch val := v.(type) { case nil: - fmt.Fprintln(w, "null") + if _, err := fmt.Fprintln(w, "null"); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } case bool: - fmt.Fprintln(w, val) + if _, err := fmt.Fprintln(w, val); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } case int: - fmt.Fprintln(w, val) + if _, err := fmt.Fprintln(w, val); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } case float64: // Use %g to avoid trailing zeros, matching jq behavior. - fmt.Fprintf(w, "%g\n", val) + if _, err := fmt.Fprintf(w, "%g\n", val); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } case *big.Int: - fmt.Fprintln(w, val.String()) + if _, err := fmt.Fprintln(w, val.String()); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } case string: // Raw output for strings (no quotes), matching jq -r. - fmt.Fprintln(w, val) + if _, err := fmt.Fprintln(w, val); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } default: // Complex value (map, array): indented JSON. b, err := json.MarshalIndent(v, "", " ") if err != nil { return Errorf(ExitInternal, "jq_error", "failed to marshal jq result: %s", err) } - fmt.Fprintln(w, string(b)) + if _, err := fmt.Fprintln(w, string(b)); err != nil { + return Errorf(ExitInternal, "jq_error", "failed to write jq result: %s", err) + } } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/output/jq.go` around lines 79 - 103, writeJqValue currently ignores errors from its writes so write failures (broken pipe/short write) are lost; update every fmt.Fprintln/fmt.Fprintf call in writeJqValue (cases nil, bool, int, float64, *big.Int, string) to check the returned error (e.g., if _, err := fmt.Fprintln(w, ...); err != nil { return err }) and return it immediately so write errors propagate; keep the existing JSON marshal error handling in the default branch as-is.
106-118:⚠️ Potential issue | 🟠 MajorKeep large integral JSON numbers exact.
Lines 111-114 first narrow
Int64()toint, then fall back toFloat64(). That makes jq behavior architecture-dependent and rounds plain integers above2^53, so filters against large IDs can silently mis-match. Parse non-decimal/non-exponent tokens into*big.Intinstead. A regression case around something likejson.Number("4722366482869645213696")would lock this down.Suggested fix
import ( "encoding/json" "fmt" "io" "math/big" + "strings" "github.com/itchyny/gojq" ) @@ case json.Number: - if i, err := val.Int64(); err == nil { - return int(i) - } - if f, err := val.Float64(); err == nil { - return f - } - // Fallback: return as string (shouldn't happen for valid JSON numbers). - return val.String() + s := val.String() + if !strings.ContainsAny(s, ".eE") { + if bi, ok := new(big.Int).SetString(s, 10); ok { + return bi + } + } + if f, err := val.Float64(); err == nil { + return f + } + return sFor github.com/itchyny/gojq v0.12.17, when using Compile(...).Run(...) with Go values, which numeric types preserve large JSON integers exactly, and is *big.Int supported?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/output/jq.go` around lines 106 - 118, convertNumbers currently narrows json.Number to int/float64 which loses precision for huge integers; update convertNumbers to inspect val.String(): if it contains '.' or 'e'/'E' treat as float (use Float64()), otherwise attempt to parse as a big integer using new(big.Int).SetString(s, 10) and return the *big.Int on success; if SetString fails fall back to Int64()/int or Float64() as before; add the math/big import and ensure callers (and gojq usage) accept *big.Int to preserve exact large integers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/client/pagination.go`:
- Around line 22-33: PaginateWithJq currently always writes failing API
responses to the provided out writer (via output.FormatValue) which forces
stdout behavior on all callers; change PaginateWithJq to accept a separate
error-output writer (e.g., add errOut io.Writer parameter) or allow nil to skip
dumping, then use errOut for output.FormatValue when checkErr returns an error
instead of out; update callers (cmd/api/api.go should pass its stdout writer,
cmd/service/service.go should pass its stderr writer or nil to preserve previous
no-stdout-on-error contract) and adjust all call sites to the new signature.
In `@internal/output/jq.go`:
- Around line 79-103: writeJqValue currently ignores errors from its writes so
write failures (broken pipe/short write) are lost; update every
fmt.Fprintln/fmt.Fprintf call in writeJqValue (cases nil, bool, int, float64,
*big.Int, string) to check the returned error (e.g., if _, err :=
fmt.Fprintln(w, ...); err != nil { return err }) and return it immediately so
write errors propagate; keep the existing JSON marshal error handling in the
default branch as-is.
- Around line 106-118: convertNumbers currently narrows json.Number to
int/float64 which loses precision for huge integers; update convertNumbers to
inspect val.String(): if it contains '.' or 'e'/'E' treat as float (use
Float64()), otherwise attempt to parse as a big integer using
new(big.Int).SetString(s, 10) and return the *big.Int on success; if SetString
fails fall back to Int64()/int or Float64() as before; add the math/big import
and ensure callers (and gojq usage) accept *big.Int to preserve exact large
integers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 134ad475-0f10-4a95-a53f-70b22b4d58f9
📒 Files selected for processing (7)
.gitignorecmd/api/api.gocmd/service/service.gointernal/client/pagination.gointernal/output/jq.gointernal/output/jq_test.goshortcuts/common/runner.go
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/common/runner.go
- HandleResponse now returns a validation error when --jq is used with a non-JSON Content-Type instead of silently falling through to binary save. - Shortcut runtime jq errors are captured in RuntimeContext.outputErr and propagated as the command exit code, matching api/service behavior. Change-Id: Id4b5d2376e7613e1d247ec63c042bc3129d94eb0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/common/runner.go`:
- Around line 422-432: The current RuntimeContext.Out wraps responses in an
output.Envelope before applying JqFilter, so jq expressions must use .data.*
which differs from api/service commands; change RuntimeContext.Out to pass the
original data (not the Envelope) to output.JqFilter (call
output.JqFilter(ctx.IO().Out, data, ctx.JqExpr)) so jq usage matches api/service
behavior, and keep error handling identical (set ctx.outputErr on failure) while
leaving Envelope construction for non-jq paths unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5855e138-1535-409e-836c-0d9aa7769417
📒 Files selected for processing (4)
internal/client/response.gointernal/client/response_test.goshortcuts/common/runner.goshortcuts/common/runner_jq_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/client/response.go
- shortcuts/common/runner_jq_test.go
Summary
--jq/-qflag to all three command types (api, service, shortcuts) for filtering JSON output using jq expressions, powered by gojqjq -rbehavior), complex values print indented JSON--outputand non-json--format, pagination+jq forces aggregate-then-filter pathChanges
internal/output/jq.go: CoreJqFilter(),ValidateJqExpression(),convertNumbers()for gojq compatibilitycmd/api/api.go,cmd/service/service.go: Flag registration, validation, pagination+jq pathinternal/client/response.go:JqExprfield inResponseOptions, jq routing inHandleResponseshortcuts/common/runner.go:JqExpronRuntimeContext, jq routing inOut()/OutFormat()cmd/root.go: Help text updatego.mod: Addgojqdependency, bump Go to 1.24.0Test plan
internal/output: JqFilter with various expressions (field access, array iteration, pipe+select, builtins, structs, invalid expressions)cmd/api: Flag parsing (--jq / -q), --jq+--output conflict, --jq+--format conflict, invalid expression early validation, end-to-end filtering, --page-all+--jq combined pathcmd/service: Same coverage as apishortcuts/common: RuntimeContext.Out/OutFormat with jq, format conflict, invalid expression validation, normal output without jq🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Documentation
Tests
Chores