Skip to content

Commit 245c18d

Browse files
hugenshenNIO
hugenshen
and
NIO
authored
fix(agents): retry compaction on provider-side AbortErrors (#97504)
Align inner compaction retry and compaction-safeguard provider fallback with the #90908 signal.aborted pattern so undici disconnect AbortErrors retry or fall back to LLM instead of producing degraded placeholder summaries. Caller abort remains terminal. Co-authored-by: NIO <[email protected]>
1 parent fb7e10e commit 245c18d

5 files changed

Lines changed: 244 additions & 15 deletions

File tree

‎src/agents/agent-hooks/compaction-safeguard.test.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,6 +1864,107 @@ describe("compaction-safeguard recent-turn preservation", () => {
18641864
expect(JSON.stringify(messages[0])).toContain("Old duplicated section");
18651865
});
18661866

1867+
it("falls back to LLM when provider throws a provider-side AbortError with signal not aborted", async () => {
1868+
// Reproduce the undici AbortError("This operation was aborted") shape that
1869+
// arrives when the compaction provider's HTTP connection drops mid-stream while
1870+
// the caller has NOT yet fired their abort signal. Before the fix,
1871+
// isAbortError() matched this shape so tryProviderSummarize rethrew and the
1872+
// extension runner swallowed the error — the LLM fallback path was skipped.
1873+
mockSummarizeInStages.mockReset();
1874+
mockSummarizeInStages.mockResolvedValue("llm fallback summary");
1875+
1876+
const providerAbortErr = Object.assign(new Error("This operation was aborted"), {
1877+
name: "AbortError",
1878+
});
1879+
const failingProviderSummarize = vi.fn().mockRejectedValue(providerAbortErr);
1880+
registerCompactionProvider({
1881+
id: "disconnecting-provider",
1882+
label: "Disconnecting Provider",
1883+
summarize: failingProviderSummarize,
1884+
});
1885+
1886+
const sessionManager = stubSessionManager();
1887+
const model = createAnthropicModelFixture();
1888+
setCompactionSafeguardRuntime(sessionManager, {
1889+
provider: "disconnecting-provider",
1890+
model,
1891+
recentTurnsPreserve: 0,
1892+
});
1893+
1894+
const event = {
1895+
preparation: {
1896+
messagesToSummarize: [
1897+
{ role: "user", content: "older context", timestamp: 1 },
1898+
{ role: "assistant", content: "older reply", timestamp: 2 } as unknown as AgentMessage,
1899+
],
1900+
turnPrefixMessages: [] as AgentMessage[],
1901+
firstKeptEntryId: "entry-1",
1902+
tokensBefore: 1_500,
1903+
fileOps: {
1904+
read: [],
1905+
edited: [],
1906+
written: [],
1907+
},
1908+
settings: { reserveTokens: 4_000 },
1909+
},
1910+
customInstructions: "",
1911+
signal: new AbortController().signal, // not aborted
1912+
};
1913+
const { result } = await runCompactionScenario({ sessionManager, event, apiKey: "key" });
1914+
1915+
// Provider failure → LLM fallback ran, not { cancel: true }.
1916+
expect(result.cancel).not.toBe(true);
1917+
expect(mockSummarizeInStages).toHaveBeenCalled();
1918+
});
1919+
1920+
it("propagates provider AbortError and cancels when caller signal is already aborted", async () => {
1921+
mockSummarizeInStages.mockReset();
1922+
1923+
const providerAbortErr = Object.assign(new Error("This operation was aborted"), {
1924+
name: "AbortError",
1925+
});
1926+
const failingProviderSummarize = vi.fn().mockRejectedValue(providerAbortErr);
1927+
registerCompactionProvider({
1928+
id: "aborted-provider",
1929+
label: "Aborted Provider",
1930+
summarize: failingProviderSummarize,
1931+
});
1932+
1933+
const controller = new AbortController();
1934+
controller.abort();
1935+
1936+
const sessionManager = stubSessionManager();
1937+
setCompactionSafeguardRuntime(sessionManager, {
1938+
provider: "aborted-provider",
1939+
});
1940+
1941+
const event = {
1942+
preparation: {
1943+
messagesToSummarize: [
1944+
{ role: "user", content: "older context", timestamp: 1 },
1945+
] as AgentMessage[],
1946+
turnPrefixMessages: [] as AgentMessage[],
1947+
firstKeptEntryId: "entry-1",
1948+
tokensBefore: 1_500,
1949+
fileOps: {
1950+
read: [],
1951+
edited: [],
1952+
written: [],
1953+
},
1954+
settings: { reserveTokens: 4_000 },
1955+
},
1956+
customInstructions: "",
1957+
signal: controller.signal, // already aborted
1958+
};
1959+
1960+
await expect(
1961+
runCompactionScenario({ sessionManager, event, apiKey: "key" }),
1962+
).rejects.toMatchObject({ name: "AbortError" });
1963+
1964+
// Caller abort is terminal — LLM fallback should not have run.
1965+
expect(mockSummarizeInStages).not.toHaveBeenCalled();
1966+
});
1967+
18671968
it("passes compaction instructions to providers and preserves suffix context", async () => {
18681969
mockSummarizeInStages.mockReset();
18691970
const providerSummarize = vi.fn().mockResolvedValue("provider summary body");

‎src/agents/agent-hooks/compaction-safeguard.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,13 @@ async function tryProviderSummarize(
207207
log.warn(`Compaction provider "${provider.id}" returned empty result, falling back to LLM.`);
208208
return undefined;
209209
} catch (err) {
210-
// Abort/timeout errors must propagate — the caller requested cancellation.
211-
if (isAbortError(err) || isTimeoutError(err)) {
210+
// Propagate only when the caller explicitly cancelled. Provider-side
211+
// AbortErrors (signal not aborted) fall through to LLM summarization.
212+
if (params.signal?.aborted) {
213+
throw err;
214+
}
215+
// Real non-abort transport timeouts (e.g. ETIMEDOUT) still propagate.
216+
if (!isAbortError(err) && isTimeoutError(err)) {
212217
throw err;
213218
}
214219
log.warn(
@@ -1009,9 +1014,12 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
10091014
// Provider returned empty — fall through to LLM path.
10101015
log.info("Compaction provider did not produce a result; falling back to LLM path.");
10111016
} catch (err) {
1012-
// tryProviderSummarize rethrows abort/timeout — if we reach here it is
1013-
// an unexpected error from the assembly step. Fall through to LLM path.
1014-
if (isAbortError(err) || isTimeoutError(err)) {
1017+
// tryProviderSummarize rethrows on caller cancellation; reaching here
1018+
// means an unexpected error in the assembly step. Fall through to LLM.
1019+
if (signal?.aborted) {
1020+
throw err;
1021+
}
1022+
if (!isAbortError(err) && isTimeoutError(err)) {
10151023
throw err;
10161024
}
10171025
log.warn(

‎src/agents/compaction-partial-summary.test.ts‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,26 +116,57 @@ describe("summarizeChunks partial summary preservation (#82952)", () => {
116116
);
117117
});
118118

119-
it("re-throws abort errors instead of returning partial summary", async () => {
119+
it("re-throws abort errors when the caller signal is already aborted", async () => {
120120
const abortErr = new Error("aborted");
121121
abortErr.name = "AbortError";
122122

123123
compactionMocks.generateSummary
124124
.mockResolvedValueOnce("Summary of chunk 1")
125125
.mockRejectedValue(abortErr);
126126

127-
const result = await callSummarize();
128-
129-
// Abort errors represent caller intent, so partial recovery must not mask
130-
// cancellation as successful summarization.
131-
expect(result).not.toBe("Summary of chunk 1");
132-
expect(result).toContain("Context contained");
127+
const controller = new AbortController();
128+
controller.abort();
129+
130+
await expect(
131+
summarizeWithFallback({
132+
messages: twoChunkMessages,
133+
model: testModel,
134+
apiKey: "test-key", // pragma: allowlist secret
135+
signal: controller.signal,
136+
reserveTokens: 1000,
137+
maxChunkTokens: 150,
138+
contextWindow: 200_000,
139+
}),
140+
).rejects.toMatchObject({ name: "AbortError" });
141+
142+
// Caller abort is terminal — partial recovery must not mask cancellation.
133143
expect(compactionMocks.logWarn).not.toHaveBeenCalledWith(
134144
"chunk summarization failed after retries; partial summary available",
135145
expect.anything(),
136146
);
137147
});
138148

149+
it("returns partial summary when a later chunk fails with a provider-side AbortError", async () => {
150+
const providerAbortErr = Object.assign(new Error("This operation was aborted"), {
151+
name: "AbortError",
152+
});
153+
154+
compactionMocks.generateSummary
155+
.mockResolvedValueOnce("Summary of chunk 1")
156+
.mockRejectedValue(providerAbortErr);
157+
158+
const result = await callSummarize();
159+
160+
// Provider-side disconnects are not caller cancellation; preserve completed work.
161+
expect(result).toContain("Summary of chunk 1");
162+
expect(result).toContain("[Partial summary:");
163+
expect(result).toMatch(/chunks 1-1 of 2 were summarized/);
164+
expect(compactionMocks.logWarn).toHaveBeenCalledWith(
165+
"chunk summarization failed after retries; partial summary available",
166+
expect.objectContaining({ err: providerAbortErr }),
167+
);
168+
});
169+
139170
it("re-throws timeout errors instead of returning partial summary", async () => {
140171
const timeoutErr = new Error("request timed out");
141172
timeoutErr.name = "TimeoutError";

‎src/agents/compaction.summarize-fallback.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,71 @@ describe("summarizeWithFallback", () => {
7373
expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(1);
7474
});
7575

76+
it("retries provider-side AbortError and returns a real summary when caller signal is not aborted", async () => {
77+
// Reproduce the undici AbortError("This operation was aborted") shape thrown
78+
// when the LLM API closes the connection mid-stream without the caller signal
79+
// being fired. Before the fix, isAbortError() + isTimeoutError() both matched
80+
// this error shape, so shouldRetry returned false and no retry was attempted —
81+
// the compaction fell back to the "Summary unavailable" placeholder instead.
82+
const providerAbortErr = Object.assign(new Error("This operation was aborted"), {
83+
name: "AbortError",
84+
});
85+
agentSessionMocks.generateSummary
86+
.mockRejectedValueOnce(providerAbortErr)
87+
.mockResolvedValueOnce("recovered summary after provider disconnect");
88+
89+
const result = await summarizeWithFallback({
90+
messages: [
91+
{
92+
role: "user",
93+
content: "hello",
94+
timestamp: 1,
95+
} satisfies UserMessage,
96+
],
97+
model: testModel,
98+
apiKey: "test-key", // pragma: allowlist secret
99+
signal: new AbortController().signal, // not aborted
100+
reserveTokens: 1000,
101+
maxChunkTokens: 50_000,
102+
contextWindow: 200_000,
103+
});
104+
105+
expect(result).toBe("recovered summary after provider disconnect");
106+
// Two calls: first fails with provider-side AbortError, second succeeds.
107+
expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(2);
108+
});
109+
110+
it("does not retry and propagates AbortError immediately when caller signal is already aborted", async () => {
111+
const controller = new AbortController();
112+
controller.abort();
113+
114+
const providerAbortErr = Object.assign(new Error("This operation was aborted"), {
115+
name: "AbortError",
116+
});
117+
agentSessionMocks.generateSummary.mockRejectedValueOnce(providerAbortErr);
118+
119+
await expect(
120+
summarizeWithFallback({
121+
messages: [
122+
{
123+
role: "user",
124+
content: "hello",
125+
timestamp: 1,
126+
} satisfies UserMessage,
127+
],
128+
model: testModel,
129+
apiKey: "test-key", // pragma: allowlist secret
130+
signal: controller.signal, // already aborted
131+
reserveTokens: 1000,
132+
maxChunkTokens: 50_000,
133+
contextWindow: 200_000,
134+
}),
135+
).rejects.toMatchObject({ name: "AbortError" });
136+
137+
// Caller abort is terminal — no retry, no fallback to placeholder.
138+
expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(1);
139+
});
140+
76141
it("still attempts partial summarization when oversized messages were excluded", async () => {
77142
// Oversized-message fallback tries the safe subset so a huge attachment or
78143
// tool output does not prevent summarizing the rest of the transcript.

‎src/agents/compaction.ts‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,31 @@ async function summarizeChunks(params: {
175175
maxDelayMs: 5000,
176176
jitter: 0.2,
177177
label: "compaction/generateSummary",
178-
shouldRetry: (err) => !isAbortError(err) && !isTimeoutError(err),
178+
shouldRetry: (err) => {
179+
// Stop retrying when the caller explicitly cancelled.
180+
if (params.signal.aborted) {
181+
return false;
182+
}
183+
// Preserve existing non-retry policy for real network/transport
184+
// timeouts (e.g. "fetch failed", ETIMEDOUT) that are not AbortErrors.
185+
if (!isAbortError(err) && isTimeoutError(err)) {
186+
return false;
187+
}
188+
// Provider-side AbortErrors with signal not yet aborted are
189+
// transient disconnects — retrying is correct.
190+
return true;
191+
},
179192
},
180193
);
181194
hasGeneratedChunk = true;
182195
} catch (err) {
183-
// Abort and timeout errors always propagate immediately.
184-
if (isAbortError(err) || isTimeoutError(err)) {
196+
// Propagate only when the caller explicitly cancelled. Provider-side
197+
// AbortErrors (signal not aborted) fall through to partial/fallback paths.
198+
if (params.signal.aborted) {
199+
throw err;
200+
}
201+
// Real non-abort transport timeouts still propagate immediately.
202+
if (!isAbortError(err) && isTimeoutError(err)) {
185203
throw err;
186204
}
187205
// No chunk has succeeded yet — rethrow so summarizeWithFallback
@@ -270,6 +288,9 @@ export async function summarizeWithFallback(params: {
270288
try {
271289
return await summarizeChunks(params);
272290
} catch (fullError) {
291+
if (params.signal.aborted) {
292+
throw fullError;
293+
}
273294
log.warn(`Full summarization failed: ${formatErrorMessage(fullError)}`);
274295
partialSummaryFallback = (fullError as PartialSummaryError).partialSummary;
275296
}
@@ -292,6 +313,9 @@ export async function summarizeWithFallback(params: {
292313
const notes = oversizedNotes.length > 0 ? `\n\n${oversizedNotes.join("\n")}` : "";
293314
return partialSummary + notes;
294315
} catch (partialError) {
316+
if (params.signal.aborted) {
317+
throw partialError;
318+
}
295319
log.warn(`Partial summarization also failed: ${formatErrorMessage(partialError)}`);
296320
// Prefer the oversized retry's partial summary over the full attempt's,
297321
// since it covers the non-oversized transcript. Append oversized notes

0 commit comments

Comments
 (0)