Skip to content

fix(daemon): keep daemon binary in lockstep with curl-installed CLI - #308

Merged
AnnatarHe merged 1 commit into
mainfrom
fix/daemon-binary-resolution
Oct 4, 2026
Merged

AnnatarHe merged 1 commit into
mainfrom
fix/daemon-binary-resolution

Conversation

@AnnatarHe

Copy link
Copy Markdown
Contributor

Problem

Reinstalling with curl -sSL https://shelltime.xyz/i | bash updated the CLI to 0.1.90, but shelltime daemon status still reported 0.1.89.

The cause, as diagnosed on an affected machine:

  1. /opt/homebrew/bin/shelltime-daemon was a regular file (0.1.89) that Homebrew didn't own. The only code path that writes a daemon there is shelltime update → resolveDaemonDest() → ResolveDaemonBinaryPath(), which ReplaceBinarys over the cask symlink.
  2. Because of that file, the installer's brew install failed ("It seems there is already a Binary at …"), so it fell back to the manual install and put 0.1.90 in ~/.shelltime/bin/.
  3. shelltime daemon reinstall then resolved the daemon to the stale /opt/homebrew/bin copy (PATH/Homebrew is preferred over the curl location). It renamed the fresh 0.1.90 daemon to shelltime-daemon.bak and pointed launchd at 0.1.89.
  4. On the next curl reinstall, daemon install would also have restored that .bak over the newly installed daemon unconditionally.

Changes

  • model/path.go ResolveDaemonBinaryPath: if the running CLI is the curl-installer copy (DetectInstallKind == InstallKindCurl) and ~/.shelltime/bin/shelltime-daemon exists, return it first. Homebrew CLIs keep the existing order, which returns the stable /opt/homebrew/bin path. The CLI path is read through a swappable currentCLIBinaryPath var for tests.
  • commands/update.go resolveDaemonDest: always returns GetCurlInstallerDaemonPath(). update only proceeds for curl installs, so it must never write into a Homebrew prefix.
  • commands/daemon.install.go: the .bak restore moves into restoreCurlDaemonBak and only runs when the live curl daemon is missing.

The companion installer fix is in shelltime/installation (fix/brew-install-orphans).

Tests

  • model/path_test.go: a curl CLI prefers its sibling daemon over PATH/Homebrew, and falls back to Homebrew when its daemon is missing. Existing cases are unchanged; the isolation helper now pins a neutral running CLI.
  • commands/daemon.install_test.go: table test for restoreCurlDaemonBak.
  • commands/update_test.go: resolveDaemonDest returns the curl path even when a daemon is on PATH.

The new tests pass and go vet is clean. go test ./commands/ still has 10 local failures (socket and bolt tests); the same 10 fail on main.

🤖 Generated with Claude Code

A curl reinstall left the daemon service on the previous release:
ResolveDaemonBinaryPath preferred an unmanaged shelltime-daemon in
/opt/homebrew/bin over the fresh one next to the curl CLI, and daemon
install then moved the fresh binary aside to .bak.

- ResolveDaemonBinaryPath: when the running CLI is the curl-installer
  copy, return its sibling daemon first.
- shelltime update: always write the daemon to ~/.shelltime/bin instead
  of whatever path resolves, so it never overwrites a Homebrew cask
  symlink with an unmanaged file (which also blocks later brew installs).
- daemon install: restore shelltime-daemon.bak only when the live daemon
  is missing, instead of clobbering a newly installed release.

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
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@codecov

codecov Bot commented Oct 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 61.11111% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
commands/daemon.install.go 50.00% 6 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (61.11%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Flag Coverage Δ
unittests 79.54% <61.11%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
commands/update.go 2.00% <100.00%> (+2.00%) ⬆️
model/path.go 95.94% <100.00%> (+0.17%) ⬆️
commands/daemon.install.go 14.75% <50.00%> (+10.98%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AnnatarHe
AnnatarHe merged commit 8291ec5 into main Oct 4, 2026
3 of 4 checks passed
@AnnatarHe
AnnatarHe deleted the fix/daemon-binary-resolution branch October 4, 2026 16:29
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