[Enhancement]: About - Startup GitHub update check is unconditional and unthrottled

[laurentiu021]

Problem

Every launch contacts GitHub, and the user cannot turn it off. AboutViewModel's public constructor hardcodes the check on (: this(updates, reportService, autoCheck: true), AboutViewModel.cs:62-63 -> if (autoCheck) InitializeAsync(InitAsync) at :75-76), and the About VM is deliberately resolved EAGERLY at startup precisely so this fires - MainWindowViewModel.cs:80 (About = Eager<AboutViewModel>();) with the reason stated at MainWindowViewModel.cs:30-32. The request goes to api.github.com/repos/.../releases/latest (UpdateService.cs:81), and CheckAtStartupAsync also chains a second call for the last 10 releases (AboutViewModel.cs:96 -> GetRecentAsync(10) at :159 -> UpdateService.cs:113-118). I grepped for a toggle: grep -rn "AutoCheck|UpdateCheckEnabled|CheckForUpdatesOnStartup|updateCheck" across the app returns only the internal test-only constructor parameter and the unrelated UpdateCheckFailed display flag - no setting, no UI, no persistence. There is also no throttle: nothing records when the last check ran, so N launches in an hour make 2N requests against a limit the code itself documents as 60/hour/IP (UpdateService.cs:19-20). Only the autoCheck: false path exists, and it is internal for tests (AboutViewModel.cs:71).

Proposed solution

Persist one boolean and gate the call on it, reusing the pattern that already exists rather than inventing one. ThemeService writes its settings to %APPDATA%\SysManager\theme.json via JsonSerializer + File.WriteAllText (ThemeService.cs:19-21, Save at :354-367, Load at :369-376) - follow it exactly. Then: (1) gate AboutViewModel.cs:75 on the persisted value instead of the hardcoded true at line 63; (2) add a plain-language checkbox in the About version card next to the existing "Check for updates" button (AboutView.xaml:44-47) - wording for the persona, e.g. "Check GitHub for a new version when SysManager starts", with AutomationProperties.Name per Gate-UX; (3) record a last-checked timestamp alongside it and skip the startup check if it ran within the last 24h - the manual "Check for updates" button (AboutView.xaml:45) and "Retry" (:149) always bypass the throttle, so the user is never blocked; (4) register the new file in ProfileService's Catalog (ProfileService.cs:48-51, which today lists exactly two sections - theme.json and speedtest-history.json) so the preference survives profile export/import.

Rationale

This is the one place the product's strongest marketing claim is contestable. The README's competitor table leads with "No telemetry / no account" and "Fully local (no cloud)" (README.md:74-76), and the network disclosure at README.md:48-52 lists the update check among things that happen only "for features you explicitly use" - but the startup check is not a feature the user chose to use; it fires before any interaction, on every launch, with no switch. A skeptical reviewer will find that gap, and so will anyone on a metered connection, behind a corporate proxy, or on an air-gapped machine. Adding the toggle costs little and makes the privacy claim airtight instead of arguable. The 24h throttle additionally removes a real failure mode: a user who restarts the app repeatedly can exhaust the documented anonymous limit and then sees the error state (AboutViewModel.cs:114-118) for no reason.

Evidence

AboutViewModel.cs:62-63 (public ctor forces autoCheck: true), :71 (internal ctor is the only opt-out, for tests), :75-76 (if (autoCheck) InitializeAsync(InitAsync)), :90-100 (CheckAtStartupAsync -> CheckForUpdates + LoadHistory), :159 (GetRecentAsync(10)). Eager resolution and its stated rationale: MainWindowViewModel.cs:26-32 and :78-80. Endpoints: UpdateService.cs:81, :117. Rate limit documented in the service's own summary: UpdateService.cs:19-20 ("no auth needed as long as we stay under the anonymous rate limit (60 req/hour/IP)"). Absence of any setting: grep for AutoCheck|UpdateCheckEnabled|CheckForUpdatesOnStartup|updateCheck -> only AboutViewModel.cs:36 (_updateCheckFailed), :63, :66, :71, :75. Settings-persistence pattern to copy: ThemeService.cs:19-21, :354-367, :369-376. Profile catalog with its current two entries: ProfileService.cs:48-51.

Risk / trade-off

If a user switches the check off, they stop learning about fixes - including security fixes - so the default must stay ON and the copy must not read as a scare-off. Turning it off must also blank the shell update banner cleanly (MainWindow.xaml:385 binds About.UpdateAvailable) rather than leaving a stale value. The About VM is eager specifically to feed that banner (MainWindowViewModel.cs:30-32); gating the network call must not accidentally make the VM lazy, or the version label at MainWindow.xaml:332 goes blank. Adding a settings file also expands ProfileService's surface - cover it with a round-trip test.

Affected area

About

---

Effort: M | priority: value 4/5, fit 5/5
Identified during the trust, distribution and reach audit audit.

[laurentiu021]

Done in v1.58.0 (PR #1724).

Implemented as proposed: a persisted preference gates the startup path, a 24h throttle sits behind it, the manual Check for updates and Retry buttons bypass both, and the new file is registered in ProfileService's catalog so it survives export/import.

The checkbox is worded for the persona and shaped like Bulk Installer's existing "Load app icons from the web" toggle — plain language, AutomationProperties.Name, and a tooltip that names exactly what does and does not leave the machine:

☑ Check GitHub for a new version when SysManager starts

Two deliberate departures from the proposal, both for reasons found while implementing:

1. Not copying ThemeService's persistence. ThemeService uses static readonly paths, which is precisely the pattern that made ResourceHistoryService impossible to test (fixed in fix: serialize the Resource History reloads and handle an unreadable history file #1713 — SpecialFolder.LocalApplicationData resolves through the Win32 known-folder API and ignores the LOCALAPPDATA env var, so nothing can redirect it). This follows the modern seam instead: an optional configDir, as in ClosePreferenceService and seven other persistence services. That is what lets the throttle be tested against a temp directory.
2. Malformed input falls back to ENABLED, not off — the opposite of ClosePreferenceService's Ask. Defaulting to off would quietly close the only channel that tells a user about a fix, and they would never notice.

Details worth recording:

• A future-dated timestamp (clock moved back, or a preference file copied from another machine) is treated as stale rather than trusted — otherwise a bad clock could suppress update checks for years.
• RecordCheck runs only after the calls actually go out, so a failed check does not start the 24h clock and strand the user with stale information for a day.

Docs. Your rationale about README:48-55 was the sharpest point in the issue, and it was right — the paragraph claimed network access happens only "for features you explicitly use" while this check ran unprompted. It now states the version check separately and honestly. ARCHITECTURE gains the service and no longer says the check happens unconditionally "at startup". Also caught while there: SECURITY.md's supported-version table was stale at 1.56.x — updated.

Tests. 20 on the service (defaults, the switch, throttle boundaries at 23/24/72h, the future-dated clock, malformed input, directory isolation) and 5 on the view-model. UpdateService is sealed with no interface so the request itself cannot be counted; the VM tests assert what is observable — with the check off, no version is fetched, no error is shown, and the status explains why. CI went from 4135 to 4165 unit tests, 0 failures.

Also verified the catalog addition does not break ProfileServiceTests' section-count assertion (AvailableSections only includes files that exist), plus a round-trip proving the new section restores.

#1653 stays open — it is disclosure only, explicitly says not to add a consent gate, and depends on a Welcome overlay that does not exist yet.