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

FN-7162: reopen actionable steps after review bounces

Browse Source 
Post-verdict remediation now resumes executable work before rechecking optional gates.

- Reopen the terminal verification/delivery suffix together with the nearest actionable implementation step for revision bounces.
- Cover Code Review, Browser Verification, gate-promoted revisions, and non-revision optional outcomes with regression tests.
- Document post-verdict bounce semantics and add a patch changeset for the published CLI package.

Files changed:
 .changeset/fn-7162-post-verdict-bounce-step-completion.md |   7 ++
 docs/workflow-steps.md                             |   9 +-
 packages/engine/src/__tests__/executor-step-session.test.ts    | 117 +++++++++++++++++++--
 packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts       |  60 +++++++++++
 packages/engine/src/executor.ts                    |  51 +++++++--
 5 files changed, 222 insertions(+), 22 deletions(-)

Fusion-Task-Id: FN-7162
Fusion-Task-Lineage: 8d422fc9-54e8-431d-97bb-e7a4a6763eaa
Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-06-27 23:01:10 -07:00
parent 5d9184d380
commit 987abc763b
5 changed files with 222 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/fn-7162-post-verdict-bounce-step-completion.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Tasks sent back by Code Review or Browser Verification verdicts now re-run and complete their steps before re-checking.
category: fix
dev: Fixes the post-verdict remediation bounce (requestPreMergeOptionalStepFix → sendTaskBackForFix → reopenLastStepForRevision → scheduleWorkflowRerun → graph re-run) so resumed execution re-launches the executor and drives reopened implementation/verification/delivery steps and the verdict-demanded fix to done across both in-progress and in-review bounce sources, bounded by the existing maxRevisions/maxPostReviewFixes budget.

9
docs/workflow-steps.md
View File

@@ -558,8 +558,8 @@ For pre-merge gate hard failures, the graph executor drives remediation through
1. Retry the failing check up to `MAX_WORKFLOW_STEP_RETRIES` within the same execution lifecycle
2. On retry exhaustion, add a steering comment with failure details and inject a `Workflow Step Failure` section into `PROMPT.md`
3. Reopen only the last implementation step (`pending`) so prior completed work remains preserved
4. Schedule `todo → in-progress` after guard unwind, triggering a fresh executor remediation run
3. Reopen the terminal verification/delivery suffix plus the nearest preceding implementation step (`pending`) so the resumed pass can address feedback without discarding unrelated completed work
4. Schedule `todo → in-progress` after guard unwind, triggering a fresh executor remediation run that must complete every reopened step before the workflow step re-evaluates
Tasks are not parked in `in-review` for this remediable path unless additional terminal failures occur.
@@ -590,9 +590,12 @@ Enabled PRE-merge optional-group REVISE findings should be acted on before revie
FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:55:
Workflow authors can override the global optional-step remediation budget per optional-group via `maxRevisions`, including `"unbounded"` for loops that should continue until the step approves. Document both inline executor and self-healing semantics because they must resolve the same budget for parked review recovery.
FNXC:WorkflowOptionalStepFix 2026-06-27-22:51:
A post-verdict bounce from Code Review or Browser Verification must not reopen only a trivial trailing documentation step. The next executor pass reopens the actionable implementation step plus terminal verification/delivery steps, completes them to `done`, and only then lets the optional group re-evaluate across both in-progress and in-review bounce sources.
-->
During a live graph run, an enabled **pre-merge** optional step that returns `REVISE` (including the built-in **Code Review** / `code-review` and **Browser Verification** / `browser-verification` groups) sends the task back to the executor for a fix pass before the graph continues to review or merge. The workflow graph restarts on the next executor pass, so the optional step re-runs against the fixed diff; the cycle repeats until the step returns `APPROVE` / `APPROVE_WITH_NOTES` or the resolved revision budget is exhausted. By default, each step uses the workflow/project `maxPostReviewFixes` value (built-in default: 3 fix passes). A workflow author can override that for a specific `optional-group` with `config.maxRevisions`: a non-negative integer sets that step's ceiling, `0` disables automatic fixes for that step, and `"unbounded"` removes the ceiling check. The counter remains the task's shared `postReviewFixCount`; per-step counters are not maintained.
During a live graph run, an enabled **pre-merge** optional step that returns `REVISE` (including the built-in **Code Review** / `code-review` and **Browser Verification** / `browser-verification` groups) sends the task back to the executor for a fix pass before the graph continues to review or merge. The workflow graph restarts on the next executor pass, re-launches task execution, and reopens the terminal verification/delivery suffix plus the nearest preceding implementation step so the verdict-demanded fix can be made rather than merely replaying a trivial trailing step. The optional step re-runs only after the executor drives those reopened steps back to `done`; the cycle repeats until the step returns `APPROVE` / `APPROVE_WITH_NOTES` or the resolved revision budget is exhausted. By default, each step uses the workflow/project `maxPostReviewFixes` value (built-in default: 3 fix passes). A workflow author can override that for a specific `optional-group` with `config.maxRevisions`: a non-negative integer sets that step's ceiling, `0` disables automatic fixes for that step, and `"unbounded"` removes the ceiling check. The counter remains the task's shared `postReviewFixCount`; per-step counters are not maintained.
The same resolved per-step budget is used by self-healing when it revives an `in-review` task that is parked with a failed pre-merge workflow result. If the failed step's IR cannot be resolved, self-healing falls back to `maxPostReviewFixes` so existing behavior is preserved. `"unbounded"` relies on the optional step eventually approving; a step that always returns `REVISE` will continue cycling until a human intervenes or another guard (pause, worktree/lease, auto-merge policy, dependency blocker) stops recovery. When the budget is exhausted or disabled, behavior falls through to the prior semantics: advisory results remain non-blocking and gate failures remain failed/parked.

117
packages/engine/src/__tests__/executor-step-session.test.ts
View File

@@ -481,15 +481,15 @@ describe("Workflow Steps Execution", () => {
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
});
it("routes exhausted prompt-mode workflow hard failures back to remediation and only reopens the last step", async () => {
it("routes exhausted prompt-mode workflow hard failures back to remediation and reopens actionable steps", async () => {
// This test was previously written as an end-to-end run through
// executor.execute(...) with vi.useFakeTimers(), but that path hung
// deterministically under the 15 s budget: createResolvedAgentSession's
// workflow-step Promise.race used a frozen 360 s setTimeout, and the
// rejection from the mock prompt never reached the catch block in time.
// The behavior we actually need to lock down is:
// 1. sendTaskBackForFix re-opens only the last completed step
// (reopenLastStepForRevision) — earlier done steps stay done.
// 1. sendTaskBackForFix re-opens the actionable implementation step plus
// any trailing verification/delivery step, not just a trivial last step.
// 2. The rerun bounce uses preserveResumeState so step progress and
// the worktree survive the in-progress → todo hop.
// 3. PROMPT.md gains the Workflow Step Failure section with the
@@ -519,8 +519,8 @@ describe("Workflow Steps Execution", () => {
column: "in-progress" as const,
dependencies: [] as string[],
steps: [
{ name: "Step 0", status: "done" as const },
{ name: "Step 1", status: "done" as const },
{ name: "Implementation", status: "done" as const },
{ name: "Documentation & Delivery", status: "done" as const },
],
currentStep: 1,
log: [] as any[],
@@ -590,7 +590,10 @@ describe("Workflow Steps Execution", () => {
"Workflow step failed",
);
// (1) failure comment + only the last step re-opened
// (1) failure comment + the implementation-bearing step is re-opened with the trailing delivery step.
// Before FN-7162, reopenLastStepForRevision returned only [1] here, so a
// Code Review / Browser Verification REVISE could re-run Documentation &
// Delivery against unchanged implementation work and loop until budget exhaustion.
expect(store.addTaskComment).toHaveBeenCalledWith(
"FN-001",
expect.stringContaining("Workflow step failed"),
@@ -599,8 +602,7 @@ describe("Workflow Steps Execution", () => {
const reopenedStepIndexes = store.updateStep.mock.calls
.filter((call: any[]) => call[0] === "FN-001" && call[2] === "pending")
.map((call: any[]) => call[1]);
expect(reopenedStepIndexes).toContain(1);
expect(reopenedStepIndexes).not.toContain(0);
expect(reopenedStepIndexes).toEqual([0, 1]);
// performWorkflowRerunBounce was invoked synchronously by the spy
// above; flush microtasks so its awaited store calls settle before
@@ -633,6 +635,105 @@ describe("Workflow Steps Execution", () => {
injectSpy.mockRestore();
});
it("keeps post-verdict reopening bounded across all-done, mixed, and single-step states", async () => {
const cases = [
{
id: "all-done-terminal",
steps: [
{ name: "Implementation", status: "done" as const },
{ name: "Documentation & Delivery", status: "done" as const },
],
expectedIndexes: [0, 1],
expectedCurrent: 0,
},
{
id: "mixed-terminal-pending",
steps: [
{ name: "Implementation", status: "done" as const },
{ name: "Documentation & Delivery", status: "pending" as const },
],
expectedIndexes: [0],
expectedCurrent: 0,
},
{
id: "single-step",
steps: [{ name: "Implementation", status: "done" as const }],
expectedIndexes: [0],
expectedCurrent: 0,
},
];
for (const testCase of cases) {
const store = createMockStore();
const mutableTask = {
id: `FN-7162-${testCase.id}`,
title: "Test",
description: "Test task",
column: "in-progress" as const,
dependencies: [] as string[],
steps: testCase.steps.map((step) => ({ ...step })),
currentStep: testCase.steps.length - 1,
log: [] as any[],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
store.updateStep.mockImplementation(async (_taskId: string, stepIndex: number, status: string) => {
mutableTask.steps[stepIndex]!.status = status as any;
return {};
});
const executor = new TaskExecutor(store, "/tmp/test");
const reopened = await (executor as unknown as {
reopenLastStepForRevision: (
taskId: string,
task: typeof mutableTask,
) => Promise<{ index: number; name: string; indexes: number[] } | null>;
}).reopenLastStepForRevision(mutableTask.id, mutableTask);
expect(reopened?.indexes).toEqual(testCase.expectedIndexes);
expect(reopened?.index).toBe(testCase.expectedCurrent);
expect(store.updateStep.mock.calls.map((call: any[]) => call[1])).toEqual(testCase.expectedIndexes);
expect(store.updateTask).toHaveBeenCalledWith(mutableTask.id, { currentStep: testCase.expectedCurrent });
expect(mutableTask.steps.some((step) => step.status === "pending")).toBe(true);
}
});
it("reopens terminal verification and delivery suffix with the implementation step", async () => {
const store = createMockStore();
const mutableTask = {
id: "FN-7162-SUFFIX",
title: "Test",
description: "Test task",
column: "in-progress" as const,
dependencies: [] as string[],
steps: [
{ name: "Implementation", status: "done" as const },
{ name: "Testing & Verification", status: "done" as const },
{ name: "Documentation & Delivery", status: "done" as const },
],
currentStep: 2,
log: [] as any[],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
store.updateStep.mockImplementation(async (_taskId: string, stepIndex: number, status: string) => {
mutableTask.steps[stepIndex]!.status = status as any;
return {};
});
const executor = new TaskExecutor(store, "/tmp/test");
const reopened = await (executor as unknown as {
reopenLastStepForRevision: (
taskId: string,
task: typeof mutableTask,
) => Promise<{ index: number; name: string; indexes: number[] } | null>;
}).reopenLastStepForRevision(mutableTask.id, mutableTask);
expect(reopened).toEqual({ index: 0, name: "Implementation", indexes: [0, 1, 2] });
expect(store.updateStep.mock.calls.map((call: any[]) => call[1])).toEqual([0, 1, 2]);
expect(store.updateTask).toHaveBeenCalledWith("FN-7162-SUFFIX", { currentStep: 0 });
});
// FNXC:WorkflowOptionalStepFix 2026-06-27-13:30:
// Regression for the FN-7122 deadlock: a pre-merge optional-step REVISE
// reopens the last plan step to `pending` and schedules a rerun bounce, but

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

@@ -38,6 +38,66 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
resetExecutorMocks();
});
it("sends Code Review, Browser Verification, and gate-promoted pre-merge revisions back for remediation", async () => {
const cases = [
{ stepName: "Code Review", status: "advisory_failure" as const, feedback: "review finding" },
{ stepName: "Browser Verification", status: "advisory_failure" as const, feedback: "browser finding" },
{ stepName: "Code Review", status: "failed" as const, feedback: "gate-promoted finding" },
];
for (const testCase of cases) {
const store = createMockStore();
const liveTask = task({ postReviewFixCount: 0, worktree: "/tmp/fusion/fn-7066" });
store.getTask.mockResolvedValue(liveTask);
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
const executor = new TaskExecutor(store, "/tmp/test");
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
const scheduled = await (executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, {
...reviseInfo,
stepName: testCase.stepName,
status: testCase.status,
feedback: testCase.feedback,
});
expect(scheduled).toBe(true);
expect(sendBack).toHaveBeenCalledWith(
liveTask,
"/tmp/fusion/fn-7066",
testCase.feedback,
testCase.stepName,
expect.stringContaining("requested revision"),
);
}
});
it("does not bounce post-merge, fast-mode skipped, approved, or non-revision optional outcomes", async () => {
const cases = [
{ phase: "post-merge" as const, status: "advisory_failure" as const, verdict: "REVISE" },
{ phase: "pre-merge" as const, status: "passed" as const, verdict: "APPROVE" },
{ phase: "pre-merge" as const, status: "passed" as const, verdict: "workflow-step-skipped" },
{ phase: "pre-merge" as const, status: "advisory_failure" as const, verdict: "APPROVE_WITH_NOTES" },
];
for (const testCase of cases) {
const store = createMockStore();
const liveTask = task({ postReviewFixCount: 0 });
store.getTask.mockResolvedValue(liveTask);
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
const executor = new TaskExecutor(store, "/tmp/test");
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
const scheduled = await (executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, {
...reviseInfo,
...testCase,
});
expect(scheduled).toBe(false);
expect(sendBack).not.toHaveBeenCalled();
expect(store.updateTask).not.toHaveBeenCalledWith(liveTask.id, expect.objectContaining({ postReviewFixCount: expect.any(Number) }), undefined);
}
});
it("consumes budget before sending the task back for optional-step remediation", async () => {
const store = createMockStore();
const liveTask = task({ postReviewFixCount: 0 });

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

@@ -11913,34 +11913,63 @@ export class TaskExecutor {
}
/**
* Re-open the last non-pending step so a revision/failure handler gives the
* executor exactly one pending slot to re-enter through. Returns the index
* and name of the step that was flipped to `pending`, or null when there
* was nothing to re-open.
* Re-open the implementation-bearing slice of work for a revision/failure
* handler. Returns the earliest reopened step and all reopened indexes, or
* null when there was nothing to re-open.
*/
private async reopenLastStepForRevision(
taskId: string,
task: Task,
): Promise<{ index: number; name: string } | null> {
): Promise<{ index: number; name: string; indexes: number[] } | null> {
const steps = task.steps;
if (steps.length === 0) return null;
let targetIndex = -1;
let lastNonPendingIndex = -1;
for (let i = steps.length - 1; i >= 0; i--) {
if (steps[i].status !== "pending") {
targetIndex = i;
lastNonPendingIndex = i;
break;
}
}
if (targetIndex === -1) {
if (lastNonPendingIndex === -1) {
await this.store.updateTask(taskId, { currentStep: 0 });
return null;
}
await this.store.updateStep(taskId, targetIndex, "pending");
await this.store.updateTask(taskId, { currentStep: targetIndex });
return { index: targetIndex, name: steps[targetIndex].name };
// Match step-title words rather than arbitrary substrings so an implementation step
// like "DataVerificationLayer" is not treated as a trailing delivery/check step.
const isTerminalVerificationOrDeliveryStep = (name: string): boolean =>
/(^|[^a-z])(testing|verification|documentation|delivery)([^a-z]|$)/i.test(name);
const resetIndexes = new Set<number>([lastNonPendingIndex]);
if (isTerminalVerificationOrDeliveryStep(steps[lastNonPendingIndex].name)) {
let cursor = lastNonPendingIndex;
while (cursor >= 0 && isTerminalVerificationOrDeliveryStep(steps[cursor].name)) {
resetIndexes.add(cursor);
cursor--;
}
while (cursor >= 0 && steps[cursor].status === "pending") {
cursor--;
}
if (cursor >= 0) {
resetIndexes.add(cursor);
}
}
const indexes = [...resetIndexes].sort((a, b) => a - b);
/*
FNXC:WorkflowOptionalStepFix 2026-06-27-18:03:
Code Review / Browser Verification REVISE bounces must reopen the step that can actually make the requested code change, not only a trailing Documentation & Delivery or Testing & Verification step. Otherwise the graph rerun can complete a trivial terminal step, re-evaluate the optional group against unchanged code, and loop or strand pending work. Reopen the trailing verification/delivery suffix plus the nearest preceding implementation step so both in-progress and in-review bounce sources re-launch execution on actionable work before optional-step re-evaluation.
*/
for (const index of indexes) {
if (steps[index].status !== "pending") {
await this.store.updateStep(taskId, index, "pending");
}
}
const currentStep = indexes[0] ?? lastNonPendingIndex;
await this.store.updateTask(taskId, { currentStep });
return { index: currentStep, name: steps[currentStep].name, indexes };
}
/**