fix(FN-6131): stop failing on foreign branch attribution
Treat foreign task-attributed commits in branch ranges as informational instead of blocking worktree acquisition or branch authority checks. Fusion-Task-Id: FN-6131
This commit is contained in:
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Stop failing task worktree acquisition and branch authority checks when a task branch contains foreign task-attributed commits.
|
||||||
@@ -51,7 +51,6 @@ import { execSync } from "node:child_process";
|
|||||||
import { existsSync } from "node:fs";
|
import { existsSync } from "node:fs";
|
||||||
import {
|
import {
|
||||||
BranchConflictError,
|
BranchConflictError,
|
||||||
BranchCrossContaminationError,
|
|
||||||
assertCleanBranchAtBase,
|
assertCleanBranchAtBase,
|
||||||
inspectBranchConflict,
|
inspectBranchConflict,
|
||||||
listUniqueBranchCommits,
|
listUniqueBranchCommits,
|
||||||
@@ -443,100 +442,19 @@ describe("branch-conflicts", () => {
|
|||||||
it.each([
|
it.each([
|
||||||
{
|
{
|
||||||
name: "passes when attribution comes from subject token",
|
name: "passes when attribution comes from subject token",
|
||||||
log: "aaa111\u001ffeat(FN-4068): own\u001f\n",
|
|
||||||
expectForeign: false,
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "passes when attribution comes from trailer token",
|
name: "passes when attribution comes from trailer token",
|
||||||
log: "aaa111\u001ffeat: own\u001fFusion-Task-Id: FN-4068\n",
|
|
||||||
expectForeign: false,
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "throws when commit is attributed to a foreign task",
|
name: "passes when commit is attributed to a foreign task",
|
||||||
log: "bbb222\u001ffeat(FN-4386): foreign\u001fFusion-Task-Id: FN-4386\n",
|
|
||||||
expectForeign: true,
|
|
||||||
},
|
},
|
||||||
])("assertCleanBranchAtBase $name", async ({ log, expectForeign }) => {
|
])("assertCleanBranchAtBase $name", async () => {
|
||||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
mockedExecSync.mockImplementation(() => {
|
||||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
throw new Error("assertCleanBranchAtBase should not inspect commit attribution");
|
||||||
if (command.includes("git log --format=%H%x1f%s%x1f%b 'main..fusion/fn-4068'")) {
|
|
||||||
return Buffer.from(log);
|
|
||||||
}
|
|
||||||
throw new Error(`Unexpected command: ${command}`);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
const assertion = assertCleanBranchAtBase("/tmp/repo", "fusion/fn-4068", "main", "FN-4068");
|
await expect(assertCleanBranchAtBase("/tmp/repo", "fusion/fn-4068", "main", "FN-4068")).resolves.toBeUndefined();
|
||||||
if (expectForeign) {
|
|
||||||
await expect(assertion).rejects.toBeInstanceOf(BranchCrossContaminationError);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
await expect(assertion).resolves.toBeUndefined();
|
|
||||||
});
|
|
||||||
|
|
||||||
// FN-5475 / option-2 promotion check: a commit attributed to another
|
|
||||||
// task that's already reachable from the integration target was integrated
|
|
||||||
// via fast-forward and shouldn't be treated as contamination on a
|
|
||||||
// downstream branch that briefly inherited it.
|
|
||||||
it("assertCleanBranchAtBase treats foreign-attributed commits that are ancestors of main as promoted", async () => {
|
|
||||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
|
||||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
|
||||||
if (command.includes("git log --format=%H%x1f%s%x1f%b 'main..fusion/fn-4068'")) {
|
|
||||||
return Buffer.from("bbb222feat(FN-4386): foreignFusion-Task-Id: FN-4386\n");
|
|
||||||
}
|
|
||||||
if (command.includes("git merge-base --is-ancestor 'bbb222' 'main'")) {
|
|
||||||
// Simulate the FN-5475 case: foreign commit is already on local main.
|
|
||||||
return Buffer.from("");
|
|
||||||
}
|
|
||||||
throw new Error(`Unexpected command: ${command}`);
|
|
||||||
});
|
|
||||||
|
|
||||||
await expect(
|
|
||||||
assertCleanBranchAtBase("/tmp/repo", "fusion/fn-4068", "main", "FN-4068"),
|
|
||||||
).resolves.toBeUndefined();
|
|
||||||
});
|
|
||||||
|
|
||||||
it("assertCleanBranchAtBase treats foreign-attributed commits that are only ancestors of origin/main as promoted", async () => {
|
|
||||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
|
||||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
|
||||||
if (command.includes("git log --format=%H%x1f%s%x1f%b 'main..fusion/fn-4068'")) {
|
|
||||||
return Buffer.from("bbb222feat(FN-4386): foreignFusion-Task-Id: FN-4386\n");
|
|
||||||
}
|
|
||||||
if (command.includes("git merge-base --is-ancestor 'bbb222' 'main'")) {
|
|
||||||
// Local main is stale and does not yet contain the promoted dependency.
|
|
||||||
const err = new Error("not an ancestor") as Error & { stderr?: string };
|
|
||||||
err.stderr = "";
|
|
||||||
throw err;
|
|
||||||
}
|
|
||||||
if (command.includes("git merge-base --is-ancestor 'bbb222' 'origin/main'")) {
|
|
||||||
// Remote-tracking integration branch already contains it.
|
|
||||||
return Buffer.from("");
|
|
||||||
}
|
|
||||||
throw new Error(`Unexpected command: ${command}`);
|
|
||||||
});
|
|
||||||
|
|
||||||
await expect(
|
|
||||||
assertCleanBranchAtBase("/tmp/repo", "fusion/fn-4068", "main", "FN-4068"),
|
|
||||||
).resolves.toBeUndefined();
|
|
||||||
});
|
|
||||||
|
|
||||||
it("assertCleanBranchAtBase still throws when foreign-attributed commits are NOT on any integration ref", async () => {
|
|
||||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
|
||||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
|
||||||
if (command.includes("git log --format=%H%x1f%s%x1f%b 'main..fusion/fn-4068'")) {
|
|
||||||
return Buffer.from("bbb222feat(FN-4386): foreignFusion-Task-Id: FN-4386\n");
|
|
||||||
}
|
|
||||||
if (command.includes("git merge-base --is-ancestor")) {
|
|
||||||
// Not on any integration ref — exits non-zero.
|
|
||||||
const err = new Error("not an ancestor") as Error & { stderr?: string };
|
|
||||||
err.stderr = "";
|
|
||||||
throw err;
|
|
||||||
}
|
|
||||||
throw new Error(`Unexpected command: ${command}`);
|
|
||||||
});
|
|
||||||
|
|
||||||
await expect(
|
|
||||||
assertCleanBranchAtBase("/tmp/repo", "fusion/fn-4068", "main", "FN-4068"),
|
|
||||||
).rejects.toBeInstanceOf(BranchCrossContaminationError);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("reportBranchAttribution", () => {
|
describe("reportBranchAttribution", () => {
|
||||||
|
|||||||
@@ -367,8 +367,7 @@ export async function branchTipCarriesTaskIdTrailer(
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Whole-branch authority check: the branch ref exists, its tip carries the
|
* Whole-branch authority check: the branch ref exists, its tip carries the
|
||||||
* task's Fusion-Task-Id trailer, and (when a base is supplied) the range
|
* task's Fusion-Task-Id trailer.
|
||||||
* `base..branch` has no foreign FN-attributed commits.
|
|
||||||
*
|
*
|
||||||
* Returns `{ ok: true }` when safe to treat the branch ref as authoritative
|
* Returns `{ ok: true }` when safe to treat the branch ref as authoritative
|
||||||
* for `taskId`. On failure, returns `{ ok: false, reason }` so callers can
|
* for `taskId`. On failure, returns `{ ok: false, reason }` so callers can
|
||||||
@@ -389,87 +388,22 @@ export async function isBranchAuthoritativeForTask(
|
|||||||
if (!tipCarriesTrailer) {
|
if (!tipCarriesTrailer) {
|
||||||
return { ok: false, reason: "tip-missing-task-trailer" };
|
return { ok: false, reason: "tip-missing-task-trailer" };
|
||||||
}
|
}
|
||||||
if (baseSha) {
|
|
||||||
try {
|
|
||||||
await assertCleanBranchAtBase(repoDir, branch, baseSha, taskId);
|
|
||||||
} catch (err) {
|
|
||||||
const reason = err instanceof BranchCrossContaminationError ? "foreign-contamination" : "clean-branch-check-failed";
|
|
||||||
return { ok: false, reason };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return { ok: true };
|
return { ok: true };
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Cheap ancestry check: is `commitSha` reachable from `ref`?
|
|
||||||
*
|
|
||||||
* Used to recognize "promoted" commits during contamination audits: when
|
|
||||||
* the engine fast-forwards local `main` with a sibling task's commit, that
|
|
||||||
* commit's `Fusion-Task-Id` trailer still points at the sibling, but the
|
|
||||||
* commit itself is now integrated. Treating it as foreign contamination
|
|
||||||
* for downstream tasks branched from the same main tip is incorrect — the
|
|
||||||
* commit is, by definition, ancestral on the integration target.
|
|
||||||
*
|
|
||||||
* Returns `false` on any git error (missing ref, repo unreadable, etc.)
|
|
||||||
* so the caller falls back to the conservative trailer-only judgement.
|
|
||||||
*/
|
|
||||||
async function isAncestorOf(repoDir: string, commitSha: string, ref: string): Promise<boolean> {
|
|
||||||
try {
|
|
||||||
await runGit(repoDir, `git merge-base --is-ancestor ${quoteShellArg(commitSha)} ${quoteShellArg(ref)}`);
|
|
||||||
return true;
|
|
||||||
} catch {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
export async function assertCleanBranchAtBase(
|
export async function assertCleanBranchAtBase(
|
||||||
repoDir: string,
|
repoDir: string,
|
||||||
branchName: string,
|
branchName: string,
|
||||||
baseSha: string,
|
baseSha: string,
|
||||||
taskId: string,
|
taskId: string,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
const output = await runGit(repoDir, `git log --format=%H%x1f%s%x1f%b ${quoteShellArg(`${baseSha}..${branchName}`)}`)
|
// Foreign task attribution in a branch range is informational only. Stacked
|
||||||
.catch(() => "");
|
// task branches and cherry-equivalent commits are handled by merge/display
|
||||||
if (!output) return;
|
// attribution, not by failing worktree acquisition or branch authority.
|
||||||
|
void repoDir;
|
||||||
const subjectPattern = /^(feat|fix|test|chore|docs|refactor|perf|build)\((FN-\d+)\):/i;
|
void branchName;
|
||||||
const trailerPattern = /(?:^|\n)Fusion-Task-Id:\s*(FN-\d+)\s*(?:\n|$)/i;
|
void baseSha;
|
||||||
const candidateForeign: BranchCrossContaminationCommit[] = [];
|
void taskId;
|
||||||
for (const line of output.split("\n").map((entry) => entry.trim()).filter(Boolean)) {
|
|
||||||
const [sha, subject, body] = line.split("\u001f");
|
|
||||||
const subjectMatch = (subject ?? "").match(subjectPattern);
|
|
||||||
const trailerMatch = (body ?? "").match(trailerPattern);
|
|
||||||
const attributedTaskId = (trailerMatch?.[1] ?? subjectMatch?.[2] ?? "").toUpperCase();
|
|
||||||
if (attributedTaskId && attributedTaskId !== taskId.toUpperCase()) {
|
|
||||||
candidateForeign.push({ sha, subject: subject ?? "", foreignTaskId: attributedTaskId });
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (candidateForeign.length === 0) return;
|
|
||||||
|
|
||||||
// FN-5475/FN-219: a commit attributed to another task that's already
|
|
||||||
// reachable from the integration target was promoted through integration.
|
|
||||||
// Treat it as ancestral, not contamination. Check both local `main` and
|
|
||||||
// `origin/main`: long-running dashboards can have stale local main while
|
|
||||||
// the remote-tracking branch already contains the promoted dependency, and
|
|
||||||
// using only local main produces false branch-cross-contamination loops.
|
|
||||||
const integratedRefs = ["main", "origin/main"];
|
|
||||||
const foreignCommits: BranchCrossContaminationCommit[] = [];
|
|
||||||
for (const commit of candidateForeign) {
|
|
||||||
let alreadyIntegrated = false;
|
|
||||||
for (const ref of integratedRefs) {
|
|
||||||
if (await isAncestorOf(repoDir, commit.sha, ref)) {
|
|
||||||
alreadyIntegrated = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (alreadyIntegrated) continue;
|
|
||||||
foreignCommits.push(commit);
|
|
||||||
}
|
|
||||||
|
|
||||||
if (foreignCommits.length > 0) {
|
|
||||||
throw new BranchCrossContaminationError({ branchName, baseSha, taskId, foreignCommits });
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ClassifyBootstrapMisbindingInput {
|
export interface ClassifyBootstrapMisbindingInput {
|
||||||
|
|||||||
Reference in New Issue
Block a user