Allow permanently disabling lstk update checks - #491
Conversation
7838fda to
6c6a856
Compare
|
@gtsiolis What's your thought regarding the open question (see Section "Open questions")? Do you see a UX issue in having the |
|
@carillan81 What's your suggestion on how update notifications for bundled extensions work in this context (e.g., externally-managed lstk)? Do we to treat lstk and lstk extension updates separately? Happy to discuss. |
|
Extensions are only updated when |
Great, thank you for the clarification @carillan81 👍 Then, our PRs complement each other nicely ✨ |
anisaoshafi
left a comment
There was a problem hiding this comment.
I didn't review in detail, but the overall logic looks good ✨
Thanks for the comprehensive set of screenshots in Manual testing section in PR description.
I also like the tradeoff you took on the open question to drop the option [N] when config.toml doesn't exist.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6c6a856 to
4e0b093
Compare
|
|
||
| # CLI behavior | ||
| [cli] | ||
| # update_check = "notify" # Update check on start: "prompt" (default), "notify", "off" |
There was a problem hiding this comment.
suggestion: Could we simplify this to a boolean setting, like check_for_update_on_startup = false? When enabled, we could preserve the existing prompt for self-managed interactive installations and show a non-blocking notice for externally managed ones, as discussed in the relevant discussion. When disabled, it would skip the check entirely. This could cover the use cases without asking users to choose between prompting and notifying. 💭
Boolean behavior coverage:
| Setting | Self-managed, interactive | Externally managed |
|---|---|---|
| true | Prompt | Non-blocking notice |
| false | No check | No check |
There was a problem hiding this comment.
My thoughts:
- Pros: Simpler user configuration with self-describing flag
LSTK_CHECK_FOR_UPDATE_ON_STARTUP=0; no more magic [N] Never ask again conversion to notify - Con: Self-managed users (and failed detection of externally-managed) cannot opt into the non-blocking notify-only behavior anymore -> more users disable update check altogether
I think we should go for the simplification if we don't plan to offer a notify-only option.
There was a problem hiding this comment.
I can’t think of a use case or user need for an explicit notify-only preference on self-managed installations. Externally managed installations would already get that behavior automatically.
I’d go with the simpler boolean for now and revisit if user feedback shows a need for more control. 💯
There was a problem hiding this comment.
Done ✅
I migrated the config to use the simplified boolean approach:
[cli] check_for_update_on_startup and LSTK_CHECK_FOR_UPDATE_ON_STARTUP
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@carillan81 I discovered and fixed an interaction between the bundled extension (#482) and this PR (see PR description).
Could you review this part and validate whether the adjusted message for externally-managed installations is appropriate? |
Motivation
lstk asks to update on every start with no way to turn it off. With weekly releases, "skip this version" buys a few days — the reporter sees the prompt almost daily.
It is also wrong when another tool owns the binary. mise, nix, asdf, scoop and chocolatey manage the version themselves; replacing it desynchronises them, and on nix the store is read-only so the update fails after a full download.
Solution
[cli] check_for_update_on_startup, alsoLSTK_CHECK_FOR_UPDATE_ON_STARTUP:true(default)falseA boolean rather than a
prompt/notify/offenum (review): install detection already decides between a prompt and a note, so the only choice left to the user is whether to check at all. Externally-managed installs get the note and name the manager rather than advisinglstk update.lstk updaterefuses on them (--forceoverrides); an explicitlstk updateandlstk update --checkare never gated by the setting.Prompt: before / after
[S] Skip this version— per-version, buys days[N] Never check again— persistscheck_for_update_on_startup = false[S]is removed rather than kept alongside[N]: it is the option the ticket says does not solve the problem, it was the third way to say "no" on one prompt, and it was the only per-version persisted state.cli.update_skipped_versiongoes with it; a leftover key is inert.Docs
Docs needed
New user-facing surface to document:
[cli] check_for_update_on_startupconfig key and theLSTK_CHECK_FOR_UPDATE_ON_STARTUPenvironment variable (boolean, default true), and that neither gates an explicitlstk update.notifyand are not updated in place;lstk update --forceoverrides.--jsongains theUPDATE_EXTERNALLY_MANAGEDerror code (docs/structured-output.mdupdated in this PR).[S] Skip this versionand gains[N] Never check again.Manual testing (using fish shell) — 13 scenarios, all passing
Covered: enabled and disabled; env var overriding config; an invalid value rejected rather than coerced; a mise install naming the manager in its note;
lstk updaterefusing on mise and nix (plain and--json); a read-only install directory refused;update --checkstill working when the check is disabled;[N]persisting the opt-out and the next run staying silent; the first-run prompt omitting[N]; and a mise install never prompting.To replicate. Build a version-stamped binary (a
devbuild skips the check entirely) and point Docker at nothing, sostartemits the update output and then fails at the health check without starting an emulator:Pressing
[U]performs a real download; setLSTK_UPDATE_GITHUB_API_ENDPOINTandLSTK_UPDATE_GITHUB_DOWNLOAD_ENDPOINTto a dead address to make it inert.Interaction with #482 (bundled extensions)
#482 landed in parallel and touches the same file.
A silent one.
detectMissingBundleguards onInstallBinary, so introducingInstallExternalwould have quietly removed the missing-bundle hint for mise/nix/asdf installs — they used to classify asInstallBinary. It now covers external installs and names the managing tool (reinstall lstk through mise) instead of#482's release-download instruction, which would install outside the manager and be overwritten on its next sync.Open questions
Where the update prompt sits in the start flow.
NotifyUpdateruns as the first action of the start goroutine, ahead of the Docker health check, auth, and the emulator picker. On a genuine first runconfig.tomldoes not exist yet — the picker creates it — so[N]has nowhere to persist and is omitted rather than offered and silently dropped:The trade-off: moving the notification after the picker would make
[N]available on every run, but prompting early is worth more — a user on an old or broken CLI should be offered the update before the CLI attempts real work, and a late prompt would be preempted by exactly the Docker failure an update might fix. The gap is narrow, since a first run almost always means a fresh install already on the latest version. Deliberately left as is; flagging it for a second opinion.Review
Worth a human review. New user-facing surface (config key, env var, flag, error code) and a changed prompt, plus one behaviour change that is not opt-in: a detected mise/nix/asdf install gets a note instead of the prompt.
Automatic detection of externally managed (e.g., mise package installer) provides convenience by default, but add a bunch of complexity. The alternative would be solely relying on manual configuration.
Openspec preview: https://github.com/localstack/lstk/tree/4330e4d/openspec/changes/add-update-check-config
Related
#465 was an earlier, independent attempt at the same design, closed 2026-08-25.
#482 extending updates for extensions
Closes DEVX-1029