feat(update): detect install method to tailor or suppress update notices

[pjcdawkins]

Summary

Detects how the CLI was installed and adjusts the "new release available" notice accordingly, instead of showing the same generic GitHub link to everyone after every command.

• System package managers (apt, yum/dnf, apk) are detected via a marker file installed by the nfpm packages. Package installs that predate the marker are detected by querying dpkg/rpm/apk before a notice is printed. The notice is suppressed and the network check is skipped, since the OS handles updates.
• Homebrew, Scoop, npm, and the bash installer get the exact upgrade command, built from config fields so vendor builds stay correct. For the installer, the command sets INSTALL_METHOD=raw and INSTALL_DIR to the current binary's directory, so it replaces the binary in place (otherwise the installer would pick apt/yum/apk or Homebrew where available).
• Unknown installs keep the generic GitHub link.

The notice is now shown before the command runs, using a latest-version cache in state.json refreshed by the background check, and is throttled to once a week. This removes the per-run nagging. --quiet runs don't show it or consume the throttle.

Detection precedence

Explicit override (<PREFIX>INSTALL_METHOD env or wrapper.install_method config) → package marker → resolved executable path (npm / scoop / homebrew) → standard bin dirs (script) → unknown.

Changes

• internal/install.go: InstallMethod type, DetectInstallMethod, cheap IsAutoUpdating.
• internal/update.go: cache KnownLatestVersion; PendingNotification / MarkNotified with a weekly throttle; suppress checks for auto-updating installs.
• commands/root.go: show the notice in PersistentPreRun from the cache; per-method upgrade command.
• Config: Wrapper.NpmPackage, Wrapper.InstallerURL, Wrapper.InstallMethod. Only the upsun flavor sets npm_package/installer_url, since npm and installer.sh only ship upsun.
• Packaging: packaging/install-source marker added to both nfpm entries.
• Docs: README, CLAUDE.md, and docs/design/update-message-install-detection.md (which also describes the planned Phase 2: opt-in self-update).

Scope

This is Phase 1 (detection + tailored/suppressed messages + weekly throttle + show-before-via-cache). Phase 2 (interactive prompt + auto-update + re-exec) and the external docs update are tracked separately in the design doc.

Verification

• Unit tests for detection, method parsing, the weekly throttle, and per-method upgrade commands.
• Live TTY smoke test confirming npm/homebrew wording, silence for package installs, and that the weekly throttle persists.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds install-method detection to tailor (or suppress) update notifications, moves the “new release available” notice to run before command execution using a cached latest-version in state.json, and throttles notices to once per week while skipping update checks entirely for auto-updating package-manager installs.

Changes:

• Introduces install-method detection (homebrew, scoop, npm, package, script, unknown) and a cheap IsAutoUpdating gate to suppress notices and network checks for system packages.
• Adds cached update metadata (KnownLatestVersion, LastNotified) to state.json and uses it to show notices in PersistentPreRun.
• Adds tailored per-method upgrade commands and updates packaging/docs/config to support detection and messaging.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
README.md	Documents new update-notification behavior and env overrides.
packaging/install-source	Adds package-manager marker file content used for suppression.
internal/update.go	Caches latest version in state; adds weekly throttle + notification helpers; skips checks for auto-updating installs.
internal/update_test.go	Unit tests for cached-notification/throttle logic.
internal/state/state.go	Extends persisted state with LastNotified and KnownLatestVersion.
internal/install.go	Adds install-method detection + cheap auto-updating check.
internal/install_test.go	Unit tests for method detection and parsing.
internal/config/upsun-cli.yaml	Populates npm/installer fields used to build upgrade commands.
internal/config/schema.go	Extends wrapper config schema with npm/installer/install_method fields.
internal/config/platformsh-cli.yaml	Adds installer URL for the platform flavor (see review comments).
docs/design/update-message-install-detection.md	Design doc describing approach, precedence, and planned Phase 2.
commands/root.go	Moves notice to PersistentPreRun; prints from cache; runs background refresh; adds per-method upgrade commands.
commands/root_test.go	Tests upgrade-command selection per install method.
CLAUDE.md	Updates internal documentation about update checks and install detection.
.goreleaser.yaml	Installs package marker file in nfpm packages to enable suppression.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning · 🔵 4 minor points

🔍 Full review · 14 files reviewed

Verification

• The nfpm marker paths /usr/share/{upsun,platformsh-cli}/install-source match Application.Slug in both bundled configs, so packageMarkerExists (prefix = parent of /usr/bin) resolves them.
• scoop update <Application.Executable> matches the scoop manifest names (upsun, platform) in .goreleaser.yaml.
• The notice block runs after the quiet/colour writer setup and after the --version early exit, so it never writes to a discarded or uncoloured stderr.
• MarkNotified runs synchronously before the CheckForUpdate goroutine is launched, so that goroutine's state save does not clobber the just-written LastNotified in-process.
• shellQuote escapes single quotes as '\'' and the tests cover spaces, quotes and $(…) in the INSTALL_DIR path.

The diff adds unit tests for detection (internal/install_test.go), the weekly throttle core (internal/update_test.go) and the per-method upgrade strings (commands/root_test.go); the CI test job runs make test, golangci-lint and make goreleaser-check. Nothing covers the new PersistentPreRun wiring, IsAutoUpdating/marker lookup against a real filesystem, or that the generated installer command actually performs a raw install.

Review details

• Commit: 61e1b66
• Model: claude-opus-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points

🔁 Incremental · 10 files reviewed

🔵 Minor points

• internal/state/state.go:64 — state.Update returns early when Load fails, so an unparsable state.json is never rewritten. Previously CheckForUpdate and alt.Update loaded state, ignored the load error and called state.Save unconditionally, which overwrote a bad file with valid JSON. Save uses a plain os.WriteFile (non-atomic) and the CheckForUpdate/alt.Update goroutines are never joined — exitWithError/os.Exit can kill the process mid-write, or two concurrent CLI processes can interleave writes, leaving trailing garbage. After that, json.Unmarshal errors forever: Update never saves, LastChecked is never refreshed so every command performs the GitHub release request and the config fetch, and KnownLatestVersion/LastNotified are never stored, so the update notice never appears again. Treating an unmarshal error as empty state (or writing anyway) restores self-healing.
• internal/install.go:218 — ownedBySystemPackage runs dpkg-query -S, rpm -qf and apk info --who-owns with exec.Command(...).Run() — no context and no timeout — and it is reached synchronously from PersistentPreRun via internal.DetectInstallMethod(cnf) (commands/root.go:84) whenever a pending notice exists and the binary is in a standard bin dir on Linux. If the package database is slow or wedged (rpmdb on NFS, a stalled dpkg-query on a slow disk, an apk reading a hung network-mounted cache), the user's command blocks indefinitely before it has run at all, for a cosmetic notice. Using exec.CommandContext with a short deadline bounds the delay.

Verification

• INSTALL_METHOD=raw in the generated installer command is honoured: installer.sh's check_install_method only auto-picks apt/yum/apk when INSTALL_METHOD is empty, and the top-level flow then runs check_directories, which uses INSTALL_DIR.
• isHomebrew now requires /cellar/<formula>/ with formula from path.Base(cnf.Wrapper.HomebrewTap) ("upsun-cli"), and the empty-tap case is guarded by formula != ".", so /srv/cellar/bin/upsun no longer reports homebrew.
• state.Update serialises load-modify-save under a package mutex and all three writers (CheckForUpdate, MarkNotified, alt.Update) now go through it, so one goroutine no longer drops the other's fields.
• The clock-skew guard LastNotified <= now makes a future timestamp fall through to notifying, and LastNotified == now still throttles.
• The pkgOwned probe only runs on Linux after inStandardBinDir, so npm/scoop/homebrew paths still short-circuit before any subprocess.

Unit tests in the diff cover the new package-ownership branch, the cellar-formula narrowing and the future-LastNotified case (internal/install_test.go, internal/update_test.go), plus the updated INSTALL_METHOD=raw expectation in commands/root_test.go; they run in the CI test job (make test) alongside golangci-lint. Nothing tests state.Update's behaviour on an unreadable state file, and no test exercises ownedBySystemPackage itself (only the injected fake).

Review details

• Commits: 61e1b66..e1abdbf
• Model: claude-opus-5

---

Review 2 of 10 for this pull request · View the full run

Superseded: the latest Upsun Dispatch review no longer requests changes.

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points

🔁 Incremental · 3 files reviewed

🔵 Minor points

• internal/install.go:215 — The one-second deadline is created once and shared by all three package-database queries, and a timeout is indistinguishable from "not owned". On a Debian/Ubuntu host with a cold or large dpkg database, dpkg-query -S /usr/bin/upsun can exceed 1s; the context then expires, rpm and apk return immediately with the deadline error, ownedBySystemPackage returns false and detectInstallMethod reports InstallScript for a package install. The user is then shown curl -fsSL <installer> | INSTALL_METHOD=raw INSTALL_DIR=/usr/bin sh instead of the intended silence, i.e. a suggestion to overwrite a dpkg-owned file. A per-query deadline, or treating timeout as "unknown" rather than "not owned", would bound this.
• internal/state/state.go:66 — Update now discards every Load error, not just JSON corruption, and replaces the state with a zero State. Because Save uses a non-atomic os.WriteFile (truncate + write), a concurrent process — e.g. the alt.Update goroutine of another CLI invocation — can leave the file momentarily truncated/partial; a reader hitting that window unmarshals with an error, resets to State{} and writes back, dropping KnownLatestVersion, LastNotified and ConfigUpdates.LastChecked. The visible effect is a lost update notice or a repeated one. Writing via a temp file + rename, or only resetting on json.SyntaxError, would keep the repair behaviour without the cross-process data loss.

Verification

• state.Update takes mu and reloads before applying fn, so the CheckForUpdate and alt.Update goroutines in one process no longer clobber each other's fields.
• notificationFromState now requires LastNotified <= now before throttling, so a future timestamp from clock skew is treated as stale rather than suppressing notices forever.
• The script upgrade command now sets INSTALL_METHOD=raw alongside INSTALL_DIR, which is what installer.sh's check_install_method honours before falling back to apt/yum/brew.
• detectInstallMethod queries dpkg/rpm/apk only after inStandardBinDir and only on Linux, so the subprocess cost is not paid on macOS/Windows or for npm/scoop/brew paths.
• TestUpdateRepairsCorruptFile writes trailing garbage after valid JSON, which does make json.Unmarshal fail, so it genuinely exercises the new reset branch.

This increment adds internal/state/state_test.go covering the corrupt-file repair path; the shared-context timeout in ownedBySystemPackage has no test (it shells out to real package managers). Tests run via make test in the .github/workflows/ci.yml job.

Review details

• Commits: e1abdbf..e8bf4a2
• Model: claude-opus-5

---

Review 3 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points · ⚪ 1 nitpick

🔁 Incremental · 3 files reviewed

🔵 Minor points

• internal/state/state.go:67 — Save now replaces the state file with os.Rename over the destination. On Windows, os.ReadFile/os.Open (used by state.Load, in this process and in any other concurrently running CLI invocation) opens the file with FILE_SHARE_READ|FILE_SHARE_WRITE only — no FILE_SHARE_DELETE — so while a reader holds the handle, MoveFileEx(..., REPLACE_EXISTING) fails with a sharing violation and Save returns an error. Every caller (state.Update from CheckForUpdate, MarkNotified, alt.Update) discards that error with //nolint:errcheck, so on windows/amd64 the write is silently dropped: KnownLatestVersion/LastNotified are not persisted and the notice is lost or repeated. The previous os.WriteFile succeeded in that window because write sharing is permitted.
• internal/install.go:226 — The 1s timeout is now created per query instead of once for the whole function, and a timeout is treated as "owned". Two consequences: (a) worst case the function blocks 3s (dpkg-query + rpm + apk each timing out) instead of the previous 1s, and this runs synchronously in PersistentPreRun via DetectInstallMethod before the user's command starts; (b) a transient stall (dpkg frontend lock, cold/slow rpm DB, NFS home) on a machine where the CLI was actually installed by the script makes detection return InstallPackage, so commands/root.go skips printUpdateMessage yet still calls internal.MarkNotified(cnf) — the notice is suppressed and the weekly throttle is consumed, delaying the message another week.

---

⚪ Nitpick

• internal/state/state.go:55 — If the process dies (SIGKILL, Ctrl-C handling that bypasses defers, or a os.Exit such as the --version path in root.go) between os.CreateTemp and the defer os.Remove, a .state-*.tmp file is left behind in the user's writable dir and nothing ever cleans it up; repeated occurrences accumulate. The new test only observes the happy path, where the deferred remove runs.

Verification

• state.Save writes the temp file with os.CreateTemp's 0600 mode and renames in the same directory, so the state file stays private and on one filesystem.
• state.Update reloads under the package mutex before applying fn, so two goroutines in one process no longer drop each other's fields.
• ownedBySystemPackage now skips a missing tool via exec.LookPath before spending a timeout on it, and cancel() is called on every loop iteration (no context leak).
• ctx.Err() != nil in the loop can only be set by this query's own timeout, since cancel() runs after the check.

The diff adds assertions to TestUpdateRepairsCorruptFile (no leftover temp file, 0600 perms) covering the new atomic Save; the changed ownedBySystemPackage timeout/loop behaviour has no test. Both are exercised only by the make test step of the .github/workflows/ci.yml Go job, which runs on ubuntu-latest, so the rename-on-Windows path is not covered there.

Review details

• Commits: e8bf4a2..9097dd3
• Model: claude-opus-5

---

Review 4 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 1 minor point

🔁 Incremental · 2 files reviewed

🔵 Minor point

• internal/install.go:228 — ownedBySystemPackage now shares one 1-second context across all three probes instead of giving each its own. The loop's success test is err == nil || ctx.Err() != nil, so once the budget is consumed by an earlier probe, every remaining probe reports "owned" without running: exec.CommandContext(...).Run() on an already-expired context fails immediately at Start and ctx.Err() is non-nil. On a host where the first probe is slow (e.g. dpkg-query -S against a cold/large database taking ~1s and exiting non-zero for an unowned binary) and a second tool exists (rpm or apk, common on containers with mixed tooling), detection returns InstallPackage for a binary the OS does not manage. That makes detectInstallMethod classify a script install as package, so the CLI permanently suppresses update notices and skips the network check for that user.

Verification

• state.Save now falls back to a direct os.WriteFile only after writeAndRename fails, and the temp file is still removed by the deferred os.Remove on that path.
• writeAndRename correctly renames onto its new path parameter, matching the statePath passed by Save.
• The 1s timeout context in ownedBySystemPackage is cancelled via defer, so no context leak remains after the refactor.
• state.Update still serialises load-modify-save under the package mutex, so the new Save path does not reintroduce lost fields.

internal/state/state_test.go covers Update repairing a corrupt state file and leaving no temp files behind; nothing in the diff tests the Windows rename-fallback path or the shared-context timeout in ownedBySystemPackage. Tests run via .github/workflows/ci.yml.

Review details

• Commits: 9097dd3..f8c20ab
• Model: claude-opus-5

---

Review 5 of 10 for this pull request · View the full run