Add determinate operation progress and ACKit repository workflow - #5390
Cyranth (Cynrath) wants to merge 8 commits into
Conversation
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.
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.
|
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 PRA 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 regressionsThis PR is presented primarily as adding determinate operation progress, but it also changes how WinGet operations are executed. For many operations, the established Among the issues identified during review:
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. |
Summary
Progress feature:
OperationProgress.BytesPerSecond, calculated generically inAbstractOperationfrom cumulative byte samples (EMA alpha=0.3), so WinGet native COM, HTTPDownloadOperationand future byte-counter managers share one mechanism while mappers stay stateless. Formatter appends live· 1,2 MB/s(B/s–GB/s,FormatAsSizeconventions, no ETA); first sample has no speed; stage/unknown/retry transitions reset.ACKit repository integration (same PR):
ackit init(ACKit 0.5.2):ackit.yml, canonicalAGENTS.mdworkflow block,CLAUDE.md/GEMINI.md/copilot shims, 4 builtin skills.AGENTS.md(formatting discipline, Windows guards, localization, task/evidence, git hygiene, CI, completion criteria).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).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.
WinGet Native structured progress
Fallback semantics
Tests
Real commands run (Windows, .NET 10.0.401, x64, tr-TR system locale):
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.UniGetUI.PackageEngine.OperationsandUniGetUI.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 mutatingdotnet formatused).OperationHistorymarker 1 +WinGetManagerTestsUpdateNotApplicable/HashMismatch message assertions 4) and 2 are launcher-asset discovery (OperationCallArgsWiringTestsScoop/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...Downloading · 2% · 1,0 MB / 46,7 MB→ … →Downloading · 100% · 46,7 MB / 46,7 MB· 1,2 MB/s,· 985,8 KB/s,· 775,7 KB/s,· 1,3 MB/s, …,· 915,9 KB/sInstalling...,Installing · 1%, thenFinalizing...,Native WinGet result: Ok, success messagepackage downloadof 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 sameProgressChangedpath; 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.exepayload itself reports FileVersion 6.34.4.6974 — stockwinget upgradeCLI reproduces the identical "Successfully installed" with unchanged version. WinGet/UniGetUI reported the realOkfaithfully.Known limitations
ACKit repository integration
ackit.yml(schemaVersion 1,ackit config checkOK),AGENTS.mdextended plus canonical managed block,CLAUDE.md(@AGENTS.md),GEMINI.md+ copilot shims (allackit syncup-to-date).ackit skills validate0 issues. The historical stricttranslation-source-syncreference tosrc/Languages/lang_en.jsonis fixed (skill-relative../../../src/...), not pre-existing.ackit policy checkOK,ackit task doctorOK (TASK-0001active in-PR with synchronized evidence;docs/taskscommitted, not excluded).--strictpass); changed-scope 92. Baselinedocs/ackit/scan-baseline.jsoncommits the 71 pre-existing findings; fullackit scanshows 74 (the +3 areACKIT070mutable-pin notices on the new workflow itself, kept repo-consistent instead of guessing SHAs).ackit pack --profile codex --max-tokens 50000validated (20 instruction nodes, active task included)..github/workflows/ackit.yml(least privilege, pinnednpx @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.glob(notpath);patternrules are forbidden-only;policy checkchain 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