fix(FN-5846): commit-ownership-anchor already-merged attribution (U3)
Audit of all shared-member merge + self-healing finalize paths: routing, merger finalize-success, and the 6 self-healing recovery paths were already group-branch-safe (FN-5846). Found a residual of the 2026-05-23 lost-work incident bug #2: already-merged-detector's ancestry strategy used bare git log --grep first-hit, and the ownership regex made the conventional scope optional (bare 'feat:' matched). Anchor attribution on trailers or task-scoped subject; scan candidates instead of accepting the first grep hit. Adds real-git characterization tests.
This commit is contained in:
@@ -2,4 +2,4 @@
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix shared-branch-group member finalization so routed members land on the group's shared branch instead of being auto-finalized against the project default branch.
|
||||
Fix shared-branch-group member finalization so routed members land on the group's shared branch instead of being auto-finalized against the project default branch. Also harden already-landed commit attribution so the recovery detector never claims a commit that merely mentions a task ID in prose (2026-05-23 lost-work regression): the `git log --grep` ancestry fallback is now ownership-anchored on a Fusion trailer or a task-scoped conventional-commit subject.
|
||||
|
||||
@@ -0,0 +1,143 @@
|
||||
// Real-git characterization of findAlreadyMergedTaskCommit's ownership
|
||||
// anchoring. These tests pin the 2026-05-23 lost-work incident's bug #2:
|
||||
// the detector must NOT attribute a task to a commit that merely *mentions*
|
||||
// the task ID in prose (the historical `git log --grep` first-hit bug).
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { execSync, spawnSync } from "node:child_process";
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import { findAlreadyMergedTaskCommit } from "../already-merged-detector.js";
|
||||
|
||||
const hasGit = spawnSync("git", ["--version"], { stdio: "pipe" }).status === 0;
|
||||
const describeIfGit = hasGit ? describe : describe.skip;
|
||||
|
||||
function git(repo: string, command: string): string {
|
||||
return execSync(command, { cwd: repo, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim();
|
||||
}
|
||||
|
||||
describeIfGit("findAlreadyMergedTaskCommit ownership anchoring (real git)", () => {
|
||||
const repos: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const repo of repos.splice(0)) {
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
function setupRepo(): string {
|
||||
const repo = mkdtempSync(path.join(os.tmpdir(), "fn-amd-"));
|
||||
repos.push(repo);
|
||||
git(repo, "git init -b main");
|
||||
git(repo, 'git config user.email "test@example.com"');
|
||||
git(repo, 'git config user.name "Test"');
|
||||
git(repo, "git commit --allow-empty -m 'init'");
|
||||
return repo;
|
||||
}
|
||||
|
||||
it("attributes via trailer when the owned commit carries Fusion-Task-Id", async () => {
|
||||
const repo = setupRepo();
|
||||
mkdirSync(path.join(repo, "src"), { recursive: true });
|
||||
writeFileSync(path.join(repo, "src", "owned.txt"), "owned\n", "utf-8");
|
||||
git(repo, "git add src/owned.txt && git commit -m 'feat: landed work' -m 'Fusion-Task-Id: FN-AMD-1'");
|
||||
const landedSha = git(repo, "git rev-parse HEAD");
|
||||
|
||||
const result = await findAlreadyMergedTaskCommit({
|
||||
taskId: "FN-AMD-1",
|
||||
repoDir: repo,
|
||||
baseBranch: "main",
|
||||
});
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.sha).toBe(landedSha);
|
||||
expect(result!.strategy).toBe("trailer");
|
||||
});
|
||||
|
||||
it("attributes via lineage trailer when present", async () => {
|
||||
const repo = setupRepo();
|
||||
mkdirSync(path.join(repo, "src"), { recursive: true });
|
||||
writeFileSync(path.join(repo, "src", "lineage.txt"), "lineage\n", "utf-8");
|
||||
git(repo, "git add src/lineage.txt && git commit -m 'feat: lineage work' -m 'Fusion-Task-Lineage: LINEAGE-XYZ'");
|
||||
const landedSha = git(repo, "git rev-parse HEAD");
|
||||
|
||||
const result = await findAlreadyMergedTaskCommit({
|
||||
taskId: "FN-AMD-LIN",
|
||||
lineageId: "LINEAGE-XYZ",
|
||||
repoDir: repo,
|
||||
baseBranch: "main",
|
||||
});
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.sha).toBe(landedSha);
|
||||
expect(result!.strategy).toBe("trailer");
|
||||
});
|
||||
|
||||
// Incident bug #2 regression: a commit that merely *mentions* the task ID in
|
||||
// its prose body (no anchored trailer) must NOT be attributed to the task,
|
||||
// even when the task's own branch tip is already an ancestor of base. The
|
||||
// ancestry `git log --grep=<taskId>` strategy historically accepted the first
|
||||
// such prose-mention hit and stranded/mis-attributed work.
|
||||
it("does NOT attribute to a commit that only mentions the task ID in prose (ancestry path)", async () => {
|
||||
const repo = setupRepo();
|
||||
|
||||
// An unrelated commit whose BODY mentions FN-AMD-2 in prose only.
|
||||
mkdirSync(path.join(repo, "src"), { recursive: true });
|
||||
writeFileSync(path.join(repo, "src", "unrelated.txt"), "unrelated\n", "utf-8");
|
||||
git(
|
||||
repo,
|
||||
"git add src/unrelated.txt && git commit -m 'feat: unrelated change' -m 'This also touches things related to FN-AMD-2 in passing.'",
|
||||
);
|
||||
|
||||
// The task's own branch landed by being merged into main, but its commits
|
||||
// carry NO trailer and NO conventional-subject anchor — only a generic
|
||||
// message — so the only `--grep=FN-AMD-2` hit is the prose-mention above.
|
||||
git(repo, "git checkout -b fusion/fn-amd-2");
|
||||
writeFileSync(path.join(repo, "src", "task.txt"), "task work\n", "utf-8");
|
||||
git(repo, "git add src/task.txt && git commit -m 'wip: generic message with no anchor'");
|
||||
git(repo, "git checkout main");
|
||||
git(repo, "git merge --no-ff --no-edit fusion/fn-amd-2 -m 'merge generic branch'");
|
||||
|
||||
const result = await findAlreadyMergedTaskCommit({
|
||||
taskId: "FN-AMD-2",
|
||||
repoDir: repo,
|
||||
baseBranch: "main",
|
||||
taskBranch: "fusion/fn-amd-2",
|
||||
});
|
||||
|
||||
// It may legitimately attribute via patch-id/tree-equal to the REAL owned
|
||||
// content, but it must NEVER return the unrelated prose-mention commit.
|
||||
if (result && result.strategy === "ancestry") {
|
||||
const subject = git(repo, `git show -s --format=%s ${result.sha}`);
|
||||
const body = git(repo, `git show -s --format=%b ${result.sha}`);
|
||||
const ownedBySubject = /^(?:[A-Za-z]+\([^)]*FN-AMD-2[^)]*\):|FN-AMD-2:)/.test(subject);
|
||||
const ownedByTrailer = /(?:^|\n)Fusion-Task-Id: FN-AMD-2\s*(?:\n|$)/.test(body);
|
||||
expect(ownedBySubject || ownedByTrailer).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("attributes via ancestry when the landed commit carries a conventional-subject anchor", async () => {
|
||||
const repo = setupRepo();
|
||||
|
||||
// The merge into main carries a conventional subject anchored on the task
|
||||
// ID; ancestry attribution should accept it (it is genuinely owned).
|
||||
git(repo, "git checkout -b fusion/fn-amd-3");
|
||||
mkdirSync(path.join(repo, "src"), { recursive: true });
|
||||
writeFileSync(path.join(repo, "src", "anchored.txt"), "anchored\n", "utf-8");
|
||||
git(repo, "git add src/anchored.txt && git commit -m 'feat(FN-AMD-3): real anchored work'");
|
||||
git(repo, "git checkout main");
|
||||
git(repo, "git merge --ff-only fusion/fn-amd-3");
|
||||
|
||||
const result = await findAlreadyMergedTaskCommit({
|
||||
taskId: "FN-AMD-3",
|
||||
repoDir: repo,
|
||||
baseBranch: "main",
|
||||
taskBranch: "fusion/fn-amd-3",
|
||||
});
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
// Trailer path won't match (no trailer); ownership-anchored ancestry should.
|
||||
const subject = git(repo, `git show -s --format=%s ${result!.sha}`);
|
||||
expect(subject).toContain("FN-AMD-3");
|
||||
});
|
||||
});
|
||||
@@ -74,6 +74,61 @@ describe("FN-5782 reliability interactions: branch group merge routing", () => {
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("routes a shared member to the group branch even when it inherited a sibling fusion/fn-* baseBranch (lost-work regression)", async () => {
|
||||
const fixture = await makeReliabilityFixture({ taskId: "FN-5782-RI-SIBLING", settings: { testMode: true } as any });
|
||||
|
||||
try {
|
||||
const { rootDir, store, task } = fixture;
|
||||
await stageMergeBranch(store, rootDir, task.id, "fn5782SiblingInherit");
|
||||
|
||||
const group = store.createBranchGroup({
|
||||
sourceType: "planning",
|
||||
sourceId: "PS-FN5782-SIBLING",
|
||||
branchName: "fusion/groups/fn-5782-sibling",
|
||||
});
|
||||
await store.setTaskBranchGroup(task.id, group.id);
|
||||
|
||||
// 2026-05-23 lost-work shape: a shared member inherited a sibling
|
||||
// `fusion/fn-*` branch as its base/inherited base (propagated from a
|
||||
// sibling-dispatched parent). The resolver MUST still land it on the
|
||||
// group branch, never on the sibling, and never on main.
|
||||
await store.updateTask(task.id, {
|
||||
baseBranch: "fusion/fn-9999-sibling-parent",
|
||||
branchContext: {
|
||||
groupId: group.id,
|
||||
source: "planning",
|
||||
assignmentMode: "shared",
|
||||
inheritedBaseBranch: "fusion/fn-9999-sibling-parent",
|
||||
},
|
||||
} as any);
|
||||
|
||||
const auditSpy = vi.spyOn(store as any, "recordRunAuditEvent");
|
||||
const result = await aiMergeTask(store, rootDir, task.id);
|
||||
expect(result.merged).toBe(true);
|
||||
|
||||
// Landed on the group branch; NOT on the sibling, NOT on main.
|
||||
expect(git(rootDir, `git show ${group.branchName}:packages/engine/src/fn5782SiblingInherit.ts`)).toContain("fn5782SiblingInherit");
|
||||
expect(() => git(rootDir, "git show main:packages/engine/src/fn5782SiblingInherit.ts")).toThrow();
|
||||
expect(() => git(rootDir, "git show fusion/fn-9999-sibling-parent:packages/engine/src/fn5782SiblingInherit.ts")).toThrow();
|
||||
|
||||
const recovered = await store.getTask(task.id);
|
||||
expect(recovered?.mergeDetails?.mergeTargetSource).toBe("branch-group-integration");
|
||||
expect(recovered?.mergeDetails?.mergeTargetBranch).toBe(group.branchName);
|
||||
|
||||
expect(auditSpy).toHaveBeenCalledWith(expect.objectContaining({
|
||||
domain: "git",
|
||||
mutationType: "merge:branch-group-routed",
|
||||
target: task.id,
|
||||
metadata: expect.objectContaining({
|
||||
mergeTargetBranch: group.branchName,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
}),
|
||||
}));
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
}, 45_000);
|
||||
|
||||
it.skipIf(!hasGit)("records shared-member landing even when autoMerge is false", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5819-RI-AUTO-OFF",
|
||||
|
||||
@@ -2772,6 +2772,12 @@ describe("SelfHealingManager", () => {
|
||||
if (cmd.includes("Fusion-Task-Id: FN-2900")) {
|
||||
return "trailerSha123feat: ship something opaque\n" as any;
|
||||
}
|
||||
// Ownership-verification body fetch (FN-5441/5446): the real commit
|
||||
// located via trailer grep carries the anchored trailer in its body,
|
||||
// so commitOwnedByTask accepts it though the subject lacks the task ID.
|
||||
if (cmd.includes("--format=%b") && cmd.includes("trailerSha123")) {
|
||||
return "Fusion-Task-Id: FN-2900\n" as any;
|
||||
}
|
||||
if (cmd.includes("--fixed-strings")) return "" as any;
|
||||
}
|
||||
if (cmd.includes("git show --shortstat")) {
|
||||
@@ -2829,6 +2835,11 @@ describe("SelfHealingManager", () => {
|
||||
if (cmd.includes("git log") && cmd.includes("Fusion-Task-Id: FN-2901")) {
|
||||
return "rangeSha901\u001ffeat: ship something opaque\n" as any;
|
||||
}
|
||||
// Ownership-verification body fetch (FN-5441/5446): real trailer-grep
|
||||
// hit carries the anchored trailer in its body.
|
||||
if (cmd.includes("git log") && cmd.includes("--format=%b") && cmd.includes("rangeSha901")) {
|
||||
return "Fusion-Task-Id: FN-2901\n" as any;
|
||||
}
|
||||
if (cmd.includes("git diff --shortstat") && cmd.includes("rebasebase901..rangeSha901")) {
|
||||
return " 4 files changed, 104 insertions(+), 1 deletion(-)\n" as any;
|
||||
}
|
||||
|
||||
@@ -34,6 +34,49 @@ function shellQuote(value: string): string {
|
||||
return `'${value.replace(/'/g, "'\\''")}'`;
|
||||
}
|
||||
|
||||
function escapeRegex(value: string): string {
|
||||
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||
}
|
||||
|
||||
/**
|
||||
* Ownership anchor shared with self-healing's `commitOwnedByTask`.
|
||||
*
|
||||
* The 2026-05-23 lost-work incident (bug #2) was a `git log --grep=<taskId>`
|
||||
* first-hit attribution: a commit whose body merely *mentioned* a task ID in
|
||||
* prose was accepted as that task's landed commit, stranding/mis-attributing
|
||||
* the real work. The trailer strategies above are already anchored; the
|
||||
* ancestry strategy below uses a loose `--grep=<taskId>`, so its candidate must
|
||||
* be ownership-verified here before it is accepted.
|
||||
*
|
||||
* Accept when ANY of:
|
||||
* - `Fusion-Task-Lineage: <lineageId>` is a complete trailer line in the body
|
||||
* - `Fusion-Task-Id: <taskId>` is a complete trailer line in the body
|
||||
* - the subject is anchored on the task ID in conventional-commit form:
|
||||
* `<type>(<taskId>...): …` or `<taskId>: …`
|
||||
*/
|
||||
function commitOwnedByTask(
|
||||
taskId: string,
|
||||
lineageId: string | undefined,
|
||||
subject: string,
|
||||
body: string,
|
||||
): boolean {
|
||||
if (lineageId && new RegExp(`(?:^|\\n)Fusion-Task-Lineage: ${escapeRegex(lineageId)}\\s*(?:\\n|$)`).test(body)) {
|
||||
return true;
|
||||
}
|
||||
if (new RegExp(`(?:^|\\n)Fusion-Task-Id: ${escapeRegex(taskId)}\\s*(?:\\n|$)`).test(body)) {
|
||||
return true;
|
||||
}
|
||||
// Subject anchor MUST mention the task ID — either inside a conventional
|
||||
// scope (`<type>(<…taskId…>): …`) or as a leading `<taskId>: …`. The scope
|
||||
// group is intentionally NOT optional here: a bare `feat: …` with no task ID
|
||||
// is NOT ownership evidence (a prose commit such as `feat: unrelated change`
|
||||
// whose body merely mentions the task must be rejected — incident bug #2).
|
||||
const subjectAnchor = new RegExp(
|
||||
`^(?:[A-Za-z]+\\([^)]*\\b${escapeRegex(taskId)}\\b[^)]*\\):|${escapeRegex(taskId)}:)`,
|
||||
);
|
||||
return subjectAnchor.test(subject);
|
||||
}
|
||||
|
||||
export async function findAlreadyMergedTaskCommit(
|
||||
input: AlreadyMergedLookupInput,
|
||||
): Promise<AlreadyMergedLookupResult | null> {
|
||||
@@ -97,22 +140,33 @@ export async function findAlreadyMergedTaskCommit(
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
|
||||
// FN-5441/5446 (2026-05-23 lost-work bug #2): `--grep=<taskId>` is a loose
|
||||
// match that also hits commits merely mentioning the task ID in prose.
|
||||
// Gather candidates (bounded) and accept only the first whose subject/body
|
||||
// is OWNERSHIP-anchored on the task — never the first raw grep hit.
|
||||
const ancestryCommand = [
|
||||
"git log",
|
||||
"--first-parent",
|
||||
"--format=%H",
|
||||
"--format=%H%x1f%s%x1f%b%x1e",
|
||||
`--grep=${shellQuote(taskId)}`,
|
||||
"--max-count=1",
|
||||
"--max-count=20",
|
||||
shellQuote(baseBranch),
|
||||
].join(" ");
|
||||
const { stdout } = await execAsync(ancestryCommand, {
|
||||
cwd: repoDir,
|
||||
timeout: 30_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
maxBuffer: 4 * 1024 * 1024,
|
||||
});
|
||||
const sha = stdout.trim();
|
||||
if (sha) {
|
||||
return { sha, strategy: "ancestry" };
|
||||
const records = stdout
|
||||
.split("\x1e")
|
||||
.map((record) => record.trim())
|
||||
.filter((record) => record.length > 0);
|
||||
for (const record of records) {
|
||||
const [candidateSha, candidateSubject = "", candidateBody = ""] = record.split("\x1f");
|
||||
const sha = candidateSha?.trim();
|
||||
if (sha && commitOwnedByTask(taskId, lineageId, candidateSubject, candidateBody)) {
|
||||
return { sha, strategy: "ancestry" };
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Fall through to patch-id checks.
|
||||
|
||||
@@ -493,9 +493,13 @@ function commitOwnedByTask(taskId: string, lineageId: string | undefined, subjec
|
||||
if (new RegExp(`(?:^|\\n)Fusion-Task-Id: ${escapeRegex(taskId)}\\s*(?:\\n|$)`).test(body)) {
|
||||
return true;
|
||||
}
|
||||
// Subject anchor: `<scope>(<taskId>...): …` or `<taskId>: …` at start.
|
||||
// Subject anchor: `<type>(<…taskId…>): …` or `<taskId>: …` at start.
|
||||
// The conventional scope group is intentionally NOT optional: a bare
|
||||
// `<type>: …` (e.g. `feat: unrelated change`) carries no task ID and is NOT
|
||||
// ownership evidence, even if the body mentions the task in prose (incident
|
||||
// bug #2 — a prose-mention must never claim a task).
|
||||
const subjectAnchor = new RegExp(
|
||||
`^(?:[A-Za-z]+(?:\\([^)]*\\b${escapeRegex(taskId)}\\b[^)]*\\))?:|${escapeRegex(taskId)}:)`,
|
||||
`^(?:[A-Za-z]+\\([^)]*\\b${escapeRegex(taskId)}\\b[^)]*\\):|${escapeRegex(taskId)}:)`,
|
||||
);
|
||||
return subjectAnchor.test(subject);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user