Skip to content

Commit 80c00a1

Browse files
AnnatarHeclaude
andcommitted
fix(daemon): prefer Homebrew binary over curl-installer location
Reorder ResolveDaemonBinaryPath to check PATH/Homebrew before ~/.shelltime/bin/shelltime-daemon, so that brew-managed installs actually drive the running daemon. On install, remove the stale curl-installer daemon when a system-managed binary is being used. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c2c0e2f commit 80c00a1

3 files changed

Lines changed: 163 additions & 19 deletions

File tree

commands/daemon.install.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func commandDaemonInstall(c *cli.Context) error {
3939
}
4040
}
4141

42-
// Resolve daemon binary (curl-installer, Homebrew, or PATH)
42+
// Resolve daemon binary (Homebrew/PATH preferred, curl-installer fallback)
4343
daemonBinPath, err := model.ResolveDaemonBinaryPath()
4444
if err != nil {
4545
color.Yellow.Println("⚠️ shelltime-daemon not found.")
@@ -49,6 +49,20 @@ func commandDaemonInstall(c *cli.Context) error {
4949
}
5050
color.Green.Printf("✅ Found daemon binary at: %s\n", daemonBinPath)
5151

52+
// If we picked a system-managed binary but a stale curl-installer copy
53+
// still lives under ~/.shelltime/bin, remove it so future resolution
54+
// stays unambiguous.
55+
curlDaemonPath := model.GetCurlInstallerDaemonPath()
56+
if daemonBinPath != curlDaemonPath {
57+
if info, statErr := os.Stat(curlDaemonPath); statErr == nil && !info.IsDir() {
58+
if rmErr := os.Remove(curlDaemonPath); rmErr == nil {
59+
color.Yellow.Printf("🧹 Removed stale curl-installer daemon at %s\n", curlDaemonPath)
60+
} else {
61+
color.Yellow.Printf("⚠️ Could not remove stale daemon at %s: %v\n", curlDaemonPath, rmErr)
62+
}
63+
}
64+
}
65+
5266
installer, err := model.NewDaemonInstaller(baseFolder, username, daemonBinPath)
5367
if err != nil {
5468
return err

model/path.go

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -114,34 +114,50 @@ func GetDaemonErrFilePath() string {
114114
return GetStoragePath("logs", "shelltime-daemon.err")
115115
}
116116

117+
// GetCurlInstallerDaemonPath returns the legacy curl-installer daemon location
118+
// (~/.shelltime/bin/shelltime-daemon).
119+
func GetCurlInstallerDaemonPath() string {
120+
return filepath.Join(GetBaseStoragePath(), "bin", "shelltime-daemon")
121+
}
122+
123+
// daemonHomebrewSearchPaths lists explicit Homebrew/Linuxbrew bin dirs to
124+
// probe when PATH is stripped (e.g. launchd-spawned shells). Exposed as a var
125+
// so tests can swap it out.
126+
var daemonHomebrewSearchPaths = []string{
127+
"/opt/homebrew/bin",
128+
"/usr/local/bin",
129+
"/home/linuxbrew/.linuxbrew/bin",
130+
}
131+
117132
// ResolveDaemonBinaryPath finds the shelltime-daemon binary.
118-
// It checks the curl-installer location first, then PATH (Homebrew),
119-
// then known Homebrew prefix paths as fallback.
133+
// It prefers a system-managed binary (Homebrew or anything on PATH) over the
134+
// legacy curl-installer location, so that `brew upgrade shelltime` is what
135+
// actually drives the running daemon.
120136
func ResolveDaemonBinaryPath() (string, error) {
121137
const binaryName = "shelltime-daemon"
138+
curlPath := GetCurlInstallerDaemonPath()
122139

123-
// 1. Check curl-installer location
124-
curlPath := filepath.Join(GetBaseStoragePath(), "bin", binaryName)
125-
if info, err := os.Stat(curlPath); err == nil && !info.IsDir() {
126-
return curlPath, nil
127-
}
128-
129-
// 2. Check PATH (covers Homebrew and other package managers)
140+
// 1. Check PATH (covers Homebrew and other package managers). Ignore the
141+
// result if it happens to resolve to the curl-installer path — we want
142+
// step 3 to be the only branch that returns that location.
130143
if path, err := exec.LookPath(binaryName); err == nil {
131-
return path, nil
144+
if resolved, _ := filepath.Abs(path); resolved != curlPath {
145+
return path, nil
146+
}
132147
}
133148

134-
// 3. Explicit Homebrew fallback paths
135-
homebrewPaths := []string{
136-
filepath.Join("/opt/homebrew/bin", binaryName),
137-
filepath.Join("/usr/local/bin", binaryName),
138-
filepath.Join("/home/linuxbrew/.linuxbrew/bin", binaryName),
139-
}
140-
for _, p := range homebrewPaths {
149+
// 2. Explicit Homebrew/Linuxbrew fallback paths, in case PATH was stripped.
150+
for _, dir := range daemonHomebrewSearchPaths {
151+
p := filepath.Join(dir, binaryName)
141152
if info, err := os.Stat(p); err == nil && !info.IsDir() {
142153
return p, nil
143154
}
144155
}
145156

146-
return "", fmt.Errorf("%s not found at %s, on PATH, or in standard Homebrew locations", binaryName, curlPath)
157+
// 3. Curl-installer fallback.
158+
if info, err := os.Stat(curlPath); err == nil && !info.IsDir() {
159+
return curlPath, nil
160+
}
161+
162+
return "", fmt.Errorf("%s not found on PATH, in standard Homebrew locations, or at %s", binaryName, curlPath)
147163
}

model/path_test.go

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,117 @@ func TestPathsAreClean(t *testing.T) {
289289
}
290290
}
291291
}
292+
293+
// writeFakeDaemon creates an executable file at the given path so exec.LookPath
294+
// will accept it as resolvable. Returns the absolute path actually written.
295+
func writeFakeDaemon(t *testing.T, dir string) string {
296+
t.Helper()
297+
if err := os.MkdirAll(dir, 0o755); err != nil {
298+
t.Fatalf("failed to mkdir %s: %v", dir, err)
299+
}
300+
p := filepath.Join(dir, "shelltime-daemon")
301+
if err := os.WriteFile(p, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
302+
t.Fatalf("failed to write fake daemon: %v", err)
303+
}
304+
return p
305+
}
306+
307+
// withIsolatedDaemonResolution sets up a hermetic environment for
308+
// ResolveDaemonBinaryPath: a temp $HOME, an empty $PATH (caller can re-add
309+
// dirs), and an empty Homebrew search list so the host machine's installs
310+
// (e.g. real /opt/homebrew/bin/shelltime-daemon) don't leak into assertions.
311+
// Returns the temp home directory.
312+
func withIsolatedDaemonResolution(t *testing.T) string {
313+
t.Helper()
314+
home := t.TempDir()
315+
t.Setenv("HOME", home)
316+
t.Setenv("PATH", "")
317+
318+
prev := daemonHomebrewSearchPaths
319+
daemonHomebrewSearchPaths = nil
320+
t.Cleanup(func() { daemonHomebrewSearchPaths = prev })
321+
322+
return home
323+
}
324+
325+
func TestResolveDaemonBinaryPath(t *testing.T) {
326+
t.Run("returns curl-installer path when nothing else is on PATH", func(t *testing.T) {
327+
home := withIsolatedDaemonResolution(t)
328+
329+
curl := writeFakeDaemon(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin"))
330+
331+
got, err := ResolveDaemonBinaryPath()
332+
if err != nil {
333+
t.Fatalf("unexpected error: %v", err)
334+
}
335+
if got != curl {
336+
t.Errorf("expected %s, got %s", curl, got)
337+
}
338+
})
339+
340+
t.Run("prefers PATH-resolved binary over curl-installer path", func(t *testing.T) {
341+
home := withIsolatedDaemonResolution(t)
342+
343+
brewDir := t.TempDir()
344+
brewPath := writeFakeDaemon(t, brewDir)
345+
t.Setenv("PATH", brewDir)
346+
347+
// stale curl-installer copy that should be ignored
348+
writeFakeDaemon(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin"))
349+
350+
got, err := ResolveDaemonBinaryPath()
351+
if err != nil {
352+
t.Fatalf("unexpected error: %v", err)
353+
}
354+
if got != brewPath {
355+
t.Errorf("expected PATH-resolved %s, got %s", brewPath, got)
356+
}
357+
})
358+
359+
t.Run("uses explicit Homebrew search list when PATH is empty", func(t *testing.T) {
360+
home := withIsolatedDaemonResolution(t)
361+
362+
brewDir := t.TempDir()
363+
brewPath := writeFakeDaemon(t, brewDir)
364+
daemonHomebrewSearchPaths = []string{brewDir}
365+
366+
// stale curl-installer copy that should be ignored
367+
writeFakeDaemon(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin"))
368+
369+
got, err := ResolveDaemonBinaryPath()
370+
if err != nil {
371+
t.Fatalf("unexpected error: %v", err)
372+
}
373+
if got != brewPath {
374+
t.Errorf("expected Homebrew search result %s, got %s", brewPath, got)
375+
}
376+
})
377+
378+
t.Run("skips PATH result that points back at the curl-installer path", func(t *testing.T) {
379+
home := withIsolatedDaemonResolution(t)
380+
381+
curlBin := filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin")
382+
curl := writeFakeDaemon(t, curlBin)
383+
// Put only the curl-installer dir on PATH; LookPath would return curl,
384+
// but the resolver must fall through and still return the curl path
385+
// from step 3 (so the cleanup logic in daemon.install can detect it).
386+
t.Setenv("PATH", curlBin)
387+
388+
got, err := ResolveDaemonBinaryPath()
389+
if err != nil {
390+
t.Fatalf("unexpected error: %v", err)
391+
}
392+
if got != curl {
393+
t.Errorf("expected curl-installer fallback %s, got %s", curl, got)
394+
}
395+
})
396+
397+
t.Run("returns error when no daemon is found anywhere", func(t *testing.T) {
398+
withIsolatedDaemonResolution(t)
399+
400+
_, err := ResolveDaemonBinaryPath()
401+
if err == nil {
402+
t.Fatal("expected error when no daemon binary exists, got nil")
403+
}
404+
})
405+
}

0 commit comments

Comments
 (0)