From b1454c198e3631aa8c72f5fbe69fb400b851ca0d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 10:01:53 -0700 Subject: [PATCH] feat(FN-branch-group): create single real GitHub PR on group promotion (U5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group promotion in PR mode previously flipped prState to 'open' without ever calling GitHub — prNumber/prUrl were never populated. Add an injected CreateGroupPrFn (mirrors the processPullRequestMerge seam, no engine→dashboard import): coordinator creates-or-reuses exactly one PR per group, persists prNumber/prUrl/prState, and leaves state untouched on GitHub failure so re-promotion retries. Idempotent via persisted prNumber + getBranchGroupByBranchName. Wired at all three CLI engine-construction sites (daemon/dashboard/serve). --- .changeset/fn-branch-group-single-pr.md | 5 + .../cli/src/commands/__tests__/daemon.test.ts | 1 + .../cli/src/commands/__tests__/serve.test.ts | 1 + packages/cli/src/commands/daemon.ts | 2 + packages/cli/src/commands/dashboard.ts | 2 + packages/cli/src/commands/serve.ts | 2 + packages/cli/src/commands/task-lifecycle.ts | 38 +++- .../__tests__/github-create-group-pr.test.ts | 131 +++++++++++ packages/dashboard/src/github.ts | 87 +++++++- packages/dashboard/src/index.ts | 2 +- .../__tests__/group-merge-coordinator.test.ts | 206 ++++++++++++++++++ .../engine/src/group-merge-coordinator.ts | 76 ++++++- packages/engine/src/index.ts | 1 + packages/engine/src/project-engine-manager.ts | 2 + packages/engine/src/project-engine.ts | 12 +- 15 files changed, 562 insertions(+), 6 deletions(-) create mode 100644 .changeset/fn-branch-group-single-pr.md create mode 100644 packages/dashboard/src/__tests__/github-create-group-pr.test.ts diff --git a/.changeset/fn-branch-group-single-pr.md b/.changeset/fn-branch-group-single-pr.md new file mode 100644 index 0000000000..81cbe2dbf1 --- /dev/null +++ b/.changeset/fn-branch-group-single-pr.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": minor +--- + +Branch-group promotion now creates a single real GitHub PR for the group integration branch when promoting a completed PR-mode group. The PR number/url/state are persisted on the branch group and promotion is idempotent — re-running never opens a second PR (an existing persisted or open PR is reused). The GitHub client is injected into the engine via the same option-callback seam as `processPullRequestMerge`, wired at the `fn daemon`, `fn dashboard`, and `fn serve` construction sites. PR creation only happens for eligible (completion-gated, auto-merge-allowed) groups, and a GitHub failure leaves the group recoverable rather than persisting a false PR state. diff --git a/packages/cli/src/commands/__tests__/daemon.test.ts b/packages/cli/src/commands/__tests__/daemon.test.ts index b34295b8a6..57db955bcb 100644 --- a/packages/cli/src/commands/__tests__/daemon.test.ts +++ b/packages/cli/src/commands/__tests__/daemon.test.ts @@ -640,6 +640,7 @@ vi.mock("@earendil-works/pi-coding-agent", () => ({ vi.mock("../task-lifecycle.js", () => ({ getMergeStrategy: vi.fn((settings: { mergeStrategy?: "direct" | "pull-request" }) => settings.mergeStrategy ?? "direct"), processPullRequestMergeTask: vi.fn().mockResolvedValue("waiting"), + createGroupPrCallback: vi.fn(() => vi.fn()), })); vi.mock("../project-context.js", () => ({ diff --git a/packages/cli/src/commands/__tests__/serve.test.ts b/packages/cli/src/commands/__tests__/serve.test.ts index 068d3fa7d2..86911ae9a9 100644 --- a/packages/cli/src/commands/__tests__/serve.test.ts +++ b/packages/cli/src/commands/__tests__/serve.test.ts @@ -694,6 +694,7 @@ vi.mock("../port-prompt.js", () => ({ vi.mock("../task-lifecycle.js", () => ({ getMergeStrategy: vi.fn((settings: { mergeStrategy?: "direct" | "pull-request" }) => settings.mergeStrategy ?? "direct"), processPullRequestMergeTask: vi.fn().mockResolvedValue("waiting"), + createGroupPrCallback: vi.fn(() => vi.fn()), })); vi.mock("../project-context.js", () => ({ diff --git a/packages/cli/src/commands/daemon.ts b/packages/cli/src/commands/daemon.ts index 8b6c065443..3826827f3a 100644 --- a/packages/cli/src/commands/daemon.ts +++ b/packages/cli/src/commands/daemon.ts @@ -42,6 +42,7 @@ import { import { getMergeStrategy, processPullRequestMergeTask, + createGroupPrCallback, } from "./task-lifecycle.js"; import { promptForPort } from "./port-prompt.js"; import { createReadOnlyProviderSettingsView } from "./provider-settings.js"; @@ -334,6 +335,7 @@ export async function runDaemon(opts: DaemonOptions = {}) { getMergeStrategy, processPullRequestMerge: (s, wd, taskId, pool) => processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool), + createGroupPr: createGroupPrCallback(githubClient), getTaskMergeBlocker, onInsightRunProcessed: (s: unknown, r: unknown) => onMemoryInsightRunProcessed(s as ScheduledTask, r as AutomationRunResult), }); diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index 16e3ad4bd8..9663e6d82d 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -46,6 +46,7 @@ import { getMergeStrategy, getTaskBranchName, processPullRequestMergeTask, + createGroupPrCallback, } from "./task-lifecycle.js"; import { promptForPort } from "./port-prompt.js"; import { ensureCwdProjectRegistered } from "./ensure-project-registered.js"; @@ -1559,6 +1560,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: getMergeStrategy, processPullRequestMerge: (s, wd, taskId, pool) => processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool), + createGroupPr: createGroupPrCallback(githubClient), getTaskMergeBlocker, }); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 3d475f955a..32b022c04d 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -42,6 +42,7 @@ import { import { getMergeStrategy, processPullRequestMergeTask, + createGroupPrCallback, } from "./task-lifecycle.js"; import { promptForPort } from "./port-prompt.js"; import { createReadOnlyProviderSettingsView } from "./provider-settings.js"; @@ -360,6 +361,7 @@ export async function runServe( getMergeStrategy, processPullRequestMerge: (s, wd, taskId, pool) => processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool), + createGroupPr: createGroupPrCallback(githubClient), getTaskMergeBlocker, onInsightRunProcessed: (s: unknown, r: unknown) => onMemoryInsightRunProcessed(s as ScheduledTask, r as AutomationRunResult), }); diff --git a/packages/cli/src/commands/task-lifecycle.ts b/packages/cli/src/commands/task-lifecycle.ts index 2e867a480d..eaa277003e 100644 --- a/packages/cli/src/commands/task-lifecycle.ts +++ b/packages/cli/src/commands/task-lifecycle.ts @@ -20,7 +20,7 @@ import type { TaskStore } from "@fusion/core"; import { resolveTaskMergeTarget } from "@fusion/core"; import type { Settings, TaskDetail, PrInfo, MergeResult, BranchGroup, BranchGroupPrState, Task } from "@fusion/core"; import { activeSessionRegistry, resolveIntegrationBranch } from "@fusion/engine"; -import type { WorktreePool } from "@fusion/engine"; +import type { CreateGroupPrFn, WorktreePool } from "@fusion/engine"; /** * Minimal interface for GitHub operations needed by the PR merge workflow. @@ -163,6 +163,42 @@ function toBranchGroupPrState(prInfo: PrInfo | null): BranchGroupPrState { return "open"; } +/** + * Build the `createGroupPr` engine callback (KTD7) used by the branch-group + * promotion coordinator. Closes over a GitHub client so the engine never imports + * the dashboard client directly. Pushes the group integration branch to origin + * (so `gh pr create --head` / the REST API can find it), then creates or reuses + * the single managed PR for the group. + * + * Idempotency: reuses an existing PR for the group head branch on GitHub. The + * coordinator additionally skips this call when a `prNumber` is already persisted, + * so a re-promotion never opens a second PR. + */ +export function createGroupPrCallback( + github: Pick, +): CreateGroupPrFn { + return async ({ cwd, group, members, headBranch, baseBranch }) => { + const existing = await github.findPrForBranch({ head: headBranch, state: "all" }); + if (existing) { + return { prNumber: existing.number, prUrl: existing.url, prState: toBranchGroupPrState(existing) }; + } + + await pushTaskBranchToOrigin(cwd, headBranch); + const membersWithBranch = members.map((member) => ({ + id: member.id, + title: member.title, + branchName: getTaskBranchName(member.id), + })); + const created = await github.createPr({ + title: buildGroupPullRequestTitle(group, members), + body: buildGroupPullRequestBody(group, membersWithBranch), + head: headBranch, + base: baseBranch, + }); + return { prNumber: created.number, prUrl: created.url, prState: toBranchGroupPrState(created) }; + }; +} + async function hasCommitsRelativeToBranch(cwd: string, branch: string, baseBranch: string): Promise { try { const { stdout } = await execAsync(`git rev-list --count "${baseBranch}..${branch}"`, { cwd, timeout: 30_000 }); diff --git a/packages/dashboard/src/__tests__/github-create-group-pr.test.ts b/packages/dashboard/src/__tests__/github-create-group-pr.test.ts new file mode 100644 index 0000000000..fdc1c0f11f --- /dev/null +++ b/packages/dashboard/src/__tests__/github-create-group-pr.test.ts @@ -0,0 +1,131 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@fusion/core", async () => { + const actual = await vi.importActual("@fusion/core"); + return { + ...actual, + isGhAvailable: vi.fn(() => true), + isGhAuthenticated: vi.fn(() => true), + runGh: vi.fn(), + runGhAsync: vi.fn(), + runGhJson: vi.fn(), + runGhJsonAsync: vi.fn(), + getGhErrorMessage: vi.fn((err) => (err instanceof Error ? err.message : String(err))), + getCurrentRepo: vi.fn(() => ({ owner: "owner", repo: "repo" })), + }; +}); + +import { runGh, runGhJsonAsync } from "@fusion/core"; +import { GitHubClient, createGroupPullRequest, buildGroupPullRequestTitle, buildGroupPullRequestBody } from "../github.js"; + +const mockRunGh = vi.mocked(runGh); +const mockRunGhJsonAsync = vi.mocked(runGhJsonAsync); + +const group = { + id: "BG-1", + branchName: "fusion/groups/planning-x", + sourceType: "planning" as const, + sourceId: "PS-1", +}; +const members = [ + { id: "FN-A", title: "Alpha" }, + { id: "FN-B", title: "Beta" }, +]; + +describe("createGroupPullRequest", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("creates a PR via the gh-CLI backend and returns persisted shape", async () => { + // findPrForBranch (gh): no existing PR. + mockRunGhJsonAsync.mockResolvedValueOnce([] as any); + // createPr (gh): returns the PR url on stdout. + mockRunGh.mockReturnValue("https://github.com/owner/repo/pull/55\n"); + const client = new GitHubClient({ forceMode: "gh-cli" }); + + const result = await createGroupPullRequest(client, { + group, + members, + headBranch: group.branchName, + baseBranch: "main", + }); + + expect(result).toEqual({ + prNumber: 55, + prUrl: "https://github.com/owner/repo/pull/55", + prState: "open", + }); + const createArgs = mockRunGh.mock.calls[0][0]; + expect(createArgs).toEqual(expect.arrayContaining(["pr", "create", "--head", group.branchName, "--base", "main"])); + }); + + it("creates a PR via the REST API backend and returns persisted shape", async () => { + const client = new GitHubClient({ token: "ghp_token", forceMode: "token" }); + const fetchSpy = vi.spyOn(global, "fetch" as any) + // findPrForBranch (API): empty list. + .mockResolvedValueOnce({ ok: true, json: async () => [] } as any) + // createPr (API). + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + number: 77, + html_url: "https://github.com/owner/repo/pull/77", + title: "T", + state: "open", + head: { ref: group.branchName }, + base: { ref: "main" }, + comments: 0, + }), + } as any); + + const result = await createGroupPullRequest(client, { + group, + members, + headBranch: group.branchName, + baseBranch: "main", + }); + + expect(result).toEqual({ + prNumber: 77, + prUrl: "https://github.com/owner/repo/pull/77", + prState: "open", + }); + fetchSpy.mockRestore(); + }); + + it("reuses an existing open PR instead of creating a second one (idempotent)", async () => { + mockRunGhJsonAsync.mockResolvedValueOnce([ + { number: 12, url: "https://github.com/owner/repo/pull/12", title: "T", state: "OPEN", baseRefName: "main", headRefName: group.branchName, mergedAt: null }, + ] as any); + const client = new GitHubClient({ forceMode: "gh-cli" }); + + const result = await createGroupPullRequest(client, { + group, + members, + headBranch: group.branchName, + baseBranch: "main", + }); + + expect(result).toEqual({ + prNumber: 12, + prUrl: "https://github.com/owner/repo/pull/12", + prState: "open", + }); + // createPr must NOT have been called. + expect(mockRunGh).not.toHaveBeenCalled(); + }); +}); + +describe("group PR title/body builders", () => { + it("title includes the group id, source, and member count", () => { + expect(buildGroupPullRequestTitle(group, members)).toBe("BG-1: planning/PS-1 (2 tasks)"); + }); + + it("body lists every member task", () => { + const body = buildGroupPullRequestBody(group, members); + expect(body).toContain("Automated group PR for BG-1."); + expect(body).toContain("- FN-A: Alpha"); + expect(body).toContain("- FN-B: Beta"); + }); +}); diff --git a/packages/dashboard/src/github.ts b/packages/dashboard/src/github.ts index 636f0f3169..a739faec25 100644 --- a/packages/dashboard/src/github.ts +++ b/packages/dashboard/src/github.ts @@ -3,7 +3,7 @@ import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; -import type { DirectMergeCommitStrategy, IssueInfo, PrConflictDiagnostics, PrConflictState, PrInfo, TaskReviewData, TaskReviewItem, TaskReviewSummary } from "@fusion/core"; +import type { BranchGroup, BranchGroupPrState, DirectMergeCommitStrategy, IssueInfo, PrConflictDiagnostics, PrConflictState, PrInfo, Task, TaskReviewData, TaskReviewItem, TaskReviewSummary } from "@fusion/core"; import { isGhAvailable, isGhAuthenticated, @@ -3693,3 +3693,88 @@ export function parseGitHubBadgeUrl(url: string): { owner: string; repo: string return { owner: parsed.owner, repo: parsed.repo }; } +/** Map a `PrInfo.status` to the persisted `BranchGroup.prState`. */ +function prInfoToBranchGroupPrState(prInfo: PrInfo | null): BranchGroupPrState { + if (!prInfo) return "none"; + if (prInfo.status === "merged") return "merged"; + if (prInfo.status === "closed") return "closed"; + return "open"; +} + +/** Build the title for a single managed group PR. */ +export function buildGroupPullRequestTitle( + group: Pick, + members: Pick[], +): string { + return `${group.id}: ${group.sourceType}/${group.sourceId} (${members.length} tasks)`; +} + +/** Build the body for a single managed group PR (member checklist + completion). */ +export function buildGroupPullRequestBody( + group: Pick, + members: Pick[], +): string { + const lines = members.map((member) => `- ${member.id}: ${member.title || "(untitled)"}`); + return [ + `Automated group PR for ${group.id}.`, + `Source: ${group.sourceType}/${group.sourceId}`, + `Integration branch: \`${group.branchName}\``, + "", + "Included tasks:", + ...(lines.length > 0 ? lines : ["- (none)"]), + ].join("\n"); +} + +export interface CreateGroupPrInput { + group: Pick; + members: Pick[]; + /** Head branch — the group integration branch. */ + headBranch: string; + /** Base branch — the project default / integration target. */ + baseBranch: string; +} + +export interface CreateGroupPrResult { + prNumber: number; + prUrl: string; + prState: BranchGroupPrState; +} + +/** + * Create (or reuse) the single managed GitHub PR for a branch group. + * + * Idempotency: if an existing PR is already open for the group head branch on + * GitHub, it is reused rather than opening a second one. This is the GitHub-side + * idempotency guard; the coordinator additionally checks the persisted + * `prNumber` before ever calling this helper. + * + * Backend parity: dispatches through `GitHubClient.findPrForBranch` / + * `GitHubClient.createPr`, which transparently use the `gh` CLI when available + * and fall back to the REST API, so both paths produce the same result shape. + */ +export async function createGroupPullRequest( + github: Pick, + input: CreateGroupPrInput, +): Promise { + const existing = await github.findPrForBranch({ head: input.headBranch, state: "all" }); + if (existing) { + return { + prNumber: existing.number, + prUrl: existing.url, + prState: prInfoToBranchGroupPrState(existing), + }; + } + + const created = await github.createPr({ + title: buildGroupPullRequestTitle(input.group, input.members), + body: buildGroupPullRequestBody(input.group, input.members), + head: input.headBranch, + base: input.baseBranch, + }); + return { + prNumber: created.number, + prUrl: created.url, + prState: prInfoToBranchGroupPrState(created), + }; +} + diff --git a/packages/dashboard/src/index.ts b/packages/dashboard/src/index.ts index d1741a4210..0e903a0234 100644 --- a/packages/dashboard/src/index.ts +++ b/packages/dashboard/src/index.ts @@ -11,7 +11,7 @@ export { type RuntimeLogSink, } from "./runtime-logger.js"; export { createSkillsAdapter, getProjectSettingsPath, type SkillsAdapter, type DiscoveredSkill, type CatalogEntry, type CatalogFetchResult, type ToggleSkillResult, type UpstreamError, type UpstreamErrorCode, type SkillContent, type SkillFileEntry } from "./skills-adapter.js"; -export { GitHubClient, isPrMergeReady, type GitHubClientOptions, type PrMergeStatus, type PrCheckStatus, type ReviewDecision, type MergePrParams, type FindPrParams, type CreateIssueParams, type CreatedIssue } from "./github.js"; +export { GitHubClient, isPrMergeReady, createGroupPullRequest, buildGroupPullRequestTitle, buildGroupPullRequestBody, type GitHubClientOptions, type PrMergeStatus, type PrCheckStatus, type ReviewDecision, type MergePrParams, type FindPrParams, type CreateIssueParams, type CreatedIssue, type CreateGroupPrInput, type CreateGroupPrResult } from "./github.js"; export { generatePrMetadata, type GeneratedPrMetadata } from "./pr-metadata-generator.js"; export { maybeCreateTrackingIssue, type MaybeCreateTrackingIssueDeps } from "./github-tracking.js"; export { diff --git a/packages/engine/src/__tests__/group-merge-coordinator.test.ts b/packages/engine/src/__tests__/group-merge-coordinator.test.ts index ff1f3e084b..8889e630db 100644 --- a/packages/engine/src/__tests__/group-merge-coordinator.test.ts +++ b/packages/engine/src/__tests__/group-merge-coordinator.test.ts @@ -337,6 +337,211 @@ describe("promoteBranchGroup", () => { }); }); +describe("promoteBranchGroup PR creation (U5)", () => { + function makeGroup(overrides?: Partial): any { + return { + id: "BG-PR-1", + sourceType: "planning", + sourceId: "planning:x", + branchName: "fusion/groups/planning-x", + autoMerge: true, + prState: "none", + status: "open", + createdAt: Date.now(), + updatedAt: Date.now(), + ...overrides, + }; + } + + const landedMember = (id: string, branchName: string) => ({ + id, + title: `${id} title`, + column: "done" as const, + mergeDetails: { + mergeConfirmed: true, + mergeTargetSource: "branch-group-integration", + mergeTargetBranch: branchName, + }, + }); + + function makePrRepo(): string { + const rootDir = makeRepo(); + execSync("git checkout -b fusion/groups/planning-x", { cwd: rootDir }); + execSync("echo promoted > group.txt", { cwd: rootDir, shell: "/bin/bash" }); + execSync("git add group.txt && git commit -m group", { cwd: rootDir, shell: "/bin/bash" }); + execSync("git checkout main", { cwd: rootDir }); + return rootDir; + } + + function makeStore(getGroup: () => any, setGroup: (g: any) => void, members: any[], byBranch?: () => any) { + return { + getBranchGroup: () => getGroup(), + getBranchGroupByBranchName: byBranch ?? (() => null), + listTasksByBranchGroup: async () => members, + updateBranchGroup: (_id: string, patch: Record) => { + setGroup({ ...getGroup(), ...patch }); + return getGroup(); + }, + } as any; + } + + const prSettings = { + autoMerge: true, + globalPause: false, + enginePaused: false, + mergeStrategy: "pull-request" as const, + baseBranch: "main", + }; + + it("creates exactly one PR for a complete PR-mode group and persists prNumber/prUrl/prState=open", async () => { + const rootDir = makePrRepo(); + let group = makeGroup(); + let createCalls = 0; + const result = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore(() => group, (g) => { group = g; }, [landedMember("FN-A", group.branchName)]), + createGroupPr: async ({ headBranch, baseBranch, members }) => { + createCalls += 1; + expect(headBranch).toBe("fusion/groups/planning-x"); + expect(baseBranch).toBe("main"); + expect(members.map((m: any) => m.id)).toEqual(["FN-A"]); + return { prNumber: 42, prUrl: "https://github.com/x/y/pull/42", prState: "open" }; + }, + }); + + expect(result.reason).toBe("promoted"); + expect(createCalls).toBe(1); + expect(group.status).toBe("finalized"); + expect(group.prState).toBe("open"); + expect(group.prNumber).toBe(42); + expect(group.prUrl).toBe("https://github.com/x/y/pull/42"); + }); + + it("is idempotent: a persisted prNumber means re-promotion never opens a second PR", async () => { + const rootDir = makePrRepo(); + let createCalls = 0; + const createGroupPr = async () => { + createCalls += 1; + return { prNumber: 7, prUrl: "https://github.com/x/y/pull/7", prState: "open" as const }; + }; + + // First promotion creates the PR. + let group = makeGroup(); + await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore(() => group, (g) => { group = g; }, [landedMember("FN-A", group.branchName)]), + createGroupPr, + }); + expect(createCalls).toBe(1); + expect(group.prNumber).toBe(7); + + // Re-running while the group already has prState=open short-circuits at the + // top guard (already-finalized) — the creator is NOT called again. + const again = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore(() => group, (g) => { group = g; }, [landedMember("FN-A", group.branchName)]), + createGroupPr, + }); + expect(again.reason).toBe("already-finalized"); + expect(createCalls).toBe(1); + }); + + it("reuses an existing PR via getBranchGroupByBranchName without invoking the creator", async () => { + const rootDir = makePrRepo(); + let group = makeGroup(); + let createCalls = 0; + const sibling = makeGroup({ id: "BG-PR-OTHER", prNumber: 99, prUrl: "https://github.com/x/y/pull/99", prState: "open" }); + const result = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore( + () => group, + (g) => { group = g; }, + [landedMember("FN-A", group.branchName)], + () => sibling, + ), + createGroupPr: async () => { + createCalls += 1; + return { prNumber: 1, prUrl: "x", prState: "open" as const }; + }, + }); + + expect(result.reason).toBe("promoted"); + expect(createCalls).toBe(0); + expect(group.prNumber).toBe(99); + expect(group.prUrl).toBe("https://github.com/x/y/pull/99"); + expect(group.prState).toBe("open"); + }); + + it("does not create a PR for an incomplete group (gate blocks before creation)", async () => { + const rootDir = makePrRepo(); + let group = makeGroup(); + let createCalls = 0; + const result = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore(() => group, (g) => { group = g; }, [{ id: "FN-A", column: "todo" }]), + createGroupPr: async () => { + createCalls += 1; + return { prNumber: 1, prUrl: "x", prState: "open" as const }; + }, + }); + + expect(result.reason).toBe("incomplete"); + expect(createCalls).toBe(0); + expect(group.prState).toBe("none"); + expect(group.status).toBe("open"); + }); + + it("leaves the group recoverable when PR creation fails (no partial prState lie)", async () => { + const rootDir = makePrRepo(); + let group = makeGroup(); + await expect( + promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore(() => group, (g) => { group = g; }, [landedMember("FN-A", group.branchName)]), + createGroupPr: async () => { + throw new Error("gh: network down"); + }, + }), + ).rejects.toThrow("gh: network down"); + + // prState/status must NOT be flipped to a lie; re-promotion can retry. + expect(group.prState).toBe("none"); + expect(group.status).toBe("open"); + }); + + it("autoMerge:false group is not promoted (PR creation only on eligible/explicit promote)", async () => { + const rootDir = makePrRepo(); + let group = makeGroup({ autoMerge: false }); + let createCalls = 0; + const result = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store: makeStore(() => group, (g) => { group = g; }, [landedMember("FN-A", group.branchName)]), + createGroupPr: async () => { + createCalls += 1; + return { prNumber: 1, prUrl: "x", prState: "open" as const }; + }, + }); + + expect(result.reason).toBe("gated"); + expect(createCalls).toBe(0); + expect(group.prState).toBe("none"); + }); +}); + describe("ProjectEngine.promoteBranchGroup (U4 bridge method)", () => { // The dashboard promote route calls engine.promoteBranchGroup AS A METHOD. // These tests invoke the REAL method body bound to a minimal engine-shaped @@ -383,6 +588,7 @@ describe("ProjectEngine.promoteBranchGroup (U4 bridge method)", () => { context: { runtime: { getTaskStore: () => fullStore }, config: { workingDirectory: rootDir }, + options: {}, }, getSettingsCalls, }; diff --git a/packages/engine/src/group-merge-coordinator.ts b/packages/engine/src/group-merge-coordinator.ts index 204c0e511e..e2ee141e55 100644 --- a/packages/engine/src/group-merge-coordinator.ts +++ b/packages/engine/src/group-merge-coordinator.ts @@ -12,6 +12,28 @@ export interface BranchGroupMergeRouting { mergeTarget: MergeTargetResolution; } +/** + * Injected callback (KTD7) that creates — or reuses — the single managed GitHub + * PR for a branch group. Closes over a dashboard-built `GitHubClient` at the CLI + * construction sites so the engine never statically imports `@fusion/dashboard` + * (avoids the engine ↔ dashboard import cycle). Mirrors the `processPullRequestMerge` + * injection seam. + * + * Returns the GitHub PR number/url and the persisted-state mapping. Idempotency is + * enforced both here (reuse an existing open PR for the head branch) and by the + * coordinator (skip the call entirely when a `prNumber` is already persisted). + */ +export type CreateGroupPrFn = (input: { + /** Project working directory — needed to push the head branch to origin. */ + cwd: string; + group: BranchGroup; + members: Task[]; + /** Head branch — the group integration branch. */ + headBranch: string; + /** Base branch — the integration/default target. */ + baseBranch: string; +}) => Promise<{ prNumber: number; prUrl: string; prState: BranchGroupPrState }>; + export interface BranchGroupCompletionStatus { complete: boolean; totalMembers: number; @@ -118,10 +140,16 @@ async function ensureGroupBranchExists(rootDir: string, branchName: string, star * Promotion is intentionally idempotent and must never run inline in aiMergeTask. */ export async function promoteBranchGroup(input: { - store: Pick; + store: Pick; rootDir: string; groupId: string; settings: Pick & Partial>; + /** + * Injected GitHub PR creator (KTD7). When PR mode is active and the group is + * complete, the coordinator uses this to create the single managed PR. Omitted + * for direct-merge mode and in tests that don't exercise PR creation. + */ + createGroupPr?: CreateGroupPrFn; recordAudit?: (event: { domain: string; mutationType: string; @@ -219,9 +247,53 @@ export async function promoteBranchGroup(input: { } const isPrMode = input.settings.mergeStrategy === "pull-request"; + + let prNumber: number | undefined = group.prNumber; + let prUrl: string | undefined = group.prUrl; + let prState: BranchGroupPrState = isPrMode ? "open" : "merged"; + + if (isPrMode) { + // Idempotency (KTD4): never open a second PR. Prefer a PR already persisted + // on this group; otherwise reuse any open PR another group row may hold for + // the same head branch. Only when neither exists do we invoke the injected + // creator. The injected creator itself also reuses an existing GitHub PR. + const persistedPr = group.prNumber + ? { prNumber: group.prNumber, prUrl: group.prUrl } + : (() => { + const existing = input.store.getBranchGroupByBranchName(group.branchName); + return existing && existing.id !== group.id && existing.prNumber + ? { prNumber: existing.prNumber, prUrl: existing.prUrl } + : null; + })(); + + if (persistedPr) { + prNumber = persistedPr.prNumber; + prUrl = persistedPr.prUrl; + prState = "open"; + } else if (input.createGroupPr) { + // GitHub failure must leave the group recoverable: do NOT flip prState to a + // lie. The group is already merged to the integration branch locally; we + // surface the error so the caller can retry promotion (which is idempotent). + const created = await input.createGroupPr({ + cwd: input.rootDir, + group, + members, + headBranch: group.branchName, + baseBranch: integrationBranch, + }); + prNumber = created.prNumber; + prUrl = created.prUrl; + prState = created.prState; + } + // If neither a persisted PR nor a createGroupPr callback is available, fall + // back to the legacy behaviour (flip prState to "open" without a number). + } + const updatedGroup = input.store.updateBranchGroup(group.id, { status: "finalized", - prState: isPrMode ? "open" : "merged", + prState, + prNumber: prNumber ?? null, + prUrl: prUrl ?? null, }); await input.recordAudit?.({ diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 71bed8fbb2..02febd4933 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -65,6 +65,7 @@ export { type BranchGroupPromotionDecision, type BranchGroupCompletionStatus, type BranchGroupPromotionResult, + type CreateGroupPrFn, } from "./group-merge-coordinator.js"; export { resolveMergeIntegrationRoot, diff --git a/packages/engine/src/project-engine-manager.ts b/packages/engine/src/project-engine-manager.ts index 0006d8826a..1e33434976 100644 --- a/packages/engine/src/project-engine-manager.ts +++ b/packages/engine/src/project-engine-manager.ts @@ -36,6 +36,7 @@ import { runtimeLog } from "./logger.js"; export interface EngineManagerOptions { getMergeStrategy?: ProjectEngineOptions["getMergeStrategy"]; processPullRequestMerge?: ProjectEngineOptions["processPullRequestMerge"]; + createGroupPr?: ProjectEngineOptions["createGroupPr"]; getTaskMergeBlocker?: ProjectEngineOptions["getTaskMergeBlocker"]; onInsightRunProcessed?: ProjectEngineOptions["onInsightRunProcessed"]; } @@ -481,6 +482,7 @@ export class ProjectEngineManager { projectId: project.id, getMergeStrategy: this.options.getMergeStrategy, processPullRequestMerge: this.options.processPullRequestMerge, + createGroupPr: this.options.createGroupPr, getTaskMergeBlocker: this.options.getTaskMergeBlocker, onInsightRunProcessed: this.options.onInsightRunProcessed, ...overrides, diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index ea77b1081b..6b6912f9ad 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -27,7 +27,7 @@ import { CronRunner, createAiPromptExecutor } from "./cron-runner.js"; import type { RoutineRunner } from "./routine-runner.js"; import { aiMergeTask, sweepStaleAutostashes, VerificationError } from "./merger.js"; import { runAiMerge } from "./merger-ai.js"; -import { promoteBranchGroup, type BranchGroupPromotionResult } from "./group-merge-coordinator.js"; +import { promoteBranchGroup, type BranchGroupPromotionResult, type CreateGroupPrFn } from "./group-merge-coordinator.js"; import { PRIORITY_MERGE } from "./concurrency.js"; import { runtimeLog } from "./logger.js"; import type { HeartbeatTriggerScheduler } from "./agent-heartbeat.js"; @@ -205,6 +205,14 @@ export interface ProjectEngineOptions { * can be "pull-request". Injected from CLI layer. */ processPullRequestMerge?: ProcessPullRequestMergeFn; + /** + * Creates (or reuses) the single managed GitHub PR for a branch group during + * promotion (KTD7). Injected from the CLI layer because it depends on the + * dashboard `GitHubClient`; the engine must not statically import it. Mirrors + * the `processPullRequestMerge` seam. When absent, PR-mode promotion flips + * `prState` to "open" without creating a real PR (legacy behaviour). + */ + createGroupPr?: CreateGroupPrFn; /** * Returns the merge blocker reason for a task, or null/undefined if * the task is eligible for merge. Imported from @fusion/core. @@ -952,6 +960,7 @@ export class ProjectEngine { rootDir: cwd, groupId, settings: promotionSettings, + createGroupPr: this.options.createGroupPr, recordAudit: async (event) => { await store.recordRunAuditEvent({ domain: event.domain as any, @@ -1894,6 +1903,7 @@ export class ProjectEngine { rootDir: cwd, groupId: taskForPromotion.branchContext!.groupId, settings: promotionSettings, + createGroupPr: this.options.createGroupPr, recordAudit: async (event) => { await store.recordRunAuditEvent({ domain: event.domain as any,