From 6e6ee0b30546521b9de14f0729dd619b86229e7e Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Tue, 5 May 2026 10:12:40 -0400 Subject: [PATCH 1/5] cli: convert api browse commands to structured --- internal/cmd/api/info.go | 106 ++++++++++----------------- internal/cmd/api/ls.go | 116 +++++++++++++----------------- internal/cmd/api/ls_test.go | 16 +++++ internal/structured/structured.go | 15 ++++ 4 files changed, 121 insertions(+), 132 deletions(-) diff --git a/internal/cmd/api/info.go b/internal/cmd/api/info.go index ee6573f..5da0a32 100644 --- a/internal/cmd/api/info.go +++ b/internal/cmd/api/info.go @@ -1,85 +1,57 @@ package api import ( - "encoding/json" - "fmt" + "context" "github.com/langchain-ai/langsmith-cli/internal/cache" "github.com/langchain-ai/langsmith-cli/internal/cmdutil" + "github.com/langchain-ai/langsmith-cli/internal/structured" "github.com/spf13/cobra" ) -func newInfoCmd() *cobra.Command { - var refresh bool +type infoInput struct { + Refresh bool +} - cmd := &cobra.Command{ - Use: "info METHOD PATH", - Short: "Show details for a specific API endpoint", - Long: `Show full details for a specific API endpoint including parameters, +var infoCommand = structured.Command[*infoInput]{ + Use: "info METHOD PATH", + Short: "Show details for a specific API endpoint", + Long: `Show full details for a specific API endpoint including parameters, request body schema, and response schema. Examples: langsmith api info GET /api/v1/sessions langsmith api info GET sessions langsmith api info POST runs/query`, - Args: cobra.ExactArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - method := args[0] - path := args[1] - - apiURL := cmdutil.ResolveAPIURL(cmd) - cacheDir := cache.DefaultDir() - format := cmdutil.ResolveFormat(cmd) - - spec, err := loadSpec(apiURL, cacheDir, refresh) - if err != nil { - return err - } - - detail, err := spec.LookupEndpoint(method, path) - if err != nil { - return err - } - - w := cmd.OutOrStdout() - - if format == "pretty" { - fmt.Fprintf(w, "%s %s\n", detail.Method, detail.Path) - fmt.Fprintf(w, "Tag: %s\n", detail.Tag) - fmt.Fprintf(w, "Summary: %s\n", detail.Summary) - if detail.Description != "" { - fmt.Fprintf(w, "Description: %s\n", detail.Description) - } - if len(detail.Parameters) > 0 { - fmt.Fprintf(w, "\nParameters:\n") - for _, p := range detail.Parameters { - req := "" - if p.Required { - req = " (required)" - } - fmt.Fprintf(w, " %-20s %-10s %s%s\n", p.Name, p.Type, p.Description, req) - } - } - if detail.RequestBody != nil { - fmt.Fprintf(w, "\nRequest Body:\n") - b, _ := json.MarshalIndent(detail.RequestBody, " ", " ") - fmt.Fprintf(w, " %s\n", b) - } - if detail.Response != nil { - fmt.Fprintf(w, "\nResponse Schema:\n") - b, _ := json.MarshalIndent(detail.Response, " ", " ") - fmt.Fprintf(w, " %s\n", b) - } - } else { - data, _ := json.MarshalIndent(detail, "", " ") - fmt.Fprintln(w, string(data)) - } - - return nil - }, - } - - cmd.Flags().BoolVar(&refresh, "refresh", false, "Force re-fetch of the OpenAPI spec") + Args: cobra.ExactArgs(2), + Input: func(cmd *cobra.Command) *infoInput { + in := &infoInput{} + cmd.Flags().BoolVar(&in.Refresh, "refresh", false, "Force re-fetch of the OpenAPI spec") + return in + }, + Action: func(_ context.Context, cmd *cobra.Command, in *infoInput, args []string) (any, error) { + spec, err := loadSpec(cmdutil.ResolveAPIURL(cmd), cache.DefaultDir(), in.Refresh) + if err != nil { + return nil, err + } + return spec.LookupEndpoint(args[0], args[1]) + }, + Render: structured.Template(`{{.Method}} {{.Path}} +Tag: {{.Tag}} +Summary: {{.Summary}}{{if .Description}} +Description: {{.Description}}{{end}}{{if .Parameters}} + +Parameters: +{{range .Parameters}} {{printf "%-20s %-10s %s" .Name .Type .Description}}{{if .Required}} (required){{end}} +{{end}}{{end}}{{if .RequestBody}} +Request Body: + {{jsonIndent .RequestBody " " " "}} +{{end}}{{if .Response}} +Response Schema: + {{jsonIndent .Response " " " "}} +{{end}}`), +} - return cmd +func newInfoCmd() *cobra.Command { + return infoCommand.Cobra() } diff --git a/internal/cmd/api/ls.go b/internal/cmd/api/ls.go index 9cbf56e..9b39a6f 100644 --- a/internal/cmd/api/ls.go +++ b/internal/cmd/api/ls.go @@ -1,27 +1,25 @@ package api import ( - "encoding/json" - "fmt" + "context" "strings" "github.com/langchain-ai/langsmith-cli/internal/cache" "github.com/langchain-ai/langsmith-cli/internal/cmdutil" - "github.com/olekukonko/tablewriter" + "github.com/langchain-ai/langsmith-cli/internal/structured" "github.com/spf13/cobra" ) -func newLsCmd() *cobra.Command { - var ( - tag string - search string - refresh bool - ) +type lsInput struct { + Tag string + Search string + Refresh bool +} - cmd := &cobra.Command{ - Use: "ls", - Short: "List available API endpoints from the OpenAPI spec", - Long: `List all available LangSmith API endpoints. +var lsCommand = structured.Command[*lsInput]{ + Use: "ls", + Short: "List available API endpoints from the OpenAPI spec", + Long: `List all available LangSmith API endpoints. The endpoint list is fetched from the OpenAPI spec and cached locally for 24 hours. @@ -31,64 +29,52 @@ Examples: langsmith api ls --search create langsmith api ls --tag run --search query langsmith api ls --refresh`, - RunE: func(cmd *cobra.Command, args []string) error { - apiURL := cmdutil.ResolveAPIURL(cmd) - cacheDir := cache.DefaultDir() - format := cmdutil.ResolveFormat(cmd) - - spec, err := loadSpec(apiURL, cacheDir, refresh) - if err != nil { - return err - } + Input: func(cmd *cobra.Command) *lsInput { + in := &lsInput{} + cmd.Flags().StringVarP(&in.Tag, "tag", "t", "", "Filter by tag") + cmd.Flags().StringVarP(&in.Search, "search", "s", "", "Search path, summary, or tag (case-insensitive)") + cmd.Flags().BoolVar(&in.Refresh, "refresh", false, "Force re-fetch of the OpenAPI spec") + return in + }, + Action: func(_ context.Context, cmd *cobra.Command, in *lsInput, _ []string) (any, error) { + spec, err := loadSpec(cmdutil.ResolveAPIURL(cmd), cache.DefaultDir(), in.Refresh) + if err != nil { + return nil, err + } - endpoints := spec.Endpoints() - - // Apply filters - if tag != "" || search != "" { - var filtered []Endpoint - for _, e := range endpoints { - if tag != "" && e.Tag != tag { + endpoints := spec.Endpoints() + if in.Tag != "" || in.Search != "" { + var filtered []Endpoint + for _, e := range endpoints { + if in.Tag != "" && e.Tag != in.Tag { + continue + } + if in.Search != "" { + q := strings.ToLower(in.Search) + if !strings.Contains(strings.ToLower(e.Path), q) && + !strings.Contains(strings.ToLower(e.Summary), q) && + !strings.Contains(strings.ToLower(e.Tag), q) { continue } - if search != "" { - q := strings.ToLower(search) - if !strings.Contains(strings.ToLower(e.Path), q) && - !strings.Contains(strings.ToLower(e.Summary), q) && - !strings.Contains(strings.ToLower(e.Tag), q) { - continue - } - } - filtered = append(filtered, e) } - endpoints = filtered + filtered = append(filtered, e) } + endpoints = filtered + } - w := cmd.OutOrStdout() - - if format == "pretty" { - table := tablewriter.NewWriter(w) - table.SetHeader([]string{"Method", "Path", "Tag", "Summary"}) - table.SetBorder(false) - table.SetColumnSeparator(" ") - table.SetHeaderLine(true) - table.SetAutoWrapText(false) - for _, e := range endpoints { - table.Append([]string{e.Method, e.Path, e.Tag, e.Summary}) - } - table.Render() - fmt.Fprintf(w, "(%d endpoints)\n", len(endpoints)) - } else { - data, _ := json.MarshalIndent(endpoints, "", " ") - fmt.Fprintln(w, string(data)) - } - - return nil + return endpoints, nil + }, + Render: structured.Table{ + Columns: []structured.Column{ + {Header: "Method", Template: "{{.Method}}"}, + {Header: "Path", Template: "{{.Path}}"}, + {Header: "Tag", Template: "{{.Tag}}"}, + {Header: "Summary", Template: "{{.Summary}}"}, }, - } - - cmd.Flags().StringVarP(&tag, "tag", "t", "", "Filter by tag") - cmd.Flags().StringVarP(&search, "search", "s", "", "Search path, summary, or tag (case-insensitive)") - cmd.Flags().BoolVar(&refresh, "refresh", false, "Force re-fetch of the OpenAPI spec") + Footer: structured.Template("({{len .}} endpoints)\n"), + }, +} - return cmd +func newLsCmd() *cobra.Command { + return lsCommand.Cobra() } diff --git a/internal/cmd/api/ls_test.go b/internal/cmd/api/ls_test.go index 632162f..808d42e 100644 --- a/internal/cmd/api/ls_test.go +++ b/internal/cmd/api/ls_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "strings" "testing" + + "github.com/stretchr/testify/require" ) func newTestSpecServer(t *testing.T) *httptest.Server { @@ -110,6 +112,20 @@ func TestLsCmd_Search(t *testing.T) { } } +func TestLsCmd_JQ(t *testing.T) { + ts := newTestSpecServer(t) + defer ts.Close() + + root := newTestRoot() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"api", "ls", "--api-url", ts.URL, "--jq", ".[0].method", "--refresh"}) + + require.NoError(t, root.Execute()) + require.Equal(t, "GET", strings.TrimSpace(out.String())) +} + func TestLsCmd_Pretty(t *testing.T) { ts := newTestSpecServer(t) defer ts.Close() diff --git a/internal/structured/structured.go b/internal/structured/structured.go index ba4ad76..c2641cd 100644 --- a/internal/structured/structured.go +++ b/internal/structured/structured.go @@ -178,6 +178,7 @@ func templateFuncs() template.FuncMap { "formatBytesOrDash": formatBytesOrDash, "formatCount": formatCount, "formatTime": formatTime, + "jsonIndent": jsonIndent, "shortID": shortID, } } @@ -230,6 +231,14 @@ func shortID(id string) string { return id } +func jsonIndent(v any, prefix, indent string) (string, error) { + b, err := json.MarshalIndent(v, prefix, indent) + if err != nil { + return "", err + } + return string(b), nil +} + type Template string func (s Template) RenderText(w io.Writer, model any) error { @@ -244,6 +253,7 @@ type Table struct { Title string Rows string Columns []Column + Footer Template } type Column struct { @@ -293,6 +303,11 @@ func (t Table) RenderText(w io.Writer, model any) error { } table.Render() + if t.Footer != "" { + if err := t.Footer.RenderText(w, model); err != nil { + return err + } + } return nil } From 8fdcdd66211107a8d95726e0ac9eb3cfe18491c6 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Tue, 5 May 2026 11:38:02 -0400 Subject: [PATCH 2/5] cli: make api requests structured --- internal/cmd/api/api.go | 41 +------- internal/cmd/api/api_test.go | 54 +++++++++- internal/cmd/api/request.go | 120 +++++++++++++++++---- internal/cmd/api/request_test.go | 167 +++++++++++++++++++++++------- internal/structured/structured.go | 32 +++++- 5 files changed, 310 insertions(+), 104 deletions(-) diff --git a/internal/cmd/api/api.go b/internal/cmd/api/api.go index a8a8786..d42d070 100644 --- a/internal/cmd/api/api.go +++ b/internal/cmd/api/api.go @@ -2,20 +2,12 @@ package api import ( "fmt" - "strings" - "github.com/langchain-ai/langsmith-cli/internal/cmdutil" "github.com/spf13/cobra" ) // NewCmd creates the top-level `langsmith api` command. func NewCmd() *cobra.Command { - var ( - body string - headers []string - include bool - ) - cmd := &cobra.Command{ Use: "api", Short: "Browse API endpoints and make authenticated requests", @@ -36,41 +28,18 @@ Make requests: langsmith api GET sessions --include`, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) < 2 { + if len(args) == 0 { return cmd.Help() } - - method := strings.ToUpper(args[0]) - if !isHTTPMethod(method) { - return fmt.Errorf("unknown subcommand or HTTP method: %q\nRun 'langsmith api --help' for usage", args[0]) - } - - path := args[1] - - c, err := cmdutil.GetClient(cmd) - if err != nil { - return err - } - - w := cmd.OutOrStdout() - statusCode, err := runRequest(c, method, path, body, headers, include, w) - if err != nil { - return err - } - if statusCode >= 400 { - return fmt.Errorf("HTTP %d", statusCode) - } - return nil + return fmt.Errorf("unknown subcommand or HTTP method: %q\nRun 'langsmith api --help' for usage", args[0]) }, } - // Flags for request mode - cmd.Flags().StringVar(&body, "body", "", `Request body (JSON string, @file, or @- for stdin)`) - cmd.Flags().StringArrayVarP(&headers, "header", "H", nil, "Additional headers (Key:Value, repeatable)") - cmd.Flags().BoolVarP(&include, "include", "i", false, "Include HTTP response headers in output") - cmd.AddCommand(newLsCmd()) cmd.AddCommand(newInfoCmd()) + for _, method := range []string{"GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"} { + cmd.AddCommand(requestCommand(method).Cobra()) + } return cmd } diff --git a/internal/cmd/api/api_test.go b/internal/cmd/api/api_test.go index c61f545..745c60f 100644 --- a/internal/cmd/api/api_test.go +++ b/internal/cmd/api/api_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestNewCmd_HasSubcommands(t *testing.T) { @@ -21,6 +23,12 @@ func TestNewCmd_HasSubcommands(t *testing.T) { if !names["info"] { t.Error("missing subcommand 'info'") } + if !names["GET"] { + t.Error("missing subcommand 'GET'") + } + if !names["POST"] { + t.Error("missing subcommand 'POST'") + } } func TestNewCmd_UseField(t *testing.T) { @@ -32,10 +40,14 @@ func TestNewCmd_UseField(t *testing.T) { func TestNewCmd_RequestFlags(t *testing.T) { cmd := NewCmd() + getCmd, _, err := cmd.Find([]string{"GET"}) + if err != nil { + t.Fatalf("finding GET command: %v", err) + } for _, name := range []string{"body", "header", "include"} { - f := cmd.Flags().Lookup(name) + f := getCmd.Flags().Lookup(name) if f == nil { - t.Errorf("flag --%s not found on api command", name) + t.Errorf("flag --%s not found on GET command", name) } } } @@ -71,6 +83,44 @@ func TestNewCmd_GETRequest(t *testing.T) { } } +func TestNewCmd_GETRequestJQ(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + w.WriteHeader(200) + _, _ = w.Write([]byte(`{"ok":true,"name":"alpha"}`)) + })) + defer ts.Close() + + root := newTestRoot() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"api", "--api-key", "test-key", "--api-url", ts.URL, "GET", "sessions", "--jq", ".name"}) + + require.NoError(t, root.Execute()) + require.Equal(t, "alpha", strings.TrimSpace(out.String())) +} + +func TestNewCmd_GETRequestJQNonJSON(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + _, _ = w.Write([]byte(`plain text`)) + })) + defer ts.Close() + + root := newTestRoot() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"api", "--api-key", "test-key", "--api-url", ts.URL, "GET", "sessions", "--jq", ".name"}) + + err := root.Execute() + require.Error(t, err) + require.Contains(t, err.Error(), "response body is not JSON") +} + func TestNewCmd_POSTWithBody(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/openapi.json" { diff --git a/internal/cmd/api/request.go b/internal/cmd/api/request.go index 44d559c..a00fd58 100644 --- a/internal/cmd/api/request.go +++ b/internal/cmd/api/request.go @@ -11,11 +11,67 @@ import ( "strings" "github.com/langchain-ai/langsmith-cli/internal/client" + "github.com/langchain-ai/langsmith-cli/internal/cmdutil" + "github.com/langchain-ai/langsmith-cli/internal/structured" + "github.com/spf13/cobra" ) -// runRequest executes an HTTP request and writes the response to w. -// Returns the HTTP status code and any transport-level error. -func runRequest(c *client.Client, method, path, body string, headers []string, include bool, w io.Writer) (int, error) { +type requestInput struct { + Body string + Headers []string + Include bool +} + +type apiResponse struct { + StatusCode int + Proto string + Headers http.Header + Body any + RawBody []byte + IsJSON bool + Include bool +} + +func requestCommand(method string) structured.Command[*requestInput] { + return structured.Command[*requestInput]{ + Use: method + " PATH", + Short: fmt.Sprintf("Make an authenticated %s request", method), + Args: cobra.ExactArgs(1), + Input: func(cmd *cobra.Command) *requestInput { + in := &requestInput{} + cmd.Flags().StringVar(&in.Body, "body", "", `Request body (JSON string, @file, or @- for stdin)`) + cmd.Flags().StringArrayVarP(&in.Headers, "header", "H", nil, "Additional headers (Key:Value, repeatable)") + cmd.Flags().BoolVarP(&in.Include, "include", "i", false, "Include HTTP response headers in output") + return in + }, + Action: func(_ context.Context, cmd *cobra.Command, in *requestInput, args []string) (any, error) { + c, err := cmdutil.GetClient(cmd) + if err != nil { + return nil, err + } + resp, err := runRequest(c, method, args[0], in.Body, in.Headers, in.Include) + if err != nil { + return nil, err + } + if cmdutil.ResolveJQ(cmd) != "" && !resp.IsJSON { + return nil, fmt.Errorf("response body is not JSON") + } + var afterRender error + if resp.StatusCode >= 400 { + afterRender = fmt.Errorf("HTTP %d", resp.StatusCode) + } + return structured.Result{ + Model: resp.Body, + TextModel: resp, + ErrAfterRender: afterRender, + }, nil + }, + Render: apiResponseRenderer{}, + } +} + +// runRequest executes an HTTP request and returns the response model. +func runRequest(c *client.Client, method, path, body string, headers []string, include bool) (apiResponse, error) { apiURL := c.APIURL() fullURL := resolveEndpoint(apiURL, path) @@ -34,7 +90,7 @@ func runRequest(c *client.Client, method, path, body string, headers []string, i // Resolve body bodyReader, err := resolveBody(body) if err != nil { - return 0, err + return apiResponse{}, err } // Parse extra headers @@ -42,39 +98,61 @@ func runRequest(c *client.Client, method, path, body string, headers []string, i for _, h := range headers { k, v, ok := strings.Cut(h, ":") if !ok { - return 0, fmt.Errorf("invalid header format %q (expected Key:Value)", h) + return apiResponse{}, fmt.Errorf("invalid header format %q (expected Key:Value)", h) } extraHeaders.Add(strings.TrimSpace(k), strings.TrimSpace(v)) } statusCode, proto, respHeaders, respBody, err := reqClient.RawDo(context.Background(), method, relPath, bodyReader, extraHeaders) if err != nil { - return 0, err + return apiResponse{}, err } - // Print response headers if --include - if include { - fmt.Fprintf(w, "%s %d %s\n", proto, statusCode, http.StatusText(statusCode)) - for k, vals := range respHeaders { + resp := apiResponse{ + StatusCode: statusCode, + Proto: proto, + Headers: respHeaders, + RawBody: respBody, + Body: string(respBody), + Include: include, + } + var decodedBody any + if err := json.Unmarshal(respBody, &decodedBody); err == nil { + resp.Body = decodedBody + resp.IsJSON = true + } + return resp, nil +} + +type apiResponseRenderer struct{} + +func (apiResponseRenderer) RenderText(w io.Writer, model any) error { + resp, ok := model.(apiResponse) + if !ok { + return fmt.Errorf("expected apiResponse, got %T", model) + } + if resp.Include { + fmt.Fprintf(w, "%s %d %s\n", resp.Proto, resp.StatusCode, http.StatusText(resp.StatusCode)) + for k, vals := range resp.Headers { for _, v := range vals { fmt.Fprintf(w, "%s: %s\n", k, v) } } fmt.Fprintln(w) } - - // Pretty-print JSON if possible, otherwise print raw - var prettyBuf bytes.Buffer - if json.Indent(&prettyBuf, respBody, "", " ") == nil { - fmt.Fprintln(w, prettyBuf.String()) - } else { - if _, err := w.Write(respBody); err != nil { - return statusCode, fmt.Errorf("writing response: %w", err) + if resp.IsJSON { + var prettyBuf bytes.Buffer + if err := json.Indent(&prettyBuf, resp.RawBody, "", " "); err != nil { + return err } - fmt.Fprintln(w) + fmt.Fprintln(w, prettyBuf.String()) + return nil } - - return statusCode, nil + if _, err := w.Write(resp.RawBody); err != nil { + return fmt.Errorf("writing response: %w", err) + } + fmt.Fprintln(w) + return nil } // resolveBody resolves a --body value to an io.Reader. diff --git a/internal/cmd/api/request_test.go b/internal/cmd/api/request_test.go index 15a287f..b172b94 100644 --- a/internal/cmd/api/request_test.go +++ b/internal/cmd/api/request_test.go @@ -3,6 +3,7 @@ package api import ( "bytes" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -11,8 +12,24 @@ import ( "testing" "github.com/langchain-ai/langsmith-cli/internal/client" + "github.com/langchain-ai/langsmith-cli/internal/structured" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" ) +func renderTestResponse(t *testing.T, resp apiResponse, args ...string) (string, error) { + t.Helper() + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("format", "pretty", "") + cmd.Flags().String("jq", "", "") + cmd.SetArgs(args) + require.NoError(t, cmd.ParseFlags(args)) + var out bytes.Buffer + cmd.SetOut(&out) + err := structured.Render(cmd, structured.Result{Model: resp.Body, TextModel: resp}, apiResponseRenderer{}) + return out.String(), err +} + func TestRunRequest_GET(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != "GET" { @@ -29,17 +46,18 @@ func TestRunRequest_GET(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("test-key", ts.URL) - code, err := runRequest(c, "GET", "sessions", "", nil, false, &out) + resp, err := runRequest(c, "GET", "sessions", "", nil, false) if err != nil { t.Fatalf("unexpected error: %v", err) } - if code != 200 { - t.Errorf("expected status 200, got %d", code) + if resp.StatusCode != 200 { + t.Errorf("expected status 200, got %d", resp.StatusCode) } - if !strings.Contains(out.String(), `"id"`) { - t.Errorf("expected JSON output, got %q", out.String()) + out, err := renderTestResponse(t, resp) + require.NoError(t, err) + if !strings.Contains(out, `"id"`) { + t.Errorf("expected JSON output, got %q", out) } } @@ -59,14 +77,13 @@ func TestRunRequest_POSTWithBody(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("key", ts.URL) - code, err := runRequest(c, "POST", "sessions", `{"name":"test"}`, nil, false, &out) + resp, err := runRequest(c, "POST", "sessions", `{"name":"test"}`, nil, false) if err != nil { t.Fatalf("unexpected error: %v", err) } - if code != 201 { - t.Errorf("expected 201, got %d", code) + if resp.StatusCode != 201 { + t.Errorf("expected 201, got %d", resp.StatusCode) } } @@ -80,9 +97,8 @@ func TestRunRequest_ExtraHeaders(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("key", ts.URL) - _, err := runRequest(c, "GET", "sessions", "", []string{"X-Custom:val"}, false, &out) + _, err := runRequest(c, "GET", "sessions", "", []string{"X-Custom:val"}, false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -96,20 +112,92 @@ func TestRunRequest_Include(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("key", ts.URL) - _, err := runRequest(c, "GET", "sessions", "", nil, true, &out) + resp, err := runRequest(c, "GET", "sessions", "", nil, true) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !strings.Contains(out.String(), "200") { - t.Errorf("expected status line, got %q", out.String()) + out, err := renderTestResponse(t, resp) + require.NoError(t, err) + if !strings.Contains(out, "200") { + t.Errorf("expected status line, got %q", out) } - if !strings.Contains(out.String(), "X-Request-Id") { - t.Errorf("expected header in output, got %q", out.String()) + if !strings.Contains(out, "X-Request-Id") { + t.Errorf("expected header in output, got %q", out) } } +func TestAPIResponseRenderer_FormatJSONBodyOnly(t *testing.T) { + resp := apiResponse{ + StatusCode: 200, + Proto: "HTTP/1.1", + Headers: http.Header{"X-Request-Id": []string{"abc"}}, + Body: map[string]any{"ok": true}, + RawBody: []byte(`{"ok":true}`), + IsJSON: true, + Include: true, + } + + out, err := renderTestResponse(t, resp, "--format", "json") + + require.NoError(t, err) + require.JSONEq(t, `{"ok":true}`, out) + require.NotContains(t, out, "X-Request-Id") +} + +func TestAPIResponseRenderer_NonJSONRawOutput(t *testing.T) { + resp := apiResponse{ + StatusCode: 200, + Proto: "HTTP/1.1", + RawBody: []byte("plain text"), + Body: "plain text", + } + + out, err := renderTestResponse(t, resp) + + require.NoError(t, err) + require.Equal(t, "plain text\n", out) +} + +func TestAPIResponseRenderer_JQScalar(t *testing.T) { + resp := apiResponse{ + StatusCode: 200, + Proto: "HTTP/1.1", + RawBody: []byte(`{"name":"alpha"}`), + Body: map[string]any{"name": "alpha"}, + IsJSON: true, + } + + out, err := renderTestResponse(t, resp, "--jq", ".name") + + require.NoError(t, err) + require.Equal(t, "alpha\n", out) +} + +func TestAPIResponseRenderer_ReturnsErrorAfterRender(t *testing.T) { + resp := apiResponse{ + StatusCode: 404, + Proto: "HTTP/1.1", + RawBody: []byte(`{"detail":"not found"}`), + Body: map[string]any{"detail": "not found"}, + IsJSON: true, + } + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("format", "pretty", "") + cmd.Flags().String("jq", "", "") + var out bytes.Buffer + cmd.SetOut(&out) + + err := structured.Render(cmd, structured.Result{ + Model: resp.Body, + TextModel: resp, + ErrAfterRender: fmt.Errorf("HTTP 404"), + }, apiResponseRenderer{}) + + require.EqualError(t, err, "HTTP 404") + require.Contains(t, out.String(), "not found") +} + func TestRunRequest_4xxPrintsBody(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(404) @@ -117,17 +205,18 @@ func TestRunRequest_4xxPrintsBody(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("key", ts.URL) - code, err := runRequest(c, "GET", "sessions", "", nil, false, &out) + resp, err := runRequest(c, "GET", "sessions", "", nil, false) if err != nil { t.Fatalf("unexpected error: %v", err) } - if code != 404 { - t.Errorf("expected 404, got %d", code) + if resp.StatusCode != 404 { + t.Errorf("expected 404, got %d", resp.StatusCode) } - if !strings.Contains(out.String(), "not found") { - t.Errorf("expected error body in output, got %q", out.String()) + out, err := renderTestResponse(t, resp) + require.NoError(t, err) + if !strings.Contains(out, "not found") { + t.Errorf("expected error body in output, got %q", out) } } @@ -143,17 +232,18 @@ func TestRunRequest_BodyFromFile(t *testing.T) { _, _ = f.WriteString(`{"from":"file"}`) f.Close() - var out bytes.Buffer c := client.New("key", ts.URL) - code, err := runRequest(c, "POST", "sessions", "@"+f.Name(), nil, false, &out) + resp, err := runRequest(c, "POST", "sessions", "@"+f.Name(), nil, false) if err != nil { t.Fatalf("unexpected error: %v", err) } - if code != 200 { - t.Errorf("expected 200, got %d", code) + if resp.StatusCode != 200 { + t.Errorf("expected 200, got %d", resp.StatusCode) } - if !strings.Contains(out.String(), "from") { - t.Errorf("expected file body echoed, got %q", out.String()) + out, err := renderTestResponse(t, resp) + require.NoError(t, err) + if !strings.Contains(out, "from") { + t.Errorf("expected file body echoed, got %q", out) } } @@ -170,14 +260,13 @@ func TestRunRequest_FullURLDifferentHost(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("key", "https://different.host") - code, err := runRequest(c, "GET", ts.URL+"/custom/endpoint", "", nil, false, &out) + resp, err := runRequest(c, "GET", ts.URL+"/custom/endpoint", "", nil, false) if err != nil { t.Fatalf("unexpected error: %v", err) } - if code != 200 { - t.Errorf("expected 200, got %d", code) + if resp.StatusCode != 200 { + t.Errorf("expected 200, got %d", resp.StatusCode) } } @@ -192,9 +281,8 @@ func TestRunRequest_MultiValueHeaders(t *testing.T) { })) defer ts.Close() - var out bytes.Buffer c := client.New("key", ts.URL) - _, err := runRequest(c, "GET", "sessions", "", []string{"X-Multi:one", "X-Multi:two"}, false, &out) + _, err := runRequest(c, "GET", "sessions", "", []string{"X-Multi:one", "X-Multi:two"}, false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -221,13 +309,12 @@ func TestRunRequest_PrefixConfusionAttack(t *testing.T) { apiURL := ts.URL[:len(ts.URL)-1] // e.g. "http://127.0.0.1:5432" → "http://127.0.0.1:543" c := client.New("secret-key", apiURL) - var out bytes.Buffer - code, err := runRequest(c, "GET", ts.URL+"/steal", "", nil, false, &out) + resp, err := runRequest(c, "GET", ts.URL+"/steal", "", nil, false) if err != nil { t.Fatalf("unexpected error: %v", err) } - if code != 200 { - t.Errorf("expected 200, got %d", code) + if resp.StatusCode != 200 { + t.Errorf("expected 200, got %d", resp.StatusCode) } } diff --git a/internal/structured/structured.go b/internal/structured/structured.go index c2641cd..2ad7cbc 100644 --- a/internal/structured/structured.go +++ b/internal/structured/structured.go @@ -35,6 +35,12 @@ type Command[I any] struct { Render Spec } +type Result struct { + Model any + TextModel any + ErrAfterRender error +} + func (c Command[I]) Cobra() *cobra.Command { var input I cmd := &cobra.Command{ @@ -77,16 +83,32 @@ func (p Parent) Cobra() *cobra.Command { } func Render(cmd *cobra.Command, model any, spec Spec) error { + errAfterRender := error(nil) + textModel := model + if result, ok := model.(Result); ok { + model = result.Model + textModel = result.TextModel + if textModel == nil { + textModel = model + } + errAfterRender = result.ErrAfterRender + } + w := cmd.OutOrStdout() + var err error if expr := cmdutil.ResolveJQ(cmd); expr != "" { - return renderJQ(w, model, expr) - } - if cmdutil.ResolveFormat(cmd) != "pretty" || spec == nil { + err = renderJQ(w, model, expr) + } else if cmdutil.ResolveFormat(cmd) != "pretty" || spec == nil { enc := json.NewEncoder(w) enc.SetIndent("", " ") - return enc.Encode(model) + err = enc.Encode(model) + } else { + err = spec.RenderText(w, textModel) + } + if err != nil { + return err } - return spec.RenderText(w, model) + return errAfterRender } func renderJQ(w io.Writer, model any, expr string) error { From cf73bcf81aace70785020a9889b14ce9b29c605c Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Tue, 5 May 2026 12:51:53 -0400 Subject: [PATCH 3/5] cli: keep jq resolution in structured --- internal/cmd/api/request.go | 10 +++++++--- internal/cmdutil/resolve.go | 5 ----- internal/cmdutil/resolve_test.go | 8 -------- internal/structured/structured.go | 17 ++++++++++++++++- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/internal/cmd/api/request.go b/internal/cmd/api/request.go index a00fd58..7effdc3 100644 --- a/internal/cmd/api/request.go +++ b/internal/cmd/api/request.go @@ -32,6 +32,13 @@ type apiResponse struct { Include bool } +func (r apiResponse) CanRenderJQ() error { + if !r.IsJSON { + return fmt.Errorf("response body is not JSON") + } + return nil +} + func requestCommand(method string) structured.Command[*requestInput] { return structured.Command[*requestInput]{ Use: method + " PATH", @@ -53,9 +60,6 @@ func requestCommand(method string) structured.Command[*requestInput] { if err != nil { return nil, err } - if cmdutil.ResolveJQ(cmd) != "" && !resp.IsJSON { - return nil, fmt.Errorf("response body is not JSON") - } var afterRender error if resp.StatusCode >= 400 { afterRender = fmt.Errorf("HTTP %d", resp.StatusCode) diff --git a/internal/cmdutil/resolve.go b/internal/cmdutil/resolve.go index 4e418eb..19ba78f 100644 --- a/internal/cmdutil/resolve.go +++ b/internal/cmdutil/resolve.go @@ -78,11 +78,6 @@ func ResolveFormat(cmd *cobra.Command) string { return v } -// ResolveJQ reads the jq filter from cobra's flag tree. -func ResolveJQ(cmd *cobra.Command) string { - return getFlagString(cmd, "jq") -} - // GetClient creates a LangSmith client from cobra flags, returning an error // if no API key or OAuth access token is available. func GetClient(cmd *cobra.Command) (*client.Client, error) { diff --git a/internal/cmdutil/resolve_test.go b/internal/cmdutil/resolve_test.go index dc41087..82b1f77 100644 --- a/internal/cmdutil/resolve_test.go +++ b/internal/cmdutil/resolve_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/spf13/cobra" - "github.com/stretchr/testify/require" ) func newTestCmd() *cobra.Command { @@ -91,13 +90,6 @@ func TestResolveFormat_Default(t *testing.T) { } } -func TestResolveJQ_Flag(t *testing.T) { - cmd := newTestCmd() - cmd.PersistentFlags().String("jq", "", "") - _ = cmd.PersistentFlags().Set("jq", ".name") - require.Equal(t, ".name", ResolveJQ(cmd)) -} - func TestGetClient_Success(t *testing.T) { cmd := newTestCmd() _ = cmd.PersistentFlags().Set("api-key", "test-key") diff --git a/internal/structured/structured.go b/internal/structured/structured.go index 2ad7cbc..fa6ef58 100644 --- a/internal/structured/structured.go +++ b/internal/structured/structured.go @@ -96,7 +96,12 @@ func Render(cmd *cobra.Command, model any, spec Spec) error { w := cmd.OutOrStdout() var err error - if expr := cmdutil.ResolveJQ(cmd); expr != "" { + if expr := resolveJQ(cmd); expr != "" { + if typed, ok := textModel.(interface{ CanRenderJQ() error }); ok { + if err := typed.CanRenderJQ(); err != nil { + return err + } + } err = renderJQ(w, model, expr) } else if cmdutil.ResolveFormat(cmd) != "pretty" || spec == nil { enc := json.NewEncoder(w) @@ -111,6 +116,16 @@ func Render(cmd *cobra.Command, model any, spec Spec) error { return errAfterRender } +func resolveJQ(cmd *cobra.Command) string { + if f := cmd.Flags().Lookup("jq"); f != nil { + return f.Value.String() + } + if f := cmd.PersistentFlags().Lookup("jq"); f != nil { + return f.Value.String() + } + return "" +} + func renderJQ(w io.Writer, model any, expr string) error { query, err := gojq.Parse(expr) if err != nil { From 01dfee7969a5a81cf45d7de7b067e853065b5a93 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Tue, 5 May 2026 12:53:56 -0400 Subject: [PATCH 4/5] cli: default structured commands to json --- internal/cmd/api/api.go | 3 +- internal/cmd/api/api_test.go | 2 +- internal/cmd/api/request.go | 48 ++----------------- internal/cmd/api/request_test.go | 64 ++++++-------------------- internal/structured/structured_test.go | 10 ++++ 5 files changed, 30 insertions(+), 97 deletions(-) diff --git a/internal/cmd/api/api.go b/internal/cmd/api/api.go index d42d070..594f8cf 100644 --- a/internal/cmd/api/api.go +++ b/internal/cmd/api/api.go @@ -24,8 +24,7 @@ Make requests: langsmith api POST runs/query --body '{"session_id":"abc"}' langsmith api DELETE sessions/abc-123 langsmith api POST datasets --body @body.json - echo '{"name":"x"}' | langsmith api POST sessions --body @- - langsmith api GET sessions --include`, + echo '{"name":"x"}' | langsmith api POST sessions --body @-`, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { diff --git a/internal/cmd/api/api_test.go b/internal/cmd/api/api_test.go index 745c60f..50d1e84 100644 --- a/internal/cmd/api/api_test.go +++ b/internal/cmd/api/api_test.go @@ -44,7 +44,7 @@ func TestNewCmd_RequestFlags(t *testing.T) { if err != nil { t.Fatalf("finding GET command: %v", err) } - for _, name := range []string{"body", "header", "include"} { + for _, name := range []string{"body", "header"} { f := getCmd.Flags().Lookup(name) if f == nil { t.Errorf("flag --%s not found on GET command", name) diff --git a/internal/cmd/api/request.go b/internal/cmd/api/request.go index 7effdc3..1fbe48a 100644 --- a/internal/cmd/api/request.go +++ b/internal/cmd/api/request.go @@ -19,17 +19,12 @@ import ( type requestInput struct { Body string Headers []string - Include bool } type apiResponse struct { StatusCode int - Proto string - Headers http.Header Body any - RawBody []byte IsJSON bool - Include bool } func (r apiResponse) CanRenderJQ() error { @@ -48,7 +43,6 @@ func requestCommand(method string) structured.Command[*requestInput] { in := &requestInput{} cmd.Flags().StringVar(&in.Body, "body", "", `Request body (JSON string, @file, or @- for stdin)`) cmd.Flags().StringArrayVarP(&in.Headers, "header", "H", nil, "Additional headers (Key:Value, repeatable)") - cmd.Flags().BoolVarP(&in.Include, "include", "i", false, "Include HTTP response headers in output") return in }, Action: func(_ context.Context, cmd *cobra.Command, in *requestInput, args []string) (any, error) { @@ -56,7 +50,7 @@ func requestCommand(method string) structured.Command[*requestInput] { if err != nil { return nil, err } - resp, err := runRequest(c, method, args[0], in.Body, in.Headers, in.Include) + resp, err := runRequest(c, method, args[0], in.Body, in.Headers) if err != nil { return nil, err } @@ -70,12 +64,11 @@ func requestCommand(method string) structured.Command[*requestInput] { ErrAfterRender: afterRender, }, nil }, - Render: apiResponseRenderer{}, } } // runRequest executes an HTTP request and returns the response model. -func runRequest(c *client.Client, method, path, body string, headers []string, include bool) (apiResponse, error) { +func runRequest(c *client.Client, method, path, body string, headers []string) (apiResponse, error) { apiURL := c.APIURL() fullURL := resolveEndpoint(apiURL, path) @@ -107,18 +100,14 @@ func runRequest(c *client.Client, method, path, body string, headers []string, i extraHeaders.Add(strings.TrimSpace(k), strings.TrimSpace(v)) } - statusCode, proto, respHeaders, respBody, err := reqClient.RawDo(context.Background(), method, relPath, bodyReader, extraHeaders) + statusCode, _, _, respBody, err := reqClient.RawDo(context.Background(), method, relPath, bodyReader, extraHeaders) if err != nil { return apiResponse{}, err } resp := apiResponse{ StatusCode: statusCode, - Proto: proto, - Headers: respHeaders, - RawBody: respBody, Body: string(respBody), - Include: include, } var decodedBody any if err := json.Unmarshal(respBody, &decodedBody); err == nil { @@ -128,37 +117,6 @@ func runRequest(c *client.Client, method, path, body string, headers []string, i return resp, nil } -type apiResponseRenderer struct{} - -func (apiResponseRenderer) RenderText(w io.Writer, model any) error { - resp, ok := model.(apiResponse) - if !ok { - return fmt.Errorf("expected apiResponse, got %T", model) - } - if resp.Include { - fmt.Fprintf(w, "%s %d %s\n", resp.Proto, resp.StatusCode, http.StatusText(resp.StatusCode)) - for k, vals := range resp.Headers { - for _, v := range vals { - fmt.Fprintf(w, "%s: %s\n", k, v) - } - } - fmt.Fprintln(w) - } - if resp.IsJSON { - var prettyBuf bytes.Buffer - if err := json.Indent(&prettyBuf, resp.RawBody, "", " "); err != nil { - return err - } - fmt.Fprintln(w, prettyBuf.String()) - return nil - } - if _, err := w.Write(resp.RawBody); err != nil { - return fmt.Errorf("writing response: %w", err) - } - fmt.Fprintln(w) - return nil -} - // resolveBody resolves a --body value to an io.Reader. // - Empty string → nil (no body). // - "@-" → stdin. diff --git a/internal/cmd/api/request_test.go b/internal/cmd/api/request_test.go index b172b94..9ae6cf3 100644 --- a/internal/cmd/api/request_test.go +++ b/internal/cmd/api/request_test.go @@ -26,7 +26,7 @@ func renderTestResponse(t *testing.T, resp apiResponse, args ...string) (string, require.NoError(t, cmd.ParseFlags(args)) var out bytes.Buffer cmd.SetOut(&out) - err := structured.Render(cmd, structured.Result{Model: resp.Body, TextModel: resp}, apiResponseRenderer{}) + err := structured.Render(cmd, structured.Result{Model: resp.Body, TextModel: resp}, nil) return out.String(), err } @@ -47,7 +47,7 @@ func TestRunRequest_GET(t *testing.T) { defer ts.Close() c := client.New("test-key", ts.URL) - resp, err := runRequest(c, "GET", "sessions", "", nil, false) + resp, err := runRequest(c, "GET", "sessions", "", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -78,7 +78,7 @@ func TestRunRequest_POSTWithBody(t *testing.T) { defer ts.Close() c := client.New("key", ts.URL) - resp, err := runRequest(c, "POST", "sessions", `{"name":"test"}`, nil, false) + resp, err := runRequest(c, "POST", "sessions", `{"name":"test"}`, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -98,72 +98,40 @@ func TestRunRequest_ExtraHeaders(t *testing.T) { defer ts.Close() c := client.New("key", ts.URL) - _, err := runRequest(c, "GET", "sessions", "", []string{"X-Custom:val"}, false) + _, err := runRequest(c, "GET", "sessions", "", []string{"X-Custom:val"}) if err != nil { t.Fatalf("unexpected error: %v", err) } } -func TestRunRequest_Include(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("X-Request-Id", "abc") - w.WriteHeader(200) - _, _ = w.Write([]byte(`{}`)) - })) - defer ts.Close() - - c := client.New("key", ts.URL) - resp, err := runRequest(c, "GET", "sessions", "", nil, true) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - out, err := renderTestResponse(t, resp) - require.NoError(t, err) - if !strings.Contains(out, "200") { - t.Errorf("expected status line, got %q", out) - } - if !strings.Contains(out, "X-Request-Id") { - t.Errorf("expected header in output, got %q", out) - } -} - -func TestAPIResponseRenderer_FormatJSONBodyOnly(t *testing.T) { +func TestRunRequest_FormatJSONBodyOnly(t *testing.T) { resp := apiResponse{ StatusCode: 200, - Proto: "HTTP/1.1", - Headers: http.Header{"X-Request-Id": []string{"abc"}}, Body: map[string]any{"ok": true}, - RawBody: []byte(`{"ok":true}`), IsJSON: true, - Include: true, } out, err := renderTestResponse(t, resp, "--format", "json") require.NoError(t, err) require.JSONEq(t, `{"ok":true}`, out) - require.NotContains(t, out, "X-Request-Id") } -func TestAPIResponseRenderer_NonJSONRawOutput(t *testing.T) { +func TestRunRequest_NilRenderOutputsJSON(t *testing.T) { resp := apiResponse{ StatusCode: 200, - Proto: "HTTP/1.1", - RawBody: []byte("plain text"), Body: "plain text", } out, err := renderTestResponse(t, resp) require.NoError(t, err) - require.Equal(t, "plain text\n", out) + require.JSONEq(t, `"plain text"`, out) } -func TestAPIResponseRenderer_JQScalar(t *testing.T) { +func TestRunRequest_JQScalar(t *testing.T) { resp := apiResponse{ StatusCode: 200, - Proto: "HTTP/1.1", - RawBody: []byte(`{"name":"alpha"}`), Body: map[string]any{"name": "alpha"}, IsJSON: true, } @@ -174,11 +142,9 @@ func TestAPIResponseRenderer_JQScalar(t *testing.T) { require.Equal(t, "alpha\n", out) } -func TestAPIResponseRenderer_ReturnsErrorAfterRender(t *testing.T) { +func TestRunRequest_ReturnsErrorAfterRender(t *testing.T) { resp := apiResponse{ StatusCode: 404, - Proto: "HTTP/1.1", - RawBody: []byte(`{"detail":"not found"}`), Body: map[string]any{"detail": "not found"}, IsJSON: true, } @@ -192,7 +158,7 @@ func TestAPIResponseRenderer_ReturnsErrorAfterRender(t *testing.T) { Model: resp.Body, TextModel: resp, ErrAfterRender: fmt.Errorf("HTTP 404"), - }, apiResponseRenderer{}) + }, nil) require.EqualError(t, err, "HTTP 404") require.Contains(t, out.String(), "not found") @@ -206,7 +172,7 @@ func TestRunRequest_4xxPrintsBody(t *testing.T) { defer ts.Close() c := client.New("key", ts.URL) - resp, err := runRequest(c, "GET", "sessions", "", nil, false) + resp, err := runRequest(c, "GET", "sessions", "", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -233,7 +199,7 @@ func TestRunRequest_BodyFromFile(t *testing.T) { f.Close() c := client.New("key", ts.URL) - resp, err := runRequest(c, "POST", "sessions", "@"+f.Name(), nil, false) + resp, err := runRequest(c, "POST", "sessions", "@"+f.Name(), nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -261,7 +227,7 @@ func TestRunRequest_FullURLDifferentHost(t *testing.T) { defer ts.Close() c := client.New("key", "https://different.host") - resp, err := runRequest(c, "GET", ts.URL+"/custom/endpoint", "", nil, false) + resp, err := runRequest(c, "GET", ts.URL+"/custom/endpoint", "", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -282,7 +248,7 @@ func TestRunRequest_MultiValueHeaders(t *testing.T) { defer ts.Close() c := client.New("key", ts.URL) - _, err := runRequest(c, "GET", "sessions", "", []string{"X-Multi:one", "X-Multi:two"}, false) + _, err := runRequest(c, "GET", "sessions", "", []string{"X-Multi:one", "X-Multi:two"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -309,7 +275,7 @@ func TestRunRequest_PrefixConfusionAttack(t *testing.T) { apiURL := ts.URL[:len(ts.URL)-1] // e.g. "http://127.0.0.1:5432" → "http://127.0.0.1:543" c := client.New("secret-key", apiURL) - resp, err := runRequest(c, "GET", ts.URL+"/steal", "", nil, false) + resp, err := runRequest(c, "GET", ts.URL+"/steal", "", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/structured/structured_test.go b/internal/structured/structured_test.go index 5721ad3..dbaac4d 100644 --- a/internal/structured/structured_test.go +++ b/internal/structured/structured_test.go @@ -38,6 +38,16 @@ func TestTemplateRenderText(t *testing.T) { require.Equal(t, "Name: sandbox", out.String()) } +func TestRenderNilSpecUsesJSON(t *testing.T) { + var out bytes.Buffer + cmd := testCmd("pretty", &out) + + err := Render(cmd, map[string]any{"name": "sandbox"}, nil) + + require.NoError(t, err) + require.JSONEq(t, `{"name":"sandbox"}`, out.String()) +} + func TestRenderJQFiltersModel(t *testing.T) { var out bytes.Buffer cmd := testCmd("pretty", &out) From c3889f47ddb9beed926914f52dcc93f9940bbe51 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Tue, 5 May 2026 13:00:50 -0400 Subject: [PATCH 5/5] cli: simplify structured api rendering --- internal/cmd/api/api.go | 2 +- internal/cmd/api/api_test.go | 2 +- internal/cmd/api/request.go | 17 ++++++--------- internal/cmd/api/request_test.go | 18 ++++++++++------ internal/cmd/api/resolve.go | 2 +- internal/cmd/api/resolve_test.go | 4 ++-- internal/structured/structured.go | 30 +++++++++++++------------- internal/structured/structured_test.go | 21 ++++++++++++++++++ 8 files changed, 59 insertions(+), 37 deletions(-) diff --git a/internal/cmd/api/api.go b/internal/cmd/api/api.go index 594f8cf..fa87807 100644 --- a/internal/cmd/api/api.go +++ b/internal/cmd/api/api.go @@ -36,7 +36,7 @@ Make requests: cmd.AddCommand(newLsCmd()) cmd.AddCommand(newInfoCmd()) - for _, method := range []string{"GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"} { + for _, method := range []string{"GET", "POST", "PUT", "PATCH", "DELETE"} { cmd.AddCommand(requestCommand(method).Cobra()) } diff --git a/internal/cmd/api/api_test.go b/internal/cmd/api/api_test.go index 50d1e84..4125c2d 100644 --- a/internal/cmd/api/api_test.go +++ b/internal/cmd/api/api_test.go @@ -118,7 +118,7 @@ func TestNewCmd_GETRequestJQNonJSON(t *testing.T) { err := root.Execute() require.Error(t, err) - require.Contains(t, err.Error(), "response body is not JSON") + require.Contains(t, err.Error(), "JSON model is not available") } func TestNewCmd_POSTWithBody(t *testing.T) { diff --git a/internal/cmd/api/request.go b/internal/cmd/api/request.go index 1fbe48a..7485c2b 100644 --- a/internal/cmd/api/request.go +++ b/internal/cmd/api/request.go @@ -27,13 +27,6 @@ type apiResponse struct { IsJSON bool } -func (r apiResponse) CanRenderJQ() error { - if !r.IsJSON { - return fmt.Errorf("response body is not JSON") - } - return nil -} - func requestCommand(method string) structured.Command[*requestInput] { return structured.Command[*requestInput]{ Use: method + " PATH", @@ -58,10 +51,14 @@ func requestCommand(method string) structured.Command[*requestInput] { if resp.StatusCode >= 400 { afterRender = fmt.Errorf("HTTP %d", resp.StatusCode) } + model := any(nil) + if resp.IsJSON { + model = resp.Body + } return structured.Result{ - Model: resp.Body, - TextModel: resp, - ErrAfterRender: afterRender, + Model: model, + UnstructuredModel: resp, + ErrAfterRender: afterRender, }, nil }, } diff --git a/internal/cmd/api/request_test.go b/internal/cmd/api/request_test.go index 9ae6cf3..c76cfe2 100644 --- a/internal/cmd/api/request_test.go +++ b/internal/cmd/api/request_test.go @@ -26,7 +26,11 @@ func renderTestResponse(t *testing.T, resp apiResponse, args ...string) (string, require.NoError(t, cmd.ParseFlags(args)) var out bytes.Buffer cmd.SetOut(&out) - err := structured.Render(cmd, structured.Result{Model: resp.Body, TextModel: resp}, nil) + var model any + if resp.IsJSON { + model = resp.Body + } + err := structured.Render(cmd, structured.Result{Model: model, UnstructuredModel: resp}, nil) return out.String(), err } @@ -117,7 +121,7 @@ func TestRunRequest_FormatJSONBodyOnly(t *testing.T) { require.JSONEq(t, `{"ok":true}`, out) } -func TestRunRequest_NilRenderOutputsJSON(t *testing.T) { +func TestRunRequest_NonJSONHasNoJSONModel(t *testing.T) { resp := apiResponse{ StatusCode: 200, Body: "plain text", @@ -125,8 +129,8 @@ func TestRunRequest_NilRenderOutputsJSON(t *testing.T) { out, err := renderTestResponse(t, resp) - require.NoError(t, err) - require.JSONEq(t, `"plain text"`, out) + require.EqualError(t, err, "JSON model is not available") + require.Empty(t, out) } func TestRunRequest_JQScalar(t *testing.T) { @@ -155,9 +159,9 @@ func TestRunRequest_ReturnsErrorAfterRender(t *testing.T) { cmd.SetOut(&out) err := structured.Render(cmd, structured.Result{ - Model: resp.Body, - TextModel: resp, - ErrAfterRender: fmt.Errorf("HTTP 404"), + Model: resp.Body, + UnstructuredModel: resp, + ErrAfterRender: fmt.Errorf("HTTP 404"), }, nil) require.EqualError(t, err, "HTTP 404") diff --git a/internal/cmd/api/resolve.go b/internal/cmd/api/resolve.go index 91203c6..1ea06ae 100644 --- a/internal/cmd/api/resolve.go +++ b/internal/cmd/api/resolve.go @@ -22,7 +22,7 @@ func resolveEndpoint(baseURL, path string) string { // isHTTPMethod returns true if s is an uppercase HTTP method name. func isHTTPMethod(s string) bool { switch s { - case "GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS": + case "GET", "POST", "PUT", "PATCH", "DELETE": return true } return false diff --git a/internal/cmd/api/resolve_test.go b/internal/cmd/api/resolve_test.go index 172845d..74dc414 100644 --- a/internal/cmd/api/resolve_test.go +++ b/internal/cmd/api/resolve_test.go @@ -31,12 +31,12 @@ func TestResolveEndpoint(t *testing.T) { } func TestIsHTTPMethod(t *testing.T) { - for _, m := range []string{"GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"} { + for _, m := range []string{"GET", "POST", "PUT", "PATCH", "DELETE"} { if !isHTTPMethod(m) { t.Errorf("expected %q to be recognized as HTTP method", m) } } - for _, m := range []string{"get", "ls", "info", "FOO", ""} { + for _, m := range []string{"get", "ls", "info", "FOO", "HEAD", "OPTIONS", ""} { if isHTTPMethod(m) { t.Errorf("expected %q to NOT be recognized as HTTP method", m) } diff --git a/internal/structured/structured.go b/internal/structured/structured.go index fa6ef58..20c97ed 100644 --- a/internal/structured/structured.go +++ b/internal/structured/structured.go @@ -36,9 +36,9 @@ type Command[I any] struct { } type Result struct { - Model any - TextModel any - ErrAfterRender error + Model any + UnstructuredModel any + ErrAfterRender error } func (c Command[I]) Cobra() *cobra.Command { @@ -84,31 +84,34 @@ func (p Parent) Cobra() *cobra.Command { func Render(cmd *cobra.Command, model any, spec Spec) error { errAfterRender := error(nil) - textModel := model + unstructuredModel := model + jsonUnavailable := false if result, ok := model.(Result); ok { model = result.Model - textModel = result.TextModel - if textModel == nil { - textModel = model + unstructuredModel = result.UnstructuredModel + if unstructuredModel == nil { + unstructuredModel = model } + jsonUnavailable = model == nil && unstructuredModel != nil errAfterRender = result.ErrAfterRender } w := cmd.OutOrStdout() var err error if expr := resolveJQ(cmd); expr != "" { - if typed, ok := textModel.(interface{ CanRenderJQ() error }); ok { - if err := typed.CanRenderJQ(); err != nil { - return err - } + if jsonUnavailable { + return fmt.Errorf("JSON model is not available") } err = renderJQ(w, model, expr) } else if cmdutil.ResolveFormat(cmd) != "pretty" || spec == nil { + if jsonUnavailable { + return fmt.Errorf("JSON model is not available") + } enc := json.NewEncoder(w) enc.SetIndent("", " ") err = enc.Encode(model) } else { - err = spec.RenderText(w, textModel) + err = spec.RenderText(w, unstructuredModel) } if err != nil { return err @@ -120,9 +123,6 @@ func resolveJQ(cmd *cobra.Command) string { if f := cmd.Flags().Lookup("jq"); f != nil { return f.Value.String() } - if f := cmd.PersistentFlags().Lookup("jq"); f != nil { - return f.Value.String() - } return "" } diff --git a/internal/structured/structured_test.go b/internal/structured/structured_test.go index dbaac4d..2cdbc30 100644 --- a/internal/structured/structured_test.go +++ b/internal/structured/structured_test.go @@ -48,6 +48,27 @@ func TestRenderNilSpecUsesJSON(t *testing.T) { require.JSONEq(t, `{"name":"sandbox"}`, out.String()) } +func TestRenderResultWithoutModelCannotRenderJSON(t *testing.T) { + var out bytes.Buffer + cmd := testCmd("pretty", &out) + + err := Render(cmd, Result{UnstructuredModel: map[string]any{"name": "sandbox"}}, nil) + + require.EqualError(t, err, "JSON model is not available") + require.Empty(t, out.String()) +} + +func TestRenderResultWithoutModelCannotRenderJQ(t *testing.T) { + var out bytes.Buffer + cmd := testCmd("pretty", &out) + require.NoError(t, cmd.Flags().Set("jq", ".name")) + + err := Render(cmd, Result{UnstructuredModel: map[string]any{"name": "sandbox"}}, nil) + + require.EqualError(t, err, "JSON model is not available") + require.Empty(t, out.String()) +} + func TestRenderJQFiltersModel(t *testing.T) { var out bytes.Buffer cmd := testCmd("pretty", &out)