From d0649f99172b7e39b0d5fbfe2a40f8c5994a92dc Mon Sep 17 00:00:00 2001
From: roboomp <omp@can.ac>
Date: Thu, 30 Jul 2026 05:22:14 +0000
Subject: [PATCH] fix(browser): reused shared temp removal retries

Route Chromium profile cleanup through pi-utils removeWithRetries while preserving browser-specific warn-and-leave behavior.

Fixes #7058
---
 packages/coding-agent/src/tools/browser/launch.ts  | 10 +++++-----
 .../test/tools/browser-profile-cleanup.test.ts     | 14 +++++++-------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/packages/coding-agent/src/tools/browser/launch.ts b/packages/coding-agent/src/tools/browser/launch.ts
index 8c9e3c033..565296a98 100644
--- a/packages/coding-agent/src/tools/browser/launch.ts
+++ b/packages/coding-agent/src/tools/browser/launch.ts
@@ -1,7 +1,7 @@
 import * as fs from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
-import { $which, getPuppeteerDir, logger } from "@oh-my-pi/pi-utils";
+import { $which, getPuppeteerDir, logger, removeWithRetries } from "@oh-my-pi/pi-utils";
 import type * as BrowsersNs from "@puppeteer/browsers";
 import type { Browser, CDPSession, Page, default as Puppeteer, Target } from "puppeteer-core";
 import stealthTamperingScript from "../puppeteer/00_stealth_tampering.txt" with { type: "text" };
@@ -360,14 +360,14 @@ export async function launchHeadlessBrowser(opts: LaunchHeadlessOptions): Promis
 /**
  * Remove an OMP-owned headless Chromium profile directory, tolerating the brief
  * window on Windows in which Chromium (or an orphaned browser subprocess) still
- * holds the profile lock. `fs.rm`'s native `maxRetries`/`retryDelay` back off on
- * EBUSY/EPERM/ENOTEMPTY; if the directory is still busy afterwards we warn and
- * leave it for a later cleanup pass rather than throwing — a shutdown cleanup
+ * holds the profile lock. The shared temp remover centralizes retry handling
+ * for EBUSY/EPERM/ENOTEMPTY; if the directory is still busy afterwards we warn
+ * and leave it for a later cleanup pass rather than throwing — a shutdown cleanup
  * failure must never crash the process (issue #7058).
  */
 export async function removeUserDataDir(dir: string): Promise<void> {
 	try {
-		await fs.promises.rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+		await removeWithRetries(dir);
 	} catch (error) {
 		logger.warn("Left Chromium profile directory in place after cleanup failure", {
 			dir,
diff --git a/packages/coding-agent/test/tools/browser-profile-cleanup.test.ts b/packages/coding-agent/test/tools/browser-profile-cleanup.test.ts
index 0fb35ca09..5e21b284e 100644
--- a/packages/coding-agent/test/tools/browser-profile-cleanup.test.ts
+++ b/packages/coding-agent/test/tools/browser-profile-cleanup.test.ts
@@ -12,7 +12,7 @@ import * as os from "node:os";
 import * as path from "node:path";
 import { removeUserDataDir } from "@oh-my-pi/pi-coding-agent/tools/browser/launch";
 import { type BrowserHandle, releaseBrowser } from "@oh-my-pi/pi-coding-agent/tools/browser/registry";
-import { logger } from "@oh-my-pi/pi-utils";
+import * as piUtils from "@oh-my-pi/pi-utils";
 
 async function makeProfileDir(): Promise<string> {
 	const dir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "omp-chrome-profile-test-"));
@@ -23,8 +23,8 @@ async function makeProfileDir(): Promise<string> {
 
 describe("headless Chromium profile cleanup (issue #7058)", () => {
 	afterEach(() => {
-		spyOn(fs.promises, "rm").mockRestore();
-		spyOn(logger, "warn").mockRestore();
+		spyOn(piUtils, "removeWithRetries").mockRestore();
+		spyOn(piUtils.logger, "warn").mockRestore();
 	});
 
 	it("removes an owned profile directory", async () => {
@@ -36,15 +36,15 @@ describe("headless Chromium profile cleanup (issue #7058)", () => {
 	it("warns and leaves the directory instead of throwing when it stays locked (EBUSY)", async () => {
 		const dir = await makeProfileDir();
 		const ebusy = Object.assign(new Error(`EBUSY: resource busy or locked, rm '${dir}'`), { code: "EBUSY" });
-		const rmSpy = spyOn(fs.promises, "rm").mockRejectedValue(ebusy);
-		const warnSpy = spyOn(logger, "warn");
+		const removeSpy = spyOn(piUtils, "removeWithRetries").mockRejectedValue(ebusy);
+		const warnSpy = spyOn(piUtils.logger, "warn");
 		try {
 			// Must resolve — a cleanup failure never propagates as a crash.
 			await expect(removeUserDataDir(dir)).resolves.toBeUndefined();
-			expect(rmSpy).toHaveBeenCalledTimes(1);
+			expect(removeSpy).toHaveBeenCalledTimes(1);
 			expect(warnSpy).toHaveBeenCalledTimes(1);
 		} finally {
-			rmSpy.mockRestore();
+			removeSpy.mockRestore();
 			// Real removal so the fixture does not leak.
 			await fs.promises.rm(dir, { recursive: true, force: true });
 		}