diff --git a/README.md b/README.md index 2037478..d4680ea 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,18 @@ modelslab update --check modelslab update ``` -The CLI also checks for updates periodically during normal human-readable commands and prints a short notice when a newer release is available. It stays silent for `--output json`, `--jq`, shell completions, and MCP server mode. Disable the startup check for one command with `--no-update-check`, or persistently with: +If a package manager installed the CLI, update it with that package manager. `modelslab update` detects this and prints the right command instead of replacing a file the package manager owns: + +| Installed with | Update with | +|---|---| +| pip | `pip install --upgrade modelslab-cli` | +| pipx | `pipx upgrade modelslab-cli` | +| uv | `uv tool upgrade modelslab-cli` | +| npm | `npm install -g modelslab-cli@latest` | +| Homebrew | `brew upgrade modelslab/tap/modelslab` | +| Scoop | `scoop update modelslab` | + +The CLI also checks for updates once a day during normal human-readable commands and prints a short notice, with the right update command, when a newer release is available. It stays silent for `--output json`, `--jq`, shell completions, MCP server mode, in CI (`CI` is set), and when stderr is not a terminal. Disable the startup check for one command with `--no-update-check`, or persistently with: ```bash modelslab config set updates.auto_check false @@ -93,8 +104,8 @@ modelslab config set updates.auto_check false # New user? Sign up first modelslab auth signup --name "Your Name" --email you@example.com --password "..." --confirm-password "..." -# Login to your account in the browser -modelslab auth login --browser +# Login to your account in the browser (the default in a terminal) +modelslab auth login # Or login with email/password modelslab auth login --email you@example.com --password "..." @@ -157,7 +168,7 @@ modelslab auth signup --name "Your Name" --email you@example.com --password "... modelslab auth verify-email --token # 3. Login in the browser (auto-stores bearer token + API key in OS keychain) -modelslab auth login --browser +modelslab auth login # Or use email/password modelslab auth login --email you@example.com --password "..." @@ -177,15 +188,21 @@ modelslab auth tokens create --name "ci-token" ### Existing Users ```bash -# Browser login opens Chrome, asks you to grant CLI access, and stores both credentials -modelslab auth login --browser +# Browser login opens Chrome, asks you to grant CLI access, and stores both credentials. +# It is the default in an interactive terminal. Over SSH, with no display, or with +# piped stdin, the CLI asks for email and password instead. +modelslab auth login # Email/password login also gets both token and API key modelslab auth login --email you@example.com --password "..." +# Force one mode or the other +modelslab auth login --browser +modelslab auth login --browser=false + # NOTE: accounts created with "Continue with Google" or "Continue with GitHub" -# have no password, so --email/--password can never work for them. Use -# --browser, or set a password first with `modelslab auth forgot-password`. +# have no password, so --email/--password can never work for them. Use the +# browser login, or set a password first with `modelslab auth forgot-password`. # Or set API key manually modelslab config set api_key "your-api-key" diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 0bfb473..2e166f6 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -47,14 +47,34 @@ type browserLoginCallback struct { var authLoginCmd = &cobra.Command{ Use: "login", Short: "Login to ModelsLab", + Long: `Login to ModelsLab. + +In an interactive terminal this opens your browser, asks you to grant the CLI +access, and stores both the access token and the API key. This works for every +account, including accounts created with "Continue with Google" or GitHub. + +The CLI uses email and password instead when you pass --email or --password, +when you pass --browser=false, when stdin is not a terminal, or when no local +browser is available (an SSH session, or Linux with no display).`, RunE: func(cmd *cobra.Command, args []string) error { + email, _ := cmd.Flags().GetString("email") + password, _ := cmd.Flags().GetString("password") + useBrowser, _ := cmd.Flags().GetBool("browser") + if !cmd.Flags().Changed("browser") { + interactive := stdinIsTerminal() + useBrowser = defaultToBrowserLogin(email, password, interactive, runtime.GOOS, os.Getenv) + + if !useBrowser && interactive && email == "" && password == "" { + fmt.Fprintln(os.Stderr, "No local browser found (SSH session or no display), so the CLI uses email and password.") + fmt.Fprintln(os.Stderr, "Google or GitHub account? It has no password. Set one first with: modelslab auth forgot-password") + fmt.Fprintln(os.Stderr) + } + } if useBrowser { return runBrowserLogin(cmd) } - email, _ := cmd.Flags().GetString("email") - password, _ := cmd.Flags().GetString("password") expiry, _ := cmd.Flags().GetString("expiry") deviceName, _ := cmd.Flags().GetString("device-name") @@ -67,6 +87,9 @@ var authLoginCmd = &cobra.Command{ } if password == "" { value, err := promptSecret("Password: ") + if errors.Is(err, errNoInput) { + return errNoPasswordEntered + } if err != nil { return err } @@ -143,6 +166,57 @@ var authLoginCmd = &cobra.Command{ }, } +// errNoPasswordEntered replaces the bare "password: no input provided". +// +// An empty answer at the password prompt is almost never a typo. It is someone +// whose account came from "Continue with Google" or GitHub, looking at a prompt +// for a password that does not exist. +var errNoPasswordEntered = errors.New("no password entered.\n" + + " Signed up with Google or GitHub? That account has no password — run: modelslab auth login --browser\n" + + " Or set a password first: modelslab auth forgot-password") + +// defaultToBrowserLogin decides how `auth login` signs in when --browser is not +// given. +// +// Email and password used to be the default, and it is a dead end for every +// account created with "Continue with Google" or GitHub: the account has no +// password, so the prompt had nothing the user could type and the login died with +// "password: no input provided". The npm and PyPI READMEs send new users straight +// to a bare `modelslab auth login`, so that was the first thing they saw. +// +// Email and password is still the choice when the caller supplied either one, +// when stdin is not a terminal (a script piping credentials must not start +// waiting on a browser), and when there is no browser here to finish the grant. +func defaultToBrowserLogin(email, password string, interactive bool, goos string, getenv func(string) string) bool { + if email != "" || password != "" || !interactive { + return false + } + + return hasLocalBrowser(goos, getenv) +} + +// hasLocalBrowser reports whether a browser on this machine can finish the OAuth +// grant. +// +// "On this machine" matters more than "a browser exists": the callback server +// listens on 127.0.0.1, so a browser on the far side of an SSH session opens the +// grant page and then cannot deliver the result, and the login hangs until it +// times out. +func hasLocalBrowser(goos string, getenv func(string) string) bool { + for _, key := range []string{"SSH_CONNECTION", "SSH_CLIENT", "SSH_TTY"} { + if getenv(key) != "" { + return false + } + } + + switch goos { + case "darwin", "windows": + return true + default: + return getenv("DISPLAY") != "" || getenv("WAYLAND_DISPLAY") != "" + } +} + // loginFailureHints turns a control-plane error code into next steps a user can // actually act on. // @@ -270,6 +344,7 @@ func runBrowserLogin(cmd *cobra.Command) error { } } fmt.Fprintln(os.Stderr, "Waiting for browser authorization...") + fmt.Fprintln(os.Stderr, "(To use email and password instead, press Ctrl+C and run: modelslab auth login --email you@example.com)") timer := time.NewTimer(timeout) defer timer.Stop() @@ -794,7 +869,7 @@ func init() { // auth login authLoginCmd.Flags().String("email", "", "Account email") authLoginCmd.Flags().String("password", "", "Account password") - authLoginCmd.Flags().Bool("browser", false, "Log in with browser OAuth instead of email and password") + authLoginCmd.Flags().Bool("browser", false, "Log in through the browser (the default in an interactive terminal); --browser=false forces email and password") authLoginCmd.Flags().Int("callback-port", 0, "Local callback port for browser OAuth (0 chooses a free port)") authLoginCmd.Flags().String("expiry", "1_month", "Token expiry: 1_week, 1_month, 3_months, 6_months, 1_year, never") authLoginCmd.Flags().String("device-name", "", "Device name for token") diff --git a/internal/cmd/auth_login_mode_test.go b/internal/cmd/auth_login_mode_test.go new file mode 100644 index 0000000..013aa92 --- /dev/null +++ b/internal/cmd/auth_login_mode_test.go @@ -0,0 +1,45 @@ +package cmd + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func env(vars map[string]string) func(string) string { + return func(key string) string { return vars[key] } +} + +// A Google or GitHub sign-up has no password. A bare `auth login` used to prompt +// for one anyway and die with "password: no input provided". +func TestDefaultToBrowserLogin_BareLoginInATerminalUsesTheBrowser(t *testing.T) { + assert.True(t, defaultToBrowserLogin("", "", true, "windows", env(nil))) + assert.True(t, defaultToBrowserLogin("", "", true, "darwin", env(nil))) + assert.True(t, defaultToBrowserLogin("", "", true, "linux", env(map[string]string{"DISPLAY": ":0"}))) + assert.True(t, defaultToBrowserLogin("", "", true, "linux", env(map[string]string{"WAYLAND_DISPLAY": "wayland-0"}))) +} + +func TestDefaultToBrowserLogin_CredentialFlagsKeepEmailAndPassword(t *testing.T) { + assert.False(t, defaultToBrowserLogin("ada@example.com", "", true, "darwin", env(nil))) + assert.False(t, defaultToBrowserLogin("", "hunter2", true, "darwin", env(nil))) +} + +// A script piping credentials must never start waiting on a browser. +func TestDefaultToBrowserLogin_NonInteractiveStdinKeepsEmailAndPassword(t *testing.T) { + assert.False(t, defaultToBrowserLogin("", "", false, "darwin", env(nil))) +} + +// The callback listens on 127.0.0.1, which a browser across an SSH session cannot +// reach — the login would hang until it timed out. +func TestDefaultToBrowserLogin_NoLocalBrowserKeepsEmailAndPassword(t *testing.T) { + for _, key := range []string{"SSH_CONNECTION", "SSH_CLIENT", "SSH_TTY"} { + assert.False(t, defaultToBrowserLogin("", "", true, "darwin", env(map[string]string{key: "x"})), key) + } + + assert.False(t, defaultToBrowserLogin("", "", true, "linux", env(nil))) +} + +func TestErrNoPasswordEntered_PointsAtBrowserLogin(t *testing.T) { + assert.Contains(t, errNoPasswordEntered.Error(), "modelslab auth login --browser") + assert.Contains(t, errNoPasswordEntered.Error(), "forgot-password") +} diff --git a/internal/cmd/prompt.go b/internal/cmd/prompt.go index 320f573..26fa8e3 100644 --- a/internal/cmd/prompt.go +++ b/internal/cmd/prompt.go @@ -50,7 +50,7 @@ func promptLine(label string) (string, error) { // which lands in shell history, ps output and CI logs. Fall back to reading the // line instead; there is no echo to suppress when nobody is typing. func promptSecret(label string) (string, error) { - if !term.IsTerminal(int(syscall.Stdin)) { + if !stdinIsTerminal() { return promptLine(label) } @@ -69,6 +69,12 @@ func promptSecret(label string) (string, error) { return secret, nil } +// stdinIsTerminal reports whether a person could be typing at stdin, as opposed to +// a pipe, a heredoc or CI. +func stdinIsTerminal() bool { + return term.IsTerminal(int(syscall.Stdin)) +} + // fieldName turns a prompt label ("Email: ") into something an error can read // naturally ("email"). func fieldName(label string) string { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index a5ea172..3e92b1e 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "time" @@ -14,6 +15,7 @@ import ( "github.com/ModelsLab/modelslab-cli/internal/output" "github.com/ModelsLab/modelslab-cli/internal/updater" "github.com/spf13/cobra" + "golang.org/x/term" ) var ( @@ -80,6 +82,9 @@ Designed for both humans and AI agents.`, flagNoColor = true } + if runtime.GOOS == "windows" { + updater.RemoveStaleBackups() + } maybeNotifyUpdate(cmd) return nil @@ -181,7 +186,8 @@ func maybeNotifyUpdate(cmd *cobra.Command) { return } - fmt.Fprintf(os.Stderr, "Update available: ModelsLab CLI %s (current %s). Run `modelslab update`.\n\n", formatVersion(info.LatestVersion), formatVersion(info.CurrentVersion)) + fmt.Fprintf(os.Stderr, "Update available: ModelsLab CLI %s (current %s). Run: %s\n\n", + formatVersion(info.LatestVersion), formatVersion(info.CurrentVersion), updater.CurrentInstallMethod().UpdateCommand) } func shouldSkipUpdateNotification(cmd *cobra.Command) bool { @@ -194,6 +200,11 @@ func shouldSkipUpdateNotification(cmd *cobra.Command) bool { if flagOutput == string(output.FormatJSON) || flagJQ != "" { return true } + // Nobody reads a notice in CI, and a script that captures stderr should not + // find one in its logs. + if os.Getenv("CI") != "" || !term.IsTerminal(int(os.Stderr.Fd())) { + return true + } if !updater.IsComparableVersion(cliVersion) { return true } diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 783db26..8469db6 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -26,12 +26,17 @@ var updateCmd = &cobra.Command{ Long: `Check GitHub Releases for a newer ModelsLab CLI version and install it. The updater downloads the release archive for your platform, verifies it against -the release checksums.txt file, and replaces the current executable.`, +the release checksums.txt file, and replaces the current executable. + +If pip, pipx, uv, npm, Homebrew or Scoop installed the CLI, that package manager +owns the file. The updater does not replace it and shows the command to run.`, RunE: func(cmd *cobra.Command, args []string) error { if updateTimeout <= 0 { updateTimeout = 2 * time.Minute } + method := updater.CurrentInstallMethod() + ctx, cancel := context.WithTimeout(cmd.Context(), updateTimeout) defer cancel() @@ -48,11 +53,21 @@ the release checksums.txt file, and replaces the current executable.`, SkipChecksum: updateSkipChecksum, } - if updateCheckOnly { + if updateCheckOnly || method.Managed() { info, err := updater.Check(ctx, options.CheckOptions) if err != nil { return err } + info.InstallMethod = method.Name + info.UpdateCommand = method.UpdateCommand + + if !updateCheckOnly && (info.UpdateAvailable || updateForce) { + return fmt.Errorf( + "%s installed this copy of the ModelsLab CLI, so %s must update it.\n Run: %s", + method.Name, method.Name, method.UpdateCommand, + ) + } + outputResult(info, func() { printUpdateCheck(info) }) @@ -83,7 +98,7 @@ func printUpdateCheck(info *updater.Info) { if info.UpdateAvailable { fmt.Printf("Update available: %s -> %s\n", formatVersion(info.CurrentVersion), formatVersion(info.LatestVersion)) - fmt.Println("Run `modelslab update` to install it.") + fmt.Printf("Run: %s\n", info.UpdateCommand) return } diff --git a/internal/updater/cache_test.go b/internal/updater/cache_test.go new file mode 100644 index 0000000..c2c7070 --- /dev/null +++ b/internal/updater/cache_test.go @@ -0,0 +1,131 @@ +package updater + +import ( + "context" + "errors" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// roundTripFunc stands in for api.github.com. +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func countingClient(calls *int, respond func() (*http.Response, error)) *http.Client { + return &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + *calls++ + return respond() + })} +} + +func release(tag string) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{"tag_name":"` + tag + `"}`)), + Header: http.Header{}, + }, nil +} + +// Offline, every command used to re-run the check and wait out its timeout. +func TestCachedCheck_CachesAFailedCheck(t *testing.T) { + cachePath := filepath.Join(t.TempDir(), "update-check.json") + calls := 0 + opts := CheckOptions{ + CurrentVersion: "0.2.0", + HTTPClient: countingClient(&calls, func() (*http.Response, error) { return nil, errors.New("offline") }), + } + + _, err := CachedCheck(context.Background(), opts, cachePath, time.Hour) + require.Error(t, err) + + _, err = CachedCheck(context.Background(), opts, cachePath, time.Hour) + require.NoError(t, err) + + assert.Equal(t, 1, calls, "the second command must not hit the network again") +} + +// A failed check must not forget an update that the last good check found. +func TestCachedCheck_FailureKeepsTheLastKnownUpdate(t *testing.T) { + cachePath := filepath.Join(t.TempDir(), "update-check.json") + calls := 0 + online := true + opts := CheckOptions{ + CurrentVersion: "0.2.0", + HTTPClient: countingClient(&calls, func() (*http.Response, error) { + if online { + return release("v0.2.1") + } + return nil, errors.New("offline") + }), + } + + info, err := CachedCheck(context.Background(), opts, cachePath, 0) + require.NoError(t, err) + require.True(t, info.UpdateAvailable) + + online = false + _, err = CachedCheck(context.Background(), opts, cachePath, 0) + require.Error(t, err) + + info, err = CachedCheck(context.Background(), opts, cachePath, time.Hour) + require.NoError(t, err) + assert.True(t, info.UpdateAvailable) + assert.Equal(t, "0.2.1", info.LatestVersion) +} + +// After an update the cache describes the old binary, so it must be ignored. +func TestCachedCheck_IgnoresACacheForAnotherVersion(t *testing.T) { + cachePath := filepath.Join(t.TempDir(), "update-check.json") + require.NoError(t, WriteCache(cachePath, Cache{CheckedAt: time.Now(), CurrentVersion: "0.1.0", LatestVersion: "0.2.0", UpdateAvailable: true})) + calls := 0 + opts := CheckOptions{ + CurrentVersion: "0.2.0", + HTTPClient: countingClient(&calls, func() (*http.Response, error) { return release("v0.2.0") }), + } + + info, err := CachedCheck(context.Background(), opts, cachePath, time.Hour) + + require.NoError(t, err) + assert.Equal(t, 1, calls) + assert.False(t, info.UpdateAvailable) +} + +func TestReplaceExecutable_SwapsTheFileAndCleansUp(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "modelslab") + fresh := filepath.Join(dir, "download") + require.NoError(t, os.WriteFile(target, []byte("old"), 0o755)) + require.NoError(t, os.WriteFile(fresh, []byte("new"), 0o644)) + + require.NoError(t, replaceExecutable(target, fresh)) + + got, err := os.ReadFile(target) + require.NoError(t, err) + assert.Equal(t, "new", string(got)) + + info, err := os.Stat(target) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), info.Mode().Perm(), "the new binary keeps the old one's mode") + + assert.NoFileExists(t, target+".new") + assert.NoFileExists(t, target+".old") +} + +func TestPermissionHint(t *testing.T) { + err := permissionHint("/usr/local/bin/modelslab", &os.PathError{Op: "open", Path: "/usr/local/bin/modelslab.new", Err: os.ErrPermission}) + + assert.ErrorIs(t, err, os.ErrPermission) + assert.Contains(t, err.Error(), "/usr/local/bin") + + other := errors.New("disk full") + assert.Same(t, other, permissionHint("/usr/local/bin/modelslab", other)) +} diff --git a/internal/updater/install_method.go b/internal/updater/install_method.go new file mode 100644 index 0000000..4f3a173 --- /dev/null +++ b/internal/updater/install_method.go @@ -0,0 +1,69 @@ +package updater + +import ( + "os" + "path/filepath" + "strings" +) + +// InstallMethod says who owns the running binary, and so who has to replace it. +type InstallMethod struct { + // Name is the package manager that installed the binary, or "" when the + // binary stands alone (install.sh, a release archive, go build). + Name string `json:"name,omitempty"` + // UpdateCommand is what the user should run to get the latest release. + UpdateCommand string `json:"update_command"` +} + +// Managed reports whether a package manager owns the binary. +// +// `modelslab update` must not replace a managed binary itself. It works, once, +// and then the package manager's record no longer matches the file on disk: pip +// still reports the old version, the next `brew upgrade` or `npm install` puts +// the old file back, and on Windows the pip and npm launchers hold the file +// open, so the replace fails outright. +func (m InstallMethod) Managed() bool { + return m.Name != "" +} + +var standalone = InstallMethod{UpdateCommand: "modelslab update"} + +// CurrentInstallMethod detects how the running binary was installed. +func CurrentInstallMethod() InstallMethod { + exe, err := os.Executable() + if err != nil { + return standalone + } + // Homebrew runs the CLI through a symlink in bin/; the Caskroom path that + // identifies it only shows up once the link is resolved. + if resolved, err := filepath.EvalSymlinks(exe); err == nil { + exe = resolved + } + + return DetectInstallMethod(exe) +} + +// DetectInstallMethod maps an executable path to the package manager that owns +// it. It matches on the directory layout each one uses, which is stable, rather +// than asking the package managers, which may not be on PATH. +func DetectInstallMethod(exePath string) InstallMethod { + // Lowercase and forward slashes, so one set of patterns covers Windows too. + path := strings.ToLower(strings.ReplaceAll(exePath, `\`, "/")) + + switch { + case strings.Contains(path, "/modelslab_cli/bin/") && strings.Contains(path, "/pipx/"): + return InstallMethod{Name: "pipx", UpdateCommand: "pipx upgrade modelslab-cli"} + case strings.Contains(path, "/modelslab_cli/bin/") && strings.Contains(path, "/uv/tools/"): + return InstallMethod{Name: "uv", UpdateCommand: "uv tool upgrade modelslab-cli"} + case strings.Contains(path, "/modelslab_cli/bin/"): + return InstallMethod{Name: "pip", UpdateCommand: "pip install --upgrade modelslab-cli"} + case strings.Contains(path, "/node_modules/@modelslab/cli-"): + return InstallMethod{Name: "npm", UpdateCommand: "npm install -g modelslab-cli@latest"} + case strings.Contains(path, "/caskroom/modelslab/"): + return InstallMethod{Name: "homebrew", UpdateCommand: "brew upgrade modelslab/tap/modelslab"} + case strings.Contains(path, "/scoop/apps/modelslab/"): + return InstallMethod{Name: "scoop", UpdateCommand: "scoop update modelslab"} + default: + return standalone + } +} diff --git a/internal/updater/install_method_test.go b/internal/updater/install_method_test.go new file mode 100644 index 0000000..b6614e6 --- /dev/null +++ b/internal/updater/install_method_test.go @@ -0,0 +1,38 @@ +package updater + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDetectInstallMethod(t *testing.T) { + tests := []struct { + path string + name string + command string + }{ + // pip on Windows, inside a venv: the setup from the support ticket. + {`G:\ModelsLab\venv\Lib\site-packages\modelslab_cli\bin\modelslab.exe`, "pip", "pip install --upgrade modelslab-cli"}, + {"/usr/lib/python3/dist-packages/modelslab_cli/bin/modelslab", "pip", "pip install --upgrade modelslab-cli"}, + {"/Users/ada/.local/pipx/venvs/modelslab-cli/lib/python3.12/site-packages/modelslab_cli/bin/modelslab", "pipx", "pipx upgrade modelslab-cli"}, + {"/home/ada/.local/share/uv/tools/modelslab-cli/lib/python3.12/site-packages/modelslab_cli/bin/modelslab", "uv", "uv tool upgrade modelslab-cli"}, + {`C:\Users\Ada\AppData\Roaming\npm\node_modules\modelslab-cli\node_modules\@modelslab\cli-win32-x64\bin\modelslab.exe`, "npm", "npm install -g modelslab-cli@latest"}, + {"/Users/ada/proj/node_modules/.pnpm/@modelslab+cli-darwin-arm64@0.2.0/node_modules/@modelslab/cli-darwin-arm64/bin/modelslab", "npm", "npm install -g modelslab-cli@latest"}, + {"/opt/homebrew/Caskroom/modelslab/0.2.0/modelslab", "homebrew", "brew upgrade modelslab/tap/modelslab"}, + {`C:\Users\Ada\scoop\apps\modelslab\current\modelslab.exe`, "scoop", "scoop update modelslab"}, + {"/usr/local/bin/modelslab", "", "modelslab update"}, + {"/home/ada/.local/bin/modelslab", "", "modelslab update"}, + {`C:\tools\modelslab.exe`, "", "modelslab update"}, + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + method := DetectInstallMethod(tt.path) + + assert.Equal(t, tt.name, method.Name) + assert.Equal(t, tt.command, method.UpdateCommand) + assert.Equal(t, tt.name != "", method.Managed()) + }) + } +} diff --git a/internal/updater/updater.go b/internal/updater/updater.go index d7bc971..8d8b60d 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -42,6 +42,8 @@ type Info struct { AssetName string `json:"asset_name,omitempty"` AssetURL string `json:"asset_url,omitempty"` PublishedAt time.Time `json:"published_at"` + InstallMethod string `json:"install_method,omitempty"` + UpdateCommand string `json:"update_command,omitempty"` } type CheckOptions struct { @@ -172,21 +174,37 @@ func Install(ctx context.Context, opts InstallOptions) (*InstallResult, error) { } func CachedCheck(ctx context.Context, opts CheckOptions, cachePath string, interval time.Duration) (*Info, error) { - if cache, err := ReadCache(cachePath); err == nil { - if cache.CurrentVersion == opts.CurrentVersion && time.Since(cache.CheckedAt) < interval { - return &Info{ - CurrentVersion: cache.CurrentVersion, - LatestVersion: cache.LatestVersion, - UpdateAvailable: cache.UpdateAvailable, - CanCompare: cache.CanCompare, - ReleaseURL: cache.ReleaseURL, - PublishedAt: cache.PublishedAt, - }, nil - } + cache, cacheErr := ReadCache(cachePath) + if cacheErr == nil && cache.CurrentVersion != opts.CurrentVersion { + cache, cacheErr = nil, errors.New("cache is for another version") + } + if cacheErr == nil && time.Since(cache.CheckedAt) < interval { + return &Info{ + CurrentVersion: cache.CurrentVersion, + LatestVersion: cache.LatestVersion, + UpdateAvailable: cache.UpdateAvailable, + CanCompare: cache.CanCompare, + ReleaseURL: cache.ReleaseURL, + PublishedAt: cache.PublishedAt, + }, nil } info, err := Check(ctx, opts) if err != nil { + /* + * A failed check is cached too. It used to return without writing, so + * offline, behind a proxy that blocks api.github.com, or after GitHub's + * anonymous rate limit, EVERY command re-ran the check and waited out its + * timeout first. Keep what the last good check learned and try again + * after the interval. + */ + failed := Cache{CheckedAt: time.Now(), CurrentVersion: opts.CurrentVersion} + if cacheErr == nil { + failed = *cache + failed.CheckedAt = time.Now() + } + _ = WriteCache(cachePath, failed) + return nil, err } @@ -544,18 +562,27 @@ func replaceExecutable(targetPath, newBinaryPath string) error { replacement := targetPath + ".new" if err := copyFile(newBinaryPath, replacement, mode); err != nil { - return err - } - - if runtime.GOOS == "windows" { - return fmt.Errorf("downloaded update to %s, but Windows cannot replace a running executable automatically", replacement) - } - + return permissionHint(targetPath, err) + } + + /* + * The same rename dance works on Windows, which this used to refuse outright. + * + * Windows will not delete or overwrite a running .exe, but it will rename + * one: the loaded image stays mapped under the new name. So the running + * binary moves aside to .old, the new one takes its name, and only the final + * Remove of .old fails — that file is cleaned up by RemoveStaleBackups on the + * next run, once nothing has it open. + */ backup := targetPath + ".old" - _ = os.Remove(backup) + if err := os.Remove(backup); err != nil && !errors.Is(err, os.ErrNotExist) { + // Still held open by a CLI process that is running the previous + // version. Step around it rather than fail the update. + backup = fmt.Sprintf("%s.old.%d", targetPath, time.Now().UnixNano()) + } if err := os.Rename(targetPath, backup); err != nil { _ = os.Remove(replacement) - return fmt.Errorf("could not prepare executable replacement: %w", err) + return permissionHint(targetPath, fmt.Errorf("could not prepare executable replacement: %w", err)) } if err := os.Rename(replacement, targetPath); err != nil { _ = os.Rename(backup, targetPath) @@ -567,6 +594,39 @@ func replaceExecutable(targetPath, newBinaryPath string) error { return nil } +// permissionHint says how to get past a directory the user cannot write to, such +// as /usr/local/bin or Program Files, instead of a bare "permission denied". +func permissionHint(targetPath string, err error) error { + if !errors.Is(err, os.ErrPermission) { + return err + } + if runtime.GOOS == "windows" { + return fmt.Errorf("%w\n You cannot write to %s. Run the update from a terminal opened as Administrator", err, filepath.Dir(targetPath)) + } + + return fmt.Errorf("%w\n You cannot write to %s. Run: sudo modelslab update", err, filepath.Dir(targetPath)) +} + +// RemoveStaleBackups deletes the .old binaries a Windows update leaves behind. +// See replaceExecutable. It is best effort: a backup that another running CLI +// still holds open stays until a later run. +func RemoveStaleBackups() { + exe, err := os.Executable() + if err != nil { + return + } + if resolved, err := filepath.EvalSymlinks(exe); err == nil { + exe = resolved + } + + for _, pattern := range []string{exe + ".old", exe + ".old.*"} { + matches, _ := filepath.Glob(pattern) + for _, match := range matches { + _ = os.Remove(match) + } + } +} + func copyFile(src, dst string, mode os.FileMode) error { in, err := os.Open(src) if err != nil {