Skip to content

chore(onboarding): make all interactive onboarding opt-in#363

Open
brycelelbach wants to merge 1 commit intobrevdev:mainfrom
brycelelbach:chore/opt-in-onboarding
Open

chore(onboarding): make all interactive onboarding opt-in#363
brycelelbach wants to merge 1 commit intobrevdev:mainfrom
brycelelbach:chore/opt-in-onboarding

Conversation

@brycelelbach
Copy link
Copy Markdown
Contributor

Summary

brev currently runs three pieces of onboarding implicitly — with no way for humans or AI coding agents to suppress them short of answering each prompt by hand:

  1. hello.CanWeOnboard ("Want a quick tour?") fires from the root command PostRun (so it triggers after any brev subcommand, not just login) and again from brev login's PostRunE.
  2. The analytics opt-in prompt fires from brev login's handleOnboarding.
  3. agentskill.RunInstallSkillIfWanted ("Install agent skill?") fires at the end of brev login.
  4. hello.Step1 (guided "now run brev shell" flow) fires from brev ls's PersistentPostRunE.

None of these have an opt-in signal before they run. That's the wrong default for an agentic CLI: the brev login --token $TOKEN snippet at https://brev.nvidia.com/settings/cli is run verbatim by scripts and Claude Code / Codex / Cursor sessions, and each prompt surfaces as a blocked shell waiting on stdin.

Fix

Make onboarding entirely opt-in. The automatic triggers are removed; users who want any of these flows invoke them explicitly:

  • interactive tour: brev hello (already existed, unchanged)
  • install AI agent skill: brev agent-skill install (already existed, unchanged)
  • share anonymous usage: brev analytics enable (new in this PR)

At the end of a successful brev login, the CLI now prints a single "Optional next steps:" block pointing at those three commands, so the discoverability that the prompts provided is preserved without the stdin dependency.

Code changes

  • pkg/cmd/cmd.go — drop the root PostRun that ran CanWeOnboard on every invocation of any brev subcommand.
  • pkg/cmd/login/login.go — drop the PostRunE that ran CanWeOnboard, drop the RunInstallSkillIfWanted call, drop the analytics prompt. Add printOptInHints at the end of a successful login.
  • pkg/cmd/ls/ls.go — drop the PersistentPostRunE that ran hello.Step1.
  • pkg/cmd/hello/onboarding_utils.go — delete CanWeOnboard, ShouldWeRunOnboarding, ShouldWeRunOnboardingLSStep (all dead after trigger removal).
  • pkg/cmd/agentskill/agentskill.go — delete RunInstallSkillIfWanted, PromptInstallSkill, PromptInstallSkillSimple (dead after trigger removal).
  • pkg/cmd/analytics/analytics.go — new brev analytics command with enable / disable / status subcommands, backed by the existing SetAnalyticsPreference / IsAnalyticsEnabled APIs.

Relationship to other PRs

Behavioral details worth reviewing

  • Analytics default: already off when the preference file is absent (IsAnalyticsEnabled returns false, false in that case). Removing the prompt does not change the default — it only removes the interactive path to opt in. brev analytics enable replaces it.
  • Server-side finishedOnboarding flag: previously flipped as a side effect of ShouldWeRunOnboardingLSStep. Not flipped anymore — but that flag's only readers were the onboarding gates we removed, so nothing depends on it on the CLI side. If server-side logic reads it, a follow-up could flip it from brev hello on success.
  • hello.Step1 and its downstream helpers (doVsCodeOnboarding, doBrevShellOnboarding, printLsIntroText, …) are left in pkg/cmd/hello/steps.go. They are unused by this PR but kept as a starting point for any future explicit onboarding entry point (e.g. a brev hello ls subcommand). Dead-code cleanup can be a follow-up if not desired.

Test plan

  • go build ./...
  • go vet ./...
  • go test ./... (only the e2etest/setup package fails, and it also fails on stock main — environmental, unrelated)
  • Manual: fresh install, brev login --token $TOKEN completes without any interactive prompt and prints the "Optional next steps" block.
  • Manual: brev ls on a fresh account does not trigger hello.Step1.
  • Manual: brev analytics status / enable / disable round-trips through the settings file.

🤖 Generated with Claude Code

brev currently runs three pieces of onboarding *implicitly*, with no way
for humans or AI coding agents to suppress them short of answering each
one by hand:

  1. `hello.CanWeOnboard` ("Want a quick tour?") — fires from the root
     command PostRun AND from `brev login`'s PostRunE.
  2. The analytics opt-in prompt — fires from `brev login`'s handle-
     Onboarding.
  3. `agentskill.RunInstallSkillIfWanted` ("Install agent skill?") —
     fires at the end of `brev login`.
  4. `hello.Step1` (guided "now run brev shell" flow) — fires from
     `brev ls`'s PersistentPostRunE.

None of these have a consent signal before they run. That's the wrong
default for an agentic CLI: the snippet at brev.nvidia.com/settings/cli
is run verbatim by scripts and Claude Code / Codex / Cursor sessions,
and each prompt surfaces as a blocked shell waiting on stdin.

This change makes onboarding **entirely opt-in**. The automatic
triggers are removed; users who want any of these flows invoke them
explicitly:

  - interactive tour:         brev hello   (already existed)
  - install AI agent skill:   brev agent-skill install   (already existed)
  - share anonymous usage:    brev analytics enable   (new in this PR)

`brev login` prints a single "Optional next steps:" block at the end
pointing at those three commands, so the discoverability that the
prompts provided is preserved without the stdin dependency.

Code changes:

  - pkg/cmd/cmd.go: drop the root PostRun that ran CanWeOnboard on every
    invocation of any brev subcommand.
  - pkg/cmd/login/login.go: drop the PostRunE that ran CanWeOnboard,
    drop the RunInstallSkillIfWanted call, drop the analytics prompt.
    Add printOptInHints at the end of a successful login.
  - pkg/cmd/ls/ls.go: drop the PersistentPostRunE that ran hello.Step1.
  - pkg/cmd/hello/onboarding_utils.go: delete CanWeOnboard,
    ShouldWeRunOnboarding, ShouldWeRunOnboardingLSStep (all dead after
    trigger removal).
  - pkg/cmd/agentskill/agentskill.go: delete RunInstallSkillIfWanted,
    PromptInstallSkill, PromptInstallSkillSimple (dead after trigger
    removal).
  - pkg/cmd/analytics/analytics.go: new brev analytics command with
    enable / disable / status subcommands, backed by the existing
    SetAnalyticsPreference / IsAnalyticsEnabled APIs.
Copilot AI review requested due to automatic review settings April 18, 2026 08:20
@brycelelbach brycelelbach requested a review from a team as a code owner April 18, 2026 08:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Makes interactive onboarding flows fully opt-in to avoid blocking scripted/agentic CLI usage, and introduces an explicit command for managing analytics consent.

Changes:

  • Removes implicit onboarding triggers from brev root, brev login, and brev ls.
  • Adds post-login “Optional next steps” guidance instead of interactive prompts.
  • Introduces brev analytics {enable|disable|status} for explicit analytics opt-in/out.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/cmd/cmd.go Removes root-level onboarding trigger and wires in new brev analytics command.
pkg/cmd/login/login.go Removes post-login onboarding/skill/analytics prompts; prints opt-in hints after successful login.
pkg/cmd/ls/ls.go Removes implicit onboarding step previously triggered after brev ls.
pkg/cmd/hello/onboarding_utils.go Deletes onboarding gate/prompt helpers that are now dead code.
pkg/cmd/agentskill/agentskill.go Removes login-triggered agent-skill prompt helpers (dead after trigger removal).
pkg/cmd/analytics/analytics.go Adds new command surface for managing analytics preference.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +16 to +30
cmd := &cobra.Command{
Annotations: map[string]string{"configuration": ""},
Use: "analytics",
DisableFlagsInUseLine: true,
Short: "Manage anonymous usage analytics",
Long: `Enable, disable, or check the status of anonymous usage analytics.

Analytics are opt-in. When enabled, Brev reports command usage and error
rates to help the team prioritize fixes and improvements. No command
arguments, file contents, or credentials are ever captured.`,
Example: "brev analytics enable\nbrev analytics disable\nbrev analytics status",
RunE: func(cmd *cobra.Command, args []string) error {
return runStatus(t)
},
}
Comment on lines +24 to +25
rates to help the team prioritize fixes and improvements. No command
arguments, file contents, or credentials are ever captured.`,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants