FN-5874: persist fast-forward merge details
Persist merge metadata for AI fast-forward landings and done-task recovery. - capture landed files and shortstat metadata from the single landed commit in the AI merge finalizer - persist mergeDetails and modifiedFiles for landed squash commits, and record commit associations without setting no-op attribution flags - extend done-task self-healing to backfill merge metadata when baseCommitSha exists but mergeDetails is empty - add reliability and self-healing coverage for landed-file persistence, empty AI merges, and recovery behavior Files changed: AGENTS.md | 1 + .../ai-merge-ff-landed-files.test.ts | 150 +++++++++++++++++++++ packages/engine/src/__tests__/self-healing.test.ts | 76 +++++++++++ packages/engine/src/merger-ai.ts | 40 +++++- packages/engine/src/merger.ts | 30 ++++- packages/engine/src/self-healing.ts | 11 +- 6 files changed, 303 insertions(+), 5 deletions(-) Fusion-Task-Id: FN-5874 Fusion-Task-Lineage: a407910c-9ce8-4049-86ef-e80f045c981a
This commit is contained in:
@@ -173,6 +173,7 @@ Scoped exception (FN-5819): shared-branch-group members (`branchContext.assignme
|
||||
- FN-5830 backstop: `packages/engine/src/__tests__/reliability-interactions/branch-group-promotion.test.ts` guards branch-group completion-gate + promotion lifecycle so completion detection drives exactly one shared→default promotion, re-calls stay idempotent, and gated paths emit promotion-gated telemetry without promoting.
|
||||
- FN-5820 backstop: `packages/engine/src/__tests__/reliability-interactions/shared-branch-group-lifecycle.test.ts` guards the full shared-branch-group lifecycle—concurrent distinct-worktree execution, member→shared-branch accumulation, single shared→main completion-gate promotion with idempotent re-evaluation, gate-disabled integration-without-promotion, and per-task-derived/ungrouped no-regression.
|
||||
- FN-5866 backstop: `packages/engine/src/__tests__/reliability-interactions/post-done-continuation-no-wedge.test.ts` guards the post-done non-continuable-session seam so completed executor work stays cleanly in `in-review` while incomplete tasks still fail normally.
|
||||
- FN-5874 backstop: `packages/engine/src/__tests__/reliability-interactions/ai-merge-ff-landed-files.test.ts` guards AI-merge fast-forward finalizer persistence of `mergeDetails.commitSha`, `landedFiles`, and `modifiedFiles`, verifies no-op landings do not fabricate metadata, and confirms normal squash landings do not set FN-5103 attribution-restriction flags; companion coverage in `packages/engine/src/__tests__/self-healing.test.ts` extends `recoverDoneTaskMergeMetadata` so done tasks with empty `mergeDetails` but a recorded `baseCommitSha` are backfilled via owned-commit discovery while FN-5103 skip guards still prevent overwrite.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
import { afterAll, describe, expect, it, vi } from "vitest";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { execSync } from "node:child_process";
|
||||
import { DEFAULT_SETTINGS, TaskStore, type Settings } from "@fusion/core";
|
||||
import { runAiMerge } from "../../merger-ai.js";
|
||||
import { hasGit } from "./_helpers.js";
|
||||
|
||||
const tracked = new Set<string>();
|
||||
const RM = { recursive: true, force: true, maxRetries: 5, retryDelay: 50 } as const;
|
||||
|
||||
afterAll(() => {
|
||||
for (const dir of tracked) {
|
||||
try {
|
||||
rmSync(dir, RM);
|
||||
} catch {
|
||||
// best effort cleanup
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
function git(cwd: string, args: string): string {
|
||||
return execSync(`git ${args}`, { cwd, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim();
|
||||
}
|
||||
|
||||
function realMergeAgent(branch: string) {
|
||||
return vi.fn(async (cwd: string) => {
|
||||
execSync(`git merge --squash ${branch}`, { cwd, stdio: "pipe" });
|
||||
execSync("git add -A", { cwd, stdio: "pipe" });
|
||||
execSync('git commit -q -m "squash: feature"', { cwd, stdio: "pipe" });
|
||||
});
|
||||
}
|
||||
|
||||
async function createFixture(taskId: string, branch = `fusion/${taskId.toLowerCase()}`) {
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "fusion-ai-merge-ff-"));
|
||||
tracked.add(rootDir);
|
||||
git(rootDir, "init -q -b main");
|
||||
git(rootDir, 'config user.email "test@example.com"');
|
||||
git(rootDir, 'config user.name "Test User"');
|
||||
writeFileSync(join(rootDir, "README.md"), "# fixture\n");
|
||||
git(rootDir, "add README.md");
|
||||
git(rootDir, 'commit -q -m "chore: init"');
|
||||
|
||||
const store = new TaskStore(rootDir, undefined, { inMemoryDb: true });
|
||||
await store.init();
|
||||
const settings: Settings = {
|
||||
...DEFAULT_SETTINGS,
|
||||
autoMerge: true,
|
||||
includeTaskIdInCommit: true,
|
||||
commitAuthorEnabled: false,
|
||||
merger: { ...(DEFAULT_SETTINGS.merger ?? {}), mode: "ai", maxReviewPasses: 1 },
|
||||
} as Settings;
|
||||
await store.updateSettings(settings);
|
||||
|
||||
const created = await store.createTask({
|
||||
title: taskId,
|
||||
description: "AI merge landed-files fixture",
|
||||
column: "in-review",
|
||||
branch,
|
||||
baseBranch: "main",
|
||||
prompt: "## File Scope\n- packages/engine/src/**\n",
|
||||
} as any);
|
||||
await store.updateTask(created.id, {
|
||||
column: "in-review",
|
||||
branch,
|
||||
baseBranch: "main",
|
||||
steps: [{ title: "ready", status: "done" }],
|
||||
status: null,
|
||||
} as any);
|
||||
const task = await store.getTask(created.id);
|
||||
|
||||
return {
|
||||
rootDir,
|
||||
store,
|
||||
task,
|
||||
cleanup: async () => {
|
||||
store.close();
|
||||
rmSync(rootDir, RM);
|
||||
tracked.delete(rootDir);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("FN-5874 AI-merge ff landed-files persistence (real git)", () => {
|
||||
it.skipIf(!hasGit)("persists mergeDetails and modifiedFiles for a landed squash commit", async () => {
|
||||
const fixture = await createFixture("FN-5874-RI");
|
||||
const { rootDir, store, task, cleanup } = fixture;
|
||||
|
||||
try {
|
||||
git(rootDir, `checkout -q -b ${task.branch}`);
|
||||
writeFileSync(join(rootDir, "feature.txt"), "feature work\n");
|
||||
writeFileSync(join(rootDir, "notes.txt"), "details\n");
|
||||
git(rootDir, "add feature.txt notes.txt");
|
||||
git(rootDir, 'commit -q -m "feat: task work"');
|
||||
git(rootDir, "checkout -q main");
|
||||
const mainBefore = git(rootDir, "rev-parse main");
|
||||
|
||||
const result = await runAiMerge(store, rootDir, task!.id, { manual: true, allowDirtyLocalCheckoutSync: true }, {
|
||||
mergeAgent: realMergeAgent(task!.branch!),
|
||||
reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"),
|
||||
});
|
||||
|
||||
const landedTask = await store.getTask(task!.id);
|
||||
expect(result.merged).toBe(true);
|
||||
expect(git(rootDir, "rev-parse main")).not.toBe(mainBefore);
|
||||
expect(landedTask?.column).toBe("done");
|
||||
expect(landedTask?.mergeDetails).toEqual(expect.objectContaining({
|
||||
commitSha: result.commitSha,
|
||||
mergeConfirmed: true,
|
||||
filesChanged: 2,
|
||||
landedFiles: ["feature.txt", "notes.txt"],
|
||||
}));
|
||||
expect(landedTask?.mergeDetails?.landedFilesAttributionRestricted).toBeUndefined();
|
||||
expect(landedTask?.mergeDetails?.noOpVerifiedShortCircuit).toBeUndefined();
|
||||
expect(landedTask?.modifiedFiles).toEqual(["feature.txt", "notes.txt"]);
|
||||
} finally {
|
||||
await cleanup();
|
||||
}
|
||||
}, 20_000);
|
||||
|
||||
it.skipIf(!hasGit)("does not fabricate merge metadata for an empty AI merge", async () => {
|
||||
const fixture = await createFixture("FN-5874-NOOP");
|
||||
const { rootDir, store, task, cleanup } = fixture;
|
||||
|
||||
try {
|
||||
git(rootDir, `checkout -q -b ${task.branch}`);
|
||||
git(rootDir, "checkout -q main");
|
||||
const mainBefore = git(rootDir, "rev-parse main");
|
||||
|
||||
const result = await runAiMerge(store, rootDir, task!.id, { manual: true, allowDirtyLocalCheckoutSync: true }, {
|
||||
mergeAgent: vi.fn(async () => {
|
||||
// Leave HEAD at the integration tip so mergeAndReview treats it as empty.
|
||||
}),
|
||||
reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"),
|
||||
});
|
||||
|
||||
const landedTask = await store.getTask(task!.id);
|
||||
expect(result.noOp).toBe(true);
|
||||
expect(result.merged).toBe(false);
|
||||
expect(git(rootDir, "rev-parse main")).toBe(mainBefore);
|
||||
expect(landedTask?.column).toBe("done");
|
||||
expect(landedTask?.mergeDetails?.commitSha).toBeUndefined();
|
||||
expect(landedTask?.mergeDetails?.landedFiles).toBeUndefined();
|
||||
expect(landedTask?.modifiedFiles).toBeUndefined();
|
||||
} finally {
|
||||
await cleanup();
|
||||
}
|
||||
}, 20_000);
|
||||
});
|
||||
@@ -7016,6 +7016,82 @@ describe("recoverDoneTaskMergeMetadata", () => {
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("repairs empty mergeDetails for landed done task with recorded base commit", async () => {
|
||||
const store = createMockStore();
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-5854",
|
||||
column: "done",
|
||||
paused: false,
|
||||
baseCommitSha: "base5854",
|
||||
mergeDetails: undefined,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.spyOn(manager as any, "findLandedTaskCommit").mockResolvedValue({
|
||||
sha: "be95b3f2c1234567",
|
||||
subject: "FN-5854: landed",
|
||||
filesChanged: 2,
|
||||
insertions: 75,
|
||||
deletions: 0,
|
||||
});
|
||||
vi.spyOn(manager as any, "readLandedFilesForSha").mockResolvedValue([
|
||||
"packages/engine/src/merger-ai.ts",
|
||||
"packages/engine/src/self-healing.ts",
|
||||
]);
|
||||
|
||||
const repaired = await manager.recoverDoneTaskMergeMetadata();
|
||||
|
||||
expect(repaired).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-5854", {
|
||||
mergeDetails: expect.objectContaining({
|
||||
commitSha: "be95b3f2c1234567",
|
||||
filesChanged: 2,
|
||||
insertions: 75,
|
||||
deletions: 0,
|
||||
mergeCommitMessage: "FN-5854: landed",
|
||||
mergeConfirmed: true,
|
||||
landedFiles: [
|
||||
"packages/engine/src/merger-ai.ts",
|
||||
"packages/engine/src/self-healing.ts",
|
||||
],
|
||||
}),
|
||||
modifiedFiles: [
|
||||
"packages/engine/src/merger-ai.ts",
|
||||
"packages/engine/src/self-healing.ts",
|
||||
],
|
||||
});
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("leaves empty mergeDetails untouched when no owned landed commit is found", async () => {
|
||||
const store = createMockStore();
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-5854-MISS",
|
||||
column: "done",
|
||||
paused: false,
|
||||
baseCommitSha: "base5854",
|
||||
mergeDetails: undefined,
|
||||
},
|
||||
]);
|
||||
|
||||
vi.spyOn(manager as any, "findLandedTaskCommit").mockResolvedValue(null);
|
||||
|
||||
const repaired = await manager.recoverDoneTaskMergeMetadata();
|
||||
|
||||
expect(repaired).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).not.toHaveBeenCalled();
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("FN-3862: confirmed task with reachable owned stored SHA preserves canonical commitSha", async () => {
|
||||
const store = createMockStore();
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
|
||||
@@ -37,12 +37,14 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
buildTaskLineageTrailer,
|
||||
getPrimaryPrInfo,
|
||||
getTaskMergeBlocker,
|
||||
resolveAgentPrompt,
|
||||
resolvePersistAgentThinkingLog,
|
||||
resolveTaskMergeTarget,
|
||||
resolveValidatorSettingsModel,
|
||||
type AgentPromptsConfig,
|
||||
type MergeDetails,
|
||||
type MergeResult,
|
||||
type Settings,
|
||||
type Task,
|
||||
@@ -59,7 +61,7 @@ import { checkSessionError } from "./usage-limit-detector.js";
|
||||
import { accumulateSessionTokenUsage } from "./session-token-usage.js";
|
||||
import { createRunAuditor, generateSyntheticRunId, type RunAuditor } from "./run-audit.js";
|
||||
import { createLogger } from "./logger.js";
|
||||
import type { MergerOptions } from "./merger.js";
|
||||
import { captureSingleCommitLandedMetadata, type MergerOptions } from "./merger.js";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
const aiMergeLog = createLogger("merger-ai");
|
||||
@@ -933,6 +935,40 @@ async function finalizeMerged(
|
||||
log: (message: string) => Promise<void>,
|
||||
opts: { empty: boolean },
|
||||
): Promise<MergeResult> {
|
||||
let mergeDetails: MergeDetails | undefined;
|
||||
let modifiedFiles: string[] | undefined;
|
||||
if (!opts.empty && landedSha) {
|
||||
const [{ landedFiles: capturedLandedFiles, filesChanged, insertions, deletions }, mergeCommitMessage] = await Promise.all([
|
||||
captureSingleCommitLandedMetadata(projectRootDir, landedSha),
|
||||
git(["log", "-1", "--format=%s", landedSha], projectRootDir).catch(() => ""),
|
||||
]);
|
||||
const landedFiles = capturedLandedFiles ?? [];
|
||||
const mergedAt = new Date().toISOString();
|
||||
mergeDetails = {
|
||||
commitSha: landedSha,
|
||||
landedFiles,
|
||||
filesChanged,
|
||||
insertions,
|
||||
deletions,
|
||||
mergeCommitMessage: mergeCommitMessage || undefined,
|
||||
mergedAt,
|
||||
mergeConfirmed: true,
|
||||
prNumber: getPrimaryPrInfo(task)?.number,
|
||||
};
|
||||
modifiedFiles = landedFiles.length > 0 ? landedFiles : undefined;
|
||||
await store.updateTask(taskId, { mergeDetails, modifiedFiles });
|
||||
if (task.lineageId && typeof (store as Partial<TaskStore>).upsertTaskCommitAssociation === "function") {
|
||||
await store.upsertTaskCommitAssociation({
|
||||
taskLineageId: task.lineageId,
|
||||
taskIdSnapshot: task.id,
|
||||
commitSha: landedSha,
|
||||
commitSubject: mergeCommitMessage || task.title || task.id,
|
||||
authoredAt: mergedAt,
|
||||
matchedBy: "canonical-lineage-trailer",
|
||||
confidence: "canonical",
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
let branchDeleted = false;
|
||||
// NEVER delete the integration branch itself — a task whose branch name
|
||||
// coincides with the target (or merges into its own branch) must not have the
|
||||
@@ -955,7 +991,7 @@ async function finalizeMerged(
|
||||
noOp: opts.empty,
|
||||
ok: true,
|
||||
reason: opts.empty ? "no-net-changes" : undefined,
|
||||
commitSha: opts.empty ? undefined : landedSha,
|
||||
commitSha: opts.empty ? undefined : mergeDetails?.commitSha ?? landedSha,
|
||||
mergeConfirmed: !opts.empty,
|
||||
worktreeRemoved,
|
||||
branchDeleted,
|
||||
|
||||
@@ -5934,7 +5934,7 @@ function quoteArg(value: string): string {
|
||||
return `"${value.replace(/(["\\$`])/g, "\\$1")}"`;
|
||||
}
|
||||
|
||||
function parseShortstatSummary(statsOutput: string): { filesChanged: number; insertions: number; deletions: number } {
|
||||
export function parseShortstatSummary(statsOutput: string): { filesChanged: number; insertions: number; deletions: number } {
|
||||
const normalized = statsOutput.trim().replace(/\n/g, " ");
|
||||
const filesMatch = normalized.match(/(\d+) files? changed/);
|
||||
const insertionsMatch = normalized.match(/(\d+) insertions?\(\+\)/);
|
||||
@@ -5946,6 +5946,34 @@ function parseShortstatSummary(statsOutput: string): { filesChanged: number; ins
|
||||
};
|
||||
}
|
||||
|
||||
export async function captureSingleCommitLandedMetadata(
|
||||
rootDir: string,
|
||||
sha: string,
|
||||
): Promise<Pick<MergeDetails, "landedFiles" | "filesChanged" | "insertions" | "deletions">> {
|
||||
const [{ stdout: landedFilesOutput }, { stdout: shortstatOutput }] = await Promise.all([
|
||||
execAsync(`git show --name-only --format= ${quoteArg(sha)}`, {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
maxBuffer: 2 * 1024 * 1024,
|
||||
}),
|
||||
execAsync(`git show --shortstat --format= ${quoteArg(sha)}`, {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
maxBuffer: 2 * 1024 * 1024,
|
||||
}),
|
||||
]);
|
||||
const landedFiles = Array.from(new Set(
|
||||
landedFilesOutput
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean),
|
||||
));
|
||||
return {
|
||||
landedFiles,
|
||||
...parseShortstatSummary(shortstatOutput),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Sums per-commit shortstat output for owned commits. This is intentionally
|
||||
* per-commit (instead of range-based) so rebased/cherry-picked SHAs do not
|
||||
|
||||
@@ -5758,7 +5758,11 @@ export class SelfHealingManager {
|
||||
async recoverDoneTaskMergeMetadata(): Promise<number> {
|
||||
try {
|
||||
const tasks = await this.store.listTasks({ column: "done", slim: true });
|
||||
const candidates = tasks.filter((task) => task.column === "done" && !task.paused && Boolean(task.mergeDetails?.commitSha));
|
||||
const candidates = tasks.filter((task) => {
|
||||
if (task.column !== "done" || task.paused) return false;
|
||||
if (task.mergeDetails?.commitSha) return true;
|
||||
return Boolean(task.baseCommitSha);
|
||||
});
|
||||
if (candidates.length === 0) return 0;
|
||||
|
||||
let repaired = 0;
|
||||
@@ -5769,9 +5773,9 @@ export class SelfHealingManager {
|
||||
}
|
||||
try {
|
||||
const storedSha = task.mergeDetails?.commitSha;
|
||||
if (!storedSha) continue;
|
||||
|
||||
if (task.mergeDetails?.mergeConfirmed === true) {
|
||||
if (!storedSha) continue;
|
||||
const landed = await this.findLandedTaskCommit(task);
|
||||
if (!landed || landed.sha !== storedSha) {
|
||||
log.warn(
|
||||
@@ -5841,6 +5845,9 @@ export class SelfHealingManager {
|
||||
|
||||
const landed = await this.findLandedTaskCommit(task, { preferEarliestOwnedCommit: true });
|
||||
if (!landed) {
|
||||
if (!storedSha) {
|
||||
continue;
|
||||
}
|
||||
await this.store.updateTask(task.id, { mergeDetails: undefined });
|
||||
await this.store.logEntry(task.id, "Auto-recovered: cleared unowned done-task mergeDetails commitSha");
|
||||
repaired++;
|
||||
|
||||
Reference in New Issue
Block a user