Report cancelled SDK uninstalls accurately by GaTTGeng · Pull Request #2731 · dotnet/vscode-dotnet-runtime

GaTTGeng · 2026-06-19T01:40:44Z

Summary

preserve the failure reason returned by the global SDK uninstaller
recognize the @vscode/sudo-prompt UAC dismissal message as user cancellation
report a dismissed elevation prompt as a cancelled uninstall instead of suggesting that another install may be in progress
add regression coverage for the library failure propagation and the final language-model-facing message

Root cause

When the Windows UAC prompt was dismissed, @vscode/sudo-prompt returned
User did not grant permission.. The global uninstall worker discarded that
status and replaced it with an arbitrary error code and a fixed
Another install may be in progress message.

User impact

Copilot and other callers now receive an accurate cancellation message with
guidance to retry and accept the elevation prompt. Other uninstall failures
continue to use the existing generic guidance.

Validation

npm test in vscode-dotnet-runtime-library: 255 passing, 7 pending
npm run test:lm-tools in vscode-dotnet-runtime-extension: 68 passing

Fixes #2698

When a Windows user dismisses the UAC dialog during global SDK uninstall, @vscode/sudo-prompt surfaces "User did not grant permission." That string matched none of the existing tokens in isUserCancellationMessage, so the LLM tool path classified the cancellation as a generic failure and showed the wrong retry guidance. Broaden the regex to recognize it, and export the helper so the extension command path can reuse it (dotnet#2698).

uninstallGlobal previously discarded the installer's status string and returned the arbitrary code 117778 with a fixed "Another install may be in progress" message. When the user dismisses the UAC dialog the status already carries "User did not grant permission.", so propagate it into the DotnetUninstallFailed event and the return value instead. Add a unit test that stubs both global installers to assert the surfaced text (dotnet#2698).

The wrapper at the extension command path used to claim every non-zero uninstallGlobal result was "may have been cancelled, blocked by another install in progress, or require manual removal", and embedded the raw result inside "(code ...)". Now that uninstallGlobal forwards the elevation provider's reason string, detect the cancellation case via isUserCancellationMessage and emit a clear "admin/elevation prompt was dismissed" message; for non-numeric reasons drop the "code" wrapping since the result already reads as text (dotnet#2698).

Move uninstall failure formatting into a testable helper and verify that dismissing the elevation prompt is reported as a cancellation rather than an unrelated installer conflict. Fix dotnet#2698

GaTTGeng · 2026-06-19T01:43:10Z

@dotnet-policy-service agree

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

nagilson

Thank you for taking this on! You did a great job investigating. My feedback is mostly nitpicking and enhancements but happy to move this forward afterward and don't see any overall issues.

nagilson · 2026-06-19T22:35:09Z

                if (result !== '0' && result !== '')
                {
-                    throw new Error(`Uninstall of .NET ${commandContext.version} did not succeed (code ${result}). The uninstaller may have been cancelled, blocked by another install in progress, or require manual removal.`);
+                    throw new Error(buildUninstallFailureMessage(commandContext.version, result));


nit: I like the use of buildUninstallFailureMessage here. I think outside of registering the language model tools, I'd rather avoid importing any implementation from that file here because it will increase the risk of circular dependencies.

I suggest we move buildUninstallFailureMessage into a separate file now, such as ErrorMessageUtilities.ts like how there is a VersionUtilities.ts file.

I would do similarly for isUserCancellationMessage or ideally not export it at all. That method doesn't consider exit code 126 and I'd rather it not be used elsewhere as it may be confused with the check in parseVSCodeSudoExecError which is more robust.

nagilson · 2026-06-19T22:36:20Z

+
+export function buildUninstallFailureMessage(version: string, result: string): string
+{
+    const detail = /^-?\d+$/.test(result) ? `code ${result}` : result;


If at all possible, I'd rather use a native or nodejs method to compare if the string is a whole integer as '^-?\d+$/' is harder to read imo.

nagilson · 2026-06-19T22:37:38Z

+                    // When command execution is non-terminal, the status may contain a useful error from the elevation
+                    // provider (for example, "User did not grant permission.") instead of only a numeric exit code.
+                    const failureReason = uninstallResult.trim();
+                    const failureDetails = failureReason ? ` ${failureReason}` : '';


I don't think we need to create failureDetails.

nagilson · 2026-06-19T22:38:58Z

+        const originalUninstallSDK = WinMacGlobalInstaller.prototype.uninstallSDK;
+        const originalDisableMutex = process.env.VSCODE_DOTNET_RUNTIME_DISABLE_MUTEX;
+
+        process.env.VSCODE_DOTNET_RUNTIME_DISABLE_MUTEX = 'true';


I don't think this should be necessary, although it could be argued that it may increase test consistency, I'd err towards simplicity here.

nagilson · 2026-06-19T22:40:09Z

+        } as GlobalInstallerResolver;
+        const originalLinuxGetExpectedGlobalSDKPath = LinuxGlobalInstaller.prototype.getExpectedGlobalSDKPath;
+        const originalLinuxUninstallSDK = LinuxGlobalInstaller.prototype.uninstallSDK;
+        const originalGetExpectedGlobalSDKPath = WinMacGlobalInstaller.prototype.getExpectedGlobalSDKPath;


The implementation of this method is nearly identical between the LinuxGlobalInstaller and WinMac one. I wonder if this prototype modification can be parameterized and deduplicated so that the test doesn't need to do the same thing for each type?

GaTTGeng · 2026-06-20T01:50:17Z

Thank you so much for the thorough review, @nagilson ! This is my first contribution to an open-source project, and receiving such detailed and constructive feedback genuinely means a lot to me. I'll work through each of your suggestions.

- Fix uninstallGlobal to emit DotnetUninstallSkipped and return '0' when other dependents remain instead of falling through to the failure path (Copilot) - Move buildUninstallFailureMessage into ErrorMessageUtilities.ts to avoid circular dependency between extension.ts and LanguageModelTools (nagilson) - Keep isUserCancellationMessage private in both modules (nagilson) - Trim result before numeric detection and use Number.isInteger instead of regex for readability (Copilot + nagilson) - Remove redundant failureDetails variable (nagilson) - Drop unnecessary VSCODE_DOTNET_RUNTIME_DISABLE_MUTEX env toggle in test (nagilson) - Parameterize prototype patching for Linux/WinMac installers to reduce duplication in regression test (nagilson)

GaTTGeng · 2026-06-22T00:55:12Z

Hi @nagilson, are there any areas where I need to improve? If so, please let me know. Thank u!

nagilson

Thank you for your kind words, you've done a great job for your first open-source PR especially! I appreciate the effort you put into applying the fixes. I have a remaining nitpick and once that is resolved this looks good to go.

nagilson · 2026-06-22T17:39:19Z

 function isUserCancellationMessage(message: string): boolean
 {
-    return /cancel|user rejected|user denied|password request/i.test(message);
+    return /cancel|user rejected|user denied|password request|did not grant permission/i.test(message);


This should be able to import the error utilities function, so we can delete this function instead.

nagilson · 2026-06-22T17:42:22Z


                    // Note: it's ok not to check live dependents here (though we could) since this will require UAC and extensions do not depend on us to auto-manage admin installs
-                    if (force || await InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).installHasNoRegisteredDependentsBesidesId(install, context.installDirectoryProvider, false, context.acquisitionContext.requestingExtensionId ?? ''))
+                    if (!force && !(await InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).installHasNoRegisteredDependentsBesidesId(install, context.installDirectoryProvider, false, context.acquisitionContext.requestingExtensionId ?? '')))


Nice little optimization here by changing the tautology.

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…tilities

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

GaTTGeng · 2026-06-23T06:39:10Z

Hi @nagilson, all review feedback has been addressed. Also force-pushed to fix an earlier commit that accidentally normalized line endings and made the diff hard to read. The current diff should now be clean. Happy to iterate further if anything still needs attention.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

nagilson

Looks excellent! Thank you for so diligently following up with my feedback.

Most likely the actual release including this fix will be in July in 3.2.0

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

+ * Heuristically detects whether an error/installer message indicates the user cancelled or declined an
+ * elevation/credential prompt. Kept private to this module: the more robust check in CommandExecutor's
+ * parseVSCodeSudoExecError is preferred everywhere else, and this heuristic does not consider exit code 126.
+ * "did not grant permission" comes from @vscode/sudo-prompt when the UAC dialog is dismissed on Windows.


+    const detailSuffix = detailText ? ` (${detailText})` : '';
+    return isUserCancellationMessage(normalized)
+        ? `Uninstall of .NET ${version} was cancelled. A permission/elevation prompt was dismissed or rejected. Retry and accept the prompt to continue.${detailSuffix}`
+        : `Uninstall of .NET ${version} did not succeed${detailSuffix}. The uninstaller may be blocked by another install in progress or require manual removal.`;


+                    // When command execution is non-terminal, the status may contain a useful error from the elevation
+                    // provider (for example, "User did not grant permission.") instead of only a numeric exit code.
+                    const failureReason = uninstallResult.trim();
+                    context.eventStream.post(new DotnetUninstallFailed(`Failed to uninstall .NET ${install.installId}.${failureReason ? ` ${failureReason}` : ''}`));
+                    return failureReason || '1';


GaTTGeng · 2026-06-24T05:09:48Z

Thank you for the kind words, and for the thorough review — it made the fix much better than what I started with. It's been a great experience contributing to this project!

GaTTGeng added 4 commits June 19, 2026 09:35

Test cancelled uninstall message

cb4ff66

Move uninstall failure formatting into a testable helper and verify that dismissing the elevation prompt is reported as a cancellation rather than an unrelated installer conflict. Fix dotnet#2698

GaTTGeng changed the title ~~[codex] Report cancelled SDK uninstalls accurately~~ Report cancelled SDK uninstalls accurately Jun 19, 2026

GaTTGeng marked this pull request as ready for review June 19, 2026 01:55

Copilot AI review requested due to automatic review settings June 19, 2026 01:55

Copilot AI reviewed Jun 19, 2026

nagilson requested a review from Copilot June 19, 2026 22:27

Copilot started reviewing on behalf of nagilson June 19, 2026 22:27 View session

nagilson self-requested a review June 19, 2026 22:31

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts Outdated

Comment thread vscode-dotnet-runtime-extension/src/LanguageModelTools.ts Outdated

nagilson reviewed Jun 19, 2026

View reviewed changes

nagilson reviewed Jun 22, 2026

View reviewed changes

Comment thread vscode-dotnet-runtime-extension/src/ErrorMessageUtilities.ts Outdated

nagilson reviewed Jun 22, 2026

View reviewed changes

Comment thread vscode-dotnet-runtime-extension/src/ErrorMessageUtilities.ts Outdated

nagilson reviewed Jun 22, 2026

View reviewed changes

Improve cancellation message wording

297f125

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>

Copilot AI review requested due to automatic review settings June 23, 2026 01:02

Copilot AI reviewed Jun 23, 2026

Remove unnecessary comma in uninstall failure message

912cdbc

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>

Copilot AI review requested due to automatic review settings June 23, 2026 01:03

Copilot AI reviewed Jun 23, 2026

Deduplicate isUserCancellationMessage by importing from ErrorMessageU…

afe9e6f

…tilities

Copilot AI review requested due to automatic review settings June 23, 2026 06:35

GaTTGeng force-pushed the fix/2698-surface-uninstall-error branch from c6cbdae to afe9e6f Compare June 23, 2026 06:35

Copilot AI reviewed Jun 23, 2026

nagilson requested a review from Copilot June 23, 2026 23:46

Copilot started reviewing on behalf of nagilson June 23, 2026 23:47 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread vscode-dotnet-runtime-extension/src/ErrorMessageUtilities.ts

Comment thread vscode-dotnet-runtime-extension/src/ErrorMessageUtilities.ts

nagilson approved these changes Jun 23, 2026

View reviewed changes

nagilson enabled auto-merge June 23, 2026 23:50

Don't potentially include missing space . in error msg

3c80f3b

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings June 23, 2026 23:53

Copilot started reviewing on behalf of nagilson June 23, 2026 23:54 View session

nagilson approved these changes Jun 23, 2026

View reviewed changes

nagilson requested a review from a team June 23, 2026 23:54

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Uh oh!

Conversation

GaTTGeng commented Jun 19, 2026

Summary

Root cause

User impact

Validation

Uh oh!

GaTTGeng commented Jun 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

nagilson left a comment

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

GaTTGeng commented Jun 20, 2026

Uh oh!

GaTTGeng commented Jun 22, 2026

Uh oh!

Uh oh!

Uh oh!

nagilson left a comment

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

nagilson Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

GaTTGeng commented Jun 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

nagilson left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

GaTTGeng commented Jun 24, 2026

Uh oh!

Reviewers

Assignees