root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(FN-5455): merge fusion/fn-5455

Browse Source
This commit is contained in:
gsxdsm
2026-05-22 21:08:00 -07:00
parent ba9d6326e4
commit 4a99e3fba2
8 changed files with 138 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
packages/cli/src/commands/__tests__/task-lifecycle.test.ts
View File

@@ -23,7 +23,11 @@ vi.mock("node:child_process", () => ({
},
}));
import { processPullRequestMergeTask, getTaskBranchName } from "../task-lifecycle.js";
import {
cleanupMergedTaskArtifacts,
processPullRequestMergeTask,
getTaskBranchName,
} from "../task-lifecycle.js";
interface MockTask {
id: string;
@@ -915,3 +919,41 @@ describe("processPullRequestMergeTask", () => {
});
});
});
describe("cleanupMergedTaskArtifacts FN-5455", () => {
beforeEach(() => {
execMock.mockReset();
execMock.mockReturnValue("");
});
it("FN-5455: releases pool lease before removing worktree and deleting branch", async () => {
const pool = { release: vi.fn() };
await cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-A", worktree: "/repo/wt" } as never, { pool } as never);
expect(pool.release).toHaveBeenCalledWith("/repo/wt", "FN-5455-A");
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git worktree remove "/repo/wt" --force'), expect.any(Object));
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-a"'), expect.any(Object));
});
it("FN-5455: pool omitted keeps backward-compatible cleanup behavior", async () => {
await cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-B", worktree: "/repo/wt-b" } as never);
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git worktree remove "/repo/wt-b" --force'), expect.any(Object));
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-b"'), expect.any(Object));
});
it("FN-5455: undefined worktree skips pool interaction and worktree removal", async () => {
const pool = { release: vi.fn() };
await cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-C", worktree: undefined } as never, { pool } as never);
expect(pool.release).not.toHaveBeenCalled();
expect(execMock).not.toHaveBeenCalledWith(expect.stringContaining("git worktree remove"), expect.anything());
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-c"'), expect.any(Object));
});
it("FN-5455: release errors are swallowed and cleanup continues", async () => {
const pool = { release: vi.fn(() => { throw new Error("boom"); }) };
await expect(
cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-D", worktree: "/repo/wt-d" } as never, { pool } as never),
).resolves.toBeUndefined();
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git worktree remove "/repo/wt-d" --force'), expect.any(Object));
expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-d"'), expect.any(Object));
});
});

4
packages/cli/src/commands/daemon.ts
View File

@@ -332,8 +332,8 @@ export async function runDaemon(opts: DaemonOptions = {}) {
const engineManager = new ProjectEngineManager(sharedCentralCore, {
getMergeStrategy,
processPullRequestMerge: (s, wd, taskId) =>
processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker),
processPullRequestMerge: (s, wd, taskId, pool) =>
processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool),
getTaskMergeBlocker,
onInsightRunProcessed: (s: unknown, r: unknown) => onMemoryInsightRunProcessed(s as ScheduledTask, r as AutomationRunResult),
});

13
packages/cli/src/commands/dashboard.ts
View File

@@ -1197,7 +1197,14 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
const settings = await store.getSettings();
if (getMergeStrategy(settings) === "pull-request") {
const githubClient = new GitHubClient();
const outcome = await processPullRequestMergeTask(store, cwd, taskId, githubClient, getTaskMergeBlocker);
const outcome = await processPullRequestMergeTask(
store,
cwd,
taskId,
githubClient,
getTaskMergeBlocker,
worktreePool,
);
const task = await store.getTask(taskId);
return {
task,
@@ -1510,8 +1517,8 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
const engineManager = new ProjectEngineManager(centralCoreForEngine, {
getMergeStrategy,
processPullRequestMerge: (s, wd, taskId) =>
processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker),
processPullRequestMerge: (s, wd, taskId, pool) =>
processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool),
getTaskMergeBlocker,
});

4
packages/cli/src/commands/serve.ts
View File

@@ -358,8 +358,8 @@ export async function runServe(
const engineManager = new ProjectEngineManager(sharedCentralCore, {
getMergeStrategy,
processPullRequestMerge: (s, wd, taskId) =>
processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker),
processPullRequestMerge: (s, wd, taskId, pool) =>
processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool),
getTaskMergeBlocker,
onInsightRunProcessed: (s: unknown, r: unknown) => onMemoryInsightRunProcessed(s as ScheduledTask, r as AutomationRunResult),
});

28
packages/cli/src/commands/task-lifecycle.ts
View File

@@ -20,6 +20,7 @@ import type { TaskStore } from "@fusion/core";
import { resolveTaskMergeTarget } from "@fusion/core";
import type { Settings, TaskDetail, PrInfo, MergeResult } from "@fusion/core";
import { resolveIntegrationBranch } from "@fusion/engine";
import type { WorktreePool } from "@fusion/engine";
/**
* Minimal interface for GitHub operations needed by the PR merge workflow.
@@ -140,10 +141,26 @@ function buildPullRequestBody(task: Pick<TaskDetail, "id" | "description">): str
* Clean up worktree and branch artifacts after a successful merge.
* Both operations are best-effort; errors are logged but don't propagate.
*/
export async function cleanupMergedTaskArtifacts(cwd: string, task: Pick<TaskDetail, "id" | "worktree">): Promise<void> {
/**
* @param options.pool Optional runtime worktree pool; FN-5455/FN-4954 require best-effort
* release before force-removing merged PR worktrees.
*/
export async function cleanupMergedTaskArtifacts(
cwd: string,
task: Pick<TaskDetail, "id" | "worktree">,
options?: { pool?: WorktreePool },
): Promise<void> {
const branch = getTaskBranchName(task.id);
if (task.worktree) {
if (options?.pool) {
try {
options.pool.release(task.worktree, task.id);
} catch {
// Best-effort cleanup — release may fail if pool state is already divergent.
}
}
try {
await execAsync(`git worktree remove "${task.worktree}" --force`, {
cwd,
@@ -177,8 +194,9 @@ async function finalizePullRequestMerge(
task: TaskDetail,
prInfo: PrInfo,
message = "Pull request merged",
pool?: WorktreePool,
): Promise<void> {
await cleanupMergedTaskArtifacts(cwd, task);
await cleanupMergedTaskArtifacts(cwd, task, { pool });
await store.updateTask(task.id, { status: null, mergeRetries: 0 });
const movedTask = await store.moveTask(task.id, "done");
const mergedTask = movedTask ?? (await store.getTask(task.id));
@@ -240,6 +258,7 @@ export async function processPullRequestMergeTask(
taskId: string,
github: GitHubOperations,
getTaskMergeBlocker: TaskMergeBlockerFn,
pool?: WorktreePool,
): Promise<ProcessPullRequestResult> {
const task = await store.getTask(taskId);
if (getTaskMergeBlocker(task)) {
@@ -304,7 +323,7 @@ export async function processPullRequestMergeTask(
await store.updatePrInfo(task.id, refreshedPrInfo);
if (mergeStatus.prInfo.status === "merged") {
await finalizePullRequestMerge(store, cwd, task, prInfo);
await finalizePullRequestMerge(store, cwd, task, prInfo, "Pull request merged", pool);
return "merged";
}
@@ -359,6 +378,7 @@ export async function processPullRequestMergeTask(
task,
refreshedAfterFailure,
"Pull request already merged after merge command failed; reconciled task state from GitHub",
pool,
);
return "merged";
}
@@ -366,6 +386,6 @@ export async function processPullRequestMergeTask(
throw err;
}
await store.updatePrInfo(task.id, { ...mergedPr, lastCheckedAt: new Date().toISOString() });
await finalizePullRequestMerge(store, cwd, task, mergedPr);
await finalizePullRequestMerge(store, cwd, task, mergedPr, "Pull request merged", pool);
return "merged";
}

52
packages/engine/src/__tests__/reliability-interactions/pr-mode-worktree-invariants.test.ts
View File

@@ -239,18 +239,54 @@ describe("FN-5420 reliability interactions: PR mode worktree invariants", () =>
}
});
it.fails("FN-5420/FN-4954/FN-4811 follow-up required: cleanup should release pool lease and clear active-session registry", async () => {
it("FN-5455: PR-mode cleanupMergedTaskArtifacts releases WorktreePool lease for task worktree", async () => {
const pool = new WorktreePool();
const path = "/tmp/fn-5420-leased";
(pool as any).getLeasedPaths().set(path, "FN-5420-LEAK");
activeSessionRegistry.registerPath(path, { taskId: "FN-5420-LEAK", kind: "executor", ownerKey: "FN-5420-LEAK" });
const path = "/tmp/fn-5455-leased";
(pool as any).getLeasedPaths().set(path, "FN-5455-LEAK");
const spy = vi.spyOn((pool as any).constructor.prototype, "release");
const releaseSpy = vi.spyOn(pool, "release");
const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
await cleanupMergedTaskArtifacts("/tmp", { id: "FN-5420-LEAK", worktree: path } as any);
await cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-LEAK", worktree: path } as any, { pool });
expect(spy).toHaveBeenCalled();
expect(activeSessionRegistry.lookupByPath(path)).toBeNull();
expect(releaseSpy).toHaveBeenCalledTimes(1);
expect(releaseSpy).toHaveBeenCalledWith(path, "FN-5455-LEAK");
expect(pool.getLeasedPaths().has(path)).toBe(false);
expect(pool.getLeasedPaths().get(path)).toBeUndefined();
});
it("FN-5455: cleanupMergedTaskArtifacts is a no-op for pool when options.pool is omitted", async () => {
const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
await expect(
cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-NO-POOL", worktree: "/tmp/fn-5455-no-pool" } as any),
).resolves.toBeUndefined();
});
it("FN-5455: cleanupMergedTaskArtifacts calls pool.release even when worktree directory is already gone", async () => {
const pool = new WorktreePool();
const path = "/tmp/fn-5455-missing";
(pool as any).getLeasedPaths().set(path, "FN-5455-MISSING");
const releaseSpy = vi.spyOn(pool, "release");
const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
await cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-MISSING", worktree: path } as any, { pool });
expect(releaseSpy).toHaveBeenCalledWith(path, "FN-5455-MISSING");
expect(pool.getLeasedPaths().has(path)).toBe(false);
});
it("FN-5455: cleanupMergedTaskArtifacts swallows pool.release errors (best-effort)", async () => {
const pool = new WorktreePool();
const path = "/tmp/fn-5455-release-throws";
const releaseSpy = vi.spyOn(pool, "release").mockImplementation(() => {
throw new Error("release failed");
});
const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
await expect(cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-THROW", worktree: path } as any, { pool })).resolves.toBeUndefined();
expect(releaseSpy).toHaveBeenCalledWith(path, "FN-5455-THROW");
});
it.skip("FN-5456 follow-up required: cleanup should clear active-session registry entry", async () => {
const path = "/tmp/fn-5456-session";
activeSessionRegistry.registerPath(path, { taskId: "FN-5456", kind: "executor", ownerKey: "FN-5456" });
expect(activeSessionRegistry.lookupByPath(path)).not.toBeNull();
});
it("FN-5420/FN-5279: mergeIntegrationWorktree setting does not gate PR-mode processing", async () => {

9
packages/engine/src/project-engine.ts
View File

@@ -14,6 +14,7 @@ import { compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, sortTasksB
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import { InProcessRuntime } from "./runtimes/in-process-runtime.js";
import type { WorktreePool } from "./worktree-pool.js";
import type { ProjectRuntimeConfig } from "./project-runtime.js";
import { PrMonitor } from "./pr-monitor.js";
import { PrCommentHandler } from "./pr-comment-handler.js";
@@ -55,6 +56,7 @@ export type ProcessPullRequestMergeFn = (
store: TaskStore,
cwd: string,
taskId: string,
pool?: WorktreePool,
) => Promise<"merged" | "waiting" | "skipped">;
const execFileAsync = promisify(execFile);
@@ -1483,7 +1485,12 @@ export class ProjectEngine {
if (mergeStrategy === "pull-request" && this.options.processPullRequestMerge) {
this.activeMergeTaskId = taskId;
runtimeLog.log(`${manualResolver ? "Manual" : "Auto"}-merge processing PR flow for ${taskId}...`);
const result = await this.options.processPullRequestMerge(store, cwd, taskId);
const result = await this.options.processPullRequestMerge(
store,
cwd,
taskId,
(this.runtime as any).worktreePool,
);
if (result === "merged") {
runtimeLog.log(`${manualResolver ? "Manual" : "Auto"}-merge PR merged: ${taskId}`);
const mergedTask = await store.getTask(taskId).catch(() => null);