From 61d8c3fd91982dd593c142f248cfb2f003df9d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=86=AF=E5=9F=BA=E9=AD=81?= <56265583+fengjikui@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:09:29 +0800 Subject: [PATCH] fix(agents): keep missing error details out of auth health (#100617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): keep missing error details out of auth health Signed-off-by: 冯基魁 <1412414664@qq.com> * test(agents): prove no-details auth health fallback * fix(agents): preserve OpenAI no-details auth handling * fix(agents): gate detail-less auth cooldowns Co-authored-by: 冯基魁 <1412414664@qq.com> * fix(agents): validate WHAM credential snapshot --------- Signed-off-by: 冯基魁 <1412414664@qq.com> Co-authored-by: Peter Steinberger --- CHANGELOG.md | 1 + src/agents/auth-profiles/usage.test.ts | 72 ++++++++++++++- src/agents/auth-profiles/usage.ts | 84 +++++++++++++----- .../model-fallback.run-embedded.e2e.test.ts | 88 +++++++++++++++---- .../embedded-agent-runner-e2e-mocks.ts | 3 + 5 files changed, 204 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 362fb3ddd6d0..f2cd39482e4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai - **Control UI agent model labels:** show each selected agent's effective model in the Default picker option instead of the global model. (#100719, #77690, #77440) Thanks @hyspacex. - **Control UI inbound image previews:** render canonical inbound media references through the authenticated ticket route after chat-history reloads. (#100725, #90172, #89591) Thanks @sweetcornna. - **Small-context compaction:** cap the effective reserve against the known model context window so small local models do not enter compaction from the first token. (#100621) Thanks @vincentkoc. +- **Detail-less provider failures:** keep opaque upstream failures from cooling API-key auth profiles while preserving WHAM-backed OpenAI OAuth health checks and configured model fallback. (#100600, #100617) Thanks @fengjikui. - **Plugin install diagnostics:** suppress the misleading hook-pack fallback after plugin install failures only when the hook manifest is absent, while preserving actionable malformed hook-pack errors. (#100554) Thanks @vincentkoc. - **Config validation diagnostics:** emit each unchanged sanitized validation-warning payload once per config path, reset deduplication after a clean validation, and preserve the warning fingerprint across transient invalid reads and failed refreshes. (#100569, #25574) Thanks @vincentkoc. - **Config size-drop guard:** compare writes against canonical bytes for parseable object configs instead of raw BOM and indentation overhead, while preserving raw audit telemetry and the conservative malformed-input fallback. (#100591, #71865) Thanks @vincentkoc. diff --git a/src/agents/auth-profiles/usage.test.ts b/src/agents/auth-profiles/usage.test.ts index b71196d3f5ca..4abe7358058c 100644 --- a/src/agents/auth-profiles/usage.test.ts +++ b/src/agents/auth-profiles/usage.test.ts @@ -952,6 +952,30 @@ describe("markAuthProfileBlockedUntil", () => { }); }); +describe("markAuthProfileFailure — detail-less provider failures", () => { + it("does not persist unverifiable failures for API-key profiles", async () => { + const store = makeStore(undefined); + store.profiles["azure-foundry:default"] = { + type: "api_key", + provider: "azure-foundry", + key: "azure-foundry-test-key", + }; + + for (const profileId of ["azure-foundry:default", "openai:api-key"]) { + await markAuthProfileFailure({ + store, + profileId, + reason: "no_error_details", + }); + } + + expect(store.usageStats).toBeUndefined(); + expect(fetchMock).not.toHaveBeenCalled(); + expect(storeMocks.updateAuthProfileStoreWithLock).not.toHaveBeenCalled(); + expect(storeMocks.saveAuthProfileStore).not.toHaveBeenCalled(); + }); +}); + describe("markAuthProfileFailure — WHAM-aware Codex cooldowns", () => { function mockWhamResponse(status: number, body?: unknown): void { fetchMock.mockResolvedValueOnce( @@ -965,7 +989,7 @@ describe("markAuthProfileFailure — WHAM-aware Codex cooldowns", () => { async function markCodexFailureAt(params: { store: ReturnType; now: number; - reason?: "rate_limit" | "unknown"; + reason?: "rate_limit" | "no_error_details" | "unknown"; useLock?: boolean; }): Promise { const dateNowSpy = vi.spyOn(Date, "now").mockReturnValue(params.now); @@ -1061,6 +1085,52 @@ describe("markAuthProfileFailure — WHAM-aware Codex cooldowns", () => { } }); + it("probes WHAM before recording an OpenAI OAuth detail-less failure", async () => { + const now = 1_700_000_000_000; + const store = makeStore(undefined); + mockWhamResponse(200, { + rate_limit: { + limit_reached: false, + primary_window: { used_percent: 45, reset_after_seconds: 9_000 }, + }, + }); + + await markCodexFailureAt({ store, now, reason: "no_error_details" }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(store.usageStats?.["openai:default"]?.cooldownUntil).toBe(now + 15_000); + expect(store.usageStats?.["openai:default"]?.failureCounts?.no_error_details).toBe(1); + }); + + it("does not apply a stale WHAM result after the profile changes", async () => { + const now = 1_700_000_000_000; + const store = makeStore(undefined); + mockWhamResponse(200, { + rate_limit: { + limit_reached: false, + primary_window: { used_percent: 45, reset_after_seconds: 9_000 }, + }, + }); + storeMocks.updateAuthProfileStoreWithLock.mockImplementationOnce( + async (lockParams: { updater: (store: AuthProfileStore) => boolean }) => { + const freshStore = structuredClone(store); + freshStore.profiles["openai:default"] = { + type: "api_key", + provider: "openai", + key: "rotated-api-key", + }; + lockParams.updater(freshStore); + return freshStore; + }, + ); + + await markCodexFailureAt({ store, now, reason: "no_error_details" }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(store.usageStats).toBeUndefined(); + expect(storeMocks.saveAuthProfileStore).not.toHaveBeenCalled(); + }); + it("maps HTTP 401 to a 12h cooldown", async () => { const now = 1_700_000_000_000; const store = makeStore({}); diff --git a/src/agents/auth-profiles/usage.ts b/src/agents/auth-profiles/usage.ts index 02a1e6e3a9c0..bc510e5f835c 100644 --- a/src/agents/auth-profiles/usage.ts +++ b/src/agents/auth-profiles/usage.ts @@ -13,8 +13,8 @@ import { } from "@openclaw/normalization-core/number-coercion"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; -import { resolveProviderRequestHeaders } from "../provider-request-config.js"; import { readProviderJsonResponse } from "../provider-http-errors.js"; +import { resolveProviderRequestHeaders } from "../provider-request-config.js"; import { notifyAuthProfileFailureHook, setAuthProfileFailureHook } from "./failure-hook.js"; import { logAuthProfileFailureStateChange } from "./state-observation.js"; @@ -22,6 +22,7 @@ const authProfileUsageLog = createSubsystemLogger("agent/embedded"); import { saveAuthProfileStore, updateAuthProfileStoreWithLock } from "./store.js"; import type { AuthProfileBlockedSource, + AuthProfileCredential, AuthProfileFailureReason, AuthProfileStore, ProfileUsageStats, @@ -111,11 +112,13 @@ type WhamCooldownProbeResult = { }; function shouldProbeWhamForFailure( - provider: string | undefined, + profile: AuthProfileCredential | undefined, reason: AuthProfileFailureReason, ): boolean { - const normalizedProvider = normalizeProviderId(provider ?? ""); + const normalizedProvider = normalizeProviderId(profile?.provider ?? ""); return ( + profile?.type === "oauth" && + Boolean(profile.access) && normalizedProvider === "openai" && (reason === "rate_limit" || reason === "empty_response" || @@ -125,6 +128,19 @@ function shouldProbeWhamForFailure( ); } +function isSameWhamCredential( + expected: AuthProfileCredential, + current: AuthProfileCredential | undefined, +): boolean { + return ( + expected.type === "oauth" && + current?.type === "oauth" && + normalizeProviderId(expected.provider) === normalizeProviderId(current.provider) && + expected.access === current.access && + expected.accountId === current.accountId + ); +} + function resolveActiveWindowUntil(value: unknown, now: number): number { const timestampMs = asDateTimestampMs(value); return timestampMs !== undefined && timestampMs > now ? timestampMs : 0; @@ -716,9 +732,14 @@ export async function markAuthProfileFailure(params: { return; } - const whamResult = shouldProbeWhamForFailure(profile.provider, reason) - ? await probeWhamForCooldown(store, profileId) - : null; + const shouldProbeWham = shouldProbeWhamForFailure(profile, reason); + // A detail-less provider failure carries no credential-health evidence. + // Only OpenAI OAuth can disambiguate it with the canonical WHAM probe. + if (reason === "no_error_details" && !shouldProbeWham) { + return; + } + + const whamResult = shouldProbeWham ? await probeWhamForCooldown(store, profileId) : null; let nextStats: ProfileUsageStats | undefined; let previousStats: ProfileUsageStats | undefined; @@ -730,6 +751,17 @@ export async function markAuthProfileFailure(params: { if (!profileValue || isAuthCooldownBypassedForProvider(profileValue.provider)) { return false; } + const currentWhamResult = + whamResult && + shouldProbeWhamForFailure(profileValue, reason) && + isSameWhamCredential(profile, profileValue) + ? whamResult + : null; + // The WHAM response belongs to the credential snapshot used for the + // probe. A concurrent profile replacement must not inherit its result. + if (reason === "no_error_details" && !currentWhamResult) { + return false; + } const now = Date.now(); const providerKey = normalizeProviderId(profileValue.provider); const cfgResolved = resolveAuthCooldownConfig({ @@ -746,15 +778,14 @@ export async function markAuthProfileFailure(params: { cfgResolved, modelId, }); - nextStats = - whamResult && shouldProbeWhamForFailure(profileValue.provider, reason) - ? applyWhamCooldownResult({ - existing: previousStats ?? {}, - computed, - now, - whamResult, - }) - : computed; + nextStats = currentWhamResult + ? applyWhamCooldownResult({ + existing: previousStats ?? {}, + computed, + now, + whamResult: currentWhamResult, + }) + : computed; updateUsageStatsEntry(freshStore, profileId, () => nextStats ?? computed); return true; }, @@ -788,6 +819,12 @@ export async function markAuthProfileFailure(params: { return; } + const currentWhamResult = + whamResult && isSameWhamCredential(profile, store.profiles[profileId]) ? whamResult : null; + if (reason === "no_error_details" && !currentWhamResult) { + return; + } + const now = Date.now(); const providerKey = normalizeProviderId(store.profiles[profileId]?.provider ?? ""); const cfgResolved = resolveAuthCooldownConfig({ @@ -803,15 +840,14 @@ export async function markAuthProfileFailure(params: { cfgResolved, modelId, }); - nextStats = - whamResult && shouldProbeWhamForFailure(store.profiles[profileId]?.provider, reason) - ? applyWhamCooldownResult({ - existing: previousStats ?? {}, - computed, - now, - whamResult, - }) - : computed; + nextStats = currentWhamResult + ? applyWhamCooldownResult({ + existing: previousStats ?? {}, + computed, + now, + whamResult: currentWhamResult, + }) + : computed; updateUsageStatsEntry(store, profileId, () => nextStats ?? computed); authProfileUsageDeps.saveAuthProfileStore(store, agentDir); logAuthProfileFailureStateChange({ diff --git a/src/agents/model-fallback.run-embedded.e2e.test.ts b/src/agents/model-fallback.run-embedded.e2e.test.ts index 88955786b6e5..9770b775207d 100644 --- a/src/agents/model-fallback.run-embedded.e2e.test.ts +++ b/src/agents/model-fallback.run-embedded.e2e.test.ts @@ -83,6 +83,7 @@ const OVERLOADED_ERROR_PAYLOAD = '{"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}}'; const RATE_LIMIT_ERROR_MESSAGE = "rate limit exceeded"; const NO_ENDPOINTS_FOUND_ERROR_MESSAGE = "404 No endpoints found for deepseek/deepseek-r1:free."; +const NO_ERROR_DETAILS_MESSAGE = "Unknown error (no error details in response)"; type EmbeddedAttemptParams = { provider: string; @@ -90,23 +91,23 @@ type EmbeddedAttemptParams = { authProfileId?: string; }; -function makeConfig(): OpenClawConfig { +function makeConfig(primaryProvider = "openai"): OpenClawConfig { const apiKeyField = ["api", "Key"].join(""); return { agents: { defaults: { model: { - primary: "openai/mock-1", + primary: `${primaryProvider}/mock-1`, fallbacks: ["groq/mock-2"], }, }, }, models: { providers: { - openai: { + [primaryProvider]: { api: "openai-responses", - [apiKeyField]: "openai-test-key", // pragma: allowlist secret - baseUrl: "https://example.com/openai", + [apiKeyField]: `${primaryProvider}-test-key`, // pragma: allowlist secret + baseUrl: `https://example.com/${primaryProvider}`, models: [ { id: "mock-1", @@ -169,18 +170,25 @@ async function writeAuthStore( failureCounts?: Partial>; } >, + options?: { primaryProvider?: string }, ) { + const primaryProvider = options?.primaryProvider ?? "openai"; + const primaryProfileId = `${primaryProvider}:p1`; saveAuthProfileStore( { version: 1, profiles: { - "openai:p1": { type: "api_key", provider: "openai", key: "sk-openai" }, + [primaryProfileId]: { + type: "api_key", + provider: primaryProvider, + key: "sk-primary", + }, "groq:p1": { type: "api_key", provider: "groq", key: "sk-groq" }, }, usageStats: usageStats ?? ({ - "openai:p1": { lastUsed: 1 }, + [primaryProfileId]: { lastUsed: 1 }, "groq:p1": { lastUsed: 2 }, } as const), }, @@ -228,6 +236,7 @@ async function runEmbeddedFallback(params: { workspaceDir: string; sessionKey: string; runId: string; + provider?: string; sessionId?: string; lane?: string; abortSignal?: AbortSignal; @@ -239,7 +248,7 @@ async function runEmbeddedFallback(params: { const sessionId = params.sessionId ?? `session:${params.runId}`; return await runWithModelFallback({ cfg, - provider: "openai", + provider: params.provider ?? "openai", model: "mock-1", runId: params.runId, sessionId: params.sessionId, @@ -289,10 +298,12 @@ function mockPrimaryFailureThenFallbackSuccess( makePrimaryAttempt: ( attemptParams: EmbeddedAttemptParams, ) => EmbeddedRunAttemptResult | Promise, + options?: { primaryProvider?: string }, ) { + const primaryProvider = options?.primaryProvider ?? "openai"; runEmbeddedAttemptMock.mockImplementation(async (params: unknown) => { const attemptParams = params as EmbeddedAttemptParams; - if (attemptParams.provider === "openai") { + if (attemptParams.provider === primaryProvider) { return await makePrimaryAttempt(attemptParams); } if (attemptParams.provider === "groq") { @@ -324,17 +335,22 @@ function mockPrimarySuspendingPromptErrorThenFallbackSuccess(sessionId: string) ); } -function mockPrimaryErrorThenFallbackSuccess(errorMessage: string) { - mockPrimaryFailureThenFallbackSuccess(() => - makeEmbeddedRunnerAttempt({ - assistantTexts: [], - lastAssistant: buildEmbeddedRunnerAssistant({ - provider: "openai", - model: "mock-1", - stopReason: "error", - errorMessage, +function mockPrimaryErrorThenFallbackSuccess( + errorMessage: string, + options?: { primaryProvider?: string }, +) { + mockPrimaryFailureThenFallbackSuccess( + (attemptParams) => + makeEmbeddedRunnerAttempt({ + assistantTexts: [], + lastAssistant: buildEmbeddedRunnerAssistant({ + provider: attemptParams.provider, + model: attemptParams.modelId ?? "mock-1", + stopReason: "error", + errorMessage, + }), }), - }), + options, ); } @@ -484,6 +500,40 @@ describe("runWithModelFallback + runEmbeddedAgent failover behavior", () => { }); }); + it("falls back after Azure Foundry omits error details without cooling down the profile", async () => { + await withAgentWorkspace(async ({ agentDir, workspaceDir }) => { + await writeAuthStore(agentDir, undefined, { primaryProvider: "azure-foundry" }); + mockPrimaryErrorThenFallbackSuccess(NO_ERROR_DETAILS_MESSAGE, { + primaryProvider: "azure-foundry", + }); + + const result = await runEmbeddedFallback({ + agentDir, + workspaceDir, + sessionKey: "agent:test:no-error-details-no-cooldown", + runId: "run:no-error-details-no-cooldown", + config: makeConfig("azure-foundry"), + provider: "azure-foundry", + }); + + expect(result.provider).toBe("groq"); + expect(result.model).toBe("mock-2"); + expect(result.attempts[0]?.reason).toBe("no_error_details"); + expect(result.result.payloads?.[0]?.text ?? "").toContain("fallback ok"); + + const usageStats = await readUsageStats(agentDir); + expect(usageStats["azure-foundry:p1"]?.cooldownUntil).toBeUndefined(); + expect(usageStats["azure-foundry:p1"]?.failureCounts?.no_error_details).toBeUndefined(); + expect(typeof usageStats["groq:p1"]?.lastUsed).toBe("number"); + + expect(countProviderAttempts("azure-foundry")).toBeGreaterThan(0); + expect(countProviderAttempts("openai")).toBe(0); + expect(countProviderAttempts("groq")).toBe(1); + expect(computeBackoffMock).not.toHaveBeenCalled(); + expect(sleepWithAbortMock).not.toHaveBeenCalled(); + }); + }); + it("falls back across providers after overloaded primary failure and persists transient cooldown", async () => { await withAgentWorkspace(async ({ agentDir, workspaceDir }) => { await writeAuthStore(agentDir); diff --git a/src/agents/test-helpers/embedded-agent-runner-e2e-mocks.ts b/src/agents/test-helpers/embedded-agent-runner-e2e-mocks.ts index c629112bbfbe..a7c28f241eca 100644 --- a/src/agents/test-helpers/embedded-agent-runner-e2e-mocks.ts +++ b/src/agents/test-helpers/embedded-agent-runner-e2e-mocks.ts @@ -61,6 +61,9 @@ export function installEmbeddedRunnerFastRunE2eMocks( options: EmbeddedRunnerFastRunMockOptions, ): void { vi.doMock("../harness/selection.js", () => ({ + agentHarnessBuildsOpenClawTools: vi.fn( + (harnessId: string) => harnessId === "codex" || harnessId === "copilot", + ), selectAgentHarness: vi.fn( (params: { provider?: string;