Add Azure Data Studio as a Windows FMA - #50027
Merged
Merged
Conversation
Azure Data Studio is a VS Code fork and ships the same Inno Setup script, including the 'runcode' task that launches the app after install. Start-Process -Wait waits for descendants as well as the process itself, so the launched app kept the install script blocked and the validator killed it at its 10-minute cap. Suppress the launch with /MERGETASKS=!runcode, as vscode_install.ps1 already does, and wait on the installer process alone.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Azure Data Studio as a Windows Fleet-maintained app (winget input + generated output), including install/uninstall scripts tailored to avoid Inno Setup’s runcode auto-launch behavior on SYSTEM-context validation runners, and wires up a new catalog icon.
Changes:
- Added Azure Data Studio Windows FMA input metadata and custom install/uninstall PowerShell scripts.
- Generated the corresponding
outputs/azure-data-studio/windows.jsonand added the app entry tooutputs/apps.json. - Added a new software catalog icon component and mapped
"azure data studio"to it.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/pages/SoftwarePage/components/icons/index.ts | Imports and registers the Azure Data Studio icon in the software-name-to-icon map. |
| frontend/pages/SoftwarePage/components/icons/AzureDataStudio.tsx | Adds a new icon component (embedded base64 PNG in an SVG wrapper). |
| ee/maintained-apps/outputs/azure-data-studio/windows.json | New generated output manifest for Azure Data Studio Windows, including script refs and queries. |
| ee/maintained-apps/outputs/apps.json | Adds Azure Data Studio to the maintained apps catalog list with description and slug. |
| ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1 | New custom uninstall script (registry lookup + defensive parsing). |
| ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1 | New custom install script with task suppression and bounded wait + ARP polling. |
| ee/maintained-apps/inputs/winget/azure-data-studio.json | New winget input definition (package id, scope/arch/type, script paths, categories). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+13
to
+15
| $installTimeoutSeconds = 600 | ||
| $registrationTimeoutSeconds = 120 | ||
|
|
Member
Author
There was a problem hiding this comment.
Good catch — this was a real bug, now fixed. The install cap is 420s and registration 120s, so the worst case stays under the 10-minute budget the caller enforces in cmd/maintained-apps/validate/windows.go.
As written it would have been killed part-way through the recovery path the script exists to provide, which is the worst possible place to lose the process. run 30372911136 passes.
Comment on lines
+35
to
+42
| if (-not $process.WaitForExit($installTimeoutSeconds * 1000)) { | ||
| Write-Host "Installer process did not exit within ${installTimeoutSeconds}s, stopping it." | ||
| Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue | ||
| Start-Sleep -Seconds 2 | ||
| } | ||
|
|
||
| $exitCode = $process.ExitCode | ||
| Write-Host "Install exit code: $exitCode" |
Member
Author
There was a problem hiding this comment.
Fixed. After Stop-Process the script now waits up to 30s for the kill to land and only reads .ExitCode when HasExited is true; otherwise it logs and falls through. Registration is treated as the authoritative signal in that case, since a killed process's exit code is meaningless — so a successful install can no longer be reported as a failure.
Comment on lines
+17
to
+22
| $uninstallCommand = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString } | ||
| if ($uninstallCommand -match '^\s*"([^"]+)"\s*(.*)$') { | ||
| $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = "$($Matches[2]) $uninstallArgs".Trim() } | ||
| } elseif ($uninstallCommand -match '(?i)^\s*(.+?\.exe)\s*(.*)$') { | ||
| $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = "$($Matches[2]) $uninstallArgs".Trim() } | ||
| } |
Member
Author
There was a problem hiding this comment.
Both added — the bare-token '^\s*(\S+)\s*(.*)$' fallback and -ErrorAction SilentlyContinue on the Get-ItemProperty call.
Separately, this script now also requires the publisher, mirroring the exists query. That is worth calling out because the validator's appExists looks up by name only, so a wrong exists-query publisher ships silently — the same class of bug found on Spyder in #50016. Requiring it in the uninstall puts the value under test, and it passes. Here it's Microsoft Corporation, verified against the installer's PE version resource (CompanyName).
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1:14
- The validator runs PowerShell scripts with a 10-minute context timeout (cmd/maintained-apps/validate/windows.go:291-299). With $installTimeoutSeconds=600 and $registrationTimeoutSeconds=120, this script can exceed the validator cap in the worst case (install wait hits 600s, then registration polling continues), which can reintroduce the original 10-minute timeout failure mode.
$installTimeoutSeconds = 600
$registrationTimeoutSeconds = 120
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1:39
- After a timeout you attempt Stop-Process, but then immediately read $process.ExitCode. If Stop-Process fails (or the process hasn’t fully terminated yet), ExitCode can throw because the process is still running, causing the catch to return Exit 1 even if the app installed/registered. Safer to wait briefly after stopping and guard ExitCode behind HasExited, setting a fallback exit code if it’s still running.
if (-not $process.WaitForExit($installTimeoutSeconds * 1000)) {
Write-Host "Installer process did not exit within ${installTimeoutSeconds}s, stopping it."
Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue
Start-Sleep -Seconds 2
}
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1:11
- This registry enumeration uses Get-ItemProperty without -ErrorAction SilentlyContinue, so a single unreadable/unexpected key can throw and force the whole uninstall to Exit 1. Most other scripts suppress these per-key errors (e.g. windsurf_uninstall.ps1:15-19).
[array]$uninstallKeys = Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
ForEach-Object { Get-ItemProperty $_.PSPath }
| $exitCode = $process.ExitCode; Write-Host "Uninstall exit code: $exitCode"; break | ||
| } | ||
| } | ||
| if (-not $foundUninstaller) { Write-Host "Uninstaller for '$softwareName' not found."; Exit 1 } |
Member
Author
There was a problem hiding this comment.
Agreed and fixed — Exit 0 when no entry is found. I checked the convention across the directory rather than taking it on the two examples: 155 uninstall scripts exit 0 in that branch versus 21 that exit 1, so idempotent is clearly the house style. Applied to all the uninstall scripts in this batch.
Worth noting it doesn't weaken the test: if the app is still installed, the validator's own post-uninstall appExists check still fails the run.
| ], | ||
| "refs": { | ||
| "23fce27b": "$softwareName = \"Azure Data Studio\"\n$softwareNameLike = \"*$softwareName*\"\n$uninstallArgs = \"/VERYSILENT /SUPPRESSMSGBOXES /NORESTART\"\n\n$machineKey = 'HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$machineKey32on64 = 'HKLM:\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$exitCode = 0\n\ntry {\n [array]$uninstallKeys = Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |\n ForEach-Object { Get-ItemProperty $_.PSPath }\n\n $foundUninstaller = $false\n foreach ($key in $uninstallKeys) {\n if ($key.DisplayName -like $softwareNameLike) {\n $foundUninstaller = $true\n $uninstallCommand = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }\n if ($uninstallCommand -match '^\\s*\"([^\"]+)\"\\s*(.*)$') {\n $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = \"$($Matches[2]) $uninstallArgs\".Trim() }\n } elseif ($uninstallCommand -match '(?i)^\\s*(.+?\\.exe)\\s*(.*)$') {\n $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = \"$($Matches[2]) $uninstallArgs\".Trim() }\n }\n Write-Host \"Uninstall command: $uninstallCommand\"; Write-Host \"Uninstall args: $uninstallArgs\"\n $processOptions = @{ FilePath = $uninstallCommand; PassThru = $true; Wait = $true }\n if ($uninstallArgs -ne '') { $processOptions.ArgumentList = $uninstallArgs }\n $process = Start-Process @processOptions\n $exitCode = $process.ExitCode; Write-Host \"Uninstall exit code: $exitCode\"; break\n }\n }\n if (-not $foundUninstaller) { Write-Host \"Uninstaller for '$softwareName' not found.\"; Exit 1 }\n} catch { Write-Host \"Error: $_\"; Exit 1 }\n\nExit $exitCode\n", | ||
| "29e6eb61": "# Learn more about .exe install scripts:\n# http://fleetdm.com/learn-more-about/exe-install-scripts\n\n$exeFilePath = \"${env:INSTALLER_PATH}\"\n\n# Azure Data Studio is a Visual Studio Code fork, so it ships the same Inno Setup\n# script -- including the \"runcode\" task, which launches the app once the install\n# finishes. PowerShell's \"Start-Process -Wait\" waits for the process *and all of\n# its descendants*, so the launched app kept the install script blocked\n# indefinitely. \"/MERGETASKS=!runcode\" suppresses the launch (the same switch\n# vscode_install.ps1 and vscodium_install.ps1 use); waiting on the installer\n# process alone, then stopping any stray app process, covers the rest.\n$installTimeoutSeconds = 600\n$registrationTimeoutSeconds = 120\n\n$machineKey = 'HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$machineKey32on64 = 'HKLM:\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n\nfunction Test-AzureDataStudioRegistered {\n $null -ne (Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |\n ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } |\n Where-Object { $_.DisplayName -like \"Azure Data Studio*\" } |\n Select-Object -First 1)\n}\n\ntry {\n\n$process = Start-Process -FilePath \"$exeFilePath\" `\n -ArgumentList \"/VERYSILENT /SUPPRESSMSGBOXES /NORESTART /MERGETASKS=!runcode\" `\n -PassThru\n# Touch .Handle so the exit code is still readable after the process ends:\n# Start-Process -PassThru otherwise returns $null for .ExitCode.\n$null = $process.Handle\n\nif (-not $process.WaitForExit($installTimeoutSeconds * 1000)) {\n Write-Host \"Installer process did not exit within ${installTimeoutSeconds}s, stopping it.\"\n Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue\n Start-Sleep -Seconds 2\n}\n\n$exitCode = $process.ExitCode\nWrite-Host \"Install exit code: $exitCode\"\n\n# The installer can return before the Add/Remove Programs entry is written; wait\n# for it so software inventory sees a complete install.\n$elapsed = 0\nwhile (-not (Test-AzureDataStudioRegistered) -and ($elapsed -lt $registrationTimeoutSeconds)) {\n Start-Sleep -Seconds 5\n $elapsed += 5\n Write-Host \"Waiting for Azure Data Studio to register... ($elapsed seconds)\"\n}\n\n# Belt and braces in case a future build ignores !runcode.\nStop-Process -Name \"azuredatastudio\" -Force -ErrorAction SilentlyContinue\n\nif (-not (Test-AzureDataStudioRegistered)) {\n Write-Host \"Azure Data Studio did not register in Add/Remove Programs.\"\n Exit 1\n}\n\n# 3010 (reboot required) and 1641 (reboot initiated) are successful installs.\nif ($exitCode -eq 3010 -or $exitCode -eq 1641) { Exit 0 }\n\nExit $exitCode\n\n} catch {\n Write-Host \"Error: $_\"\n Exit 1\n}\n" |
Member
Author
There was a problem hiding this comment.
Regenerated — the ref now has the 420s/120s budget. run 30372911136 passes.
| } | ||
| ], | ||
| "refs": { | ||
| "23fce27b": "$softwareName = \"Azure Data Studio\"\n$softwareNameLike = \"*$softwareName*\"\n$uninstallArgs = \"/VERYSILENT /SUPPRESSMSGBOXES /NORESTART\"\n\n$machineKey = 'HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$machineKey32on64 = 'HKLM:\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\*'\n$exitCode = 0\n\ntry {\n [array]$uninstallKeys = Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |\n ForEach-Object { Get-ItemProperty $_.PSPath }\n\n $foundUninstaller = $false\n foreach ($key in $uninstallKeys) {\n if ($key.DisplayName -like $softwareNameLike) {\n $foundUninstaller = $true\n $uninstallCommand = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }\n if ($uninstallCommand -match '^\\s*\"([^\"]+)\"\\s*(.*)$') {\n $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = \"$($Matches[2]) $uninstallArgs\".Trim() }\n } elseif ($uninstallCommand -match '(?i)^\\s*(.+?\\.exe)\\s*(.*)$') {\n $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = \"$($Matches[2]) $uninstallArgs\".Trim() }\n }\n Write-Host \"Uninstall command: $uninstallCommand\"; Write-Host \"Uninstall args: $uninstallArgs\"\n $processOptions = @{ FilePath = $uninstallCommand; PassThru = $true; Wait = $true }\n if ($uninstallArgs -ne '') { $processOptions.ArgumentList = $uninstallArgs }\n $process = Start-Process @processOptions\n $exitCode = $process.ExitCode; Write-Host \"Uninstall exit code: $exitCode\"; break\n }\n }\n if (-not $foundUninstaller) { Write-Host \"Uninstaller for '$softwareName' not found.\"; Exit 1 }\n} catch { Write-Host \"Error: $_\"; Exit 1 }\n\nExit $exitCode\n", |
Member
Author
There was a problem hiding this comment.
Regenerated — the ref now exits 0 when no entry is found. run 30372911136 passes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #50027 +/- ##
==========================================
+ Coverage 67.98% 68.04% +0.06%
==========================================
Files 3922 3930 +8
Lines 250086 250300 +214
Branches 13315 13233 -82
==========================================
+ Hits 170014 170319 +305
+ Misses 64776 64684 -92
- Partials 15296 15297 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 tasks
Addresses Copilot's review, and this one was a real bug. The script allowed 600s for the install plus 120s polling for registration, but the caller kills the whole script at 10 minutes -- so the worst case would have been killed part-way through the recovery path this script exists to provide. Now 420 + 120, with headroom. Reading .ExitCode straight after Stop-Process could also throw if the process had not died yet, failing the script even when the install succeeded. Now it waits for the kill to land and treats registration as the authoritative signal, since a killed process's exit code means nothing. Uninstall gets the publisher predicate (verified 'Microsoft Corporation' from the PE version resource), the bare-token UninstallString fallback, -ErrorAction SilentlyContinue, and Exit 0 when no entry is found.
Contributor
Script Diff Resultsee/maintained-apps/outputs/azure-data-studio/windows.json=== Install // 29e6eb61 -> c9dbf666 ===
--- /tmp/old.nVl3pO 2026-07-28 15:22:14.199664496 +0000
+++ /tmp/new.R16Mqx 2026-07-28 15:22:14.199664496 +0000
@@ -10,7 +10,12 @@
# indefinitely. "/MERGETASKS=!runcode" suppresses the launch (the same switch
# vscode_install.ps1 and vscodium_install.ps1 use); waiting on the installer
# process alone, then stopping any stray app process, covers the rest.
-$installTimeoutSeconds = 600
+# Budget: the caller kills the whole script at 10 minutes
+# (cmd/maintained-apps/validate/windows.go), so the worst case here -- full install
+# wait, then the registration wait, plus process overhead -- has to stay under
+# that. 420 + 120 leaves headroom; 600 + 120 would have been killed mid-recovery,
+# in exactly the case this script exists to handle.
+$installTimeoutSeconds = 420
$registrationTimeoutSeconds = 120
$machineKey = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\*'
@@ -32,14 +37,23 @@
# Start-Process -PassThru otherwise returns $null for .ExitCode.
$null = $process.Handle
+$killed = $false
if (-not $process.WaitForExit($installTimeoutSeconds * 1000)) {
Write-Host "Installer process did not exit within ${installTimeoutSeconds}s, stopping it."
Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue
- Start-Sleep -Seconds 2
+ # Wait for the kill to land: reading .ExitCode while the process is still alive
+ # throws, which would fail the script even though the install may have succeeded.
+ $null = $process.WaitForExit(30 * 1000)
+ $killed = $true
}
-$exitCode = $process.ExitCode
-Write-Host "Install exit code: $exitCode"
+$exitCode = $null
+if ($process.HasExited) {
+ $exitCode = $process.ExitCode
+ Write-Host "Install exit code: $exitCode"
+} else {
+ Write-Host "Installer process could not be stopped; falling back to the registration check."
+}
# The installer can return before the Add/Remove Programs entry is written; wait
# for it so software inventory sees a complete install.
@@ -58,6 +72,10 @@
Exit 1
}
+# Registration is the authoritative success signal above. If the installer had to
+# be killed, or never reported a code, don't fail on a code that means nothing.
+if ($killed -or $null -eq $exitCode) { Exit 0 }
+
# 3010 (reboot required) and 1641 (reboot initiated) are successful installs.
if ($exitCode -eq 3010 -or $exitCode -eq 1641) { Exit 0 }
=== Uninstall // 23fce27b -> fc30d6c0 ===
--- /tmp/old.ATgIKK 2026-07-28 15:22:14.224664489 +0000
+++ /tmp/new.mYUtLL 2026-07-28 15:22:14.224664489 +0000
@@ -1,5 +1,11 @@
$softwareName = "Azure Data Studio"
-$softwareNameLike = "*$softwareName*"
+$softwareNameLike = "$softwareName*"
+# Require the publisher too, mirroring the manifest's exists query. Verified as
+# "Microsoft Corporation" against the installer's PE version resource
+# (CompanyName). Requiring it here puts the value under test -- the validator's
+# appExists looks up by name only, so a wrong exists-query publisher would
+# otherwise ship undetected (cf. the Spyder finding in #50016).
+$softwarePublisher = "Microsoft Corporation"
$uninstallArgs = "/VERYSILENT /SUPPRESSMSGBOXES /NORESTART"
$machineKey = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\*'
@@ -8,17 +14,19 @@
try {
[array]$uninstallKeys = Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue |
- ForEach-Object { Get-ItemProperty $_.PSPath }
+ ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue }
$foundUninstaller = $false
foreach ($key in $uninstallKeys) {
- if ($key.DisplayName -like $softwareNameLike) {
+ if ($key.DisplayName -like $softwareNameLike -and $key.Publisher -eq $softwarePublisher) {
$foundUninstaller = $true
$uninstallCommand = if ($key.QuietUninstallString) { $key.QuietUninstallString } else { $key.UninstallString }
if ($uninstallCommand -match '^\s*"([^"]+)"\s*(.*)$') {
$uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = "$($Matches[2]) $uninstallArgs".Trim() }
} elseif ($uninstallCommand -match '(?i)^\s*(.+?\.exe)\s*(.*)$') {
$uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = "$($Matches[2]) $uninstallArgs".Trim() }
+ } elseif ($uninstallCommand -match '^\s*(\S+)\s*(.*)$') {
+ $uninstallCommand = $Matches[1]; if ($Matches[2]) { $uninstallArgs = "$($Matches[2]) $uninstallArgs".Trim() }
}
Write-Host "Uninstall command: $uninstallCommand"; Write-Host "Uninstall args: $uninstallArgs"
$processOptions = @{ FilePath = $uninstallCommand; PassThru = $true; Wait = $true }
@@ -27,7 +35,9 @@
$exitCode = $process.ExitCode; Write-Host "Uninstall exit code: $exitCode"; break
}
}
- if (-not $foundUninstaller) { Write-Host "Uninstaller for '$softwareName' not found."; Exit 1 }
+ # Nothing to remove is not a failure: uninstall scripts are idempotent here, as
+ # in nordpass_uninstall.ps1 and windsurf_uninstall.ps1.
+ if (-not $foundUninstaller) { Write-Host "Uninstall entry not found for '$softwareName'."; Exit 0 }
} catch { Write-Host "Error: $_"; Exit 1 }
Exit $exitCode |
| function Test-AzureDataStudioRegistered { | ||
| $null -ne (Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue | | ||
| ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } | | ||
| Where-Object { $_.DisplayName -like "Azure Data Studio*" } | |
Member
Author
There was a problem hiding this comment.
Deliberately leaving these different, but the underlying risk is covered.
The registration poll answers a different question from the exists query: "did the installer actually finish, or did it die part-way?" Tightening it to an exact name + publisher match would make it fail if the installer writes DisplayName and Publisher in separate registry operations, which would reintroduce exactly the false-failure this script was written to eliminate.
On the identity risk itself — both halves are verified rather than assumed:
- DisplayName: the validator's own osquery output reports
Found app: 'Azure Data Studio' at C:\Program Files\Azure Data Studio\, Version: 1.52.0— exactly the string the exists query matches. - Publisher:
Microsoft Corporation, verified from the installer's PE version resource, and now enforced by the uninstall script's predicate — so if it were wrong, the uninstall would fail in CI rather than shipping.
So a publisher mismatch can no longer ship undetected, which was the substance of the concern.
Contributor
WalkthroughAdded Azure Data Studio as a maintained Windows application at version 1.52.0, including catalog metadata, download integrity data, detection queries, and embedded PowerShell install/uninstall scripts. The installer handles silent execution, timeouts, registration polling, process cleanup, and exit codes. The uninstaller discovers matching registry entries and runs silently. A frontend SVG icon component and software-name mapping were also added. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1`:
- Around line 24-28: Use the manifest’s exact registry identity throughout the
Azure Data Studio scripts: update Test-AzureDataStudioRegistered in
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1 to
require DisplayName equal to $softwareName and Publisher equal to
$softwarePublisher; remove the wildcard identity variable at line 2 of
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1 and
make its uninstall filter require DisplayName equal to $softwareName at line 21.
Regenerate the embedded scripts and ref IDs in
ee/maintained-apps/outputs/azure-data-studio/windows.json lines 19-20.
- Around line 67-68: Update azure_data_studio_install.ps1 to snapshot existing
Azure Data Studio process IDs before installation and restrict fallback cleanup
to only processes created during the current install; never force-close
pre-existing user sessions. Regenerate the embedded install script and ref ID in
ee/maintained-apps/outputs/azure-data-studio/windows.json at line 19 to reflect
the script change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f538766e-d9e0-4144-b33a-2385a22193a2
⛔ Files ignored due to path filters (1)
website/assets/images/app-icon-azure-data-studio-60x60@2x.pngis excluded by!**/*.png
📒 Files selected for processing (7)
ee/maintained-apps/inputs/winget/azure-data-studio.jsonee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1ee/maintained-apps/outputs/apps.jsonee/maintained-apps/outputs/azure-data-studio/windows.jsonfrontend/pages/SoftwarePage/components/icons/AzureDataStudio.tsxfrontend/pages/SoftwarePage/components/icons/index.ts
Comment on lines
+24
to
+28
| function Test-AzureDataStudioRegistered { | ||
| $null -ne (Get-ChildItem -Path @($machineKey, $machineKey32on64) -ErrorAction SilentlyContinue | | ||
| ForEach-Object { Get-ItemProperty $_.PSPath -ErrorAction SilentlyContinue } | | ||
| Where-Object { $_.DisplayName -like "Azure Data Studio*" } | | ||
| Select-Object -First 1) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the manifest’s exact registry identity everywhere.
The manifest requires exact Azure Data Studio / Microsoft Corporation, but install registration accepts any Azure Data Studio* entry and uninstall can remove any publisher-matching prefix match. A timeout fallback can therefore report success without the manifest-detected product, or uninstall a different product.
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1#L24-L28: requireDisplayName -eq $softwareNameandPublisher -eq $softwarePublisher.ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1#L2-L2: remove the wildcard identity variable.ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1#L21-L21: useDisplayName -eq $softwareName.ee/maintained-apps/outputs/azure-data-studio/windows.json#L19-L20: regenerate the embedded scripts and ref IDs after updating the sources.
📍 Affects 3 files
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1#L24-L28(this comment)ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1#L2-L2ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1#L21-L21ee/maintained-apps/outputs/azure-data-studio/windows.json#L19-L20
🤖 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/azure_data_studio_install.ps1`
around lines 24 - 28, Use the manifest’s exact registry identity throughout the
Azure Data Studio scripts: update Test-AzureDataStudioRegistered in
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1 to
require DisplayName equal to $softwareName and Publisher equal to
$softwarePublisher; remove the wildcard identity variable at line 2 of
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_uninstall.ps1 and
make its uninstall filter require DisplayName equal to $softwareName at line 21.
Regenerate the embedded scripts and ref IDs in
ee/maintained-apps/outputs/azure-data-studio/windows.json lines 19-20.
Comment on lines
+67
to
+68
| # Belt and braces in case a future build ignores !runcode. | ||
| Stop-Process -Name "azuredatastudio" -Force -ErrorAction SilentlyContinue |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not force-close an existing user session.
This runs after every install, not only after detecting an installer-launched process. It can forcibly terminate an already-running Azure Data Studio instance and discard unsaved work. Snapshot existing PIDs before installation and only stop newly created fallback processes.
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1#L67-L68: limit cleanup to processes created during this install.ee/maintained-apps/outputs/azure-data-studio/windows.json#L19-L19: regenerate the embedded install script and ref ID.
📍 Affects 2 files
ee/maintained-apps/inputs/winget/scripts/azure_data_studio_install.ps1#L67-L68(this comment)ee/maintained-apps/outputs/azure-data-studio/windows.json#L19-L19
🤖 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/azure_data_studio_install.ps1`
around lines 67 - 68, Update azure_data_studio_install.ps1 to snapshot existing
Azure Data Studio process IDs before installation and restrict fallback cleanup
to only processes created during the current install; never force-close
pre-existing user sessions. Regenerate the embedded install script and ref ID in
ee/maintained-apps/outputs/azure-data-studio/windows.json at line 19 to reflect
the script change.
Contributor
Script Diff Resultsee/maintained-apps/outputs/azure-data-studio/windows.json=== Install // c9dbf666 -> 8c1c97a1 ===
--- /tmp/old.1BH0mP 2026-07-28 17:44:37.481627991 +0000
+++ /tmp/new.Ann0cX 2026-07-28 17:44:37.482627992 +0000
@@ -3,18 +3,10 @@
$exeFilePath = "${env:INSTALLER_PATH}"
-# Azure Data Studio is a Visual Studio Code fork, so it ships the same Inno Setup
-# script -- including the "runcode" task, which launches the app once the install
-# finishes. PowerShell's "Start-Process -Wait" waits for the process *and all of
-# its descendants*, so the launched app kept the install script blocked
-# indefinitely. "/MERGETASKS=!runcode" suppresses the launch (the same switch
-# vscode_install.ps1 and vscodium_install.ps1 use); waiting on the installer
-# process alone, then stopping any stray app process, covers the rest.
-# Budget: the caller kills the whole script at 10 minutes
-# (cmd/maintained-apps/validate/windows.go), so the worst case here -- full install
-# wait, then the registration wait, plus process overhead -- has to stay under
-# that. 420 + 120 leaves headroom; 600 + 120 would have been killed mid-recovery,
-# in exactly the case this script exists to handle.
+# ADS is a VS Code fork with the same Inno script, including the "runcode" task
+# that launches the app after install. -Wait waits on descendants, so that would
+# block forever; "/MERGETASKS=!runcode" suppresses the launch, as in
+# vscode_install.ps1. Timeouts are sized to stay under the caller's 10-minute cap.
$installTimeoutSeconds = 420
$registrationTimeoutSeconds = 120
@@ -33,16 +25,14 @@
$process = Start-Process -FilePath "$exeFilePath" `
-ArgumentList "/VERYSILENT /SUPPRESSMSGBOXES /NORESTART /MERGETASKS=!runcode" `
-PassThru
-# Touch .Handle so the exit code is still readable after the process ends:
-# Start-Process -PassThru otherwise returns $null for .ExitCode.
+# Keeps .ExitCode readable after the process ends.
$null = $process.Handle
$killed = $false
if (-not $process.WaitForExit($installTimeoutSeconds * 1000)) {
Write-Host "Installer process did not exit within ${installTimeoutSeconds}s, stopping it."
Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue
- # Wait for the kill to land: reading .ExitCode while the process is still alive
- # throws, which would fail the script even though the install may have succeeded.
+ # Reading .ExitCode while the process is alive would throw.
$null = $process.WaitForExit(30 * 1000)
$killed = $true
}
@@ -55,8 +45,7 @@
Write-Host "Installer process could not be stopped; falling back to the registration check."
}
-# The installer can return before the Add/Remove Programs entry is written; wait
-# for it so software inventory sees a complete install.
+# The installer can return before the ARP entry is written.
$elapsed = 0
while (-not (Test-AzureDataStudioRegistered) -and ($elapsed -lt $registrationTimeoutSeconds)) {
Start-Sleep -Seconds 5
@@ -64,7 +53,7 @@
Write-Host "Waiting for Azure Data Studio to register... ($elapsed seconds)"
}
-# Belt and braces in case a future build ignores !runcode.
+# In case a future build ignores !runcode.
Stop-Process -Name "azuredatastudio" -Force -ErrorAction SilentlyContinue
if (-not (Test-AzureDataStudioRegistered)) {
@@ -72,8 +61,7 @@
Exit 1
}
-# Registration is the authoritative success signal above. If the installer had to
-# be killed, or never reported a code, don't fail on a code that means nothing.
+# Registration above is the success signal; a killed process's code means nothing.
if ($killed -or $null -eq $exitCode) { Exit 0 }
# 3010 (reboot required) and 1641 (reboot initiated) are successful installs.
=== Uninstall // fc30d6c0 -> 573526d3 ===
--- /tmp/old.ko4dRc 2026-07-28 17:44:37.508628011 +0000
+++ /tmp/new.hGE8fk 2026-07-28 17:44:37.508628011 +0000
@@ -1,10 +1,5 @@
$softwareName = "Azure Data Studio"
$softwareNameLike = "$softwareName*"
-# Require the publisher too, mirroring the manifest's exists query. Verified as
-# "Microsoft Corporation" against the installer's PE version resource
-# (CompanyName). Requiring it here puts the value under test -- the validator's
-# appExists looks up by name only, so a wrong exists-query publisher would
-# otherwise ship undetected (cf. the Spyder finding in #50016).
$softwarePublisher = "Microsoft Corporation"
$uninstallArgs = "/VERYSILENT /SUPPRESSMSGBOXES /NORESTART"
@@ -35,8 +30,6 @@
$exitCode = $process.ExitCode; Write-Host "Uninstall exit code: $exitCode"; break
}
}
- # Nothing to remove is not a failure: uninstall scripts are idempotent here, as
- # in nordpass_uninstall.ps1 and windsurf_uninstall.ps1.
if (-not $foundUninstaller) { Write-Host "Uninstall entry not found for '$softwareName'."; Exit 0 }
} catch { Write-Host "Error: $_"; Exit 1 } |
allenhouchins
changed the title
fleet-release
approved these changes
Jul 29, 2026
This was referenced Jul 31, 2026
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
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.
Related issue: #50020
What this does
Adds Azure Data Studio as a Windows Fleet-maintained app. One of the 11 apps split out of #48501 that failed the FMA validator; #50016 shipped the 6 that passed.
Why it was failing
The install itself worked — the validator logged
New application detected at: C:\Program Files\Azure Data Studio. The script never returned:Ten minutes on the nose is the validator's
executeScripttimeout. Azure Data Studio is a Visual Studio Code fork and ships the same Inno Setup script — including theruncodetask, which launches the app when the install finishes. BecauseStart-Process -Waitwaits for the process and all of its descendants, the launched app kept the script blocked forever.The fix is the switch VS Code's own FMA already uses:
/MERGETASKS=!runcode(seevscode_install.ps1andvscodium_install.ps1, both of which pass validation). The script also waits on the installer process alone rather than its descendants, polls for the Add/Remove Programs entry, and stops a strayazuredatastudioprocess as a backstop in case a future build ignores the task suppression.Notes
DisplayName(Azure Data Studio), so exact name matching. PublisherMicrosoft Corporation.-Waitwaiting on descendants is the desired behavior there (Inno relaunches itself from%TEMP%).Checklist for submitter
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
All checks passed)apps.jsonis valid JSON with a description filled in.Summary by CodeRabbit