mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 04:47:03 -06:00
fix: redact compaction rejection diagnostics
This commit is contained in:
@@ -17,6 +17,7 @@ import { buildEmbeddedExtensionFactories } from "../embedded-agent-runner/extens
|
||||
import { castAgentMessage } from "../test-helpers/agent-message-fixtures.js";
|
||||
import { jsonResult } from "../tools/common.js";
|
||||
import { MAX_WORKSPACE_BOOTSTRAP_FILE_BYTES } from "../workspace-bootstrap-read.js";
|
||||
import * as compactionQualityModule from "./compaction-safeguard-quality.js";
|
||||
import {
|
||||
consumeCompactionSafeguardCancelReason,
|
||||
getCompactionSafeguardRuntime,
|
||||
@@ -26,6 +27,37 @@ import {
|
||||
import compactionSafeguardExtension from "./compaction-safeguard.js";
|
||||
import { testing } from "./compaction-safeguard.test-support.js";
|
||||
|
||||
const { compactionLogger } = vi.hoisted(() => {
|
||||
const logger = {
|
||||
subsystem: "compaction-safeguard",
|
||||
isEnabled: vi.fn(() => false),
|
||||
trace: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
fatal: vi.fn(),
|
||||
raw: vi.fn(),
|
||||
child: vi.fn(),
|
||||
};
|
||||
logger.child.mockReturnValue(logger);
|
||||
return { compactionLogger: logger };
|
||||
});
|
||||
|
||||
vi.mock("../../logging/subsystem.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("../../logging/subsystem.js")>(
|
||||
"../../logging/subsystem.js",
|
||||
);
|
||||
return { ...actual, createSubsystemLogger: () => compactionLogger };
|
||||
});
|
||||
|
||||
vi.mock("./compaction-safeguard-quality.js", async () => {
|
||||
const actual = await vi.importActual<typeof compactionQualityModule>(
|
||||
"./compaction-safeguard-quality.js",
|
||||
);
|
||||
return { ...actual, auditSummaryQuality: vi.fn(actual.auditSummaryQuality) };
|
||||
});
|
||||
|
||||
vi.mock("../compaction.js", async () => {
|
||||
const actual = await vi.importActual<typeof compactionModule>("../compaction.js");
|
||||
return {
|
||||
@@ -36,6 +68,10 @@ vi.mock("../compaction.js", async () => {
|
||||
|
||||
const mockSummarizeInStages = vi.mocked(compactionModule.summarizeInStages);
|
||||
const actualCompactionModule = await vi.importActual<typeof compactionModule>("../compaction.js");
|
||||
const actualCompactionQualityModule = await vi.importActual<typeof compactionQualityModule>(
|
||||
"./compaction-safeguard-quality.js",
|
||||
);
|
||||
const mockAuditSummaryQuality = vi.mocked(compactionQualityModule.auditSummaryQuality);
|
||||
|
||||
function summaryResult(text: string) {
|
||||
return { kind: "summary" as const, text };
|
||||
@@ -70,6 +106,9 @@ const {
|
||||
|
||||
beforeEach(() => {
|
||||
testing.setSummarizeInStagesForTest(mockSummarizeInStages);
|
||||
mockAuditSummaryQuality.mockImplementation(actualCompactionQualityModule.auditSummaryQuality);
|
||||
mockAuditSummaryQuality.mockClear();
|
||||
compactionLogger.warn.mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -2078,9 +2117,45 @@ describe("compaction-safeguard recent-turn preservation", () => {
|
||||
|
||||
expect(result).toEqual({ cancel: true });
|
||||
expect(mockSummarizeInStages).not.toHaveBeenCalled();
|
||||
expect(mockAuditSummaryQuality).toHaveBeenCalledTimes(1);
|
||||
const auditInput = requireRecord(mockCallArg(mockAuditSummaryQuality));
|
||||
expect(auditInput.latestAsk).toBe(sourceText);
|
||||
expect(auditInput.identifiers).toEqual([identifier]);
|
||||
expect(auditInput.summary).toBe(
|
||||
[
|
||||
"## Decisions",
|
||||
"No prior history.",
|
||||
"",
|
||||
"## Open TODOs",
|
||||
"None.",
|
||||
"",
|
||||
"## Constraints/Rules",
|
||||
"None.",
|
||||
"",
|
||||
"## Pending user asks",
|
||||
"None.",
|
||||
"",
|
||||
"## Exact identifiers",
|
||||
"None captured.",
|
||||
"",
|
||||
"## Recent turns preserved verbatim",
|
||||
`- User: ${"x".repeat(600)}...`,
|
||||
].join("\n"),
|
||||
);
|
||||
expect(mockAuditSummaryQuality.mock.results[0]?.value).toEqual({
|
||||
ok: false,
|
||||
reasons: [`missing_identifiers:${identifier}`, "latest_user_ask_not_reflected"],
|
||||
});
|
||||
expect(consumeCompactionSafeguardCancelReason(sessionManager)).toBe(
|
||||
"Compaction safeguard finalized summary failed quality checks.",
|
||||
);
|
||||
const terminalWarnings = compactionLogger.warn.mock.calls.flat().join("\n");
|
||||
expect(terminalWarnings).toContain(
|
||||
"reasonCodes=missing_identifiers,latest_user_ask_not_reflected",
|
||||
);
|
||||
expect(terminalWarnings).toContain("reasonCount=2");
|
||||
expect(terminalWarnings).not.toContain(identifier);
|
||||
expect(terminalWarnings).not.toContain(sourceText);
|
||||
});
|
||||
|
||||
it("retries when generated summary misses headings even if preserved turns contain them", async () => {
|
||||
|
||||
@@ -1214,8 +1214,12 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
|
||||
}
|
||||
lastAuditReasons = quality.reasons;
|
||||
if (!canRegenerate || attempt >= totalAttempts - 1) {
|
||||
const reasonCodes = [
|
||||
...new Set(quality.reasons.map((reason) => reason.split(":", 1)[0])),
|
||||
];
|
||||
log.warn(
|
||||
`Compaction safeguard: finalized summary failed quality checks: ${quality.reasons.join(", ")}`,
|
||||
"Compaction safeguard: finalized summary failed quality checks; " +
|
||||
`reasonCodes=${reasonCodes.join(",")} reasonCount=${quality.reasons.length}`,
|
||||
);
|
||||
setCompactionSafeguardCancelReason(
|
||||
ctx.sessionManager,
|
||||
|
||||
@@ -1,10 +1,19 @@
|
||||
import path from "node:path";
|
||||
import {
|
||||
createAssistantMessageEventStream,
|
||||
type Context,
|
||||
type Model,
|
||||
} from "openclaw/plugin-sdk/llm";
|
||||
import { Type } from "typebox";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../../test/helpers/temp-dir.js";
|
||||
import {
|
||||
appendTranscriptMessage,
|
||||
loadTranscriptEvents,
|
||||
upsertSessionEntryCore,
|
||||
} from "../../config/sessions/session-accessor.js";
|
||||
import { resolveSqliteTargetFromSessionStorePath } from "../../config/sessions/session-sqlite-target.js";
|
||||
import { closeOpenClawAgentDatabaseByPath } from "../../state/openclaw-agent-db.js";
|
||||
import { steerActiveSessionWithOptionalDeliveryWait } from "../embedded-agent-runner/run/attempt-queue-message.js";
|
||||
import { agentSessionAutomaticCompaction } from "./agent-session-compaction.js";
|
||||
import {
|
||||
@@ -30,6 +39,7 @@ import { SettingsManager } from "./settings-manager.js";
|
||||
import { getSteeringMessageIdentity } from "./steering-message-identity.js";
|
||||
|
||||
registerAgentSessionLoopTestLifecycle();
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
describe("AgentSession loop correctness", () => {
|
||||
it("publishes a queued user message only after its transcript entry is committed", async () => {
|
||||
@@ -361,10 +371,24 @@ describe("AgentSession loop correctness", () => {
|
||||
});
|
||||
|
||||
it("does not append when a compaction extension rejects the finalized summary", async () => {
|
||||
const sessionManager = SessionManager.inMemory();
|
||||
appendHistory(
|
||||
sessionManager,
|
||||
createAssistant(testModel, [{ type: "text", text: "authoritative history" }]),
|
||||
const dir = tempDirs.make("openclaw-rejected-compaction-");
|
||||
const target = {
|
||||
agentId: "main",
|
||||
sessionId: "rejected-compaction-reopen",
|
||||
sessionKey: "agent:main:rejected-compaction-reopen",
|
||||
storePath: path.join(dir, "sessions.json"),
|
||||
};
|
||||
await upsertSessionEntryCore(target, {
|
||||
sessionId: target.sessionId,
|
||||
updatedAt: 1,
|
||||
});
|
||||
await appendTranscriptMessage(target, {
|
||||
cwd: dir,
|
||||
message: { role: "user", content: "authoritative question", timestamp: 1 },
|
||||
});
|
||||
const sessionManager = SessionManager.open(target, dir);
|
||||
sessionManager.appendMessage(
|
||||
createAssistant(testModel, [{ type: "text", text: "authoritative answer" }]),
|
||||
);
|
||||
const handlers = new Map<string, Array<(...args: unknown[]) => Promise<unknown>>>([
|
||||
["session_before_compact", [async () => ({ cancel: true })]],
|
||||
@@ -373,12 +397,30 @@ describe("AgentSession loop correctness", () => {
|
||||
sessionManager,
|
||||
resourceLoader: createResourceLoader(handlers),
|
||||
});
|
||||
const before = sessionManager.getBranch();
|
||||
const persistedBefore = await loadTranscriptEvents(target);
|
||||
const contextBefore = sessionManager.buildSessionContext();
|
||||
|
||||
await expect(session.compact()).rejects.toThrow("Compaction cancelled");
|
||||
|
||||
expect(sessionManager.getBranch()).toEqual(before);
|
||||
expect(sessionManager.getBranch().some((entry) => entry.type === "compaction")).toBe(false);
|
||||
sessionManager.flushPendingPersistence();
|
||||
const persistedAfterRejection = await loadTranscriptEvents(target);
|
||||
expect(JSON.stringify(persistedAfterRejection)).toBe(JSON.stringify(persistedBefore));
|
||||
expect(
|
||||
persistedAfterRejection.some(
|
||||
(entry) =>
|
||||
typeof entry === "object" &&
|
||||
entry !== null &&
|
||||
"type" in entry &&
|
||||
entry.type === "compaction",
|
||||
),
|
||||
).toBe(false);
|
||||
|
||||
const databasePath = resolveSqliteTargetFromSessionStorePath(target.storePath).path;
|
||||
expect(closeOpenClawAgentDatabaseByPath(databasePath)).toBe(true);
|
||||
const reopened = SessionManager.open(target, dir);
|
||||
expect(reopened.getBranch()).toEqual(persistedBefore.slice(1));
|
||||
expect(reopened.getBranch().some((entry) => entry.type === "compaction")).toBe(false);
|
||||
expect(reopened.buildSessionContext()).toEqual(contextBefore);
|
||||
});
|
||||
|
||||
it("keeps a successful high-usage response and performs threshold maintenance without retry", async () => {
|
||||
|
||||
Reference in New Issue
Block a user