fix(agent): replace self-wait with deferred release in retained-lock abort cleanup (#96100)

* fix(agent): wait for retained session write before releasing held lock on abort

* fix(agent): replace self-wait with deferred release in retained-lock abort cleanup

* fix(test): reject fallback acquire with SessionWriteLockTimeoutError in active-scope cleanup test

* fix(agent): trim retained-lock comments

Signed-off-by: sallyom <somalley@redhat.com>

---------

Signed-off-by: sallyom <somalley@redhat.com>
Co-authored-by: sallyom <somalley@redhat.com>

src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts

@@ -3992,6 +3992,103 @@ describe("embedded attempt session lock lifecycle", () => {
     expect(acquireSessionWriteLockLocal).toHaveBeenCalledTimes(2);
   });
 
+  it("releaseHeldLockWithFence sets deferred flag when bailed out during active scope; re-attempted after scope deactivation (#95915)", async () => {
+    const events: string[] = [];
+    const releasePrep = vi.fn(async () => events.push("prep-release"));
+    const releaseRetained = vi.fn(async () => events.push("retained-release"));
+    const acquireSessionWriteLockLocal = vi
+      .fn()
+      .mockResolvedValueOnce({ release: releasePrep })
+      .mockResolvedValueOnce({ release: releaseRetained });
+
+    const controller = await createEmbeddedAttemptSessionLockController({
+      acquireSessionWriteLock: acquireSessionWriteLockLocal,
+      lockOptions,
+    });
+
+    await controller.releaseForPrompt();
+    await controller.reacquireAfterPrompt();
+
+    await controller.withSessionWriteLock(async () => {
+      events.push("write-start");
+      await controller.releaseHeldLockForAbort();
+      events.push("write-end");
+    });
+
+    expect(events).toEqual(["prep-release", "write-start", "write-end", "retained-release"]);
+    expect(acquireSessionWriteLockLocal).toHaveBeenCalledTimes(2);
+  });
+
+  it("controls the held lock lifecycle across deferred abort release, reacquisition, and prompt release", async () => {
+    const events: string[] = [];
+    const acquireSessionWriteLockLocal = vi
+      .fn()
+      .mockResolvedValueOnce({ release: vi.fn(async () => events.push("init-release")) })
+      .mockResolvedValueOnce({ release: vi.fn(async () => events.push("held-release")) })
+      .mockResolvedValueOnce({ release: vi.fn(async () => events.push("reacquire-release")) });
+
+    const controller = await createEmbeddedAttemptSessionLockController({
+      acquireSessionWriteLock: acquireSessionWriteLockLocal,
+      lockOptions,
+    });
+
+    await controller.releaseForPrompt();
+    await controller.reacquireAfterPrompt();
+
+    await controller.withSessionWriteLock(async () => {
+      events.push("write");
+      await controller.releaseHeldLockForAbort();
+    });
+
+    expect(events).toEqual(["init-release", "write", "held-release"]);
+
+    await controller.reacquireAfterPrompt();
+    await controller.releaseForPrompt();
+
+    expect(events).toEqual(["init-release", "write", "held-release", "reacquire-release"]);
+    expect(acquireSessionWriteLockLocal).toHaveBeenCalledTimes(3);
+  });
+
+  it("takeHeldLockAfterRetainedIdle does not self-deadlock when called from inside active write scope (#95915)", async () => {
+    const events: string[] = [];
+    const acquireSessionWriteLockLocal = vi
+      .fn()
+      .mockResolvedValueOnce({ release: vi.fn(async () => events.push("init-release")) })
+      .mockResolvedValueOnce({ release: vi.fn(async () => events.push("held-release")) })
+      .mockRejectedValueOnce(
+        new SessionWriteLockTimeoutError({
+          timeoutMs: lockOptions.timeoutMs,
+          owner: "pid=test",
+          lockPath: `${lockOptions.sessionFile}.lock`,
+        }),
+      );
+
+    const controller = await createEmbeddedAttemptSessionLockController({
+      acquireSessionWriteLock: acquireSessionWriteLockLocal,
+      lockOptions,
+    });
+
+    await controller.releaseForPrompt();
+    await controller.reacquireAfterPrompt();
+
+    const takeoverError = await controller
+      .withSessionWriteLock(async () => {
+        events.push("write-start");
+        const cleanupLock = await controller.acquireForCleanup();
+        await cleanupLock.release();
+        events.push("cleanup-inside-done");
+      })
+      .catch((error: unknown) => error);
+
+    expect(takeoverError).toBeInstanceOf(EmbeddedAttemptSessionTakeoverError);
+
+    const cleanupLock = await controller.acquireForCleanup();
+    await cleanupLock.release();
+
+    expect(events).toEqual(["init-release", "write-start", "cleanup-inside-done", "held-release"]);
+    expect(acquireSessionWriteLockLocal).toHaveBeenCalledTimes(3);
+  });
+
   it("returns a no-op cleanup lock after prompt lock reacquisition times out", async () => {
     const releases: string[] = [];
     const acquireSessionWriteLockResult = vi

src/agents/embedded-agent-runner/run/attempt.session-lock.ts

@@ -1171,6 +1171,9 @@ export async function createEmbeddedAttemptSessionLockController(params: {
   let fenceGeneration = 0;
   let fenceActive = false;
   let takeoverDetected = false;
+  // Set when an active retained write prevents immediate held-lock release.
+  // The scope completion path retries release after the retained use unwinds.
+  let releaseHeldLockDeferred = false;
   let retainedLockUseCount = 0;
   const retainedLockIdleWaiters = new Set<() => void>();
   let heldLockDraining = false;
@@ -1603,6 +1606,7 @@ export async function createEmbeddedAttemptSessionLockController(params: {
     const drainOwner = await beginHeldLockDrain();
     try {
       if (!(await waitForRetainedLockIdle())) {
+        releaseHeldLockDeferred = true;
         return;
       }
       if (!heldLock) {
@@ -1639,6 +1643,8 @@ export async function createEmbeddedAttemptSessionLockController(params: {
     const drainOwner = await beginHeldLockDrain();
     try {
       if (!(await waitForRetainedLockIdle())) {
+        // Do not wait for retained idle from inside the active scope; that
+        // scope must unwind before the retained-use waiter can resolve.
         return undefined;
       }
       if (!heldLock) {
@@ -1660,6 +1666,7 @@ export async function createEmbeddedAttemptSessionLockController(params: {
     const drainOwner = await beginHeldLockDrain();
     try {
       if (!(await waitForRetainedLockIdle())) {
+        // Same active-scope self-deadlock guard as takeHeldLockAfterRetainedIdle.
         return;
       }
       if (!heldLock) {
@@ -1721,6 +1728,12 @@ export async function createEmbeddedAttemptSessionLockController(params: {
       }
     }
     await releaseHeldLockAfterTakeover();
+    // Retained use has been released and the active scope is no longer live,
+    // so a prior active-scope release bailout can drain the held file lock now.
+    if (releaseHeldLockDeferred) {
+      releaseHeldLockDeferred = false;
+      await releaseHeldLockWithFence();
+    }
     if (!outcome.ok) {
       throw outcome.error;
     }