Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 76 additions & 2 deletions internal/update/upgrade/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: <tap>/<tool> 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.
Expand All @@ -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).
Expand All @@ -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 <prefix>/bin into
// <prefix>/Cellar/<formula>/<version>/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
}
139 changes: 115 additions & 24 deletions internal/update/upgrade/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: <tap>/<tool> 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)",
Expand All @@ -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",
Expand All @@ -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)
Expand All @@ -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
Expand Down