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:
gsxdsm
2026-04-17 23:28:32 -07:00
parent a8a4d0a3d1
commit 074868dd59
14 changed files with 390 additions and 24 deletions

View File

@@ -51,7 +51,7 @@ describe("TaskStore task documents", () => {
expect(tableNames.has("task_documents")).toBe(true); expect(tableNames.has("task_documents")).toBe(true);
expect(tableNames.has("task_document_revisions")).toBe(true); expect(tableNames.has("task_document_revisions")).toBe(true);
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
const index = db const index = db
.prepare( .prepare(

View File

@@ -119,7 +119,7 @@ describe("Database", () => {
}); });
it("seeds schema version", () => { it("seeds schema version", () => {
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
}); });
it("seeds lastModified", () => { it("seeds lastModified", () => {
@@ -142,7 +142,7 @@ describe("Database", () => {
it("is idempotent - calling init() twice does not fail", () => { it("is idempotent - calling init() twice does not fail", () => {
expect(() => db.init()).not.toThrow(); expect(() => db.init()).not.toThrow();
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
}); });
it("does not overwrite existing config on re-init", () => { it("does not overwrite existing config on re-init", () => {
@@ -749,7 +749,7 @@ describe("schema migrations", () => {
db.init(); db.init();
// Verify version bumped to 29 (includes v1→v2 through v26→v29) // Verify version bumped to 29 (includes v1→v2 through v26→v29)
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
// Verify new columns exist and existing data is intact // Verify new columns exist and existing data is intact
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
@@ -774,11 +774,11 @@ describe("schema migrations", () => {
const db = new Database(kbDir); const db = new Database(kbDir);
db.init(); db.init();
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
// Re-init should not fail // Re-init should not fail
db.init(); db.init();
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
db.close(); db.close();
}); });
@@ -794,7 +794,7 @@ describe("schema migrations", () => {
db.init(); db.init();
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>; const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>;
expect(tables).toEqual([{ name: "agentRatings" }]); expect(tables).toEqual([{ name: "agentRatings" }]);
@@ -818,7 +818,7 @@ describe("schema migrations", () => {
db.init(); db.init();
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>; const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>;
expect(tables).toEqual([{ name: "mission_events" }]); expect(tables).toEqual([{ name: "mission_events" }]);
@@ -922,7 +922,7 @@ describe("schema migrations", () => {
db.init(); db.init();
// Verify version bumped to 29 // Verify version bumped to 29
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
// Verify new columns exist and existing data is intact // Verify new columns exist and existing data is intact
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>; const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
@@ -1291,7 +1291,7 @@ describe("createDatabase factory", () => {
const db = createDatabase(kbDir); const db = createDatabase(kbDir);
db.init(); db.init();
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
expect(db.getLastModified()).toBeGreaterThan(0); expect(db.getLastModified()).toBeGreaterThan(0);
db.close(); db.close();

View File

@@ -59,7 +59,7 @@ export function fromJson<T>(json: string | null | undefined): T | undefined {
// ── Schema Definition ──────────────────────────────────────────────── // ── Schema Definition ────────────────────────────────────────────────
const SCHEMA_VERSION = 37; const SCHEMA_VERSION = 38;
function normalizeTaskComments( function normalizeTaskComments(
steeringComments: SteeringComment[] | undefined, steeringComments: SteeringComment[] | undefined,
@@ -1470,6 +1470,15 @@ export class Database {
}); });
} }
if (version < 38) {
// Tracks self-healing auto-revivals of in-review tasks whose pre-merge
// workflow steps failed. Bounded by settings.maxPostReviewFixes so a
// persistently-failing verifier cannot ping-pong a task forever.
this.applyMigration(38, () => {
this.addColumnIfMissing("tasks", "postReviewFixCount", "INTEGER DEFAULT 0");
});
}
} }
/** /**

View File

@@ -776,7 +776,7 @@ describe("Migration: pre-33 DB upgrade", () => {
// Step 1: Create a fresh database at v33 (runs all migrations up to 33) // Step 1: Create a fresh database at v33 (runs all migrations up to 33)
const db1 = createDatabase(legacyDir); const db1 = createDatabase(legacyDir);
db1.init(); db1.init();
expect(db1.getSchemaVersion()).toBe(37); expect(db1.getSchemaVersion()).toBe(38);
db1.close(); db1.close();
// Step 2: Manually downgrade to version 32 and drop insight tables // Step 2: Manually downgrade to version 32 and drop insight tables
@@ -811,7 +811,7 @@ describe("Migration: pre-33 DB upgrade", () => {
expect(tableNamesBefore).not.toContain("project_insight_runs"); expect(tableNamesBefore).not.toContain("project_insight_runs");
// Now run init — this triggers the v32→v33 migration // Now run init — this triggers the v32→v33 migration
db3.init(); db3.init();
expect(db3.getSchemaVersion()).toBe(37); expect(db3.getSchemaVersion()).toBe(38);
// Step 4: Verify insight tables exist after migration // Step 4: Verify insight tables exist after migration
const tablesAfter = db3.prepare( const tablesAfter = db3.prepare(
@@ -842,12 +842,12 @@ describe("Migration: pre-33 DB upgrade", () => {
try { try {
const db1 = createDatabase(testDir); const db1 = createDatabase(testDir);
db1.init(); db1.init();
expect(db1.getSchemaVersion()).toBe(37); expect(db1.getSchemaVersion()).toBe(38);
db1.close(); db1.close();
const db2 = createDatabase(testDir); const db2 = createDatabase(testDir);
expect(() => db2.init()).not.toThrow(); expect(() => db2.init()).not.toThrow();
expect(db2.getSchemaVersion()).toBe(37); expect(db2.getSchemaVersion()).toBe(38);
db2.close(); db2.close();
} finally { } finally {
rmSync(testDir, { recursive: true, force: true }); rmSync(testDir, { recursive: true, force: true });

View File

@@ -2543,8 +2543,8 @@ describe("MissionStore", () => {
// ── Loop State & Validator Run Schema Tests ─────────────────────────── // ── Loop State & Validator Run Schema Tests ───────────────────────────
describe("Loop State & Validator Run Schema (v31)", () => { describe("Loop State & Validator Run Schema (v31)", () => {
it("schema version is 37 after migration", () => { it("schema version is 38 after migration", () => {
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
}); });
it("mission_features table has loop state columns", () => { it("mission_features table has loop state columns", () => {

View File

@@ -739,7 +739,7 @@ describe("RoadmapStore", () => {
describe("schema version", () => { describe("schema version", () => {
it("schema version is 32 after init", () => { it("schema version is 32 after init", () => {
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
}); });
}); });

View File

@@ -465,7 +465,7 @@ describe("Run Audit", () => {
}); });
it("schema version is bumped to 28", () => { it("schema version is bumped to 28", () => {
expect(db.getSchemaVersion()).toBe(37); expect(db.getSchemaVersion()).toBe(38);
}); });
}); });
}); });

View File

@@ -103,6 +103,7 @@ export const DEFAULT_PROJECT_SETTINGS = {
autoUnpauseBaseDelayMs: 300_000, autoUnpauseBaseDelayMs: 300_000,
autoUnpauseMaxDelayMs: 3_600_000, autoUnpauseMaxDelayMs: 3_600_000,
maxStuckKills: 6, maxStuckKills: 6,
maxPostReviewFixes: 1,
maxSpawnedAgentsPerParent: 5, maxSpawnedAgentsPerParent: 5,
maxSpawnedAgentsGlobal: 20, maxSpawnedAgentsGlobal: 20,
maintenanceIntervalMs: 900_000, maintenanceIntervalMs: 900_000,

View File

@@ -277,6 +277,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
mergeRetries: row.mergeRetries ?? undefined, mergeRetries: row.mergeRetries ?? undefined,
workflowStepRetries: row.workflowStepRetries ?? undefined, workflowStepRetries: row.workflowStepRetries ?? undefined,
stuckKillCount: row.stuckKillCount ?? undefined, stuckKillCount: row.stuckKillCount ?? undefined,
postReviewFixCount: row.postReviewFixCount ?? undefined,
recoveryRetryCount: row.recoveryRetryCount ?? undefined, recoveryRetryCount: row.recoveryRetryCount ?? undefined,
nextRecoveryAt: row.nextRecoveryAt || undefined, nextRecoveryAt: row.nextRecoveryAt || undefined,
error: row.error || undefined, error: row.error || undefined,
@@ -527,7 +528,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
"modelPresetId", "modelProvider", "modelId", "modelPresetId", "modelProvider", "modelId",
"validatorModelProvider", "validatorModelId", "validatorModelProvider", "validatorModelId",
"planningModelProvider", "planningModelId", "planningModelProvider", "planningModelId",
"mergeRetries", "workflowStepRetries", "stuckKillCount", "recoveryRetryCount", "nextRecoveryAt", "mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "nextRecoveryAt",
"error", "summary", "thinkingLevel", "error", "summary", "thinkingLevel",
"createdAt", "updatedAt", "columnMovedAt", "createdAt", "updatedAt", "columnMovedAt",
"dependencies", "steps", "comments", "workflowStepResults", "steeringComments", "dependencies", "steps", "comments", "workflowStepResults", "steeringComments",
@@ -545,7 +546,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
"modelPresetId", "modelProvider", "modelId", "modelPresetId", "modelProvider", "modelId",
"validatorModelProvider", "validatorModelId", "validatorModelProvider", "validatorModelId",
"planningModelProvider", "planningModelId", "planningModelProvider", "planningModelId",
"mergeRetries", "workflowStepRetries", "stuckKillCount", "recoveryRetryCount", "nextRecoveryAt", "mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "nextRecoveryAt",
"error", "summary", "thinkingLevel", "error", "summary", "thinkingLevel",
"createdAt", "updatedAt", "columnMovedAt", "createdAt", "updatedAt", "columnMovedAt",
"dependencies", "steps", "attachments", "steeringComments", "dependencies", "steps", "attachments", "steeringComments",
@@ -586,14 +587,14 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
id, title, description, "column", status, size, reviewLevel, currentStep, id, title, description, "column", status, size, reviewLevel, currentStep,
worktree, blockedBy, paused, baseBranch, branch, baseCommitSha, modelPresetId, modelProvider, worktree, blockedBy, paused, baseBranch, branch, baseCommitSha, modelPresetId, modelProvider,
modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, mergeRetries, modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, mergeRetries,
workflowStepRetries, stuckKillCount, recoveryRetryCount, nextRecoveryAt, error, workflowStepRetries, stuckKillCount, postReviewFixCount, recoveryRetryCount, nextRecoveryAt, error,
summary, thinkingLevel, createdAt, updatedAt, columnMovedAt, summary, thinkingLevel, createdAt, updatedAt, columnMovedAt,
dependencies, steps, log, attachments, steeringComments, dependencies, steps, log, attachments, steeringComments,
comments, workflowStepResults, prInfo, issueInfo, mergeDetails, comments, workflowStepResults, prInfo, issueInfo, mergeDetails,
breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, assigneeUserId, checkedOutBy, checkedOutAt breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, assigneeUserId, checkedOutBy, checkedOutAt
) VALUES ( ) VALUES (
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
) )
`).run( `).run(
task.id, task.id,
@@ -620,6 +621,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
task.mergeRetries ?? null, task.mergeRetries ?? null,
task.workflowStepRetries ?? null, task.workflowStepRetries ?? null,
task.stuckKillCount ?? 0, task.stuckKillCount ?? 0,
task.postReviewFixCount ?? 0,
task.recoveryRetryCount ?? null, task.recoveryRetryCount ?? null,
task.nextRecoveryAt ?? null, task.nextRecoveryAt ?? null,
task.error ?? null, task.error ?? null,
@@ -2043,7 +2045,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
async updateTask( async updateTask(
id: string, id: string,
updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null }, updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
runContext?: RunMutationContext, runContext?: RunMutationContext,
): Promise<Task> { ): Promise<Task> {
return this.withTaskLock(id, async () => { return this.withTaskLock(id, async () => {
@@ -2143,6 +2145,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
} else if (updates.stuckKillCount !== undefined) { } else if (updates.stuckKillCount !== undefined) {
task.stuckKillCount = updates.stuckKillCount; task.stuckKillCount = updates.stuckKillCount;
} }
if (updates.postReviewFixCount === null) {
task.postReviewFixCount = undefined;
} else if (updates.postReviewFixCount !== undefined) {
task.postReviewFixCount = updates.postReviewFixCount;
}
if (updates.recoveryRetryCount === null) { if (updates.recoveryRetryCount === null) {
task.recoveryRetryCount = undefined; task.recoveryRetryCount = undefined;
} else if (updates.recoveryRetryCount !== undefined) { } else if (updates.recoveryRetryCount !== undefined) {

View File

@@ -723,6 +723,12 @@ export interface Task {
* Incremented by the self-healing manager on each stuck kill. When this reaches * Incremented by the self-healing manager on each stuck kill. When this reaches
* `maxStuckKills`, the task is marked as permanently failed instead of re-queued. */ * `maxStuckKills`, the task is marked as permanently failed instead of re-queued. */
stuckKillCount?: number; stuckKillCount?: number;
/** Number of times the self-healing manager has auto-revived this task from
* `in-review` after a failed pre-merge workflow step. Incremented each time the
* `recoverReviewTasksWithFailedPreMergeSteps` scan sends the task back with the
* failure feedback injected. Capped by `maxPostReviewFixes`; when exhausted the
* task remains parked in `in-review` for human intervention. */
postReviewFixCount?: number;
/** Number of bounded recovery retry attempts for transient executor/triage failures. /** Number of bounded recovery retry attempts for transient executor/triage failures.
* Distinct from `mergeRetries` (merge-conflict-specific). Incremented by the * Distinct from `mergeRetries` (merge-conflict-specific). Incremented by the
* recovery-policy module on each recoverable failure; cleared when work restarts * recovery-policy module on each recoverable failure; cleared when work restarts
@@ -1185,6 +1191,11 @@ export interface ProjectSettings {
/** Maximum number of times the stuck-task detector can kill and re-queue a task /** Maximum number of times the stuck-task detector can kill and re-queue a task
* before it is marked as permanently failed. Default: 6. */ * before it is marked as permanently failed. Default: 6. */
maxStuckKills?: number; maxStuckKills?: number;
/** Maximum number of times the self-healing manager may auto-revive a task parked
* in `in-review` with a failed pre-merge workflow step. Each revival injects the
* failure feedback into `PROMPT.md`, resets steps, and sends the task back through
* the normal todo → in-progress flow. Set to 0 to disable. Default: 1. */
maxPostReviewFixes?: number;
/** Maximum number of child agents a single parent agent can spawn. /** Maximum number of child agents a single parent agent can spawn.
* Limits the fan-out per executor task to prevent resource exhaustion. * Limits the fan-out per executor task to prevent resource exhaustion.
* Default: 5. */ * Default: 5. */

View File

@@ -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). * Resume orphaned in-progress tasks (e.g., after crash/restart).
* Call once after engine startup. * Call once after engine startup.

View File

@@ -575,6 +575,7 @@ export class InProcessRuntime
this.selfHealingManager = new SelfHealingManager(this.taskStore, { this.selfHealingManager = new SelfHealingManager(this.taskStore, {
rootDir: this.config.workingDirectory, rootDir: this.config.workingDirectory,
recoverCompletedTask: (task) => this.executor.recoverCompletedTask(task), recoverCompletedTask: (task) => this.executor.recoverCompletedTask(task),
recoverFailedPreMergeStep: (task) => this.executor.recoverFailedPreMergeWorkflowStep(task),
getExecutingTaskIds: () => this.executor.getExecutingTaskIds(), getExecutingTaskIds: () => this.executor.getExecutingTaskIds(),
recoverApprovedTriageTask: (task) => this.triageProcessor?.recoverApprovedTask(task) ?? Promise.resolve(false), recoverApprovedTriageTask: (task) => this.triageProcessor?.recoverApprovedTask(task) ?? Promise.resolve(false),
getSpecifyingTaskIds: () => this.triageProcessor?.getProcessingTaskIds() ?? new Set<string>(), getSpecifyingTaskIds: () => this.triageProcessor?.getProcessingTaskIds() ?? new Set<string>(),

View File

@@ -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", () => { describe("recoverOrphanedExecutions", () => {
it("requeues in-progress tasks whose reserved worktree is missing", async () => { it("requeues in-progress tasks whose reserved worktree is missing", async () => {
const getExecuting = vi.fn().mockReturnValue(new Set<string>()); const getExecuting = vi.fn().mockReturnValue(new Set<string>());

View File

@@ -59,6 +59,14 @@ export interface SelfHealingOptions {
* Called before recovery checks so stale entries don't block recovery. * Called before recovery checks so stale entries don't block recovery.
*/ */
evictStaleTriageProcessing?: () => Set<string>; 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; const APPROVED_TRIAGE_RECOVERY_GRACE_MS = 60_000;
@@ -143,6 +151,7 @@ export class SelfHealingManager {
await this.recoverNoProgressNoTaskDoneFailures(); await this.recoverNoProgressNoTaskDoneFailures();
await this.recoverCompletedTasks(); await this.recoverCompletedTasks();
await this.recoverStaleIncompleteReviewTasks(); await this.recoverStaleIncompleteReviewTasks();
await this.recoverReviewTasksWithFailedPreMergeSteps();
await this.recoverInterruptedMergingTasks(); await this.recoverInterruptedMergingTasks();
await this.recoverMisclassifiedFailures(); await this.recoverMisclassifiedFailures();
await this.recoverOrphanedExecutions(); await this.recoverOrphanedExecutions();
@@ -477,6 +486,7 @@ export class SelfHealingManager {
const batch2Results = await Promise.allSettled([ const batch2Results = await Promise.allSettled([
this.recoverCompletedTasks(), this.recoverCompletedTasks(),
this.recoverStaleIncompleteReviewTasks(), this.recoverStaleIncompleteReviewTasks(),
this.recoverReviewTasksWithFailedPreMergeSteps(),
this.recoverInterruptedMergingTasks(), this.recoverInterruptedMergingTasks(),
this.recoverMergeableReviewTasks(), this.recoverMergeableReviewTasks(),
this.recoverMergedReviewTasks(), 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 * Recover tasks that reached `in-review` while a task step was still marked
* pending/in-progress. These tasks are not tracked by StuckTaskDetector * pending/in-progress. These tasks are not tracked by StuckTaskDetector