fix(engine): preserve manual handoff holds during recovery (#3422)
## Summary Supersedes #3421 — fork head not writable for main-merge. Same change rebased onto current main so lifecycle-column lint stays green. ## Test plan - [x] merges cleanly onto main - [ ] CI green Co-authored-by: BESA-Franz <49682134+BESA-Franz@users.noreply.github.com>
This commit is contained in:
7
.changeset/manual-handoff-limbo.md
Normal file
7
.changeset/manual-handoff-limbo.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Keep completed manual-review tasks parked instead of retrying automatic merge handoffs.
|
||||
category: fix
|
||||
dev: Reuses the shared merge-recovery consent gate for completion-handoff self-healing.
|
||||
@@ -27,6 +27,7 @@ function createStore(task: Task, mergeQueuedTaskIds: string[] = []) {
|
||||
return {
|
||||
getSettings: vi.fn(async () => ({ globalPause: false, enginePaused: false })),
|
||||
listTasks: vi.fn(async () => [current]),
|
||||
getBranchGroup: vi.fn(async () => null),
|
||||
updateTask: vi.fn(async (_id: string, updates: Partial<Task>) => {
|
||||
current = { ...current, ...updates } as Task;
|
||||
return current;
|
||||
@@ -104,6 +105,69 @@ describe("FN-4999 reliability interactions: completion-handoff-limbo", () => {
|
||||
expect(requeueForAutoMerge).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// FNXC:CompletionHandoffRecovery 2026-08-11-12:05: Manual holds suppress recovery unless a permitted live shared group owns the integration path.
|
||||
it.each([
|
||||
{
|
||||
label: "task-level user hold while project auto-merge is on",
|
||||
task: { autoMerge: false, autoMergeProvenance: "user" as const },
|
||||
projectAutoMerge: true,
|
||||
},
|
||||
{
|
||||
label: "standalone mission-policy hold while project auto-merge is on",
|
||||
task: { autoMerge: false, autoMergeProvenance: "mission" as const },
|
||||
projectAutoMerge: true,
|
||||
},
|
||||
{
|
||||
label: "inherited hold while project auto-merge is off",
|
||||
task: {},
|
||||
projectAutoMerge: false,
|
||||
},
|
||||
])("preserves $label instead of recreating merge work", async ({ task, projectAutoMerge }) => {
|
||||
const store = createStore(limboTask(task));
|
||||
store.getSettings.mockResolvedValue({
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
autoMerge: projectAutoMerge,
|
||||
integrationBranch: "main",
|
||||
});
|
||||
const requeueForAutoMerge = vi.fn(() => true);
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/repo", requeueForAutoMerge });
|
||||
|
||||
await manager.recoverCompletionHandoffLimbo();
|
||||
|
||||
expect(requeueForAutoMerge).not.toHaveBeenCalled();
|
||||
expect(store.enqueueMergeQueue).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.recordRunAuditEvent).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps a permitted live shared-group member recovery flowing", async () => {
|
||||
const store = createStore(limboTask({
|
||||
autoMerge: false,
|
||||
autoMergeProvenance: "mission",
|
||||
branchContext: { assignmentMode: "shared", groupId: "BG-4999", source: "mission" },
|
||||
}));
|
||||
store.getSettings.mockResolvedValue({
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
autoMerge: true,
|
||||
integrationBranch: "main",
|
||||
});
|
||||
store.getBranchGroup.mockResolvedValue({
|
||||
id: "BG-4999",
|
||||
status: "open",
|
||||
branchName: "mission/M-4999",
|
||||
});
|
||||
const requeueForAutoMerge = vi.fn(() => true);
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/repo", requeueForAutoMerge });
|
||||
|
||||
await manager.recoverCompletionHandoffLimbo();
|
||||
|
||||
expect(store.getBranchGroup).toHaveBeenCalledWith("BG-4999");
|
||||
expect(store.enqueueMergeQueue).toHaveBeenCalledWith("FN-4999-T");
|
||||
expect(requeueForAutoMerge).toHaveBeenCalledWith("FN-4999-T");
|
||||
});
|
||||
|
||||
it("honors legitimate merge blockers", async () => {
|
||||
const requeueForAutoMerge = vi.fn(() => true);
|
||||
const store = createStore(limboTask({ status: "failed" }));
|
||||
@@ -191,12 +255,21 @@ describe("FN-4999 reliability interactions: completion-handoff-limbo", () => {
|
||||
expect(store.logEntry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// FNXC:CompletionHandoffRecovery 2026-08-11-12:05: Merge-queue ownership clears false exhaustion before auto-merge admission is evaluated.
|
||||
it("clears false handoff exhaustion for tasks already held by the merge queue", async () => {
|
||||
const store = createStore(limboTask({
|
||||
status: "failed",
|
||||
error: "Completion handoff limbo recovery exhausted",
|
||||
completionHandoffLimboRecoveryCount: MAX_COMPLETION_HANDOFF_LIMBO_RECOVERIES,
|
||||
autoMerge: false,
|
||||
autoMergeProvenance: "user",
|
||||
}), ["FN-4999-T"]);
|
||||
store.getSettings.mockResolvedValue({
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
autoMerge: true,
|
||||
integrationBranch: "main",
|
||||
});
|
||||
const requeueForAutoMerge = vi.fn(() => false);
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/repo", requeueForAutoMerge });
|
||||
|
||||
|
||||
@@ -8663,6 +8663,29 @@ const movedTask = await this.store.moveTask(task.id, completeLane);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:MergeRecoveryConsent 2026-08-11-11:46:
|
||||
* Every self-healing path that can create merge work must preserve the same
|
||||
* operator consent boundary. Standalone effective Off tasks remain parked for
|
||||
* manual integration, while a live shared-group member may still advance only
|
||||
* when project/task policy permits its intermediate member-to-group merge.
|
||||
*/
|
||||
private async canRecoverMergeRequest(task: Task, settings: Settings): Promise<boolean> {
|
||||
if (hasSharedBranchMemberAutoMergeHold(task, settings)) return false;
|
||||
|
||||
const groupId = task.branchContext?.groupId?.trim();
|
||||
if (groupId) {
|
||||
const branchGroup = await this.store.getBranchGroup(groupId);
|
||||
const projectDefaultBranch = await resolveIntegrationBranch(this.options.rootDir, settings);
|
||||
if (isLiveSharedBranchGroupMemberIntegration(task, branchGroup, projectDefaultBranch)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return allowsAutoMergeProcessing(task, settings)
|
||||
&& resolveEffectiveAutoMerge(task, settings) !== false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover `in-review` tasks that are fully mergeable but never had
|
||||
* `mergeTask()` invoked.
|
||||
@@ -8731,38 +8754,9 @@ const movedTask = await this.store.moveTask(task.id, completeLane);
|
||||
*/
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
|
||||
/*
|
||||
FNXC:SharedBranchMemberHold 2026-08-05-23:14:
|
||||
Recovery is a merge requester, not merely cleanup. It must use the same
|
||||
member→group admission rule as ProjectEngine: an open intermediate group
|
||||
keeps mission-policy/inherited Off flowing, while an operator-authored Off
|
||||
remains a durable manual hold. Filtering only with allowsAutoMergeProcessing
|
||||
stranded mission members whenever the project switch was Off and, conversely,
|
||||
would enqueue a user hold when the project switch was On.
|
||||
*/
|
||||
const canRecoverMergeableReviewTask = async (task: Task): Promise<boolean> => {
|
||||
if (hasSharedBranchMemberAutoMergeHold(task, settings)) return false;
|
||||
|
||||
const groupId = task.branchContext?.groupId?.trim();
|
||||
const branchGroup = groupId ? await this.store.getBranchGroup(groupId) : null;
|
||||
const projectDefaultBranch = await resolveIntegrationBranch(this.options.rootDir, settings);
|
||||
if (isLiveSharedBranchGroupMemberIntegration(task, branchGroup, projectDefaultBranch)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:SharedBranchMemberHold 2026-08-05-23:22:
|
||||
A stale or default-branch group is not an intermediate member integration.
|
||||
Its false task value must retain the standalone manual-hold path even when
|
||||
the project switch is On; recovery must not turn that durable hold into a
|
||||
fresh merge request merely because it runs after the graph paused.
|
||||
*/
|
||||
return allowsAutoMergeProcessing(task, settings)
|
||||
&& resolveEffectiveAutoMerge(task, settings) !== false;
|
||||
};
|
||||
const mergeAdmission = await Promise.all(tasks.map(async (task) => [
|
||||
task.id,
|
||||
await canRecoverMergeableReviewTask(task),
|
||||
await this.canRecoverMergeRequest(task, settings),
|
||||
] as const));
|
||||
const mergeAdmissionByTaskId = new Map(mergeAdmission);
|
||||
const mergeable = tasks.filter((t) =>
|
||||
@@ -12260,7 +12254,6 @@ const movedTask = await this.store.moveTask(task.id, completeLane);
|
||||
if (task.paused) continue;
|
||||
const limboReviewLanes = await ownLimboReviewLanes(task);
|
||||
if (!limboReviewLanes.has(task.column)) continue;
|
||||
if (!allowsAutoMergeProcessing(task, settings)) continue;
|
||||
if (await this.isFalseCompletionHandoffExhaustionWhileMergeOwned(task)) {
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
@@ -12273,6 +12266,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane);
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if (!(await this.canRecoverMergeRequest(task, settings))) continue;
|
||||
if (task.status != null || task.mergeDetails != null || task.review != null || task.reviewState != null) continue;
|
||||
if (this.options.isTaskActive?.(task.id)) continue;
|
||||
if (await this.isMergeLaneOwned(task.id)) continue;
|
||||
|
||||
Reference in New Issue
Block a user