Skip to content

Retry transient Windows bundle directory moves#18839

Open
danegsta wants to merge 2 commits into
mainfrom
danegsta-retry-bundle-directory-move
Open

Retry transient Windows bundle directory moves#18839
danegsta wants to merge 2 commits into
mainfrom
danegsta-retry-bundle-directory-move

Conversation

@danegsta

Copy link
Copy Markdown
Member

Description

Freshly extracted bundle files can be opened briefly by antivirus, indexing, or other automated scanners on Windows. When those handles do not share delete access, the atomic move from the temporary extraction directory to the active version directory fails immediately.

Retry only the bundle promotion move on Windows using the CLI's existing bounded exponential-backoff pattern. Retries are limited to the Win32 access-denied, sharing-violation, and lock-violation HRESULTs so deterministic failures such as a full disk or an existing destination still fail immediately. Tests cover the error classification, non-Windows behavior, and promotion after a temporary Windows file lock is released.

Fixes #18275

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d0bf13f7-53de-4d9d-951d-cf6bcde7e820
Copilot AI review requested due to automatic review settings July 20, 2026 23:43
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18839

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18839"

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 20, 2026
@github-actions

This comment has been minimized.

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

Adds bounded retries for transient Windows bundle-promotion failures.

Changes:

  • Classifies retryable Windows lock errors.
  • Retries directory promotion with exponential backoff.
  • Adds unit and Windows integration coverage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/Aspire.Cli/Bundles/BundleService.cs Implements promotion retries.
tests/Aspire.Cli.Tests/BundleServiceTests.cs Tests error classification.
tests/Aspire.Cli.Tests/BundleServiceIntegrationTests.cs Tests promotion after releasing a lock.

Comment thread src/Aspire.Cli/Bundles/BundleService.cs
@danegsta

Copy link
Copy Markdown
Member Author

PR Testing Report

PR Information

Artifact Version Verification

  • Expected Commit: a03480c
  • Installed Version: 13.5.0-pr.18839.ga03480cf
  • Installed CLI: $(System.Collections.Hashtable['cliPath'])
  • Status: ✅ Verified - installed CLI version contains $shortSha

Changes Analyzed

Files Changed

  • src/Aspire.Cli/Bundles/BundleService.cs - Modified
  • ests/Aspire.Cli.Tests/BundleServiceIntegrationTests.cs - Modified
  • ests/Aspire.Cli.Tests/BundleServiceTests.cs - Modified

Change Categories

  • CLI changes detected - retries transient Windows directory move failures during bundle promotion
  • Hosting integration changes
  • Dashboard changes
  • Template changes
  • Client/component changes
  • VS Code extension changes
  • CI infrastructure changes
  • Test changes detected - HRESULT classification and Windows temporary file-lock integration coverage

Test Scenarios Executed

Scenario 1: PR CLI artifact install/version verification

Objective: Install the dogfood CLI artifact into an isolated Windows temp directory and verify it matches the PR head commit.
Coverage Type: Artifact verification
Status: ✅ Passed

Steps:

  1. Ran the PR dogfood PowerShell installer with -InstallPath, -SkipExtension, and -SkipPath.
  2. Executed the installed binary directly: $(System.Collections.Hashtable['cliPath']) --version.
  3. Compared the reported version against PR head commit $(System.Collections.Hashtable['expectedCommit']).

Evidence:

  • Install log: $(System.Collections.Hashtable['installLog'])
  • Installed version: $(System.Collections.Hashtable['cliVersion'])

Observations:


Scenario 2: Bundle-backed template happy path

Objective: Validate the installed PR CLI can create a new starter project from the PR hive using non-interactive template flags.
Coverage Type: Happy path
Status: ✅ Passed

Steps:

  1. Created a fresh temp scenario directory.
  2. Ran �spire new aspire-starter with explicit --source, --version, --test-framework None, --use-redis-cache false, --localhost-tld false, and --suppress-agent-init.
  3. Verified the generated AppHost project exists.

Evidence:

  • Generated app: $(System.Collections.Hashtable['templateAppRoot'])
  • Command log: $(System.Collections.Hashtable['templateHappyLog'])

Observations:

  • Template version 13.5.0-pr.18839.ga03480cf was selected from the PR hive.
  • Project creation completed successfully.

Scenario 3: AppHost start/wait/describe/stop smoke

Objective: Exercise the installed PR CLI through AppHost orchestration after project creation and verify generated resources reach healthy state.
Coverage Type: Happy path / bundle-use smoke
Status: ✅ Passed

Steps:

  1. Started the generated AppHost with the installed PR CLI.
  2. Waited for �piservice and webfrontend to become healthy.
  3. Ran �spire describe to capture resource state.
  4. Stopped the AppHost.

Evidence:

  • Start log: $(System.Collections.Hashtable['apphostStartFinalLog'])
  • Wait log (�piservice): $(System.Collections.Hashtable['apphostWaitApiLog'])
  • Wait log (webfrontend): $(System.Collections.Hashtable['apphostWaitWebLog'])
  • Describe log: $(System.Collections.Hashtable['apphostDescribeFinalLog'])
  • Stop log: $(System.Collections.Hashtable['apphostStopFinalLog'])

Observations:

  • �piservice reached Healthy in 2.7s.
  • webfrontend reached Healthy in 5.7s.
  • �spire describe showed both resources Running and Healthy.

Scenario 4: Non-empty output directory unhappy path

Objective: Verify deterministic user errors still fail cleanly and are not masked by retry behavior.
Coverage Type: Unhappy path
Status: ✅ Passed

Steps:

  1. Created a non-empty output directory containing �xisting.txt.
  2. Ran �spire new aspire-starter targeting that non-empty directory.
  3. Verified the command failed with a non-zero exit code and a clear validation message.

Evidence:

  • Command log: $(System.Collections.Hashtable['negativeLog'])
  • Exit code: $(System.Collections.Hashtable['negativeExitCode'])

Expected Unhappy-Path Outcome: Non-zero exit code with a clear message that the output directory already exists and is not empty.

Observations:

  • Command failed with exit code $(System.Collections.Hashtable['negativeExitCode']) and reported that the output directory already exists and is not empty.

Scenario 5: Targeted Windows source tests

Objective: Directly exercise the PR's new retry classification and Windows file-lock behavior in the PR source checkout.
Coverage Type: Unit/integration targeted validation
Status: ✅ Passed

Steps:

  1. Checked out PR Retry transient Windows bundle directory moves #18839 source at commit $(System.Collections.Hashtable['sourceCommit']) into a short path to avoid pre-existing Windows long-path snapshot filenames.
  2. Ran
    estore.cmd from the PR checkout.
  3. Ran dotnet test for the changed BundleServiceTests and BundleServiceIntegrationTests methods with quarantined and outerloop tests excluded.

Evidence:

  • Source checkout: $(System.Collections.Hashtable['sourceDir'])
  • Restore log: $(System.Collections.Hashtable['sourceRestoreLog'])
  • Test log: $(System.Collections.Hashtable['sourceTestLog'])

Observations:

  • Build succeeded with 0 warnings and 0 errors.
  • Targeted test run passed: 9 total, 0 failed, 9 succeeded, 0 skipped.
  • The Windows lock scenario uses an open file handle without delete sharing, which reproduces the Directory.Move sharing failure class this PR addresses without needing to instrument the dogfood CLI extraction timing.

Summary

Scenario Status Notes
PR CLI artifact install/version verification ✅ Passed Installed version matched $shortSha
Bundle-backed template happy path ✅ Passed Starter project created from PR hive
AppHost start/wait/describe/stop smoke ✅ Passed �piservice and webfrontend healthy
Non-empty output directory unhappy path ✅ Passed Failed safely with exit code $(System.Collections.Hashtable['negativeExitCode'])
Targeted Windows source tests ✅ Passed 9 targeted tests passed

Overall Result

✅ PR VERIFIED

Recommendations

  • No PR changes recommended based on this test pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d0bf13f7-53de-4d9d-951d-cf6bcde7e820
Copilot AI review requested due to automatic review settings July 21, 2026 01:29

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

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

@github-actions

Copy link
Copy Markdown
Contributor

Tests selector (audit mode)

The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement.

2 / 100 test projects · 5 jobs, from 3 changed files.

Selected test projects (2 / 100)

Aspire.Cli.EndToEnd.Tests, Aspire.Cli.Tests

Selected jobs (5)

cli-starter, deployment-e2e, extension-e2e, polyglot, typescript-api-compat


How these were chosen — grouped by what changed

📦 affected project Aspire.Cli
1 test: Aspire.Cli.EndToEnd.Tests

🧪 tests/Aspire.Cli.Tests/BundleServiceIntegrationTests.cs (changed test)
1 directly: Aspire.Cli.Tests

🧪 tests/Aspire.Cli.Tests/BundleServiceTests.cs (changed test)
1 directly: Aspire.Cli.Tests

Job reasons

Job Triggered by
cli-starter • affected project Aspire.Cli
• selected test Aspire.Cli.Tests
deployment-e2e affected project Aspire.Cli
extension-e2e src/Aspire.Cli/Bundles/BundleService.cs, tests/Aspire.Cli.Tests/BundleServiceIntegrationTests.cs, tests/Aspire.Cli.Tests/BundleServiceTests.cs
• affected project Aspire.Cli
polyglot affected project Aspire.Cli
typescript-api-compat affected project Aspire.Cli

Selection computed for commit 1b83899.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@IEvangelist

Copy link
Copy Markdown
Member

PR Testing Report

PR Information

Artifact Version Verification

  • Expected Commit: 1b83899
  • Installed Version: N/A — tested the source checkout at the PR head commit (worktree at HEAD)
  • Status: ✅ Verified

Changes Analyzed

Files Changed

  • src/Aspire.Cli/Bundles/BundleService.cs — Modified (+61/-1)
  • tests/Aspire.Cli.Tests/BundleServiceTests.cs — Modified (+28)
  • tests/Aspire.Cli.Tests/BundleServiceIntegrationTests.cs — Modified (+29)

Change Categories

  • CLI changes detected (bundle promotion move-with-retry)
  • Hosting integration changes
  • Dashboard changes
  • CI infrastructure changes
  • VS Code extension changes
  • Test changes

What the change does

  • Adds MoveDirectoryWithRetryAsync that wraps the bundle-promotion Directory.Move(tempDir → activeVersionDir) in a bounded exponential-backoff retry (100ms → cap 1s, max 3s elapsed).
  • Retries only on Windows and only for the transient-lock HRESULTs ERROR_ACCESS_DENIED (0x80070005), ERROR_SHARING_VIOLATION (0x80070020), ERROR_LOCK_VIOLATION (0x80070021), via IsRetryableDirectoryMoveException. Deterministic failures (disk full, already-exists) fail immediately.
  • Adds an OperationCanceledException handler that deletes the temp dir and rethrows, so a canceled promotion doesn't leak the extraction directory.

Rationale for Test Approach

The fix targets a Windows race where an AV scanner/indexer briefly opens a freshly-extracted bundle file without delete-sharing, making the atomic promotion Directory.Move fail. This cannot be reliably reproduced through a dogfood aspire new/aspire run install (it needs a scanner holding a handle at the exact wrong moment). The high-value validation is to build Aspire.Cli and run the PR's own unit + integration tests directly on a Windows host — the integration test is [SkipOnPlatform] for non-Windows, so Windows is the only place it exercises the retry path.

Test Scenarios Executed

Scenario 1: HRESULT classification (unit)

Objective: Only the three transient Windows lock HRESULTs are retryable; deterministic errors are not.
Coverage Type: Happy path + boundary/negative
Status: ✅ Passed
Details: IsRetryableDirectoryMoveException_OnlyRetriesTransientWindowsLockErrors theory — retryable: 0x80070005, 0x80070020, 0x80070021; non-retryable: 0x80070027 (ERROR_HANDLE_DISK_FULL), 0x80070070 (ERROR_DISK_FULL), 0x800700B7 (ERROR_ALREADY_EXISTS). Also IsRetryableDirectoryMoveException_RetriesUnauthorizedAccess (UnauthorizedAccessException retries).

Scenario 2: Platform guard (unit)

Objective: Retry logic never engages off Windows.
Coverage Type: Negative / platform
Status: ✅ Passed
Details: IsRetryableDirectoryMoveException_DoesNotRetryOnNonWindows — a sharing-violation IOException with isWindows: false returns false.

Scenario 3: End-to-end retry after a temporary Windows file lock (integration)

Objective: A move blocked by an open, non-delete-shared file retries and then succeeds once the lock is released.
Coverage Type: Happy path (real Windows file-sharing semantics)
Status: ✅ Passed
Details: MoveDirectoryWithRetryAsync_FileTemporarilyLocked_RetriesUntilReleased opens locked.dll with FileShare.Read (no delete share), starts the move (asserts it does NOT complete while locked), disposes the lock, awaits completion, and asserts the source is gone and destination exists. Not skipped on this Windows host.
Flakiness check: Ran 3 additional times → 3/3 passed (715–828ms), no timing flakiness on the Assert.False(moveTask.IsCompleted) assertion.

Scenario 4: Regression — full BundleService suites

Objective: The Directory.Move refactor doesn't break existing extraction/promotion/cleanup behavior.
Coverage Type: Regression
Status: ✅ Passed
Details: Full BundleServiceTests + BundleServiceIntegrationTests — 35 total, 35 passed, 0 failed, 0 skipped.

Commands & Evidence

  • Restore: ./restore.cmd → Build succeeded, 0 warnings/0 errors.
  • Targeted new tests (build + run): 9 total, 9 passed, 0 skipped (6 theory cases + 2 unit + 1 Windows integration).
  • Full suites: --filter-class *.BundleServiceTests --filter-class *.BundleServiceIntegrationTests → 35/35 passed.
  • Integration test 3× rerun: 3/3 passed.
  • All runs excluded quarantined=true and outerloop=true per repo policy.

Summary

Scenario Status Notes
HRESULT classification (unit) ✅ Passed Retryable vs. deterministic HRESULTs correct
Non-Windows / platform guard ✅ Passed No retry off Windows
Windows temporary-lock integration ✅ Passed Ran on Windows (not skipped); 3/3 rerun stable
Full BundleService regression ✅ Passed 35/35, no regression

Overall Result

✅ PR VERIFIED

Observations / Recommendations

  • Behavior is well-scoped: retry is gated on both environment.IsWindows() and the exact transient HRESULT set, so deterministic failures (disk full, already-exists) still fail fast. Bounded to ≤3s elapsed with a 1s delay cap — no unbounded waiting.
  • Cancellation path is handled (temp dir cleaned up, exception rethrown), matching the second commit "Clean up canceled bundle promotion."
  • No public API added; test coverage matches the described scenarios (classification, non-Windows, and real lock-release promotion).
  • No issues found.

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

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BundleService: MoveDirectory fails with "Access denied" when migrating from legacy layout to versioned layout on Windows

3 participants