tell Inno Setup to kill the minidump server process if it failed to exit by acarl005 · Pull Request #10766 · warpdotdev/warp

acarl005 · 2026-05-12T20:27:20Z

Description

This PR is to help with Windows auto-update failures. Specifically, we are getting blocked updates due to the minidump server not exiting.

This is a follow-up to #10528.

As of that PR, the minidump server should exit on its own. However, if for some that fails, we add a fail-safe into inno setup to close this process by force.

I've also added the appropriate telemetry for when this fail-safe fails.

Linked Issue

This PR also helps with #10202. Master issue is #10044

Testing

I ran the powershell command manually in a terminal session to verify that it kills the minidump server as intended.

oz-for-oss · 2026-05-12T20:27:35Z

@acarl005

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adds a Windows installer fallback that attempts to stop orphaned minidump-server processes after Warp exits, plus telemetry for logged cleanup failures.

Concerns

The PowerShell cleanup command suppresses Stop-Process errors, so failures to terminate the minidump server can still return exit code 0 and skip the new failure log/telemetry.
No security findings.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-12T20:38:47Z

+          which causes the file-copy step to fail with "Access is denied".
+          We identify it by the "minidump-server" argument in its command line. }
+        Exec('powershell.exe',
+          '-NoProfile -NoLogo -Command "Get-CimInstance Win32_Process -Filter \"Name=''''{#MyAppExeName}'''' and CommandLine like ''''%minidump-server%''''\" | ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorAction SilentlyContinue }"',


⚠️ [IMPORTANT] -ErrorAction SilentlyContinue suppresses Stop-Process failures, so PowerShell can exit 0 even when the process was not killed and the new telemetry on line 225 will not fire; make stop failures terminating or verify no matching process remains before returning success.

acarl005 · 2026-05-12T21:15:48Z

/oz-review

oz-for-oss · 2026-05-12T21:16:00Z

@acarl005

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adds a Windows installer fallback that attempts to stop orphaned minidump-server processes after Warp exits, plus telemetry for non-zero cleanup exit codes.

Concerns

The PowerShell Win32_Process filter is quoted incorrectly in the Inno Setup string, so it is likely to look for a process whose Name literally includes quotes instead of matching {#MyAppExeName}. That would make the new fail-safe a no-op while still allowing the installer to continue.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-12T21:24:43Z

+          which causes the file-copy step to fail with "Access is denied".
+          We identify it by the "minidump-server" argument in its command line. }
+        Exec('powershell.exe',
+          '-NoProfile -NoLogo -Command "Get-CimInstance Win32_Process -Filter \"Name=''''{#MyAppExeName}'''' and CommandLine like ''''%minidump-server%''''\" | ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorAction SilentlyContinue }"',


⚠️ [IMPORTANT] The PascalScript escaping here emits doubled quotes around both WMI string literals (for example Name=''warp.exe''), so the CIM filter will not match the process; use a single escaped quote on each side of the literal, e.g. Name=''{#MyAppExeName}'' and CommandLine like ''%minidump-server%''.

acarl005 · 2026-05-12T22:41:41Z

/oz-review

oz-for-oss · 2026-05-12T22:41:53Z

@acarl005

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adds a Windows installer fallback that stops orphaned minidump-server processes after Warp exits, plus telemetry that reports when that fallback fails.

Concerns

The new telemetry parser misses signed exit codes even though the installer can log signed HRESULT-style values from PowerShell failure paths.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-12T22:50:39Z

+    let after_marker = &after_failed[marker_pos + EXIT_CODE_MARKER.len()..];
+    let digit_len = after_marker
+        .iter()
+        .take_while(|b| b.is_ascii_digit())


⚠️ [IMPORTANT] The installer records $e[0].Exception.HResult, which is commonly a signed negative value; because this parser only consumes ASCII digits, log lines such as exit code: -214... return None and suppress the new failure telemetry. Accept an optional leading - or normalize the logged value before parsing.

acarl005 · 2026-05-13T03:17:44Z

/oz-review

oz-for-oss · 2026-05-13T03:17:56Z

@acarl005

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adds a Windows installer fallback that tries to stop orphaned minidump-server processes after Warp exits, plus telemetry that reports cleanup failures from the next app launch.

Concerns

The PowerShell WQL filter is quoted with doubled single quotes, so the runtime filter becomes Name=''warp.exe'' ... instead of valid WQL string literals like Name='warp.exe'. That can make the cleanup command fail before it ever finds or stops the minidump server.
The telemetry parser only consumes ASCII digits after exit code: , but the PowerShell command logs HRESULT-style failures from Exception.HResult, which are commonly negative. Those failures will be logged but not emitted as AutoupdateMinidumpCleanupFailed.

Verdict

Found: 0 critical, 2 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-13T03:25:36Z

+          which causes the file-copy step to fail with "Access is denied".
+          We identify it by the "minidump-server" argument in its command line. }
+        Exec('powershell.exe',
+          '-NoProfile -NoLogo -Command "$stopError = 0; Get-CimInstance Win32_Process -Filter \"Name=''''{#MyAppExeName}'''' and CommandLine like ''''%minidump-server%''''\" | ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorVariable e -ErrorAction SilentlyContinue; if ($e) { $stopError = $e[0].Exception.HResult } }; exit $stopError"',


⚠️ [IMPORTANT] This escapes each WQL string quote twice, so PowerShell receives Name=''warp.exe''/like ''%minidump-server%''; use one escaped Inno quote per WQL quote (Name=''{#MyAppExeName}'' ...) so the filter is valid and the cleanup can find the process.

oz-for-oss · 2026-05-13T03:25:36Z

+    let after_marker = &after_failed[marker_pos + EXIT_CODE_MARKER.len()..];
+    let digit_len = after_marker
+        .iter()
+        .take_while(|b| b.is_ascii_digit())


⚠️ [IMPORTANT] PowerShell exits with $e[0].Exception.HResult, which is often negative; this parser accepts only digits, so logs like exit code: -214... return None and the new telemetry never fires.

lucieleblanc

The bot comments about the exception parsing and process name filtering string seem valid.

lucieleblanc · 2026-05-13T15:55:04Z

+    let failed_pos = memchr::memmem::find(contents_lowercase, FAILED_MARKER)?;
+    let after_failed = &contents_lowercase[failed_pos..];
+    let marker_pos = memchr::memmem::find(after_failed, EXIT_CODE_MARKER)?;
+    let after_marker = &after_failed[marker_pos + EXIT_CODE_MARKER.len()..];
+    let digit_len = after_marker
+        .iter()
+        .take_while(|b| b.is_ascii_digit())
+        .count();
+    std::str::from_utf8(&after_marker[..digit_len])
+        .ok()?
+        .parse()
+        .ok()


Is there some existing parsing setup we can use here instead of manually handling all these cases?

acarl005 · 2026-05-13T22:55:17Z

/oz-review

oz-for-oss · 2026-05-13T22:55:29Z

@acarl005

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adds a Windows installer fail-safe that stops lingering minidump-server processes after Warp exits and reports a new telemetry event when that cleanup fails.

Concerns

The PowerShell cleanup command only records Stop-Process failures. If the CIM query itself fails or the filter cannot be evaluated, the command exits successfully, so Inno skips the failure log and the new telemetry never fires while the process may still keep the executable locked.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-13T23:01:23Z

+          which causes the file-copy step to fail with "Access is denied".
+          We identify it by the "minidump-server" argument in its command line. }
+        Exec('powershell.exe',
+          '-NoProfile -NoLogo -Command "$stopError = 0; Get-CimInstance Win32_Process -Filter \"Name=''{#MyAppExeName}'' and CommandLine like ''%minidump-server%''\" | ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorVariable e -ErrorAction SilentlyContinue; if ($e) { $stopError = $e[0].Exception.HResult } }; exit $stopError"',


⚠️ [IMPORTANT] Get-CimInstance errors are not handled here, so a WMI/CIM/query failure exits with $stopError = 0, skips the failure log, and prevents the new telemetry from firing; make the query use -ErrorAction Stop and return a non-zero code from catch.

I think that's ok. Since Get-CimInstance is read-only, it's much less risk of errors, e.g. access denied, compared to Stop-Process.

acarl005 · 2026-05-13T23:32:02Z

@lucieleblanc Oops! I should've resolved those before sending to you. Those are fixed now and I think the latest review is acceptable.

lucieleblanc

LGTM!

…xit (warpdotdev#10766)

tell Inno Setup to kill the minidump server process if it failed to exit

2919c7c

acarl005 added this to the 2026 Week 20 (May 11 - 15) milestone May 12, 2026

acarl005 requested a review from lucieleblanc May 12, 2026 20:27

acarl005 self-assigned this May 12, 2026

cla-bot Bot added the cla-signed label May 12, 2026

oz-for-oss Bot reviewed May 12, 2026

View reviewed changes

cargo fmt

d9dafa1

oz-for-oss Bot reviewed May 12, 2026

View reviewed changes

collect the error code

e97eaa8

oz-for-oss Bot reviewed May 12, 2026

View reviewed changes

oz-for-oss Bot reviewed May 13, 2026

View reviewed changes

lucieleblanc requested changes May 13, 2026

View reviewed changes

bot feedback

79286ea

oz-for-oss Bot reviewed May 13, 2026

View reviewed changes

acarl005 requested a review from lucieleblanc May 13, 2026 23:32

lucieleblanc approved these changes May 15, 2026

View reviewed changes

acarl005 merged commit ed01fe4 into master May 15, 2026
28 checks passed

acarl005 deleted the andy/inno-setup-kills-minidump-server branch May 15, 2026 21:54

lawsmd pushed a commit to lawsmd/cortex that referenced this pull request May 22, 2026

tell Inno Setup to kill the minidump server process if it failed to e…

abfa0d5

…xit (warpdotdev#10766)

Conversation

acarl005 commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Linked Issue

Testing

Uh oh!

oz-for-oss Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

acarl005 commented May 12, 2026

Uh oh!

oz-for-oss Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

acarl005 commented May 12, 2026

Uh oh!

oz-for-oss Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

acarl005 commented May 13, 2026

Uh oh!

oz-for-oss Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc left a comment

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 13, 2026

Choose a reason for hiding this comment

Uh oh!

acarl005 commented May 13, 2026

Uh oh!

oz-for-oss Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

acarl005 commented May 12, 2026 •

edited

Loading

oz-for-oss Bot commented May 12, 2026 •

edited

Loading

oz-for-oss Bot commented May 12, 2026 •

edited

Loading

oz-for-oss Bot commented May 12, 2026 •

edited

Loading

oz-for-oss Bot commented May 13, 2026 •

edited

Loading

oz-for-oss Bot commented May 13, 2026 •

edited

Loading