feat(FN-1036): add mergeDetails collection and storage in merger

- Add MergeDetails type import and mergeDetails field to TaskStore.updateTask signature - Store merge details (commitSha, filesChanged, insertions, deletions, mergeCommitMessage, resolution info) after successful squash merge - Capture best-effort commitSha even when branch is not found (mergeConfirmed: false) - Add comprehensive tests for store updateTask with mergeDetails and merger detail collection
2026-04-06 21:36:31 -07:00
parent 1c232a17c4
commit ff8a8682c3
4 changed files with 320 additions and 2 deletions
--- a/packages/core/src/store.test.ts
+++ b/packages/core/src/store.test.ts
@@ -5648,4 +5648,62 @@ Task with acceptance criteria
      expect(branchCommands).toHaveLength(0);
    });
  });
+
+  describe("mergeDetails via updateTask", () => {
+    it("can set mergeDetails on a task", async () => {
+      const task = await store.createTask({ description: "test merge details" });
+      await store.moveTask(task.id, "todo");
+      await store.moveTask(task.id, "in-progress");
+      await store.moveTask(task.id, "in-review");
+      await store.moveTask(task.id, "done");
+
+      const mergeDetails = {
+        commitSha: "abc123",
+        filesChanged: 5,
+        insertions: 10,
+        deletions: 3,
+        mergeCommitMessage: "Merge task",
+        mergedAt: new Date().toISOString(),
+        mergeConfirmed: true,
+      };
+
+      const updated = await store.updateTask(task.id, { mergeDetails });
+      expect(updated.mergeDetails).toEqual(mergeDetails);
+
+      // Verify it persists
+      const reloaded = await store.getTask(task.id);
+      expect(reloaded.mergeDetails).toEqual(mergeDetails);
+    });
+
+    it("can clear mergeDetails by passing null", async () => {
+      const task = await store.createTask({ description: "test merge details clear" });
+      await store.moveTask(task.id, "todo");
+      await store.moveTask(task.id, "in-progress");
+      await store.moveTask(task.id, "in-review");
+      await store.moveTask(task.id, "done");
+
+      await store.updateTask(task.id, {
+        mergeDetails: { commitSha: "abc123", mergeConfirmed: true },
+      });
+
+      const cleared = await store.updateTask(task.id, { mergeDetails: null });
+      expect(cleared.mergeDetails).toBeUndefined();
+    });
+
+    it("does not modify mergeDetails when not included in updates", async () => {
+      const task = await store.createTask({ description: "test merge details no-op" });
+      await store.moveTask(task.id, "todo");
+      await store.moveTask(task.id, "in-progress");
+      await store.moveTask(task.id, "in-review");
+      await store.moveTask(task.id, "done");
+
+      await store.updateTask(task.id, {
+        mergeDetails: { commitSha: "def456", mergeConfirmed: true },
+      });
+
+      // Update something unrelated
+      const updated = await store.updateTask(task.id, { summary: "some summary" });
+      expect(updated.mergeDetails).toEqual({ commitSha: "def456", mergeConfirmed: true });
+    });
+  });
 });
--- a/packages/core/src/store.ts
+++ b/packages/core/src/store.ts
@@ -1095,7 +1095,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {

  async updateTask(
    id: string,
-    updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; blockedBy?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
+    updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; blockedBy?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
  ): Promise<Task> {
    return this.withTaskLock(id, async () => {
      // Validate that task doesn't depend on itself
@@ -1223,6 +1223,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
      } else if (updates.workflowStepResults !== undefined) {
        task.workflowStepResults = updates.workflowStepResults;
      }
+      if (updates.mergeDetails === null) {
+        task.mergeDetails = undefined;
+      } else if (updates.mergeDetails !== undefined) {
+        task.mergeDetails = updates.mergeDetails;
+      }
      if (updates.modifiedFiles === null) {
        task.modifiedFiles = undefined;
      } else if (updates.modifiedFiles !== undefined) {
--- a/packages/engine/src/merger.test.ts
+++ b/packages/engine/src/merger.test.ts
@@ -93,6 +93,7 @@ function setupHappyPathExecSync() {
  mockedExecSync.mockImplementation((cmd: any) => {
    const cmdStr = String(cmd);
    if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
+    if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
    if (cmdStr.includes("git log")) return "- feat: something" as any;
    if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
    if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
@@ -101,6 +102,7 @@ function setupHappyPathExecSync() {
    if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
    // Post-agent check: "did agent commit?" → "0" = yes
    if (cmdStr.includes("diff --cached")) return "0" as any;
+    if (cmdStr.includes("show --shortstat")) return "3 files changed, 10 insertions(+), 2 deletions(-)" as any;
    if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
    if (cmdStr.includes("worktree remove")) return Buffer.from("");
    return Buffer.from("");
@@ -2385,3 +2387,189 @@ describe("aiMergeTask — post-merge workflow steps", () => {
    expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
  });
 });
+
+// ── Merge Details Collection Tests ─────────────────────────────────────
+
+describe("aiMergeTask — merge details collection", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockedExistsSync.mockReturnValue(true);
+
+    mockedCreateHaiAgent.mockResolvedValue({
+      session: {
+        prompt: vi.fn().mockResolvedValue(undefined),
+        dispose: vi.fn(),
+      },
+    } as any);
+  });
+
+  it("stores mergeDetails with commitSha and stats after successful merge", async () => {
+    const store = createMockStore(
+      { id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
+      [{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
+    );
+
+    mockedExecSync.mockImplementation((cmd: any) => {
+      const cmdStr = String(cmd);
+      if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
+      if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD "))
+        return "mergedcommit123456789"; // encoding: utf-8 → string
+      if (cmdStr.includes("git log")) return "- feat: something";
+      if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
+      if (cmdStr.includes("--stat")) return "1 file changed";
+      if (cmdStr.includes("merge --squash")) return Buffer.from("");
+      if (cmdStr.includes("diff --name-only --diff-filter=U")) return "";
+      if (cmdStr.includes("diff --cached --quiet")) return "1";
+      if (cmdStr.includes("git commit")) return Buffer.from("");
+      if (cmdStr.includes("show --shortstat"))
+        return "3 files changed, 10 insertions(+), 2 deletions(-)";
+      if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
+      if (cmdStr.includes("worktree remove")) return Buffer.from("");
+      return Buffer.from("");
+    });
+
+    const result = await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    expect(result.merged).toBe(true);
+
+    // Find the updateTask call that set mergeDetails
+    const updateCalls = (store.updateTask as ReturnType<typeof vi.fn>).mock.calls;
+    const mergeDetailsCall = updateCalls.find(
+      (call: any[]) => call[1]?.mergeDetails !== undefined,
+    );
+    expect(mergeDetailsCall).toBeDefined();
+
+    const mergeDetails = mergeDetailsCall![1].mergeDetails;
+    expect(mergeDetails.commitSha).toBe("mergedcommit123456789");
+    expect(mergeDetails.filesChanged).toBe(3);
+    expect(mergeDetails.insertions).toBe(10);
+    expect(mergeDetails.deletions).toBe(2);
+    expect(mergeDetails.mergeCommitMessage).toBe("- feat: something");
+    expect(mergeDetails.mergedAt).toBeDefined();
+    expect(mergeDetails.mergeConfirmed).toBe(true);
+    expect(mergeDetails.resolutionStrategy).toBe("ai");
+    expect(mergeDetails.resolutionMethod).toBe("ai");
+    expect(mergeDetails.attemptsMade).toBe(1);
+  });
+
+  it("stores partial mergeDetails when branch is not found", async () => {
+    const store = createMockStore(
+      { id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
+      [{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
+    );
+
+    mockedExecSync.mockImplementation((cmd: any) => {
+      const cmdStr = String(cmd);
+      // Branch verification fails → branch not found
+      if (cmdStr.includes("rev-parse --verify")) throw new Error("not found");
+      // But rev-parse HEAD still works → can capture commitSha (encoding: utf-8 → string)
+      if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD "))
+        return "existingheadsha999";
+      return Buffer.from("");
+    });
+
+    const result = await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    expect(result.merged).toBe(false);
+    expect(result.error).toContain("not found");
+
+    // Find the updateTask call that set mergeDetails
+    const updateCalls = (store.updateTask as ReturnType<typeof vi.fn>).mock.calls;
+    const mergeDetailsCall = updateCalls.find(
+      (call: any[]) => call[1]?.mergeDetails !== undefined,
+    );
+    expect(mergeDetailsCall).toBeDefined();
+
+    const mergeDetails = mergeDetailsCall![1].mergeDetails;
+    expect(mergeDetails.commitSha).toBe("existingheadsha999");
+    expect(mergeDetails.mergedAt).toBeDefined();
+    expect(mergeDetails.mergeConfirmed).toBe(false);
+  });
+
+  it("completes merge even when git commands fail during merge details collection", async () => {
+    const store = createMockStore(
+      { id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
+      [{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
+    );
+
+    let revParseHeadCalled = false;
+    mockedExecSync.mockImplementation((cmd: any) => {
+      const cmdStr = String(cmd);
+      if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
+      if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) {
+        revParseHeadCalled = true;
+        throw new Error("git rev-parse HEAD failed");
+      }
+      if (cmdStr.includes("git log")) return "- feat: something";
+      if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
+      if (cmdStr.includes("--stat")) return "1 file changed";
+      if (cmdStr.includes("merge --squash")) return Buffer.from("");
+      if (cmdStr.includes("diff --name-only --diff-filter=U")) return "";
+      if (cmdStr.includes("diff --cached --quiet")) return "1";
+      if (cmdStr.includes("git commit")) return Buffer.from("");
+      if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
+      if (cmdStr.includes("worktree remove")) return Buffer.from("");
+      return Buffer.from("");
+    });
+
+    const result = await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    // Merge should still succeed even though merge details collection failed
+    expect(result.merged).toBe(true);
+    expect(revParseHeadCalled).toBe(true);
+
+    // No mergeDetails should have been stored
+    const updateCalls = (store.updateTask as ReturnType<typeof vi.fn>).mock.calls;
+    const mergeDetailsCall = updateCalls.find(
+      (call: any[]) => call[1]?.mergeDetails !== undefined,
+    );
+    expect(mergeDetailsCall).toBeUndefined();
+
+    // Task should still be moved to done
+    expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
+  });
+
+  it("handles missing shortstat gracefully when show --shortstat fails", async () => {
+    const store = createMockStore(
+      { id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
+      [{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
+    );
+
+    mockedExecSync.mockImplementation((cmd: any) => {
+      const cmdStr = String(cmd);
+      if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
+      if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD "))
+        return "mergedcommit123"; // encoding: utf-8 → string
+      if (cmdStr.includes("git log")) return "- feat: something";
+      if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
+      if (cmdStr.includes("--stat")) return "1 file changed";
+      if (cmdStr.includes("merge --squash")) return Buffer.from("");
+      if (cmdStr.includes("diff --name-only --diff-filter=U")) return "";
+      if (cmdStr.includes("diff --cached --quiet")) return "1";
+      if (cmdStr.includes("git commit")) return Buffer.from("");
+      // show --shortstat fails
+      if (cmdStr.includes("show --shortstat")) throw new Error("show failed");
+      if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
+      if (cmdStr.includes("worktree remove")) return Buffer.from("");
+      return Buffer.from("");
+    });
+
+    const result = await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    expect(result.merged).toBe(true);
+
+    // mergeDetails should still be stored with commitSha but without stats
+    const updateCalls = (store.updateTask as ReturnType<typeof vi.fn>).mock.calls;
+    const mergeDetailsCall = updateCalls.find(
+      (call: any[]) => call[1]?.mergeDetails !== undefined,
+    );
+    expect(mergeDetailsCall).toBeDefined();
+
+    const mergeDetails = mergeDetailsCall![1].mergeDetails;
+    expect(mergeDetails.commitSha).toBe("mergedcommit123");
+    // Stats should be undefined since show --shortstat failed (inner catch sets them as undefined)
+    expect(mergeDetails.filesChanged).toBeUndefined();
+    expect(mergeDetails.insertions).toBeUndefined();
+    expect(mergeDetails.deletions).toBeUndefined();
+  });
+});
--- a/packages/engine/src/merger.ts
+++ b/packages/engine/src/merger.ts
@@ -1,6 +1,6 @@
 import { execSync } from "node:child_process";
 import { existsSync } from "node:fs";
-import { getTaskMergeBlocker, type TaskStore, type MergeResult, type WorkflowStep, type WorkflowStepResult, type Settings } from "@fusion/core";
+import { getTaskMergeBlocker, type TaskStore, type MergeResult, type MergeDetails, type WorkflowStep, type WorkflowStepResult, type Settings } from "@fusion/core";
 import { createKbAgent, describeModel, promptWithFallback } from "./pi.js";
 import type { WorktreePool } from "./worktree-pool.js";
 import { AgentLogger } from "./agent-logger.js";
@@ -618,6 +618,26 @@ export async function aiMergeTask(
    });
  } catch {
    result.error = `Branch '${branch}' not found — moving to done without merge`;
+    // Best-effort: try to capture current HEAD commitSha even though branch is missing
+    try {
+      const commitSha = execSync("git rev-parse HEAD", {
+        cwd: rootDir,
+        stdio: "pipe",
+        encoding: "utf-8",
+      }).trim() || undefined;
+      if (commitSha) {
+        await store.updateTask(taskId, {
+          mergeDetails: {
+            commitSha,
+            mergedAt: new Date().toISOString(),
+            mergeConfirmed: false,
+          },
+        });
+        mergerLog.log(`${taskId}: branch not found but captured commitSha ${commitSha.slice(0, 8)}`);
+      }
+    } catch {
+      // No commit SHA available — task will show summary fallback
+    }
    await completeTask(store, taskId, result);
    return result;
  }
@@ -792,6 +812,53 @@ export async function aiMergeTask(
    throw new Error(`AI merge failed for ${taskId}: all 3 attempts exhausted`);
  }

+  // 5b. Collect merge details and store on task
+  try {
+    const commitSha = execSync("git rev-parse HEAD", {
+      cwd: rootDir,
+      stdio: "pipe",
+      encoding: "utf-8",
+    }).trim() || undefined;
+
+    let filesChanged: number | undefined;
+    let insertions: number | undefined;
+    let deletions: number | undefined;
+
+    try {
+      const statsOutput = execSync("git show --shortstat --format= HEAD", {
+        cwd: rootDir,
+        stdio: "pipe",
+        encoding: "utf-8",
+      }).trim();
+      const normalized = statsOutput.replace(/\n/g, " ");
+      const filesMatch = normalized.match(/(\d+) files? changed/);
+      const insertionsMatch = normalized.match(/(\d+) insertions?\(\+\)/);
+      const deletionsMatch = normalized.match(/(\d+) deletions?\(-\)/);
+      filesChanged = filesMatch ? Number.parseInt(filesMatch[1], 10) : 0;
+      insertions = insertionsMatch ? Number.parseInt(insertionsMatch[1], 10) : 0;
+      deletions = deletionsMatch ? Number.parseInt(deletionsMatch[1], 10) : 0;
+    } catch { /* non-fatal */ }
+
+    const mergeDetails: MergeDetails = {
+      commitSha,
+      filesChanged,
+      insertions,
+      deletions,
+      mergeCommitMessage: commitLog,
+      mergedAt: new Date().toISOString(),
+      mergeConfirmed: true,
+      resolutionStrategy: result.resolutionStrategy,
+      resolutionMethod: result.resolutionMethod,
+      attemptsMade: result.attemptsMade,
+      autoResolvedCount: result.autoResolvedCount,
+    };
+
+    await store.updateTask(taskId, { mergeDetails });
+    mergerLog.log(`${taskId}: merge details stored (commitSha: ${commitSha?.slice(0, 8)})`);
+  } catch (err: any) {
+    mergerLog.warn(`${taskId}: failed to collect/store merge details: ${err.message}`);
+  }
+
  // 6. Delete branch
  try {
    execSync(`git branch -d "${branch}"`, { cwd: rootDir, stdio: "pipe" });