Skip to content

Add determinate operation progress and ACKit repository workflow - #5390

Closed
Cyranth (Cynrath) wants to merge 8 commits into
Devolutions:mainfrom
Cynrath:feature/operation-progress
Closed

Cyranth (Cynrath) wants to merge 8 commits into
Devolutions:mainfrom
Cynrath:feature/operation-progress

Conversation

@Cynrath

@Cynrath Cyranth (Cynrath) commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor
  • I have read the contributing guidelines, and I agree with the Code of Conduct.
  • Have you checked that there aren't other open pull requests for the same changes?
  • Have you tested that the committed code can be executed without errors?
  • Have you confirmed that this issue is caused by UniGetUI itself, and not by the package manager or the package involved?

Summary

Progress feature:

  • Generic operation progress pipeline added (OperationProgress model, throttled ProgressChanged event, formatter).
  • WinGet Native COM structured progress wired in (InstallPackageAsync / UpgradePackageAsync / UninstallPackageAsync).
  • UI supports determinate progress (IsIndeterminate=false, 0-100 value, stage + downloaded/total bytes).
  • Operation-card mapping extracted to OperationCardProgressState so the ViewModel contract is unit-tested without Avalonia.
  • Measured download throughput: optional OperationProgress.BytesPerSecond, calculated generically in AbstractOperation from cumulative byte samples (EMA alpha=0.3), so WinGet native COM, HTTP DownloadOperation and future byte-counter managers share one mechanism while mappers stay stateless. Formatter appends live · 1,2 MB/s (B/s–GB/s, FormatAsSize conventions, no ETA); first sample has no speed; stage/unknown/retry transitions reset.
  • Fallback behavior preserved: no structured progress means the existing indeterminate animation.

ACKit repository integration (same PR):

  • Real ackit init (ACKit 0.5.2): ackit.yml, canonical AGENTS.md workflow block, CLAUDE.md/GEMINI.md/copilot shims, 4 builtin skills.
  • Extended root AGENTS.md (formatting discipline, Windows guards, localization, task/evidence, git hygiene, CI, completion criteria).
  • 6 custom UniGetUI skills (dotnet-build-test, avalonia-ui, package-manager-integration, winget-native, github-pr-ci, ackit-repo-workflow); 16 skills total, 0 validation issues (strict translation-source-sync ref fixed).
  • Policy pack, committed task/evidence workflow (TASK-0001), scan baseline, readiness gates, provider-aware context pack, docs/ACKIT.md, ACKit CI workflow.

Problem

The operation progress bar only shows the indeterminate animation, so users never see real download progress even when the package manager (WinGet Native COM API) provides byte counts and percentages.

Architecture

WinGet Native structured progress -> generic OperationProgress abstraction -> AbstractOperation.ProgressChanged -> OperationViewModel -> Avalonia ProgressBar.

  • Generic model lives in UniGetUI.PackageEngine.Enums with no manager-specific types; UI never sees COM types.
  • AbstractOperation.ReportProgress is thread-safe, deduplicated, and coalesced (sub-1pct deltas within 200ms); ResetProgress runs on every attempt so retries never show stale values.
  • DownloadOperation reports HTTP byte progress; PackageOperation tries native WinGet COM first under #if WINDOWS, then falls back to CLI.
  • OperationCardProgressState holds the exact card mapping (status plus progress to indeterminate/value/LiveLine); OperationViewModel only copies it onto bindable properties on the UI thread via Dispatcher.UIThread.Post.
  • Existing MainWindow.axaml IsIndeterminate/Value/ItemStatus bindings already support determinate mode; no layout redesign.

WinGet Native structured progress

  • WinGetProgressMapper maps InstallProgress/UninstallProgress states: Queued/Finalizing stay indeterminate, Downloading prefers byte-based percent, Installing/Uninstalling only honor positive percentages, Finished maps to Completed.
  • Percentage is clamped 0-100; NaN/Infinity map to unknown; BytesRequired==0 falls back to reported percent only when positive; BytesDownloaded greater than BytesRequired clamps without losing counters; no synthetic download-to-install total.
  • CanUseNative rejects virtual sources, missing COM backend, non-WinGet managers, custom CLI args, and unelevated admin-required operations.
  • TryApplyInstallOptions mirrors CLI scope/mode/hash/agreements/force/location/arch/version semantics; version pin resolves PackageVersionId from AvailableVersions or falls back to CLI.
  • InterpretInstallResult mirrors CLI success paths and feeds MarkUpgradeAsDoneForNative so [BUG] 4 updates are missing #5042 suppression and [BUG] Update Loop (Unkown) #5158 stuck-loop detection keep working (updates count, installs do not).

Fallback semantics

  • Managers without structured progress behave exactly as before (indeterminate).
  • Unknown install progress falls back to indeterminate; queue/cancel/retry/cancellation semantics unchanged.
  • Native pre-execution misses return null to CLI; mid-execution COM exceptions surface as Failure (no silent double-execution).
  • Broker operations bypass native and keep existing broker behavior.
  • New user-facing strings go through CoreTools.Translate with English fallback; AutomationProperties.ItemStatus binding untouched.

Tests

Real commands run (Windows, .NET 10.0.401, x64, tr-TR system locale):

  • Focused progress suites: dotnet test src/UniGetUI.PackageEngine.Tests --filter OperationProgress|WinGetNativeProgress|OperationCardProgressState|DownloadOperationThroughput /p:Platform=x64: 101/101 passed (portable TFM) + 111/111 passed (windows TFM), 0 failed. Covers first-sample-null, delta math, EMA determinism, zero-time/backward/repeated counters, stage/retry/unknown resets, non-Downloading sanitize, formatter units + NaN/Infinity omit, 8x50 parallel thread-safety, WinGet-mapped downstream speed, and a real throttled-loopback HTTP download test.
  • Builds: UniGetUI.PackageEngine.Operations and UniGetUI.Avalonia (/p:Platform=x64): 0 errors (only pre-existing NU1903/NU1510/CA1822/CA2008/AVLN5001 warnings; CS2012 lock investigated and proven a transient stale-process lock via handle.exe + no UniGetUI/testhost processes + clean rebuild, no repo change needed).
  • dotnet format whitespace src --folder --verify-no-changes: clean. dotnet format style src/UniGetUI.Windows.slnx --no-restore --verify-no-changes: clean (no bare mutating dotnet format used).
  • Full PackageEngine suite: 1595 passed / 7 failed, and all 7 fail identically on the stashed baseline (no WIP changes): 5 are tr-TR locale assertions (OperationHistory marker 1 + WinGetManagerTests UpdateNotApplicable/HashMismatch message assertions 4) and 2 are launcher-asset discovery (OperationCallArgsWiringTests Scoop/PowerShell). No new failures from this change.

Manual runtime verification

VERIFIED with a real local Debug x64 build (UniGetUI.exe --headless, UNIGETUI_WINGET_COM=enabled) driving Free Download Manager over the native WinGet COM path (op 8371548, 46.7 MB):

  • Starting native WinGet upgrade for SoftDeluxe.FreeDownloadManager...
  • Determinate percentage + downloaded/total, live: Downloading · 2% · 1,0 MB / 46,7 MB → … → Downloading · 100% · 46,7 MB / 46,7 MB
  • Real measured throughput, varying live (no synthetic values; first sample correctly has no speed): · 1,2 MB/s, · 985,8 KB/s, · 775,7 KB/s, · 1,3 MB/s, …, · 915,9 KB/s
  • Download→install transition drops the speed: Installing..., Installing · 1%, then Finalizing..., Native WinGet result: Ok, success message
  • HTTP package download of the same installer also completed 0→100% with success and a 46.7 MB file on disk (its speed reaches the GUI card via the same ProgressChanged path; proven at unit level with a throttled-loopback test)

Observation (not a PR defect): the installed FDM version stayed 6.34.4.6974 because the vendor .../6/latest/fdm_x64_setup.exe payload itself reports FileVersion 6.34.4.6974 — stock winget upgrade CLI reproduces the identical "Successfully installed" with unchanged version. WinGet/UniGetUI reported the real Ok faithfully.

Known limitations

  • Install phase with no usable InstallationProgress stays indeterminate (Installing...); no synthetic total.
  • Native path falls back to CLI for COM inactive, virtual sources, custom CLI args, version-lookup miss, unelevated admin-required ops, and broker ops.

ACKit repository integration

  • Actual ACKit init on this branch: ackit.yml (schemaVersion 1, ackit config check OK), AGENTS.md extended plus canonical managed block, CLAUDE.md (@AGENTS.md), GEMINI.md + copilot shims (all ackit sync up-to-date).
  • Skills: 4 builtin + 6 custom UniGetUI + 6 existing translation = 16 total, ackit skills validate 0 issues. The historical strict translation-source-sync reference to src/Languages/lang_en.json is fixed (skill-relative ../../../src/...), not pre-existing.
  • Policy/config/task: ackit policy check OK, ackit task doctor OK (TASK-0001 active in-PR with synchronized evidence; docs/tasks committed, not excluded).
  • Readiness: 89/100 full (--strict pass); changed-scope 92. Baseline docs/ackit/scan-baseline.json commits the 71 pre-existing findings; full ackit scan shows 74 (the +3 are ACKIT070 mutable-pin notices on the new workflow itself, kept repo-consistent instead of guessing SHAs).
  • Context pack: ackit pack --profile codex --max-tokens 50000 validated (20 instruction nodes, active task included).
  • CI: .github/workflows/ackit.yml (least privilege, pinned npx @cynrath/agent-context-kit@0.5.2, gates config/policy/skills/task-doctor/readiness-strict/changed-scan plus baseline SARIF artifact, no duplicate .NET work). Docs: docs/ACKIT.md.
  • Dogfooding notes: presence rules need glob (not path); pattern rules are forbidden-only; policy check chain stays 0 while rule packs evaluate during scan; copilot-shim shadowing and skill-link cycle diagnostics are expected delegation artifacts. No separate ACKit source changes were justified.

Relates to #1164

Add a generic OperationProgress pipeline (model, throttled
ProgressChanged event, formatter) so operation cards switch from
indeterminate to determinate whenever a manager reports real progress.
Wire WinGet native COM install/upgrade/uninstall with InstallProgress
mapping (byte-based percent, clamped, no synthetic totals) and fall
back to the CLI path otherwise. DownloadOperation reports HTTP
progress. Unknown progress keeps the existing indeterminate behavior.
Extract the determinate/indeterminate mapping from OperationViewModel into
OperationCardProgressState so it runs without Avalonia, and keep the ViewModel
as a thin Dispatcher/UI-thread owner that copies the result onto bindable
properties. No visual behavior change.

Add OperationCardProgressStateTests for: running with no/unknown progress,
known download percent plus byte text, unknown fallback, download-to-install
transition, known/unknown install and uninstall, queue/success/fail/cancel
visuals, retry reset without stale leakage, clamping, staged labels, event
wiring via fake operations, and background-thread reporting.

Relates to Devolutions#1164
Real ackit init from clean origin/main: GEMINI and copilot managed shims, 4 builtin skills, ackit.yml (schemaVersion 1), canonical Claude shim. Task TASK-0001.
AGENTS extended with formatting, Windows guards, localization, task/evidence, git hygiene, CI, ACKit workflow, completion criteria. 6 custom skills, translation-source-sync strict fix, policy pack, docs/ACKIT.md, TASK-0001, scan baseline (71 pre-existing).
Least-privilege ACKit job on ACKit surfaces: config, policy, skills, task doctor, readiness strict, changed-files scan gate, full baseline scan with SARIF artifact. Actions follow repo mutable-tag policy (checkout/setup-node/upload-artifact v7); SHAs not guessed. ACKIT070 reports 3 mediums on this file, consistent with all existing workflows.
Final gates, before/after metrics, gate digests, commit SHAs, and remaining push/review steps. Single active checklist item preserved.
TASK-0001 now describes the combined feature/operation-progress scope for PR Devolutions#5390: progress tests present, ACKit files included, strict skill issue fixed, 16 skills clean. chore/ackit-integration retained only as redundant source.
@Cynrath Cyranth (Cynrath) changed the title Show determinate progress for supported package operations Add determinate operation progress and ACKit repository workflow Sep 17, 2026
Adds optional BytesPerSecond to OperationProgress, calculated
generically in AbstractOperation from cumulative byte samples
(EMA alpha=0.3) so WinGet native COM, HTTP DownloadOperation and
future managers share one mechanism while mappers stay stateless.
Formatter appends live B/s-KB/s-MB/s-GB/s; first sample has no
speed; stage/unknown/retry transitions reset. Runtime-verified on
a real 46.7MB native WinGet update.
@GabrielDuf

Copy link
Copy Markdown
Contributor

hi Cyranth (@Cynrath), After reviewing the PR in detail, I’m not going to merge it.

There are two major concerns with this PR.

1. ACKit does not belong in this PR

A significant part of this PR has nothing to do with operation progress. It integrates ACKit, a separate project maintained by you, into the UniGetUI repository.

This includes changes to the repository's CI, agent instructions, configuration, documentation, and development workflow, all specifically to integrate ACKit.

I do not want to introduce ACKit as a dependency of UniGetUI's CI or development workflow. This integration is also unrelated to the purpose of this PR and should not have been included.

2. The progress feature introduces major WinGet regressions

This PR is presented primarily as adding determinate operation progress, but it also changes how WinGet operations are executed.

For many operations, the established winget.exe execution path is replaced with the WinGet COM API. This bypasses behavior and workarounds that have accumulated in the existing implementation.

Among the issues identified during review:

  • Existing AutoRetry paths are bypassed, including elevation, permissions, installer elevation restrictions, version fallback, and architecture/scope retry cases.
  • NoApplicableInstallers sets the architecture/scope fallback state but does not actually perform the expected retry.
  • Existing proxy handling is not carried over to the native path.
  • NoApplicableUpgrade has different success/failure semantics from the existing implementation.
  • Existing reboot-required result handling is not fully preserved.
  • Existing failure interpretation, including not-applicable and installer-hash-mismatch handling, is bypassed.
  • Operation history loses the normal WinGet return code and detailed CLI output, reducing the information available for troubleshooting.
  • Native uninstall does not preserve all existing scope/version behavior.
  • The native path relies on cached COM package information, which introduces additional state/staleness concerns that need to be accounted for.
  • The new execution path does not have sufficient tests for the behavior it replaces. Most of the new tests cover progress plumbing rather than WinGet execution compatibility.
  • Several new translatable strings are missing from the translation source.
  • Progress throttling mixes the injected clock with DateTime.UtcNow, making that behavior inconsistent and not properly covered by the clock-controlled tests.
  • The native logging path can still emit every COM progress callback, effectively bypassing some of the progress throttling.

These aren't minor review comments around the implementation of a progress bar. The PR changes a critical package-operation path and introduces a substantial regression risk for existing WinGet functionality.

Because of both the unrelated ACKit integration and the scope/number of regressions introduced by the WinGet execution changes, I don't think this PR is suitable to continue reviewing in its current form, so I'm closing it.

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