everything: Update checkver, fix persistence & uninstaller logic#17079
everything: Update checkver, fix persistence & uninstaller logic#17079SorYoshino wants to merge 5 commits intoScoopInstaller:masterfrom
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:
WalkthroughManifest for Everything was restructured: license changed to an object; Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Manifest as Manifest Scripts
participant FS as Filesystem
participant Reg as Registry Scripts
participant Updater as Checkver
User->>Manifest: trigger install
Manifest->>FS: extract files, rename arm64 binary, copy install files
Manifest->>FS: if $persist_dir contains *.ini/*.db... copy persisted files into install dir
Manifest->>Reg: read .reg templates, replace {{everything_dir}}, encode UTF-16LE, write outputs
Manifest->>Updater: use checkver {url, regex} for version checks
User->>Manifest: trigger uninstall
Manifest->>FS: copy *.db/*.csv/*.ico/*.ini/*.txt to persist dir
Manifest->>Reg: import un*.reg via reg.exe to unregister
Manifest->>FS: stop Everything processes/services (PS-version-aware)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@bucket/everything.json`:
- Around line 33-37: The persistence pattern list used in the ForEach-Object
loop is missing the wildcard for text files—'txt' should be '*.txt'—so update
the pattern array/string used with ForEach-Object (the block referencing
$persist_dir, Test-Path and Copy-Item) to include '*.txt' instead of 'txt' in
both occurrences (the loop that iterates over the patterns and the similar block
at lines ~60-64) so Test-Path and Copy-Item will correctly match and persist
.txt files; ensure you update the same pattern source used by the ForEach-Object
invocation that checks Test-Path -PathType Leaf and calls Copy-Item.
- Around line 99-101: The current "checkver" regex only captures the numeric
portion and drops pre-release letters (e.g., `a`/`b`), so update the "regex"
value in the checkver block to capture an optional single-letter pre-release
suffix (or broader letter sequence if needed) appended to the numeric version
(e.g., capture "1.5.0.1400a" not just "1.5.0.1400"); alternatively, if you only
want stable releases, change the logic in the checkver parsing to filter out any
captures that include a trailing letter and only accept pure numeric versions.
Make this change where the "checkver" object and its "regex" value are defined
so version detection preserves or filters pre-release suffixes as required.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bucket/everything.json`:
- Around line 91-95: The removal step currently hardcodes the service name
('Everything') instead of using the discovered $service, which breaks named
instances; change the Remove-Service and the sc.exe delete invocation to use
$service.Name (and guard that $service is non-null) so both branches call
Remove-Service -Name $service.Name (or Start-Process -FilePath 'sc.exe'
-ArgumentList @('delete', $service.Name) ...) rather than the literal
'Everything'.
♻️ Duplicate comments (1)
bucket/everything.json (1)
99-102: Regex doesn't capture pre-release suffixes (alpha/beta).Per Changes.txt format, pre-release versions use suffixes like
1.5.0.1400a(alpha) or1.4.1.867b(beta). The current regex([\\d.]+)only captures the numeric portion, potentially causing version detection issues when pre-release builds are present in the changelog.
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
SorYoshino
force-pushed
the
everything
branch
from
ba82321 to
0ea9221
Compare
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
|
/verify |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bucket/everything.json`:
- Around line 87-95: The service lookup fails on PowerShell 6+ because
Get-Service does not expose BinaryPathName cross-platform; update the branch
that currently uses Get-Service to instead call Get-CimInstance -ClassName
Win32_Service (same as the else branch) and then create the computed property
for BinaryPathName from PathName (or change the subsequent match to use PathName
instead of BinaryPathName) so that $services, $service and the $path_regex match
work consistently across PS versions; ensure you still use $base_service_name
for the initial name filter and preserve -ErrorAction SilentlyContinue.
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
|
The test failure occurs because Since the author may provide the ARM64 hash in |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
bucket/everything.json (3)
109-113:⚠️ Potential issue | 🟠 MajorHardcoded service name may fail for named instances.
The code correctly identifies the Scoop-managed service via path-based filtering into
$service, but then hardcodes'Everything'when removing the service. If a user has a named instance, this will attempt to remove the wrong service or fail.🩹 Proposed fix
" if ($PSVersionTable.PSVersion -ge [version]::new(6, 0)) {", - " Remove-Service -Name 'Everything' -ErrorAction SilentlyContinue", + " Remove-Service -Name $service.Name -ErrorAction SilentlyContinue", " } else {", - " Start-Process -FilePath 'sc.exe' -ArgumentList @('delete', 'Everything') -NoNewWindow -Wait", + " Start-Process -FilePath 'sc.exe' -ArgumentList @('delete', $service.Name) -NoNewWindow -Wait", " }",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/everything.json` around lines 109 - 113, The removal step hardcodes the service name 'Everything' instead of using the discovered $service, so update the code paths that call Remove-Service and the sc.exe delete invocation to use the discovered service's name (e.g., $service.Name) and guard with a null check on $service; specifically replace Remove-Service -Name 'Everything' and the Start-Process sc.exe argument @('delete', 'Everything') with calls that use $service.Name (or appropriate property) and only run when $service is found.
93-100:⚠️ Potential issue | 🟠 MajorService lookup may fail on PowerShell 6+ due to missing BinaryPathName property.
The PS6+ branch uses
Get-Servicewhich doesn't reliably exposeBinaryPathNameon all platforms. The filter at line 100 expects this property in both branches, but it may be$nullon PS6+ cross-platform, preventing service identification.Consider unifying on
Get-CimInstance Win32_Servicefor both branches and filtering onPathNameinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/everything.json` around lines 93 - 100, The PS6+ branch uses Get-Service which may not populate BinaryPathName, causing the downstream filter that assigns $service (the pipeline using $_.BinaryPathName -match $path_regex) to miss matches; change both branches to use Get-CimInstance -ClassName Win32_Service (or otherwise ensure PathName is present) so $services contains the PathName property, and update the filter to use $_.PathName -match $path_regex (keep $base_service_name for the initial filter if desired) so service lookup works consistently across PowerShell versions.
117-120:⚠️ Potential issue | 🟡 MinorRegex doesn't capture pre-release suffixes (alpha/beta versions).
Changes.txt includes versions like
1.5.0.1400a(alpha) or1.4.1.867b(beta). The current regexVersion\\s*([\\d.]+)(?=\\s)captures only the numeric portion, dropping the letter suffix. This may cause version mismatches if pre-release builds are present.🔎 Suggested regex adjustment
- "regex": "Version\\s*([\\d.]+)(?=\\s)" + "regex": "Version\\s*([\\d.]+[a-z]?)(?=\\s)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/everything.json` around lines 117 - 120, The checkver regex currently captures only the numeric portion and drops pre-release suffixes; update the "regex" value in the "checkver" entry so the capture group includes an optional alphabetic suffix (e.g. "a" or "b") by changing Version\\s*([\\d.]+)(?=\\s) to a pattern like Version\\s*([\\d.]+[a-zA-Z]?)(?=\\s) so entries such as 1.5.0.1400a or 1.4.1.867b are fully captured.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bucket/everything.json`:
- Line 52: The $everything_dir assignment uses -replace with an incorrect regex
that matches two backslashes; update the -replace call on the $everything_dir
line so the pattern matches a single literal backslash (use a doubled-escape for
the regex) and the replacement produces two literal backslashes (use a
quadruple-escape in the replacement), keeping the change within the same
$everything_dir = ... -replace expression so Windows paths like C:\Users\... are
correctly transformed for registry files.
---
Duplicate comments:
In `@bucket/everything.json`:
- Around line 109-113: The removal step hardcodes the service name 'Everything'
instead of using the discovered $service, so update the code paths that call
Remove-Service and the sc.exe delete invocation to use the discovered service's
name (e.g., $service.Name) and guard with a null check on $service; specifically
replace Remove-Service -Name 'Everything' and the Start-Process sc.exe argument
@('delete', 'Everything') with calls that use $service.Name (or appropriate
property) and only run when $service is found.
- Around line 93-100: The PS6+ branch uses Get-Service which may not populate
BinaryPathName, causing the downstream filter that assigns $service (the
pipeline using $_.BinaryPathName -match $path_regex) to miss matches; change
both branches to use Get-CimInstance -ClassName Win32_Service (or otherwise
ensure PathName is present) so $services contains the PathName property, and
update the filter to use $_.PathName -match $path_regex (keep $base_service_name
for the initial filter if desired) so service lookup works consistently across
PowerShell versions.
- Around line 117-120: The checkver regex currently captures only the numeric
portion and drops pre-release suffixes; update the "regex" value in the
"checkver" entry so the capture group includes an optional alphabetic suffix
(e.g. "a" or "b") by changing Version\\s*([\\d.]+)(?=\\s) to a pattern like
Version\\s*([\\d.]+[a-zA-Z]?)(?=\\s) so entries such as 1.5.0.1400a or
1.4.1.867b are fully captured.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc6bf8de-21ed-49a1-9b24-9718dda69a82
📒 Files selected for processing (1)
bucket/everything.json
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
2 participants
Summary
Refactors
everythingmanifest to ensure reliable data persistence, enhance uninstaller safety with registry cleanup, and modernize version detection.Related issues or pull requests
Changes
licensefield.Copy-Iteminpre_installandpre_uninstallfor*.ini,*.db,*.csv, etc.post_installto dynamically process registry scripts with correct escaping and encoding (unicode).uninstallerblock for lifecycle compliance.un*.reg) during the uninstall phase.Get-Servicefor PS 6+ andGet-CimInstancefor PS 5.1) with path-based regex validation to ensure only the Scoop-managed service is modified.Changes.txtfor more accurate version tracking.Notes
At present, the only script used to extend.everythingis(un)install-context.reg. However, additional scripts could be added in the future, such as those for startup registration or protocol handling. I will submit a separate PR to add them when I have the time. Therefore, I have proactively updated the related logic to useGet-ChildItemto dynamically retrieve the names of.regfilesTesting
The test results are as follows:
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
New Features
Refactor
Documentation