feat: Add advanced dynamic search for package managers - #4955
feat: Add advanced dynamic search for package managers#4955vyas-devgna wants to merge 8 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe search flow queries Chocolatey or Winget asynchronously, merges non-curated results into a dynamic WPF category, generates package links, prevents stale results and duplicates, and refreshes searches when the selected package manager changes. Package manager search
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant InstallTab
participant FindApps as Find-AppsByNameOrDescription
participant PackageSearch as Find-WinUtilPackageManagerApps
participant WPF
User->>InstallTab: select Chocolatey or Winget
InstallTab->>FindApps: refresh current search and category
FindApps->>PackageSearch: query selected manager asynchronously
PackageSearch-->>FindApps: return package Name and Id results
FindApps->>WPF: render deduplicated package entries
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
functions/private/Find-AppsByNameOrDescription.ps1 (2)
329-331: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated stale-search check.
Lines 329 and 331 perform the same comparison with no code between them. Delete one of them.
♻️ Proposed fix
if ($sync.LatestPackageManagerSearch -ne $SearchString) { return } - if ($sync.LatestPackageManagerSearch -ne $SearchString) { return } - if ($null -ne $sync.ItemsControl -and $null -ne $sync.ItemsControl.Dispatcher) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@functions/private/Find-AppsByNameOrDescription.ps1` around lines 329 - 331, Remove the duplicated LatestPackageManagerSearch comparison in the stale-search validation block, keeping a single check that returns when it differs from $SearchString.
55-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe silent guard removes user feedback on invalid state.
The function now returns without any message when
$sync.ItemsControlor the catalog is missing. The project keeps user-feedback patterns for search actions. Add aWrite-DebugorWrite-Warningcall so a failed Install-tab search is diagnosable from the session log.As per coding guidelines: "Preserve existing logging and user-feedback patterns for long-running or destructive operations."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@functions/private/Find-AppsByNameOrDescription.ps1` around lines 55 - 57, Add diagnostic feedback to the early guard in Find-AppsByNameOrDescription by issuing an appropriate Write-Debug or Write-Warning message before returning when sync state, ItemsControl, configs, or applicationsHashtable is missing. Preserve the existing validation and return behavior, and match the function’s established search-action logging style.Source: Coding guidelines
scripts/main.ps1 (1)
137-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated refresh logic.
Both handlers contain the same three lines. Move them into a small helper, for example
Update-WinUtilInstallSearchResults, and call it from each handler. This keeps the two package managers in sync when the refresh condition changes.♻️ Proposed refactor
+function Update-WinUtilInstallSearchResults { + if ($sync.currentTab -eq "Install" -and -not [string]::IsNullOrWhiteSpace($sync.SearchBar.Text)) { + Find-AppsByNameOrDescription -SearchString $sync.SearchBar.Text -Category $sync.SearchBar.Tag + } +} + $sync.ChocoRadioButton.Add_Checked({ $sync.preferences.packagemanager = "Choco" - if ($sync.currentTab -eq "Install" -and -not [string]::IsNullOrWhiteSpace($sync.SearchBar.Text)) { - Find-AppsByNameOrDescription -SearchString $sync.SearchBar.Text -Category $sync.SearchBar.Tag - } + Update-WinUtilInstallSearchResults }) $sync.WingetRadioButton.Add_Checked({ $sync.preferences.packagemanager = "Winget" - if ($sync.currentTab -eq "Install" -and -not [string]::IsNullOrWhiteSpace($sync.SearchBar.Text)) { - Find-AppsByNameOrDescription -SearchString $sync.SearchBar.Text -Category $sync.SearchBar.Tag - } + Update-WinUtilInstallSearchResults })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/main.ps1` around lines 137 - 148, Extract the shared install-search refresh condition and Find-AppsByNameOrDescription call from the ChocoRadioButton and WingetRadioButton handlers into a helper named Update-WinUtilInstallSearchResults, then invoke that helper from both handlers after updating their package manager preference.functions/private/Find-WinUtilPackageManagerApps.ps1 (2)
76-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWinget column split can absorb trailing non-package lines.
Lines after the dash separator can include informational text, for example the truncation notice that winget prints when results exceed the terminal width. Such a line splits into two or more columns and becomes a package entry with an invalid
Id. Add a filter that requires theIdcolumn to contain no whitespace.♻️ Proposed filter
if ($name -and $id) { + if ($id -match '\s') { continue } $results.Add([pscustomobject]@{🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@functions/private/Find-WinUtilPackageManagerApps.ps1` around lines 76 - 93, Update the package-row parsing in Find-WinUtilPackageManagerApps so entries are added only when the trimmed Id contains no whitespace. Apply this validation alongside the existing non-empty Name and Id checks before results.Add, while preserving valid package rows.
40-51: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNative command failures do not throw, so error output can be parsed as packages.
choco search ... 2>&1merges stderr into$out. A non-zero exit code does not raise an exception, so thecatchblock at Line 96 never runs for CLI failures. Any stderr line that contains|becomes a package entry. Check$LASTEXITCODEafter the call and return an empty array when the command fails.♻️ Proposed guard
$out = @(choco search $SearchString --limit-output 2>&1) + if ($LASTEXITCODE -ne 0) { return ,@() } foreach ($line in $out) {The same check applies to the
winget searchcall at Line 61.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@functions/private/Find-WinUtilPackageManagerApps.ps1` around lines 40 - 51, Update the choco search flow in Find-WinUtilPackageManagerApps by checking $LASTEXITCODE immediately after the command and returning an empty array when it is non-zero, before parsing $out. Apply the same failure guard to the winget search call so native command error output is never treated as package data.pester/search-filter.Tests.ps1 (1)
475-488: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe deduplication test can pass without running the package-manager flow.
The assertion only checks that a key is absent. It also passes when
Find-WinUtilPackageManagerAppsis never called, or when the UI update block returns early. AddShould -Invoke Find-WinUtilPackageManagerApps -Times 1and a companion test where a non-curated result creates the dynamic entry. The positive test proves that dynamic entry creation works and gives the negative test meaning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pester/search-filter.Tests.ps1` around lines 475 - 488, Strengthen the deduplication coverage around Find-AppsByNameOrDescription by asserting Find-WinUtilPackageManagerApps is invoked exactly once in the curated-result test. Add a companion test using a non-curated package-manager result and verify its dynamic applicationsHashtable entry is created, so the negative assertion proves deduplication rather than an unexecuted or early-returned flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@functions/private/Find-AppsByNameOrDescription.ps1`:
- Around line 339-341: Update the fallback around $sync.UpdatePackageManagerUI
so background-runspace execution never modifies WPF controls directly. Use
$sync.Form.Dispatcher as the secondary dispatcher when
$sync.ItemsControl.Dispatcher is unavailable, and skip the UI update when
neither dispatcher exists; retain direct invocation only for explicitly
supported test hosts.
- Around line 234-268: Update the dynamic package-manager result rendering
around the $appKey and Initialize-InstallAppEntry logic to remove or otherwise
cap previously generated WPFInstall_dynamic_ entries from
$sync.configs.applicationsHashtable and their corresponding controls in
$pmWrap.Children before adding the new result set. Ensure stale dynamic entries
are no longer visible, selectable, or retained by downstream consumers, while
preserving current rendering and deduplication for the active results.
- Around line 23-30: Update the publisher-derived URL logic in
Find-AppsByNameOrDescription to use ToLowerInvariant() and validate the
publisher token against a valid host-label pattern before interpolation. Fall
back to https://github.com when the token is invalid or too short, and preserve
the existing URL behavior for valid tokens; keep the unrelated source-URL
redesign out of scope.
In `@functions/private/Find-WinUtilPackageManagerApps.ps1`:
- Around line 58-65: Update the Winget search flow around the OutputEncoding
assignment in Find-WinUtilPackageManagerApps so a failed
[Console]::OutputEncoding update is caught locally and does not abort the
search. Preserve the original encoding when available, but continue executing
winget search without changing encoding when the setter throws, while retaining
cleanup of any successfully applied change.
In `@pester/search-filter.Tests.ps1`:
- Around line 357-359: Update Remove-WinUtilSearchGlobals to also remove the
global-scope sync variable, ensuring $global:sync assigned by
New-WinUtilAppSearchContext and New-WinUtilTweakSearchContext is cleared between
tests while preserving the existing script-scope cleanup.
---
Nitpick comments:
In `@functions/private/Find-AppsByNameOrDescription.ps1`:
- Around line 329-331: Remove the duplicated LatestPackageManagerSearch
comparison in the stale-search validation block, keeping a single check that
returns when it differs from $SearchString.
- Around line 55-57: Add diagnostic feedback to the early guard in
Find-AppsByNameOrDescription by issuing an appropriate Write-Debug or
Write-Warning message before returning when sync state, ItemsControl, configs,
or applicationsHashtable is missing. Preserve the existing validation and return
behavior, and match the function’s established search-action logging style.
In `@functions/private/Find-WinUtilPackageManagerApps.ps1`:
- Around line 76-93: Update the package-row parsing in
Find-WinUtilPackageManagerApps so entries are added only when the trimmed Id
contains no whitespace. Apply this validation alongside the existing non-empty
Name and Id checks before results.Add, while preserving valid package rows.
- Around line 40-51: Update the choco search flow in
Find-WinUtilPackageManagerApps by checking $LASTEXITCODE immediately after the
command and returning an empty array when it is non-zero, before parsing $out.
Apply the same failure guard to the winget search call so native command error
output is never treated as package data.
In `@pester/search-filter.Tests.ps1`:
- Around line 475-488: Strengthen the deduplication coverage around
Find-AppsByNameOrDescription by asserting Find-WinUtilPackageManagerApps is
invoked exactly once in the curated-result test. Add a companion test using a
non-curated package-manager result and verify its dynamic applicationsHashtable
entry is created, so the negative assertion proves deduplication rather than an
unexecuted or early-returned flow.
In `@scripts/main.ps1`:
- Around line 137-148: Extract the shared install-search refresh condition and
Find-AppsByNameOrDescription call from the ChocoRadioButton and
WingetRadioButton handlers into a helper named
Update-WinUtilInstallSearchResults, then invoke that helper from both handlers
after updating their package manager preference.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eb5a018a-5205-45dc-8bc3-9f835f410057
📒 Files selected for processing (4)
functions/private/Find-AppsByNameOrDescription.ps1functions/private/Find-WinUtilPackageManagerApps.ps1pester/search-filter.Tests.ps1scripts/main.ps1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4223b4795e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $originalEncoding = [Console]::OutputEncoding | ||
| try { | ||
| [Console]::OutputEncoding = [System.Text.UTF8Encoding]::new() | ||
| $out = @(winget search $SearchString --accept-source-agreements --disable-interactivity 2>&1) |
There was a problem hiding this comment.
On default WinGet installs, winget search can return rows from both winget and msstore sources (Microsoft documents --source as the way to narrow search results). This command neither constrains the source nor parses the Source column, so the dynamic entry later stores only $res.Id; Install-WinUtilProgramWinget then forces --source winget unless the ID is prefixed with msstore:. Store-only results surfaced here will be selectable but fail to install, so either search only --source winget or carry the parsed source through.
Useful? React with 👍 / 👎.
| # 2. Query selected package manager repository for non-curated apps | ||
| if (-not [string]::IsNullOrWhiteSpace($SearchString) -and [string]::IsNullOrWhiteSpace($Category)) { | ||
| $manager = if ($null -ne $sync.preferences -and $null -ne $sync.preferences.packagemanager) { $sync.preferences.packagemanager } else { "Winget" } | ||
| $sync.LatestPackageManagerSearch = $SearchString |
There was a problem hiding this comment.
Because this freshness token is only the search text, an in-flight Winget search for the same text still passes after the user switches to Choco and starts the refreshed search, so whichever CLI finishes last can overwrite the current Package Manager Results with the wrong manager. Clearing the box has the same stale-update path because the empty-search branch returns before changing this value. Track the manager/empty state too, or use a monotonically increasing request id, before allowing the runspace to update the UI.
Useful? React with 👍 / 👎.
|
|
||
| if ($null -eq $ctrl) { | ||
| if (Get-Command Initialize-InstallAppEntry -ErrorAction SilentlyContinue) { | ||
| Initialize-InstallAppEntry -TargetElement $pmWrap -appKey $appKey | Out-Null |
There was a problem hiding this comment.
Initialize-InstallAppEntry returns the checkbox, and the app tile's border click handler looks it up with $sync.$($this.Tag); the normal render path assigns that return value into $sync.$appKey, but dynamic entries discard it here. For Package Manager Results, clicking the card outside the checkbox will therefore dereference $null instead of toggling selection, so store the returned control under the dynamic key just like the curated render path.
Useful? React with 👍 / 👎.
| if ($null -ne $item.PSObject.Properties['Tag'] -and $item.Tag -eq "CategoryContainer_PackageManagerResults") { | ||
| continue |
There was a problem hiding this comment.
If a text search has already populated Package Manager Results, clicking a category chip calls this function with a non-empty $Category; this branch skips the dynamic container and the package-manager query block is also skipped, so stale package-manager cards remain visible alongside the category-filtered curated apps. Collapse or clear the dynamic results when applying category filters so the filter view only shows apps from the requested category.
Useful? React with 👍 / 👎.
|
|
||
| $pmResults = @() | ||
| if (Get-Command Find-WinUtilPackageManagerApps -ErrorAction SilentlyContinue) { | ||
| $pmResults = @(Find-WinUtilPackageManagerApps -SearchString $SearchString -ManagerPreference $Manager) |
There was a problem hiding this comment.
Find-WinUtilPackageManagerApps returns its array with a unary comma, so wrapping the call in @(...) preserves the whole result set as a single array object rather than the individual package objects. The later foreach/indexing then treats all matches—or even an empty result array—as one $res, producing at most one combined/bogus dynamic tile instead of the actual package results. Flatten the returned value or stop suppressing enumeration before deduping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pester/search-filter.Tests.ps1 (2)
509-527: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the link and initialization calls.
The test mocks
Get-WinUtilPackageLinkandInitialize-InstallAppEntry, but it never verifies either call. The test can pass if dynamic entries are created without a package link or initialization.Proposed assertions
Should -Invoke Find-WinUtilPackageManagerApps -Times 1 + Should -Invoke Get-WinUtilPackageLink -Times 1 -Exactly + Should -Invoke Initialize-InstallAppEntry -Times 1 -Exactly $sync.configs.applicationsHashtable.ContainsKey("WPFInstall_dynamic_winget_Some_New_App") | Should -Be $true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pester/search-filter.Tests.ps1` around lines 509 - 527, Add assertions to the “creates dynamic entry for non-curated package manager search results” test verifying Get-WinUtilPackageLink and Initialize-InstallAppEntry are each invoked once with the expected dynamic app data. Keep the existing dynamic-entry and isDynamic assertions unchanged.
493-507: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a Chocolatey case to the deduplication test.
This test only supplies
Browser.Appand checks the Winget-shaped dynamic key. It does not set$sync.preferences.packagemanageror exercise thechoco = "browserapp"fixture. Add an explicit Chocolatey case and assert that its dynamic key is absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pester/search-filter.Tests.ps1` around lines 493 - 507, Extend the “deduplicates package manager search results against curated applications” test to explicitly set $sync.preferences.packagemanager to Chocolatey, exercise the existing choco = "browserapp" fixture through Find-AppsByNameOrDescription, and assert that the corresponding Chocolatey dynamic key is absent, while preserving the existing Winget assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pester/search-filter.Tests.ps1`:
- Around line 509-527: Add assertions to the “creates dynamic entry for
non-curated package manager search results” test verifying
Get-WinUtilPackageLink and Initialize-InstallAppEntry are each invoked once with
the expected dynamic app data. Keep the existing dynamic-entry and isDynamic
assertions unchanged.
- Around line 493-507: Extend the “deduplicates package manager search results
against curated applications” test to explicitly set
$sync.preferences.packagemanager to Chocolatey, exercise the existing choco =
"browserapp" fixture through Find-AppsByNameOrDescription, and assert that the
corresponding Chocolatey dynamic key is absent, while preserving the existing
Winget assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 298a18cc-e87e-4523-be26-8c146934ad5d
📒 Files selected for processing (4)
functions/private/Find-AppsByNameOrDescription.ps1functions/private/Find-WinUtilPackageManagerApps.ps1pester/search-filter.Tests.ps1scripts/main.ps1
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/main.ps1
- functions/private/Find-WinUtilPackageManagerApps.ps1
- functions/private/Find-AppsByNameOrDescription.ps1
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 732acab038
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| foreach ($sk in $staleKeys) { | ||
| $sync.configs.applicationsHashtable.Remove($sk) |
There was a problem hiding this comment.
When a user checks a dynamic Package Manager Results app and then types a new search or switches managers, this pruning deletes the selected app's backing applicationsHashtable entry but never removes its key from $sync.selectedApps or the selected-apps popup. The install path builds packages from selectedApps, so that selection resolves to $null and the workflow can finish without installing the package while the UI still reports it selected; uncheck/remove dynamic selections before deleting their entries, or keep selected dynamic entries until they are explicitly deselected.
Useful? React with 👍 / 👎.
| if ($null -ne $sync -and $null -eq $sync.PackageLinkCache) { | ||
| $sync.PackageLinkCache = [Hashtable]::Synchronized(@{}) |
There was a problem hiding this comment.
This is the first of several new literal $sync.<name> members (PackageLinkCache, LatestPackageManagerSearch, and UpdatePackageManagerUI) added by the patch, but the existing pester/xaml.Tests.ps1 wiring test only permits XAML/generated names or entries in its dynamic-state allow-list. Because these new names are not added there, the CI Pester suite will reject the source before the app can be validated.
Useful? React with 👍 / 👎.
| $publisher = ($PackageId -split '\.')[0] | ||
| if ($publisher.Length -gt 1 -and $publisher -match '^[a-zA-Z0-9\-]+$') { | ||
| $url = "https://$($publisher.ToLowerInvariant()).com" | ||
| } |
There was a problem hiding this comment.
For dynamic package-manager results, this derives $app.link by taking the first package-ID segment and appending .com; the existing app popup opens that link for the Info action. Package-manager IDs are not guaranteed to be website domains, especially Chocolatey slugs, so users can be sent to an unrelated or squatted site for a package that otherwise installs correctly; use real package metadata/official package pages or leave the link unavailable until it is known.
Useful? React with 👍 / 👎.
|
@chatgpt-codex-connector For the third point ('Avoid inventing package website URLs'), this URL generation logic was explicitly requested in the prior review by CodeRabbit to preserve the existing behavior (preserve the existing URL behavior for valid tokens), while falling back to GitHub when the token is invalid. Adding synchronous fetch calls for real metadata would block the UI thread during search, which this PR is specifically designed to fix. Therefore, I will leave this specific part unchanged. |
|
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a69c3b3613
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| winget = if ($Manager -eq "Winget") { $res.Id } else { "na" } | ||
| choco = if ($Manager -eq "Choco") { $res.Id } else { "na" } |
There was a problem hiding this comment.
If a user selects a dynamic Choco result and then switches the radio button back to Winget before pressing Install, this entry only carries winget = "na"; Invoke-WPFInstall later uses the current preference, and Get-WinUtilSelectedPackages reads only the Winget field in that mode, so the selected Choco package is silently skipped. Dynamic results should either install from the manager they came from or prevent cross-manager selections from being carried into a different preference.
Useful? React with 👍 / 👎.
| if (Get-Command Invoke-WPFRunspace -ErrorAction SilentlyContinue) { | ||
| Invoke-WPFRunspace -ParameterList @( | ||
| @("SearchString", $SearchString), | ||
| @("Manager", $manager) | ||
| ) -ScriptBlock { |
There was a problem hiding this comment.
While an install or uninstall is running, the workflow only disables $sync.ItemsControl; the search box and package-manager radio buttons can still call this function. This new branch starts another runspace for every non-empty Install-tab search without checking $sync.ProcessRunning, so typing during an active package operation can run winget search/choco search concurrently with the install/uninstall package-manager process. Short-circuit dynamic repository searches while ProcessRunning or disable those inputs during package operations.
AGENTS.md reference: AGENTS.md:L86-L87
Useful? React with 👍 / 👎.
| if ($null -ne $pmWrap) { $pmWrap.Visibility = [Windows.Visibility]::Visible } | ||
|
|
||
| foreach ($res in $finalResults) { | ||
| $appKey = "WPFInstall_dynamic_$($Manager.ToLower())_$($res.Id -replace '[^a-zA-Z0-9_]', '_')" |
There was a problem hiding this comment.
Because dynamic results use synthetic WPFInstall_dynamic_* keys, selecting one puts that key in $sync.selectedApps, and the existing export path writes selected app keys without package metadata. On a fresh run or config import, applicationsHashtable is rebuilt only from config/applications.json, so the imported dynamic key resolves to $null in the selected-apps popup/install path and the package cannot be installed from the exported config. Exclude these session-only entries from export or persist enough manager/id metadata to reconstruct them.
Useful? React with 👍 / 👎.
6f18aec to
4d455e6
Compare
|
This PR is stacked on top of #4906 to pre-resolve conflicts in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d455e6f35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $item.Visibility = [Windows.Visibility]::Collapsed | ||
| break | ||
| } | ||
| >>>>>>> 5911079 (feat: Add advanced dynamic search for package managers) |
There was a problem hiding this comment.
This unresolved merge marker is left in a function source file, so the compiled WinUtil script will contain >>>>>>> ...; PowerShell treats that as invalid redirection syntax before any UI code can run. Remove the marker and resolve the surrounding branch (there are also markers in the added search-filter test) before shipping.
AGENTS.md reference: AGENTS.md:L103-L105
Useful? React with 👍 / 👎.
|
|
||
| function Update-WinUtilInstallSearchResults { | ||
| if ($sync.currentTab -eq "Install" -and -not [string]::IsNullOrWhiteSpace($sync.SearchBar.Text)) { | ||
| Find-AppsByNameOrDescription -SearchString $sync.SearchBar.Text -Categories $sync.SelectedAppCategories.ToArray() |
There was a problem hiding this comment.
When the user switches the package-manager radio button while an Install search is active, this handler runs, but Find-AppsByNameOrDescription only declares -SearchString and -Category in this commit, and repo-wide search shows no SelectedAppCategories sync state. The event therefore throws before refreshing results, so use the existing SearchBar.Tag/-Category path or implement the plural parameter and state first.
Useful? React with 👍 / 👎.
| "PackageLinkCache", | ||
| "LatestPackageManagerSearch", | ||
| "UpdatePackageManagerUI", | ||
| "MockedTest", |
There was a problem hiding this comment.
This allow-list now includes several new package-manager sync members, but it still omits PackageManagerSearchCache while the new search path references $sync.PackageManagerSearchCache multiple times. The references only XAML, generated, or intentionally dynamic sync members Pester test will still reject the source after the other syntax issues are fixed, so add that cache name to the intentional dynamic state list as well.
Useful? React with 👍 / 👎.
| } catch { | ||
| Write-Progress -Id 2 -ParentId 0 -Activity "Stopping Services" -Status "Failed to stop $svc" -PercentComplete $pct | ||
| Set-WinUtilTaskbaritem -state "Error" -overlay "warning" | ||
| throw "Failed to stop service $svc - cannot continue with Windows Update repair: $_" |
There was a problem hiding this comment.
If stopping a later Windows Update service fails after earlier services such as BITS or wuauserv were already stopped, this new throw exits the repair flow before the startup block runs, leaving those services stopped. Either defer aborting until cleanup/restart has run, or wrap the stop phase in a finally that restores any services already changed.
AGENTS.md reference: AGENTS.md:L84-L87
Useful? React with 👍 / 👎.
|
|
||
| # deduplicate against curated catalog package IDs and app keys | ||
| $curatedIds = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) | ||
| foreach ($key in $sync.configs.applicationsHashtable.Keys) { |
There was a problem hiding this comment.
When the Winget and Choco searches overlap, this background loop can enumerate applicationsHashtable.Keys while the UI-dispatched result handler is adding or removing dynamic entries. A synchronized hashtable does not make enumeration safe during concurrent writes, so one result arriving while the other runspace is deduping can throw and drop that search's results; take a locked/snapshotted copy of the keys before iterating.
Useful? React with 👍 / 👎.
…ce loops - Replace O(N^2) array concatenations with generic lists in GUI item rendering and tweak checks - Convert slow pipeline loops (ForEach-Object) to direct foreach runtime enumeration - Replace wildcard regex matches in app and tweak search with fast string index lookups - Add timeout protection and batch file cleanup in ISO mounting workflows - Prevent file-locking exceptions when logging within an active transcript session - Streamline Windows Update service repair routines and throttle progress updates during DLL reregistration - Ensure command quote resilience in sanity tests when invoking nested Windows PowerShell parsers
- Find-TweaksByNameOrDescription: respect collapsed category state on search reset (mirrors Find-AppsByNameOrDescription); rename $matches to $isMatch to avoid shadowing the PS automatic variable - Invoke-WinUtilCurrentSystem: treat a missing service as a mismatch instead of silently passing validation - Invoke-WinUtilISO: dismount ISO before throwing timeout error to prevent stale mounts; restore per-workdir log file for diagnostics - Test-WinUtilPackageManager: check both managers when both -winget and -choco switches are passed - Invoke-WPFFixesUpdate: restore per-service PercentComplete in the Stop-Service loop; abort on failure instead of silently continuing
Write the modify log to <workDir>.log in %TEMP% instead of a file gated on $sync["Win11ISOWorkDir"], which is only assigned after a successful run. The log now starts at the first line, before the work directory is created, and survives the cleanup that removes that directory, so early failures leave a diagnostic behind. Update the empty-search tweak test for the collapsed-category reset behavior and cover the expanded branch as well.
The reset branch leaves Label.Content alone while the search branch rewrites "+ X" to "- X". Without these assertions a reset that started rewriting the marker would desync the label from its collapsed items and still pass.
4d455e6 to
88333ea
Compare
88333ea to
931fc6f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 931fc6fef2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Find-AppsByNameOrDescription -SearchString "Browser2" | ||
|
|
||
| # Should not create dynamic entry for Browser.App since it's already in applicationsHashtable | ||
| Should -Invoke Find-WinUtilPackageManagerApps -Times 2 |
There was a problem hiding this comment.
This test calls Find-AppsByNameOrDescription twice with two different non-cached search strings, and each call starts both Winget and Choco catalog searches, so Find-WinUtilPackageManagerApps is invoked four times in this mocked synchronous path. Expecting only two invocations makes the Pester suite fail even when the dedupe behavior is correct; either run one search in this test or assert the four expected calls.
Useful? React with 👍 / 👎.
|
Idea... looks promising. I had an idea to just get rid of applications list at all and just make some PS-based alternative of UniGet embedded in WinUtil. @ChrisTitusTech note this down, may be useful for both WinUtil and OneShot |
Thanks, appreciate it. That's an interesting direction , a proper PS-native package layer would be a nice thing to have. Curious to see where it goes |
|
The Idea and implementation look mostly good to me.
Maybe a bit of information on why it was not done like that originally. It was once decided that it was more ideal to "hardcode" applications by design to only offer trusted applicaions. If u watch the PR history applications were removed once a contributor found out about one having malware/trust issues/was hacked/whatever. Issue is maintaining such a list is not easy, but also a reason a lot of people trust this utility. But it is clear that the limited list .. well has limitations. |
Thanks for the context, that history is useful, and I agree the curated list is a big part of why people trust WinUtil. |
|
hmm maybe i did not get the latest version of ur branch, for me results were just mixed with the ones from winget, and either way all had "(Winget)", sorry for this I'mma check thanks for adressing my comment either way! |
I would even call that "First aid toolset" and just leave there really necessary tools like browsers, communication tools, mandatory libraries, most popular apps etc. So it won't be debatable anymore, what to choose and what to remove. And that will draw strict borders for that block
Hehe, #4808 is one of wide examples XD |
|
@FluffyPunk the "First aid toolset" framing is a good one, it turns the curated list from something people argue about into something with an obvious bar. And #4808 is a fair example, that is exactly the maintenance cost you are describing. |
|
I think the dynamic search is useful, but I don’t think it should always mix package-manager results with the curated catalog. For example, searching AnyDesk already returns the curated WinUtil entry, but the dynamic section also shows anydesk, anydesk.install, anydesk.portable, HopToDesk, Splashtop, etc. At that point the extra results are mostly noise and make the curated choice less clear. 
I’d prefer package-manager search to be optional (through a toggle), separate, or only kick in when the curated catalog has no match. That keeps applications.json meaningful while still allowing users to install anything available through Winget/Chocolatey. |
|
I also don’t believe calling the curated catalog a “first aid toolset” really removes the debate around what belongs there. It just moves the debate to what counts as “necessary,” “popular enough,” or worthy of that smaller list, which are still subjective decisions. The more important question is the direction we want for WinUtil. Dynamic package search is useful as an additional capability, but that doesn't necessarily mean shrinking the curated catalog or turning the Install tab into a general package-manager frontend. Personally I’d keep the curated catalog meaningful and use dynamic search as an optional/fallback capability, such as when no curated result exists. Ultimately that direction is up to Chris 😄 |
|
The AnyDesk screenshot is a fair hit, when the curated catalog already answers the query the extra rows are noise and they do make the curated choice less clear. To be clear on intent, this was never meant to replace or shrink the curated catalog. It only adds results for things the catalog does not cover, so the two coexist and the curated list stays the meaningful part. That said, "always on" was the wrong default and your screenshot shows why. Of the three options you listed I lean toward the third, package manager results only kick in when the curated catalog returns no match. It needs no new control, it keeps applications.json as the first answer, and it still lets users reach anything in Winget or Chocolatey when curated comes up empty. One small addition if you think it is worth it, keep the section as a collapsed header with a count when curated did match, for example "+ Package Manager Results (5)", and auto expand it only in the no match case. That reuses the collapse behaviour already in the Install tab, so nothing new to configure, and the results are one click away without competing with the curated entry. Either way the change is small on my side, the package manager search already runs in its own block after curated filtering, so gating it is a condition rather than a rework. Happy to go with plain fallback if the collapsed section feels like extra surface. On the naming point, agreed, that is a subjective line either way. And yes, the broader direction is Chris's call, I am only aiming for this to be an additive capability rather than a shift in what the Install tab is. |
|
Yep, I agree with the direction. Keeping the curated list as the first answer and using dynamic search only as an additive fallback makes much more sense to me. Thanks, and good work! I can see us adding this to the Import/Export workflow and having people customize those even further 😄 |
|
Thanks, will get the fallback gating pushed shortly. Import/Export sounds like a good fit for this too. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49bfed5afa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else { | ||
| if ($null -ne $sync.ItemsControl -and $null -ne $sync.ItemsControl.Items) { | ||
| foreach ($item in $sync.ItemsControl.Items) { | ||
| if ($null -ne $item.PSObject.Properties['Tag'] -and $item.Tag -eq "CategoryContainer_PackageManagerResults") { | ||
| $item.Visibility = [Windows.Visibility]::Collapsed |
There was a problem hiding this comment.
When a user has a text search in progress and then clicks a category chip, Set-WinUtilAppCategoryFilter calls this function with the same $SearchString plus categories, so this else path only collapses the package-results container. It leaves $sync.LatestPackageManagerSearch unchanged, and the runspace guards above compare only that text; the pending Winget/Choco result can therefore pass the guard and call UpdatePackageManagerUI, making Package Manager Results reappear in a category-filtered view. Clear/change the freshness token or include category state/request id in the guard before hiding results here.
Useful? React with 👍 / 👎.
| foreach ($mgr in @("Winget", "Choco")) { | ||
| if (-not $sync.PackageManagerSearchCache.ContainsKey("${SearchString}_${mgr}")) { | ||
| Invoke-WPFRunspace -ParameterList @( |
There was a problem hiding this comment.
If the user types while the Install tab is still lazily rendering app categories, Start-WinUtilInstallAppRendering reapplies the active search after each rendered batch, but this code only checks the completed-result cache. Until the first CLI result is cached, every batch starts another Winget and Choco search for the same text, which can fan out into many concurrent package-manager processes for one query; mark (search, manager) requests as in-flight or otherwise coalesce them before calling Invoke-WPFRunspace.
Useful? React with 👍 / 👎.
4 participants
Type of Change
This PR is stacked on top of #4906 to pre-resolve conflicts in Find-AppsByNameOrDescription.ps1. The diff currently shows changes from both PRs. Once #4906 is merged, the diff here will automatically shrink to show only the package manager search changes, and it can be merged immediately after without conflicts.
Description
This PR introduces an Advanced Dynamic Search for the "Install" tab that allows users to search the active package manager's entire catalog seamlessly from the UI.
Key Features & Improvements:
Invoke-WPFRunspaceto query Winget and Chocolatey in the background. The search executes while typing without stuttering or freezing the main UI thread.applications.jsonare dynamically added to a new "Package Manager Results" category on the fly.Issue related to PR