diff --git a/.changeset/fn-7532-branch-group-completion.md b/.changeset/fn-7532-branch-group-completion.md new file mode 100644 index 0000000000..8b18b6cb39 --- /dev/null +++ b/.changeset/fn-7532-branch-group-completion.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix branch group completion checklists to show accurate landed/finished counts. +category: fix +dev: runAiMerge (the sole merge path since master-plan U0) never resolved branch-group routing or stamped mergeDetails.mergeTargetBranch/mergeTargetSource, so isBranchGroupMemberLanded permanently reported shared-group members as not landed. Routes through resolveBranchGroupMergeRouting (matching the legacy merger.ts pattern) and stamps the target fields on both the landed and no-op finalize paths; preserves merge-target-safety in isBranchGroupMemberLanded (a sibling/mismatched-branch member still never counts as landed). diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 306c98afde..4454bf46c4 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -1821,6 +1821,8 @@ UI surfaces: The Group Task Modal shows shared branch name/status, member list (`taskId`, title, column, landed state), quick links to open each member task detail, completion progress (`X of Y members finished`), and tracked PR state when present. Branch groups are durable SQLite state keyed by real `BG-*` ids, so valid grouped tasks continue to list/show after a server restart. It live-refreshes from the same dashboard task-update stream and ignores stale cross-project events. +> **FN-7532:** a member only counts as "landed" once it merge-confirms onto its OWN group's branch via the branch-group-integration path (`mergeDetails.mergeTargetSource === "branch-group-integration"` and a matching `mergeTargetBranch`) — this is the same predicate the engine's promotion gate uses, so the checklist can never show "complete" when a real promotion would still be refused (or vice versa). The merge engine now stamps this attribution for every merge (previously only the legacy merge path did, so shared-group members merged through the current path were undercounted). + Both the modal and branch-group card are completion-gated: while members are still pending, they show progress only. PR / merge controls are only revealed after all members are landed into the shared branch. When auto-merge is off, promote/open-PR is explicit user action (no automatic push-to-origin behavior). ### CLI-onboarding backfill runbook diff --git a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts index 07131e10cb..f6d148c97b 100644 --- a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts +++ b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts @@ -1462,6 +1462,35 @@ describe("syncGroupPrCallback (U6)", () => { const sync = syncGroupPrCallback(github as never); await expect(sync({ cwd: "/tmp/project", group: { ...group, prNumber: undefined } as never, members })).rejects.toThrow(/no persisted prNumber/); }); + + /* + FNXC:BranchGroupCompletion 2026-07-04-00:00: + FN-7532 surface-parity regression: the PR-body checklist must use the SAME + isBranchGroupMemberLanded predicate as the dashboard route and CLI serializer — + a landed member ticks [x] and counts toward "Completion: x/N landed", while a + member merge-confirmed against a sibling/mismatched branch (merge-target-safety) + must NOT tick or count, even though it is otherwise "merge confirmed". + */ + it("ticks landed members and counts only genuinely-landed ones toward the completion line", async () => { + const landedMembers = [ + { id: "FN-A", title: "Alpha", mergeDetails: { mergeConfirmed: true, mergeTargetSource: "branch-group-integration", mergeTargetBranch: group.branchName } }, + { id: "FN-B", title: "Beta", mergeDetails: { mergeConfirmed: true, mergeTargetSource: "branch-group-integration", mergeTargetBranch: "fusion/fn-sibling" } }, + { id: "FN-C", title: "Gamma" }, + ] as never[]; + const github = { + getPrStatus: vi.fn(async () => ({ number: 42, url: "https://github.com/owner/repo/pull/42", status: "open", title: "T", headBranch: "h", baseBranch: "main", commentCount: 0 })), + updatePr: vi.fn(async () => ({ number: 42, url: "https://github.com/owner/repo/pull/42", status: "open", title: "T2", headBranch: "h", baseBranch: "main", commentCount: 0 })), + }; + const sync = syncGroupPrCallback(github as never); + await sync({ cwd: "/tmp/project", group: group as never, members: landedMembers }); + const body = (github.updatePr.mock.calls[0][0] as { body: string }).body; + // Only FN-A landed (matching branch); FN-B (sibling-branch mismatch) and + // FN-C (no merge details) must NOT count, even though FN-B is mergeConfirmed. + expect(body).toContain("Completion: 1/3 landed"); + expect(body).toContain("- [x] FN-A: Alpha"); + expect(body).toContain("- [ ] FN-B: Beta"); + expect(body).toContain("- [ ] FN-C: Gamma"); + }); }); describe("createGroupPrCallback", () => { diff --git a/packages/dashboard/src/__tests__/routes-branch-groups.test.ts b/packages/dashboard/src/__tests__/routes-branch-groups.test.ts index cd9085839b..e6b888d577 100644 --- a/packages/dashboard/src/__tests__/routes-branch-groups.test.ts +++ b/packages/dashboard/src/__tests__/routes-branch-groups.test.ts @@ -575,4 +575,49 @@ describe("branch group list N+1 elimination (Fix #6)", () => { expect(byId["BG-B"].completion).toEqual({ landed: 1, total: 1, complete: true }); expect(byId["BG-C"].completion).toEqual({ landed: 0, total: 0, complete: false }); }); + + /* + FNXC:BranchGroupCompletion 2026-07-04-00:00: + FN-7532 archived-member semantics: the list route fetches membership via + `listTasks({ includeArchived: false, slim: true })`, so an archived member + drops out of BOTH `landed` and `total` uniformly — archiving a LANDED member + preserves the ratio (both counts drop by one, `complete` is unaffected when it + was already complete), matching every other completion-reading surface (CLI, + PR-body checklist, engine promotion gate) which all resolve membership through + the same includeArchived:false path. This is intentional, consistent behavior + (no cross-surface divergence), asserted here so a future change to the fetch + option on only one surface would be caught as a regression. + */ + it("drops an archived member from both landed and total uniformly (no ratio distortion when the archived member was landed)", async () => { + const groups = buildGroups(); + const archivedLandedMember = memberTask("FN-A3", "BG-A", "feature/a", true); + // listTasks with includeArchived:false never returns this row — simulate + // the store contract precisely rather than assuming. + const nonArchivedTasks: Task[] = [ + memberTask("FN-A1", "BG-A", "feature/a", true), + memberTask("FN-A2", "BG-A", "feature/a", true), + ]; + const allTasksIncludingArchived = [...nonArchivedTasks, { ...archivedLandedMember, column: "archived" as const }]; + const listTasks = vi.fn(async (opts?: { includeArchived?: boolean }) => + opts?.includeArchived ? allTasksIncludingArchived : nonArchivedTasks, + ); + const store = { + getRootDir: vi.fn(() => "/tmp/project"), + listBranchGroups: vi.fn(() => [groups[0]]), + getBranchGroup: vi.fn((id: string) => groups.find((g) => g.id === id) ?? null), + listTasks, + listTasksByBranchGroup: vi.fn(), + } as unknown as TaskStore; + + const app = express(); + app.use(express.json()); + app.use("/branch-groups", createBranchGroupsRouter(store)); + attachErrorHandler(app); + + const res = await REQUEST(app, "GET", "/branch-groups"); + expect(res.status).toBe(200); + // Both members landed, both counted — the archived 3rd (also landed) member + // is invisible to the route entirely: 2/2 complete, not 2/3 incomplete. + expect(res.body.groups[0].completion).toEqual({ landed: 2, total: 2, complete: true }); + }); }); diff --git a/packages/engine/src/__tests__/merger-ai.test.ts b/packages/engine/src/__tests__/merger-ai.test.ts index 986992a68e..1b509a4bd8 100644 --- a/packages/engine/src/__tests__/merger-ai.test.ts +++ b/packages/engine/src/__tests__/merger-ai.test.ts @@ -72,7 +72,12 @@ function initRepoWithBranch(opts: { branch: string; conflict?: boolean } = { bra return { dir }; } -function makeStore(_dir: string, taskOverrides: Record = {}, settingsOverrides: Record = {}) { +function makeStore( + _dir: string, + taskOverrides: Record = {}, + settingsOverrides: Record = {}, + branchGroup?: any, +) { const task: any = { id: "FN-1", column: "in-review", @@ -86,6 +91,7 @@ function makeStore(_dir: string, taskOverrides: Record = {}, se }; const emitted: Array<{ event: string; payload: unknown }> = []; const logs: string[] = []; + const group = branchGroup ? { ...branchGroup } : undefined; const store: any = { getTask: vi.fn(async () => task), getSettings: vi.fn(async () => ({ merger: { mode: "ai", maxReviewPasses: 1 }, ...settingsOverrides })), @@ -94,8 +100,19 @@ function makeStore(_dir: string, taskOverrides: Record = {}, se emit: vi.fn((event: string, payload: unknown) => { emitted.push({ event, payload }); }), logEntry: vi.fn(async (_id: string, m: string) => { logs.push(m); }), appendAgentLog: vi.fn(async (_id: string, m: string) => { logs.push(m); }), + getBranchGroup: vi.fn((id: string) => (group && id === group.id ? group : null)), + recordBranchGroupMemberLanded: vi.fn(async (id: string, patch: Record) => { + if (group && id === group.id) Object.assign(group, patch); + return group; + }), + updateBranchGroup: vi.fn((id: string, patch: Record) => { + if (group && id === group.id) Object.assign(group, patch); + return group; + }), + listTasksByBranchGroup: vi.fn(async () => [task]), + recordRunAuditEvent: vi.fn(), }; - return { store, task, emitted, logs }; + return { store, task, emitted, logs, group }; } // A merge agent that actually performs the squash merge with git. @@ -573,6 +590,53 @@ describe("runAiMerge", () => { expect(git(dir, "rev-parse release")).not.toBe(releaseBefore); expect(git(dir, "rev-parse main")).toBe(mainBefore); }); + + /* + FNXC:BranchGroupCompletion 2026-07-04-00:00: + FN-7532 regression: runAiMerge is the SOLE merge path, so a shared-branch-group + member landed through it must come out with mergeDetails.mergeTargetBranch/ + mergeTargetSource stamped to the group's own branch via "branch-group-integration" — + exactly what isBranchGroupMemberLanded requires — not merged straight onto the + project default branch mislabeled (or unlabeled). + */ + it("routes a shared-branch-group member onto the group's branch and stamps mergeTargetSource: branch-group-integration", async () => { + const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); + const groupBranch = "fusion/groups/shared-x"; + const branchGroup = { id: "BG-1", branchName: groupBranch, sourceType: "planning", sourceId: "PS-1", status: "open", prState: "none" }; + const { store, task, group } = makeStore( + dir, + { branchContext: { assignmentMode: "shared", groupId: "BG-1" } }, + {}, + branchGroup, + ); + const mainBefore = git(dir, "rev-parse main"); + + const result = await runAiMerge(store, dir, "FN-1", { manual: true }, { + mergeAgent: realMergeAgent("fusion/fn-1"), + reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"), + }); + + expect(result.merged).toBe(true); + // Landed onto the GROUP's branch, not the project default. + expect(git(dir, `rev-parse ${groupBranch}`)).not.toBe(git(dir, "rev-parse main")); + expect(git(dir, "rev-parse main")).toBe(mainBefore); + expect(task.mergeDetails).toEqual( + expect.objectContaining({ + mergeConfirmed: true, + mergeTargetBranch: groupBranch, + mergeTargetSource: "branch-group-integration", + }), + ); + + // The exact invariant the checklist/PR-body/dashboard/CLI serializers all + // read from — prove the shared predicate now agrees the member landed. + const { isBranchGroupMemberLanded } = await import("@fusion/core"); + expect(isBranchGroupMemberLanded(task, { branchName: groupBranch })).toBe(true); + + // Group-row landing bookkeeping (worktreePath/status) was updated best-effort. + expect(store.recordBranchGroupMemberLanded).toHaveBeenCalledWith("BG-1", expect.objectContaining({ status: "open" })); + expect(group?.status).toBe("open"); + }); }); describe("landSquash (advance + local-checkout sync)", () => { diff --git a/packages/engine/src/merger-ai.ts b/packages/engine/src/merger-ai.ts index 3cc5e3b82b..27c5ab95a5 100644 --- a/packages/engine/src/merger-ai.ts +++ b/packages/engine/src/merger-ai.ts @@ -50,6 +50,7 @@ import { resolveValidatorSettingsModel, type MergeDetails, type MergeResult, + type MergeTargetResolution, type Settings, type Task, type TaskStore, @@ -66,7 +67,8 @@ import { checkSessionError } from "./usage-limit-detector.js"; import { accumulateSessionTokenUsage } from "./session-token-usage.js"; import { createRunAuditor, generateSyntheticRunId, type RunAuditor } from "./run-audit.js"; import { createLogger } from "./logger.js"; -import { captureSingleCommitLandedMetadata, type MergerOptions } from "./merger.js"; +import { captureSingleCommitLandedMetadata, syncGroupPrOnLanding, type MergerOptions } from "./merger.js"; +import { resolveBranchGroupMergeRouting, type BranchGroupMergeRouting, type SyncGroupPrFn } from "./group-merge-coordinator.js"; import { DEFAULT_COMMIT_AUTHOR_EMAIL, DEFAULT_COMMIT_AUTHOR_NAME } from "./worktree-hooks.js"; import { installWorktreeDependencies } from "./merge-dependency-sync.js"; import { activeSessionRegistry } from "./active-session-registry.js"; @@ -818,7 +820,26 @@ export async function runAiMerge( // integration branch. The local checkout is only synced if it is on this same // target branch (see syncLocalCheckout). const projectDefaultBranch = await resolveIntegrationBranch(projectRootDir, settings); - const mergeTarget = resolveTaskMergeTarget(task, { projectDefaultBranch }); + /* + FNXC:BranchGroupCompletion 2026-07-04-00:00: + FN-7532: runAiMerge is the SOLE merge path (master-plan U0 FNXC:MergerUnification), but it never + consulted branch-group routing, so a shared-branch-group member's mergeDetails never got + mergeTargetBranch/mergeTargetSource stamped. isBranchGroupMemberLanded requires + mergeTargetSource === "branch-group-integration" AND a matching mergeTargetBranch (merge-target + safety, see branch-group-completion.ts) — with both fields permanently undefined, every shared + member landed via the production path was reported as NOT landed forever (the branch-group + checklist/PR body "x/N landed" never advanced and promotion never became eligible). Route through + the same resolveBranchGroupMergeRouting used by the legacy merger.ts executeMergeAttempt so a + shared member's actual merge target is the group's branch (never a sibling/mismatched branch) and + the persisted mergeDetails correctly attribute the landing. + */ + const groupRouting = await resolveBranchGroupMergeRouting({ + task, + store, + projectDefaultBranch, + rootDir: projectRootDir, + }); + const mergeTarget = groupRouting?.mergeTarget ?? resolveTaskMergeTarget(task, { projectDefaultBranch }); const integrationBranch = mergeTarget.branch; const audit = createRunAuditor(store, { runId: generateSyntheticRunId("ai-merge", taskId), @@ -951,10 +972,10 @@ export async function runAiMerge( }; } await log(`AI merge: ${branch} had no net changes vs ${integrationBranch} — finalizing as no-op`); - return await finalizeMerged(store, projectRootDir, taskId, task, branch, integrationBranch, landResult.tipSha, audit, log, { empty: true }); + return await finalizeMerged(store, projectRootDir, taskId, task, branch, integrationBranch, landResult.tipSha, audit, log, { empty: true }, mergeTarget, groupRouting, options.syncGroupPr); } - return await finalizeMerged(store, projectRootDir, taskId, task, branch, integrationBranch, landResult.squashSha, audit, log, { empty: false }); + return await finalizeMerged(store, projectRootDir, taskId, task, branch, integrationBranch, landResult.squashSha, audit, log, { empty: false }, mergeTarget, groupRouting, options.syncGroupPr); } // --------------------------------------------------------------------------- @@ -1511,7 +1532,20 @@ async function finalizeMerged( audit: RunAuditor, log: (message: string) => Promise, opts: { empty: boolean }, + mergeTarget?: MergeTargetResolution, + groupRouting?: BranchGroupMergeRouting | null, + syncGroupPr?: SyncGroupPrFn, ): Promise { + /* + FNXC:BranchGroupCompletion 2026-07-04-00:00: + FN-7532: stamp mergeTargetBranch/mergeTargetSource on every finalize path (landed AND no-op), + not only the landed one — isBranchGroupMemberLanded needs both fields regardless of whether the + landing produced a real commit, otherwise a no-op-finalized shared-group member would also be + reported as not-landed forever. + */ + const mergeTargetPatch: Pick | undefined = mergeTarget + ? { mergeTargetBranch: mergeTarget.branch, mergeTargetSource: mergeTarget.source } + : undefined; let mergeDetails: MergeDetails | undefined; let modifiedFiles: string[] | undefined; if (!opts.empty && landedSha) { @@ -1531,6 +1565,7 @@ async function finalizeMerged( mergedAt, mergeConfirmed: true, prNumber: getPrimaryPrInfo(task)?.number, + ...mergeTargetPatch, }; modifiedFiles = landedFiles.length > 0 ? landedFiles : undefined; await store.updateTask(taskId, { mergeDetails, modifiedFiles }); @@ -1549,6 +1584,10 @@ async function finalizeMerged( deletions, }).catch(() => undefined); } + } else if (mergeTargetPatch) { + mergeDetails = { ...(task.mergeDetails ?? {}), ...mergeTargetPatch }; + await store.updateTask(taskId, { mergeDetails }); + task.mergeDetails = mergeDetails; } let branchDeleted = false; // NEVER delete the integration branch itself — a task whose branch name @@ -1585,6 +1624,49 @@ async function finalizeMerged( await log(opts.empty ? `AI merge: finalized ${taskId} (no-op), finalizing task row` : `AI merge: landed ${short(landedSha)}, finalizing task row`); const finalized = await finalizeTask(store, taskId, result, audit, log, projectRootDir); await log(opts.empty ? `AI merge: finalized ${taskId} (no-op) → done` : `AI merge: landed ${short(landedSha)}, task → done`); + + /* + FNXC:BranchGroupCompletion 2026-07-04-00:00: + FN-7532: mirror the legacy merger.ts executeMergeAttempt's shared-group landing bookkeeping so a + member merged via the (now sole) runAiMerge path also updates the group row (worktreePath/status) + and pushes the up-to-date checklist body onto any already-open managed group PR. Both are + best-effort — a failure here must never fail an otherwise-successful merge. + */ + if (groupRouting) { + try { + await Promise.resolve((store as { recordBranchGroupMemberLanded?: TaskStore["recordBranchGroupMemberLanded"] }).recordBranchGroupMemberLanded?.(groupRouting.branchGroup.id, { + worktreePath: task.worktree ?? null, + status: "open", + })); + } catch { + // best-effort persistence + } + if (syncGroupPr) { + try { + await syncGroupPrOnLanding({ + store, + groupId: groupRouting.branchGroup.id, + cwd: projectRootDir, + syncGroupPr, + }); + } catch (err) { + try { + store.recordRunAuditEvent?.({ + taskId, + agentId: "merger", + runId: `merge-${taskId}`, + domain: "git", + mutationType: "merge:branch-group-pr-sync-failed", + target: groupRouting.branchGroup.id, + metadata: { groupId: groupRouting.branchGroup.id, error: err instanceof Error ? err.message : String(err) }, + }); + } catch { + // best-effort audit + } + } + } + } + return finalized; }