feat: COM under Native AOT (in-proc server) + Verify/activation fixes + TUI polish#11
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses correctness issues in the WinGet COM backend discovered during a real Windows COM run, and improves the TUI UX to better reflect installed/search state and behave more predictably on narrow/resized terminals.
Changes:
- Fix COM catalog activation by removing an illegal
AcceptSourceAgreementsset on composite catalog references, and improve COM search rows with correlated installed/upgrade signals. - Correct
VerifyInstalledAsyncbehavior to evaluateCheckInstalledStatusper-manifest-installer and choose the best-matching installer’s check set. - TUI polish: installed badge/actions in Search details, responsive column widths on resize, status/result persistence across refresh, and upgrades bulk-select hints; plus updated Windows testing handoff docs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| WINDOWS-TESTING.md | Updates Windows COM testing notes and session findings (includes new session-3 details). |
| HANDOFF.md | Expanded session notes and conclusions from Windows COM verification work. |
| src/Ui.cs | Adds Upgrades-specific status-bar hint pairs for bulk selection/upgrade. |
| src/Models.cs | Adds InstalledVersion to models and merges it from context into detail. |
| src/DetailPanel.cs | Shows an installed badge in Search details and adjusts actions to offer Upgrade/Uninstall when installed. |
| src/ComBackend.cs | Fixes composite-catalog agreement handling; exposes installed/upgrade info in search results; improves verify result selection logic. |
| src/App.cs | Reflows column widths on resize; preserves operation result message through post-op reload; refactors column width pinning logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
harder
changed the title
Owner
Author
|
Addressed the Copilot review feedback in d59ecfa, and merged
Reverified after the merge: both TFMs build clean with TG 2.4.12-develop.7 (no API breaks), unit tests pass, and the AOT build still activates COM in-proc ( |
…ss server The AOT build couldn't activate the out-of-process WinGet COM server (new PackageManager() threw 0x80073D54 APPMODEL_ERROR_NO_PACKAGE): the winrtact.dll manual-activation shim was dropped from ComInterop >= 1.10.x and AOT has no CsWinRT runtime fallback to reach the registered server (JIT did, which is why JIT worked). Switch to the in-process server, which needs neither the OOP server nor package identity, so it activates fine under AOT: - Reference Microsoft.WindowsPackageManager.InProcCom (native-only: ships WindowsPackageManager.dll + Microsoft.Management.Deployment.InProc.dll), with ExcludeAssets=compile so its winmd doesn't clash with ComInterop's managed projection (kept), and NoWarn=NU1701 for the native-package restore tag. - Add app.manifest embedding the InProc package's registration-free WinRT activatableClass entries, so new PackageManager() activates in-proc. Verified on a true AOT image (no coreclr.dll) with Terminal.Gui 2.4.12-develop.7: --comdiag activates (3 catalogs, both MTA threads); --comsmoke shows the full read-only surface working in-proc. Badge reads 'COM · winget 1.29.190-preview' (bundled in-proc engine). Cost: +~7.3 MB beside the ~22.4 MB AOT exe. Bonus: in-proc also sidesteps the OOP server-wedge issue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…h installed-state Bugs latent until COM actually ran (the AOT build used to fall back to CLI): - ConnectAsync: stop re-setting AcceptSourceAgreements on the *composite* catalog reference (throws E_ILLEGAL_STATE_CHANGE). The source refs already accept it in RemoteRefs; this had blocked every COM search/list/detail the moment COM activated. - VerifyInstalledAsync: CheckInstalledStatus returns a status block per *manifest installer*, and the installers that aren't the one installed legitimately report 'ARP entry not found' (0x8A150201). Old code flattened all installers and reported Issues on any failure, so healthy multi-installer/portable packages looked corrupt. Now grouped per installer: Ok when any one installer's checks all pass; the dialog shows that clean installer. A per-check read error records a failing check so an installer can't be reported Ok on incomplete data. - SearchAsync now reads the composite's correlated InstalledVersion so installed search rows can offer Uninstall/Upgrade instead of a bare Install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nce, bulk-select hint
From interactive testing on the COM build:
- Installed-in-Search: an installed search row shows a green '✓ Installed' badge and
Uninstall/Upgrade actions instead of a bare Install (Models gains InstalledVersion).
- Responsive column widths: Name/Id/Version shrink toward minimums on a narrow
terminal so the Available column stays visible; reflows on resize via ViewportChanged.
- The operation result line ('Done', etc.) persists through the post-op list reload
instead of being masked by 'Loading Installed…'.
- Upgrades status bar shows 'Spc Select' / 'U Upgrade sel' to make batch select
discoverable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AOT is now the ship target (COM activates in-proc). Documents the in-proc fix, the Verify/ConnectAsync bug fixes, the read-only COM verification, and the TUI polish. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
harder
force-pushed
the
fix/com-verify-and-tui-polish
branch
from
d59ecfa to
d604cd1
Compare
Comment on lines
+690
to
+696
| List<VerifyCheck> best = perInstaller.OrderBy (cs => cs.Count (c => !c.Ok)).First (); | ||
|
|
||
| return new () { Outcome = outcome, Checks = checks }; | ||
| return new () | ||
| { | ||
| Outcome = best.TrueForAll (c => c.Ok) ? VerifyOutcome.Ok : VerifyOutcome.Issues, | ||
| Checks = best | ||
| }; |
| **Backend badge + version** (`PackageManager.Version`): | ||
|
|
||
| - [ ] The top-right header shows the live backend + winget version, e.g. **`COM · winget 1.x`** (or `CLI · winget 1.x` / `Mock backend`). On the AOT build this is the quickest confirmation of whether COM actually activated or silently fell back to CLI. | ||
| - [x] The top-right header shows the live backend + winget version. *(Session 3: `DescribeAsync` — which produces the badge string — returns **`COM · winget 1.29.250`** on the JIT/COM build and `CLI · winget 1.29.250` on the AOT build (ComBackend's ctor throws → CLI fallback). Confirmed at the data layer; the header render itself wants a quick TUI eyeball but the string is exactly what the badge shows.)* |
Comment on lines
+166
to
+171
| > **Session 3:** the `IProgress<OpProgress>` callback path **works on COM (JIT)** — `DownloadAsync(zoxide)` | ||
| > delivered progress samples (phase **Downloading**, fraction → 1.00) with no crash, so the managed→native | ||
| > progress delivery is sound. The original "**under AOT**, the one CCW-callback unknown" framing is now | ||
| > **moot**: COM doesn't activate under AOT at all (see the resolved banner), so progress under AOT never runs; | ||
| > under JIT (the COM ship vehicle) the CCW path is standard. Install/uninstall progress *rendering* in the | ||
| > status bar still wants an interactive recheck. |
Comment on lines
+84
to
+85
| NOTE: the "live progress under **AOT**" CCW-marshaling item is **moot** — COM doesn't run under AOT; under JIT | ||
| (the COM ship vehicle) the callback path is standard and was confirmed via `DownloadAsync`. |
| - [ ] **Same-id-across-catalogs** (rare): if a package id exists in multiple sources, operations resolve the first match. Only worth checking if you hit an odd case. | ||
| - [ ] **CLI-backend cancel** (`--cli`, then Esc mid-install): confirm it stops watching but does **not** kill `winget.exe` (the install continues) — documented, lower priority. | ||
| - [ ] **Measure the AOT binary size** of the COM (Windows) build and compare to the CLI/mock build, to budget the COM backend's cost. *(Open spike question.)* | ||
| - [x] **Measure the AOT binary size** of the COM (Windows) build and compare to the CLI/mock build, to budget the COM backend's cost. *(Session 3: **AOT win-x64 single exe = 22.4 MB** (no `coreclr.dll`). The **JIT self-contained** alternative — the recommended COM ship vehicle since AOT can't activate COM — is **112.7 MB** for the whole folder (~5× the AOT exe, main exe only 0.2 MB + the shared CoreCLR/framework). So keeping COM costs the AOT size advantage regardless of which non-AOT form is chosen.)* |
- VerifyInstalledAsync: when the best installer's failure is an unreadable entry (a
read-error sentinel), report VerifyOutcome.Error ('couldn't verify') instead of
Issues ('may be corrupt'). Tracked via a per-installer ReadError flag rather than
string-matching the sentinel label. Happy path unchanged (clean → Ok, real failures
→ Issues).
- Docs: remove stale 'COM doesn't activate under AOT / JIT is the ship vehicle' claims
that contradicted the resolved in-proc finding — the backend badge line, the
live-progress blockquote, the AOT size-measurement note (WINDOWS-TESTING.md), and the
'still not verified' note (HANDOFF.md) now reflect that COM runs under AOT in-proc and
AOT is the ship target.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Addressed the latest Copilot review in feb0673. I verified each comment against the current tree first: Valid → fixed (5):
Already resolved in the current tree (4 — stale re-posts after the force-push), verified, no action:
Verification: both TFMs build clean, unit tests pass. The Verify change is happy-path-identical (the new |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Headline: COM now works under Native AOT 🎉
The Native-AOT build previously could not activate the WinGet COM backend (
new PackageManager()threw0x80073D54APPMODEL_ERROR_NO_PACKAGE) and silently fell back to CLI — the long-standing blocker. Fixed by switching to the in-process WinGet server:Microsoft.WindowsPackageManager.InProcCom(native-only:WindowsPackageManager.dll+Microsoft.Management.Deployment.InProc.dll),ExcludeAssets="compile"so its winmd doesn't clash with ComInterop's managed projection.app.manifestwith the package's registration-free WinRTactivatableClassentries, routingnew PackageManager()to the in-proc server (needs no OOP server, no package identity → activates under AOT).Root cause: the
winrtact.dllmanual-activation shim was dropped from ComInterop ≥ 1.10.x (winget-cli#5459, #4839); AOT has no CsWinRT runtime fallback to reach the registered OOP server (JIT did).Verified on a true AOT image (no
coreclr.dll):--comdiagactivates (3 catalogs, both MTA threads); full read-only surface works in-proc — search, installed (299), upgrades, versions (113), installer preview, COM detail (Tags/Support/Docs), Verify. Badge readsCOM · winget 1.29.190-preview. Bonus: in-proc also avoids the OOP server-wedge issue. Cost: +~7.3 MB beside the ~22.4 MB AOT exe (vs the ~112 MB JIT folder that was the fallback plan).COM backend bug fixes (latent until COM actually ran)
ConnectAsynccomposite agreement — stopped re-settingAcceptSourceAgreementson the composite reference (throwsE_ILLEGAL_STATE_CHANGE); blocked every COM search/list/detail.VerifyInstalledAsyncfalse "Issues" —CheckInstalledStatusreturns a block per manifest installer; non-installed installers report "ARP entry not found" (0x8A150201). Now grouped per installer: Ok when any one installer's checks all pass. zoxide/PowerShell/7-Zip now report Ok.TUI polish (from interactive testing)
✓ Installedbadge + Uninstall/Upgrade actions instead of a bare Install.Spc Select/U Upgrade sel.Verification
dotnet test -f net10.0).--comdiag/--comsmoke).Docs (
HANDOFF.md,WINDOWS-TESTING.md) updated: AOT is now the ship target.🤖 Generated with Claude Code