Skip to content

Fix install script swallowing file-lock errors and reporting false success #15419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 7 commits into
base: main
Choose a base branch
from
+49 −32
Draft

Fix install script swallowing file-lock errors and reporting false success #15419

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7362786
Initial plan
Copilot Mar 20, 2026
ac00fa2
Fix silent error handling in PowerShell install scripts causing false…
Copilot Mar 20, 2026
5f99dc6
Fix installer PowerShell error handling
radical Mar 20, 2026
1830f02
Handle installer cleanup errors
radical Mar 20, 2026
c2efe75
Align PowerShell installer backup semantics
radical Mar 20, 2026
8056486
Merge remote-tracking branch 'origin/main' into copilot/fix-aspire-in…
radical Mar 22, 2026
3518336
Merge remote-tracking branch 'refs/remotes/origin/copilot/fix-aspire-…
radical Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions eng/scripts/get-aspire-cli-pr.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,12 @@ function Backup-ExistingCliExecutable {
Write-Message "Backing up existing CLI: $TargetExePath -> $backupPath" -Level Verbose

# Rename existing executable to .old.[timestamp]
Move-Item -Path $TargetExePath -Destination $backupPath -Force
try {
Move-Item -Path $TargetExePath -Destination $backupPath -Force -ErrorAction Stop
}
catch {
throw "Failed to back up existing CLI at '$TargetExePath'. The file may be in use by another process. Please close any running Aspire CLI instances and try again. Error: $($_.Exception.Message)"
}
return $backupPath
}
}
Expand All @@ -427,7 +432,7 @@ function Restore-CliExecutableFromBackup {
Remove-Item -Path $TargetExePath -Force -ErrorAction SilentlyContinue
}

Move-Item -Path $BackupPath -Destination $TargetExePath -Force
Move-Item -Path $BackupPath -Destination $TargetExePath -Force -ErrorAction Stop
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Restore-CliExecutableFromBackup now uses Move-Item -ErrorAction Stop, which will throw if the restore fails (e.g., the target is still locked). In Expand-AspireCliArchive the restore call in the catch block isn’t wrapped, so a restore failure will abort the catch block and can hide the original extraction error. Consider handling restore failures inside Expand-AspireCliArchive so the final error surfaced to the user retains the primary failure cause while still reporting any restore failure.

Suggested change
Move-Item -Path $BackupPath -Destination $TargetExePath -Force -ErrorAction Stop
try {
Move-Item -Path $BackupPath -Destination $TargetExePath -Force -ErrorAction Stop
}
catch {
# Do not rethrow here: this runs during error recovery and should not mask the original failure.
Write-Message "Failed to restore CLI from backup '$BackupPath' to '$TargetExePath'. The backup file may be in use or the target may be locked. Error: $($_.Exception.Message)" -Level Warning
}

Copilot uses AI. Check for mistakes.
}
}

Expand Down Expand Up @@ -482,7 +487,7 @@ function Expand-AspireCliArchive {
# Create destination directory if it doesn't exist
if (-not (Test-Path $DestinationPath)) {
Write-Message "Creating destination directory: $DestinationPath" -Level Verbose
New-Item -ItemType Directory -Path $DestinationPath -Force | Out-Null
New-Item -ItemType Directory -Path $DestinationPath -Force -ErrorAction Stop | Out-Null
}
else {
# Backup existing executable before extraction
Expand All @@ -498,7 +503,7 @@ function Expand-AspireCliArchive {
throw "Expand-Archive cmdlet not found. Please use PowerShell 5.0 or later to extract ZIP files."
}

Expand-Archive -Path $ArchiveFile -DestinationPath $DestinationPath -Force
Expand-Archive -Path $ArchiveFile -DestinationPath $DestinationPath -Force -ErrorAction Stop
}
elseif ($ArchiveFile -match "\.tar\.gz$") {
# Use tar for tar.gz files
Expand Down Expand Up @@ -657,7 +662,7 @@ function New-TempDirectory {

Write-Message "Creating temporary directory: $tempDir" -Level Verbose
try {
New-Item -ItemType Directory -Path $tempDir -Force | Out-Null
New-Item -ItemType Directory -Path $tempDir -Force -ErrorAction Stop | Out-Null
return $tempDir
}
catch {
Expand Down Expand Up @@ -1062,7 +1067,7 @@ function Install-BuiltNugets {
}

if ($PSCmdlet.ShouldProcess($NugetHiveDir, "Create directory")) {
New-Item -ItemType Directory -Path $NugetHiveDir -Force | Out-Null
New-Item -ItemType Directory -Path $NugetHiveDir -Force -ErrorAction Stop | Out-Null
}

Write-Message "Copying nugets from $DownloadDir to $NugetHiveDir" -Level Verbose
Expand All @@ -1078,7 +1083,7 @@ function Install-BuiltNugets {

foreach ($file in $nupkgFiles) {
if ($PSCmdlet.ShouldProcess($file.FullName, "Copy to $NugetHiveDir")) {
Copy-Item $file.FullName -Destination $NugetHiveDir
Copy-Item $file.FullName -Destination $NugetHiveDir -ErrorAction Stop
}
}

Expand Down
22 changes: 15 additions & 7 deletions eng/scripts/get-aspire-cli.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,12 @@ function Backup-ExistingCliExecutable {
Write-Message "Backing up existing CLI: $TargetExePath -> $backupPath" -Level Verbose

# Rename existing executable to .old.[timestamp]
Move-Item -Path $TargetExePath -Destination $backupPath -Force
try {
Move-Item -Path $TargetExePath -Destination $backupPath -Force -ErrorAction Stop
}
catch {
throw "Failed to back up existing CLI at '$TargetExePath'. The file may be in use by another process. Please close any running Aspire CLI instances and try again. Error: $($_.Exception.Message)"
Comment on lines 637 to +642
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Backup-ExistingCliExecutable now correctly fails when Move-Item can’t back up a locked executable, but the surrounding wording (“allows installation to proceed even when the CLI is running” / “file can be renamed”) can be misleading because rename may still fail depending on the handle’s sharing mode. Consider updating the nearby comment/message to clarify that the script attempts to rename and will stop with an actionable error if the file can’t be moved (e.g., locked by another process).

Copilot uses AI. Check for mistakes.
}
return $backupPath
}
}
Expand All @@ -659,7 +664,7 @@ function Restore-CliExecutableFromBackup {
Remove-Item -Path $TargetExePath -Force -ErrorAction SilentlyContinue
}

Move-Item -Path $BackupPath -Destination $TargetExePath -Force
Move-Item -Path $BackupPath -Destination $TargetExePath -Force -ErrorAction Stop
}
Comment on lines +669 to 670
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Restore-CliExecutableFromBackup now uses Move-Item -ErrorAction Stop, which will throw if the restore fails (e.g., the target is still locked). In Expand-AspireCliArchive the restore call is not wrapped, so a restore failure will short-circuit the catch block and can mask the original extraction failure. Consider catching restore errors inside Expand-AspireCliArchive and either (a) include both the original extraction error and the restore error in the final thrown message, or (b) log the restore error and still rethrow the original extraction failure.

Copilot uses AI. Check for mistakes.
}

Expand Down Expand Up @@ -711,7 +716,7 @@ function Expand-AspireCliArchive {
# Create destination directory if it doesn't exist
if (-not (Test-Path $DestinationPath)) {
Write-Message "Creating destination directory: $DestinationPath" -Level Verbose
New-Item -ItemType Directory -Path $DestinationPath -Force | Out-Null
New-Item -ItemType Directory -Path $DestinationPath -Force -ErrorAction Stop | Out-Null
}
else {
# Backup existing executable before extraction
Expand All @@ -725,7 +730,7 @@ function Expand-AspireCliArchive {
throw "Expand-Archive cmdlet not found. Please use PowerShell 5.0 or later to extract ZIP files."
}

Expand-Archive -Path $ArchiveFile -DestinationPath $DestinationPath -Force
Expand-Archive -Path $ArchiveFile -DestinationPath $DestinationPath -Force -ErrorAction Stop
}
else {
# Use tar for tar.gz files on Unix systems
Expand All @@ -737,6 +742,9 @@ function Expand-AspireCliArchive {
try {
Set-Location $DestinationPath
& tar -xzf $ArchiveFile
if ($LASTEXITCODE -ne 0) {
throw "tar command failed with exit code $LASTEXITCODE"
Comment on lines 746 to +748
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The thrown tar failure message currently only includes the exit code. Including $ArchiveFile (and ideally tar’s stderr/stdout) would make failures much easier to diagnose, especially when multiple archives/paths are involved.

Suggested change
& tar -xzf $ArchiveFile
if ($LASTEXITCODE -ne 0) {
throw "tar command failed with exit code $LASTEXITCODE"
$tarOutput = & tar -xzf $ArchiveFile 2>&1
$tarExitCode = $LASTEXITCODE
if ($tarExitCode -ne 0) {
$message = "tar command failed for archive '$ArchiveFile' with exit code $tarExitCode."
if ($tarOutput) {
$message += " Output:`n$tarOutput"
}
throw $message

Copilot uses AI. Check for mistakes.
}
}
finally {
Set-Location $currentLocation
Expand Down Expand Up @@ -1003,7 +1011,7 @@ function Install-AspireExtension {
if ($PSCmdlet.ShouldProcess($extractDir, "Extract extension archive")) {
# Expand the zip archive
if ($Script:IsModernPowerShell) {
Expand-Archive -Path $ExtensionArchive -DestinationPath $extractDir -Force
Expand-Archive -Path $ExtensionArchive -DestinationPath $extractDir -Force -ErrorAction Stop
} else {
Add-Type -AssemblyName System.IO.Compression.FileSystem
[System.IO.Compression.ZipFile]::ExtractToDirectory($ExtensionArchive, $extractDir)
Expand Down Expand Up @@ -1166,7 +1174,7 @@ function Install-AspireCli {
if ($PSCmdlet.ShouldProcess($InstallPath, "Create temporary directory")) {
Write-Message "Creating temporary directory: $tempDir" -Level Verbose
try {
New-Item -ItemType Directory -Path $tempDir -Force | Out-Null
New-Item -ItemType Directory -Path $tempDir -Force -ErrorAction Stop | Out-Null
}
catch {
throw "Failed to create temporary directory: $tempDir - $($_.Exception.Message)"
Expand Down Expand Up @@ -1314,7 +1322,7 @@ function Start-AspireCliInstallation {
Write-Message "Creating installation directory: $resolvedInstallPath" -Level Info
if ($PSCmdlet.ShouldProcess($resolvedInstallPath, "Create installation directory")) {
try {
New-Item -ItemType Directory -Path $resolvedInstallPath -Force | Out-Null
New-Item -ItemType Directory -Path $resolvedInstallPath -Force -ErrorAction Stop | Out-Null
}
catch {
throw "Failed to create installation directory: $resolvedInstallPath - $($_.Exception.Message)"
Expand Down
Loading