feat(FN-4830): complete Step 3 — executor 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
753068482e
commit
161cb565e6
@@ -113,6 +113,15 @@ vi.mock("../worktree-pool.js", async (importOriginal) => {
|
||||
isUsableTaskWorktree: vi.fn().mockResolvedValue(true),
|
||||
};
|
||||
});
|
||||
vi.mock("../worktree-stale-lock.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("../worktree-stale-lock.js")>("../worktree-stale-lock.js");
|
||||
return {
|
||||
...actual,
|
||||
parseIndexLockPath: vi.fn(actual.parseIndexLockPath),
|
||||
classifyStaleLock: vi.fn(),
|
||||
tryRemoveStaleLock: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("node:child_process", async () => {
|
||||
const { promisify } = await import("node:util");
|
||||
@@ -238,6 +247,7 @@ import { exec, execSync } from "node:child_process";
|
||||
import { existsSync, realpathSync } from "node:fs";
|
||||
import { hydrateWorktreeDb } from "../worktree-db-hydrate.js";
|
||||
import { isUsableTaskWorktree } from "../worktree-pool.js";
|
||||
import { classifyStaleLock, tryRemoveStaleLock } from "../worktree-stale-lock.js";
|
||||
|
||||
export const mockedCreateFnAgent = vi.mocked(createFnAgent);
|
||||
export const mockedSessionManager = vi.mocked(SessionManager);
|
||||
@@ -251,6 +261,8 @@ export const mockedExistsSync = vi.mocked(existsSync);
|
||||
export const mockedRealpathSync = vi.mocked(realpathSync);
|
||||
export const mockedHydrateWorktreeDb = vi.mocked(hydrateWorktreeDb);
|
||||
export const mockedIsUsableTaskWorktree = vi.mocked(isUsableTaskWorktree);
|
||||
export const mockedClassifyStaleLock = vi.mocked(classifyStaleLock);
|
||||
export const mockedTryRemoveStaleLock = vi.mocked(tryRemoveStaleLock);
|
||||
|
||||
export type EventListener = (...args: unknown[]) => void;
|
||||
|
||||
@@ -324,6 +336,10 @@ export function resetExecutorMocks() {
|
||||
mockedExec.mockReset();
|
||||
mockedExecSync.mockReset();
|
||||
mockedIsUsableTaskWorktree.mockResolvedValue(true);
|
||||
mockedClassifyStaleLock.mockReset();
|
||||
mockedTryRemoveStaleLock.mockReset();
|
||||
mockedClassifyStaleLock.mockResolvedValue({ kind: "fresh", reason: "fresh" } as any);
|
||||
mockedTryRemoveStaleLock.mockResolvedValue({ removed: true });
|
||||
mockExecuteAll.mockResolvedValue([]);
|
||||
mockTerminateAllSessions.mockResolvedValue(undefined);
|
||||
mockCleanup.mockResolvedValue(undefined);
|
||||
|
||||
@@ -33,6 +33,8 @@ import {
|
||||
mockedExistsSync,
|
||||
mockedHydrateWorktreeDb,
|
||||
mockedIsUsableTaskWorktree,
|
||||
mockedClassifyStaleLock,
|
||||
mockedTryRemoveStaleLock,
|
||||
mockExecuteAll,
|
||||
mockTerminateAllSessions,
|
||||
mockCleanup,
|
||||
@@ -1235,6 +1237,61 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
expect(worktreeAddCalls.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
describe("index.lock stale recovery", () => {
|
||||
it("recovers stale lock and succeeds", async () => {
|
||||
const store = createMockStore();
|
||||
let addCalls = 0;
|
||||
mockedClassifyStaleLock.mockResolvedValue({ kind: "stale", reason: "old-lock", ageMs: 60000 } as any);
|
||||
mockedTryRemoveStaleLock.mockResolvedValue({ removed: true });
|
||||
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
||||
if (command.includes("git worktree add") && addCalls++ === 0) {
|
||||
const error: any = new Error("fatal: unable to create '/tmp/test/.git/worktrees/swift-falcon/index.lock': File exists");
|
||||
error.stderr = Buffer.from("fatal: unable to create '/tmp/test/.git/worktrees/swift-falcon/index.lock': File exists");
|
||||
throw error;
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask());
|
||||
|
||||
expect(mockedClassifyStaleLock).toHaveBeenCalled();
|
||||
expect(mockedTryRemoveStaleLock).toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
expect.stringContaining("Recovered stale worktree index.lock and retrying"),
|
||||
expect.any(String),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("refuses fresh lock and fails with actionable error", async () => {
|
||||
const store = createMockStore();
|
||||
mockedClassifyStaleLock.mockResolvedValue({ kind: "active-session", reason: "active-session-owns-worktree", owningWorktreePath: "/tmp/test/.worktrees/swift-falcon" } as any);
|
||||
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
||||
if (command.includes("git worktree add")) {
|
||||
const error: any = new Error("fatal: unable to create '/tmp/test/.git/worktrees/swift-falcon/index.lock': File exists");
|
||||
error.stderr = Buffer.from("fatal: unable to create '/tmp/test/.git/worktrees/swift-falcon/index.lock': File exists");
|
||||
throw error;
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask());
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
expect.objectContaining({ status: "failed", error: expect.stringContaining("index.lock") }),
|
||||
);
|
||||
expect(mockedTryRemoveStaleLock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("removes stale branch and retries when branch exists without worktree", async () => {
|
||||
const store = createMockStore();
|
||||
let callCount = 0;
|
||||
|
||||
@@ -45,6 +45,12 @@ import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession }
|
||||
import { PRIORITY_EXECUTE, type AgentSemaphore } from "./concurrency.js";
|
||||
import { RemovalReason, getRegisteredWorktreePaths, isGitRepository, isInsideWorktreesDir, isRegisteredGitWorktree, isUsableTaskWorktree, removeWorktree, type WorktreePool } from "./worktree-pool.js";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
import {
|
||||
StaleWorktreeIndexLockError,
|
||||
classifyStaleLock,
|
||||
parseIndexLockPath,
|
||||
tryRemoveStaleLock,
|
||||
} from "./worktree-stale-lock.js";
|
||||
import {
|
||||
BranchConflictError,
|
||||
BranchCrossContaminationError,
|
||||
@@ -7637,7 +7643,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
const isLastAttempt = attempt === this.MAX_WORKTREE_RETRIES - 1;
|
||||
const isBranchConflict = isBranchConflictError(error);
|
||||
const isTerminalWorktreeError = error instanceof NonRetryableWorktreeError || isBranchConflict;
|
||||
const isTerminalWorktreeError = error instanceof NonRetryableWorktreeError || error instanceof StaleWorktreeIndexLockError || isBranchConflict;
|
||||
|
||||
if (isLastAttempt || isTerminalWorktreeError) {
|
||||
await this.store.logEntry(
|
||||
@@ -7967,6 +7973,72 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
}
|
||||
}
|
||||
|
||||
private async emitStaleLockAudit(taskId: string, event: string, targetPath: string, metadata: Record<string, unknown>): Promise<void> {
|
||||
if (!this.currentRunContext?.runId || !this.currentRunContext.agentId) return;
|
||||
const auditor = createRunAuditor(this.store, {
|
||||
runId: this.currentRunContext.runId,
|
||||
agentId: this.currentRunContext.agentId,
|
||||
taskId,
|
||||
phase: this.currentRunContext.phase,
|
||||
source: this.currentRunContext.source,
|
||||
});
|
||||
await auditor.git({ type: event as any, target: targetPath, metadata });
|
||||
}
|
||||
|
||||
private async recoverIndexLockIfStale(taskId: string, path: string, conflictInfo: { lockPath?: string; message?: string }): Promise<boolean> {
|
||||
const lockPath = conflictInfo.lockPath;
|
||||
if (!lockPath) return false;
|
||||
|
||||
const classification = await classifyStaleLock({
|
||||
rootDir: this.rootDir,
|
||||
lockPath,
|
||||
activeSessionRegistry,
|
||||
});
|
||||
await this.emitStaleLockAudit(taskId, "worktree:stale-lock-detected", path, {
|
||||
lockPath,
|
||||
classification: classification.kind,
|
||||
reason: classification.reason,
|
||||
ageMs: classification.ageMs ?? null,
|
||||
owningWorktreePath: classification.owningWorktreePath ?? null,
|
||||
});
|
||||
|
||||
if (classification.kind !== "stale") {
|
||||
await this.emitStaleLockAudit(taskId, "worktree:stale-lock-refused", path, {
|
||||
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 ${resolvePath(this.rootDir, lockPath)} is held by another git process (reason: ${classification.reason}, owning worktree ${classification.owningWorktreePath ?? "unknown"}). Resolve manually before retrying.`,
|
||||
lockPath: resolvePath(this.rootDir, lockPath),
|
||||
classification: classification.kind,
|
||||
reason: classification.reason,
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const removed = await tryRemoveStaleLock({ lockPath: resolvePath(this.rootDir, lockPath) });
|
||||
if (removed.removed) {
|
||||
await this.emitStaleLockAudit(taskId, "worktree:stale-lock-recovered", path, { lockPath });
|
||||
await this.store.logEntry(taskId, `Recovered stale worktree index.lock and retrying`, resolvePath(this.rootDir, lockPath), this.currentRunContext);
|
||||
return true;
|
||||
}
|
||||
await this.emitStaleLockAudit(taskId, "worktree:stale-lock-recovery-failed", path, {
|
||||
lockPath,
|
||||
reason: removed.reason ?? "not-removed",
|
||||
});
|
||||
return false;
|
||||
} catch (error) {
|
||||
await this.emitStaleLockAudit(taskId, "worktree:stale-lock-recovery-failed", path, {
|
||||
lockPath,
|
||||
reason: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Single attempt to create a worktree with conflict detection and recovery.
|
||||
* Returns the actual worktree path used (may differ from input if recovery generated new name).
|
||||
@@ -8043,6 +8115,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
}
|
||||
};
|
||||
|
||||
let staleLockRecoveryAttempted = false;
|
||||
try {
|
||||
await createWithBranch(branch);
|
||||
executorLog.log(`Worktree created: ${path}${startPoint ? ` (from ${startPoint})` : ""}`);
|
||||
@@ -8053,6 +8126,16 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
} catch (initialError: unknown) {
|
||||
const conflictInfo = this.extractWorktreeConflictInfo(initialError);
|
||||
|
||||
if (conflictInfo.type === "index-lock-contention" && !staleLockRecoveryAttempted) {
|
||||
staleLockRecoveryAttempted = true;
|
||||
const recovered = await this.recoverIndexLockIfStale(taskId, path, conflictInfo);
|
||||
if (recovered) {
|
||||
await createWithBranch(branch);
|
||||
executorLog.log(`Worktree created after stale lock recovery: ${path}`);
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
|
||||
if (conflictInfo.type === "not-git-repo") {
|
||||
throw new NonRetryableWorktreeError(
|
||||
"Project directory is not a Git repository. Fusion requires a Git repository for worktree creation. Initialize with 'git init' or run from a Git project directory.",
|
||||
@@ -8113,6 +8196,16 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
const fallbackErrorMessage = fallbackError instanceof Error ? fallbackError.message : String(fallbackError);
|
||||
// Check if the fallback also hit an "already used" conflict
|
||||
const fallbackConflictInfo = this.extractWorktreeConflictInfo(fallbackError);
|
||||
if (fallbackConflictInfo.type === "index-lock-contention" && !staleLockRecoveryAttempted) {
|
||||
staleLockRecoveryAttempted = true;
|
||||
const recovered = await this.recoverIndexLockIfStale(taskId, path, fallbackConflictInfo);
|
||||
if (recovered) {
|
||||
await createFromExistingBranch();
|
||||
executorLog.log(`Worktree created from existing branch after stale lock recovery: ${path}`);
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
|
||||
if (fallbackConflictInfo.type === "not-git-repo") {
|
||||
throw new NonRetryableWorktreeError(
|
||||
"Project directory is not a Git repository. Fusion requires a Git repository for worktree creation. Initialize with 'git init' or run from a Git project directory.",
|
||||
@@ -8552,8 +8645,9 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
* - "working tree already exists"
|
||||
*/
|
||||
private extractWorktreeConflictInfo(error: unknown): {
|
||||
type: "already-used" | "invalid-reference" | "leading-directories" | "already-exists" | "not-git-repo" | "unknown";
|
||||
type: "already-used" | "invalid-reference" | "leading-directories" | "already-exists" | "not-git-repo" | "index-lock-contention" | "unknown";
|
||||
path?: string;
|
||||
lockPath?: string;
|
||||
message?: string;
|
||||
} {
|
||||
const execError = error instanceof Error ? error : new Error(String(error));
|
||||
@@ -8577,6 +8671,11 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
return { type: "already-used", path: alreadyCheckedOutMatch[1], message: output };
|
||||
}
|
||||
|
||||
const lockPath = parseIndexLockPath(output);
|
||||
if (lockPath) {
|
||||
return { type: "index-lock-contention", lockPath, message: output };
|
||||
}
|
||||
|
||||
// Pattern: invalid reference: 'branch-name'
|
||||
// Also covers: unable to resolve reference, stale file handle, not a valid ref
|
||||
if (
|
||||
|
||||
@@ -94,6 +94,10 @@ export type GitMutationType =
|
||||
| "worktree:worktrunk-fallback-native"
|
||||
| "worktree:removal-refused-active-session"
|
||||
| "worktree:removal-forced-over-active-session"
|
||||
| "worktree:stale-lock-detected"
|
||||
| "worktree:stale-lock-recovered"
|
||||
| "worktree:stale-lock-recovery-failed"
|
||||
| "worktree:stale-lock-refused"
|
||||
| "branch:create"
|
||||
| "branch:delete"
|
||||
| "branch:checkout"
|
||||
|
||||
Reference in New Issue
Block a user