feat(self-healing): add ghost-review fallback recovery

Add recoverGhostReviewTasks as a final-fallback scan in the maintenance
loop. Catches any in-review task that fell through every more-specific
recovery scan and has been idle past taskStuckTimeoutMs, kicks it back
to todo with transient status cleared. Worktree state is intentionally
ignored — the executor recreates as needed. Preserves human-handoff and
active-merge statuses; rate-limited naturally by updatedAt refresh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-04-30 04:41:56 -07:00
parent f9c08dea7a
commit 774569da86
2 changed files with 227 additions and 0 deletions

View File

@@ -2050,6 +2050,147 @@ describe("SelfHealingManager", () => {
}); });
}); });
describe("recoverGhostReviewTasks", () => {
it("kicks idle in-review tasks back to todo regardless of status or worktree", async () => {
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 1_000,
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{
id: "FN-9001",
column: "in-review",
paused: false,
status: "failed",
worktree: undefined,
updatedAt: new Date(Date.now() - 10_000).toISOString(),
steps: [],
workflowStepResults: [],
mergeDetails: undefined,
log: [],
},
]);
const result = await managerWithRecovery.recoverGhostReviewTasks();
expect(result).toBe(1);
expect(store.updateTask).toHaveBeenCalledWith("FN-9001", { status: null, error: null });
expect(store.moveTask).toHaveBeenCalledWith("FN-9001", "todo");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-9001",
expect.stringContaining("idle past stuck-task timeout"),
);
managerWithRecovery.stop();
});
it("preserves human-handoff and active-merge statuses", async () => {
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 1_000,
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{ id: "FN-A", column: "in-review", paused: false, status: "awaiting-user-review", updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
{ id: "FN-B", column: "in-review", paused: false, status: "awaiting-approval", updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
{ id: "FN-C", column: "in-review", paused: false, status: "merging", updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
{ id: "FN-D", column: "in-review", paused: false, status: "merging-pr", updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
]);
const result = await managerWithRecovery.recoverGhostReviewTasks();
expect(result).toBe(0);
expect(store.moveTask).not.toHaveBeenCalled();
managerWithRecovery.stop();
});
it("ignores fresh in-review tasks within the timeout window", async () => {
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 60_000,
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{ id: "FN-9002", column: "in-review", paused: false, status: null, updatedAt: new Date().toISOString(), steps: [], log: [] },
]);
const result = await managerWithRecovery.recoverGhostReviewTasks();
expect(result).toBe(0);
expect(store.moveTask).not.toHaveBeenCalled();
managerWithRecovery.stop();
});
it("skips paused, currently-executing, and merge-confirmed tasks", async () => {
const getExecuting = vi.fn().mockReturnValue(new Set(["FN-EXEC"]));
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
getExecutingTaskIds: getExecuting,
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 1_000,
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{ id: "FN-PAUSED", column: "in-review", paused: true, status: null, updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
{ id: "FN-EXEC", column: "in-review", paused: false, status: null, updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
{ id: "FN-MERGED", column: "in-review", paused: false, status: null, mergeDetails: { mergeConfirmed: true }, updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
]);
const result = await managerWithRecovery.recoverGhostReviewTasks();
expect(result).toBe(0);
expect(store.moveTask).not.toHaveBeenCalled();
managerWithRecovery.stop();
});
it("no-ops when stuck timeout is disabled or engine is paused", async () => {
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 0,
});
expect(await managerWithRecovery.recoverGhostReviewTasks()).toBe(0);
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 1_000,
enginePaused: true,
});
expect(await managerWithRecovery.recoverGhostReviewTasks()).toBe(0);
expect(store.moveTask).not.toHaveBeenCalled();
managerWithRecovery.stop();
});
it("skips updateTask when there is no transient status to clear", async () => {
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
taskStuckTimeoutMs: 1_000,
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{ id: "FN-9003", column: "in-review", paused: false, status: null, updatedAt: new Date(Date.now() - 10_000).toISOString(), steps: [], log: [] },
]);
const result = await managerWithRecovery.recoverGhostReviewTasks();
expect(result).toBe(1);
expect(store.updateTask).not.toHaveBeenCalled();
expect(store.moveTask).toHaveBeenCalledWith("FN-9003", "todo");
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>());
@@ -2711,6 +2852,7 @@ describe("maintenance cycle concurrency", () => {
(vi.spyOn(manager as any, "recoverOrphanedExecutions").mockResolvedValue(0) as any); (vi.spyOn(manager as any, "recoverOrphanedExecutions").mockResolvedValue(0) as any);
(vi.spyOn(manager as any, "recoverApprovedTriageTasks").mockResolvedValue(0) as any); (vi.spyOn(manager as any, "recoverApprovedTriageTasks").mockResolvedValue(0) as any);
(vi.spyOn(manager as any, "recoverOrphanedPlanningTasks").mockResolvedValue(0) as any); (vi.spyOn(manager as any, "recoverOrphanedPlanningTasks").mockResolvedValue(0) as any);
(vi.spyOn(manager as any, "recoverGhostReviewTasks").mockResolvedValue(0) as any);
(vi.spyOn(manager as any, "archiveStaleDoneTasks").mockResolvedValue(0) as any); (vi.spyOn(manager as any, "archiveStaleDoneTasks").mockResolvedValue(0) as any);
await (manager as any).runMaintenance(); await (manager as any).runMaintenance();

View File

@@ -81,6 +81,14 @@ const APPROVED_TRIAGE_RECOVERY_GRACE_MS = 60_000;
const ORPHANED_EXECUTION_RECOVERY_GRACE_MS = 60_000; const ORPHANED_EXECUTION_RECOVERY_GRACE_MS = 60_000;
const ACTIVE_MERGE_STATUSES = new Set(["merging", "merging-pr"]); const ACTIVE_MERGE_STATUSES = new Set(["merging", "merging-pr"]);
const NON_TERMINAL_STEP_STATUSES = new Set(["pending", "in-progress"]); const NON_TERMINAL_STEP_STATUSES = new Set(["pending", "in-progress"]);
/** Statuses that represent an explicit human-handoff or active merge —
* the ghost-review fallback must not disturb tasks parked in these states. */
const GHOST_REVIEW_PRESERVED_STATUSES = new Set([
"awaiting-user-review",
"awaiting-approval",
"merging",
"merging-pr",
]);
/** /**
* Longer grace period for tasks that still have a worktree on disk. * Longer grace period for tasks that still have a worktree on disk.
* This avoids racing with `executor.resumeOrphaned()` which runs on * This avoids racing with `executor.resumeOrphaned()` which runs on
@@ -620,6 +628,7 @@ export class SelfHealingManager {
{ name: "recover-orphaned-executions", fn: () => this.recoverOrphanedExecutions() }, { name: "recover-orphaned-executions", fn: () => this.recoverOrphanedExecutions() },
{ name: "recover-approved-triage", fn: () => this.recoverApprovedTriageTasks() }, { name: "recover-approved-triage", fn: () => this.recoverApprovedTriageTasks() },
{ name: "recover-orphaned-planning", fn: () => this.recoverOrphanedPlanningTasks() }, { name: "recover-orphaned-planning", fn: () => this.recoverOrphanedPlanningTasks() },
{ name: "recover-ghost-review", fn: () => this.recoverGhostReviewTasks() },
]; ];
for (const fn of batch2Fns) { for (const fn of batch2Fns) {
try { try {
@@ -988,6 +997,82 @@ export class SelfHealingManager {
} }
} }
/**
* Final-fallback recovery for `in-review` tasks that fell through every other
* scan and have sat untouched longer than `taskStuckTimeoutMs`.
*
* The other review-recovery scans each require a specific shape (failed
* pre-merge step, incomplete steps, mergeable + worktree present, confirmed
* merge, transient merge status). A task whose state doesn't match any of
* those shapes — e.g. `status: "failed"` with no failed pre-merge step, or
* any other unanticipated combination — has no recovery path and stays
* silent in review forever.
*
* This catch-all kicks any such task back to `todo`, clearing transient
* `status` so the scheduler can pick it up. Worktree state is intentionally
* not considered: the executor will recreate one if needed.
*
* Preserved statuses (skipped):
* - `awaiting-user-review`, `awaiting-approval`: explicit human handoff
* - `merging`, `merging-pr`: handled by `recoverInterruptedMergingTasks`
*
* Rate-limiting comes from the `updatedAt >= taskStuckTimeoutMs` gate —
* each kick refreshes `updatedAt`, so a task that re-enters review and gets
* stuck again can only be kicked once per `taskStuckTimeoutMs` window.
*
* @returns Number of tasks kicked back to todo
*/
async recoverGhostReviewTasks(): Promise<number> {
try {
const settings = await this.store.getSettings();
const timeoutMs = settings.taskStuckTimeoutMs;
if (!timeoutMs || timeoutMs <= 0) return 0;
if (settings.globalPause || settings.enginePaused) return 0;
const now = Date.now();
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
const ghosts = tasks.filter((task) =>
task.column === "in-review" &&
!task.paused &&
!executingIds.has(task.id) &&
!(task.status && GHOST_REVIEW_PRESERVED_STATUSES.has(task.status)) &&
// Confirmed merges belong in `done` (handled by `recoverMergedReviewTasks`).
task.mergeDetails?.mergeConfirmed !== true &&
now - new Date(task.updatedAt).getTime() >= timeoutMs
);
if (ghosts.length === 0) return 0;
log.warn(`Found ${ghosts.length} ghost in-review task(s) — kicking back to todo`);
let recovered = 0;
for (const task of ghosts) {
try {
if (task.status) {
await this.store.updateTask(task.id, { status: null, error: null });
}
await this.store.logEntry(
task.id,
"Auto-recovered: in-review task idle past stuck-task timeout — kicked back to todo",
);
await this.store.moveTask(task.id, "todo");
log.log(`Kicked ghost review task ${task.id} back to todo`);
recovered++;
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
log.error(`Failed to kick ghost review task ${task.id}: ${errorMessage}`);
}
}
return recovered;
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
log.error(`Ghost review recovery failed: ${errorMessage}`);
return 0;
}
}
/** /**
* Recover stale `in-review` tasks left in a transient merge status. * Recover stale `in-review` tasks left in a transient merge status.
* *