Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/localstack/lstk/internal/runtime"
"github.com/localstack/lstk/internal/telemetry"
"github.com/localstack/lstk/internal/ui"
"github.com/localstack/lstk/internal/update"
"github.com/localstack/lstk/internal/version"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -119,10 +120,19 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t
Telemetry: tel,
}

notifyOpts := update.NotifyOptions{
GitHubToken: cfg.GitHubToken,
UpdatePrompt: config.IsUpdatePromptEnabled(),
PersistDisable: config.DisableUpdatePrompt,
}

if isInteractiveMode(cfg) {
return ui.Run(ctx, rt, version.Version(), opts)
return ui.Run(ctx, rt, version.Version(), opts, notifyOpts)
}
return container.Start(ctx, rt, output.NewPlainSink(os.Stdout), opts, false)

sink := output.NewPlainSink(os.Stdout)
update.NotifyUpdate(ctx, sink, update.NotifyOptions{GitHubToken: cfg.GitHubToken})
return container.Start(ctx, rt, sink, opts, false)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: we're adding latency to all commands with that check, so I am wondering if we should do the check asynchronously or just less often in the future (say once per 24h). Just a note for the future, no action required.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue already for this after noticing. What other tools do is do the check in the background, write it down and on next start show it.

For now I've set the timeout to 500ms. If it fails because of the timeout we won't show it.

}

func runStart(ctx context.Context, cmdFlags *pflag.FlagSet, rt runtime.Runtime, cfg *env.Env, tel *telemetry.Client, logger log.Logger) error {
Expand Down
14 changes: 14 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func setDefaults() {
"port": "4566",
},
})
viper.SetDefault("update_prompt", true)
}

func loadConfig(path string) error {
Expand Down Expand Up @@ -90,6 +91,19 @@ func resolvedConfigPath() string {
return viper.ConfigFileUsed()
}

func Set(key string, value any) error {
viper.Set(key, value)
return viper.WriteConfig()
}

func IsUpdatePromptEnabled() bool {
return viper.GetBool("update_prompt")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: shouldn't this go as a new field in the Config struct instead?

UpdatePrompt bool `mapstructure:"update_prompt"`

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes 😬

}

func DisableUpdatePrompt() error {
return Set("update_prompt", false)
}

func Get() (*Config, error) {
var cfg Config
if err := viper.Unmarshal(&cfg); err != nil {
Expand Down
10 changes: 8 additions & 2 deletions internal/ui/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/localstack/lstk/internal/endpoint"
"github.com/localstack/lstk/internal/output"
"github.com/localstack/lstk/internal/runtime"
"github.com/localstack/lstk/internal/update"
"golang.org/x/term"
)

Expand All @@ -24,7 +25,7 @@ func (s programSender) Send(msg any) {
s.p.Send(msg)
}

func Run(parentCtx context.Context, rt runtime.Runtime, version string, opts container.StartOptions) error {
func Run(parentCtx context.Context, rt runtime.Runtime, version string, opts container.StartOptions, notifyOpts update.NotifyOptions) error {
ctx, cancel := context.WithCancel(parentCtx)
defer cancel()

Expand All @@ -45,7 +46,12 @@ func Run(parentCtx context.Context, rt runtime.Runtime, version string, opts con
go func() {
var err error
defer func() { runErrCh <- err }()
err = container.Start(ctx, rt, output.NewTUISink(programSender{p: p}), opts, true)
sink := output.NewTUISink(programSender{p: p})
if update.NotifyUpdate(ctx, sink, notifyOpts) {
p.Send(runDoneMsg{})
return
}
err = container.Start(ctx, rt, sink, opts, true)
if err != nil {
if errors.Is(err, context.Canceled) {
return
Expand Down
7 changes: 3 additions & 4 deletions internal/update/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ import (
goruntime "runtime"
)

const (
githubRepo = "localstack/lstk"
latestReleaseURL = "https://api.github.com/repos/" + githubRepo + "/releases/latest"
)
const githubRepo = "localstack/lstk"

const latestReleaseURL = "https://api.github.com/repos/" + githubRepo + "/releases/latest"

type githubRelease struct {
TagName string `json:"tag_name"`
Expand Down
119 changes: 119 additions & 0 deletions internal/update/notify.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package update

import (
"context"
"fmt"
"time"

"github.com/localstack/lstk/internal/output"
"github.com/localstack/lstk/internal/version"
)

type versionFetcher func(ctx context.Context, token string) (string, error)

type NotifyOptions struct {
GitHubToken string
UpdatePrompt bool
PersistDisable func() error
}

const checkTimeout = 500 * time.Millisecond

func CheckQuietly(ctx context.Context, githubToken string) (current, latest string, available bool) {
return checkQuietlyWithVersion(ctx, githubToken, version.Version(), fetchLatestVersion)
}

func checkQuietlyWithVersion(ctx context.Context, githubToken string, currentVersion string, fetch versionFetcher) (current, latest string, available bool) {
current = currentVersion
if current == "dev" {
return current, "", false
}

ctx, cancel := context.WithTimeout(ctx, checkTimeout)
defer cancel()

latestVer, err := fetch(ctx, githubToken)
if err != nil {
return current, "", false
}

if normalizeVersion(current) == normalizeVersion(latestVer) {
return current, latestVer, false
}

return current, latestVer, true
}

func UpdateCommandHint(info InstallInfo) string {
switch info.Method {
case InstallHomebrew:
return "brew upgrade localstack/tap/lstk"
case InstallNPM:
return "npm install -g @localstack/lstk@latest"
default:
return "lstk update"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Differentiate local and global npm installs in the hint.

UpdateCommandHint currently maps every npm install to npm install -g ..., but internal/update/update.go:57-83 already distinguishes project-local installs via npmProjectDir(info.ResolvedPath). When the CLI is running from node_modules, the note path tells the user to update the wrong installation. Please branch on npmProjectDir(info.ResolvedPath) here too and add a local-install case to TestUpdateCommandHint.

💡 Suggested fix
 func UpdateCommandHint(info InstallInfo) string {
 	switch info.Method {
 	case InstallHomebrew:
 		return "brew upgrade localstack/tap/lstk"
 	case InstallNPM:
-		return "npm install -g `@localstack/lstk`@latest"
+		if npmProjectDir(info.ResolvedPath) != "" {
+			return "npm install `@localstack/lstk`@latest"
+		}
+		return "npm install -g `@localstack/lstk`@latest"
 	default:
 		return "lstk update"
 	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func UpdateCommandHint(info InstallInfo) string {
switch info.Method {
case InstallHomebrew:
return "brew upgrade localstack/tap/lstk"
case InstallNPM:
return "npm install -g @localstack/lstk@latest"
default:
return "lstk update"
}
func UpdateCommandHint(info InstallInfo) string {
switch info.Method {
case InstallHomebrew:
return "brew upgrade localstack/tap/lstk"
case InstallNPM:
if npmProjectDir(info.ResolvedPath) != "" {
return "npm install `@localstack/lstk`@latest"
}
return "npm install -g `@localstack/lstk`@latest"
default:
return "lstk update"
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/update/notify.go` around lines 47 - 55, UpdateCommandHint currently
returns the global npm update command for any InstallNPM case; change
UpdateCommandHint(info InstallInfo) to detect a project-local install by calling
npmProjectDir(info.ResolvedPath) and, if it returns a non-empty/project dir,
return the local update command (e.g., "npm install `@localstack/lstk`@latest")
instead of the "-g" variant; update the test TestUpdateCommandHint to include a
case where info.Method == InstallNPM and info.ResolvedPath simulates a
node_modules context so the function returns the local-install hint.

}

func NotifyUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions) (exitAfter bool) {
return notifyUpdateWithVersion(ctx, sink, opts, version.Version(), fetchLatestVersion)
}

func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, opts NotifyOptions, currentVersion string, fetch versionFetcher) (exitAfter bool) {
current, latest, available := checkQuietlyWithVersion(ctx, opts.GitHubToken, currentVersion, fetch)
if !available {
return false
}

if !opts.UpdatePrompt {
output.EmitNote(sink, fmt.Sprintf("Update available: %s → %s (run %s)", current, latest, UpdateCommandHint(DetectInstallMethod())))
return false
}

return promptAndUpdate(ctx, sink, opts.GitHubToken, current, latest, opts.PersistDisable)
}

func promptAndUpdate(ctx context.Context, sink output.Sink, githubToken string, current, latest string, persistDisable func() error) (exitAfter bool) {
output.EmitWarning(sink, fmt.Sprintf("Update available: %s → %s", current, latest))

responseCh := make(chan output.InputResponse, 1)
output.EmitUserInputRequest(sink, output.UserInputRequestEvent{
Prompt: "A new version is available",
Options: []output.InputOption{
{Key: "u", Label: "Update"},
{Key: "s", Label: "SKIP"},
{Key: "n", Label: "Never ask again"},
},
ResponseCh: responseCh,
})

var resp output.InputResponse
select {
case resp = <-responseCh:
case <-ctx.Done():
return false
}

if resp.Cancelled {
return false
}

switch resp.SelectedKey {
case "u":
if err := applyUpdate(ctx, sink, latest, githubToken); err != nil {
output.EmitWarning(sink, fmt.Sprintf("Update failed: %v", err))
return false
}
output.EmitSuccess(sink, fmt.Sprintf("Updated to %s — please re-run your command.", latest))
return true
case "n":
if persistDisable != nil {
if err := persistDisable(); err != nil {
output.EmitWarning(sink, fmt.Sprintf("Failed to save preference: %v", err))
}
}
return false
default:
return false
}
}
Loading
Loading