Skip to content

Commit 8f20d03

Browse files
steipeteadhirajhangal
steipete
and
adhirajhangal
committed
fix: cap idle timeouts without completed progress (#76345)
Co-authored-by: adhiraj <[email protected]>
1 parent ccce342 commit 8f20d03

4 files changed

Lines changed: 318 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ Docs: https://docs.openclaw.ai
8989
- Gateway/models: keep read-only model-list responses on registry-compatible fallbacks and metadata defaults, so empty or minimal persisted model files do not hide built-ins or custom model capabilities. Thanks @Marvinthebored.
9090
- CLI/doctor: load the configured memory-slot plugin when resolving memory diagnostics so bundled `memory-core` no longer triggers a false “no active memory plugin” warning on standalone `doctor` / `status` runs. Fixes #76367. Thanks @neeravmakwana.
9191
- Gateway: preserve stack diagnostics when `chat.send` or agent attachment parsing/staging fails, improving image-send failure triage. Refs #63432. (#75135) Thanks @keen0206.
92+
- Agents/idle-timeout: add a cost-runaway breaker to the outer embedded-run retry loop that halts further attempts after 5 consecutive idle timeouts without completed model progress, so a wedged provider can no longer fan paid model calls out across the same run; completed text or tool-call progress resets the breaker, but partial tool-argument token dribbles do not. Fixes #76293. Thanks @ThePuma312.
9293
- Heartbeats/Codex: stop sending the legacy `HEARTBEAT_OK` prompt instruction when heartbeat turns have the structured `heartbeat_respond` tool, while keeping the text sentinel for legacy automatic heartbeat replies. Thanks @pashpashpash.
9394
- Agent runtimes: fail explicit plugin runtime selections honestly when the requested harness is unavailable instead of silently falling back to the embedded PI runtime. Thanks @pashpashpash.
9495
- Maintainer workflow: push prepared PR heads through GitHub's verified commit API by default and require an explicit override before git-protocol pushes can publish unsigned commits. Thanks @BunsDev.

‎src/agents/pi-embedded-runner/run.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,11 @@ import {
120120
type RuntimeAuthState,
121121
scrubAnthropicRefusalMagic,
122122
} from "./run/helpers.js";
123+
import {
124+
MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT,
125+
createIdleTimeoutBreakerState,
126+
stepIdleTimeoutBreaker,
127+
} from "./run/idle-timeout-breaker.js";
123128
import {
124129
DEFAULT_EMPTY_RESPONSE_RETRY_LIMIT,
125130
DEFAULT_REASONING_ONLY_RETRY_LIMIT,
@@ -214,6 +219,16 @@ function normalizeEmbeddedRunAttemptResult(
214219
};
215220
}
216221

222+
function hasCompletedModelProgressForIdleBreaker(attempt: EmbeddedRunAttemptForRunner): boolean {
223+
return (
224+
attempt.assistantTexts.some((text) => text.trim().length > 0) ||
225+
attempt.toolMetas.length > 0 ||
226+
(attempt.clientToolCalls?.length ?? 0) > 0 ||
227+
hasMessagingToolDeliveryEvidence(attempt) ||
228+
attempt.itemLifecycle.completedCount > 0
229+
);
230+
}
231+
217232
function createEmptyAuthProfileStore(): AuthProfileStore {
218233
return {
219234
version: 1,
@@ -755,6 +770,13 @@ export async function runEmbeddedPiAgent(
755770
let reasoningOnlyRetryAttempts = 0;
756771
let emptyResponseRetryAttempts = 0;
757772
let sameModelIdleTimeoutRetries = 0;
773+
// Cost-runaway breaker for #76293. State lives at the run-loop level
774+
// on purpose so it survives across attempt boundaries and across
775+
// profile/auth retries within this embedded run (a wrapper-local
776+
// counter would reset on every iteration). The helper is pure and
777+
// unit-tested in run/idle-timeout-breaker.test.ts; the run loop just
778+
// feeds it the outcome of each attempt.
779+
const idleTimeoutBreakerState = createIdleTimeoutBreakerState();
758780
let lastRetryFailoverReason: FailoverReason | null = null;
759781
let planningOnlyRetryInstruction: string | null = null;
760782
let reasoningOnlyRetryInstruction: string | null = null;
@@ -1162,6 +1184,55 @@ export async function runEmbeddedPiAgent(
11621184
// reflects current context usage, not accumulated tool-loop usage.
11631185
lastRunPromptUsage = lastAssistantUsage ?? attemptUsage;
11641186
lastTurnTotal = lastAssistantUsage?.total ?? attemptUsage?.total;
1187+
// Idle-timeout cost-runaway breaker (#76293). Logic lives in the
1188+
// pure helper below so it stays unit-testable; the run loop just
1189+
// feeds it the latest attempt outcome and bails through the
1190+
// existing retry-limit exhaustion path when the cap is hit.
1191+
const breakerStep = stepIdleTimeoutBreaker(idleTimeoutBreakerState, {
1192+
idleTimedOut,
1193+
completedModelProgress: hasCompletedModelProgressForIdleBreaker(attempt),
1194+
outputTokens: attemptUsage?.output,
1195+
});
1196+
if (breakerStep.tripped) {
1197+
const breakerMessage =
1198+
`Idle-timeout cost-runaway breaker tripped: ` +
1199+
`${breakerStep.consecutive} consecutive idle timeouts ` +
1200+
`without completed model progress ` +
1201+
`(cap=${MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT}). ` +
1202+
`Halting further attempts to bound paid model calls. ` +
1203+
`See issue #76293.`;
1204+
log.error(
1205+
`[idle-timeout-circuit-breaker-tripped] ` +
1206+
`sessionKey=${params.sessionKey ?? params.sessionId} ` +
1207+
`provider=${provider}/${modelId} ` +
1208+
`consecutive=${breakerStep.consecutive} ` +
1209+
`cap=${MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT}`,
1210+
);
1211+
const breakerDecision = resolveRunFailoverDecision({
1212+
stage: "retry_limit",
1213+
fallbackConfigured,
1214+
failoverReason: lastRetryFailoverReason,
1215+
});
1216+
return handleRetryLimitExhaustion({
1217+
message: breakerMessage,
1218+
decision: breakerDecision,
1219+
provider,
1220+
model: modelId,
1221+
profileId: lastProfileId,
1222+
durationMs: Date.now() - started,
1223+
agentMeta: buildErrorAgentMeta({
1224+
sessionId: activeSessionId,
1225+
provider,
1226+
model: model.id,
1227+
contextTokens: ctxInfo.tokens,
1228+
usageAccumulator,
1229+
lastRunPromptUsage,
1230+
lastTurnTotal,
1231+
}),
1232+
replayInvalid: accumulatedReplayState.replayInvalid ? true : undefined,
1233+
livenessState: "blocked",
1234+
});
1235+
}
11651236
const attemptCompactionCount = Math.max(0, attempt.compactionCount ?? 0);
11661237
autoCompactionCount += attemptCompactionCount;
11671238
if (

‎src/agents/pi-embedded-runner/run/idle-timeout-breaker.test.ts‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT,
4+
createIdleTimeoutBreakerState,
5+
stepIdleTimeoutBreaker,
6+
} from "./idle-timeout-breaker.js";
7+
8+
// Issue #76293. The wedge: a stalled provider returns from each LLM call
9+
// with idleTimedOut=true and no completed model progress. Without this
10+
// breaker the outer run loop in run.ts can keep starting fresh attempts (a
11+
// new session and a new streamWithIdleTimeout wrapper each time, so any
12+
// wrapper-local counter would reset on every iteration). The breaker state
13+
// has to live at the outer-loop scope to survive across attempts and profile
14+
// failover, which is what stepIdleTimeoutBreaker captures.
15+
//
16+
// These tests exercise the helper directly. The integration in run.ts is
17+
// just `if (step.tripped) return handleRetryLimitExhaustion(...)`, so
18+
// proving the helper trips/resets correctly is what matters.
19+
describe("stepIdleTimeoutBreaker (#76293)", () => {
20+
function drive(
21+
inputs: Array<{
22+
idleTimedOut: boolean;
23+
completedModelProgress: boolean;
24+
outputTokens?: number;
25+
}>,
26+
options?: { cap?: number },
27+
) {
28+
const state = createIdleTimeoutBreakerState();
29+
const steps: Array<{ consecutive: number; tripped: boolean }> = [];
30+
for (const input of inputs) {
31+
steps.push(stepIdleTimeoutBreaker(state, input, options));
32+
}
33+
return steps;
34+
}
35+
36+
it("default cap matches the constant the run loop reads from", () => {
37+
expect(MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT).toBe(5);
38+
});
39+
40+
it("does not trip on a single wedged attempt", () => {
41+
const steps = drive([{ idleTimedOut: true, completedModelProgress: false }]);
42+
expect(steps[0]).toEqual({ consecutive: 1, tripped: false });
43+
});
44+
45+
it("trips on the Nth consecutive wedged attempt at the default cap", () => {
46+
const steps = drive([
47+
{ idleTimedOut: true, completedModelProgress: false },
48+
{ idleTimedOut: true, completedModelProgress: false },
49+
{ idleTimedOut: true, completedModelProgress: false },
50+
{ idleTimedOut: true, completedModelProgress: false },
51+
{ idleTimedOut: true, completedModelProgress: false },
52+
]);
53+
expect(steps.map((s) => s.tripped)).toEqual([false, false, false, false, true]);
54+
expect(steps.at(-1)?.consecutive).toBe(5);
55+
});
56+
57+
it("respects an explicit smaller cap", () => {
58+
const steps = drive(
59+
[
60+
{ idleTimedOut: true, completedModelProgress: false },
61+
{ idleTimedOut: true, completedModelProgress: false },
62+
{ idleTimedOut: true, completedModelProgress: false },
63+
],
64+
{ cap: 3 },
65+
);
66+
expect(steps.map((s) => s.tripped)).toEqual([false, false, true]);
67+
});
68+
69+
it("disables the breaker entirely when cap is 0 (escape hatch)", () => {
70+
const steps = drive(
71+
[
72+
{ idleTimedOut: true, completedModelProgress: false },
73+
{ idleTimedOut: true, completedModelProgress: false },
74+
{ idleTimedOut: true, completedModelProgress: false },
75+
{ idleTimedOut: true, completedModelProgress: false },
76+
{ idleTimedOut: true, completedModelProgress: false },
77+
{ idleTimedOut: true, completedModelProgress: false },
78+
{ idleTimedOut: true, completedModelProgress: false },
79+
],
80+
{ cap: 0 },
81+
);
82+
expect(steps.every((s) => !s.tripped)).toBe(true);
83+
expect(steps.at(-1)?.consecutive).toBe(7);
84+
});
85+
86+
it("does not trip when the model completed progress, even on a timeout (slow but alive)", () => {
87+
// 8 attempts that each timed out but each completed text or tool-call
88+
// progress. The model is slow at the tail of its turn, not wedged. The
89+
// breaker must stay disarmed so legitimate slow streams keep retrying.
90+
const steps = drive(
91+
Array.from({ length: 8 }, () => ({
92+
idleTimedOut: true,
93+
completedModelProgress: true,
94+
outputTokens: 220,
95+
})),
96+
);
97+
expect(steps.every((s) => !s.tripped)).toBe(true);
98+
expect(steps.at(-1)?.consecutive).toBe(0);
99+
});
100+
101+
it("resets the counter when a productive attempt arrives between wedged attempts", () => {
102+
// 4 wedged + 1 productive (completed progress) + 4 wedged. No run of 5
103+
// wedged in a row, so the breaker must stay disarmed across the whole
104+
// 9-attempt sequence even though 8 of the attempts were wedged in total.
105+
const steps = drive([
106+
{ idleTimedOut: true, completedModelProgress: false },
107+
{ idleTimedOut: true, completedModelProgress: false },
108+
{ idleTimedOut: true, completedModelProgress: false },
109+
{ idleTimedOut: true, completedModelProgress: false },
110+
{ idleTimedOut: false, completedModelProgress: true, outputTokens: 320 },
111+
{ idleTimedOut: true, completedModelProgress: false },
112+
{ idleTimedOut: true, completedModelProgress: false },
113+
{ idleTimedOut: true, completedModelProgress: false },
114+
{ idleTimedOut: true, completedModelProgress: false },
115+
]);
116+
expect(steps.map((s) => s.tripped)).toEqual([
117+
false,
118+
false,
119+
false,
120+
false,
121+
false,
122+
false,
123+
false,
124+
false,
125+
false,
126+
]);
127+
expect(steps.map((s) => s.consecutive)).toEqual([1, 2, 3, 4, 0, 1, 2, 3, 4]);
128+
});
129+
130+
it("non-timeout error attempts (no output) leave the counter unchanged", () => {
131+
// Sequence: 3 wedged, then 2 non-timeout attempts that produced no
132+
// completed progress (e.g. transport error, prompt overflow), then 2
133+
// more wedged. The non-timeout attempts must NOT reset the counter
134+
// (they're not evidence the model is alive) and must NOT increment it
135+
// (the breaker is specifically about idle timeouts). So 3+0+0+1+1 = 5,
136+
// trip on the last attempt.
137+
const steps = drive([
138+
{ idleTimedOut: true, completedModelProgress: false },
139+
{ idleTimedOut: true, completedModelProgress: false },
140+
{ idleTimedOut: true, completedModelProgress: false },
141+
{ idleTimedOut: false, completedModelProgress: false },
142+
{ idleTimedOut: false, completedModelProgress: false },
143+
{ idleTimedOut: true, completedModelProgress: false },
144+
{ idleTimedOut: true, completedModelProgress: false },
145+
]);
146+
expect(steps.map((s) => s.consecutive)).toEqual([1, 2, 3, 3, 3, 4, 5]);
147+
expect(steps.at(-1)?.tripped).toBe(true);
148+
});
149+
150+
it("does not reset for partial tool-argument tokens without completed progress", () => {
151+
const steps = drive([
152+
{ idleTimedOut: true, completedModelProgress: false, outputTokens: 12 },
153+
{ idleTimedOut: true, completedModelProgress: false, outputTokens: 18 },
154+
{ idleTimedOut: true, completedModelProgress: false, outputTokens: 24 },
155+
{ idleTimedOut: true, completedModelProgress: false, outputTokens: 30 },
156+
{ idleTimedOut: true, completedModelProgress: false, outputTokens: 36 },
157+
]);
158+
// Raw provider output tokens can come from partial tool-call argument
159+
// deltas before the provider stalls. They are billed, but they are not
160+
// completed progress, so they must not reset the breaker.
161+
expect(steps.map((s) => s.consecutive)).toEqual([1, 2, 3, 4, 5]);
162+
expect(steps.at(-1)?.tripped).toBe(true);
163+
});
164+
});

‎src/agents/pi-embedded-runner/run/idle-timeout-breaker.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Cap on consecutive attempts that ended in an idle timeout without completed
3+
* model progress, before the outer run loop refuses to start another attempt.
4+
* Distinct from MAX_SAME_MODEL_IDLE_TIMEOUT_RETRIES (which gates one extra
5+
* retry on the same model before failover) and the broad MAX_RUN_LOOP_ITERATIONS
6+
* backstop in run.ts.
7+
*
8+
* This one fires across profile/auth retries inside the same embedded run so a
9+
* wedged provider cannot fan out paid model calls across every fallback profile
10+
* in sequence. Resets when an attempt produces completed text or tool-call
11+
* progress, but not merely because the provider billed partial output tokens.
12+
*
13+
* See issue #76293 for the original report (single heartbeat fire generating
14+
* 761-1384 paid Anthropic calls in 60 seconds, costing $20-30 per incident).
15+
*/
16+
export const MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT = 5;
17+
18+
export type IdleTimeoutBreakerState = {
19+
consecutiveIdleTimeoutsBeforeOutput: number;
20+
};
21+
22+
export function createIdleTimeoutBreakerState(): IdleTimeoutBreakerState {
23+
return { consecutiveIdleTimeoutsBeforeOutput: 0 };
24+
}
25+
26+
export type IdleTimeoutBreakerInput = {
27+
idleTimedOut: boolean;
28+
completedModelProgress: boolean;
29+
outputTokens?: number;
30+
};
31+
32+
export type IdleTimeoutBreakerStep = {
33+
consecutive: number;
34+
tripped: boolean;
35+
};
36+
37+
/**
38+
* Update the breaker counter from the latest attempt's outcome and report
39+
* whether the cap is now tripped. Designed to be called from the outer run
40+
* loop right after an embedded attempt completes.
41+
*
42+
* Pure function modulo the mutable `state.consecutiveIdleTimeoutsBeforeOutput`
43+
* field, so the caller decides where the state lives (typically a `let` in
44+
* the outer loop).
45+
*
46+
* Decision table:
47+
* idleTimedOut completedModelProgress action
48+
* ------------ ---------------------- ------
49+
* true false count += 1 (wedged provider candidate)
50+
* true true count = 0 (model is alive but slow tail)
51+
* false true count = 0 (clean progress, all good)
52+
* false false count unchanged (e.g. non-timeout error;
53+
* don't poison or reset)
54+
*
55+
* The "false / false" branch matters: a non-timeout error attempt with no
56+
* completed progress should not reset the breaker (it isn't a sign the
57+
* provider is healthy), but it also shouldn't increment it (the issue at hand
58+
* is idle timeouts, not arbitrary errors).
59+
*
60+
* `outputTokens` is intentionally not part of the reset condition. Some
61+
* transports can accumulate billed output tokens from partial tool-call
62+
* argument deltas before the model stalls; those tokens are cost, not completed
63+
* progress, so they must not keep the breaker disarmed.
64+
*/
65+
export function stepIdleTimeoutBreaker(
66+
state: IdleTimeoutBreakerState,
67+
input: IdleTimeoutBreakerInput,
68+
options?: { cap?: number },
69+
): IdleTimeoutBreakerStep {
70+
const cap = options?.cap ?? MAX_CONSECUTIVE_IDLE_TIMEOUTS_BEFORE_OUTPUT;
71+
72+
if (input.idleTimedOut && !input.completedModelProgress) {
73+
state.consecutiveIdleTimeoutsBeforeOutput += 1;
74+
} else if (input.completedModelProgress) {
75+
state.consecutiveIdleTimeoutsBeforeOutput = 0;
76+
}
77+
78+
return {
79+
consecutive: state.consecutiveIdleTimeoutsBeforeOutput,
80+
tripped: cap > 0 && state.consecutiveIdleTimeoutsBeforeOutput >= cap,
81+
};
82+
}

0 commit comments

Comments
 (0)