fix(FN-5256): keep live task worktrees through pause/resume races
Three independent reliability fixes that all surfaced as the same bug:
live tasks losing their worktrees mid-execution and emitting
`wrong_toplevel` errors.
Fix A — executor stale-self-owned classifier:
`reconcileSelfOwnedActiveSessionForRemoval` now takes a process-active
probe (`executingTaskLock.has`) and a minimum-idle window (default 5s)
in addition to the existing in-memory `activeWorktrees` binding probe.
Recently-registered or still-running entries are refused with
`process-active-refuses` / `too-recent-refuses`, with audit-grade
log lines. Both the pre-remove path
(`reconcileSelfOwnedBeforeRemove`), the post-throw retry in
`removeOwnWorktreeWithReconcile`, and the defensive reconcile in
`removeWorktree` route through the same hardened gates.
Fix B — pause synchronously reaps the agent session:
New `awaitAbortInFlightTaskWork` mirrors the existing fire-and-forget
abort but awaits each `session.abort()` /
`stepExecutor.terminateAllSessions()` /
`workflowSession.abort()`. `parkTaskAfterWorkflowStepPause` calls it
before `moveTask("todo")`, and the `task:updated` user-pause handler
routes through it, so a fast re-dispatch can no longer race a still-
live shell.
Fix C — self-healing realpath + active-task skip:
`reconcileTaskWorktreeMetadata` now realpath-normalizes both sides of
the registry comparison (handling macOS `/private/var/...`) and
refuses to clear `worktree`/`branch` on in-progress or in-review
tasks. The skip emits a new
`task:auto-recover-worktree-metadata-skipped-active` audit event;
executor-level recovery paths remain in charge of active tasks.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
11
.changeset/fix-fn-5256-worktree-liveness.md
Normal file
11
.changeset/fix-fn-5256-worktree-liveness.md
Normal file
@@ -0,0 +1,11 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
fix(FN-5256): harden three independent code paths that were losing live task worktrees mid-execution and producing `wrong_toplevel` errors.
|
||||||
|
|
||||||
|
- Executor stale-self-owned classifier (`reconcileSelfOwnedActiveSessionForRemoval`) now requires two additional signals before dropping a same-task registry entry: a process-active probe (`executingTaskLock.has`) and a minimum-idle window (default 5s) since the entry was registered. This closes the pause/resume race where the new executor cycle hadn't repopulated `activeWorktrees` yet and the old session's registry entry was reaped under a still-live shell. The post-throw reconcile in `removeOwnWorktreeWithReconcile` and the `removeWorktree` defensive reconcile in `worktree-backend.ts` route through the same hardened path.
|
||||||
|
|
||||||
|
- Pause-before-park now synchronously awaits agent/step/workflow session disposal via a new `awaitAbortInFlightTaskWork` method, so by the time `parkTaskAfterWorkflowStepPause` calls `moveTask("todo")` the spawned shells are already reaped and any fast re-dispatch sees a clean slate. The user-initiated pause handler on `task:updated` was collapsed onto the same await path for the same reason.
|
||||||
|
|
||||||
|
- Self-healing `reconcileTaskWorktreeMetadata` now normalizes both sides via `realpathSync` (with ENOENT fallback) before comparing the task worktree against the registered set, fixing the macOS `/private/var/...` false-stale flag. It additionally refuses to clear `worktree`/`branch` metadata for in-progress or in-review tasks — those go through `task:auto-recover-worktree-metadata-skipped-active` audit events and leave executor-level recovery in charge.
|
||||||
@@ -94,7 +94,9 @@ describe("activeSessionRegistry", () => {
|
|||||||
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
reconcileSelfOwnedActiveSessionForRemoval(activeSessionRegistry, "/tmp/w1", "FN-1", () => false),
|
reconcileSelfOwnedActiveSessionForRemoval(activeSessionRegistry, "/tmp/w1", "FN-1", () => false, {
|
||||||
|
minIdleMs: 0,
|
||||||
|
}),
|
||||||
).toEqual({ action: "reconciled" });
|
).toEqual({ action: "reconciled" });
|
||||||
expect(activeSessionRegistry.lookupByPath("/tmp/w1")).toBeNull();
|
expect(activeSessionRegistry.lookupByPath("/tmp/w1")).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -103,10 +105,75 @@ describe("activeSessionRegistry", () => {
|
|||||||
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
reconcileSelfOwnedActiveSessionForRemoval(activeSessionRegistry, "/tmp/w1", "FN-1", () => false),
|
reconcileSelfOwnedActiveSessionForRemoval(activeSessionRegistry, "/tmp/w1", "FN-1", () => false, {
|
||||||
|
minIdleMs: 0,
|
||||||
|
}),
|
||||||
).toEqual({ action: "reconciled" });
|
).toEqual({ action: "reconciled" });
|
||||||
expect(
|
expect(
|
||||||
reconcileSelfOwnedActiveSessionForRemoval(activeSessionRegistry, "/tmp/w1", "FN-1", () => false),
|
reconcileSelfOwnedActiveSessionForRemoval(activeSessionRegistry, "/tmp/w1", "FN-1", () => false, {
|
||||||
|
minIdleMs: 0,
|
||||||
|
}),
|
||||||
).toEqual({ action: "no-entry" });
|
).toEqual({ action: "no-entry" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("FN-5256: refuses reconcile when processActiveProbe returns true", () => {
|
||||||
|
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||||
|
|
||||||
|
const outcome = reconcileSelfOwnedActiveSessionForRemoval(
|
||||||
|
activeSessionRegistry,
|
||||||
|
"/tmp/w1",
|
||||||
|
"FN-1",
|
||||||
|
() => false,
|
||||||
|
{ processActiveProbe: () => true, minIdleMs: 0 },
|
||||||
|
);
|
||||||
|
expect(outcome).toEqual({ action: "process-active-refuses", ownerTaskId: "FN-1" });
|
||||||
|
expect(activeSessionRegistry.lookupByPath("/tmp/w1")?.taskId).toBe("FN-1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("FN-5256: refuses reconcile when registration is younger than minIdleMs", () => {
|
||||||
|
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||||
|
const registeredAt = activeSessionRegistry.lookupByPath("/tmp/w1")!.registeredAt;
|
||||||
|
|
||||||
|
const outcome = reconcileSelfOwnedActiveSessionForRemoval(
|
||||||
|
activeSessionRegistry,
|
||||||
|
"/tmp/w1",
|
||||||
|
"FN-1",
|
||||||
|
() => false,
|
||||||
|
{ minIdleMs: 5000, now: () => registeredAt + 100 },
|
||||||
|
);
|
||||||
|
expect(outcome).toMatchObject({ action: "too-recent-refuses", ownerTaskId: "FN-1", minIdleMs: 5000 });
|
||||||
|
expect(activeSessionRegistry.lookupByPath("/tmp/w1")?.taskId).toBe("FN-1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("FN-5256: reconciles when all signals clean (default min-idle window elapsed)", () => {
|
||||||
|
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||||
|
const registeredAt = activeSessionRegistry.lookupByPath("/tmp/w1")!.registeredAt;
|
||||||
|
|
||||||
|
const outcome = reconcileSelfOwnedActiveSessionForRemoval(
|
||||||
|
activeSessionRegistry,
|
||||||
|
"/tmp/w1",
|
||||||
|
"FN-1",
|
||||||
|
() => false,
|
||||||
|
{
|
||||||
|
processActiveProbe: () => false,
|
||||||
|
minIdleMs: 5000,
|
||||||
|
now: () => registeredAt + 6000,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
expect(outcome).toEqual({ action: "reconciled" });
|
||||||
|
expect(activeSessionRegistry.lookupByPath("/tmp/w1")).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("FN-5256: live-binding takes precedence over process-active and too-recent", () => {
|
||||||
|
activeSessionRegistry.registerPath("/tmp/w1", { taskId: "FN-1", kind: "executor", ownerKey: "FN-1" });
|
||||||
|
|
||||||
|
const outcome = reconcileSelfOwnedActiveSessionForRemoval(
|
||||||
|
activeSessionRegistry,
|
||||||
|
"/tmp/w1",
|
||||||
|
"FN-1",
|
||||||
|
() => true,
|
||||||
|
{ processActiveProbe: () => true, minIdleMs: 5000 },
|
||||||
|
);
|
||||||
|
expect(outcome).toEqual({ action: "live-binding-refuses", ownerTaskId: "FN-1" });
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -2723,3 +2723,112 @@ describe("StepSessionExecutor integration", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("FN-5256 awaitAbortInFlightTaskWork pause synchronization", () => {
|
||||||
|
it("synchronously disposes activeSessions on pause via task:updated", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
|
||||||
|
let abortResolve: (() => void) | undefined;
|
||||||
|
const abortPromise = new Promise<void>((resolve) => {
|
||||||
|
abortResolve = resolve;
|
||||||
|
});
|
||||||
|
const abortSpy = vi.fn(() => abortPromise);
|
||||||
|
const disposeSpy = vi.fn();
|
||||||
|
(executor as any).activeSessions.set("FN-PAUSE-1", {
|
||||||
|
session: { abort: abortSpy, dispose: disposeSpy, steer: vi.fn() },
|
||||||
|
seenSteeringIds: new Set(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const taskUpdatedHandler = (store.on as unknown as ReturnType<typeof vi.fn>).mock.calls
|
||||||
|
.find((call: any[]) => call[0] === "task:updated")?.[1] as (task: any) => Promise<void>;
|
||||||
|
expect(taskUpdatedHandler).toBeTypeOf("function");
|
||||||
|
|
||||||
|
const settled = taskUpdatedHandler({
|
||||||
|
id: "FN-PAUSE-1",
|
||||||
|
title: "pause",
|
||||||
|
description: "pause",
|
||||||
|
column: "in-progress",
|
||||||
|
paused: true,
|
||||||
|
dependencies: [],
|
||||||
|
steps: [],
|
||||||
|
currentStep: 0,
|
||||||
|
log: [],
|
||||||
|
createdAt: new Date().toISOString(),
|
||||||
|
updatedAt: new Date().toISOString(),
|
||||||
|
} satisfies Task);
|
||||||
|
|
||||||
|
// Before abort resolves, dispose must NOT have been called — i.e. handler is
|
||||||
|
// truly awaiting the abort, not fire-and-forgetting it.
|
||||||
|
await new Promise((resolve) => setImmediate(resolve));
|
||||||
|
expect(disposeSpy).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
abortResolve?.();
|
||||||
|
await settled;
|
||||||
|
|
||||||
|
expect(abortSpy).toHaveBeenCalledTimes(1);
|
||||||
|
expect(disposeSpy).toHaveBeenCalledTimes(1);
|
||||||
|
expect((executor as any).activeSessions.has("FN-PAUSE-1")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("synchronously reaps workflow-step + step-session surfaces on pause", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
|
||||||
|
const stepTerminate = vi.fn().mockResolvedValue(undefined);
|
||||||
|
(executor as any).activeStepExecutors.set("FN-PAUSE-2", {
|
||||||
|
terminateAllSessions: stepTerminate,
|
||||||
|
});
|
||||||
|
|
||||||
|
const workflowAbort = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const workflowDispose = vi.fn();
|
||||||
|
(executor as any).activeWorkflowStepSessions.set("FN-PAUSE-2", {
|
||||||
|
abort: workflowAbort,
|
||||||
|
dispose: workflowDispose,
|
||||||
|
});
|
||||||
|
|
||||||
|
const taskUpdatedHandler = (store.on as unknown as ReturnType<typeof vi.fn>).mock.calls
|
||||||
|
.find((call: any[]) => call[0] === "task:updated")?.[1] as (task: any) => Promise<void>;
|
||||||
|
|
||||||
|
await taskUpdatedHandler({
|
||||||
|
id: "FN-PAUSE-2",
|
||||||
|
title: "pause",
|
||||||
|
description: "pause",
|
||||||
|
column: "in-progress",
|
||||||
|
paused: true,
|
||||||
|
dependencies: [],
|
||||||
|
steps: [],
|
||||||
|
currentStep: 0,
|
||||||
|
log: [],
|
||||||
|
createdAt: new Date().toISOString(),
|
||||||
|
updatedAt: new Date().toISOString(),
|
||||||
|
} satisfies Task);
|
||||||
|
|
||||||
|
expect(stepTerminate).toHaveBeenCalledTimes(1);
|
||||||
|
expect(workflowAbort).toHaveBeenCalledTimes(1);
|
||||||
|
expect(workflowDispose).toHaveBeenCalledTimes(1);
|
||||||
|
expect((executor as any).activeStepExecutors.has("FN-PAUSE-2")).toBe(false);
|
||||||
|
expect((executor as any).activeWorkflowStepSessions.has("FN-PAUSE-2")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("awaitAbortInFlightTaskWork awaits abort before dispose for all surfaces", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
|
|
||||||
|
const order: string[] = [];
|
||||||
|
const sessionAbort = vi.fn().mockImplementation(async () => {
|
||||||
|
await new Promise((r) => setTimeout(r, 5));
|
||||||
|
order.push("session-abort");
|
||||||
|
});
|
||||||
|
const sessionDispose = vi.fn(() => order.push("session-dispose"));
|
||||||
|
(executor as any).activeSessions.set("FN-PAUSE-3", {
|
||||||
|
session: { abort: sessionAbort, dispose: sessionDispose, steer: vi.fn() },
|
||||||
|
seenSteeringIds: new Set(),
|
||||||
|
});
|
||||||
|
|
||||||
|
await (executor as any).awaitAbortInFlightTaskWork("FN-PAUSE-3", "test reason");
|
||||||
|
|
||||||
|
expect(order).toEqual(["session-abort", "session-dispose"]);
|
||||||
|
expect((executor as any).activeSessions.has("FN-PAUSE-3")).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -1939,7 +1939,10 @@ describe("TaskExecutor task:updated listener guards", () => {
|
|||||||
} satisfies Task)).resolves.toBeUndefined();
|
} satisfies Task)).resolves.toBeUndefined();
|
||||||
|
|
||||||
expect(terminateAllSessions).toHaveBeenCalledTimes(1);
|
expect(terminateAllSessions).toHaveBeenCalledTimes(1);
|
||||||
expect(executorLog.error).toHaveBeenCalledWith("Uncaught error in task:updated listener:", terminateError);
|
// FN-5256: the pause handler now routes through awaitAbortInFlightTaskWork,
|
||||||
|
// which internally catches/logs the per-surface failure. The error still hits
|
||||||
|
// executorLog.error but via the granular message path.
|
||||||
|
expect(executorLog.error).toHaveBeenCalledWith("Failed to terminate step sessions for FN-001:", terminateError);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ describe("FN-4973: executor worktree conflict cleanup", () => {
|
|||||||
const executor = new TaskExecutor(store, "/tmp/test");
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
store.listTasks.mockResolvedValue([]);
|
store.listTasks.mockResolvedValue([]);
|
||||||
activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: "FN-4973", kind: "executor", ownerKey: "FN-4973" });
|
activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: "FN-4973", kind: "executor", ownerKey: "FN-4973" });
|
||||||
|
// FN-5256: backdate so the new min-idle window doesn't refuse the reconcile.
|
||||||
|
(activeSessionRegistry.lookupByPath(CONFLICT_PATH) as any).registeredAt = 0;
|
||||||
|
|
||||||
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
||||||
const result = await (executor as any).cleanupConflictingWorktree(CONFLICT_PATH, "fusion/fn-4973", "FN-4973");
|
const result = await (executor as any).cleanupConflictingWorktree(CONFLICT_PATH, "fusion/fn-4973", "FN-4973");
|
||||||
@@ -85,6 +87,8 @@ describe("FN-4973: executor worktree conflict cleanup", () => {
|
|||||||
removeSpy
|
removeSpy
|
||||||
.mockImplementationOnce(async () => {
|
.mockImplementationOnce(async () => {
|
||||||
activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: "FN-4973", kind: "executor", ownerKey: "FN-4973" });
|
activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: "FN-4973", kind: "executor", ownerKey: "FN-4973" });
|
||||||
|
// FN-5256: backdate so the post-throw reconcile is not refused by min-idle.
|
||||||
|
(activeSessionRegistry.lookupByPath(CONFLICT_PATH) as any).registeredAt = 0;
|
||||||
throw new ActiveSessionWorktreeRemovalError({
|
throw new ActiveSessionWorktreeRemovalError({
|
||||||
worktreePath: CONFLICT_PATH,
|
worktreePath: CONFLICT_PATH,
|
||||||
taskId: "FN-4973",
|
taskId: "FN-4973",
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin
|
|||||||
const executor = new TaskExecutor(store, ROOT);
|
const executor = new TaskExecutor(store, ROOT);
|
||||||
(executor as any).activeWorktrees.set(TASK_ID, PATH);
|
(executor as any).activeWorktrees.set(TASK_ID, PATH);
|
||||||
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
||||||
|
(activeSessionRegistry.lookupByPath(PATH) as any).registeredAt = 0;
|
||||||
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
||||||
|
|
||||||
await executor.cleanup(TASK_ID);
|
await executor.cleanup(TASK_ID);
|
||||||
@@ -43,6 +44,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin
|
|||||||
const store = createMockStore();
|
const store = createMockStore();
|
||||||
const executor = new TaskExecutor(store, ROOT);
|
const executor = new TaskExecutor(store, ROOT);
|
||||||
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
||||||
|
(activeSessionRegistry.lookupByPath(PATH) as any).registeredAt = 0;
|
||||||
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
||||||
|
|
||||||
await (executor as any).handleDepAbortCleanup(TASK_ID, PATH);
|
await (executor as any).handleDepAbortCleanup(TASK_ID, PATH);
|
||||||
@@ -96,6 +98,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin
|
|||||||
const executor = new TaskExecutor(store, ROOT);
|
const executor = new TaskExecutor(store, ROOT);
|
||||||
(executor as any).activeWorktrees.set(TASK_ID, PATH);
|
(executor as any).activeWorktrees.set(TASK_ID, PATH);
|
||||||
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
||||||
|
(activeSessionRegistry.lookupByPath(PATH) as any).registeredAt = 0;
|
||||||
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
||||||
|
|
||||||
await executor.cleanup(TASK_ID);
|
await executor.cleanup(TASK_ID);
|
||||||
|
|||||||
@@ -74,6 +74,8 @@ describe("FN-4973 reliability interactions: stale self-owned active-session reco
|
|||||||
store.listTasks.mockResolvedValue([]);
|
store.listTasks.mockResolvedValue([]);
|
||||||
const executor = new TaskExecutor(store, "/tmp/test");
|
const executor = new TaskExecutor(store, "/tmp/test");
|
||||||
activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
||||||
|
// FN-5256: backdate so the new min-idle window doesn't refuse the reconcile.
|
||||||
|
(activeSessionRegistry.lookupByPath(CONFLICT_PATH) as any).registeredAt = 0;
|
||||||
|
|
||||||
vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
||||||
|
|
||||||
|
|||||||
@@ -25,6 +25,8 @@ describe("FN-4976: stale self-owned activeSessionRegistry deadlock backstop", ()
|
|||||||
const store = createMockStore();
|
const store = createMockStore();
|
||||||
store.listTasks.mockResolvedValue([]);
|
store.listTasks.mockResolvedValue([]);
|
||||||
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID });
|
||||||
|
// FN-5256: backdate so the new min-idle window doesn't refuse the reconcile.
|
||||||
|
(activeSessionRegistry.lookupByPath(PATH) as any).registeredAt = 0;
|
||||||
|
|
||||||
const executor = new TaskExecutor(store, ROOT);
|
const executor = new TaskExecutor(store, ROOT);
|
||||||
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined);
|
||||||
|
|||||||
@@ -129,7 +129,10 @@ describeIfGit("SelfHealingManager recoverAlreadyMergedReviewTasks (real git)", (
|
|||||||
expect(task.mergeDetails?.mergeConfirmed).toBe(true);
|
expect(task.mergeDetails?.mergeConfirmed).toBe(true);
|
||||||
expect(existsSync(worktreePath)).toBe(false);
|
expect(existsSync(worktreePath)).toBe(false);
|
||||||
expect(git(repo, "git worktree list")).not.toContain(worktreePath);
|
expect(git(repo, "git worktree list")).not.toContain(worktreePath);
|
||||||
expect((store as any).recordRunAuditEvent).toHaveBeenCalledTimes(3);
|
// FN-5256: reconcileTaskWorktreeMetadata now normalizes via realpath, so the
|
||||||
|
// (formerly false-stale) macOS realpath mismatch no longer triggers an extra
|
||||||
|
// worktree-metadata-cleared audit event for this in-review task.
|
||||||
|
expect((store as any).recordRunAuditEvent).toHaveBeenCalledTimes(2);
|
||||||
expect((store as any).recordRunAuditEvent).toHaveBeenCalledWith(
|
expect((store as any).recordRunAuditEvent).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
domain: "database",
|
domain: "database",
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { mkdtempSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
|
import { mkdtempSync, mkdirSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
|
||||||
import { fileURLToPath } from "node:url";
|
import { fileURLToPath } from "node:url";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
@@ -167,3 +167,98 @@ describe("reconcileTaskWorktreeMetadata matrix", () => {
|
|||||||
expect((store as any).updateTask).not.toHaveBeenCalled();
|
expect((store as any).updateTask).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("FN-5256 reconcileTaskWorktreeMetadata reliability", () => {
|
||||||
|
let rootDir = "";
|
||||||
|
let store: TaskStore;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
rootDir = mkdtempSync(join(tmpdir(), "fn-5256-"));
|
||||||
|
git(rootDir, "init -b main");
|
||||||
|
git(rootDir, "config user.name 'Fusion'");
|
||||||
|
git(rootDir, "config user.email 'hi@runfusion.ai'");
|
||||||
|
writeFileSync(join(rootDir, "README.md"), "root\n");
|
||||||
|
git(rootDir, "add README.md");
|
||||||
|
git(rootDir, "commit -m 'init'");
|
||||||
|
|
||||||
|
store = new TaskStore(rootDir, undefined, { inMemoryDb: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
try {
|
||||||
|
store?.close();
|
||||||
|
} catch {
|
||||||
|
// noop
|
||||||
|
}
|
||||||
|
if (rootDir) rmSync(rootDir, { recursive: true, force: true });
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("FN-5256: realpath-normalizes both sides so a symlinked task.worktree is not falsely flagged stale", async () => {
|
||||||
|
await store.createTask({ title: "FN-5256 symlink", description: "symlink case" });
|
||||||
|
const [task] = await store.listTasks();
|
||||||
|
|
||||||
|
mkdirSync(join(rootDir, ".worktrees"), { recursive: true });
|
||||||
|
const realWorktreeDir = join(realpathSync(rootDir), ".worktrees", "real-leaf");
|
||||||
|
const branch = `fusion/${task.id.toLowerCase()}`;
|
||||||
|
git(rootDir, `branch ${branch}`);
|
||||||
|
git(rootDir, `worktree add ${realWorktreeDir} ${branch}`);
|
||||||
|
|
||||||
|
// Create a symlink that points at the registered worktree's parent. The task
|
||||||
|
// metadata persists the symlinked path; the registry will surface realpath.
|
||||||
|
const symlinkParent = join(rootDir, ".worktrees-symlink");
|
||||||
|
symlinkSync(join(rootDir, ".worktrees"), symlinkParent, "dir");
|
||||||
|
const symlinkedTaskWorktree = join(symlinkParent, "real-leaf");
|
||||||
|
|
||||||
|
await store.updateTask(task.id, { worktree: symlinkedTaskWorktree, branch });
|
||||||
|
|
||||||
|
const auditSpy = vi.spyOn(store, "recordRunAuditEvent");
|
||||||
|
const manager = new SelfHealingManager(store, {
|
||||||
|
rootDir,
|
||||||
|
getExecutingTaskIds: () => new Set<string>(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const repaired = await (manager as any).reconcileTaskWorktreeMetadata();
|
||||||
|
expect(repaired).toBe(0);
|
||||||
|
|
||||||
|
const updated = await store.getTask(task.id);
|
||||||
|
expect(updated?.worktree).toBe(symlinkedTaskWorktree);
|
||||||
|
expect(updated?.branch).toBe(branch);
|
||||||
|
expect(auditSpy).not.toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ mutationType: "task:auto-recover-worktree-metadata-cleared" }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("FN-5256: refuses to clear worktree metadata for an in-progress task with a stale-flagged worktree", async () => {
|
||||||
|
await store.createTask({ title: "FN-5256 in-progress", description: "stale active task" });
|
||||||
|
const [task] = await store.listTasks();
|
||||||
|
|
||||||
|
await store.moveTask(task.id, "todo");
|
||||||
|
await store.moveTask(task.id, "in-progress");
|
||||||
|
// The worktree path is bogus (not registered) but the task is active. The
|
||||||
|
// reconciler must not yank metadata out from under a running task.
|
||||||
|
const stalePath = join(rootDir, ".worktrees", "ghost-leaf");
|
||||||
|
mkdirSync(stalePath, { recursive: true });
|
||||||
|
await store.updateTask(task.id, { worktree: stalePath, branch: `fusion/${task.id.toLowerCase()}` });
|
||||||
|
|
||||||
|
const auditSpy = vi.spyOn(store, "recordRunAuditEvent");
|
||||||
|
const manager = new SelfHealingManager(store, {
|
||||||
|
rootDir,
|
||||||
|
getExecutingTaskIds: () => new Set<string>(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const repaired = await (manager as any).reconcileTaskWorktreeMetadata();
|
||||||
|
expect(repaired).toBe(0);
|
||||||
|
|
||||||
|
const updated = await store.getTask(task.id);
|
||||||
|
expect(updated?.worktree).toBe(stalePath);
|
||||||
|
expect(updated?.branch).toBe(`fusion/${task.id.toLowerCase()}`);
|
||||||
|
|
||||||
|
expect(auditSpy).not.toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ mutationType: "task:auto-recover-worktree-metadata-cleared" }),
|
||||||
|
);
|
||||||
|
expect(auditSpy).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ mutationType: "task:auto-recover-worktree-metadata-skipped-active" }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -873,6 +873,9 @@ describe("removeWorktree", () => {
|
|||||||
reason: RemovalReason.ExecutorDispose,
|
reason: RemovalReason.ExecutorDispose,
|
||||||
expectedOwnerTaskId: "FN-1",
|
expectedOwnerTaskId: "FN-1",
|
||||||
liveOwnerProbe: () => false,
|
liveOwnerProbe: () => false,
|
||||||
|
// FN-5256: opt out of the min-idle window so this defensive-reconcile test
|
||||||
|
// is unaffected by the new warm-up gate.
|
||||||
|
reconcileMinIdleMs: 0,
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(activeSessionRegistry.lookupByPath("/repo/.worktrees/fn-1")).toBeNull();
|
expect(activeSessionRegistry.lookupByPath("/repo/.worktrees/fn-1")).toBeNull();
|
||||||
|
|||||||
@@ -16,13 +16,24 @@ export interface ReconcileStaleSelfOwnedResult {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export type LiveBindingProbe = (worktreePath: string, taskId: string) => boolean;
|
export type LiveBindingProbe = (worktreePath: string, taskId: string) => boolean;
|
||||||
|
export type ProcessActiveProbe = (taskId: string) => boolean;
|
||||||
|
|
||||||
export type SelfOwnedReconcileOutcome =
|
export type SelfOwnedReconcileOutcome =
|
||||||
| { action: "no-entry" }
|
| { action: "no-entry" }
|
||||||
| { action: "foreign-task"; ownerTaskId: string }
|
| { action: "foreign-task"; ownerTaskId: string }
|
||||||
| { action: "live-binding-refuses"; ownerTaskId: string }
|
| { action: "live-binding-refuses"; ownerTaskId: string }
|
||||||
|
| { action: "process-active-refuses"; ownerTaskId: string }
|
||||||
|
| { action: "too-recent-refuses"; ownerTaskId: string; ageMs: number; minIdleMs: number }
|
||||||
| { action: "reconciled" };
|
| { action: "reconciled" };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* FN-5256: default minimum age before a self-owned registry entry can be classified
|
||||||
|
* as stale. Recently-registered entries belong to an executor cycle that is still
|
||||||
|
* warming up (e.g., a pause/resume that hasn't repopulated activeWorktrees yet), so
|
||||||
|
* dropping them races with the live shell that just attached to the worktree.
|
||||||
|
*/
|
||||||
|
export const DEFAULT_SELF_OWNED_MIN_IDLE_MS = 5000;
|
||||||
|
|
||||||
export class ActiveSessionRegistry {
|
export class ActiveSessionRegistry {
|
||||||
private readonly records = new Map<string, ActiveSessionRecord>();
|
private readonly records = new Map<string, ActiveSessionRecord>();
|
||||||
|
|
||||||
@@ -76,11 +87,29 @@ export class ActiveSessionRegistry {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export interface SelfOwnedReconcileOptions {
|
||||||
|
/**
|
||||||
|
* Process-wide "executor still owns this task" probe. When this returns true the
|
||||||
|
* caller's task is still in the middle of an `execute()` invocation, so dropping
|
||||||
|
* the registry entry would yank the worktree from a live shell (FN-5256).
|
||||||
|
*/
|
||||||
|
processActiveProbe?: ProcessActiveProbe;
|
||||||
|
/**
|
||||||
|
* Minimum age (ms since `registeredAt`) before a same-task entry is eligible for
|
||||||
|
* stale reconciliation. Recently-registered entries belong to a warming executor
|
||||||
|
* cycle and must be left alone. Defaults to `DEFAULT_SELF_OWNED_MIN_IDLE_MS`.
|
||||||
|
*/
|
||||||
|
minIdleMs?: number;
|
||||||
|
/** Test seam — defaults to `Date.now()`. */
|
||||||
|
now?: () => number;
|
||||||
|
}
|
||||||
|
|
||||||
export function reconcileSelfOwnedActiveSessionForRemoval(
|
export function reconcileSelfOwnedActiveSessionForRemoval(
|
||||||
registry: ActiveSessionRegistry,
|
registry: ActiveSessionRegistry,
|
||||||
worktreePath: string,
|
worktreePath: string,
|
||||||
requestingTaskId: string,
|
requestingTaskId: string,
|
||||||
liveBindingProbe: LiveBindingProbe,
|
liveBindingProbe: LiveBindingProbe,
|
||||||
|
options: SelfOwnedReconcileOptions = {},
|
||||||
): SelfOwnedReconcileOutcome {
|
): SelfOwnedReconcileOutcome {
|
||||||
const record = registry.lookupByPath(worktreePath);
|
const record = registry.lookupByPath(worktreePath);
|
||||||
if (!record) {
|
if (!record) {
|
||||||
@@ -95,6 +124,19 @@ export function reconcileSelfOwnedActiveSessionForRemoval(
|
|||||||
return { action: "live-binding-refuses", ownerTaskId: requestingTaskId };
|
return { action: "live-binding-refuses", ownerTaskId: requestingTaskId };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (options.processActiveProbe?.(requestingTaskId)) {
|
||||||
|
return { action: "process-active-refuses", ownerTaskId: requestingTaskId };
|
||||||
|
}
|
||||||
|
|
||||||
|
const minIdleMs = options.minIdleMs ?? DEFAULT_SELF_OWNED_MIN_IDLE_MS;
|
||||||
|
if (minIdleMs > 0) {
|
||||||
|
const now = options.now?.() ?? Date.now();
|
||||||
|
const ageMs = now - record.registeredAt;
|
||||||
|
if (ageMs < minIdleMs) {
|
||||||
|
return { action: "too-recent-refuses", ownerTaskId: requestingTaskId, ageMs, minIdleMs };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
registry.unregisterPath(worktreePath);
|
registry.unregisterPath(worktreePath);
|
||||||
return { action: "reconciled" };
|
return { action: "reconciled" };
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1196,6 +1196,13 @@ export class TaskExecutor {
|
|||||||
undefined,
|
undefined,
|
||||||
this.getRunContextFor(taskId),
|
this.getRunContextFor(taskId),
|
||||||
).catch(() => undefined);
|
).catch(() => undefined);
|
||||||
|
// FN-5256: synchronously reap any spawned shells BEFORE moving the task. The
|
||||||
|
// task:moved listener fires `abortInFlightTaskWork` (fire-and-forget) so by the
|
||||||
|
// time it runs, the abort needs to already be complete — otherwise a fast
|
||||||
|
// re-dispatch races the live shell and yanks the worktree.
|
||||||
|
await this.awaitAbortInFlightTaskWork(taskId, "pause-before-park").catch((err) => {
|
||||||
|
executorLog.warn(`${taskId}: awaitAbortInFlightTaskWork failed in pause-before-park: ${err}`);
|
||||||
|
});
|
||||||
if (latestTask.column === "in-progress") {
|
if (latestTask.column === "in-progress") {
|
||||||
await this.store.moveTask(taskId, "todo", { preserveResumeState: true });
|
await this.store.moveTask(taskId, "todo", { preserveResumeState: true });
|
||||||
}
|
}
|
||||||
@@ -1445,6 +1452,88 @@ export class TaskExecutor {
|
|||||||
this.activeSubagentSessions.delete(taskId);
|
this.activeSubagentSessions.delete(taskId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* FN-5256: synchronously await session disposal so callers (e.g. pause-before-park)
|
||||||
|
* can rely on the worktree-bound shells being reaped before they return. Mirrors
|
||||||
|
* `abortInFlightTaskWork`, but awaits the async `abort()` / `terminateAllSessions()`
|
||||||
|
* calls instead of fire-and-forget.
|
||||||
|
*/
|
||||||
|
async awaitAbortInFlightTaskWork(taskId: string, reason: string, options: { userCanceled?: boolean } = {}): Promise<void> {
|
||||||
|
let hadActiveSurface = false;
|
||||||
|
|
||||||
|
if (options.userCanceled) {
|
||||||
|
this.userCanceledTaskIds.add(taskId);
|
||||||
|
}
|
||||||
|
this.pausedAborted.add(taskId);
|
||||||
|
this.options.stuckTaskDetector?.untrackTask(taskId);
|
||||||
|
this.clearWorkflowRerunWatchdog(taskId);
|
||||||
|
this.clearCompletedTaskWatchdog(taskId);
|
||||||
|
|
||||||
|
if (this.activeSessions.has(taskId)) {
|
||||||
|
hadActiveSurface = true;
|
||||||
|
const { session } = this.activeSessions.get(taskId)!;
|
||||||
|
const sessionWithAbort = session as AgentSession & { abort?: () => Promise<void> };
|
||||||
|
if (typeof sessionWithAbort.abort === "function") {
|
||||||
|
await sessionWithAbort.abort().catch((err) => {
|
||||||
|
executorLog.warn(`Failed to abort agent session for ${taskId}: ${err}`);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
session.dispose();
|
||||||
|
} catch (err) {
|
||||||
|
executorLog.warn(`Failed to dispose agent session for ${taskId}: ${err}`);
|
||||||
|
}
|
||||||
|
this.deleteActiveSession(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (this.activeStepExecutors.has(taskId)) {
|
||||||
|
hadActiveSurface = true;
|
||||||
|
const stepExecutor = this.activeStepExecutors.get(taskId)!;
|
||||||
|
const stepExecutorWithAbort = stepExecutor as StepSessionExecutor & { abortAllSessionBash?: () => void };
|
||||||
|
if (typeof stepExecutorWithAbort.abortAllSessionBash === "function") {
|
||||||
|
try {
|
||||||
|
stepExecutorWithAbort.abortAllSessionBash();
|
||||||
|
} catch (err) {
|
||||||
|
executorLog.warn(`Failed to abort step-session bash for ${taskId}: ${err}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
await stepExecutor.terminateAllSessions().catch((err) =>
|
||||||
|
executorLog.error(`Failed to terminate step sessions for ${taskId}:`, err),
|
||||||
|
);
|
||||||
|
this.deleteActiveStepExecutor(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (this.activeWorkflowStepSessions.has(taskId)) {
|
||||||
|
hadActiveSurface = true;
|
||||||
|
const workflowSession = this.activeWorkflowStepSessions.get(taskId)!;
|
||||||
|
const sessionWithAbort = workflowSession as AgentSession & { abort?: () => Promise<void> };
|
||||||
|
if (typeof sessionWithAbort.abort === "function") {
|
||||||
|
await sessionWithAbort.abort().catch((err) => {
|
||||||
|
executorLog.warn(`Failed to abort workflow step session for ${taskId}: ${err}`);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
workflowSession.dispose();
|
||||||
|
} catch (err) {
|
||||||
|
executorLog.warn(`Failed to dispose workflow step session for ${taskId}: ${err}`);
|
||||||
|
}
|
||||||
|
this.deleteActiveWorkflowStepSession(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (this.activeSubagentSessions.has(taskId)) {
|
||||||
|
hadActiveSurface = true;
|
||||||
|
this.disposeSubagentsForTask(taskId, reason);
|
||||||
|
}
|
||||||
|
|
||||||
|
this.loopRecoveryState.delete(taskId);
|
||||||
|
this.spawnedAgents.delete(taskId);
|
||||||
|
this.stuckAborted.delete(taskId);
|
||||||
|
|
||||||
|
if (hadActiveSurface) {
|
||||||
|
executorLog.log(`${taskId}: awaited abort of in-flight work — ${reason}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private abortInFlightTaskWork(taskId: string, reason: string, options: { userCanceled?: boolean } = {}): void {
|
private abortInFlightTaskWork(taskId: string, reason: string, options: { userCanceled?: boolean } = {}): void {
|
||||||
let hadActiveSurface = false;
|
let hadActiveSurface = false;
|
||||||
|
|
||||||
@@ -1595,50 +1684,19 @@ export class TaskExecutor {
|
|||||||
// 5. Each injection is logged to the task for user visibility
|
// 5. Each injection is logged to the task for user visibility
|
||||||
store.on("task:updated", async (task) => {
|
store.on("task:updated", async (task) => {
|
||||||
try {
|
try {
|
||||||
// Handle pause - terminate the agent session or step sessions
|
// FN-5256: handle pause by synchronously reaping every active session
|
||||||
if (task.paused && this.activeSessions.has(task.id)) {
|
// surface in one shot. Awaiting the abort ensures spawned shells are
|
||||||
executorLog.log(`Pausing ${task.id} — terminating agent session`);
|
// disposed before any re-dispatch can race the worktree.
|
||||||
this.pausedAborted.add(task.id);
|
if (
|
||||||
this.options.stuckTaskDetector?.untrackTask(task.id);
|
task.paused
|
||||||
const { session } = this.activeSessions.get(task.id)!;
|
&& (
|
||||||
session.dispose();
|
this.activeSessions.has(task.id)
|
||||||
// Also clean up in-memory state to prevent leaks if the task is later unpaused
|
|| this.activeStepExecutors.has(task.id)
|
||||||
this.loopRecoveryState.delete(task.id);
|
|| this.activeWorkflowStepSessions.has(task.id)
|
||||||
this.spawnedAgents.delete(task.id);
|
)
|
||||||
this.stuckAborted.delete(task.id);
|
) {
|
||||||
this.disposeSubagentsForTask(task.id, "task paused");
|
executorLog.log(`Pausing ${task.id} — awaiting in-flight session disposal`);
|
||||||
return;
|
await this.awaitAbortInFlightTaskWork(task.id, "task paused");
|
||||||
}
|
|
||||||
if (task.paused && this.activeStepExecutors.has(task.id)) {
|
|
||||||
executorLog.log(`Pausing ${task.id} — terminating step sessions`);
|
|
||||||
this.pausedAborted.add(task.id);
|
|
||||||
this.options.stuckTaskDetector?.untrackTask(task.id);
|
|
||||||
const stepExecutor = this.activeStepExecutors.get(task.id)!;
|
|
||||||
await stepExecutor.terminateAllSessions();
|
|
||||||
// Also clean up in-memory state to prevent leaks if the task is later unpaused
|
|
||||||
this.loopRecoveryState.delete(task.id);
|
|
||||||
this.spawnedAgents.delete(task.id);
|
|
||||||
this.stuckAborted.delete(task.id);
|
|
||||||
this.disposeSubagentsForTask(task.id, "task paused");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
if (task.paused && this.activeWorkflowStepSessions.has(task.id)) {
|
|
||||||
executorLog.log(`Pausing ${task.id} — terminating workflow step session`);
|
|
||||||
this.pausedAborted.add(task.id);
|
|
||||||
this.options.stuckTaskDetector?.untrackTask(task.id);
|
|
||||||
const workflowSession = this.activeWorkflowStepSessions.get(task.id)!;
|
|
||||||
const sessionWithAbort = workflowSession as AgentSession & { abort?: () => Promise<void> };
|
|
||||||
if (typeof sessionWithAbort.abort === "function") {
|
|
||||||
await sessionWithAbort.abort().catch((err) =>
|
|
||||||
executorLog.warn(`Failed to abort workflow step session for pause ${task.id}: ${err}`),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
workflowSession.dispose();
|
|
||||||
this.deleteActiveWorkflowStepSession(task.id);
|
|
||||||
this.loopRecoveryState.delete(task.id);
|
|
||||||
this.spawnedAgents.delete(task.id);
|
|
||||||
this.stuckAborted.delete(task.id);
|
|
||||||
this.disposeSubagentsForTask(task.id, "task paused");
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -9172,12 +9230,33 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
|||||||
worktreePath,
|
worktreePath,
|
||||||
taskId,
|
taskId,
|
||||||
(path, ownerTaskId) => this.hasActiveWorktreeBinding(ownerTaskId, path),
|
(path, ownerTaskId) => this.hasActiveWorktreeBinding(ownerTaskId, path),
|
||||||
|
{
|
||||||
|
processActiveProbe: (probeTaskId) => executingTaskLock.has(probeTaskId),
|
||||||
|
},
|
||||||
);
|
);
|
||||||
if (outcome.action === "reconciled") {
|
if (outcome.action === "reconciled") {
|
||||||
executorLog.warn(
|
executorLog.warn(
|
||||||
`[FN-5346] ${taskId}: dropped stale self-owned activeSessionRegistry entry before removeWorktree at ${worktreePath}`,
|
`[FN-5346] ${taskId}: dropped stale self-owned activeSessionRegistry entry before removeWorktree at ${worktreePath}`,
|
||||||
);
|
);
|
||||||
await this.store.logEntry(taskId, "Cleared stale self-owned active-session entry before remove", worktreePath);
|
await this.store.logEntry(taskId, "Cleared stale self-owned active-session entry before remove", worktreePath);
|
||||||
|
} else if (outcome.action === "process-active-refuses") {
|
||||||
|
executorLog.warn(
|
||||||
|
`[FN-5256] refused stale-self-owned reconcile for ${taskId}: process-active=true at ${worktreePath}`,
|
||||||
|
);
|
||||||
|
await this.store.logEntry(
|
||||||
|
taskId,
|
||||||
|
"Refused stale self-owned reconcile — task still actively executing",
|
||||||
|
worktreePath,
|
||||||
|
).catch(() => undefined);
|
||||||
|
} else if (outcome.action === "too-recent-refuses") {
|
||||||
|
executorLog.warn(
|
||||||
|
`[FN-5256] refused stale-self-owned reconcile for ${taskId}: age=${outcome.ageMs}ms (<${outcome.minIdleMs}ms) at ${worktreePath}`,
|
||||||
|
);
|
||||||
|
await this.store.logEntry(
|
||||||
|
taskId,
|
||||||
|
`Refused stale self-owned reconcile — registration too recent (${outcome.ageMs}ms < ${outcome.minIdleMs}ms)`,
|
||||||
|
worktreePath,
|
||||||
|
).catch(() => undefined);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -9198,6 +9277,9 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
|||||||
audit: input.audit,
|
audit: input.audit,
|
||||||
expectedOwnerTaskId: input.taskId,
|
expectedOwnerTaskId: input.taskId,
|
||||||
liveOwnerProbe: (path: string, ownerTaskId: string) => this.hasActiveWorktreeBinding(ownerTaskId, path),
|
liveOwnerProbe: (path: string, ownerTaskId: string) => this.hasActiveWorktreeBinding(ownerTaskId, path),
|
||||||
|
// FN-5256: route the worktree-backend defensive reconcile through the
|
||||||
|
// hardened gates (process-active + min-idle window).
|
||||||
|
processActiveProbe: (probeTaskId: string) => executingTaskLock.has(probeTaskId),
|
||||||
} as const;
|
} as const;
|
||||||
try {
|
try {
|
||||||
await removeWorktree(removeArgs);
|
await removeWorktree(removeArgs);
|
||||||
@@ -9207,17 +9289,33 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
|||||||
&& error.details.taskId === input.taskId
|
&& error.details.taskId === input.taskId
|
||||||
&& !this.hasActiveWorktreeBinding(input.taskId, input.worktreePath)
|
&& !this.hasActiveWorktreeBinding(input.taskId, input.worktreePath)
|
||||||
) {
|
) {
|
||||||
const reconcileResult = activeSessionRegistry.reconcileStaleSelfOwned(input.worktreePath, input.taskId);
|
// FN-5256: route the post-throw reconcile through the hardened path so
|
||||||
if (reconcileResult.reconciled) {
|
// process-active and too-recent signals also gate this leg.
|
||||||
|
const outcome = reconcileSelfOwnedActiveSessionForRemoval(
|
||||||
|
activeSessionRegistry,
|
||||||
|
input.worktreePath,
|
||||||
|
input.taskId,
|
||||||
|
(path, ownerTaskId) => this.hasActiveWorktreeBinding(ownerTaskId, path),
|
||||||
|
{
|
||||||
|
processActiveProbe: (probeTaskId) => executingTaskLock.has(probeTaskId),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
if (outcome.action === "reconciled") {
|
||||||
await this.store.logEntry(
|
await this.store.logEntry(
|
||||||
input.taskId,
|
input.taskId,
|
||||||
"Reconciled stale self-owned active-session registration (post-throw)",
|
"Reconciled stale self-owned active-session registration (post-throw)",
|
||||||
input.worktreePath,
|
input.worktreePath,
|
||||||
);
|
);
|
||||||
}
|
|
||||||
await removeWorktree(removeArgs);
|
await removeWorktree(removeArgs);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if (outcome.action === "process-active-refuses" || outcome.action === "too-recent-refuses") {
|
||||||
|
executorLog.warn(
|
||||||
|
`[FN-5256] post-throw reconcile refused for ${input.taskId} at ${input.worktreePath}: action=${outcome.action}`,
|
||||||
|
);
|
||||||
|
// Refused — surface the original error so the caller can decide.
|
||||||
|
}
|
||||||
|
}
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -208,6 +208,7 @@ export type DatabaseMutationType =
|
|||||||
| "task:auto-recover-node-unreachable"
|
| "task:auto-recover-node-unreachable"
|
||||||
| "task:auto-recover-worktree-metadata-rebound"
|
| "task:auto-recover-worktree-metadata-rebound"
|
||||||
| "task:auto-recover-worktree-metadata-cleared"
|
| "task:auto-recover-worktree-metadata-cleared"
|
||||||
|
| "task:auto-recover-worktree-metadata-skipped-active"
|
||||||
// task:auto-archived-ghost-bug metadata: { findings: Array<{ construct: { kind: string; raw: string; filePath?: string; line?: number }; matched: boolean; probeError?: string; output?: string }>; reason: string }
|
// task:auto-archived-ghost-bug metadata: { findings: Array<{ construct: { kind: string; raw: string; filePath?: string; line?: number }; matched: boolean; probeError?: string; output?: string }>; reason: string }
|
||||||
// task:auto-archived-duplicate metadata: { siblingTaskIds: string[]; scores: Record<string, number> }
|
// task:auto-archived-duplicate metadata: { siblingTaskIds: string[]; scores: Record<string, number> }
|
||||||
// task:broad-scope-flagged-at-triage metadata: { score: number; reasons: string[]; signals: { size: "S"|"M"|"L"|null; stepCount: number; fileScopeCount: number; failingFileMentions: number }; thresholds: { stepsHigh: number; fileScopeHigh: number; failingFileMentionsHigh: number; sizeLStepsThreshold: number }; version: number }
|
// task:broad-scope-flagged-at-triage metadata: { score: number; reasons: string[]; signals: { size: "S"|"M"|"L"|null; stepCount: number; fileScopeCount: number; failingFileMentions: number }; thresholds: { stepsHigh: number; fileScopeHigh: number; failingFileMentionsHigh: number; sizeLStepsThreshold: number }; version: number }
|
||||||
|
|||||||
@@ -25,7 +25,7 @@
|
|||||||
|
|
||||||
import { exec, execSync } from "node:child_process";
|
import { exec, execSync } from "node:child_process";
|
||||||
import { promisify } from "node:util";
|
import { promisify } from "node:util";
|
||||||
import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
|
import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
|
||||||
import { isAbsolute, join, relative, resolve } from "node:path";
|
import { isAbsolute, join, relative, resolve } from "node:path";
|
||||||
import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, countRecentIdenticalStallEntries, detectSelfDefeatingDependency, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, parseExplicitDuplicateMarker, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core";
|
import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, countRecentIdenticalStallEntries, detectSelfDefeatingDependency, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, parseExplicitDuplicateMarker, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core";
|
||||||
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
||||||
@@ -2576,7 +2576,10 @@ export class SelfHealingManager {
|
|||||||
|
|
||||||
private async emitWorktreeMetadataAuditEvent(input: {
|
private async emitWorktreeMetadataAuditEvent(input: {
|
||||||
taskId: string;
|
taskId: string;
|
||||||
mutationType: "task:auto-recover-worktree-metadata-rebound" | "task:auto-recover-worktree-metadata-cleared";
|
mutationType:
|
||||||
|
| "task:auto-recover-worktree-metadata-rebound"
|
||||||
|
| "task:auto-recover-worktree-metadata-cleared"
|
||||||
|
| "task:auto-recover-worktree-metadata-skipped-active";
|
||||||
previousWorktree: string | null;
|
previousWorktree: string | null;
|
||||||
newWorktree: string | null;
|
newWorktree: string | null;
|
||||||
previousBranch: string | null;
|
previousBranch: string | null;
|
||||||
@@ -2809,7 +2812,20 @@ export class SelfHealingManager {
|
|||||||
|
|
||||||
const allTasks = await this.store.listTasks({ slim: true, includeArchived: false });
|
const allTasks = await this.store.listTasks({ slim: true, includeArchived: false });
|
||||||
const branchMap = await getRegisteredWorktreeBranchMap(this.options.rootDir);
|
const branchMap = await getRegisteredWorktreeBranchMap(this.options.rootDir);
|
||||||
const registeredPaths = new Set(branchMap.values());
|
// FN-5256: macOS git surfaces realpath-normalized worktree paths (/private/var/...)
|
||||||
|
// while task.worktree may be persisted as the symlinked path. Compare on realpath
|
||||||
|
// to avoid false-stale flagging that yanks a live worktree.
|
||||||
|
const safeRealpath = (path: string): string => {
|
||||||
|
try {
|
||||||
|
return realpathSync(path);
|
||||||
|
} catch {
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
const registeredRealpaths = new Set<string>();
|
||||||
|
for (const path of branchMap.values()) {
|
||||||
|
registeredRealpaths.add(safeRealpath(path));
|
||||||
|
}
|
||||||
let repaired = 0;
|
let repaired = 0;
|
||||||
|
|
||||||
for (const task of allTasks) {
|
for (const task of allTasks) {
|
||||||
@@ -2823,8 +2839,9 @@ export class SelfHealingManager {
|
|||||||
if (activeSessionRegistry.isPathActive(task.worktree)) continue;
|
if (activeSessionRegistry.isPathActive(task.worktree)) continue;
|
||||||
|
|
||||||
const normalizedBranch = canonicalFusionBranchName(task.id);
|
const normalizedBranch = canonicalFusionBranchName(task.id);
|
||||||
const canonicalTaskWorktree = resolve(task.worktree);
|
const resolvedTaskWorktree = resolve(task.worktree);
|
||||||
const stale = !existsSync(task.worktree) || !registeredPaths.has(canonicalTaskWorktree);
|
const realpathTaskWorktree = safeRealpath(resolvedTaskWorktree);
|
||||||
|
const stale = !existsSync(task.worktree) || !registeredRealpaths.has(realpathTaskWorktree);
|
||||||
if (!stale) continue;
|
if (!stale) continue;
|
||||||
|
|
||||||
const previousWorktree = task.worktree;
|
const previousWorktree = task.worktree;
|
||||||
@@ -2848,6 +2865,25 @@ export class SelfHealingManager {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// FN-5256: never null out worktree/branch metadata for an active task. If a
|
||||||
|
// live task's worktree looks stale here (and we couldn't rebind to a live
|
||||||
|
// fusion/<id>), the executor's own recovery paths will detect and recreate
|
||||||
|
// it. Clearing here yanks the worktree from a still-running shell.
|
||||||
|
if (task.column === "in-progress" || task.column === "in-review") {
|
||||||
|
await this.emitWorktreeMetadataAuditEvent({
|
||||||
|
taskId: task.id,
|
||||||
|
mutationType: "task:auto-recover-worktree-metadata-skipped-active",
|
||||||
|
previousWorktree,
|
||||||
|
newWorktree: previousWorktree,
|
||||||
|
previousBranch,
|
||||||
|
newBranch: previousBranch,
|
||||||
|
});
|
||||||
|
worktreeMetadataReconcileLog.warn(
|
||||||
|
`[FN-5256] skipped clearing worktree metadata for active ${task.column} task ${task.id}: ${previousWorktree} (${previousBranch ?? "<none>"})`,
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
await this.store.updateTask(task.id, { worktree: null, branch: null });
|
await this.store.updateTask(task.id, { worktree: null, branch: null });
|
||||||
await this.emitWorktreeMetadataAuditEvent({
|
await this.emitWorktreeMetadataAuditEvent({
|
||||||
taskId: task.id,
|
taskId: task.id,
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import {
|
|||||||
activeSessionRegistry,
|
activeSessionRegistry,
|
||||||
reconcileSelfOwnedActiveSessionForRemoval,
|
reconcileSelfOwnedActiveSessionForRemoval,
|
||||||
type LiveBindingProbe,
|
type LiveBindingProbe,
|
||||||
|
type ProcessActiveProbe,
|
||||||
} from "./active-session-registry.js";
|
} from "./active-session-registry.js";
|
||||||
import type { RunAuditor } from "./run-audit.js";
|
import type { RunAuditor } from "./run-audit.js";
|
||||||
import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
||||||
@@ -782,6 +783,8 @@ export async function removeWorktree(input: {
|
|||||||
timeout?: number;
|
timeout?: number;
|
||||||
expectedOwnerTaskId?: string;
|
expectedOwnerTaskId?: string;
|
||||||
liveOwnerProbe?: LiveBindingProbe;
|
liveOwnerProbe?: LiveBindingProbe;
|
||||||
|
processActiveProbe?: ProcessActiveProbe;
|
||||||
|
reconcileMinIdleMs?: number;
|
||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
const logger = {
|
const logger = {
|
||||||
log: (_message: string): void => {},
|
log: (_message: string): void => {},
|
||||||
@@ -798,6 +801,10 @@ export async function removeWorktree(input: {
|
|||||||
input.worktreePath,
|
input.worktreePath,
|
||||||
input.expectedOwnerTaskId,
|
input.expectedOwnerTaskId,
|
||||||
input.liveOwnerProbe,
|
input.liveOwnerProbe,
|
||||||
|
{
|
||||||
|
processActiveProbe: input.processActiveProbe,
|
||||||
|
minIdleMs: input.reconcileMinIdleMs,
|
||||||
|
},
|
||||||
);
|
);
|
||||||
if (reconciled.action === "reconciled") {
|
if (reconciled.action === "reconciled") {
|
||||||
await input.audit?.git({
|
await input.audit?.git({
|
||||||
|
|||||||
Reference in New Issue
Block a user