From 91466f3ff76ced41ad5b586d22be55490b27f1f0 Mon Sep 17 00:00:00 2001 From: Adhik Joshi Date: Fri, 18 Sep 2026 20:38:59 +0530 Subject: [PATCH] Make browser login the default, and make `modelslab update` work on Windows and under package managers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A customer on Windows (pip, in a venv) ran `modelslab auth login` and got "Error: password: no input provided". Their account came from "Continue with Google", so it has no password: the prompt asked for something that does not exist, and the error never said so. The npm and PyPI READMEs send every new user to exactly that bare `modelslab auth login`. Login - Browser OAuth is now the default in an interactive terminal. Email and password are still used with --email, --password or --browser=false, when stdin is not a terminal (scripts piping credentials), and when there is no local browser to finish the grant: over SSH, or on Linux with no display. The callback listens on 127.0.0.1, which a browser across SSH cannot reach. - An empty password now says "no password entered" and points Google and GitHub accounts at `auth login --browser` and `auth forgot-password`. Update - Windows: `modelslab update` always failed with "Windows cannot replace a running executable". Windows will rename a running .exe even though it will not delete one, so the old binary moves aside to .old, the new one takes its name, and the .old file is removed on the next run. - Package managers: under pip, pipx, uv, npm, Homebrew or Scoop the updater overwrote a file the package manager owns — pip kept reporting the old version, the next `brew upgrade` or `npm install` put the old file back, and on Windows the launchers hold the file open. The install method is now read from the binary's path; `modelslab update` prints that manager's command instead, and `update --check` reports it. - The notification said "Run `modelslab update`" to everyone. It now shows the command for the install method, e.g. `pip install --upgrade modelslab-cli`. - A failed check was never cached, so offline, behind a proxy, or after GitHub's anonymous rate limit, every command waited out the 1.5s timeout. Failures are cached now and keep what the last good check found. - No notification in CI or when stderr is not a terminal. - "permission denied" on /usr/local/bin or Program Files now says to use sudo or an Administrator terminal. Verified against the real GitHub release with a 0.1.4 build: the notification shows the right command for a standalone and a pip layout and stays silent with CI=1 or redirected stderr; `update` in the pip layout refuses with the pip command; the standalone binary self-updated to 0.2.0 with its checksum verified and no .old/.new left behind. The Windows rename path is covered by cross-compilation only and still needs a run on a real Windows machine. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NJxK2YGapyVchfyE1F8CpR --- README.md | 33 ++++-- internal/cmd/auth.go | 81 ++++++++++++++- internal/cmd/auth_login_mode_test.go | 45 ++++++++ internal/cmd/prompt.go | 8 +- internal/cmd/root.go | 13 ++- internal/cmd/update.go | 21 +++- internal/updater/cache_test.go | 131 ++++++++++++++++++++++++ internal/updater/install_method.go | 69 +++++++++++++ internal/updater/install_method_test.go | 38 +++++++ internal/updater/updater.go | 100 ++++++++++++++---- 10 files changed, 503 insertions(+), 36 deletions(-) create mode 100644 internal/cmd/auth_login_mode_test.go create mode 100644 internal/updater/cache_test.go create mode 100644 internal/updater/install_method.go create mode 100644 internal/updater/install_method_test.go 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 {