Skip to content

Show determinate download progress without changing WinGet execution - #5402

Merged
Gabriel Dufresne (GabrielDuf) merged 6 commits into
Devolutions:mainfrom
Cynrath:feature/operation-progress-v2
Sep 21, 2026
Merged

Gabriel Dufresne (GabrielDuf) merged 6 commits into
Devolutions:mainfrom
Cynrath:feature/operation-progress-v2

Conversation

@Cynrath

@Cynrath Cyranth (Cynrath) commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

This is a clean reimplementation following the feedback on #5390. It keeps the existing WinGet CLI execution path intact and limits the change to observational progress reporting.

  • winget.exe remains the execution path; no WinGet COM install/update/uninstall execution was added.
  • Existing retry/proxy/elevation/error/history behavior is preserved (see table).
  • Progress reporting is observational only: a side-car event that never logs and never touches execution, return codes, or history. Subscriber exceptions are isolated per-subscriber and cannot fail the operation.
  • Unknown progress falls back to indeterminate UI; no fake percentages or speeds. Stale speeds expire after 2 s instead of displaying indefinitely.
  • No ACKit integration is included.

What it does

Operations with real byte counters (the existing HTTP installer-download path, which already reads Content-Length and cumulative bytes) now surface determinate cards, e.g. Downloading · 21% · 10.0 MB / 46.7 MB · 1.2 MB/s, with speed measured generically as Δbytes/Δtime on one injectable monotonic clock (Stopwatch.GetTimestamp / GetElapsedTime). Reports are gated by the existing integer-percent change (max ~101 determinate reports per download, 0→100 preserved), so UI events no longer track socket reads 1:1. Role stages (Installing, Updating, Uninstalling via translatable {0}... template) render indeterminate while running.

Raw capture (winget v1.29.290, winget download with stdout redirected) showed zero progress frames: six plain CR LF lines over an ~8.5 s download, empty stderr, no ANSI, no counters. Piped winget.exe therefore stays indeterminate by design — there is nothing reliable to parse, and no parser was added.

Regression evidence

All rows: existing path structurally unchanged (git diff touches none of these files).

Concern Preserved? Evidence
AutoRetry Yes — untouched _runOperation loop unchanged
Elevation retry Yes — untouched PrepareProcessStartInfo, ApplyElevationRequirements unchanged
Permissions retry Yes — untouched WinGetPkgOperationHelper._getOperationResult unchanged
Installer elevation restriction Yes — untouched same file unchanged
Version fallback Yes — untouched --version construction unchanged
Arch/scope retry Yes — untouched WinGet_DropArchAndScope path unchanged
NoApplicableInstallers retry Yes — untouched same file unchanged
Proxy Yes — untouched WinGet.GetProxyArgument unchanged
NoApplicableUpgrade semantics Yes — untouched same result function unchanged
Reboot-required handling Yes — untouched 0x8A150109 branch unchanged
Not-applicable handling Yes — untouched same file unchanged
Hash mismatch Yes — untouched 0x8A150011 branch unchanged
Return code Yes — untouched LastReturnCode path unchanged
Detailed CLI output/history Yes — untouched Line/GetOutput/history store unchanged (no invented progress; piped-WinGet observation kept as prose only)
Uninstall scope/version Yes — untouched same file unchanged
Cached COM state None added no COM execution references added

Checks

  • dotnet build src/UniGetUI.Windows.slnx /p:Platform=x64: 0 errors
  • New focused tests: 37/37 pass (both TFMs: net10.0 and net10.0-windows10.0.26100.0)
  • Full PackageEngine.Tests: only pre-existing environment failures, each reproduced on clean origin/main (OperationHistory tail-marker, WinGet explainers, PowerShell launcher, Scoop/PS call-args wiring)
  • dotnet format whitespace/style --verify-no-changes: clean
  • Verify-Translations.ps1: clean (no placeholder issues); English sources use positional templates ({0} · {1}% · {2} / {3}[ · {4}], {0}...) so translators control ordering/separators; stage keys reduced to Downloading/Installing/Updating/Uninstalling (no punctuation-duplicate ... keys)
  • Runtime: in-memory DownloadOperation coverage plus real-clock manual verification of 0→100 determinate flow; history record intact

Introduce OperationProgress (stage, nullable percentage, byte counters,
measured throughput) and side-car ReportProgress/ResetProgress plumbing on
AbstractOperation with a single injected clock. Reporting never logs and
never touches execution, retries, or history.
Surface real byte counters from the existing HTTP download path as
determinate card progress (percent, downloaded/total, measured MB/s) and
report role stage markers otherwise. WinGet CLI execution, retries,
elevation, proxy, return codes, and history behavior are unchanged;
piped winget.exe emits no progress frames, so those cards stay
indeterminate.
…egression

Model and single-clock throughput rules, card mapping, loopback-server
measured-speed test, and real captured winget download output proving
history preservation with no invented progress.
… card

The ProgressIndicator gate used card indeterminacy, which is also false
while queued or before any report arrives. That hid status/queue lines
and left verbose OperationInformation text stuck on the card. Gate only
while a determinate report is active; clear ownership when leaving
Running.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I left nine comments on the pr. if you could take a look at them, it would be appreciated.

Comment thread src/UniGetUI.PackageEngine.Operations/DownloadOperation.cs Outdated
Comment thread src/UniGetUI.PackageEngine.Enums/OperationProgress.cs Outdated
Comment thread src/UniGetUI.PackageEngine.Tests/WingetCliOutputProgressRegressionTests.cs Outdated
Comment thread src/UniGetUI.Avalonia/ViewModels/DialogPages/OperationViewModel.cs Outdated
Comment thread src/UniGetUI.PackageEngine.Operations/AbstractOperation_Progress.cs Outdated
Comment thread src/UniGetUI.PackageEngine.Operations/AbstractOperation_Progress.cs
Comment thread src/UniGetUI.PackageEngine.Operations/OperationProgressFormatter.cs Outdated
Comment thread src/UniGetUI.PackageEngine.Tests/OperationProgressTests.cs
Comment thread src/UniGetUI.PackageEngine.Operations/AbstractOperation_Progress.cs Outdated
- Throttle DownloadOperation progress to integer-percent gate (bounded UI events, 0/100 preserved)
- Remove dead FromInstall/FromUpdate/FromUninstall/FromStagePercent and ResetProgress
- Delete WingetCliOutputProgressRegressionTests (fixture-only)
- Fix OperationViewModel constructor determinate init via OperationCardController
- Add direct controller state-machine tests (construct, failure order, retry, stage)
- Switch throughput to monotonic Stopwatch timestamps (injectable)
- Add stale-speed expiry (2s, timer only while fresh, disposed cleanly)
- Isolate ProgressChanged subscribers (observational, per-subscriber try/catch)
- Localize full progress templates, drop duplicate ellipsis keys and TB branch
- Shrink suite to focused coverage (37 tests, both TFMs)
@Cynrath

Copy link
Copy Markdown
Contributor Author

Hi Gabriel Dufresne (@GabrielDuf), I’ve addressed all nine review comments in ac9284f. I also replied to each thread with the corresponding change and verification. The PR is ready for another review when you have time. Thanks for the detailed feedback.

@Cynrath

Copy link
Copy Markdown
Contributor Author

Updated the branch to the latest \main\ (\1f832f05) and resolved the translation conflict while preserving the reviewed progress changes. I reran the focused progress tests, build, formatting, and translation validation on the merged tree.

  • Focused progress tests: 37 passed on each TFM (
    et10.0\ +
    et10.0-windows10.0.26100.0), 0 failed
  • \Verify-Translations.ps1: passed (60 files, no placeholder issues)
  • \dotnet build UniGetUI.Windows.slnx /p:Platform=x64: 0 errors
  • \dotnet format\ whitespace + style verify: clean
  • Full-suite failures on the merged tree reproduce identically on clean \origin/main, so they are pre-existing and unrelated to this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@GabrielDuf
Gabriel Dufresne (GabrielDuf) merged commit 8220a4b into Devolutions:main Sep 21, 2026
6 checks passed
@Cynrath
Cyranth (Cynrath) deleted the feature/operation-progress-v2 branch September 21, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants