Add daily update check and upgrade command to handle upgrading from CLI#424
Add daily update check and upgrade command to handle upgrading from CLI#424
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a daily “new version available” notification and a winapp upgrade command so the CLI can guide (and in some cases perform) self-upgrades based on install channel, addressing issue #392.
Changes:
- Introduces
ICliUpgradeService/CliUpgradeServiceand wires it into startup to check for updates once per day. - Adds a new
upgradecommand and registers it in the root command/help categories and CLI schema output. - Updates the npm-generated command wrappers (including
run --symbols) and refreshes schema references.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/winapp-npm/src/winapp-commands.ts | Regenerated npm command wrappers/docs; adds run --symbols and updates manifest wording. |
| src/winapp-CLI/WinApp.Cli/Services/ICliUpgradeService.cs | New interface + InstallChannel enum for upgrade/update-check behavior. |
| src/winapp-CLI/WinApp.Cli/Services/CliUpgradeService.cs | Implements daily update check + upgrade flows for MSIX/exe and instructions for npm/NuGet. |
| src/winapp-CLI/WinApp.Cli/Program.cs | Invokes update check during startup (non-quiet, non-json/schema modes). |
| src/winapp-CLI/WinApp.Cli/NativeMethods.txt | Adds GetCurrentPackageFullName for MSIX identity detection. |
| src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs | Registers CliUpgradeService and hooks up UpgradeCommand handler. |
| src/winapp-CLI/WinApp.Cli/Commands/WinAppRootCommand.cs | Adds upgrade to root command and “Setup” help category. |
| src/winapp-CLI/WinApp.Cli/Commands/UpgradeCommand.cs | New winapp upgrade command delegating to ICliUpgradeService. |
| src/winapp-CLI/WinApp.Cli.Tests/CliUpgradeServiceTests.cs | Adds tests for install channel detection, cache file behavior, and command registration. |
| docs/cli-schema.json | Adds upgrade command metadata to the published CLI schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Build Metrics ReportBinary Sizes
Test Results✅ 843 passed, 2 skipped out of 845 tests in 474.1s (+61 tests, +83.4s vs. baseline) Test Coverage❌ 21.6% line coverage, 36.8% branch coverage · ✅ +0.2% vs. baseline CLI Startup Time37ms median (x64, Updated 2026-04-22 00:41:32 UTC · commit |
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/26b6a1b4-d856-4da4-afa7-e2ef81662b68 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Implemented the requested review-thread fixes in commit d62b629. This includes startup timeout + early caller parsing in Program, culture-safe update-cache timestamp parsing, nodejs-package caller detection, corrected npm upgrade package name, unsafe package-identity P/Invoke usage, network-isolated upgrade service tests, and regenerated schema/npm wrapper docs (including manifest wording alignment and the upgrade-options generator lint fix). Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/96407090-ac13-4e42-b85d-cf2da3d95598 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Applied the requested review-thread updates in commit 0ed5dfb. This re-aligns |
nmetulev
left a comment
nmetulev
left a comment
There was a problem hiding this comment.
Can you update the description of the pr - we use the description for release notes and blog posts and the more info we have the better.
Looks like the upgrade command needs to be added to the generate-llm-docs.ps1 script so it's added to the skills
Add upgrade skill template and register it in generate-llm-docs.ps1 so the upgrade command appears in the auto-generated agent skills. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The recent merge of main dropped the declarations of rootCommand and
parseResult in Program.cs while leaving call sites that reference them,
causing five CS0103 "name does not exist in the current context" errors.
Restore the canonical structure from main:
- resolve rootCommand from the service provider just before the
args.Length == 0 short-circuit (so it can serve --help too)
- call rootCommand.Parse(args) once and assign to parseResult
- re-add the WINAPP_CLI_CALLER env var setup
Also drop the leftover effectiveParseResult = parseResult ?? ... that
guarded against a nullable parseResult that no longer exists in this
control flow.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pushed 03c5835 to fix the build break. The recent merge of main dropped the declarations of |
nmetulev
left a comment
nmetulev
left a comment
There was a problem hiding this comment.
Did an end-to-end review (code + manual smoke test on a Release-published build). Full test suite is green (787/787). Findings below — one is a blocker, a few others worth addressing before merge.
🔴 Critical — must fix before merge
1. winapp upgrade can silently DOWNGRADE users. UpgradeAsync (CliUpgradeService.cs:136) never calls IsNewerVersion — it unconditionally downloads and installs whatever /releases/latest returns. I reproduced this: published a fresh Release build (v1.0.0+<sha>, 23 MB), ran winapp upgrade, and my binary was replaced with v0.2.1 (18 MB), leaving a .old backup. Anyone on a nightly/dev/preview build running winapp upgrade will be silently downgraded. Suggested fix: gate UpgradeMsixAsync/UpgradeExeAsync on IsNewerVersion(latest, current) and print "already up to date" otherwise, and/or add a --force flag.
2. No signature/checksum verification on the downloaded MSIX/EXE (lines ~180 and ~230). The exe rename trick then puts an unverified binary in place. Given this is an auto-update path, at minimum the downloaded artifact should have its Authenticode signature verified before install.
🟡 Should-fix
3. No overall timeout on UpgradeAsync. The static HttpClient has no Timeout; only CheckAndNotifyAsync is gated by the 2s WaitAsync in Program.cs. A slow/hung download from upgrade will hang indefinitely.
4. Notification uses static AnsiConsole.MarkupLine (line 335) instead of the DI'd IAnsiConsole — same testability issue I just fixed in #485. No test actually asserts the notification text is shown.
5. No tests for the risky paths (MSIX install, standalone exe swap, GitHub API parsing, IsNewerVersion edge cases). Tests today cover npm/nuget (which just log) + channel detection + cache behavior.
🟢 Minor
6. Cache file's second line is dead data — WriteCacheFile writes {timestamp}\n{newVersion}, but CheckAndNotifyAsync reads only lines[0]. Either use the version (e.g., to avoid re-notifying the same version next day) or drop it.
7. UpgradeCommand_HasCorrectDescription asserts on exact description text → brittle.
8. Update check fires on winapp --version, potentially adding up to 2 s to a query that should be instant.
✅ Verified working
- Channel detection (npm/nuget/standalone via
WINAPP_CLI_CALLER) - 24-hour cache debounce
--quiet/--json/--cli-schemacorrectly skip the check- Notification renders correctly ("v99.0.0 is available...")
- Exe rename self-update actually works on Windows
- PR #485's clean stdout for
get-winapp-pathstill holds - Full suite: 787/787 passing
Happy to approve once #1 is addressed — that's an active regression risk. #2–#5 are worth doing before merge; the rest are follow-up candidates.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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>
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/0cfb4f46-1570-4d97-88d4-a356056c1d11 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/0cfb4f46-1570-4d97-88d4-a356056c1d11 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/0cfb4f46-1570-4d97-88d4-a356056c1d11 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Applied the remaining thread changes in commits 0f84faf and 2f53f0c. This moves
Updated in this branch:
Addressed the critical/should-fix upgrade items in this branch, including downgrade guard + Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
fixes #392
This PR adds a daily check for a new version of WinAppCli and adds an
upgradecommand that handles upgrading the user's CLI to the latest version with whatever original installation method they used.