Skip to content

Fix Slack MSIX uninstall matching unrelated provisioned packages - #47039

Merged
allenhouchins merged 1 commit into
mainfrom
fix-slack-msix-uninstall-matching
Jun 7, 2026
Merged

Fix Slack MSIX uninstall matching unrelated provisioned packages#47039
allenhouchins merged 1 commit into
mainfrom
fix-slack-msix-uninstall-matching

Conversation

@allenhouchins

@allenhouchins allenhouchins commented Jun 7, 2026

Copy link
Copy Markdown
Member

Problem

The Slack Windows (winget/MSIX) uninstall script selected provisioned packages with:

$packageFamilyName = $PACKAGE_ID
Get-AppxProvisionedPackage -Online | Where-Object { $_.PackageFamilyName -eq $packageFamilyName }

Two bugs compound here:

  1. Get-AppxProvisionedPackage objects have no PackageFamilyName property (that's on Get-AppxPackage results), so $_.PackageFamilyName is $null for every provisioned package.
  2. The FMA validator never substitutes $PACKAGE_ID — it runs the raw ref script with no substitution — so $packageFamilyName is also $null.

The filter reduces to $null -eq $nulltrue for every package, so the script tries to remove all provisioned packages on the machine and fails on protected ones like Microsoft.DesktopAppInstaller (exit code 1603). This is the same failure mode just fixed for Affinity.

Fix

Rewrite slack_uninstall.ps1 to match Slack by literal identity — across DisplayName/PackageName for provisioned packages and null-guarded Name/PackageFamilyName/Publisher for installed packages — following the working claude_uninstall.ps1 pattern. No longer depends on $PACKAGE_ID substitution or a property the provisioned object lacks, so it works in both the validator and production.

Regenerated the output manifest ref: ae79ce281be2e38e (content hash verified against the source script).

Note on MS Teams

msteams_uninstall.ps1 is not affected — it matches a literal DisplayName ("MSTeams"), a real property on provisioned packages, rather than PackageFamilyName/$PACKAGE_ID. No change needed.

Testing

  • FMA validator run on a Windows host with Slack installed

Summary by CodeRabbit

  • Bug Fixes
    • Improved Slack uninstallation reliability by enhancing package detection logic to handle various package identification methods instead of exact ID matching.
    • Enhanced uninstall process with more robust error handling, timeout protection (5-minute limit), and verbose progress logging for better visibility during removal operations.

The Slack Windows uninstall script matched provisioned packages with
$_.PackageFamilyName -eq $PACKAGE_ID. Get-AppxProvisionedPackage objects
don't expose a PackageFamilyName property (it's $null), and the FMA validator
doesn't substitute $PACKAGE_ID (also $null), so the filter reduced to
$null -eq $null and selected every provisioned package on the machine,
failing on protected packages like Microsoft.DesktopAppInstaller (exit 1603).

Rewrite to match Slack by literal identity across DisplayName/PackageName
(provisioned) and null-guarded Name/PackageFamilyName/Publisher (installed),
following the working claude_uninstall.ps1 pattern. Regenerate the output
manifest ref (ae79ce28 -> 1be2e38e).

MS Teams is unaffected: msteams_uninstall.ps1 already matches a literal
DisplayName, not PackageFamilyName/$PACKAGE_ID.
Copilot AI review requested due to automatic review settings June 7, 2026 17:14

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Script Diff Results

ee/maintained-apps/outputs/slack/windows.json

=== Install Script (no changes) ===
=== Uninstall // ae79ce28 -> 1be2e38e ===

--- /tmp/old.y1bjQI	2026-06-07 17:15:49.676156020 +0000
+++ /tmp/new.LoFyWq	2026-06-07 17:15:49.676156020 +0000
@@ -1,23 +1,46 @@
-$packageFamilyName = 'com.tinyspeck.slackdesktop_8yrtsj140pw4g'
 $timeoutSeconds = 300  # 5 minute timeout
 
+# Match only Slack (published by Slack Technologies). We deliberately do NOT rely
+# on $PACKAGE_ID or on a PackageFamilyName property: Get-AppxProvisionedPackage
+# objects don't expose PackageFamilyName, so an "-eq" match against it is $null on
+# every package and would select unrelated packages (e.g. DesktopAppInstaller).
+function ShouldRemoveSlackPackage {
+  param([Parameter(Mandatory=$true)]$pkg)
+  try {
+    $name = [string]$pkg.Name
+    $family = [string]$pkg.PackageFamilyName
+    $publisher = [string]$pkg.Publisher
+
+    if ($name -and ($name -like "*Slack*")) { return $true }
+    if ($family -and ($family -like "*Slack*")) { return $true }
+    if ($publisher -and ($publisher -like "*Slack Technologies*") -and $name -and ($name -like "*Slack*")) { return $true }
+  } catch {}
+  return $false
+}
+
 try {
 
   $start = Get-Date
-  $provisioned = Get-AppxProvisionedPackage -Online -ErrorAction Stop |
-    Where-Object { $_.PackageFamilyName -eq $packageFamilyName }
+
+  $provisioned = Get-AppxProvisionedPackage -Online -ErrorAction Stop | Where-Object {
+    ($_.DisplayName -and ($_.DisplayName -like "*Slack*")) -or
+    ($_.PackageName -and ($_.PackageName -like "*Slack*"))
+  }
   foreach ($pkg in $provisioned) {
-    Remove-AppxProvisionedPackage -Online -PackageName $pkg.PackageName -AllUsers -ErrorAction Stop
+    Write-Host "Removing provisioned package: $($pkg.PackageName)"
+    Remove-AppxProvisionedPackage -Online -PackageName $pkg.PackageName -AllUsers -ErrorAction Stop | Out-String | Write-Host
     $elapsed = (New-TimeSpan -Start $start).TotalSeconds
     if ($elapsed -gt $timeoutSeconds) {
       Exit 1603
     }
   }
 
-  $installed = Get-AppxPackage -AllUsers -PackageTypeFilter Main -ErrorAction SilentlyContinue |
-    Where-Object { $_.PackageFamilyName -eq $packageFamilyName }
+  $installed = Get-AppxPackage -AllUsers -PackageTypeFilter Main -ErrorAction SilentlyContinue | Where-Object {
+    ShouldRemoveSlackPackage $_
+  }
   foreach ($app in $installed) {
-    Remove-AppxPackage -Package $app.PackageFullName -AllUsers -ErrorAction Stop
+    Write-Host "Removing installed package: $($app.PackageFullName)"
+    Remove-AppxPackage -Package $app.PackageFullName -AllUsers -ErrorAction Stop | Out-String | Write-Host
     $elapsed = (New-TimeSpan -Start $start).TotalSeconds
     if ($elapsed -gt $timeoutSeconds) {
       Exit 1603

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Slack (Windows MSIX/winget) uninstall logic to avoid accidentally matching and attempting to remove unrelated provisioned Appx packages, and refreshes the generated maintained-app manifest output to reference the new uninstall script.

Changes:

  • Rewrites slack_uninstall.ps1 to avoid $PACKAGE_ID substitution and to filter provisioned/installed packages by properties available at runtime.
  • Updates the Slack Windows maintained-app output manifest to point at the regenerated uninstall script ref.

Reviewed changes

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

File Description
ee/maintained-apps/outputs/slack/windows.json Updates uninstall_script_ref and embedded uninstall script ref content for Slack Windows MSIX.
ee/maintained-apps/inputs/winget/scripts/slack_uninstall.ps1 Replaces uninstall matching logic to avoid selecting unrelated Appx provisioned packages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +14 to +16
if ($name -and ($name -like "*Slack*")) { return $true }
if ($family -and ($family -like "*Slack*")) { return $true }
if ($publisher -and ($publisher -like "*Slack Technologies*") -and $name -and ($name -like "*Slack*")) { return $true }
Comment on lines +25 to +28
$provisioned = Get-AppxProvisionedPackage -Online -ErrorAction Stop | Where-Object {
($_.DisplayName -and ($_.DisplayName -like "*Slack*")) -or
($_.PackageName -and ($_.PackageName -like "*Slack*"))
}
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR updates the Windows Slack uninstall process to use flexible pattern matching instead of hardcoded package identity checks. It introduces a ShouldRemoveSlackPackage helper function that identifies Slack packages by matching Name, PackageFamilyName, or Publisher fields rather than exact equality. The helper is applied to both provisioned and installed package removal flows, with added verbose logging and output formatting. The corresponding metadata reference is updated to point to the new script implementation.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix Slack MSIX uninstall matching unrelated provisioned packages' accurately and specifically describes the main issue being addressed—a bug where the uninstall script incorrectly matches all provisioned packages instead of just Slack.
Description check ✅ Passed The description covers the problem clearly, explains the two underlying bugs, describes the fix with reference to a working pattern, and notes scope boundaries (MS Teams unaffected). However, the testing checklist item is unchecked despite being described as present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-slack-msix-uninstall-matching

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ee/maintained-apps/inputs/winget/scripts/slack_uninstall.ps1 (1)

14-16: ⚡ Quick win

Remove unreachable condition on line 16.

Line 16 can never return true because:

  • If $name contains "Slack", line 14 would have already returned true
  • So by the time execution reaches line 16, we know $name is either null/empty OR does not contain "Slack"
  • But line 16 requires $name to be non-empty AND contain "Slack" – a contradiction

The current logic with lines 14-15 already correctly matches Slack packages by Name or PackageFamilyName. Line 16 adds no additional coverage.

♻️ Proposed fix to remove dead code
     if ($name -and ($name -like "*Slack*")) { return $true }
     if ($family -and ($family -like "*Slack*")) { return $true }
-    if ($publisher -and ($publisher -like "*Slack Technologies*") -and $name -and ($name -like "*Slack*")) { return $true }
   } catch {}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ee/maintained-apps/inputs/winget/scripts/slack_uninstall.ps1` around lines 14
- 16, The third conditional that checks "if ($publisher -and ($publisher -like
"*Slack Technologies*") -and $name -and ($name -like "*Slack*")) { return $true
}" is unreachable because earlier checks already return true when $name contains
"Slack"; remove this redundant if entirely, or if the intent was to match by
publisher even when $name is missing, replace it with a publisher-only check
using $publisher -like "*Slack Technologies*" to return true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ee/maintained-apps/inputs/winget/scripts/slack_uninstall.ps1`:
- Around line 14-16: The third conditional that checks "if ($publisher -and
($publisher -like "*Slack Technologies*") -and $name -and ($name -like
"*Slack*")) { return $true }" is unreachable because earlier checks already
return true when $name contains "Slack"; remove this redundant if entirely, or
if the intent was to match by publisher even when $name is missing, replace it
with a publisher-only check using $publisher -like "*Slack Technologies*" to
return true.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ecdaf112-ab14-4e40-9a61-1fd4e791c95c

📥 Commits

Reviewing files that changed from the base of the PR and between 1f26238 and 54fcf4e.

📒 Files selected for processing (2)
  • ee/maintained-apps/inputs/winget/scripts/slack_uninstall.ps1
  • ee/maintained-apps/outputs/slack/windows.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants