diff --git a/apps/code/src/main/services/archive/service.ts b/apps/code/src/main/services/archive/service.ts index 06c98e29ad..1c3dec2600 100644 --- a/apps/code/src/main/services/archive/service.ts +++ b/apps/code/src/main/services/archive/service.ts @@ -7,6 +7,7 @@ import { deleteCheckpoint, RevertCheckpointSaga, } from "@posthog/git/sagas/checkpoint"; +import { forceRemove } from "@posthog/git/utils"; import { type WorktreeInfo, WorktreeManager } from "@posthog/git/worktree"; import { inject, injectable } from "inversify"; import type { @@ -232,7 +233,7 @@ export class ArchiveService { }); await manager.deleteWorktree(worktreePath); const parentDir = path.dirname(worktreePath); - await fs.rm(parentDir, { recursive: true, force: true }); + await forceRemove(parentDir); }, async () => {}, ); @@ -352,7 +353,7 @@ export class ArchiveService { ); await manager.deleteWorktree(worktreePath); const parentDir = path.dirname(worktreePath); - await fs.rm(parentDir, { recursive: true, force: true }); + await forceRemove(parentDir); } }, ); diff --git a/apps/code/src/main/services/suspension/service.test.ts b/apps/code/src/main/services/suspension/service.test.ts index de47862218..5447f26ccb 100644 --- a/apps/code/src/main/services/suspension/service.test.ts +++ b/apps/code/src/main/services/suspension/service.test.ts @@ -45,9 +45,20 @@ vi.mock("@posthog/git/worktree", () => ({ mockWorktreeManagerProto.createDetachedWorktreeAtCommit; }, })); -vi.mock("node:fs/promises", () => ({ - default: { rm: vi.fn(), access: vi.fn() }, -})); +vi.mock("node:fs/promises", () => { + const fns = { + rm: vi.fn(), + access: vi.fn(), + lstat: vi + .fn() + .mockRejectedValue( + Object.assign(new Error("ENOENT"), { code: "ENOENT" }), + ), + chmod: vi.fn(), + readdir: vi.fn().mockResolvedValue([]), + }; + return { default: fns, ...fns }; +}); vi.mock("../../utils/logger.js", () => ({ logger: { diff --git a/apps/code/src/main/services/suspension/service.ts b/apps/code/src/main/services/suspension/service.ts index 032051cbf6..5409bcff61 100644 --- a/apps/code/src/main/services/suspension/service.ts +++ b/apps/code/src/main/services/suspension/service.ts @@ -6,6 +6,7 @@ import { deleteCheckpoint, RevertCheckpointSaga, } from "@posthog/git/sagas/checkpoint"; +import { forceRemove } from "@posthog/git/utils"; import { type WorktreeInfo, WorktreeManager } from "@posthog/git/worktree"; import { inject, injectable } from "inversify"; import type { IArchiveRepository } from "../../db/repositories/archive-repository.js"; @@ -278,7 +279,7 @@ export class SuspensionService extends TypedEventEmitter { const manager = this.createWorktreeManager(folderPath); await manager.deleteWorktree(worktreePath); - await fs.rm(path.dirname(worktreePath), { recursive: true, force: true }); + await forceRemove(path.dirname(worktreePath)); } private async killTaskProcesses( diff --git a/packages/git/src/sagas/worktree.ts b/packages/git/src/sagas/worktree.ts index 4af5e5947c..9f9e1fea88 100644 --- a/packages/git/src/sagas/worktree.ts +++ b/packages/git/src/sagas/worktree.ts @@ -2,7 +2,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import { GitSaga, type GitSagaInput } from "../git-saga"; import { addToLocalExclude, branchExists, getDefaultBranch } from "../queries"; -import { safeSymlink } from "../utils"; +import { forceRemove, safeSymlink } from "../utils"; import { processWorktreeInclude, runPostCheckoutHook } from "../worktree"; export interface CreateWorktreeInput extends GitSagaInput { @@ -50,7 +50,7 @@ export class CreateWorktreeSaga extends GitSaga< try { await this.git.raw(["worktree", "remove", worktreePath, "--force"]); } catch { - await fs.rm(worktreePath, { recursive: true, force: true }); + await forceRemove(worktreePath); await this.git.raw(["worktree", "prune"]); } try { @@ -157,7 +157,7 @@ export class CreateWorktreeForBranchSaga extends GitSaga< try { await this.git.raw(["worktree", "remove", worktreePath, "--force"]); } catch { - await fs.rm(worktreePath, { recursive: true, force: true }); + await forceRemove(worktreePath); await this.git.raw(["worktree", "prune"]); } }, @@ -274,7 +274,7 @@ export class DeleteWorktreeSaga extends GitSaga< try { await this.git.raw(["worktree", "remove", worktreePath, "--force"]); } catch { - await fs.rm(worktreePath, { recursive: true, force: true }); + await forceRemove(worktreePath); await this.git.raw(["worktree", "prune"]); } }, diff --git a/packages/git/src/utils.test.ts b/packages/git/src/utils.test.ts new file mode 100644 index 0000000000..ab558abead --- /dev/null +++ b/packages/git/src/utils.test.ts @@ -0,0 +1,68 @@ +import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { forceRemove } from "./utils"; + +async function fileExists(p: string): Promise { + try { + await stat(p); + return true; + } catch { + return false; + } +} + +describe("forceRemove", () => { + let workDir: string | undefined; + + afterEach(async () => { + if (workDir) { + await forceRemove(workDir).catch(() => {}); + workDir = undefined; + } + }); + + it("is a no-op for a missing path", async () => { + workDir = await mkdtemp(path.join(tmpdir(), "posthog-force-remove-")); + await expect( + forceRemove(path.join(workDir, "missing")), + ).resolves.toBeUndefined(); + }); + + it.each([ + { + name: "writable tree", + setup: async (target: string) => { + await mkdir(path.join(target, "nested"), { recursive: true }); + await writeFile(path.join(target, "nested", "file.txt"), "x"); + }, + }, + { + name: "read-only directories (Go module cache shape)", + setup: async (target: string) => { + const inner = path.join(target, "yaml.v3@v3.0.1", ".github"); + await mkdir(inner, { recursive: true }); + await writeFile(path.join(inner, "workflow.yml"), "on: push\n"); + await chmod(path.join(target, "yaml.v3@v3.0.1", ".github"), 0o555); + await chmod(path.join(target, "yaml.v3@v3.0.1"), 0o555); + // Confirm plain fs.rm cannot remove it. + await expect( + rm(target, { recursive: true, force: true }), + ).rejects.toMatchObject({ code: "EACCES" }); + // Restore read-only state so forceRemove starts clean. + await chmod(path.join(target, "yaml.v3@v3.0.1"), 0o555); + await chmod(path.join(target, "yaml.v3@v3.0.1", ".github"), 0o555); + }, + }, + ])("removes a $name", async ({ setup }) => { + workDir = await mkdtemp(path.join(tmpdir(), "posthog-force-remove-")); + const target = path.join(workDir, "tree"); + await mkdir(target); + await setup(target); + + await forceRemove(target); + + expect(await fileExists(target)).toBe(false); + }); +}); diff --git a/packages/git/src/utils.ts b/packages/git/src/utils.ts index 1947cc62e5..d54b2e04f7 100644 --- a/packages/git/src/utils.ts +++ b/packages/git/src/utils.ts @@ -107,6 +107,51 @@ function execFileAsync( }); } +async function chmodTreeWritable(target: string): Promise { + let stat: import("node:fs").Stats; + try { + stat = await fs.lstat(target); + } catch { + return; + } + if (stat.isSymbolicLink()) return; + try { + await fs.chmod(target, stat.isDirectory() ? 0o700 : 0o600); + } catch (error) { + console.warn(`forceRemove: chmod failed on ${target}`, error); + } + if (!stat.isDirectory()) return; + let entries: string[]; + try { + entries = await fs.readdir(target); + } catch (error) { + console.warn(`forceRemove: readdir failed on ${target}`, error); + return; + } + await Promise.all( + entries.map((entry) => chmodTreeWritable(path.join(target, entry))), + ); +} + +/** + * Recursively remove a path, retrying after chmod'ing the tree writable when + * the kernel rejects the initial removal with EACCES/EPERM. Worktrees commonly + * contain read-only subtrees populated by Go's module cache (which marks every + * cached directory mode 0555); plain `fs.rm` cannot unlink entries from those + * parents until we restore the write bit. + */ +export async function forceRemove(target: string): Promise { + try { + await fs.rm(target, { recursive: true, force: true, maxRetries: 3 }); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "EACCES" && code !== "EPERM") throw error; + } + await chmodTreeWritable(target); + await fs.rm(target, { recursive: true, force: true, maxRetries: 3 }); +} + export interface GitHubPr { owner: string; repo: string; diff --git a/packages/git/src/worktree.ts b/packages/git/src/worktree.ts index 4eb299fa87..5fe1e7cd99 100644 --- a/packages/git/src/worktree.ts +++ b/packages/git/src/worktree.ts @@ -10,7 +10,7 @@ import { getHeadSha, listWorktrees as listWorktreesRaw, } from "./queries"; -import { clonePath, safeSymlink } from "./utils"; +import { clonePath, forceRemove, safeSymlink } from "./utils"; export interface WorktreeInfo { worktreePath: string; @@ -394,7 +394,7 @@ export class WorktreeManager { try { await git.raw(["worktree", "remove", worktreePath, "--force"]); } catch { - await fs.rm(worktreePath, { recursive: true, force: true }); + await forceRemove(worktreePath); await git.raw(["worktree", "prune"]); } });