feat: auto-revive in-review tasks with failed pre-merge workflow steps
Adds a SelfHealingManager scan that finds tasks parked in in-review with a failed pre-merge workflow step and no active session, and sends them back through the existing sendTaskBackForFix flow (PROMPT.md injection, step reset, todo → in-progress). Bounded by a new maxPostReviewFixes setting (default 1) and a per-task postReviewFixCount so a persistently- failing verifier cannot ping-pong a task indefinitely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -819,6 +819,55 @@ export class TaskExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Auto-revive an `in-review` task whose pre-merge workflow step(s) failed, by
|
||||
* replaying the same send-back-for-fix flow the executor uses during a live
|
||||
* run. Invoked by SelfHealingManager's `recoverReviewTasksWithFailedPreMergeSteps`
|
||||
* scan when a task is parked in review with a failed pre-merge step and no
|
||||
* active session.
|
||||
*
|
||||
* Picks the latest failed pre-merge workflow step result (there is usually only
|
||||
* one, but if several ran we want the most recent), injects its feedback into
|
||||
* `PROMPT.md`, resets steps, and schedules todo → in-progress. The call site
|
||||
* is responsible for enforcing the `maxPostReviewFixes` budget before invoking
|
||||
* this method — this method itself does no accounting.
|
||||
*
|
||||
* @returns true when the task was sent back, false when no eligible failed
|
||||
* step exists (caller should skip).
|
||||
*/
|
||||
async recoverFailedPreMergeWorkflowStep(task: Task): Promise<boolean> {
|
||||
try {
|
||||
const failed = (task.workflowStepResults ?? [])
|
||||
.filter((r) => (r.phase || "pre-merge") === "pre-merge" && r.status === "failed")
|
||||
.sort((a, b) => {
|
||||
const aTs = Date.parse(a.completedAt || a.startedAt || "");
|
||||
const bTs = Date.parse(b.completedAt || b.startedAt || "");
|
||||
return (Number.isFinite(bTs) ? bTs : 0) - (Number.isFinite(aTs) ? aTs : 0);
|
||||
});
|
||||
const target = failed[0];
|
||||
if (!target) {
|
||||
executorLog.warn(`${task.id}: no failed pre-merge workflow step to recover from`);
|
||||
return false;
|
||||
}
|
||||
|
||||
const feedback = target.output?.trim() || "(no feedback captured)";
|
||||
const stepName = target.workflowStepName || target.workflowStepId || "Unknown";
|
||||
|
||||
await this.sendTaskBackForFix(
|
||||
task,
|
||||
task.worktree ?? "",
|
||||
feedback,
|
||||
stepName,
|
||||
`Auto-revived from in-review: pre-merge workflow step "${stepName}" had failed`,
|
||||
);
|
||||
return true;
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
executorLog.error(`Failed to recover failed pre-merge workflow step for ${task.id}: ${errorMessage}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resume orphaned in-progress tasks (e.g., after crash/restart).
|
||||
* Call once after engine startup.
|
||||
|
||||
@@ -575,6 +575,7 @@ export class InProcessRuntime
|
||||
this.selfHealingManager = new SelfHealingManager(this.taskStore, {
|
||||
rootDir: this.config.workingDirectory,
|
||||
recoverCompletedTask: (task) => this.executor.recoverCompletedTask(task),
|
||||
recoverFailedPreMergeStep: (task) => this.executor.recoverFailedPreMergeWorkflowStep(task),
|
||||
getExecutingTaskIds: () => this.executor.getExecutingTaskIds(),
|
||||
recoverApprovedTriageTask: (task) => this.triageProcessor?.recoverApprovedTask(task) ?? Promise.resolve(false),
|
||||
getSpecifyingTaskIds: () => this.triageProcessor?.getProcessingTaskIds() ?? new Set<string>(),
|
||||
|
||||
@@ -1249,6 +1249,184 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverReviewTasksWithFailedPreMergeSteps", () => {
|
||||
const baseTask = {
|
||||
id: "FN-1572",
|
||||
column: "in-review" as const,
|
||||
paused: false,
|
||||
status: null as string | null,
|
||||
worktree: "/tmp/test-project/.worktrees/fn-1572",
|
||||
steps: [
|
||||
{ name: "Preflight", status: "done" as const },
|
||||
{ name: "Implementation", status: "done" as const },
|
||||
],
|
||||
workflowStepResults: [
|
||||
{
|
||||
workflowStepId: "WS-004",
|
||||
workflowStepName: "Browser Verification",
|
||||
phase: "pre-merge" as const,
|
||||
status: "failed" as const,
|
||||
output: "SSE reconnect leaks /api/events connections when view toggles.",
|
||||
startedAt: "2026-04-17T21:08:24.135Z",
|
||||
completedAt: "2026-04-17T21:35:32.036Z",
|
||||
},
|
||||
],
|
||||
postReviewFixCount: 0,
|
||||
log: [],
|
||||
};
|
||||
|
||||
it("sends a review task back for fix when a pre-merge workflow step failed and budget remains", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 1,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask }]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-1572", { postReviewFixCount: 1 });
|
||||
expect(recoverFn).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-1572" }));
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-1572",
|
||||
expect.stringContaining("Auto-reviving in-review task"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips tasks whose postReviewFixCount has reached maxPostReviewFixes", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 2,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{ ...baseTask, postReviewFixCount: 2 },
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("no-ops when recoverFailedPreMergeStep callback is not supplied", async () => {
|
||||
const managerWithoutCallback = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask }]);
|
||||
|
||||
const result = await managerWithoutCallback.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.listTasks).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
|
||||
managerWithoutCallback.stop();
|
||||
});
|
||||
|
||||
it("skips tasks without a worktree (cannot re-execute safely)", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 1,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{ ...baseTask, worktree: undefined },
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips tasks already executing (avoid double-send-back while a run is in flight)", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
getExecutingTaskIds: () => new Set(["FN-1572"]),
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 1,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask }]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("leaves tasks with non-pre-merge blockers alone (e.g. incomplete steps)", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 1,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
...baseTask,
|
||||
// Task has a failed WS *and* an incomplete step — the "incomplete
|
||||
// steps" blocker wins in getTaskMergeBlocker, so this scan should
|
||||
// defer to recoverStaleIncompleteReviewTasks instead.
|
||||
steps: [
|
||||
{ name: "Preflight", status: "done" as const },
|
||||
{ name: "Implementation", status: "in-progress" as const },
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("disables itself when maxPostReviewFixes is 0", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 0,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask }]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverOrphanedExecutions", () => {
|
||||
it("requeues in-progress tasks whose reserved worktree is missing", async () => {
|
||||
const getExecuting = vi.fn().mockReturnValue(new Set<string>());
|
||||
|
||||
@@ -59,6 +59,14 @@ export interface SelfHealingOptions {
|
||||
* Called before recovery checks so stale entries don't block recovery.
|
||||
*/
|
||||
evictStaleTriageProcessing?: () => Set<string>;
|
||||
/**
|
||||
* Auto-revive an `in-review` task whose pre-merge workflow step failed.
|
||||
* Delegates to the executor, which injects the failure feedback into
|
||||
* `PROMPT.md`, resets steps, and schedules todo → in-progress.
|
||||
*
|
||||
* Should return true if the task was successfully sent back, false otherwise.
|
||||
*/
|
||||
recoverFailedPreMergeStep?: (task: Task) => Promise<boolean>;
|
||||
}
|
||||
|
||||
const APPROVED_TRIAGE_RECOVERY_GRACE_MS = 60_000;
|
||||
@@ -143,6 +151,7 @@ export class SelfHealingManager {
|
||||
await this.recoverNoProgressNoTaskDoneFailures();
|
||||
await this.recoverCompletedTasks();
|
||||
await this.recoverStaleIncompleteReviewTasks();
|
||||
await this.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
await this.recoverInterruptedMergingTasks();
|
||||
await this.recoverMisclassifiedFailures();
|
||||
await this.recoverOrphanedExecutions();
|
||||
@@ -477,6 +486,7 @@ export class SelfHealingManager {
|
||||
const batch2Results = await Promise.allSettled([
|
||||
this.recoverCompletedTasks(),
|
||||
this.recoverStaleIncompleteReviewTasks(),
|
||||
this.recoverReviewTasksWithFailedPreMergeSteps(),
|
||||
this.recoverInterruptedMergingTasks(),
|
||||
this.recoverMergeableReviewTasks(),
|
||||
this.recoverMergedReviewTasks(),
|
||||
@@ -666,6 +676,106 @@ export class SelfHealingManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover `in-review` tasks parked by a failed pre-merge workflow step.
|
||||
*
|
||||
* When a pre-merge workflow step (e.g. Browser Verification) fails during an
|
||||
* active executor run, `executor.handleWorkflowStepFailure` retries up to
|
||||
* `MAX_WORKFLOW_STEP_RETRIES` times in-session. If all retries exhaust the
|
||||
* task ends up in `in-review` with the failed workflow step result still on
|
||||
* record, which `getTaskMergeBlocker` correctly treats as a merge block —
|
||||
* leaving the task stranded with no live session to un-stick it.
|
||||
*
|
||||
* This scan delegates back to the executor's `recoverFailedPreMergeWorkflowStep`
|
||||
* path (which reuses the same `sendTaskBackForFix` flow the executor uses
|
||||
* internally) so the agent gets another attempt with the failure feedback
|
||||
* injected into `PROMPT.md`. Bounded by `settings.maxPostReviewFixes` and the
|
||||
* per-task `postReviewFixCount` so a persistently-failing verifier cannot
|
||||
* ping-pong a task forever.
|
||||
*
|
||||
* @returns Number of tasks sent back for fix
|
||||
*/
|
||||
async recoverReviewTasksWithFailedPreMergeSteps(): Promise<number> {
|
||||
const recoverFn = this.options.recoverFailedPreMergeStep;
|
||||
if (!recoverFn) return 0;
|
||||
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
const maxFixes = settings.maxPostReviewFixes ?? 1;
|
||||
if (!Number.isFinite(maxFixes) || maxFixes <= 0) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review" });
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
|
||||
const candidates = tasks.filter((task) => {
|
||||
if (task.column !== "in-review") return false;
|
||||
if (task.paused) return false;
|
||||
// Preserve terminal/human-handoff statuses (failed, awaiting-user-review,
|
||||
// merging, etc.). Only revive tasks that are otherwise idle.
|
||||
if (task.status) return false;
|
||||
if (executingIds.has(task.id)) return false;
|
||||
if ((task.postReviewFixCount ?? 0) >= maxFixes) return false;
|
||||
|
||||
// Must have at least one failed pre-merge workflow step result.
|
||||
const hasFailedPreMerge = (task.workflowStepResults ?? []).some(
|
||||
(r) => (r.phase || "pre-merge") === "pre-merge" && r.status === "failed",
|
||||
);
|
||||
if (!hasFailedPreMerge) return false;
|
||||
|
||||
// Merge must be blocked *specifically* by the failed pre-merge step —
|
||||
// not by an unrelated condition (incomplete steps, etc.) that is
|
||||
// already handled by a dedicated scan.
|
||||
const blocker = getTaskMergeBlocker(task);
|
||||
if (blocker !== "task has failed pre-merge workflow steps") return false;
|
||||
|
||||
// The retry flow injects into PROMPT.md + re-executes on the worktree.
|
||||
// If the worktree was cleaned up we can't reliably resume here; leave
|
||||
// such tasks for human intervention.
|
||||
if (!task.worktree) return false;
|
||||
|
||||
return true;
|
||||
});
|
||||
|
||||
if (candidates.length === 0) return 0;
|
||||
|
||||
log.warn(`Found ${candidates.length} in-review task(s) with failed pre-merge workflow steps — auto-reviving`);
|
||||
|
||||
let recovered = 0;
|
||||
for (const task of candidates) {
|
||||
const nextCount = (task.postReviewFixCount ?? 0) + 1;
|
||||
try {
|
||||
// Increment the counter BEFORE delegating so that even if the
|
||||
// executor path crashes or races, the budget is still consumed and
|
||||
// we can't enter an infinite revival loop.
|
||||
await this.store.updateTask(task.id, { postReviewFixCount: nextCount });
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-reviving in-review task with failed pre-merge workflow step (attempt ${nextCount}/${maxFixes})`,
|
||||
);
|
||||
const sentBack = await recoverFn(task);
|
||||
if (sentBack) {
|
||||
log.log(`Revived ${task.id}: sent back for fix (${nextCount}/${maxFixes})`);
|
||||
recovered++;
|
||||
} else {
|
||||
log.warn(`Revival of ${task.id} was skipped by executor — budget already consumed`);
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Failed to revive ${task.id}: ${errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (recovered > 0) {
|
||||
log.log(`Auto-revived ${recovered} in-review task(s) for pre-merge workflow step fix`);
|
||||
}
|
||||
return recovered;
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Failed pre-merge workflow step revival failed: ${errorMessage}`);
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover tasks that reached `in-review` while a task step was still marked
|
||||
* pending/in-progress. These tasks are not tracked by StuckTaskDetector
|
||||
|
||||
Reference in New Issue
Block a user