mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 07:01:24 +00:00
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 <cursoragent@cursor.com>
* 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 <cursoragent@cursor.com>
* Changelog: note compaction cancellation safeguard fix
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -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.
|
- 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/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/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.
|
- 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
|
## 2026.2.23
|
||||||
|
|||||||
@@ -388,22 +388,17 @@ describe("compaction-safeguard extension model fallback", () => {
|
|||||||
model: undefined, // ctx.model is undefined (simulates compact.ts workflow)
|
model: undefined, // ctx.model is undefined (simulates compact.ts workflow)
|
||||||
sessionManager,
|
sessionManager,
|
||||||
modelRegistry: {
|
modelRegistry: {
|
||||||
getApiKey: getApiKeyMock, // No API key, should use fallback
|
getApiKey: getApiKeyMock, // No API key should now cancel compaction
|
||||||
},
|
},
|
||||||
} as unknown as Partial<ExtensionContext>;
|
} as unknown as Partial<ExtensionContext>;
|
||||||
|
|
||||||
// Call the handler and wait for result
|
// Call the handler and wait for result
|
||||||
// oxlint-disable-next-line typescript/no-non-null-assertion
|
// oxlint-disable-next-line typescript/no-non-null-assertion
|
||||||
const result = (await compactionHandler!(mockEvent, mockContext)) as {
|
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(result).toEqual({ cancel: true });
|
||||||
expect(compactionResult).toBeDefined();
|
|
||||||
expect(compactionResult?.summary).toBeDefined();
|
|
||||||
expect(compactionResult?.summary).toContain("Summary unavailable");
|
|
||||||
expect(compactionResult?.firstKeptEntryId).toBe("entry-1");
|
|
||||||
|
|
||||||
// KEY ASSERTION: Prove the fallback path was exercised
|
// KEY ASSERTION: Prove the fallback path was exercised
|
||||||
// The handler should have called getApiKey with runtime.model (via ctx.model ?? runtime?.model)
|
// 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);
|
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();
|
const sessionManager = stubSessionManager();
|
||||||
|
|
||||||
// Do NOT set runtime.model (both ctx.model and runtime.model will be undefined)
|
// 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
|
// oxlint-disable-next-line typescript/no-non-null-assertion
|
||||||
const result = (await compactionHandler!(mockEvent, mockContext)) as {
|
const result = (await compactionHandler!(mockEvent, mockContext)) as {
|
||||||
compaction?: { summary?: string; firstKeptEntryId?: string };
|
cancel?: boolean;
|
||||||
};
|
};
|
||||||
const compactionResult = result?.compaction;
|
|
||||||
|
|
||||||
expect(compactionResult).toBeDefined();
|
expect(result).toEqual({ cancel: true });
|
||||||
expect(compactionResult?.summary).toBe(
|
|
||||||
"Summary unavailable due to context limits. Older messages were truncated.",
|
|
||||||
);
|
|
||||||
expect(compactionResult?.firstKeptEntryId).toBe("entry-1");
|
|
||||||
|
|
||||||
// Verify early return: getApiKey should NOT have been called when both models are missing
|
// Verify early return: getApiKey should NOT have been called when both models are missing
|
||||||
expect(getApiKeyMock).not.toHaveBeenCalled();
|
expect(getApiKeyMock).not.toHaveBeenCalled();
|
||||||
|
|||||||
@@ -20,8 +20,6 @@ import { collectTextContentBlocks } from "../content-blocks.js";
|
|||||||
import { getCompactionSafeguardRuntime } from "./compaction-safeguard-runtime.js";
|
import { getCompactionSafeguardRuntime } from "./compaction-safeguard-runtime.js";
|
||||||
|
|
||||||
const log = createSubsystemLogger("compaction-safeguard");
|
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.
|
// Track session managers that have already logged the missing-model warning to avoid log spam.
|
||||||
const missedModelWarningSessions = new WeakSet<object>();
|
const missedModelWarningSessions = new WeakSet<object>();
|
||||||
@@ -200,7 +198,6 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
|
|||||||
...preparation.turnPrefixMessages,
|
...preparation.turnPrefixMessages,
|
||||||
]);
|
]);
|
||||||
const toolFailureSection = formatToolFailuresSection(toolFailures);
|
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).
|
// 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.
|
// 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.",
|
"was not called and model was not passed through runtime registry.",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
return {
|
return { cancel: true };
|
||||||
compaction: {
|
|
||||||
summary: fallbackSummary,
|
|
||||||
firstKeptEntryId: preparation.firstKeptEntryId,
|
|
||||||
tokensBefore: preparation.tokensBefore,
|
|
||||||
details: { readFiles, modifiedFiles },
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const apiKey = await ctx.modelRegistry.getApiKey(model);
|
const apiKey = await ctx.modelRegistry.getApiKey(model);
|
||||||
if (!apiKey) {
|
if (!apiKey) {
|
||||||
return {
|
console.warn(
|
||||||
compaction: {
|
"Compaction safeguard: no API key available; cancelling compaction to preserve history.",
|
||||||
summary: fallbackSummary,
|
);
|
||||||
firstKeptEntryId: preparation.firstKeptEntryId,
|
return { cancel: true };
|
||||||
tokensBefore: preparation.tokensBefore,
|
|
||||||
details: { readFiles, modifiedFiles },
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
@@ -375,18 +361,11 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
|
|||||||
};
|
};
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
log.warn(
|
log.warn(
|
||||||
`Compaction summarization failed; truncating history: ${
|
`Compaction summarization failed; cancelling compaction to preserve history: ${
|
||||||
error instanceof Error ? error.message : String(error)
|
error instanceof Error ? error.message : String(error)
|
||||||
}`,
|
}`,
|
||||||
);
|
);
|
||||||
return {
|
return { cancel: true };
|
||||||
compaction: {
|
|
||||||
summary: fallbackSummary,
|
|
||||||
firstKeptEntryId: preparation.firstKeptEntryId,
|
|
||||||
tokensBefore: preparation.tokensBefore,
|
|
||||||
details: { readFiles, modifiedFiles },
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user