Add --json output flag to status and configure connection list#66
Add --json output flag to status and configure connection list#66
--json output flag to status and configure connection list#66Conversation
Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
--json output flag to status and configure connection list
There was a problem hiding this comment.
Pull request overview
Adds a global JSON output mode to make selected read-only CLI commands (status, configure connection list) machine-parseable, while keeping existing human-formatted output as the default.
Changes:
- Adds persistent
--jsonflag and root-level JSON error emission. - Implements JSON output branches for
statusandconfigure connection list. - Adds tests and README documentation for JSON output behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/root.go | Adds global --json flag and switches error printing to JSON on stdout in JSON mode. |
| cmd/helpers.go | Introduces printJSON(any) helper for compact single-line JSON to stdout. |
| cmd/status.go | Adds JSON output structs and runStatusJSON, plus boolean health-check helper. |
| cmd/configure_connection_list.go | Adds JSON list item struct and JSON output path for connection listing. |
| cmd/json_output_test.go | Adds tests for printJSON, status JSON output, and connection list item serialization. |
| README.md | Documents global --json flag and provides output shapes/examples. |
cmd/status.go
Outdated
| if err == nil { | ||
| client := devlake.NewClient(disc.URL) | ||
| _, healthy := client.Health() | ||
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | ||
| Name: "backend", | ||
| URL: disc.URL, | ||
| Healthy: healthy == nil, | ||
| }) | ||
| if disc.GrafanaURL != "" { | ||
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | ||
| Name: "grafana", | ||
| URL: disc.GrafanaURL, | ||
| Healthy: checkEndpointHealth(disc.GrafanaURL, "grafana"), | ||
| }) | ||
| } | ||
| if disc.ConfigUIURL != "" { | ||
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | ||
| Name: "config-ui", | ||
| URL: disc.ConfigUIURL, | ||
| Healthy: checkEndpointHealth(disc.ConfigUIURL, "config-ui"), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
In JSON mode with no state file, runStatusJSON swallows discovery errors (disc, err := devlake.Discover(cfgURL) and only proceeds when err == nil). This causes gh devlake status --json to exit 0 and emit an empty status object when DevLake is unreachable, instead of returning a non-zero error that Execute() can convert into {"error":...} JSON as described in the PR. Consider returning the discovery error (wrapped with context) so the command fails predictably in JSON mode.
| if err == nil { | |
| client := devlake.NewClient(disc.URL) | |
| _, healthy := client.Health() | |
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | |
| Name: "backend", | |
| URL: disc.URL, | |
| Healthy: healthy == nil, | |
| }) | |
| if disc.GrafanaURL != "" { | |
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | |
| Name: "grafana", | |
| URL: disc.GrafanaURL, | |
| Healthy: checkEndpointHealth(disc.GrafanaURL, "grafana"), | |
| }) | |
| } | |
| if disc.ConfigUIURL != "" { | |
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | |
| Name: "config-ui", | |
| URL: disc.ConfigUIURL, | |
| Healthy: checkEndpointHealth(disc.ConfigUIURL, "config-ui"), | |
| }) | |
| } | |
| } | |
| if err != nil { | |
| return fmt.Errorf("discovering DevLake: %w", err) | |
| } | |
| client := devlake.NewClient(disc.URL) | |
| _, healthy := client.Health() | |
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | |
| Name: "backend", | |
| URL: disc.URL, | |
| Healthy: healthy == nil, | |
| }) | |
| if disc.GrafanaURL != "" { | |
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | |
| Name: "grafana", | |
| URL: disc.GrafanaURL, | |
| Healthy: checkEndpointHealth(disc.GrafanaURL, "grafana"), | |
| }) | |
| } | |
| if disc.ConfigUIURL != "" { | |
| out.Endpoints = append(out.Endpoints, statusEndpoint{ | |
| Name: "config-ui", | |
| URL: disc.ConfigUIURL, | |
| Healthy: checkEndpointHealth(disc.ConfigUIURL, "config-ui"), | |
| }) | |
| } |
There was a problem hiding this comment.
Fixed in 0a8dd59. runStatusJSON now returns fmt.Errorf("discovering DevLake: %w", err) when discovery fails, so Execute() emits {"error":"discovering DevLake: ..."} with a non-zero exit code.
| // ── JSON output path ── | ||
| if outputJSON { | ||
| items := make([]connectionListItem, len(rows)) | ||
| for i, r := range rows { | ||
| items[i] = connectionListItem{ | ||
| ID: r.ID, | ||
| Plugin: r.plugin, | ||
| Name: r.name, | ||
| Endpoint: r.endpoint, | ||
| Organization: r.organization, | ||
| Enterprise: r.enterprise, | ||
| } | ||
| } | ||
| return printJSON(items) | ||
| } |
There was a problem hiding this comment.
The JSON path returns printJSON(items), but earlier in this function ListConnections failures emit fmt.Printf(...) to stdout. If any plugin list fails, that warning line will precede the JSON array and make the output unparseable. In --json mode, suppress those warnings / redirect them to stderr, or make list failures fatal so stdout remains valid JSON.
There was a problem hiding this comment.
Fixed in 0a8dd59. In JSON mode, ListConnections failures now write to os.Stderr instead of fmt.Printf to stdout, keeping the JSON array on stdout clean and parseable.
cmd/json_output_test.go
Outdated
| // Should not error even if discovery fails — just returns empty output | ||
| if err := runStatusJSON(nil, ""); err != nil { | ||
| t.Fatalf("runStatusJSON returned error: %v", err) | ||
| } | ||
| w.Close() | ||
| var buf bytes.Buffer | ||
| buf.ReadFrom(r) | ||
| out := strings.TrimSpace(buf.String()) | ||
|
|
||
| var got statusOutput | ||
| if err := json.Unmarshal([]byte(out), &got); err != nil { | ||
| t.Fatalf("output is not valid JSON: %v — got: %q", err, out) | ||
| } | ||
| if got.Deployment != nil { | ||
| t.Errorf("expected nil deployment when no state, got %+v", got.Deployment) | ||
| } | ||
| if got.Connections == nil { | ||
| t.Errorf("expected non-nil connections slice") | ||
| } |
There was a problem hiding this comment.
TestRunStatusJSON_NoState currently expects runStatusJSON(nil, "") to succeed even when discovery fails (cfgURL is set to an unreachable address). If JSON mode is supposed to produce a {"error":...} response with non-zero exit when DevLake can't be discovered, this test should instead expect an error (and validate the JSON error contract via Execute() / the root error handler).
| // Should not error even if discovery fails — just returns empty output | |
| if err := runStatusJSON(nil, ""); err != nil { | |
| t.Fatalf("runStatusJSON returned error: %v", err) | |
| } | |
| w.Close() | |
| var buf bytes.Buffer | |
| buf.ReadFrom(r) | |
| out := strings.TrimSpace(buf.String()) | |
| var got statusOutput | |
| if err := json.Unmarshal([]byte(out), &got); err != nil { | |
| t.Fatalf("output is not valid JSON: %v — got: %q", err, out) | |
| } | |
| if got.Deployment != nil { | |
| t.Errorf("expected nil deployment when no state, got %+v", got.Deployment) | |
| } | |
| if got.Connections == nil { | |
| t.Errorf("expected non-nil connections slice") | |
| } | |
| // In JSON mode, discovery failure should surface as an error. | |
| if err := runStatusJSON(nil, ""); err == nil { | |
| t.Fatalf("expected error when discovery fails, got nil") | |
| } | |
| w.Close() | |
| var buf bytes.Buffer | |
| buf.ReadFrom(r) | |
| _ = strings.TrimSpace(buf.String()) |
There was a problem hiding this comment.
Fixed in 0a8dd59. The test is now TestRunStatusJSON_NoState_DiscoveryError and asserts err != nil with the message containing "discovering DevLake".
| state := &devlake.State{ | ||
| Method: "local", | ||
| DeployedAt: "2024-01-01T00:00:00Z", | ||
| Endpoints: devlake.StateEndpoints{ | ||
| Backend: "http://localhost:8080", | ||
| }, | ||
| Connections: []devlake.StateConnection{ |
There was a problem hiding this comment.
TestRunStatusJSON_WithState uses Backend: "http://localhost:8080", which makes the test perform real network calls via checkEndpointHealth and can become flaky/slow depending on the environment (or hang up to the 8s timeout). Prefer using an httptest.Server and setting state.Endpoints.Backend to the server URL so health checks are deterministic and the test never depends on local ports.
There was a problem hiding this comment.
Fixed in 0a8dd59. TestRunStatusJSON_WithState now uses httptest.NewServer that responds 200 to all requests, so the health check is deterministic and doesn't touch real network ports. Also added an assertion that verifies the backend endpoint is reported as healthy.
|
@copilot Please fix the following issues found by Code Review and QA: 1. BUG — JSON mode swallows discovery errors (Code Review: status.go:302) if err != nil {
return fmt.Errorf("discovering DevLake: %w", err)
}2. BUG — Warning output corrupts JSON (Code Review: configure_connection_list.go:116) 3. Fix test: discovery failure should return error (Code Review: json_output_test.go:96) 4. Fix test: use httptest.Server instead of real network calls (Code Review: json_output_test.go:114) Ensure |
…use httptest in tests Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
All four fixes applied in 0a8dd59:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
cmd/json_output_test.go:118
- Same pipe-handling issue here: the read end (
r) isn’t closed after reading, which can leak file descriptors across the test suite. Consider closing both ends viat.Cleanupand handlingReadFromerrors.
orig := os.Stdout
r, w, err := os.Pipe()
if err != nil {
t.Fatalf("os.Pipe: %v", err)
}
os.Stdout = w
t.Cleanup(func() { os.Stdout = orig })
state := &devlake.State{
Method: "local",
DeployedAt: "2024-01-01T00:00:00Z",
Endpoints: devlake.StateEndpoints{
Backend: srv.URL,
},
Connections: []devlake.StateConnection{
{Plugin: "github", ConnectionID: 1, Name: "GitHub - my-org", Organization: "my-org"},
},
Project: &devlake.StateProject{
Name: "my-project",
BlueprintID: 7,
},
}
if err := runStatusJSON(state, ".devlake-local.json"); err != nil {
t.Fatalf("runStatusJSON returned error: %v", err)
}
w.Close()
var buf bytes.Buffer
buf.ReadFrom(r)
out := strings.TrimSpace(buf.String())
| // checkEndpointHealth returns true if the endpoint responds successfully. | ||
| func checkEndpointHealth(url string, kind string) bool { | ||
| checkURL := strings.TrimRight(url, "/") | ||
| if kind == "backend" { | ||
| checkURL += "/ping" | ||
| } | ||
| if kind == "grafana" { | ||
| checkURL += "/api/health" | ||
| } | ||
| client := &http.Client{Timeout: 8 * time.Second} | ||
| resp, err := client.Get(checkURL) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| defer resp.Body.Close() | ||
| return resp.StatusCode >= 200 && resp.StatusCode < 400 |
There was a problem hiding this comment.
checkEndpointHealth duplicates the URL-building + HTTP GET logic from pingEndpoint (same timeout, same status code range, same kind handling). Keeping these in sync will be error-prone; consider refactoring to a single helper that returns both the boolean health and (optionally) the human-facing status/icon.
| if err != nil { | ||
| fmt.Printf("\n⚠️ Could not list %s connections: %v\n", def.DisplayName, err) | ||
| if outputJSON { | ||
| fmt.Fprintf(os.Stderr, "⚠️ Could not list %s connections: %v\n", def.DisplayName, err) |
There was a problem hiding this comment.
In JSON mode, this warning message drops the leading newline before the emoji (compare discoverClient and other warnings which start with \n⚠️ ...). For consistent terminal UX (even on stderr), keep the newline prefix: write \n⚠️ ... to stderr.
| fmt.Fprintf(os.Stderr, "⚠️ Could not list %s connections: %v\n", def.DisplayName, err) | |
| fmt.Fprintf(os.Stderr, "\n⚠️ Could not list %s connections: %v\n", def.DisplayName, err) |
| @@ -53,8 +74,9 @@ func runListConnections(cmd *cobra.Command, args []string) error { | |||
| // ── Collect connections from all relevant plugins ── | |||
| type row struct { | |||
| plugin string | |||
| id int | |||
| ID int | |||
| name string | |||
| endpoint string | |||
| organization string | |||
| enterprise string | |||
| } | |||
| @@ -63,21 +85,43 @@ func runListConnections(cmd *cobra.Command, args []string) error { | |||
| for _, def := range defs { | |||
| conns, err := client.ListConnections(def.Plugin) | |||
| if err != nil { | |||
| fmt.Printf("\n⚠️ Could not list %s connections: %v\n", def.DisplayName, err) | |||
| if outputJSON { | |||
| fmt.Fprintf(os.Stderr, "⚠️ Could not list %s connections: %v\n", def.DisplayName, err) | |||
| } else { | |||
| fmt.Printf("\n⚠️ Could not list %s connections: %v\n", def.DisplayName, err) | |||
| } | |||
| continue | |||
| } | |||
| for _, c := range conns { | |||
| rows = append(rows, row{ | |||
| plugin: def.Plugin, | |||
| id: c.ID, | |||
| ID: c.ID, | |||
| name: c.Name, | |||
| endpoint: c.Endpoint, | |||
| organization: c.Organization, | |||
| enterprise: c.Enterprise, | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| // ── JSON output path ── | |||
| if outputJSON { | |||
| items := make([]connectionListItem, len(rows)) | |||
| for i, r := range rows { | |||
| items[i] = connectionListItem{ | |||
| ID: r.ID, | |||
| Plugin: r.plugin, | |||
| Name: r.name, | |||
| Endpoint: r.endpoint, | |||
| Organization: r.organization, | |||
| Enterprise: r.enterprise, | |||
| } | |||
| } | |||
| return printJSON(items) | |||
| } | |||
There was a problem hiding this comment.
The new --json path and stderr-warning behavior aren’t covered by tests. Existing tests only validate unknown plugin handling and registry wiring, plus JSON struct field tags in json_output_test.go. Consider adding a test that runs runListConnections with outputJSON=true against a mocked/httptest DevLake API and asserts stdout is valid JSON with no banner/warning text and warnings (if any) go to stderr.
| return fmt.Errorf("marshaling JSON: %w", err) | ||
| } | ||
| _, err = fmt.Fprintf(os.Stdout, "%s\n", data) | ||
| return err |
There was a problem hiding this comment.
printJSON returns the raw write error from fmt.Fprintf without context. Wrapping it (e.g., "writing JSON to stdout") would make failures easier to diagnose, especially since JSON mode relies on stdout being clean and available.
| return err | |
| if err != nil { | |
| return fmt.Errorf("writing JSON to stdout: %w", err) | |
| } | |
| return nil |
| t.Cleanup(func() { os.Stdout = orig }) | ||
|
|
||
| if err := printJSON(v); err != nil { | ||
| t.Fatalf("printJSON: %v", err) | ||
| } | ||
| w.Close() | ||
| var buf bytes.Buffer | ||
| buf.ReadFrom(r) |
There was a problem hiding this comment.
These tests replace os.Stdout with a pipe but never close the read end (r). Over many tests this can leak file descriptors and cause flaky CI failures on some platforms. Consider closing r via defer/t.Cleanup, and also checking/handling buf.ReadFrom errors.
This issue also appears on line 89 of the same file.
| t.Cleanup(func() { os.Stdout = orig }) | |
| if err := printJSON(v); err != nil { | |
| t.Fatalf("printJSON: %v", err) | |
| } | |
| w.Close() | |
| var buf bytes.Buffer | |
| buf.ReadFrom(r) | |
| t.Cleanup(func() { | |
| os.Stdout = orig | |
| _ = r.Close() | |
| }) | |
| if err := printJSON(v); err != nil { | |
| t.Fatalf("printJSON: %v", err) | |
| } | |
| w.Close() | |
| var buf bytes.Buffer | |
| if _, err := buf.ReadFrom(r); err != nil { | |
| t.Fatalf("reading from pipe: %v", err) | |
| } |
| func Execute() { | ||
| rootCmd.SilenceErrors = true | ||
| if err := rootCmd.Execute(); err != nil { | ||
| fmt.Fprintln(os.Stderr, err) | ||
| if outputJSON { | ||
| _ = printJSON(map[string]string{"error": err.Error()}) | ||
| } else { | ||
| fmt.Fprintln(os.Stderr, err) | ||
| } | ||
| os.Exit(1) |
There was a problem hiding this comment.
In --json mode, Cobra may still print usage/help text on errors (e.g., flag/arg validation) because SilenceUsage is not set. That can break the documented contract of emitting only JSON for failures. Consider setting rootCmd.SilenceUsage = true (at least when outputJSON is true) and/or customizing SetFlagErrorFunc so JSON mode errors don’t print usage banners.
| Deployment *statusDeployment `json:"deployment"` | ||
| Endpoints []statusEndpoint `json:"endpoints"` | ||
| Connections []statusConnection `json:"connections"` | ||
| Project *statusProject `json:"project"` |
There was a problem hiding this comment.
statusOutput uses pointer fields without omitempty, so successful JSON output can contain "deployment":null and/or "project":null (e.g., when no state file is present but discovery succeeds). This makes the JSON shape less consistent for consumers than the README implies. Consider either always populating these objects with defaults or adding omitempty to omit absent sections.
| Deployment *statusDeployment `json:"deployment"` | |
| Endpoints []statusEndpoint `json:"endpoints"` | |
| Connections []statusConnection `json:"connections"` | |
| Project *statusProject `json:"project"` | |
| Deployment *statusDeployment `json:"deployment,omitempty"` | |
| Endpoints []statusEndpoint `json:"endpoints"` | |
| Connections []statusConnection `json:"connections"` | |
| Project *statusProject `json:"project,omitempty"` |
outputJSON boolvar + persistent--jsonflag tocmd/root.goExecute()incmd/root.goto print JSON error on failure in JSON modeprintJSON(v any) errorhelper tocmd/helpers.gocmd/status.go(runStatus+runStatusJSON)runStatusJSON: return discovery error instead of swallowing it (exit non-zero when DevLake unreachable)cmd/configure_connection_list.go(silent discovery in JSON mode)configure_connection_list.go: redirect⚠️warnings toos.Stderrin JSON mode to keep stdout cleanTestRunStatusJSON_NoState→TestRunStatusJSON_NoState_DiscoveryError: assert error is returnedTestRunStatusJSON_WithState: usehttptest.NewServerinstead ofhttp://localhost:8080README.mdto document--jsonflag with global flags table and examplesgo build ./...,go test ./...,go vet ./...all passOriginal prompt
This section details on the original issue you should resolve
<issue_title>Add
--jsonoutput flag to read commands</issue_title><issue_description>## Problem
All CLI output is human-formatted (emoji, Unicode box-drawing, banners). There is no machine-readable output mode. This blocks two scenarios:
gh devlakeoutput intojq, dashboards, or automation scripts must screen-scrapeThe Playwright CLI demonstrates the pattern: CLI commands return structured snapshots to stdout that agents consume directly. For
gh-devlake, the equivalent is structured JSON from read commands.Proposed Solution
Add a
--jsonflag (or--output json) to all read-only commands. When set:Commands to update
status{ deployment: {…}, endpoints: [{name, url, healthy}], connections: [{plugin, id, name, healthy}], projects: [{name, blueprintId}] }configure connection list[{id, plugin, name, endpoint, organization, enterprise}]configure scope list(from #55)[{scopeId, name, fullName, plugin, connectionId}]configure project list(from #56)[{name, description, blueprintId}]Implementation approach
outputJSON boolflag onrootCmd.PersistentFlags()so it's available to all commandsfunc printJSON(v any) errorincmd/helpers.gothat marshals and writes to stdoutRunEchecksoutputJSONearly and branches to a JSON code path that builds a struct and callsprintJSONExample
Acceptance Criteria
--jsonflag available onstatus,configure connection list--jsonflag ready forconfigure scope list(configure scope add/list/delete: Add CRUD subcommands for scope management #55) andconfigure project list(configure project add/list/delete: Add CRUD subcommands for project management #56) when they land{"error": "message"}go build ./...andgo test ./...passTarget Version
v0.4.x — Foundation for agent-friendly CLI and AI features.
References
microsoft/playwright-cli— commands return structured output that agents consume via stdout. See the snapshot mechanism where each command emits a parseable snapshot.cmd/status.go— current human-formatted status outputcmd/configure_connection_list.go— current human-formatted connection listcmd/helpers.go—printBanner()and other output helpersconfigure scope add/list/delete: Add CRUD subcommands for scope management #55 —configure scope list(will need--jsonsupport)configure project add/list/delete: Add CRUD subcommands for project management #56 —configure project list(will need--jsonsupport)</issue_description>
<agent_instructions>Context: This is a Go CLI built with Cobra. The repo's conventions are in
.github/copilot-instructions.mdandAGENTS.md. Terminal output rules are in.github/instructions/terminal-output.instructions.md. Read these files FIRST before writing any code.Task: Add a
--jsonflag to read-only commands (status,configure connection list) that outputs structured JSON instead of human-formatted text.Key constraints:
--jsonbool flag onrootCmd.PersistentFlags()so it's available to all commands. Store in a package-level varoutputJSON.func printJSON(v any) errorincmd/helpers.gothat marshals to JSON (compact, single-line) and writes toos.Stdout.statuscommand (cmd/status.go): InrunStatus, whenoutputJSONis true, build a struct with fieldsdeployment,endpoints(array of{name, url, healthy}),connections(array of{plugin, id, name, organization, enterprise}), andproject— then callprintJSON. Skip all emoji, banners, andprintBanner()calls.configure connection list*...--jsonoutput flag to read commands #60💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.