fix: stop self-healing from reaping worktrees with live sessions
The idle-worktree sweep (cleanupOrphans) and cap-enforcement sweep only guarded isWorktreeResumeReserved (CLI resume sessions), not in-process active sessions. scanIdleWorktrees treats a worktree as active only while a non-done task points at it, but the executor transiently moves tasks to done or nulls the worktree field mid-run — so a checkout backing a live executor/merger/step/workflow session could be removed before the work finished. Add the activeSessionRegistry.isPathActive guard to both loops, mirroring reapUnregisteredOrphans (FN-4811/FN-5065). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -20,6 +20,7 @@ import type { TaskStore } from "@fusion/core";
|
||||
import { SelfHealingManager } from "../self-healing.js";
|
||||
import * as worktreePool from "../worktree-pool.js";
|
||||
import { StuckTaskDetector, type DisposableSession } from "../stuck-task-detector.js";
|
||||
import { activeSessionRegistry } from "../active-session-registry.js";
|
||||
|
||||
function createStore(settings: Record<string, unknown>): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter() as TaskStore & EventEmitter;
|
||||
@@ -46,6 +47,7 @@ describe("self-healing idle-worktree sweeps skip resume-eligible CLI session wor
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(rootDir, { recursive: true, force: true });
|
||||
activeSessionRegistry.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
@@ -85,6 +87,42 @@ describe("self-healing idle-worktree sweeps skip resume-eligible CLI session wor
|
||||
expect(cleaned).toBe(1);
|
||||
});
|
||||
|
||||
it("cleanupOrphans skips a worktree backing a live (active-session) executor session", async () => {
|
||||
// FN-4811/FN-5065 regression: a registered idle worktree whose task transiently
|
||||
// sits in "done" (so scanIdleWorktrees lists it) must NOT be reaped while a live
|
||||
// executor/merger/step session is still bound to it — that yanks the checkout out
|
||||
// from under in-flight work ("removed before the work is done").
|
||||
const store = createStore({ recycleWorktrees: false });
|
||||
vi.spyOn(worktreePool, "scanIdleWorktrees").mockResolvedValue([reservedPath, freePath]);
|
||||
const removeSpy = vi.spyOn(worktreePool, "removeWorktree").mockResolvedValue(undefined as never);
|
||||
|
||||
activeSessionRegistry.registerPath(reservedPath, { taskId: "FN-1", kind: "executor", ownerKey: "owner-1" });
|
||||
|
||||
// No isWorktreeResumeReserved seam — protection comes solely from the active session.
|
||||
const manager = new SelfHealingManager(store, { rootDir });
|
||||
const cleaned = await (manager as any).cleanupOrphans();
|
||||
|
||||
const removed = removeSpy.mock.calls.map((c) => (c[0] as { worktreePath: string }).worktreePath);
|
||||
expect(removed).toEqual([freePath]);
|
||||
expect(cleaned).toBe(1);
|
||||
});
|
||||
|
||||
it("enforceWorktreeCap skips a worktree backing a live (active-session) executor session", async () => {
|
||||
mkdirSync(join(worktreesDir, "wt-extra"));
|
||||
const store = createStore({ maxWorktrees: 1, recycleWorktrees: false });
|
||||
vi.spyOn(worktreePool, "scanIdleWorktrees").mockResolvedValue([reservedPath, freePath, join(worktreesDir, "wt-extra")]);
|
||||
const removeSpy = vi.spyOn(worktreePool, "removeWorktree").mockResolvedValue(undefined as never);
|
||||
|
||||
activeSessionRegistry.registerPath(reservedPath, { taskId: "FN-1", kind: "executor", ownerKey: "owner-1" });
|
||||
|
||||
const manager = new SelfHealingManager(store, { rootDir });
|
||||
await (manager as any).enforceWorktreeCap();
|
||||
|
||||
const removed = removeSpy.mock.calls.map((c) => (c[0] as { worktreePath: string }).worktreePath);
|
||||
expect(removed).not.toContain(reservedPath);
|
||||
expect(removed).toContain(freePath);
|
||||
});
|
||||
|
||||
it("without the seam predicate, both worktrees are reaped (no behavior change)", async () => {
|
||||
const store = createStore({ recycleWorktrees: false });
|
||||
vi.spyOn(worktreePool, "scanIdleWorktrees").mockResolvedValue([reservedPath, freePath]);
|
||||
|
||||
@@ -8760,6 +8760,14 @@ export class SelfHealingManager {
|
||||
|
||||
let cleaned = 0;
|
||||
for (const worktreePath of orphaned) {
|
||||
// FN-4811/FN-5065: never reap a worktree bound to a live executor/merger/
|
||||
// step/workflow session. Such a task can sit transiently in "done" (or have
|
||||
// null worktree metadata mid-transition) while the owning process is still
|
||||
// working in the checkout — scanIdleWorktrees would otherwise flag it idle.
|
||||
if (activeSessionRegistry.isPathActive(worktreePath) || activeSessionRegistry.isPathActive(resolve(worktreePath))) {
|
||||
log.log(`[self-healing] deferring idle-sweep for ${worktreePath}: active session present`);
|
||||
continue;
|
||||
}
|
||||
// U8: never reclaim a worktree backing a resume-eligible CLI session.
|
||||
if (this.isWorktreeResumeReserved(worktreePath)) {
|
||||
log.log(`[self-healing] deferring idle-sweep for ${worktreePath}: resume-eligible CLI session present`);
|
||||
@@ -9218,6 +9226,13 @@ export class SelfHealingManager {
|
||||
|
||||
for (const { path: worktreePath } of withMtime) {
|
||||
if (removed >= excess) break;
|
||||
// FN-4811/FN-5065: never reap a worktree bound to a live executor/merger/
|
||||
// step/workflow session — cap pressure must not yank a checkout out from
|
||||
// under a process that is still working in it.
|
||||
if (activeSessionRegistry.isPathActive(worktreePath) || activeSessionRegistry.isPathActive(resolve(worktreePath))) {
|
||||
log.log(`[self-healing] cap-enforcement skipping ${worktreePath}: active session present`);
|
||||
continue;
|
||||
}
|
||||
// U8: never reclaim a worktree backing a resume-eligible CLI session.
|
||||
if (this.isWorktreeResumeReserved(worktreePath)) {
|
||||
log.log(`[self-healing] cap-enforcement skipping ${worktreePath}: resume-eligible CLI session present`);
|
||||
|
||||
Reference in New Issue
Block a user