FN-6244: protect active AI merge worktrees
Ensure AI merge cleanup only reaps stale, inactive temp worktrees. - Register live AI merge clean-room paths in the active session registry while merges run. - Add a minimum temp-worktree reap age and conservative lookup-error handling to sweep and prune paths. - Expand cleanup tests and release the repaired AI merge cleanup test from quarantine. Files changed: docs/architecture.md | 2 +- .../engine/src/__tests__/merger-ai-cleanup.test.ts | 38 ++++++++++++++-- .../__tests__/self-healing-tempdir-sweep.test.ts | 52 +++++++++++++++++++--- packages/engine/src/active-session-registry.ts | 2 +- packages/engine/src/merger-ai.ts | 29 +++++++++++- packages/engine/src/self-healing.ts | 28 +++++++++--- packages/engine/vitest.config.ts | 1 - scripts/lib/test-quarantine.json | 5 --- 8 files changed, 132 insertions(+), 25 deletions(-) Fusion-Task-Id: FN-6244 Fusion-Task-Lineage: 5c8ced12-d38f-4e45-9ea0-119b3236a2cb
This commit is contained in:
@@ -1,11 +1,12 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, utimesSync, writeFileSync } from "node:fs";
|
||||
import { rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { execSync } from "node:child_process";
|
||||
import { cleanupAiMergeWorktree, pruneExistingAiMergeWorktrees, runAiMerge } from "../merger-ai.js";
|
||||
import { activeSessionRegistry } from "../active-session-registry.js";
|
||||
import { MIN_TEMP_WORKTREE_REAP_AGE_MS } from "../self-healing.js";
|
||||
import type { RunAuditor } from "../run-audit.js";
|
||||
|
||||
const fsState = vi.hoisted(() => ({ failReaddirPath: "" }));
|
||||
@@ -117,6 +118,11 @@ function tempAiMergeDir(name: string): string {
|
||||
return dir;
|
||||
}
|
||||
|
||||
function makeAge(path: string, ageMs: number): void {
|
||||
const old = new Date(Date.now() - ageMs);
|
||||
utimesSync(path, old, old);
|
||||
}
|
||||
|
||||
function realMergeAgent(taskId = "FN-1") {
|
||||
return vi.fn(async (cwd: string) => {
|
||||
execSync(`git merge --squash fusion/${taskId.toLowerCase()}`, { cwd, stdio: "pipe" });
|
||||
@@ -177,6 +183,7 @@ describe("AI merge temp worktree cleanup", () => {
|
||||
|
||||
it("pruneExistingAiMergeWorktrees removes stale same-task directories", async () => {
|
||||
const stale = tempAiMergeDir("fusion-ai-merge-fn-777-stale");
|
||||
makeAge(stale, MIN_TEMP_WORKTREE_REAP_AGE_MS + 1_000);
|
||||
const { audit, events } = makeAudit();
|
||||
const logs: string[] = [];
|
||||
|
||||
@@ -188,6 +195,18 @@ describe("AI merge temp worktree cleanup", () => {
|
||||
]));
|
||||
});
|
||||
|
||||
it("pruneExistingAiMergeWorktrees skips too-new same-task directories", async () => {
|
||||
const fresh = tempAiMergeDir("fusion-ai-merge-fn-777-fresh");
|
||||
const { audit, events } = makeAudit();
|
||||
const logs: string[] = [];
|
||||
|
||||
await expect(pruneExistingAiMergeWorktrees("FN-777", process.cwd(), audit, vi.fn(async (message: string) => { logs.push(message); }))).resolves.toBe(0);
|
||||
|
||||
expect(existsSync(fresh)).toBe(true);
|
||||
expect(events).toEqual([]);
|
||||
expect(logs.join("\n")).toContain("skipping too-new worktree");
|
||||
});
|
||||
|
||||
it("pruneExistingAiMergeWorktrees skips directories for other tasks", async () => {
|
||||
const other = tempAiMergeDir("fusion-ai-merge-fn-778-stale");
|
||||
const { audit, events } = makeAudit();
|
||||
@@ -201,7 +220,7 @@ describe("AI merge temp worktree cleanup", () => {
|
||||
it("pruneExistingAiMergeWorktrees skips active-session paths", async () => {
|
||||
const stale = tempAiMergeDir("fusion-ai-merge-fn-777-active");
|
||||
const canonical = realpathSync(stale);
|
||||
activeSessionRegistry.registerPath(canonical, { taskId: "FN-777", kind: "executor", ownerKey: "FN-777" });
|
||||
activeSessionRegistry.registerPath(canonical, { taskId: "FN-777", kind: "ai-merge", ownerKey: "ai-merge:FN-777" });
|
||||
const { audit, events } = makeAudit();
|
||||
|
||||
await expect(pruneExistingAiMergeWorktrees("FN-777", process.cwd(), audit, vi.fn(async () => undefined))).resolves.toBe(0);
|
||||
@@ -209,19 +228,29 @@ describe("AI merge temp worktree cleanup", () => {
|
||||
expect(events).toEqual([]);
|
||||
|
||||
activeSessionRegistry.unregisterPath(canonical);
|
||||
makeAge(stale, MIN_TEMP_WORKTREE_REAP_AGE_MS + 1_000);
|
||||
await expect(pruneExistingAiMergeWorktrees("FN-777", process.cwd(), audit, vi.fn(async () => undefined))).resolves.toBe(1);
|
||||
expect(existsSync(stale)).toBe(false);
|
||||
});
|
||||
|
||||
it("runAiMerge emits success cleanup audit events", async () => {
|
||||
it("runAiMerge registers the clean-room worktree while merging and unregisters after", async () => {
|
||||
const { dir } = initRepoWithBranch();
|
||||
const { store, audits } = makeStore();
|
||||
let observedMergeRoot = "";
|
||||
const mergeAgent = vi.fn(async (cwd: string) => {
|
||||
observedMergeRoot = cwd;
|
||||
expect(activeSessionRegistry.isPathActive(realpathSync(cwd))).toBe(true);
|
||||
expect(activeSessionRegistry.isPathActive(cwd)).toBe(true);
|
||||
await realMergeAgent()(cwd);
|
||||
});
|
||||
|
||||
await runAiMerge(store, dir, "FN-1", { manual: true }, {
|
||||
mergeAgent: realMergeAgent(),
|
||||
mergeAgent,
|
||||
reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"),
|
||||
});
|
||||
|
||||
expect(observedMergeRoot).toContain("fusion-ai-merge-fn-1-");
|
||||
expect(activeSessionRegistry.pathsForTask("FN-1")).toEqual([]);
|
||||
const cleanupEvents = audits.filter((event) => event.mutationType === "merge:ai-worktree-cleanup");
|
||||
expect(cleanupEvents).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ metadata: expect.objectContaining({ phase: "git-remove", success: true }) }),
|
||||
@@ -233,6 +262,7 @@ describe("AI merge temp worktree cleanup", () => {
|
||||
const taskId = "FN-777";
|
||||
const { dir } = initRepoWithBranch(taskId);
|
||||
const orphan = tempAiMergeDir("fusion-ai-merge-fn-777-orphan");
|
||||
makeAge(orphan, MIN_TEMP_WORKTREE_REAP_AGE_MS + 1_000);
|
||||
const { store, audits } = makeStore(taskId);
|
||||
|
||||
await runAiMerge(store, dir, taskId, { manual: true }, {
|
||||
|
||||
@@ -45,7 +45,7 @@ vi.mock("node:child_process", async () => {
|
||||
});
|
||||
|
||||
import { activeSessionRegistry } from "../active-session-registry.js";
|
||||
import { SelfHealingManager } from "../self-healing.js";
|
||||
import { DONE_TASK_TEMP_WORKTREE_GRACE_MS, MIN_TEMP_WORKTREE_REAP_AGE_MS, SelfHealingManager, STALE_TEMP_MERGE_WORKTREE_MS } from "../self-healing.js";
|
||||
|
||||
const RM = { recursive: true, force: true, maxRetries: 5, retryDelay: 50 } as const;
|
||||
let sandboxRoot = "";
|
||||
@@ -115,6 +115,10 @@ function missingTask(): () => Promise<any> {
|
||||
return async () => { throw new Error("Task FN-999 not found"); };
|
||||
}
|
||||
|
||||
function transientErrorTask(): () => Promise<any> {
|
||||
return async () => { throw new Error("SQLITE_BUSY: database is locked"); };
|
||||
}
|
||||
|
||||
async function sweep(manager: SelfHealingManager): Promise<number> {
|
||||
return await (manager as any).cleanupStaleTempMergeWorktrees();
|
||||
}
|
||||
@@ -150,7 +154,7 @@ describe("SelfHealingManager temp-dir AI merge worktree sweep", () => {
|
||||
const stale = tempMergeDir();
|
||||
makeStale(stale);
|
||||
const canonical = realpathSync(stale);
|
||||
activeSessionRegistry.registerPath(canonical, { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||
activeSessionRegistry.registerPath(canonical, { taskId: "FN-1", kind: "ai-merge", ownerKey: "ai-merge:FN-1" });
|
||||
const { manager, audits } = makeManager();
|
||||
|
||||
await expect(sweep(manager)).resolves.toBe(0);
|
||||
@@ -231,18 +235,54 @@ describe("SelfHealingManager temp-dir AI merge worktree sweep", () => {
|
||||
]));
|
||||
});
|
||||
|
||||
it("removes worktree for deleted task immediately", async () => {
|
||||
const fresh = tempMergeDir("fusion-ai-merge-fn-999-deletedtask");
|
||||
it("keeps fresh worktree for deleted task until minimum age floor", async () => {
|
||||
const fresh = tempMergeDir("fusion-ai-merge-fn-999-deletedtaskfresh");
|
||||
makeAge(fresh, MIN_TEMP_WORKTREE_REAP_AGE_MS - 1_000);
|
||||
const { manager, audits } = makeManager({}, missingTask());
|
||||
|
||||
await expect(sweep(manager)).resolves.toBe(0);
|
||||
|
||||
expect(existsSync(fresh)).toBe(true);
|
||||
expect(sweepAudits(audits)).toEqual([]);
|
||||
});
|
||||
|
||||
it("removes worktree for deleted task after minimum age floor", async () => {
|
||||
const stale = tempMergeDir("fusion-ai-merge-fn-999-deletedtaskstale");
|
||||
makeAge(stale, MIN_TEMP_WORKTREE_REAP_AGE_MS + 1_000);
|
||||
const { manager, audits } = makeManager({}, missingTask());
|
||||
|
||||
await expect(sweep(manager)).resolves.toBe(1);
|
||||
|
||||
expect(existsSync(fresh)).toBe(false);
|
||||
expect(existsSync(stale)).toBe(false);
|
||||
expect(sweepAudits(audits)).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ metadata: expect.objectContaining({ success: true, reason: "deleted-task" }) }),
|
||||
]));
|
||||
});
|
||||
|
||||
it("keeps fresh worktree on transient task lookup error", async () => {
|
||||
const fresh = tempMergeDir("fusion-ai-merge-fn-999-lookuperrorfresh");
|
||||
makeAge(fresh, MIN_TEMP_WORKTREE_REAP_AGE_MS - 1_000);
|
||||
const { manager, audits } = makeManager({}, transientErrorTask());
|
||||
|
||||
await expect(sweep(manager)).resolves.toBe(0);
|
||||
|
||||
expect(existsSync(fresh)).toBe(true);
|
||||
expect(sweepAudits(audits)).toEqual([]);
|
||||
});
|
||||
|
||||
it("removes worktree on transient task lookup error only after full stale gate", async () => {
|
||||
const stale = tempMergeDir("fusion-ai-merge-fn-999-lookuperrorstale");
|
||||
makeAge(stale, STALE_TEMP_MERGE_WORKTREE_MS + 1_000);
|
||||
const { manager, audits } = makeManager({}, transientErrorTask());
|
||||
|
||||
await expect(sweep(manager)).resolves.toBe(1);
|
||||
|
||||
expect(existsSync(stale)).toBe(false);
|
||||
expect(sweepAudits(audits)).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ metadata: expect.objectContaining({ success: true, reason: "lookup-error" }) }),
|
||||
]));
|
||||
});
|
||||
|
||||
it("keeps worktree for in-progress task within 2h gate", async () => {
|
||||
const fresh = tempMergeDir("fusion-ai-merge-fn-999-inprogressfresh");
|
||||
const { manager } = makeManager({}, taskWithColumn("in-progress"));
|
||||
@@ -267,7 +307,7 @@ describe("SelfHealingManager temp-dir AI merge worktree sweep", () => {
|
||||
|
||||
it("keeps fresh worktree for done task within grace period", async () => {
|
||||
const fresh = tempMergeDir("fusion-ai-merge-fn-999-donefresh");
|
||||
makeAge(fresh, 5 * 60 * 1000);
|
||||
makeAge(fresh, DONE_TASK_TEMP_WORKTREE_GRACE_MS - 1_000);
|
||||
const { manager } = makeManager({}, taskWithColumn("done"));
|
||||
|
||||
await expect(sweep(manager)).resolves.toBe(0);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
export type ActiveSessionKind = "executor" | "step-session" | "workflow-step" | "step-session-parallel";
|
||||
export type ActiveSessionKind = "executor" | "step-session" | "workflow-step" | "step-session-parallel" | "ai-merge";
|
||||
|
||||
export interface ActiveSessionRegistration {
|
||||
taskId: string;
|
||||
|
||||
@@ -32,7 +32,7 @@
|
||||
*/
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { readdirSync, realpathSync, rmSync } from "node:fs";
|
||||
import { readdirSync, realpathSync, rmSync, statSync } from "node:fs";
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
@@ -64,6 +64,7 @@ import { createRunAuditor, generateSyntheticRunId, type RunAuditor } from "./run
|
||||
import { createLogger } from "./logger.js";
|
||||
import { captureSingleCommitLandedMetadata, type MergerOptions } from "./merger.js";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
import { MIN_TEMP_WORKTREE_REAP_AGE_MS } from "./self-healing.js";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
const aiMergeLog = createLogger("merger-ai");
|
||||
@@ -136,6 +137,18 @@ export async function pruneExistingAiMergeWorktrees(
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
const stat = statSync(canonicalPath);
|
||||
const ageMs = Date.now() - stat.mtimeMs;
|
||||
if (ageMs < MIN_TEMP_WORKTREE_REAP_AGE_MS) {
|
||||
await log(`AI merge pre-merge prune: skipping too-new worktree ${canonicalPath} (age ${Math.max(0, Math.round(ageMs))}ms)`);
|
||||
continue;
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
await log(`AI merge pre-merge prune: failed to stat ${canonicalPath}: ${getErrorMessage(err)} — skipping candidate`);
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
await execFileAsync("git", ["worktree", "remove", "--force", canonicalPath], {
|
||||
cwd: projectRootDir,
|
||||
@@ -910,9 +923,20 @@ export async function runAiMerge(
|
||||
// 1. Clean-room worktree at the integration tip.
|
||||
const mergeRoot = await mkdtemp(join(tmpdir(), `fusion-ai-merge-${taskId.toLowerCase()}-`));
|
||||
let worktreeAdded = false;
|
||||
const registeredMergePaths = new Set<string>();
|
||||
try {
|
||||
await git(["worktree", "add", "--detach", mergeRoot, tipSha], projectRootDir);
|
||||
worktreeAdded = true;
|
||||
let canonicalMergeRoot = mergeRoot;
|
||||
try {
|
||||
canonicalMergeRoot = realpathSync(mergeRoot);
|
||||
} catch {
|
||||
canonicalMergeRoot = mergeRoot;
|
||||
}
|
||||
for (const pathToRegister of new Set([canonicalMergeRoot, mergeRoot])) {
|
||||
activeSessionRegistry.registerPath(pathToRegister, { taskId, kind: "ai-merge", ownerKey: `ai-merge:${taskId}` });
|
||||
registeredMergePaths.add(pathToRegister);
|
||||
}
|
||||
await audit.git({ type: "merge:ai-clean-room", target: integrationBranch, metadata: { taskId, tipSha, mergeRoot } });
|
||||
await log(`AI merge: merging ${branch} into ${integrationBranch} (clean room at ${short(tipSha)})${advanceRetries ? ` — retry ${advanceRetries} after concurrent advance` : ""}`);
|
||||
|
||||
@@ -948,6 +972,9 @@ export async function runAiMerge(
|
||||
await log(`AI merge: advanced ${integrationBranch} → ${short(squashSha)} (local checkout: ${landed.localSync})`);
|
||||
return await finalizeMerged(store, projectRootDir, taskId, task, branch, integrationBranch, squashSha, audit, log, { empty: false });
|
||||
} finally {
|
||||
for (const registeredPath of registeredMergePaths) {
|
||||
activeSessionRegistry.unregisterPath(registeredPath);
|
||||
}
|
||||
await cleanupAiMergeWorktree({ taskId, mergeRoot, projectRootDir, worktreeAdded, audit, log });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,8 +75,9 @@ const BOARD_STALL_NOTIFICATION_COOLDOWN_MS = 60 * 60_000;
|
||||
const DB_CORRUPTION_NOTIFICATION_COOLDOWN_MS = 60 * 60 * 1000;
|
||||
const FTS_MAINTENANCE_MERGE_CADENCE_TICKS = 1;
|
||||
const FTS_MAINTENANCE_OPTIMIZE_CADENCE_TICKS = 4;
|
||||
const STALE_TEMP_MERGE_WORKTREE_MS = 2 * 60 * 60 * 1000;
|
||||
const DONE_TASK_TEMP_WORKTREE_GRACE_MS = 10 * 60 * 1000;
|
||||
export const STALE_TEMP_MERGE_WORKTREE_MS = 2 * 60 * 60 * 1000;
|
||||
export const DONE_TASK_TEMP_WORKTREE_GRACE_MS = 10 * 60 * 1000;
|
||||
export const MIN_TEMP_WORKTREE_REAP_AGE_MS = DONE_TASK_TEMP_WORKTREE_GRACE_MS;
|
||||
// Live pathology peaked around 775 KB/task (~96 MB for ~120 tasks), while a
|
||||
// rebuilt healthy index was ~0.1 MB. Keep the steady-state budget generous but
|
||||
// bounded so sustained text churn heals before segment growth becomes material.
|
||||
@@ -100,6 +101,14 @@ function extractTaskIdFromTempMergeDir(dirname: string): string | null {
|
||||
return match?.[1]?.toUpperCase() ?? null;
|
||||
}
|
||||
|
||||
function getErrorMessage(err: unknown): string {
|
||||
return err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
function isTaskNotFoundError(err: unknown): boolean {
|
||||
return /\btask\s+fn-\d+\s+not found\b/i.test(getErrorMessage(err));
|
||||
}
|
||||
|
||||
type BranchGroupLandingRecorder = {
|
||||
recordBranchGroupMemberLanded?: (groupId: string, payload: {
|
||||
taskId: string;
|
||||
@@ -8919,12 +8928,19 @@ export class SelfHealingManager {
|
||||
ageGateMs = DONE_TASK_TEMP_WORKTREE_GRACE_MS;
|
||||
cleanupReason = "done-task-stale";
|
||||
}
|
||||
} catch {
|
||||
ageGateMs = 0;
|
||||
cleanupReason = "deleted-task";
|
||||
} catch (err: unknown) {
|
||||
if (isTaskNotFoundError(err)) {
|
||||
ageGateMs = MIN_TEMP_WORKTREE_REAP_AGE_MS;
|
||||
cleanupReason = "deleted-task";
|
||||
} else {
|
||||
const errorMessage = getErrorMessage(err);
|
||||
cleanupReason = "lookup-error";
|
||||
log.warn(`[self-healing] temp-dir sweep: task lookup failed for ${taskId}: ${errorMessage}; using conservative age gate`);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (ageGateMs > 0 && ageMs < ageGateMs) continue;
|
||||
ageGateMs = Math.max(ageGateMs, MIN_TEMP_WORKTREE_REAP_AGE_MS);
|
||||
if (ageMs < ageGateMs) continue;
|
||||
try {
|
||||
canonicalPath = realpathSync(path);
|
||||
} catch {
|
||||
|
||||
@@ -107,7 +107,6 @@ export default defineConfig({
|
||||
"dist/**",
|
||||
"src/__tests__/merger-file-scope-invariant.test.ts",
|
||||
"src/__tests__/project-engine-manager.test.ts",
|
||||
"src/__tests__/merger-ai-cleanup.test.ts",
|
||||
"src/__tests__/self-healing-already-merged.real-git.test.ts",
|
||||
],
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user