fix(daemon): auto-download shelltime-daemon when missing on install#281
Conversation
When `shelltime daemon install/reinstall` cannot find the daemon binary locally, fetch the matching release archive from GitHub and extract shelltime-daemon to ~/.shelltime/bin/. This recovers curl-installer users who installed before commit shelltime/installation@36a09e9 (where the daemon was renamed to .bak and download URLs 404'd) without forcing them to rerun the installer. - Add model.EnsureDaemonBinary: resolves locally first; on miss downloads the archive for the CLI's own version (falling back to latest for dev builds or 404'd tags), extracts shelltime-daemon, places it at ~/.shelltime/bin/shelltime-daemon via existing ReplaceBinary. - Refuse auto-download for Homebrew installs (point users at `brew reinstall`) and for Windows (no daemon binary built). - Make github base URLs swappable for tests in updater.go. - Wire the new helper into commands/daemon.install.go after the existing ResolveDaemonBinaryPath() call so the .bak restore path still runs. https://claude.ai/code/session_01ThKSJA9sGXQm8jMeCZZdYY
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an automatic download mechanism for the shelltime-daemon binary when it is missing during installation. It adds logic to fetch the appropriate version from GitHub releases, handles platform-specific restrictions (e.g., Windows and Homebrew), and includes comprehensive tests for the new functionality. Feedback was provided regarding the brittle nature of error string matching for 404 responses, the security implications of ignoring checksum fetch errors, and the lack of error handling when resolving the CLI binary path.
| color.Yellow.Println("Or via curl installer: curl -sSL https://shelltime.xyz/i | bash") | ||
| return fmt.Errorf("shelltime-daemon binary not found: %w", err) | ||
| color.Yellow.Println("⚠️ shelltime-daemon binary not found locally.") | ||
| cliPath, _ := model.ResolveCLIBinaryPath() |
There was a problem hiding this comment.
The error from ResolveCLIBinaryPath is ignored. While this function is generally reliable, an error here would result in an empty cliPath, which causes EnsureDaemonBinary to skip the Homebrew installation check. It's safer to handle the error or at least log a warning.
cliPath, err := model.ResolveCLIBinaryPath()
if err != nil {
color.Yellow.Printf("⚠️ Could not resolve CLI path: %v\n", err)
}| } | ||
|
|
||
| // Fall back once to latest if the tagged release 404'd (deleted/yanked). | ||
| if strings.Contains(err.Error(), "status 404") { |
There was a problem hiding this comment.
Checking for "status 404" in the error string is brittle as it depends on the exact error message format returned by DownloadAndVerify in updater.go. If that message changes, the fallback logic will break. Consider having DownloadAndVerify return a structured error or a specific error type to make this check more robust.
| func fetchDaemonToCurlPath(ctx context.Context, tag, archiveName string) (string, error) { | ||
| downloadURL := BuildDownloadURL(tag, archiveName) | ||
|
|
||
| sum, _, _ := FetchChecksum(ctx, tag, archiveName) |
There was a problem hiding this comment.
Ignoring the error from FetchChecksum means that any network failure or API error (other than a 404) while retrieving the checksum will result in the download proceeding without integrity verification. This creates a potential security risk where a compromised or intercepted download could go undetected. It is recommended to handle non-404 errors explicitly rather than treating all errors as "checksum not found".
|
@claude review it |
|
Claude finished @AnnatarHe's task in 2m 35s —— View job Claude Code Review
Overall: the self-heal flow is the right shape — fast-path on Findings1.
sum, _, _ := FetchChecksum(ctx, tag, archiveName)
// FetchChecksum errors and absent checksums are both non-fatal; we proceed
// without verification, matching the `shelltime update` behavior.The comment isn't quite right.
By throwing away the error, you collapse case 3 into case 1. A MITM (or a transient 502 from GitHub) silently downgrades you to unverified download. The narrow fix is to treat only sum, found, err := FetchChecksum(ctx, tag, archiveName)
if err != nil {
return "", fmt.Errorf("fetch checksum: %w", err)
}
if !found {
// legitimately absent (old release) — proceed unverified, but warn
}2.
// updater.go
var ErrAssetNotFound = errors.New("release asset not found")
if resp.StatusCode == http.StatusNotFound {
return fmt.Errorf("%w: %s", ErrAssetNotFound, url)
}Then 3.
cliPath, err := model.ResolveCLIBinaryPath()
if err != nil {
color.Yellow.Printf("⚠️ could not resolve CLI path (%v) — Homebrew detection may be inaccurate\n", err)
}4. The latest-tag fallback discards the second error nit
if destPath, ferr := fetchDaemonToCurlPath(ctx, latest, archiveName); ferr == nil {
return destPath, nil
}
// ferr lost; we return the original 404 errIf the fallback also fails (say the latest archive is missing the daemon binary), the user only sees "status 404 for v0.1.83" with no hint that we tried 5. Codecov calls this out. The orchestration ( 6. A commit SHA passed as version goes through silently edge case
7. Tests mutate package-level globals nit
Things done well
Suggested commit message for follow-up fix
|
Previously fetchDaemonToCurlPath discarded the FetchChecksum error,
treating any failure the same as a legitimately-absent checksum (404 /
no entry for this archive). That meant a transient 5xx from the GitHub
CDN — or an attacker blocking/tampering with checksums.txt — could
silently downgrade the install to an unverified download.
Now only ("", false, nil) is treated as absent. Real errors propagate
out of EnsureDaemonBinary, the temp dir is cleaned via defer, and no
daemon binary is written to ~/.shelltime/bin/.
https://claude.ai/code/session_01ThKSJA9sGXQm8jMeCZZdYY
2 participants
Summary
Fixes Linux users hitting
shelltime-daemon binary not foundonshelltime daemon reinstall.Root cause: users who ran the curl installer before
shelltime/installation@36a09e9(Apr 17 2026) ended up with no daemon binary — the old script downloaded from the legacymalamtime/cliURL (404) and even when extraction worked it renamedshelltime-daemonto.bak. Today, those users have to know to rerun the installer to recover. On macOS, the Homebrew formula installs both binaries, so the issue doesn't manifest.This PR makes
shelltime daemon install/reinstallself-heal: if the daemon binary can't be resolved locally, the CLI now downloads the matching release archive from GitHub and extractsshelltime-daemonto~/.shelltime/bin/shelltime-daemonbefore continuing with service setup.model.EnsureDaemonBinary(ctx, cliBinPath, cliVersion):ResolveDaemonBinaryPath()already finds a binary (fast path).brew reinstall shelltime/tap/shelltime) and for Windows (no daemon binary built).FetchLatestVersionfordevbuilds or 404'd tags.DownloadAndVerify/ExtractBinaries/ReplaceBinaryplumbing fromupdate.go.~/.shelltime/binon demand (covers Linuxbrew users who never ran the curl installer).commands/daemon.install.gonow callsEnsureDaemonBinaryafter the existing.bakrestore logic and the originalResolveDaemonBinaryPathlookup — the stale-curl cleanup block stays correct because a freshly-downloaded binary lives atGetCurlInstallerDaemonPath().model/updater.goexposesgithubAPIBaseURL/githubReleaseBaseURLas package vars so tests can point them at anhttptest.Server. No production behavior change.Paired with the install.bash hardening in
shelltime/installation(warn when the daemon binary is missing from the extracted archive).Test plan
go build ./...cleangofmt -dclean on all touched filesgo test ./model/...— all model tests pass, including 11 newTestEnsureDaemon_*cases (existing binary short-circuit, download-when-missing, Homebrew aborts, Windows aborts, version selection, dev → latest fallback, network error, archive missing daemon, creates bin dir, 404 → latest fallback, tag normalization)shelltimesmoke-tested locally (shelltime daemon --helplistsreinstalletc.)~/.shelltime/bin/shelltime-daemon*, runshelltime daemon reinstall, confirm download happens and service starts undersystemctl --user$(brew --prefix)/bin/, runshelltime daemon reinstall, confirm we point atbrew reinstalland do not drop a binary into~/.shelltime/bin/https://claude.ai/code/session_01ThKSJA9sGXQm8jMeCZZdYY
Generated by Claude Code