Repository navigation
fix(desktop): Windows updates no longer fail when one app process is left running - #25831
juliusmarminge wants to merge 2 commits into
Conversation
…ore updating On Windows PowerShell 5.1, `.Count` on a single CimInstance is empty, so electron-builder's FIND_PROCESS reported "not running" when exactly one T3 Code process was left in the install folder. KILL_PROCESS was skipped and the old uninstaller failed with "Failed to uninstall old application files ...: 2". Extend the existing app-builder-lib@26.16.1 patch to wrap the pipeline in `@(...)`, the fix proposed in electron-userland/electron-builder#10454. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When an upgrade runs the old uninstaller in place from the install directory, the PowerShell process check now finds the uninstaller itself and Stop-Process kills it. Exclude the installer/uninstaller's own PID in both PowerShell filters, as the taskkill path already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This localized Windows installer fix corrects single-process detection in PowerShell 5.1 and prevents the installer from terminating itself. The accompanying lockfile edits only update the dependency patch hash. You can add or adjust custom eligibility rules. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Use UninstallerReader on Linux patches/app-builder-lib@26.16.1.patch |
Linux NSIS builds now use UninstallerReader to extract the uninstaller, as macOS Catalina builds do. |
PowerShell process handling
| Layer / File(s) | Summary |
|---|---|
Detect and terminate app processes patches/app-builder-lib@26.16.1.patch |
The PowerShell process check wraps results in an array before checking .Count. Process detection and termination exclude the current process by PID. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- pingdotgg/t3code#23965: Changes updater-related quit behavior, a different stage of the Windows update flow.
Suggested reviewers: jahands
Merge Risk: 
2f9ec
-
No actionable merge-blocking risk remains in the reviewed changes.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check Issue #25450has two coding objectives. The patch implements the installer-side objective:FIND_PROCESSuses@(...), and both process commands exclude the installer or uninstaller PID. The reporte…Investigate and fix the Windows main-process teardown hang described in #25450. Add automated coverage where feasible.Out of Scope Changes check The whole-PR summary reports a Linux NSIS change that makes builds use UninstallerReader. Issue#25450concerns Windows NSIS process detection and termination. The reportedUninstallerReaderchang…Remove the unrelated UninstallerReaderchange from this pull request, or provide concrete evidence that it is required for the#25450fix.-
Check name Status Explanation Title check The title names the Windows update fix and the single leftover app process, which matches the main change: the NSIS FIND_PROCESS and KILL_PROCESS patch in app-builder-lib. It uses a conventional commi… Description check The description follows the template and fills in Problem, Change, Scope and approval, and Verification. The Problem section gives the error, the trigger, and the root cause in PowerShell 5.1. The Cha… Full details: Title check
-
Explanation
The title names the Windows update fix and the single leftover app process, which matches the main change: the NSIS FIND_PROCESS and KILL_PROCESS patch in app-builder-lib. It uses a conventional commit prefix and is one sentence. It is specific enough for a reviewer scanning history to understand the primary change. It is 81 characters, which is a little long but acceptable.
Full details: Description check
-
Explanation
The description follows the template and fills in Problem, Change, Scope and approval, and Verification. The Problem section gives the error, the trigger, and the root cause in PowerShell 5.1. The Change section lists both patch hunks and the lockfile update. Scope links issue
#25450. Verification gives compiled-installer and filter-logic evidence, test results, and a clear list of what was not checked. Two points are weaker. The Scope section cites an automated triage flow, not a maintainer approval comment. The template asks for a link to the approval comment. The description also says the Linux-built installer was checked, not run on Windows. Neither gap makes the description largely incomplete.
-
Full details: Linked Issues check
-
Explanation
Issue
#25450has two coding objectives. The patch implements the installer-side objective:FIND_PROCESSuses@(...), and both process commands exclude the installer or uninstaller PID. The reported build and filter checks support this behavior. The PR does not investigate or fix the separate Windows main-process teardown problem that#25450also requests. The Windows PowerShell 5.1 and end-to-end limitations are evidence limits, not additional failures.Resolution
Investigate and fix the Windows main-process teardown hang described in
#25450. Add automated coverage where feasible.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR summary reports a Linux NSIS change that makes builds use
UninstallerReader. Issue#25450concerns Windows NSIS process detection and termination. The reportedUninstallerReaderchange has no stated connection to that objective. The PR description says that only the process-check patch hunk and its lockfile hash changed, so the supplied evidence contains a scope conflict.Resolution
Remove the unrelated
UninstallerReaderchange from this pull request, or provide concrete evidence that it is required for the#25450fix.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
juliusmarminge
added
bugfix
Problem
On Windows, in-app updates fail with
Failed to uninstall old application files. Please try running the installer again.: 2when one T3 Code process is still running from the install folder (for example, a main process that outlived the updater-controlled quit). After that the app comes back on the old version. electron-builder's NSISFIND_PROCESScheck runs(Get-CimInstance Win32_Process | ? {...}).Count -gt 0. In Windows PowerShell 5.1,.Counton a singleCimInstanceis empty, so with exactly one matching process the installer decides nothing is running and never kills it. The old uninstaller then can't move the locked exe and rolls back. Upstream: electron-userland/electron-builder#10454 (open, unfixed in 26.16.1).Change
This adds a hunk to the existing
patches/app-builder-lib@26.16.1.patchfortemplates/nsis/include/allowOnlyOneInstallerInstance.nsh:FIND_PROCESSwraps the pipeline in@(...), so.Countis 1 for a single process on PowerShell 5.1 (the upstream proposal).FIND_PROCESSandKILL_PROCESSboth skip the installer's or uninstaller's own PID ($$_.ProcessId -ne $pid), the same exclusion upstream'staskkillpath already uses. Without it, the corrected count would make an uninstaller running in place from the install folder (theTryInPlacefallback ininstallUtil.nsh) find and kill itself.pnpm-lock.yamlonly picks up the new patch hash.Scope and approval
Fixes #25450
Triaged via the automated issue flow.
Verification
Compiled installer, before and after. This VM can't run Windows, so I built a real NSIS installer with makensis on Linux through each patched
app-builder-libcopy. The probe project is synthetic, withcompression: store. I then decoded the installer's UTF-16LE string table. (<var>is how NSIS encodes$INSTDIR/$pid, which are runtime variables.)if ((Get-CimInstance … | ? {$_.Path -and …}).Count -gt 0)if (@(Get-CimInstance … | ? {$_.ProcessId -ne <var #180> -and $_.Path -and …}).Count -gt 0)Get-CimInstance … | ? {$_.Path -and …} | % { Stop-Process … }Get-CimInstance … | ? {$_.ProcessId -ne <var #180> -and $_.Path -and …} | % { Stop-Process … }<var #180>is the same variable as upstream'staskkill /FI "PID ne $pid". There is no literal$pidtext in the binary.Transcripts: before vs first commit, head. Scripts: nsis-probe-build.mjs, README, decode-installer-strings.py.
Filter logic. I ran the expanded commands in PowerShell 7.4.6 against synthetic process objects (own PID 4242):
Transcript, script, driver.
Patched dependency. The installed package contains both template edits, and the existing
NsisTarget.jshunk is unchanged.Tests.
vp test run scripts/build-desktop-artifact.test.ts: 42/42 pass.vp run check:changedpasses.Not checked.
cannot-verify windows. Nothing here runs Windows PowerShell 5.1 with realCimInstanceobjects or does an end-to-end update on Windows. The PowerShell 7 check covers the filter logic only, not the 5.1.Countquirk itself. The reporter's separate observation, that the main process sometimes doesn't exit after the updater quit, is not addressed here.Claude Opus 5.5 (Claude Code, via T3 Code) did the work. GPT-6.1 Sol (Codex) and Claude Opus 5.5 reviewed it.
🤖 Generated with Claude Code