App update available notice on first run of the day - no upgrade command by zateutsch · Pull Request #503 · microsoft/winappCli

zateutsch · 2026-04-27T17:16:11Z

Once a day, on first command run, the CLI will check if a newer version of the CLI is available and show a notification at the top of the command output if so.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a once-per-day update check to the WinApp CLI and prints a non-blocking “update available” notice (with install-channel-specific hints) on the first interactive command run of the day.

Changes:

Introduces UpdateNotificationService + IUpdateNotificationService to fetch the latest GitHub release and cache update checks for 24 hours.
Wires the update check into CLI startup flow (skipping quiet/machine-readable/completions as appropriate) and reuses an early parse to set WINAPP_CLI_CALLER.
Adds unit tests validating caching behavior, notification output, and version comparison logic.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/winapp-CLI/WinApp.Cli/Services/UpdateNotificationService.cs	Implements the daily update check, SemVer comparison, and notification rendering.
src/winapp-CLI/WinApp.Cli/Services/IUpdateNotificationService.cs	Defines the update notification service contract used during startup.
src/winapp-CLI/WinApp.Cli/Program.cs	Hooks the update check into startup and sets `WINAPP_CLI_CALLER` from `--caller`.
src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs	Registers the update notification service in DI.
src/winapp-CLI/WinApp.Cli.Tests/UpdateNotificationServiceTests.cs	Adds coverage for caching, notification output, and version comparison.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-04-27T17:34:07Z

Build Metrics Report

Binary Sizes

Artifact	Baseline	Current	Delta
CLI (ARM64)	30.70 MB	30.72 MB	📈 +19.0 KB (+0.06%)
CLI (x64)	31.06 MB	31.08 MB	📈 +18.0 KB (+0.06%)
MSIX (ARM64)	12.95 MB	12.95 MB	📉 -5.0 KB (-0.04%)
MSIX (x64)	13.77 MB	13.76 MB	📉 -2.6 KB (-0.02%)
NPM Package	26.94 MB	26.95 MB	📈 +10.1 KB (+0.04%)
NuGet Package	27.02 MB	27.03 MB	📈 +9.7 KB (+0.04%)
VS Code Extension	19.76 MB	19.77 MB	📈 +16.0 KB (+0.08%)

Test Results

✅ 855 passed, 1 skipped out of 856 tests in 484.1s (+30 tests, +67.7s vs. baseline)

Test Coverage

❌ 21.6% line coverage, 36.9% branch coverage · ✅ +0.1% vs. baseline

CLI Startup Time

47ms median (x64, winapp --version) · ✅ no change vs. baseline

Updated 2026-05-01 17:38:24 UTC · commit bd74c16 · workflow run

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…sion Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/30586b2a-a92a-468d-8e95-e527acc67aea Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>

nmetulev

Instead of blocking and introducing latency, I wonder if we could instead run the update check in the background and show it the next time the command is ran if there is an update, removing any kind of latency. Following this pattern:

on startup:

read cache
if cache says "update X available" and not yet shown today:
- print notice (instant, no network)
- mark "shown today" in cache
if cache is older than 24h:
- Task.Run(() => RefreshCacheAsync()) // fire and forget
- // do NOT await — let it race the command

on shutdown (best-effort):

if background task is complete, no-op
if still running, abandon it

In addition:

We should have a way for a user to opt out of update checks - maybe a env variable that we document
Should also disable update checks during CI

…able for disabling and check for CI environment

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T00:56:12Z

+        // Save and clear env vars to avoid interference
+        _originalCaller = Environment.GetEnvironmentVariable("WINAPP_CLI_CALLER");
+        _originalLatestVersion = Environment.GetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION");
+        _originalUpdateCheck = Environment.GetEnvironmentVariable("WINAPP_CLI_UPDATE_CHECK");
+        _originalCI = Environment.GetEnvironmentVariable("CI");
+        Environment.SetEnvironmentVariable("WINAPP_CLI_CALLER", null);
+        Environment.SetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION", "0.0.0");
+        Environment.SetEnvironmentVariable("WINAPP_CLI_UPDATE_CHECK", null);
+        Environment.SetEnvironmentVariable("CI", null);
+    }


The tests only clear the CI environment variable, but CIEnvironmentDetectorForTelemetry.IsCIEnvironment() also checks GITHUB_ACTIONS, TF_BUILD, APPVEYOR, TRAVIS, CIRCLECI, etc. When these tests run in CI, CheckAndNotify() will early-return and assertions expecting a notification will fail. Save/clear (and restore) all CI-detection variables used by CIEnvironmentDetectorForTelemetry in Setup/Cleanup, or provide a test hook to bypass CI detection.

Copilot · 2026-04-29T00:56:12Z

+        // Save and clear env vars to avoid interference
+        _originalCaller = Environment.GetEnvironmentVariable("WINAPP_CLI_CALLER");
+        _originalLatestVersion = Environment.GetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION");
+        _originalUpdateCheck = Environment.GetEnvironmentVariable("WINAPP_CLI_UPDATE_CHECK");
+        _originalCI = Environment.GetEnvironmentVariable("CI");
+        Environment.SetEnvironmentVariable("WINAPP_CLI_CALLER", null);
+        Environment.SetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION", "0.0.0");


The tests set WINAPP_CLI_LATEST_VERSION, but VersionHelper.GetVersionString() currently does not read this variable, so it doesn't actually control the "current" version used by CheckAndNotify(). This makes tests like the "older version" case dependent on whatever version the test assembly was built with. Either remove this env var usage and derive test versions from VersionHelper.GetVersionString(), or add a supported version override mechanism used by tests.

Suggested change

// Save and clear env vars to avoid interference

_originalCaller = Environment.GetEnvironmentVariable("WINAPP_CLI_CALLER");

_originalLatestVersion = Environment.GetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION");

_originalUpdateCheck = Environment.GetEnvironmentVariable("WINAPP_CLI_UPDATE_CHECK");

_originalCI = Environment.GetEnvironmentVariable("CI");

Environment.SetEnvironmentVariable("WINAPP_CLI_CALLER", null);

Environment.SetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION", "0.0.0");

var currentVersion = VersionHelper.GetVersionString();

// Save and clear env vars to avoid interference. Default the "latest" version

// to the actual current version used by the product code so tests do not

// accidentally depend on a hardcoded value that does not control the

// installed-version comparison.

_originalCaller = Environment.GetEnvironmentVariable("WINAPP_CLI_CALLER");

_originalLatestVersion = Environment.GetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION");

_originalUpdateCheck = Environment.GetEnvironmentVariable("WINAPP_CLI_UPDATE_CHECK");

_originalCI = Environment.GetEnvironmentVariable("CI");

Environment.SetEnvironmentVariable("WINAPP_CLI_CALLER", null);

Environment.SetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION", currentVersion);

Copilot · 2026-04-29T00:56:13Z

+    public void CheckAndNotify_NoCacheFile_NoNotificationAndStartsBackgroundRefresh()
+    {
+        // First run with no cache — nothing to show, background refresh should start
+        _updateNotificationService.CheckAndNotify();
+
+        var output = TestAnsiConsole.Output;
+        Assert.IsFalse(output.Contains("available"), $"Should not notify on first run (no cache), got: {output}");
+    }


This test path (no cache file) will start the background refresh, which performs a real HTTP call to GitHub. Even though it’s fire-and-forget, this introduces external network I/O into unit tests. Prefer stubbing the latest-version lookup (e.g., injectable provider/HttpClient) or add a test switch to disable refresh tasks.

Copilot · 2026-04-29T00:56:13Z

+    [TestMethod]
+    public void CheckAndNotify_StaleCache_DoesNotBlockOnNetwork()
+    {
+        // Write an expired cache entry — CheckAndNotify should return instantly
+        // (the background refresh is fire-and-forget)
+        var cacheDir = _testCacheDirectory.FullName;
+        var cacheFile = Path.Combine(cacheDir, ".update-check");
+        Directory.CreateDirectory(cacheDir);
+        var expiredTime = DateTime.UtcNow.AddHours(-25).ToString("O");
+        File.WriteAllText(cacheFile, $"{expiredTime}\n0.0.0\n");
+
+        var sw = System.Diagnostics.Stopwatch.StartNew();
+        _updateNotificationService.CheckAndNotify();
+        sw.Stop();
+
+        // Should complete nearly instantly (no network call in the foreground)
+        Assert.IsTrue(sw.ElapsedMilliseconds < 1000, $"CheckAndNotify took {sw.ElapsedMilliseconds}ms — should be instant (no blocking network call)");
+    }


This stale-cache test intentionally triggers the background refresh, which performs a real HTTP call to GitHub. Even if the foreground call is fast, unit tests become nondeterministic and may be noisy in offline/locked-down CI. Prefer stubbing the latest-version lookup (injectable provider/HttpClient) or disabling refresh tasks in tests.

zateutsch · 2026-04-29T01:03:53Z

@copilot apply changes based on the comments in this thread

…rent guard, InvariantCulture Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/d7e88a21-1475-46c7-8881-fe9619103108 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>

Copilot · 2026-04-29T01:13:40Z

Instead of blocking and introducing latency, I wonder if we could instead run the update check in the background and show it the next time the command is ran if there is an update, removing any kind o...

The current implementation already follows this pattern exactly — CheckAndNotify() is synchronous and instant: it reads the cache, shows any cached update notice that hasn't been shown today, then fires RefreshCacheAsync() as a background Task.Run (fire-and-forget) only when the cache is stale. No network call ever blocks the foreground command. The opt-out env var (WINAPP_CLI_UPDATE_CHECK=0) and CI detection via CIEnvironmentDetectorForTelemetry are both in place.

@copilot apply changes based on the comments in this thread

Applied all changes from the review thread in commit 8e664e8:

All CI env vars saved/restored — Setup/Cleanup now save and clear all 13 variables checked by CIEnvironmentDetectorForTelemetry (CI, GITHUB_ACTIONS, TF_BUILD, APPVEYOR, TRAVIS, CIRCLECI, TEAMCITY_VERSION, JB_SPACE_API_URL, CODEBUILD_BUILD_ID, AWS_REGION, BUILD_ID, BUILD_URL, PROJECT_ID).
Removed unused WINAPP_CLI_LATEST_VERSION — vestigial env var that isn't read by the production code.
No real HTTP calls in unit tests — Added internal bool SkipBackgroundRefreshForTesting field; set to true in test Setup() so no background task is spawned.
Markup.Escape(newVersion) — version string from GitHub API is now escaped to prevent markup injection.
Concurrent refresh guard — Static _refreshScheduled int with Interlocked.CompareExchange/Interlocked.Exchange (using named NotScheduled/Scheduled constants) ensures at most one background refresh runs at a time per process.
CultureInfo.InvariantCulture in WriteCacheFile — ToString("O", CultureInfo.InvariantCulture) for the DateTimeOffset.
RefreshCacheAsync re-reads before writing — already done in the previous commit; LastShownDate is read from disk after the network call completes, so a concurrent WriteCacheFile from CheckAndNotify is never overwritten.

nmetulev

PR review notes from a multi-dimensional pass. One high-severity item (stdout pollution), one correctness/UX gap on first-run latency, plus two test-coverage suggestions. Skipping a couple of medium nits I had earlier.

H1 — Update notice goes to stdout, will break script-parseable commands (e.g. get-winapp-path, --version, --help). Recommend routing the notice to stderr.
M1 — Background refresh is fire-and-forget Task.Run; short-lived invocations exit before the HTTP call completes, so the cache may never populate. Recommend a tight bounded foreground wait (~250ms) when the cache is empty so the first-ever run still has a chance to populate.
M3 — GetLatestVersionAsync (HTTP + JSON + v-strip + error suppression) has no deterministic test.
M4 — Program.cs gating for --json / --quiet / --caller plumbing has no integration coverage.

nmetulev · 2026-04-30T00:42:23Z

+        for (var i = 0; i < len; i++)
+        {
+            var aIsNum = int.TryParse(aIds[i], out var aNum);
+            var bIsNum = int.TryParse(bIds[i], out var bNum);


[high] Notice goes to stdout — will break scripted commands

ansiConsole.MarkupLine(...) writes to the registered IAnsiConsole, which is AnsiConsole.Console (default = stdout). The notice fires before command invocation for every non---json/non---quiet run, so it silently corrupts the stdout of commands whose contract is "machine-readable single value" — e.g. winapp get-winapp-path (whose source comment explicitly says "just the path for easy consumption by scripts"). It will also prepend an unrelated line to winapp --version and winapp --help, which users frequently pipe to grep/parsers.

This is what npm, gh, cargo, etc. all hit and why they emit upgrade notices to stderr.

Recommendation: route the notice to stderr — e.g. construct a separate IAnsiConsole bound to Console.Error and use it only for this notification:

var errConsole = AnsiConsole.Create(new AnsiConsoleSettings { Out = new AnsiConsoleOutput(Console.Error) }); errConsole.MarkupLine($"[yellow]v{Markup.Escape(newVersion)} is available. To update, {Markup.Escape(upgradeHint)}.[/]");

That keeps the existing gating logic intact and unblocks every script-consumer use case at the same time.

nmetulev · 2026-04-30T00:42:23Z

+    {
+        var upgradeHint = DetectInstallChannel() switch
+        {
+            InstallChannel.Npm => "npm update -g @microsoft/winappcli",


[medium] Fire-and-forget refresh may never finish

Most CLI commands return in well under a second, but HttpClient.Timeout is 10s. The Task.Run lambda has no awaiter and no graceful-shutdown hook, so on short invocations the process exits before the HTTP call completes and the cache is never populated. In practice, users who run only short commands may never see an update notice.

Recommendation: when the cache is empty / has never been populated, await the refresh in the foreground with a tight bounded timeout (e.g. ~250ms) before falling back to fire-and-forget. That keeps steady-state latency at zero (cache hit path is unchanged) while giving first-run a real chance to populate without noticeably slowing anything down.

nmetulev · 2026-04-30T00:42:23Z

+        // "beta.1.2" > "beta.1" because more fields when prefix matches
+        Assert.IsTrue(UpdateNotificationService.IsNewerVersion("1.0.0-beta.1.2", "1.0.0-beta.1"));
+    }
+}


[medium] GetLatestVersionAsync HTTP/JSON path is essentially untested

The HTTP fetch + tag_name extract + leading-v strip + error suppression in GetLatestVersionAsync has no deterministic coverage. The closest tests (RefreshCacheAsync_WritesUpdateCheckCacheFile, RefreshCacheAsync_PreservesLastShownDate) both pass even when the network call returns nothing, so a regression in JSON parsing or v-prefix handling could ship silently.

Recommendation: inject the HttpClient/HttpMessageHandler (or extract the parser into a static helper that takes a JSON string), then add tests for:

{"tag_name":"v1.2.3"} → 1.2.3

{"tag_name":"1.2.3"} → 1.2.3 (no strip)

missing/null/empty tag_name → null

non-2xx response → null (no throw)

invalid JSON → null (no throw)

network exception → null (no throw)

nmetulev · 2026-04-30T00:42:23Z

+            var caller = parseResult.GetValue(WinAppRootCommand.CallerOption);
+            if (!string.IsNullOrWhiteSpace(caller))
+            {
+                Environment.SetEnvironmentVariable("WINAPP_CLI_CALLER", caller);


[medium] Program-level gating has no integration coverage

The new gating logic added here — skip update notice for --json / --quiet / --cli-schema / --complete, plus setting WINAPP_CLI_CALLER from --caller before invocation — isn't exercised by any test. The new UpdateNotificationServiceTests drive the service directly with env vars already set, so a regression that pollutes --json output or fails to plumb --caller through would still pass the suite.

Recommendation: add a small set of Program-level tests that seed .update-check with a newer version and assert:

stdout for --json contains no notice

stdout for --quiet contains no notice

--caller npm produces the npm hint via the real entrypoint

add upgrade notice

cf66a1e

Copilot AI review requested due to automatic review settings April 27, 2026 17:16

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Copilot started reviewing on behalf of zateutsch April 27, 2026 17:27 View session

zateutsch and others added 5 commits April 27, 2026 16:11

Update src/winapp-CLI/WinApp.Cli/Services/UpdateNotificationService.cs

a39bf63

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/winapp-CLI/WinApp.Cli/Services/UpdateNotificationService.cs

a166c52

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/winapp-CLI/WinApp.Cli/Services/UpdateNotificationService.cs

314b83c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/winapp-CLI/WinApp.Cli/Services/UpdateNotificationService.cs

073aadc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/winapp-CLI/WinApp.Cli.Tests/UpdateNotificationServiceTests.cs

79064fd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of zateutsch April 27, 2026 21:27 View session

fix: implement SemVer pre-release identifier comparison in IsNewerVer…

f879a5b

…sion Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/30586b2a-a92a-468d-8e95-e527acc67aea Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>

Copilot finished work on behalf of zateutsch April 27, 2026 21:34

nmetulev requested changes Apr 28, 2026

View reviewed changes

Comment thread src/winapp-CLI/WinApp.Cli/Services/UpdateNotificationService.cs Outdated

zateutsch added 2 commits April 28, 2026 20:46

address Nikola feedback: cache check to prevent latency, add env vari…

af89a7d

…able for disabling and check for CI environment

remove override version check

0dcbeee

zateutsch requested a review from Copilot April 29, 2026 00:51

Copilot started reviewing on behalf of zateutsch April 29, 2026 00:51 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Copilot started work on behalf of zateutsch April 29, 2026 01:04 View session

fix: address second review round - CI env vars, markup escape, concur…

8e664e8

…rent guard, InvariantCulture Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/d7e88a21-1475-46c7-8881-fe9619103108 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>

Copilot finished work on behalf of zateutsch April 29, 2026 01:14

nmetulev and others added 2 commits April 29, 2026 10:19

Merge branch 'main' into zt/upgrade-notice

e7faf83

Merge branch 'main' into zt/upgrade-notice

0f982f7

nmetulev reviewed Apr 30, 2026

View reviewed changes

nmetulev added 3 commits April 30, 2026 13:16

Merge branch 'main' into zt/upgrade-notice

c2d37be

Merge branch 'main' into zt/upgrade-notice

670fcdb

Merge branch 'main' into zt/upgrade-notice

96e0fbb

Conversation

zateutsch commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Metrics Report

Binary Sizes

Test Results

Test Coverage

CLI Startup Time

Uh oh!

nmetulev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zateutsch commented Apr 29, 2026

Uh oh!

Copilot AI commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nmetulev left a comment

Choose a reason for hiding this comment

Uh oh!

nmetulev Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

nmetulev Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

nmetulev Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

nmetulev Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Apr 27, 2026 •

edited

Loading

Copilot AI commented Apr 29, 2026 •

edited

Loading