(TRYING) Added some apps to the JSON and some PS1 files to the private functions - #4963
(TRYING) Added some apps to the JSON and some PS1 files to the private functions#4963WilliamWaldrop wants to merge 43 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
WalkthroughThe change adds four application catalog entries, GitHub release installation, global npm package installation and uninstallation, GitHub uninstall warnings, desktop-user process launching, progress tracking, package logging, and installer metadata validation. ChangesApplication installation workflows
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GetWinUtilSelectedPackages
participant InvokeWPFInstall
participant InstallWinUtilProgramGithub
participant GitHubReleasesAPI
participant InstallWinUtilProgramNpm
participant npm
GetWinUtilSelectedPackages->>InvokeWPFInstall: Pass selected GitHub and npm packages
InvokeWPFInstall->>InstallWinUtilProgramGithub: Install GitHub packages
InstallWinUtilProgramGithub->>GitHubReleasesAPI: Query release metadata
GitHubReleasesAPI-->>InstallWinUtilProgramGithub: Return matching asset
InvokeWPFInstall->>InstallWinUtilProgramNpm: Install global npm packages
InstallWinUtilProgramNpm->>npm: Run global npm command
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 13
🧹 Nitpick comments (8)
functions/private/Install-WinUtilProgramDirect.ps1 (1)
16-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAlso reject non-HTTPS URLs.
The function validates only that
urlis non-empty. A catalog entry with anhttp://URL downloads an executable over an unauthenticated channel and then runs it. Add a scheme check next to the empty check, and reject anything other thanhttps.🤖 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/Install-WinUtilProgramDirect.ps1` around lines 16 - 19, Update the URL validation in Install-WinUtilProgramDirect to reject any non-HTTPS scheme in addition to null or whitespace values. Keep invalid entries on the existing error-and-continue path, and ensure only URLs with an https scheme proceed to download and execute.functions/private/Install-WinUtilProgramGithub.ps1 (3)
62-65: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMatch the MSI branch on the extension, not on a wildcard over the whole path.
$dest -like "*.msi"matches the full path. A file name such astool.msi.exedoes not match, which is correct, but an asset with a query-free name is the only supported case. Prefer[IO.Path]::GetExtension($dest) -eq '.msi'for an explicit extension test.🤖 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/Install-WinUtilProgramGithub.ps1` around lines 62 - 65, Update the MSI condition in the installation flow to use [IO.Path]::GetExtension($dest) and compare it exactly with '.msi', replacing the wildcard path match while leaving the existing Start-Process, logging, and cleanup behavior unchanged.
3-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the fork-specific naming from the doc comment and the User-Agent.
The synopsis describes "Channels DVR community projects", and the User-Agent is
cdvr-winutil. This function is generic GitHub-release installation for WinUtil. Use a WinUtil-specific User-Agent and a generic description so the code matches project naming conventions.♻️ Proposed change
- Downloads and runs the newest matching release asset from a GitHub repo - for - Channels DVR community projects not published to winget/choco. + Downloads and runs the newest matching release asset from a GitHub repo - for + packages that are not published to winget/choco. #> @@ - $headers = @{ "User-Agent" = "cdvr-winutil" } + $headers = @{ "User-Agent" = "winutil" }As per coding guidelines: "Use approved PowerShell verb-noun names and follow existing
WPF/WinUtilnaming conventions."🤖 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/Install-WinUtilProgramGithub.ps1` around lines 3 - 12, Update the documentation synopsis in the GitHub release installer to use a generic WinUtil-oriented description instead of mentioning Channels DVR community projects, and change the $headers User-Agent from “cdvr-winutil” to the approved WinUtil-specific identifier. Keep the function behavior and parameter handling unchanged.Source: Coding guidelines
51-54: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConstrain the download path to a file name.
$asset.namecomes from the GitHub API response. Wrap it with[IO.Path]::GetFileName()beforeJoin-Path, so a name with path separators cannot write outside$env:TEMP.🤖 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/Install-WinUtilProgramGithub.ps1` around lines 51 - 54, Update the destination path construction in the download block to pass $asset.name through [IO.Path]::GetFileName() before Join-Path, ensuring the downloaded file remains directly under $env:TEMP while preserving the existing Invoke-WebRequest flow.config/applications.json (1)
1418-1426: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMatch the file's indentation and remove trailing whitespace.
The other entries in this file use 4-space indentation for keys. This entry uses 2 spaces, and line 1425 has trailing whitespace.
♻️ Proposed formatting fix
"qownnotes": { - "category": "Document", - "choco": "qownnotes", - "content": "QOwnNotes", - "description": "QOwnNotes is a free open-source note taking app with Nextcloud/ownCloud integration.", - "link": "https://www.qownnotes.org/", - "winget": "pbek.QOwnNotes", - "foss": true + "category": "Document", + "choco": "qownnotes", + "content": "QOwnNotes", + "description": "QOwnNotes is a free open-source note taking app with Nextcloud/ownCloud integration.", + "link": "https://www.qownnotes.org/", + "winget": "pbek.QOwnNotes", + "foss": 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 `@config/applications.json` around lines 1418 - 1426, Update the qownnotes entry to use the file’s standard 4-space indentation for all keys and remove the trailing whitespace after the foss value, preserving the existing configuration values and structure.functions/private/Install-WinUtilStreamLinkManager.ps1 (2)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the download URL and the task name into the catalog entry.
The download URL is hardcoded in the function. The other installers in this cohort read
urlfrom the package object. A URL change then requires a code change instead of a JSON change. Read$package.urlwith this value as the fallback.As per coding guidelines: "Keep config-driven features in JSON when they fit the existing schema, and follow required fields and key-renaming rules."
🤖 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/Install-WinUtilStreamLinkManager.ps1` around lines 28 - 30, Update Install-WinUtilStreamLinkManager to read the download URL from the package catalog entry via $package.url, while retaining the current Dropbox URL as its fallback. Move the task name into the catalog entry as well, and use the package-provided value in the installer instead of hardcoding it.Source: Coding guidelines
42-42: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRestrict the process stop to the installed executable.
Get-Process -Name "slm"matches any process namedslm, including one started from another location. Filter by thePathunder$installDirbefore you stop it.🤖 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/Install-WinUtilStreamLinkManager.ps1` at line 42, Update the process termination logic in Install-WinUtilStreamLinkManager to filter Get-Process results by the executable Path under $installDir before invoking Stop-Process, ensuring only the installed slm process is stopped.functions/public/Invoke-WPFInstall.ps1 (1)
146-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the post-install command execution into one helper. The same block appears three times: create a script block from
postInstallCommand, run it, log success, log the error. Each copy repeats the logging text and the error handling, so a change to the contract must be applied in three places.
functions/public/Invoke-WPFInstall.ps1#L146-L155: replace the winget post-install block with a call to a shared helper, for exampleInvoke-WinUtilPostInstallCommand -Name $postInstallName -Command $postInstallCommandById[$r.Program].functions/public/Invoke-WPFInstall.ps1#L177-L186: replace the identical Chocolatey block with the same helper call.functions/private/Install-WinUtilProgramNpm.ps1#L38-L46: replace the npm post-install block with the same helper call, keeping the exit-code condition.As per coding guidelines: "Prefer existing helper functions for WinGet, Chocolatey, registry, services, progress, and UI updates."
🤖 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/public/Invoke-WPFInstall.ps1` around lines 146 - 155, Extract the duplicated post-install execution, success logging, and error handling into a shared Invoke-WinUtilPostInstallCommand helper, then replace the blocks at functions/public/Invoke-WPFInstall.ps1:146-155 (anchor), functions/public/Invoke-WPFInstall.ps1:177-186, and functions/private/Install-WinUtilProgramNpm.ps1:38-46 with helper calls passing the post-install name and command; preserve the npm block’s existing exit-code condition.Source: Coding guidelines
🤖 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 `@config/applications.json`:
- Around line 416-428: Update Get-WinUtilSelectedPackages to create and populate
a Github bucket for packages with installType "github", then update
Invoke-WPFInstall to dispatch that bucket through Install-WinUtilProgramGithub.
Correct the emudeck repo configuration to EmuDeck/emudeck-electron-early while
preserving its existing assetPattern.
In `@functions/private/Install-WinUtilProgramDirect.ps1`:
- Around line 21-23: Update the download-path setup around $ext and $dest to
parse the URL path without its query or fragment before deriving the extension,
preserving the fallback to .exe so the $ext -eq ".msi" handling remains correct.
Sanitize the catalog-derived $name by removing characters invalid in Windows
file names before constructing $dest, while preserving the intended downloaded
filename.
- Around line 45-47: Define the missing Set-WinUtilProcessForeground helper
before its calls in the installer flows, accepting the -Process parameter and
bringing the launched process to the foreground. Ensure both installer functions
can invoke it successfully without triggering the catch path that removes $dest.
In `@functions/private/Install-WinUtilProgramNpm.ps1`:
- Around line 29-32: Wrap the Start-Process invocation in the installer’s
existing error-handling flow with try/catch, and handle a null $process before
accessing ExitCode. Log the npm launch failure, preserve continuation of
remaining install buckets, and only report the exit code when a process object
was returned.
In `@functions/private/Install-WinUtilProgramWinget.ps1`:
- Around line 61-66: Define Start-WinUtilProcessAsStandardUser or replace its
invocation in the winget retry path with an existing process helper that returns
a process result containing ExitCode. Ensure the reassigned $process remains
compatible with the subsequent $process.ExitCode -eq 0 check.
In `@functions/private/Install-WinUtilStreamLinkManager.ps1`:
- Around line 65-66: Update scheduled-task registration in the
Install-WinUtilStreamLinkManager flow to preserve the executable/action quoting
and detect registration failures. Prefer Register-ScheduledTask with explicit
task action and trigger objects; otherwise retain schtasks with correctly
escaped /tr quoting, inspect $LASTEXITCODE, and reject or surface any nonzero
result instead of discarding it.
In `@functions/private/Install-WinUtilWSLCommand.ps1`:
- Around line 56-63: Update the validation branch in Install-WinUtilWSLCommand
to check distro and command independently, rather than combining them before the
uninstall message. Ensure the Uninstall log identifies whether distro or
uninstallCommand is missing, while preserving the existing install error and
continue behavior.
- Around line 79-80: Update the script-name construction in
Install-WinUtilWSLCommand to avoid relying solely on $package.Key and remove the
fork-specific cdvr- prefix. Derive a stable package identifier with an
appropriate fallback for raw catalog entries, then use it with the lowercased
action so distinct WSL-command packages produce distinct temporary paths.
- Around line 86-98: The WSL execution paths call an undefined
Invoke-WinUtilWithTimeout helper. Add this function in the functions sources
with the required timeout, default value, ArgumentList, waiting interval,
OnWaiting callback, and ScriptBlock parameters; invoke the script block with the
supplied ArgumentList so its execution context correctly captures $LASTEXITCODE,
while returning the default value on timeout.
In `@functions/private/Install-WinUtilWSLDistro.ps1`:
- Around line 49-53: Define or restore the missing
Test-WinUtilWSLDistroInstalled and Invoke-WinUtilWithTimeout helpers used by the
WSL installation flow, ensuring both are available in the tracked source and
preserve the existing timeout, default-value, argument-list, and
waiting-callback behavior. Keep Set-WinUtilTweaksProgressIndicator unchanged,
including its dispatcher-based WPF updates.
In `@functions/public/Invoke-WPFInstall.ps1`:
- Around line 196-221: Update the installers invoked by the installBucket
loop—Install-WinUtilProgramDirect, Install-WinUtilProgramGithub,
Install-WinUtilProgramNpm, Install-WinUtilWSLCommand, and
Install-WinUtilStreamLinkManager—to return per-package result objects containing
Program, Success, and ExitCode, matching Install-WinUtilProgramWinget. Capture
each installer’s output in the loop, preserve successful progress accounting,
and add unsuccessful results to $failedPackages so the final status reports
failures from these buckets.
- Around line 262-263: Update the Invoke-WPFRunspace call within
Invoke-WPFInstall so its return value remains suppressed, preventing PowerShell
or job handles from reaching callers through the button handlers. Restore the
existing output suppression behavior without changing the surrounding
installation flow.
- Around line 8-11: In the PackagesToInstall construction, copy each object
retrieved from $sync.configs.applicationsHashtable before adding the Key
property, so Add-Member cannot mutate the shared catalog object. Preserve the
existing Key derivation and ensure downstream PromptValues or other install
state is stored only on the per-run package copy.
---
Nitpick comments:
In `@config/applications.json`:
- Around line 1418-1426: Update the qownnotes entry to use the file’s standard
4-space indentation for all keys and remove the trailing whitespace after the
foss value, preserving the existing configuration values and structure.
In `@functions/private/Install-WinUtilProgramDirect.ps1`:
- Around line 16-19: Update the URL validation in Install-WinUtilProgramDirect
to reject any non-HTTPS scheme in addition to null or whitespace values. Keep
invalid entries on the existing error-and-continue path, and ensure only URLs
with an https scheme proceed to download and execute.
In `@functions/private/Install-WinUtilProgramGithub.ps1`:
- Around line 62-65: Update the MSI condition in the installation flow to use
[IO.Path]::GetExtension($dest) and compare it exactly with '.msi', replacing the
wildcard path match while leaving the existing Start-Process, logging, and
cleanup behavior unchanged.
- Around line 3-12: Update the documentation synopsis in the GitHub release
installer to use a generic WinUtil-oriented description instead of mentioning
Channels DVR community projects, and change the $headers User-Agent from
“cdvr-winutil” to the approved WinUtil-specific identifier. Keep the function
behavior and parameter handling unchanged.
- Around line 51-54: Update the destination path construction in the download
block to pass $asset.name through [IO.Path]::GetFileName() before Join-Path,
ensuring the downloaded file remains directly under $env:TEMP while preserving
the existing Invoke-WebRequest flow.
In `@functions/private/Install-WinUtilStreamLinkManager.ps1`:
- Around line 28-30: Update Install-WinUtilStreamLinkManager to read the
download URL from the package catalog entry via $package.url, while retaining
the current Dropbox URL as its fallback. Move the task name into the catalog
entry as well, and use the package-provided value in the installer instead of
hardcoding it.
- Line 42: Update the process termination logic in
Install-WinUtilStreamLinkManager to filter Get-Process results by the executable
Path under $installDir before invoking Stop-Process, ensuring only the installed
slm process is stopped.
In `@functions/public/Invoke-WPFInstall.ps1`:
- Around line 146-155: Extract the duplicated post-install execution, success
logging, and error handling into a shared Invoke-WinUtilPostInstallCommand
helper, then replace the blocks at
functions/public/Invoke-WPFInstall.ps1:146-155 (anchor),
functions/public/Invoke-WPFInstall.ps1:177-186, and
functions/private/Install-WinUtilProgramNpm.ps1:38-46 with helper calls passing
the post-install name and command; preserve the npm block’s existing exit-code
condition.
🪄 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: 4fad1e4f-4289-4fdf-a815-ccf459e1d43f
📒 Files selected for processing (9)
config/applications.jsonfunctions/private/Install-WinUtilProgramDirect.ps1functions/private/Install-WinUtilProgramGithub.ps1functions/private/Install-WinUtilProgramNpm.ps1functions/private/Install-WinUtilProgramWinget.ps1functions/private/Install-WinUtilStreamLinkManager.ps1functions/private/Install-WinUtilWSLCommand.ps1functions/private/Install-WinUtilWSLDistro.ps1functions/public/Invoke-WPFInstall.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: 88b67ab9b0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Removed detailed comments and exit code handling for context issues.
Refactor package installation logic to simplify package selection and remove unused variables and checks.
WilliamWaldrop
left a comment
WilliamWaldrop
left a comment
There was a problem hiding this comment.
Should be final review
Added support for GitHub installers in Get-WinUtilSelectedPackages function.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Get-WinUtilSelectedPackages.ps1`:
- Around line 48-53: The GitHub package collection built in
Get-WinUtilSelectedPackages is not connected to installation. Update
Invoke-WPFInstall to include the Github collection in totalPackages and invoke
Install-WinUtilProgramGithub with the complete package objects, while preserving
the existing progress and logging updates for Winget and Choco installations.
🪄 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: 052f7e5d-a553-4519-8a94-dc583e8d5401
📒 Files selected for processing (1)
functions/private/Get-WinUtilSelectedPackages.ps1
|
🥳🥳🥳🥳🥳🥳🥳🥳🥳🥳 I did a thing! We now have GitHub plumbing!!!!!!!! I'm so happy now that we can expand the horizons of this Util's applications. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0773d755b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/public/Invoke-WPFInstall.ps1`:
- Around line 36-43: Initialize $completedPackages before the GitHub package
block, then move the Install-WinUtilProgramGithub block into the existing
protected installation section after $sync.ProcessRunning and UI-disabled state
are established. Update the GitHub progress label using the existing
user-feedback pattern before installation, and retain the completed-package
increment for progress calculations.
🪄 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: 64b2608f-2a38-464f-b00c-6e4eff615c8a
📒 Files selected for processing (2)
functions/private/Get-WinUtilSelectedPackages.ps1functions/public/Invoke-WPFInstall.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- functions/private/Get-WinUtilSelectedPackages.ps1
|
Also a better way to handle it is go through the same way that we do with apps: review it via trustworthiness from the repo. The one I added(EmuDeck electron wrapper for windows) is from EmuDeck which is one of the most trusted Emulation apps out there. But I feel if we did add Node and GitHub direct installs we could have better access to certain applications. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3452c4af99
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Refactor test cases for package selection and installation. Improved readability and structure of test cases for better maintainability.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Install-WinUtilProgramNpm.ps1`:
- Around line 78-94: Update the post-install execution in the $Action and
postInstallCommand block to run the dynamically created script block with a
local $ErrorActionPreference of 'Stop', ensuring PowerShell cmdlet failures
reach the existing catch. After invocation, inspect $LASTEXITCODE and throw a
descriptive error when it is nonzero before writing the "$name post-install step
completed" log; preserve the existing error logging and rethrow behavior.
In `@pester/package.Tests.ps1`:
- Around line 250-321: Revert the formatting-only changes in the existing tests
around Test-WinUtilPackageManager and Install-WinUtilProgramWinget, including
the additionally referenced section, while preserving only functional test
changes. Keep unrelated formatting out of this PR.
🪄 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: 23f4cbaf-090d-4f5b-9799-c3ae2bf899df
📒 Files selected for processing (6)
config/applications.jsonfunctions/private/Get-WinUtilPackageLogSummary.ps1functions/private/Install-WinUtilProgramGithub.ps1functions/private/Install-WinUtilProgramNpm.ps1functions/public/Invoke-WPFUnInstall.ps1pester/package.Tests.ps1
🚧 Files skipped from review as they are similar to previous changes (2)
- config/applications.json
- functions/private/Install-WinUtilProgramGithub.ps1
Node packages are too dangerous for general user. Nowadays it is good for devs who know and who trust some packages or frameworks, since they can track CVE's themselves, but giving this out to clueless user is bad decision. About Github releases... That's the point which destroys idea itself. Fraction of apps that are not present in Choco or Winget is that small and representatives of this case are such niche, that having this codebase will only add pain to hold and maintain this code in case something goes off. I can't tell at first any popular app in Windows that is not present in any of these two managers, because even some independent game launchers like Warframe or osu! are present in winget |
I can agree. But there are things such as EmuDeck and Heroic launcher that are on neither. But there are apps that are popular within the Windows community that are only hosted here on GitHub. I can remove NPM if it ACTUALLY becomes a problem. NPM currently isn't that large of an issue. But I can agree with some concerns. |
|
I agree with Chris and Fluffy on this. npm and direct GitHub downloads are not something we should leave in until they cause a problem. The risk is already there. A GitHub repo can look trustworthy and still have a compromised account, release pipeline, or uploaded asset. The same applies to npm packages. The recent Unit 42 report is a good example of why this is not a safe default for WinUtil's general audience. The claim that only There will always be niche, proprietary, or corporate internal software that needs to be installed manually. That is fine. It does not mean WinUtil needs to download and run arbitrary GitHub releases or npm packages. Chris has already said no to both approaches, so I think the npm and GitHub installer work should be removed from this PR. |
|
I think both sides are actually closer than this thread makes it look. The Tailscale restore plus EmuDeck, ES-DE and QOwnNotes are exactly what the linked issue asked for and are basically ready to go. The sticking point is that a second, much bigger feature got bundled in alongside it: making GitHub releases and npm first class install sources. Chris's pushback makes sense from a maintainer seat. Winget and choco give a review layer before something reaches users. A raw GitHub asset download or global npm install skips that, and once merged, every future addition through that path becomes something this repo has to trust and support long term, not just something you added once. Suggestion: split this into two PRs. Ship the four app entries now since nobody is arguing about those. Move Install-WinUtilProgramGithub and Install-WinUtilProgramNpm into a separate PR or discussion where the trust model gets defined properly, the leftover fork-specific naming gets cleaned up, and the still-undefined helper functions get resolved before anything depends on them. That unblocks the apps everyone wants without forcing a decision on installer architecture under time pressure. |
I actually quite like that idea, but I do have a question; how would I be able to add EmuDeck without GitHub plumbing? Like I can't comment it out in JSON and the only way to get it is by either Direct link or GitHub calling. |
Changed the 'foss' attribute for Tailscale from true to false.
I would recommend just nuking that down. I feel that chances of getting more than few apps from GH that is not present in any package manager and will be asked in Winutil are extremely low. |
So nuke the PR? Or just completely remove the GitHub and NPM from the PR? I can do both. |
Removed EmuDeck application configuration from JSON.
|
I'm gonna close this PR and just edit the applications. Toodaloo! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3de230e4e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Set-WinUtilTweaksProgressIndicator -Visible $true -Label "Downloading GitHub release package(s) ($position/$totalPackages)" -Percent $startPercent | ||
| } | ||
|
|
||
| Install-WinUtilProgramGithub -Packages @($packagesGithub) |
There was a problem hiding this comment.
When a selected or future application uses installType: "github", Get-WinUtilSelectedPackages routes it into this branch, but a repo-wide search of the tracked functions/ and scripts/ sources shows no Install-WinUtilProgramGithub definition. That means the newly supported GitHub install path reaches a command-not-found error instead of launching the installer, so add the compiled helper or remove this route until it exists.
Useful? React with 👍 / 👎.
| Set-WinUtilTweaksProgressIndicator -Visible $true -Label "Installing npm package(s) ($position/$totalPackages)" -Percent $startPercent | ||
| } | ||
|
|
||
| Install-WinUtilProgramNpm -Packages @($packagesNpm) |
There was a problem hiding this comment.
When an application entry uses the newly accepted installType: "npm", this branch dispatches to Install-WinUtilProgramNpm, but there is no matching function definition anywhere under the tracked functions/ or scripts/ sources. The first npm-backed app would therefore fail at runtime with command-not-found instead of installing, so include the helper in the compiled sources or keep npm entries out of the shared sorter.
Useful? React with 👍 / 👎.
5 participants
Type of Change
Description
Provides requested apps to the JSON. I tried to add compatibility with more install types(as seen In the new PS1 files: NPM and GitHub are two of the biggest) but it's not exactly the best ever. I will remove them if they become problematic. This now fully allows NPM and GitHub packages to be added into applications. I have added Tailscale, EmuDeck, ES-DE, and Emulationstation. Many thanks to @mewclouds and @ChrisTitusTech for reviewing the code.
Issue related to PR