fix(cli): harden shelltime update (cask detection, checksums, daemon refresh, downgrades)

[AnnatarHe]

Summary

I reviewed shelltime update end to end against the real v0.1.93 release. On a plain curl install on Linux it works: a 0.1.90 build downloaded cli_Linux_x86_64.tar.gz, verified it, replaced both binaries, and shelltime -v then reported 0.1.93. The review also found the bugs below, which are fixed here.

Bug	Effect	Fix
Homebrew casks not detected on Intel Macs	Cask binaries resolve to /usr/local/Caskroom/..., which matched none of the Homebrew checks. Users saw "not in a known auto-updatable location, reinstall via curl or Homebrew", even though doctor had told them to run shelltime update.	Also match /Caskroom/.
Symlinked $HOME or ~/.shelltime	The running CLI path is symlink-resolved, but the expected ~/.shelltime/bin was not, so curl installs were reported as not auto-updatable. Reproduced against the original code.	Also compare against the resolved bin dir.
Daemon refreshed by the old binary	commandDaemonReinstall(c) ran inside the update process, which is still the previous release. The service unit/plist was therefore written by the old code, and new-release fixes to daemon install didn't apply until a manual reinstall.	Exec <new cli> daemon reinstall.
Version check was string equality	A build newer than the latest release (e.g. 0.1.94-next) showed "update available", and shelltime update silently downgraded it.	Compare MAJOR.MINOR.PATCH numerically (pre-release < release). Refuse a downgrade unless --force. doctor uses the same comparison.
Checksum fetch errors ignored	A 5xx or network error on checksums.txt led to an unverified install, unlike the daemon auto-download, which refuses.	Return the error. A 404 or missing entry still warns, as before.
shelltime briefly missing during the swap	The old binary was moved to .bak before the copy across filesystems (/tmp → $HOME), so hooks firing in other shells could find no shelltime.	Stage dest.new next to the destination first, then swap with two renames.
Homebrew formula users stuck	The tap moved to a cask, so brew upgrade shelltime/tap/shelltime no longer updates a Cellar keg.	Print the migration command (cask on macOS, curl installer on Linuxbrew), in both update and doctor.

Testing

• New unit tests cover:
  • Caskroom and symlinked-HOME detection
  • CompareVersions and compareToLatest
  • the Homebrew hints
  • the staging-failure case of ReplaceBinary, which keeps the old binary in place
  • runDaemonReinstall exec'ing the new CLI
  • doctor's "ahead of latest" case
• The Caskroom, symlinked-HOME and staging tests failed before the fix.
• go vet ./... and go test ./... pass, with mocks generated as in CI. Cross-compiles for darwin/arm64, darwin/amd64, windows/amd64 and linux/arm64.
• End to end against the real v0.1.93 release, in a sandboxed $HOME:
  • 0.1.90 → v0.1.93 with a fake systemctl on PATH. The is-active probe came from the old process (shelltime.bak), and every reinstall call (stop/disable/daemon-reload/enable/start) came from the new v0.1.93 binary.
  • A 0.1.94-next build under a symlinked $HOME now reports "newer than the latest release" and refuses to downgrade. The original code rejected the same install as "not auto-updatable".

Not changed

shelltime update replaces binaries only. It does not refresh ~/.shelltime/hooks/* (hooks are embedded in the binary, and hooks install writes them only when missing), while the curl installer does refresh them. That's a separate design decision, so it isn't part of this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NZWhKsvmpGDpM953J58RwY

---

Generated by Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[codecov]

Codecov Report

❌ Patch coverage is 72.36842% with 21 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
commands/update.go	42.85%	12 Missing ⚠️
model/updater.go	84.61%	8 Missing ⚠️
commands/doctor_checks.go	66.66%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (72.36%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Flag	Coverage Δ
unittests	85.85% <72.36%> (?)

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

Files with missing lines	Coverage Δ
commands/doctor_checks.go	95.14% <66.66%> (+0.01%)	⬆️
model/updater.go	84.41% <84.61%> (+1.46%)	⬆️
commands/update.go	9.43% <42.85%> (+8.35%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

[github-actions]

⏱️ shelltime track performance

✅ No significant change in shelltime track latency

Comparing main @ b285af2 → #324 @ 27cde35. Each scenario execs the binary the way the shell hooks do; values are the median of 10 interleaved rounds. Lower is better.

Scenario	base	head	Δ	p
Startup/floor	174 µs ±5%	173 µs ±5%	~	0.971	A/A
Startup/version	2.00 ms ±12%	1.95 ms ±3%	~	0.165	✅
Track/daemon/pre	2.13 ms ±10%	2.14 ms ±6%	~	0.912	✅
Track/daemon/post	2.14 ms ±5%	2.12 ms ±2%	~	0.280	✅
Track/direct/pre	2.05 ms ±4%	2.07 ms ±2%	~	0.796	✅
Track/direct/post	3.76 ms ±6%	3.75 ms ±2%	~	0.436	✅
Track/direct/post-sync	4.79 ms ±5%	4.79 ms ±4%	~	1.000	✅

Binary size: 19.1 MiB → 19.1 MiB (+0.0%).

CPU, tail latency and memory

Per exec, base → head (Δ when significant).

Scenario	p95 wall	user CPU	sys CPU	peak RSS
Startup/floor	196 µs → 192 µs	149 µs → 151 µs	5 µs → 5 µs	9.1 MiB → 9.2 MiB
Startup/version	2.20 ms → 2.14 ms	1.46 ms → 1.44 ms	647 µs → 565 µs	15.0 MiB → 14.9 MiB (-0.79%)
Track/daemon/pre	2.29 ms → 2.32 ms	1.57 ms → 1.55 ms	675 µs → 730 µs	15.9 MiB → 15.8 MiB (-0.80%)
Track/daemon/post	2.30 ms → 2.27 ms	1.56 ms → 1.55 ms	653 µs → 644 µs	15.7 MiB → 15.6 MiB (-0.08%)
Track/direct/pre	2.24 ms → 2.21 ms	1.53 ms → 1.57 ms	611 µs → 556 µs	15.8 MiB → 15.7 MiB (-0.79%)
Track/direct/post	4.09 ms → 4.03 ms	2.65 ms → 2.57 ms	1.36 ms → 1.37 ms	16.8 MiB → 16.8 MiB
Track/direct/post-sync	5.27 ms → 5.33 ms	3.55 ms → 3.42 ms	1.70 ms → 1.78 ms	18.7 MiB → 18.7 MiB (-0.31%)

benchstat

goos: linux
goarch: amd64
pkg: github.com/malamtime/cli/perf
cpu: AMD EPYC
                         │     base     │                head                │
                         │    sec/op    │   sec/op     vs base               │
Startup/floor-2            174.3µ ±  5%   173.0µ ± 5%       ~ (p=0.971 n=10)
Startup/version-2          1.996m ± 12%   1.949m ± 3%       ~ (p=0.165 n=10)
Track/daemon/pre-2         2.131m ± 10%   2.141m ± 6%       ~ (p=0.912 n=10)
Track/daemon/post-2        2.135m ±  5%   2.117m ± 2%       ~ (p=0.280 n=10)
Track/direct/pre-2         2.046m ±  4%   2.067m ± 2%       ~ (p=0.796 n=10)
Track/direct/post-2        3.765m ±  6%   3.751m ± 2%       ~ (p=0.436 n=10)
Track/direct/post-sync-2   4.792m ±  5%   4.794m ± 4%       ~ (p=1.000 n=10)
geomean                    1.788m         1.781m       -0.40%

                         │    base     │                head                │
                         │ p50-sec/op  │ p50-sec/op   vs base               │
Startup/floor-2            165.9µ ± 3%   165.3µ ± 2%       ~ (p=0.912 n=10)
Startup/version-2          1.988m ± 5%   1.942m ± 4%       ~ (p=0.165 n=10)
Track/daemon/pre-2         2.147m ± 4%   2.151m ± 5%       ~ (p=0.971 n=10)
Track/daemon/post-2        2.137m ± 3%   2.132m ± 2%       ~ (p=0.529 n=10)
Track/direct/pre-2         2.026m ± 4%   2.049m ± 2%       ~ (p=0.481 n=10)
Track/direct/post-2        3.757m ± 5%   3.746m ± 1%       ~ (p=0.739 n=10)
Track/direct/post-sync-2   4.800m ± 4%   4.772m ± 4%       ~ (p=1.000 n=10)
geomean                    1.774m        1.768m       -0.36%

                         │     base     │                head                 │
                         │  p95-sec/op  │  p95-sec/op   vs base               │
Startup/floor-2            195.6µ ± 24%   191.9µ ± 12%       ~ (p=0.353 n=10)
Startup/version-2          2.196m ± 42%   2.135m ±  4%       ~ (p=0.123 n=10)
Track/daemon/pre-2         2.287m ± 28%   2.321m ±  5%       ~ (p=1.000 n=10)
Track/daemon/post-2        2.302m ± 19%   2.272m ±  3%       ~ (p=0.280 n=10)
Track/direct/pre-2         2.238m ±  5%   2.214m ±  4%       ~ (p=0.971 n=10)
Track/direct/post-2        4.086m ± 18%   4.026m ±  6%       ~ (p=0.353 n=10)
Track/direct/post-sync-2   5.270m ± 15%   5.328m ±  5%       ~ (p=0.971 n=10)
geomean                    1.955m         1.938m        -0.85%

                         │     base     │                head                 │
                         │  peak-rss-B  │  peak-rss-B   vs base               │
Startup/floor-2            9.121Mi ± 1%   9.179Mi ± 1%       ~ (p=0.279 n=10)
Startup/version-2          15.01Mi ± 0%   14.89Mi ± 0%  -0.79% (p=0.000 n=10)
Track/daemon/pre-2         15.90Mi ± 0%   15.77Mi ± 0%  -0.80% (p=0.000 n=10)
Track/daemon/post-2        15.66Mi ± 0%   15.64Mi ± 0%  -0.08% (p=0.004 n=10)
Track/direct/pre-2         15.79Mi ± 0%   15.66Mi ± 0%  -0.79% (p=0.000 n=10)
Track/direct/post-2        16.82Mi ± 0%   16.82Mi ± 0%       ~ (p=0.380 n=10)
Track/direct/post-sync-2   18.73Mi ± 0%   18.67Mi ± 0%  -0.31% (p=0.005 n=10)
geomean                    14.98Mi        14.94Mi       -0.31%

                         │     base     │                head                 │
                         │  sys-sec/op  │  sys-sec/op   vs base               │
Startup/floor-2            5.130µ ± 83%   5.325µ ± 68%       ~ (p=0.481 n=10)
Startup/version-2          646.7µ ± 31%   565.5µ ± 12%       ~ (p=0.739 n=10)
Track/daemon/pre-2         675.1µ ± 20%   730.3µ ± 19%       ~ (p=0.247 n=10)
Track/daemon/post-2        653.2µ ± 21%   644.1µ ± 18%       ~ (p=0.684 n=10)
Track/direct/pre-2         610.7µ ± 18%   555.9µ ± 22%       ~ (p=0.481 n=10)
Track/direct/post-2        1.357m ± 37%   1.372m ± 28%       ~ (p=0.739 n=10)
Track/direct/post-sync-2   1.704m ± 14%   1.782m ± 17%       ~ (p=0.481 n=10)
geomean                    413.5µ         409.3µ        -1.01%

                         │     base     │                head                 │
                         │ user-sec/op  │ user-sec/op   vs base               │
Startup/floor-2            148.6µ ±  3%   151.3µ ±  3%       ~ (p=0.436 n=10)
Startup/version-2          1.464m ±  9%   1.438m ±  7%       ~ (p=0.579 n=10)
Track/daemon/pre-2         1.573m ±  6%   1.555m ±  8%       ~ (p=0.190 n=10)
Track/daemon/post-2        1.563m ±  9%   1.547m ±  6%       ~ (p=0.796 n=10)
Track/direct/pre-2         1.528m ±  8%   1.575m ±  7%       ~ (p=0.796 n=10)
Track/direct/post-2        2.646m ±  7%   2.574m ± 13%       ~ (p=0.796 n=10)
Track/direct/post-sync-2   3.547m ± 10%   3.415m ±  9%       ~ (p=0.353 n=10)
geomean                    1.338m         1.327m        -0.82%

_{Δ compares medians; ~ means no significant difference (Mann-Whitney U, p ≥ 0.05). A scenario is flagged when it is more than 10% and more than 250µs slower with p < 0.05. Startup/floor runs true, not shelltime: it is an A/A check of runner noise. Runner: linux/amd64, AMD EPYC. go1.27.1, 10 rounds × 100 execs per scenario and binary.}

[AnnatarHe]

@claude review

[claude]

Claude finished @AnnatarHe's task in 19s —— View job

---

Review of #324

I read the full diff (commands/update.go, commands/doctor_checks.go, model/updater.go). I didn't run the tests or build it. The change looks correct and I found no blocking issues. The notes below are minor.

What looks good

• ReplaceBinary now stages dest.new before touching dest. The old binary stays in place if staging fails, and the two renames are atomic on the same filesystem. Cleanup of .new and the .bak restore are handled on each failure path.
• runDaemonReinstall runs cliPath, which is the freshly installed binary by that point. The service unit is therefore written by the new release.
• Returning an error from FetchChecksum (instead of installing unverified) matches the daemon auto-download behaviour.
• compareToLatest falls back to string equality for non-semver values like dev, so those still show "update available". doctor shares the same comparison.
• The downgrade guard (status > 0 && !force) comes after the --check branch, so --check never changes anything.

Minor / non-blocking

1. Pre-release ordering (model/updater.go, CompareVersions): strings.Compare on the pre-release tag is lexical, so rc.10 sorts before rc.2. That doesn't matter for the current -next style tags. A short comment saying it's not full semver precedence would stop it surprising someone later.
2. Ambiguous brew name (HomebrewUpgradeCommand): brew upgrade shelltime/tap/shelltime could be ambiguous if a formula and a cask of the same name both exist in the tap. brew upgrade --cask shelltime/tap/shelltime is more explicit for Caskroom paths.
3. Linuxbrew detection (HomebrewUpgradeCommand): the check for .linuxbrew only runs for Cellar paths. A non-Cellar path under /home/linuxbrew/.linuxbrew/ would get the cask upgrade command, which doesn't exist on Linux. This is unlikely in practice.
4. Exec failure after a successful swap: if the new binary fails to exec, runDaemonReinstall only warns and prints the manual command. That's reasonable. The error is returned, so it's shown.
5. Coverage: Codecov flags the patch at 72%, mostly the commandUpdate flow in commands/update.go, which needs network and filesystem. Tests for the downgrade-refusal and checksum-error branches would raise it if you want to clear the 80% gate.

Overall, this is good to merge once you've decided on the coverage gate.