fix: don't flag a task failed while its code-review remediation is still executing

A pre-merge-remediation/plan-replan node (e.g. code-review-remediation) is a
fire-and-forget async scheduler with no failure out-edge. When its schedule call
can't re-arm (missing rehydrated failureContext after restart,
remediation-not-scheduled, or an exhausted rework budget), the failure bubbled
out as the terminal graph outcome and handleGraphFailure stamped status:"failed"
— surfacing a spurious "Task Failed" even while the previously-scheduled
fix/reviewer session was still live.

Guard the terminal sink: skip the failed park when the failed node is a
remediation node AND a live agent session surface is still registered for the
task. Scoped via isRemediationGraphNode (IR workflowAction + built-in node-id
fallback) and hasLiveTaskSessionSurface; genuine execute/merge failures and
remediation failures with no live session still park failed unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-01 21:30:48 -07:00
parent 69f754f868
commit ca88a6c6a2
3 changed files with 139 additions and 0 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Stop showing "Task Failed" on a task whose code-review remediation is still running.
category: fix
dev: handleGraphFailure now skips the terminal `status:"failed"` park when the failed graph node is a `pre-merge-remediation`/`plan-replan` node (e.g. `code-review-remediation`) AND a live agent session surface is still registered for the task. These nodes are fire-and-forget async schedulers with no `failure` out-edge, so a failed re-arm (missing rehydrated failureContext after restart, remediation-not-scheduled, or exhausted rework budget) bubbled out as the terminal graph outcome and stamped a spurious failure over live work. Scoped via `isRemediationGraphNode` (IR `workflowAction` with built-in node-id fallback) + `hasLiveTaskSessionSurface`; genuine execute/merge failures and remediation failures with no live session still park failed unchanged.

View File

@@ -165,6 +165,92 @@ describe("executor graph execute self-requeue gate", () => {
); );
}); });
it("does not flag a remediation-node graph failure as failed while a live agent session is executing", async () => {
resetExecutorMocks();
const store = createMockStore();
const live = task({
id: "FN-REMEDIATION-LIVE",
column: "in-progress",
steps: [{ name: "Implement", status: "in-progress" }],
});
store.getTask.mockResolvedValue(live);
store.getSettings.mockResolvedValue({
autoMerge: true,
maxAutoMergeRetries: 3,
maxConcurrent: 2,
maxWorktrees: 4,
pollIntervalMs: 15000,
});
const executor = new TaskExecutor(store, "/tmp/test");
/*
* FNXC:WorkflowRemediation 2026-07-01-23:40:
* `code-review-remediation` is a fire-and-forget async scheduler with no
* `failure` out-edge; a failed re-arm bubbles out as the terminal graph
* outcome. When a SEPARATE live agent session surface is still registered
* (the previously-scheduled fix/reviewer is mid-flight), the terminal sink
* must NOT stamp `status:"failed"` over live work.
*/
(executor as any).activeSessions.set(live.id, { session: {} });
await (executor as any).handleGraphFailure(live, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["code-review", "code-review-remediation"],
context: {},
});
expect(store.updateTask).not.toHaveBeenCalledWith(
live.id,
expect.objectContaining({ status: "failed" }),
expect.anything(),
);
expect(store.logEntry).toHaveBeenCalledWith(
live.id,
expect.stringContaining("not flagging as failed"),
undefined,
undefined,
);
expect(store.moveTask).not.toHaveBeenCalledWith(live.id, "in-review", expect.anything());
});
it("still parks a remediation-node graph failure as failed when NO live session exists", async () => {
resetExecutorMocks();
const store = createMockStore();
const live = task({
id: "FN-REMEDIATION-DEAD",
column: "in-progress",
steps: [{ name: "Implement", status: "in-progress" }],
});
store.getTask.mockResolvedValue(live);
store.getSettings.mockResolvedValue({
autoMerge: true,
maxAutoMergeRetries: 3,
maxConcurrent: 2,
maxWorktrees: 4,
pollIntervalMs: 15000,
});
const executor = new TaskExecutor(store, "/tmp/test");
// Surface enumeration: the guard is scoped to a LIVE session surface. With
// no session (e.g. a genuinely exhausted rework budget), the remediation
// failure remains terminal and parks failed exactly as before.
await (executor as any).handleGraphFailure(live, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["code-review", "code-review-remediation"],
context: {},
});
expect(store.updateTask).toHaveBeenCalledWith(
live.id,
expect.objectContaining({
status: "failed",
error: expect.stringContaining("Workflow graph terminated with failure at node 'code-review-remediation'"),
}),
undefined,
);
});
it("does not hand generic graph failures to review", async () => { it("does not hand generic graph failures to review", async () => {
resetExecutorMocks(); resetExecutorMocks();
const store = createMockStore(); const store = createMockStore();

View File

@@ -7651,6 +7651,41 @@ export class TaskExecutor {
return failedNode === "merge-manual-hold" || failedNode === "merge-retry"; return failedNode === "merge-manual-hold" || failedNode === "merge-retry";
} }
/*
FNXC:WorkflowRemediation 2026-07-01-23:40:
A live agent session surface for a task proves the work is still executing, independent of the persisted column/pause/status row that handleGraphFailure re-fetches. This mirrors clearPhantomExecutorBinding's `hasLiveSessionSurface` (FN-6736) but deliberately EXCLUDES `this.executing` and graph-routing membership: those are still set for the graph run that is currently ending (graphRouting is cleared in maybeExecuteWorkflowGraph's finally, AFTER handleGraphFailure returns), so including them would report every ending run as "still executing" and suppress all failures. Only a registered coding/step/CLI session surface means a SEPARATE, live agent is working the task.
*/
private hasLiveTaskSessionSurface(taskId: string): boolean {
return (
this.activeSessions.has(taskId)
|| this.activeStepExecutors.has(taskId)
|| this.activeWorkflowStepSessions.has(taskId)
|| this.activeCliTaskSessions.has(taskId)
);
}
/*
FNXC:WorkflowRemediation 2026-07-01-23:40:
A `pre-merge-remediation` / `plan-replan` node (e.g. `code-review-remediation`) is a FIRE-AND-FORGET async scheduler, not a terminal work node: its job is to hand off an implementation fix (sendTaskBackForFix re-dispatches the coding session) and stop traversal. These nodes carry only a `success` rework edge back to their gate and NO `failure` out-edge, so when their schedule call cannot re-arm (missing rehydrated failureContext after a restart → `missing-remediation-context`, `remediation-not-scheduled`, or an exhausted rework budget) the failure bubbles out as the terminal graph outcome and handleGraphFailure would stamp `status:"failed"` — even while a previously-scheduled fix/reviewer session is still live. Classify these nodes so that terminal sink can preserve a still-executing task instead of flagging a spurious failure. Detection prefers the resolved IR `workflowAction` (covers custom workflows), with a node-id fallback for the built-in ids when the IR cannot be resolved.
*/
private async isRemediationGraphNode(taskId: string, failedNode: string | undefined): Promise<boolean> {
if (!failedNode) return false;
try {
const ir = await resolveWorkflowIrForTask(this.store, taskId);
const node = ir?.nodes?.find((n) => n.id === failedNode);
const action = node?.config?.workflowAction;
if (action === "pre-merge-remediation" || action === "plan-replan") return true;
if (node) return false;
} catch {
// Best-effort IR resolution; fall through to the built-in id fallback.
}
return (
failedNode === "code-review-remediation"
|| failedNode === "browser-verification-remediation"
|| failedNode === "plan-replan"
);
}
private isTerminalMergeGraphFailureValue(value: string | undefined): boolean { private isTerminalMergeGraphFailureValue(value: string | undefined): boolean {
if (!value) return false; if (!value) return false;
const normalized = value.toLowerCase(); const normalized = value.toLowerCase();
@@ -8451,6 +8486,17 @@ export class TaskExecutor {
return; return;
} }
} }
/*
FNXC:WorkflowRemediation 2026-07-01-23:40:
Do NOT flag a still-executing task as failed. A `pre-merge-remediation` / `plan-replan` node (e.g. `code-review-remediation`) is a fire-and-forget async scheduler with no `failure` out-edge, so a failed re-arm (missing rehydrated failureContext after restart, remediation-not-scheduled, or an exhausted rework budget) bubbles out as the terminal graph outcome here. When a SEPARATE live agent session surface is still registered for this task, the previously-scheduled fix/reviewer is genuinely mid-flight — parking `status:"failed"` would surface a spurious "Task Failed" over live work. Preserve the row and let the live session drive its own terminal handoff instead. Scoped strictly to remediation nodes + a live session surface so genuine execute/merge terminal failures (and remediation failures with NO live session, e.g. a truly exhausted budget) still park exactly as before.
*/
if (this.hasLiveTaskSessionSurface(task.id) && await this.isRemediationGraphNode(task.id, failedNode)) {
const benignMessage = `Workflow graph ended at remediation node '${failedNode ?? "unknown"}' while a live agent session is still executing — not flagging as failed; live session preserved`;
executorLog.warn(`${task.id}: ${benignMessage}`);
await this.store.logEntry(task.id, benignMessage, undefined, this.getRunContextFor(task.id));
await this.persistTokenUsage(task.id);
return;
}
const message = `Workflow graph terminated with failure at node '${failedNode ?? "unknown"}'`; const message = `Workflow graph terminated with failure at node '${failedNode ?? "unknown"}'`;
executorLog.warn(`${task.id}: ${message}`); executorLog.warn(`${task.id}: ${message}`);
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id)); await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));