Merge pull request #312 from lets-cli/codex/adapt-cli-update-plan-to

Add background update notifier

Author: kindermax

.golangci.yaml

@@ -5,6 +5,7 @@ run:
 linters:
   default: all
   disable:
+    - goconst
     - containedctx
     - copyloopvar
     - cyclop

.zed/debug.json

@@ -0,0 +1,23 @@
+// Project-local debug tasks
+//
+// For more documentation on how to configure debug tasks,
+// see: https://zed.dev/docs/debugger
+[
+  {
+    "label": "Go (Delve)",
+    "adapter": "Delve",
+    "program": "$ZED_FILE",
+    "request": "launch",
+    "mode": "debug",
+  },
+  {
+    "label": "Run fmt",
+    "adapter": "Delve",
+    "request": "launch",
+    "mode": "debug",
+    // For Delve, the program can be a package name
+    "program": "./cmd/lets",
+    "args": ["fmt"],
+    // "buildFlags": [],
+  },
+]

docker-compose.yml

@@ -25,6 +25,7 @@ services:
   test-bats:
     <<: *base
     environment:
+      LETS_CHECK_UPDATE: 0
       NO_COLOR: 1
       BATS_UTILS_PATH: /bats
     command:

docs/docs/changelog.md

@@ -15,6 +15,7 @@ title: Changelog
 * `[Changed]` Migrate the LSP YAML parser from the CGO-based tree-sitter bindings to pure-Go [`gotreesitter`](https://github.com/odvcencio/gotreesitter), removing the C toolchain requirement from normal builds and release packaging.
 * `[Refactoring]` Move CLI startup flow from `cmd/lets/main.go` into `internal/cli/cli.go`, keeping `main.go` as a thin launcher.
 * `[Added]` Add `lets self doc` command to open the online documentation in a browser.
+* `[Added]` Show background update notifications for interactive sessions, with Homebrew-aware guidance and `LETS_CHECK_UPDATE` opt-out.
 
 ## [0.0.59](https://github.com/lets-cli/lets/releases/tag/v0.0.59)
 

docs/docs/env.md

@@ -10,6 +10,7 @@ title: Environment
 * `LETS_DEBUG` - enable debug messages
 * `LETS_CONFIG` - changes default `lets.yaml` file path (e.g. LETS_CONFIG=lets.my.yaml)
 * `LETS_CONFIG_DIR` - changes path to dir where `lets.yaml` file placed
+* `LETS_CHECK_UPDATE` - disables background update checks and notifications
 * `NO_COLOR` - disables colored output. See https://no-color.org/
 
 ### Environment variables available at command runtime

internal/cli/cli.go

@@ -3,10 +3,12 @@ package cli
 import (
 	"context"
 	"errors"
+	"fmt"
 	"os"
 	"os/signal"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/lets-cli/lets/internal/cmd"
 	"github.com/lets-cli/lets/internal/config"
@@ -17,10 +19,18 @@ import (
 	"github.com/lets-cli/lets/internal/upgrade"
 	"github.com/lets-cli/lets/internal/upgrade/registry"
 	"github.com/lets-cli/lets/internal/workdir"
+	"github.com/mattn/go-isatty"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
+const updateCheckTimeout = 3 * time.Second
+
+type updateCheckResult struct {
+	notifier *upgrade.UpdateNotifier
+	notice   *upgrade.UpdateNotice
+}
+
 func Main(version string, buildDate string) int {
 	ctx := getContext()
 
@@ -94,9 +104,9 @@ func Main(version string, buildDate string) int {
 	}
 
 	if rootFlags.upgrade {
-		upgrader, err := upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(ctx), version)
+		upgrader, err := upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(), version)
 		if err == nil {
-			err = upgrader.Upgrade()
+			err = upgrader.Upgrade(ctx)
 		}
 
 		if err != nil {
@@ -118,16 +128,22 @@ func Main(version string, buildDate string) int {
 		return 0
 	}
 
+	updateCh, cancelUpdateCheck := maybeStartUpdateCheck(ctx, version, command)
+	defer cancelUpdateCheck()
+
 	if err := rootCmd.ExecuteContext(ctx); err != nil {
 		var depErr *executor.DependencyError
 		if errors.As(err, &depErr) {
 			executor.PrintDependencyTree(depErr, os.Stderr)
 		}
 
 		log.Errorf("lets: %s", err.Error())
+
 		return getExitCode(err, 1)
 	}
 
+	printUpdateNotice(updateCh)
+
 	return 0
 }
 
@@ -186,6 +202,82 @@ func allowsMissingConfig(current *cobra.Command) bool {
 	return false
 }
 
+func maybeStartUpdateCheck(
+	ctx context.Context,
+	version string,
+	command *cobra.Command,
+) (<-chan updateCheckResult, context.CancelFunc) {
+	if !shouldCheckForUpdate(command.Name(), isInteractiveStderr()) {
+		return nil, func() {}
+	}
+
+	log.Debugf("lets: start update check")
+
+	notifier, err := upgrade.NewUpdateNotifier(registry.NewGithubRegistry())
+	if err != nil {
+		return nil, func() {}
+	}
+
+	ch := make(chan updateCheckResult, 1)
+	checkCtx, cancel := context.WithTimeout(ctx, updateCheckTimeout)
+
+	go func() {
+		notice, err := notifier.Check(checkCtx, version)
+		if err != nil {
+			upgrade.LogUpdateCheckError(err)
+		}
+
+		log.Debugf("lets: update check done")
+
+		ch <- updateCheckResult{
+			notifier: notifier,
+			notice:   notice,
+		}
+	}()
+
+	return ch, cancel
+}
+
+func printUpdateNotice(updateCh <-chan updateCheckResult) {
+	if updateCh == nil {
+		return
+	}
+
+	select {
+	case result := <-updateCh:
+		if result.notice == nil {
+			return
+		}
+
+		if _, err := fmt.Fprintln(os.Stderr, result.notice.Message()); err != nil {
+			return
+		}
+
+		if err := result.notifier.MarkNotified(result.notice); err != nil {
+			upgrade.LogUpdateCheckError(err)
+		}
+	default:
+	}
+}
+
+func shouldCheckForUpdate(commandName string, interactive bool) bool {
+	if !interactive || os.Getenv("CI") != "" || os.Getenv("LETS_CHECK_UPDATE") != "" {
+		return false
+	}
+
+	switch commandName {
+	case "completion", "help", "lsp", "self":
+		return false
+	default:
+		return true
+	}
+}
+
+func isInteractiveStderr() bool {
+	fd := os.Stderr.Fd()
+	return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd)
+}
+
 type flags struct {
 	config  string
 	debug   int

internal/cli/cli_test.go

@@ -57,3 +57,42 @@ func TestAllowsMissingConfig(t *testing.T) {
 		}
 	})
 }
+
+func TestShouldCheckForUpdate(t *testing.T) {
+	t.Run("should allow normal interactive commands", func(t *testing.T) {
+		t.Setenv("CI", "")
+		t.Setenv("LETS_CHECK_UPDATE", "")
+
+		if !shouldCheckForUpdate("lets", true) {
+			t.Fatal("expected update check to be enabled")
+		}
+	})
+
+	t.Run("should skip non interactive sessions", func(t *testing.T) {
+		if shouldCheckForUpdate("lets", false) {
+			t.Fatal("expected non-interactive session to skip update check")
+		}
+	})
+
+	t.Run("should skip when CI is set", func(t *testing.T) {
+		t.Setenv("CI", "1")
+		if shouldCheckForUpdate("lets", true) {
+			t.Fatal("expected CI to skip update check")
+		}
+	})
+
+	t.Run("should skip when notifier disabled", func(t *testing.T) {
+		t.Setenv("LETS_CHECK_UPDATE", "1")
+		if shouldCheckForUpdate("lets", true) {
+			t.Fatal("expected opt-out env to skip update check")
+		}
+	})
+
+	t.Run("should skip internal commands", func(t *testing.T) {
+		for _, name := range []string{"completion", "help", "lsp", "self"} {
+			if shouldCheckForUpdate(name, true) {
+				t.Fatalf("expected %q to skip update check", name)
+			}
+		}
+	})
+}