fix(FN-4428): address classification safety and recovery coverage

Fusion-Task-Id: FN-4428
Fusion-Task-Lineage: ea174f50-49ac-4105-b6cf-db3bab0984fd

packages/engine/src/__tests__/branch-conflicts-recovery.test.ts

@@ -5,6 +5,7 @@ import path from "node:path";
 import { exec } from "node:child_process";
 import { promisify } from "node:util";
 import {
+  autoRecoverCrossContamination,
   classifyForeignCommits,
   type BranchCrossContaminationCommit,
 } from "../branch-conflicts.js";
@@ -40,9 +41,9 @@ describe("branch contamination recovery classification", () => {
     return { repoDir, baseSha };
   }
 
-  async function makeCommit(repoDir: string, body: string, subject: string, foreignTaskId: string): Promise<BranchCrossContaminationCommit> {
+  async function makeCommit(repoDir: string, body: string, subject: string, foreignTaskId: string, file = "note.txt"): Promise<BranchCrossContaminationCommit> {
-    await appendFile(path.join(repoDir, "note.txt"), `${body}\n`, "utf-8");
+    await appendFile(path.join(repoDir, file), `${body}\n`, "utf-8");
-    await run("git add note.txt", repoDir);
+    await run(`git add ${file}`, repoDir);
     await run(`git commit -m ${JSON.stringify(subject)} -m ${JSON.stringify(`Fusion-Task-Id: ${foreignTaskId}`)}`, repoDir);
     const sha = await run("git rev-parse HEAD", repoDir);
     return { sha, subject, foreignTaskId };
@@ -104,4 +105,33 @@ describe("branch contamination recovery classification", () => {
     expect(result.alreadyUpstream.map((entry) => entry.sha)).toEqual([upstreamCommit.sha]);
     expect(result.unique.map((entry) => entry.sha)).toEqual([uniqueCommit.sha]);
   });
+
+  it("auto-recovers by dropping already-upstream foreign commits and preserving remaining branch work", async () => {
+    const { repoDir, baseSha } = await setupRepo();
+    await writeFile(path.join(repoDir, "foreign.txt"), "", "utf-8");
+    await writeFile(path.join(repoDir, "own.txt"), "", "utf-8");
+    const foreign = await makeCommit(repoDir, "foreign-e", "feat(FN-4412): upstream duplicate", "FN-4412", "foreign.txt");
+    await appendFile(path.join(repoDir, "own.txt"), "own-work\n", "utf-8");
+    await run("git add own.txt", repoDir);
+    await run("git commit -m 'feat(FN-4428): own work' -m 'Fusion-Task-Id: FN-4428'", repoDir);
+
+    await run("git checkout main", repoDir);
+    await run(`git cherry-pick ${foreign.sha}`, repoDir);
+    await run("git checkout feature", repoDir);
+
+    const originalTip = await run("git rev-parse HEAD", repoDir);
+    const result = await autoRecoverCrossContamination({
+      repoDir,
+      branchName: "feature",
+      baseSha,
+      taskId: "FN-4428",
+      alreadyUpstreamShas: [foreign.sha],
+    });
+
+    const history = await run(`git log --format=%s ${baseSha}..feature`, repoDir);
+    expect(history).toContain("feat(FN-4428): own work");
+    expect(history).not.toContain("feat(FN-4412): upstream duplicate");
+    expect(result.droppedShas).toEqual([foreign.sha]);
+    expect(result.newTipSha).not.toEqual(originalTip);
+  });
 });

packages/engine/src/branch-conflicts.ts

@@ -282,6 +282,38 @@ export interface ClassifyForeignCommitsResult {
   unique: BranchCrossContaminationCommit[];
 }
 
+async function classifyForeignCommitsViaPatchId(
+  repoDir: string,
+  mainRef: string,
+  commits: BranchCrossContaminationCommit[],
+): Promise<ClassifyForeignCommitsResult> {
+  const upstreamPatchIdsOutput = await runGit(
+    repoDir,
+    `git rev-list ${quoteShellArg(mainRef)} | while read c; do git show "$c" | git patch-id --stable; done`,
+  ).catch(() => "");
+
+  const upstreamPatchIds = new Set(
+    upstreamPatchIdsOutput
+      .split("\n")
+      .map((line) => line.trim().split(" ")[0])
+      .filter(Boolean),
+  );
+
+  const alreadyUpstream: BranchCrossContaminationCommit[] = [];
+  const unique: BranchCrossContaminationCommit[] = [];
+  for (const commit of commits) {
+    const patchIdLine = await runGit(repoDir, `git show ${quoteShellArg(commit.sha)} | git patch-id --stable`).catch(() => "");
+    const patchId = patchIdLine.trim().split(" ")[0];
+    if (patchId && upstreamPatchIds.has(patchId)) {
+      alreadyUpstream.push(commit);
+    } else {
+      unique.push(commit);
+    }
+  }
+
+  return { alreadyUpstream, unique };
+}
+
 export async function classifyForeignCommits(
   input: ClassifyForeignCommitsInput,
 ): Promise<ClassifyForeignCommitsResult> {
@@ -291,7 +323,7 @@ export async function classifyForeignCommits(
     return { alreadyUpstream: [], unique: [] };
   }
 
-  const classifyFromCherryOutput = (output: string): ClassifyForeignCommitsResult => {
+  const classifyFromCherryOutput = async (output: string): Promise<ClassifyForeignCommitsResult> => {
     const alreadyUpstreamSha = new Set<string>();
     const uniqueSha = new Set<string>();
     const resolveFullSha = (token: string): string | null => {
@@ -314,40 +346,29 @@ export async function classifyForeignCommits(
       }
     }
 
-    const alreadyUpstream = foreignCommits.filter((commit) => alreadyUpstreamSha.has(commit.sha) || !uniqueSha.has(commit.sha));
+    const unresolved = foreignCommits.filter((commit) => !alreadyUpstreamSha.has(commit.sha) && !uniqueSha.has(commit.sha));
-    const unique = foreignCommits.filter((commit) => uniqueSha.has(commit.sha));
+    const unresolvedClassified = unresolved.length > 0
-    return { alreadyUpstream, unique };
+      ? await classifyForeignCommitsViaPatchId(repoDir, mainRef, unresolved)
+      : { alreadyUpstream: [], unique: [] };
+
+    return {
+      alreadyUpstream: [
+        ...foreignCommits.filter((commit) => alreadyUpstreamSha.has(commit.sha)),
+        ...unresolvedClassified.alreadyUpstream,
+      ],
+      unique: [
+        ...foreignCommits.filter((commit) => uniqueSha.has(commit.sha)),
+        ...unresolvedClassified.unique,
+      ],
+    };
   };
 
   try {
     const comparisonBase = baseSha || await runGit(repoDir, `git merge-base ${quoteShellArg(mainRef)} ${quoteShellArg(branchName)}`);
     const output = await runGit(repoDir, `git cherry ${quoteShellArg(mainRef)} ${quoteShellArg(branchName)} ${quoteShellArg(comparisonBase)}`);
-    return classifyFromCherryOutput(output);
+    return await classifyFromCherryOutput(output);
   } catch {
-    const upstreamPatchIdsOutput = await runGit(
+    return classifyForeignCommitsViaPatchId(repoDir, mainRef, foreignCommits);
-      repoDir,
-      `git rev-list ${quoteShellArg(mainRef)} | while read c; do git show "$c" | git patch-id --stable; done`,
-    ).catch(() => "");
-    const upstreamPatchIds = new Set(
-      upstreamPatchIdsOutput
-        .split("\n")
-        .map((line) => line.trim().split(" ")[0])
-        .filter(Boolean),
-    );
-
-    const alreadyUpstream: BranchCrossContaminationCommit[] = [];
-    const unique: BranchCrossContaminationCommit[] = [];
-    for (const commit of foreignCommits) {
-      const patchIdLine = await runGit(repoDir, `git show ${quoteShellArg(commit.sha)} | git patch-id --stable`).catch(() => "");
-      const patchId = patchIdLine.trim().split(" ")[0];
-      if (patchId && upstreamPatchIds.has(patchId)) {
-        alreadyUpstream.push(commit);
-      } else {
-        unique.push(commit);
-      }
-    }
-
-    return { alreadyUpstream, unique };
   }
 }