fix(agents): release session write lock if fence read throws on prompt release

spencer2211 · claude · spencer2211 · commit 1d91f1acb810 · 2026-06-02T20:11:58.000-05:00
releaseHeldLockWithFence (the embedded-attempt session-lock controller's
release-for-prompt path) cleared its in-memory `heldLock` reference and then
performed fence bookkeeping — readSessionFileFingerprint and
readSessionFileFenceSnapshot — before calling lock.release(). Both perform
filesystem I/O that can reject with a transient non-ENOENT error (e.g.
EIO/EMFILE under load). Because release() was not in a finally, such a throw
exited the function with `heldLock` already nulled and the underlying file
lock never released.

The lock then leaked on the live gateway process for the full maxHoldMs lease.
Since the controller holds the lock with maxHoldMs derived from the agent
timeout (~17 min) and the holder is the live pid (its /proc starttime matches
the lock payload), the stale-lock breaker correctly refuses to reclaim it — only
the watchdog TTL frees it. Meanwhile the attempt teardown still releases the
in-process session-file owner mutex, so the next interactive turn sails past the
mutex and blocks on the orphaned file lock, failing every turn with
SessionWriteLockTimeoutError for up to 17 minutes (observed: a Discord channel
dead 00:08–00:27).

Wrap the fence bookkeeping in try/finally so the file lock is always released
once heldLock is cleared, even if the fence reads throw. The original error
still propagates; only the lock-release invariant is made exception-safe.

Add regression coverage: a fence-read failure during releaseForPrompt must still
release the underlying lock.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts b/src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts
@@ -136,6 +136,31 @@ describe("embedded attempt session lock lifecycle", () => {
     expect(releases).toEqual(["held"]);
   });
 
+  it("releases the eagerly-held lock even when the fence read throws during prompt release (FAD-845)", async () => {
+    const release = vi.fn(async () => {});
+    const acquireSessionWriteLockLocalFad845 = vi.fn(async () => ({ release }));
+    const controller = await createEmbeddedAttemptSessionLockController({
+      acquireSessionWriteLock: acquireSessionWriteLockLocalFad845,
+      lockOptions,
+    });
+
+    // Simulate a transient, non-ENOENT filesystem error (e.g. EIO/EMFILE) while the
+    // prompt-release path reads the session-file fence. This fires AFTER the controller
+    // has cleared its in-memory `heldLock` reference but BEFORE the underlying file lock
+    // is released, which is the window that orphans the lock.
+    const statError = Object.assign(new Error("simulated I/O failure"), { code: "EIO" });
+    const statSpy = vi.spyOn(fs, "stat").mockRejectedValueOnce(statError);
+
+    await expect(controller.releaseForPrompt()).rejects.toThrow();
+
+    // The underlying file lock MUST still be released; otherwise it is orphaned on the
+    // live gateway process for the full maxHoldMs lease (~17 min), wedging every
+    // subsequent interactive turn with SessionWriteLockTimeoutError. See FAD-845.
+    expect(release).toHaveBeenCalledTimes(1);
+
+    statSpy.mockRestore();
+  });
+
   it("releaseHeldLockForAbort and dispose are idempotent in succession (#86816)", async () => {
     const release = vi.fn(async () => {});
     const acquireSessionWriteLockLocal26 = vi.fn(async () => ({ release }));
diff --git a/src/agents/embedded-agent-runner/run/attempt.session-lock.ts b/src/agents/embedded-agent-runner/run/attempt.session-lock.ts
@@ -801,17 +801,27 @@ export async function createEmbeddedAttemptSessionLockController(params: {
       }
       const lock = heldLock;
       heldLock = undefined;
-      const fingerprint = await readSessionFileFingerprint(params.lockOptions.sessionFile);
-      const ownedWrite = ownedSessionFileWrites.get(sessionFileFenceKey);
-      const trustedGeneration = trustSessionFileState(sessionFileFenceKey, fingerprint);
-      fenceFingerprint = fingerprint;
-      fenceSnapshot = await readSessionFileFenceSnapshot(params.lockOptions.sessionFile);
-      fenceGeneration =
-        ownedWrite && sameSessionFileFingerprint(ownedWrite.fingerprint, fingerprint)
-          ? ownedWrite.generation
-          : (trustedGeneration ?? fenceGeneration);
-      fenceActive = true;
-      await lock.release();
+      // Once `heldLock` is cleared the controller no longer tracks this lock, so the
+      // underlying file lock MUST be released even if the fence bookkeeping below
+      // throws. The fence reads perform filesystem I/O that can fail with a transient
+      // non-ENOENT error (e.g. EIO/EMFILE under load); without this guard such a throw
+      // would orphan the lock on the live gateway process for the full maxHoldMs lease
+      // (~17 min), wedging every subsequent interactive turn with
+      // SessionWriteLockTimeoutError until the watchdog reclaims it. See FAD-845.
+      try {
+        const fingerprint = await readSessionFileFingerprint(params.lockOptions.sessionFile);
+        const ownedWrite = ownedSessionFileWrites.get(sessionFileFenceKey);
+        const trustedGeneration = trustSessionFileState(sessionFileFenceKey, fingerprint);
+        fenceFingerprint = fingerprint;
+        fenceSnapshot = await readSessionFileFenceSnapshot(params.lockOptions.sessionFile);
+        fenceGeneration =
+          ownedWrite && sameSessionFileFingerprint(ownedWrite.fingerprint, fingerprint)
+            ? ownedWrite.generation
+            : (trustedGeneration ?? fenceGeneration);
+        fenceActive = true;
+      } finally {
+        await lock.release();
+      }
     } finally {
       finishHeldLockDrain(drainOwner);
     }
