fix(FN-4948): harden guard install failure cleanup paths
Fusion-Task-Id: FN-4948 Fusion-Task-Lineage: dc622643-4c3e-4217-9219-a6a6e5424427
This commit is contained in:
committed by
gsxdsm
parent
487dfcc10c
commit
4b16429618
@@ -9,6 +9,7 @@ import {
|
||||
import { AgentLogger } from "../agent-logger.js";
|
||||
import * as worktreeBackendModule from "../worktree-backend.js";
|
||||
import type { TaskDetail, Settings, TaskStore } from "@fusion/core";
|
||||
import { installTaskWorktreeIdentityGuard } from "../worktree-hooks.js";
|
||||
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -692,6 +693,7 @@ import { createLogger } from "../logger.js";
|
||||
|
||||
const mockedCreateFnAgent = vi.mocked(createFnAgent);
|
||||
const mockedExecSync = vi.mocked(execSync);
|
||||
const mockedInstallTaskWorktreeIdentityGuard = vi.mocked(installTaskWorktreeIdentityGuard);
|
||||
const mockedGenerateWorktreeName = vi.mocked(generateWorktreeName);
|
||||
const mockedCreateLogger = vi.mocked(createLogger);
|
||||
|
||||
@@ -1171,6 +1173,7 @@ describe("StepSessionExecutor", () => {
|
||||
expect.stringContaining("git worktree add"),
|
||||
expect.anything(),
|
||||
);
|
||||
expect(mockedInstallTaskWorktreeIdentityGuard).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("handles parallel step failure: successful step cherry-picked, failed cleaned up", async () => {
|
||||
|
||||
@@ -86,6 +86,25 @@ describe("NativeWorktreeBackend", () => {
|
||||
expect(installGuardMock).toHaveBeenCalledWith({ worktreePath: "/repo/.worktrees/fn-1", taskId: "FN-1" });
|
||||
});
|
||||
|
||||
it("propagates installer failure after cleanup", async () => {
|
||||
execMock.mockResolvedValue({ stdout: "", stderr: "" });
|
||||
installGuardMock.mockRejectedValueOnce(new Error("guard failed"));
|
||||
|
||||
await expect(
|
||||
new NativeWorktreeBackend().create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
}),
|
||||
).rejects.toThrow("guard failed");
|
||||
|
||||
expect(execMock).toHaveBeenCalledWith(
|
||||
'rm -rf "/repo/.worktrees/fn-1"',
|
||||
expect.objectContaining({ cwd: "/repo", timeout: 60000, maxBuffer: 10485760 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("retries with suffix and resolves", async () => {
|
||||
execMock.mockRejectedValueOnce(new Error("exists")).mockResolvedValueOnce({ stdout: "", stderr: "" });
|
||||
|
||||
|
||||
@@ -8411,7 +8411,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
}
|
||||
} else {
|
||||
executorLog.log(`Worktree already exists: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
await installGuardOrCleanup();
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
@@ -8451,6 +8451,19 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
}
|
||||
};
|
||||
|
||||
const installGuardOrCleanup = async () => {
|
||||
try {
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
} catch (error) {
|
||||
try {
|
||||
await execAsync(`rm -rf "${path}"`, { cwd: this.rootDir });
|
||||
} catch {
|
||||
executorLog.log(`Warning: failed to remove worktree after identity-guard install failure: ${path}`);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
|
||||
let staleLockRecoveryAttempted = false;
|
||||
try {
|
||||
await createWithBranch(branch);
|
||||
@@ -8458,7 +8471,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
if (attemptNumber > 0) {
|
||||
await this.store.logEntry(taskId, `Worktree created on attempt ${attemptNumber + 1}`, path);
|
||||
}
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
await installGuardOrCleanup();
|
||||
return { path, branch };
|
||||
} catch (initialError: unknown) {
|
||||
const conflictInfo = this.extractWorktreeConflictInfo(initialError);
|
||||
@@ -8469,7 +8482,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
if (recovered) {
|
||||
await createWithBranch(branch);
|
||||
executorLog.log(`Worktree created after stale lock recovery: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
await installGuardOrCleanup();
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
@@ -8529,7 +8542,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
try {
|
||||
await createFromExistingBranch();
|
||||
executorLog.log(`Worktree created from existing branch: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
await installGuardOrCleanup();
|
||||
return { path, branch };
|
||||
} catch (fallbackError: unknown) {
|
||||
const fallbackErrorMessage = fallbackError instanceof Error ? fallbackError.message : String(fallbackError);
|
||||
@@ -8541,7 +8554,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
if (recovered) {
|
||||
await createFromExistingBranch();
|
||||
executorLog.log(`Worktree created from existing branch after stale lock recovery: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
await installGuardOrCleanup();
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1346,10 +1346,19 @@ Follow instructions precisely and avoid unrelated changes.`,
|
||||
throw err;
|
||||
}
|
||||
|
||||
await installTaskWorktreeIdentityGuard({
|
||||
worktreePath,
|
||||
taskId: this.options.taskDetail.id,
|
||||
});
|
||||
try {
|
||||
await installTaskWorktreeIdentityGuard({
|
||||
worktreePath,
|
||||
taskId: this.options.taskDetail.id,
|
||||
});
|
||||
} catch (err) {
|
||||
try {
|
||||
await execAsync(`rm -rf "${worktreePath}"`, { cwd: rootDir, env: this.options.taskEnv });
|
||||
} catch {
|
||||
stepExecLog.log(`Warning: failed to remove worktree after identity-guard install failure: ${worktreePath}`);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
this.registerParallelWorktree(stepIndex, worktreePath);
|
||||
this.parallelBranches.set(stepIndex, branchName);
|
||||
|
||||
@@ -177,6 +177,19 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
|
||||
async create(input: WorktreeCreateInput): Promise<WorktreeCreateResult> {
|
||||
const startArg = input.startPoint ? ` ${quoteShellArg(input.startPoint)}` : "";
|
||||
const installGuardOrCleanup = async (worktreePath: string) => {
|
||||
try {
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath, taskId: input.taskId });
|
||||
} catch (error) {
|
||||
await execAsync(`rm -rf ${quoteShellArg(worktreePath)}`, {
|
||||
cwd: input.rootDir,
|
||||
encoding: "utf-8",
|
||||
timeout: REMOVE_TIMEOUT_MS,
|
||||
maxBuffer: MAX_BUFFER,
|
||||
}).catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
const createWithBranch = async (branchName: string): Promise<WorktreeCreateResult> => {
|
||||
await execAsync(
|
||||
`git worktree add -b ${quoteShellArg(branchName)} ${quoteShellArg(input.worktreePath)}${startArg}`,
|
||||
@@ -193,7 +206,7 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
let staleLockRecoveryAttempted = false;
|
||||
try {
|
||||
const created = await createWithBranch(input.branch);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: created.path, taskId: input.taskId });
|
||||
await installGuardOrCleanup(created.path);
|
||||
return created;
|
||||
} catch (error) {
|
||||
const lockPath = parseIndexLockPath(`${(error as { message?: string })?.message ?? ""}\n${getErrorStderr(error) ?? ""}`);
|
||||
@@ -225,7 +238,7 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
metadata: { lockPath },
|
||||
});
|
||||
const created = await createWithBranch(input.branch);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: created.path, taskId: input.taskId });
|
||||
await installGuardOrCleanup(created.path);
|
||||
return created;
|
||||
}
|
||||
await this.deps.audit?.git({
|
||||
@@ -269,7 +282,7 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
const candidateBranch = `${input.branch}-${suffix}`;
|
||||
try {
|
||||
const created = await createWithBranch(candidateBranch);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: created.path, taskId: input.taskId });
|
||||
await installGuardOrCleanup(created.path);
|
||||
return created;
|
||||
} catch {
|
||||
// continue probing suffixes
|
||||
@@ -453,7 +466,17 @@ export class WorktrunkWorktreeBackend implements WorktreeBackend {
|
||||
);
|
||||
}
|
||||
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: resolvedPath, taskId: input.taskId });
|
||||
try {
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: resolvedPath, taskId: input.taskId });
|
||||
} catch (error) {
|
||||
await execAsync(`rm -rf ${quoteShellArg(resolvedPath)}`, {
|
||||
cwd: input.rootDir,
|
||||
encoding: "utf-8",
|
||||
timeout: REMOVE_TIMEOUT_MS,
|
||||
maxBuffer: MAX_BUFFER,
|
||||
}).catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
return { path: resolvedPath, branch: input.branch };
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user