From 03a2936ac69242fa0c1d6459dfa3fb0bf62ef0c0 Mon Sep 17 00:00:00 2001 From: Ramon Niebla Date: Tue, 17 Mar 2026 20:02:42 -0700 Subject: [PATCH 1/4] adding agent flag for agent telemtry --- cmd/analytics/analytics.go | 61 ++++++++++ cmd/analytics/analytics_test.go | 170 ++++++++++++++++++++++++++++ cmd/root.go | 5 +- internal/analytics/client.go | 32 +++--- internal/analytics/client_test.go | 180 ++++++++++++++++++++++++++++++ 5 files changed, 433 insertions(+), 15 deletions(-) create mode 100644 cmd/analytics/analytics_test.go create mode 100644 internal/analytics/client_test.go diff --git a/cmd/analytics/analytics.go b/cmd/analytics/analytics.go index 6c714195..2cce594a 100644 --- a/cmd/analytics/analytics.go +++ b/cmd/analytics/analytics.go @@ -1,13 +1,74 @@ package analytics import ( + "os" + "github.com/launchdarkly/ldcli/cmd/cliflags" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" + "golang.org/x/term" ) +type envChecker interface { + Getenv(key string) string + IsTerminal(fd int) bool + StdinFd() int + StdoutFd() int +} + +type osEnvChecker struct{} + +func (osEnvChecker) Getenv(key string) string { return os.Getenv(key) } +func (osEnvChecker) IsTerminal(fd int) bool { return term.IsTerminal(fd) } +func (osEnvChecker) StdinFd() int { return int(os.Stdin.Fd()) } +func (osEnvChecker) StdoutFd() int { return int(os.Stdout.Fd()) } + +type agentEnvVar struct { + envVar string + label string +} + +// Ordered list so detection priority is deterministic. +var knownAgentEnvVars = []agentEnvVar{ + {"CURSOR_SESSION_ID", "cursor"}, + {"CURSOR_TRACE_ID", "cursor"}, + {"CLAUDE_CODE", "claude-code"}, + {"CLAUDE_CODE_SESSION", "claude-code"}, + {"CODEX_SESSION", "codex"}, + {"CODEX_SANDBOX_ID", "codex"}, + {"DEVIN_SESSION", "devin"}, + {"GITHUB_COPILOT", "copilot"}, + {"WINDSURF_SESSION", "windsurf"}, + {"CLINE_TASK_ID", "cline"}, + {"AIDER_MODEL", "aider"}, +} + +// DetectAgentContext returns a label identifying the agent environment, or "" +// if the CLI appears to be running in an interactive human terminal. +func DetectAgentContext() string { + return detectAgentContext(osEnvChecker{}) +} + +func detectAgentContext(env envChecker) string { + if v := env.Getenv("LD_CLI_AGENT"); v != "" { + return "explicit:" + v + } + + for _, a := range knownAgentEnvVars { + if env.Getenv(a.envVar) != "" { + return a.label + } + } + + if !env.IsTerminal(env.StdinFd()) && !env.IsTerminal(env.StdoutFd()) { + return "no-tty" + } + + return "" +} + func CmdRunEventProperties( cmd *cobra.Command, name string, diff --git a/cmd/analytics/analytics_test.go b/cmd/analytics/analytics_test.go new file mode 100644 index 00000000..632a1d52 --- /dev/null +++ b/cmd/analytics/analytics_test.go @@ -0,0 +1,170 @@ +package analytics + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +type mockEnvChecker struct { + envVars map[string]string + stdinTerminal bool + stdoutTerminal bool +} + +func newMockEnv(envVars map[string]string, bothTerminal bool) mockEnvChecker { + return mockEnvChecker{envVars: envVars, stdinTerminal: bothTerminal, stdoutTerminal: bothTerminal} +} + +func (m mockEnvChecker) Getenv(key string) string { return m.envVars[key] } +func (m mockEnvChecker) IsTerminal(fd int) bool { + if fd == 0 { + return m.stdinTerminal + } + return m.stdoutTerminal +} +func (m mockEnvChecker) StdinFd() int { return 0 } +func (m mockEnvChecker) StdoutFd() int { return 1 } + +func TestDetectAgentContext(t *testing.T) { + tests := []struct { + name string + env mockEnvChecker + expected string + }{ + { + name: "explicit LD_CLI_AGENT", + env: newMockEnv(map[string]string{"LD_CLI_AGENT": "my-skill"}, false), + expected: "explicit:my-skill", + }, + { + name: "explicit LD_CLI_AGENT takes precedence over known env vars", + env: newMockEnv(map[string]string{"LD_CLI_AGENT": "custom", "CURSOR_SESSION_ID": "abc"}, false), + expected: "explicit:custom", + }, + { + name: "CURSOR_SESSION_ID detected", + env: newMockEnv(map[string]string{"CURSOR_SESSION_ID": "abc"}, false), + expected: "cursor", + }, + { + name: "CURSOR_TRACE_ID detected", + env: newMockEnv(map[string]string{"CURSOR_TRACE_ID": "xyz"}, false), + expected: "cursor", + }, + { + name: "CLAUDE_CODE detected", + env: newMockEnv(map[string]string{"CLAUDE_CODE": "1"}, false), + expected: "claude-code", + }, + { + name: "CLAUDE_CODE_SESSION detected", + env: newMockEnv(map[string]string{"CLAUDE_CODE_SESSION": "sess"}, false), + expected: "claude-code", + }, + { + name: "CODEX_SESSION detected", + env: newMockEnv(map[string]string{"CODEX_SESSION": "s"}, false), + expected: "codex", + }, + { + name: "CODEX_SANDBOX_ID detected", + env: newMockEnv(map[string]string{"CODEX_SANDBOX_ID": "sb"}, false), + expected: "codex", + }, + { + name: "DEVIN_SESSION detected", + env: newMockEnv(map[string]string{"DEVIN_SESSION": "d"}, false), + expected: "devin", + }, + { + name: "GITHUB_COPILOT detected", + env: newMockEnv(map[string]string{"GITHUB_COPILOT": "1"}, false), + expected: "copilot", + }, + { + name: "WINDSURF_SESSION detected", + env: newMockEnv(map[string]string{"WINDSURF_SESSION": "w"}, false), + expected: "windsurf", + }, + { + name: "CLINE_TASK_ID detected", + env: newMockEnv(map[string]string{"CLINE_TASK_ID": "t"}, false), + expected: "cline", + }, + { + name: "AIDER_MODEL detected", + env: newMockEnv(map[string]string{"AIDER_MODEL": "gpt-4"}, false), + expected: "aider", + }, + { + name: "no TTY and no env vars returns no-tty", + env: newMockEnv(map[string]string{}, false), + expected: "no-tty", + }, + { + name: "interactive terminal with no agent env vars returns empty", + env: newMockEnv(map[string]string{}, true), + expected: "", + }, + { + name: "first matching env var wins by priority order", + env: newMockEnv(map[string]string{"CURSOR_SESSION_ID": "c", "CLAUDE_CODE": "1"}, false), + expected: "cursor", + }, + { + name: "stdin is TTY but stdout is not still counts as interactive", + env: mockEnvChecker{envVars: map[string]string{}, stdinTerminal: true, stdoutTerminal: false}, + expected: "", + }, + { + name: "stdout is TTY but stdin is not still counts as interactive", + env: mockEnvChecker{envVars: map[string]string{}, stdinTerminal: false, stdoutTerminal: true}, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := detectAgentContext(tt.env) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestCmdRunEventProperties(t *testing.T) { + t.Run("does not set agent_context itself", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.SetArgs([]string{}) + _ = cmd.Execute() + + props := CmdRunEventProperties(cmd, "test-resource", nil) + + _, hasAgentCtx := props["agent_context"] + assert.False(t, hasAgentCtx, "agent_context is injected by sendEvent, not CmdRunEventProperties") + }) + + t.Run("overrides can still set agent_context", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.SetArgs([]string{}) + _ = cmd.Execute() + + overrides := map[string]interface{}{"agent_context": "explicit:test-agent"} + props := CmdRunEventProperties(cmd, "test-resource", overrides) + + assert.Equal(t, "explicit:test-agent", props["agent_context"]) + }) + + t.Run("returns expected base properties", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.SetArgs([]string{}) + _ = cmd.Execute() + + props := CmdRunEventProperties(cmd, "flags", nil) + + assert.Equal(t, "flags", props["name"]) + assert.Contains(t, props, "action") + assert.Contains(t, props, "flags") + }) +} diff --git a/cmd/root.go b/cmd/root.go index 5d06d1ff..67c2ce3d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -242,8 +242,9 @@ func Execute(version string) { } configService := config.NewService(resources.NewClient(version)) trackerFn := analytics.ClientFn{ - ID: uuid.New().String(), - Version: version, + ID: uuid.New().String(), + Version: version, + AgentContext: cmdAnalytics.DetectAgentContext(), } rootCmd, err := NewRootCommand( configService, diff --git a/internal/analytics/client.go b/internal/analytics/client.go index 938882b1..2a8ca524 100644 --- a/internal/analytics/client.go +++ b/internal/analytics/client.go @@ -12,8 +12,9 @@ import ( ) type ClientFn struct { - ID string - Version string + ID string + Version string + AgentContext string } func (fn ClientFn) Tracker(accessToken string, baseURI string, optOut bool) Tracker { @@ -25,25 +26,30 @@ func (fn ClientFn) Tracker(accessToken string, baseURI string, optOut bool) Trac httpClient: &http.Client{ Timeout: time.Second * 3, }, - id: fn.ID, - version: fn.Version, - accessToken: accessToken, - baseURI: baseURI, + id: fn.ID, + version: fn.Version, + accessToken: accessToken, + baseURI: baseURI, + agentContext: fn.AgentContext, } } type Client struct { - accessToken string - baseURI string - httpClient *http.Client - id string - version string - wg sync.WaitGroup + accessToken string + agentContext string + baseURI string + httpClient *http.Client + id string + version string + wg sync.WaitGroup } -// SendEvent makes an async request to track the given event with properties. +// sendEvent makes an async request to track the given event with properties. func (c *Client) sendEvent(eventName string, properties map[string]interface{}) { properties["id"] = c.id + if c.agentContext != "" { + properties["agent_context"] = c.agentContext + } input := struct { Event string `json:"event"` Properties map[string]interface{} `json:"properties"` diff --git a/internal/analytics/client_test.go b/internal/analytics/client_test.go new file mode 100644 index 00000000..5bb182f5 --- /dev/null +++ b/internal/analytics/client_test.go @@ -0,0 +1,180 @@ +package analytics + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestClientFn_Tracker(t *testing.T) { + t.Run("returns NoopClient when opted out", func(t *testing.T) { + fn := ClientFn{ID: "test-id", Version: "1.0.0"} + tracker := fn.Tracker("token", "https://app.launchdarkly.com", true) + + _, ok := tracker.(*NoopClient) + assert.True(t, ok, "expected NoopClient when optOut is true") + }) + + t.Run("returns Client when not opted out", func(t *testing.T) { + fn := ClientFn{ID: "test-id", Version: "1.0.0"} + tracker := fn.Tracker("token", "https://app.launchdarkly.com", false) + + c, ok := tracker.(*Client) + require.True(t, ok, "expected *Client when optOut is false") + assert.Equal(t, "test-id", c.id) + assert.Equal(t, "1.0.0", c.version) + assert.Equal(t, "token", c.accessToken) + assert.Equal(t, "https://app.launchdarkly.com", c.baseURI) + }) + + t.Run("passes AgentContext to Client", func(t *testing.T) { + fn := ClientFn{ID: "test-id", Version: "1.0.0", AgentContext: "cursor"} + tracker := fn.Tracker("token", "https://app.launchdarkly.com", false) + + c, ok := tracker.(*Client) + require.True(t, ok) + assert.Equal(t, "cursor", c.agentContext) + }) + + t.Run("AgentContext ignored when opted out", func(t *testing.T) { + fn := ClientFn{ID: "test-id", Version: "1.0.0", AgentContext: "cursor"} + tracker := fn.Tracker("token", "https://app.launchdarkly.com", true) + + _, ok := tracker.(*NoopClient) + assert.True(t, ok, "expected NoopClient regardless of AgentContext when optOut is true") + }) +} + +type trackingPayload struct { + Event string `json:"event"` + Properties map[string]interface{} `json:"properties"` +} + +func TestClient_SendEvent(t *testing.T) { + t.Run("includes agent_context when set", func(t *testing.T) { + var received trackingPayload + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(body, &received) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + fn := ClientFn{ID: "test-id", Version: "1.0.0", AgentContext: "cursor"} + tracker := fn.Tracker("test-token", server.URL, false) + + tracker.SendCommandRunEvent(map[string]interface{}{ + "name": "flags", + "action": "list", + }) + tracker.Wait() + + assert.Equal(t, "CLI Command Run", received.Event) + assert.Equal(t, "cursor", received.Properties["agent_context"]) + assert.Equal(t, "test-id", received.Properties["id"]) + assert.Equal(t, "flags", received.Properties["name"]) + }) + + t.Run("omits agent_context when empty", func(t *testing.T) { + var received trackingPayload + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(body, &received) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + fn := ClientFn{ID: "test-id", Version: "1.0.0"} + tracker := fn.Tracker("test-token", server.URL, false) + + tracker.SendCommandRunEvent(map[string]interface{}{ + "name": "flags", + "action": "list", + }) + tracker.Wait() + + assert.Equal(t, "CLI Command Run", received.Event) + _, hasAgentCtx := received.Properties["agent_context"] + assert.False(t, hasAgentCtx, "agent_context should not be present when empty") + assert.Equal(t, "test-id", received.Properties["id"]) + }) + + t.Run("SendCommandCompletedEvent includes agent_context", func(t *testing.T) { + var received trackingPayload + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(body, &received) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + fn := ClientFn{ID: "test-id", Version: "1.0.0", AgentContext: "claude-code"} + tracker := fn.Tracker("test-token", server.URL, false) + + tracker.SendCommandCompletedEvent("success") + tracker.Wait() + + assert.Equal(t, "CLI Command Completed", received.Event) + assert.Equal(t, "claude-code", received.Properties["agent_context"]) + assert.Equal(t, "success", received.Properties["outcome"]) + }) + + t.Run("sends correct HTTP headers", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "test-token", r.Header.Get("Authorization")) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + assert.Equal(t, "launchdarkly-cli/1.0.0", r.Header.Get("User-Agent")) + assert.Equal(t, "POST", r.Method) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + fn := ClientFn{ID: "test-id", Version: "1.0.0"} + tracker := fn.Tracker("test-token", server.URL, false) + + tracker.SendCommandRunEvent(map[string]interface{}{"name": "test"}) + tracker.Wait() + }) + + t.Run("posts to /internal/tracking path", func(t *testing.T) { + var requestPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestPath = r.URL.Path + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + fn := ClientFn{ID: "test-id", Version: "1.0.0"} + tracker := fn.Tracker("test-token", server.URL, false) + + tracker.SendCommandRunEvent(map[string]interface{}{"name": "test"}) + tracker.Wait() + + assert.Equal(t, "/internal/tracking", requestPath) + }) + + t.Run("SendSetupStepStartedEvent sends correct event name", func(t *testing.T) { + var received trackingPayload + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(body, &received) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + fn := ClientFn{ID: "test-id", Version: "1.0.0", AgentContext: "codex"} + tracker := fn.Tracker("test-token", server.URL, false) + + tracker.SendSetupStepStartedEvent("connect") + tracker.Wait() + + assert.Equal(t, "CLI Setup Step Started", received.Event) + assert.Equal(t, "connect", received.Properties["step"]) + assert.Equal(t, "codex", received.Properties["agent_context"]) + }) +} From ae23ad3782dde09770ed044a63065d560fc6d282 Mon Sep 17 00:00:00 2001 From: Ramon Niebla Date: Tue, 17 Mar 2026 20:18:41 -0700 Subject: [PATCH 2/4] (feat) adding TTY detection for auto-JSON output --- README.md | 2 +- cmd/cliflags/flags.go | 2 +- cmd/cmdtest.go | 1 + cmd/config/testdata/help.golden | 4 +- cmd/root.go | 12 +- cmd/root_test.go | 165 +++++++++++++++++++++++- internal/output/output_test.go | 42 +++++- internal/output/resource_output_test.go | 44 ++++++- 8 files changed, 262 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index f83e81f8..eedd8133 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ Supported settings: * `base-uri` LaunchDarkly base URI (default "https://app.launchdarkly.com") - `environment`: Default environment key - `flag`: Default feature flag key -- `output`: Command response output format in either JSON or plain text +- `output`: Output format: json or plaintext (default: plaintext in a terminal, json otherwise) - `project`: Default project key Available `config` commands: diff --git a/cmd/cliflags/flags.go b/cmd/cliflags/flags.go index 822d93c5..592894d0 100644 --- a/cmd/cliflags/flags.go +++ b/cmd/cliflags/flags.go @@ -44,7 +44,7 @@ const ( EnvironmentFlagDescription = "Default environment key" FlagFlagDescription = "Default feature flag key" JSONFlagDescription = "Output JSON format (shorthand for --output json)" - OutputFlagDescription = "Command response output format in either JSON or plain text" + OutputFlagDescription = "Output format: json or plaintext (default: plaintext in a terminal, json otherwise)" PortFlagDescription = "Port for the dev server to run on" ProjectFlagDescription = "Default project key" SyncOnceFlagDescription = "Only sync new projects. Existing projects will neither be resynced nor have overrides specified by CLI flags applied." diff --git a/cmd/cmdtest.go b/cmd/cmdtest.go index 342bfd68..6e522644 100644 --- a/cmd/cmdtest.go +++ b/cmd/cmdtest.go @@ -31,6 +31,7 @@ func CallCmd( clients, "test", false, + func() bool { return true }, ) cmd := rootCmd.Cmd() require.NoError(t, err) diff --git a/cmd/config/testdata/help.golden b/cmd/config/testdata/help.golden index 71f3652f..a0aaeab2 100644 --- a/cmd/config/testdata/help.golden +++ b/cmd/config/testdata/help.golden @@ -9,7 +9,7 @@ Supported settings: - `dev-stream-uri`: Streaming service endpoint that the dev server uses to obtain authoritative flag data. This may be a LaunchDarkly or Relay Proxy endpoint - `environment`: Default environment key - `flag`: Default feature flag key -- `output`: Command response output format in either JSON or plain text +- `output`: Output format: json or plaintext (default: plaintext in a terminal, json otherwise) - `port`: Port for the dev server to run on - `project`: Default project key - `sync-once`: Only sync new projects. Existing projects will neither be resynced nor have overrides specified by CLI flags applied. @@ -28,4 +28,4 @@ Global Flags: --analytics-opt-out Opt out of analytics tracking --base-uri string LaunchDarkly base URI (default "https://app.launchdarkly.com") --json Output JSON format (shorthand for --output json) - -o, --output string Command response output format in either JSON or plain text (default "plaintext") + -o, --output string Output format: json or plaintext (default: plaintext in a terminal, json otherwise) (default "plaintext") diff --git a/cmd/root.go b/cmd/root.go index 67c2ce3d..792f36ca 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -13,6 +13,7 @@ import ( "github.com/google/uuid" "github.com/spf13/cobra" "github.com/spf13/viper" + "golang.org/x/term" cmdAnalytics "github.com/launchdarkly/ldcli/cmd/analytics" "github.com/launchdarkly/ldcli/cmd/cliflags" @@ -87,6 +88,7 @@ func NewRootCommand( clients APIClients, version string, useConfigFile bool, + isTerminal func() bool, ) (*RootCmd, error) { cmd := &cobra.Command{ Use: "ldcli", @@ -188,10 +190,17 @@ func NewRootCommand( return nil, err } + // When stdout is not a TTY (e.g. piped, CI, agent), default to JSON. + // FORCE_TTY: any non-empty value treats stdout as a terminal (like NO_COLOR convention). + defaultOutput := "plaintext" + if os.Getenv("FORCE_TTY") == "" && (isTerminal == nil || !isTerminal()) { + defaultOutput = "json" + } + cmd.PersistentFlags().StringP( cliflags.OutputFlag, "o", - "plaintext", + defaultOutput, cliflags.OutputFlagDescription, ) err = viper.BindPFlag(cliflags.OutputFlag, cmd.PersistentFlags().Lookup(cliflags.OutputFlag)) @@ -252,6 +261,7 @@ func Execute(version string) { clients, version, true, + func() bool { return term.IsTerminal(int(os.Stdout.Fd())) }, ) if err != nil { log.Fatal(err) diff --git a/cmd/root_test.go b/cmd/root_test.go index 59bd69da..88646f13 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,13 +1,18 @@ package cmd_test import ( + "bytes" + "io" + "os" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/launchdarkly/ldcli/cmd" + "github.com/launchdarkly/ldcli/cmd/cliflags" "github.com/launchdarkly/ldcli/internal/analytics" + "github.com/launchdarkly/ldcli/internal/config" "github.com/launchdarkly/ldcli/internal/resources" ) @@ -29,7 +34,7 @@ func TestCreate(t *testing.T) { }) } -func TestJSONFlag(t *testing.T) { +func TestOutputFlags(t *testing.T) { mockClient := &resources.MockClient{ Response: []byte(`{"key": "test-key", "name": "test-name"}`), } @@ -55,4 +60,162 @@ func TestJSONFlag(t *testing.T) { assert.Contains(t, string(output), `"key": "test-key"`) assert.NotContains(t, string(output), "Successfully updated") }) + + t.Run("--output json returns raw JSON output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + } + + output, err := cmd.CallCmd( + t, + cmd.APIClients{ResourcesClient: mockClient}, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), `"key": "test-key"`) + assert.NotContains(t, string(output), "Successfully updated") + }) + + t.Run("--output plaintext returns human-readable output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "plaintext", + } + + output, err := cmd.CallCmd( + t, + cmd.APIClients{ResourcesClient: mockClient}, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), "Successfully updated") + assert.Contains(t, string(output), "test-name (test-key)") + }) +} + +func newRootCmdWithTerminal(t *testing.T, isTerminal func() bool) *cmd.RootCmd { + t.Helper() + rootCmd, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{}, + "test", + false, + isTerminal, + ) + require.NoError(t, err) + return rootCmd +} + +func execNonTTYCmd(t *testing.T, mockClient *resources.MockClient, extraArgs ...string) []byte { + t.Helper() + rootCmd, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{ResourcesClient: mockClient}, + "test", + false, + func() bool { return false }, + ) + require.NoError(t, err) + + c := rootCmd.Cmd() + b := bytes.NewBufferString("") + c.SetOut(b) + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + } + args = append(args, extraArgs...) + c.SetArgs(args) + + err = c.Execute() + require.NoError(t, err) + + out, err := io.ReadAll(b) + require.NoError(t, err) + return out +} + +func TestTTYDefaultOutput(t *testing.T) { + mockClient := &resources.MockClient{ + Response: []byte(`{"key": "test-key", "name": "test-name"}`), + } + + t.Run("non-TTY defaults to json output", func(t *testing.T) { + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "json", f.DefValue) + }) + + t.Run("TTY defaults to plaintext output", func(t *testing.T) { + rootCmd := newRootCmdWithTerminal(t, func() bool { return true }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("nil isTerminal defaults to json output", func(t *testing.T) { + rootCmd := newRootCmdWithTerminal(t, nil) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "json", f.DefValue) + }) + + t.Run("explicit --output plaintext overrides non-TTY", func(t *testing.T) { + out := execNonTTYCmd(t, mockClient, "--output", "plaintext") + assert.Contains(t, string(out), "Successfully updated") + }) + + t.Run("non-TTY without explicit flag returns JSON", func(t *testing.T) { + out := execNonTTYCmd(t, mockClient) + assert.Contains(t, string(out), `"key"`) + assert.NotContains(t, string(out), "Successfully updated") + }) + + t.Run("--json overrides non-TTY default", func(t *testing.T) { + out := execNonTTYCmd(t, mockClient, "--json") + assert.Contains(t, string(out), `"key"`) + assert.NotContains(t, string(out), "Successfully updated") + }) + + t.Run("LD_OUTPUT=plaintext overrides non-TTY default", func(t *testing.T) { + os.Setenv("LD_OUTPUT", "plaintext") + defer os.Unsetenv("LD_OUTPUT") + + out := execNonTTYCmd(t, mockClient) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") + }) + + t.Run("FORCE_TTY overrides non-TTY detection", func(t *testing.T) { + os.Setenv("FORCE_TTY", "1") + defer os.Unsetenv("FORCE_TTY") + + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) } diff --git a/internal/output/output_test.go b/internal/output/output_test.go index cdb31746..df701f9d 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -1,13 +1,40 @@ package output_test import ( - "github.com/launchdarkly/ldcli/internal/output" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/launchdarkly/ldcli/internal/output" ) +func TestNewOutputKind(t *testing.T) { + t.Run("returns json for valid json input", func(t *testing.T) { + kind, err := output.NewOutputKind("json") + require.NoError(t, err) + assert.Equal(t, output.OutputKindJSON, kind) + }) + + t.Run("returns plaintext for valid plaintext input", func(t *testing.T) { + kind, err := output.NewOutputKind("plaintext") + require.NoError(t, err) + assert.Equal(t, output.OutputKindPlaintext, kind) + }) + + t.Run("returns error for invalid input", func(t *testing.T) { + kind, err := output.NewOutputKind("xml") + assert.ErrorIs(t, err, output.ErrInvalidOutputKind) + assert.Equal(t, output.OutputKindNull, kind) + }) + + t.Run("returns error for empty string", func(t *testing.T) { + kind, err := output.NewOutputKind("") + assert.ErrorIs(t, err, output.ErrInvalidOutputKind) + assert.Equal(t, output.OutputKindNull, kind) + }) +} + func TestCmdOutputSingular(t *testing.T) { tests := map[string]struct { expected string @@ -63,4 +90,17 @@ func TestCmdOutputSingular(t *testing.T) { assert.Equal(t, tt.expected, output) }) } + + t.Run("with json output kind returns raw JSON", func(t *testing.T) { + input := `{"key": "test-key", "name": "test-name"}` + + result, err := output.CmdOutputSingular( + "json", + []byte(input), + output.SingularPlaintextOutputFn, + ) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) } diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index dae78fa0..1d1ec434 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -310,16 +310,54 @@ func TestCmdOutputError(t *testing.T) { assert.JSONEq(t, expected, result) }) - t.Run("with plaintext output", func(t *testing.T) { - expected := "invalid JSON" + t.Run("with plaintext output returns formatted message", func(t *testing.T) { + err := errors.NewError(`{"code":"conflict", "message":"an error"}`) + + result := output.CmdOutputError("plaintext", err) + + assert.Equal(t, "an error (code: conflict)", result) + }) + }) + + t.Run("with a json.UnmarshalTypeError", func(t *testing.T) { + t.Run("with plaintext output returns invalid JSON", func(t *testing.T) { + type testType any + invalid := []byte(`{"invalid": true}`) + err := json.Unmarshal(invalid, &[]testType{}) + require.Error(t, err) + + result := output.CmdOutputError("plaintext", err) + + assert.Equal(t, "invalid JSON", result) + }) + + t.Run("with JSON output returns invalid JSON message", func(t *testing.T) { type testType any invalid := []byte(`{"invalid": true}`) err := json.Unmarshal(invalid, &[]testType{}) require.Error(t, err) + result := output.CmdOutputError("json", err) + + assert.JSONEq(t, `{"message":"invalid JSON"}`, result) + }) + }) + + t.Run("with a generic error", func(t *testing.T) { + t.Run("with JSON output wraps message in JSON", func(t *testing.T) { + err := fmt.Errorf("something went wrong") + + result := output.CmdOutputError("json", err) + + assert.JSONEq(t, `{"message":"something went wrong"}`, result) + }) + + t.Run("with plaintext output returns message", func(t *testing.T) { + err := fmt.Errorf("something went wrong") + result := output.CmdOutputError("plaintext", err) - assert.Equal(t, expected, result) + assert.Equal(t, "something went wrong", result) }) }) } From 7a8c18cdd753dc46ca535986fd6c275dcb6486ff Mon Sep 17 00:00:00 2001 From: Ramon Niebla Date: Thu, 19 Mar 2026 19:38:43 -0700 Subject: [PATCH 3/4] improvements --- CHANGELOG.md | 6 +++ README.md | 8 ++++ cmd/cmdtest.go | 3 ++ cmd/root.go | 18 ++++++-- cmd/root_test.go | 109 ++++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 135 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e24b907c..e092987a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### ⚠ BREAKING CHANGES + +* When stdout is not a TTY, the default `--output` format is now **json** instead of plaintext. Scripts that assumed plaintext when output was piped or redirected should set `LD_OUTPUT=plaintext`, run `ldcli config --set output plaintext`, or pass `--output plaintext` (or `--output json` explicitly if you want JSON regardless of TTY). + ## [2.2.0](https://github.com/launchdarkly/ldcli/compare/v2.1.0...v2.2.0) (2026-02-20) diff --git a/README.md b/README.md index eedd8133..6d7d0f76 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,14 @@ ldcli config --set access-token api-00000000-0000-0000-0000-000000000000 Running this command creates a configuration file located at `$XDG_CONFIG_HOME/ldcli/config.yml` with the access token. Subsequent commands read from this file, so you do not need to specify the access token each time. +### Output format defaults + +When you do not pass `--output` or `--json`, the default format depends on whether standard output is a terminal: **plaintext** in an interactive terminal, **json** when stdout is not a TTY (for example when piped, in CI, or in agent environments). + +To force the plaintext default even when stdout is not a TTY, set either **`FORCE_TTY`** or **`LD_FORCE_TTY`** to any non-empty value (similar to tools that use `NO_COLOR`). That only affects the default; explicit `--output`, `--json`, `LD_OUTPUT`, and the `output` setting in your config file still apply. + +Effective output is resolved in this order: **`--json`** and **`--output`** flags, then **`LD_OUTPUT`**, then the **`output`** value from your config file, then the TTY-based default above. + ## Commands LaunchDarkly CLI commands: diff --git a/cmd/cmdtest.go b/cmd/cmdtest.go index 6e522644..38258fb9 100644 --- a/cmd/cmdtest.go +++ b/cmd/cmdtest.go @@ -19,6 +19,9 @@ var StubbedSuccessResponse = `{ "name": "test-name" }` +// CallCmd runs the root command for integration-style tests. It passes isTerminal always true so +// the default --output matches an interactive terminal (plaintext); non-TTY JSON defaults are +// covered in root_test.go. func CallCmd( t *testing.T, clients APIClients, diff --git a/cmd/root.go b/cmd/root.go index 792f36ca..91ac5acf 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -82,6 +82,18 @@ func init() { cobra.AddTemplateFunc("HasOptionalFlags", HasOptionalFlags) } +// forceTTYDefaultOutput is true when FORCE_TTY or LD_FORCE_TTY is non-empty, so the default +// --output is plaintext even if stdout is not a TTY (similar to NO_COLOR). Explicit --output, +// --json, LD_OUTPUT, and config file values still take precedence via Viper/Cobra after parse. +func forceTTYDefaultOutput() bool { + return os.Getenv("FORCE_TTY") != "" || os.Getenv("LD_FORCE_TTY") != "" +} + +// NewRootCommand constructs the ldcli root command tree. +// +// isTerminal should reflect whether stdout is a TTY (see Execute). For nil or a function that +// returns false, the default --output is json unless forceTTYDefaultOutput applies—intended for +// tests and embeddings; production should always pass a non-nil detector. func NewRootCommand( configService config.Service, analyticsTrackerFn analytics.TrackerFn, @@ -190,10 +202,10 @@ func NewRootCommand( return nil, err } - // When stdout is not a TTY (e.g. piped, CI, agent), default to JSON. - // FORCE_TTY: any non-empty value treats stdout as a terminal (like NO_COLOR convention). + // When stdout is not a TTY (e.g. piped, CI, agent), default to JSON unless FORCE_TTY or + // LD_FORCE_TTY is set (any non-empty value), like NO_COLOR. defaultOutput := "plaintext" - if os.Getenv("FORCE_TTY") == "" && (isTerminal == nil || !isTerminal()) { + if !forceTTYDefaultOutput() && (isTerminal == nil || !isTerminal()) { defaultOutput = "json" } diff --git a/cmd/root_test.go b/cmd/root_test.go index 88646f13..ede33776 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -4,8 +4,10 @@ import ( "bytes" "io" "os" + "path/filepath" "testing" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -153,12 +155,15 @@ func execNonTTYCmd(t *testing.T, mockClient *resources.MockClient, extraArgs ... return out } -func TestTTYDefaultOutput(t *testing.T) { +func TestOutputDefaultsAndOverrides(t *testing.T) { mockClient := &resources.MockClient{ Response: []byte(`{"key": "test-key", "name": "test-name"}`), } t.Run("non-TTY defaults to json output", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) @@ -167,6 +172,9 @@ func TestTTYDefaultOutput(t *testing.T) { }) t.Run("TTY defaults to plaintext output", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + rootCmd := newRootCmdWithTerminal(t, func() bool { return true }) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) @@ -175,6 +183,9 @@ func TestTTYDefaultOutput(t *testing.T) { }) t.Run("nil isTerminal defaults to json output", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + rootCmd := newRootCmdWithTerminal(t, nil) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) @@ -183,34 +194,44 @@ func TestTTYDefaultOutput(t *testing.T) { }) t.Run("explicit --output plaintext overrides non-TTY", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + out := execNonTTYCmd(t, mockClient, "--output", "plaintext") assert.Contains(t, string(out), "Successfully updated") }) t.Run("non-TTY without explicit flag returns JSON", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + out := execNonTTYCmd(t, mockClient) assert.Contains(t, string(out), `"key"`) assert.NotContains(t, string(out), "Successfully updated") }) t.Run("--json overrides non-TTY default", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + out := execNonTTYCmd(t, mockClient, "--json") assert.Contains(t, string(out), `"key"`) assert.NotContains(t, string(out), "Successfully updated") }) t.Run("LD_OUTPUT=plaintext overrides non-TTY default", func(t *testing.T) { - os.Setenv("LD_OUTPUT", "plaintext") - defer os.Unsetenv("LD_OUTPUT") + t.Setenv("LD_OUTPUT", "plaintext") + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") out := execNonTTYCmd(t, mockClient) assert.Contains(t, string(out), "Successfully updated") assert.Contains(t, string(out), "test-name (test-key)") }) - t.Run("FORCE_TTY overrides non-TTY detection", func(t *testing.T) { - os.Setenv("FORCE_TTY", "1") - defer os.Unsetenv("FORCE_TTY") + t.Run("FORCE_TTY=0 yields plaintext DefValue when non-TTY", func(t *testing.T) { + t.Setenv("FORCE_TTY", "0") + t.Setenv("LD_FORCE_TTY", "") rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) @@ -218,4 +239,80 @@ func TestTTYDefaultOutput(t *testing.T) { require.NotNil(t, f) assert.Equal(t, "plaintext", f.DefValue) }) + + t.Run("FORCE_TTY=1 yields plaintext DefValue when non-TTY", func(t *testing.T) { + t.Setenv("FORCE_TTY", "1") + t.Setenv("LD_FORCE_TTY", "") + + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("LD_FORCE_TTY=1 yields plaintext DefValue when non-TTY", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "1") + + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("LD_FORCE_TTY=1 yields plaintext output when non-TTY", func(t *testing.T) { + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "1") + + out := execNonTTYCmd(t, mockClient) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") + }) +} + +func TestConfigOutputPrecedenceNonTTY(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + mockClient := &resources.MockClient{ + Response: []byte(`{"key": "test-key", "name": "test-name"}`), + } + + cfgRoot := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", cfgRoot) + t.Setenv("FORCE_TTY", "") + t.Setenv("LD_FORCE_TTY", "") + + ldcliDir := filepath.Join(cfgRoot, "ldcli") + require.NoError(t, os.MkdirAll(ldcliDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(ldcliDir, "config.yml"), []byte("output: plaintext\n"), 0o644)) + + rootCmd, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{ResourcesClient: mockClient}, + "test", + true, + func() bool { return false }, + ) + require.NoError(t, err) + + c := rootCmd.Cmd() + b := bytes.NewBufferString("") + c.SetOut(b) + c.SetArgs([]string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + }) + require.NoError(t, c.Execute()) + + out, err := io.ReadAll(b) + require.NoError(t, err) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") } From 82177a38e4c312a3654c5a73503cefc55cb4998c Mon Sep 17 00:00:00 2001 From: Ramon Niebla Date: Thu, 19 Mar 2026 19:53:31 -0700 Subject: [PATCH 4/4] addressing pr feedback --- CHANGELOG.md | 2 +- README.md | 4 +- cmd/cmdtest.go | 1 + cmd/root.go | 27 ++++++++--- cmd/root_test.go | 114 +++++++++++++++++++++++++++-------------------- 5 files changed, 92 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e092987a..a8cb7694 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### ⚠ BREAKING CHANGES -* When stdout is not a TTY, the default `--output` format is now **json** instead of plaintext. Scripts that assumed plaintext when output was piped or redirected should set `LD_OUTPUT=plaintext`, run `ldcli config --set output plaintext`, or pass `--output plaintext` (or `--output json` explicitly if you want JSON regardless of TTY). +* When stdout is not a TTY, the default `--output` format is now **json** instead of plaintext. Scripts that assumed plaintext when output was piped or redirected should set `LD_OUTPUT=plaintext`, run `ldcli config --set output plaintext`, or pass `--output plaintext` (or `--output json` explicitly if you want JSON regardless of TTY). You can also set **`FORCE_TTY`** or **`LD_FORCE_TTY`** to any non-empty value to keep plaintext as the default when stdout is not a TTY, without changing the saved `output` setting. ## [2.2.0](https://github.com/launchdarkly/ldcli/compare/v2.1.0...v2.2.0) (2026-02-20) diff --git a/README.md b/README.md index 6d7d0f76..621da7a1 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,9 @@ When you do not pass `--output` or `--json`, the default format depends on wheth To force the plaintext default even when stdout is not a TTY, set either **`FORCE_TTY`** or **`LD_FORCE_TTY`** to any non-empty value (similar to tools that use `NO_COLOR`). That only affects the default; explicit `--output`, `--json`, `LD_OUTPUT`, and the `output` setting in your config file still apply. -Effective output is resolved in this order: **`--json`** and **`--output`** flags, then **`LD_OUTPUT`**, then the **`output`** value from your config file, then the TTY-based default above. +**`LD_OUTPUT`** is the same setting as `output` in the config file, exposed as an environment variable (see the `LD_` prefix above). It is not new with TTY detection; the test suite locks in that it overrides the non-TTY JSON default when set to `plaintext`. + +Effective output is resolved in this order: **`--json`** (if set, wins over `--output` when both are present), then **`--output`**, then **`LD_OUTPUT`**, then the **`output`** value from your config file, then the TTY-based default above. ## Commands diff --git a/cmd/cmdtest.go b/cmd/cmdtest.go index 38258fb9..54ea73bd 100644 --- a/cmd/cmdtest.go +++ b/cmd/cmdtest.go @@ -35,6 +35,7 @@ func CallCmd( "test", false, func() bool { return true }, + nil, ) cmd := rootCmd.Cmd() require.NoError(t, err) diff --git a/cmd/root.go b/cmd/root.go index 91ac5acf..684e50fc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -85,15 +85,25 @@ func init() { // forceTTYDefaultOutput is true when FORCE_TTY or LD_FORCE_TTY is non-empty, so the default // --output is plaintext even if stdout is not a TTY (similar to NO_COLOR). Explicit --output, // --json, LD_OUTPUT, and config file values still take precedence via Viper/Cobra after parse. -func forceTTYDefaultOutput() bool { - return os.Getenv("FORCE_TTY") != "" || os.Getenv("LD_FORCE_TTY") != "" +// +// getenv, if non-nil, is used only for those two keys (tests can inject without mutating +// process environment). If nil, os.Getenv is used. +func forceTTYDefaultOutput(getenv func(string) string) bool { + lookup := getenv + if lookup == nil { + lookup = os.Getenv + } + return lookup("FORCE_TTY") != "" || lookup("LD_FORCE_TTY") != "" } // NewRootCommand constructs the ldcli root command tree. // -// isTerminal should reflect whether stdout is a TTY (see Execute). For nil or a function that -// returns false, the default --output is json unless forceTTYDefaultOutput applies—intended for -// tests and embeddings; production should always pass a non-nil detector. +// isTerminal must be non-nil; it should reflect whether stdout is a TTY (see Execute). When it +// returns false, the default --output is json unless forceTTYDefaultOutput applies. +// +// getenv is optional: when non-nil, it is used to read FORCE_TTY and LD_FORCE_TTY only; when +// nil, os.Getenv is used. Viper still reads LD_OUTPUT and other LD_ vars from the real +// environment. func NewRootCommand( configService config.Service, analyticsTrackerFn analytics.TrackerFn, @@ -101,7 +111,11 @@ func NewRootCommand( version string, useConfigFile bool, isTerminal func() bool, + getenv func(string) string, ) (*RootCmd, error) { + if isTerminal == nil { + return nil, errors.New("NewRootCommand: isTerminal must not be nil") + } cmd := &cobra.Command{ Use: "ldcli", Short: "LaunchDarkly CLI", @@ -205,7 +219,7 @@ func NewRootCommand( // When stdout is not a TTY (e.g. piped, CI, agent), default to JSON unless FORCE_TTY or // LD_FORCE_TTY is set (any non-empty value), like NO_COLOR. defaultOutput := "plaintext" - if !forceTTYDefaultOutput() && (isTerminal == nil || !isTerminal()) { + if !forceTTYDefaultOutput(getenv) && !isTerminal() { defaultOutput = "json" } @@ -274,6 +288,7 @@ func Execute(version string) { version, true, func() bool { return term.IsTerminal(int(os.Stdout.Fd())) }, + nil, ) if err != nil { log.Fatal(err) diff --git a/cmd/root_test.go b/cmd/root_test.go index ede33776..18229042 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -108,7 +108,8 @@ func TestOutputFlags(t *testing.T) { }) } -func newRootCmdWithTerminal(t *testing.T, isTerminal func() bool) *cmd.RootCmd { +// getenv is passed to NewRootCommand for FORCE_TTY / LD_FORCE_TTY only; nil uses os.Getenv. +func newRootCmdWithTerminal(t *testing.T, isTerminal func() bool, getenv func(string) string) *cmd.RootCmd { t.Helper() rootCmd, err := cmd.NewRootCommand( config.NewService(&resources.MockClient{}), @@ -117,12 +118,19 @@ func newRootCmdWithTerminal(t *testing.T, isTerminal func() bool) *cmd.RootCmd { "test", false, isTerminal, + getenv, ) require.NoError(t, err) return rootCmd } func execNonTTYCmd(t *testing.T, mockClient *resources.MockClient, extraArgs ...string) []byte { + t.Helper() + return execNonTTYCmdGetenv(t, mockClient, nil, extraArgs...) +} + +// getenv is forwarded to NewRootCommand for FORCE_TTY / LD_FORCE_TTY only (nil uses os.Getenv). +func execNonTTYCmdGetenv(t *testing.T, mockClient *resources.MockClient, getenv func(string) string, extraArgs ...string) []byte { t.Helper() rootCmd, err := cmd.NewRootCommand( config.NewService(&resources.MockClient{}), @@ -131,6 +139,7 @@ func execNonTTYCmd(t *testing.T, mockClient *resources.MockClient, extraArgs ... "test", false, func() bool { return false }, + getenv, ) require.NoError(t, err) @@ -161,10 +170,7 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { } t.Run("non-TTY defaults to json output", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") - - rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, func(string) string { return "" }) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) require.NotNil(t, f) @@ -172,48 +178,25 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { }) t.Run("TTY defaults to plaintext output", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") - - rootCmd := newRootCmdWithTerminal(t, func() bool { return true }) + rootCmd := newRootCmdWithTerminal(t, func() bool { return true }, func(string) string { return "" }) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) require.NotNil(t, f) assert.Equal(t, "plaintext", f.DefValue) }) - t.Run("nil isTerminal defaults to json output", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") - - rootCmd := newRootCmdWithTerminal(t, nil) - - f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) - require.NotNil(t, f) - assert.Equal(t, "json", f.DefValue) - }) - t.Run("explicit --output plaintext overrides non-TTY", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") - out := execNonTTYCmd(t, mockClient, "--output", "plaintext") assert.Contains(t, string(out), "Successfully updated") }) t.Run("non-TTY without explicit flag returns JSON", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") - out := execNonTTYCmd(t, mockClient) assert.Contains(t, string(out), `"key"`) assert.NotContains(t, string(out), "Successfully updated") }) t.Run("--json overrides non-TTY default", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") - out := execNonTTYCmd(t, mockClient, "--json") assert.Contains(t, string(out), `"key"`) assert.NotContains(t, string(out), "Successfully updated") @@ -221,8 +204,6 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { t.Run("LD_OUTPUT=plaintext overrides non-TTY default", func(t *testing.T) { t.Setenv("LD_OUTPUT", "plaintext") - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") out := execNonTTYCmd(t, mockClient) assert.Contains(t, string(out), "Successfully updated") @@ -230,10 +211,13 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { }) t.Run("FORCE_TTY=0 yields plaintext DefValue when non-TTY", func(t *testing.T) { - t.Setenv("FORCE_TTY", "0") - t.Setenv("LD_FORCE_TTY", "") - - rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + getenv := func(k string) string { + if k == "FORCE_TTY" { + return "0" + } + return "" + } + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, getenv) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) require.NotNil(t, f) @@ -241,10 +225,13 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { }) t.Run("FORCE_TTY=1 yields plaintext DefValue when non-TTY", func(t *testing.T) { - t.Setenv("FORCE_TTY", "1") - t.Setenv("LD_FORCE_TTY", "") - - rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + getenv := func(k string) string { + if k == "FORCE_TTY" { + return "1" + } + return "" + } + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, getenv) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) require.NotNil(t, f) @@ -252,10 +239,13 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { }) t.Run("LD_FORCE_TTY=1 yields plaintext DefValue when non-TTY", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "1") - - rootCmd := newRootCmdWithTerminal(t, func() bool { return false }) + getenv := func(k string) string { + if k == "LD_FORCE_TTY" { + return "1" + } + return "" + } + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, getenv) f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) require.NotNil(t, f) @@ -263,15 +253,44 @@ func TestOutputDefaultsAndOverrides(t *testing.T) { }) t.Run("LD_FORCE_TTY=1 yields plaintext output when non-TTY", func(t *testing.T) { - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "1") + getenv := func(k string) string { + if k == "LD_FORCE_TTY" { + return "1" + } + return "" + } + out := execNonTTYCmdGetenv(t, mockClient, getenv) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") + }) - out := execNonTTYCmd(t, mockClient) + t.Run("FORCE_TTY=1 yields plaintext output when non-TTY", func(t *testing.T) { + getenv := func(k string) string { + if k == "FORCE_TTY" { + return "1" + } + return "" + } + out := execNonTTYCmdGetenv(t, mockClient, getenv) assert.Contains(t, string(out), "Successfully updated") assert.Contains(t, string(out), "test-name (test-key)") }) } +func TestNewRootCommandNilIsTerminalRejects(t *testing.T) { + _, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{}, + "test", + false, + nil, + nil, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "isTerminal") +} + func TestConfigOutputPrecedenceNonTTY(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) @@ -282,8 +301,6 @@ func TestConfigOutputPrecedenceNonTTY(t *testing.T) { cfgRoot := t.TempDir() t.Setenv("XDG_CONFIG_HOME", cfgRoot) - t.Setenv("FORCE_TTY", "") - t.Setenv("LD_FORCE_TTY", "") ldcliDir := filepath.Join(cfgRoot, "ldcli") require.NoError(t, os.MkdirAll(ldcliDir, 0o755)) @@ -296,6 +313,7 @@ func TestConfigOutputPrecedenceNonTTY(t *testing.T) { "test", true, func() bool { return false }, + nil, ) require.NoError(t, err)