Got rid of forced tutorial and added auto opt in for CLI analytics (#370)

Author: rvvance

pkg/analytics/analytics.go

@@ -17,6 +17,10 @@ type EventData struct {
 }
 
 func TrackEvent(data EventData) error {
+	if !IsAnalyticsEnabled() {
+		return nil
+	}
+
 	conf := config.NewConstants()
 
 	url := conf.GetBrevAPIURl() + "/api/brevent"

pkg/analytics/posthog.go

@@ -48,8 +48,7 @@ func getClient() (posthog.Client, error) {
 	return client, clientErr
 }
 
-// IsAnalyticsFeatureEnabled checks the PostHog feature flag to determine
-// whether to prompt the user about analytics opt-in.
+// IsAnalyticsFeatureEnabled is the remote kill switch for PostHog telemetry only — gating PostHog capture lets us turn it off without a release. It does NOT gate analytics.TrackEvent (the brev-internal endpoint), which has its own channel.
 func IsAnalyticsFeatureEnabled() bool {
 	anonID := GetOrCreateAnalyticsID()
 	if anonID == "" {
@@ -81,13 +80,26 @@ func RecordCommandStart(cmd *cobra.Command, args []string) {
 	storedArgs = args
 }
 
-// IsAnalyticsEnabled returns whether analytics is enabled and whether the user has been asked.
-func IsAnalyticsEnabled() (enabled bool, hasBeenAsked bool) {
+// IsAnalyticsEnabled defaults to true; DO_NOT_TRACK and BREV_NO_ANALYTICS override.
+func IsAnalyticsEnabled() bool {
+	if disabled, _ := IsDisabledByEnv(); disabled {
+		return false
+	}
 	settings := readSettings()
 	if settings.AnalyticsEnabled == nil {
-		return false, false
+		return true
+	}
+	return *settings.AnalyticsEnabled
+}
+
+func IsDisabledByEnv() (disabled bool, varName string) {
+	if os.Getenv("DO_NOT_TRACK") == "1" {
+		return true, "DO_NOT_TRACK"
+	}
+	if os.Getenv("BREV_NO_ANALYTICS") == "1" {
+		return true, "BREV_NO_ANALYTICS"
 	}
-	return *settings.AnalyticsEnabled, true
+	return false, ""
 }
 
 // SetAnalyticsPreference persists the user's analytics preference.
@@ -139,34 +151,15 @@ func GetOrCreateAnalyticsID() string {
 	return settings.AnalyticsID
 }
 
-// CaptureAnalyticsOptIn sends an event recording the user's analytics consent choice.
-// This is sent regardless of the user's choice so we can measure opt-in rates.
-func CaptureAnalyticsOptIn(optedIn bool) {
-	anonID := GetOrCreateAnalyticsID()
-	if anonID == "" {
-		return
-	}
-
-	c, err := getClient()
-	if err != nil {
-		return
-	}
-
-	_ = c.Enqueue(posthog.Capture{
-		DistinctId: anonID,
-		Event:      "analytics_opt_in",
-		Properties: posthog.NewProperties().
-			Set("opted_in", optedIn).
-			Set("os", runtime.GOOS).
-			Set("arch", runtime.GOARCH).
-			Set("cli_version", version.Version),
-	})
+// shouldCapturePostHog returns true only when both the local opt-out and
+// the remote PostHog kill switch agree.
+func shouldCapturePostHog() bool {
+	return IsAnalyticsEnabled() && IsAnalyticsFeatureEnabled()
 }
 
 // IdentifyUser links the anonymous analytics ID to a real user ID using PostHog Alias.
 func IdentifyUser(userID string) {
-	enabled, asked := IsAnalyticsEnabled()
-	if !asked || !enabled {
+	if !shouldCapturePostHog() {
 		return
 	}
 
@@ -202,22 +195,19 @@ 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) {
-	enabled, asked := IsAnalyticsEnabled()
-	if !asked || !enabled {
+	if !shouldCapturePostHog() {
 		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/analytics/posthog_test.go

@@ -0,0 +1,112 @@
+package analytics
+
+import (
+	"testing"
+
+	"github.com/brevdev/brev-cli/pkg/files"
+)
+
+func boolPtr(b bool) *bool { return &b }
+
+func TestIsDisabledByEnv(t *testing.T) {
+	cases := []struct {
+		name         string
+		envs         map[string]string
+		wantDisabled bool
+		wantVar      string
+	}{
+		{"no env vars set", nil, false, ""},
+		{"DO_NOT_TRACK=1", map[string]string{"DO_NOT_TRACK": "1"}, true, "DO_NOT_TRACK"},
+		{"BREV_NO_ANALYTICS=1", map[string]string{"BREV_NO_ANALYTICS": "1"}, true, "BREV_NO_ANALYTICS"},
+		{"DO_NOT_TRACK=0 (only \"1\" disables)", map[string]string{"DO_NOT_TRACK": "0"}, false, ""},
+		{"DO_NOT_TRACK=true (only \"1\" disables)", map[string]string{"DO_NOT_TRACK": "true"}, false, ""},
+		{"both set — DO_NOT_TRACK reported first", map[string]string{"DO_NOT_TRACK": "1", "BREV_NO_ANALYTICS": "1"}, true, "DO_NOT_TRACK"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			t.Setenv("DO_NOT_TRACK", "")
+			t.Setenv("BREV_NO_ANALYTICS", "")
+			for k, v := range c.envs {
+				t.Setenv(k, v)
+			}
+			disabled, varName := IsDisabledByEnv()
+			if disabled != c.wantDisabled {
+				t.Errorf("disabled = %v, want %v", disabled, c.wantDisabled)
+			}
+			if varName != c.wantVar {
+				t.Errorf("varName = %q, want %q", varName, c.wantVar)
+			}
+		})
+	}
+}
+
+func TestIsAnalyticsEnabled(t *testing.T) {
+	cases := []struct {
+		name   string
+		stored *bool
+		envs   map[string]string
+		want   bool
+	}{
+		{"no preference, no env → default on", nil, nil, true},
+		{"explicit opt-in, no env", boolPtr(true), nil, true},
+		{"explicit opt-out, no env", boolPtr(false), nil, false},
+		{"DO_NOT_TRACK overrides nil", nil, map[string]string{"DO_NOT_TRACK": "1"}, false},
+		{"DO_NOT_TRACK overrides explicit opt-in", boolPtr(true), map[string]string{"DO_NOT_TRACK": "1"}, false},
+		{"BREV_NO_ANALYTICS overrides explicit opt-in", boolPtr(true), map[string]string{"BREV_NO_ANALYTICS": "1"}, false},
+		{"explicit opt-out stays opt-out under env override", boolPtr(false), map[string]string{"DO_NOT_TRACK": "1"}, false},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			tmp := t.TempDir()
+			t.Setenv("HOME", tmp)
+			t.Setenv("DO_NOT_TRACK", "")
+			t.Setenv("BREV_NO_ANALYTICS", "")
+			for k, v := range c.envs {
+				t.Setenv(k, v)
+			}
+
+			if c.stored != nil {
+				if err := files.WritePersonalSettings(files.AppFs, tmp, &files.PersonalSettings{
+					AnalyticsEnabled: c.stored,
+				}); err != nil {
+					t.Fatalf("write settings: %v", err)
+				}
+			}
+
+			if got := IsAnalyticsEnabled(); got != c.want {
+				t.Errorf("IsAnalyticsEnabled() = %v, want %v", got, c.want)
+			}
+		})
+	}
+}
+
+// SetAnalyticsPreference must not lose other PersonalSettings fields.
+func TestSetAnalyticsPreferencePreservesOtherFields(t *testing.T) {
+	tmp := t.TempDir()
+	t.Setenv("HOME", tmp)
+
+	if err := files.WritePersonalSettings(files.AppFs, tmp, &files.PersonalSettings{
+		DefaultEditor: "vim",
+		AnalyticsID:   "preexisting-id",
+	}); err != nil {
+		t.Fatalf("write seed: %v", err)
+	}
+
+	if err := SetAnalyticsPreference(false); err != nil {
+		t.Fatalf("SetAnalyticsPreference: %v", err)
+	}
+
+	got, err := files.ReadPersonalSettings(files.AppFs, tmp)
+	if err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if got.DefaultEditor != "vim" {
+		t.Errorf("DefaultEditor = %q, want %q (other fields must survive)", got.DefaultEditor, "vim")
+	}
+	if got.AnalyticsID != "preexisting-id" {
+		t.Errorf("AnalyticsID = %q, want %q", got.AnalyticsID, "preexisting-id")
+	}
+	if got.AnalyticsEnabled == nil || *got.AnalyticsEnabled != false {
+		t.Errorf("AnalyticsEnabled = %v, want pointer to false", got.AnalyticsEnabled)
+	}
+}

pkg/cmd/agentskill/agentskill.go

@@ -2,20 +2,16 @@
 package agentskill
 
 import (
-	"bufio"
 	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 
 	breverrors "github.com/brevdev/brev-cli/pkg/errors"
 	"github.com/brevdev/brev-cli/pkg/terminal"
-	"github.com/fatih/color"
-	"github.com/manifoldco/promptui"
 	"github.com/spf13/cobra"
 )
 
@@ -181,11 +177,14 @@ func GetSkillDir(homeDir string) string {
 	return filepath.Join(homeDir, ".claude", "skills", skillName)
 }
 
-// IsClaudeInstalled checks if Claude Code appears to be installed
-func IsClaudeInstalled(homeDir string) bool {
-	claudeDir := filepath.Join(homeDir, ".claude")
-	_, err := os.Stat(claudeDir)
-	return err == nil
+// IsAnyAgentInstalled returns true if any of installDirs exists under homeDir.
+func IsAnyAgentInstalled(homeDir string) bool {
+	for _, dir := range installDirs {
+		if _, err := os.Stat(filepath.Join(homeDir, dir)); err == nil {
+			return true
+		}
+	}
+	return false
 }
 
 // IsSkillInstalled checks if the brev-cli skill is installed in any location
@@ -199,45 +198,6 @@ func IsSkillInstalled(homeDir string) bool {
 	return false
 }
 
-// PromptInstallSkill asks the user if they want to install the agent skill
-// Returns true if they want to install, false otherwise
-func PromptInstallSkill(t *terminal.Terminal, homeDir string) bool {
-	// Skip if skill is already installed
-	if IsSkillInstalled(homeDir) {
-		return false
-	}
-
-	// Check if Claude Code appears to be installed
-	if !IsClaudeInstalled(homeDir) {
-		return false
-	}
-
-	fmt.Println()
-	caretType := color.New(color.FgCyan, color.Bold).SprintFunc()
-	fmt.Println("  ", caretType("▸"), "    AI Agent Integration")
-	fmt.Println()
-	fmt.Println("       We detected an AI coding agent on your system.")
-	fmt.Println("       Would you like to install the Brev CLI skill?")
-	fmt.Println()
-	fmt.Println("       This enables natural language commands like:")
-	fmt.Println(t.Yellow("         \"Create an A100 instance for ML training\""))
-	fmt.Println(t.Yellow("         \"Search for GPUs with 40GB VRAM\""))
-	fmt.Println(t.Yellow("         \"Stop all my running instances\""))
-	fmt.Println()
-
-	prompt := promptui.Select{
-		Label: "Install agent skill",
-		Items: []string{"Yes, install it", "No, skip for now"},
-	}
-
-	idx, _, err := prompt.Run()
-	if err != nil {
-		return false
-	}
-
-	return idx == 0
-}
-
 // InstallSkill downloads and installs the agent skill to all install paths
 func InstallSkill(t *terminal.Terminal, homeDir string, quiet bool) error {
 	skillDirs := GetSkillDirs(homeDir)
@@ -322,18 +282,6 @@ func UninstallSkill(t *terminal.Terminal, homeDir string) error {
 	return nil
 }
 
-// RunInstallSkillIfWanted prompts and installs if user wants it
-// This is called from the login flow
-func RunInstallSkillIfWanted(t *terminal.Terminal, homeDir string) {
-	if PromptInstallSkill(t, homeDir) {
-		err := InstallSkill(t, homeDir, false)
-		if err != nil {
-			// Don't fail login for skill install errors
-			fmt.Printf("  %s Failed to install skill: %v\n", t.Yellow("Warning:"), err)
-		}
-	}
-}
-
 // downloadAndInstallFile downloads a single file and writes it to all skill dirs.
 // Returns true on success, false if the download or any write failed.
 func downloadAndInstallFile(client *http.Client, baseURL, file string, skillDirs []string, t *terminal.Terminal, quiet bool) bool {
@@ -386,12 +334,3 @@ func downloadBytes(client *http.Client, url string) ([]byte, error) {
 
 	return body, nil
 }
-
-// PromptInstallSkillSimple is a simpler yes/no prompt for the login flow
-func PromptInstallSkillSimple() bool {
-	reader := bufio.NewReader(os.Stdin)
-	fmt.Print("Install agent skill? [y/N]: ")
-	response, _ := reader.ReadString('\n')
-	response = strings.ToLower(strings.TrimSpace(response))
-	return response == "y" || response == "yes"
-}