App update available notice on first run of the day - no upgrade command#503
App update available notice on first run of the day - no upgrade command#503
Conversation
There was a problem hiding this comment.
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+IUpdateNotificationServiceto 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.
| // Same core version: a stable release (no pre-release) is newer than a pre-release | ||
| if (currentPre != null && latestPre == null) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
When latest and current share the same core version and both are pre-releases (e.g., 1.0.0-beta.2 vs 1.0.0-beta.1), this returns false, so the CLI will never notify about a newer pre-release. Implement proper prerelease ordering (SemVer identifier-by-identifier comparison), or use a SemVer-aware version type already used in the repo (e.g., NuGet’s NuGetVersion) to compare full SemVer strings.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Implemented proper SemVer 2.0.0 pre-release comparison in commit f879a5b. Added a ComparePreRelease helper that compares pre-release strings identifier-by-identifier per the spec: numeric identifiers are compared numerically, alphanumeric lexically, numeric always sorts before alphanumeric, and a longer pre-release string beats a shorter matching prefix. Cases like 1.0.0-beta.2 vs 1.0.0-beta.1 now correctly return true (newer). Added unit tests covering numeric ordering, lexical ordering, numeric-vs-alphanumeric precedence, and length tiebreaking.
Build Metrics ReportBinary Sizes
Test Results❌ 806 passed, 1 failed, 1 skipped out of 808 tests in 466.5s (+24 tests, -12.5s vs. baseline) Test Coverage❌ 21.6% line coverage, 36.9% branch coverage · ✅ +0.1% vs. baseline CLI Startup Time48ms median (x64, Updated 2026-04-27 22:21:26 UTC · commit |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
left a comment
nmetulev
left a comment
There was a problem hiding this comment.
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
| internal async Task<string?> GetLatestVersionAsync(CancellationToken cancellationToken = default) | ||
| { | ||
| // Allow overriding the latest version for testing (skips GitHub API call) | ||
| var overrideVersion = Environment.GetEnvironmentVariable("WINAPP_CLI_LATEST_VERSION"); |
There was a problem hiding this comment.
This ends up in the production - do we need it in production?
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.