feat(FN-4830): complete Step 2 — backend stale-lock recovery
Fusion-Task-Id: FN-4830 Fusion-Task-Lineage: d9b8ad72-669f-488e-8e85-be2dce9b8341
This commit is contained in:
committed by
gsxdsm
parent
9e2a2e37d0
commit
753068482e
@@ -8,10 +8,17 @@ import {
|
||||
RemovalReason,
|
||||
} from "../worktree-backend.js";
|
||||
|
||||
const { execMock, accessMock, existsSyncMock } = vi.hoisted(() => {
|
||||
const { execMock, accessMock, existsSyncMock, parseIndexLockPathMock, classifyStaleLockMock, tryRemoveStaleLockMock } = vi.hoisted(() => {
|
||||
const mock = vi.fn();
|
||||
(mock as any)[Symbol.for("nodejs.util.promisify.custom")] = mock;
|
||||
return { execMock: mock, accessMock: vi.fn(), existsSyncMock: vi.fn() };
|
||||
return {
|
||||
execMock: mock,
|
||||
accessMock: vi.fn(),
|
||||
existsSyncMock: vi.fn(),
|
||||
parseIndexLockPathMock: vi.fn(),
|
||||
classifyStaleLockMock: vi.fn(),
|
||||
tryRemoveStaleLockMock: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("node:child_process", () => ({ exec: execMock }));
|
||||
@@ -20,6 +27,23 @@ vi.mock("node:fs/promises", () => ({ access: accessMock }));
|
||||
vi.mock("../branch-conflicts.js", () => ({
|
||||
inspectBranchConflict: vi.fn().mockResolvedValue({ kind: "stale" }),
|
||||
}));
|
||||
vi.mock("../worktree-stale-lock.js", () => ({
|
||||
StaleWorktreeIndexLockError: class StaleWorktreeIndexLockError extends Error {
|
||||
lockPath: string;
|
||||
classification: string;
|
||||
reason: string;
|
||||
constructor(input: { message: string; lockPath: string; classification: string; reason: string }) {
|
||||
super(input.message);
|
||||
this.name = "StaleWorktreeIndexLockError";
|
||||
this.lockPath = input.lockPath;
|
||||
this.classification = input.classification;
|
||||
this.reason = input.reason;
|
||||
}
|
||||
},
|
||||
parseIndexLockPath: parseIndexLockPathMock,
|
||||
classifyStaleLock: classifyStaleLockMock,
|
||||
tryRemoveStaleLock: tryRemoveStaleLockMock,
|
||||
}));
|
||||
|
||||
beforeEach(() => {
|
||||
execMock.mockReset();
|
||||
@@ -27,6 +51,12 @@ beforeEach(() => {
|
||||
existsSyncMock.mockReset();
|
||||
accessMock.mockResolvedValue(undefined);
|
||||
existsSyncMock.mockReturnValue(true);
|
||||
parseIndexLockPathMock.mockReset();
|
||||
classifyStaleLockMock.mockReset();
|
||||
tryRemoveStaleLockMock.mockReset();
|
||||
parseIndexLockPathMock.mockReturnValue(null);
|
||||
classifyStaleLockMock.mockResolvedValue({ kind: "fresh", reason: "fresh" });
|
||||
tryRemoveStaleLockMock.mockResolvedValue({ removed: true });
|
||||
});
|
||||
|
||||
describe("NativeWorktreeBackend", () => {
|
||||
@@ -115,6 +145,63 @@ describe("NativeWorktreeBackend", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("resolves stale index.lock and retries create once", async () => {
|
||||
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
||||
parseIndexLockPathMock.mockReturnValue("/repo/.git/worktrees/fn-1/index.lock");
|
||||
classifyStaleLockMock.mockResolvedValue({ kind: "stale", reason: "old-lock", ageMs: 60000 });
|
||||
tryRemoveStaleLockMock.mockResolvedValue({ removed: true });
|
||||
execMock
|
||||
.mockRejectedValueOnce({ message: "fatal", stderr: "fatal: unable to create '/repo/.git/worktrees/fn-1/index.lock': File exists" })
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" });
|
||||
|
||||
const result = await new NativeWorktreeBackend({ audit }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ path: "/repo/.worktrees/fn-1", branch: "fusion/fn-1" });
|
||||
expect(tryRemoveStaleLockMock).toHaveBeenCalledWith({ lockPath: "/repo/.git/worktrees/fn-1/index.lock" });
|
||||
expect(audit.git).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ type: "worktree:stale-lock-detected" }),
|
||||
);
|
||||
expect(audit.git).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({ type: "worktree:stale-lock-recovered" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("throws StaleWorktreeIndexLockError when lock is non-stale", async () => {
|
||||
const audit = { git: vi.fn().mockResolvedValue(undefined) };
|
||||
parseIndexLockPathMock.mockReturnValue("/repo/.git/worktrees/fn-1/index.lock");
|
||||
classifyStaleLockMock.mockResolvedValue({ kind: "fresh", reason: "lock-younger-than-threshold", ageMs: 1000 });
|
||||
execMock.mockRejectedValueOnce({
|
||||
message: "fatal",
|
||||
stderr: "fatal: unable to create '/repo/.git/worktrees/fn-1/index.lock': File exists",
|
||||
});
|
||||
|
||||
await expect(
|
||||
new NativeWorktreeBackend({ audit }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
}),
|
||||
).rejects.toMatchObject({ name: "StaleWorktreeIndexLockError" });
|
||||
|
||||
expect(tryRemoveStaleLockMock).not.toHaveBeenCalled();
|
||||
expect(audit.git).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ type: "worktree:stale-lock-detected" }),
|
||||
);
|
||||
expect(audit.git).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({ type: "worktree:stale-lock-refused" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("resolves native worktree path via configured worktreesDir", async () => {
|
||||
const backend = new NativeWorktreeBackend({ settings: { worktreesDir: "../{repo}.worktrees" } as any });
|
||||
await expect(
|
||||
|
||||
@@ -142,7 +142,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
||||
|
||||
let backend: WorktreeBackend;
|
||||
try {
|
||||
backend = opts.backend ?? resolveWorktreeBackend(settings, { logger });
|
||||
backend = opts.backend ?? resolveWorktreeBackend(settings, { logger, audit });
|
||||
} catch (error) {
|
||||
if (
|
||||
settings.worktrunk?.enabled
|
||||
|
||||
@@ -9,6 +9,12 @@ import type { RunAuditor } from "./run-audit.js";
|
||||
import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
||||
import { inspectBranchConflict } from "./branch-conflicts.js";
|
||||
import { formatError } from "./logger.js";
|
||||
import {
|
||||
StaleWorktreeIndexLockError,
|
||||
classifyStaleLock,
|
||||
parseIndexLockPath,
|
||||
tryRemoveStaleLock,
|
||||
} from "./worktree-stale-lock.js";
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
const NATIVE_TIMEOUT_MS = 120_000;
|
||||
@@ -164,6 +170,7 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
private readonly deps: {
|
||||
logger?: { log: (m: string) => void; warn: (m: string) => void };
|
||||
settings?: Pick<Settings, "worktreesDir">;
|
||||
audit?: Pick<RunAuditor, "git">;
|
||||
} = {},
|
||||
) {}
|
||||
|
||||
@@ -182,9 +189,73 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
return { path: input.worktreePath, branch: branchName };
|
||||
};
|
||||
|
||||
let staleLockRecoveryAttempted = false;
|
||||
try {
|
||||
return await createWithBranch(input.branch);
|
||||
} catch (error) {
|
||||
const lockPath = parseIndexLockPath(`${(error as { message?: string })?.message ?? ""}\n${getErrorStderr(error) ?? ""}`);
|
||||
if (lockPath && !staleLockRecoveryAttempted) {
|
||||
staleLockRecoveryAttempted = true;
|
||||
const classification = await classifyStaleLock({
|
||||
rootDir: input.rootDir,
|
||||
lockPath,
|
||||
activeSessionRegistry,
|
||||
});
|
||||
await this.deps.audit?.git({
|
||||
type: "worktree:stale-lock-detected",
|
||||
target: input.worktreePath,
|
||||
metadata: {
|
||||
lockPath,
|
||||
classification: classification.kind,
|
||||
reason: classification.reason,
|
||||
ageMs: classification.ageMs ?? null,
|
||||
owningWorktreePath: classification.owningWorktreePath ?? null,
|
||||
},
|
||||
});
|
||||
if (classification.kind === "stale") {
|
||||
try {
|
||||
const removed = await tryRemoveStaleLock({ lockPath: resolve(input.rootDir, lockPath) });
|
||||
if (removed.removed) {
|
||||
await this.deps.audit?.git({
|
||||
type: "worktree:stale-lock-recovered",
|
||||
target: input.worktreePath,
|
||||
metadata: { lockPath },
|
||||
});
|
||||
return await createWithBranch(input.branch);
|
||||
}
|
||||
await this.deps.audit?.git({
|
||||
type: "worktree:stale-lock-recovery-failed",
|
||||
target: input.worktreePath,
|
||||
metadata: { lockPath, reason: removed.reason ?? "not-removed" },
|
||||
});
|
||||
} catch (removeError) {
|
||||
await this.deps.audit?.git({
|
||||
type: "worktree:stale-lock-recovery-failed",
|
||||
target: input.worktreePath,
|
||||
metadata: { lockPath, reason: formatError(removeError).detail },
|
||||
});
|
||||
}
|
||||
} else {
|
||||
await this.deps.audit?.git({
|
||||
type: "worktree:stale-lock-refused",
|
||||
target: input.worktreePath,
|
||||
metadata: {
|
||||
lockPath,
|
||||
classification: classification.kind,
|
||||
reason: classification.reason,
|
||||
ageMs: classification.ageMs ?? null,
|
||||
owningWorktreePath: classification.owningWorktreePath ?? null,
|
||||
},
|
||||
});
|
||||
throw new StaleWorktreeIndexLockError({
|
||||
message: `Worktree creation blocked: index.lock at ${resolve(input.rootDir, lockPath)} is held by another git process (reason: ${classification.reason}). Resolve manually before retrying.`,
|
||||
lockPath: resolve(input.rootDir, lockPath),
|
||||
classification: classification.kind,
|
||||
reason: classification.reason,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (!input.allowSiblingBranchRename) {
|
||||
throw error;
|
||||
}
|
||||
@@ -617,7 +688,7 @@ export async function removeWorktree(input: {
|
||||
});
|
||||
}
|
||||
|
||||
const backend = resolveWorktreeBackend(input.settings, { logger });
|
||||
const backend = resolveWorktreeBackend(input.settings, { logger, audit: input.audit });
|
||||
const removeInput: WorktreeRemoveInput = {
|
||||
rootDir: input.rootDir,
|
||||
worktreePath: input.worktreePath,
|
||||
@@ -666,6 +737,7 @@ export function resolveWorktreeBackend(
|
||||
deps: {
|
||||
logger?: { log: (m: string) => void; warn: (m: string) => void };
|
||||
binaryPathResolver?: () => Promise<string | null>;
|
||||
audit?: Pick<RunAuditor, "git">;
|
||||
} = {},
|
||||
): WorktreeBackend {
|
||||
if (settings.worktrunk?.enabled === true) {
|
||||
@@ -678,5 +750,5 @@ export function resolveWorktreeBackend(
|
||||
});
|
||||
}
|
||||
|
||||
return new NativeWorktreeBackend({ logger: deps.logger, settings });
|
||||
return new NativeWorktreeBackend({ logger: deps.logger, settings, audit: deps.audit });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user