chore: Relocate process termination logic to uninstaller script for safety#17051
Open
SorYoshino wants to merge 5 commits intoScoopInstaller:masterfrom
Open
chore: Relocate process termination logic to uninstaller script for safety#17051SorYoshino wants to merge 5 commits intoScoopInstaller:masterfrom
SorYoshino wants to merge 5 commits intoScoopInstaller:masterfrom
Conversation
WalkthroughRefactors uninstall handling across bucket manifests: moves or converts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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. 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 |
Contributor
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with apifox
attribute-changer
audioswitcher
buttercup
coretemp
fastcopy
folder-marker
iconview
jackett
mellowplayer
mobaxterm
mp3tag
openvpn
playnite
powertoys
protonmail-bridge
smarttaskbar
stremio
tailscale
teamviewer
workspacer
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@bucket/fastcopy.json`:
- Around line 35-40: The uninstaller script calls Start-Process on
"$dir\\setup.exe" without checking that the file exists, which can cause the
uninstall to abort if setup.exe is missing; update the "uninstaller" script to
guard the Start-Process invocation (the line with Start-Process
"$dir\\setup.exe" -Args @('/SILENT', '/r') -Wait) by first checking Test-Path
"$dir\\setup.exe" (or by adding -ErrorAction SilentlyContinue) so the script
skips Start-Process when the executable is absent and continues with
Stop-Process and Start-Sleep reliably.
In `@bucket/jackett.json`:
- Around line 17-19: The process names used with Stop-Process are misspelled
(single 't') so they won't match running Jackett processes; update the array
passed to ForEach-Object to use the correct executable base names 'JackettTray',
'JackettConsole', and 'JackettService' (so Stop-Process -Name $_ -Force
-ErrorAction SilentlyContinue targets the real processes) and ensure the
ForEach-Object block wrapping Stop-Process remains unchanged.
In `@bucket/stremio.json`:
- Around line 32-37: The Stop-Process call in the uninstaller script uses the
process name with the .exe extension which won't match PowerShell's -Name
filter; update the uninstaller "script" entry that calls Stop-Process (the line
containing Stop-Process -Name 'stremio.exe' -Force -ErrorAction
SilentlyContinue) to use the base process name 'stremio' (remove the .exe) so
the process is correctly matched and terminated during uninstall.
🧹 Nitpick comments (4)
bucket/mp3tag.json (1)
65-69: Consider gating Explorer termination on shell-extension presence.
Right now Explorer is stopped unconditionally, even if the user never registered the shell extension. A small guard avoids unnecessary Explorer restarts.💡 Suggested tweak
"uninstaller": { "script": [ - "Stop-Process -Name 'explorer' -Force -ErrorAction SilentlyContinue", - "Start-Sleep -Milliseconds 1500" + "if (Test-Path 'HKLM:\\SOFTWARE\\Classes\\Directory\\shellex\\ContextMenuHandlers\\Mp3tagShell\\') {", + " Stop-Process -Name 'explorer' -Force -ErrorAction SilentlyContinue", + " Start-Sleep -Milliseconds 1500", + "}" ] }bucket/tailscale.json (1)
52-58: Consider a short delay after stopping service/process.
Other manifests add a brief sleep to avoid handle timing issues. Optional, but would align with the PR’s safety intent.💡 Suggested tweak
"uninstaller": { "script": [ "Stop-Service -Name 'Tailscale' -Force -ErrorAction SilentlyContinue", "Stop-Process -Name 'tailscale-ipn' -Force -ErrorAction SilentlyContinue", + "Start-Sleep -Milliseconds 1500", "if ($cmd -eq 'uninstall') { reg import \"$dir\\remove-startup.reg\" }", "tailscaled.exe uninstall-system-daemon" ] }bucket/smarttaskbar.json (1)
43-47: Avoid Invoke-Expression; call the script directly.Invoke-Expression evaluates a constructed string; the call operator safely executes a known path without eval risks.
♻️ Proposed change
- "Invoke-Expression -Command \"& `\"$dir\\remove-smarttaskbar-startup.ps1`\"\"", + "& \"$dir\\remove-smarttaskbar-startup.ps1\"",bucket/coretemp.json (1)
45-47: Add -Force for consistent termination.This matches the PR-wide standardization and reduces file-lock risk.
♻️ Proposed change
- "Stop-Process -Name 'Core Temp' -ErrorAction SilentlyContinue", + "Stop-Process -Name 'Core Temp' -Force -ErrorAction SilentlyContinue",
Contributor
Author
|
/verify |
Contributor
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with apifox
attribute-changer
audioswitcher
buttercup
coretemp
fastcopy
folder-marker
iconview
jackett
mellowplayer
mobaxterm
mp3tag
openvpn
playnite
powertoys
protonmail-bridge
smarttaskbar
stremio
tailscale
teamviewer
workspacer
|
2 tasks
SorYoshino
force-pushed
the
process-kill
branch
from
a9940d9 to
6d0cb54
Compare
Contributor
Author
|
/verify |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@bucket/coretemp.json`:
- Around line 43-48: Update the uninstaller script so Stop-Process uses the
standardized termination flags; locate the "uninstaller" -> "script" block in
coretemp.json and modify the Stop-Process command (the line containing
"Stop-Process -Name 'Core Temp'") to include "-Force -ErrorAction
SilentlyContinue" to match other manifests (e.g., smarttaskbar.json,
powertoys.json).
In `@bucket/mp3tag.json`:
- Around line 65-70: The uninstaller currently forces termination of
explorer.exe unconditionally in the uninstaller.script; change this to first
test whether the shell extension was actually registered (reuse the same
registry-check logic used in pre_uninstall) and only call Stop-Process -Name
'explorer' when that registry key exists. Update uninstaller.script to run a
conditional PowerShell registry test (matching the key checked in pre_uninstall)
and wrap the Start-Sleep/Stop-Process calls inside that conditional so explorer
is only killed when the extension was installed.
♻️ Duplicate comments (1)
bucket/fastcopy.json (1)
35-41: Uninstaller block aligns with PR objectives.The structure correctly:
- Runs the silent uninstaller first
- Terminates
explorerto release shell extension handles- Sleeps to allow file handles to close before Scoop deletes directories
The guard for
setup.exeexistence was already flagged in a prior review—please address that feedback to prevent uninstall failures if the executable is missing.,
🧹 Nitpick comments (1)
bucket/tailscale.json (1)
52-58: Consider a short post-termination sleep for handle release.
This matches the PR’s reliability goal for unattended uninstall flows.♻️ Proposed tweak
"script": [ "Stop-Service -Name 'Tailscale' -Force -ErrorAction SilentlyContinue", "Stop-Process -Name 'tailscale-ipn' -Force -ErrorAction SilentlyContinue", + "Start-Sleep -Milliseconds 1500", "if ($cmd -eq 'uninstall') { reg import \"$dir\\remove-startup.reg\" }", "tailscaled.exe uninstall-system-daemon" ]
Contributor
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with apifox
attribute-changer
audioswitcher
buttercup
coretemp
fastcopy
folder-marker
iconview
jackett
mellowplayer
mobaxterm
mp3tag
openvpn
playnite
powertoys
protonmail-bridge
smarttaskbar
stremio
tailscale
teamviewer
workspacer
|
2 tasks
* Standardize Stop-Process usage with -Force and SilentlyContinue * Migrate termination logic to uninstaller.script with Start-Sleep delays * Automate Explorer termination for shell extensions and remove related notes * Optimize field ordering in multiple manifests
SorYoshino
force-pushed
the
process-kill
branch
from
6d0cb54 to
ab796fb
Compare
Contributor
Author
|
/verify |
Contributor
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with apifox
attribute-changer
audioswitcher
buttercup
coretemp
fastcopy
folder-marker
iconview
jackett
mellowplayer
mobaxterm
mp3tag
openvpn
playnite
protonmail-bridge
smarttaskbar
stremio
tailscale
teamviewer
workspacer
|
This was referenced Jan 23, 2026
Contributor
Author
|
/verify |
Contributor
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with apifox
attribute-changer
audioswitcher
buttercup
coretemp
fastcopy
folder-marker
iconview
jackett
mellowplayer
mobaxterm
mp3tag
openvpn
playnite
protonmail-bridge
smarttaskbar
stremio
tailscale
teamviewer
workspacer
|
Contributor
Author
|
The test failure appears to be caused by network-related issues and is unrelated to the changes made in this PR. |
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
1 participant
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.
Summary
Refactors multiple manifests to align with Scoop's uninstallation lifecycle. By relocating process and service termination logic from
pre_uninstallto theuninstallerscript block, this change ensures that Scoop's built-in running process checks are respected, preventing accidental data loss while ensuring reliability for unattended operations.Changes
Stop-ProcessandStop-Servicecalls frompre_uninstalltouninstaller.script. This ensures Scoop can prompt the user if an application is active before any termination occurs, preventing silent data loss.-Force -ErrorAction SilentlyContinue.Start-Sleep -Milliseconds 1500after termination to ensure file handles are fully released before the directory is deleted by Scoop.ignore_running_processesenabled.exploreranddllhosttermination into the uninstaller script for manifests with shell integrations (e.g.,fastcopy,mp3tag,powertoys,smarttaskbar,attribute-changer). This removes the need for manual intervention previously mentioned innotes.Notes
powertoyseverythingsimple-dnscryptfilezilla-servermanifests go beyond the scope of this PR, I will submit separate PRs to address those issues when time permits.<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.