Extend no-cache to remote mixins

docs/docs/changelog.md

@@ -6,6 +6,7 @@ title: Changelog
 ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X)
 
 * `[Added]` Show interactive download progress for remote configs and remote mixins. Issue [#360](https://github.com/lets-cli/lets/issues/360)
+* `[Fixed]` Make `--no-cache` re-download remote mixins for local and remote configs. Issue [#365](https://github.com/lets-cli/lets/issues/365)
 * `[Added]` Remote config support: `lets -c https://url` downloads and caches config to `~/.config/lets/remote-configs/`. Use `--no-cache` to force re-download.
 * `[Added]` Add `lets self skills` commands to show, install, and update the bundled lets agent skill.
 * `[Docs]` Document the bundled lets Agent Skill and link it from the config reference.

internal/cli/cli.go

@@ -90,6 +90,10 @@ func Main(version string, buildDate string) int {
 		)))
 	}
 
+	if rootFlags.noCache {
+		loadOptions = append(loadOptions, loader.WithNoCache())
+	}
+
 	if isRemoteURL(rootFlags.config) {
 		if configDir != "" {
 			log.Warnf("LETS_CONFIG_DIR is ignored when using a remote config URL")

internal/cmd/root.go

@@ -105,8 +105,8 @@ func initRootFlags(rootCmd *cobra.Command) {
 	rootCmd.Flags().StringArray("exclude", []string{}, "run all but excluded command(s) described in cmd as map")
 	rootCmd.Flags().Bool("init", false, "create a new lets.yaml in the current folder")
 	rootCmd.Flags().Bool("no-depends", false, "skip 'depends' for running command")
-	rootCmd.Flags().CountP("debug", "d", "show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs") //nolint:lll
+	rootCmd.Flags().CountP("debug", "d", "show debug logs; repeat for verbose output")
 	rootCmd.Flags().StringP("config", "c", "", "config file (default is lets.yaml)")
 	rootCmd.Flags().Bool("all", false, "show all commands (including the ones with _)")
-	rootCmd.Flags().Bool("no-cache", false, "re-download remote config instead of using cached version")
+	rootCmd.Flags().Bool("no-cache", false, "re-download remote config and mixins instead of using cached versions")
 }

internal/cmd/testdata/TestHelpGolden/basic.golden

@@ -16,12 +16,12 @@ INTERNAL COMMANDS:
 FLAGS
   --all         Show all commands (including the ones with _)
   -c --config   Config file (default is lets.yaml)
-  -d --debug    Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs
+  -d --debug    Show debug logs; repeat for verbose output
   -E --env      Set env variable for running command KEY=VALUE
   --exclude     Run all but excluded command(s) described in cmd as map
   -h --help     Help for lets
   --init        Create a new lets.yaml in the current folder
-  --no-cache    Re-Download remote config instead of using cached version
+  --no-cache    Re-Download remote config and mixins instead of using cached versions
   --no-depends  Skip 'depends' for running command
   --only        Run only specified command(s) described in cmd as map
   -v --version  Version for lets

internal/cmd/testdata/TestHelpGolden/grouped_commands.golden

@@ -21,12 +21,12 @@ INTERNAL COMMANDS:
 FLAGS
   --all         Show all commands (including the ones with _)
   -c --config   Config file (default is lets.yaml)
-  -d --debug    Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs
+  -d --debug    Show debug logs; repeat for verbose output
   -E --env      Set env variable for running command KEY=VALUE
   --exclude     Run all but excluded command(s) described in cmd as map
   -h --help     Help for lets
   --init        Create a new lets.yaml in the current folder
-  --no-cache    Re-Download remote config instead of using cached version
+  --no-cache    Re-Download remote config and mixins instead of using cached versions
   --no-depends  Skip 'depends' for running command
   --only        Run only specified command(s) described in cmd as map
   -v --version  Version for lets

internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden

@@ -22,12 +22,12 @@ INTERNAL COMMANDS:
 FLAGS
   --all                                   Show all commands (including the ones with _)
   -c --config                             Config file (default is lets.yaml)
-  -d --debug                              Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs
+  -d --debug                              Show debug logs; repeat for verbose output
   -E --env                                Set env variable for running command KEY=VALUE
   --exclude                               Run all but excluded command(s) described in cmd as map
   -h --help                               Help for lets
   --init                                  Create a new lets.yaml in the current folder
-  --no-cache                              Re-Download remote config instead of using cached version
+  --no-cache                              Re-Download remote config and mixins instead of using cached versions
   --no-depends                            Skip 'depends' for running command
   --only                                  Run only specified command(s) described in cmd as map
   -v --version                            Version for lets

internal/cmd/testdata/TestHelpGolden/long_command.golden

@@ -17,12 +17,12 @@ INTERNAL COMMANDS:
 FLAGS
   --all                                 Show all commands (including the ones with _)
   -c --config                           Config file (default is lets.yaml)
-  -d --debug                            Show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs
+  -d --debug                            Show debug logs; repeat for verbose output
   -E --env                              Set env variable for running command KEY=VALUE
   --exclude                             Run all but excluded command(s) described in cmd as map
   -h --help                             Help for lets
   --init                                Create a new lets.yaml in the current folder
-  --no-cache                            Re-Download remote config instead of using cached version
+  --no-cache                            Re-Download remote config and mixins instead of using cached versions
   --no-depends                          Skip 'depends' for running command
   --only                                Run only specified command(s) described in cmd as map
   -v --version                          Version for lets

internal/config/config/config.go

@@ -14,6 +14,7 @@ import (
 	"github.com/lets-cli/lets/internal/fetch"
 	"github.com/lets-cli/lets/internal/set"
 	"github.com/lets-cli/lets/internal/util"
+	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v3"
 )
 
@@ -57,12 +58,14 @@ type Config struct {
 	cachedEnv        map[string]string
 	downloadContext  context.Context
 	downloadProgress fetch.ProgressObserver
+	noCache          bool
 	isMixin          bool // if true, we consider config as mixin and apply different parsing and validation
 }
 
-func (c *Config) SetDownloadOptions(ctx context.Context, progress fetch.ProgressObserver) {
+func (c *Config) SetDownloadOptions(ctx context.Context, progress fetch.ProgressObserver, noCache bool) {
 	c.downloadContext = ctx
 	c.downloadProgress = progress
+	c.noCache = noCache
 }
 
 func (c *Config) context() context.Context {
@@ -236,18 +239,44 @@ func (c *Config) readMixin(mixin *Mixin) error {
 
 		rm := mixin.Remote
 
-		data, err := rm.tryRead()
-		if err != nil {
-			return err
-		}
+		var (
+			data       []byte
+			err        error
+			downloaded bool
+		)
 
-		if data == nil {
-			data, err = rm.download(c.context(), c.downloadProgress)
+		if !c.noCache {
+			data, err = rm.tryRead()
 			if err != nil {
 				return err
 			}
 		}
 
+		if data == nil {
+			downloadedData, downloadErr := rm.download(c.context(), c.downloadProgress)
+			if downloadErr != nil {
+				if c.noCache {
+					cachedData, readErr := rm.tryRead()
+					if readErr != nil {
+						return readErr
+					}
+
+					if cachedData != nil {
+						log.Warnf("failed to download remote mixin (%v), falling back to cached version", downloadErr)
+
+						data = cachedData
+					} else {
+						return downloadErr
+					}
+				} else {
+					return downloadErr
+				}
+			} else {
+				data = downloadedData
+				downloaded = true
+			}
+		}
+
 		// TODO: what if multiple mixins have same commands
 		//  1 option - fail and suggest use to namespace all commands in remote mixin
 		//  2 option - namespace it (this may require specifying namespace in mixin config or in main config mixin section)
@@ -263,8 +292,10 @@ func (c *Config) readMixin(mixin *Mixin) error {
 			return fmt.Errorf("failed to merge remote mixin config '%s' with main config: %w", rm.URL, err)
 		}
 
-		if err := rm.persist(data); err != nil {
-			return fmt.Errorf("failed to persist remote mixin config %s: %w", rm.URL, err)
+		if downloaded {
+			if err := rm.persist(data); err != nil {
+				return fmt.Errorf("failed to persist remote mixin config %s: %w", rm.URL, err)
+			}
 		}
 	} else {
 		mixinAbsPath, err := path.GetFullConfigPath(mixin.FileName, c.WorkDir)
@@ -373,7 +404,7 @@ func NewConfig(workDir string, configAbsPath string, dotLetsDir string) *Config
 func NewMixinConfig(cfg *Config, configAbsPath string) *Config {
 	mixin := NewConfig(cfg.WorkDir, configAbsPath, cfg.DotLetsDir)
 	mixin.isMixin = true
-	mixin.SetDownloadOptions(cfg.context(), cfg.downloadProgress)
+	mixin.SetDownloadOptions(cfg.context(), cfg.downloadProgress, cfg.noCache)
 
 	return mixin
 }

internal/config/config/mixin.go

@@ -47,10 +47,11 @@ func (rm *RemoteMixin) Path() string {
 }
 
 func (rm *RemoteMixin) persist(data []byte) error {
-	f, err := os.OpenFile(rm.Path(), os.O_CREATE|os.O_WRONLY, 0o755) //nolint:nosnakecase
+	f, err := os.OpenFile(rm.Path(), os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644) //nolint:nosnakecase
 	if err != nil {
 		return fmt.Errorf("can not open file %s to persist mixin: %w", rm.Path(), err)
 	}
+	defer f.Close()
 
 	_, err = f.Write(data)
 	if err != nil {

internal/config/load.go

@@ -18,6 +18,7 @@ import (
 
 type loadOptions struct {
 	progress fetch.ProgressObserver
+	noCache  bool
 }
 
 type LoadOption func(*loadOptions)
@@ -28,6 +29,12 @@ func WithProgress(progress fetch.ProgressObserver) LoadOption {
 	}
 }
 
+func WithNoCache() LoadOption {
+	return func(opts *loadOptions) {
+		opts.noCache = true
+	}
+}
+
 func newLoadOptions(options []LoadOption) loadOptions {
 	opts := loadOptions{}
 	for _, option := range options {
@@ -56,6 +63,9 @@ func LoadWithContext(ctx context.Context, configName string, configDir string, v
 // returns a Config with the working directory set to the caller's CWD.
 func LoadRemote(ctx context.Context, url string, noCache bool, version string, options ...LoadOption) (*config.Config, error) {
 	opts := newLoadOptions(options)
+	if noCache {
+		opts.noCache = true
+	}
 
 	cachedPath, err := ensureRemoteConfig(ctx, url, noCache, opts.progress)
 	if err != nil {
@@ -100,7 +110,7 @@ func loadConfigFromFile(
 	defer f.Close()
 
 	c := config.NewConfig(workDir, absPath, dotLetsDir)
-	c.SetDownloadOptions(ctx, opts.progress)
+	c.SetDownloadOptions(ctx, opts.progress, opts.noCache)
 
 	if err := yaml.NewDecoder(f).Decode(c); err != nil {
 		return nil, fmt.Errorf("failed to parse %s: %w", displayName, err)

internal/config/load_test.go

@@ -208,7 +208,9 @@ func TestLoadRemote(t *testing.T) {
 
 	t.Run("reports progress for remote mixin cache miss only", func(t *testing.T) {
 		mixinConfig := "commands:\n  mixed:\n    cmd: echo mixed\n"
+		requests := 0
 		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			requests++
 			w.Header().Set("Content-Type", "application/yaml")
 			_, _ = w.Write([]byte(mixinConfig))
 		}))
@@ -238,5 +240,73 @@ func TestLoadRemote(t *testing.T) {
 		if len(cacheHitProgress.starts) != 0 {
 			t.Fatalf("expected no progress starts for remote mixin cache hit, got %d", len(cacheHitProgress.starts))
 		}
+		if requests != 1 {
+			t.Fatalf("expected 1 HTTP request, got %d", requests)
+		}
+	})
+
+	t.Run("re-downloads remote mixins with --no-cache", func(t *testing.T) {
+		requests := 0
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			requests++
+			w.Header().Set("Content-Type", "application/yaml")
+			_, _ = w.Write([]byte("commands:\n  mixed:\n    cmd: echo mixed\n"))
+		}))
+		defer srv.Close()
+
+		tempDir := t.TempDir()
+		mainConfig := "shell: bash\nmixins:\n  - url: " + srv.URL + "\ncommands:\n  ok:\n    cmd: echo ok\n"
+		if err := os.WriteFile(filepath.Join(tempDir, "lets.yaml"), []byte(mainConfig), 0o644); err != nil {
+			t.Fatalf("write config: %v", err)
+		}
+
+		if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test"); err != nil {
+			t.Fatalf("prime cache error: %v", err)
+		}
+		progress := &recordingProgress{}
+		if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test", WithNoCache(), WithProgress(progress)); err != nil {
+			t.Fatalf("no-cache load error: %v", err)
+		}
+		if requests != 2 {
+			t.Fatalf("expected 2 HTTP requests, got %d", requests)
+		}
+		if len(progress.starts) != 1 {
+			t.Fatalf("expected 1 progress start, got %d", len(progress.starts))
+		}
+		if progress.starts[0].Kind != fetch.SourceRemoteMixin {
+			t.Fatalf("expected remote mixin progress, got %q", progress.starts[0].Kind)
+		}
+	})
+
+	t.Run("does not replace remote mixin cache until downloaded mixin is valid", func(t *testing.T) {
+		mixinConfig := "commands:\n  mixed:\n    cmd: echo mixed\n"
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/yaml")
+			_, _ = w.Write([]byte(mixinConfig))
+		}))
+		defer srv.Close()
+
+		tempDir := t.TempDir()
+		mainConfig := "shell: bash\nmixins:\n  - url: " + srv.URL + "\ncommands:\n  ok:\n    cmd: echo ok\n"
+		if err := os.WriteFile(filepath.Join(tempDir, "lets.yaml"), []byte(mainConfig), 0o644); err != nil {
+			t.Fatalf("write config: %v", err)
+		}
+
+		if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test"); err != nil {
+			t.Fatalf("prime cache error: %v", err)
+		}
+
+		mixinConfig = "commands:\n  broken:\n    xxx\n    cmd: echo broken\n"
+		if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test", WithNoCache()); err == nil {
+			t.Fatal("expected no-cache load to fail for invalid downloaded mixin")
+		}
+
+		cfg, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test")
+		if err != nil {
+			t.Fatalf("cached load error: %v", err)
+		}
+		if _, ok := cfg.Commands["mixed"]; !ok {
+			t.Fatal("expected valid cached mixin command")
+		}
 	})
 }