FN-7717: release active-session locks when a task is archived
Archiving a task from triage/planning/todo (not just in-progress) previously left leaked active-session-registry entries, so a successor task could hit ActiveSessionPathHeldByForeignTaskError and get blocked from Plan Review. - Add an explicit `to === "archived"` branch in the task-move handler that awaits abort of in-flight task work and sweeps any leftover activeSessionRegistry paths for the task, checked before the narrower `from === "in-progress"` branch so direct in-progress→archived transitions are covered too. - Deliberately exclude `to === "done"` / `to === "in-review"` from this sweep since those columns legitimately hold ai-merge / workspace-repo-land merge leases that must survive the transition. - Add regression test coverage for archive releasing active sessions across originating columns. - Add changeset and architecture doc note. Files changed: .../fn-7717-archive-active-session-release.md | 7 + docs/architecture.md | 1 + ...xecutor-archive-releases-active-session.test.ts | 167 +++++++++++++++++++++ packages/engine/src/executor.ts | 35 +++++ 4 files changed, 210 insertions(+) Fusion-Task-Id: FN-7717 Fusion-Task-Lineage: 7cff6821-7bb3-4b75-b502-a26467ca7f51 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7717-archive-active-session-release.md
Normal file
7
.changeset/fn-7717-archive-active-session-release.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Archiving a task now releases its active-session lock so the next task can run Plan Review.
|
||||
category: fix
|
||||
dev: task:moved handler in packages/engine/src/executor.ts now disposes active surfaces and sweeps activeSessionRegistry paths for any move to the terminal "archived" column (previously only from==="in-progress"); done/in-review merge leases are deliberately untouched.
|
||||
@@ -2102,6 +2102,7 @@ This section preserves the detailed lifecycle/self-healing contracts that were f
|
||||
- **Landed-files attribution (FN-5103)**: Rebase-strategy `mergeDetails.landedFiles` / `filesChanged` / `insertions` / `deletions` are captured from task-attributable commits only via `filterFilesToOwnTaskCommits` (subject-prefix + trailer + bracket-prefix evidence), tagged `landedFilesAttributionRestricted: true`. Zero own commits → `landedFiles: []` and `noOpVerifiedShortCircuit: true`. FN-5304 guard: when `<rebaseBaseSha>..HEAD` reports zero own commits, merger must also validate the source `fusion/<id>` tip; if that source tip still has attributable own commits relative to `rebaseBaseSha`, throw `SilentNoOpAttributionMismatchError`, refuse writing `mergeConfirmed: true`, park the task in `in-review` with `status: "failed"`, and emit `merge:no-op-attribution-mismatch`. If source ref is unavailable, skip with diagnostic + `merge:no-op-attribution-mismatch-skipped` (`reason: "source-ref-unavailable"`). Attribution-helper failures fall back to the unrestricted `rebaseBaseSha..sha` walk and set `landedFilesCaptureFallback: 'attribution-failed'`. Self-healing `recoverDoneTaskMergeMetadata` skips reconcile when `landedFilesAttributionRestricted` or `noOpVerifiedShortCircuit` is set so the narrower set is not overwritten with the full range. Squash-strategy capture is unchanged.
|
||||
- **Soft-delete scheduler invalidation (FN-5137)**: `task:deleted` events must invalidate `AutoClaimSnapshotManager` and clear scheduler bookkeeping (`pausedTaskIds`, `failedTaskIds`, `wasNodeDispatchValidationBlocked`, `wasNodeBlocked`); `executor.execute()` / `resumeOrphaned()` / `resumeTaskForAgent()` refuse any task with `deletedAt` set.
|
||||
- **Soft-delete in-flight abort (FN-5142)**: `task:deleted` must immediately abort/dispose active executor work (`activeSessions`, `activeStepExecutors`, `activeWorkflowStepSessions`, reviewer subagents), interrupt active merge state (`mergeAbortController`, `activeMergeSession`, `activeMergeTaskId`, `mergeActive`, `mergeQueue`, `pausedReviewTaskIds`), and abort triage specify/subagent sessions for that id. Handlers are per-task and idempotent.
|
||||
- **Archive releases active-session locks (FN-7717)**: `task:moved` with `to === "archived"` (from ANY column, including in-progress via a direct single-hop `fn_task_archive`, and triage/planning where Plan Review and other workflow-step sessions run) now disposes in-flight session surfaces via `awaitAbortInFlightTaskWork` and sweeps any remaining `activeSessionRegistry` paths for the task, so a leaked lock can never survive archive and block a successor task's `registerPath` on the same session path. The `to === "archived"` check is ordered BEFORE the `from === "in-progress"` branch so a direct in-progress→archived move gets the same full cleanup instead of falling into the narrower in-progress-only branch. `to === "done"`/`"in-review"` are deliberately excluded — those columns legitimately hold `ai-merge`/`workspace-repo-land` merge leases.
|
||||
- **Soft-delete audit + column reconcile (FN-5175)**: `TaskStore.deleteTask` records a `runAuditEvents` row (`mutationType: "task:deleted"`, `domain: "database"`) inside the same transaction that sets `deletedAt`, and sets `"column" = 'archived'` on the row. Callers without a heartbeat run context (`fn task delete`, pi extension, dashboard delete route) pass an `auditContext` with `agentId: "system"` and a synthetic `runId`. The watcher cross-instance emit path does NOT re-record the audit event. The row stays in `tasks` (not `archivedTasks`); `archiveTask` is unchanged.
|
||||
- **Soft-delete resurrection guard (FN-5208)**: `TaskStore.readTaskJson()` must never fall back to `.fusion/tasks/<id>/task.json` when the DB row exists with `deletedAt` set — it throws `TaskDeletedError`. `atomicCreateTaskJson` / `atomicWriteTaskJson` / `atomicWriteTaskJsonWithAudit` refuse to upsert a task whose row is currently soft-deleted (unless the in-memory task carries `deletedAt` itself, for soft-delete maintenance paths), emit a `[soft-delete-resurrection-blocked]` log line, and record a `task:resurrection-blocked` run-audit event. Stale in-flight planner/triage writes for a soft-deleted ID surface `TaskDeletedError` and abort cleanly without emitting `task:created`.
|
||||
- **Exhausted in-review visibility surfaces (FN-5513/FN-6569)**: retry-exhausted merge failures (`column='in-review'`, `status='failed'`, `mergeRetries >= maxAutoMergeRetries`, default `3`) can remain soft-deleted for lifecycle safety, but are now intentionally discoverable through opt-in read paths: `TaskStore.listExhaustedInReviewTasks({ includeDeleted })`, `GET /api/tasks/exhausted-in-review`, `GET /api/tasks/:id?includeDeleted=true`, CLI `fn_task_show` soft-delete fallback marker, CLI `fn_task_list({ includeDeleted: true })`, and the dashboard ReliabilityView "Exhausted in-review (hidden blockers)" panel. This complements FN-5488/FN-5496 downstream blocker healing by surfacing the upstream blocker without mutating lifecycle state.
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-07-09-00:00 (FN-7717 regression):
|
||||
Archiving a task must release every activeSessionRegistry entry it holds. Plan Review and
|
||||
other workflow-step / step-session sessions run while a task is in triage/planning/todo (not
|
||||
in-progress), so the executor's task:moved handler previously only disposed session surfaces
|
||||
via the `from === "in-progress"` branch — a task archived from any OTHER column leaked its
|
||||
registry entry and blocked a successor task from registering the same session path with
|
||||
ActiveSessionPathHeldByForeignTaskError (NEXT-508 -> NEXT-433). This suite proves the fix
|
||||
across all three registration surfaces (executor / step-session / workflow-step), the
|
||||
leaked-entry sweep path (no in-memory session) including when archiving DIRECTLY from
|
||||
in-progress in a single task:moved hop (a branch-ordering gap the fix also closes), the
|
||||
done/in-review merge-lease exclusion, and the no-op case (task with no held paths).
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { TaskStore } from "@fusion/core";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { activeSessionRegistry, ActiveSessionPathHeldByForeignTaskError } from "../active-session-registry.js";
|
||||
|
||||
const SHARED_ROOT = "/tmp/fusion-test-archive-shared-root";
|
||||
|
||||
function createStore(): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
return Object.assign(emitter, {
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getRunContextFor: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
function makeExecutor(): { executor: TaskExecutor; store: TaskStore & EventEmitter } {
|
||||
const store = createStore();
|
||||
const executor = new TaskExecutor(store, SHARED_ROOT);
|
||||
return { executor, store };
|
||||
}
|
||||
|
||||
function makeTask(id: string): any {
|
||||
return { id, column: "archived" };
|
||||
}
|
||||
|
||||
describe("archiving a task releases its active-session registry entries (FN-7717)", () => {
|
||||
beforeEach(() => activeSessionRegistry.clear());
|
||||
afterEach(() => activeSessionRegistry.clear());
|
||||
|
||||
it("releases a workflow-step session held by a task archived from triage, letting a successor acquire the same path", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
// Task A registers a workflow-step (Plan Review) session on the shared root — the
|
||||
// reported NEXT-508 case.
|
||||
(executor as any).setActiveWorkflowStepSession("TASK-A", {}, SHARED_ROOT);
|
||||
expect(activeSessionRegistry.isPathActive(SHARED_ROOT)).toBe(true);
|
||||
|
||||
// Drive the archive transition: to === "archived", from a NON-in-progress column
|
||||
// (Plan Review runs in triage), exactly like archiveTask emits.
|
||||
store.emit("task:moved", { task: makeTask("TASK-A"), from: "triage", to: "archived", source: "user" });
|
||||
|
||||
// Await the disposal chain the handler kicked off via trackTaskDisposal.
|
||||
await (executor as any).pendingTaskDisposals.get("TASK-A");
|
||||
|
||||
expect(activeSessionRegistry.isPathActive(SHARED_ROOT)).toBe(false);
|
||||
expect(activeSessionRegistry.pathsForTask("TASK-A")).toHaveLength(0);
|
||||
|
||||
// Successor task B can now register the same path without throwing.
|
||||
expect(() =>
|
||||
activeSessionRegistry.registerPath(SHARED_ROOT, { taskId: "TASK-B", kind: "workflow-step", ownerKey: "TASK-B#workflow-step" }),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it("releases executor and step-session surfaces archived from planning/todo columns", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
(executor as any).setActiveSession("TASK-C", { session: { dispose: vi.fn() } }, `${SHARED_ROOT}-c`);
|
||||
(executor as any).setActiveStepExecutor("TASK-D", { terminateAllSessions: vi.fn().mockResolvedValue(undefined) }, `${SHARED_ROOT}-d`);
|
||||
|
||||
store.emit("task:moved", { task: makeTask("TASK-C"), from: "planning", to: "archived", source: "user" });
|
||||
store.emit("task:moved", { task: makeTask("TASK-D"), from: "todo", to: "archived", source: "user" });
|
||||
|
||||
await Promise.all([
|
||||
(executor as any).pendingTaskDisposals.get("TASK-C"),
|
||||
(executor as any).pendingTaskDisposals.get("TASK-D"),
|
||||
]);
|
||||
|
||||
expect(activeSessionRegistry.pathsForTask("TASK-C")).toHaveLength(0);
|
||||
expect(activeSessionRegistry.pathsForTask("TASK-D")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("sweeps a leaked registry entry with no in-memory session on archive", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
// Simulate a LEAKED entry: registered directly in the registry with no corresponding
|
||||
// in-memory activeSessions/activeStepExecutors/activeWorkflowStepSessions entry, so the
|
||||
// abort call itself finds nothing to dispose — only the sweep clears it.
|
||||
activeSessionRegistry.registerPath(`${SHARED_ROOT}-leak`, { taskId: "TASK-E", kind: "workflow-step", ownerKey: "TASK-E#workflow-step" });
|
||||
expect(activeSessionRegistry.isPathActive(`${SHARED_ROOT}-leak`)).toBe(true);
|
||||
|
||||
store.emit("task:moved", { task: makeTask("TASK-E"), from: "in-review", to: "archived", source: "user" });
|
||||
await (executor as any).pendingTaskDisposals.get("TASK-E");
|
||||
|
||||
expect(activeSessionRegistry.isPathActive(`${SHARED_ROOT}-leak`)).toBe(false);
|
||||
expect(activeSessionRegistry.pathsForTask("TASK-E")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("sweeps a leaked registry entry when a task is archived DIRECTLY from in-progress (single task:moved hop, no todo stop)", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
// fn_task_archive can move a live in-progress task straight to archived in one
|
||||
// `task:moved` event (from: "in-progress", to: "archived") with no intermediate
|
||||
// stop in "todo". Before the branch-ordering fix, this hit the narrower
|
||||
// `from === "in-progress"` branch first and skipped the archive-only leaked-entry
|
||||
// sweep, so a registry entry with no matching in-memory session would survive.
|
||||
activeSessionRegistry.registerPath(`${SHARED_ROOT}-inprogress-leak`, { taskId: "TASK-I", kind: "workflow-step", ownerKey: "TASK-I#workflow-step" });
|
||||
expect(activeSessionRegistry.isPathActive(`${SHARED_ROOT}-inprogress-leak`)).toBe(true);
|
||||
|
||||
store.emit("task:moved", { task: makeTask("TASK-I"), from: "in-progress", to: "archived", source: "user" });
|
||||
await (executor as any).pendingTaskDisposals.get("TASK-I");
|
||||
|
||||
expect(activeSessionRegistry.isPathActive(`${SHARED_ROOT}-inprogress-leak`)).toBe(false);
|
||||
expect(activeSessionRegistry.pathsForTask("TASK-I")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("does NOT clear a held merge lease when a task moves to done or in-review", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
activeSessionRegistry.registerPath(`${SHARED_ROOT}-merge-done`, { taskId: "TASK-F", kind: "ai-merge", ownerKey: "TASK-F#ai-merge" });
|
||||
activeSessionRegistry.registerPath(`${SHARED_ROOT}-merge-review`, { taskId: "TASK-G", kind: "workspace-repo-land", ownerKey: "TASK-G#workspace-repo-land" });
|
||||
|
||||
store.emit("task:moved", { task: makeTask("TASK-F"), from: "in-progress", to: "done", source: "engine" });
|
||||
store.emit("task:moved", { task: makeTask("TASK-G"), from: "in-progress", to: "in-review", source: "engine" });
|
||||
|
||||
// These moves go through the existing `from === "in-progress"` branch, which is
|
||||
// unrelated to and does not fire the new archive-only sweep — the merge lease survives.
|
||||
await Promise.resolve();
|
||||
|
||||
expect(activeSessionRegistry.isPathActive(`${SHARED_ROOT}-merge-done`)).toBe(true);
|
||||
expect(activeSessionRegistry.isPathActive(`${SHARED_ROOT}-merge-review`)).toBe(true);
|
||||
});
|
||||
|
||||
it("is a no-op that does not throw when archiving a task with no held registry paths", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
expect(() =>
|
||||
store.emit("task:moved", { task: makeTask("TASK-H"), from: "triage", to: "archived", source: "user" }),
|
||||
).not.toThrow();
|
||||
|
||||
await (executor as any).pendingTaskDisposals.get("TASK-H");
|
||||
expect(activeSessionRegistry.pathsForTask("TASK-H")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("reproduces the original ActiveSessionPathHeldByForeignTaskError before archive, and confirms it is gone after", async () => {
|
||||
const { executor, store } = makeExecutor();
|
||||
|
||||
(executor as any).setActiveWorkflowStepSession("NEXT-508", {}, SHARED_ROOT);
|
||||
|
||||
// Before archive: a second task trying to register the same path is rejected.
|
||||
expect(() =>
|
||||
activeSessionRegistry.registerPath(SHARED_ROOT, { taskId: "NEXT-433", kind: "workflow-step", ownerKey: "NEXT-433#workflow-step" }),
|
||||
).toThrow(ActiveSessionPathHeldByForeignTaskError);
|
||||
|
||||
store.emit("task:moved", { task: makeTask("NEXT-508"), from: "triage", to: "archived", source: "user" });
|
||||
await (executor as any).pendingTaskDisposals.get("NEXT-508");
|
||||
|
||||
// After archive: the successor can now acquire the path.
|
||||
expect(() =>
|
||||
activeSessionRegistry.registerPath(SHARED_ROOT, { taskId: "NEXT-433", kind: "workflow-step", ownerKey: "NEXT-433#workflow-step" }),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -2793,6 +2793,41 @@ export class TaskExecutor {
|
||||
})().catch((err) =>
|
||||
executorLog.error(`Failed to start ${task.id}:`, err),
|
||||
);
|
||||
} else if (to === "archived") {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-07-09-00:05:
|
||||
Archived is terminal, so it must release every active-session registry entry the
|
||||
task holds. Plan Review / other workflow-step and step-session sessions run while
|
||||
the task is in triage/planning/todo (not in-progress), so the old
|
||||
`from === "in-progress"`-only disposal branch below never fired for them — the
|
||||
registry entry (activeSessions / activeStepExecutors / activeWorkflowStepSessions,
|
||||
keyed on the shared project browse root) leaked past archive and blocked a
|
||||
successor task from acquiring the same session path with
|
||||
ActiveSessionPathHeldByForeignTaskError (FN-7717 / NEXT-508 -> NEXT-433). We
|
||||
deliberately do NOT do this for to === "done" / "in-review": those columns
|
||||
legitimately hold ai-merge / workspace-repo-land merge leases that must survive
|
||||
the transition (FN-6736 / Phase C/D merge-lease guarantees).
|
||||
|
||||
This branch is checked BEFORE `from === "in-progress"` (and handles it too — a
|
||||
task can be archived directly from in-progress via fn_task_archive, a single
|
||||
`task:moved` event with no intermediate todo hop). Ordering the plain
|
||||
`from === "in-progress"`-only branch first would let that direct
|
||||
in-progress → archived transition fall into the narrower branch and skip the
|
||||
leaked-entry sweep below, re-opening the exact class of leak this fix closes for
|
||||
that one origin column. `awaitAbortInFlightTaskWork` here is the same call the
|
||||
in-progress branch makes (superset of its cleanup), so no case regresses.
|
||||
*/
|
||||
this.trackTaskDisposal(
|
||||
task.id,
|
||||
this.awaitAbortInFlightTaskWork(task.id, "task archived").then(() => {
|
||||
// Belt-and-suspenders sweep: clear any registry entry that survived the
|
||||
// abort above because its in-memory session map was already empty
|
||||
// (a leaked entry with no live session to abort).
|
||||
for (const path of activeSessionRegistry.pathsForTask(task.id)) {
|
||||
activeSessionRegistry.unregisterPath(path);
|
||||
}
|
||||
}),
|
||||
);
|
||||
} else if (from === "in-progress") {
|
||||
this.trackTaskDisposal(
|
||||
task.id,
|
||||
|
||||
Reference in New Issue
Block a user