fix(agents): bound abort-path session lock release; force-release on unsettled retained writes (#6)

matin · claude · matin · commit 817dcd6cc73c · 2026-06-03T16:56:35.000-07:00
A turn timeout aborts the run, but releaseHeldLockForAbort delegated to the graceful fence release, whose waitForRetainedLockIdle is unbounded. A retained transcript write pinned behind a hung provider stream (one that ignores abort, e.g. a stalled streamGenerateContent) therefore held the session .jsonl lock until the maxHoldMs watchdog (5-30 min), and every turn in between failed with SessionWriteLockTimeoutError. releaseHeldLockForAbort now races the graceful path against the abort settle bound (OPENCLAW_EMBEDDED_ABORT_SETTLE_TIMEOUT_MS). When the bound expires the underlying file lock is force-released and the controller poisoned via takeoverDetected, so the aborted run cannot perform torn writes after losing ownership. Retained writes that settle within the bound keep the existing graceful fence semantics. Prod incident: tulgey#225 (membrane 2026-06-01..03). Completes the lock-leak family of #87278 / #88623 / #89811, which release on settled aborts but not on aborts a hung provider call never lets settle. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.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
@@ -163,6 +163,73 @@ describe("embedded attempt session lock lifecycle", () => {
     }
   });
 
+  it("force-releases the held lock when a retained write never settles (tulgey#225)", async () => {
+    const release = vi.fn(async () => {});
+    const acquireSessionWriteLockStuck = vi.fn(async () => ({ release }));
+    const controller = await createEmbeddedAttemptSessionLockController({
+      acquireSessionWriteLock: acquireSessionWriteLockStuck,
+      lockOptions,
+      abortReleaseTimeoutMs: 20,
+    });
+
+    let markWriteStarted!: () => void;
+    const writeStarted = new Promise<void>((resolve) => {
+      markWriteStarted = resolve;
+    });
+    // A retained-lock write pinned behind a provider call that never settles
+    // (hung stream that ignores abort).
+    void controller.withSessionWriteLock(async () => {
+      markWriteStarted();
+      await new Promise<void>(() => {});
+    });
+    await writeStarted;
+
+    await controller.releaseHeldLockForAbort();
+
+    expect(release).toHaveBeenCalledTimes(1);
+    // The aborted run lost ownership; later writes from it must not tear the
+    // session file out from under the next turn's lock.
+    await expect(controller.withSessionWriteLock(async () => {})).rejects.toBeInstanceOf(
+      EmbeddedAttemptSessionTakeoverError,
+    );
+    await controller.dispose();
+    expect(release).toHaveBeenCalledTimes(1);
+  });
+
+  it("abort release stays graceful when retained writes settle within the bound (tulgey#225)", async () => {
+    const release = vi.fn(async () => {});
+    const acquireSessionWriteLockSettles = vi.fn(async () => ({ release }));
+    const controller = await createEmbeddedAttemptSessionLockController({
+      acquireSessionWriteLock: acquireSessionWriteLockSettles,
+      lockOptions,
+      abortReleaseTimeoutMs: 5_000,
+    });
+
+    let finishWrite!: () => void;
+    const writeCanFinish = new Promise<void>((resolve) => {
+      finishWrite = resolve;
+    });
+    let markWriteStarted!: () => void;
+    const writeStarted = new Promise<void>((resolve) => {
+      markWriteStarted = resolve;
+    });
+    const activeWrite = controller.withSessionWriteLock(async () => {
+      markWriteStarted();
+      await writeCanFinish;
+    });
+    await writeStarted;
+
+    const abortRelease = controller.releaseHeldLockForAbort();
+    finishWrite();
+    await activeWrite;
+    await abortRelease;
+
+    expect(release).toHaveBeenCalledTimes(1);
+    // Graceful release keeps the fence active — no takeover poisoning.
+    await expect(controller.withSessionWriteLock(async () => {})).resolves.toBeUndefined();
+    await controller.dispose();
+  });
+
   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
@@ -8,6 +8,7 @@ import { resolveGlobalSingleton } from "../../../shared/global-singleton.js";
 import { isSessionWriteLockAcquireError } from "../../session-write-lock-error.js";
 import type { acquireSessionWriteLock } from "../../session-write-lock.js";
 import { resolveEmbeddedSessionFileKey } from "../session-file-key.js";
+import { resolveEmbeddedAbortSettleTimeoutMs } from "./attempt.abort-settle-timeout.js";
 
 type SessionLock = Awaited<ReturnType<typeof acquireSessionWriteLock>>;
 type AcquireSessionWriteLock = typeof acquireSessionWriteLock;
@@ -586,6 +587,8 @@ export type EmbeddedAttemptSessionLockController = {
 export async function createEmbeddedAttemptSessionLockController(params: {
   acquireSessionWriteLock: AcquireSessionWriteLock;
   lockOptions: LockOptions;
+  /** Bounded wait before an abort release force-releases the held lock. */
+  abortReleaseTimeoutMs?: number;
 }): Promise<EmbeddedAttemptSessionLockController> {
   const acquireLock = async (): Promise<SessionLock> =>
     await params.acquireSessionWriteLock({
@@ -823,6 +826,97 @@ export async function createEmbeddedAttemptSessionLockController(params: {
     }
   }
 
+  /**
+   * Abort-path release. Races the graceful fence release against a bounded
+   * settle window. A retained-lock write pinned behind an unsettled provider
+   * call (a hung stream that ignores abort) keeps `waitForRetainedLockIdle`
+   * pending forever; without the bound, the lock is only reclaimed by the
+   * `maxHoldMs` watchdog and every turn in between fails with
+   * `SessionWriteLockTimeoutError` (tulgey#225, upstream #86816 family).
+   * On timeout the underlying file lock is force-released and the controller
+   * is poisoned (`takeoverDetected`) so the aborted run cannot perform torn
+   * writes after losing ownership.
+   */
+  async function releaseHeldLockForAbortBounded(): Promise<void> {
+    if (!heldLock) {
+      await waitForHeldLockDrain();
+      return;
+    }
+    const boundMs = params.abortReleaseTimeoutMs ?? resolveEmbeddedAbortSettleTimeoutMs();
+    const raceWithBound = async <T>(promise: Promise<T>): Promise<T | "abort-bound"> => {
+      let timer: NodeJS.Timeout | undefined;
+      try {
+        return await Promise.race([
+          promise,
+          new Promise<"abort-bound">((resolve) => {
+            timer = setTimeout(() => resolve("abort-bound"), boundMs);
+            timer.unref?.();
+          }),
+        ]);
+      } finally {
+        if (timer) {
+          clearTimeout(timer);
+        }
+      }
+    };
+    const forceRelease = async (): Promise<void> => {
+      if (!heldLock) {
+        return;
+      }
+      const lock = heldLock;
+      heldLock = undefined;
+      takeoverDetected = true;
+      await lock.release();
+    };
+
+    const drainAcquisition = beginHeldLockDrain();
+    const drainOwner = await raceWithBound(drainAcquisition);
+    if (drainOwner === "abort-bound") {
+      // Another release path owns the drain and is itself stuck. Take release
+      // ownership directly (clearing `heldLock` makes the stuck path no-op when
+      // it resumes) and hand the eventually-acquired drain straight back.
+      void drainAcquisition.then((owner) => finishHeldLockDrain(owner));
+      await forceRelease();
+      return;
+    }
+    try {
+      const idle = await raceWithBound(waitForRetainedLockIdle());
+      if (!heldLock) {
+        return;
+      }
+      if (idle === true) {
+        // Retained writes settled in time: keep the graceful fence semantics.
+        const lock = heldLock;
+        heldLock = undefined;
+        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();
+        }
+        return;
+      }
+      if (idle === false) {
+        // Abort issued from inside an active write context; mirror the
+        // graceful path and leave release to that write's unwind.
+        return;
+      }
+      // Bounded wait expired: a retained write is pinned behind an unsettled
+      // provider call. Force-release so the next turn can acquire the lock.
+      await forceRelease();
+    } finally {
+      finishHeldLockDrain(drainOwner);
+    }
+  }
+
   async function takeHeldLockAfterRetainedIdle(): Promise<SessionLock | undefined> {
     if (!heldLock) {
       return undefined;
@@ -902,7 +996,7 @@ export async function createEmbeddedAttemptSessionLockController(params: {
       await releaseHeldLockWithFence();
     },
     async releaseHeldLockForAbort(): Promise<void> {
-      await releaseHeldLockWithFence();
+      await releaseHeldLockForAbortBounded();
     },
     refreshAfterOwnedSessionWrite(): void {
       if (fenceActive && !takeoverDetected) {
