fix(core): dependency update deadlocked on a self-blocked task (#2810)

## The bug

`updateTaskDependenciesImpl` wraps its whole body in
`store.withTaskLock(id, …)`, then reads the current blocker with
`readDepTask(task.blockedBy)` → `store.getTask()`. `getTaskImpl` opens
with `withTaskLock(id, …)` too, and the per-task lock is
**non-reentrant**.

So when `blockedBy` is the task's **own id**, the call waits forever on
a lock its own frame holds — and holds that lock while doing so, leaving
the row permanently unlockable.

## Found by generalising #2809, not by luck

#2809 removed one `getTask`-inside-`withTaskLock`. An AST scan for the
same shape across `packages/core` and `packages/engine` returned
**exactly three sites**:

| site | verdict |
|---|---|
| `lifecycle-ops.ts:1049` | the deadlock fixed in #2809 |
| `update-task-deps.ts:233` (`assertTaskExists`) | **safe** — a
self-dependency is rejected 15 lines earlier |
| `update-task-deps.ts:344` (`readDepTask`) | **this bug** |

Both surviving sites carry the same `FNXC:SqliteDualPathCleanup` note —
*"In backend mode, readTaskFromDb uses store.db (SQLite) which is
unavailable. Replace with async store.getTask() calls."* That port is
the common cause across the whole class: it swapped a **lock-free** read
for a **lock-acquiring** one.

## Why `blockedBy === id` is reachable

The dependencies list rejects self-reference explicitly (*"Task X cannot
depend on itself"*) — and that guard is precisely why the sibling
`assertTaskExists` read on this same lock is safe, so it is left
unchanged. **`blockedBy` has no such guard:** `updateTask({ blockedBy
})` accepts the task's own id.

The first test asserts that rather than assuming it. The whole
regression rests on that state being reachable, so it is proven, not
stipulated — and it also pins the asymmetry, so a future guard on
`blockedBy` will show up here as a deliberate change.

## The fix

Return the in-lock copy already in scope instead of re-reading. One
line, no new read path, and **strictly more correct than a re-read**: it
is the state this mutation is reasoning about, rather than whatever a
concurrent writer left behind.

## Verification

- **Mutation-verified against the real defect.** With the fix reverted
the regression case fails by name — `updateTaskDependencies did not
settle within 8000ms — deadlock` — while the precondition and the
ordinary-path cases stay green. That is the actual pre-fix behaviour.
- **A differential** covering the ordinary case (blocked by *another*
task). Without it, a fix that short-circuited *every* blocker read would
pass everything else.
- Timeboxed for the same reason as #2809: a deadlock otherwise surfaces
as a suite-level timeout naming no case. Not a flake knob — the fixed
path settles in ~0.5 s and the broken one never settles.
- `pnpm test:gate` — **exit 0**
- `pnpm lint` — clean

Changeset included (`patch`, category `fix`). Independent of #2809 —
different file, no overlap — but the same class, and the scan above is
the argument that the class is now closed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

.changeset/self-blocked-dependency-deadlock.md

@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Fix a hang when editing dependencies on a task that was blocked by itself.
+category: fix
+dev: `updateTaskDependenciesImpl` runs inside `withTaskLock(id)` and read the current blocker via `store.getTask()`, which re-enters the same non-reentrant lock when `blockedBy === id`. Returns the in-lock task copy instead. Second instance of the class fixed in the transition-pending recovery; found by an AST scan for `getTask` nested inside `withTaskLock`.

packages/core/src/task-store/update-task-deps.ts

@@ -336,6 +336,23 @@ export async function updateTaskDependenciesImpl(store: TaskStore, id: string, m
        * to resolve unresolved dependency and current blocker columns.
        */
       const readDepTask = async (depId: string): Promise<Task | null> => {
+        /*
+        FNXC:PostgresCutover 2026-07-31-17:10 (DEADLOCK, same class as PR #2809):
+        THE TASK WE ALREADY HOLD THE LOCK FOR IS ALREADY IN SCOPE. This closure runs inside
+        `store.withTaskLock(id, ...)` (the wrapper at the top of `updateTaskDependenciesImpl`), and
+        `store.getTask()` acquires that same lock — `getTaskImpl` opens with `withTaskLock(id, ...)`
+        and the per-task lock is NON-REENTRANT. Re-reading `id` through it waits forever on a lock
+        this frame holds.
+
+        REACHED VIA `task.blockedBy`, whose only caller passes exactly that. `blockedBy === id` is
+        writable today: `updateTask({ blockedBy })` has no self-reference guard, unlike the
+        dependencies list, which rejects `dependencyId === id` a few lines above (that guard is why
+        the sibling `assertTaskExists` read on this same lock is safe and is left unchanged).
+
+        Returning the in-lock copy is also strictly more correct than a re-read: it is the state this
+        mutation is reasoning about, not whatever a concurrent writer left behind.
+        */
+        if (depId === id) return task;
         /*
         FNXC:SqliteDualPathCleanup 2026-07-26-15:00:
         Treat not-found as null; rethrow unexpected PostgreSQL failures.

packages/engine/src/__tests__/self-blocked-dependency-deadlock.pg.test.ts

@@ -0,0 +1,123 @@
+/*
+FNXC:PostgresCutover 2026-07-31-17:25 (regression — the SECOND instance of PR #2809's deadlock class):
+
+`updateTaskDependenciesImpl` wraps its whole body in `store.withTaskLock(id, ...)` and then reads the
+current blocker with `readDepTask(task.blockedBy)`, which calls `store.getTask()`. `getTaskImpl` opens
+with `store.withTaskLock(id, ...)` too, and the per-task lock is NON-REENTRANT — the invariant stated
+in prose in `branch-and-pr-entities.ts` and `workflow-ops.ts`. So when `blockedBy` happens to be the
+task's OWN id, the call waits forever on a lock its own frame holds.
+
+FOUND BY GENERALISING #2809 rather than by luck. That fix removed one `getTask`-inside-`withTaskLock`;
+an AST scan for the same shape across core and engine returned exactly three sites — the one #2809
+fixed and the two in this file. This is the reachable one.
+
+WHY `blockedBy === id` IS REACHABLE. The dependencies list rejects self-reference explicitly
+("Task X cannot depend on itself"), and that guard is why the sibling `assertTaskExists` read on this
+same lock is safe. `blockedBy` has no such guard: `updateTask({ blockedBy })` accepts the task's own
+id, which the first case below asserts rather than assumes — the whole regression rests on it, so it
+is proven, not stipulated.
+
+THE FIX returns the in-lock copy already in scope instead of re-reading. That is also strictly more
+correct than a re-read: it is the state this mutation is reasoning about, rather than whatever a
+concurrent writer left behind.
+
+TIMEBOXED for the same reason as #2809: a deadlock otherwise surfaces as a suite-level timeout naming
+no case. The deadline is not a flake knob — the fixed path completes in milliseconds and the broken
+one never completes, so there is no value in between to tune to.
+
+LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable. Throwaway per-file
+database; never port 4040.
+*/
+import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest";
+import "@fusion/core"; // registers the built-in column traits
+import type { TaskStore } from "@fusion/core";
+
+import {
+  pgDescribe,
+  createSharedPgTaskStoreTestHarness,
+  type SharedPgTaskStoreHarness,
+} from "../../../core/src/__test-utils__/pg-test-harness.js";
+
+const DEADLINE_MS = 8_000;
+
+async function within<T>(work: Promise<T>, label: string): Promise<T> {
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  try {
+    return await Promise.race([
+      work,
+      new Promise<never>((_, reject) => {
+        timer = setTimeout(() => reject(new Error(`${label} did not settle within ${DEADLINE_MS}ms — deadlock`)), DEADLINE_MS);
+      }),
+    ]);
+  } finally {
+    if (timer) clearTimeout(timer);
+  }
+}
+
+pgDescribe("dependency update does not deadlock on a self-blocked task", () => {
+  const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
+    prefix: "fusion_selfblock_deadlock",
+  });
+
+  beforeAll(h.beforeAll);
+  afterAll(h.afterAll);
+  beforeEach(async () => { await h.beforeEach(); });
+  afterEach(async () => { await h.afterEach(); });
+
+  it("PRECONDITION — `blockedBy` accepts the task's own id (dependencies do not)", async () => {
+    /*
+    The regression below is only meaningful if this state is reachable, so it is proven here rather
+    than assumed. The asymmetry is the point: the dependencies list rejects self-reference and
+    `blockedBy` does not.
+    */
+    const store = h.store();
+    const task = await store.createTask({ description: "self-blocked precondition" });
+
+    await store.updateTask(task.id, { blockedBy: task.id });
+    store.taskCache.delete(task.id);
+    expect((await store.getTask(task.id))?.blockedBy).toBe(task.id);
+
+    await expect(
+      store.updateTaskDependencies(task.id, { operation: "add", dependency: task.id } as never),
+    ).rejects.toThrow(/cannot depend on itself/i);
+  });
+
+  it("REGRESSION — updating dependencies on a self-blocked task settles instead of hanging", async () => {
+    /*
+    Before the fix this never returned, and it held the task's lock while not returning, so the row
+    was left permanently unlockable as well.
+    */
+    const store = h.store();
+    const blocked = await store.createTask({ description: "self-blocked" });
+    const other = await store.createTask({ description: "a real dependency" });
+    await store.updateTask(blocked.id, { blockedBy: blocked.id });
+    store.taskCache.delete(blocked.id);
+
+    const updated = await within(
+      store.updateTaskDependencies(blocked.id, { operation: "add", dependency: other.id } as never),
+      "updateTaskDependencies",
+    );
+
+    expect(updated.dependencies).toContain(other.id);
+  });
+
+  it("the ordinary path — a task blocked by ANOTHER task — is unchanged", async () => {
+    /*
+    The differential. Without it the fix could have short-circuited every blocker read, not just the
+    self-referential one, and every assertion above would still pass.
+    */
+    const store = h.store();
+    const blocker = await store.createTask({ description: "blocker" });
+    const dependent = await store.createTask({ description: "dependent" });
+    const other = await store.createTask({ description: "second dependency" });
+    await store.updateTask(dependent.id, { blockedBy: blocker.id, dependencies: [blocker.id] });
+    store.taskCache.delete(dependent.id);
+
+    const updated = await within(
+      store.updateTaskDependencies(dependent.id, { operation: "add", dependency: other.id } as never),
+      "updateTaskDependencies (ordinary)",
+    );
+
+    expect(updated.dependencies).toEqual(expect.arrayContaining([blocker.id, other.id]));
+  });
+});