root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FN-8910: allow held tasks to remediate review findings

Browse Source 
Allow pre-merge remediation to proceed under project-level merge holds while preserving task-level operator holds.

- Restrict remediation holds to operator-authored task-level auto-merge settings
- Record remediation and revision-budget refusals for parked review tasks
- Keep fire-and-forget remediation failures in their review lane
- Cover shared-branch recovery behavior and document the policy

Files changed:
 .changeset/fn-8910-premerge-remediation-hold.md    |  7 ++
 docs/workflow-steps.md                             |  2 +
 packages/core/src/__tests__/task-merge.test.ts     | 44 ++++++-------
 packages/core/src/merge/task-merge.ts              | 20 +++---
 .../__tests__/executor-graph-requeue-gate.test.ts  | 60 ++++++++++++++++-
 ...cutor-live-branch-group-auto-merge-hold.test.ts | 63 ++++++++++++++++--
 .../workflow-graph-optional-step-fix.test.ts       | 24 +++++--
 .../src/__tests__/workflow-task-runtime.test.ts    | 10 ++-
 packages/engine/src/executor.ts                    | 76 ++++++++++++++++++----
 9 files changed, 244 insertions(+), 62 deletions(-)

Fusion-Task-Id: FN-8910

Fusion-Task-Lineage: b5e10f87-67cf-4acf-b125-91be5ade17a8

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-08-09 15:16:12 -07:00
parent 6bd178bdcf
commit 3aa32a846c
9 changed files with 243 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/fn-8910-premerge-remediation-hold.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Review REVISE now schedules fixes for shared-branch tasks in auto-merge-off projects.
category: fix
dev: Narrows hasPreMergeRemediationAutoMergeHold; logs both executor recovery-budget refusals and guards the resume router.

2
docs/workflow-steps.md
View File

@@ -757,6 +757,8 @@ Authoritative cutover now depends on existing/current parity summary evidence, n
If a task is found in `in-review` with failed pre-merge workflow results and no active executor, self-healing can auto-revive it by replaying the same remediation send-back flow. Generic optional gates use the resolved workflow/project budget; built-in Plan Review and most Code Review groups are unbounded unless workflow settings or node config set a numeric cap. Compound Engineering's Code Review node supplies a two-pass cap.
Project-level `autoMerge: false` gates **merge admission**, not pre-merge remediation: Plan Review replans, Code Review/optional-gate fixes, required-artifact recovery, and failed-step revival remain available for shared-branch members and standalone tasks. Only an operator-authored task-level auto-merge Off fences those remediation seams. Every such hold or revision-budget refusal is recorded on the task; when a fire-and-forget remediation node cannot schedule after implementation is complete, the card stays visibly parked in its resolved review lane with its merge blocker rather than bouncing back to planning.
<!--
FNXC:WorkflowOptionalStepFix 2026-06-26-17:05:
Enabled PRE-merge optional-group REVISE findings should be acted on before review/merge when the executor is still in the graph run. The inline path consumes the same `postReviewFixCount` / `maxPostReviewFixes` budget before scheduling `sendTaskBackForFix`; exhausted budgets preserve the older advisory/gate behavior so optional advisory gates remain ultimately non-blocking.

42
packages/core/src/__tests__/task-merge.test.ts
View File

@@ -103,37 +103,31 @@ describe("hasSharedBranchMemberAutoMergeHold", () => {
describe("hasPreMergeRemediationAutoMergeHold", () => {
const taskValues = [undefined, true, false] as const;
const provenances = [undefined, "user", "mission", "legacy-stamp"] as const;
const branchContexts = [
undefined,
{ assignmentMode: "shared" as const, groupId: "BG-1" },
{ assignmentMode: "shared" as const, groupId: "" },
{ assignmentMode: "shared" as const, groupId: " " },
{ assignmentMode: "per-task-derived" as const },
];
it.each([false, true] as const)("uses the user hold for standalone tasks when project autoMerge is %s", (projectAutoMerge) => {
it.each([false, true] as const)("uses only the user task hold across branch contexts when project autoMerge is %s", (projectAutoMerge) => {
for (const autoMerge of taskValues) {
for (const autoMergeProvenance of provenances) {
expect(hasPreMergeRemediationAutoMergeHold(
{ autoMerge, autoMergeProvenance },
{ autoMerge: projectAutoMerge },
)).toBe(autoMerge === false && autoMergeProvenance === "user");
for (const branchContext of branchContexts) {
expect(hasPreMergeRemediationAutoMergeHold(
{ autoMerge, autoMergeProvenance, branchContext },
{ autoMerge: projectAutoMerge },
)).toBe(autoMerge === false && autoMergeProvenance === "user");
}
}
}
});
it.each([false, true] as const)("matches the shared-member hold when project autoMerge is %s", (projectAutoMerge) => {
for (const autoMerge of taskValues) {
for (const autoMergeProvenance of provenances) {
const task = {
autoMerge,
autoMergeProvenance,
branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" },
};
expect(hasPreMergeRemediationAutoMergeHold(task, { autoMerge: projectAutoMerge }))
.toBe(hasSharedBranchMemberAutoMergeHold(task, { autoMerge: projectAutoMerge }));
}
}
});
it.each(["", " "])("treats a blank shared group id as standalone (%j)", (groupId) => {
expect(hasPreMergeRemediationAutoMergeHold({
autoMerge: undefined,
branchContext: { assignmentMode: "shared", groupId },
}, { autoMerge: false })).toBe(false);
it("diverges from merge admission for a project-Off shared member", () => {
const task = { autoMerge: undefined, branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" } };
expect(hasPreMergeRemediationAutoMergeHold(task, { autoMerge: false })).toBe(false);
expect(hasSharedBranchMemberAutoMergeHold(task, { autoMerge: false })).toBe(true);
});
});

20
packages/core/src/merge/task-merge.ts
View File

@@ -126,21 +126,19 @@ export function hasUserAutoMergeHold(
}
/**
* FNXC:SharedBranchMemberHold 2026-08-09-06:11:
* FN-8863 narrows FN-8823's project-Off consent rule to shared members at
* pre-merge remediation seams. Applying it to standalone tasks disabled Plan
* Review replans, required-artifact recovery, provider-failure diagnostics,
* and Code Review fix handoffs in every auto-merge-Off project. Remediation
* reopens implementation rather than merging, so standalone tasks are fenced
* only by an operator-authored task-level Off.
* FNXC:SharedBranchMemberHold 2026-08-09-21:41:
* FN-8910 narrows the FN-8823 project-Off consent arm after FN-8863 exposed
* that it also reached remediation. Remediation reopens implementation; it
* never merges. The broad shared-member hold remains the merge-admission
* contract in merge-runner, project-engine, and self-healing, so only an
* operator-authored task-level Off may fence shared and standalone remediation.
* This preserves the merge checkpoint while allowing review findings to be fixed.
*/
export function hasPreMergeRemediationAutoMergeHold(
task: Pick<Task, "autoMerge" | "autoMergeProvenance" | "branchContext">,
settings: Pick<Settings, "autoMerge">,
_settings: Pick<Settings, "autoMerge">,
): boolean {
return isSharedBranchGroupMemberIntegration(task)
? hasSharedBranchMemberAutoMergeHold(task, settings)
: hasUserAutoMergeHold(task);
return hasUserAutoMergeHold(task);
}
/**

60
packages/engine/src/__tests__/executor-graph-requeue-gate.test.ts
View File

@@ -333,8 +333,64 @@ describe("executor graph execute self-requeue gate", () => {
expect.anything(),
expect.anything(),
);
expect(store.updateTask).toHaveBeenCalledWith(live.id, { status: null, error: null }, undefined);
expect(store.updateTask).toHaveBeenCalledWith(live.id, { workflowStepResults: [] }, undefined);
expect(store.updateTask).not.toHaveBeenCalledWith(live.id, { status: null, error: null }, undefined);
expect(store.updateTask).not.toHaveBeenCalledWith(live.id, { workflowStepResults: [] }, undefined);
});
it.each(["remediation-not-scheduled", "missing-remediation-context"])("FN-8910 keeps a completed review card in place when remediation reports %s", async (failureValue) => {
resetExecutorMocks();
const store = createMockStore();
const live = task({
id: `FN-8910-${failureValue}`,
column: "in-review",
steps: [{ name: "Implement", status: "done" }],
});
store.getTask.mockResolvedValue(live);
const executor = new TaskExecutor(store, "/tmp/test");
await expect((executor as any).routeGraphFailureToExecutionResume(
live,
"code-review-remediation",
failureValue,
)).resolves.toBe(false);
await (executor as any).handleGraphFailure(live, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["code-review", "code-review-remediation"],
context: { "node:code-review-remediation:value": failureValue },
});
/*
FNXC:WorkflowRemediation 2026-08-09-21:53:
FN-8910 requires policy-refused remediation to remain visibly parked in the
review lane after implementation is complete, never rebound to execution.
*/
expect(store.moveTask).not.toHaveBeenCalled();
});
it("still resumes an incomplete review card after a refused remediation", async () => {
resetExecutorMocks();
const store = createMockStore();
const live = task({
id: "FN-8910-INCOMPLETE",
column: "in-review",
steps: [{ name: "Implement", status: "pending" }],
});
store.getTask.mockResolvedValue(live);
const executor = new TaskExecutor(store, "/tmp/test");
await expect((executor as any).routeGraphFailureToExecutionResume(
live,
"code-review-remediation",
"remediation-not-scheduled",
)).resolves.toBe(true);
expect(store.moveTask).toHaveBeenCalledWith(
live.id,
"todo",
expect.objectContaining({ preserveProgress: true, recoveryRehome: true }),
);
});
it("still parks a remediation-node graph failure as failed when no durable failed gate result exists", async () => {

63
packages/engine/src/__tests__/executor-live-branch-group-auto-merge-hold.test.ts
View File

@@ -135,7 +135,7 @@ describe("executor shared-branch autoMerge:false liveness gates", () => {
)).resolves.toBe(true);
});
it("holds an unset shared member at both pre-merge remediation seams when project auto-merge is Off", async () => {
it("FN-8910 reopens an unset project-Off shared member at live and failed-step remediation seams", async () => {
const { executor, store } = makeExecutor({ status: "open", branchName: "mission/M-1980" });
const task = makeInReviewTask({
workflowStepResults: [{
@@ -156,10 +156,51 @@ describe("executor shared-branch autoMerge:false liveness gates", () => {
status: "failed",
verdict: "REVISE",
nodeId: "code-review",
})).resolves.toBe(false);
await expect(executor.recoverFailedPreMergeWorkflowStep(task)).resolves.toBe(false);
})).resolves.toBe(true);
await expect(executor.recoverFailedPreMergeWorkflowStep(task)).resolves.toBe(true);
expect(sendBack).not.toHaveBeenCalled();
expect(sendBack).toHaveBeenCalledTimes(2);
});
it("FN-8910 replans Plan Review for an unset project-Off shared member", async () => {
const { executor, store } = makeExecutor({ status: "open", branchName: "mission/M-1980" });
const task = makeInReviewTask({ column: "in-progress", status: null });
store.getTask.mockResolvedValue(task);
await expect((executor as any).requestPreMergeOptionalStepFix(task.id, task, {
stepName: "Plan Review",
feedback: "Revise the task specification.",
phase: "pre-merge",
status: "failed",
verdict: "REVISE",
nodeId: "plan-review",
})).resolves.toBe(true);
expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", { preserveWorktree: true });
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({ status: "needs-replan" }), undefined);
});
it("FN-8910 routes retryable remediation failure through the reopened shared-member seam", async () => {
const { executor, store } = makeExecutor({ status: "open", branchName: "mission/M-1980" });
const task = makeInReviewTask({
workflowStepResults: [{
workflowStepId: "code-review",
workflowStepName: "Code Review",
phase: "pre-merge",
status: "failed",
output: "Please revise",
}],
});
store.getTask.mockResolvedValue(task);
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
await expect((executor as any).routeRetryableRemediationGraphFailureToPreMergeFix(
task,
"code-review-remediation",
"remediation-not-scheduled",
)).resolves.toBe(true);
expect(sendBack).toHaveBeenCalledOnce();
});
it("does not let live pre-merge remediation reopen an operator-held member", async () => {
@@ -178,10 +219,16 @@ describe("executor shared-branch autoMerge:false liveness gates", () => {
})).resolves.toBe(false);
expect(sendBack).not.toHaveBeenCalled();
expect(store.logEntry).toHaveBeenCalledWith(
task.id,
expect.stringContaining("operator task hold"),
expect.stringContaining("operator-authored task-level auto-merge Off"),
undefined,
);
});
it("does not let failed-step recovery reopen an operator-held member", async () => {
const { executor } = makeExecutor({ status: "open", branchName: "mission/M-1980" });
const { executor, store } = makeExecutor({ status: "open", branchName: "mission/M-1980" });
const task = makeInReviewTask({
autoMerge: false,
autoMergeProvenance: "user",
@@ -198,6 +245,12 @@ describe("executor shared-branch autoMerge:false liveness gates", () => {
await expect(executor.recoverFailedPreMergeWorkflowStep(task)).resolves.toBe(false);
expect(sendBack).not.toHaveBeenCalled();
expect(store.logEntry).toHaveBeenCalledWith(
task.id,
expect.stringContaining("operator task hold"),
expect.stringContaining("operator-authored task-level auto-merge Off"),
undefined,
);
});
it("holds an open shared group that would integrate directly into main", async () => {

24
packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts
View File

@@ -388,9 +388,8 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
expect(store.updateTask).toHaveBeenCalledWith(liveTask.id, expect.objectContaining({ status: "needs-replan" }), undefined);
});
it("holds shared members and user-held standalone tasks during project-Off Plan Review remediation", async () => {
it("holds only user-held tasks during project-Off Plan Review remediation", async () => {
for (const overrides of [
{ branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" } },
{ autoMerge: false, autoMergeProvenance: "user" as const },
]) {
const store = createMockStore();
@@ -409,6 +408,12 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
expect(store.moveTask).not.toHaveBeenCalled();
expect(store.updateTask).not.toHaveBeenCalled();
expect(store.logEntry).toHaveBeenCalledWith(
liveTask.id,
expect.stringContaining("operator task hold"),
expect.stringContaining("operator-authored task-level auto-merge Off"),
undefined,
);
}
});
@@ -826,11 +831,16 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(true);
expect(sendBack).toHaveBeenCalledOnce();
expect(store.logEntry).not.toHaveBeenCalledWith(
liveTask.id,
expect.stringContaining("recovery not scheduled — revision budget"),
expect.anything(),
undefined,
);
});
it("does not recover shared members or user-held standalone tasks during project-Off failed-step recovery", async () => {
it("does not recover user-held tasks during project-Off failed-step recovery", async () => {
for (const overrides of [
{ branchContext: { assignmentMode: "shared" as const, groupId: "BG-1" } },
{ autoMerge: false, autoMergeProvenance: "user" as const },
]) {
const store = createMockStore();
@@ -916,6 +926,12 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(false);
expect(sendBack).not.toHaveBeenCalled();
expect(store.logEntry).toHaveBeenCalledWith(
liveTask.id,
expect.stringContaining(codeReviewMaxRevisions === 0 ? "zero/invalid" : "exhausted"),
expect.stringContaining(`Attempts: ${attempts}\nMax: ${codeReviewMaxRevisions}`),
undefined,
);
});
it("writes an unbounded retry label into Code Review remediation instructions", async () => {

10
packages/engine/src/__tests__/workflow-task-runtime.test.ts
View File

@@ -716,7 +716,7 @@ describe("WorkflowTaskRuntime", () => {
]);
});
it("routes merge-gate work items off when task auto-merge is disabled", async () => {
it("FN-8910 routes a project-Off shared member to manual merge hold", async () => {
const transitions: Array<{ id: string; state: WorkflowWorkItemState; patch?: Record<string, unknown> }> = [];
const workItem = {
id: "work-merge-gate",
@@ -736,7 +736,11 @@ describe("WorkflowTaskRuntime", () => {
} satisfies WorkflowWorkItem;
const runtime = new WorkflowTaskRuntime({
store: {
getTask: async () => ({ ...task, autoMerge: false } as TaskDetail),
getTask: async () => ({
...task,
autoMerge: undefined,
branchContext: { assignmentMode: "shared", groupId: "BG-8910" },
} as TaskDetail),
getTaskWorkflowSelection: () => undefined,
getWorkflowDefinition: async () => undefined,
transitionWorkflowWorkItem: (id, state, patch) => {
@@ -748,7 +752,7 @@ describe("WorkflowTaskRuntime", () => {
runCustomNode: async () => ({ outcome: "success" }),
});
const result = await runtime.runWorkItem(workItem, { ...flagOff, autoMerge: true } as Settings);
const result = await runtime.runWorkItem(workItem, { ...flagOff, autoMerge: false } as Settings);
expect(result.disposition).toBe("completed");
expect(result.context["node:merge-gate:value"]).toBe("auto-off");

76
packages/engine/src/executor.ts
View File

@@ -6020,12 +6020,22 @@ export class TaskExecutor {
* implementation and thereby bypass that checkpoint before the operator
* releases or revises the held member.
*
* FNXC:SharedBranchMemberHold 2026-08-09-06:11:
* FN-8863 confines FN-8823's project-Off consent arm to shared members at
* this remediation seam. Standalone remediation reopens implementation,
* not a merge, so only an operator-authored task Off holds it.
* FNXC:SharedBranchMemberHold 2026-08-09-21:41:
* FN-8910: remediation reopens implementation rather than merging. The
* merge boundary independently enforces project Off, so this seam fences
* only an operator-authored task-level Off and records every refusal.
*/
if (hasPreMergeRemediationAutoMergeHold(liveTask, await this.store.getSettings())) return false;
if (hasPreMergeRemediationAutoMergeHold(liveTask, await this.store.getSettings())) {
const reason = "operator-authored task-level auto-merge Off holds pre-merge remediation";
executorLog.warn(`${taskId}: pre-merge remediation NOT scheduled for step "${info.stepName}" — ${reason}. Card left parked.`);
await this.store.logEntry(
taskId,
"Pre-merge remediation not scheduled — operator task hold",
`Step/node: ${info.nodeId ?? info.stepName}\nReason: ${reason}`,
this.getRunContextFor(taskId),
);
return false;
}
const missingArtifactKeys = parseRequiredArtifactMissingValue(info.failureValue);
if (missingArtifactKeys) {
await this.recoverMissingRequiredArtifacts(liveTask, missingArtifactKeys, {
@@ -6351,12 +6361,22 @@ export class TaskExecutor {
* Do not let it send a user-held member back to execution: only an explicit
* operator release or revision may advance that manual checkpoint.
*
* FNXC:SharedBranchMemberHold 2026-08-09-06:11:
* FN-8863 keeps the project-Off consent hold for shared members while
* allowing standalone failed-step recovery unless the operator authored a
* task-level Off; recovery reopens implementation and never merges.
* FNXC:SharedBranchMemberHold 2026-08-09-21:41:
* FN-8910: recovery reopens implementation rather than merging. Project
* Off remains enforced at merge admission; only an operator task Off
* fences this seam, and a refusal must be visible to the operator.
*/
if (hasPreMergeRemediationAutoMergeHold(task, await this.store.getSettings())) return false;
if (hasPreMergeRemediationAutoMergeHold(task, await this.store.getSettings())) {
const reason = "operator-authored task-level auto-merge Off holds failed-step recovery";
executorLog.warn(`${task.id}: failed pre-merge step recovery NOT scheduled — ${reason}. Card left parked.`);
await this.store.logEntry(
task.id,
"Failed pre-merge step recovery not scheduled — operator task hold",
`Reason: ${reason}`,
this.getRunContextFor(task.id),
);
return false;
}
/*
FNXC:WorkflowPostMerge 2026-06-26-14:00:
U7c: gate-ness is now sourced from the recorded `WorkflowStepResult.status`, NOT a
@@ -6393,9 +6413,31 @@ export class TaskExecutor {
* unlimited, while zero or an exhausted explicit cap cannot silently send
* work back for another fix. Progress-loop termination stays owned by the
* graph executor's signature guard rather than this budget check.
*
* FNXC:WorkflowRevisionBudget 2026-08-09-21:41:
* FN-8910: recovery-budget refusals park a card with no new session, so
* they must log their concrete attempt/max values before returning false.
*/
if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) return false;
if (!budget.unbounded && budget.attempts >= budget.max) return false;
if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) {
executorLog.warn(`${task.id}: failed pre-merge step recovery NOT scheduled for "${stepName}" — revision budget is zero/invalid (attempts=${budget.attempts}, max=${String(budget.max)}). Card left parked.`);
await this.store.logEntry(
task.id,
"Failed pre-merge step recovery not scheduled — revision budget zero/invalid",
`Step: ${stepName}\nAttempts: ${budget.attempts}\nMax: ${String(budget.max)}`,
this.getRunContextFor(task.id),
);
return false;
}
if (!budget.unbounded && budget.attempts >= budget.max) {
executorLog.warn(`${task.id}: failed pre-merge step recovery NOT scheduled for "${stepName}" — revision budget exhausted (attempts=${budget.attempts}, max=${String(budget.max)}). Card left parked.`);
await this.store.logEntry(
task.id,
"Failed pre-merge step recovery not scheduled — revision budget exhausted",
`Step: ${stepName}\nAttempts: ${budget.attempts}\nMax: ${String(budget.max)}`,
this.getRunContextFor(task.id),
);
return false;
}
await this.sendTaskBackForFix(
task,
@@ -13573,6 +13615,16 @@ export class TaskExecutor {
*/
if (failedNode === COMPLETION_SUMMARY_NODE_ID) return false;
const incompleteSteps = hasNonTerminalWorkflowSteps(live);
/*
* FNXC:WorkflowRemediation 2026-08-09-21:41:
* FN-8910: fire-and-forget remediation nodes have no failure edge. A policy
* or budget refusal after implementation is complete must park visibly in
* the resolved review lane, not clear blockers and eject the card to planning.
* IR workflowAction detection keeps custom renamed remediation nodes covered.
*/
if (!incompleteSteps
&& (failureValue === "remediation-not-scheduled" || failureValue === "missing-remediation-context")
&& await this.isRemediationGraphNode(live.id, failedNode)) return false;
const implementationIncompleteMergeFailure = this.isMergeGraphFailure(failedNode) && failureValue === "implementation-incomplete";
if (implementationIncompleteMergeFailure && !incompleteSteps) return false;
const prematureMergeWithIncompleteSteps = implementationIncompleteMergeFailure && incompleteSteps;