Skip to content

fix(daemon): preserve curl-installer daemon as .bak instead of deleting#282

Merged
AnnatarHe merged 1 commit into
mainfrom
claude/fix-linux-cli-install-J6QRV
May 20, 2026
Merged

fix(daemon): preserve curl-installer daemon as .bak instead of deleting#282
AnnatarHe merged 1 commit into
mainfrom
claude/fix-linux-cli-install-J6QRV

Conversation

@AnnatarHe
Copy link
Copy Markdown
Contributor

Summary

  • When commandDaemonInstall resolves to a system-managed binary (Homebrew on macOS, or a stale /usr/local/bin/shelltime-daemon on Linux), the cleanup branch previously deleted ~/.shelltime/bin/shelltime-daemon. Combined with install.bash running shelltime daemon reinstall on upgrades, this removed the binary install.bash had just extracted — and the next daemon install re-downloaded it from GitHub releases.
  • Rename the curl-installer copy to shelltime-daemon.bak instead of removing it. The existing .bak recovery branch at the top of commandDaemonInstall (lines 32-40) restores it on the next run, so once the user clears the stale system binary the daemon binary returns with no network fetch.
  • No changes to install.bash. No changes to model/path.go resolution preference.

Repro / before vs after

Setup (Linux, simulating the stale-system-binary case):

rm -rf ~/.shelltime
touch /usr/local/bin/shelltime-daemon
# install.bash placed a fresh daemon here:
go build -o ~/.shelltime/bin/shelltime-daemon ./cmd/daemon/main.go
shelltime daemon reinstall
  • Before: ~/.shelltime/bin/shelltime-daemon is gone. Next shelltime daemon install hits GitHub releases to re-download.
  • After: ~/.shelltime/bin/shelltime-daemon.bak exists. After the user clears /usr/local/bin/shelltime-daemon and re-runs shelltime daemon install, the existing .bak recovery branch promotes it back to shelltime-daemon — no network fetch.

Test plan

  • go build ./... succeeds
  • go test ./model/... green (path_test.go resolution cases unchanged)
  • Linux: stale /usr/local/bin/shelltime-daemon + fresh ~/.shelltime/bin/shelltime-daemon → after daemon install, .bak exists; after clearing the stale system binary and re-running, .bak is consumed and shelltime-daemon is restored
  • macOS Homebrew: with a leftover curl-installer copy in ~/.shelltime/bin, daemon install renames it to .bak (logged) and service starts against the Homebrew binary
  • Repeated reinstalls don't pile up .bak files (pre-rename os.Remove(preservedPath) clears any stale .bak)

Out of scope

  • The platform-unguarded /usr/local/bin entry in daemonHomebrewSearchPaths (resolution preference unchanged).
  • The service still being registered against the stale /usr/local/bin binary for one reinstall cycle — user still needs to manually clear that. This fix only ensures their good binary survives.

Generated by Claude Code

When `commandDaemonInstall` resolves to a system-managed binary (e.g.
Homebrew, or a stale `/usr/local/bin/shelltime-daemon` on Linux), the
cleanup branch previously deleted `~/.shelltime/bin/shelltime-daemon`
outright. Combined with `install.bash` running `shelltime daemon
reinstall` on upgrades, this caused the freshly-extracted daemon binary
to be removed — and any subsequent `daemon install` would re-download
it from GitHub releases.

Rename the curl-installer copy to `shelltime-daemon.bak` instead. The
existing `.bak` recovery branch at the top of `commandDaemonInstall`
restores it on the next run, so once the user clears the stale system
binary the daemon binary returns with no network fetch needed.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
commands/daemon.install.go 0.00% 5 Missing ⚠️
Flag Coverage Δ
unittests 40.40% <0.00%> (?)

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

Files with missing lines Coverage Δ
commands/daemon.install.go 0.00% <0.00%> (ø)

... and 1 file 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 7b5eecd into main May 20, 2026
2 of 3 checks passed
@AnnatarHe AnnatarHe deleted the claude/fix-linux-cli-install-J6QRV branch May 20, 2026 09:17
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the daemon installation logic to preserve existing curl-installer binaries by renaming them to a backup file instead of deleting them. Feedback was provided regarding a redundant file removal operation before renaming, noting that atomic replacement is already handled by the operating system on Unix-like platforms.

Comment on lines +73 to +74
_ = os.Remove(preservedPath)
if rnErr := os.Rename(curlDaemonPath, preservedPath); rnErr == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

On Unix-like systems (Linux and macOS), os.Rename atomically replaces the destination file if it already exists. The explicit os.Remove(preservedPath) is redundant and introduces a small non-atomic window where the backup file is missing. While harmless in this context, relying on os.Rename's native behavior is more idiomatic and robust.

if rnErr := os.Rename(curlDaemonPath, preservedPath); rnErr == nil {

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.

2 participants