address codex review

Author: rvvance

pkg/analytics/posthog.go

@@ -48,9 +48,7 @@ func getClient() (posthog.Client, error) {
 	return client, clientErr
 }
 
-// IsAnalyticsFeatureEnabled checks the PostHog feature flag that acts as a
-// remote kill switch for CLI telemetry. Lets us globally disable capture
-// without shipping a CLI release.
+// IsAnalyticsFeatureEnabled is the remote kill switch for CLI telemetry; gating capture lets us turn it off without a release.
 func IsAnalyticsFeatureEnabled() bool {
 	anonID := GetOrCreateAnalyticsID()
 	if anonID == "" {
@@ -82,11 +80,9 @@ func RecordCommandStart(cmd *cobra.Command, args []string) {
 	storedArgs = args
 }
 
-// IsAnalyticsEnabled returns whether analytics capture is currently enabled.
-// Default is on (opt-out model). Honors DO_NOT_TRACK and BREV_NO_ANALYTICS env
-// vars as a kill switch for CI / scripted use.
+// IsAnalyticsEnabled defaults to true; DO_NOT_TRACK and BREV_NO_ANALYTICS override.
 func IsAnalyticsEnabled() bool {
-	if os.Getenv("DO_NOT_TRACK") == "1" || os.Getenv("BREV_NO_ANALYTICS") == "1" {
+	if disabled, _ := IsDisabledByEnv(); disabled {
 		return false
 	}
 	settings := readSettings()
@@ -96,8 +92,6 @@ func IsAnalyticsEnabled() bool {
 	return *settings.AnalyticsEnabled
 }
 
-// IsDisabledByEnv reports whether analytics is currently disabled by an
-// environment-variable kill switch (vs. the user's persisted preference).
 func IsDisabledByEnv() (disabled bool, varName string) {
 	if os.Getenv("DO_NOT_TRACK") == "1" {
 		return true, "DO_NOT_TRACK"
@@ -108,13 +102,6 @@ func IsDisabledByEnv() (disabled bool, varName string) {
 	return false, ""
 }
 
-// IsAnalyticsExplicitlySet reports whether the user has explicitly chosen a
-// preference (true/false), as opposed to leaving it at the default.
-func IsAnalyticsExplicitlySet() bool {
-	settings := readSettings()
-	return settings.AnalyticsEnabled != nil
-}
-
 // SetAnalyticsPreference persists the user's analytics preference.
 func SetAnalyticsPreference(enabled bool) error {
 	fs := files.AppFs
@@ -202,14 +189,7 @@ func CaptureCommandError() {
 	if storedCmd == nil {
 		return
 	}
-	// If CaptureCommand already ran (success path), don't double-capture.
-	// storedUser being set means PersistentPostRunE ran.
-	// We only get here on error, so PersistentPostRunE didn't run.
-	userID := storedUser
-	if userID == "" {
-		userID = GetOrCreateAnalyticsID()
-	}
-	captureEvent(userID, storedCmd, storedArgs, false)
+	captureEvent(storedUser, storedCmd, storedArgs, false)
 }
 
 func captureEvent(userID string, cmd *cobra.Command, args []string, succeeded bool) {
@@ -220,6 +200,11 @@ func captureEvent(userID string, cmd *cobra.Command, args []string, succeeded bo
 		return
 	}
 
+	// Resolve the analytics ID lazily, only after gates pass — avoids writing a
+	// persistent UUID to ~/.brev/personal_settings.json for opted-out users.
+	if userID == "" {
+		userID = GetOrCreateAnalyticsID()
+	}
 	if userID == "" {
 		return
 	}

pkg/cmd/analytics/analytics.go

@@ -1,24 +1,22 @@
-// Package analytics provides the `brev analytics` command for managing usage analytics preferences.
 package analytics
 
 import (
 	"github.com/brevdev/brev-cli/pkg/analytics"
+	"github.com/brevdev/brev-cli/pkg/cmd/cmderrors"
 	breverrors "github.com/brevdev/brev-cli/pkg/errors"
 	"github.com/brevdev/brev-cli/pkg/terminal"
 	"github.com/spf13/cobra"
 )
 
-// AnalyticsStore is provided for parity with other commands; no store calls are required today.
-type AnalyticsStore interface{}
-
-func NewCmdAnalytics(t *terminal.Terminal, _ AnalyticsStore) *cobra.Command {
+func NewCmdAnalytics(t *terminal.Terminal) *cobra.Command {
 	cmd := &cobra.Command{
 		Annotations:           map[string]string{"configuration": ""},
 		Use:                   "analytics",
 		DisableFlagsInUseLine: true,
 		Short:                 "Manage usage analytics",
 		Long:                  "Show or change whether the Brev CLI sends usage analytics.",
 		Example:               "brev analytics\nbrev analytics on\nbrev analytics off",
+		Args:                  cmderrors.TransformToValidationError(cobra.NoArgs),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			printStatus(t)
 			return nil
@@ -36,11 +34,15 @@ func newCmdOn(t *terminal.Terminal) *cobra.Command {
 		Use:                   "on",
 		DisableFlagsInUseLine: true,
 		Short:                 "Enable usage analytics",
+		Args:                  cmderrors.TransformToValidationError(cobra.NoArgs),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if err := analytics.SetAnalyticsPreference(true); err != nil {
 				return breverrors.WrapAndTrace(err)
 			}
 			t.Vprintf("%s\n", t.Green("Analytics enabled."))
+			if disabled, varName := analytics.IsDisabledByEnv(); disabled {
+				t.Vprintf("Note: still disabled at runtime by %s=1.\n", varName)
+			}
 			return nil
 		},
 	}
@@ -51,6 +53,7 @@ func newCmdOff(t *terminal.Terminal) *cobra.Command {
 		Use:                   "off",
 		DisableFlagsInUseLine: true,
 		Short:                 "Disable usage analytics",
+		Args:                  cmderrors.TransformToValidationError(cobra.NoArgs),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if err := analytics.SetAnalyticsPreference(false); err != nil {
 				return breverrors.WrapAndTrace(err)

pkg/cmd/cmd.go

@@ -155,7 +155,7 @@ func NewBrevCommand() *cobra.Command { //nolint:funlen,gocognit,gocyclo // defin
       Find more information at:
             https://brev.nvidia.com`,
 		PersistentPostRunE: func(cmd *cobra.Command, args []string) error {
-			analytics.CaptureCommand(analytics.GetOrCreateAnalyticsID(), cmd, args)
+			analytics.CaptureCommand("", cmd, args)
 			return nil
 		},
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
@@ -321,7 +321,7 @@ func createCmdTree(cmd *cobra.Command, t *terminal.Terminal, loginCmdStore *stor
 	cmd.AddCommand(open.NewCmdOpen(t, loginCmdStore, noLoginCmdStore))
 	cmd.AddCommand(ollama.NewCmdOllama(t, loginCmdStore))
 	cmd.AddCommand(agentskill.NewCmdAgentSkill(t, noLoginCmdStore))
-	cmd.AddCommand(analyticscmd.NewCmdAnalytics(t, nil))
+	cmd.AddCommand(analyticscmd.NewCmdAnalytics(t))
 	cmd.AddCommand(background.NewCmdBackground(t, loginCmdStore))
 	cmd.AddCommand(status.NewCmdStatus(t, loginCmdStore))
 	cmd.AddCommand(sshkeys.NewCmdSSHKeys(t, loginCmdStore))