From ea47ab29bd6d92394185636a27c3572c19aac8e5 Mon Sep 17 00:00:00 2001 From: DukeDeSouth <51200688+DukeDeSouth@users.noreply.github.com> Date: Mon, 23 Feb 2026 10:23:13 -0500 Subject: [PATCH] fix: cancel compaction instead of truncating history when summarization fails (#10711) * fix: cancel compaction instead of truncating history when summarization fails When the compaction safeguard cannot generate a summary (no model, no API key, or LLM error), it previously returned a "Summary unavailable" fallback string and still truncated history. This caused irreversible data loss - older messages were discarded even though no meaningful summary was produced. Now returns `{ cancel: true }` in all three failure paths so the framework aborts compaction entirely and preserves the full conversation history. Fixes #10332 Co-authored-by: Cursor * fix: use deterministic timestamps in compaction safeguard tests Replace Date.now() with fixed timestamp (0) in test data to prevent nondeterministic behavior in snapshot-based or order-dependent tests. Co-authored-by: Cursor * Changelog: note compaction cancellation safeguard fix --------- Co-authored-by: Cursor Co-authored-by: Vincent Koc --- CHANGELOG.md | 1 + .../compaction-safeguard.test.ts | 22 ++++-------- .../pi-extensions/compaction-safeguard.ts | 35 ++++--------------- 3 files changed, 14 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8598d8a90..24316eb86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Docs: https://docs.openclaw.ai - Config/Write: apply `unsetPaths` with immutable path-copy updates so config writes never mutate caller-provided objects, and harden `openclaw config get/set/unset` path traversal by rejecting prototype-key segments and inherited-property traversal. (#24134) thanks @frankekn. - Agents/Compaction: pass model metadata through the embedded runtime so safeguard summarization can run when `ctx.model` is unavailable, avoiding repeated `"Summary unavailable due to context limits"` fallback summaries. (#3479) Thanks @battman21, @hanxiao and @vincentkoc. - Agents/Overflow: detect additional provider context-overflow error shapes (including `input length` + `max_tokens` exceed-context variants) so failures route through compaction/recovery paths instead of leaking raw provider errors to users. (#9951) Thanks @echoVic and @Glucksberg. +- Agents/Compaction: cancel safeguard compaction when summary generation cannot run (missing model/API key or summarization failure), preserving history instead of truncating to fallback `"Summary unavailable"` text. (#10711) Thanks @DukeDeSouth and @vincentkoc. - Agents/Failover: treat HTTP 502/503/504 errors as failover-eligible transient timeouts so fallback chains can switch providers/models during upstream outages instead of retrying the same failing target. (#20999) Thanks @taw0002 and @vincentkoc. ## 2026.2.23 diff --git a/src/agents/pi-extensions/compaction-safeguard.test.ts b/src/agents/pi-extensions/compaction-safeguard.test.ts index b5e4915d0..e0033b0bb 100644 --- a/src/agents/pi-extensions/compaction-safeguard.test.ts +++ b/src/agents/pi-extensions/compaction-safeguard.test.ts @@ -388,22 +388,17 @@ describe("compaction-safeguard extension model fallback", () => { model: undefined, // ctx.model is undefined (simulates compact.ts workflow) sessionManager, modelRegistry: { - getApiKey: getApiKeyMock, // No API key, should use fallback + getApiKey: getApiKeyMock, // No API key should now cancel compaction }, } as unknown as Partial; // Call the handler and wait for result // oxlint-disable-next-line typescript/no-non-null-assertion const result = (await compactionHandler!(mockEvent, mockContext)) as { - compaction?: { summary?: string; firstKeptEntryId?: string }; + cancel?: boolean; }; - const compactionResult = result?.compaction; - // Verify that compaction returned a result (even with null API key, should use fallback) - expect(compactionResult).toBeDefined(); - expect(compactionResult?.summary).toBeDefined(); - expect(compactionResult?.summary).toContain("Summary unavailable"); - expect(compactionResult?.firstKeptEntryId).toBe("entry-1"); + expect(result).toEqual({ cancel: true }); // KEY ASSERTION: Prove the fallback path was exercised // The handler should have called getApiKey with runtime.model (via ctx.model ?? runtime?.model) @@ -414,7 +409,7 @@ describe("compaction-safeguard extension model fallback", () => { expect(retrieved?.model).toEqual(model); }); - it("returns fallback summary when both ctx.model and runtime.model are undefined", async () => { + it("cancels compaction when both ctx.model and runtime.model are undefined", async () => { const sessionManager = stubSessionManager(); // Do NOT set runtime.model (both ctx.model and runtime.model will be undefined) @@ -464,15 +459,10 @@ describe("compaction-safeguard extension model fallback", () => { // oxlint-disable-next-line typescript/no-non-null-assertion const result = (await compactionHandler!(mockEvent, mockContext)) as { - compaction?: { summary?: string; firstKeptEntryId?: string }; + cancel?: boolean; }; - const compactionResult = result?.compaction; - expect(compactionResult).toBeDefined(); - expect(compactionResult?.summary).toBe( - "Summary unavailable due to context limits. Older messages were truncated.", - ); - expect(compactionResult?.firstKeptEntryId).toBe("entry-1"); + expect(result).toEqual({ cancel: true }); // Verify early return: getApiKey should NOT have been called when both models are missing expect(getApiKeyMock).not.toHaveBeenCalled(); diff --git a/src/agents/pi-extensions/compaction-safeguard.ts b/src/agents/pi-extensions/compaction-safeguard.ts index a728a4a72..b7c15d503 100644 --- a/src/agents/pi-extensions/compaction-safeguard.ts +++ b/src/agents/pi-extensions/compaction-safeguard.ts @@ -20,8 +20,6 @@ import { collectTextContentBlocks } from "../content-blocks.js"; import { getCompactionSafeguardRuntime } from "./compaction-safeguard-runtime.js"; const log = createSubsystemLogger("compaction-safeguard"); -const FALLBACK_SUMMARY = - "Summary unavailable due to context limits. Older messages were truncated."; // Track session managers that have already logged the missing-model warning to avoid log spam. const missedModelWarningSessions = new WeakSet(); @@ -200,7 +198,6 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void { ...preparation.turnPrefixMessages, ]); const toolFailureSection = formatToolFailuresSection(toolFailures); - const fallbackSummary = `${FALLBACK_SUMMARY}${toolFailureSection}${fileOpsSummary}`; // Model resolution: ctx.model is undefined in compact.ts workflow (extensionRunner.initialize() is never called). // Fall back to runtime.model which is explicitly passed when building extension paths. @@ -217,26 +214,15 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void { "was not called and model was not passed through runtime registry.", ); } - return { - compaction: { - summary: fallbackSummary, - firstKeptEntryId: preparation.firstKeptEntryId, - tokensBefore: preparation.tokensBefore, - details: { readFiles, modifiedFiles }, - }, - }; + return { cancel: true }; } const apiKey = await ctx.modelRegistry.getApiKey(model); if (!apiKey) { - return { - compaction: { - summary: fallbackSummary, - firstKeptEntryId: preparation.firstKeptEntryId, - tokensBefore: preparation.tokensBefore, - details: { readFiles, modifiedFiles }, - }, - }; + console.warn( + "Compaction safeguard: no API key available; cancelling compaction to preserve history.", + ); + return { cancel: true }; } try { @@ -375,18 +361,11 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void { }; } catch (error) { log.warn( - `Compaction summarization failed; truncating history: ${ + `Compaction summarization failed; cancelling compaction to preserve history: ${ error instanceof Error ? error.message : String(error) }`, ); - return { - compaction: { - summary: fallbackSummary, - firstKeptEntryId: preparation.firstKeptEntryId, - tokensBefore: preparation.tokensBefore, - details: { readFiles, modifiedFiles }, - }, - }; + return { cancel: true }; } }); }