From 0006408181fc21adf94b4364d9e24b46e5578042 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 2 Jun 2026 08:35:47 -0700 Subject: [PATCH] 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 --- AGENTS.md | 1 + .../ai-merge-ff-landed-files.test.ts | 150 ++++++++++++++++++ .../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(-) create mode 100644 packages/engine/src/__tests__/reliability-interactions/ai-merge-ff-landed-files.test.ts diff --git a/AGENTS.md b/AGENTS.md index ab17ea6ba..c0aedf88b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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. --- diff --git a/packages/engine/src/__tests__/reliability-interactions/ai-merge-ff-landed-files.test.ts b/packages/engine/src/__tests__/reliability-interactions/ai-merge-ff-landed-files.test.ts new file mode 100644 index 000000000..3c2c27f0f --- /dev/null +++ b/packages/engine/src/__tests__/reliability-interactions/ai-merge-ff-landed-files.test.ts @@ -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(); +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); +}); diff --git a/packages/engine/src/__tests__/self-healing.test.ts b/packages/engine/src/__tests__/self-healing.test.ts index 184d7abc7..18e547aa5 100644 --- a/packages/engine/src/__tests__/self-healing.test.ts +++ b/packages/engine/src/__tests__/self-healing.test.ts @@ -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).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).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" }); diff --git a/packages/engine/src/merger-ai.ts b/packages/engine/src/merger-ai.ts index e86c1e432..04c2260b8 100644 --- a/packages/engine/src/merger-ai.ts +++ b/packages/engine/src/merger-ai.ts @@ -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, opts: { empty: boolean }, ): Promise { + 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).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, diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 965cc56be..64e364283 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -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> { + 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 diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 7e99d0733..f5763ed01 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -5758,7 +5758,11 @@ export class SelfHealingManager { async recoverDoneTaskMergeMetadata(): Promise { 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++;