Skip to content

fix(install): clear blockers before brew cask install and surface daemon errors - #12

Merged
AnnatarHe merged 1 commit into
mainfrom
fix/brew-install-orphans
Oct 4, 2026
Merged

AnnatarHe merged 1 commit into
mainfrom
fix/brew-install-orphans

Conversation

@AnnatarHe

Copy link
Copy Markdown
Contributor

Problem

On macOS, curl -sSL https://shelltime.xyz/i | bash could fail the Homebrew install and quietly fall back to the manual install, leaving the daemon service on the previous release:

  • An unmanaged regular file at $(brew --prefix)/bin/shelltime-daemon (written by older shelltime update runs) makes the cask's binary link fail with "It seems there is already a Binary at …".
  • Users from before the formula→cask migration still have a shelltime formula keg. tap_migrations.json maps shelltime to the same tap and name, which Homebrew treats as a no-op, so these users are never migrated.
  • shelltime daemon reinstall output went to /dev/null, which hid failures.

Changes (macOS Homebrew branch of install.bash)

  • Run brew update --quiet so a reinstall sees the newest cask despite brew's auto-update throttle.
  • Remove regular (non-symlink) shelltime / shelltime-daemon files from brew's bin dir before installing.
  • If $(brew --cellar)/shelltime exists, brew uninstall --formula shelltime.
  • Install with brew install --cask shelltime/tap/shelltime.
  • Capture shelltime daemon reinstall output and print it to stderr when the command fails.

Companion CLI fix: shelltime/cli fix/daemon-binary-resolution. It makes a curl-installed CLI always use its own daemon and stops shelltime update from writing into the Homebrew prefix.

Testing

  • bash -n install.bash passes.
  • Not yet run end to end: needs a macOS machine with Homebrew. That run should start from a stray /opt/homebrew/bin/shelltime-daemon regular file and the old formula keg installed, and should end with shelltime daemon status reporting the latest version.

🤖 Generated with Claude Code

…mon errors

On macOS the Homebrew path could fail and silently fall back to the
manual install, leaving the daemon service on an older binary:

- Run `brew update` so a reinstall sees the newest cask.
- Remove regular (non-symlink) shelltime/shelltime-daemon files in
  brew's bin dir; the cask refuses to link over them ("It seems there is
  already a Binary") and they shadow the cask.
- Uninstall a leftover pre-cask `shelltime` formula keg, which Homebrew
  can't migrate to the same-named cask in the same tap.
- Install with `--cask` explicitly.
- Print `shelltime daemon reinstall` output when it fails instead of
  discarding it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@claude

claude Bot commented Oct 4, 2026

Copy link
Copy Markdown

Review

Clear, well-commented fix that targets the three failure modes described. bash -n passing is good, but the real behaviour is untested (see Tests).

Looks good

  • Output of shelltime daemon reinstall is captured and sent to stderr only on failure, so errors are no longer hidden.
  • The -f and ! -L guard means Homebrew-managed symlinks are never touched.
  • The script has no set -e, so a failing brew update doesn't abort the install.

Suggestions

  1. Destructive removal happens before the install is known to succeed. If brew install --cask fails, the removed regular file may have been the user's only working binary. The fallback reinstalls to ~/.shelltime/bin, so the risk is low. Moving it to *.bak instead of rm -f would be safer and match the existing rename step. rm failures (for example permissions) are also unchecked, so "Removed..." can print when it wasn't. Consider rm -f ... && echo ....
  2. The result of brew uninstall --formula shelltime is ignored. If it fails, the cask install will fail with a confusing conflict, so a warning would help.
  3. brew list --formula shelltime >/dev/null 2>&1 is a slightly more idiomatic check than testing the Cellar path.
  4. brew update --quiet adds latency to every install. A short "Updating Homebrew..." message would keep the pause from looking like a hang.
  5. bin and daemon_output are globals. This is harmless, but bin is a generic name.

Tests

  • CI runners lack the old-formula and stray-file preconditions, so none of the new branches are covered. Please run the manual macOS scenario from the description before merging. A macOS CI step that creates a dummy regular file at $(brew --prefix)/bin/shelltime-daemon and then runs the installer would give lasting coverage.
  • The companion CLI PR (fix/daemon-binary-resolution) should be released together with this one, since the daemon version outcome depends on both.

No security concerns beyond the rm scope above: paths come from brew --prefix and fixed names.

🤖 Generated with Claude Code

@AnnatarHe
AnnatarHe merged commit 5995479 into main Oct 4, 2026
2 of 7 checks passed
@AnnatarHe
AnnatarHe deleted the fix/brew-install-orphans branch October 4, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant