From 5f299350566453fcd46f8541f16d3585ee5dbfa3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 17 Aug 2026 15:10:54 -0700 Subject: [PATCH] fix: hold task checkouts through progress-preserving recovery rebounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten self-healing rebounds passed preserveProgress without preserveWorktree, so the reopen-into-planning move hook cleared task.worktree and the idle sweep reaped the checkout (uncommitted work included) — the same loss mechanism as the in-review branch-rebind incident. Those rebounds (stuck-loop park, undeclared-column rehome, finalize-integrity blocks, stale-incomplete-review, ghost-review, terminal-failure retry, legacy rehome, partial-progress) now pass preserveWorktree: true; deliberate discards (branch proven merged, zero unique commits, worktree already missing) carry an explicit worktree-discard-intended marker. A new static ratchet test requires every preserveProgress rebound in self-healing.ts to either preserve the worktree or carry the marker. Co-Authored-By: Claude Fable 5 --- .changeset/rebound-preserve-worktree-class.md | 7 +++ ...self-healing-paused-abort-recovery.test.ts | 2 +- ...-healing-preserve-worktree-ratchet.test.ts | 43 +++++++++++++++++++ .../engine/src/__tests__/self-healing.test.ts | 14 +++--- packages/engine/src/self-healing.ts | 21 ++++++--- 5 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 .changeset/rebound-preserve-worktree-class.md create mode 100644 packages/engine/src/__tests__/self-healing-preserve-worktree-ratchet.test.ts diff --git a/.changeset/rebound-preserve-worktree-class.md b/.changeset/rebound-preserve-worktree-class.md new file mode 100644 index 0000000000..1e1cdb3545 --- /dev/null +++ b/.changeset/rebound-preserve-worktree-class.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Progress-preserving recovery rebounds now keep the task's checkout instead of leaving it to the idle sweep. +category: fix +dev: "Ten self-healing rebounds gained `preserveWorktree: true`; deliberate discards carry a `worktree-discard-intended` marker enforced by a new ratchet test." diff --git a/packages/engine/src/__tests__/self-healing-paused-abort-recovery.test.ts b/packages/engine/src/__tests__/self-healing-paused-abort-recovery.test.ts index 887b42a3b1..75cf7e879b 100644 --- a/packages/engine/src/__tests__/self-healing-paused-abort-recovery.test.ts +++ b/packages/engine/src/__tests__/self-healing-paused-abort-recovery.test.ts @@ -135,7 +135,7 @@ describe("recoverPausedAbortFailures", () => { expect(store.moveTask).toHaveBeenCalledWith( "FN-7001", "todo", - { preserveProgress: true, moveSource: "engine", recoveryRehome: true }, + { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }, ); expect(store.updateTask).toHaveBeenNthCalledWith(2, "FN-7001", { workflowTransitionNotification: { diff --git a/packages/engine/src/__tests__/self-healing-preserve-worktree-ratchet.test.ts b/packages/engine/src/__tests__/self-healing-preserve-worktree-ratchet.test.ts new file mode 100644 index 0000000000..9eb4a7c0af --- /dev/null +++ b/packages/engine/src/__tests__/self-healing-preserve-worktree-ratchet.test.ts @@ -0,0 +1,43 @@ +/* +FNXC:WorktreeLifecycleHold 2026-08-17-22:04: +Ratchet for the worktree-held-across-lifecycle invariant (see +reliability-interactions/worktree-lifecycle-certification.test.ts for the incident anatomy). + +A self-healing rebound that passes `preserveProgress: true` intends to KEEP the task's work, +but the reopen-into-planning move hook clears `task.worktree` unless the caller also passes +`preserveWorktree: true` — and a cleared pointer makes the checkout invisible to +`scanIdleWorktrees`' active set, so the idle sweep reaps the directory (uncommitted work +included). Six sweeps silently dropped checkouts this way. + +Rule: every `preserveProgress: true` in self-healing.ts must sit within a few lines of either +`preserveWorktree` (the default: progress-preserving rebounds hold the checkout) or an explicit +`worktree-discard-intended: ` marker (only for rebounds that have PROVEN the checkout +holds nothing — branch already merged, zero unique commits, or the worktree is already gone). +Static source scan: no git, no store, no timers. +*/ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +const WINDOW = 6; + +describe("self-healing preserveProgress rebounds hold the worktree", () => { + it("every preserveProgress site preserves the worktree or carries a worktree-discard-intended marker", () => { + const selfHealingPath = join(dirname(fileURLToPath(import.meta.url)), "..", "self-healing.ts"); + const lines = readFileSync(selfHealingPath, "utf-8").split("\n"); + const violations: string[] = []; + lines.forEach((line, i) => { + if (!line.includes("preserveProgress: true")) return; + const start = Math.max(0, i - WINDOW); + const end = Math.min(lines.length, i + WINDOW + 1); + const window = lines.slice(start, end).join("\n"); + if (window.includes("preserveWorktree") || window.includes("worktree-discard-intended")) return; + violations.push(`self-healing.ts:${i + 1}: ${line.trim()}`); + }); + expect( + violations, + "preserveProgress rebound without preserveWorktree or a worktree-discard-intended marker — a progress-preserving rebound that drops the checkout makes it reap-bait", + ).toEqual([]); + }); +}); diff --git a/packages/engine/src/__tests__/self-healing.test.ts b/packages/engine/src/__tests__/self-healing.test.ts index e6b58a8081..3972207230 100644 --- a/packages/engine/src/__tests__/self-healing.test.ts +++ b/packages/engine/src/__tests__/self-healing.test.ts @@ -526,6 +526,7 @@ describe("SelfHealingManager", () => { })); expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true, + preserveWorktree: true, preserveStatus: true, moveSource: "engine", recoveryRehome: true, @@ -594,6 +595,7 @@ describe("SelfHealingManager", () => { })); expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true, + preserveWorktree: true, preserveStatus: true, moveSource: "engine", recoveryRehome: true, @@ -695,6 +697,7 @@ describe("SelfHealingManager", () => { expect(result).toBe(false); expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true, + preserveWorktree: true, preserveStatus: true, moveSource: "engine", recoveryRehome: true, @@ -4564,7 +4567,7 @@ describe("SelfHealingManager", () => { "FN-2164", expect.stringContaining("Auto-retry 1/3"), ); - expect(store.moveTask).toHaveBeenCalledWith("FN-2164", "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + expect(store.moveTask).toHaveBeenCalledWith("FN-2164", "todo", { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); managerWithRecovery.stop(); }); @@ -6600,7 +6603,7 @@ describe("SelfHealingManager", () => { "FN-1572", expect.stringContaining("in-review task still had incomplete steps"), ); - expect(store.moveTask).toHaveBeenCalledWith("FN-1572", "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + expect(store.moveTask).toHaveBeenCalledWith("FN-1572", "todo", { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); managerWithRecovery.stop(); }); @@ -6659,7 +6662,7 @@ describe("SelfHealingManager", () => { const result = await managerWithRecovery.recoverStaleIncompleteReviewTasks(); expect(result).toBe(1); - expect(store.moveTask).toHaveBeenCalledWith("FN-407-test-1", "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + expect(store.moveTask).toHaveBeenCalledWith("FN-407-test-1", "todo", { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); managerWithRecovery.stop(); }); @@ -6687,7 +6690,7 @@ describe("SelfHealingManager", () => { const result = await managerWithRecovery.recoverStaleIncompleteReviewTasks(); expect(result).toBe(1); - expect(store.moveTask).toHaveBeenCalledWith("FN-407-test-2", "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + expect(store.moveTask).toHaveBeenCalledWith("FN-407-test-2", "todo", { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); managerWithRecovery.stop(); }); @@ -6938,6 +6941,7 @@ describe("SelfHealingManager", () => { expect(result).toBe(1); expect(store.moveTask).toHaveBeenCalledWith("FN-7229", "todo", { preserveProgress: true, + preserveWorktree: true, moveSource: "engine", recoveryRehome: true, }); @@ -9000,7 +9004,7 @@ describe("SelfHealingManager", () => { expect(result).toBe(1); expect(store.updateTask).not.toHaveBeenCalled(); - expect(store.moveTask).toHaveBeenCalledWith("FN-9003", "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + expect(store.moveTask).toHaveBeenCalledWith("FN-9003", "todo", { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); managerWithRecovery.stop(); }); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 400cd89a38..6c1d63d93c 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -2139,6 +2139,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { try { await this.store.moveTask(taskId, await resolveReboundTargetForTask(this.store, taskId), { preserveProgress: true, + preserveWorktree: true, preserveStatus: true, // #1411: backward recovery — skip order-derived adjacency. moveSource: "engine", @@ -4604,6 +4605,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, + // worktree-discard-intended: reclaim proved the branch already merged / has zero unique commits; the checkout holds nothing worth keeping and worktree was explicitly nulled above. preserveProgress: true, preserveResumeState: true, }); @@ -4724,6 +4726,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, + // worktree-discard-intended: reclaim proved the branch already merged / has zero unique commits; the checkout holds nothing worth keeping and worktree was explicitly nulled above. preserveProgress: true, preserveResumeState: true, }); @@ -7667,6 +7670,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { recoveryRehome: true, bypassGuards: true, preserveProgress: true, + preserveWorktree: true, }); rehomed += 1; await createRunAuditor(this.store, { @@ -8457,7 +8461,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { continue; } // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); continue; } @@ -8520,7 +8524,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { lane: "self-healing-finalize-no-op-review", }); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); recovered++; continue; } @@ -8540,7 +8544,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { baseRef: classification.baseRef, }); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); recovered++; continue; } @@ -9353,7 +9357,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); "Auto-recovered: in-review task still had incomplete steps — moved back to todo for retry", ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); log.log(`Recovered stale incomplete review task ${task.id}: moved back to todo`); recovered++; } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); @@ -9841,7 +9845,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); "Auto-recovered: in-review task idle past stuck-task timeout — kicked back to todo", ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); log.log(`Kicked ghost review task ${task.id} back to todo`); recovered++; } catch (err: unknown) { @@ -13267,7 +13271,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); nextRecoveryAt: new Date(Date.now() + delayMs).toISOString(), }, targetColumn: await resolveReboundTargetForTask(this.store, task.id), - moveOptions: { preserveProgress: true, moveSource: "engine" }, + moveOptions: { preserveProgress: true, preserveWorktree: true, moveSource: "engine" }, }); if (applied.outcome === "applied") { await this.store.logEntry(task.id, `Auto-recovered generic terminal failure (attempt ${claim.attempt}/${MAX_TERMINAL_FAILURE_AUTO_RETRIES})`); @@ -13496,6 +13500,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); if (route.kind === "node-requeue" && fresh.column !== requeueTarget) { await this.store.moveTask(task.id, requeueTarget, { preserveProgress: true, + preserveWorktree: true, moveSource: "engine", recoveryRehome: true, }); @@ -13831,6 +13836,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); const leaseRecovered = await this.options.leaseManager.recoverAbandonedLease( task.id, `in-progress limbo: ${describeWorktreeState(task)} + null branch`, + // worktree-discard-intended: limbo recovery — the worktree is already missing or its metadata cleared; there is no live checkout to hold. { preserveProgress: true }, ); if (!leaseRecovered) { @@ -13914,6 +13920,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); }, }); // #1411: backward recovery — skip order-derived adjacency. + // worktree-discard-intended: limbo recovery — the worktree is already missing or its metadata cleared; there is no live checkout to hold. await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); recovered++; } catch (err: unknown) { @@ -15351,7 +15358,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); `Auto-retry ${nextCount}/${MAX_TASK_DONE_RETRIES}: agent finished without fn_task_done — requeuing to todo to resume partial work`, ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, moveSource: "engine", recoveryRehome: true }); recovered++; } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err);