fix(agents): skip fallback for session coordination errors

luyao618 · web-flow · commit 6a5a1353c7f0 · 2026-05-18T14:30:58.000+01:00
Preserve provider fallback metadata when session coordination errors are nested under provider failures.

Co-authored-by: luyao618 &lt;364939526@qq.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,7 @@ Docs: https://docs.openclaw.ai
 - Gateway: defer update-check startup until after readiness so package update checks no longer block sidecar-ready startup, while preserving update broadcasts and shutdown cleanup. (#83520) Thanks @samzong.
 - Telegram: keep `/btw` and read-only status commands from aborting active runs, and avoid retaining raw update payloads in timed-out spool tombstones. Refs #83272.
 - Agents: log strict-agentic execution contract diagnostics only when the planning-only retry path actually triggers.
+- Agents: stop embedded session takeover and session write-lock errors from consuming model fallbacks while preserving provider fallback metadata. Fixes #83510. Thanks @luyao618.
 - Agents/video: hide `video_generate` reference-audio parameters unless a registered video provider supports audio inputs.
 - Plugins: fall back to npm for official ClawHub updates when artifact downloads are unavailable, including beta-to-default fallback and dry-run version reporting.
 - Plugins/xAI: echo PKCE challenge fields during OAuth authorization-code token exchange for xAI token-endpoint compatibility. (#83499) Thanks @fuller-stack-dev.
diff --git a/src/agents/failover-error.test.ts b/src/agents/failover-error.test.ts
@@ -3,6 +3,7 @@ import {
   coerceToFailoverError,
   describeFailoverError,
   FailoverError,
+  isNonProviderRuntimeCoordinationError,
   isTimeoutError,
   resolveFailoverReasonFromError,
   resolveFailoverStatus,
@@ -1112,4 +1113,54 @@ describe("failover-error", () => {
     expect(err?.lane).toBe("draft");
     expect(err?.provider).toBe("openai");
   });
+
+  describe("isNonProviderRuntimeCoordinationError", () => {
+    const makeSessionLockError = () =>
+      new SessionWriteLockTimeoutError({
+        timeoutMs: 10_000,
+        owner: "pid=37121",
+        lockPath: "/tmp/openclaw/session.jsonl.lock",
+      });
+    const makeEmbeddedTakeoverError = () => {
+      const err = new Error(
+        "session file changed while embedded prompt lock was released: /tmp/openclaw/session.jsonl",
+      );
+      err.name = "EmbeddedAttemptSessionTakeoverError";
+      return err;
+    };
+
+    it("returns true for direct session write-lock timeout errors", () => {
+      expect(isNonProviderRuntimeCoordinationError(makeSessionLockError())).toBe(true);
+    });
+
+    it("returns true for direct embedded attempt session takeover errors", () => {
+      expect(isNonProviderRuntimeCoordinationError(makeEmbeddedTakeoverError())).toBe(true);
+    });
+
+    it("returns true when the coordination error is nested via cause", () => {
+      const wrapped = new Error("wrapper", { cause: makeSessionLockError() });
+      expect(isNonProviderRuntimeCoordinationError(wrapped)).toBe(true);
+
+      const wrappedTakeover = new Error("wrapper", { cause: makeEmbeddedTakeoverError() });
+      expect(isNonProviderRuntimeCoordinationError(wrappedTakeover)).toBe(true);
+    });
+
+    it("returns false for plain timeouts and provider errors", () => {
+      const timeoutErr = Object.assign(new Error("operation timed out"), { name: "TimeoutError" });
+      expect(isNonProviderRuntimeCoordinationError(timeoutErr)).toBe(false);
+      expect(isNonProviderRuntimeCoordinationError({ status: 429, message: "rate limit" })).toBe(
+        false,
+      );
+      expect(
+        isNonProviderRuntimeCoordinationError({
+          status: 429,
+          code: "RESOURCE_EXHAUSTED",
+          message: "upstream quota pressure",
+          cause: makeSessionLockError(),
+        }),
+      ).toBe(false);
+      expect(isNonProviderRuntimeCoordinationError(null)).toBe(false);
+      expect(isNonProviderRuntimeCoordinationError(undefined)).toBe(false);
+    });
+  });
 });
diff --git a/src/agents/failover-error.ts b/src/agents/failover-error.ts
@@ -234,6 +234,49 @@ function hasSessionWriteLockTimeout(err: unknown, seen: Set<object> = new Set())
   );
 }
 
+function isEmbeddedAttemptSessionTakeover(err: unknown): boolean {
+  // Match by name to avoid importing pi-embedded-runner here (would create a cycle).
+  return Boolean(
+    err && typeof err === "object" && readErrorName(err) === "EmbeddedAttemptSessionTakeoverError",
+  );
+}
+
+function hasEmbeddedAttemptSessionTakeover(err: unknown, seen: Set<object> = new Set()): boolean {
+  if (isEmbeddedAttemptSessionTakeover(err)) {
+    return true;
+  }
+  if (!err || typeof err !== "object") {
+    return false;
+  }
+  if (seen.has(err)) {
+    return false;
+  }
+  seen.add(err);
+  const candidate = err as { error?: unknown; cause?: unknown; reason?: unknown };
+  return (
+    hasEmbeddedAttemptSessionTakeover(candidate.error, seen) ||
+    hasEmbeddedAttemptSessionTakeover(candidate.cause, seen) ||
+    hasEmbeddedAttemptSessionTakeover(candidate.reason, seen)
+  );
+}
+
+/**
+ * True when the error is a local runtime coordination error (session write-lock
+ * timeout or embedded attempt session takeover) rather than a provider/model
+ * failure. The model fallback chain must abort on these instead of consuming
+ * candidate slots — retrying any model would hit the same local condition.
+ * See #83510.
+ */
+export function isNonProviderRuntimeCoordinationError(err: unknown): boolean {
+  if (!hasSessionWriteLockTimeout(err) && !hasEmbeddedAttemptSessionTakeover(err)) {
+    return false;
+  }
+  if (isFailoverError(err)) {
+    return false;
+  }
+  return resolveFailoverClassificationFromError(err) === null;
+}
+
 function hasTimeoutHint(err: unknown): boolean {
   if (!err) {
     return false;
diff --git a/src/agents/model-fallback.test.ts b/src/agents/model-fallback.test.ts
@@ -23,6 +23,7 @@ import {
 } from "./model-fallback.js";
 import { classifyEmbeddedPiRunResultForModelFallback } from "./pi-embedded-runner/result-fallback-classifier.js";
 import type { EmbeddedPiRunResult } from "./pi-embedded-runner/types.js";
+import { SessionWriteLockTimeoutError } from "./session-write-lock-error.js";
 import { makeModelFallbackCfg } from "./test-helpers/model-fallback-config-fixture.js";
 
 vi.mock("../infra/file-lock.js", () => ({
@@ -768,6 +769,106 @@ describe("runWithModelFallback", () => {
     expect(run).toHaveBeenCalledTimes(1);
   });
 
+  it("aborts the fallback chain on embedded session takeover instead of trying every model (#83510)", async () => {
+    const cfg = makeCfg({
+      agents: {
+        defaults: {
+          model: {
+            primary: "openai/gpt-5.4",
+            fallbacks: ["anthropic/claude-sonnet-4-6", "openai/gpt-4.1-mini"],
+          },
+        },
+      },
+    });
+    const takeoverError = new Error(
+      "session file changed while embedded prompt lock was released: /tmp/session.jsonl",
+    );
+    takeoverError.name = "EmbeddedAttemptSessionTakeoverError";
+    const run = vi.fn().mockRejectedValue(takeoverError);
+
+    await expect(
+      runWithModelFallback({
+        cfg,
+        provider: "openai",
+        model: "gpt-5.4",
+        run,
+      }),
+    ).rejects.toBe(takeoverError);
+    expect(run).toHaveBeenCalledTimes(1);
+  });
+
+  it("aborts the fallback chain on session write-lock timeout instead of trying every model (#83510)", async () => {
+    const cfg = makeCfg({
+      agents: {
+        defaults: {
+          model: {
+            primary: "openai/gpt-5.4",
+            fallbacks: ["anthropic/claude-sonnet-4-6", "openai/gpt-4.1-mini"],
+          },
+        },
+      },
+    });
+    const lockError = new SessionWriteLockTimeoutError({
+      timeoutMs: 10_000,
+      owner: "pid=37121",
+      lockPath: "/tmp/openclaw/session.jsonl.lock",
+    });
+    const run = vi.fn().mockRejectedValue(lockError);
+
+    await expect(
+      runWithModelFallback({
+        cfg,
+        provider: "openai",
+        model: "gpt-5.4",
+        run,
+      }),
+    ).rejects.toBe(lockError);
+    expect(run).toHaveBeenCalledTimes(1);
+  });
+
+  it("keeps provider failover metadata authoritative over nested session locks", async () => {
+    const cfg = makeCfg({
+      agents: {
+        defaults: {
+          model: {
+            primary: "openai/gpt-5.4",
+            fallbacks: ["anthropic/claude-sonnet-4-6"],
+          },
+        },
+      },
+    });
+    const lockError = new SessionWriteLockTimeoutError({
+      timeoutMs: 10_000,
+      owner: "pid=37121",
+      lockPath: "/tmp/openclaw/session.jsonl.lock",
+    });
+    const providerError = {
+      status: 429,
+      code: "RESOURCE_EXHAUSTED",
+      message: "upstream quota pressure",
+      cause: lockError,
+    };
+    const run = vi.fn().mockRejectedValueOnce(providerError).mockResolvedValueOnce("fallback ok");
+
+    const result = await runWithModelFallback({
+      cfg,
+      provider: "openai",
+      model: "gpt-5.4",
+      run,
+    });
+
+    expect(result.result).toBe("fallback ok");
+    expect(result.provider).toBe("anthropic");
+    expect(run).toHaveBeenCalledTimes(2);
+    expect(result.attempts[0]).toMatchObject({
+      provider: "openai",
+      model: "gpt-5.4",
+      reason: "rate_limit",
+      status: 429,
+      code: "RESOURCE_EXHAUSTED",
+    });
+  });
+
   it("keeps raw provider schema errors in fallback summaries", async () => {
     const cfg = makeCfg({
       agents: {
diff --git a/src/agents/model-fallback.ts b/src/agents/model-fallback.ts
@@ -19,6 +19,7 @@ import {
   coerceToFailoverError,
   describeFailoverError,
   isFailoverError,
+  isNonProviderRuntimeCoordinationError,
   isTimeoutError,
 } from "./failover-error.js";
 import {
@@ -1197,6 +1198,14 @@ export async function runWithModelFallback<T>(
     }
     const err = attemptRun.error;
     {
+      // Local runtime coordination errors (session write-lock timeout, embedded
+      // attempt session takeover) are not provider/model failures. Aborting
+      // here prevents the fallback chain from consuming candidates retrying
+      // the same local condition and surfacing a misleading "All models
+      // failed" summary. See #83510.
+      if (isNonProviderRuntimeCoordinationError(err)) {
+        throw err;
+      }
       if (transientProbeProviderForAttempt) {
         const probeFailureReason = describeFailoverError(err).reason;
         if (!shouldPreserveTransientCooldownProbeSlot(probeFailureReason)) {
