diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 740ed40d..387b5c27 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -98,18 +98,24 @@ func isOldTapError(stderr string) bool { return strings.Contains(stderr, "brew tap kernel/tap") } -// upgradeCommandArgs returns the command and arguments for a given installation method. -// Returns nil if the method is unknown. -func upgradeCommandArgs(method update.InstallMethod) []string { +// upgradeCommandArgs returns the ordered commands to run for a given installation +// method. Returns nil if the method is unknown. +func upgradeCommandArgs(method update.InstallMethod) [][]string { switch method { case update.InstallMethodBrew: - return []string{"brew", "upgrade", "kernel/tap/kernel"} + // brew upgrade doesn't refresh the tap first, so a freshly released + // formula is invisible and the upgrade silently no-ops ("already + // installed"). Update the tap before upgrading. + return [][]string{ + {"brew", "update"}, + {"brew", "upgrade", "kernel/tap/kernel"}, + } case update.InstallMethodPNPM: - return []string{"pnpm", "add", "-g", "@onkernel/cli@latest"} + return [][]string{{"pnpm", "add", "-g", "@onkernel/cli@latest"}} case update.InstallMethodNPM: - return []string{"npm", "i", "-g", "@onkernel/cli@latest"} + return [][]string{{"npm", "i", "-g", "@onkernel/cli@latest"}} case update.InstallMethodBun: - return []string{"bun", "add", "-g", "@onkernel/cli@latest"} + return [][]string{{"bun", "add", "-g", "@onkernel/cli@latest"}} default: return nil } @@ -117,31 +123,37 @@ func upgradeCommandArgs(method update.InstallMethod) []string { // getUpgradeCommand returns the command string for display (e.g., dry-run output). func getUpgradeCommand(method update.InstallMethod) string { - args := upgradeCommandArgs(method) - if args == nil { + cmds := upgradeCommandArgs(method) + if cmds == nil { return "" } - return strings.Join(args, " ") + joined := make([]string, len(cmds)) + for i, args := range cmds { + joined[i] = strings.Join(args, " ") + } + return strings.Join(joined, " && ") } -// executeUpgrade runs the appropriate upgrade command based on the installation method. -// Returns the captured stderr (for error diagnosis) and any error. +// executeUpgrade runs the appropriate upgrade commands based on the installation +// method. Returns the captured stderr (for error diagnosis) and any error. func executeUpgrade(method update.InstallMethod) (stderr string, err error) { - args := upgradeCommandArgs(method) - if args == nil { + cmds := upgradeCommandArgs(method) + if cmds == nil { return "", fmt.Errorf("unknown installation method") } - cmd := exec.Command(args[0], args[1:]...) - cmd.Stdout = os.Stdout - cmd.Stdin = os.Stdin - - // Capture stderr while also displaying it to the user var stderrBuf bytes.Buffer - cmd.Stderr = io.MultiWriter(os.Stderr, &stderrBuf) - - err = cmd.Run() - return stderrBuf.String(), err + for _, args := range cmds { + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdout = os.Stdout + cmd.Stdin = os.Stdin + // Capture stderr while also displaying it to the user + cmd.Stderr = io.MultiWriter(os.Stderr, &stderrBuf) + if err = cmd.Run(); err != nil { + return stderrBuf.String(), err + } + } + return stderrBuf.String(), nil } // printManualUpgradeInstructions prints instructions for manually upgrading kernel diff --git a/cmd/upgrade_test.go b/cmd/upgrade_test.go new file mode 100644 index 00000000..1fd6a3d7 --- /dev/null +++ b/cmd/upgrade_test.go @@ -0,0 +1,54 @@ +package cmd + +import ( + "testing" + + "github.com/kernel/cli/pkg/update" + "github.com/stretchr/testify/assert" +) + +func TestUpgradeCommandArgs(t *testing.T) { + tests := []struct { + method update.InstallMethod + expected [][]string + }{ + { + // brew upgrade no-ops on a stale tap, so update must run first. + update.InstallMethodBrew, + [][]string{ + {"brew", "update"}, + {"brew", "upgrade", "kernel/tap/kernel"}, + }, + }, + {update.InstallMethodNPM, [][]string{{"npm", "i", "-g", "@onkernel/cli@latest"}}}, + {update.InstallMethodPNPM, [][]string{{"pnpm", "add", "-g", "@onkernel/cli@latest"}}}, + {update.InstallMethodBun, [][]string{{"bun", "add", "-g", "@onkernel/cli@latest"}}}, + {update.InstallMethodUnknown, nil}, + } + for _, tt := range tests { + t.Run(string(tt.method), func(t *testing.T) { + assert.Equal(t, tt.expected, upgradeCommandArgs(tt.method)) + }) + } +} + +func TestUpgradeCommandArgsBrewUpdatesBeforeUpgrade(t *testing.T) { + cmds := upgradeCommandArgs(update.InstallMethodBrew) + updateIdx, upgradeIdx := -1, -1 + for i, args := range cmds { + if len(args) >= 2 && args[0] == "brew" && args[1] == "update" { + updateIdx = i + } + if len(args) >= 2 && args[0] == "brew" && args[1] == "upgrade" { + upgradeIdx = i + } + } + assert.NotEqual(t, -1, updateIdx, "expected a brew update step") + assert.NotEqual(t, -1, upgradeIdx, "expected a brew upgrade step") + assert.Less(t, updateIdx, upgradeIdx, "brew update must run before brew upgrade") +} + +func TestGetUpgradeCommandBrew(t *testing.T) { + assert.Equal(t, "brew update && brew upgrade kernel/tap/kernel", getUpgradeCommand(update.InstallMethodBrew)) + assert.Equal(t, "", getUpgradeCommand(update.InstallMethodUnknown)) +} diff --git a/pkg/update/check.go b/pkg/update/check.go index 59f86ef3..3529aa3b 100644 --- a/pkg/update/check.go +++ b/pkg/update/check.go @@ -434,7 +434,8 @@ func SuggestUpgradeCommand() string { func suggestUpgradeCommandForMethod(method InstallMethod) string { switch method { case InstallMethodBrew: - return "brew upgrade kernel/tap/kernel" + // brew upgrade alone can no-op on a stale tap, so refresh it first. + return "brew update && brew upgrade kernel/tap/kernel" case InstallMethodPNPM: return "pnpm add -g @onkernel/cli@latest" case InstallMethodNPM: @@ -442,7 +443,7 @@ func suggestUpgradeCommandForMethod(method InstallMethod) string { case InstallMethodBun: return "bun add -g @onkernel/cli@latest" default: - return "brew upgrade kernel/tap/kernel" + return "brew update && brew upgrade kernel/tap/kernel" } } diff --git a/pkg/update/check_test.go b/pkg/update/check_test.go index 55ccb354..2a6d0920 100644 --- a/pkg/update/check_test.go +++ b/pkg/update/check_test.go @@ -11,11 +11,11 @@ func TestSuggestUpgradeCommandForMethod(t *testing.T) { method InstallMethod expected string }{ - {InstallMethodBrew, "brew upgrade kernel/tap/kernel"}, + {InstallMethodBrew, "brew update && brew upgrade kernel/tap/kernel"}, {InstallMethodNPM, "npm i -g @onkernel/cli@latest"}, {InstallMethodPNPM, "pnpm add -g @onkernel/cli@latest"}, {InstallMethodBun, "bun add -g @onkernel/cli@latest"}, - {InstallMethodUnknown, "brew upgrade kernel/tap/kernel"}, + {InstallMethodUnknown, "brew update && brew upgrade kernel/tap/kernel"}, } for _, tt := range tests { t.Run(string(tt.method), func(t *testing.T) {