From 3d62836f2589ff6a28e96bfa7b63628c45ee14fe Mon Sep 17 00:00:00 2001 From: vivganes Date: Fri, 13 Feb 2026 23:10:15 +0530 Subject: [PATCH 1/2] Fixes #18884 --- packages/cli/src/config/extension-manager.ts | 8 +- packages/cli/src/config/extensions/update.ts | 4 +- packages/cli/src/utils/retry.test.ts | 87 ++++++++++++++++++++ packages/cli/src/utils/retry.ts | 64 ++++++++++++++ 4 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 packages/cli/src/utils/retry.test.ts create mode 100644 packages/cli/src/utils/retry.ts diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 179959d83e..747334a30b 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -55,6 +55,7 @@ import { import { maybeRequestConsentOrFail } from './extensions/consent.js'; import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; import { ExtensionStorage } from './extensions/storage.js'; +import { removeDirectoryWithRetry } from '../utils/retry.js'; import { EXTENSIONS_CONFIG_FILENAME, INSTALL_METADATA_FILENAME, @@ -406,7 +407,7 @@ Would you like to attempt to install via "git clone" instead?`, } } finally { if (tempDir) { - await fs.promises.rm(tempDir, { recursive: true, force: true }); + await removeDirectoryWithRetry(tempDir); } } return extension; @@ -475,10 +476,7 @@ Would you like to attempt to install via "git clone" instead?`, : path.basename(extension.path), ); - await fs.promises.rm(storage.getExtensionDir(), { - recursive: true, - force: true, - }); + await removeDirectoryWithRetry(storage.getExtensionDir()); // The rest of the cleanup below here is only for true uninstalls, not // uninstalls related to updates. diff --git a/packages/cli/src/config/extensions/update.ts b/packages/cli/src/config/extensions/update.ts index bdb43e0975..db05509af3 100644 --- a/packages/cli/src/config/extensions/update.ts +++ b/packages/cli/src/config/extensions/update.ts @@ -12,10 +12,10 @@ import { import { loadInstallMetadata } from '../extension.js'; import { checkForExtensionUpdate } from './github.js'; import { debugLogger, type GeminiCLIExtension } from '@google/gemini-cli-core'; -import * as fs from 'node:fs'; import { getErrorMessage } from '../../utils/errors.js'; import { copyExtension, type ExtensionManager } from '../extension-manager.js'; import { ExtensionStorage } from './storage.js'; +import { removeDirectoryWithRetry } from '../../utils/retry.js'; export interface ExtensionUpdateInfo { name: string; @@ -103,7 +103,7 @@ export async function updateExtension( await copyExtension(tempDir, extension.path); throw e; } finally { - await fs.promises.rm(tempDir, { recursive: true, force: true }); + await removeDirectoryWithRetry(tempDir); } } diff --git a/packages/cli/src/utils/retry.test.ts b/packages/cli/src/utils/retry.test.ts new file mode 100644 index 0000000000..397859c8a6 --- /dev/null +++ b/packages/cli/src/utils/retry.test.ts @@ -0,0 +1,87 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import { removeDirectoryWithRetry } from './retry.js'; + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + return { + ...actual, + promises: { + ...actual.promises, + rm: vi.fn(), + }, + }; +}); + +describe('removeDirectoryWithRetry', () => { + const mockedRm = vi.mocked(fs.promises.rm); + + beforeEach(() => { + mockedRm.mockClear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should succeed on first attempt', async () => { + mockedRm.mockResolvedValue(undefined); + + await removeDirectoryWithRetry('/test/path'); + + expect(mockedRm).toHaveBeenCalledTimes(1); + expect(mockedRm).toHaveBeenCalledWith('/test/path', { + recursive: true, + force: true, + }); + }); + + it('should retry on EBUSY error', async () => { + const ebusyError = Object.assign(new Error('EBUSY'), { code: 'EBUSY' }); + mockedRm + .mockRejectedValueOnce(ebusyError) + .mockRejectedValueOnce(ebusyError) + .mockResolvedValue(undefined); + + await removeDirectoryWithRetry('/test/path'); + + expect(mockedRm).toHaveBeenCalledTimes(3); + }); + + it('should retry on ENOTEMPTY error', async () => { + const notEmptyError = Object.assign(new Error('ENOTEMPTY'), { + code: 'ENOTEMPTY', + }); + mockedRm.mockRejectedValueOnce(notEmptyError).mockResolvedValue(undefined); + + await removeDirectoryWithRetry('/test/path'); + + expect(mockedRm).toHaveBeenCalledTimes(2); + }); + + it('should throw after max retries', async () => { + const ebusyError = Object.assign(new Error('EBUSY'), { code: 'EBUSY' }); + mockedRm.mockRejectedValue(ebusyError); + + await expect(removeDirectoryWithRetry('/test/path')).rejects.toThrow( + 'EBUSY', + ); + expect(mockedRm).toHaveBeenCalledTimes(5); + }); + + it('should throw immediately on non-retryable errors', async () => { + const otherError = new Error('Some other error'); + mockedRm.mockRejectedValue(otherError); + + await expect(removeDirectoryWithRetry('/test/path')).rejects.toThrow( + 'Some other error', + ); + expect(mockedRm).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/cli/src/utils/retry.ts b/packages/cli/src/utils/retry.ts new file mode 100644 index 0000000000..db0b9912cd --- /dev/null +++ b/packages/cli/src/utils/retry.ts @@ -0,0 +1,64 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs'; +import { debugLogger } from '@google/gemini-cli-core'; + +/** + * Retries a file system operation with exponential backoff. + * Useful for Windows where file locks may cause EBUSY errors. + */ +async function retryWithBackoff( + operation: () => Promise, + maxRetries: number = 5, + initialDelayMs: number = 100, +): Promise { + let lastError: Error | undefined; + + for (let attempt = 0; attempt < maxRetries; attempt++) { + try { + return await operation(); + } catch (e) { + if (!(e instanceof Error)) { + throw e; + } + lastError = e; + + const code = (e as NodeJS.ErrnoException).code; + if (code !== 'EBUSY' && code !== 'ENOTEMPTY' && code !== 'EPERM') { + throw e; + } + + if (attempt < maxRetries - 1) { + const delay = initialDelayMs * Math.pow(2, attempt); + debugLogger.debug( + `Retry attempt ${attempt + 1}/${maxRetries} after ${delay}ms due to: ${lastError.message}`, + ); + await new Promise((resolve) => setTimeout(resolve, delay)); + } + } + } + + throw lastError; +} + +/** + * Removes a directory recursively with retries for Windows file lock issues. + * On Windows, file handles may remain open briefly after operations like copy, + * causing EBUSY errors. This function retries with exponential backoff. + */ +export async function removeDirectoryWithRetry( + path: string, + options: { recursive?: boolean; force?: boolean } = {}, +): Promise { + await retryWithBackoff( + async () => { + await fs.promises.rm(path, { recursive: true, force: true, ...options }); + }, + 5, + 100, + ); +} From b879cf526e4f5aabab7106b11c9477f582968c0f Mon Sep 17 00:00:00 2001 From: vivganes Date: Fri, 13 Feb 2026 23:23:01 +0530 Subject: [PATCH 2/2] fix review comment --- packages/cli/src/utils/retry.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/retry.ts b/packages/cli/src/utils/retry.ts index db0b9912cd..7fedffef61 100644 --- a/packages/cli/src/utils/retry.ts +++ b/packages/cli/src/utils/retry.ts @@ -18,6 +18,10 @@ async function retryWithBackoff( ): Promise { let lastError: Error | undefined; + if (maxRetries <= 0) { + throw new Error('maxRetries must be a positive number.'); + } + for (let attempt = 0; attempt < maxRetries; attempt++) { try { return await operation(); @@ -56,7 +60,7 @@ export async function removeDirectoryWithRetry( ): Promise { await retryWithBackoff( async () => { - await fs.promises.rm(path, { recursive: true, force: true, ...options }); + await fs.promises.rm(path, { ...options, recursive: true, force: true }); }, 5, 100,