smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(browser): reused shared temp removal retries

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

Fixes #7058
This commit is contained in:
roboomp
2026-07-30 05:22:14 +00:00
parent 7ab3d2ecb6
commit d0649f9917
2 changed files with 12 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/tools/browser/launch.ts
+5 -5
View File
@@ -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,
packages/coding-agent/test/tools/browser-profile-cleanup.test.ts
+7 -7
View File
@@ -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 });
}