fix(FN-branch-group): unify landed/completion predicate in core (U2)
Route and coordinator disagreed on landed/complete: the route required mergeConfirmed + matching mergeTargetBranch, the coordinator accepted bare column===done/in-review and never checked the branch. Extract canonical isBranchGroupMemberLanded/isBranchGroupComplete in @fusion/core (stricter route semantics win — load-bearing for merge-target safety) and consume from both sides. Tightens promotion gating to fire only when all members are merge-confirmed onto the group branch.
This commit is contained in:
87
packages/core/src/__tests__/branch-group-completion.test.ts
Normal file
87
packages/core/src/__tests__/branch-group-completion.test.ts
Normal file
@@ -0,0 +1,87 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { isBranchGroupComplete, isBranchGroupMemberLanded } from "../branch-group-completion.js";
|
||||
import type { BranchGroup, Task } from "../types.js";
|
||||
|
||||
const GROUP_BRANCH = "fusion/groups/planning-x";
|
||||
|
||||
const group = { branchName: GROUP_BRANCH } as Pick<BranchGroup, "branchName">;
|
||||
|
||||
function landedMember(): Pick<Task, "mergeDetails"> {
|
||||
return {
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: GROUP_BRANCH,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("isBranchGroupMemberLanded", () => {
|
||||
it("returns true when merge is confirmed onto the group branch via integration", () => {
|
||||
expect(isBranchGroupMemberLanded(landedMember(), group)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when mergeTargetBranch does not match the group branch", () => {
|
||||
expect(
|
||||
isBranchGroupMemberLanded(
|
||||
{
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: "fusion/fn-sibling",
|
||||
},
|
||||
},
|
||||
group,
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when the merge is not confirmed", () => {
|
||||
expect(
|
||||
isBranchGroupMemberLanded(
|
||||
{
|
||||
mergeDetails: {
|
||||
mergeConfirmed: false,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: GROUP_BRANCH,
|
||||
},
|
||||
},
|
||||
group,
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when the merge target source is not branch-group-integration", () => {
|
||||
expect(
|
||||
isBranchGroupMemberLanded(
|
||||
{
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergeTargetSource: "project-default",
|
||||
mergeTargetBranch: GROUP_BRANCH,
|
||||
},
|
||||
},
|
||||
group,
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when there are no merge details", () => {
|
||||
expect(isBranchGroupMemberLanded({}, group)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isBranchGroupComplete", () => {
|
||||
it("returns true when every member is landed", () => {
|
||||
expect(isBranchGroupComplete([landedMember(), landedMember()], group)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when one member is not landed", () => {
|
||||
expect(isBranchGroupComplete([landedMember(), {}], group)).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for an empty membership", () => {
|
||||
expect(isBranchGroupComplete([], group)).toBe(false);
|
||||
});
|
||||
});
|
||||
35
packages/core/src/branch-group-completion.ts
Normal file
35
packages/core/src/branch-group-completion.ts
Normal file
@@ -0,0 +1,35 @@
|
||||
import type { BranchGroup, Task } from "./types.js";
|
||||
|
||||
/**
|
||||
* Canonical "member landed" predicate, shared by the dashboard branch-groups
|
||||
* route and the engine group-merge coordinator so the two gates can never
|
||||
* diverge (the historical divergence: the route required `mergeConfirmed` +
|
||||
* matching `mergeTargetBranch`, while the coordinator accepted bare
|
||||
* `column === "done"` or `in-review` + integration source and never checked
|
||||
* the target branch).
|
||||
*
|
||||
* The stricter route semantics win: a member is landed iff it was actually
|
||||
* merge-confirmed onto THIS group's branch via the branch-group-integration
|
||||
* path. This is load-bearing for merge-target safety — a member marked done
|
||||
* against a sibling `fusion/fn-*` branch or a mismatched branch MUST NOT count
|
||||
* as landed (root cause of the 2026-05-23 lost-work incident).
|
||||
*/
|
||||
export function isBranchGroupMemberLanded(
|
||||
task: Pick<Task, "mergeDetails">,
|
||||
group: Pick<BranchGroup, "branchName">,
|
||||
): boolean {
|
||||
return task.mergeDetails?.mergeConfirmed === true
|
||||
&& task.mergeDetails?.mergeTargetSource === "branch-group-integration"
|
||||
&& task.mergeDetails?.mergeTargetBranch === group.branchName;
|
||||
}
|
||||
|
||||
/**
|
||||
* Canonical "group complete" predicate. A group is complete iff it has at
|
||||
* least one member and every member is landed by {@link isBranchGroupMemberLanded}.
|
||||
*/
|
||||
export function isBranchGroupComplete(
|
||||
members: Pick<Task, "mergeDetails">[],
|
||||
group: Pick<BranchGroup, "branchName">,
|
||||
): boolean {
|
||||
return members.length > 0 && members.every((member) => isBranchGroupMemberLanded(member, group));
|
||||
}
|
||||
@@ -325,6 +325,10 @@ export {
|
||||
type MergeTargetResolution,
|
||||
type MergeTargetResolverOptions,
|
||||
} from "./task-merge.js";
|
||||
export {
|
||||
isBranchGroupMemberLanded,
|
||||
isBranchGroupComplete,
|
||||
} from "./branch-group-completion.js";
|
||||
export {
|
||||
countRecentIdenticalStallEntries,
|
||||
getInReviewStallReason,
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import express from "express";
|
||||
import type { BranchGroup, Task, TaskStore } from "@fusion/core";
|
||||
import { evaluateBranchGroupCompletion } from "@fusion/engine";
|
||||
import { createApiRoutes } from "../routes.js";
|
||||
import { request as REQUEST } from "../test-request.js";
|
||||
|
||||
@@ -109,6 +110,31 @@ describe("branch group routes", () => {
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("route serialization and coordinator agree on landed/complete for the same fixture", async () => {
|
||||
// Same fixture exercised through BOTH paths must yield identical results.
|
||||
const completeTasks = [buildTask("FN-1", group.id, true), buildTask("FN-2", group.id, true)];
|
||||
const mixedTasks = [buildTask("FN-1", group.id, true), buildTask("FN-2", group.id, false)];
|
||||
|
||||
// Coordinator path.
|
||||
const completeCoord = evaluateBranchGroupCompletion({ members: completeTasks, group });
|
||||
const mixedCoord = evaluateBranchGroupCompletion({ members: mixedTasks, group });
|
||||
expect(completeCoord.complete).toBe(true);
|
||||
expect(mixedCoord.complete).toBe(false);
|
||||
|
||||
// Route serialization path.
|
||||
const completeApp = buildApp(createStore(group, completeTasks));
|
||||
const completeRes = await REQUEST(completeApp, "GET", "/api/branch-groups/BG-1");
|
||||
expect(completeRes.body.group.completion.complete).toBe(true);
|
||||
|
||||
const mixedApp = buildApp(createStore(group, mixedTasks));
|
||||
const mixedRes = await REQUEST(mixedApp, "GET", "/api/branch-groups/BG-1");
|
||||
expect(mixedRes.body.group.completion.complete).toBe(false);
|
||||
|
||||
// No divergence between the two gates.
|
||||
expect(completeRes.body.group.completion.complete).toBe(completeCoord.complete);
|
||||
expect(mixedRes.body.group.completion.complete).toBe(mixedCoord.complete);
|
||||
});
|
||||
|
||||
it("creates group on assign when groupId absent", async () => {
|
||||
const store = createStore(group, tasks);
|
||||
const app = buildApp(store);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Router, type Request } from "express";
|
||||
import type { BranchGroup, Task, TaskStore } from "@fusion/core";
|
||||
import type { BranchGroup, TaskStore } from "@fusion/core";
|
||||
import { isBranchGroupComplete, isBranchGroupMemberLanded } from "@fusion/core";
|
||||
import { badRequest, notFound } from "../api-error.js";
|
||||
|
||||
export interface BranchGroupsRouterOptions {
|
||||
@@ -11,19 +12,13 @@ function parseProjectId(req: Request): string | undefined {
|
||||
return typeof value === "string" && value.trim() ? value.trim() : undefined;
|
||||
}
|
||||
|
||||
function isMemberLanded(task: Task, group: BranchGroup): boolean {
|
||||
return task.mergeDetails?.mergeConfirmed === true
|
||||
&& task.mergeDetails?.mergeTargetSource === "branch-group-integration"
|
||||
&& task.mergeDetails?.mergeTargetBranch === group.branchName;
|
||||
}
|
||||
|
||||
async function serializeGroup(store: TaskStore, group: BranchGroup) {
|
||||
const members = await store.listTasksByBranchGroup(group.id);
|
||||
const memberRows = members.map((task) => ({
|
||||
taskId: task.id,
|
||||
title: task.title ?? task.description,
|
||||
column: task.column,
|
||||
landed: isMemberLanded(task, group),
|
||||
landed: isBranchGroupMemberLanded(task, group),
|
||||
}));
|
||||
const landedCount = memberRows.filter((member) => member.landed).length;
|
||||
return {
|
||||
@@ -32,7 +27,7 @@ async function serializeGroup(store: TaskStore, group: BranchGroup) {
|
||||
completion: {
|
||||
landed: landedCount,
|
||||
total: memberRows.length,
|
||||
complete: memberRows.length > 0 && landedCount === memberRows.length,
|
||||
complete: isBranchGroupComplete(members, group),
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -100,8 +95,7 @@ export function createBranchGroupsRouter(store: TaskStore, options?: BranchGroup
|
||||
if (!group) throw notFound("Branch group not found");
|
||||
|
||||
const members = await store.listTasksByBranchGroup(group.id);
|
||||
const landed = members.filter((member) => isMemberLanded(member, group)).length;
|
||||
if (members.length === 0 || landed !== members.length) {
|
||||
if (!isBranchGroupComplete(members, group)) {
|
||||
throw badRequest("Branch group completion gate not satisfied");
|
||||
}
|
||||
|
||||
|
||||
@@ -30,12 +30,22 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("evaluateBranchGroupCompletion", () => {
|
||||
it("returns complete when all members are landed", () => {
|
||||
const branchName = "fusion/groups/planning-x";
|
||||
const group = { branchName } as const;
|
||||
const landed = (id: string) => ({
|
||||
id,
|
||||
column: "done" as const,
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: branchName,
|
||||
} as any,
|
||||
});
|
||||
|
||||
it("returns complete when all members are landed onto the group branch", () => {
|
||||
const result = evaluateBranchGroupCompletion({
|
||||
members: [
|
||||
{ id: "FN-A", column: "done" as const },
|
||||
{ id: "FN-B", column: "in-review" as const, mergeDetails: { mergeTargetSource: "branch-group-integration" } as any },
|
||||
] as any,
|
||||
members: [landed("FN-A"), landed("FN-B")] as any,
|
||||
group,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
@@ -49,9 +59,10 @@ describe("evaluateBranchGroupCompletion", () => {
|
||||
it("returns pending ids when one member is not landed", () => {
|
||||
const result = evaluateBranchGroupCompletion({
|
||||
members: [
|
||||
{ id: "FN-A", column: "done" as const },
|
||||
{ id: "FN-B", column: "todo" as const },
|
||||
landed("FN-A"),
|
||||
{ id: "FN-B", column: "todo" as const } as any,
|
||||
] as any,
|
||||
group,
|
||||
});
|
||||
|
||||
expect(result.complete).toBe(false);
|
||||
@@ -60,7 +71,7 @@ describe("evaluateBranchGroupCompletion", () => {
|
||||
});
|
||||
|
||||
it("treats empty groups as incomplete", () => {
|
||||
const result = evaluateBranchGroupCompletion({ members: [] });
|
||||
const result = evaluateBranchGroupCompletion({ members: [], group });
|
||||
expect(result).toEqual({
|
||||
complete: false,
|
||||
totalMembers: 0,
|
||||
@@ -69,16 +80,46 @@ describe("evaluateBranchGroupCompletion", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("counts mixed done + landed in-review members as complete", () => {
|
||||
it("does NOT count a member confirmed onto a mismatched branch", () => {
|
||||
const result = evaluateBranchGroupCompletion({
|
||||
members: [
|
||||
{ id: "FN-A", column: "done" as const },
|
||||
{ id: "FN-B", column: "in-review" as const, mergeDetails: { mergeTargetSource: "branch-group-integration" } as any },
|
||||
landed("FN-A"),
|
||||
{
|
||||
id: "FN-B",
|
||||
column: "done" as const,
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: "fusion/fn-sibling",
|
||||
} as any,
|
||||
} as any,
|
||||
] as any,
|
||||
group,
|
||||
});
|
||||
|
||||
expect(result.complete).toBe(true);
|
||||
expect(result.pendingMemberIds).toEqual([]);
|
||||
expect(result.complete).toBe(false);
|
||||
expect(result.landedMemberIds).toEqual(["FN-A"]);
|
||||
expect(result.pendingMemberIds).toEqual(["FN-B"]);
|
||||
});
|
||||
|
||||
it("does NOT count a member whose merge is not confirmed", () => {
|
||||
const result = evaluateBranchGroupCompletion({
|
||||
members: [
|
||||
{
|
||||
id: "FN-A",
|
||||
column: "in-review" as const,
|
||||
mergeDetails: {
|
||||
mergeConfirmed: false,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: branchName,
|
||||
} as any,
|
||||
} as any,
|
||||
] as any,
|
||||
group,
|
||||
});
|
||||
|
||||
expect(result.complete).toBe(false);
|
||||
expect(result.pendingMemberIds).toEqual(["FN-A"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -191,6 +232,16 @@ describe("promoteBranchGroup", () => {
|
||||
};
|
||||
}
|
||||
|
||||
const landedMember = (id: string, branchName: string) => ({
|
||||
id,
|
||||
column: "done" as const,
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergeTargetSource: "branch-group-integration",
|
||||
mergeTargetBranch: branchName,
|
||||
},
|
||||
});
|
||||
|
||||
it("returns incomplete without merging when members are pending", async () => {
|
||||
const rootDir = makeRepo();
|
||||
const group = makeGroup();
|
||||
@@ -222,7 +273,7 @@ describe("promoteBranchGroup", () => {
|
||||
recordAudit: async (event) => { audits.push(event as Record<string, unknown>); },
|
||||
store: {
|
||||
getBranchGroup: () => group,
|
||||
listTasksByBranchGroup: async () => [{ id: "FN-A", column: "done" }],
|
||||
listTasksByBranchGroup: async () => [landedMember("FN-A", group.branchName)],
|
||||
updateBranchGroup: () => {
|
||||
throw new Error("should not update");
|
||||
},
|
||||
@@ -251,7 +302,7 @@ describe("promoteBranchGroup", () => {
|
||||
recordAudit: async (event) => { audits.push(event as Record<string, unknown>); },
|
||||
store: {
|
||||
getBranchGroup: () => group,
|
||||
listTasksByBranchGroup: async () => [{ id: "FN-A", column: "done" }],
|
||||
listTasksByBranchGroup: async () => [landedMember("FN-A", group.branchName)],
|
||||
updateBranchGroup: (_id: string, patch: Partial<typeof group>) => {
|
||||
group = { ...group, ...patch };
|
||||
return group;
|
||||
@@ -272,7 +323,7 @@ describe("promoteBranchGroup", () => {
|
||||
recordAudit: async (event) => { audits.push(event as Record<string, unknown>); },
|
||||
store: {
|
||||
getBranchGroup: () => group,
|
||||
listTasksByBranchGroup: async () => [{ id: "FN-A", column: "done" }],
|
||||
listTasksByBranchGroup: async () => [landedMember("FN-A", group.branchName)],
|
||||
updateBranchGroup: (_id: string, patch: Partial<typeof group>) => {
|
||||
group = { ...group, ...patch };
|
||||
return group;
|
||||
|
||||
@@ -215,7 +215,7 @@ describe("FN-5820 reliability interactions: shared branch group lifecycle", () =
|
||||
updateBranchGroup: (...args: any[]) => (store as any).updateBranchGroup(...args),
|
||||
listTasksByBranchGroup: async () => {
|
||||
const members = [await store.getTask(task.id), await store.getTask(second.id)].filter(Boolean) as any[];
|
||||
expect(evaluateBranchGroupCompletion({ members: members as any }).complete).toBe(true);
|
||||
expect(evaluateBranchGroupCompletion({ members: members as any, group }).complete).toBe(true);
|
||||
return members as any;
|
||||
},
|
||||
} as any,
|
||||
|
||||
@@ -2,7 +2,7 @@ import { exec } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
import type { BranchGroup, BranchGroupPrState, MergeTargetResolution, Settings, Task, TaskStore } from "@fusion/core";
|
||||
import { resolveEffectiveGroupAutoMerge, resolveTaskMergeTarget } from "@fusion/core";
|
||||
import { isBranchGroupMemberLanded, resolveEffectiveGroupAutoMerge, resolveTaskMergeTarget } from "@fusion/core";
|
||||
import { resolveIntegrationBranch } from "./integration-branch.js";
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
@@ -41,16 +41,25 @@ export interface BranchGroupPromotionDecision {
|
||||
| "eligible";
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluates branch-group completion using the canonical `@fusion/core`
|
||||
* `isBranchGroupMemberLanded` predicate so the engine gate can never diverge
|
||||
* from the dashboard route gate. A member is landed iff it was merge-confirmed
|
||||
* onto THIS group's branch via the branch-group-integration path; the group is
|
||||
* complete iff it has at least one member and every member is landed.
|
||||
*
|
||||
* `group` (its `branchName`) is required: landing is branch-anchored, so a
|
||||
* member done against a sibling/mismatched branch must NOT count as landed.
|
||||
*/
|
||||
export function evaluateBranchGroupCompletion(input: {
|
||||
members: Pick<Task, "id" | "column" | "branchContext" | "mergeDetails">[];
|
||||
group: Pick<BranchGroup, "branchName">;
|
||||
}): BranchGroupCompletionStatus {
|
||||
const landedMemberIds: string[] = [];
|
||||
const pendingMemberIds: string[] = [];
|
||||
|
||||
for (const member of input.members) {
|
||||
const landed = member.column === "done"
|
||||
|| (member.column === "in-review" && member.mergeDetails?.mergeTargetSource === "branch-group-integration");
|
||||
if (landed) {
|
||||
if (isBranchGroupMemberLanded(member, input.group)) {
|
||||
landedMemberIds.push(member.id);
|
||||
} else {
|
||||
pendingMemberIds.push(member.id);
|
||||
@@ -159,7 +168,7 @@ export async function promoteBranchGroup(input: {
|
||||
}
|
||||
|
||||
const members = await input.store.listTasksByBranchGroup(group.id);
|
||||
const completion = evaluateBranchGroupCompletion({ members });
|
||||
const completion = evaluateBranchGroupCompletion({ members, group });
|
||||
if (!completion.complete) {
|
||||
return {
|
||||
groupId: group.id,
|
||||
|
||||
Reference in New Issue
Block a user