Skip to content

Commit 4f61c4b

Browse files
altaywtfaltaywtf
committed
fix(agents): preserve wrapped restart aborts
1 parent e5b9b8a commit 4f61c4b

11 files changed

Lines changed: 175 additions & 285 deletions

File tree

‎src/agents/embedded-agent-runner/compact.abort-signal.test.ts‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import { beforeEach, describe, expect, it, vi } from "vitest";
22
import type { OpenClawConfig } from "../../config/types.openclaw.js";
33

4-
// Mock the model-fallback module BEFORE importing compact.ts so the import
5-
// resolves to our captured mock. We only need to assert the call shape — the
6-
// inner `run` callback never has to execute, because we resolve a fake result.
74
vi.mock("../model-fallback.js", () => ({
85
runWithModelFallback: vi.fn(async (params: Record<string, unknown>) => ({
96
result: { ok: true, compacted: false, reason: "no-op" },
@@ -14,9 +11,6 @@ vi.mock("../model-fallback.js", () => ({
1411
isFallbackSummaryError: () => false,
1512
}));
1613

17-
// Stub the inner once-fn dependencies. compactEmbeddedAgentSessionDirectOnce isn't
18-
// reached in this test (runWithModelFallback is mocked to short-circuit), so we
19-
// don't need to wire its runtime — but the module-level imports still resolve.
2014
vi.mock("./compact.queued.js", () => ({ compactEmbeddedAgentSession: vi.fn() }));
2115

2216
import { runWithModelFallback } from "../model-fallback.js";
@@ -44,17 +38,12 @@ function configWithFallbacks(fallbacks: string[]): OpenClawConfig {
4438
} as OpenClawConfig;
4539
}
4640

47-
describe("compactEmbeddedAgentSessionDirect — abortSignal threading (regression for openclaw/openclaw#62682)", () => {
41+
describe("compactEmbeddedAgentSessionDirect abortSignal threading", () => {
4842
beforeEach(() => {
4943
runMock.mockClear();
5044
});
5145

5246
it("forwards params.abortSignal to runWithModelFallback so terminal aborts during compaction short-circuit", async () => {
53-
// Flagged by @Lellansin reviewing #62682: the compaction model-fallback
54-
// call must receive the caller's abort signal, otherwise a terminal abort
55-
// mid-compaction is classified as a failed compaction candidate and
56-
// cascades into the next compaction fallback model — exactly the kind of
57-
// wasted retry this PR is supposed to stop.
5847
const controller = new AbortController();
5948

6049
await compactEmbeddedAgentSessionDirect({

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,11 +2324,6 @@ async function runEmbeddedAgentInternal(
23242324
attempt.setTerminalLifecycleMeta?.({ ...meta, aborted });
23252325
};
23262326
const timedOutDuringToolExecution = attempt.timedOutDuringToolExecution ?? false;
2327-
// Optional in the public harness SDK contract; default to false. The
2328-
// embedded runner sets this explicitly when its run-budget timer
2329-
// fires. Used below by the failover-policy and model-fallback layer
2330-
// to skip the fallback chain when the whole-run deadline has elapsed
2331-
// (no other model can help). Closes #60388.
23322327
const timedOutByRunBudget = attempt.timedOutByRunBudget ?? false;
23332328
adoptActiveSessionId(sessionIdUsed);
23342329
if (sessionFileUsed && sessionFileUsed !== activeSessionFile) {
@@ -2506,11 +2501,6 @@ async function runEmbeddedAgentInternal(
25062501
);
25072502
throw new LiveSessionModelSwitchError(requestedSelection);
25082503
}
2509-
// ── Timeout-triggered compaction ──────────────────────────────────
2510-
// When the LLM times out with high context usage, compact before
2511-
// retrying to break the death spiral of repeated timeouts. Skip when
2512-
// the run-budget timer fired (the whole-run deadline is already
2513-
// exhausted; compacting is wasted work). Closes #60388.
25142504
if (
25152505
timedOut &&
25162506
!timedOutDuringCompaction &&
@@ -3248,6 +3238,7 @@ async function runEmbeddedAgentInternal(
32483238
failoverReason: promptFailoverReason,
32493239
harnessOwnsTransport: pluginHarnessOwnsTransport,
32503240
promptTimeoutFallbackSafe,
3241+
timedOutByRunBudget,
32513242
profileRotated: false,
32523243
});
32533244
if (
@@ -3288,6 +3279,7 @@ async function runEmbeddedAgentInternal(
32883279
failoverReason: promptFailoverReason,
32893280
harnessOwnsTransport: pluginHarnessOwnsTransport,
32903281
promptTimeoutFallbackSafe,
3282+
timedOutByRunBudget,
32913283
profileRotated: true,
32923284
});
32933285
}

‎src/agents/embedded-agent-runner/run/abortable.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,7 @@ function getAbortReason(signal: AbortSignal): unknown {
77
return "reason" in signal ? (signal as { reason?: unknown }).reason : undefined;
88
}
99

10-
/**
11-
* Marker placed on every error produced by `makeAbortError()` (i.e., every
12-
* rejection emitted by `abortable()` when its signal fires). Used by the model
13-
* fallback layer to positively identify "this AbortError wraps a signal that
14-
* fired from within the OpenClaw embedded runner" — vs. a provider/SDK that
15-
* happens to throw an `AbortError(cause: TimeoutError)` for its own per-request
16-
* timeout (which must remain retryable through the configured fallback chain).
17-
* See `src/agents/model-fallback.ts` `isTerminalAbortFromError`.
18-
*/
10+
/** Marks AbortErrors produced by abortable() so provider aborts stay retryable. */
1911
export const OPENCLAW_ABORTABLE_WRAPPER = Symbol.for("openclaw.abortable.wrapper");
2012

2113
export function isOpenClawAbortableWrapper(err: unknown): boolean {

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -952,10 +952,6 @@ export async function runEmbeddedAttempt(
952952
let idleTimedOut = false;
953953
let timedOutDuringCompaction = false;
954954
let timedOutDuringToolExecution = false;
955-
// True when the embedded run-budget timer (`scheduleAbortTimer`) fires.
956-
// Distinct from idleTimedOut (per-LLM-call streaming idle) and from
957-
// caller-signal aborts. This is the "the whole run's deadline is exhausted
958-
// — no fallback model can help" case. Closes #60388.
959955
let timedOutByRunBudget = false;
960956
let promptError: unknown = null;
961957
let emitDiagnosticRunCompleted:
@@ -3877,9 +3873,6 @@ export async function runEmbeddedAttempt(
38773873
) {
38783874
timedOutDuringCompaction = true;
38793875
}
3880-
// Mark the run-budget exhaustion explicitly so the fallback layer
3881-
// can stop the model-fallback chain (no other model can help once
3882-
// the whole run's deadline has elapsed). Closes #60388.
38833876
timedOutByRunBudget = true;
38843877
abortRun(true);
38853878
if (!abortWarnTimer) {

‎src/agents/embedded-agent-runner/run/failover-policy.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,43 @@ describe("resolveRunFailoverDecision", () => {
9595
});
9696
});
9797

98+
it("surfaces prompt run-budget timeouts instead of model fallback (#60388)", () => {
99+
expect(
100+
resolveRunFailoverDecision({
101+
stage: "prompt",
102+
aborted: true,
103+
externalAbort: false,
104+
fallbackConfigured: true,
105+
failoverFailure: true,
106+
failoverReason: "timeout",
107+
promptTimeoutFallbackSafe: true,
108+
timedOutByRunBudget: true,
109+
profileRotated: true,
110+
}),
111+
).toEqual({
112+
action: "surface_error",
113+
reason: "timeout",
114+
});
115+
});
116+
117+
it("does not rotate prompt failures after the run budget is exhausted (#60388)", () => {
118+
expect(
119+
resolveRunFailoverDecision({
120+
stage: "prompt",
121+
aborted: true,
122+
externalAbort: false,
123+
fallbackConfigured: true,
124+
failoverFailure: true,
125+
failoverReason: "rate_limit",
126+
timedOutByRunBudget: true,
127+
profileRotated: false,
128+
}),
129+
).toEqual({
130+
action: "surface_error",
131+
reason: "rate_limit",
132+
});
133+
});
134+
98135
it("surfaces deterministic prompt format failures instead of rotating or falling back", () => {
99136
expect(
100137
resolveRunFailoverDecision({

‎src/agents/embedded-agent-runner/run/failover-policy.ts‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ type PromptDecisionParams = {
5151
failoverReason: FailoverReason | null;
5252
harnessOwnsTransport?: boolean;
5353
promptTimeoutFallbackSafe?: boolean;
54+
timedOutByRunBudget?: boolean;
5455
profileRotated: boolean;
5556
};
5657

@@ -67,14 +68,6 @@ type AssistantDecisionParams = {
6768
timedOutDuringCompaction: boolean;
6869
timedOutDuringToolExecution: boolean;
6970
harnessOwnsTransport?: boolean;
70-
/**
71-
* True when the embedded run-budget timer fired (whole-run deadline
72-
* exhausted). When set, no fallback model can help — the run is over
73-
* regardless of which provider handles it. Optional (defaults to falsy =
74-
* "not a run-budget timeout") so callers that predate this signal keep their
75-
* existing rotation behavior; the embedded runner always passes it
76-
* explicitly. Closes #60388.
77-
*/
7871
timedOutByRunBudget?: boolean;
7972
profileRotated: boolean;
8073
};
@@ -101,6 +94,9 @@ function isTerminalFormatFailure(params: {
10194
}
10295

10396
function shouldRotatePrompt(params: PromptDecisionParams): boolean {
97+
if (params.timedOutByRunBudget) {
98+
return false;
99+
}
104100
return (
105101
params.failoverFailure &&
106102
params.failoverReason !== "timeout" &&
@@ -125,9 +121,6 @@ function shouldRotateAssistant(params: AssistantDecisionParams): boolean {
125121
if (isTerminalFormatFailure(params)) {
126122
return false;
127123
}
128-
// Run-budget timeouts are terminal — the run is out of time regardless of
129-
// which model handles it. Don't rotate to another profile/model just to have
130-
// it time out again. (#62682, closes #60388.)
131124
if (params.timedOutByRunBudget) {
132125
return false;
133126
}
@@ -190,6 +183,12 @@ export function resolveRunFailoverDecision(params: RunFailoverDecisionParams): R
190183
reason: params.failoverReason,
191184
};
192185
}
186+
if (params.timedOutByRunBudget) {
187+
return {
188+
action: "surface_error",
189+
reason: params.failoverReason,
190+
};
191+
}
193192
if (params.harnessOwnsTransport && params.failoverReason === "timeout") {
194193
// Plugin harness lifecycle timeouts must stay inside the harness boundary;
195194
// only prompt request timeouts proven replay-safe may enter model fallback.

‎src/agents/embedded-agent-runner/run/types.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,6 @@ export type EmbeddedRunAttemptResult = {
123123
timedOutDuringCompaction: boolean;
124124
/** Optional because this type is re-exported as `AgentHarnessAttemptResult`. */
125125
timedOutDuringToolExecution?: boolean;
126-
/**
127-
* True when the embedded run-budget timer (`scheduleAbortTimer`) fired,
128-
* exhausting the whole-run deadline. Distinct from `idleTimedOut`
129-
* (per-LLM-call streaming idle, where retrying with a fallback model can
130-
* help) and from caller-signal aborts: this is the "run is over regardless
131-
* of which model handles it" case. The model-fallback layer must skip the
132-
* fallback chain when this is set. Closes #60388.
133-
*
134-
* Optional because this type is re-exported as `AgentHarnessAttemptResult`.
135-
*/
136126
timedOutByRunBudget?: boolean;
137127
promptError: unknown;
138128
/**

0 commit comments

Comments
 (0)