chore(onboarding): make all interactive onboarding opt-in#363
Open
brycelelbach wants to merge 1 commit intobrevdev:mainfrom
Open
chore(onboarding): make all interactive onboarding opt-in#363brycelelbach wants to merge 1 commit intobrevdev:mainfrom
brycelelbach wants to merge 1 commit intobrevdev:mainfrom
Conversation
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.
There was a problem hiding this comment.
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
brevroot,brev login, andbrev 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.`, |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
brevcurrently 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:hello.CanWeOnboard("Want a quick tour?") fires from the root commandPostRun(so it triggers after anybrevsubcommand, not justlogin) and again frombrev login'sPostRunE.brev login'shandleOnboarding.agentskill.RunInstallSkillIfWanted("Install agent skill?") fires at the end ofbrev login.hello.Step1(guided "now runbrev shell" flow) fires frombrev ls'sPersistentPostRunE.None of these have an opt-in signal before they run. That's the wrong default for an agentic CLI: the
brev login --token $TOKENsnippet 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:
brev hello(already existed, unchanged)brev agent-skill install(already existed, unchanged)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 rootPostRunthat ranCanWeOnboardon every invocation of anybrevsubcommand.pkg/cmd/login/login.go— drop thePostRunEthat ranCanWeOnboard, drop theRunInstallSkillIfWantedcall, drop the analytics prompt. AddprintOptInHintsat the end of a successful login.pkg/cmd/ls/ls.go— drop thePersistentPostRunEthat ranhello.Step1.pkg/cmd/hello/onboarding_utils.go— deleteCanWeOnboard,ShouldWeRunOnboarding,ShouldWeRunOnboardingLSStep(all dead after trigger removal).pkg/cmd/agentskill/agentskill.go— deleteRunInstallSkillIfWanted,PromptInstallSkill,PromptInstallSkillSimple(dead after trigger removal).pkg/cmd/analytics/analytics.go— newbrev analyticscommand withenable/disable/statussubcommands, backed by the existingSetAnalyticsPreference/IsAnalyticsEnabledAPIs.Relationship to other PRs
--no-interactivetobrev loginto suppress exactly these three prompts for agent/CI scripts. With this PR, those prompts no longer run for anyone, so the flag's onboarding-suppression effect becomes a no-op. That PR could be closed, or rebased down to the "require--tokenwhen--no-interactive" assertion only if the author wants to keep an explicit agentic marker.Behavioral details worth reviewing
IsAnalyticsEnabledreturnsfalse, falsein that case). Removing the prompt does not change the default — it only removes the interactive path to opt in.brev analytics enablereplaces it.finishedOnboardingflag: previously flipped as a side effect ofShouldWeRunOnboardingLSStep. 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 frombrev helloon success.hello.Step1and its downstream helpers (doVsCodeOnboarding,doBrevShellOnboarding,printLsIntroText, …) are left inpkg/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. abrev hello lssubcommand). Dead-code cleanup can be a follow-up if not desired.Test plan
go build ./...go vet ./...go test ./...(only thee2etest/setuppackage fails, and it also fails on stockmain— environmental, unrelated)brev login --token $TOKENcompletes without any interactive prompt and prints the "Optional next steps" block.brev lson a fresh account does not triggerhello.Step1.brev analytics status/enable/disableround-trips through the settings file.🤖 Generated with Claude Code