Skip to content

Commit f79e5ad

Browse files
Jerry-XinJerry-Xin
committed
fix: restore configured retries outside session lock
1 parent 1622005 commit f79e5ad

7 files changed

Lines changed: 181 additions & 12 deletions

File tree

‎src/agents/agent-project-settings.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ function createRuntimeEmbeddedAgentSettingsManager(
4848
);
4949
}
5050

51+
/**
52+
* Resolves the configured provider retry budget (settings.retry.provider.maxRetries)
53+
* from the same project/plugin settings the embedded runner reads. The reply
54+
* orchestrator wires this once onto the prepared run so the outer whole-attempt
55+
* retry owner can restore the budget the in-window SDK pin drops (#87180); unset
56+
* leaves the shipped single outer retry.
57+
*/
58+
export function resolveConfiguredProviderRetryMaxRetries(params: {
59+
cwd: string;
60+
agentDir: string;
61+
cfg?: OpenClawConfig;
62+
pluginMetadataSnapshot?: PluginMetadataSnapshot;
63+
}): number | undefined {
64+
return createEmbeddedAgentSettingsManager(params).getProviderRetrySettings().maxRetries;
65+
}
66+
5167
/** Creates the runtime SettingsManager with project/plugin settings and compaction overrides. */
5268
export function createPreparedEmbeddedAgentSettingsManager(params: {
5369
cwd: string;

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3860,15 +3860,36 @@ describe("embedded attempt session lock lifecycle", () => {
38603860
expect(streamFn).toHaveBeenCalledWith("model", "context", { maxRetries: 0 });
38613861
});
38623862

3863-
it("preserves an explicit maxRetries over the embedded prompt default", async () => {
3863+
it("does not let the configured provider retry leak into the released-lock window (#87180)", async () => {
3864+
// Model sdk.ts resolution: optionsLocal?.maxRetries ?? providerRetrySettings.maxRetries.
3865+
// The pin injects an explicit maxRetries:0, so the configured provider budget
3866+
// (3 here) can never apply while the prompt lock is released.
3867+
const configuredMaxRetries = 3;
3868+
const received: Array<number | undefined> = [];
3869+
const streamFn = vi.fn(
3870+
async (_model: unknown, _context: unknown, options?: { maxRetries?: number }) => {
3871+
received.push(options?.maxRetries ?? configuredMaxRetries);
3872+
},
3873+
);
3874+
const session = { agent: { streamFn } };
3875+
3876+
installEmbeddedPromptRetryDefault(session);
3877+
await session.agent.streamFn("model", "context");
3878+
3879+
expect(received).toEqual([0]);
3880+
});
3881+
3882+
it("forces maxRetries:0 even when the caller sets an explicit maxRetries in-window (#87180 hardening)", async () => {
38643883
const streamFn = vi.fn(async (..._args: unknown[]) => {});
38653884
const session = { agent: { streamFn } };
38663885

38673886
installEmbeddedPromptRetryDefault(session);
38683887
await session.agent.streamFn("model", "context", { maxRetries: 3, temperature: 0.2 });
38693888

3889+
// The released-lock window pins retries to 0; an explicit per-call maxRetries
3890+
// cannot widen it. Other request options are preserved.
38703891
expect(streamFn).toHaveBeenCalledWith("model", "context", {
3871-
maxRetries: 3,
3892+
maxRetries: 0,
38723893
temperature: 0.2,
38733894
});
38743895
});
@@ -3910,8 +3931,10 @@ describe("embedded attempt session lock lifecycle", () => {
39103931
// Inner provider streamFn runs once per turn, not once per stacked wrapper.
39113932
expect(streamFn).toHaveBeenCalledTimes(2);
39123933
expect(streamFn).toHaveBeenNthCalledWith(1, "model", "context", { maxRetries: 0 });
3934+
// The explicit per-call maxRetries is forced to 0 in the released-lock window;
3935+
// other options survive.
39133936
expect(streamFn).toHaveBeenNthCalledWith(2, "model", "context", {
3914-
maxRetries: 3,
3937+
maxRetries: 0,
39153938
temperature: 0.2,
39163939
});
39173940

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,11 +2240,12 @@ export function installPromptSubmissionLockRelease(params: {
22402240
/* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */
22412241

22422242
// Pin SDK retries to 0 for calls made inside the embedded prompt lock window.
2243-
// The in-window default is always 0, never the configured provider retry: the
2244-
// lock is released across the model call, model-fallback owns the one whole-attempt
2245-
// retry, and an in-window SDK retry would race session takeover and silently lose
2246-
// the message (#87180). Only an explicit per-call options.maxRetries overrides it,
2247-
// because the caller-provided options spread last over the injected default.
2243+
// The in-window value is always 0, never the configured provider retry and never
2244+
// a caller-provided maxRetries: the lock is released across the model call, the
2245+
// outer reply orchestrator owns the one whole-attempt retry (which reacquires the
2246+
// lock), and an in-window SDK retry would race session takeover and silently lose
2247+
// the message (#87180). maxRetries:0 is applied last so nothing — config default
2248+
// or explicit per-call override — can widen the retry count in this window.
22482249
export function installEmbeddedPromptRetryDefault(session: unknown): void {
22492250
const agent = (session as SessionWithAgentPrompt).agent;
22502251
if (typeof agent?.streamFn !== "function") {
@@ -2260,9 +2261,11 @@ export function installEmbeddedPromptRetryDefault(session: unknown): void {
22602261
const wrappedStreamFn: PromptReleaseStreamFn = (...args: unknown[]) => {
22612262
const [model, context, callOptions] = args;
22622263
const requestOptions = callOptions as { maxRetries?: number } | undefined;
2263-
// Inject maxRetries:0 as the in-window default; caller-provided options spread
2264-
// last so an explicit maxRetries still wins.
2265-
return innerStreamFn(model, context, { maxRetries: 0, ...requestOptions });
2264+
// Force maxRetries:0 last so it wins over both the configured provider default
2265+
// (resolved downstream in sdk.ts) and any explicit per-call maxRetries. The
2266+
// released-lock window must never retry in-window (#87180); the outer owner
2267+
// restores the configured retry budget where each retry reacquires the lock.
2268+
return innerStreamFn(model, context, { ...requestOptions, maxRetries: 0 });
22662269
};
22672270
wrappedStreamFn["__openclawEmbeddedPromptRetryDefaultInstalled"] = true;
22682271
// The outermost wrapper hides inner markers, so carry the lock-release marker

‎src/auto-reply/reply/agent-runner-execution-transient-retry.test.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,108 @@ describe("runAgentTurnWithFallback: transient connection/timeout retry (#87180)"
277277
// No transient retry through the timeout disjunct: a single cycle only.
278278
expect(state.runWithModelFallbackMock).toHaveBeenCalledTimes(1);
279279
});
280+
281+
it("restores the configured provider retry budget as the whole-attempt retry count (#87180)", async () => {
282+
// settings.retry.provider.maxRetries is dropped in-window by the SDK pin, so
283+
// the outer owner restores it: with a resolved budget of 3, a persistently
284+
// transient error runs 1 initial cycle + 3 retries (4 cycles). Each retry
285+
// re-runs the whole fallback chain, so the session lock is reacquired per
286+
// attempt, then a terminal failure is surfaced once the budget is exhausted.
287+
state.runWithModelFallbackMock.mockRejectedValue(new Error("Connection error."));
288+
289+
const runAgentTurnWithFallback = await getRunAgentTurnWithFallback();
290+
const followupRun = createFollowupRun();
291+
followupRun.run.providerRetryMaxRetries = 3;
292+
vi.useFakeTimers();
293+
try {
294+
const promise = runAgentTurnWithFallback({
295+
commandBody: "hello",
296+
followupRun,
297+
sessionCtx: {
298+
Provider: "whatsapp",
299+
MessageSid: "msg",
300+
} as unknown as TemplateContext,
301+
opts: {},
302+
typingSignals: createMockTypingSignaler(),
303+
blockReplyPipeline: null,
304+
blockStreamingEnabled: false,
305+
resolvedBlockStreamingBreak: "message_end",
306+
applyReplyToMode: (payload) => payload,
307+
shouldEmitToolResult: () => true,
308+
shouldEmitToolOutput: () => false,
309+
pendingToolTasks: new Set(),
310+
resetSessionAfterRoleOrderingConflict: async () => false,
311+
isHeartbeat: false,
312+
sessionKey: "main",
313+
getActiveSessionEntry: () => undefined,
314+
resolvedVerboseLevel: "off",
315+
});
316+
// Three fixed backoffs (2_500ms each) separate the four cycles.
317+
await vi.advanceTimersByTimeAsync(8_000);
318+
const result = await promise;
319+
320+
expect(result.kind).toBe("final");
321+
// 1 initial cycle + exactly 3 configured retries.
322+
expect(state.runWithModelFallbackMock).toHaveBeenCalledTimes(4);
323+
} finally {
324+
vi.useRealTimers();
325+
}
326+
});
327+
328+
it("succeeds within the configured provider retry budget when a transient error clears (#87180)", async () => {
329+
// With a budget of 3, two transient failures are absorbed before a clean
330+
// cycle succeeds — the extra budget is used, not the shipped single retry.
331+
state.runWithModelFallbackMock
332+
.mockRejectedValueOnce(new Error("Connection error."))
333+
.mockRejectedValueOnce(new Error("socket hang up"))
334+
.mockImplementationOnce(async (params: FallbackRunnerParams) => ({
335+
result: await params.run("anthropic", "claude"),
336+
provider: "anthropic",
337+
model: "claude",
338+
attempts: [],
339+
}));
340+
state.runEmbeddedAgentMock.mockResolvedValueOnce({
341+
payloads: [{ text: "recovered" }],
342+
meta: {
343+
agentMeta: { sessionId: "session", provider: "anthropic", model: "claude" },
344+
},
345+
});
346+
347+
const runAgentTurnWithFallback = await getRunAgentTurnWithFallback();
348+
const followupRun = createFollowupRun();
349+
followupRun.run.providerRetryMaxRetries = 3;
350+
vi.useFakeTimers();
351+
try {
352+
const promise = runAgentTurnWithFallback({
353+
commandBody: "hello",
354+
followupRun,
355+
sessionCtx: {
356+
Provider: "whatsapp",
357+
MessageSid: "msg",
358+
} as unknown as TemplateContext,
359+
opts: {},
360+
typingSignals: createMockTypingSignaler(),
361+
blockReplyPipeline: null,
362+
blockStreamingEnabled: false,
363+
resolvedBlockStreamingBreak: "message_end",
364+
applyReplyToMode: (payload) => payload,
365+
shouldEmitToolResult: () => true,
366+
shouldEmitToolOutput: () => false,
367+
pendingToolTasks: new Set(),
368+
resetSessionAfterRoleOrderingConflict: async () => false,
369+
isHeartbeat: false,
370+
sessionKey: "main",
371+
getActiveSessionEntry: () => undefined,
372+
resolvedVerboseLevel: "off",
373+
});
374+
await vi.advanceTimersByTimeAsync(6_000);
375+
const result = await promise;
376+
377+
expect(result.kind).toBe("success");
378+
// 1 initial cycle + 2 retries before the third cycle recovers.
379+
expect(state.runWithModelFallbackMock).toHaveBeenCalledTimes(3);
380+
} finally {
381+
vi.useRealTimers();
382+
}
383+
});
280384
});

‎src/auto-reply/reply/agent-runner-execution.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,12 @@ async function runAgentTurnWithFallbackInternalWithRetryState(
258258
onError: (error) =>
259259
logVerbose(`agent model patch reconciliation failed: ${formatErrorMessage(error)}`),
260260
});
261-
let transientHttpRetriesRemaining = 1;
261+
// The embedded prompt-lock window pins SDK maxRetries to 0 (#87180), dropping
262+
// the configured provider retry budget in-window. Restore it here at the outer
263+
// full-attempt owner, where each retry re-runs the whole cycle and reacquires
264+
// the session lock. The budget is a prepared fact on the run (resolved once at
265+
// run preparation, not re-read per request); unset keeps the shipped single retry.
266+
let transientHttpRetriesRemaining = params.followupRun.run.providerRetryMaxRetries ?? 1;
262267
const consumeTransientHttpRetry = () => transientHttpRetriesRemaining-- > 0;
263268
let liveModelSwitchRetries = 0;
264269
const fallbackCycleState: AgentFallbackCycleState = {

‎src/auto-reply/reply/get-reply-run.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import crypto from "node:crypto";
33
import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id";
44
import { asDateTimestampMs } from "@openclaw/normalization-core/number-coercion";
55
import { type FastMode, normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
6+
import { resolveConfiguredProviderRetryMaxRetries } from "../../agents/agent-project-settings.js";
67
import {
78
clearAutoFallbackPrimaryProbeSelection,
89
hasLegacyAutoFallbackWithoutOrigin,
@@ -1633,6 +1634,14 @@ export async function runPreparedReply(
16331634
const replyPolicyChannel =
16341635
(replyRoute.channel as OriginatingChannelType | undefined) ??
16351636
(messageProvider as OriginatingChannelType | undefined);
1637+
// Resolve the configured provider retry budget once as a prepared run fact. The
1638+
// embedded prompt-lock window pins SDK retries to 0 (#87180); the outer reply
1639+
// orchestrator restores this budget as its whole-attempt transient-retry count.
1640+
const providerRetryMaxRetries = resolveConfiguredProviderRetryMaxRetries({
1641+
cwd: normalizeOptionalString(sessionEntry?.spawnedCwd) ?? workspaceDir,
1642+
agentDir,
1643+
cfg,
1644+
});
16361645
const followupRun = {
16371646
prompt: queuedBody,
16381647
transcriptPrompt: transcriptCommandBody,
@@ -1699,6 +1708,7 @@ export async function runPreparedReply(
16991708
skillsSnapshot,
17001709
provider,
17011710
model,
1711+
...(providerRetryMaxRetries !== undefined ? { providerRetryMaxRetries } : {}),
17021712
modelSelectionLocked: preparedSessionState.sessionEntry?.modelSelectionLocked === true,
17031713
hasSessionModelOverride: runHasSessionModelOverride,
17041714
modelOverrideSource: runModelOverrideSource,

‎src/auto-reply/reply/queue/types.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ export type FollowupRun = {
155155
skillsSnapshot?: SkillSnapshot;
156156
provider: string;
157157
model: string;
158+
/**
159+
* Configured provider retry budget (settings.retry.provider.maxRetries),
160+
* resolved once at run preparation. The embedded prompt-lock window pins SDK
161+
* retries to 0 (#87180), so the outer reply orchestrator restores this budget
162+
* as its whole-attempt transient-retry count where each retry reacquires the
163+
* session lock. Unset means the shipped single outer retry.
164+
*/
165+
providerRetryMaxRetries?: number;
158166
/** Prevents the queued run from selecting configured fallback models. */
159167
modelSelectionLocked?: boolean;
160168
hasSessionModelOverride?: boolean;

0 commit comments

Comments
 (0)