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

clawsweeper[bot] · spencer2211 · claude · web-flow · commit 409d1a713541 · 2026-06-03T11:51:43.000Z
Summary: - The PR makes prompt-release fence bookkeeping exception-safe so the session write lock is released even when fence reads throw, and adds a regression test for that path. - PR surface: Source +6, Tests +27. Total +33 across 2 files. - Reproducibility: yes. source-reproducible with provided real-output proof: current main clears `heldLock` be ... ire timing out after an injected `EIO`. I did not run the harness locally because this review is read-only. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(agents): release session write lock if fence read throws on promp… Validation: - ClawSweeper review passed for head 394d978. - Required merge gates passed before the squash merge. Prepared head SHA: 394d978 Review: #89811 (comment) Co-authored-by: Spencer Fuller <spencer.p.fuller@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
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
@@ -34,6 +34,7 @@ const lockOptions = {
 const tempDirs: string[] = [];
 
 afterEach(async () => {
+  vi.restoreAllMocks();
   resetEmbeddedAttemptSessionFileOwnersForTest();
   resetSessionWriteLockStateForTest();
   for (const dir of tempDirs.splice(0)) {
@@ -136,6 +137,32 @@ describe("embedded attempt session lock lifecycle", () => {
     expect(releases).toEqual(["held"]);
   });
 
+  it("releases the eagerly-held lock when the fence read throws during prompt release", 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);
+
+    try {
+      await expect(controller.releaseForPrompt()).rejects.toThrow();
+
+      // The underlying file lock must still be released so later turns do not wait for
+      // the full maxHoldMs watchdog before the stale lease is reclaimed.
+      expect(release).toHaveBeenCalledTimes(1);
+    } finally {
+      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,23 @@ 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();
+      // Clearing `heldLock` transfers release ownership to this block. Fence reads can
+      // throw after that transfer; release the underlying file lock anyway so later
+      // turns do not wait for the maxHoldMs watchdog.
+      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);
     }
