diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 79406403..0998cea5 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -282,6 +282,14 @@ nfpms: dst: /etc/bash_completion.d/platform - src: completion/zsh/_platform dst: /usr/local/share/zsh/site-functions/_platform + # Marks the install as managed by a system package manager, so the CLI + # suppresses its update message. See internal/install.go. The dst must stay + # at /share//install-source, where is the parent of + # the binary's dir (default bindir /usr/bin -> prefix /usr); changing the + # nfpm bindir would silently break suppression. + - src: packaging/install-source + dst: /usr/share/platformsh-cli/install-source + file_info: { mode: 0644 } apk: signature: key_file: "{{ with .Env.RSA_SIGNING_KEY_FILE }}{{ . }}{{ end }}" @@ -309,6 +317,14 @@ nfpms: dst: /etc/bash_completion.d/upsun - src: completion/zsh/_upsun dst: /usr/local/share/zsh/site-functions/_upsun + # Marks the install as managed by a system package manager, so the CLI + # suppresses its update message. See internal/install.go. The dst must stay + # at /share//install-source, where is the parent of + # the binary's dir (default bindir /usr/bin -> prefix /usr); changing the + # nfpm bindir would silently break suppression. + - src: packaging/install-source + dst: /usr/share/upsun/install-source + file_info: { mode: 0644 } apk: signature: key_file: "{{ with .Env.RSA_SIGNING_KEY_FILE }}{{ . }}{{ end }}" diff --git a/CLAUDE.md b/CLAUDE.md index c9cea0d8..dea872ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -163,4 +163,12 @@ PHP version is still injected via ldflags: ### Update Checks -The CLI checks for updates from GitHub releases (when Wrapper.GitHubRepo is set in config). This runs in a background goroutine and prints a message after command execution. +The CLI checks for updates from GitHub releases (when Wrapper.GitHubRepo is set in config). The network check runs in a background goroutine and caches the latest known version in `state.json`; the notice is shown before the command on a later run (see `internal/update.go`). + +Install-method detection (`internal/install.go`) tailors or suppresses the notice: +- System package managers (apt, yum/dnf, apk) are detected via a marker file installed by the nfpm packages (`packaging/install-source` → `/usr/share//install-source`). The notice is suppressed because the OS handles updates. +- Homebrew, Scoop, npm, and the bash installer get a tailored upgrade command, built from config fields (`Wrapper.HomebrewTap`, `Wrapper.NpmPackage`, `Wrapper.InstallerURL`, `Application.Executable`). +- The notice is throttled to once a week (`LastNotified` in state) and only shown in an interactive terminal. +- `INSTALL_METHOD` forces the method; `UPDATES_CHECK=0` disables checks entirely. + +See `docs/design/update-message-install-detection.md` for the full design, including the planned Phase 2 (opt-in self-update). diff --git a/README.md b/README.md index f495708d..3ddc382f 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,24 @@ sudo apt-get update && sudo apt-get upgrade upsun-cli sudo dnf upgrade -y upsun-cli ``` +### Update notifications + +When a newer release is available, the CLI prints a short notice with the +upgrade command for the tool you installed it with. It detects the install +method automatically and, for system package managers (apt, yum/dnf, apk), stays +silent because those update the CLI through the OS. + +The notice is shown at most once a week and only in an interactive terminal. To +control it: + +- `UPSUN_CLI_UPDATES_CHECK=0` disables update checks and notices entirely. +- `UPSUN_CLI_INSTALL_METHOD=` forces the detected install method, where + `` is one of `homebrew`, `scoop`, `npm`, `package`, or `script`. Use + `package` to silence the notice when the OS manages updates. + +(For the `platform` command, the prefix is `PLATFORMSH_CLI_` instead of +`UPSUN_CLI_`.) + ## Building Build a single binary: diff --git a/commands/root.go b/commands/root.go index 49178829..b4e2d13b 100644 --- a/commands/root.go +++ b/commands/root.go @@ -40,10 +40,7 @@ func Execute(cnf *config.Config) error { } func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cobra.Command { - var ( - updateMessageChan = make(chan *internal.ReleaseInfo, 1) - versionCommand = newVersionCommand(cnf) - ) + versionCommand := newVersionCommand(cnf) cmd := &cobra.Command{ Use: cnf.Application.Executable, Short: cnf.Application.Name, @@ -73,9 +70,16 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob os.Exit(0) } if cnf.Wrapper.GitHubRepo != "" { + // Show any update found by a previous run, before the command's + // output. The check itself runs in the background (below) and + // caches its result for the next invocation. + if rel := internal.PendingNotification(cnf, config.Version); rel != nil { + printUpdateMessage(cmd.ErrOrStderr(), rel, cnf) + internal.MarkNotified(cnf) + } go func() { - rel, _ := internal.CheckForUpdate(cnf, config.Version) - updateMessageChan <- rel + //nolint:errcheck // a failed update check should not affect the command + internal.CheckForUpdate(cnf, config.Version) }() } if alt.ShouldUpdate(cnf) { @@ -94,11 +98,6 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob }, PersistentPostRun: func(cmd *cobra.Command, _ []string) { checkShellConfigLeftovers(cmd.ErrOrStderr(), cnf) - select { - case rel := <-updateMessageChan: - printUpdateMessage(cmd.ErrOrStderr(), rel, cnf) - default: - } }, } @@ -204,19 +203,14 @@ func printUpdateMessage(w io.Writer, newRelease *internal.ReleaseInfo, cnf *conf return } - fmt.Fprintf(w, "\n\n%s %s → %s\n", + fmt.Fprintf(w, "\n%s %s → %s\n", color.YellowString(fmt.Sprintf("A new release of the %s is available:", cnf.Application.Name)), color.CyanString(config.Version), color.CyanString(newRelease.Version), ) - executable, err := os.Executable() - if err == nil && cnf.Wrapper.HomebrewTap != "" && isUnderHomebrew(executable) { - fmt.Fprintf( - w, - "To upgrade, run: brew update && brew upgrade %s\n", - color.YellowString(cnf.Wrapper.HomebrewTap), - ) + if cmd := upgradeCommand(cnf); cmd != "" { + fmt.Fprintf(w, "To upgrade, run: %s\n", color.YellowString(cmd)) } else if cnf.Wrapper.GitHubRepo != "" { fmt.Fprintf( w, @@ -228,19 +222,31 @@ func printUpdateMessage(w io.Writer, newRelease *internal.ReleaseInfo, cnf *conf fmt.Fprintf(w, "%s\n\n", color.YellowString(newRelease.URL)) } -func isUnderHomebrew(binary string) bool { - brewExe, err := exec.LookPath("brew") - if err != nil { - return false - } +// upgradeCommand returns the upgrade command for the detected install method, or +// an empty string when there is no tailored command (the caller then falls back +// to a generic link). +func upgradeCommand(cnf *config.Config) string { + return upgradeCommandFor(cnf, internal.DetectInstallMethod(cnf)) +} - brewPrefixBytes, err := exec.Command(brewExe, "--prefix").Output() - if err != nil { - return false +func upgradeCommandFor(cnf *config.Config, method internal.InstallMethod) string { + switch method { + case internal.InstallHomebrew: + if cnf.Wrapper.HomebrewTap != "" { + return "brew update && brew upgrade " + cnf.Wrapper.HomebrewTap + } + case internal.InstallScoop: + return "scoop update " + cnf.Application.Executable + case internal.InstallNpm: + if cnf.Wrapper.NpmPackage != "" { + return "npm install -g " + cnf.Wrapper.NpmPackage + "@latest" + } + case internal.InstallScript: + if cnf.Wrapper.InstallerURL != "" { + return "curl -fsSL " + cnf.Wrapper.InstallerURL + " | bash" + } } - - brewBinPrefix := filepath.Join(strings.TrimSpace(string(brewPrefixBytes)), "bin") + string(filepath.Separator) - return strings.HasPrefix(binary, brewBinPrefix) + return "" } func debugLogf(format string, v ...any) { diff --git a/commands/root_test.go b/commands/root_test.go new file mode 100644 index 00000000..29ba46e1 --- /dev/null +++ b/commands/root_test.go @@ -0,0 +1,42 @@ +package commands + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/internal" +) + +func TestUpgradeCommandFor(t *testing.T) { + cnf := testConfig() + cnf.Wrapper.HomebrewTap = "upsun/tap/upsun-cli" + cnf.Wrapper.NpmPackage = "upsun" + cnf.Wrapper.InstallerURL = "https://example.com/installer.sh" + cnf.Application.Executable = "upsun" + + cases := []struct { + method internal.InstallMethod + want string + }{ + {internal.InstallHomebrew, "brew update && brew upgrade upsun/tap/upsun-cli"}, + {internal.InstallScoop, "scoop update upsun"}, + {internal.InstallNpm, "npm install -g upsun@latest"}, + {internal.InstallScript, "curl -fsSL https://example.com/installer.sh | bash"}, + {internal.InstallPackage, ""}, // suppressed; no tailored command + {internal.InstallUnknown, ""}, // falls back to the generic link + } + for _, c := range cases { + t.Run(string(c.method), func(t *testing.T) { + assert.Equal(t, c.want, upgradeCommandFor(cnf, c.method)) + }) + } +} + +func TestUpgradeCommandForMissingConfigFallsBack(t *testing.T) { + cnf := testConfig() // no Wrapper.* fields set + // Homebrew/npm/script require their config field; without it, fall back (empty). + assert.Empty(t, upgradeCommandFor(cnf, internal.InstallHomebrew)) + assert.Empty(t, upgradeCommandFor(cnf, internal.InstallNpm)) + assert.Empty(t, upgradeCommandFor(cnf, internal.InstallScript)) +} diff --git a/docs/design/update-message-install-detection.md b/docs/design/update-message-install-detection.md new file mode 100644 index 00000000..721ad1e7 --- /dev/null +++ b/docs/design/update-message-install-detection.md @@ -0,0 +1,422 @@ +# Design: install-method detection for update messages + +Status: draft +Author: Claude Code (for Patrick Dawkins) +Date: 2026-06-14 + +## Background + +The CLI checks GitHub for a newer release and, if one exists, prints a message +after the command runs. The check runs in a background goroutine started in +`PersistentPreRun` (`commands/root.go:75`), and the message is printed in +`PersistentPostRun` via a non-blocking channel read (`commands/root.go:95`). + +The check itself lives in `internal/update.go`: + +- `CheckForUpdate` gates on `shouldCheckForUpdate`, throttles to + `Updates.CheckInterval` (default 3600s) using `state.json`, fetches + `releases/latest` from GitHub, and compares versions. +- `shouldCheckForUpdate` (`internal/update.go:61`) skips the check for dev + builds, when `Updates.Check` is false, when `UPDATES_CHECK=0`, in CI, + and when stdout/stderr are not terminals. + +The message is formatted by `printUpdateMessage` (`commands/root.go:202`). Today +it tailors the upgrade instruction in exactly one way: if the binary sits under +the Homebrew prefix (`isUnderHomebrew`, which shells out to `brew --prefix`), it +prints `brew update && brew upgrade `. Otherwise it prints a generic +`https://github.com/#upgrade` link. + +## Problem + +The message is shown identically regardless of how the CLI was installed, and it +is shown even when the user cannot act on it usefully: + +1. For users who installed via a system package manager (`apt`, `yum`/`dnf`, + `apk`), the OS updates the CLI on its normal schedule. Telling them to visit a + GitHub page is noise, and following it would fight the package manager. +2. For Scoop, npm, and the bash installer, the generic GitHub link is less + helpful than the one command that actually upgrades their installation. + +## Goals + +- Suppress the update message entirely when the CLI is managed by an + auto-updating system package manager (`apt`, `yum`/`dnf`, `apk`). +- For manually-updated channels, print the exact upgrade command for that + channel (Homebrew, Scoop, npm, bash installer). +- Fall back to today's generic GitHub link when the method is unknown. +- Make the notice substantially less naggy: interactive-only, throttled to once + a week, shown before the command rather than buried after it. +- Offer one-tap, opt-in self-update for channels where the upgrade command is + local and unprivileged, then re-run the original command on the new version. +- Keep detection off the synchronous command path (the network check already + runs in the background goroutine) and avoid new latency on every invocation. +- Keep the design vendorizable — no hard-coded `upsun`-specific strings in Go. +- Provide an escape hatch to force or override detection. + +## Non-goals + +- Silent or unattended self-update. Updates are only ever performed after an + explicit interactive prompt. `self:update` (the PHP command) stays disabled. +- Auto-running privileged or remote-code upgrades (`sudo`, `curl … | sh`). For + those channels we print the command rather than executing it (see Phase 2). +- Changing the network-check throttle, the CI gate, or the TTY gate. +- Detecting manual `dpkg -i` / `rpm -i` installs that bypass a repo. These are + treated as "package" (auto-updating); see Edge cases. + +## Phasing + +This grew from "tailor the message" into "opt-in self-update", so it ships in +phases: + +- **Phase 1** — install-method detection, suppress for packages, tailor wording, + weekly display throttle, show-before-via-cache. No process re-exec; low risk. +- **Phase 2** — interactive prompt + auto-update + re-exec for the unprivileged + channels (Homebrew, Scoop, npm). +- **Phase 3 (optional)** — move the bash installer's `raw` method to a + user-local path, which removes `sudo` and lets `script` installs join the + one-tap auto-update set. + +## Distribution channel inventory + +Verified from `.goreleaser.yaml`, `installer.sh`, and `npm/wrapper/`. + +| Channel | Where the running binary lives | Auto-updates? | Desired behavior | +|---|---|---|---| +| Homebrew (`brews:`) | `$(brew --prefix)/bin/` (symlink into `…/Cellar/`) | No | `brew update && brew upgrade ` | +| Scoop (`scoops:`, Windows) | `…\scoop\apps\\current\.exe` (shim in `…\scoop\shims\`) | No | `scoop update ` | +| npm (`npm/wrapper/`) | `…/node_modules/@upsun/cli-/bin/` | No | `npm install -g @latest` | +| apt / `.deb` (`nfpms:`) | `/usr/bin/` | Yes | suppress | +| yum·dnf / `.rpm` (`nfpms:`) | `/usr/bin/` | Yes | suppress | +| apk / `.apk` (`nfpms:`) | `/usr/bin/` | Yes | suppress | +| bash installer, raw method | Linux: `/usr/bin/`; macOS: `/usr/local/bin/`; or `$INSTALL_DIR` | No | re-run installer command | +| Unknown / built from source | anywhere | — | generic GitHub `#upgrade` link | + +### The core ambiguity + +On Linux the bash installer's `raw` method installs to `/usr/bin` (`installer.sh:37`), +which is the same location nfpm packages use. **Path alone cannot distinguish a +distro package (auto-updating, suppress) from a raw script install (manual, +show).** This is exactly the distinction the feature needs, so a path heuristic +is insufficient on Linux; we need a positive signal that a package manager owns +the binary. + +The bash installer's `apt`/`yum`/`apk` methods install the distro package from +`repositories.upsun.com`, so those collapse into the package case and carry +whatever signal the package carries. + +## Detection design + +### A positive package marker (resolves the Linux ambiguity) + +Have the nfpm packages install a small marker file that the binary can read at +runtime. nfpm `contents` can place a file at any absolute path: + +```yaml +# .goreleaser.yaml, in each nfpms entry +contents: + - src: packaging/install-source + dst: /usr/share//install-source + file_info: { mode: 0644 } +``` + +`packaging/install-source` contains the single word `package`. We do not need to +distinguish apt/yum/apk for suppression — all three auto-update — so one marker +value is enough, and we avoid splitting the single nfpm entry into three. + +At runtime, presence of the marker (found relative to the resolved executable, or +at the known `/usr/share//install-source` path) means "managed by a system +package manager → auto-updating". Its absence at `/usr/bin/` means the +binary got there some other way (raw installer) → not suppressed. + +This keeps all packaging changes inside this repo's `.goreleaser.yaml`; the +Homebrew tap and Scoop manifests (in `upsun/homebrew-tap`) need no changes +because those channels are detected by path. + +### Detection precedence + +A new function `DetectInstallMethod(cnf) InstallMethod` resolves the method once +(memoized with `sync.Once`). Order, first match wins: + +1. **Override.** `INSTALL_METHOD` env var (e.g. `UPSUN_CLI_INSTALL_METHOD`), + or an optional `wrapper.install_method` config key. Parsed into the enum; + unknown values are ignored with a debug log. Lets CI, distro maintainers, and + power users force behavior. +2. **Package marker.** Marker file present → `package`. +3. **npm.** Resolved executable path contains `node_modules` and the package + scope (`@/cli-` derived from config, or simply `node_modules`). → `npm`. +4. **Scoop** (Windows only). Path contains a `scoop{sep}` segment. → `scoop`. +5. **Homebrew.** Existing `isUnderHomebrew` logic (path under `brew --prefix`), + plus a cheap pre-check for `/Cellar/` in the path to avoid the subprocess when + obviously not Homebrew. → `homebrew`. +6. **Script.** Otherwise, if the binary is in a normal bin dir + (`/usr/bin`, `/usr/local/bin`, `$INSTALL_DIR`-style) → `script`. +7. **Unknown.** Anything else (e.g. `go run`, source builds). → `unknown`. + +Resolve symlinks with `filepath.EvalSymlinks(os.Executable())` before matching so +that shimmed/symlinked installs report their real location. + +Because detection runs inside the background goroutine (`CheckForUpdate`) and at +message time — never on the synchronous hot path — shelling out to `brew --prefix` +remains acceptable. Memoization means it happens at most once per process. + +### Enum + +```go +type InstallMethod string + +const ( + InstallUnknown InstallMethod = "" + InstallHomebrew InstallMethod = "homebrew" + InstallScoop InstallMethod = "scoop" + InstallNpm InstallMethod = "npm" + InstallPackage InstallMethod = "package" // apt / yum / dnf / apk + InstallScript InstallMethod = "script" // bash installer, raw method +) + +// AutoUpdating reports whether the host system updates the CLI on its own. +func (m InstallMethod) AutoUpdating() bool { return m == InstallPackage } +``` + +### Wiring into the check + +In `shouldCheckForUpdate`, short-circuit when auto-updating so we skip both the +message and the network request: + +```go +if DetectInstallMethod(cnf).AutoUpdating() { + return false +} +``` + +`printUpdateMessage` switches on the method to choose the upgrade line. Since +`AutoUpdating` methods never reach the message, the switch only handles the +manual channels plus the unknown fallback (current behavior preserved). + +## Behavior matrix + +"Auto-updatable" means the upgrade command is local and unprivileged, so we may +offer to run it for the user (Phase 2). `script`/`unknown` only ever print a +hint. + +| Method | Notice shown? | Upgrade command | Auto-updatable? | +|---|---|---|---| +| `package` | No (suppressed) | — | — (OS handles it) | +| `homebrew` | Yes | `brew update && brew upgrade ` | Yes | +| `scoop` | Yes | `scoop update ` | Yes | +| `npm` | Yes | `npm install -g @latest` | Yes | +| `script` | Yes | `curl -fsSL \| sh` | No (remote + maybe sudo)¹ | +| `unknown` | Yes | `follow the instructions at https://github.com/#upgrade` (today's text) | No | + +¹ Becomes auto-updatable in Phase 3 if raw installs move to a user-local, +sudo-free path. + +All upgrade strings are built from config fields, not literals, so vendor builds +get correct instructions. + +## Reducing nagginess + +The point of the notice is to be useful, not to interrupt. Four changes make it +quiet enough to keep on by default, plus an opt-in updater. + +### Interactive-only + +The passive one-line notice shows on any TTY (today's gate). The *prompt* and any +auto-update only happen when fully interactive: TTY **and** not `--yes` / +`--no-interaction`. Also skip plumbing commands (`completion`, `--version`) so we +never corrupt machine-readable output. + +### Weekly display throttle + +Decouple "how often we check the network" (hourly, `LastChecked`) from "how often +we tell the user" (weekly). Add a separate timestamp; set it whenever we show the +notice or prompt, regardless of whether the user accepts or declines, so we stay +quiet for a week either way. + +### Show before the command, via a cache + +Today the notice prints in `PersistentPostRun`, after the output, because the +background check may not have finished. Instead, cache the latest known version +in `state.json`. `PersistentPreRun` reads the cache and shows/prompts +immediately — no blocking — while the background goroutine refreshes the cache +for the *next* invocation. The notice therefore lags real availability by one +command, which is acceptable, and it appears before output (and is a prerequisite +for prompting, which cannot happen after the command has run). + +State (`internal/state/state.go`) gains: + +```go +Updates struct { + LastChecked int64 `json:"last_checked"` + LastNotified int64 `json:"last_notified,omitempty"` + KnownLatestVersion string `json:"known_latest_version,omitempty"` +} +``` + +### Opt-in one-tap update + re-exec (Phase 2) + +When fully interactive, an update is known, the weekly throttle allows it, and +the method is auto-updatable (Homebrew/Scoop/npm), prompt: + +``` +A new release of the Upsun CLI is available: v2.3.1 → v2.4.0 +Update now? [Y/n] +``` + +- **Default.** Enter = yes (your "just hit enter" flow) — but *only* for the + unprivileged channels. We never default-run a `sudo` or `curl … | sh` upgrade; + for `script`/`unknown` we print the command and do not prompt to run it. +- **On accept.** Run the channel's upgrade command, streaming its output. On + success, re-resolve the (possibly relocated) executable and re-run the original + command on the new version: + - Unix: `syscall.Exec(newBinary, os.Args, os.Environ())` — replaces the process + image. Safe because we prompt in `PersistentPreRun`, before any command logic + has run. + - Windows: spawn the new binary with the original args, forward stdio, exit with + its status (the pattern `npm/wrapper/bin/upsun.js` already uses). + - On upgrade failure: print a warning and continue with the current version — + never block the user's command. +- **On decline.** Set `LastNotified` and run the command normally; stay quiet for + a week. + +Because the prompt and update run in `PersistentPreRun`, and the legacy PHP layer +is already invoked with `UPDATES_CHECK=0` (`internal/legacy/legacy.go:139`), +there is no double-notification when delegating to PHP commands. + +### The bash installer's destination (Phase 3, optional) + +The `raw` method installs to `/usr/bin` on Linux (`installer.sh:37`), which needs +`sudo` and collides with the package path (the source of the detection +ambiguity). Targeting a user-local dir instead (`$XDG_BIN_HOME`, else +`~/.local/bin`) would remove `sudo`, make `script` unambiguous from `package` +without relying on the marker, and let `script` installs join the one-tap update +set. Back-compat: existing `/usr/bin` installs stay where they are; PATH ordering +must be considered. Tracked separately from Phases 1–2. + +## Config schema changes + +Add to the `Wrapper` struct (`internal/config/schema.go:22`): + +```go +Wrapper struct { + HomebrewTap string `yaml:"homebrew_tap,omitempty"` + GitHubRepo string `yaml:"github_repo,omitempty"` + NpmPackage string `yaml:"npm_package,omitempty"` // e.g. "upsun" + InstallerURL string `yaml:"installer_url,omitempty"` // e.g. "https://raw.githubusercontent.com/upsun/cli/main/installer.sh" + InstallMethod string `yaml:"install_method,omitempty"` // optional forced override +} +``` + +Populate `npm_package` and `installer_url` in `internal/config/upsun-cli.yaml` and +`internal/config/platformsh-cli.yaml`. A channel with an empty config field +simply falls back to the generic GitHub link for that branch (graceful for vendor +builds that don't ship that channel). + +## Code changes + +Phase 1: + +- `internal/update.go` + - New `InstallMethod` type + `AutoUpdating`. + - New `DetectInstallMethod(cnf)` (memoized) with the precedence above. + - `shouldCheckForUpdate`: short-circuit on `AutoUpdating()`. + - Cache the latest known version into state during the background check. +- `internal/state/state.go` + - Add `LastNotified` and `KnownLatestVersion`. +- `commands/root.go` + - Read the cached version in `PersistentPreRun`; show the tailored notice there + (throttled weekly, interactive-only) instead of in `PersistentPostRun`. + - `printUpdateMessage`: replace the Homebrew-only `if/else` with a `switch` on + `DetectInstallMethod(cnf)`. Move/keep `isUnderHomebrew` as a detector helper. + +Phase 2: + +- New `internal/update` updater: `Upgrade(ctx, method, cnf)` runs the channel's + command; `reexec(args)` does `syscall.Exec` (Unix) / spawn-and-forward (Windows). +- `commands/root.go` `PersistentPreRun`: when interactive + auto-updatable + + update known + weekly-allowed, prompt; on accept, `Upgrade` then `reexec`. + +## Packaging changes + +- `.goreleaser.yaml`: add the `install-source` marker to both `nfpms` entries' + `contents`. +- `packaging/install-source`: new one-line file containing `package`. +- `installer.sh`: no change required for suppression (its `apt`/`yum`/`apk` + methods install the marked package). Optional follow-up: have the `raw` method + export `UPSUN_CLI_INSTALL_METHOD=script` guidance in its success output, or + write a `script` marker, so `script` detection is explicit rather than inferred. + +## Edge cases + +- **Manual `dpkg -i` / `rpm -i` of a downloaded package** still carries the + marker, so it is treated as `package` and suppressed even though it will not + auto-update from a repo. Acceptable: the user chose a package format and can + re-run the same install, and the GitHub releases page is still discoverable. +- **`brew --prefix` unavailable / slow.** The `/Cellar/` pre-check avoids the + subprocess in the common non-Homebrew case; if `brew` is missing we fall + through to `script`/`unknown`, matching today's behavior. +- **npm global vs local, pnpm, nested installs.** Matching on `node_modules` in + the resolved executable path covers flat, nested, and pnpm layouts (the same + resolution `npm/wrapper/bin/upsun.js` relies on). +- **`$INSTALL_DIR` to a non-standard dir.** Reports `script` (no marker, not a + known package path) — correct. +- **Windows non-Scoop (raw zip on PATH).** No marker, not under scoop → `unknown` + → generic link. Acceptable until a Windows installer exists. + +## Known limitations + +- `state.json` has no file locking (`state.Save` is a bare write). Two CLI + invocations running concurrently can lose each other's writes: the background + `CheckForUpdate` of one process can overwrite the `LastNotified` just written + by another process's `MarkNotified`, so the weekly throttle may occasionally + let a notice through more than once. This races on a cosmetic field only, and + the file was already written without locking before this change. Fixing it + properly (advisory lock around load-modify-save) is out of scope for Phase 1. + +## Testing plan + +Table-driven tests in `internal/update_test.go` for `DetectInstallMethod`, +injecting the executable path, GOOS, env, and a marker-file probe via small +interfaces/fakes (avoid real `brew`/filesystem): + +- marker present → `package`; `AutoUpdating()` true. +- `…/node_modules/@upsun/cli-linux-x64/bin/upsun` → `npm`. +- `…\scoop\apps\upsun\current\upsun.exe` (GOOS=windows) → `scoop`. +- brew-prefix path → `homebrew`. +- `/usr/bin/upsun` no marker → `script`. +- `INSTALL_METHOD=homebrew` overrides a `/usr/bin` path. +- `go run` temp path → `unknown`. + +For `shouldCheckForUpdate`: assert it returns false when the method is a package, +true otherwise (given the other gates pass). For `printUpdateMessage`: assert the +upgrade line per method, including empty-config fallbacks. + +## Docs to update + +- `README.md` `#upgrade` section — note that package-manager installs update via + the OS and that the message is suppressed there. +- Upsun docs CLI admin page + (`sites/upsun/src/administration/cli`) — document `INSTALL_METHOD` and + reconfirm `UPDATES_CHECK=0`. +- `CLAUDE.md` "Update Checks" section — mention install-method detection. + +## Resolved decisions + +- `package` installs: total silence, no hint. The OS handles updates. +- Manual `dpkg -i`/`rpm -i`: treated as `package` (suppressed). Acceptable. +- npm upgrade command: global `npm install -g @latest`. +- Ambiguous / no marker: fall back to `script`/`unknown`, advise re-running the + installer (or the GitHub link). +- Prompt default: Enter = yes, but only for unprivileged channels + (Homebrew/Scoop/npm). Never default-run `sudo`/`curl | sh`. + +## Open questions + +1. Phase 3: do we move the bash installer's `raw` destination to a user-local + path? It simplifies detection and unlocks `script` auto-update, but changes + long-standing behavior and PATH expectations. +2. Windows re-exec after update: spawn-and-forward is more complex than Unix + `syscall.Exec`. Acceptable to ship Phase 2 for macOS/Linux first and print the + command on Windows until the spawn path is built? +3. Should there be a config/env switch to disable just the *prompt* (keep the + passive notice) for users who want to be told but never asked to act? Or is + `UPDATES_CHECK=0` (disables everything) enough? diff --git a/internal/config/platformsh-cli.yaml b/internal/config/platformsh-cli.yaml index dd7592e0..606b0d8c 100644 --- a/internal/config/platformsh-cli.yaml +++ b/internal/config/platformsh-cli.yaml @@ -6,6 +6,7 @@ wrapper: homebrew_tap: upsun/tap/platformsh-cli github_repo: upsun/cli + installer_url: https://raw.githubusercontent.com/upsun/cli/main/installer.sh application: name: "Upsun CLI (Platform.sh compatibility)" diff --git a/internal/config/schema.go b/internal/config/schema.go index e995c802..476f5f6a 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -20,8 +20,11 @@ import ( type Config struct { // Fields only used by to the Go wrapper. Wrapper struct { - HomebrewTap string `yaml:"homebrew_tap,omitempty"` // e.g. "upsun/tap/platformsh-cli" - GitHubRepo string `yaml:"github_repo,omitempty"` // e.g. "upsun/cli" + HomebrewTap string `yaml:"homebrew_tap,omitempty"` // e.g. "upsun/tap/platformsh-cli" + GitHubRepo string `yaml:"github_repo,omitempty"` // e.g. "upsun/cli" + NpmPackage string `yaml:"npm_package,omitempty"` // e.g. "upsun" + InstallerURL string `yaml:"installer_url,omitempty"` // e.g. "https://raw.githubusercontent.com/upsun/cli/main/installer.sh" + InstallMethod string `yaml:"install_method,omitempty"` // forces the detected install method (homebrew, scoop, npm, package, script) } `yaml:"wrapper,omitempty"` Application struct { diff --git a/internal/config/upsun-cli.yaml b/internal/config/upsun-cli.yaml index 7fff2036..d72bd35f 100644 --- a/internal/config/upsun-cli.yaml +++ b/internal/config/upsun-cli.yaml @@ -4,6 +4,8 @@ wrapper: homebrew_tap: upsun/tap/upsun-cli github_repo: upsun/cli + npm_package: upsun + installer_url: https://raw.githubusercontent.com/upsun/cli/main/installer.sh application: name: "Upsun CLI" diff --git a/internal/install.go b/internal/install.go new file mode 100644 index 00000000..9c60dc5c --- /dev/null +++ b/internal/install.go @@ -0,0 +1,211 @@ +package internal + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + + "github.com/upsun/cli/internal/config" +) + +// InstallMethod identifies how the CLI binary was installed, so update messages +// can be tailored or suppressed. +type InstallMethod string + +const ( + InstallUnknown InstallMethod = "" + InstallHomebrew InstallMethod = "homebrew" + InstallScoop InstallMethod = "scoop" + InstallNpm InstallMethod = "npm" + InstallPackage InstallMethod = "package" // a system package manager: apt, yum/dnf, apk + InstallScript InstallMethod = "script" // the bash installer's raw method +) + +// AutoUpdating reports whether the host system updates the CLI on its own, in +// which case the CLI should stay quiet about new releases. +func (m InstallMethod) AutoUpdating() bool { + return m == InstallPackage +} + +// installProbe holds the (injectable) inputs to install-method detection. +type installProbe struct { + goos string + exe string // the resolved (symlinks followed) executable path + envPrefix string + slug string + configMethod string + getenv func(string) string + fileExists func(string) bool + brewPrefix func() (string, bool) +} + +var ( + detectOnce sync.Once + detectedMethod InstallMethod +) + +// DetectInstallMethod resolves the install method once per process. It may shell +// out to "brew", so it is only called when about to print an update message, +// never on the hot path of every command. +func DetectInstallMethod(cnf *config.Config) InstallMethod { + detectOnce.Do(func() { + detectedMethod = detectInstallMethod(&installProbe{ + goos: runtime.GOOS, + exe: resolvedExecutable(), + envPrefix: cnf.Application.EnvPrefix, + slug: cnf.Application.Slug, + configMethod: cnf.Wrapper.InstallMethod, + getenv: getenvFunc, + fileExists: fileExists, + brewPrefix: brewPrefix, + }) + }) + return detectedMethod +} + +// IsAutoUpdating is a cheap check (no subprocess) suitable for the hot path. It +// only considers an explicit override and the package marker file. +func IsAutoUpdating(cnf *config.Config) bool { + if m, ok := overrideMethod(cnf.Application.EnvPrefix, cnf.Wrapper.InstallMethod, getenvFunc); ok { + return m.AutoUpdating() + } + return packageMarkerExists(resolvedExecutable(), cnf.Application.Slug, fileExists) +} + +func detectInstallMethod(p *installProbe) InstallMethod { + if m, ok := overrideMethod(p.envPrefix, p.configMethod, p.getenv); ok { + return m + } + if packageMarkerExists(p.exe, p.slug, p.fileExists) { + return InstallPackage + } + n := normPath(p.exe) + if strings.Contains(n, "/node_modules/") { + return InstallNpm + } + if p.goos == "windows" && strings.Contains(n, "/scoop/") { + return InstallScoop + } + if isHomebrew(p.exe, p.brewPrefix) { + return InstallHomebrew + } + if inStandardBinDir(p.exe) { + return InstallScript + } + return InstallUnknown +} + +// overrideMethod reads a forced install method from the environment or config. +func overrideMethod(envPrefix, configMethod string, getenv func(string) string) (InstallMethod, bool) { + if v := getenv(envPrefix + "INSTALL_METHOD"); v != "" { + return parseMethod(v) + } + if configMethod != "" { + return parseMethod(configMethod) + } + return InstallUnknown, false +} + +func parseMethod(v string) (InstallMethod, bool) { + switch strings.ToLower(strings.TrimSpace(v)) { + case string(InstallHomebrew), "brew": + return InstallHomebrew, true + case string(InstallScoop): + return InstallScoop, true + case string(InstallNpm): + return InstallNpm, true + case string(InstallScript): + return InstallScript, true + case string(InstallPackage), "apt", "yum", "dnf", "apk", "deb", "rpm": + return InstallPackage, true + } + return InstallUnknown, false +} + +// packageMarkerExists reports whether the system package installed a marker file. +// Packages install the binary at /bin/; the marker sits alongside at +// /share//install-source. +func packageMarkerExists(exe, slug string, fileExists func(string) bool) bool { + if exe == "" || slug == "" { + return false + } + prefix := filepath.Dir(filepath.Dir(exe)) + return fileExists(filepath.Join(prefix, "share", slug, "install-source")) +} + +func isHomebrew(exe string, brewPrefix func() (string, bool)) bool { + n := normPath(exe) + // Homebrew bins symlink into the Cellar; the resolved path reveals it cheaply. + if strings.Contains(n, "/cellar/") { + return true + } + if prefix, ok := brewPrefix(); ok && prefix != "" { + if strings.HasPrefix(n, normPath(prefix)+"/bin/") { + return true + } + } + return false +} + +func inStandardBinDir(exe string) bool { + if exe == "" { + return false + } + dir := strings.TrimSuffix(normPath(filepath.Dir(exe)), "/") + switch dir { + case "/usr/bin", "/usr/local/bin", "/bin", "/usr/sbin", "/sbin", "/opt/bin": + return true + } + return strings.HasSuffix(dir, "/.local/bin") +} + +// normPath lowercases and normalizes separators so matching works regardless of +// the host OS (useful for cross-platform tests and Windows paths). +func normPath(p string) string { + return strings.ToLower(strings.ReplaceAll(p, "\\", "/")) +} + +var ( + resolvedExeOnce sync.Once + resolvedExe string +) + +// resolvedExecutable returns the current executable with symlinks followed, so +// Homebrew/Scoop shims resolve to their real location. Memoized. +func resolvedExecutable() string { + resolvedExeOnce.Do(func() { + exe, err := os.Executable() + if err != nil { + return + } + if r, err := filepath.EvalSymlinks(exe); err == nil { + exe = r + } + resolvedExe = exe + }) + return resolvedExe +} + +func getenvFunc(k string) string { + return os.Getenv(k) +} + +func fileExists(p string) bool { + _, err := os.Stat(p) + return err == nil +} + +func brewPrefix() (string, bool) { + exe, err := exec.LookPath("brew") + if err != nil { + return "", false + } + out, err := exec.Command(exe, "--prefix").Output() + if err != nil { + return "", false + } + return strings.TrimSpace(string(out)), true +} diff --git a/internal/install_test.go b/internal/install_test.go new file mode 100644 index 00000000..5094c9ec --- /dev/null +++ b/internal/install_test.go @@ -0,0 +1,164 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDetectInstallMethod(t *testing.T) { + noBrew := func() (string, bool) { return "", false } + brewAt := func(prefix string) func() (string, bool) { + return func() (string, bool) { return prefix, true } + } + noFile := func(string) bool { return false } + fileAt := func(want string) func(string) bool { + return func(p string) bool { return p == want } + } + + cases := []struct { + name string + probe installProbe + want InstallMethod + }{ + { + name: "env override wins over path", + probe: installProbe{ + goos: "linux", exe: "/usr/bin/upsun", envPrefix: "UPSUN_CLI_", slug: "upsun", + getenv: func(k string) string { return map[string]string{"UPSUN_CLI_INSTALL_METHOD": "homebrew"}[k] }, + fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallHomebrew, + }, + { + name: "config override", + probe: installProbe{ + goos: "linux", exe: "/usr/bin/upsun", slug: "upsun", configMethod: "npm", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallNpm, + }, + { + name: "override alias apt maps to package", + probe: installProbe{ + goos: "linux", exe: "/somewhere/upsun", envPrefix: "UPSUN_CLI_", slug: "upsun", + getenv: func(k string) string { return map[string]string{"UPSUN_CLI_INSTALL_METHOD": "apt"}[k] }, + fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallPackage, + }, + { + name: "package marker present", + probe: installProbe{ + goos: "linux", exe: "/usr/bin/upsun", slug: "upsun", + getenv: func(string) string { return "" }, brewPrefix: noBrew, + fileExists: fileAt("/usr/share/upsun/install-source"), + }, + want: InstallPackage, + }, + { + name: "usr-bin without marker is script", + probe: installProbe{ + goos: "linux", exe: "/usr/bin/upsun", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallScript, + }, + { + name: "npm node_modules path", + probe: installProbe{ + goos: "linux", exe: "/home/u/project/node_modules/@upsun/cli-linux-x64/bin/upsun", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallNpm, + }, + { + name: "scoop path on windows", + probe: installProbe{ + goos: "windows", exe: `C:\Users\u\scoop\apps\upsun\current\upsun.exe`, slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallScoop, + }, + { + name: "scoop path on linux is not detected as scoop", + probe: installProbe{ + goos: "linux", exe: "/home/u/scoop/apps/upsun/upsun", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallUnknown, + }, + { + name: "user-local bin is script", + probe: installProbe{ + goos: "linux", exe: "/home/u/.local/bin/upsun", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallScript, + }, + { + name: "homebrew via cellar symlink target", + probe: installProbe{ + goos: "darwin", exe: "/opt/homebrew/Cellar/upsun-cli/2.0.0/bin/upsun", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallHomebrew, + }, + { + name: "homebrew via brew prefix", + probe: installProbe{ + goos: "darwin", exe: "/usr/local/bin/upsun", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: brewAt("/usr/local"), + }, + want: InstallHomebrew, + }, + { + name: "unknown for go-build temp path", + probe: installProbe{ + goos: "linux", exe: "/tmp/go-build123/b001/exe/platform", slug: "upsun", + getenv: func(string) string { return "" }, fileExists: noFile, brewPrefix: noBrew, + }, + want: InstallUnknown, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, detectInstallMethod(&c.probe)) + }) + } +} + +func TestInstallMethodAutoUpdating(t *testing.T) { + assert.True(t, InstallPackage.AutoUpdating()) + assert.False(t, InstallHomebrew.AutoUpdating()) + assert.False(t, InstallScript.AutoUpdating()) + assert.False(t, InstallUnknown.AutoUpdating()) +} + +func TestParseMethod(t *testing.T) { + cases := []struct { + in string + want InstallMethod + ok bool + }{ + {"homebrew", InstallHomebrew, true}, + {"brew", InstallHomebrew, true}, + {"APT", InstallPackage, true}, + {"dnf", InstallPackage, true}, + {"apk", InstallPackage, true}, + {"npm", InstallNpm, true}, + {" scoop ", InstallScoop, true}, + {"nonsense", InstallUnknown, false}, + {"", InstallUnknown, false}, + } + for _, c := range cases { + t.Run(c.in, func(t *testing.T) { + got, ok := parseMethod(c.in) + assert.Equal(t, c.ok, ok) + if c.ok { + assert.Equal(t, c.want, got) + } + }) + } +} diff --git a/internal/state/state.go b/internal/state/state.go index 1a0e65a1..cef446d5 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -10,7 +10,9 @@ import ( type State struct { Updates struct { - LastChecked int64 `json:"last_checked"` + LastChecked int64 `json:"last_checked"` + LastNotified int64 `json:"last_notified,omitempty"` + KnownLatestVersion string `json:"known_latest_version,omitempty"` } `json:"updates,omitempty"` ConfigUpdates struct { diff --git a/internal/update.go b/internal/update.go index e7dee168..86196d52 100644 --- a/internal/update.go +++ b/internal/update.go @@ -46,6 +46,10 @@ func CheckForUpdate(cnf *config.Config, currentVersion string) (*ReleaseInfo, er return nil, fmt.Errorf("could not determine latest release: %w", err) } + // Cache the latest known version so the next invocation can show a message + // before its command runs, without blocking on the network. + s.Updates.KnownLatestVersion = releaseInfo.Version + cmp, err := version.Compare(releaseInfo.Version, currentVersion) if err != nil { return nil, fmt.Errorf("could not compare versions: %w", err) @@ -57,13 +61,64 @@ func CheckForUpdate(cnf *config.Config, currentVersion string) (*ReleaseInfo, er return nil, nil } +// notifyInterval is the minimum time between showing update messages. +const notifyInterval = 7 * 24 * 60 * 60 // one week, in seconds + +// PendingNotification returns a release to tell the user about, based on the +// version cached by a previous background check. It returns nil when there is +// nothing to show, when the weekly throttle has not elapsed, or when the +// environment is not eligible. The caller should print the result and then call +// MarkNotified. +func PendingNotification(cnf *config.Config, currentVersion string) *ReleaseInfo { + if !shouldCheckForUpdate(cnf) { + return nil + } + s, err := state.Load(cnf) + if err != nil { + return nil + } + return notificationFromState(s, cnf.Wrapper.GitHubRepo, currentVersion, time.Now().Unix()) +} + +// notificationFromState decides whether to notify about the cached latest +// version, given the current time. It is the pure core of PendingNotification. +func notificationFromState(s state.State, repo, currentVersion string, now int64) *ReleaseInfo { + if s.Updates.KnownLatestVersion == "" { + return nil + } + if s.Updates.LastNotified != 0 && now-s.Updates.LastNotified < notifyInterval { + return nil + } + cmp, err := version.Compare(s.Updates.KnownLatestVersion, currentVersion) + if err != nil || cmp <= 0 { + return nil + } + return &ReleaseInfo{ + Version: s.Updates.KnownLatestVersion, + URL: fmt.Sprintf("https://github.com/%s/releases/tag/%s", repo, s.Updates.KnownLatestVersion), + } +} + +// MarkNotified records that an update message was just shown, so it is not +// repeated until the next notify interval. +func MarkNotified(cnf *config.Config) { + s, err := state.Load(cnf) + if err != nil { + return + } + s.Updates.LastNotified = time.Now().Unix() + //nolint:errcheck // failing to save state should not affect the rest of the program + state.Save(s, cnf) +} + // shouldCheckForUpdate checks updates are not disabled and the environment is a terminal func shouldCheckForUpdate(cnf *config.Config) bool { return config.Version != "0.0.0" && cnf.Wrapper.GitHubRepo != "" && cnf.Updates.Check && os.Getenv(cnf.Application.EnvPrefix+"UPDATES_CHECK") != "0" && - !isCI() && terminal.IsTerminal(os.Stdout) && terminal.IsTerminal(os.Stderr) + !isCI() && !IsAutoUpdating(cnf) && + terminal.IsTerminal(os.Stdout) && terminal.IsTerminal(os.Stderr) } func isCI() bool { diff --git a/internal/update_test.go b/internal/update_test.go new file mode 100644 index 00000000..e2639aca --- /dev/null +++ b/internal/update_test.go @@ -0,0 +1,47 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/internal/state" +) + +func TestNotificationFromState(t *testing.T) { + const now = 1_700_000_000 + + newState := func(latest string, lastNotified int64) state.State { + var s state.State + s.Updates.KnownLatestVersion = latest + s.Updates.LastNotified = lastNotified + return s + } + + t.Run("newer version not yet notified", func(t *testing.T) { + rel := notificationFromState(newState("v2.0.0", 0), "upsun/cli", "v1.0.0", now) + if assert.NotNil(t, rel) { + assert.Equal(t, "v2.0.0", rel.Version) + assert.Equal(t, "https://github.com/upsun/cli/releases/tag/v2.0.0", rel.URL) + } + }) + + t.Run("no cached version", func(t *testing.T) { + assert.Nil(t, notificationFromState(newState("", 0), "upsun/cli", "v1.0.0", now)) + }) + + t.Run("cached version not newer", func(t *testing.T) { + assert.Nil(t, notificationFromState(newState("v1.0.0", 0), "upsun/cli", "v1.0.0", now)) + assert.Nil(t, notificationFromState(newState("v0.9.0", 0), "upsun/cli", "v1.0.0", now)) + }) + + t.Run("notified within the past week is throttled", func(t *testing.T) { + lastNotified := int64(now - 3*24*60*60) // 3 days ago + assert.Nil(t, notificationFromState(newState("v2.0.0", lastNotified), "upsun/cli", "v1.0.0", now)) + }) + + t.Run("notified over a week ago shows again", func(t *testing.T) { + lastNotified := int64(now - 8*24*60*60) // 8 days ago + assert.NotNil(t, notificationFromState(newState("v2.0.0", lastNotified), "upsun/cli", "v1.0.0", now)) + }) +} diff --git a/packaging/install-source b/packaging/install-source new file mode 100644 index 00000000..ba3bd787 --- /dev/null +++ b/packaging/install-source @@ -0,0 +1 @@ +package