feat(FN-4917): complete Step 3 — gate pooled and resume worktree validity
Fusion-Task-Id: FN-4917 Fusion-Task-Lineage: 1422a545-f4b5-4e1f-a347-8be2a4202f45
This commit is contained in:
committed by
gsxdsm
parent
2cb141a511
commit
2bc76cf00f
@@ -1,10 +1,15 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { promisify } from "node:util";
|
||||
import { acquireTaskWorktree } from "../worktree-acquisition.js";
|
||||
import { classifyTaskWorktree } from "../worktree-pool.js";
|
||||
|
||||
vi.mock("../worktree-pool.js", async () => {
|
||||
const actual = await vi.importActual<any>("../worktree-pool.js");
|
||||
return { ...actual, isUsableTaskWorktree: vi.fn().mockResolvedValue(true) };
|
||||
return {
|
||||
...actual,
|
||||
classifyTaskWorktree: vi.fn().mockResolvedValue({ ok: true }),
|
||||
isInsideWorktreesDir: vi.fn().mockReturnValue(true),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../worktree-db-hydrate.js", () => ({
|
||||
@@ -90,6 +95,57 @@ describe("acquireTaskWorktree", () => {
|
||||
expect(release).toHaveBeenCalledWith("/tmp/pooled");
|
||||
});
|
||||
|
||||
it("falls through to fresh creation when pooled worktree is incomplete and emits detection audit", async () => {
|
||||
vi.mocked(classifyTaskWorktree).mockResolvedValueOnce({ ok: false, classification: "incomplete", reason: "missing or invalid .git metadata" });
|
||||
const createWorktree = vi.fn().mockResolvedValue({ path: "/tmp/new", branch: "fusion/fn-1" });
|
||||
const auditGit = vi.fn().mockResolvedValue(undefined);
|
||||
const remove = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const result = await acquireTaskWorktree({
|
||||
task,
|
||||
rootDir: process.cwd(),
|
||||
store,
|
||||
settings: { recycleWorktrees: true } as any,
|
||||
pool: {
|
||||
acquire: () => "/tmp/pooled",
|
||||
prepareForTask: vi.fn().mockResolvedValue({ branch: "fusion/fn-1", worktreePath: "/tmp/pooled", reclaimed: false }),
|
||||
release: vi.fn(),
|
||||
} as any,
|
||||
createWorktree,
|
||||
audit: { git: auditGit } as any,
|
||||
backend: { kind: "native", create: vi.fn(), remove } as any,
|
||||
});
|
||||
|
||||
expect(result.source).toBe("fresh");
|
||||
expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "worktree:incomplete-detected",
|
||||
metadata: expect.objectContaining({ classification: "incomplete", source: "pool-acquire" }),
|
||||
}));
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-1", expect.stringContaining("Pool returned incomplete worktree"), undefined, undefined);
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith("FN-1", expect.stringMatching(/Refusing to start coding agent/), expect.anything(), expect.anything());
|
||||
});
|
||||
|
||||
it("emits resume detection audit and clears session file when assigned worktree is unregistered", async () => {
|
||||
vi.mocked(classifyTaskWorktree).mockResolvedValueOnce({ ok: false, classification: "unregistered", reason: "not registered in git worktree list" });
|
||||
const createWorktree = vi.fn().mockResolvedValue({ path: "/tmp/new", branch: "fusion/fn-1" });
|
||||
const auditGit = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await acquireTaskWorktree({
|
||||
task: { ...task, worktree: process.cwd(), branch: "fusion/fn-1", sessionFile: "/tmp/session.json" },
|
||||
rootDir: process.cwd(),
|
||||
store,
|
||||
settings: {} as any,
|
||||
createWorktree,
|
||||
audit: { git: auditGit } as any,
|
||||
});
|
||||
|
||||
expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "worktree:incomplete-detected",
|
||||
metadata: expect.objectContaining({ classification: "unregistered", source: "resume" }),
|
||||
}));
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: null, branch: null, sessionFile: null });
|
||||
});
|
||||
|
||||
it("creates fresh when pool disabled", async () => {
|
||||
const createWorktree = vi.fn().mockResolvedValue({ path: "/tmp/new", branch: "fusion/fn-1" });
|
||||
const result = await acquireTaskWorktree({
|
||||
|
||||
@@ -7,7 +7,11 @@ import { resolveTaskWorktreePathForBackend } from "./worktree-paths.js";
|
||||
import { hydrateWorktreeDb } from "./worktree-db-hydrate.js";
|
||||
import { formatError } from "./logger.js";
|
||||
import { isBranchConflictError } from "./branch-conflicts.js";
|
||||
import { type WorktreePool, isUsableTaskWorktree } from "./worktree-pool.js";
|
||||
import {
|
||||
type WorktreePool,
|
||||
classifyTaskWorktree,
|
||||
isInsideWorktreesDir,
|
||||
} from "./worktree-pool.js";
|
||||
import {
|
||||
NativeWorktreeBackend,
|
||||
WorktrunkOperationError,
|
||||
@@ -208,13 +212,21 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
||||
}
|
||||
|
||||
let isResume = Boolean(task.worktree && existsSync(worktreePath));
|
||||
if (task.worktree && isResume && !await isUsableTaskWorktree(rootDir, worktreePath)) {
|
||||
logger?.log(`${task.id}: assigned worktree is not usable; creating a fresh worktree instead: ${worktreePath}`);
|
||||
await store.logEntry(task.id, "Assigned worktree is not a registered, usable git worktree; creating a fresh worktree instead", worktreePath, runContext);
|
||||
await store.updateTask(task.id, { worktree: null, branch: null });
|
||||
const fallbackName = generateWorktreeName(rootDir, settings);
|
||||
worktreePath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName);
|
||||
isResume = false;
|
||||
if (task.worktree && isResume) {
|
||||
const resumeClassification = await classifyTaskWorktree(rootDir, worktreePath);
|
||||
if (!resumeClassification.ok) {
|
||||
await audit?.git({
|
||||
type: "worktree:incomplete-detected",
|
||||
target: worktreePath,
|
||||
metadata: { classification: resumeClassification.classification, reason: resumeClassification.reason, source: "resume", taskId: task.id },
|
||||
});
|
||||
logger?.log(`${task.id}: assigned worktree is not usable; creating a fresh worktree instead: ${worktreePath}`);
|
||||
await store.logEntry(task.id, "Assigned worktree is not a registered, usable git worktree; creating a fresh worktree instead", worktreePath, runContext);
|
||||
await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null });
|
||||
const fallbackName = generateWorktreeName(rootDir, settings);
|
||||
worktreePath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName);
|
||||
isResume = false;
|
||||
}
|
||||
}
|
||||
|
||||
const hydrate = async (path: string): Promise<boolean> => {
|
||||
@@ -259,41 +271,66 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
|
||||
}
|
||||
worktreePath = prepared.worktreePath;
|
||||
branch = prepared.branch;
|
||||
acquiredFromPool = true;
|
||||
logger?.log(`Acquired worktree from pool: ${worktreePath}`);
|
||||
await store.updateTask(task.id, { worktree: worktreePath, branch });
|
||||
await audit?.git({ type: "worktree:reuse", target: worktreePath, metadata: { branch, reclaimed: prepared.reclaimed } });
|
||||
if (prepared.reclaimed) {
|
||||
await store.logEntry(task.id, `Acquired reclaimed worktree from pool: ${worktreePath} (${prepared.strandedCommitCount ?? 0} commits preserved)`, undefined, runContext);
|
||||
} else if (branch !== branchName) {
|
||||
logger?.log(`Branch conflict resolved: using ${branch} instead of ${branchName}`);
|
||||
await store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath} (branch conflict: using ${branch})`, undefined, runContext);
|
||||
const pooledClassification = await classifyTaskWorktree(rootDir, worktreePath);
|
||||
if (!pooledClassification.ok) {
|
||||
await audit?.git({
|
||||
type: "worktree:incomplete-detected",
|
||||
target: worktreePath,
|
||||
metadata: {
|
||||
classification: pooledClassification.classification,
|
||||
reason: pooledClassification.reason,
|
||||
source: "pool-acquire",
|
||||
taskId: task.id,
|
||||
},
|
||||
});
|
||||
await store.logEntry(task.id, `Pool returned ${pooledClassification.classification} worktree (${pooledClassification.reason}); creating fresh worktree`, undefined, runContext);
|
||||
if (isInsideWorktreesDir(rootDir, worktreePath, settings)) {
|
||||
try {
|
||||
await backend.remove(worktreePath, { rootDir, force: true, reason: RemovalReason.PoolCleanup, taskId: task.id });
|
||||
} catch (removeErr) {
|
||||
logger?.warn(`${task.id}: failed to remove unusable pooled worktree ${worktreePath}: ${formatError(removeErr)}`);
|
||||
}
|
||||
}
|
||||
const fallbackName = generateWorktreeName(rootDir, settings);
|
||||
worktreePath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName);
|
||||
branch = branchName;
|
||||
} else {
|
||||
await store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath}`, undefined, runContext);
|
||||
acquiredFromPool = true;
|
||||
logger?.log(`Acquired worktree from pool: ${worktreePath}`);
|
||||
await store.updateTask(task.id, { worktree: worktreePath, branch });
|
||||
await audit?.git({ type: "worktree:reuse", target: worktreePath, metadata: { branch, reclaimed: prepared.reclaimed } });
|
||||
if (prepared.reclaimed) {
|
||||
await store.logEntry(task.id, `Acquired reclaimed worktree from pool: ${worktreePath} (${prepared.strandedCommitCount ?? 0} commits preserved)`, undefined, runContext);
|
||||
} else if (branch !== branchName) {
|
||||
logger?.log(`Branch conflict resolved: using ${branch} instead of ${branchName}`);
|
||||
await store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath} (branch conflict: using ${branch})`, undefined, runContext);
|
||||
} else {
|
||||
await store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath}`, undefined, runContext);
|
||||
}
|
||||
await maybeWarnForeignTaskStartPoint({
|
||||
baseBranch,
|
||||
rootDir,
|
||||
worktreePath,
|
||||
taskId: task.id,
|
||||
logger,
|
||||
store,
|
||||
runContext,
|
||||
});
|
||||
const hydrated = await hydrate(worktreePath);
|
||||
return {
|
||||
worktreePath,
|
||||
branch,
|
||||
source: "pool",
|
||||
hydrated,
|
||||
isResume: false,
|
||||
reclaimed: prepared.reclaimed
|
||||
? {
|
||||
existingTipSha: prepared.existingTipSha,
|
||||
strandedCommitCount: prepared.strandedCommitCount,
|
||||
}
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
await maybeWarnForeignTaskStartPoint({
|
||||
baseBranch,
|
||||
rootDir,
|
||||
worktreePath,
|
||||
taskId: task.id,
|
||||
logger,
|
||||
store,
|
||||
runContext,
|
||||
});
|
||||
const hydrated = await hydrate(worktreePath);
|
||||
return {
|
||||
worktreePath,
|
||||
branch,
|
||||
source: "pool",
|
||||
hydrated,
|
||||
isResume: false,
|
||||
reclaimed: prepared.reclaimed
|
||||
? {
|
||||
existingTipSha: prepared.existingTipSha,
|
||||
strandedCommitCount: prepared.strandedCommitCount,
|
||||
}
|
||||
: undefined,
|
||||
};
|
||||
} catch (poolErr) {
|
||||
pool.release(pooled);
|
||||
if (isBranchConflictError(poolErr)) throw poolErr;
|
||||
|
||||
Reference in New Issue
Block a user