diff --git a/commands/daemon.install.go b/commands/daemon.install.go index c84395c..a00f818 100644 --- a/commands/daemon.install.go +++ b/commands/daemon.install.go @@ -29,14 +29,13 @@ func commandDaemonInstall(c *cli.Context) error { baseFolder := filepath.Join(currentUser.HomeDir, ".shelltime") username := currentUser.Username - // Handle .bak upgrade for curl-installer users - bakPath := filepath.Join(baseFolder, "bin/shelltime-daemon.bak") - if _, err := os.Stat(bakPath); err == nil { - color.Yellow.Println("🔄 Found latest daemon file, restoring...") - _ = os.Remove(filepath.Join(baseFolder, "bin/shelltime-daemon")) - if err := os.Rename(bakPath, filepath.Join(baseFolder, "bin/shelltime-daemon")); err != nil { - return fmt.Errorf("failed to restore latest daemon: %w", err) - } + // Bring back a curl-installer daemon preserved as .bak by a previous install + restored, err := restoreCurlDaemonBak(filepath.Join(baseFolder, "bin", "shelltime-daemon")) + if err != nil { + return err + } + if restored { + color.Yellow.Println("🔄 Restored preserved curl-installer daemon from .bak") } // Resolve daemon binary (Homebrew/PATH preferred, curl-installer fallback). @@ -106,6 +105,24 @@ func commandDaemonInstall(c *cli.Context) error { return nil } +// restoreCurlDaemonBak moves curlDaemonPath+".bak" back to curlDaemonPath, but +// only when curlDaemonPath itself is missing. A live daemon there is whatever +// the installer or `shelltime update` just put down; overwriting it with an +// older .bak is how a reinstall ended up running the previous release. +func restoreCurlDaemonBak(curlDaemonPath string) (bool, error) { + bakPath := curlDaemonPath + ".bak" + if _, err := os.Stat(bakPath); err != nil { + return false, nil + } + if _, err := os.Stat(curlDaemonPath); err == nil { + return false, nil + } + if err := os.Rename(bakPath, curlDaemonPath); err != nil { + return false, fmt.Errorf("failed to restore daemon from %s: %w", bakPath, err) + } + return true, nil +} + // shouldPreserveCurlDaemon reports whether the curl-installer daemon at // curlDaemonPath should be renamed to .bak before installing the service. It is // true only when the resolved daemon is a genuinely different on-disk file — so diff --git a/commands/daemon.install_test.go b/commands/daemon.install_test.go index a0212ac..e5ff9a5 100644 --- a/commands/daemon.install_test.go +++ b/commands/daemon.install_test.go @@ -51,3 +51,63 @@ func TestShouldPreserveCurlDaemon(t *testing.T) { }) } } + +// TestRestoreCurlDaemonBak pins that a preserved .bak only comes back when the +// live curl daemon is gone. Restoring it over a live daemon replaced a freshly +// installed release with the previous one on every curl reinstall. +func TestRestoreCurlDaemonBak(t *testing.T) { + tests := []struct { + name string + live, bak string // file contents; "" means the file does not exist + wantRestored bool + wantLive string + wantBak string + }{ + {"no bak, no live", "", "", false, "", ""}, + {"bak only is restored", "", "old", true, "old", ""}, + {"live wins over bak", "new", "old", false, "new", "old"}, + {"live only is untouched", "new", "", false, "new", ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + live := filepath.Join(t.TempDir(), "shelltime-daemon") + bak := live + ".bak" + if tc.live != "" { + if err := os.WriteFile(live, []byte(tc.live), 0o755); err != nil { + t.Fatal(err) + } + } + if tc.bak != "" { + if err := os.WriteFile(bak, []byte(tc.bak), 0o755); err != nil { + t.Fatal(err) + } + } + + restored, err := restoreCurlDaemonBak(live) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if restored != tc.wantRestored { + t.Errorf("restored = %v, want %v", restored, tc.wantRestored) + } + if got := readOrEmpty(t, live); got != tc.wantLive { + t.Errorf("live contents = %q, want %q", got, tc.wantLive) + } + if got := readOrEmpty(t, bak); got != tc.wantBak { + t.Errorf(".bak contents = %q, want %q", got, tc.wantBak) + } + }) + } +} + +func readOrEmpty(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if os.IsNotExist(err) { + return "" + } + if err != nil { + t.Fatal(err) + } + return string(b) +} diff --git a/commands/update.go b/commands/update.go index 4048c31..eda1e60 100644 --- a/commands/update.go +++ b/commands/update.go @@ -161,13 +161,12 @@ func commandUpdate(c *cli.Context) error { return nil } -// resolveDaemonDest returns the path the daemon binary should be written to — -// the existing daemon location if installed, otherwise the curl-installer default. +// resolveDaemonDest returns the path the daemon binary should be written to. +// Update only runs for curl-installer CLIs, so the daemon always goes next to +// the CLI — never into a Homebrew prefix, where an unmanaged copy blocks +// `brew install` of the cask and shadows the curl daemon. func resolveDaemonDest() string { - if p, err := model.ResolveDaemonBinaryPath(); err == nil { - return p - } - return filepath.Join(model.GetBinFolderPath(), "shelltime-daemon") + return model.GetCurlInstallerDaemonPath() } // shouldReinstallDaemon decides whether to call commandDaemonReinstall after a diff --git a/commands/update_test.go b/commands/update_test.go new file mode 100644 index 0000000..6961549 --- /dev/null +++ b/commands/update_test.go @@ -0,0 +1,29 @@ +package commands + +import ( + "os" + "path/filepath" + "testing" + + "github.com/malamtime/cli/model" +) + +// TestResolveDaemonDest pins that `shelltime update` writes the daemon next to +// the curl CLI even when another daemon is on PATH. Writing over a Homebrew +// cask symlink left an unmanaged binary that blocked later `brew install`s and +// kept the daemon service on an old release. +func TestResolveDaemonDest(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + brewDir := t.TempDir() + if err := os.WriteFile(filepath.Join(brewDir, "shelltime-daemon"), []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", brewDir) + + want := model.GetCurlInstallerDaemonPath() + if got := resolveDaemonDest(); got != want { + t.Errorf("resolveDaemonDest() = %s, want %s", got, want) + } +} diff --git a/model/path.go b/model/path.go index 8463bb4..8ed22d6 100644 --- a/model/path.go +++ b/model/path.go @@ -134,14 +134,29 @@ var daemonHomebrewSearchPaths = []string{ "/home/linuxbrew/.linuxbrew/bin", } +// currentCLIBinaryPath resolves the running CLI binary. Exposed as a var so +// tests can pretend to be a curl-installer or Homebrew CLI. +var currentCLIBinaryPath = ResolveCLIBinaryPath + // ResolveDaemonBinaryPath finds the shelltime-daemon binary. -// It prefers a system-managed binary (Homebrew or anything on PATH) over the -// legacy curl-installer location, so that `brew upgrade shelltime` is what -// actually drives the running daemon. +// When the running CLI is the curl-installer copy, the daemon next to it wins, +// so a stale or unmanaged daemon in a Homebrew dir can't shadow the one that +// was just installed alongside the CLI. Otherwise it prefers a system-managed +// binary (Homebrew or anything on PATH) over the legacy curl-installer +// location, so that `brew upgrade shelltime` is what actually drives the +// running daemon. func ResolveDaemonBinaryPath() (string, error) { const binaryName = "shelltime-daemon" curlPath := GetCurlInstallerDaemonPath() + // 0. Curl-installer CLI: use its sibling daemon so CLI and daemon stay in + // lockstep across `curl … | bash` reinstalls and `shelltime update`. + if cliPath, err := currentCLIBinaryPath(); err == nil && DetectInstallKind(cliPath) == InstallKindCurl { + if info, err := os.Stat(curlPath); err == nil && !info.IsDir() { + return curlPath, nil + } + } + // 1. Check PATH (covers Homebrew and other package managers). Ignore the // result if it is the same on-disk file as the curl-installer binary // (symlink/alias) — step 3 is the only branch that may return that file. diff --git a/model/path_test.go b/model/path_test.go index 1bb93c8..182246b 100644 --- a/model/path_test.go +++ b/model/path_test.go @@ -319,9 +319,22 @@ func withIsolatedDaemonResolution(t *testing.T) string { daemonHomebrewSearchPaths = nil t.Cleanup(func() { daemonHomebrewSearchPaths = prev }) + // Default to a CLI that is neither curl- nor Homebrew-installed; tests that + // care about the running install kind override it via withRunningCLI. + withRunningCLI(t, filepath.Join(t.TempDir(), "shelltime")) + return home } +// withRunningCLI makes ResolveDaemonBinaryPath believe the running CLI binary +// lives at cliPath. +func withRunningCLI(t *testing.T, cliPath string) { + t.Helper() + prev := currentCLIBinaryPath + currentCLIBinaryPath = func() (string, error) { return cliPath, nil } + t.Cleanup(func() { currentCLIBinaryPath = prev }) +} + func TestResolveDaemonBinaryPath(t *testing.T) { t.Run("returns curl-installer path when nothing else is on PATH", func(t *testing.T) { home := withIsolatedDaemonResolution(t) @@ -441,6 +454,47 @@ func TestResolveDaemonBinaryPath(t *testing.T) { } }) + t.Run("curl-installed CLI prefers its sibling daemon over Homebrew", func(t *testing.T) { + home := withIsolatedDaemonResolution(t) + + curlBin := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin") + curl := writeFakeDaemon(t, curlBin) + withRunningCLI(t, filepath.Join(curlBin, "shelltime")) + + // A stale/unmanaged daemon both on PATH and in the Homebrew search + // list must not shadow the daemon installed next to the curl CLI. + brewDir := t.TempDir() + writeFakeDaemon(t, brewDir) + t.Setenv("PATH", brewDir) + daemonHomebrewSearchPaths = []string{brewDir} + + got, err := ResolveDaemonBinaryPath() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != curl { + t.Errorf("expected curl-installer path %s, got %s", curl, got) + } + }) + + t.Run("curl-installed CLI falls back to Homebrew when its daemon is missing", func(t *testing.T) { + home := withIsolatedDaemonResolution(t) + + withRunningCLI(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin", "shelltime")) + + brewDir := t.TempDir() + brewPath := writeFakeDaemon(t, brewDir) + daemonHomebrewSearchPaths = []string{brewDir} + + got, err := ResolveDaemonBinaryPath() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != brewPath { + t.Errorf("expected Homebrew path %s, got %s", brewPath, got) + } + }) + t.Run("returns error when no daemon is found anywhere", func(t *testing.T) { withIsolatedDaemonResolution(t)