Rename Homebrew cask to basecamp-cli and fix upgrade detection#374
Rename Homebrew cask to basecamp-cli and fix upgrade detection#374
Conversation
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
github-actions
bot
added
commands
There was a problem hiding this comment.
Pull request overview
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
This PR updates distribution naming to the new basecamp-cli Homebrew cask / Scoop manifest and adjusts the CLI’s upgrade flow to detect legacy Homebrew installs and provide migration instructions.
Changes:
- Rename Homebrew cask/Scoop references to
basecamp-cliacross docs and GoReleaser config. - Update
upgradeto detect the renamed cask and handle legacy cask installs with a migration-required message. - Add a test covering legacy cask migration instructions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/commands/upgrade.go | Adds cask constants, legacy-cask detection, and updates Homebrew upgrade invocation to the renamed cask. |
| internal/commands/upgrade_test.go | Extends stubbing to cover legacy-cask detection and adds a migration-instructions test. |
| install.md | Updates manual install commands for the renamed cask/manifest. |
| RELEASING.md | Updates release-process docs to reflect basecamp-cli cask/manifest naming. |
| README.md | Updates install snippets to use the renamed cask/manifest. |
| .goreleaser.yaml | Renames the GoReleaser Homebrew cask and Scoop artifact names and updates install text in the release header. |
Comments suppressed due to low confidence (1)
internal/commands/upgrade.go:149
homebrewHasCaskusesbrew list --cask <cask>and then checks whether the output is non-empty.brew listprints the full list of installed files for the cask, which can be very large and makesupgradeslower than necessary for a simple presence check. Consider switching to a lighter-weight command/output (e.g., a versions/info query) and/or avoiding capturing the full file list when you only need to know whether the cask is installed.
out, err := exec.CommandContext(ctx, "brew", "list", "--cask", cask).CombinedOutput() //nolint:gosec // G204: cask is validated against known constants above
if err != nil {
return false
}
return strings.TrimSpace(string(out)) != ""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremy
left a comment
jeremy
left a comment
There was a problem hiding this comment.
Review: Upgrade detection keyed to package presence, not binary provenance
High: Legacy migration fires on cask presence, not running binary
The legacy migration check at upgrade.go:77 fires based on brew list --cask basecamp/tap/basecamp — whether the legacy cask is installed, not whether the running binary came from it. On a mixed-install machine, basecamp upgrade returns migration_required even when the user runs the new cask or a non-Homebrew binary, and it never reaches the valid upgrade path at line 94.
Fix:
- Swap check order:
homebrewCheckerfirst,legacyHomebrewCaskersecond. If the running binary is the new cask, upgrade it — don't show migration instructions just because the old cask is also installed. - Change
hasLegacyHomebrewCask: Check resolved executable path for the legacy cask's Caskroom directory (/Caskroom/basecamp/) rather than runningbrew list. This ties migration to the running binary's provenance. - Add regression test:
isBrew=true, hasLegacyCask=true→ assert"upgraded"status (notmigration_required).
Medium: Scoop rename has no migration story
.goreleaser.yaml renames the Scoop manifest from basecamp to basecamp-cli, and the docs now tell new users to scoop install basecamp-cli, but there's no equivalent migration guidance for existing scoop install basecamp users. Parallel structure to Homebrew:
| Homebrew | Scoop | |
|---|---|---|
| Path heuristic | /Caskroom/basecamp-cli/ |
\scoop\apps\basecamp-cli\ |
| Subprocess check | brew list --cask basecamp/tap/basecamp-cli |
scoop list basecamp-cli |
| Upgrade command | brew upgrade --cask basecamp/tap/basecamp-cli |
scoop update basecamp-cli |
| Legacy path | /Caskroom/basecamp/ |
\scoop\apps\basecamp\ |
Add:
scoopChecker = isScoop— path check for new scoop app dir, subprocess fallbacklegacyScoopChecker = hasLegacyScoop— path check for\scoop\apps\basecamp\- Scoop upgrade branch:
scoop update basecamp-cli - Scoop migration branch: print
scoop uninstall basecamp && scoop install basecamp-cli
Low: homebrewHasCask buffers unnecessary output
brew list --cask <cask> at upgrade.go:145 dumps the full installed file tree via CombinedOutput(). Only the exit code matters. Replace with Run().
Low: Test assertion too loose
upgrade_test.go:139 — "brew uninstall --cask basecamp/tap/basecamp" substring-matches the new cask name too. Assert with trailing newline to pin to the legacy cask specifically.
Proposed check order in runUpgrade
homebrewChecker→ auto-upgrade via brewscoopChecker→ auto-upgrade via scooplegacyHomebrewCasker→ migration instructions (only reached if running binary is from old cask)legacyScoopChecker→ migration instructions (only reached if running binary is from old scoop app)- Fallback → download URL
Verification
go test ./internal/commands/... passed. bin/ci ran through lint/tests/e2e and failed only at the pre-existing check-smoke-coverage gate (repo-wide uncovered-command list, not change-specific).
The CLI upgrade path confused the desktop app cask with the CLI because and resolve ambiguously.
Update the packaging and upgrade flow to use a renamed CLI cask:
- rename the Homebrew cask from {
"ok": true,
"data": {
"auth": {
"account": "2914079",
"status": "authenticated"
},
"commands": {
"common": [
"basecamp todo \"content\"",
"basecamp done \u003cid\u003e",
"basecamp comment \u003cid\u003e \u003ctext\u003e"
],
"quick_start": [
"basecamp projects list",
"basecamp todos list",
"basecamp search \"query\""
]
},
"context": {},
"version": "0.4.1-0.20260318172421-90913592bc41+dirty"
},
"summary": "basecamp v0.4.1-0.20260318172421-90913592bc41+dirty - logged in @ 2914079"
} to
- rename the Scoop manifest from {
"ok": true,
"data": {
"auth": {
"account": "2914079",
"status": "authenticated"
},
"commands": {
"common": [
"basecamp todo \"content\"",
"basecamp done \u003cid\u003e",
"basecamp comment \u003cid\u003e \u003ctext\u003e"
],
"quick_start": [
"basecamp projects list",
"basecamp todos list",
"basecamp search \"query\""
]
},
"context": {},
"version": "0.4.1-0.20260318172421-90913592bc41+dirty"
},
"summary": "basecamp v0.4.1-0.20260318172421-90913592bc41+dirty - logged in @ 2914079"
} to
- update install/release docs to use
- change upgrade detection to check
- add path detection for cask installs
- change upgrades to use
- detect legacy installs from and print migration steps
This avoids false positives when the desktop app cask is installed and provides a guided migration path for existing CLI users.
Tests:
- go test ./internal/commands/...
- bin/ci (fails at unrelated smoke coverage gate)
Tie Homebrew upgrade behavior to the running binary's provenance instead of installed cask presence. - detect the renamed cask via resolved executable path under /Caskroom/basecamp-cli/ - detect the legacy cask via resolved executable path under /Caskroom/basecamp/ - prefer upgrading the renamed cask before showing legacy migration instructions - restore isUpdateAvailable() handling for older latest-release values - tighten upgrade regression tests for mixed-install and migration cases
Handle the Scoop manifest rename in upgrade detection and messaging. - detect the renamed Scoop app via resolved executable path under /scoop/apps/basecamp-cli/ - detect the legacy Scoop app via resolved executable path under /scoop/apps/basecamp/ - fall back to `scoop list` checks for the renamed and legacy app names - upgrade renamed Scoop installs with `scoop update basecamp-cli` - show migration instructions for legacy `basecamp` Scoop installs - add regression coverage for renamed, legacy, and mixed-install Scoop cases
robzolkos
force-pushed
the
fix/upgrade-homebrew-detection
branch
from
b2e31b3 to
3ccfbd0
Compare
github-actions
bot
added
breaking
Review carefully before merging. Consider a major version bump. |
|
Thanks — addressed all of these. Homebrew
Scoop
Other
Note: the breaking-change impact here is limited to package-manager naming. Existing Homebrew/Scoop users on the legacy package names need to reinstall using Validation:
|
Mirror the Homebrew fix for Scoop: decide upgrade and migration behavior from the resolved executable path, not from whether a Scoop package is installed somewhere on the machine. Using `scoop list` as a fallback can misclassify mixed-install setups and upgrade or migrate a dormant Scoop install instead of the binary the user actually invoked. Keeping both Homebrew and Scoop keyed to binary provenance avoids those false positives. Add focused tests for renamed and legacy Scoop path detection.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When the CLI is invoked via Scoop, the executable on PATH is usually the shim under /scoop/shims rather than the app path under /scoop/apps. Teach upgrade detection to treat the shim as Scoop provenance and resolve renamed vs legacy installs via `scoop prefix`. Also refactor the upgrade test stubs to use a named options struct so the mixed Homebrew/Scoop cases stay readable and harder to mis-wire.
|
Fixed — kept upgrade decisions tied to the running binary’s provenance, added Scoop migration handling, then tightened Scoop detection further so shim-based installs resolve through |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/commands/upgrade.go">
<violation number="1" location="internal/commands/upgrade.go:263">
P2: Use a rooted path check for global Scoop detection. `strings.Contains` can misclassify local paths and cause `upgrade`/migration commands to incorrectly add `-g`.
(Based on your team's feedback about path directory validation with separator-safe checks.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Fixed — added focused Homebrew provenance tests, tightened global Scoop detection to use a rooted path-prefix check, and kept the Scoop global/local upgrade paths covered by tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Ref: https://3.basecamp.com/2914079/buckets/46292715/card_tables/cards/9714497196
Testing
Summary by cubic
Renamed the Homebrew cask and Scoop manifest to
basecamp-cliand tied upgrades to the running binary’s path (including Scoop shims and global vs user installs). Resolves executable paths to avoid false positives with the desktop app; docs and release config updated.Bug Fixes
/caskroom/basecamp-cli/; upgrade withbrew upgrade --cask basecamp/tap/basecamp-cli; prefer upgrading the renamed cask before showing migration steps./scoop/apps/basecamp-cli/or shims under/scoop/shims/; resolve renamed vs legacy viascoop prefixand match shim scope; detect global installs (/programdata/scoop/) and pass-g; upgrade withscoop update basecamp-cli; prefer upgrading the renamed app before showing migration steps.Migration
basecamp/tap/basecamp): runbrew uninstall --cask basecamp/tap/basecamp && brew install --cask basecamp/tap/basecamp-cli.basecamp): runscoop uninstall basecamp && scoop install basecamp-cli(use-gwhen global).Written for commit be0d3ae. Summary will update on new commits.