diff --git a/internal/update/upgrade/executor.go b/internal/update/upgrade/executor.go index d9583d47d..11b273811 100644 --- a/internal/update/upgrade/executor.go +++ b/internal/update/upgrade/executor.go @@ -603,7 +603,11 @@ func executeOne(ctx context.Context, r update.UpdateResult, profile system.Platf // Priority order matches the documented install hierarchy: plugin → brew → Windows installer → go-install → declared method. // // 1. OpenCode plugins are always handled by their own method — never overridden. -// 2. Brew-managed platforms always use brew regardless of the tool's declared method. +// 2. Brew is used ONLY when the tool's binary is actually brew-managed. A system +// can have Homebrew present (PackageManager == "brew") while a tool was +// installed via install.sh into ~/.local/bin; routing such a tool to brew +// fails with "Error: / not installed". When the tool is not +// brew-managed we fall through to its real install method. // 3. gentle-ai on Windows uses the installer so the running binary can exit before replacement. // 4. When Go is available on PATH and the tool has a GoImportPath, go-install is // preferred over a direct binary download. @@ -612,7 +616,7 @@ func effectiveMethod(tool update.ToolInfo, profile system.PlatformProfile) updat if tool.InstallMethod == update.InstallOpenCodePlugin { return update.InstallOpenCodePlugin } - if profile.PackageManager == "brew" { + if profile.PackageManager == "brew" && isBrewManaged(tool.Name) { return update.InstallBrew } // Use installer method for gentle-ai on Windows (launches PowerShell installer). @@ -624,3 +628,73 @@ func effectiveMethod(tool update.ToolInfo, profile system.PlatformProfile) updat } return tool.InstallMethod } + +// brewBinaryResolver resolves a tool's installed executable to an absolute, +// symlink-resolved path. The bool is false when the tool is not found on PATH. +// Package-level var for testability — swapped in tests to simulate brew-managed +// vs install.sh-managed binaries without touching the real filesystem. +var brewBinaryResolver = func(toolName string) (string, bool) { + path, err := lookPathCommand(toolName) + if err != nil || strings.TrimSpace(path) == "" { + return "", false + } + if resolved, err := filepath.EvalSymlinks(path); err == nil && strings.TrimSpace(resolved) != "" { + return resolved, true + } + return path, true +} + +// isBrewManaged reports whether toolName resolves to a binary that lives under a +// Homebrew prefix. This is the guard that distinguishes "Homebrew exists on the +// system" from "this tool was actually installed by Homebrew". +func isBrewManaged(toolName string) bool { + path, ok := brewBinaryResolver(toolName) + if !ok { + return false + } + return pathUnderBrewPrefix(path) +} + +// pathUnderBrewPrefix reports whether an absolute binary path belongs to a +// Homebrew installation. Homebrew links binaries from /bin into +// /Cellar///bin, so a symlink-resolved path contains a +// "Cellar" segment. It also matches well-known prefixes and $HOMEBREW_PREFIX. +func pathUnderBrewPrefix(path string) bool { + path = filepath.Clean(path) + if containsPathSegment(path, "Cellar") { + return true + } + for _, prefix := range brewPrefixes() { + prefix = strings.TrimSpace(prefix) + if prefix == "" { + continue + } + prefix = filepath.Clean(prefix) + if path == prefix || strings.HasPrefix(path, prefix+string(os.PathSeparator)) { + return true + } + } + return false +} + +// brewPrefixes returns candidate Homebrew prefixes. $HOMEBREW_PREFIX wins when +// set; the defaults cover Apple Silicon and Linuxbrew. Intel macOS uses +// /usr/local, which is too generic to list — those installs are matched by the +// Cellar segment check in pathUnderBrewPrefix instead. +func brewPrefixes() []string { + prefixes := make([]string, 0, 3) + if hp := strings.TrimSpace(os.Getenv("HOMEBREW_PREFIX")); hp != "" { + prefixes = append(prefixes, hp) + } + return append(prefixes, "/opt/homebrew", "/home/linuxbrew/.linuxbrew") +} + +// containsPathSegment reports whether seg appears as a full path component. +func containsPathSegment(path, seg string) bool { + for _, part := range strings.Split(filepath.ToSlash(path), "/") { + if part == seg { + return true + } + } + return false +} diff --git a/internal/update/upgrade/strategy_test.go b/internal/update/upgrade/strategy_test.go index 77d5b5ec6..809e2a600 100644 --- a/internal/update/upgrade/strategy_test.go +++ b/internal/update/upgrade/strategy_test.go @@ -270,28 +270,56 @@ func TestEffectiveMethod_NonGentleAIToolsOnWindowsUseBinary(t *testing.T) { func TestEffectiveMethod(t *testing.T) { tests := []struct { - name string - tool update.ToolInfo - profile system.PlatformProfile - want update.InstallMethod + name string + tool update.ToolInfo + profile system.PlatformProfile + brewManaged bool // simulated: is the tool's binary under a brew prefix? + want update.InstallMethod }{ { - name: "brew profile overrides go-install", - tool: update.ToolInfo{Name: "engram", InstallMethod: update.InstallGoInstall}, - profile: system.PlatformProfile{PackageManager: "brew"}, - want: update.InstallBrew, + name: "brew-managed tool overrides go-install", + tool: update.ToolInfo{Name: "engram", InstallMethod: update.InstallGoInstall}, + profile: system.PlatformProfile{PackageManager: "brew"}, + brewManaged: true, + want: update.InstallBrew, }, { - name: "brew profile overrides binary", - tool: update.ToolInfo{Name: "gga", InstallMethod: update.InstallBinary}, - profile: system.PlatformProfile{PackageManager: "brew"}, - want: update.InstallBrew, + name: "brew-managed tool overrides binary", + tool: update.ToolInfo{Name: "gga", InstallMethod: update.InstallBinary}, + profile: system.PlatformProfile{PackageManager: "brew"}, + brewManaged: true, + want: update.InstallBrew, }, { - name: "brew profile overrides script", - tool: update.ToolInfo{Name: "gga", InstallMethod: update.InstallScript}, - profile: system.PlatformProfile{PackageManager: "brew"}, - want: update.InstallBrew, + name: "brew-managed tool overrides script", + tool: update.ToolInfo{Name: "gga", InstallMethod: update.InstallScript}, + profile: system.PlatformProfile{PackageManager: "brew"}, + brewManaged: true, + want: update.InstallBrew, + }, + // Regression: Homebrew is present on the system but the tool was installed + // via install.sh into ~/.local/bin. Routing to brew would fail with + // "Error: / not installed"; fall through to the real method. + { + name: "brew present but tool not brew-managed → declared binary", + tool: update.ToolInfo{Name: "gentle-ai", InstallMethod: update.InstallBinary}, + profile: system.PlatformProfile{PackageManager: "brew"}, + brewManaged: false, + want: update.InstallBinary, + }, + { + name: "brew present but tool not brew-managed → declared script", + tool: update.ToolInfo{Name: "gga", InstallMethod: update.InstallScript}, + profile: system.PlatformProfile{PackageManager: "brew"}, + brewManaged: false, + want: update.InstallScript, + }, + { + name: "brew present but tool not brew-managed → go-install when go available", + tool: update.ToolInfo{Name: "engram", InstallMethod: update.InstallBinary, GoImportPath: "github.com/Gentleman-Programming/engram/cmd/engram"}, + profile: system.PlatformProfile{PackageManager: "brew", GoAvailable: true}, + brewManaged: false, + want: update.InstallGoInstall, }, { name: "apt profile respects declared method (go-install)", @@ -312,17 +340,19 @@ func TestEffectiveMethod(t *testing.T) { want: update.InstallScript, }, { - name: "brew profile does not override OpenCode plugin method", - tool: update.ToolInfo{Name: "opencode-subagent-statusline", InstallMethod: update.InstallOpenCodePlugin, NpmPackage: "opencode-subagent-statusline"}, - profile: system.PlatformProfile{PackageManager: "brew"}, - want: update.InstallOpenCodePlugin, + name: "brew profile does not override OpenCode plugin method", + tool: update.ToolInfo{Name: "opencode-subagent-statusline", InstallMethod: update.InstallOpenCodePlugin, NpmPackage: "opencode-subagent-statusline"}, + profile: system.PlatformProfile{PackageManager: "brew"}, + brewManaged: true, + want: update.InstallOpenCodePlugin, }, // Auto-detect order: brew → go-install → binary (issue #246). { - name: "auto-detect: brew available → brew wins regardless of GoImportPath", - tool: update.ToolInfo{Name: "mytool", InstallMethod: update.InstallBinary, GoImportPath: "github.com/example/mytool/cmd/mytool"}, - profile: system.PlatformProfile{PackageManager: "brew", GoAvailable: true}, - want: update.InstallBrew, + name: "auto-detect: brew-managed → brew wins regardless of GoImportPath", + tool: update.ToolInfo{Name: "mytool", InstallMethod: update.InstallBinary, GoImportPath: "github.com/example/mytool/cmd/mytool"}, + profile: system.PlatformProfile{PackageManager: "brew", GoAvailable: true}, + brewManaged: true, + want: update.InstallBrew, }, { name: "auto-detect: brew missing + go available + GoImportPath set → go-install", @@ -344,8 +374,20 @@ func TestEffectiveMethod(t *testing.T) { }, } + origResolver := brewBinaryResolver + t.Cleanup(func() { brewBinaryResolver = origResolver }) + for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + if tc.brewManaged { + brewBinaryResolver = func(string) (string, bool) { + return "/home/linuxbrew/.linuxbrew/Cellar/" + tc.tool.Name + "/1.0.0/bin/" + tc.tool.Name, true + } + } else { + brewBinaryResolver = func(string) (string, bool) { + return "/home/user/.local/bin/" + tc.tool.Name, true + } + } got := effectiveMethod(tc.tool, tc.profile) if got != tc.want { t.Errorf("effectiveMethod = %q, want %q", got, tc.want) @@ -354,6 +396,55 @@ func TestEffectiveMethod(t *testing.T) { } } +// --- TestPathUnderBrewPrefix --- + +func TestPathUnderBrewPrefix(t *testing.T) { + // Clear $HOMEBREW_PREFIX so the test is deterministic regardless of the host. + t.Setenv("HOMEBREW_PREFIX", "") + + tests := []struct { + name string + path string + want bool + }{ + {name: "linuxbrew Cellar symlink target", path: "/home/linuxbrew/.linuxbrew/Cellar/engram/1.0.0/bin/engram", want: true}, + {name: "apple silicon prefix bin", path: "/opt/homebrew/bin/gentle-ai", want: true}, + {name: "intel mac Cellar", path: "/usr/local/Cellar/gga/2.8.0/bin/gga", want: true}, + {name: "linuxbrew prefix bin", path: "/home/linuxbrew/.linuxbrew/bin/engram", want: true}, + {name: "install.sh local bin is not brew", path: "/home/user/.local/bin/gentle-ai", want: false}, + {name: "usr local bin without Cellar is not brew", path: "/usr/local/bin/engram", want: false}, + {name: "go bin is not brew", path: "/home/user/go/bin/engram", want: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := pathUnderBrewPrefix(tc.path); got != tc.want { + t.Errorf("pathUnderBrewPrefix(%q) = %v, want %v", tc.path, got, tc.want) + } + }) + } +} + +// --- TestPathUnderBrewPrefixHonorsEnv --- + +func TestPathUnderBrewPrefixHonorsEnv(t *testing.T) { + t.Setenv("HOMEBREW_PREFIX", "/custom/brew") + if !pathUnderBrewPrefix("/custom/brew/bin/engram") { + t.Errorf("expected path under $HOMEBREW_PREFIX to be detected as brew-managed") + } +} + +// --- TestIsBrewManagedFalseWhenNotOnPath --- + +func TestIsBrewManagedFalseWhenNotOnPath(t *testing.T) { + origResolver := brewBinaryResolver + t.Cleanup(func() { brewBinaryResolver = origResolver }) + brewBinaryResolver = func(string) (string, bool) { return "", false } + if isBrewManaged("engram") { + t.Errorf("isBrewManaged should be false when the tool is not on PATH") + } +} + func TestRunStrategyOpenCodePluginManualFallback(t *testing.T) { origHomeDir := openCodeHomeDir origLookPath := lookPathCommand