fix(FN-7225): clear stale pause state on workflow retry
This commit is contained in:
7
.changeset/fn-7225-stale-pause-retry.md
Normal file
7
.changeset/fn-7225-stale-pause-retry.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Prevent stale pause state from mislabeling workflow retries as engine pauses.
|
||||||
|
category: fix
|
||||||
|
dev: Clears executor pause-abort provenance on fresh dispatch, Plan Review replan, and manual retry.
|
||||||
@@ -769,15 +769,20 @@ describe("POST /tasks/:id/steer", () => {
|
|||||||
|
|
||||||
describe("POST /tasks/:id/retry", () => {
|
describe("POST /tasks/:id/retry", () => {
|
||||||
let store: TaskStore;
|
let store: TaskStore;
|
||||||
|
let engine: { getTaskStore: ReturnType<typeof vi.fn>; clearTaskPauseAbortState: ReturnType<typeof vi.fn> };
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
store = createMockStore();
|
store = createMockStore();
|
||||||
|
engine = {
|
||||||
|
getTaskStore: vi.fn(() => store),
|
||||||
|
clearTaskPauseAbortState: vi.fn(),
|
||||||
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
function buildApp() {
|
function buildApp() {
|
||||||
const app = express();
|
const app = express();
|
||||||
app.use(express.json());
|
app.use(express.json());
|
||||||
app.use("/api", createApiRoutes(store));
|
app.use("/api", createApiRoutes(store, { engine: engine as any } as any));
|
||||||
return app;
|
return app;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -793,6 +798,7 @@ describe("POST /tasks/:id/retry", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
|
expect(engine.clearTaskPauseAbortState).toHaveBeenCalledWith("KB-001");
|
||||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", {
|
expect(store.updateTask).toHaveBeenCalledWith("KB-001", {
|
||||||
status: null,
|
status: null,
|
||||||
error: null,
|
error: null,
|
||||||
@@ -815,6 +821,7 @@ describe("POST /tasks/:id/retry", () => {
|
|||||||
|
|
||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
expect(res.body.error).toContain("not in a retryable state");
|
expect(res.body.error).toContain("not in a retryable state");
|
||||||
|
expect(engine.clearTaskPauseAbortState).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("retries a failed task in any column (not just in-progress)", async () => {
|
it("retries a failed task in any column (not just in-progress)", async () => {
|
||||||
@@ -4480,4 +4487,3 @@ describe("Attachment routes", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -1640,7 +1640,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
// Retry failed, stuck-killed, or stranded triage/planning task
|
// Retry failed, stuck-killed, or stranded triage/planning task
|
||||||
router.post("/tasks/:id/retry", async (req, res) => {
|
router.post("/tasks/:id/retry", async (req, res) => {
|
||||||
try {
|
try {
|
||||||
const { store: scopedStore } = await getProjectContext(req);
|
const { store: scopedStore, engine } = await getProjectContext(req);
|
||||||
const task = await scopedStore.getTask(req.params.id);
|
const task = await scopedStore.getTask(req.params.id);
|
||||||
const retrySpecification =
|
const retrySpecification =
|
||||||
task.column === "triage" &&
|
task.column === "triage" &&
|
||||||
@@ -1669,6 +1669,12 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
throw badRequest(`Task is not in a retryable state (current status: ${task.status || 'none'})`);
|
throw badRequest(`Task is not in a retryable state (current status: ${task.status || 'none'})`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:ManualRetry 2026-06-29-00:57:
|
||||||
|
Dashboard retry is a fresh run boundary. Clear executor-only pause-abort provenance before mutating task state so stale pause/resume markers cannot relabel the next Plan Review or execution failure as an engine pause.
|
||||||
|
*/
|
||||||
|
engine?.clearTaskPauseAbortState?.(req.params.id);
|
||||||
|
|
||||||
const autoPauseClearPatch = buildAutoPauseClearPatch(task);
|
const autoPauseClearPatch = buildAutoPauseClearPatch(task);
|
||||||
const clearedDeadlockAutoPause = Object.keys(autoPauseClearPatch).length > 0;
|
const clearedDeadlockAutoPause = Object.keys(autoPauseClearPatch).length > 0;
|
||||||
const retryLogSuffix = clearedDeadlockAutoPause ? ", cleared deadlock auto-pause" : "";
|
const retryLogSuffix = clearedDeadlockAutoPause ? ", cleared deadlock auto-pause" : "";
|
||||||
|
|||||||
@@ -132,6 +132,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
|||||||
store.getTask.mockResolvedValue(liveTask);
|
store.getTask.mockResolvedValue(liveTask);
|
||||||
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
|
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
|
||||||
const executor = new TaskExecutor(store, "/tmp/test");
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
(executor as any).markPausedAborted(liveTask.id);
|
||||||
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
|
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
|
||||||
|
|
||||||
const scheduled = await (executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, {
|
const scheduled = await (executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, {
|
||||||
@@ -160,6 +161,59 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
|||||||
graphResumeRetryCount: 0,
|
graphResumeRetryCount: 0,
|
||||||
}, undefined);
|
}, undefined);
|
||||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 1 }, undefined);
|
expect(store.updateTask).not.toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 1 }, undefined);
|
||||||
|
expect((executor as any).pausedAborted.has("FN-7066")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears stale pause-abort provenance before a fresh unpaused execution dispatch", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const liveTask = task({ column: "todo", paused: false, userPaused: false });
|
||||||
|
store.getSettings.mockResolvedValue({ globalPause: false });
|
||||||
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
(executor as any).markPausedAborted(liveTask.id);
|
||||||
|
|
||||||
|
await (executor as any).clearStalePauseAbortBeforeDispatch(liveTask);
|
||||||
|
|
||||||
|
expect((executor as any).pausedAborted.has("FN-7066")).toBe(false);
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith(
|
||||||
|
"FN-7066",
|
||||||
|
"Cleared stale pause-abort marker before unpaused execution dispatch",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears pause-abort provenance for manual retry", () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
(executor as any).markPausedAborted("FN-7066");
|
||||||
|
|
||||||
|
executor.clearPauseAbortStateForManualRetry("FN-7066");
|
||||||
|
|
||||||
|
expect((executor as any).pausedAborted.has("FN-7066")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves pause-abort provenance while the task or engine is actually paused", async () => {
|
||||||
|
for (const { taskPatch, settings } of [
|
||||||
|
{ taskPatch: { paused: true }, settings: { globalPause: false } },
|
||||||
|
{ taskPatch: { userPaused: true }, settings: { globalPause: false } },
|
||||||
|
{ taskPatch: { paused: false, userPaused: false }, settings: { globalPause: true } },
|
||||||
|
]) {
|
||||||
|
const store = createMockStore();
|
||||||
|
const liveTask = task({ column: "todo", ...taskPatch });
|
||||||
|
store.getSettings.mockResolvedValue(settings);
|
||||||
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
(executor as any).markPausedAborted(liveTask.id);
|
||||||
|
|
||||||
|
await (executor as any).clearStalePauseAbortBeforeDispatch(liveTask);
|
||||||
|
|
||||||
|
expect((executor as any).pausedAborted.has("FN-7066")).toBe(true);
|
||||||
|
expect(store.logEntry).not.toHaveBeenCalledWith(
|
||||||
|
"FN-7066",
|
||||||
|
"Cleared stale pause-abort marker before unpaused execution dispatch",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("uses the default budget of 3 for repeated fix passes and then declines when exhausted", async () => {
|
it("uses the default budget of 3 for repeated fix passes and then declines when exhausted", async () => {
|
||||||
|
|||||||
@@ -1654,6 +1654,40 @@ export class TaskExecutor {
|
|||||||
this.completionFinalizedTaskIds.delete(taskId);
|
this.completionFinalizedTaskIds.delete(taskId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async clearStalePauseAbortBeforeDispatch(task: Task): Promise<void> {
|
||||||
|
if (!this.pausedAborted.has(task.id)) return;
|
||||||
|
let globalPause = false;
|
||||||
|
try {
|
||||||
|
globalPause = (await this.store.getSettings()).globalPause === true;
|
||||||
|
} catch {
|
||||||
|
globalPause = false;
|
||||||
|
}
|
||||||
|
if (task.paused === true || task.userPaused === true || globalPause) return;
|
||||||
|
/*
|
||||||
|
* FNXC:WorkflowLifecycle 2026-06-29-00:57:
|
||||||
|
* A stale pause-abort marker must not survive into a fresh unpaused dispatch.
|
||||||
|
* FN-7225 showed graph-owned Plan Review and execution failures being logged
|
||||||
|
* as "engine pause/resume" even though the task row was not paused. Clear the
|
||||||
|
* volatile marker at dispatch entry so real workflow/execution failures keep
|
||||||
|
* their actual cause and do not loop through pause recovery.
|
||||||
|
*/
|
||||||
|
this.clearPausedAborted(task.id);
|
||||||
|
await this.store.logEntry(
|
||||||
|
task.id,
|
||||||
|
"Cleared stale pause-abort marker before unpaused execution dispatch",
|
||||||
|
undefined,
|
||||||
|
this.getRunContextFor(task.id),
|
||||||
|
).catch(() => undefined);
|
||||||
|
}
|
||||||
|
|
||||||
|
clearPauseAbortStateForManualRetry(taskId: string): void {
|
||||||
|
/*
|
||||||
|
FNXC:ManualRetry 2026-06-29-00:57:
|
||||||
|
User retry is a fresh execution boundary. Clear volatile pause-abort provenance so retries cannot inherit stale engine pause/resume classification from a prior run.
|
||||||
|
*/
|
||||||
|
this.clearPausedAborted(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
FNXC:Workspace 2026-06-24-15:45 (concurrent workspace tasks — shared browse-root collision):
|
FNXC:Workspace 2026-06-24-15:45 (concurrent workspace tasks — shared browse-root collision):
|
||||||
In workspace mode `this.rootDir` is the SHARED browse-only (non-git) workspace root, and EVERY
|
In workspace mode `this.rootDir` is the SHARED browse-only (non-git) workspace root, and EVERY
|
||||||
@@ -3890,6 +3924,7 @@ export class TaskExecutor {
|
|||||||
*/
|
*/
|
||||||
const feedback = info.feedback?.trim()
|
const feedback = info.feedback?.trim()
|
||||||
|| "Plan Review failed before execution. Revise the task plan, then continue execution.";
|
|| "Plan Review failed before execution. Revise the task plan, then continue execution.";
|
||||||
|
this.clearPausedAborted(taskId);
|
||||||
await this.store.logEntry(
|
await this.store.logEntry(
|
||||||
taskId,
|
taskId,
|
||||||
"AI spec revision requested",
|
"AI spec revision requested",
|
||||||
@@ -7730,6 +7765,7 @@ export class TaskExecutor {
|
|||||||
|
|
||||||
async execute(task: Task): Promise<void> {
|
async execute(task: Task): Promise<void> {
|
||||||
this.completionFinalizedTaskIds.delete(task.id);
|
this.completionFinalizedTaskIds.delete(task.id);
|
||||||
|
await this.clearStalePauseAbortBeforeDispatch(task);
|
||||||
// Workflow graph interpreter routing (cutover M-C): graph-selected tasks
|
// Workflow graph interpreter routing (cutover M-C): graph-selected tasks
|
||||||
// are orchestrated by the interpreter. The execute seam re-enters this
|
// are orchestrated by the interpreter. The execute seam re-enters this
|
||||||
// method with a completion interceptor registered (which claims the task
|
// method with a completion interceptor registered (which claims the task
|
||||||
@@ -13071,6 +13107,7 @@ ${failureFeedback}
|
|||||||
// assumptions and proceed instead of parking on a question. Explicit opt-in
|
// assumptions and proceed instead of parking on a question. Explicit opt-in
|
||||||
// only (default false = board run); see runGraphCustomNode / KTD-3.
|
// only (default false = board run); see runGraphCustomNode / KTD-3.
|
||||||
const unattended = stepOptions?.unattended === true;
|
const unattended = stepOptions?.unattended === true;
|
||||||
|
const isPlanReviewStep = workflowStep.id === "graph:plan-review-step" || workflowStep.name === "Plan Review";
|
||||||
|
|
||||||
// Compute the diff scope so the workflow step agent reviews only what THIS
|
// Compute the diff scope so the workflow step agent reviews only what THIS
|
||||||
// task changed — not unrelated files it might wander into. Without this,
|
// task changed — not unrelated files it might wander into. Without this,
|
||||||
@@ -13099,7 +13136,19 @@ ${failureFeedback}
|
|||||||
? `${scopedFiles.slice(0, MAX_SCOPE_FILES).map((f) => `- ${f}`).join("\n")}\n- ... (${scopedFiles.length - MAX_SCOPE_FILES} more files truncated)`
|
? `${scopedFiles.slice(0, MAX_SCOPE_FILES).map((f) => `- ${f}`).join("\n")}\n- ... (${scopedFiles.length - MAX_SCOPE_FILES} more files truncated)`
|
||||||
: scopedFiles.map((f) => `- ${f}`).join("\n");
|
: scopedFiles.map((f) => `- ${f}`).join("\n");
|
||||||
|
|
||||||
const scopeBlock = `Diff Scope (files changed by THIS task vs base):
|
/*
|
||||||
|
* FNXC:PlanReviewScope 2026-06-29-00:57:
|
||||||
|
* Plan Review validates the planned PROMPT.md before execution. It must not
|
||||||
|
* inherit the generic workflow-step diff scope, because dirty worktrees or
|
||||||
|
* unrelated local commits can make a plan-only gate reject implementation
|
||||||
|
* state and loop back to triage after the planner already approved the spec.
|
||||||
|
*/
|
||||||
|
const scopeBlock = isPlanReviewStep
|
||||||
|
? `Plan Review Scope:
|
||||||
|
- Review the task plan artifact (PROMPT.md) and task metadata only.
|
||||||
|
- Do NOT judge current implementation diffs, uncommitted worktree changes, or unrelated repository changes.
|
||||||
|
- If PROMPT.md is internally consistent, complete, scoped, and verifiable, approve even when the worktree contains unrelated changes from another task.`
|
||||||
|
: `Diff Scope (files changed by THIS task vs base):
|
||||||
${scopeFileBlock}${diffShortstat ? `\nDiff stat: ${diffShortstat}` : ""}
|
${scopeFileBlock}${diffShortstat ? `\nDiff stat: ${diffShortstat}` : ""}
|
||||||
|
|
||||||
CRITICAL SCOPING RULES — read before doing anything else:
|
CRITICAL SCOPING RULES — read before doing anything else:
|
||||||
|
|||||||
@@ -924,6 +924,10 @@ export class ProjectEngine {
|
|||||||
return this.runtime.getAgentStore();
|
return this.runtime.getAgentStore();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
clearTaskPauseAbortState(taskId: string): void {
|
||||||
|
this.runtime.clearTaskPauseAbortState?.(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
/** Get the MessageStore (if initialized). Returns undefined before start(). */
|
/** Get the MessageStore (if initialized). Returns undefined before start(). */
|
||||||
getMessageStore(): import("@fusion/core").MessageStore | undefined {
|
getMessageStore(): import("@fusion/core").MessageStore | undefined {
|
||||||
return this.runtime.getMessageStore();
|
return this.runtime.getMessageStore();
|
||||||
|
|||||||
@@ -151,6 +151,12 @@ export interface ProjectRuntime extends EventEmitter<ProjectRuntimeEvents> {
|
|||||||
*/
|
*/
|
||||||
getScheduler(): Scheduler;
|
getScheduler(): Scheduler;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Clear volatile executor pause-abort provenance for a task before a manual retry.
|
||||||
|
* Optional because isolated runtimes do not expose in-memory executor state.
|
||||||
|
*/
|
||||||
|
clearTaskPauseAbortState?(taskId: string): void;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get current runtime metrics.
|
* Get current runtime metrics.
|
||||||
* @returns Metrics including in-flight tasks, active agents, and memory usage
|
* @returns Metrics including in-flight tasks, active agents, and memory usage
|
||||||
|
|||||||
@@ -1378,6 +1378,10 @@ export class InProcessRuntime
|
|||||||
return this.scheduler;
|
return this.scheduler;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
clearTaskPauseAbortState(taskId: string): void {
|
||||||
|
this.executor?.clearPauseAbortStateForManualRetry(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
configurePrMonitoring(options: {
|
configurePrMonitoring(options: {
|
||||||
prMonitor: PrMonitor;
|
prMonitor: PrMonitor;
|
||||||
onClosedPrFeedback?: (taskId: string, prInfo: PrInfo, comments: PrComment[]) => void | Promise<void>;
|
onClosedPrFeedback?: (taskId: string, prInfo: PrInfo, comments: PrComment[]) => void | Promise<void>;
|
||||||
|
|||||||
Reference in New Issue
Block a user