fix(upgrade): only route brew-managed tools to brew

[fedehertz]

🔗 Linked Issue

Closes #186

---

🏷️ PR Type

• type:bug — Bug fix (non-breaking change that fixes an issue)

---

📝 Summary

• effectiveMethod() routed every tool to brew upgrade <tool> whenever the detected platform package manager was brew. On Linux with Linuxbrew installed for unrelated packages, tools installed via install.sh into ~/.local/bin are not brew formulae, so the upgrade failed with Error: <tap>/<tool> not installed.
• Guards the brew branch with a per-tool isBrewManaged() check: the brew strategy is only chosen when the tool's binary actually lives under a Homebrew prefix; otherwise it falls through to the tool's declared InstallMethod (binary download for gentle-ai/engram, script for gga).
• Platform detection (internal/system/detect.go) is intentionally left unchanged — reporting PackageManager: "brew" is still correct for dependency install hints; the per-tool guard is the minimal, lower-risk fix scoped to the upgrade path.

Note vs. the issue's proposed fix: instead of shelling out to brew list --versions <tool>, this resolves the tool's binary (PATH + symlinks) and matches a /Cellar/ path segment or a known prefix ($HOMEBREW_PREFIX, /opt/homebrew, /home/linuxbrew/.linuxbrew). Same behavior, but offline and without spawning a process.

---

📂 Changes

File / Area	What Changed
internal/update/upgrade/executor.go	Guard brew routing in effectiveMethod with isBrewManaged(); add brewBinaryResolver (testable), pathUnderBrewPrefix, brewPrefixes, containsPathSegment helpers.
internal/update/upgrade/strategy_test.go	Restructure TestEffectiveMethod with a brewManaged axis + regression cases (brew present but tool not brew-managed → declared method); add TestPathUnderBrewPrefix, TestPathUnderBrewPrefixHonorsEnv, TestIsBrewManagedFalseWhenNotOnPath.

---

🧪 Test Plan

Unit Tests

go test ./...

• Unit tests pass (go test ./...)
• E2E tests pass (cd e2e && ./docker-test.sh)
• Manually tested locally

Manual verification on the affected environment (Linux, Linuxbrew at /home/linuxbrew/.linuxbrew, tools in ~/.local/bin): effectiveMethod now resolves gentle-ai → binary, engram → binary, gga → script (previously all brew).

---

✅ Contributor Checklist

• PR is linked to an issue with status:approved
• PR stays within 400 changed lines
• I have added the appropriate type:* label to this PR
• Unit tests pass (go test ./...)
• E2E tests pass (cd e2e && ./docker-test.sh)
• I have updated documentation if necessary (no behavior docs affected)
• My commits follow Conventional Commits format
• My commits do not include Co-Authored-By trailers

---

💬 Notes for Reviewers

I don't have triage permission on this repo, so I couldn't self-apply the type:bug label — please add it so the Check PR Has type:* Label job passes. The brew-managed detection is path-based (Cellar/prefix) rather than brew list; happy to switch to brew list --versions if you'd prefer an authoritative check over an offline one.

Summary by CodeRabbit

• Bug Fixes
  • Fixed tool upgrade routing to accurately determine whether tools are actually installed via Homebrew before using Homebrew update methods. Previously, tools would always use Homebrew for updates if the system's package manager preference was Homebrew, regardless of their actual installation source.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dedbcc4f-1d32-4bff-847c-841fbd559c05

📥 Commits

Reviewing files that changed from the base of the PR and between 8b2e2cf and c9692fe.

📒 Files selected for processing (2)

• internal/update/upgrade/executor.go
• internal/update/upgrade/strategy_test.go

---

📝 Walkthrough

Walkthrough

effectiveMethod in the upgrade executor is changed so Homebrew is selected only when the target tool's binary is detected as Homebrew-managed, rather than whenever the platform package manager is "brew". New internal helpers (brewBinaryResolver, isBrewManaged, pathUnderBrewPrefix, brewPrefixes, containsPathSegment) perform the detection via PATH lookup, symlink resolution, and prefix matching including $HOMEBREW_PREFIX. Tests are extended to cover the conditional routing and path-detection logic.

Changes

Conditional Homebrew Upgrade Routing

Layer / File(s)	Summary
Brew-managed detection helpers and effectiveMethod routing update internal/update/upgrade/executor.go	effectiveMethod now gates InstallBrew selection on isBrewManaged(tool.Name). New helpers resolve the tool binary via LookPath + EvalSymlinks, collect Homebrew prefixes (honoring $HOMEBREW_PREFIX), and classify the resolved path as brew-managed via segment matching. brewBinaryResolver is a package-level variable to allow test overrides.
Test coverage for brew routing and path detection internal/update/upgrade/strategy_test.go	TestEffectiveMethod adds a brewManaged boolean field to table cases and overrides brewBinaryResolver per case. New standalone tests TestPathUnderBrewPrefix, TestPathUnderBrewPrefixHonorsEnv, and TestIsBrewManagedFalseWhenNotOnPath cover Cellar paths, non-brew paths, $HOMEBREW_PREFIX override, and resolver-not-found behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

type:bug

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(upgrade): only route brew-managed tools to brew' directly describes the main change: adding conditional logic to route tools to brew only when they are actually Homebrew-managed.
Linked Issues check	✅ Passed	The PR fulfills all coding requirements from issue #186: it adds per-tool isBrewManaged() checks to prevent routing non-brew tools to brew, implements binary resolution with Homebrew prefix detection, and includes comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue: binary resolution helpers, Homebrew prefix detection, integration into effectiveMethod(), and test expansion remain focused on the brew-managed routing fix.
Docstring Coverage	✅ Passed	Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Alan-TheGentleman]

Thanks for taking this on. I’m closing this in favor of #904 because path-based detection is more likely to misclassify casks, aliases, or non-standard Homebrew prefixes. The lower-risk contract here is to ask Homebrew directly whether it manages the tool. This is a duplicate closure, not a rejection of the bug fix.