diff --git a/packages/engine/src/__tests__/group-merge-coordinator.test.ts b/packages/engine/src/__tests__/group-merge-coordinator.test.ts index 8889e630db..35bd67d7d2 100644 --- a/packages/engine/src/__tests__/group-merge-coordinator.test.ts +++ b/packages/engine/src/__tests__/group-merge-coordinator.test.ts @@ -9,6 +9,7 @@ import { evaluateBranchGroupCompletion, evaluateBranchGroupPromotion, promoteBranchGroup, + reconcileBranchGroupPr, resolveBranchGroupMergeRouting, } from "../group-merge-coordinator.js"; import { ProjectEngine } from "../project-engine.js"; @@ -639,6 +640,310 @@ describe("ProjectEngine.promoteBranchGroup (U4 bridge method)", () => { }); }); +describe("promoteBranchGroup concurrency lock (Fix #10)", () => { + function makeGroup(overrides?: Partial): any { + return { + id: "BG-LOCK-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; + } + + const prSettings = { + autoMerge: true, + globalPause: false, + enginePaused: false, + mergeStrategy: "pull-request" as const, + baseBranch: "main", + }; + + it("serializes two concurrent promotions: createGroupPr runs exactly once, one PR persisted", async () => { + const rootDir = makePrRepo(); + let group = makeGroup(); + let createCalls = 0; + const store = { + getBranchGroup: () => group, + getBranchGroupByBranchName: () => null, + listTasksByBranchGroup: async () => [landedMember("FN-A", group.branchName)], + updateBranchGroup: (_id: string, patch: Record) => { + group = { ...group, ...patch }; + return group; + }, + } as any; + + // The injected creator yields (await a macrotask) so that, WITHOUT the lock, + // a second concurrent call would slip past the prState/status gate (which is + // read at the top, before the first call has persisted "open") and create a + // second PR. With the per-group lock the second call only begins after the + // first persisted its result and short-circuits as already-finalized. + const createGroupPr = async () => { + createCalls += 1; + const n = createCalls; + await new Promise((resolve) => setTimeout(resolve, 25)); + return { prNumber: 40 + n, prUrl: `https://github.com/x/y/pull/${40 + n}`, prState: "open" as const }; + }; + + const [a, b] = await Promise.all([ + promoteBranchGroup({ rootDir, groupId: group.id, settings: prSettings, store, createGroupPr }), + promoteBranchGroup({ rootDir, groupId: group.id, settings: prSettings, store, createGroupPr }), + ]); + + expect(createCalls).toBe(1); + expect(group.prNumber).toBe(41); + expect(group.prState).toBe("open"); + expect(group.status).toBe("finalized"); + + // Exactly one call reports a fresh promotion; the other sees already-finalized. + const reasons = [a.reason, b.reason].sort(); + expect(reasons).toEqual(["already-finalized", "promoted"]); + const promoted = [a, b].filter((r) => r.reason === "promoted"); + expect(promoted).toHaveLength(1); + expect(promoted[0].prNumber).toBe(41); + }); +}); + +describe("promoteBranchGroup finalized-but-PR-less repair (Fix #4 part 2)", () => { + function makeGroup(overrides?: Partial): any { + return { + id: "BG-REPAIR-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; + } + + const prSettings = { + autoMerge: true, + globalPause: false, + enginePaused: false, + mergeStrategy: "pull-request" as const, + baseBranch: "main", + }; + + it("re-promotion creates the PR for a finalized PR-less group WITHOUT re-running the integration merge", async () => { + const rootDir = makePrRepo(); + // Simulate a crash AFTER the integration merge + finalize but BEFORE the PR + // was created: group is finalized, prState none, prNumber null. + let group = makeGroup({ status: "finalized", prState: "none", prNumber: null, prUrl: null }); + let createCalls = 0; + let mergeCalls = 0; + const store = { + getBranchGroup: () => group, + getBranchGroupByBranchName: () => null, + listTasksByBranchGroup: async () => [landedMember("FN-A", group.branchName)], + updateBranchGroup: (_id: string, patch: Record) => { + group = { ...group, ...patch }; + return group; + }, + } as any; + + // Detect whether the integration merge ran by recording the merge commit on + // main before re-promotion. The repair path must NOT advance main again. + const mainBefore = execSync("git rev-parse main", { cwd: rootDir, encoding: "utf8" }).trim(); + void mergeCalls; + + const result = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store, + createGroupPr: async ({ members }) => { + createCalls += 1; + expect(members.map((m: any) => m.id)).toEqual(["FN-A"]); + return { prNumber: 77, prUrl: "https://github.com/x/y/pull/77", prState: "open" as const }; + }, + }); + + expect(result.reason).toBe("promoted"); + expect(createCalls).toBe(1); + expect(group.prNumber).toBe(77); + expect(group.prState).toBe("open"); + expect(group.status).toBe("finalized"); + + // The merge step was skipped: main is unchanged from before the repair. + const mainAfter = execSync("git rev-parse main", { cwd: rootDir, encoding: "utf8" }).trim(); + expect(mainAfter).toBe(mainBefore); + }); + + it("a finalized group that already has a prNumber is still short-circuited (no repair, no PR re-create)", async () => { + const rootDir = makePrRepo(); + let group = makeGroup({ status: "finalized", prState: "open", prNumber: 5, prUrl: "https://github.com/x/y/pull/5" }); + let createCalls = 0; + const store = { + getBranchGroup: () => group, + getBranchGroupByBranchName: () => null, + listTasksByBranchGroup: async () => [landedMember("FN-A", group.branchName)], + updateBranchGroup: () => { + throw new Error("should not update an already-PR'd finalized group"); + }, + } as any; + + const result = await promoteBranchGroup({ + rootDir, + groupId: group.id, + settings: prSettings, + store, + createGroupPr: async () => { + createCalls += 1; + return { prNumber: 1, prUrl: "x", prState: "open" as const }; + }, + }); + + expect(result.reason).toBe("already-finalized"); + expect(createCalls).toBe(0); + }); +}); + +describe("reconcileBranchGroupPr (Fix #3 engine primitive)", () => { + function makeGroup(overrides?: Partial): any { + return { + id: "BG-RECON-1", + sourceType: "planning", + sourceId: "planning:x", + branchName: "fusion/groups/planning-x", + autoMerge: true, + prState: "open", + prNumber: 12, + prUrl: "https://github.com/x/y/pull/12", + status: "finalized", + createdAt: Date.now(), + updatedAt: Date.now(), + ...overrides, + }; + } + + it("persists merged state when syncGroupPr reports the PR merged", async () => { + let group = makeGroup(); + const updates: Array> = []; + const store = { + listTasksByBranchGroup: async () => [{ id: "FN-A" }], + updateBranchGroup: (_id: string, patch: Record) => { + updates.push(patch); + group = { ...group, ...patch }; + return group; + }, + } as any; + + const result = await reconcileBranchGroupPr({ + store, + group, + syncGroupPr: async () => ({ + prNumber: 12, + prUrl: "https://github.com/x/y/pull/12", + prState: "merged", + }), + }); + + expect(result.reconciled).toBe(true); + expect(result.prState).toBe("merged"); + expect(group.prState).toBe("merged"); + expect(updates).toHaveLength(1); + expect(updates[0]).toMatchObject({ prState: "merged", prNumber: 12 }); + }); + + it("is a no-op (no persist) when the PR is still open", async () => { + const group = makeGroup(); + let updateCalls = 0; + const store = { + listTasksByBranchGroup: async () => [{ id: "FN-A" }], + updateBranchGroup: () => { + updateCalls += 1; + return group; + }, + } as any; + + const result = await reconcileBranchGroupPr({ + store, + group, + syncGroupPr: async () => ({ + prNumber: 12, + prUrl: "https://github.com/x/y/pull/12", + prState: "open", + }), + }); + + expect(result.reconciled).toBe(false); + expect(result.prState).toBe("open"); + expect(updateCalls).toBe(0); + }); + + it("is a no-op when the group has no persisted prNumber", async () => { + const group = makeGroup({ prNumber: null, prState: "none" }); + let syncCalls = 0; + const store = { + listTasksByBranchGroup: async () => [{ id: "FN-A" }], + updateBranchGroup: () => { + throw new Error("should not update"); + }, + } as any; + + const result = await reconcileBranchGroupPr({ + store, + group, + syncGroupPr: async () => { + syncCalls += 1; + return { prNumber: 0, prUrl: "", prState: "open" as const }; + }, + }); + + expect(result.reconciled).toBe(false); + expect(syncCalls).toBe(0); + }); +}); + describe("resolveBranchGroupMergeRouting", () => { it("returns null for non-shared tasks", async () => { const routing = await resolveBranchGroupMergeRouting({ diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index 4d9774959a..e7b7a98de1 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -1782,6 +1782,73 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { await engine.stop(); }); + it("records an audit event (not silent) when auto-promotion of a branch-group member fails (Fix #4)", async () => { + const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + // The dequeued + merged task is a shared branch-group member, so the engine + // attempts branch-group promotion after the PR merges. + const mergedMember = { + id: "FN-bgfail", + column: "done", + paused: false, + mergeRetries: 0, + status: null, + branch: "fusion/fn-bgfail", + branchContext: { groupId: "BG-FAIL-1", source: "planning", assignmentMode: "shared" }, + mergeDetails: { mergeConfirmed: true, mergedAt: "2026-06-03T00:00:00.000Z", mergeTargetBranch: "fusion/groups/x" }, + }; + mockStore.store.getTask + .mockResolvedValueOnce({ + id: "FN-bgfail", + column: "in-review", + paused: false, + mergeRetries: 0, + status: null, + branch: "fusion/fn-bgfail", + branchContext: { groupId: "BG-FAIL-1", source: "planning", assignmentMode: "shared" }, + }) + .mockResolvedValue(mergedMember); + + const recordRunAuditEvent = vi.fn(async () => undefined); + // Drive promoteBranchGroup into throwing: getBranchGroup returns a complete- + // looking group, but listTasksByBranchGroup rejects, so promotion throws and + // the engine's catch must record the failure audit instead of swallowing it. + (mockStore.store as any).getBranchGroup = vi.fn(() => ({ + id: "BG-FAIL-1", + sourceType: "planning", + sourceId: "planning:x", + branchName: "fusion/groups/x", + autoMerge: true, + prState: "none", + status: "open", + createdAt: Date.now(), + updatedAt: Date.now(), + })); + (mockStore.store as any).getBranchGroupByBranchName = vi.fn(() => null); + (mockStore.store as any).listTasksByBranchGroup = vi.fn(async () => { + throw new Error("boom: store unavailable"); + }); + (mockStore.store as any).updateBranchGroup = vi.fn(); + (mockStore.store as any).recordRunAuditEvent = recordRunAuditEvent; + mocks.currentStore = mockStore.store; + + const processPullRequestMerge = vi.fn(async () => "merged" as const); + const engine = createEngine({ processPullRequestMerge, getMergeStrategy: () => "pull-request" }); + await engine.start(); + engine.enqueueMerge("FN-bgfail"); + + await vi.waitFor(() => { + expect(recordRunAuditEvent).toHaveBeenCalledWith( + expect.objectContaining({ + mutationType: "merge:branch-group-promotion-failed", + target: "BG-FAIL-1", + metadata: expect.objectContaining({ groupId: "BG-FAIL-1", taskId: "FN-bgfail" }), + }), + ); + }); + + await engine.stop(); + }); + it("logs and skips paused tasks dequeued for auto-merge", async () => { const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); mockStore.store.getTask.mockResolvedValueOnce({ diff --git a/packages/engine/src/group-merge-coordinator.ts b/packages/engine/src/group-merge-coordinator.ts index f0a9452e85..a77adbda3d 100644 --- a/packages/engine/src/group-merge-coordinator.ts +++ b/packages/engine/src/group-merge-coordinator.ts @@ -173,11 +173,30 @@ async function ensureGroupBranchExists(rootDir: string, branchName: string, star } } +/** + * Per-`groupId` in-process promotion lock (Fix #10). `promoteBranchGroup` can be + * invoked concurrently — e.g. the dashboard route bridge and the auto-promotion + * hook firing on the final member landing — and its body runs a long await chain + * (git checkout/merge on the shared working tree + PR creation) with no atomicity. + * Interleaving two runs can double-create the managed PR and corrupt HEAD. + * + * We serialize per group by chaining each call onto a promise stored in this map; + * each call only begins after the previous one for the same group settles, and it + * RE-READS the group state inside the lock (the inner function's first action is + * `store.getBranchGroup`), so a second waiter observes the first's persisted + * `prState`/`status` and short-circuits instead of re-doing the work. + * + * In-process only: a cross-node lease (FN-4820) is explicitly deferred. + */ +const promotionLocks = new Map>(); + /** * The only entrypoint allowed to perform shared-branch-group → default-branch promotion. * Promotion is intentionally idempotent and must never run inline in aiMergeTask. + * + * Serialized per `groupId` via {@link promotionLocks}; see that comment for why. */ -export async function promoteBranchGroup(input: { +export interface PromoteBranchGroupInput { store: Pick; rootDir: string; groupId: string; @@ -194,7 +213,32 @@ export async function promoteBranchGroup(input: { target: string; metadata?: Record; }) => Promise | void; -}): Promise { +} + +export async function promoteBranchGroup(input: PromoteBranchGroupInput): Promise { + // Chain onto any in-flight promotion for this group so two concurrent callers + // (route bridge + auto-promotion on final landing) never run the merge/PR-create + // sequence at the same time. The continuation re-reads group state inside the + // lock, so the second caller observes the first's persisted result. + const prior = promotionLocks.get(input.groupId) ?? Promise.resolve(); + const run = prior + .catch(() => { + // A failed prior promotion must not poison the chain; the next caller still + // gets a fresh, serialized attempt (re-merge is a no-op; PR-create idempotent). + }) + .then(() => promoteBranchGroupInner(input)); + promotionLocks.set(input.groupId, run); + try { + return await run; + } finally { + // Only clear if no newer call has chained on top of us. + if (promotionLocks.get(input.groupId) === run) { + promotionLocks.delete(input.groupId); + } + } +} + +async function promoteBranchGroupInner(input: PromoteBranchGroupInput): Promise { const group = input.store.getBranchGroup(input.groupId); if (!group) { return { @@ -207,7 +251,21 @@ export async function promoteBranchGroup(input: { }; } - if (group.status === "finalized" || group.prState === "merged") { + const isPrMode = input.settings.mergeStrategy === "pull-request"; + + // Fix #4 (2): a group that finalized but never gained its PR — e.g. a crash + // between the local integration merge and a successful createGroupPr — would be + // permanently stranded by the already-finalized short-circuit below. When in PR + // mode and the finalized group has no persisted PR number, fall through to the + // PR-creation step ONLY (the integration merge already happened, so we skip it) + // so a re-promotion can repair it. + const needsPrRepair = + isPrMode && + group.status === "finalized" && + group.prState !== "merged" && + (group.prNumber === null || group.prNumber === undefined); + + if (!needsPrRepair && (group.status === "finalized" || group.prState === "merged")) { return { groupId: group.id, promoted: false, @@ -234,58 +292,64 @@ export async function promoteBranchGroup(input: { } const members = await input.store.listTasksByBranchGroup(group.id); - const completion = evaluateBranchGroupCompletion({ members, group }); - if (!completion.complete) { - return { - groupId: group.id, - promoted: false, - alreadyFinalized: false, - reason: "incomplete", - status: group.status, - prState: group.prState, - prNumber: group.prNumber, - prUrl: group.prUrl, - }; - } - const eligibility = evaluateBranchGroupPromotion({ group, settings: input.settings }); - if (!eligibility.eligible) { - await input.recordAudit?.({ - domain: "git", - mutationType: "merge:branch-group-promotion-gated", - target: group.id, - metadata: { + // On the PR-repair path the group is already finalized — completion and + // eligibility were satisfied at finalization, and the integration merge already + // landed. Re-gating/re-merging would be wrong, so we skip straight to PR-create. + if (!needsPrRepair) { + const completion = evaluateBranchGroupCompletion({ members, group }); + if (!completion.complete) { + return { groupId: group.id, - branchName: group.branchName, - groupAutoMerge: eligibility.groupAutoMerge, - effectiveEligible: false, - reason: eligibility.reason, - }, - }); - return { - groupId: group.id, - promoted: false, - alreadyFinalized: false, - reason: "gated", - status: group.status, - prState: group.prState, - prNumber: group.prNumber, - prUrl: group.prUrl, - }; + promoted: false, + alreadyFinalized: false, + reason: "incomplete", + status: group.status, + prState: group.prState, + prNumber: group.prNumber, + prUrl: group.prUrl, + }; + } + + const eligibility = evaluateBranchGroupPromotion({ group, settings: input.settings }); + if (!eligibility.eligible) { + await input.recordAudit?.({ + domain: "git", + mutationType: "merge:branch-group-promotion-gated", + target: group.id, + metadata: { + groupId: group.id, + branchName: group.branchName, + groupAutoMerge: eligibility.groupAutoMerge, + effectiveEligible: false, + reason: eligibility.reason, + }, + }); + return { + groupId: group.id, + promoted: false, + alreadyFinalized: false, + reason: "gated", + status: group.status, + prState: group.prState, + prNumber: group.prNumber, + prUrl: group.prUrl, + }; + } } const integrationBranch = await resolveIntegrationBranch(input.rootDir, input.settings); - await ensureGroupBranchExists(input.rootDir, group.branchName, integrationBranch); - const currentBranch = (await execAsync("git rev-parse --abbrev-ref HEAD", { cwd: input.rootDir })).stdout.trim(); - try { - await execAsync(`git checkout ${JSON.stringify(integrationBranch)}`, { cwd: input.rootDir }); - await execAsync(`git merge --no-ff --no-edit ${JSON.stringify(group.branchName)}`, { cwd: input.rootDir }); - } finally { - await execAsync(`git checkout ${JSON.stringify(currentBranch)}`, { cwd: input.rootDir }); + if (!needsPrRepair) { + await ensureGroupBranchExists(input.rootDir, group.branchName, integrationBranch); + const currentBranch = (await execAsync("git rev-parse --abbrev-ref HEAD", { cwd: input.rootDir })).stdout.trim(); + try { + await execAsync(`git checkout ${JSON.stringify(integrationBranch)}`, { cwd: input.rootDir }); + await execAsync(`git merge --no-ff --no-edit ${JSON.stringify(group.branchName)}`, { cwd: input.rootDir }); + } finally { + await execAsync(`git checkout ${JSON.stringify(currentBranch)}`, { cwd: input.rootDir }); + } } - 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"; @@ -342,7 +406,7 @@ export async function promoteBranchGroup(input: { groupId: group.id, branchName: group.branchName, integrationBranch, - memberIds: completion.landedMemberIds, + memberIds: evaluateBranchGroupCompletion({ members, group }).landedMemberIds, ...(updatedGroup.prNumber ? { prNumber: updatedGroup.prNumber } : {}), ...(updatedGroup.prUrl ? { prUrl: updatedGroup.prUrl } : {}), }, @@ -360,6 +424,68 @@ export async function promoteBranchGroup(input: { }; } +export interface ReconcileBranchGroupPrResult { + reconciled: boolean; + prState: BranchGroupPrState; + prNumber: number | null; + prUrl: string | null; +} + +/** + * Fix #3 (engine side): out-of-band PR reconciliation primitive. + * + * Once a branch group finalizes, the member-landing sync stops firing, so nothing + * flips `prState` → "merged" after the managed GitHub PR is merged out-of-band. + * This helper, given a group carrying a persisted `prNumber` and `prState` "open", + * invokes the injected {@link SyncGroupPrFn} (which reconciles against GitHub via + * `getPrStatus`) and persists `prState`/`prUrl`/`prNumber` when GitHub reports a + * changed state. It mirrors the merger's U6 reconcile block. + * + * No-op (no write) when the group has no `prNumber`, is not "open", or GitHub still + * reports it open. The dashboard route that calls this on a schedule/refresh is + * wired in a separate batch; this is just the cleanly exported engine primitive. + */ +export async function reconcileBranchGroupPr(input: { + store: Pick; + group: BranchGroup; + syncGroupPr: SyncGroupPrFn; +}): Promise { + const { group } = input; + if (group.prNumber == null || group.prState !== "open") { + return { + reconciled: false, + prState: group.prState, + prNumber: group.prNumber ?? null, + prUrl: group.prUrl ?? null, + }; + } + + const members = await input.store.listTasksByBranchGroup(group.id); + const reconciled = await input.syncGroupPr({ group, members }); + + if (reconciled.prState === group.prState) { + return { + reconciled: false, + prState: group.prState, + prNumber: group.prNumber ?? null, + prUrl: group.prUrl ?? null, + }; + } + + const updated = input.store.updateBranchGroup(group.id, { + prState: reconciled.prState, + prNumber: reconciled.prNumber, + prUrl: reconciled.prUrl, + }); + + return { + reconciled: true, + prState: updated.prState, + prNumber: updated.prNumber ?? null, + prUrl: updated.prUrl ?? null, + }; +} + export async function resolveBranchGroupMergeRouting(input: { task: Pick; store: Pick; diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 7e19eaf2cc..fa187576c7 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -61,10 +61,13 @@ export { evaluateBranchGroupPromotion, evaluateBranchGroupCompletion, promoteBranchGroup, + reconcileBranchGroupPr, type BranchGroupMergeRouting, type BranchGroupPromotionDecision, type BranchGroupCompletionStatus, type BranchGroupPromotionResult, + type PromoteBranchGroupInput, + type ReconcileBranchGroupPrResult, type CreateGroupPrFn, type SyncGroupPrFn, type CloseGroupPrFn, diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index c5e9613d24..20beb9d7f9 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -82,7 +82,6 @@ import { type PostMergeAuditMode, type TaskSourceIssue, type Task, - type BranchGroup, type AutostashOrphanRecord, normalizeMergeAdvanceAutoSyncMode, isMergeRequestContractShadowEnabled, @@ -7529,34 +7528,28 @@ export async function aiMergeTask( // non-fatal and retryable on the next landing / explicit refresh. if (options.syncGroupPr) { try { - const latestGroup = await Promise.resolve( - (store as any).getBranchGroup?.(groupRouting.branchGroup.id), - ) as BranchGroup | null | undefined; + const latestGroup = store.getBranchGroup(groupRouting.branchGroup.id); if (latestGroup && latestGroup.prNumber != null && latestGroup.prState === "open") { - const members = (await Promise.resolve( - (store as any).listTasksByBranchGroup?.(latestGroup.id), - )) as Task[] | undefined; + const members = await store.listTasksByBranchGroup(latestGroup.id); const reconciled = await options.syncGroupPr({ group: latestGroup, - members: members ?? [], + members, }); // Out-of-band reconciliation: if GitHub reports the PR is no longer // open (closed/merged), persist the corrected prState rather than // leaving a stale "open". if (reconciled.prState !== latestGroup.prState) { - await Promise.resolve( - (store as any).updateBranchGroup?.(latestGroup.id, { - prState: reconciled.prState, - prNumber: reconciled.prNumber, - prUrl: reconciled.prUrl, - }), - ); + store.updateBranchGroup(latestGroup.id, { + prState: reconciled.prState, + prNumber: reconciled.prNumber, + prUrl: reconciled.prUrl, + }); } } } catch (err) { // Non-fatal: never fail the merge/landing because PR sync failed. try { - await (store as any).recordRunAuditEvent?.({ + store.recordRunAuditEvent({ taskId, agentId: "merger", runId: `merge-${taskId}`, diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index 1f1d3a5365..3918bd19e0 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -1921,9 +1921,33 @@ export class ProjectEngine { }, }); } catch (promotionError) { + const message = + promotionError instanceof Error ? promotionError.message : String(promotionError); runtimeLog.warn( - `Branch-group promotion evaluation failed for ${taskId}: ${promotionError instanceof Error ? promotionError.message : String(promotionError)}`, + `Branch-group promotion evaluation failed for ${taskId}: ${message}`, ); + // Fix #4 (1): a promotion failure here (e.g. createGroupPr throwing + // after the local integration merge) must NOT be swallowed silently — + // the group stays active/prState:none and is only recoverable via an + // explicit re-promote. Record an audit event so the failure is + // observable and operators/the dashboard can drive recovery. + try { + await store.recordRunAuditEvent({ + taskId, + agentId: "merger", + runId: `merge-${taskId}`, + domain: "git", + mutationType: "merge:branch-group-promotion-failed", + target: taskForPromotion.branchContext!.groupId, + metadata: { + groupId: taskForPromotion.branchContext!.groupId, + taskId, + error: message, + }, + }); + } catch { + // best-effort audit + } } };