diff --git a/src/agents/agent-hooks/compaction-safeguard.test.ts b/src/agents/agent-hooks/compaction-safeguard.test.ts index fc229dd377b0..c018126cca3f 100644 --- a/src/agents/agent-hooks/compaction-safeguard.test.ts +++ b/src/agents/agent-hooks/compaction-safeguard.test.ts @@ -1773,6 +1773,66 @@ describe("compaction-safeguard recent-turn preservation", () => { expect(providerPrompts[0]).toContain("[User]: summarize me"); }); + it("surfaces a total provider failure and leaves the safeguard transcript unchanged", async () => { + testing.setSummarizeInStagesForTest(actualCompactionModule.summarizeInStages); + const sessionManager = stubSessionManager(); + const model = createAnthropicModelFixture({ + api: "test-api" as never, + baseUrl: "", + }); + setCompactionSafeguardRuntime(sessionManager, { model, recentTurnsPreserve: 0 }); + + const streamFn: StreamFn = () => { + const stream = createAssistantMessageEventStream(); + stream.push({ + type: "error", + reason: "error", + error: { + role: "assistant", + content: [], + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "error", + errorMessage: "Cannot convert undefined or null to object", + timestamp: 1, + }, + }); + stream.end(); + return stream; + }; + const mockContext = createCompactionContext({ + sessionManager, + getApiKeyAndHeadersMock: vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" }), + }); + const compactionHandler = createCompactionHandler(); + const event = { + ...createCompactionEvent({ messageText: "summarize me", tokensBefore: 1_000 }), + streamFn, + }; + (event.preparation as { settings?: { reserveTokens: number } }).settings = { + reserveTokens: 4_000, + }; + const transcriptBefore = structuredClone(event.preparation.messagesToSummarize); + + const result = await compactionHandler(event, mockContext); + + expect(result).toEqual({ cancel: true }); + expect(result).not.toHaveProperty("compaction"); + expect(event.preparation.messagesToSummarize).toStrictEqual(transcriptBefore); + expect(consumeCompactionSafeguardCancelReason(sessionManager)).toContain( + "Cannot convert undefined or null to object", + ); + }); + it("does not retry summaries unless quality guard is explicitly enabled", async () => { mockSummarizeInStages.mockReset(); mockSummarizeInStages.mockResolvedValue(summaryResult("summary missing headings")); diff --git a/src/agents/compaction-partial-summary.test.ts b/src/agents/compaction-partial-summary.test.ts index b861fecfce5a..399592e3a912 100644 --- a/src/agents/compaction-partial-summary.test.ts +++ b/src/agents/compaction-partial-summary.test.ts @@ -167,7 +167,7 @@ describe("summarizeChunks partial summary preservation (#82952)", () => { ); }); - it("re-throws timeout errors instead of returning partial summary", async () => { + it("throws CompactionError when timeout occurs and no partial summary is available", async () => { const timeoutErr = new Error("request timed out"); timeoutErr.name = "TimeoutError"; @@ -175,10 +175,8 @@ describe("summarizeChunks partial summary preservation (#82952)", () => { .mockResolvedValueOnce("Summary of chunk 1") .mockRejectedValue(timeoutErr); - const result = await callSummarize(); - - expect(result).not.toBe("Summary of chunk 1"); - expect(result).toContain("Context contained"); + await expect(callSummarize()).rejects.toThrow("All summarization attempts failed"); + // Timeout errors propagate immediately without logging partial summary expect(compactionMocks.logWarn).not.toHaveBeenCalledWith( "chunk summarization failed after retries; partial summary available", expect.anything(), @@ -196,15 +194,12 @@ describe("summarizeChunks partial summary preservation (#82952)", () => { expect(compactionMocks.generateSummary).toHaveBeenCalledTimes(2); }); - it("falls back to default when the first chunk fails (no partial to recover)", async () => { + it("throws CompactionError when the first chunk fails (no partial to recover)", async () => { compactionMocks.generateSummary.mockRejectedValue(new Error("network error")); - const result = await callSummarize(); - - // With no successful chunk, summarizeChunks rethrows into - // summarizeWithFallback's outer catch -> final fallback path. - expect(result).toContain("Context contained"); - expect(result).not.toBe("Summary of chunk 1"); + await expect(callSummarize()).rejects.toThrow( + "All summarization attempts failed for 2 messages", + ); }); it("tries oversized-message retry before falling back to partial summary", async () => { diff --git a/src/agents/compaction.circuit-breaker.test.ts b/src/agents/compaction.circuit-breaker.test.ts index 6cc290c09390..afb69d8368d2 100644 --- a/src/agents/compaction.circuit-breaker.test.ts +++ b/src/agents/compaction.circuit-breaker.test.ts @@ -50,54 +50,39 @@ async function summarize() { }); } -describe("compaction staged fallback circuit breaker", () => { +describe("compaction staged summarization failures", () => { beforeEach(() => { agentSessionMocks.estimateTokens.mockClear(); agentSessionMocks.generateSummary.mockReset(); }); - it("stops the fallback storm and rejects the incomplete compaction", async () => { + it("throws CompactionError when any chunk summarization fails", async () => { agentSessionMocks.generateSummary.mockRejectedValue(new Error("fetch failed")); - await expect(summarize()).rejects.toThrow( - "Compaction staged summarization stopped after repeated generic fallbacks", - ); - - expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(2); + // The first chunk failure propagates as a CompactionError — no + // circuit-breaker / generic-fallback recovery. + await expect(summarize()).rejects.toThrow(); }); - it("resets after a successful split, completes the merge, and remains degraded", async () => { + it("completes the merge successfully when all chunks succeed", async () => { agentSessionMocks.generateSummary - .mockRejectedValueOnce(new Error("fetch failed")) - .mockResolvedValueOnce("middle summary") - .mockRejectedValueOnce(new Error("fetch failed")) - .mockResolvedValueOnce("merged summary"); + .mockResolvedValueOnce("summary of chunk 1") + .mockResolvedValueOnce("summary of chunk 2") + .mockResolvedValueOnce("summary of chunk 3") + .mockResolvedValue("merged: chunk 1 + chunk 2 + chunk 3"); - await expect(summarize()).resolves.toEqual({ - kind: "generic-fallback", - text: "merged summary", - }); - expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(4); - }); - - it("reports a summary when only a later split degraded and the oldest one survived", async () => { - agentSessionMocks.generateSummary - .mockResolvedValueOnce("oldest summary") - .mockRejectedValueOnce(new Error("fetch failed")) - .mockResolvedValueOnce("newest summary") - .mockResolvedValueOnce("merged: oldest summary + newest summary"); - - // The oldest split carries whatever context the caller needs redistilled, and it - // made it into the merge. Reporting a fallback here makes callers re-add it. await expect(summarize()).resolves.toEqual({ kind: "summary", - text: "merged: oldest summary + newest summary", + text: expect.stringContaining("merged"), }); - expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(4); - expect(agentSessionMocks.generateSummary.mock.calls[3]?.[0]).toEqual( - expect.arrayContaining([ - expect.objectContaining({ content: expect.stringContaining("oldest summary") }), - ]), - ); + }); + + it("throws CompactionError when a later chunk fails after earlier successes", async () => { + agentSessionMocks.generateSummary + .mockResolvedValueOnce("summary of chunk 1") + .mockRejectedValue(new Error("fetch failed on chunk 2")); + + // Chunk 2 failure stops the pipeline — no merge attempted. + await expect(summarize()).rejects.toThrow(); }); }); diff --git a/src/agents/compaction.failure-proof.test.ts b/src/agents/compaction.failure-proof.test.ts new file mode 100644 index 000000000000..2bb20c27d312 --- /dev/null +++ b/src/agents/compaction.failure-proof.test.ts @@ -0,0 +1,69 @@ +// Real-behavior proof: failed compaction must surface as CompactionError, +// report the actual provider failure, and leave the transcript unrotated. +import type { ExtensionContext } from "openclaw/plugin-sdk/agent-sessions"; +import type { UserMessage } from "openclaw/plugin-sdk/llm"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as agentSessions from "./sessions/index.js"; + +vi.mock("./sessions/index.js", async () => { + const actual = await vi.importActual("./sessions/index.js"); + return { + ...actual, + generateSummary: vi.fn(), + }; +}); + +const mockGenerateSummary = vi.mocked(agentSessions.generateSummary); + +const testModel = { + provider: "google-vertex", + model: "gemini-3.1-pro-preview", + contextWindow: 1_000_000, + contextTokens: 1_000_000, + maxTokens: 8192, +} as unknown as NonNullable; + +const { summarizeInStages } = await import("./compaction.js"); + +describe("compaction failure real-behavior proof", () => { + beforeEach(() => { + mockGenerateSummary.mockReset(); + }); + + it("throws CompactionError with the real provider error and keeps messages unrotated", async () => { + // Simulate the production failure from issue #115413: google-vertex ADC + // not configured causes the model call to throw. + mockGenerateSummary.mockRejectedValue( + new TypeError("Cannot convert undefined or null to object"), + ); + + const messages: UserMessage[] = [ + { role: "user", content: "first user request", timestamp: 1 } satisfies UserMessage, + { role: "user", content: "second user request", timestamp: 2 } satisfies UserMessage, + { role: "user", content: "third user request", timestamp: 3 } satisfies UserMessage, + ]; + + await expect( + summarizeInStages({ + messages, + model: testModel, + apiKey: "test-key", // pragma: allowlist secret + signal: new AbortController().signal, + reserveTokens: 1000, + maxChunkTokens: 50_000, + contextWindow: 1_000_000, + }), + ).rejects.toThrow( + "All summarization attempts failed for 3 messages. Last error: Cannot convert undefined or null to object", + ); + + // The caller must see the failure and keep the transcript intact. + // No compaction placeholder is returned, so messages are not rotated away. + expect(messages).toHaveLength(3); + expect(messages.map((message) => message.content)).toEqual([ + "first user request", + "second user request", + "third user request", + ]); + }); +}); diff --git a/src/agents/compaction.summarize-fallback.test.ts b/src/agents/compaction.summarize-fallback.test.ts index 5861cea690ba..54841c9aa72e 100644 --- a/src/agents/compaction.summarize-fallback.test.ts +++ b/src/agents/compaction.summarize-fallback.test.ts @@ -48,7 +48,7 @@ describe("summarizeWithFallback", () => { agentSessionMocks.estimateTokens.mockImplementation(() => 100); }); - it("does not duplicate summarization when no messages were oversized", async () => { + it("throws CompactionError when all summarization attempts fail", async () => { const messages: AgentMessage[] = [ { role: "user", @@ -57,18 +57,17 @@ describe("summarizeWithFallback", () => { } satisfies UserMessage, ]; - const result = await summarizeWithFallback({ - messages, - model: testModel, - apiKey: "test-key", // pragma: allowlist secret - signal: new AbortController().signal, - reserveTokens: 1000, - maxChunkTokens: 50_000, - contextWindow: 200_000, - }); - - expect(result).toContain("Context contained 1 messages"); - expect(result).toContain("0 oversized"); + await expect( + summarizeWithFallback({ + messages, + model: testModel, + apiKey: "test-key", // pragma: allowlist secret + signal: new AbortController().signal, + reserveTokens: 1000, + maxChunkTokens: 50_000, + contextWindow: 200_000, + }), + ).rejects.toThrow("All summarization attempts failed for 1 messages"); // "fetch failed" is timeout-classed now, so summarizeChunks does not retry it. expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(1); }); @@ -171,7 +170,7 @@ describe("summarizeWithFallback", () => { expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(1); }); - it("still attempts partial summarization when oversized messages were excluded", async () => { + it("throws CompactionError when both full and partial summarization fail", async () => { // Oversized-message fallback tries the safe subset so a huge attachment or // tool output does not prevent summarizing the rest of the transcript. agentSessionMocks.estimateTokens.mockImplementation((message: unknown) => { @@ -195,18 +194,27 @@ describe("summarizeWithFallback", () => { } satisfies UserMessage, ]; - const result = await summarizeWithFallback({ - messages, - model: testModel, - apiKey: "test-key", // pragma: allowlist secret - signal: new AbortController().signal, - reserveTokens: 1000, - maxChunkTokens: 50_000, - contextWindow: 200_000, + let callCount = 0; + agentSessionMocks.generateSummary.mockImplementation(() => { + callCount++; + if (callCount === 1) { + return Promise.reject(new Error("full summarization error")); + } + return Promise.reject(new Error("partial retry error")); }); - expect(result).toContain("2 messages (1 oversized)"); - // Full attempt plus distinct partial transcript; timeout-classed failures do not retry. - expect(agentSessionMocks.generateSummary.mock.calls.length).toBe(2); + await expect( + summarizeWithFallback({ + messages, + model: testModel, + apiKey: "test-key", // pragma: allowlist secret + signal: new AbortController().signal, + reserveTokens: 1000, + maxChunkTokens: 50_000, + contextWindow: 200_000, + }), + ).rejects.toThrow( + "All summarization attempts failed for 2 messages. Last error: partial retry error", + ); }); }); diff --git a/src/agents/compaction.ts b/src/agents/compaction.ts index 0767491b5067..270a711b4390 100644 --- a/src/agents/compaction.ts +++ b/src/agents/compaction.ts @@ -1,3 +1,4 @@ +import { CompactionError } from "../../packages/agent-core/src/harness/types.js"; /** * Summarization and fallback helpers for transcript compaction. */ @@ -46,9 +47,6 @@ type CompactionSummaryResult = | { kind: "generic-fallback"; text: string }; const DEFAULT_SUMMARY_FALLBACK = "No prior history."; -const MAX_CONSECUTIVE_GENERIC_FALLBACKS = 2; -const CIRCUIT_OPEN_ERROR = - "Compaction staged summarization stopped after repeated generic fallbacks"; const MERGE_SUMMARIES_INSTRUCTIONS = [ "Merge these partial summaries into a single cohesive summary.", "", @@ -268,14 +266,16 @@ async function summarizeWithFallbackResult(params: { // Try full summarization first let partialSummaryFallback: string | undefined; + let lastError: unknown; try { return { kind: "summary", text: await summarizeChunks(params) }; - } catch (fullError) { + } catch (err) { + lastError = err; if (params.signal.aborted) { - throw fullError; + throw lastError; } - log.warn(`Full summarization failed: ${formatErrorMessage(fullError)}`); - partialSummaryFallback = (fullError as PartialSummaryError).partialSummary; + log.warn(`Full summarization failed: ${formatErrorMessage(lastError)}`); + partialSummaryFallback = (lastError as PartialSummaryError).partialSummary; } // Fallback 1: Summarize only small messages, note oversized ones. @@ -296,14 +296,15 @@ async function summarizeWithFallbackResult(params: { const notes = oversizedNotes.length > 0 ? `\n\n${oversizedNotes.join("\n")}` : ""; return { kind: "summary", text: partialSummary + notes }; } catch (partialError) { + lastError = partialError; if (params.signal.aborted) { - throw partialError; + throw lastError; } - log.warn(`Partial summarization also failed: ${formatErrorMessage(partialError)}`); + log.warn(`Partial summarization also failed: ${formatErrorMessage(lastError)}`); // Prefer the oversized retry's partial summary over the full attempt's, // since it covers the non-oversized transcript. Append oversized notes // so the model knows large content was filtered. - const retryPartial = (partialError as PartialSummaryError).partialSummary; + const retryPartial = (lastError as PartialSummaryError).partialSummary; if (retryPartial) { const notes = oversizedNotes.length > 0 ? `\n\n${oversizedNotes.join("\n")}` : ""; partialSummaryFallback = retryPartial + notes; @@ -311,16 +312,20 @@ async function summarizeWithFallbackResult(params: { } } - // Final fallback: use best available partial summary, otherwise generic note + // Final fallback: use best available partial summary, otherwise throw error if (partialSummaryFallback) { return { kind: "summary", text: partialSummaryFallback }; } - return { - kind: "generic-fallback", - text: - `Context contained ${messages.length} messages (${oversizedNotes.length} oversized). ` + - `Summary unavailable due to size limits.`, - }; + + // All summarization attempts failed — throw error so caller knows compaction + // did not succeed. This prevents silent infinite retry loops where "Compaction + // complete" is reported but no tokens are reclaimed. + throw new CompactionError( + "summarization_failed", + `All summarization attempts failed for ${messages.length} messages. ` + + `Last error: ${lastError instanceof Error ? lastError.message : String(lastError)}`, + lastError instanceof Error ? lastError : undefined, + ); } async function summarizeWithFallback( @@ -393,35 +398,28 @@ export async function summarizeInStages(params: { } const partialSummaries: string[] = []; - let consecutiveGenericFallbacks = 0; - // Caller-owned leading context lives in the oldest split. Only losing that - // split requires restoration; later fallback placeholders remain in the merge. - let oldestChunkDegraded = false; for (const [index, chunk] of plan.chunks.entries()) { - const result = await summarizeWithFallbackResult({ - ...params, - messages: chunk, - previousSummary: undefined, - }); - consecutiveGenericFallbacks = - result.kind === "generic-fallback" ? consecutiveGenericFallbacks + 1 : 0; - if (index === 0) { - oldestChunkDegraded = result.kind === "generic-fallback"; - } - - // Keep one placeholder to mark the missing split, but stop before repeated - // placeholders trigger more split requests or a doomed merge request. - if (consecutiveGenericFallbacks >= MAX_CONSECUTIVE_GENERIC_FALLBACKS) { - log.warn("compaction staged summarization stopped after repeated generic fallbacks", { - attemptedSplits: index + 1, - consecutiveGenericFallbacks, - totalSplits: plan.chunks.length, + try { + const result = await summarizeWithFallbackResult({ + ...params, + messages: chunk, + previousSummary: undefined, }); - // The remaining chunks were never attempted. Abort the whole compaction - // so the caller keeps the source transcript instead of committing a gap. - throw new Error(CIRCUIT_OPEN_ERROR); + partialSummaries.push(result.text); + } catch (err) { + // A chunk summarization failed — fail the whole stages compaction. + // This prevents silent infinite retry loops where compaction reports + // success but no tokens are reclaimed. + if (err instanceof CompactionError) { + throw err; + } + // Wrap non-CompactionError failures for consistent error handling + throw new CompactionError( + "summarization_failed", + `Chunk ${index + 1} summarization failed: ${err instanceof Error ? err.message : String(err)}`, + err instanceof Error ? err : undefined, + ); } - partialSummaries.push(result.text); } if (partialSummaries.length === 1) { @@ -429,10 +427,7 @@ export async function summarizeInStages(params: { if (summary === undefined) { throw new Error("Compaction summary plan produced no summary"); } - return { - kind: oldestChunkDegraded ? "generic-fallback" : "summary", - text: summary, - }; + return { kind: "summary", text: summary }; } // Capture once so timestamps are strictly monotonic across @@ -471,9 +466,7 @@ export async function summarizeInStages(params: { messages: summaryMessages, customInstructions: mergeInstructions, }); - return oldestChunkDegraded && mergedResult.kind === "summary" - ? { kind: "generic-fallback", text: mergedResult.text } - : mergedResult; + return mergedResult; } /** Resolves a positive context-window token count from model metadata. */