Merge 7511c6b2f4 into c4547d25c5

src/extension.ts

@@ -25,6 +25,7 @@ import { customToolRegistry } from "@roo-code/core"
 import "./utils/path" // Necessary to have access to String.prototype.toPosix.
 import { createOutputChannelLogger, createDualLogger } from "./utils/outputChannelLogger"
 import { initializeNetworkProxy } from "./utils/networkProxy"
+import { cleanupStaleLocks } from "./utils/safeWriteJson"
 
 import { Package } from "./shared/package"
 import { formatLanguage } from "./shared/language"
@@ -131,6 +132,22 @@ export async function activate(context: vscode.ExtensionContext) {
 	// Set extension path for custom tool registry to find bundled esbuild
 	customToolRegistry.setExtensionPath(context.extensionPath)
 
+	// Clean up stale lock files early, before any file operations that use safeWriteJson.
+	// This prevents hangs on btrfs and other filesystems where lock detection may not
+	// work correctly after extension updates or crashes. See issue #10436.
+	try {
+		const globalStoragePath = context.globalStorageUri.fsPath
+		const removedCount = await cleanupStaleLocks(globalStoragePath)
+		if (removedCount > 0) {
+			outputChannel.appendLine(`Cleaned up ${removedCount} stale lock file(s) from previous session`)
+		}
+	} catch (error) {
+		// Don't block activation on cleanup failures
+		outputChannel.appendLine(
+			`Warning: Failed to cleanup stale locks: ${error instanceof Error ? error.message : String(error)}`,
+		)
+	}
+
 	// Migrate old settings to new
 	await migrateSettings(context, outputChannel)
 

src/utils/__tests__/safeWriteJson.test.ts

@@ -4,7 +4,7 @@ import { Writable } from "stream"
 import * as path from "path"
 import * as os from "os"
 
-import { safeWriteJson } from "../safeWriteJson"
+import { safeWriteJson, cleanupStaleLocks, LOCK_STALE_MS } from "../safeWriteJson"
 
 const originalFsPromisesRename = actualFsPromises.rename
 const originalFsPromisesUnlink = actualFsPromises.unlink
@@ -478,3 +478,219 @@ describe("safeWriteJson", () => {
 		consoleErrorSpy.mockRestore()
 	})
 })
+
+describe("cleanupStaleLocks", () => {
+	let tempDir: string
+
+	beforeEach(async () => {
+		// Create a temporary directory for each test
+		tempDir = await actualFsPromises.mkdtemp(path.join(os.tmpdir(), "cleanupStaleLocks-test-"))
+	})
+
+	afterEach(async () => {
+		// Clean up the temporary directory after each test
+		await actualFsPromises.rm(tempDir, { recursive: true, force: true })
+	})
+
+	test("should return 0 when directory does not exist", async () => {
+		const nonExistentPath = path.join(tempDir, "non-existent")
+		const result = await cleanupStaleLocks(nonExistentPath)
+		expect(result).toBe(0)
+	})
+
+	test("should return 0 when directory is empty", async () => {
+		const result = await cleanupStaleLocks(tempDir)
+		expect(result).toBe(0)
+	})
+
+	test("should return 0 when no lock files exist", async () => {
+		// Create some regular files (not locks)
+		await actualFsPromises.writeFile(path.join(tempDir, "file1.json"), "{}")
+		await actualFsPromises.writeFile(path.join(tempDir, "file2.txt"), "test")
+
+		const result = await cleanupStaleLocks(tempDir)
+		expect(result).toBe(0)
+
+		// Verify regular files still exist
+		await expect(actualFsPromises.access(path.join(tempDir, "file1.json"))).resolves.toBeUndefined()
+		await expect(actualFsPromises.access(path.join(tempDir, "file2.txt"))).resolves.toBeUndefined()
+	})
+
+	test("should remove stale .lock directory", async () => {
+		// Create a stale lock directory
+		const lockPath = path.join(tempDir, "file.json.lock")
+		await actualFsPromises.mkdir(lockPath)
+
+		// Set mtime to be older than LOCK_STALE_MS
+		const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
+		await actualFsPromises.utimes(lockPath, staleTime, staleTime)
+
+		const result = await cleanupStaleLocks(tempDir)
+		expect(result).toBe(1)
+
+		// Verify lock directory was removed
+		await expect(actualFsPromises.access(lockPath)).rejects.toThrow()
+	})
+
+	test("should remove stale .lock file", async () => {
+		// Create a stale lock file
+		const lockPath = path.join(tempDir, "file.json.lock")
+		await actualFsPromises.writeFile(lockPath, "")
+
+		// Set mtime to be older than LOCK_STALE_MS
+		const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
+		await actualFsPromises.utimes(lockPath, staleTime, staleTime)
+
+		const result = await cleanupStaleLocks(tempDir)
+		expect(result).toBe(1)
+
+		// Verify lock file was removed
+		await expect(actualFsPromises.access(lockPath)).rejects.toThrow()
+	})
+
+	test("should NOT remove recent .lock directory", async () => {
+		// Create a recent lock directory (not stale)
+		const lockPath = path.join(tempDir, "file.json.lock")
+		await actualFsPromises.mkdir(lockPath)
+
+		// mtime will be recent (now)
+
+		const result = await cleanupStaleLocks(tempDir)
+		expect(result).toBe(0)
+
+		// Verify lock directory still exists
+		await expect(actualFsPromises.access(lockPath)).resolves.toBeUndefined()
+	})
+
+	test("should remove multiple stale locks", async () => {
+		// Create multiple stale lock directories
+		const lock1 = path.join(tempDir, "file1.json.lock")
+		const lock2 = path.join(tempDir, "file2.json.lock")
+		const lock3 = path.join(tempDir, "settings.lock")
+
+		await actualFsPromises.mkdir(lock1)
+		await actualFsPromises.mkdir(lock2)
+		await actualFsPromises.writeFile(lock3, "")
+
+		// Set mtime to be older than LOCK_STALE_MS
+		const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
+		await actualFsPromises.utimes(lock1, staleTime, staleTime)
+		await actualFsPromises.utimes(lock2, staleTime, staleTime)
+		await actualFsPromises.utimes(lock3, staleTime, staleTime)
+
+		const result = await cleanupStaleLocks(tempDir)
+		expect(result).toBe(3)
+
+		// Verify all locks were removed
+		await expect(actualFsPromises.access(lock1)).rejects.toThrow()
+		await expect(actualFsPromises.access(lock2)).rejects.toThrow()
+		await expect(actualFsPromises.access(lock3)).rejects.toThrow()
+	})
+
+	test("should recursively clean subdirectories by default", async () => {
+		// Create nested directory structure with locks
+		const subDir = path.join(tempDir, "subdir")
+		await actualFsPromises.mkdir(subDir)
+
+		const lock1 = path.join(tempDir, "file1.json.lock")
+		const lock2 = path.join(subDir, "file2.json.lock")
+
+		await actualFsPromises.mkdir(lock1)
+		await actualFsPromises.mkdir(lock2)
+
+		// Set mtime to be older than LOCK_STALE_MS
+		const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
+		await actualFsPromises.utimes(lock1, staleTime, staleTime)
+		await actualFsPromises.utimes(lock2, staleTime, staleTime)
+
+		const result = await cleanupStaleLocks(tempDir, { recursive: true })
+		expect(result).toBe(2)
+
+		// Verify both locks were removed
+		await expect(actualFsPromises.access(lock1)).rejects.toThrow()
+		await expect(actualFsPromises.access(lock2)).rejects.toThrow()
+
+		// Verify subdir still exists (only locks removed)
+		await expect(actualFsPromises.access(subDir)).resolves.toBeUndefined()
+	})
+
+	test("should not recursively clean when recursive is false", async () => {
+		// Create nested directory structure with locks
+		const subDir = path.join(tempDir, "subdir")
+		await actualFsPromises.mkdir(subDir)
+
+		const lock1 = path.join(tempDir, "file1.json.lock")
+		const lock2 = path.join(subDir, "file2.json.lock")
+
+		await actualFsPromises.mkdir(lock1)
+		await actualFsPromises.mkdir(lock2)
+
+		// Set mtime to be older than LOCK_STALE_MS
+		const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
+		await actualFsPromises.utimes(lock1, staleTime, staleTime)
+		await actualFsPromises.utimes(lock2, staleTime, staleTime)
+
+		const result = await cleanupStaleLocks(tempDir, { recursive: false })
+		expect(result).toBe(1)
+
+		// Verify only top-level lock was removed
+		await expect(actualFsPromises.access(lock1)).rejects.toThrow()
+
+		// Lock in subdir should still exist
+		await expect(actualFsPromises.access(lock2)).resolves.toBeUndefined()
+	})
+
+	test("should use custom staleDurationMs when provided", async () => {
+		// Create a lock directory
+		const lockPath = path.join(tempDir, "file.json.lock")
+		await actualFsPromises.mkdir(lockPath)
+
+		// Set mtime to 10 seconds ago
+		const tenSecondsAgo = new Date(Date.now() - 10000)
+		await actualFsPromises.utimes(lockPath, tenSecondsAgo, tenSecondsAgo)
+
+		// Should not remove with default stale duration (31s)
+		const result1 = await cleanupStaleLocks(tempDir)
+		expect(result1).toBe(0)
+
+		// Should remove with custom stale duration (5s)
+		const result2 = await cleanupStaleLocks(tempDir, { staleDurationMs: 5000 })
+		expect(result2).toBe(1)
+
+		// Verify lock was removed
+		await expect(actualFsPromises.access(lockPath)).rejects.toThrow()
+	})
+
+	test("should handle errors gracefully and continue", async () => {
+		// Create a stale lock directory
+		const lockPath = path.join(tempDir, "file.json.lock")
+		await actualFsPromises.mkdir(lockPath)
+
+		// Set mtime to be older than LOCK_STALE_MS
+		const staleTime = new Date(Date.now() - LOCK_STALE_MS - 5000)
+		await actualFsPromises.utimes(lockPath, staleTime, staleTime)
+
+		// Suppress console output during this test
+		const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
+		const consoleLogSpy = vi.spyOn(console, "log").mockImplementation(() => {})
+
+		// Mock rm to fail for this specific lock
+		const rmSpy = vi.spyOn(actualFsPromises, "rm").mockRejectedValueOnce(new Error("Permission denied"))
+
+		const result = await cleanupStaleLocks(tempDir)
+
+		// Should return 0 because removal failed
+		expect(result).toBe(0)
+
+		// Verify console.warn was called
+		expect(consoleWarnSpy).toHaveBeenCalled()
+
+		consoleWarnSpy.mockRestore()
+		consoleLogSpy.mockRestore()
+		rmSpy.mockRestore()
+	})
+
+	test("should export LOCK_STALE_MS constant", () => {
+		expect(LOCK_STALE_MS).toBe(31000)
+	})
+})

src/utils/safeWriteJson.ts

@@ -17,6 +17,12 @@ export interface SafeWriteJsonOptions {
 	prettyPrint?: boolean
 }
 
+/**
+ * The stale timeout for locks in milliseconds (must match the value in safeWriteJson).
+ * Locks older than this are considered abandoned and can be safely removed.
+ */
+const LOCK_STALE_MS = 31000
+
 /**
  * Safely writes JSON data to a file.
  * - Creates parent directories if they don't exist
@@ -220,4 +226,78 @@ async function _streamDataToFile(targetPath: string, data: any, prettyPrint = fa
 	})
 }
 
-export { safeWriteJson }
+/**
+ * Cleans up stale lock files/directories from a directory.
+ *
+ * The `proper-lockfile` library creates `.lock` directories for advisory locking.
+ * On some filesystems (particularly btrfs with Copy-on-Write), these locks may not
+ * be properly detected as stale, causing hangs during extension activation after
+ * updates or crashes.
+ *
+ * This function scans a directory for `.lock` entries and removes any that are
+ * older than the stale timeout, ensuring clean startup.
+ *
+ * @param directoryPath - The directory to scan for stale locks
+ * @param options - Optional configuration
+ * @param options.recursive - If true, scan subdirectories as well (default: true)
+ * @param options.staleDurationMs - Custom stale duration in ms (default: LOCK_STALE_MS)
+ * @returns Promise<number> - Number of stale locks removed
+ */
+async function cleanupStaleLocks(
+	directoryPath: string,
+	options: { recursive?: boolean; staleDurationMs?: number } = {},
+): Promise<number> {
+	const { recursive = true, staleDurationMs = LOCK_STALE_MS } = options
+	let removedCount = 0
+
+	try {
+		// Check if directory exists
+		const dirStat = await fs.stat(directoryPath).catch(() => null)
+		if (!dirStat || !dirStat.isDirectory()) {
+			return 0
+		}
+
+		const entries = await fs.readdir(directoryPath, { withFileTypes: true })
+		const now = Date.now()
+
+		for (const entry of entries) {
+			const entryPath = path.join(directoryPath, entry.name)
+
+			// Check for .lock files/directories created by proper-lockfile
+			if (entry.name.endsWith(".lock")) {
+				try {
+					const stat = await fs.stat(entryPath)
+					const age = now - stat.mtimeMs
+
+					// Remove if older than stale duration
+					// Use a slightly larger threshold to account for filesystem timing quirks
+					if (age > staleDurationMs) {
+						if (stat.isDirectory()) {
+							await fs.rm(entryPath, { recursive: true, force: true })
+						} else {
+							await fs.unlink(entryPath)
+						}
+						removedCount++
+						console.log(
+							`[cleanupStaleLocks] Removed stale lock: ${entryPath} (age: ${Math.round(age / 1000)}s)`,
+						)
+					}
+				} catch (err) {
+					// Ignore errors for individual lock files - they may have been
+					// cleaned up by another process or the original operation completed
+					console.warn(`[cleanupStaleLocks] Could not process ${entryPath}:`, err)
+				}
+			} else if (recursive && entry.isDirectory()) {
+				// Recursively clean subdirectories (but not .lock directories themselves)
+				removedCount += await cleanupStaleLocks(entryPath, options)
+			}
+		}
+	} catch (err) {
+		// Log but don't throw - cleanup failures shouldn't block extension activation
+		console.error(`[cleanupStaleLocks] Error scanning ${directoryPath}:`, err)
+	}
+
+	return removedCount
+}
+
+export { safeWriteJson, cleanupStaleLocks, LOCK_STALE_MS }