fix(upgrade): drop sudo prompt and warn about stale system binary

Address PR review feedback: the install script writes to ~/.local/bin
without sudo, so brev upgrade no longer gates on sudo and the prompt
text reflects the new location. After a direct upgrade we also detect
when the running binary lives in /usr/local/bin or /usr/bin (left over
from a pre-~/.local/bin install) and tell the user to remove it so
PATH resolution doesn't keep pinning them to the old version.

Author: theFong

pkg/cmd/upgrade/runner.go

@@ -33,7 +33,7 @@ func (SystemUpgrader) UpgradeViaBrew(t *terminal.Terminal) error {
 	return nil
 }
 
-// UpgradeViaInstallScript runs the upstream install-latest.sh script via sudo.
+// UpgradeViaInstallScript runs the upstream install-latest.sh script.
 func (SystemUpgrader) UpgradeViaInstallScript(t *terminal.Terminal) error {
 	t.Vprintf("Running: bash -c \"$(curl -fsSL %s)\"\n", installScriptURL)
 	t.Vprint("")

pkg/cmd/upgrade/upgrade.go

@@ -4,12 +4,12 @@ package upgrade
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"github.com/brevdev/brev-cli/pkg/cmd/agentskill"
 	"github.com/brevdev/brev-cli/pkg/cmd/register"
 	"github.com/brevdev/brev-cli/pkg/cmd/version"
 	"github.com/brevdev/brev-cli/pkg/store"
-	"github.com/brevdev/brev-cli/pkg/sudo"
 	"github.com/brevdev/brev-cli/pkg/terminal"
 
 	"github.com/spf13/cobra"
@@ -43,7 +43,6 @@ type upgradeDeps struct {
 	detector       Detector
 	upgrader       Upgrader
 	confirmer      terminal.Confirmer
-	gater          sudo.Gater
 	skillInstaller SkillInstaller
 }
 
@@ -52,7 +51,6 @@ func defaultUpgradeDeps() upgradeDeps {
 		detector:       SystemDetector{},
 		upgrader:       SystemUpgrader{},
 		confirmer:      register.TerminalPrompter{},
-		gater:          sudo.Default,
 		skillInstaller: defaultSkillInstaller{},
 	}
 }
@@ -146,11 +144,12 @@ func upgradeViaBrew(t *terminal.Terminal, deps upgradeDeps) (bool, error) {
 
 func upgradeViaDirect(t *terminal.Terminal, deps upgradeDeps) (bool, error) {
 	t.Vprint("Detected install method: direct binary install")
-	t.Vprint("This will download the latest release and install it to /usr/local/bin/brev")
+	t.Vprint("This will download the latest release and install it to ~/.local/bin/brev.")
 	t.Vprint("")
 
-	if err := deps.gater.Gate(t, deps.confirmer, "Upgrade", false); err != nil {
-		return false, fmt.Errorf("sudo issue: %w", err)
+	if !deps.confirmer.ConfirmYesNo("Proceed with upgrade?") {
+		t.Vprint("Upgrade canceled.")
+		return false, nil
 	}
 
 	t.Vprint("")
@@ -160,5 +159,33 @@ func upgradeViaDirect(t *terminal.Terminal, deps upgradeDeps) (bool, error) {
 
 	t.Vprint("")
 	t.Vprint(t.Green("Upgrade complete."))
+
+	if stale := detectStaleSystemInstall(); stale != "" {
+		t.Vprint("")
+		t.Vprintf("  The new brev binary was installed at ~/.local/bin/brev,\n")
+		t.Vprintf("  but an older binary still exists at %s.\n", stale)
+		t.Vprint("  If that path appears earlier in your PATH, your shell will keep running the old version.")
+		t.Vprintf("  Remove it with: sudo rm %s\n", stale)
+	}
 	return true, nil
 }
+
+// detectStaleSystemInstall returns the path of the currently-running binary
+// when it lives in a system bin directory left over from a pre-~/.local/bin
+// install. Returns "" if the running binary is already in ~/.local/bin or in
+// any other non-system location.
+func detectStaleSystemInstall() string {
+	exe, err := os.Executable()
+	if err != nil {
+		return ""
+	}
+	resolved, err := filepath.EvalSymlinks(exe)
+	if err != nil {
+		resolved = exe
+	}
+	switch filepath.Dir(resolved) {
+	case "/usr/local/bin", "/usr/bin":
+		return resolved
+	}
+	return ""
+}

pkg/cmd/upgrade/upgrade_test.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/brevdev/brev-cli/pkg/cmd/version"
 	"github.com/brevdev/brev-cli/pkg/store"
-	"github.com/brevdev/brev-cli/pkg/sudo"
 	"github.com/brevdev/brev-cli/pkg/terminal"
 )
 
@@ -65,7 +64,6 @@ func Test_runUpgrade_AlreadyUpToDate(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodBrew},
 		upgrader:       upgrader,
 		confirmer:      mockConfirmer{confirm: true},
-		gater:          sudo.CachedGater{},
 		skillInstaller: skill,
 	}
 
@@ -94,7 +92,6 @@ func Test_runUpgrade_BrewPath(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodBrew},
 		upgrader:       upgrader,
 		confirmer:      mockConfirmer{confirm: true},
-		gater:          sudo.CachedGater{},
 		skillInstaller: skill,
 	}
 
@@ -126,7 +123,6 @@ func Test_runUpgrade_DirectPath(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodDirect},
 		upgrader:       upgrader,
 		confirmer:      mockConfirmer{confirm: true},
-		gater:          sudo.CachedGater{},
 		skillInstaller: skill,
 	}
 
@@ -158,7 +154,6 @@ func Test_runUpgrade_UserCancels(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodBrew},
 		upgrader:       upgrader,
 		confirmer:      mockConfirmer{confirm: false},
-		gater:          sudo.CachedGater{},
 		skillInstaller: skill,
 	}
 
@@ -181,7 +176,6 @@ func Test_runUpgrade_VersionCheckFails(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodBrew},
 		upgrader:       &mockUpgrader{},
 		confirmer:      mockConfirmer{confirm: true},
-		gater:          sudo.CachedGater{},
 		skillInstaller: &mockSkillInstaller{},
 	}
 
@@ -204,7 +198,6 @@ func Test_runUpgrade_UpgraderFails(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodBrew},
 		upgrader:       upgrader,
 		confirmer:      mockConfirmer{confirm: true},
-		gater:          sudo.CachedGater{},
 		skillInstaller: skill,
 	}
 
@@ -230,7 +223,6 @@ func Test_runUpgrade_SkillInstallFailure_DoesNotFailUpgrade(t *testing.T) {
 		detector:       mockDetector{method: InstallMethodBrew},
 		upgrader:       upgrader,
 		confirmer:      mockConfirmer{confirm: true},
-		gater:          sudo.CachedGater{},
 		skillInstaller: skill,
 	}