mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-20 06:20:55 +00:00
fix(compaction): break safeguard cancel loop for sessions with no summarizable messages (#41981) (#42215)
Merged via squash.
Prepared head SHA: 7ce6bd834e
Co-authored-by: lml2468 <39320777+lml2468@users.noreply.github.com>
Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com>
Reviewed-by: @jalehman
This commit is contained in:
@@ -123,6 +123,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Gateway/exec approvals: surface requested env override keys in gateway-host approval prompts so operators can review surviving env context without inheriting noisy base host env.
|
||||
- Telegram/network: preserve sticky IPv4 fallback state across polling restarts so hosts with unstable IPv6 to `api.telegram.org` stop re-triggering repeated Telegram timeouts after each restart. (#48282) Thanks @yassinebkr.
|
||||
- Plugins/subagents: forward per-run provider and model overrides through gateway plugin subagent dispatch so plugin-launched agent delegations honor explicit model selection again. (#48277) Thanks @jalehman.
|
||||
- Agents/compaction: write minimal boundary summaries for empty preparations while keeping split-turn prefixes on the normal path, so no-summarizable-message sessions stop retriggering the safeguard loop. (#42215) thanks @lml2468.
|
||||
|
||||
### Fixes
|
||||
|
||||
|
||||
@@ -389,7 +389,6 @@ describe("preflightDiscordMessage", () => {
|
||||
id: "m-webhook-hydrated-1",
|
||||
channelId: threadId,
|
||||
content: "",
|
||||
webhookId: undefined,
|
||||
author: {
|
||||
id: "relay-bot-1",
|
||||
bot: true,
|
||||
|
||||
@@ -138,10 +138,31 @@ async function runCompactionScenario(params: {
|
||||
});
|
||||
const result = (await compactionHandler(params.event, mockContext)) as {
|
||||
cancel?: boolean;
|
||||
compaction?: {
|
||||
summary: string;
|
||||
firstKeptEntryId: string;
|
||||
tokensBefore: number;
|
||||
};
|
||||
};
|
||||
return { result, getApiKeyMock };
|
||||
}
|
||||
|
||||
function expectCompactionResult(result: {
|
||||
cancel?: boolean;
|
||||
compaction?: {
|
||||
summary: string;
|
||||
firstKeptEntryId: string;
|
||||
tokensBefore: number;
|
||||
};
|
||||
}) {
|
||||
expect(result.cancel).not.toBe(true);
|
||||
expect(result.compaction).toBeDefined();
|
||||
if (!result.compaction) {
|
||||
throw new Error("Expected compaction result");
|
||||
}
|
||||
return result.compaction;
|
||||
}
|
||||
|
||||
describe("compaction-safeguard tool failures", () => {
|
||||
it("formats tool failures with meta and summary", () => {
|
||||
const messages: AgentMessage[] = [
|
||||
@@ -1524,10 +1545,117 @@ describe("compaction-safeguard double-compaction guard", () => {
|
||||
event: mockEvent,
|
||||
apiKey: "sk-test", // pragma: allowlist secret
|
||||
});
|
||||
expect(result).toEqual({ cancel: true });
|
||||
const compaction = expectCompactionResult(result);
|
||||
// After fix for #41981: returns a compaction result (not cancel) to write
|
||||
// a boundary entry and break the re-trigger loop.
|
||||
// buildStructuredFallbackSummary(undefined) produces a minimal structured summary
|
||||
expect(compaction.summary).toContain("## Decisions");
|
||||
expect(compaction.summary).toContain("No prior history.");
|
||||
expect(compaction.summary).toContain("## Open TODOs");
|
||||
expect(compaction.firstKeptEntryId).toBe("entry-1");
|
||||
expect(compaction.tokensBefore).toBe(1500);
|
||||
expect(getApiKeyMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns compaction result with structured fallback summary sections", async () => {
|
||||
const sessionManager = stubSessionManager();
|
||||
const model = createAnthropicModelFixture();
|
||||
setCompactionSafeguardRuntime(sessionManager, { model });
|
||||
|
||||
const mockEvent = {
|
||||
preparation: {
|
||||
messagesToSummarize: [] as AgentMessage[],
|
||||
turnPrefixMessages: [] as AgentMessage[],
|
||||
firstKeptEntryId: "entry-2",
|
||||
tokensBefore: 2000,
|
||||
previousSummary: "## Decisions\nUsed approach A.",
|
||||
fileOps: { read: [], edited: [], written: [] },
|
||||
settings: { reserveTokens: 16384 },
|
||||
},
|
||||
customInstructions: "",
|
||||
signal: new AbortController().signal,
|
||||
};
|
||||
const { result } = await runCompactionScenario({
|
||||
sessionManager,
|
||||
event: mockEvent,
|
||||
apiKey: "sk-test", // pragma: allowlist secret
|
||||
});
|
||||
const compaction = expectCompactionResult(result);
|
||||
// Fallback preserves previous summary when it has required sections
|
||||
expect(compaction.summary).toContain("## Decisions");
|
||||
expect(compaction.summary).toContain("## Open TODOs");
|
||||
expect(compaction.firstKeptEntryId).toBe("entry-2");
|
||||
});
|
||||
|
||||
it("writes boundary again on repeated empty preparation (no cancel loop after new assistant message)", async () => {
|
||||
const sessionManager = stubSessionManager();
|
||||
const model = createAnthropicModelFixture();
|
||||
setCompactionSafeguardRuntime(sessionManager, { model });
|
||||
|
||||
const mockEvent = {
|
||||
preparation: {
|
||||
messagesToSummarize: [] as AgentMessage[],
|
||||
turnPrefixMessages: [] as AgentMessage[],
|
||||
firstKeptEntryId: "entry-3",
|
||||
tokensBefore: 1000,
|
||||
fileOps: { read: [], edited: [], written: [] },
|
||||
},
|
||||
customInstructions: "",
|
||||
signal: new AbortController().signal,
|
||||
};
|
||||
|
||||
// First call — writes boundary
|
||||
const { result: result1 } = await runCompactionScenario({
|
||||
sessionManager,
|
||||
event: mockEvent,
|
||||
apiKey: "sk-test", // pragma: allowlist secret
|
||||
});
|
||||
const compaction1 = expectCompactionResult(result1);
|
||||
expect(compaction1.summary).toContain("## Decisions");
|
||||
|
||||
// Simulate: after the boundary, a new assistant message arrives, SDK
|
||||
// triggers compaction again with another empty preparation. The safeguard
|
||||
// must write another boundary (not cancel) to avoid re-entering the
|
||||
// cancel loop described in the maintainer review.
|
||||
const { result: result2 } = await runCompactionScenario({
|
||||
sessionManager,
|
||||
event: mockEvent,
|
||||
apiKey: "sk-test", // pragma: allowlist secret
|
||||
});
|
||||
const compaction2 = expectCompactionResult(result2);
|
||||
expect(compaction2.summary).toContain("## Decisions");
|
||||
expect(compaction2.firstKeptEntryId).toBe("entry-3");
|
||||
});
|
||||
|
||||
it("does not write boundary when turnPrefixMessages has real content (split-turn)", async () => {
|
||||
const sessionManager = stubSessionManager();
|
||||
const model = createAnthropicModelFixture();
|
||||
setCompactionSafeguardRuntime(sessionManager, { model });
|
||||
|
||||
const mockEvent = {
|
||||
preparation: {
|
||||
messagesToSummarize: [] as AgentMessage[],
|
||||
turnPrefixMessages: [
|
||||
{ role: "user" as const, content: "real turn prefix content" },
|
||||
] as AgentMessage[],
|
||||
firstKeptEntryId: "entry-4",
|
||||
tokensBefore: 2000,
|
||||
fileOps: { read: [], edited: [], written: [] },
|
||||
isSplitTurn: true,
|
||||
},
|
||||
customInstructions: "",
|
||||
signal: new AbortController().signal,
|
||||
};
|
||||
const { result } = await runCompactionScenario({
|
||||
sessionManager,
|
||||
event: mockEvent,
|
||||
apiKey: null,
|
||||
});
|
||||
// Should NOT take the boundary fast-path — falls through to normal compaction
|
||||
// (which cancels due to no API key, but that's the expected normal path)
|
||||
expect(result).toEqual({ cancel: true });
|
||||
});
|
||||
|
||||
it("continues when messages include real conversation content", async () => {
|
||||
const sessionManager = stubSessionManager();
|
||||
const model = createAnthropicModelFixture();
|
||||
|
||||
@@ -702,11 +702,32 @@ async function readWorkspaceContextForSummary(): Promise<string> {
|
||||
export default function compactionSafeguardExtension(api: ExtensionAPI): void {
|
||||
api.on("session_before_compact", async (event, ctx) => {
|
||||
const { preparation, customInstructions: eventInstructions, signal } = event;
|
||||
if (!preparation.messagesToSummarize.some(isRealConversationMessage)) {
|
||||
log.warn(
|
||||
"Compaction safeguard: cancelling compaction with no real conversation messages to summarize.",
|
||||
const hasRealSummarizable = preparation.messagesToSummarize.some(isRealConversationMessage);
|
||||
const hasRealTurnPrefix = preparation.turnPrefixMessages.some(isRealConversationMessage);
|
||||
if (!hasRealSummarizable && !hasRealTurnPrefix) {
|
||||
// When there are no summarizable messages AND no real turn-prefix content,
|
||||
// cancelling compaction leaves context unchanged but the SDK re-triggers
|
||||
// _checkCompaction after every assistant response — creating a cancel loop
|
||||
// that blocks cron lanes (#41981).
|
||||
//
|
||||
// Strategy: always return a minimal compaction result so the SDK writes a
|
||||
// boundary entry. The SDK's prepareCompaction() returns undefined when the
|
||||
// last entry is a compaction, which blocks immediate re-triggering within
|
||||
// the same turn. After a new assistant message arrives, if the SDK triggers
|
||||
// compaction again with an empty preparation, we write another boundary —
|
||||
// this is bounded to at most one boundary per LLM round-trip, not a tight
|
||||
// loop.
|
||||
log.info(
|
||||
"Compaction safeguard: no real conversation messages to summarize; writing compaction boundary to suppress re-trigger loop.",
|
||||
);
|
||||
return { cancel: true };
|
||||
const fallbackSummary = buildStructuredFallbackSummary(preparation.previousSummary);
|
||||
return {
|
||||
compaction: {
|
||||
summary: fallbackSummary,
|
||||
firstKeptEntryId: preparation.firstKeptEntryId,
|
||||
tokensBefore: preparation.tokensBefore,
|
||||
},
|
||||
};
|
||||
}
|
||||
const { readFiles, modifiedFiles } = computeFileLists(preparation.fileOps);
|
||||
const fileOpsSummary = formatFileOperations(readFiles, modifiedFiles);
|
||||
|
||||
Reference in New Issue
Block a user