Bound transient Windows virtual environment removal retries

src/managers/builtin/venvUtils.ts

@@ -785,6 +785,77 @@ async function validateVenvRemovalPath(envPath: string, log: LogOutputChannel):
     return undefined;
 }
 
+function hasRemovalErrorCode(error: unknown, ...codes: string[]): boolean {
+    return (
+        typeof error === 'object' &&
+        error !== null &&
+        'code' in error &&
+        typeof error.code === 'string' &&
+        codes.includes(error.code)
+    );
+}
+
+async function removeVenvDirectory(envPath: string): Promise<void> {
+    if (os.platform() !== 'win32') {
+        return fsapi.remove(envPath);
+    }
+
+    // Bound retries across the entire deletion, not separately at every directory level.
+    const deadline = performance.now() + 500;
+    const target = await fsapi.lstat(envPath, { bigint: true }).catch((error: unknown) => {
+        if (!hasRemovalErrorCode(error, 'ENOENT')) {
+            traceVerbose(`Cannot capture venv directory identity; removal will not be retried: ${error}`);
+        }
+        return undefined;
+    });
+
+    for (let attempt = 1; ; attempt++) {
+        try {
+            await fsapi.remove(envPath);
+            return;
+        } catch (error) {
+            if (
+                !target?.ino ||
+                !hasRemovalErrorCode(error, 'EPERM', 'EBUSY') ||
+                attempt >= 5 ||
+                performance.now() + 100 >= deadline
+            ) {
+                throw error;
+            }
+
+            await new Promise<void>((resolve) => setTimeout(resolve, 100));
+            if (performance.now() >= deadline) {
+                throw error;
+            }
+
+            // A partial deletion can remove pyvenv.cfg; verify directory identity instead.
+            let current;
+            try {
+                current = await fsapi.lstat(envPath, { bigint: true });
+            } catch (identityError) {
+                if (hasRemovalErrorCode(identityError, 'ENOENT')) {
+                    return;
+                }
+                throw identityError;
+            }
+            if (current.dev !== target.dev || current.ino !== target.ino) {
+                throw new Error('Virtual environment directory changed during removal');
+            }
+            if (performance.now() >= deadline) {
+                throw error;
+            }
+        }
+    }
+}
+
+/**
+ * Removes a validated virtual environment after confirmation.
+ * @param environment Environment whose directory should be removed.
+ * @param log Logger for validation and removal failures.
+ * @param options Allows an explicitly headless caller to skip confirmation.
+ * @param backends Environment trackers to update only after deletion succeeds.
+ * @returns Whether deletion and untracking succeeded; false on cancellation or failure.
+ */
 export async function removeVenv(
     environment: PythonEnvironment,
     log: LogOutputChannel,
@@ -825,7 +896,7 @@ export async function removeVenv(
             },
             async () => {
                 try {
-                    await fsapi.remove(envPath);
+                    await removeVenvDirectory(envPath);
                     await backends.uv.untrackEnvironment(environment.environmentPath.fsPath);
                     await backends.venv.untrackEnvironment(environment.environmentPath.fsPath);
                     return true;

src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts

@@ -1,8 +1,10 @@
 import * as assert from 'assert';
-import * as fse from 'fs-extra';
-import * as os from 'os';
+import type { BigIntStats } from 'fs';
+import fse from 'fs-extra';
+import os from 'os';
 import * as path from 'path';
 import * as sinon from 'sinon';
+import { Uri } from 'vscode';
 import { Common } from '../../../common/localize';
 import * as windowApis from '../../../common/window.apis';
 import * as uvEnvironments from '../../../managers/builtin/uvEnvironments';
@@ -155,6 +157,207 @@ suite('venvUtils removeVenv validation integration', () => {
         );
     });
 
+    suite('venvUtils bounded Windows removal', () => {
+        const envPath = Uri.file(path.resolve('workspace', 'project', '.venv')).fsPath;
+        const identity = { dev: 1n, ino: 10n } as BigIntStats;
+        let clock: sinon.SinonFakeTimers;
+        let platform: sinon.SinonStub;
+        let remove: sinon.SinonStub;
+        let lstat: sinon.SinonStub;
+        let pathExists: sinon.SinonStub;
+        let untrack: sinon.SinonStub;
+        let showError: sinon.SinonStub;
+        let log: ReturnType<typeof createMockLogOutputChannel>;
+
+        const sharingError = () => Object.assign(new Error('File is in use'), { code: 'EPERM' });
+        const removeEnvironment = () =>
+            removeVenv(createMockPythonEnvironment({ name: '.venv', envPath }), log, { runHeadless: true });
+
+        setup(() => {
+            clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'performance'] });
+            platform = sinon.stub(os, 'platform').returns('win32');
+            pathExists = sinon.stub(fse, 'pathExists').resolves(true);
+            lstat = sinon.stub(fse, 'lstat').resolves(identity);
+            remove = sinon.stub(fse, 'remove').resolves();
+            untrack = sinon.stub(uvEnvironments, 'removeUvEnvironment').resolves();
+            showError = sinon.stub(windowApis, 'showErrorMessage').resolves(undefined);
+            sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never));
+            log = createMockLogOutputChannel();
+        });
+
+        teardown(() => sinon.restore());
+
+        for (const code of ['EPERM', 'EBUSY']) {
+            test(`recovers from ${code} after partial deletion, then untracks once`, async () => {
+                remove.onFirstCall().callsFake(async () => {
+                    pathExists.resolves(false);
+                    throw Object.assign(new Error('Temporary file lock'), { code });
+                });
+                const pending = removeEnvironment();
+
+                await clock.tickAsync(0);
+                sinon.assert.notCalled(untrack);
+                await clock.tickAsync(100);
+
+                assert.strictEqual(await pending, true);
+                sinon.assert.calledTwice(remove);
+                sinon.assert.calledOnce(pathExists);
+                sinon.assert.calledOnce(untrack);
+                sinon.assert.notCalled(showError);
+            });
+        }
+
+        test('persistent fast failures share one bounded retry window', async () => {
+            const attempts: number[] = [];
+            remove.callsFake(async () => {
+                attempts.push(performance.now());
+                throw sharingError();
+            });
+            const pending = removeEnvironment();
+            await clock.tickAsync(1_000);
+
+            assert.strictEqual(await pending, false);
+            assert.strictEqual(attempts.length, 5);
+            assert.ok(attempts.every((time) => time < 500));
+            sinon.assert.notCalled(untrack);
+            sinon.assert.calledOnce(showError);
+            sinon.assert.calledOnce(log.error as sinon.SinonStub);
+        });
+
+        test('does not retry when the initial deletion already consumed the budget', async () => {
+            remove.callsFake(async () => {
+                await new Promise<void>((resolve) => setTimeout(resolve, 600));
+                throw sharingError();
+            });
+            const pending = removeEnvironment();
+            await clock.tickAsync(1_000);
+
+            assert.strictEqual(await pending, false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(untrack);
+            sinon.assert.calledOnce(showError);
+        });
+
+        test('does not abandon a slow deletion that is still running', async () => {
+            remove.callsFake(async () => {
+                await new Promise<void>((resolve) => setTimeout(resolve, 2_000));
+            });
+            const pending = removeEnvironment();
+            await clock.tickAsync(1_000);
+            sinon.assert.notCalled(untrack);
+            await clock.tickAsync(1_000);
+
+            assert.strictEqual(await pending, true);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.calledOnce(untrack);
+            sinon.assert.notCalled(showError);
+        });
+
+        test('does not start another attempt after a delayed timer exhausts the budget', async () => {
+            remove.rejects(sharingError());
+            const pending = removeEnvironment();
+            await clock.tickAsync(0);
+            sinon.stub(performance, 'now').returns(1_000);
+            await clock.tickAsync(100);
+
+            assert.strictEqual(await pending, false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(untrack);
+        });
+
+        test('does not retry after an identity recheck consumes the remaining budget', async () => {
+            remove.rejects(sharingError());
+            lstat.onSecondCall().callsFake(async () => {
+                await new Promise<void>((resolve) => setTimeout(resolve, 600));
+                return identity;
+            });
+            const pending = removeEnvironment();
+            await clock.tickAsync(1_000);
+
+            assert.strictEqual(await pending, false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(untrack);
+        });
+
+        test('does not delete a replacement directory at the same path', async () => {
+            remove.rejects(sharingError());
+            lstat.onSecondCall().resolves({ ...identity, ino: identity.ino + 1n });
+            const pending = removeEnvironment();
+            await clock.tickAsync(100);
+
+            assert.strictEqual(await pending, false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(untrack);
+            sinon.assert.calledOnce(showError);
+        });
+
+        test('treats a directory removed by another operation as successfully removed', async () => {
+            remove.rejects(sharingError());
+            lstat.onSecondCall().rejects(Object.assign(new Error('Already removed'), { code: 'ENOENT' }));
+            const pending = removeEnvironment();
+            await clock.tickAsync(100);
+
+            assert.strictEqual(await pending, true);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.calledOnce(untrack);
+            sinon.assert.notCalled(showError);
+        });
+
+        test('keeps the original single-attempt behavior outside Windows', async () => {
+            platform.returns('linux');
+            remove.rejects(sharingError());
+
+            assert.strictEqual(await removeEnvironment(), false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(lstat);
+            sinon.assert.notCalled(untrack);
+        });
+
+        test('does not retry non-sharing errors', async () => {
+            remove.rejects(Object.assign(new Error('Permission denied'), { code: 'EACCES' }));
+
+            assert.strictEqual(await removeEnvironment(), false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(untrack);
+            sinon.assert.calledOnce(showError);
+        });
+
+        test('identity lookup failure does not prevent the original deletion attempt', async () => {
+            lstat.rejects(Object.assign(new Error('Identity unavailable'), { code: 'EACCES' }));
+
+            assert.strictEqual(await removeEnvironment(), true);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.calledOnce(untrack);
+        });
+
+        test('does not retry when the filesystem provides no usable directory identity', async () => {
+            lstat.resolves({ ...identity, ino: 0n });
+            remove.rejects(sharingError());
+
+            assert.strictEqual(await removeEnvironment(), false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.notCalled(untrack);
+        });
+
+        test('never retries deletion after an untracking failure', async () => {
+            untrack.rejects(sharingError());
+
+            assert.strictEqual(await removeEnvironment(), false);
+            sinon.assert.calledOnce(remove);
+            sinon.assert.calledOnce(untrack);
+            sinon.assert.calledOnce(showError);
+        });
+
+        test('never attempts deletion when venv validation fails', async () => {
+            pathExists.resolves(false);
+
+            assert.strictEqual(await removeEnvironment(), false);
+            sinon.assert.notCalled(remove);
+            sinon.assert.notCalled(lstat);
+            sinon.assert.notCalled(untrack);
+        });
+    });
+
     test('headless removal skips confirmation and removes the environment', async () => {
         const tempRoot = await fse.mkdtemp(path.join(os.tmpdir(), 'remove-venv-'));
         const envPath = path.join(tempRoot, '.venv');