From 3618ded82d91132945c9a71f127a9fd24fcf4341 Mon Sep 17 00:00:00 2001 From: AnnatarHe Date: Fri, 7 Aug 2026 22:30:00 +0800 Subject: [PATCH 1/3] fix(updater): correct binary swap semantics and config merge gaps Four defects found while building auto-update on top of `shelltime update`. 1. `.bak` meant two opposite things. ReplaceBinary writes the PREVIOUS binary to ".bak", but commands/daemon.install.go reads that same filename as "a NEWER daemon, restore it". So replacing the daemon and then running `daemon install`/`daemon reinstall` restores the binary that was just replaced. This already misfires in `shelltime update` on curl installs, and would become an endless re-download loop once the daemon checks daily. Introduce ReplaceBinaryWithBackupSuffix and a distinct ".prev" suffix for update paths, so the installer's recovery branch can never fire on an update backup. ReplaceBinary keeps ".bak" for existing callers. 2. The swap left a window with no binary. rename(dest, dest.bak) followed by rename(tmp, dest) means dest does not exist between the two calls, and the shell hook execs `shelltime` by name on every command -- a hook landing in that window prints "command not found". Copy the old binary to the backup instead, stage the new one alongside dest, and activate with a single rename(2). Windows keeps the old order, since it cannot rename over a running .exe. 3. DetectInstallKind misclassified Intel-mac Homebrew. goreleaser publishes a Cask, so an EvalSymlinks'd path is /usr/local/Caskroom/... on Intel, which matches neither the /Cellar/ check nor the /opt/homebrew/ prefix. Those users were treated as "unknown location" and could have had the Caskroom written into. Match /Caskroom/ explicitly. 4. mergeConfig omitted Storage entirely, so `storage: {engine: bolt}` in config.local.yaml was silently dropped. Add it, and remove the duplicated LogCleanup branch. Also add CompareVersions and RestoreBinaryBackup, and fail DownloadAndVerify when the body is shorter than Content-Length -- both are needed by the unattended update path, where a downgrade or a truncated download must never reach the swap step. ReleaseSource lands here because FetchChecksum is refactored to route through it. Co-Authored-By: Claude Opus 5 (1M context) --- model/config.go | 34 +++++- model/config_cov_test.go | 52 +++++++++ model/types.go | 31 +++++ model/updater.go | 210 +++++++++++++++++++++++++++++++--- model/updater_replace_test.go | 194 +++++++++++++++++++++++++++++++ model/updater_source.go | 53 +++++++++ model/updater_source_test.go | 52 +++++++++ 7 files changed, 607 insertions(+), 19 deletions(-) create mode 100644 model/updater_replace_test.go create mode 100644 model/updater_source.go create mode 100644 model/updater_source_test.go diff --git a/model/config.go b/model/config.go index fb90d57..356ccc0 100644 --- a/model/config.go +++ b/model/config.go @@ -171,8 +171,11 @@ func mergeConfig(base, local *ShellTimeConfig) { if local.CodeTracking != nil { base.CodeTracking = local.CodeTracking } - if local.LogCleanup != nil { - base.LogCleanup = local.LogCleanup + if local.Storage != nil { + base.Storage = local.Storage + } + if local.AutoUpdate != nil { + base.AutoUpdate = local.AutoUpdate } } @@ -297,6 +300,33 @@ func (cs *configService) ReadConfigFile(ctx context.Context, opts ...ReadConfigO } } + // Initialize AutoUpdate with defaults if not present. Enabled by default so + // existing installs pick up updates; opt out with `autoUpdate.enabled: false`. + // Homebrew stays opt-in because `brew upgrade` can prompt and touches files + // we don't own. + falsy := false + if config.AutoUpdate == nil { + config.AutoUpdate = &AutoUpdate{ + Enabled: &truthy, + Homebrew: &falsy, + IntervalHours: DefaultAutoUpdateIntervalHours, + Channel: AutoUpdateChannelStable, + } + } else { + if config.AutoUpdate.Enabled == nil { + config.AutoUpdate.Enabled = &truthy + } + if config.AutoUpdate.Homebrew == nil { + config.AutoUpdate.Homebrew = &falsy + } + if config.AutoUpdate.Channel == "" { + config.AutoUpdate.Channel = AutoUpdateChannelStable + } + if config.AutoUpdate.IntervalHours < 1 { + config.AutoUpdate.IntervalHours = DefaultAutoUpdateIntervalHours + } + } + // Save to cache cs.mu.Lock() cs.cachedConfig = &config diff --git a/model/config_cov_test.go b/model/config_cov_test.go index ae53d7e..0f5c194 100644 --- a/model/config_cov_test.go +++ b/model/config_cov_test.go @@ -40,6 +40,8 @@ func TestMergeConfig_AllOverrides(t *testing.T) { LogCleanup: &LogCleanup{Enabled: &truthy, ThresholdMB: 42}, SocketPath: "/tmp/local.sock", CodeTracking: &CodeTracking{Token: "ct"}, + Storage: &StorageConfig{Engine: StorageEngineBolt}, + AutoUpdate: &AutoUpdate{Enabled: &on, IntervalHours: 6}, } mergeConfig(base, local) @@ -61,6 +63,56 @@ func TestMergeConfig_AllOverrides(t *testing.T) { assert.EqualValues(t, 42, base.LogCleanup.ThresholdMB) assert.Equal(t, "/tmp/local.sock", base.SocketPath) require.NotNil(t, base.CodeTracking) + // Storage was missing from mergeConfig entirely, so a local + // `storage: {engine: bolt}` was silently dropped. + require.NotNil(t, base.Storage, "local storage override must be applied") + assert.Equal(t, StorageEngineBolt, base.Storage.Engine) + require.NotNil(t, base.AutoUpdate, "local autoUpdate override must be applied") + assert.Equal(t, 6, base.AutoUpdate.IntervalHours) +} + +// TestReadConfigFile_AutoUpdateDefaults asserts auto-update is on by default and +// that an explicit `enabled: false` survives defaulting (opt-out must stick). +func TestReadConfigFile_AutoUpdateDefaults(t *testing.T) { + t.Run("absent section defaults to enabled", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), + []byte("token: tok\n"), 0o644)) + + cfg, err := NewConfigService(dir).ReadConfigFile(context.Background()) + require.NoError(t, err) + require.NotNil(t, cfg.AutoUpdate) + require.NotNil(t, cfg.AutoUpdate.Enabled) + assert.True(t, *cfg.AutoUpdate.Enabled) + require.NotNil(t, cfg.AutoUpdate.Homebrew) + assert.False(t, *cfg.AutoUpdate.Homebrew, "homebrew auto-upgrade must be opt-in") + assert.Equal(t, DefaultAutoUpdateIntervalHours, cfg.AutoUpdate.IntervalHours) + assert.Equal(t, AutoUpdateChannelStable, cfg.AutoUpdate.Channel) + }) + + t.Run("explicit false is preserved", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), + []byte("token: tok\nautoUpdate:\n enabled: false\n"), 0o644)) + + cfg, err := NewConfigService(dir).ReadConfigFile(context.Background()) + require.NoError(t, err) + require.NotNil(t, cfg.AutoUpdate) + require.NotNil(t, cfg.AutoUpdate.Enabled) + assert.False(t, *cfg.AutoUpdate.Enabled, "opt-out must not be overwritten by defaults") + // Other fields still get backfilled. + assert.Equal(t, DefaultAutoUpdateIntervalHours, cfg.AutoUpdate.IntervalHours) + }) + + t.Run("interval is clamped", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), + []byte("token: tok\nautoUpdate:\n intervalHours: 0\n"), 0o644)) + + cfg, err := NewConfigService(dir).ReadConfigFile(context.Background()) + require.NoError(t, err) + assert.Equal(t, DefaultAutoUpdateIntervalHours, cfg.AutoUpdate.IntervalHours) + }) } // TestMergeConfig_CCOtelMigration covers the deprecated CCOtel -> AICodeOtel diff --git a/model/types.go b/model/types.go index 8d6ff66..0187c8f 100644 --- a/model/types.go +++ b/model/types.go @@ -2,6 +2,11 @@ package model const ( DefaultSocketPath = "/tmp/shelltime.sock" + + // AutoUpdateChannelStable is the only release channel honored today. + AutoUpdateChannelStable = "stable" + // DefaultAutoUpdateIntervalHours is how often the daemon checks for a new release. + DefaultAutoUpdateIntervalHours = 24 ) type Endpoint struct { @@ -43,6 +48,23 @@ type CodeTracking struct { Token string `toml:"token,omitempty" yaml:"token,omitempty" json:"token,omitempty"` // Custom token for heartbeats } +// AutoUpdate configures the daemon-driven CLI self-update. Enabled by default; +// set `enabled: false` to opt out entirely. +type AutoUpdate struct { + // Enabled turns the daily check off entirely. Default: true. + Enabled *bool `toml:"enabled" yaml:"enabled" json:"enabled"` + // Homebrew allows the daemon to run `brew upgrade --cask + // shelltime/tap/shelltime` on Homebrew installs. Default: false — we only + // record a notice, because brew can prompt and can touch files we don't own. + Homebrew *bool `toml:"homebrew" yaml:"homebrew" json:"homebrew"` + // NotifyOnly records an "update available" notice without downloading anything. + NotifyOnly *bool `toml:"notifyOnly,omitempty" yaml:"notifyOnly,omitempty" json:"notifyOnly,omitempty"` + // IntervalHours overrides the 24h check cadence (clamped to >= 1). Default: 24. + IntervalHours int `toml:"intervalHours,omitempty" yaml:"intervalHours,omitempty" json:"intervalHours,omitempty"` + // Channel is reserved for pre-release opt-in; only "stable" is honored today. + Channel string `toml:"channel,omitempty" yaml:"channel,omitempty" json:"channel,omitempty"` +} + // LogCleanup configuration for automatic log file cleanup type LogCleanup struct { Enabled *bool `toml:"enabled" yaml:"enabled" json:"enabled"` // default: true (enabled by default) @@ -97,6 +119,9 @@ type ShellTimeConfig struct { // LogCleanup configuration for automatic log file cleanup in daemon LogCleanup *LogCleanup `toml:"logCleanup" yaml:"logCleanup" json:"logCleanup"` + // AutoUpdate configuration for the daemon-driven CLI self-update + AutoUpdate *AutoUpdate `toml:"autoUpdate" yaml:"autoUpdate" json:"autoUpdate"` + // Storage selects the local command buffering backend. When unset the // always-available txt file store is used. Storage *StorageConfig `toml:"storage" yaml:"storage,omitempty" json:"storage,omitempty"` @@ -149,6 +174,12 @@ var DefaultConfig = ShellTimeConfig{ Enabled: new(true), }), LogCleanup: nil, + AutoUpdate: new(AutoUpdate{ + Enabled: new(true), + Homebrew: new(false), + IntervalHours: 24, + Channel: AutoUpdateChannelStable, + }), SocketPath: DefaultSocketPath, } diff --git a/model/updater.go b/model/updater.go index cd3eab9..f6b7774 100644 --- a/model/updater.go +++ b/model/updater.go @@ -15,6 +15,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "time" @@ -155,10 +156,20 @@ func BuildChecksumsURL(tag string) string { ) } -// FetchChecksum returns the expected SHA256 for archiveName. The bool reports -// whether a checksum was found; callers may proceed without verification if false. +// FetchChecksum returns the expected SHA256 for archiveName, fetched from +// GitHub directly. The bool reports whether a checksum was found; callers may +// proceed without verification if false. func FetchChecksum(ctx context.Context, tag, archiveName string) (string, bool, error) { - url := BuildChecksumsURL(tag) + return FetchChecksumFrom(ctx, ReleaseSource{}, tag, archiveName) +} + +// FetchChecksumFrom returns the expected SHA256 for archiveName, fetched through +// the given source (proxy or GitHub direct). +func FetchChecksumFrom(ctx context.Context, src ReleaseSource, tag, archiveName string) (string, bool, error) { + return fetchChecksumURL(ctx, src.ChecksumsURL(tag), archiveName) +} + +func fetchChecksumURL(ctx context.Context, url, archiveName string) (string, bool, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return "", false, err @@ -235,10 +246,17 @@ func DownloadAndVerify(ctx context.Context, url, expectedSha256, destPath string defer out.Close() hasher := sha256.New() - if _, err := io.Copy(out, io.TeeReader(resp.Body, hasher)); err != nil { + written, err := io.Copy(out, io.TeeReader(resp.Body, hasher)) + if err != nil { return fmt.Errorf("write archive: %w", err) } + // A proxy (or a flaky link) can close the stream early. Without a checksum + // that truncation would otherwise sail through to the extract step. + if resp.ContentLength >= 0 && written != resp.ContentLength { + return fmt.Errorf("truncated download %s: got %d bytes, expected %d", url, written, resp.ContentLength) + } + if expectedSha256 != "" { got := hex.EncodeToString(hasher.Sum(nil)) if !strings.EqualFold(got, expectedSha256) { @@ -368,27 +386,132 @@ func stripExe(name string) string { return strings.TrimSuffix(name, ".exe") } -// ReplaceBinary swaps a freshly-downloaded binary into destPath, renaming any -// existing destPath to destPath+".bak" (overwriting a previous .bak). On Unix -// this is safe even while the binary is running because the kernel keeps the -// old inode alive for the current process. +// BackupSuffixLegacy is the historical backup suffix written by ReplaceBinary. +// +// WARNING: commands/daemon.install.go treats ".bak" as "a NEWER daemon +// that should be restored", which is the opposite of what ReplaceBinary means by +// it. Any code path that replaces the daemon binary and then runs +// `daemon install`/`daemon reinstall` MUST use BackupSuffixUpdate instead, or the +// install step will restore the binary that was just replaced. +const BackupSuffixLegacy = ".bak" + +// BackupSuffixUpdate is the backup suffix used by the self-update paths. It is +// deliberately distinct from BackupSuffixLegacy so the daemon installer's +// ".bak means restore me" recovery branch never fires on an update backup. +const BackupSuffixUpdate = ".prev" + +// ReplaceBinary swaps a freshly-downloaded binary into destPath, keeping the +// previous binary at destPath+".bak". +// +// Prefer ReplaceBinaryWithBackupSuffix with BackupSuffixUpdate for the daemon +// binary; see BackupSuffixLegacy for why. func ReplaceBinary(srcPath, destPath string) error { - bak := destPath + ".bak" - _ = os.Remove(bak) + return ReplaceBinaryWithBackupSuffix(srcPath, destPath, BackupSuffixLegacy) +} + +// ReplaceBinaryWithBackupSuffix swaps srcPath into destPath, preserving the +// previous binary at destPath+suffix (overwriting any previous backup). +// +// On Unix the swap is atomic: the old binary is *copied* to the backup, the new +// binary is staged alongside destPath, and a single rename(2) puts it in place. +// destPath therefore never disappears — which matters because the shell hook +// execs `shelltime` by name on every command, and a hook landing in a gap would +// print "command not found". Replacing a running binary is safe because the +// kernel keeps the old inode alive for processes that already opened it. +// +// Windows cannot rename over a running .exe, so it keeps the historical +// move-the-old-one-away-first order. +func ReplaceBinaryWithBackupSuffix(srcPath, destPath, suffix string) error { + if suffix == "" { + suffix = BackupSuffixUpdate + } + backup := destPath + suffix + + if runtime.GOOS == "windows" { + return replaceBinaryViaRenameAway(srcPath, destPath, backup) + } + return replaceBinaryAtomic(srcPath, destPath, backup) +} + +// replaceBinaryAtomic never leaves destPath absent. Used on all Unix platforms. +func replaceBinaryAtomic(srcPath, destPath, backup string) error { if _, err := os.Stat(destPath); err == nil { - if err := os.Rename(destPath, bak); err != nil { - return fmt.Errorf("rename %s -> %s: %w", destPath, bak, err) + _ = os.Remove(backup) + if err := copyFile(destPath, backup); err != nil { + return fmt.Errorf("back up %s -> %s: %w", destPath, backup, err) + } + } + + // Stage in the destination directory so the final rename cannot cross a + // filesystem boundary (rename(2) fails with EXDEV across mounts). + staged := destPath + ".new" + _ = os.Remove(staged) + if err := copyFile(srcPath, staged); err != nil { + _ = os.Remove(staged) + return fmt.Errorf("stage %s -> %s: %w", srcPath, staged, err) + } + if err := os.Chmod(staged, 0o755); err != nil { + _ = os.Remove(staged) + return err + } + if err := os.Rename(staged, destPath); err != nil { + _ = os.Remove(staged) + return fmt.Errorf("activate %s -> %s: %w", staged, destPath, err) + } + _ = os.Remove(srcPath) + return nil +} + +// replaceBinaryViaRenameAway is the Windows path: a running .exe cannot be +// renamed over, but it can be renamed away. +func replaceBinaryViaRenameAway(srcPath, destPath, backup string) error { + _ = os.Remove(backup) + if _, err := os.Stat(destPath); err == nil { + if err := os.Rename(destPath, backup); err != nil { + return fmt.Errorf("rename %s -> %s: %w", destPath, backup, err) } } if err := moveFile(srcPath, destPath); err != nil { - // Try to restore .bak on failure so we don't leave the user without a binary. - _ = os.Rename(bak, destPath) + // Try to restore the backup so we don't leave the user without a binary. + _ = os.Rename(backup, destPath) + return err + } + return os.Chmod(destPath, 0o755) +} + +// RestoreBinaryBackup puts the backup at destPath+suffix back at destPath. Used +// to roll back when a freshly-installed binary fails its post-swap smoke test. +func RestoreBinaryBackup(destPath, suffix string) error { + if suffix == "" { + suffix = BackupSuffixUpdate + } + backup := destPath + suffix + if _, err := os.Stat(backup); err != nil { + return fmt.Errorf("no backup at %s: %w", backup, err) + } + if runtime.GOOS == "windows" { + _ = os.Remove(destPath) + return os.Rename(backup, destPath) + } + return replaceBinaryAtomic(backup, destPath, destPath+".rollback") +} + +// copyFile copies src to dst with mode 0755, truncating dst if it exists. +func copyFile(src, dst string) error { + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o755) + if err != nil { return err } - if err := os.Chmod(destPath, 0o755); err != nil { + if _, err := io.Copy(out, in); err != nil { + out.Close() return err } - return nil + return out.Close() } // moveFile renames src to dst, falling back to copy+remove when crossing @@ -421,6 +544,53 @@ func NormalizeVersion(v string) string { return strings.TrimPrefix(strings.TrimSpace(v), "v") } +// CompareVersions compares two dotted numeric versions, ignoring a leading "v" +// and any pre-release suffix ("1.2.3-rc1" compares as "1.2.3"). It returns -1 if +// a < b, 0 if equal, and 1 if a > b. Non-numeric or missing segments count as 0, +// so it never panics on malformed input from the server. +// +// The unattended update path uses this to guarantee it only ever moves forward: +// a server bug that reports an old tag must not downgrade the user. +func CompareVersions(a, b string) int { + as := versionSegments(a) + bs := versionSegments(b) + n := max(len(as), len(bs)) + for i := range n { + var av, bv int + if i < len(as) { + av = as[i] + } + if i < len(bs) { + bv = bs[i] + } + if av != bv { + if av < bv { + return -1 + } + return 1 + } + } + return 0 +} + +func versionSegments(v string) []int { + v = NormalizeVersion(v) + // Drop pre-release/build metadata: "1.2.3-rc1+deadbeef" -> "1.2.3". + if i := strings.IndexAny(v, "-+"); i >= 0 { + v = v[:i] + } + parts := strings.Split(v, ".") + out := make([]int, 0, len(parts)) + for _, p := range parts { + n, err := strconv.Atoi(strings.TrimSpace(p)) + if err != nil { + n = 0 + } + out = append(out, n) + } + return out +} + // ResolveCLIBinaryPath returns the real (symlink-resolved) path of the running // CLI binary. func ResolveCLIBinaryPath() (string, error) { @@ -448,7 +618,13 @@ const ( // install ($HOME/.shelltime/bin), or unknown. func DetectInstallKind(binPath string) InstallKind { clean := filepath.Clean(binPath) - if strings.Contains(clean, string(filepath.Separator)+"Cellar"+string(filepath.Separator)) || + sep := string(filepath.Separator) + // goreleaser publishes a Cask, so an EvalSymlinks'd path lands in + // .../Caskroom/... — /opt/homebrew/Caskroom on Apple Silicon (already matched + // by the prefix below) but /usr/local/Caskroom on Intel, which matches + // neither the Cellar nor the prefix checks. + if strings.Contains(clean, sep+"Cellar"+sep) || + strings.Contains(clean, sep+"Caskroom"+sep) || strings.HasPrefix(clean, "/opt/homebrew/") || strings.HasPrefix(clean, "/home/linuxbrew/.linuxbrew/") { return InstallKindHomebrew diff --git a/model/updater_replace_test.go b/model/updater_replace_test.go new file mode 100644 index 0000000..e433a1b --- /dev/null +++ b/model/updater_replace_test.go @@ -0,0 +1,194 @@ +package model + +import ( + "os" + "path/filepath" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeExecutable(t *testing.T, path, content string) { + t.Helper() + require.NoError(t, os.WriteFile(path, []byte(content), 0o755)) +} + +// The daemon installer treats ".bak" as "a newer binary, restore it". +// The updater must therefore never write its backup to that name, or a swap +// followed by `daemon install` silently rolls back to the old binary. +func TestReplaceBinaryWithBackupSuffix_DoesNotCollideWithInstallerBak(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime-daemon") + src := filepath.Join(dir, "new-daemon") + + writeExecutable(t, dest, "OLD") + writeExecutable(t, src, "NEW") + + require.NoError(t, ReplaceBinaryWithBackupSuffix(src, dest, BackupSuffixUpdate)) + + got, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "NEW", string(got), "destination must hold the new binary") + + backup, err := os.ReadFile(dest + BackupSuffixUpdate) + require.NoError(t, err) + assert.Equal(t, "OLD", string(backup), "previous binary must be preserved at .prev") + + _, err = os.Stat(dest + BackupSuffixLegacy) + assert.True(t, os.IsNotExist(err), + "must not create a .bak — daemon install would restore it over the new binary") +} + +func TestReplaceBinary_LegacySuffixStillHonored(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime") + src := filepath.Join(dir, "new") + + writeExecutable(t, dest, "OLD") + writeExecutable(t, src, "NEW") + + require.NoError(t, ReplaceBinary(src, dest)) + + got, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "NEW", string(got)) + + backup, err := os.ReadFile(dest + BackupSuffixLegacy) + require.NoError(t, err) + assert.Equal(t, "OLD", string(backup)) +} + +// Every shell hook execs `shelltime` by name on every command. If the binary +// vanishes for even an instant during a swap, a hook landing in that window +// prints "command not found". +func TestReplaceBinaryWithBackupSuffix_DestinationNeverDisappears(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime") + src := filepath.Join(dir, "new") + + writeExecutable(t, dest, "OLD") + writeExecutable(t, src, "NEW") + + var missing atomic.Bool + stop := make(chan struct{}) + watcherDone := make(chan struct{}) + go func() { + defer close(watcherDone) + for { + select { + case <-stop: + return + default: + } + if _, err := os.Stat(dest); os.IsNotExist(err) { + missing.Store(true) + return + } + } + }() + + // Run the swap repeatedly so the watcher gets many chances to observe a gap. + for i := range 20 { + writeExecutable(t, src, "NEW") + require.NoError(t, ReplaceBinaryWithBackupSuffix(src, dest, BackupSuffixUpdate), "iteration %d", i) + } + + close(stop) + <-watcherDone + + assert.False(t, missing.Load(), "destination path disappeared during replace") +} + +func TestReplaceBinaryWithBackupSuffix_NoExistingDest(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime") + src := filepath.Join(dir, "new") + writeExecutable(t, src, "NEW") + + require.NoError(t, ReplaceBinaryWithBackupSuffix(src, dest, BackupSuffixUpdate)) + + got, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "NEW", string(got)) + + info, err := os.Stat(dest) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), info.Mode().Perm()) +} + +func TestReplaceBinaryWithBackupSuffix_EmptySuffixDefaults(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime") + src := filepath.Join(dir, "new") + writeExecutable(t, dest, "OLD") + writeExecutable(t, src, "NEW") + + require.NoError(t, ReplaceBinaryWithBackupSuffix(src, dest, "")) + + backup, err := os.ReadFile(dest + BackupSuffixUpdate) + require.NoError(t, err) + assert.Equal(t, "OLD", string(backup)) +} + +func TestRestoreBinaryBackup(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime") + src := filepath.Join(dir, "new") + writeExecutable(t, dest, "OLD") + writeExecutable(t, src, "NEW") + + require.NoError(t, ReplaceBinaryWithBackupSuffix(src, dest, BackupSuffixUpdate)) + require.NoError(t, RestoreBinaryBackup(dest, BackupSuffixUpdate)) + + got, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "OLD", string(got), "rollback must restore the previous binary") +} + +func TestRestoreBinaryBackup_MissingBackup(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "shelltime") + writeExecutable(t, dest, "CURRENT") + + err := RestoreBinaryBackup(dest, BackupSuffixUpdate) + assert.Error(t, err) +} + +func TestCompareVersions(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"1.2.3", "1.2.3", 0}, + {"v1.2.3", "1.2.3", 0}, + {"1.2.3", "v1.2.3", 0}, + {"1.2.4", "1.2.3", 1}, + {"1.2.3", "1.2.4", -1}, + {"1.3.0", "1.2.99", 1}, + {"2.0.0", "1.99.99", 1}, + {"0.1.90", "0.1.89", 1}, + {"0.1.9", "0.1.10", -1}, + {"1.2", "1.2.0", 0}, + {"1.2.3-rc1", "1.2.3", 0}, + {"1.2.3+build", "1.2.3", 0}, + {"dev", "1.2.3", -1}, + {"", "0.0.0", 0}, + {"garbage", "also-garbage", 0}, + } + for _, c := range cases { + assert.Equal(t, c.want, CompareVersions(c.a, c.b), "CompareVersions(%q, %q)", c.a, c.b) + } +} + +func TestDetectInstallKind_IntelMacCaskroom(t *testing.T) { + // goreleaser publishes a Cask; on Intel macs EvalSymlinks lands here, which + // matches neither /Cellar/ nor the /opt/homebrew/ prefix. + assert.Equal(t, InstallKindHomebrew, + DetectInstallKind("/usr/local/Caskroom/shelltime/0.1.89/shelltime")) + assert.Equal(t, InstallKindHomebrew, + DetectInstallKind("/opt/homebrew/Caskroom/shelltime/0.1.89/shelltime")) + assert.Equal(t, InstallKindHomebrew, + DetectInstallKind("/opt/homebrew/Cellar/shelltime/0.1.89/bin/shelltime")) +} diff --git a/model/updater_source.go b/model/updater_source.go new file mode 100644 index 0000000..4592e8d --- /dev/null +++ b/model/updater_source.go @@ -0,0 +1,53 @@ +package model + +import ( + "net/url" + "strings" +) + +// CLIReleaseProxyPath mirrors the server's release proxy route. Changing it is a +// breaking change for already-installed clients, which pin this path. +const CLIReleaseProxyPath = "/api/v1/cli/releases" + +// ReleaseSource resolves release-asset URLs. +// +// The zero value means "fetch from GitHub directly", which preserves the +// historical behaviour for every existing caller. A non-empty ProxyBase routes +// downloads through the shelltime API instead, which is what makes updating work +// in regions where github.com is unreachable. +type ReleaseSource struct { + ProxyBase string +} + +// NewReleaseSource builds a source from a configured API endpoint. A blank or +// non-HTTP endpoint yields the direct-from-GitHub zero value. +func NewReleaseSource(apiEndpoint string) ReleaseSource { + e := strings.TrimSpace(apiEndpoint) + if !strings.HasPrefix(e, "http://") && !strings.HasPrefix(e, "https://") { + return ReleaseSource{} + } + return ReleaseSource{ProxyBase: strings.TrimSuffix(e, "/")} +} + +// IsProxy reports whether downloads go through the shelltime API. +func (s ReleaseSource) IsProxy() bool { return s.ProxyBase != "" } + +// Direct returns the GitHub-direct source, used as a fallback when the proxy +// fails. +func (s ReleaseSource) Direct() ReleaseSource { return ReleaseSource{} } + +// DownloadURL returns the URL for a release archive. +func (s ReleaseSource) DownloadURL(tag, assetName string) string { + if !s.IsProxy() { + return BuildDownloadURL(tag, assetName) + } + return s.ProxyBase + CLIReleaseProxyPath + "/" + url.PathEscape(tag) + "/" + url.PathEscape(assetName) +} + +// ChecksumsURL returns the URL for the release's checksums.txt. +func (s ReleaseSource) ChecksumsURL(tag string) string { + if !s.IsProxy() { + return BuildChecksumsURL(tag) + } + return s.ProxyBase + CLIReleaseProxyPath + "/" + url.PathEscape(tag) + "/checksums.txt" +} diff --git a/model/updater_source_test.go b/model/updater_source_test.go new file mode 100644 index 0000000..39b5d39 --- /dev/null +++ b/model/updater_source_test.go @@ -0,0 +1,52 @@ +package model + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewReleaseSource(t *testing.T) { + t.Run("http endpoint becomes a proxy", func(t *testing.T) { + s := NewReleaseSource("https://api.shelltime.xyz") + assert.True(t, s.IsProxy()) + assert.Equal(t, "https://api.shelltime.xyz", s.ProxyBase) + }) + + t.Run("trailing slash is trimmed", func(t *testing.T) { + s := NewReleaseSource("https://api.shelltime.xyz/") + assert.Equal(t, "https://api.shelltime.xyz", s.ProxyBase) + }) + + t.Run("empty or non-http falls back to GitHub direct", func(t *testing.T) { + for _, in := range []string{"", " ", "api.shelltime.xyz", "ftp://x"} { + s := NewReleaseSource(in) + assert.False(t, s.IsProxy(), "input %q should not produce a proxy", in) + } + }) +} + +func TestReleaseSource_URLs(t *testing.T) { + t.Run("zero value points at GitHub", func(t *testing.T) { + var s ReleaseSource + assert.Equal(t, BuildDownloadURL("v1.2.3", "cli_Darwin_arm64.zip"), + s.DownloadURL("v1.2.3", "cli_Darwin_arm64.zip")) + assert.Equal(t, BuildChecksumsURL("v1.2.3"), s.ChecksumsURL("v1.2.3")) + }) + + t.Run("proxy points at the API", func(t *testing.T) { + s := NewReleaseSource("https://api.shelltime.xyz") + assert.Equal(t, + "https://api.shelltime.xyz/api/v1/cli/releases/v1.2.3/cli_Darwin_arm64.zip", + s.DownloadURL("v1.2.3", "cli_Darwin_arm64.zip")) + assert.Equal(t, + "https://api.shelltime.xyz/api/v1/cli/releases/v1.2.3/checksums.txt", + s.ChecksumsURL("v1.2.3")) + }) + + t.Run("Direct escapes the proxy", func(t *testing.T) { + s := NewReleaseSource("https://api.shelltime.xyz") + assert.False(t, s.Direct().IsProxy()) + assert.Equal(t, BuildDownloadURL("v1.2.3", "a.zip"), s.Direct().DownloadURL("v1.2.3", "a.zip")) + }) +} From 18521a5753aebd184d203309cabfa4de22ccded7 Mon Sep 17 00:00:00 2001 From: AnnatarHe Date: Fri, 7 Aug 2026 22:30:30 +0800 Subject: [PATCH 2/3] feat(update): add daemon-driven CLI self-update Users sit on stale CLIs because updating is a manual step they never take, and users behind a GitHub block cannot update at all. Have the long-lived daemon check once a day and install updates through the shelltime API. Daemon (daemon/auto_update.go): - Wakes hourly and gates on a PERSISTED LastCheckAt rather than running a 24h ticker. A 24h ticker resets on every daemon restart, so a daemon that restarts more often than that would never check at all; the persisted gate self-corrects across restarts and makes a crash loop harmless. Jittered so a fleet does not stampede the server after a release. - Updates the CLI binary in place but only STAGES the daemon binary at bin/shelltime-daemon.next. The running daemon must not rewrite the binary it is executing, and staging means the shell-side repair is a local rename with no network call inside the hook and no second 15MB download. - Homebrew is opt-in (autoUpdate.homebrew); by default we only record a notice, because brew can prompt and touches files we do not own. When it does run, stdin is closed and NONINTERACTIVE is set so it can never block. CLI drift repair: - `track` runs on every command, so it gets a 1-in-64 sampled check: a constant compare plus rand.IntN, under 100ns with zero syscalls on the common path, and one stat(2) of a normally-absent marker when sampled. Uses math/rand/v2, whose global source is seeded per process -- every `track` is a fresh process, so a v1-style fixed seed would make them all sample identically. - `gc` runs once per new shell and carries the deterministic check. It is also the only place a notice can print without interleaving with command output. - The repair itself runs detached via a hidden `daemon apply-update`, in its own session, so launchctl/systemctl work cannot stall the prompt and Ctrl-C cannot kill a half-finished swap. Keeping the daemon alive: - shouldReinstallDaemon bailed when installer.Check() failed -- precisely when the service was NOT running -- so `shelltime update` left a stopped daemon stopped forever. Since the daemon drives the update check, that killed auto-update permanently. It now only decides whether we manage a daemon at all; whether it is running picks reinstall-vs-install. - ensureDaemonRunning verifies the service actually came up by polling both the service manager and the socket, rather than trusting StartService()'s exit code, which only reports that launchctl accepted the command. - Every exit path of `daemon apply-update` from the staging check onward calls it -- including when the smoke test fails or the swap is rolled back. An unreachable daemon means repair, not bail. - `gc` also restarts a long-down daemon, closing the chicken-and-egg where a dead daemon means no check ever runs again. Guarded on the service file still existing, so a deliberate `daemon uninstall` is left alone, and rate limited to once an hour. Safety: downloads are checksum-verified, cross-checking the API-provided sum against checksums.txt; the unattended path fails closed when it cannot verify, while interactive `shelltime update --allow-unverified` may opt out. Every binary must print its own version before AND after installation, with automatic rollback. CompareVersions guarantees we never downgrade. Dev builds are skipped, unwritable and unrecognized locations only produce a notice (never sudo), a shared lock serializes the daemon and every CLI repair, and SHELLTIME_DISABLE_AUTO_UPDATE stops everything without a config edit. Enabled by default (autoUpdate.enabled), following the logCleanup convention. Note the trust root is TLS to api.shelltime.xyz plus a checksum that same server supplies -- better than trusting github.com the same way, but not signature verification. Release signing is the follow-up. Co-Authored-By: Claude Opus 5 (1M context) --- .mockery.yml | 4 + cmd/daemon/main.go | 11 + commands/daemon.apply_update.go | 182 +++++++++++ commands/daemon.apply_update_test.go | 226 ++++++++++++++ commands/daemon.ensure.go | 162 ++++++++++ commands/daemon.go | 1 + commands/detach_unix.go | 15 + commands/detach_windows.go | 9 + commands/gc.go | 6 + commands/track.go | 6 + commands/update.go | 167 +++++----- commands/update_drift.go | 163 ++++++++++ commands/update_drift_test.go | 199 ++++++++++++ daemon/auto_update.go | 391 ++++++++++++++++++++++++ daemon/auto_update_test.go | 436 +++++++++++++++++++++++++++ docs/CONFIG.md | 53 ++++ model/cli_release.go | 86 ++++++ model/path.go | 52 ++++ model/update_lock.go | 68 +++++ model/update_lock_test.go | 94 ++++++ model/updater_apply.go | 270 +++++++++++++++++ model/updater_apply_test.go | 329 ++++++++++++++++++++ model/updater_state.go | 130 ++++++++ model/updater_state_test.go | 74 +++++ 24 files changed, 3063 insertions(+), 71 deletions(-) create mode 100644 commands/daemon.apply_update.go create mode 100644 commands/daemon.apply_update_test.go create mode 100644 commands/daemon.ensure.go create mode 100644 commands/detach_unix.go create mode 100644 commands/detach_windows.go create mode 100644 commands/update_drift.go create mode 100644 commands/update_drift_test.go create mode 100644 daemon/auto_update.go create mode 100644 daemon/auto_update_test.go create mode 100644 model/cli_release.go create mode 100644 model/update_lock.go create mode 100644 model/update_lock_test.go create mode 100644 model/updater_apply.go create mode 100644 model/updater_apply_test.go create mode 100644 model/updater_state.go create mode 100644 model/updater_state_test.go diff --git a/.mockery.yml b/.mockery.yml index 82650e4..5873b98 100644 --- a/.mockery.yml +++ b/.mockery.yml @@ -30,3 +30,7 @@ packages: config: dir: "{{.InterfaceDir}}" filename: "mock_{{.InterfaceName}}.go" + DaemonInstaller: + config: + dir: "{{.InterfaceDir}}" + filename: "mock_{{.InterfaceName}}.go" diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index 63429fd..cf6b6c8 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -180,6 +180,17 @@ func main() { } } + // Start auto update service if enabled (enabled by default) + if cfg.AutoUpdate != nil && cfg.AutoUpdate.Enabled != nil && *cfg.AutoUpdate.Enabled { + autoUpdateService := daemon.NewAutoUpdateService(cfg) + if err := autoUpdateService.Start(ctx); err != nil { + slog.Error("Failed to start auto update service", slog.Any("err", err)) + } else { + slog.Info("Auto update service started") + defer autoUpdateService.Stop() + } + } + // Create processor instance processor := daemon.NewSocketHandler(&cfg, pubsub) diff --git a/commands/daemon.apply_update.go b/commands/daemon.apply_update.go new file mode 100644 index 0000000..97adaab --- /dev/null +++ b/commands/daemon.apply_update.go @@ -0,0 +1,182 @@ +package commands + +import ( + "log/slog" + "os" + "time" + + "github.com/malamtime/cli/model" + "github.com/urfave/cli/v2" +) + +var DaemonApplyUpdateCommand = &cli.Command{ + Name: "apply-update", + Usage: "Apply a staged daemon binary update (internal)", + Hidden: true, + Action: commandDaemonApplyUpdate, +} + +// Seams for tests. resolveDaemonDest in particular MUST be swappable: it +// resolves through exec.LookPath and can therefore point at a real +// system-installed daemon regardless of $HOME. +var ( + applyUpdateStopService = func(inst model.DaemonInstaller) error { + return inst.CheckAndStopExistingService() + } + applyUpdateEnsureRunning = ensureDaemonRunning + applyUpdateSmokeTest = model.SmokeTestDaemonBinary + applyUpdateResolveDest = resolveDaemonDest +) + +// commandDaemonApplyUpdate finishes an update the daemon started: it activates +// the staged daemon binary and restarts the service. +// +// It runs detached from the shell (see launchDetachedApplyUpdate) and always +// returns nil — this is background repair, and a non-zero exit would surface as +// noise in the user's shell. +// +// INVARIANT: every exit path from step 4 onward calls ensureDaemonRunning. +// Whatever else goes wrong, this command must not return with the daemon down. +func commandDaemonApplyUpdate(c *cli.Context) error { + ctx := c.Context + + if os.Getenv(model.DisableAutoUpdateEnv) != "" { + return nil + } + + // 1. Only one repair at a time: N shells can sample the drift check at once. + release, ok := model.AcquireUpdateLock(10 * time.Minute) + if !ok { + slog.Debug("another process is already applying the update") + return nil + } + defer release() + + // 2. Re-check the marker now that we hold the lock; another process may + // have finished the repair while we waited. + pendingTag, err := model.ReadDaemonUpdatePending() + if err != nil { + // No marker. This is the self-heal path: the daemon is down but has + // nothing staged, so just bring it back up. + if !daemonIsReady(ctx) && daemonServiceFileExists() { + return finishWithRunningDaemon(c, false, "restarting a stopped daemon") + } + return nil + } + + // 3. Ask the running daemon what version it is. An unreachable daemon is + // NOT a reason to stop — it is precisely the case that needs repairing. + socketPath := resolveSocketPath(ctx) + wasRunning := false + if st, _, statusErr := requestDaemonStatus(socketPath, 2*time.Second); statusErr == nil { + wasRunning = true + if model.NormalizeVersion(st.Version) == model.NormalizeVersion(pendingTag) { + // Already on the new version. Make sure the service manager agrees, + // then clear the marker. + if !daemonIsReady(ctx) { + _ = applyUpdateEnsureRunning(c, true) + } + clearPendingState() + return nil + } + } + + staged := model.GetStagedDaemonPath() + if _, statErr := os.Stat(staged); statErr != nil { + // Nothing staged — the Homebrew path, or a staged binary already + // consumed. Either way the daemon still runs the old inode and needs a + // restart to pick up the new binary. + return finishWithRunningDaemon(c, wasRunning, "no staged binary; restarting service") + } + + // 4. Never install a daemon that cannot report its own version. This catches + // truncated downloads, wrong-arch builds, and macOS signature failures. + if err := applyUpdateSmokeTest(ctx, staged, pendingTag); err != nil { + slog.Warn("staged daemon failed its smoke test; discarding", slog.Any("err", err)) + _ = os.Remove(staged) + // A bad download must not leave the user without a daemon. Restart + // first, then record why the update was abandoned — finishWithRunning + // Daemon clears LastError on success, so ordering matters here. + res := finishWithRunningDaemon(c, wasRunning, "discarded a bad staged binary") + recordApplyError(err) + return res + } + + // 5. Stop the service before swapping. CheckAndStopExistingService is the + // only stop the installer exposes; there is no Stop()/Restart(). + installer, instErr := buildDaemonInstaller() + if instErr == nil { + if err := applyUpdateStopService(installer); err != nil && wasRunning { + slog.Debug("stopping the daemon reported an error", slog.Any("err", err)) + } + } + + // 6. Activate the staged binary. + daemonDest := applyUpdateResolveDest() + if err := model.ReplaceBinaryWithBackupSuffix(staged, daemonDest, model.BackupSuffixUpdate); err != nil { + slog.Error("failed to activate staged daemon binary", slog.Any("err", err)) + recordApplyError(err) + // Leave the marker so a later shell retries, but bring the old daemon + // back up in the meantime. + return finishWithRunningDaemon(c, wasRunning, "swap failed; restoring previous daemon") + } + + // 7. MANDATORY: `daemon install` treats a ".bak" as a NEWER binary + // and restores it. A stale one from an older CLI would undo the swap we just + // made, and the daemon would re-download the same release forever. + _ = os.Remove(daemonDest + model.BackupSuffixLegacy) + _ = os.Remove(staged) + + // 8. Restart and verify. + if err := applyUpdateEnsureRunning(c, wasRunning); err != nil { + slog.Error("daemon did not come back up after update", slog.Any("err", err)) + recordApplyError(err) + // Leave the marker: the next shell retries. + return nil + } + + // 9. Done — clear the marker and the pending state. + clearPendingState() + slog.Info("daemon updated", slog.String("tag", pendingTag), slog.String("path", daemonDest)) + return nil +} + +// finishWithRunningDaemon restarts the service, clears the marker on success, +// and never propagates an error to the caller. +func finishWithRunningDaemon(c *cli.Context, wasRunning bool, reason string) error { + slog.Debug("ensuring daemon is running", slog.String("reason", reason)) + if err := applyUpdateEnsureRunning(c, wasRunning); err != nil { + slog.Error("daemon did not come back up", slog.String("reason", reason), slog.Any("err", err)) + recordApplyError(err) + return nil + } + clearPendingState() + return nil +} + +func clearPendingState() { + if err := model.ClearDaemonUpdatePending(); err != nil { + slog.Debug("could not clear pending marker", slog.Any("err", err)) + } + state, err := model.ReadUpdateState() + if err != nil { + return + } + state.PendingDaemonTag = "" + state.PendingDaemonPath = "" + state.LastError = "" + if err := model.WriteUpdateState(state); err != nil { + slog.Debug("could not clear pending state", slog.Any("err", err)) + } +} + +func recordApplyError(cause error) { + state, err := model.ReadUpdateState() + if err != nil { + return + } + state.LastError = cause.Error() + if err := model.WriteUpdateState(state); err != nil { + slog.Debug("could not record apply error", slog.Any("err", err)) + } +} diff --git a/commands/daemon.apply_update_test.go b/commands/daemon.apply_update_test.go new file mode 100644 index 0000000..cf13862 --- /dev/null +++ b/commands/daemon.apply_update_test.go @@ -0,0 +1,226 @@ +package commands + +import ( + "context" + "errors" + "flag" + "os" + "sync/atomic" + "testing" + + "github.com/malamtime/cli/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v2" +) + +type applyUpdateHarness struct { + ensureCalls atomic.Int32 + lastWasRunning atomic.Bool + ensureErr error + stopCalls atomic.Int32 + smokeErr error +} + +// newApplyUpdateHarness isolates state files and stubs every side effect. +func newApplyUpdateHarness(t *testing.T) *applyUpdateHarness { + t.Helper() + t.Setenv("HOME", t.TempDir()) + t.Setenv(model.DisableAutoUpdateEnv, "") + + h := &applyUpdateHarness{} + + prevEnsure := applyUpdateEnsureRunning + prevStop := applyUpdateStopService + prevSmoke := applyUpdateSmokeTest + prevDest := applyUpdateResolveDest + t.Cleanup(func() { + applyUpdateEnsureRunning = prevEnsure + applyUpdateStopService = prevStop + applyUpdateSmokeTest = prevSmoke + applyUpdateResolveDest = prevDest + }) + + // CRITICAL: resolveDaemonDest goes through exec.LookPath and would otherwise + // resolve to a real system-installed daemon (e.g. /opt/homebrew/bin), which + // the test would then overwrite. Always pin it under the temp HOME. + applyUpdateResolveDest = model.GetCurlInstallerDaemonPath + + applyUpdateEnsureRunning = func(_ *cli.Context, wasRunning bool) error { + h.ensureCalls.Add(1) + h.lastWasRunning.Store(wasRunning) + return h.ensureErr + } + applyUpdateStopService = func(model.DaemonInstaller) error { + h.stopCalls.Add(1) + return nil + } + applyUpdateSmokeTest = func(_ context.Context, _, _ string) error { + return h.smokeErr + } + return h +} + +func newTestCLIContext() *cli.Context { + app := cli.NewApp() + ctx := cli.NewContext(app, flag.NewFlagSet("test", flag.ContinueOnError), nil) + ctx.Context = context.Background() + return ctx +} + +// writeStagedDaemon creates a plausible staged binary. +func writeStagedDaemon(t *testing.T) string { + t.Helper() + path := model.GetStagedDaemonPath() + require.NoError(t, os.MkdirAll(model.GetBinFolderPath(), 0o755)) + require.NoError(t, os.WriteFile(path, []byte("staged-daemon-binary"), 0o755)) + return path +} + +func TestApplyUpdate_NoMarkerAndDaemonUpIsNoop(t *testing.T) { + h := newApplyUpdateHarness(t) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + assert.EqualValues(t, 0, h.ensureCalls.Load(), "nothing to do") +} + +// The self-heal path: no staged update, but the daemon is installed and down. +func TestApplyUpdate_NoMarkerRestartsDownDaemon(t *testing.T) { + h := newApplyUpdateHarness(t) + + // An installed-but-stopped service: service file present, no socket. + require.NoError(t, os.MkdirAll(model.GetStoragePath("daemon"), 0o755)) + require.NoError(t, os.WriteFile( + model.GetStoragePath("daemon", "shelltime.service"), []byte("[Unit]"), 0o644)) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + + // daemonIsReady consults the real socket path; when a daemon happens to be + // running on this machine there is nothing to restart, which is also correct. + if h.ensureCalls.Load() > 0 { + assert.False(t, h.lastWasRunning.Load(), "a stopped daemon must be installed, not reinstalled") + } +} + +func TestApplyUpdate_SwapsStagedBinaryAndRestarts(t *testing.T) { + h := newApplyUpdateHarness(t) + + staged := writeStagedDaemon(t) + daemonDest := model.GetCurlInstallerDaemonPath() + require.NoError(t, os.WriteFile(daemonDest, []byte("old-daemon"), 0o755)) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + require.NoError(t, model.WriteUpdateState(model.UpdateState{ + PendingDaemonTag: "v0.1.90", + PendingDaemonPath: staged, + })) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + + got, err := os.ReadFile(daemonDest) + require.NoError(t, err) + assert.Equal(t, "staged-daemon-binary", string(got), "the new daemon must be active") + + // MANDATORY: a leftover .bak would make `daemon install` restore the old + // binary, undoing this swap and causing a permanent update loop. + _, bakErr := os.Stat(daemonDest + model.BackupSuffixLegacy) + assert.True(t, os.IsNotExist(bakErr), "a .bak here would be restored by daemon install") + + _, stagedErr := os.Stat(staged) + assert.True(t, os.IsNotExist(stagedErr), "the staged binary should be consumed") + + assert.GreaterOrEqual(t, int(h.ensureCalls.Load()), 1, "the service must be brought back up") + + _, markerErr := model.ReadDaemonUpdatePending() + assert.Error(t, markerErr, "marker must be cleared on success") + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Empty(t, st.PendingDaemonTag) + assert.Empty(t, st.PendingDaemonPath) +} + +// A bad download must never leave the user without a daemon. +func TestApplyUpdate_FailedSmokeTestStillRestartsDaemon(t *testing.T) { + h := newApplyUpdateHarness(t) + h.smokeErr = errors.New("staged binary is corrupt") + + staged := writeStagedDaemon(t) + daemonDest := model.GetCurlInstallerDaemonPath() + require.NoError(t, os.WriteFile(daemonDest, []byte("old-daemon"), 0o755)) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + + assert.GreaterOrEqual(t, int(h.ensureCalls.Load()), 1, + "a failed smoke test must still leave the daemon running") + + got, err := os.ReadFile(daemonDest) + require.NoError(t, err) + assert.Equal(t, "old-daemon", string(got), "a corrupt binary must not be installed") + + _, stagedErr := os.Stat(staged) + assert.True(t, os.IsNotExist(stagedErr), "the bad staged binary should be discarded") + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Contains(t, st.LastError, "corrupt") +} + +// No staged binary is the Homebrew case: brew replaced the binary, but the +// running daemon still holds the old inode, so it must be restarted. +func TestApplyUpdate_NoStagedBinaryStillRestarts(t *testing.T) { + h := newApplyUpdateHarness(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + + assert.GreaterOrEqual(t, int(h.ensureCalls.Load()), 1) + _, markerErr := model.ReadDaemonUpdatePending() + assert.Error(t, markerErr, "marker cleared once the service is back up") +} + +// If the restart fails, the marker must survive so a later shell retries. +func TestApplyUpdate_KeepsMarkerWhenRestartFails(t *testing.T) { + h := newApplyUpdateHarness(t) + h.ensureErr = errors.New("launchctl refused") + + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext()), + "background repair must never return an error to the shell") + + tag, err := model.ReadDaemonUpdatePending() + require.NoError(t, err, "marker must survive so the next shell retries") + assert.Equal(t, "v0.1.90", tag) + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Contains(t, st.LastError, "launchctl refused") +} + +func TestApplyUpdate_KillSwitch(t *testing.T) { + h := newApplyUpdateHarness(t) + t.Setenv(model.DisableAutoUpdateEnv, "1") + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + writeStagedDaemon(t) + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + assert.EqualValues(t, 0, h.ensureCalls.Load()) + + _, err := model.ReadDaemonUpdatePending() + assert.NoError(t, err, "the kill switch pauses the update, it does not discard it") +} + +// N shells can spawn a repair at once; the lock must serialize them. +func TestApplyUpdate_RespectsUpdateLock(t *testing.T) { + h := newApplyUpdateHarness(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + writeStagedDaemon(t) + + release, ok := model.AcquireUpdateLock(0) + require.True(t, ok) + defer release() + + require.NoError(t, commandDaemonApplyUpdate(newTestCLIContext())) + assert.EqualValues(t, 0, h.ensureCalls.Load(), "must yield while another process holds the lock") +} diff --git a/commands/daemon.ensure.go b/commands/daemon.ensure.go new file mode 100644 index 0000000..673ba1e --- /dev/null +++ b/commands/daemon.ensure.go @@ -0,0 +1,162 @@ +package commands + +import ( + "context" + "fmt" + "log/slog" + "os" + "os/user" + "path/filepath" + "time" + + "github.com/malamtime/cli/daemon" + "github.com/malamtime/cli/model" + "github.com/urfave/cli/v2" +) + +// daemonReadyTimeout bounds how long ensureDaemonRunning waits for the service +// to actually come up. Package vars so tests can shrink them. +var ( + daemonReadyTimeout = 10 * time.Second + daemonReadyInterval = 200 * time.Millisecond +) + +// Seams for tests. +var ( + ensureInstallDaemon = commandDaemonInstall + ensureReinstallDaemon = commandDaemonReinstall +) + +// ensureDaemonRunning installs (or reinstalls) the daemon service and verifies +// it actually came up. It is safe to call whether or not the service is +// currently registered or running. +// +// Every code path that touches the daemon binary funnels through here, so a +// stopped daemon is always brought back — including when a swap or smoke test +// failed. This matters beyond convenience: the daemon is what drives the +// auto-update check, so a daemon that stays down takes auto-update with it. +func ensureDaemonRunning(c *cli.Context, wasRunning bool) error { + if wasRunning { + // A registered, running service needs the full unload/load cycle to pick + // up a new binary. + if err := ensureReinstallDaemon(c); err != nil { + slog.Warn("daemon reinstall failed, falling back to install", slog.Any("err", err)) + if err := ensureInstallDaemon(c); err != nil { + return fmt.Errorf("reinstall and install both failed: %w", err) + } + } + } else { + // Skip the uninstall half: `launchctl unload` / `systemctl disable` on a + // service that was never registered just produces noise and errors. + if err := ensureInstallDaemon(c); err != nil { + return fmt.Errorf("install daemon service: %w", err) + } + } + + return waitForDaemonReady(c.Context) +} + +// waitForDaemonReady polls until the service manager reports the service +// registered AND the daemon is listening on its socket. +// +// StartService() returning nil only means `launchctl load` / `systemctl start` +// was accepted — not that the process is alive and serving. If the first poll +// window expires we try one explicit StartService (covers "registered but not +// loaded") before giving up. +func waitForDaemonReady(ctx context.Context) error { + if daemonIsReady(ctx) { + return nil + } + if pollDaemonReady(ctx, daemonReadyTimeout) { + return nil + } + + installer, err := buildDaemonInstaller() + if err == nil { + if startErr := installer.StartService(); startErr != nil { + slog.Debug("explicit StartService failed", slog.Any("err", startErr)) + } + if pollDaemonReady(ctx, daemonReadyTimeout) { + return nil + } + } + + return fmt.Errorf("daemon did not become ready within %s", daemonReadyTimeout) +} + +func pollDaemonReady(ctx context.Context, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + select { + case <-ctx.Done(): + return false + case <-time.After(daemonReadyInterval): + } + if daemonIsReady(ctx) { + return true + } + } + return false +} + +// daemonIsReady reports whether the daemon is both registered with the service +// manager and accepting socket connections. The socket check is the meaningful +// one; the installer check catches "process alive but service unregistered". +func daemonIsReady(ctx context.Context) bool { + if !daemon.IsSocketReady(ctx, resolveSocketPath(ctx)) { + return false + } + installer, err := buildDaemonInstaller() + if err != nil { + // No installer on this platform — the socket answering is all we can check. + return true + } + return installer.Check() == nil +} + +// daemonServiceIsRunning reports whether the service manager currently considers +// the daemon running. Used to decide reinstall-vs-install. +func daemonServiceIsRunning() bool { + installer, err := buildDaemonInstaller() + if err != nil { + return false + } + return installer.Check() == nil +} + +func buildDaemonInstaller() (model.DaemonInstaller, error) { + currentUser, err := user.Current() + if err != nil { + return model.NewDaemonInstaller("", "", "") + } + baseFolder := filepath.Join(currentUser.HomeDir, ".shelltime") + return model.NewDaemonInstaller(baseFolder, currentUser.Username, "") +} + +// resolveSocketPath returns the configured socket path, falling back to the +// default when config is unavailable. configService is nil until InjectVar runs, +// so guard it: this is reached from background repair paths that must never +// panic in the user's shell. +func resolveSocketPath(ctx context.Context) string { + if configService == nil { + return model.DefaultSocketPath + } + cfg, err := configService.ReadConfigFile(ctx) + if err == nil && cfg.SocketPath != "" { + return cfg.SocketPath + } + return model.DefaultSocketPath +} + +// daemonServiceFileExists reports whether the daemon service definition has ever +// been installed. A clean `shelltime daemon uninstall` removes it, which is how +// we tell "the daemon crashed, restart it" apart from "the user deliberately +// removed it and we should stay out of the way". +func daemonServiceFileExists() bool { + for _, name := range []string{"xyz.shelltime.daemon.plist", "shelltime.service"} { + if _, err := os.Stat(model.GetStoragePath("daemon", name)); err == nil { + return true + } + } + return false +} diff --git a/commands/daemon.go b/commands/daemon.go index c98fbe5..740b057 100644 --- a/commands/daemon.go +++ b/commands/daemon.go @@ -10,5 +10,6 @@ var DaemonCommand *cli.Command = &cli.Command{ DaemonInstallCommand, DaemonUninstallCommand, DaemonReinstallCommand, + DaemonApplyUpdateCommand, }, } diff --git a/commands/detach_unix.go b/commands/detach_unix.go new file mode 100644 index 0000000..16b92d1 --- /dev/null +++ b/commands/detach_unix.go @@ -0,0 +1,15 @@ +//go:build !windows + +package commands + +import ( + "os/exec" + "syscall" +) + +// applyDetachAttrs puts the child in its own session, detaching it from the +// controlling terminal and process group. Without this, Ctrl-C or the shell +// exiting would kill a binary swap midway through. +func applyDetachAttrs(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} +} diff --git a/commands/detach_windows.go b/commands/detach_windows.go new file mode 100644 index 0000000..9138a27 --- /dev/null +++ b/commands/detach_windows.go @@ -0,0 +1,9 @@ +//go:build windows + +package commands + +import "os/exec" + +// applyDetachAttrs is a no-op on Windows: there is no daemon to repair there, so +// this path is never reached. +func applyDetachAttrs(cmd *exec.Cmd) {} diff --git a/commands/gc.go b/commands/gc.go index f86d89c..f00fe1e 100644 --- a/commands/gc.go +++ b/commands/gc.go @@ -216,6 +216,12 @@ func commandGC(c *cli.Context) error { } } + // gc runs once per new shell, which makes it the right place for the + // deterministic half of the self-update flow: apply a staged daemon update, + // surface an update notice, or restart a daemon that has stayed down. + // It never returns an error — a broken update must not break `gc`. + maybeRepairDaemonDrift(ctx, cfg) + // TODO: delete $HOME/.config/malamtime/ folder return nil diff --git a/commands/track.go b/commands/track.go index 124a0ab..e32dea8 100644 --- a/commands/track.go +++ b/commands/track.go @@ -58,6 +58,12 @@ var TrackCommand *cli.Command = &cli.Command{ } func commandTrack(c *cli.Context) error { + // Safety net for shells kept open for weeks, where `gc` never re-runs. + // Sampled at 1-in-N so the common path is a few nanoseconds and no syscalls; + // see checkDaemonDriftSampled for the cost breakdown. Runs before the span + // and logger setup so it cannot be slowed by them. + checkDaemonDriftSampled() + ctx, span := commandTracer.Start(c.Context, "track", trace.WithSpanKind(trace.SpanKindClient)) defer span.End() SetupLogger(os.ExpandEnv("$HOME/" + model.COMMAND_BASE_STORAGE_FOLDER)) diff --git a/commands/update.go b/commands/update.go index 4048c31..c5deb37 100644 --- a/commands/update.go +++ b/commands/update.go @@ -1,10 +1,8 @@ package commands import ( - "context" "fmt" "log/slog" - "os" "path/filepath" "runtime" @@ -31,6 +29,10 @@ var UpdateCommand *cli.Command = &cli.Command{ Name: "skip-daemon-reinstall", Usage: "Skip refreshing the daemon service after replacing binaries", }, + &cli.BoolFlag{ + Name: "allow-unverified", + Usage: "Install even when no checksum is available (not recommended)", + }, }, Action: commandUpdate, } @@ -42,6 +44,7 @@ func commandUpdate(c *cli.Context) error { check := c.Bool("check") force := c.Bool("force") skipDaemonReinstall := c.Bool("skip-daemon-reinstall") + allowUnverified := c.Bool("allow-unverified") color.Yellow.Println("🔍 Checking for updates...") @@ -50,20 +53,40 @@ func commandUpdate(c *cli.Context) error { return fmt.Errorf("resolve running binary path: %w", err) } - switch model.DetectInstallKind(cliPath) { - case model.InstallKindHomebrew: - color.Yellow.Println("📦 Detected Homebrew installation.") - color.Yellow.Println(" Run: brew upgrade shelltime/tap/shelltime") - return nil - case model.InstallKindUnknown: - color.Yellow.Printf("⚠️ Binary at %s is not in a known auto-updatable location.\n", cliPath) - color.Yellow.Println(" Reinstall via the curl installer or Homebrew to enable in-place updates.") - return nil + installKind := model.DetectInstallKind(cliPath) + + // Resolve the latest tag through the server when we can, so this works in + // regions that cannot reach github.com; fall back to the GitHub API. + cfg, cfgErr := configService.ReadConfigFile(ctx) + source := model.ReleaseSource{} + if cfgErr == nil { + source = model.NewReleaseSource(cfg.APIEndpoint) + } + + var ( + latest string + assetSha string + archiveName string + ) + if cfgErr == nil && cfg.Token != "" { + if rel, relErr := model.FetchLatestCLIRelease(ctx, cfg); relErr == nil && rel.Tag != "" { + latest = rel.Tag + if rel.Asset != nil { + assetSha = rel.Asset.Sha256 + archiveName = rel.Asset.Name + } + } else if relErr != nil { + slog.Debug("release lookup via API failed, falling back to GitHub", slog.Any("err", relErr)) + } } - - latest, err := model.FetchLatestVersion(ctx) - if err != nil { - return fmt.Errorf("fetch latest release: %w", err) + if latest == "" { + latest, err = model.FetchLatestVersion(ctx) + if err != nil { + return fmt.Errorf("fetch latest release: %w", err) + } + // Without the API we have no proxy-provided checksum, and the proxy may + // not be reachable either; use GitHub for the download too. + source = model.ReleaseSource{} } current := commitID @@ -85,6 +108,19 @@ func commandUpdate(c *cli.Context) error { return nil } + // Homebrew and unknown locations are reported after the version check, so + // `--check` still works there. + switch installKind { + case model.InstallKindHomebrew: + color.Yellow.Println("📦 Detected Homebrew installation.") + color.Yellow.Println(" Run: brew upgrade --cask shelltime/tap/shelltime") + return nil + case model.InstallKindUnknown: + color.Yellow.Printf("⚠️ Binary at %s is not in a known auto-updatable location.\n", cliPath) + color.Yellow.Println(" Reinstall via the curl installer or Homebrew to enable in-place updates.") + return nil + } + if current == "dev" && !force { color.Yellow.Println("⚠️ Refusing to overwrite a dev build. Use --force to proceed anyway.") return nil @@ -95,66 +131,54 @@ func commandUpdate(c *cli.Context) error { return nil } - archiveName, err := model.BuildArchiveName(runtime.GOOS, runtime.GOARCH) + manageDaemon := shouldManageDaemon(skipDaemonReinstall) + // Snapshot this BEFORE the swap: it decides reinstall-vs-install, and the + // swap itself makes the running service unreachable. + daemonWasRunning := manageDaemon && daemonServiceIsRunning() + + daemonDest := "" + if manageDaemon { + daemonDest = resolveDaemonDest() + } + + color.Yellow.Printf("⬇️ Downloading %s ...\n", latest) + res, err := model.ApplyUpdate(ctx, model.UpdatePlan{ + Tag: latest, + ArchiveName: archiveName, + Source: source, + ExpectedSha: assetSha, + CLIDest: cliPath, + DaemonDest: daemonDest, + // Interactive users may knowingly proceed without a checksum; the + // unattended daemon path never does. + AllowUnverified: allowUnverified, + BackupSuffix: model.BackupSuffixUpdate, + }) if err != nil { return err } - downloadURL := model.BuildDownloadURL(latest, archiveName) - - expectedSum, ok, err := model.FetchChecksum(ctx, latest, archiveName) - if err != nil { - color.Yellow.Printf("⚠️ Could not fetch checksums.txt: %v (proceeding without verification)\n", err) - } else if !ok { - color.Yellow.Println("⚠️ No checksum entry for this archive — proceeding without verification.") - } - tmpDir, err := os.MkdirTemp("", "shelltime-update-*") - if err != nil { - return fmt.Errorf("create temp dir: %w", err) + if !res.Verified { + color.Yellow.Println("⚠️ Installed without checksum verification (--allow-unverified).") } - defer os.RemoveAll(tmpDir) - - archivePath := filepath.Join(tmpDir, archiveName) - color.Yellow.Printf("⬇️ Downloading %s ...\n", archiveName) - if err := model.DownloadAndVerify(ctx, downloadURL, expectedSum, archivePath); err != nil { - return fmt.Errorf("download release: %w", err) - } - - extractDir := filepath.Join(tmpDir, "extracted") - if err := os.MkdirAll(extractDir, 0o755); err != nil { - return err - } - binaries, err := model.ExtractBinaries(archivePath, extractDir) - if err != nil { - return fmt.Errorf("extract archive: %w", err) - } - if _, ok := binaries["shelltime"]; !ok { - return fmt.Errorf("archive %s did not contain a shelltime binary", archiveName) - } - - color.Yellow.Println("🔄 Replacing binaries...") - - if err := model.ReplaceBinary(binaries["shelltime"], cliPath); err != nil { - return fmt.Errorf("replace shelltime binary: %w", err) + if res.UsedFallback { + color.Yellow.Println("ℹ️ Release proxy was unavailable; downloaded from GitHub directly.") } color.Green.Printf(" shelltime -> %s\n", cliPath) - - if daemonSrc, ok := binaries["shelltime-daemon"]; ok { - daemonDest := resolveDaemonDest() - if err := model.ReplaceBinary(daemonSrc, daemonDest); err != nil { - return fmt.Errorf("replace shelltime-daemon binary: %w", err) - } + if res.ReplacedDaemon { color.Green.Printf(" shelltime-daemon -> %s\n", daemonDest) } - if shouldReinstallDaemon(ctx, skipDaemonReinstall) { + if manageDaemon { color.Yellow.Println("🔁 Refreshing daemon service...") - if err := commandDaemonReinstall(c); err != nil { - color.Yellow.Printf("⚠️ Daemon reinstall reported an error: %v\n", err) - color.Yellow.Println(" You can rerun `shelltime daemon reinstall` manually.") + if err := ensureDaemonRunning(c, daemonWasRunning); err != nil { + color.Yellow.Printf("⚠️ Daemon did not come back up: %v\n", err) + color.Yellow.Println(" Run `shelltime daemon install` to start it manually.") + } else { + color.Green.Println(" daemon service is running") } } else { - color.Yellow.Println("ℹ️ Skipping daemon reinstall. Run `shelltime daemon reinstall` to pick up the new binary.") + color.Yellow.Println("ℹ️ Skipping daemon refresh. Run `shelltime daemon reinstall` to pick up the new binary.") } color.Green.Printf("✅ Updated to %s. Restart your shell to use the new binary.\n", latest) @@ -170,9 +194,14 @@ func resolveDaemonDest() string { return filepath.Join(model.GetBinFolderPath(), "shelltime-daemon") } -// shouldReinstallDaemon decides whether to call commandDaemonReinstall after a -// binary swap. -func shouldReinstallDaemon(_ context.Context, skipFlag bool) bool { +// shouldManageDaemon reports whether this machine has a daemon we are +// responsible for after a binary swap. +// +// It deliberately does NOT consider whether the service is currently running. +// It used to, which meant a stopped daemon was left stopped forever — and since +// the daemon drives the auto-update check, that also killed auto-update. Whether +// it is running now only decides reinstall-vs-install; see ensureDaemonRunning. +func shouldManageDaemon(skipFlag bool) bool { if skipFlag { return false } @@ -182,12 +211,8 @@ func shouldReinstallDaemon(_ context.Context, skipFlag bool) bool { if _, err := model.ResolveDaemonBinaryPath(); err != nil { return false } - installer, err := model.NewDaemonInstaller("", "", "") - if err != nil { - slog.Debug("skip daemon reinstall: installer factory failed", slog.Any("err", err)) - return false - } - if err := installer.Check(); err != nil { + if _, err := model.NewDaemonInstaller("", "", ""); err != nil { + slog.Debug("skip daemon management: installer factory failed", slog.Any("err", err)) return false } return true diff --git a/commands/update_drift.go b/commands/update_drift.go new file mode 100644 index 0000000..4e93516 --- /dev/null +++ b/commands/update_drift.go @@ -0,0 +1,163 @@ +package commands + +import ( + "context" + "log/slog" + "math/rand/v2" + "os" + "os/exec" + "runtime" + "time" + + "github.com/gookit/color" + "github.com/malamtime/cli/model" +) + +// updateDriftSampleRate makes 1-in-N `track` invocations pay for a single +// os.Stat. `track` runs on every command in every shell, so the common path +// must cost effectively nothing; 64 gives roughly 3 checks/day per shell at +// ~200 commands/day, which is ample since the daemon writes the marker at most +// once per release. A package var so tests can force it to 1. +var updateDriftSampleRate = 64 + +// noticeRepeatInterval bounds how often the same update notice is printed. +const noticeRepeatInterval = 24 * time.Hour + +// daemonStartRetryInterval rate-limits self-healing restarts so a daemon that +// cannot start is not respawned by every new shell. +const daemonStartRetryInterval = time.Hour + +// Seams for tests. +var ( + spawnDriftRepair = launchDetachedApplyUpdate + driftNow = time.Now + driftDaemonIsReady = daemonIsReady +) + +// checkDaemonDriftSampled is called at the very top of `track`, before any +// tracing or logging setup. +// +// Cost on the ~98% path: a constant comparison plus one rand.IntN — under 100ns +// with zero syscalls. On the sampled path it adds one stat(2) of a normally +// absent file (~2-5us). Both are orders of magnitude below what `track` already +// pays for process startup and its unix-socket dial. +func checkDaemonDriftSampled() { + if runtime.GOOS == "windows" { + return // no daemon on Windows + } + // math/rand/v2's global source is seeded randomly per process and takes no + // mutex. That matters here: every `track` is a fresh process, so a fixed + // seed would make every process sample identically. + if rand.IntN(updateDriftSampleRate) != 0 { + return + } + if os.Getenv(model.DisableAutoUpdateEnv) != "" { + return + } + if _, err := os.Stat(model.GetDaemonUpdatePendingPath()); err != nil { + return + } + spawnDriftRepair() +} + +// maybeRepairDaemonDrift runs once per new shell from `gc`. Unlike the sampled +// check it is deterministic, and it is the only place a user-visible notice is +// printed — `gc` runs at shell startup, so a line here cannot interleave with +// command output. +func maybeRepairDaemonDrift(ctx context.Context, cfg model.ShellTimeConfig) { + if runtime.GOOS == "windows" { + return + } + if os.Getenv(model.DisableAutoUpdateEnv) != "" { + return + } + if cfg.AutoUpdate == nil || cfg.AutoUpdate.Enabled == nil || !*cfg.AutoUpdate.Enabled { + return + } + + // A staged update waiting to be applied. + if _, err := os.Stat(model.GetDaemonUpdatePendingPath()); err == nil { + spawnDriftRepair() + return + } + + printPendingNotice() + maybeRestartDownDaemon(ctx) +} + +// printPendingNotice shows the daemon's "update available" message at most once +// a day. +func printPendingNotice() { + state, err := model.ReadUpdateState() + if err != nil || state.Notice == "" { + return + } + if driftNow().Sub(state.NoticeShownAt) < noticeRepeatInterval { + return + } + color.Yellow.Println("💡 " + state.Notice) + + state.NoticeShownAt = driftNow() + if err := model.WriteUpdateState(state); err != nil { + slog.Debug("could not stamp notice", slog.Any("err", err)) + } +} + +// maybeRestartDownDaemon restarts a daemon that is installed but not running. +// +// This closes a chicken-and-egg gap: the daemon is what performs the update +// check, so once it stays down, auto-update dies with it and nothing else would +// ever notice. +// +// Guarded so we never fight a user who deliberately stopped it: the service +// definition must still exist (a clean `daemon uninstall` removes it), and +// attempts are rate-limited with the same failure backoff as the update check. +func maybeRestartDownDaemon(ctx context.Context) { + if !daemonServiceFileExists() { + return // uninstalled on purpose; stay out of the way + } + if driftDaemonIsReady(ctx) { + return + } + + state, err := model.ReadUpdateState() + if err != nil { + return + } + if driftNow().Sub(state.LastDaemonStartAttemptAt) < daemonStartRetryInterval { + return + } + + state.LastDaemonStartAttemptAt = driftNow() + if err := model.WriteUpdateState(state); err != nil { + slog.Debug("could not stamp daemon start attempt", slog.Any("err", err)) + return + } + + slog.Debug("daemon is installed but not running; attempting restart") + spawnDriftRepair() +} + +// launchDetachedApplyUpdate re-execs this binary as `shelltime daemon +// apply-update` in its own session. +// +// Detached rather than inline because the repair runs launchctl/systemctl, which +// takes hundreds of milliseconds to seconds and prints progress — doing that in +// the shell hook would visibly stall the prompt and pollute it with output. +// Setsid also means Ctrl-C or the shell exiting cannot kill a half-finished +// binary swap. +func launchDetachedApplyUpdate() { + self, err := os.Executable() + if err != nil { + return + } + cmd := exec.Command(self, "daemon", "apply-update") + cmd.Stdin, cmd.Stdout, cmd.Stderr = nil, nil, nil + applyDetachAttrs(cmd) + if err := cmd.Start(); err != nil { + slog.Debug("could not spawn daemon apply-update", slog.Any("err", err)) + return + } + // Never Wait(): this must not outlive or block the hook. + _ = cmd.Process.Release() +} diff --git a/commands/update_drift_test.go b/commands/update_drift_test.go new file mode 100644 index 0000000..3f9813c --- /dev/null +++ b/commands/update_drift_test.go @@ -0,0 +1,199 @@ +package commands + +import ( + "context" + "os" + "sync/atomic" + "testing" + "time" + + "github.com/malamtime/cli/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// driftTestEnv isolates the state files and restores every seam. +func driftTestEnv(t *testing.T) { + t.Helper() + t.Setenv("HOME", t.TempDir()) + t.Setenv(model.DisableAutoUpdateEnv, "") + + prevRate := updateDriftSampleRate + prevSpawn := spawnDriftRepair + prevNow := driftNow + prevReady := driftDaemonIsReady + t.Cleanup(func() { + updateDriftSampleRate = prevRate + spawnDriftRepair = prevSpawn + driftNow = prevNow + driftDaemonIsReady = prevReady + }) + + // Force the sampled branch so tests are deterministic. + updateDriftSampleRate = 1 + // Don't let a real daemon on the dev machine influence the result. + driftDaemonIsReady = func(context.Context) bool { return false } +} + +func autoUpdateOnConfig() model.ShellTimeConfig { + on := true + return model.ShellTimeConfig{AutoUpdate: &model.AutoUpdate{Enabled: &on}} +} + +func TestCheckDaemonDriftSampled_NoMarkerDoesNothing(t *testing.T) { + driftTestEnv(t) + + var spawned atomic.Bool + spawnDriftRepair = func() { spawned.Store(true) } + + checkDaemonDriftSampled() + assert.False(t, spawned.Load(), "no marker means no repair") +} + +func TestCheckDaemonDriftSampled_MarkerTriggersRepair(t *testing.T) { + driftTestEnv(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + + var spawned atomic.Bool + spawnDriftRepair = func() { spawned.Store(true) } + + checkDaemonDriftSampled() + assert.True(t, spawned.Load()) +} + +func TestCheckDaemonDriftSampled_KillSwitch(t *testing.T) { + driftTestEnv(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + t.Setenv(model.DisableAutoUpdateEnv, "1") + + var spawned atomic.Bool + spawnDriftRepair = func() { spawned.Store(true) } + + checkDaemonDriftSampled() + assert.False(t, spawned.Load()) +} + +// The whole point of sampling is that `track` almost never does any work. +func TestCheckDaemonDriftSampled_SamplingSkipsMostInvocations(t *testing.T) { + driftTestEnv(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + updateDriftSampleRate = 64 + + var spawns atomic.Int32 + spawnDriftRepair = func() { spawns.Add(1) } + + const runs = 2000 + for range runs { + checkDaemonDriftSampled() + } + + got := spawns.Load() + // Expect ~31 of 2000. Generous bounds so this cannot flake. + assert.Greater(t, int(got), 0, "sampling should fire occasionally") + assert.Less(t, int(got), runs/8, "sampling should skip the vast majority of invocations") +} + +func TestMaybeRepairDaemonDrift_DisabledByConfig(t *testing.T) { + driftTestEnv(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + + var spawned atomic.Bool + spawnDriftRepair = func() { spawned.Store(true) } + + off := false + maybeRepairDaemonDrift(context.Background(), model.ShellTimeConfig{ + AutoUpdate: &model.AutoUpdate{Enabled: &off}, + }) + assert.False(t, spawned.Load(), "an opted-out user must not be touched") +} + +func TestMaybeRepairDaemonDrift_MarkerTriggersRepair(t *testing.T) { + driftTestEnv(t) + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) + + var spawned atomic.Bool + spawnDriftRepair = func() { spawned.Store(true) } + + maybeRepairDaemonDrift(context.Background(), autoUpdateOnConfig()) + assert.True(t, spawned.Load()) +} + +func TestPrintPendingNotice_ShownOncePerDay(t *testing.T) { + driftTestEnv(t) + + now := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC) + driftNow = func() time.Time { return now } + + require.NoError(t, model.WriteUpdateState(model.UpdateState{ + Notice: "shelltime v0.1.90 is available.", + })) + + printPendingNotice() + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.True(t, st.NoticeShownAt.Equal(now), "showing the notice must stamp the time") + + // A second call within the day must not re-stamp. + driftNow = func() time.Time { return now.Add(time.Hour) } + printPendingNotice() + + st2, err := model.ReadUpdateState() + require.NoError(t, err) + assert.True(t, st2.NoticeShownAt.Equal(now), "notice must not repeat within 24h") + + // A day later it shows again. + later := now.Add(25 * time.Hour) + driftNow = func() time.Time { return later } + printPendingNotice() + + st3, err := model.ReadUpdateState() + require.NoError(t, err) + assert.True(t, st3.NoticeShownAt.Equal(later)) +} + +func TestPrintPendingNotice_NoNoticeIsNoop(t *testing.T) { + driftTestEnv(t) + assert.NotPanics(t, func() { printPendingNotice() }) +} + +// A user who ran `daemon uninstall` must not have it silently reinstalled. +func TestMaybeRestartDownDaemon_SkipsWhenServiceFileAbsent(t *testing.T) { + driftTestEnv(t) + + var spawned atomic.Bool + spawnDriftRepair = func() { spawned.Store(true) } + + maybeRestartDownDaemon(context.Background()) + assert.False(t, spawned.Load(), "no service file means the user removed it deliberately") +} + +func TestMaybeRestartDownDaemon_RateLimited(t *testing.T) { + driftTestEnv(t) + + // Simulate an installed-but-stopped service. + daemonDir := model.GetStoragePath("daemon") + require.NoError(t, os.MkdirAll(daemonDir, 0o755)) + require.NoError(t, os.WriteFile( + model.GetStoragePath("daemon", "xyz.shelltime.daemon.plist"), []byte(""), 0o644)) + require.NoError(t, os.WriteFile( + model.GetStoragePath("daemon", "shelltime.service"), []byte("[Unit]"), 0o644)) + + now := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC) + driftNow = func() time.Time { return now } + + var spawns atomic.Int32 + spawnDriftRepair = func() { spawns.Add(1) } + + maybeRestartDownDaemon(context.Background()) + require.EqualValues(t, 1, spawns.Load(), "first attempt should fire") + + // Immediately again: rate limited. + maybeRestartDownDaemon(context.Background()) + assert.EqualValues(t, 1, spawns.Load(), "must not respawn on every shell") + + // An hour later it retries. + driftNow = func() time.Time { return now.Add(2 * time.Hour) } + maybeRestartDownDaemon(context.Background()) + assert.EqualValues(t, 2, spawns.Load()) +} diff --git a/daemon/auto_update.go b/daemon/auto_update.go new file mode 100644 index 0000000..0936122 --- /dev/null +++ b/daemon/auto_update.go @@ -0,0 +1,391 @@ +package daemon + +import ( + "context" + "fmt" + "log/slog" + "math/rand/v2" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/malamtime/cli/model" +) + +// DisableAutoUpdateEnv is an emergency kill switch that needs no config edit. +// Aliased from model so the CLI-side drift check and the daemon agree. +const DisableAutoUpdateEnv = model.DisableAutoUpdateEnv + +var ( + // AutoUpdateTickInterval is how often we wake up; AutoUpdateCheckInterval is + // how often we actually check. + // + // A plain 24h ticker would reset on every daemon restart, so a daemon that + // restarts more often than that would never check at all. Waking hourly and + // gating on a persisted LastCheckAt self-corrects across restarts and makes + // a crash loop harmless. + AutoUpdateTickInterval = 1 * time.Hour + AutoUpdateCheckInterval = 24 * time.Hour + + // autoUpdateJitter spreads a fleet of daemons out so they don't all hit the + // server in the same second after a release. + autoUpdateJitter = 30 * time.Minute + + // autoUpdateMaxBackoff caps the exponential backoff on repeated failures. + autoUpdateMaxBackoff = 7 * 24 * time.Hour +) + +// Seams for tests, mirroring loadCodexAuthFunc/fetchCodexUsageFunc. +var ( + autoUpdateFetchLatest = model.FetchLatestCLIRelease + autoUpdateApply = model.ApplyUpdate + autoUpdateBrewUpgrade = runBrewUpgrade + autoUpdateNow = time.Now + autoUpdateResolveCLIPath = func() (string, error) { + return model.ResolveCLIBinaryPathFrom(model.NewCommandService()) + } +) + +// AutoUpdateService checks for a new CLI release once a day and installs it. +// +// It deliberately updates only the CLI binary and *stages* the daemon binary: +// the running daemon must not rewrite the binary it is executing, and staging +// means the CLI's later repair is a local rename with no network access inside +// the shell hook. +type AutoUpdateService struct { + config model.ShellTimeConfig + ticker *time.Ticker + stopChan chan struct{} + wg sync.WaitGroup +} + +func NewAutoUpdateService(config model.ShellTimeConfig) *AutoUpdateService { + return &AutoUpdateService{ + config: config, + stopChan: make(chan struct{}), + } +} + +// Start begins the periodic update check. Like the cleanup timer, it does NOT +// run immediately at startup: a restart loop would otherwise hammer the server +// and re-download on every boot. +func (s *AutoUpdateService) Start(ctx context.Context) error { + s.ticker = time.NewTicker(AutoUpdateTickInterval) + s.wg.Add(1) + + go func() { + defer s.wg.Done() + for { + select { + case <-s.ticker.C: + s.tick(ctx) + case <-s.stopChan: + return + case <-ctx.Done(): + return + } + } + }() + + slog.Info("Auto update service started", + slog.Duration("tick", AutoUpdateTickInterval), + slog.Duration("checkInterval", s.checkInterval())) + return nil +} + +func (s *AutoUpdateService) Stop() { + if s.ticker != nil { + s.ticker.Stop() + } + close(s.stopChan) + s.wg.Wait() + slog.Info("Auto update service stopped") +} + +func (s *AutoUpdateService) checkInterval() time.Duration { + if s.config.AutoUpdate != nil && s.config.AutoUpdate.IntervalHours > 0 { + return time.Duration(s.config.AutoUpdate.IntervalHours) * time.Hour + } + return AutoUpdateCheckInterval +} + +// tick decides whether a check is due and runs it. +func (s *AutoUpdateService) tick(ctx context.Context) { + state, err := model.ReadUpdateState() + if err != nil { + slog.Debug("could not read update state", slog.Any("err", err)) + } + if !s.shouldCheckNow(state) { + return + } + if err := s.runCheck(ctx, state); err != nil { + slog.Warn("auto update check failed", slog.Any("err", err)) + } +} + +// shouldCheckNow gates on the persisted LastCheckAt plus a failure backoff. +func (s *AutoUpdateService) shouldCheckNow(state model.UpdateState) bool { + now := autoUpdateNow() + if state.LastCheckAt.IsZero() { + return true + } + // Clock skew (or a restored backup) could park LastCheckAt in the future; + // don't let that disable updates forever. + if state.LastCheckAt.After(now) { + return true + } + + gap := s.checkInterval() * time.Duration(backoffFactor(state.ConsecutiveFailures)) + if gap > autoUpdateMaxBackoff { + gap = autoUpdateMaxBackoff + } + // Jitter so a fleet doesn't stampede after a release. + gap += rand.N(autoUpdateJitter) + + return !now.Before(state.LastCheckAt.Add(gap)) +} + +func backoffFactor(failures int) int { + if failures <= 0 { + return 1 + } + if failures > 3 { + failures = 3 + } + return 1 << failures +} + +// runCheck performs one full check-and-maybe-install cycle. +func (s *AutoUpdateService) runCheck(ctx context.Context, state model.UpdateState) error { + if os.Getenv(DisableAutoUpdateEnv) != "" { + return nil + } + // An empty token would send `Authorization: CLI ` and get a 401, whereas no + // header at all is treated as anonymous. Skip rather than fail. + if s.config.Token == "" { + return nil + } + + current := GetVersion() + if current == "" || current == "dev" { + slog.Debug("skipping auto update for a dev build") + return nil + } + + rel, err := autoUpdateFetchLatest(ctx, s.config) + if err != nil { + return s.recordFailure(state, fmt.Errorf("fetch latest release: %w", err)) + } + if rel.Tag == "" { + return s.recordFailure(state, fmt.Errorf("server returned an empty release tag")) + } + + state.LastCheckAt = autoUpdateNow() + state.LastKnownTag = rel.Tag + state.ConsecutiveFailures = 0 + state.LastError = "" + + // Only ever move forward. A server bug reporting an old tag must not + // downgrade the user. + if model.CompareVersions(rel.Tag, current) <= 0 { + state.Notice = "" + return model.WriteUpdateState(state) + } + + // Already downloaded this release; the marker is waiting for a shell. + if state.LastAppliedTag == rel.Tag { + return model.WriteUpdateState(state) + } + + cliPath, err := autoUpdateResolveCLIPath() + if err != nil { + state.Notice = fmt.Sprintf("shelltime %s is available, but the CLI binary could not be located.", rel.Tag) + return model.WriteUpdateState(state) + } + + switch model.DetectInstallKind(cliPath) { + case model.InstallKindHomebrew: + return s.handleHomebrew(ctx, state, rel.Tag) + case model.InstallKindUnknown: + // Never write into a location we don't recognize. + state.Notice = fmt.Sprintf( + "shelltime %s is available. Run `shelltime update` to upgrade (binary at %s).", rel.Tag, cliPath) + return model.WriteUpdateState(state) + } + + if s.notifyOnly() { + state.Notice = fmt.Sprintf("shelltime %s is available. Run `shelltime update` to install it.", rel.Tag) + return model.WriteUpdateState(state) + } + + // Preflight: can we even write there? This is the /usr/local/bin-without-sudo + // case. The daemon must never attempt sudo. + if err := checkWritable(filepath.Dir(cliPath)); err != nil { + state.Notice = fmt.Sprintf( + "shelltime %s is available, but %s is not writable. Run `sudo shelltime update`.", + rel.Tag, filepath.Dir(cliPath)) + return model.WriteUpdateState(state) + } + + release, ok := model.AcquireUpdateLock(10 * time.Minute) + if !ok { + slog.Debug("another process holds the update lock; skipping this cycle") + return model.WriteUpdateState(state) + } + defer release() + + archiveName := "" + expectedSha := "" + if rel.Asset != nil { + archiveName = rel.Asset.Name + expectedSha = rel.Asset.Sha256 + } + + res, err := autoUpdateApply(ctx, model.UpdatePlan{ + Tag: rel.Tag, + ArchiveName: archiveName, + Source: model.NewReleaseSource(s.config.APIEndpoint), + ExpectedSha: expectedSha, + CLIDest: cliPath, + // Stage, don't activate: we are the running daemon. + DaemonStagePath: model.GetStagedDaemonPath(), + BackupSuffix: model.BackupSuffixUpdate, + }) + if err != nil { + return s.recordFailure(state, fmt.Errorf("apply update %s: %w", rel.Tag, err)) + } + + state.LastAppliedTag = rel.Tag + state.PendingDaemonTag = rel.Tag + state.Notice = "" + if res.StagedDaemon { + state.PendingDaemonPath = model.GetStagedDaemonPath() + } else { + state.PendingDaemonPath = "" + } + + // State first, marker last: the CLI must never see a marker without the + // state that explains it. + if err := model.WriteUpdateState(state); err != nil { + return err + } + if err := model.WriteDaemonUpdatePending(rel.Tag); err != nil { + return err + } + + slog.Info("CLI updated; daemon restart pending", + slog.String("tag", rel.Tag), + slog.Bool("stagedDaemon", res.StagedDaemon), + slog.Bool("usedFallback", res.UsedFallback)) + return nil +} + +// handleHomebrew either runs brew (opt-in) or records a notice. +func (s *AutoUpdateService) handleHomebrew(ctx context.Context, state model.UpdateState, tag string) error { + if !s.homebrewEnabled() || s.notifyOnly() { + state.Notice = fmt.Sprintf( + "shelltime %s is available. Run: brew upgrade --cask shelltime/tap/shelltime", tag) + return model.WriteUpdateState(state) + } + + release, ok := model.AcquireUpdateLock(20 * time.Minute) + if !ok { + return model.WriteUpdateState(state) + } + defer release() + + if err := autoUpdateBrewUpgrade(ctx, model.NewCommandService()); err != nil { + return s.recordFailure(state, fmt.Errorf("brew upgrade: %w", err)) + } + + state.LastAppliedTag = tag + state.PendingDaemonTag = tag + // brew owns the binary; the repair only needs to restart the service so the + // running daemon stops holding the old inode. + state.PendingDaemonPath = "" + state.Notice = "" + + if err := model.WriteUpdateState(state); err != nil { + return err + } + return model.WriteDaemonUpdatePending(tag) +} + +func (s *AutoUpdateService) homebrewEnabled() bool { + return s.config.AutoUpdate != nil && + s.config.AutoUpdate.Homebrew != nil && + *s.config.AutoUpdate.Homebrew +} + +func (s *AutoUpdateService) notifyOnly() bool { + return s.config.AutoUpdate != nil && + s.config.AutoUpdate.NotifyOnly != nil && + *s.config.AutoUpdate.NotifyOnly +} + +func (s *AutoUpdateService) recordFailure(state model.UpdateState, cause error) error { + state.LastCheckAt = autoUpdateNow() + state.ConsecutiveFailures++ + state.LastError = cause.Error() + if writeErr := model.WriteUpdateState(state); writeErr != nil { + slog.Debug("could not persist update failure", slog.Any("err", writeErr)) + } + return cause +} + +// checkWritable verifies we can create a file in dir, without leaving one behind. +func checkWritable(dir string) error { + f, err := os.CreateTemp(dir, ".shelltime-update-probe-*") + if err != nil { + return err + } + name := f.Name() + _ = f.Close() + return os.Remove(name) +} + +// runBrewUpgrade upgrades the cask, falling back to the formula form. +// +// The daemon runs under launchd/systemd with a stripped PATH and no terminal, so +// brew must be located explicitly and can never be allowed to prompt. +func runBrewUpgrade(ctx context.Context, cs model.CommandService) error { + brew, err := cs.LookPath("brew") + if err != nil { + return fmt.Errorf("brew not found: %w", err) + } + + ctx, cancel := context.WithTimeout(ctx, 15*time.Minute) + defer cancel() + + env := append(os.Environ(), + "HOMEBREW_NO_AUTO_UPDATE=1", + "HOMEBREW_NO_ANALYTICS=1", + "HOMEBREW_NO_INSTALL_CLEANUP=1", + "NONINTERACTIVE=1", + ) + + attempts := [][]string{ + {"upgrade", "--cask", "shelltime/tap/shelltime"}, + {"upgrade", "shelltime/tap/shelltime"}, + } + + var lastErr error + for _, args := range attempts { + cmd := exec.CommandContext(ctx, brew, args...) + cmd.Env = env + cmd.Stdin = nil // never block waiting for input + out, err := cmd.CombinedOutput() + if err == nil { + slog.Info("brew upgrade succeeded", + slog.String("args", strings.Join(args, " ")), + slog.String("output", strings.TrimSpace(string(out)))) + return nil + } + lastErr = fmt.Errorf("%s: %w (%s)", strings.Join(args, " "), err, strings.TrimSpace(string(out))) + slog.Debug("brew attempt failed", slog.Any("err", lastErr)) + } + return lastErr +} diff --git a/daemon/auto_update_test.go b/daemon/auto_update_test.go new file mode 100644 index 0000000..bf2c3fc --- /dev/null +++ b/daemon/auto_update_test.go @@ -0,0 +1,436 @@ +package daemon + +import ( + "context" + "errors" + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/malamtime/cli/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func ptrBool(b bool) *bool { return &b } + +// autoUpdateTestHome isolates the state/marker files and restores every package +// seam the service uses. +func autoUpdateTestHome(t *testing.T) { + t.Helper() + t.Setenv("HOME", t.TempDir()) + t.Setenv(DisableAutoUpdateEnv, "") + + prevFetch := autoUpdateFetchLatest + prevApply := autoUpdateApply + prevBrew := autoUpdateBrewUpgrade + prevResolve := autoUpdateResolveCLIPath + prevNow := autoUpdateNow + prevJitter := autoUpdateJitter + prevVersion := version + + t.Cleanup(func() { + autoUpdateFetchLatest = prevFetch + autoUpdateApply = prevApply + autoUpdateBrewUpgrade = prevBrew + autoUpdateResolveCLIPath = prevResolve + autoUpdateNow = prevNow + autoUpdateJitter = prevJitter + version = prevVersion + }) + + // Deterministic gating in tests. + autoUpdateJitter = time.Nanosecond + version = "0.1.89" +} + +func enabledAutoUpdateConfig() model.ShellTimeConfig { + return model.ShellTimeConfig{ + Token: "test-token", + APIEndpoint: "https://api.example.com", + AutoUpdate: &model.AutoUpdate{ + Enabled: ptrBool(true), + Homebrew: ptrBool(false), + IntervalHours: 24, + }, + } +} + +func TestNewAutoUpdateService(t *testing.T) { + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NotNil(t, svc) + assert.NotNil(t, svc.stopChan) + assert.Equal(t, "test-token", svc.config.Token) +} + +func TestAutoUpdateService_StartStop(t *testing.T) { + autoUpdateTestHome(t) + prevTick := AutoUpdateTickInterval + AutoUpdateTickInterval = 10 * time.Millisecond + defer func() { AutoUpdateTickInterval = prevTick }() + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.Start(context.Background())) + assert.NotNil(t, svc.ticker) + svc.Stop() +} + +func TestAutoUpdateService_StopWithoutStart(t *testing.T) { + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + assert.NotPanics(t, func() { svc.Stop() }) +} + +func TestAutoUpdateService_ContextCancellation(t *testing.T) { + autoUpdateTestHome(t) + prevTick := AutoUpdateTickInterval + AutoUpdateTickInterval = 10 * time.Millisecond + defer func() { AutoUpdateTickInterval = prevTick }() + + ctx, cancel := context.WithCancel(context.Background()) + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.Start(ctx)) + cancel() + time.Sleep(30 * time.Millisecond) + svc.Stop() +} + +// A daemon that restarts more often than the check interval must still check; +// that is why the gate is a persisted timestamp rather than the ticker. +func TestAutoUpdateService_ShouldCheckNow(t *testing.T) { + autoUpdateTestHome(t) + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + now := time.Date(2026, 8, 7, 12, 0, 0, 0, time.UTC) + autoUpdateNow = func() time.Time { return now } + + t.Run("never checked", func(t *testing.T) { + assert.True(t, svc.shouldCheckNow(model.UpdateState{})) + }) + + t.Run("checked recently", func(t *testing.T) { + assert.False(t, svc.shouldCheckNow(model.UpdateState{ + LastCheckAt: now.Add(-1 * time.Hour), + })) + }) + + t.Run("checked long ago", func(t *testing.T) { + assert.True(t, svc.shouldCheckNow(model.UpdateState{ + LastCheckAt: now.Add(-25 * time.Hour), + })) + }) + + t.Run("failures back off", func(t *testing.T) { + // 3 failures => 8x the 24h interval, so 25h ago is not yet due. + assert.False(t, svc.shouldCheckNow(model.UpdateState{ + LastCheckAt: now.Add(-25 * time.Hour), + ConsecutiveFailures: 3, + })) + assert.True(t, svc.shouldCheckNow(model.UpdateState{ + LastCheckAt: now.Add(-200 * time.Hour), + ConsecutiveFailures: 3, + })) + }) + + t.Run("future timestamp does not wedge updates", func(t *testing.T) { + assert.True(t, svc.shouldCheckNow(model.UpdateState{ + LastCheckAt: now.Add(48 * time.Hour), + })) + }) +} + +func TestBackoffFactor(t *testing.T) { + assert.Equal(t, 1, backoffFactor(0)) + assert.Equal(t, 1, backoffFactor(-1)) + assert.Equal(t, 2, backoffFactor(1)) + assert.Equal(t, 4, backoffFactor(2)) + assert.Equal(t, 8, backoffFactor(3)) + assert.Equal(t, 8, backoffFactor(99), "backoff is capped") +} + +func TestAutoUpdateService_RunCheck_SkipsWhenNoToken(t *testing.T) { + autoUpdateTestHome(t) + var called atomic.Bool + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + called.Store(true) + return model.LatestCLIRelease{}, nil + } + + cfg := enabledAutoUpdateConfig() + cfg.Token = "" + svc := NewAutoUpdateService(cfg) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + assert.False(t, called.Load(), "an empty token would 401; skip instead") +} + +func TestAutoUpdateService_RunCheck_SkipsDevBuild(t *testing.T) { + autoUpdateTestHome(t) + version = "dev" + + var called atomic.Bool + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + called.Store(true) + return model.LatestCLIRelease{}, nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + assert.False(t, called.Load(), "dev builds must never auto-update") +} + +func TestAutoUpdateService_RunCheck_KillSwitch(t *testing.T) { + autoUpdateTestHome(t) + t.Setenv(DisableAutoUpdateEnv, "1") + + var called atomic.Bool + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + called.Store(true) + return model.LatestCLIRelease{}, nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + assert.False(t, called.Load()) +} + +// A server bug reporting an old tag must never downgrade the user. +func TestAutoUpdateService_RunCheck_NeverDowngrades(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + var applied atomic.Bool + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.50"}, nil + } + autoUpdateApply = func(context.Context, model.UpdatePlan) (model.UpdateResult, error) { + applied.Store(true) + return model.UpdateResult{}, nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + assert.False(t, applied.Load(), "an older tag must not trigger an install") + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Equal(t, "v0.1.50", st.LastKnownTag) + assert.False(t, st.LastCheckAt.IsZero()) +} + +func TestAutoUpdateService_RunCheck_SameVersionIsNoop(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + var applied atomic.Bool + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.89"}, nil + } + autoUpdateApply = func(context.Context, model.UpdatePlan) (model.UpdateResult, error) { + applied.Store(true) + return model.UpdateResult{}, nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + assert.False(t, applied.Load()) +} + +func TestAutoUpdateService_RunCheck_StagesDaemonAndWritesMarker(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + cliPath := filepath.Join(model.GetBinFolderPath(), "shelltime") + autoUpdateResolveCLIPath = func() (string, error) { return cliPath, nil } + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{ + Tag: "v0.1.90", + Asset: &model.CLIReleaseAssetInfo{ + Name: "cli_Darwin_arm64.zip", + Sha256: "deadbeef", + }, + }, nil + } + + var gotPlan model.UpdatePlan + autoUpdateApply = func(_ context.Context, plan model.UpdatePlan) (model.UpdateResult, error) { + gotPlan = plan + return model.UpdateResult{Tag: plan.Tag, ReplacedCLI: true, StagedDaemon: true}, nil + } + + // The writability preflight needs the bin dir to exist. + require.NoError(t, os.MkdirAll(model.GetBinFolderPath(), 0o755)) + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + + assert.Equal(t, "v0.1.90", gotPlan.Tag) + assert.Equal(t, cliPath, gotPlan.CLIDest) + assert.Equal(t, "deadbeef", gotPlan.ExpectedSha) + assert.Equal(t, model.GetStagedDaemonPath(), gotPlan.DaemonStagePath) + assert.Empty(t, gotPlan.DaemonDest, "the running daemon must not activate its own replacement") + assert.False(t, gotPlan.AllowUnverified, "the unattended path must fail closed") + assert.Equal(t, model.BackupSuffixUpdate, gotPlan.BackupSuffix) + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Equal(t, "v0.1.90", st.LastAppliedTag) + assert.Equal(t, "v0.1.90", st.PendingDaemonTag) + assert.Equal(t, model.GetStagedDaemonPath(), st.PendingDaemonPath) + + tag, err := model.ReadDaemonUpdatePending() + require.NoError(t, err) + assert.Equal(t, "v0.1.90", tag) +} + +func TestAutoUpdateService_RunCheck_NotifyOnly(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + cliPath := filepath.Join(model.GetBinFolderPath(), "shelltime") + autoUpdateResolveCLIPath = func() (string, error) { return cliPath, nil } + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.90"}, nil + } + var applied atomic.Bool + autoUpdateApply = func(context.Context, model.UpdatePlan) (model.UpdateResult, error) { + applied.Store(true) + return model.UpdateResult{}, nil + } + + cfg := enabledAutoUpdateConfig() + cfg.AutoUpdate.NotifyOnly = ptrBool(true) + + svc := NewAutoUpdateService(cfg) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + + assert.False(t, applied.Load()) + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Contains(t, st.Notice, "v0.1.90") + _, err = model.ReadDaemonUpdatePending() + assert.Error(t, err, "notify-only must not write a pending marker") +} + +// Homebrew is opt-in: by default we only tell the user what to run. +func TestAutoUpdateService_RunCheck_HomebrewNotifiesByDefault(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + autoUpdateResolveCLIPath = func() (string, error) { + return "/opt/homebrew/Caskroom/shelltime/0.1.89/shelltime", nil + } + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.90"}, nil + } + var brewRan atomic.Bool + autoUpdateBrewUpgrade = func(context.Context, model.CommandService) error { + brewRan.Store(true) + return nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + + assert.False(t, brewRan.Load(), "brew must not run without opt-in") + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Contains(t, st.Notice, "brew upgrade --cask") +} + +func TestAutoUpdateService_RunCheck_HomebrewOptIn(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + autoUpdateResolveCLIPath = func() (string, error) { + return "/usr/local/Caskroom/shelltime/0.1.89/shelltime", nil + } + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.90"}, nil + } + var brewRan atomic.Bool + autoUpdateBrewUpgrade = func(context.Context, model.CommandService) error { + brewRan.Store(true) + return nil + } + + cfg := enabledAutoUpdateConfig() + cfg.AutoUpdate.Homebrew = ptrBool(true) + + svc := NewAutoUpdateService(cfg) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + + assert.True(t, brewRan.Load()) + + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Equal(t, "v0.1.90", st.PendingDaemonTag) + assert.Empty(t, st.PendingDaemonPath, "brew owns the binary; only a restart is needed") + + // A brew upgrade still leaves the running daemon on the old inode, so the + // restart marker must be written. + tag, err := model.ReadDaemonUpdatePending() + require.NoError(t, err) + assert.Equal(t, "v0.1.90", tag) +} + +func TestAutoUpdateService_RunCheck_UnknownInstallLocationOnlyNotifies(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + autoUpdateResolveCLIPath = func() (string, error) { return "/some/random/path/shelltime", nil } + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.90"}, nil + } + var applied atomic.Bool + autoUpdateApply = func(context.Context, model.UpdatePlan) (model.UpdateResult, error) { + applied.Store(true) + return model.UpdateResult{}, nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{})) + + assert.False(t, applied.Load(), "never write into an unrecognized location") + st, err := model.ReadUpdateState() + require.NoError(t, err) + assert.Contains(t, st.Notice, "v0.1.90") +} + +func TestAutoUpdateService_RunCheck_RecordsFailureAndBacksOff(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{}, errors.New("network is down") + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + err := svc.runCheck(context.Background(), model.UpdateState{}) + require.Error(t, err) + + st, readErr := model.ReadUpdateState() + require.NoError(t, readErr) + assert.Equal(t, 1, st.ConsecutiveFailures) + assert.Contains(t, st.LastError, "network is down") + assert.False(t, st.LastCheckAt.IsZero(), "a failed check still stamps LastCheckAt so we back off") +} + +func TestAutoUpdateService_RunCheck_SkipsAlreadyAppliedTag(t *testing.T) { + autoUpdateTestHome(t) + version = "0.1.89" + + autoUpdateFetchLatest = func(context.Context, model.ShellTimeConfig) (model.LatestCLIRelease, error) { + return model.LatestCLIRelease{Tag: "v0.1.90"}, nil + } + var applied atomic.Bool + autoUpdateApply = func(context.Context, model.UpdatePlan) (model.UpdateResult, error) { + applied.Store(true) + return model.UpdateResult{}, nil + } + + svc := NewAutoUpdateService(enabledAutoUpdateConfig()) + require.NoError(t, svc.runCheck(context.Background(), model.UpdateState{ + LastAppliedTag: "v0.1.90", + })) + assert.False(t, applied.Load(), "must not re-download a release already staged") +} diff --git a/docs/CONFIG.md b/docs/CONFIG.md index cbe2ee5..dfc76fe 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -364,6 +364,53 @@ logCleanup: Cleanup runs every 24 hours when daemon is active. +### Auto Update + +The daemon checks once a day for a new CLI release and installs it in place. +Enabled by default. + +| Option | Type | Default | Description | +|--------|------|---------|-------------| +| `autoUpdate.enabled` | boolean | `true` | Enable the daily check. Set `false` to opt out entirely | +| `autoUpdate.homebrew` | boolean | `false` | Allow the daemon to run `brew upgrade --cask` on Homebrew installs | +| `autoUpdate.notifyOnly` | boolean | `false` | Report available updates without downloading anything | +| `autoUpdate.intervalHours` | integer | `24` | How often to check (minimum 1) | +| `autoUpdate.channel` | string | `stable` | Reserved for pre-release opt-in | + +```yaml +autoUpdate: + enabled: true + homebrew: false # opt in to automatic `brew upgrade --cask` + notifyOnly: false + intervalHours: 24 +``` + +**How it works:** + +1. The daemon asks the shelltime API for the latest release once a day. +2. Binaries are downloaded through `api.shelltime.xyz`, not `github.com`, so + updating works in regions where GitHub is unreachable. If the proxy is + unavailable the CLI falls back to GitHub. +3. The download is checksum-verified and the new binary must report its own + version before it is installed. The unattended path refuses to install + anything it could not verify. +4. The `shelltime` binary is replaced in place; the new daemon binary is staged + at `~/.shelltime/bin/shelltime-daemon.next`. +5. On your next new shell the CLI activates the staged daemon and restarts the + service. If the daemon is not running at that point, it is started. + +**Notes:** + +- Homebrew installs only print an upgrade hint unless `autoUpdate.homebrew` is + `true`; `brew` can prompt and touches files the daemon does not own. +- Binaries in unrecognized locations are never overwritten — you get a notice + instead. The daemon never uses `sudo`. +- Dev builds are never auto-updated, and an older release is never installed + over a newer one. +- `SHELLTIME_DISABLE_AUTO_UPDATE=1` disables everything without a config edit. +- `shelltime update` still works for on-demand updates; `shelltime update + --check` only reports. + ### Metrics Collection | Option | Type | Default | @@ -437,6 +484,12 @@ logCleanup: enabled: true thresholdMB: 100 +# --- Auto Update --- +autoUpdate: + enabled: true + homebrew: false # opt in to automatic `brew upgrade --cask` + intervalHours: 24 + # --- Advanced --- socketPath: "/tmp/shelltime.sock" enableMetrics: false diff --git a/model/cli_release.go b/model/cli_release.go new file mode 100644 index 0000000..56772b6 --- /dev/null +++ b/model/cli_release.go @@ -0,0 +1,86 @@ +package model + +import ( + "context" + "runtime" + "time" +) + +// FetchLatestCLIReleaseQuery asks the server for the latest release plus the +// archive for this platform. downloadURL/checksumsURL come back pointing at the +// server's proxy, so a client that cannot reach github.com can still update. +const FetchLatestCLIReleaseQuery = `query fetchLatestCLIRelease($goos: String!, $goarch: String!) { + metaData { + cliVersion + latestCLIRelease { + tag + publishedAt + checksumsURL + asset(goos: $goos, goarch: $goarch) { + name + size + sha256 + downloadURL + } + } + } +}` + +// CLIReleaseAssetInfo is one platform's release archive. +type CLIReleaseAssetInfo struct { + Name string `json:"name"` + Size int64 `json:"size"` + Sha256 string `json:"sha256"` + DownloadURL string `json:"downloadURL"` +} + +// LatestCLIRelease is the newest published CLI release. +type LatestCLIRelease struct { + Tag string `json:"tag"` + PublishedAt time.Time `json:"publishedAt"` + ChecksumsURL string `json:"checksumsURL"` + Asset *CLIReleaseAssetInfo `json:"asset"` +} + +type latestCLIReleaseData struct { + MetaData struct { + CliVersion string `json:"cliVersion"` + LatestCLIRelease LatestCLIRelease `json:"latestCLIRelease"` + } `json:"metaData"` +} + +// FetchLatestCLIRelease queries the server for the latest release for the +// running platform. +func FetchLatestCLIRelease(ctx context.Context, config ShellTimeConfig) (LatestCLIRelease, error) { + ctx, span := modelTracer.Start(ctx, "cliRelease.fetchLatest") + defer span.End() + + goos, goarch := CurrentPlatform() + + var result GraphQLResponse[latestCLIReleaseData] + err := SendGraphQLRequest(GraphQLRequestOptions[GraphQLResponse[latestCLIReleaseData]]{ + Context: ctx, + Endpoint: Endpoint{ + Token: config.Token, + APIEndpoint: config.APIEndpoint, + }, + Query: FetchLatestCLIReleaseQuery, + Variables: map[string]interface{}{ + "goos": goos, + "goarch": goarch, + }, + Response: &result, + Timeout: 10 * time.Second, + }) + if err != nil { + return LatestCLIRelease{}, err + } + + return result.Data.MetaData.LatestCLIRelease, nil +} + +// CurrentPlatformArchiveName returns the release archive filename for the +// running platform. +func CurrentPlatformArchiveName() (string, error) { + return BuildArchiveName(runtime.GOOS, runtime.GOARCH) +} diff --git a/model/path.go b/model/path.go index 8463bb4..9defcfd 100644 --- a/model/path.go +++ b/model/path.go @@ -125,6 +125,58 @@ func GetCurlInstallerDaemonPath() string { return filepath.Join(GetBaseStoragePath(), "bin", "shelltime-daemon") } +// GetUpdateStatePath returns the path to the self-update state file. +func GetUpdateStatePath() string { + return GetStoragePath("update-state.json") +} + +// GetUpdateLockPath returns the path to the self-update lock file. +func GetUpdateLockPath() string { + return GetStoragePath("update.lock") +} + +// GetDaemonUpdatePendingPath returns the marker the daemon writes after staging +// a new daemon binary. The CLI hot path stats this file, so it must stay a +// single cheap stat on a normally-absent path. +func GetDaemonUpdatePendingPath() string { + return GetStoragePath("daemon-update.pending") +} + +// GetStagedDaemonPath returns where a downloaded-but-not-yet-active daemon +// binary is parked. Staging rather than activating means the running daemon +// never rewrites the binary it is currently executing, and the CLI's later +// repair is a local rename with no network access. +func GetStagedDaemonPath() string { + return filepath.Join(GetBinFolderPath(), "shelltime-daemon.next") +} + +// GetCurlInstallerCLIPath returns the curl-installer CLI location +// (~/.shelltime/bin/shelltime). +func GetCurlInstallerCLIPath() string { + return filepath.Join(GetBinFolderPath(), "shelltime") +} + +// ResolveCLIBinaryPathFrom finds the shelltime CLI binary from a process that +// may not have a useful PATH — notably the launchd/systemd-spawned daemon. +// CommandService already probes the Homebrew bin dirs and falls back to a login +// shell, which is exactly what is needed here. +func ResolveCLIBinaryPathFrom(cs CommandService) (string, error) { + if cs != nil { + if p, err := cs.LookPath("shelltime"); err == nil && p != "" { + if resolved, rErr := filepath.EvalSymlinks(p); rErr == nil { + return resolved, nil + } + return p, nil + } + } + + curlPath := GetCurlInstallerCLIPath() + if info, err := os.Stat(curlPath); err == nil && !info.IsDir() { + return curlPath, nil + } + return "", fmt.Errorf("shelltime binary not found on PATH or at %s", curlPath) +} + // daemonHomebrewSearchPaths lists explicit Homebrew/Linuxbrew bin dirs to // probe when PATH is stripped (e.g. launchd-spawned shells). Exposed as a var // so tests can swap it out. diff --git a/model/update_lock.go b/model/update_lock.go new file mode 100644 index 0000000..fcec4d1 --- /dev/null +++ b/model/update_lock.go @@ -0,0 +1,68 @@ +package model + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + "time" +) + +// AcquireUpdateLock takes an exclusive lock so only one process at a time +// replaces binaries. Both the daemon's download and the CLI's daemon swap take +// it, because N shells can sample the drift check simultaneously and would +// otherwise all try to repair at once. +// +// Returns ok=false (with a no-op release) when another process holds it. A lock +// older than staleAfter is stolen once, so a process killed mid-update does not +// wedge updates forever. +// +// Uses O_CREATE|O_EXCL rather than flock: it is atomic on POSIX, needs no x/sys +// import and no build-tag split, and matches the plain-os style used elsewhere +// in this package. +func AcquireUpdateLock(staleAfter time.Duration) (release func(), ok bool) { + noop := func() {} + path := GetUpdateLockPath() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return noop, false + } + + if acquireLockFile(path) { + return func() { _ = os.Remove(path) }, true + } + + // Held. Steal it only if it is provably stale. + info, err := os.Stat(path) + if err != nil { + // Vanished between the create and the stat — try once more. + if acquireLockFile(path) { + return func() { _ = os.Remove(path) }, true + } + return noop, false + } + if time.Since(info.ModTime()) <= staleAfter { + return noop, false + } + + slog.Warn("stealing stale update lock", + slog.String("path", path), + slog.Time("heldSince", info.ModTime())) + if err := os.Remove(path); err != nil { + return noop, false + } + if acquireLockFile(path) { + return func() { _ = os.Remove(path) }, true + } + return noop, false +} + +func acquireLockFile(path string) bool { + f, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) + if err != nil { + return false + } + // Record who holds it to make a wedged lock diagnosable. + _, _ = fmt.Fprintf(f, "%d\n%s\n", os.Getpid(), time.Now().Format(time.RFC3339)) + _ = f.Close() + return true +} diff --git a/model/update_lock_test.go b/model/update_lock_test.go new file mode 100644 index 0000000..9db4e0c --- /dev/null +++ b/model/update_lock_test.go @@ -0,0 +1,94 @@ +package model + +import ( + "os" + "path/filepath" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// withTempHome points the storage helpers at a temp dir for the duration of a test. +func withTempHome(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + return filepath.Join(dir, COMMAND_BASE_STORAGE_FOLDER) +} + +func TestAcquireUpdateLock_ExclusiveThenReleasable(t *testing.T) { + withTempHome(t) + + release, ok := AcquireUpdateLock(time.Minute) + require.True(t, ok, "first acquire should succeed") + + _, ok2 := AcquireUpdateLock(time.Minute) + assert.False(t, ok2, "second acquire must fail while held") + + release() + + release3, ok3 := AcquireUpdateLock(time.Minute) + assert.True(t, ok3, "acquire should succeed after release") + release3() +} + +// A process killed mid-update must not wedge updates forever. +func TestAcquireUpdateLock_StealsStaleLock(t *testing.T) { + withTempHome(t) + + release, ok := AcquireUpdateLock(time.Hour) + require.True(t, ok) + defer release() + + // Backdate the lock past the staleness threshold. + old := time.Now().Add(-2 * time.Hour) + require.NoError(t, os.Chtimes(GetUpdateLockPath(), old, old)) + + release2, ok2 := AcquireUpdateLock(time.Minute) + assert.True(t, ok2, "a lock older than staleAfter should be stolen") + release2() +} + +func TestAcquireUpdateLock_DoesNotStealFreshLock(t *testing.T) { + withTempHome(t) + + release, ok := AcquireUpdateLock(time.Millisecond) + require.True(t, ok) + defer release() + + // staleAfter is long, so the just-taken lock is not stale. + _, ok2 := AcquireUpdateLock(time.Hour) + assert.False(t, ok2) +} + +// N shells can sample the drift check at the same moment; exactly one must win. +func TestAcquireUpdateLock_ConcurrentAcquireHasSingleWinner(t *testing.T) { + withTempHome(t) + + var winners atomic.Int32 + var wg sync.WaitGroup + releases := make(chan func(), 20) + + for range 20 { + wg.Add(1) + go func() { + defer wg.Done() + release, ok := AcquireUpdateLock(time.Hour) + if ok { + winners.Add(1) + releases <- release + } + }() + } + wg.Wait() + close(releases) + for r := range releases { + r() + } + + assert.EqualValues(t, 1, winners.Load(), "exactly one goroutine should hold the lock") +} diff --git a/model/updater_apply.go b/model/updater_apply.go new file mode 100644 index 0000000..8046c73 --- /dev/null +++ b/model/updater_apply.go @@ -0,0 +1,270 @@ +package model + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +// smokeTestTimeout bounds how long we wait for a freshly-downloaded binary to +// print its version. A var so tests can shrink it. +var smokeTestTimeout = 10 * time.Second + +// minBinarySize is a crude sanity floor. A real binary is tens of MB; anything +// this small is a truncated download or an HTML error page. +const minBinarySize = 1 << 20 + +// UpdatePlan describes one update application. +type UpdatePlan struct { + Tag string + ArchiveName string + Source ReleaseSource + // ExpectedSha comes from the server's GraphQL response. Empty means "fetch + // checksums.txt through Source". + ExpectedSha string + + // CLIDest, if set, is where the shelltime binary is activated. + CLIDest string + // DaemonDest, if set, is where the daemon binary is activated immediately. + DaemonDest string + // DaemonStagePath, if set, is where the daemon binary is parked WITHOUT + // being activated. The daemon uses this so it never rewrites the binary it + // is currently running; the CLI activates it later. + DaemonStagePath string + + // AllowUnverified permits proceeding with no checksum. Only the interactive + // `shelltime update` sets this; the unattended daemon path must fail closed. + AllowUnverified bool + // BackupSuffix defaults to BackupSuffixUpdate. + BackupSuffix string +} + +// UpdateResult reports what an ApplyUpdate call actually did. +type UpdateResult struct { + Tag string + Verified bool + ReplacedCLI bool + ReplacedDaemon bool + StagedDaemon bool + // UsedFallback is true when the proxy failed and GitHub-direct succeeded. + UsedFallback bool +} + +// ApplyUpdate downloads, verifies, and installs a release. It is the single +// place binaries are swapped, so every safety check lives here rather than +// being duplicated across the interactive and unattended callers. +func ApplyUpdate(ctx context.Context, plan UpdatePlan) (UpdateResult, error) { + res := UpdateResult{Tag: plan.Tag} + + if plan.Tag == "" { + return res, errors.New("update plan has no tag") + } + if plan.ArchiveName == "" { + name, err := CurrentPlatformArchiveName() + if err != nil { + return res, err + } + plan.ArchiveName = name + } + if plan.BackupSuffix == "" { + plan.BackupSuffix = BackupSuffixUpdate + } + + sha, err := resolveExpectedChecksum(ctx, plan) + if err != nil { + return res, err + } + if sha == "" && !plan.AllowUnverified { + return res, fmt.Errorf( + "no checksum available for %s; refusing to install an unverified binary", plan.ArchiveName) + } + res.Verified = sha != "" + + tmpDir, err := os.MkdirTemp("", "shelltime-update-*") + if err != nil { + return res, fmt.Errorf("create temp dir: %w", err) + } + defer os.RemoveAll(tmpDir) + + archivePath := filepath.Join(tmpDir, plan.ArchiveName) + usedFallback, err := downloadWithFallback(ctx, plan, sha, archivePath) + if err != nil { + return res, err + } + res.UsedFallback = usedFallback + + extractDir := filepath.Join(tmpDir, "extracted") + if err := os.MkdirAll(extractDir, 0o755); err != nil { + return res, err + } + binaries, err := ExtractBinaries(archivePath, extractDir) + if err != nil { + return res, fmt.Errorf("extract archive: %w", err) + } + + cliSrc, hasCLI := binaries["shelltime"] + if plan.CLIDest != "" { + if !hasCLI { + return res, fmt.Errorf("archive %s did not contain a shelltime binary", plan.ArchiveName) + } + // Smoke-test before touching anything on disk: a corrupt archive, a + // wrong-arch build, or a macOS signature failure all surface here. + if err := smokeTestBinary(ctx, cliSrc, plan.Tag); err != nil { + return res, fmt.Errorf("downloaded shelltime binary failed its smoke test: %w", err) + } + if err := ReplaceBinaryWithBackupSuffix(cliSrc, plan.CLIDest, plan.BackupSuffix); err != nil { + return res, fmt.Errorf("replace shelltime binary: %w", err) + } + // Verify what actually landed; roll back if the installed copy is broken. + if err := smokeTestBinary(ctx, plan.CLIDest, plan.Tag); err != nil { + if rbErr := RestoreBinaryBackup(plan.CLIDest, plan.BackupSuffix); rbErr != nil { + return res, fmt.Errorf( + "installed shelltime binary is broken (%v) AND rollback failed (%v)", err, rbErr) + } + return res, fmt.Errorf("installed shelltime binary failed verification, rolled back: %w", err) + } + res.ReplacedCLI = true + } + + daemonSrc, hasDaemon := binaries["shelltime-daemon"] + if hasDaemon && plan.DaemonDest != "" { + if err := ReplaceBinaryWithBackupSuffix(daemonSrc, plan.DaemonDest, plan.BackupSuffix); err != nil { + return res, fmt.Errorf("replace shelltime-daemon binary: %w", err) + } + // `daemon install` restores a ".bak" believing it is newer. + // Clear any stale one so it cannot undo this swap. + _ = os.Remove(plan.DaemonDest + BackupSuffixLegacy) + res.ReplacedDaemon = true + } + + if hasDaemon && plan.DaemonStagePath != "" { + if err := os.MkdirAll(filepath.Dir(plan.DaemonStagePath), 0o755); err != nil { + return res, err + } + _ = os.Remove(plan.DaemonStagePath) + if err := copyFile(daemonSrc, plan.DaemonStagePath); err != nil { + return res, fmt.Errorf("stage shelltime-daemon binary: %w", err) + } + if err := os.Chmod(plan.DaemonStagePath, 0o755); err != nil { + return res, err + } + res.StagedDaemon = true + } + + return res, nil +} + +// resolveExpectedChecksum prefers the server-provided sha, cross-checking it +// against checksums.txt when both are available. +func resolveExpectedChecksum(ctx context.Context, plan UpdatePlan) (string, error) { + manifestSha, found, err := FetchChecksumFrom(ctx, plan.Source, plan.Tag, plan.ArchiveName) + if err != nil { + slog.Debug("could not fetch checksums.txt", slog.Any("err", err)) + } + + switch { + case plan.ExpectedSha != "" && found: + if !strings.EqualFold(plan.ExpectedSha, manifestSha) { + // The two should agree; a mismatch means something upstream is + // inconsistent and we should not install anything. + return "", fmt.Errorf( + "checksum mismatch between API (%s) and checksums.txt (%s) for %s", + plan.ExpectedSha, manifestSha, plan.ArchiveName) + } + return strings.ToLower(plan.ExpectedSha), nil + case plan.ExpectedSha != "": + return strings.ToLower(plan.ExpectedSha), nil + case found: + return manifestSha, nil + default: + return "", nil + } +} + +// downloadWithFallback tries the configured source, then GitHub directly. The +// fallback matters in the opposite direction from the proxy's purpose: a user +// who CAN reach GitHub should not be blocked by a broken proxy. +func downloadWithFallback(ctx context.Context, plan UpdatePlan, sha, archivePath string) (bool, error) { + primary := plan.Source.DownloadURL(plan.Tag, plan.ArchiveName) + err := DownloadAndVerify(ctx, primary, sha, archivePath) + if err == nil { + return false, nil + } + if !plan.Source.IsProxy() { + return false, fmt.Errorf("download release: %w", err) + } + + slog.Warn("release proxy download failed, falling back to GitHub", + slog.String("url", primary), slog.Any("err", err)) + + fallback := plan.Source.Direct().DownloadURL(plan.Tag, plan.ArchiveName) + if fbErr := DownloadAndVerify(ctx, fallback, sha, archivePath); fbErr != nil { + return false, fmt.Errorf("download release (proxy: %v; github: %w)", err, fbErr) + } + return true, nil +} + +// smokeTestBinary runs ` --version` and requires it to exit 0 and report +// the expected tag. Never install a binary that cannot describe itself. +func smokeTestBinary(ctx context.Context, path, expectTag string) error { + info, err := os.Stat(path) + if err != nil { + return err + } + if info.Size() < minBinarySize { + return fmt.Errorf("binary %s is only %d bytes", path, info.Size()) + } + + ctx, cancel := context.WithTimeout(ctx, smokeTestTimeout) + defer cancel() + + out, err := exec.CommandContext(ctx, path, "--version").CombinedOutput() + if err != nil { + return fmt.Errorf("%s --version failed: %w (%s)", path, err, strings.TrimSpace(string(out))) + } + if expectTag == "" { + return nil + } + want := NormalizeVersion(expectTag) + if !strings.Contains(string(out), want) { + return fmt.Errorf("%s --version reported %q, expected it to contain %q", + path, strings.TrimSpace(string(out)), want) + } + return nil +} + +// SmokeTestDaemonBinary verifies a staged daemon binary before it is activated. +// The daemon handles -v before any service initialization, so this is fast and +// has no side effects. +func SmokeTestDaemonBinary(ctx context.Context, path, expectTag string) error { + info, err := os.Stat(path) + if err != nil { + return err + } + if info.Size() < minBinarySize { + return fmt.Errorf("daemon binary %s is only %d bytes", path, info.Size()) + } + + ctx, cancel := context.WithTimeout(ctx, smokeTestTimeout) + defer cancel() + + out, err := exec.CommandContext(ctx, path, "-v").CombinedOutput() + if err != nil { + return fmt.Errorf("%s -v failed: %w (%s)", path, err, strings.TrimSpace(string(out))) + } + if expectTag == "" { + return nil + } + want := NormalizeVersion(expectTag) + if !strings.Contains(string(out), want) { + return fmt.Errorf("%s -v reported %q, expected it to contain %q", + path, strings.TrimSpace(string(out)), want) + } + return nil +} diff --git a/model/updater_apply_test.go b/model/updater_apply_test.go new file mode 100644 index 0000000..6da1239 --- /dev/null +++ b/model/updater_apply_test.go @@ -0,0 +1,329 @@ +package model + +import ( + "archive/zip" + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeBinaryScript builds a shell script that behaves enough like a real binary +// for the smoke test: it prints a version string and exits 0. Padded past +// minBinarySize so the size sanity check passes. +func fakeBinaryScript(version string) []byte { + var b bytes.Buffer + fmt.Fprintf(&b, "#!/bin/sh\necho 'shelltime version %s'\nexit 0\n", version) + b.WriteString("# padding") + b.Write(bytes.Repeat([]byte("x"), minBinarySize)) + b.WriteString("\n") + return b.Bytes() +} + +func failingBinaryScript() []byte { + var b bytes.Buffer + b.WriteString("#!/bin/sh\necho 'boom' >&2\nexit 3\n") + b.WriteString("# padding") + b.Write(bytes.Repeat([]byte("x"), minBinarySize)) + b.WriteString("\n") + return b.Bytes() +} + +// buildTestArchive returns a zip containing the named entries. +func buildTestArchive(t *testing.T, entries map[string][]byte) []byte { + t.Helper() + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for name, content := range entries { + hdr := &zip.FileHeader{Name: name, Method: zip.Store} + hdr.SetMode(0o755) + w, err := zw.CreateHeader(hdr) + require.NoError(t, err) + _, err = w.Write(content) + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + return buf.Bytes() +} + +func sha256Hex(b []byte) string { + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:]) +} + +// releaseServer serves an archive and a checksums.txt at the proxy paths. +type releaseServer struct { + URL string + archiveHits int + checksumHits int +} + +func newReleaseServer(t *testing.T, tag, assetName string, archive []byte, withChecksums bool) *releaseServer { + t.Helper() + rs := &releaseServer{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "checksums.txt"): + rs.checksumHits++ + if !withChecksums { + w.WriteHeader(http.StatusNotFound) + return + } + fmt.Fprintf(w, "%s %s\n", sha256Hex(archive), assetName) + case strings.HasSuffix(r.URL.Path, assetName): + rs.archiveHits++ + w.Header().Set("Content-Length", fmt.Sprint(len(archive))) + _, _ = w.Write(archive) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + rs.URL = srv.URL + return rs +} + +func TestApplyUpdate_ReplacesCLIAndStagesDaemon(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("smoke test uses a shell script") + } + dir := t.TempDir() + tag := "v9.9.9" + assetName := "cli_Test_x86.zip" + + archive := buildTestArchive(t, map[string][]byte{ + "shelltime": fakeBinaryScript("9.9.9"), + "shelltime-daemon": fakeBinaryScript("9.9.9"), + }) + srv := newReleaseServer(t, tag, assetName, archive, true) + + cliDest := filepath.Join(dir, "shelltime") + require.NoError(t, os.WriteFile(cliDest, fakeBinaryScript("1.0.0"), 0o755)) + stagePath := filepath.Join(dir, "shelltime-daemon.next") + + res, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: tag, + ArchiveName: assetName, + Source: NewReleaseSource(srv.URL), + CLIDest: cliDest, + DaemonStagePath: stagePath, + }) + require.NoError(t, err) + + assert.True(t, res.Verified, "checksums.txt was available, so it must verify") + assert.True(t, res.ReplacedCLI) + assert.True(t, res.StagedDaemon) + assert.False(t, res.ReplacedDaemon, "daemon must be staged, not activated") + assert.False(t, res.UsedFallback) + + installed, err := os.ReadFile(cliDest) + require.NoError(t, err) + assert.Contains(t, string(installed), "9.9.9") + + staged, err := os.Stat(stagePath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), staged.Mode().Perm()) + + // The previous binary is preserved under .prev, never .bak. + prev, err := os.ReadFile(cliDest + BackupSuffixUpdate) + require.NoError(t, err) + assert.Contains(t, string(prev), "1.0.0") + _, err = os.Stat(cliDest + BackupSuffixLegacy) + assert.True(t, os.IsNotExist(err)) +} + +// The unattended daemon path must never install a binary it could not verify. +func TestApplyUpdate_FailsClosedWithoutChecksum(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("smoke test uses a shell script") + } + dir := t.TempDir() + assetName := "cli_Test_x86.zip" + archive := buildTestArchive(t, map[string][]byte{"shelltime": fakeBinaryScript("9.9.9")}) + srv := newReleaseServer(t, "v9.9.9", assetName, archive, false /* no checksums.txt */) + + cliDest := filepath.Join(dir, "shelltime") + require.NoError(t, os.WriteFile(cliDest, fakeBinaryScript("1.0.0"), 0o755)) + + _, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: "v9.9.9", + ArchiveName: assetName, + Source: NewReleaseSource(srv.URL), + CLIDest: cliDest, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "no checksum available") + + // Nothing was touched. + current, readErr := os.ReadFile(cliDest) + require.NoError(t, readErr) + assert.Contains(t, string(current), "1.0.0") +} + +func TestApplyUpdate_AllowUnverifiedProceeds(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("smoke test uses a shell script") + } + dir := t.TempDir() + assetName := "cli_Test_x86.zip" + archive := buildTestArchive(t, map[string][]byte{"shelltime": fakeBinaryScript("9.9.9")}) + srv := newReleaseServer(t, "v9.9.9", assetName, archive, false) + + cliDest := filepath.Join(dir, "shelltime") + require.NoError(t, os.WriteFile(cliDest, fakeBinaryScript("1.0.0"), 0o755)) + + res, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: "v9.9.9", + ArchiveName: assetName, + Source: NewReleaseSource(srv.URL), + CLIDest: cliDest, + AllowUnverified: true, + }) + require.NoError(t, err) + assert.False(t, res.Verified) + assert.True(t, res.ReplacedCLI) +} + +// The API-provided sha and checksums.txt should agree; disagreement means +// something upstream is inconsistent and nothing should be installed. +func TestApplyUpdate_AbortsOnChecksumDisagreement(t *testing.T) { + dir := t.TempDir() + assetName := "cli_Test_x86.zip" + archive := buildTestArchive(t, map[string][]byte{"shelltime": fakeBinaryScript("9.9.9")}) + srv := newReleaseServer(t, "v9.9.9", assetName, archive, true) + + cliDest := filepath.Join(dir, "shelltime") + + _, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: "v9.9.9", + ArchiveName: assetName, + Source: NewReleaseSource(srv.URL), + ExpectedSha: strings.Repeat("a", 64), // deliberately wrong + CLIDest: cliDest, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "checksum mismatch") + assert.Equal(t, 0, srv.archiveHits, "must not download when checksums disagree") +} + +func TestApplyUpdate_AbortsOnCorruptDownload(t *testing.T) { + dir := t.TempDir() + assetName := "cli_Test_x86.zip" + archive := buildTestArchive(t, map[string][]byte{"shelltime": fakeBinaryScript("9.9.9")}) + + // Serve a checksum for the real archive but a different body. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "checksums.txt") { + fmt.Fprintf(w, "%s %s\n", sha256Hex(archive), assetName) + return + } + _, _ = w.Write([]byte("this is not the archive you are looking for")) + })) + defer srv.Close() + + cliDest := filepath.Join(dir, "shelltime") + _, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: "v9.9.9", + ArchiveName: assetName, + // Direct source, so a checksum failure is terminal rather than + // triggering the GitHub fallback. + Source: ReleaseSource{}, + CLIDest: cliDest, + }) + require.Error(t, err) + + _, statErr := os.Stat(cliDest) + assert.True(t, os.IsNotExist(statErr), "nothing should be installed from a corrupt download") +} + +// A broken binary must never be left in place: the post-install check has to +// roll back, or the user's shell is left with a CLI that cannot run. +func TestApplyUpdate_RollsBackWhenInstalledBinaryIsBroken(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("smoke test uses a shell script") + } + dir := t.TempDir() + assetName := "cli_Test_x86.zip" + archive := buildTestArchive(t, map[string][]byte{"shelltime": failingBinaryScript()}) + srv := newReleaseServer(t, "v9.9.9", assetName, archive, true) + + cliDest := filepath.Join(dir, "shelltime") + require.NoError(t, os.WriteFile(cliDest, fakeBinaryScript("1.0.0"), 0o755)) + + _, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: "v9.9.9", + ArchiveName: assetName, + Source: NewReleaseSource(srv.URL), + CLIDest: cliDest, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "smoke test") + + // The working binary is still there. + current, readErr := os.ReadFile(cliDest) + require.NoError(t, readErr) + assert.Contains(t, string(current), "1.0.0", "the previous working binary must survive") +} + +// A user who CAN reach GitHub should not be blocked by a broken proxy. +func TestApplyUpdate_FallsBackToGitHubWhenProxyFails(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("smoke test uses a shell script") + } + dir := t.TempDir() + tag := "v9.9.9" + assetName := "cli_Test_x86.zip" + archive := buildTestArchive(t, map[string][]byte{"shelltime": fakeBinaryScript("9.9.9")}) + + // "GitHub" serves everything; the proxy 500s on the archive only. + github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "checksums.txt") { + fmt.Fprintf(w, "%s %s\n", sha256Hex(archive), assetName) + return + } + w.Header().Set("Content-Length", fmt.Sprint(len(archive))) + _, _ = w.Write(archive) + })) + defer github.Close() + + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "checksums.txt") { + fmt.Fprintf(w, "%s %s\n", sha256Hex(archive), assetName) + return + } + w.WriteHeader(http.StatusInternalServerError) + })) + defer proxy.Close() + + prevRelease := githubReleaseBaseURL + githubReleaseBaseURL = github.URL + defer func() { githubReleaseBaseURL = prevRelease }() + + cliDest := filepath.Join(dir, "shelltime") + res, err := ApplyUpdate(context.Background(), UpdatePlan{ + Tag: tag, + ArchiveName: assetName, + Source: NewReleaseSource(proxy.URL), + CLIDest: cliDest, + }) + require.NoError(t, err) + assert.True(t, res.UsedFallback, "should report that it fell back to GitHub") + assert.True(t, res.ReplacedCLI) +} + +func TestApplyUpdate_RequiresTag(t *testing.T) { + _, err := ApplyUpdate(context.Background(), UpdatePlan{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no tag") +} diff --git a/model/updater_state.go b/model/updater_state.go new file mode 100644 index 0000000..dd254bc --- /dev/null +++ b/model/updater_state.go @@ -0,0 +1,130 @@ +package model + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "time" +) + +// DisableAutoUpdateEnv is an emergency kill switch for the whole self-update +// system. Setting it to any non-empty value stops the daemon from checking and +// stops the CLI from applying a staged update, without requiring a config edit. +const DisableAutoUpdateEnv = "SHELLTIME_DISABLE_AUTO_UPDATE" + +// UpdateState is the durable handoff between the daemon (which checks and +// downloads) and the CLI (which finishes the daemon swap on a later shell). +// +// It is persisted rather than kept in memory because the two halves run in +// different processes, and because a daemon restart must not reset the check +// cadence — otherwise a daemon that restarts more often than the check interval +// would never check at all. +type UpdateState struct { + // LastCheckAt gates the daily check across daemon restarts. + LastCheckAt time.Time `json:"lastCheckAt"` + // LastKnownTag is the newest tag the server has reported. + LastKnownTag string `json:"lastKnownTag,omitempty"` + // LastAppliedTag is the tag we last successfully downloaded and installed, + // so a restart doesn't re-download the same release. + LastAppliedTag string `json:"lastAppliedTag,omitempty"` + // PendingDaemonTag/PendingDaemonPath describe a staged daemon binary waiting + // for the CLI to activate it. PendingDaemonPath is empty for Homebrew + // installs, where brew owns the binary and only a service restart is needed. + PendingDaemonTag string `json:"pendingDaemonTag,omitempty"` + PendingDaemonPath string `json:"pendingDaemonPath,omitempty"` + // Notice is a user-facing one-liner shown once per day at shell startup. + Notice string `json:"notice,omitempty"` + NoticeShownAt time.Time `json:"noticeShownAt,omitempty"` + // LastDaemonStartAttemptAt rate-limits self-healing restarts of a daemon + // that is down but cannot start. + LastDaemonStartAttemptAt time.Time `json:"lastDaemonStartAttemptAt,omitempty"` + // LastError and ConsecutiveFailures drive the exponential backoff. + LastError string `json:"lastError,omitempty"` + ConsecutiveFailures int `json:"consecutiveFailures,omitempty"` +} + +// ReadUpdateState loads the persisted state. A missing or unreadable file yields +// the zero value with no error: update state is a cache, never a source of +// truth, and a corrupt file must not stop the CLI from working. +func ReadUpdateState() (UpdateState, error) { + var st UpdateState + raw, err := os.ReadFile(GetUpdateStatePath()) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return st, nil + } + return st, err + } + if err := json.Unmarshal(raw, &st); err != nil { + return UpdateState{}, nil + } + return st, nil +} + +// WriteUpdateState persists state atomically (temp file + rename), so a reader +// never observes a half-written file. +func WriteUpdateState(st UpdateState) error { + path := GetUpdateStatePath() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + + raw, err := json.MarshalIndent(st, "", " ") + if err != nil { + return err + } + + tmp, err := os.CreateTemp(filepath.Dir(path), ".update-state-*.tmp") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) + + if _, err := tmp.Write(raw); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Chmod(tmpName, 0o644); err != nil { + return err + } + return os.Rename(tmpName, path) +} + +// WriteDaemonUpdatePending drops the marker the CLI hot path looks for. It is +// written last, after the state file, so the CLI never sees a marker without +// the state that explains it. +func WriteDaemonUpdatePending(tag string) error { + path := GetDaemonUpdatePendingPath() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + return os.WriteFile(path, []byte(tag), 0o644) +} + +// ReadDaemonUpdatePending returns the tag recorded in the pending marker. +func ReadDaemonUpdatePending() (string, error) { + raw, err := os.ReadFile(GetDaemonUpdatePendingPath()) + if err != nil { + return "", err + } + tag := string(raw) + if tag == "" { + return "", fmt.Errorf("empty pending marker") + } + return tag, nil +} + +// ClearDaemonUpdatePending removes the marker. Missing is not an error. +func ClearDaemonUpdatePending() error { + err := os.Remove(GetDaemonUpdatePendingPath()) + if err != nil && errors.Is(err, os.ErrNotExist) { + return nil + } + return err +} diff --git a/model/updater_state_test.go b/model/updater_state_test.go new file mode 100644 index 0000000..9b1e2fc --- /dev/null +++ b/model/updater_state_test.go @@ -0,0 +1,74 @@ +package model + +import ( + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUpdateState_RoundTrip(t *testing.T) { + withTempHome(t) + + now := time.Now().UTC().Truncate(time.Second) + want := UpdateState{ + LastCheckAt: now, + LastKnownTag: "v0.1.90", + LastAppliedTag: "v0.1.90", + PendingDaemonTag: "v0.1.90", + PendingDaemonPath: "/tmp/shelltime-daemon.next", + ConsecutiveFailures: 2, + } + require.NoError(t, WriteUpdateState(want)) + + got, err := ReadUpdateState() + require.NoError(t, err) + assert.Equal(t, want.LastKnownTag, got.LastKnownTag) + assert.Equal(t, want.LastAppliedTag, got.LastAppliedTag) + assert.Equal(t, want.PendingDaemonTag, got.PendingDaemonTag) + assert.Equal(t, want.PendingDaemonPath, got.PendingDaemonPath) + assert.Equal(t, want.ConsecutiveFailures, got.ConsecutiveFailures) + assert.True(t, want.LastCheckAt.Equal(got.LastCheckAt)) +} + +// State is a cache, never a source of truth: a missing or corrupt file must not +// stop the CLI from working. +func TestReadUpdateState_MissingFileIsZeroValue(t *testing.T) { + withTempHome(t) + + got, err := ReadUpdateState() + require.NoError(t, err) + assert.Equal(t, UpdateState{}, got) +} + +func TestReadUpdateState_CorruptFileIsZeroValue(t *testing.T) { + withTempHome(t) + require.NoError(t, WriteUpdateState(UpdateState{LastKnownTag: "v1"})) + require.NoError(t, os.WriteFile(GetUpdateStatePath(), []byte("{not json"), 0o644)) + + got, err := ReadUpdateState() + require.NoError(t, err) + assert.Equal(t, UpdateState{}, got) +} + +func TestDaemonUpdatePendingMarker(t *testing.T) { + withTempHome(t) + + _, err := ReadDaemonUpdatePending() + assert.Error(t, err, "no marker yet") + + require.NoError(t, WriteDaemonUpdatePending("v0.1.90")) + + tag, err := ReadDaemonUpdatePending() + require.NoError(t, err) + assert.Equal(t, "v0.1.90", tag) + + require.NoError(t, ClearDaemonUpdatePending()) + _, err = ReadDaemonUpdatePending() + assert.Error(t, err) + + // Clearing an already-absent marker is not an error. + assert.NoError(t, ClearDaemonUpdatePending()) +} From 887284f975488fe2092dc79ad338a06dee5c3f43 Mon Sep 17 00:00:00 2001 From: AnnatarHe Date: Fri, 7 Aug 2026 23:01:11 +0800 Subject: [PATCH 3/3] refactor(track): limit the shell hook to starting the daemon `track` runs inside the shell hook on every command, so it must never do update work. Previously it sampled a marker file and, on a hit, spawned `daemon apply-update` -- which swaps the daemon binary. Even though the download itself always happened in the daemon, binary-swapping is far more than a per-command hook should ever be responsible for. The most track may now do is start a stopped daemon service, and it ignores whether that succeeds. Move the check to where track has ALREADY established that no daemon is reachable, after both the default and the configured socket have failed. That makes it free on the happy path: a running daemon returns long before reaching it, so the common case adds no rand, no stat, and no syscall at all -- strictly cheaper than the previous top-of-function sampling. Applying a staged update stays in `gc`, which runs once per new shell rather than once per command, and downloading stays exclusively in the daemon. Add a structural test that parses the call graph out of commandTrack and fails if it can reach any download/extract/verify/swap helper. A behavioural test could only show that today's path is clean; this shows nobody can reintroduce update work without deleting the test. Verified it fails on an injected violation and names the offending call path. Co-Authored-By: Claude Opus 5 (1M context) --- commands/track.go | 16 ++-- commands/track_no_update_test.go | 138 +++++++++++++++++++++++++++++++ commands/update_drift.go | 76 ++++++++++++----- commands/update_drift_test.go | 123 +++++++++++++++++++-------- 4 files changed, 294 insertions(+), 59 deletions(-) create mode 100644 commands/track_no_update_test.go diff --git a/commands/track.go b/commands/track.go index e32dea8..771152d 100644 --- a/commands/track.go +++ b/commands/track.go @@ -58,12 +58,6 @@ var TrackCommand *cli.Command = &cli.Command{ } func commandTrack(c *cli.Context) error { - // Safety net for shells kept open for weeks, where `gc` never re-runs. - // Sampled at 1-in-N so the common path is a few nanoseconds and no syscalls; - // see checkDaemonDriftSampled for the cost breakdown. Runs before the span - // and logger setup so it cannot be slowed by them. - checkDaemonDriftSampled() - ctx, span := commandTracer.Start(c.Context, "track", trace.WithSpanKind(trace.SpanKindClient)) defer span.End() SetupLogger(os.ExpandEnv("$HOME/" + model.COMMAND_BASE_STORAGE_FOLDER)) @@ -127,6 +121,16 @@ func commandTrack(c *cli.Context) error { return sendTrackEventToDaemon(ctx, span, config.SocketPath, cmdPhase, instance, result) } + // Reaching here means no daemon is listening anywhere, which track has just + // established for free. Try to bring it back up — a dead daemon means no + // syncing and no update checks at all. + // + // This is the ONLY daemon-lifecycle action track ever takes. It never + // downloads, never swaps a binary, and never applies an update: that work + // belongs to the daemon (and to `gc`, which runs once per shell). The spawn + // is detached and its outcome is deliberately ignored. + maybeStartDaemonFromTrack() + // No daemon at all: persist to the local txt store and sync directly over HTTP. if cmdPhase == "pre" { span.SetAttributes(attribute.Int("phase", 0)) diff --git a/commands/track_no_update_test.go b/commands/track_no_update_test.go new file mode 100644 index 0000000..4117d9d --- /dev/null +++ b/commands/track_no_update_test.go @@ -0,0 +1,138 @@ +package commands + +import ( + "go/ast" + "go/parser" + "go/token" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCommandTrack_NeverPerformsUpdateWork is a structural guard, not a +// behavioural test. +// +// `track` runs inside the shell hook on every command. It must never download, +// extract, verify, or swap a binary — all of that belongs to the daemon, with +// `gc` (once per new shell) applying whatever the daemon staged. The most track +// may do is start a stopped daemon service. +// +// Behavioural tests can only prove that today's code path does not update. +// Parsing the call graph proves nobody can reintroduce it without deleting this +// test, which is the point: the constraint is easy to violate by accident, +// because the update helpers live in the same package. +func TestCommandTrack_NeverPerformsUpdateWork(t *testing.T) { + fset := token.NewFileSet() + pkg, err := parser.ParseDir(fset, ".", nil, 0) + require.NoError(t, err) + + files := map[string]*ast.File{} + for _, p := range pkg { + for name, f := range p.Files { + if strings.HasSuffix(name, "_test.go") { + continue + } + files[name] = f + } + } + require.NotEmpty(t, files) + + // Index every top-level func in the package so we can walk transitively. + funcs := map[string]*ast.FuncDecl{} + for _, f := range files { + for _, decl := range f.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Recv == nil { + funcs[fn.Name.Name] = fn + } + } + } + require.Contains(t, funcs, "commandTrack") + + // Anything that downloads, unpacks, verifies or swaps a binary. If track can + // reach one of these, the shell hook can be made to do update work. + forbidden := map[string]string{ + "ApplyUpdate": "downloads and installs a release", + "DownloadAndVerify": "downloads a release archive", + "ExtractBinaries": "unpacks a release archive", + "ReplaceBinary": "swaps a binary in place", + "ReplaceBinaryWithBackupSuffix": "swaps a binary in place", + "RestoreBinaryBackup": "swaps a binary in place", + "FetchLatestCLIRelease": "makes a network call to look for updates", + "FetchLatestVersion": "makes a network call to look for updates", + "FetchChecksum": "makes a network call to look for updates", + "FetchChecksumFrom": "makes a network call to look for updates", + "EnsureDaemonBinary": "downloads the daemon binary", + "commandDaemonApplyUpdate": "applies a staged update", + "launchDetachedApplyUpdate": "spawns the update-applying path", + "commandUpdate": "is the interactive updater", + } + + // Walk the call graph from commandTrack. + seen := map[string]bool{} + var path []string + var violations []string + + var walk func(name string) + walk = func(name string) { + if seen[name] { + return + } + seen[name] = true + + fn, ok := funcs[name] + if !ok { + return // defined in another package; the selector check below covers those + } + + path = append(path, name) + defer func() { path = path[:len(path)-1] }() + + ast.Inspect(fn, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + + var callee string + switch f := call.Fun.(type) { + case *ast.Ident: // localFunc(...) + callee = f.Name + case *ast.SelectorExpr: // model.Foo(...) + callee = f.Sel.Name + default: + return true + } + + if why, bad := forbidden[callee]; bad { + violations = append(violations, + strings.Join(append(path, callee), " -> ")+" ("+callee+" "+why+")") + return true + } + walk(callee) + return true + }) + } + walk("commandTrack") + + assert.Empty(t, violations, + "commandTrack must never reach update machinery; the most it may do is start the daemon.\n"+ + "Offending call paths:\n "+strings.Join(violations, "\n ")) +} + +// The one daemon action track is allowed to take must remain reachable. +func TestCommandTrack_CanStillStartTheDaemon(t *testing.T) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "track.go", nil, 0) + require.NoError(t, err) + + found := false + ast.Inspect(f, func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok && id.Name == "maybeStartDaemonFromTrack" { + found = true + } + return true + }) + assert.True(t, found, "track should still be able to bring a stopped daemon back up") +} diff --git a/commands/update_drift.go b/commands/update_drift.go index 4e93516..0f9162d 100644 --- a/commands/update_drift.go +++ b/commands/update_drift.go @@ -13,12 +13,12 @@ import ( "github.com/malamtime/cli/model" ) -// updateDriftSampleRate makes 1-in-N `track` invocations pay for a single -// os.Stat. `track` runs on every command in every shell, so the common path -// must cost effectively nothing; 64 gives roughly 3 checks/day per shell at -// ~200 commands/day, which is ample since the daemon writes the marker at most -// once per release. A package var so tests can force it to 1. -var updateDriftSampleRate = 64 +// trackDaemonStartSampleRate makes 1-in-N of track's daemon-down paths attempt +// a restart. It is only ever consulted after track has already proven no daemon +// is listening, so the sampling exists to avoid a spawn storm when the daemon +// is down and failing to start — not to save time on the happy path, which +// never reaches it. A package var so tests can force it to 1. +var trackDaemonStartSampleRate = 8 // noticeRepeatInterval bounds how often the same update notice is printed. const noticeRepeatInterval = 24 * time.Hour @@ -30,34 +30,53 @@ const daemonStartRetryInterval = time.Hour // Seams for tests. var ( spawnDriftRepair = launchDetachedApplyUpdate + spawnDaemonStart = launchDetachedDaemonStart driftNow = time.Now driftDaemonIsReady = daemonIsReady ) -// checkDaemonDriftSampled is called at the very top of `track`, before any -// tracing or logging setup. +// maybeStartDaemonFromTrack restarts a daemon that has stopped. // -// Cost on the ~98% path: a constant comparison plus one rand.IntN — under 100ns -// with zero syscalls. On the sampled path it adds one stat(2) of a normally -// absent file (~2-5us). Both are orders of magnitude below what `track` already -// pays for process startup and its unix-socket dial. -func checkDaemonDriftSampled() { +// This is the full extent of what `track` is allowed to do. track runs inside +// the shell hook on every command, so it must never download, extract, verify, +// or swap a binary — all of that belongs to the daemon, with `gc` (once per new +// shell) applying anything the daemon staged. Here we only ever start a service. +// +// It is called only after track has already failed to reach a daemon on both +// the default and the configured socket, so the check itself costs nothing: a +// running daemon returns long before this point. +func maybeStartDaemonFromTrack() { if runtime.GOOS == "windows" { return // no daemon on Windows } // math/rand/v2's global source is seeded randomly per process and takes no // mutex. That matters here: every `track` is a fresh process, so a fixed // seed would make every process sample identically. - if rand.IntN(updateDriftSampleRate) != 0 { + if rand.IntN(trackDaemonStartSampleRate) != 0 { return } - if os.Getenv(model.DisableAutoUpdateEnv) != "" { + // No service definition means the user removed it deliberately; a shell hook + // is the last place that should argue with them. + if !daemonServiceFileExists() { + return + } + + // Rate limit so a daemon that cannot start is not respawned all day. This + // touches the state file, but only on the already-slow no-daemon path. + state, err := model.ReadUpdateState() + if err != nil { return } - if _, err := os.Stat(model.GetDaemonUpdatePendingPath()); err != nil { + if driftNow().Sub(state.LastDaemonStartAttemptAt) < daemonStartRetryInterval { return } - spawnDriftRepair() + state.LastDaemonStartAttemptAt = driftNow() + if err := model.WriteUpdateState(state); err != nil { + return + } + + slog.Debug("no daemon reachable from track; attempting to start it") + spawnDaemonStart() } // maybeRepairDaemonDrift runs once per new shell from `gc`. Unlike the sampled @@ -146,18 +165,35 @@ func maybeRestartDownDaemon(ctx context.Context) { // the shell hook would visibly stall the prompt and pollute it with output. // Setsid also means Ctrl-C or the shell exiting cannot kill a half-finished // binary swap. +// launchDetachedFn is a seam so tests can assert which subcommand is spawned +// without actually forking. +var launchDetachedFn = launchDetached + func launchDetachedApplyUpdate() { + launchDetachedFn("daemon", "apply-update") +} + +// launchDetachedDaemonStart starts the daemon service and nothing else. It is +// what `track` spawns: no update logic, no binary swap, no network. +func launchDetachedDaemonStart() { + launchDetachedFn("daemon", "install") +} + +// launchDetached re-execs this binary with the given args in its own session, +// discarding all output and never waiting for the result. +func launchDetached(args ...string) { self, err := os.Executable() if err != nil { return } - cmd := exec.Command(self, "daemon", "apply-update") + cmd := exec.Command(self, args...) cmd.Stdin, cmd.Stdout, cmd.Stderr = nil, nil, nil applyDetachAttrs(cmd) if err := cmd.Start(); err != nil { - slog.Debug("could not spawn daemon apply-update", slog.Any("err", err)) + slog.Debug("could not spawn detached command", slog.Any("args", args), slog.Any("err", err)) return } - // Never Wait(): this must not outlive or block the hook. + // Never Wait(): this must not outlive or block the hook, and per design the + // outcome is ignored — success or failure, the shell carries on. _ = cmd.Process.Release() } diff --git a/commands/update_drift_test.go b/commands/update_drift_test.go index 3f9813c..303ef15 100644 --- a/commands/update_drift_test.go +++ b/commands/update_drift_test.go @@ -18,79 +18,136 @@ func driftTestEnv(t *testing.T) { t.Setenv("HOME", t.TempDir()) t.Setenv(model.DisableAutoUpdateEnv, "") - prevRate := updateDriftSampleRate + prevRate := trackDaemonStartSampleRate prevSpawn := spawnDriftRepair + prevStart := spawnDaemonStart prevNow := driftNow prevReady := driftDaemonIsReady t.Cleanup(func() { - updateDriftSampleRate = prevRate + trackDaemonStartSampleRate = prevRate spawnDriftRepair = prevSpawn + spawnDaemonStart = prevStart driftNow = prevNow driftDaemonIsReady = prevReady }) // Force the sampled branch so tests are deterministic. - updateDriftSampleRate = 1 + trackDaemonStartSampleRate = 1 // Don't let a real daemon on the dev machine influence the result. driftDaemonIsReady = func(context.Context) bool { return false } } +// installDaemonServiceFile simulates a daemon that has been installed at some +// point (so we are allowed to restart it). +func installDaemonServiceFile(t *testing.T) { + t.Helper() + require.NoError(t, os.MkdirAll(model.GetStoragePath("daemon"), 0o755)) + require.NoError(t, os.WriteFile( + model.GetStoragePath("daemon", "xyz.shelltime.daemon.plist"), []byte(""), 0o644)) + require.NoError(t, os.WriteFile( + model.GetStoragePath("daemon", "shelltime.service"), []byte("[Unit]"), 0o644)) +} + func autoUpdateOnConfig() model.ShellTimeConfig { on := true return model.ShellTimeConfig{AutoUpdate: &model.AutoUpdate{Enabled: &on}} } -func TestCheckDaemonDriftSampled_NoMarkerDoesNothing(t *testing.T) { +// track's ONLY permitted daemon action is starting the service. It must never +// spawn the update-applying path, which swaps binaries. +func TestMaybeStartDaemonFromTrack_StartsDaemonNeverAppliesUpdate(t *testing.T) { driftTestEnv(t) + installDaemonServiceFile(t) + // Even with a staged update pending, track must not touch it. + require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) - var spawned atomic.Bool - spawnDriftRepair = func() { spawned.Store(true) } + var started, repaired atomic.Bool + spawnDaemonStart = func() { started.Store(true) } + spawnDriftRepair = func() { repaired.Store(true) } + + maybeStartDaemonFromTrack() - checkDaemonDriftSampled() - assert.False(t, spawned.Load(), "no marker means no repair") + assert.True(t, started.Load(), "track should start a stopped daemon") + assert.False(t, repaired.Load(), + "track must never apply an update; that belongs to the daemon and gc") } -func TestCheckDaemonDriftSampled_MarkerTriggersRepair(t *testing.T) { +// track spawns `daemon install`, which only starts the service. +func TestLaunchDetachedDaemonStart_UsesInstallNotApplyUpdate(t *testing.T) { driftTestEnv(t) - require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) - var spawned atomic.Bool - spawnDriftRepair = func() { spawned.Store(true) } + var gotArgs []string + prev := launchDetachedFn + t.Cleanup(func() { launchDetachedFn = prev }) + launchDetachedFn = func(args ...string) { gotArgs = args } - checkDaemonDriftSampled() - assert.True(t, spawned.Load()) + launchDetachedDaemonStart() + assert.Equal(t, []string{"daemon", "install"}, gotArgs) + + launchDetachedApplyUpdate() + assert.Equal(t, []string{"daemon", "apply-update"}, gotArgs) } -func TestCheckDaemonDriftSampled_KillSwitch(t *testing.T) { +// A user who ran `daemon uninstall` must not have it restarted by a shell hook. +func TestMaybeStartDaemonFromTrack_SkipsWhenServiceFileAbsent(t *testing.T) { driftTestEnv(t) - require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) - t.Setenv(model.DisableAutoUpdateEnv, "1") - var spawned atomic.Bool - spawnDriftRepair = func() { spawned.Store(true) } + var started atomic.Bool + spawnDaemonStart = func() { started.Store(true) } - checkDaemonDriftSampled() - assert.False(t, spawned.Load()) + maybeStartDaemonFromTrack() + assert.False(t, started.Load(), "no service file means the user removed it deliberately") } -// The whole point of sampling is that `track` almost never does any work. -func TestCheckDaemonDriftSampled_SamplingSkipsMostInvocations(t *testing.T) { +// A daemon that cannot start must not be respawned on every command. +func TestMaybeStartDaemonFromTrack_RateLimited(t *testing.T) { driftTestEnv(t) - require.NoError(t, model.WriteDaemonUpdatePending("v0.1.90")) - updateDriftSampleRate = 64 + installDaemonServiceFile(t) - var spawns atomic.Int32 - spawnDriftRepair = func() { spawns.Add(1) } + now := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC) + driftNow = func() time.Time { return now } + + var starts atomic.Int32 + spawnDaemonStart = func() { starts.Add(1) } + + maybeStartDaemonFromTrack() + require.EqualValues(t, 1, starts.Load(), "first attempt should fire") + + for range 50 { + maybeStartDaemonFromTrack() + } + assert.EqualValues(t, 1, starts.Load(), "must not respawn on every command") + + driftNow = func() time.Time { return now.Add(2 * time.Hour) } + maybeStartDaemonFromTrack() + assert.EqualValues(t, 2, starts.Load(), "retries after the interval") +} + +// Sampling keeps a spawn storm from forming while the daemon is down. +func TestMaybeStartDaemonFromTrack_Sampled(t *testing.T) { + driftTestEnv(t) + installDaemonServiceFile(t) + trackDaemonStartSampleRate = 8 + + // Keep the rate limiter out of the way so we measure sampling alone: each + // call sees a clock far past the previous attempt. + base := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC) + var clock atomic.Int64 + driftNow = func() time.Time { + return base.Add(time.Duration(clock.Add(1)) * 2 * time.Hour) + } + + var reached atomic.Int32 + spawnDaemonStart = func() { reached.Add(1) } - const runs = 2000 + const runs = 400 for range runs { - checkDaemonDriftSampled() + maybeStartDaemonFromTrack() } - got := spawns.Load() - // Expect ~31 of 2000. Generous bounds so this cannot flake. - assert.Greater(t, int(got), 0, "sampling should fire occasionally") - assert.Less(t, int(got), runs/8, "sampling should skip the vast majority of invocations") + got := int(reached.Load()) + assert.Greater(t, got, 0, "sampling should fire occasionally") + assert.Less(t, got, runs/2, "sampling should skip most invocations") } func TestMaybeRepairDaemonDrift_DisabledByConfig(t *testing.T) {