feat(FN-5058): merge fusion/fn-5058
This commit is contained in:
155
packages/engine/src/__tests__/worktree-admin-entry-prune.test.ts
Normal file
155
packages/engine/src/__tests__/worktree-admin-entry-prune.test.ts
Normal file
@@ -0,0 +1,155 @@
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { execSync } from "node:child_process";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
vi.resetModules();
|
||||
vi.unmock("node:child_process");
|
||||
vi.unmock("node:fs");
|
||||
vi.unmock("../worktree-hooks.js");
|
||||
vi.unmock("../worktree-prune.js");
|
||||
});
|
||||
|
||||
describe("worktree prune wiring", () => {
|
||||
it("step-session createStepWorktree pairs cleanup deletes with prune reasons", async () => {
|
||||
const pruneSpy = vi.fn().mockResolvedValue(undefined);
|
||||
const execMock = vi.fn();
|
||||
(execMock as any)[Symbol.for("nodejs.util.promisify.custom")] = execMock;
|
||||
|
||||
execMock.mockRejectedValueOnce(new Error("create failed")).mockResolvedValueOnce({ stdout: "", stderr: "" });
|
||||
|
||||
vi.doMock("node:child_process", () => ({ exec: execMock }));
|
||||
vi.doMock("../worktree-prune.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../worktree-prune.js")>();
|
||||
return { ...actual, pruneWorktreeAdminEntries: pruneSpy };
|
||||
});
|
||||
vi.doMock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockRejectedValue(new Error("guard failed")),
|
||||
}));
|
||||
|
||||
const { StepSessionExecutor } = await import("../step-session-executor.js");
|
||||
const task = {
|
||||
id: "FN-5058",
|
||||
title: "t",
|
||||
description: "d",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as any;
|
||||
|
||||
const executor = new StepSessionExecutor({ taskDetail: task, worktreePath: "/repo", rootDir: "/repo", settings: {} as any });
|
||||
await expect((executor as any).createStepWorktree(1)).rejects.toThrow("create failed");
|
||||
expect(pruneSpy).toHaveBeenCalledWith(expect.objectContaining({ reason: "step-session-create-failed" }));
|
||||
|
||||
execMock.mockReset();
|
||||
execMock.mockResolvedValueOnce({ stdout: "", stderr: "" }).mockResolvedValueOnce({ stdout: "", stderr: "" });
|
||||
await expect((executor as any).createStepWorktree(2)).rejects.toThrow("guard failed");
|
||||
expect(pruneSpy).toHaveBeenCalledWith(expect.objectContaining({ reason: "step-session-guard-failed" }));
|
||||
});
|
||||
|
||||
it("step-session cleanup swallows prune helper rejection", async () => {
|
||||
const pruneSpy = vi.fn().mockRejectedValue(new Error("prune boom"));
|
||||
const execMock = vi.fn();
|
||||
(execMock as any)[Symbol.for("nodejs.util.promisify.custom")] = execMock;
|
||||
execMock.mockRejectedValueOnce(new Error("create failed")).mockResolvedValueOnce({ stdout: "", stderr: "" });
|
||||
|
||||
vi.doMock("node:child_process", () => ({ exec: execMock }));
|
||||
vi.doMock("../worktree-prune.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../worktree-prune.js")>();
|
||||
return { ...actual, pruneWorktreeAdminEntries: pruneSpy };
|
||||
});
|
||||
const { StepSessionExecutor } = await import("../step-session-executor.js");
|
||||
const task = {
|
||||
id: "FN-5058",
|
||||
title: "t",
|
||||
description: "d",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as any;
|
||||
|
||||
const executor = new StepSessionExecutor({ taskDetail: task, worktreePath: "/repo", rootDir: "/repo", settings: {} as any });
|
||||
await expect((executor as any).createStepWorktree(3)).rejects.toThrow("create failed");
|
||||
});
|
||||
|
||||
it("native backend create calls prune after guard cleanup", async () => {
|
||||
const pruneSpy = vi.fn().mockResolvedValue(undefined);
|
||||
const execMock = vi.fn();
|
||||
(execMock as any)[Symbol.for("nodejs.util.promisify.custom")] = execMock;
|
||||
execMock.mockResolvedValueOnce({ stdout: "", stderr: "" }).mockResolvedValueOnce({ stdout: "", stderr: "" });
|
||||
|
||||
vi.doMock("node:child_process", () => ({ exec: execMock }));
|
||||
vi.doMock("../worktree-prune.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../worktree-prune.js")>();
|
||||
return { ...actual, pruneWorktreeAdminEntries: pruneSpy };
|
||||
});
|
||||
vi.doMock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockRejectedValue(new Error("guard failed")),
|
||||
}));
|
||||
|
||||
const { NativeWorktreeBackend } = await import("../worktree-backend.js");
|
||||
await expect(
|
||||
new NativeWorktreeBackend({ audit: { git: vi.fn() } as any }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-5058",
|
||||
branch: "fusion/fn-5058",
|
||||
taskId: "FN-5058",
|
||||
}),
|
||||
).rejects.toThrow("guard failed");
|
||||
|
||||
expect(pruneSpy).toHaveBeenCalledWith(expect.objectContaining({ reason: "backend-guard-failed" }));
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
describe("pruneWorktreeAdminEntries helper", () => {
|
||||
it("swallows git prune failure and audits success=false metadata", async () => {
|
||||
const execMock = vi.fn();
|
||||
(execMock as any)[Symbol.for("nodejs.util.promisify.custom")] = execMock;
|
||||
execMock.mockRejectedValue(new Error("boom"));
|
||||
vi.doMock("node:child_process", async () => {
|
||||
const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
|
||||
return { ...actual, exec: execMock };
|
||||
});
|
||||
|
||||
vi.unmock("../worktree-prune.js");
|
||||
const { pruneWorktreeAdminEntries } = await import("../worktree-prune.js");
|
||||
const audit = vi.fn().mockResolvedValue(undefined);
|
||||
await pruneWorktreeAdminEntries({ rootDir: "/repo", auditor: { git: audit }, reason: "test-failure", target: "/repo/.worktrees/x" });
|
||||
});
|
||||
|
||||
it("runs git worktree prune end-to-end in a real repository", async () => {
|
||||
vi.unmock("../worktree-prune.js");
|
||||
const { pruneWorktreeAdminEntries } = await import("../worktree-prune.js");
|
||||
const root = mkdtempSync(join(tmpdir(), "fn-5058-prune-"));
|
||||
const repo = join(root, "repo");
|
||||
const wt = join(root, "repo-wt");
|
||||
mkdirSync(repo, { recursive: true });
|
||||
|
||||
execSync("git init", { cwd: repo, stdio: "ignore" });
|
||||
execSync('git config user.email "test@example.com"', { cwd: repo });
|
||||
execSync('git config user.name "Test"', { cwd: repo });
|
||||
writeFileSync(join(repo, "README.md"), "ok\n");
|
||||
execSync("git add README.md", { cwd: repo });
|
||||
execSync('git commit -m "init"', { cwd: repo, stdio: "ignore" });
|
||||
execSync(`git worktree add ${wt} -b fusion/fn-5058-test`, { cwd: repo, stdio: "ignore" });
|
||||
rmSync(wt, { recursive: true, force: true });
|
||||
|
||||
await pruneWorktreeAdminEntries({ rootDir: repo, reason: "integration", target: wt });
|
||||
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
});
|
||||
@@ -49,7 +49,12 @@ vi.mock("node:fs", () => ({
|
||||
rmSync: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../worktree-prune.js", () => ({
|
||||
pruneWorktreeAdminEntries: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
import * as desktopArtifacts from "../worktree-desktop-artifacts.js";
|
||||
import * as worktreePrune from "../worktree-prune.js";
|
||||
import {
|
||||
WorktreePool,
|
||||
getRegisteredWorktreeBranchMap,
|
||||
@@ -71,6 +76,7 @@ const mockedExistsSync = vi.mocked(existsSync);
|
||||
const mockedLstatSync = vi.mocked(lstatSync);
|
||||
const mockedReaddirSync = vi.mocked(readdirSync);
|
||||
const mockedRmSync = vi.mocked(rmSync);
|
||||
const mockedPruneWorktreeAdminEntries = vi.mocked(worktreePrune.pruneWorktreeAdminEntries);
|
||||
const TEST_TASK_ID = "FN-test";
|
||||
|
||||
let errorSpy: ReturnType<typeof vi.spyOn>;
|
||||
@@ -847,6 +853,7 @@ describe("cleanupOrphanedWorktrees", () => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
mockRegisteredWorktrees("/root", []);
|
||||
mockedPruneWorktreeAdminEntries.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("removes worktrees not assigned to any active task", async () => {
|
||||
@@ -997,6 +1004,9 @@ describe("cleanupOrphanedWorktrees", () => {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
expect(mockedPruneWorktreeAdminEntries).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: "pool-cleanup-orphan", target: "/root/.worktrees/broken-wt" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1189,6 +1199,9 @@ describe("reapOrphanWorktrees", () => {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
expect(mockedPruneWorktreeAdminEntries).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: "pool-reap-orphan", target: "/root/.worktrees/pale-raven" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("does NOT remove a directory that is a registered git worktree", async () => {
|
||||
|
||||
@@ -93,6 +93,11 @@ export {
|
||||
} from "./agent-instructions.js";
|
||||
export { HEARTBEAT_PROCEDURE, HEARTBEAT_SYSTEM_PROMPT, HEARTBEAT_NO_TASK_SYSTEM_PROMPT } from "./agent-heartbeat.js";
|
||||
export { WorktreePool, scanIdleWorktrees, cleanupOrphanedWorktrees, reapOrphanWorktrees } from "./worktree-pool.js";
|
||||
export {
|
||||
pruneWorktreeAdminEntries,
|
||||
pruneWorktreeAdminEntriesSync,
|
||||
type PruneWorktreeAdminEntriesOptions,
|
||||
} from "./worktree-prune.js";
|
||||
export {
|
||||
BranchConflictError,
|
||||
BranchCrossContaminationError,
|
||||
|
||||
@@ -120,6 +120,18 @@ export type GitMutationType =
|
||||
| "worktree:worktrunk-fallback"
|
||||
| "worktree:worktrunk-failure"
|
||||
| "worktree:worktrunk-fallback-native"
|
||||
/**
|
||||
* Metadata shape:
|
||||
* ```ts
|
||||
* {
|
||||
* success: boolean;
|
||||
* reason: string;
|
||||
* target?: string;
|
||||
* error?: string;
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
| "worktree:admin-entry-pruned"
|
||||
| "worktree:removal-refused-active-session"
|
||||
| "worktree:removal-forced-over-active-session"
|
||||
| "worktree:stale-lock-detected"
|
||||
|
||||
@@ -50,6 +50,7 @@ import {
|
||||
createTaskLogTool,
|
||||
} from "./agent-tools.js";
|
||||
import { RemovalReason, removeWorktree } from "./worktree-backend.js";
|
||||
import { pruneWorktreeAdminEntries } from "./worktree-prune.js";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
|
||||
const stepExecLog = createLogger("step-session-executor");
|
||||
@@ -1343,6 +1344,12 @@ Follow instructions precisely and avoid unrelated changes.`,
|
||||
// best-effort cleanup; log but don't mask the original error
|
||||
stepExecLog.log(`Warning: failed to remove partial worktree directory after creation failure: ${worktreePath}`);
|
||||
}
|
||||
await pruneWorktreeAdminEntries({
|
||||
rootDir,
|
||||
reason: "step-session-create-failed",
|
||||
target: worktreePath,
|
||||
logger: stepExecLog,
|
||||
}).catch(() => undefined);
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -1357,6 +1364,12 @@ Follow instructions precisely and avoid unrelated changes.`,
|
||||
} catch {
|
||||
stepExecLog.log(`Warning: failed to remove worktree after identity-guard install failure: ${worktreePath}`);
|
||||
}
|
||||
await pruneWorktreeAdminEntries({
|
||||
rootDir,
|
||||
reason: "step-session-guard-failed",
|
||||
target: worktreePath,
|
||||
logger: stepExecLog,
|
||||
}).catch(() => undefined);
|
||||
throw err;
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
||||
import { inspectBranchConflict } from "./branch-conflicts.js";
|
||||
import { formatError } from "./logger.js";
|
||||
import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js";
|
||||
import { pruneWorktreeAdminEntries } from "./worktree-prune.js";
|
||||
import {
|
||||
StaleWorktreeIndexLockError,
|
||||
classifyStaleLock,
|
||||
@@ -187,6 +188,13 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
timeout: REMOVE_TIMEOUT_MS,
|
||||
maxBuffer: MAX_BUFFER,
|
||||
}).catch(() => undefined);
|
||||
await pruneWorktreeAdminEntries({
|
||||
rootDir: input.rootDir,
|
||||
auditor: this.deps.audit,
|
||||
reason: "backend-guard-failed",
|
||||
target: worktreePath,
|
||||
logger: this.deps.logger,
|
||||
}).catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
@@ -363,6 +371,7 @@ export class WorktrunkWorktreeBackend implements WorktreeBackend {
|
||||
private readonly deps: {
|
||||
binaryPath: string | (() => Promise<string | null>) | null;
|
||||
logger?: { log: (m: string) => void; warn: (m: string) => void };
|
||||
audit?: Pick<RunAuditor, "git">;
|
||||
},
|
||||
) {}
|
||||
|
||||
@@ -475,6 +484,13 @@ export class WorktrunkWorktreeBackend implements WorktreeBackend {
|
||||
timeout: REMOVE_TIMEOUT_MS,
|
||||
maxBuffer: MAX_BUFFER,
|
||||
}).catch(() => undefined);
|
||||
await pruneWorktreeAdminEntries({
|
||||
rootDir: input.rootDir,
|
||||
auditor: this.deps.audit,
|
||||
reason: "backend-guard-failed",
|
||||
target: resolvedPath,
|
||||
logger: this.deps.logger,
|
||||
}).catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
return { path: resolvedPath, branch: input.branch };
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
import { cleanupSecretsEnvFile } from "./secrets-env-writer.js";
|
||||
import { removeDesktopBuildArtifacts } from "./worktree-desktop-artifacts.js";
|
||||
import type { RunAuditor } from "./run-audit.js";
|
||||
import { pruneWorktreeAdminEntries } from "./worktree-prune.js";
|
||||
|
||||
export {
|
||||
NativeWorktreeBackend,
|
||||
@@ -695,6 +696,12 @@ export async function cleanupOrphanedWorktrees(
|
||||
throw new Error(`Refusing to remove path outside .worktrees: ${worktreePath}`);
|
||||
}
|
||||
rmSync(worktreePath, { recursive: true, force: true });
|
||||
await pruneWorktreeAdminEntries({
|
||||
rootDir,
|
||||
reason: "pool-cleanup-orphan",
|
||||
target: worktreePath,
|
||||
logger: worktreePoolLog,
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
worktreePoolLog.log(`Cleaned up orphaned worktree: ${worktreePath}`);
|
||||
cleaned++;
|
||||
@@ -805,6 +812,12 @@ export async function reapOrphanWorktrees(
|
||||
worktreePoolLog.warn(`secrets-env cleanup failed for orphan ${name}: ${error instanceof Error ? error.message : String(error)}`);
|
||||
}
|
||||
rmSync(resolvedFull, { recursive: true, force: true });
|
||||
await pruneWorktreeAdminEntries({
|
||||
rootDir: projectRoot,
|
||||
reason: "pool-reap-orphan",
|
||||
target: resolvedFull,
|
||||
logger: worktreePoolLog,
|
||||
}).catch(() => undefined);
|
||||
worktreePoolLog.log(`reapOrphanWorktrees: removed half-initialized orphan ${name}`);
|
||||
removed++;
|
||||
} catch (err: unknown) {
|
||||
|
||||
96
packages/engine/src/worktree-prune.ts
Normal file
96
packages/engine/src/worktree-prune.ts
Normal file
@@ -0,0 +1,96 @@
|
||||
import { exec, execSync } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
import type { RunAuditor } from "./run-audit.js";
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
const PRUNE_TIMEOUT_MS = 30_000;
|
||||
const PRUNE_MAX_BUFFER = 10 * 1024 * 1024;
|
||||
|
||||
type PruneAuditPayload = {
|
||||
success: boolean;
|
||||
reason: string;
|
||||
target?: string;
|
||||
error?: string;
|
||||
};
|
||||
|
||||
export type PruneWorktreeAdminEntriesOptions = {
|
||||
rootDir: string;
|
||||
auditor?: Pick<RunAuditor, "git">;
|
||||
reason: string;
|
||||
target?: string;
|
||||
logger?: { log: (m: string) => void };
|
||||
};
|
||||
|
||||
async function emitAudit(
|
||||
opts: PruneWorktreeAdminEntriesOptions,
|
||||
metadata: PruneAuditPayload,
|
||||
): Promise<void> {
|
||||
await opts.auditor?.git({
|
||||
type: "worktree:admin-entry-pruned",
|
||||
target: opts.target ?? opts.rootDir,
|
||||
metadata,
|
||||
});
|
||||
}
|
||||
|
||||
export async function pruneWorktreeAdminEntries(opts: PruneWorktreeAdminEntriesOptions): Promise<void> {
|
||||
try {
|
||||
await execAsync("git worktree prune", {
|
||||
cwd: opts.rootDir,
|
||||
timeout: PRUNE_TIMEOUT_MS,
|
||||
maxBuffer: PRUNE_MAX_BUFFER,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
|
||||
opts.logger?.log?.(
|
||||
`[worktree-prune] git worktree prune succeeded (reason=${opts.reason}${opts.target ? ` target=${opts.target}` : ""})`,
|
||||
);
|
||||
await emitAudit(opts, {
|
||||
success: true,
|
||||
reason: opts.reason,
|
||||
target: opts.target,
|
||||
});
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
opts.logger?.log?.(
|
||||
`[worktree-prune] git worktree prune failed (reason=${opts.reason}${opts.target ? ` target=${opts.target}` : ""}): ${errorMessage}`,
|
||||
);
|
||||
await emitAudit(opts, {
|
||||
success: false,
|
||||
reason: opts.reason,
|
||||
target: opts.target,
|
||||
error: errorMessage,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
export function pruneWorktreeAdminEntriesSync(opts: PruneWorktreeAdminEntriesOptions): void {
|
||||
try {
|
||||
execSync("git worktree prune", {
|
||||
cwd: opts.rootDir,
|
||||
timeout: PRUNE_TIMEOUT_MS,
|
||||
maxBuffer: PRUNE_MAX_BUFFER,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
|
||||
opts.logger?.log?.(
|
||||
`[worktree-prune] git worktree prune (sync) succeeded (reason=${opts.reason}${opts.target ? ` target=${opts.target}` : ""})`,
|
||||
);
|
||||
void emitAudit(opts, {
|
||||
success: true,
|
||||
reason: opts.reason,
|
||||
target: opts.target,
|
||||
});
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
opts.logger?.log?.(
|
||||
`[worktree-prune] git worktree prune (sync) failed (reason=${opts.reason}${opts.target ? ` target=${opts.target}` : ""}): ${errorMessage}`,
|
||||
);
|
||||
void emitAudit(opts, {
|
||||
success: false,
|
||||
reason: opts.reason,
|
||||
target: opts.target,
|
||||
error: errorMessage,
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user