fix(FN-branch-group): promotion lock, PR repair, audit on failure, typed sync block
Review residuals #3/#4/#6/#10: per-group in-process promotion lock (concurrent route+auto promotion could double-create PRs), finalized-but-PR-less groups can be repaired by re-promotion without re-merging, auto-promotion failures emit merge:branch-group-promotion-failed instead of silent swallow, exported reconcileBranchGroupPr for out-of-band merged reconciliation, and the merger sync block drops its (store as any) casts (TaskStore already carries the methods).
This commit is contained in:
@@ -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>): 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<string, unknown>) => {
|
||||
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>): 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<string, unknown>) => {
|
||||
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>): 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<Record<string, unknown>> = [];
|
||||
const store = {
|
||||
listTasksByBranchGroup: async () => [{ id: "FN-A" }],
|
||||
updateBranchGroup: (_id: string, patch: Record<string, unknown>) => {
|
||||
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({
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<string, Promise<unknown>>();
|
||||
|
||||
/**
|
||||
* 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<TaskStore, "getBranchGroup" | "getBranchGroupByBranchName" | "listTasksByBranchGroup" | "updateBranchGroup">;
|
||||
rootDir: string;
|
||||
groupId: string;
|
||||
@@ -194,7 +213,32 @@ export async function promoteBranchGroup(input: {
|
||||
target: string;
|
||||
metadata?: Record<string, unknown>;
|
||||
}) => Promise<void> | void;
|
||||
}): Promise<BranchGroupPromotionResult> {
|
||||
}
|
||||
|
||||
export async function promoteBranchGroup(input: PromoteBranchGroupInput): Promise<BranchGroupPromotionResult> {
|
||||
// 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<BranchGroupPromotionResult> {
|
||||
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<TaskStore, "listTasksByBranchGroup" | "updateBranchGroup">;
|
||||
group: BranchGroup;
|
||||
syncGroupPr: SyncGroupPrFn;
|
||||
}): Promise<ReconcileBranchGroupPrResult> {
|
||||
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<Task, "branchContext" | "baseBranch">;
|
||||
store: Pick<TaskStore, "getBranchGroup">;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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}`,
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user