Prevent stale pre-compaction assistant usage from retriggering auto-compaction after session recovery payloads and prove it with a regression test
This commit is contained in:
committed by
Mario Zechner
parent
58f8fcd8f1
commit
a4f4d91fa5
@@ -1696,17 +1696,18 @@ export class AgentSession {
|
|||||||
const sameModel =
|
const sameModel =
|
||||||
this.model && assistantMessage.provider === this.model.provider && assistantMessage.model === this.model.id;
|
this.model && assistantMessage.provider === this.model.provider && assistantMessage.model === this.model.id;
|
||||||
|
|
||||||
// Skip overflow check if the error is from before a compaction in the current path.
|
// Skip compaction checks if this assistant message is older than the latest
|
||||||
// This handles the case where an error was kept after compaction (in the "kept" region).
|
// compaction boundary. This prevents a stale pre-compaction usage/error
|
||||||
// The error shouldn't trigger another compaction since we already compacted.
|
// from retriggering compaction on the first prompt after compaction.
|
||||||
// Example: opus fails → switch to codex → compact → switch back to opus → opus error
|
|
||||||
// is still in context but shouldn't trigger compaction again.
|
|
||||||
const compactionEntry = getLatestCompactionEntry(this.sessionManager.getBranch());
|
const compactionEntry = getLatestCompactionEntry(this.sessionManager.getBranch());
|
||||||
const errorIsFromBeforeCompaction =
|
const assistantIsFromBeforeCompaction =
|
||||||
compactionEntry !== null && assistantMessage.timestamp < new Date(compactionEntry.timestamp).getTime();
|
compactionEntry !== null && assistantMessage.timestamp <= new Date(compactionEntry.timestamp).getTime();
|
||||||
|
if (assistantIsFromBeforeCompaction) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Case 1: Overflow - LLM returned context overflow error
|
// Case 1: Overflow - LLM returned context overflow error
|
||||||
if (sameModel && !errorIsFromBeforeCompaction && isContextOverflow(assistantMessage, contextWindow)) {
|
if (sameModel && isContextOverflow(assistantMessage, contextWindow)) {
|
||||||
if (this._overflowRecoveryAttempted) {
|
if (this._overflowRecoveryAttempted) {
|
||||||
this._emit({
|
this._emit({
|
||||||
type: "auto_compaction_end",
|
type: "auto_compaction_end",
|
||||||
|
|||||||
@@ -12,7 +12,13 @@ import { SettingsManager } from "../src/core/settings-manager.js";
|
|||||||
import { createTestResourceLoader } from "./utilities.js";
|
import { createTestResourceLoader } from "./utilities.js";
|
||||||
|
|
||||||
vi.mock("../src/core/compaction/index.js", () => ({
|
vi.mock("../src/core/compaction/index.js", () => ({
|
||||||
calculateContextTokens: () => 0,
|
calculateContextTokens: (usage: {
|
||||||
|
input: number;
|
||||||
|
output: number;
|
||||||
|
cacheRead: number;
|
||||||
|
cacheWrite: number;
|
||||||
|
totalTokens?: number;
|
||||||
|
}) => usage.totalTokens ?? usage.input + usage.output + usage.cacheRead + usage.cacheWrite,
|
||||||
collectEntriesForBranchSummary: () => ({ entries: [], commonAncestorId: null }),
|
collectEntriesForBranchSummary: () => ({ entries: [], commonAncestorId: null }),
|
||||||
compact: async () => ({
|
compact: async () => ({
|
||||||
summary: "compacted",
|
summary: "compacted",
|
||||||
@@ -23,11 +29,16 @@ vi.mock("../src/core/compaction/index.js", () => ({
|
|||||||
estimateContextTokens: () => ({ tokens: 0, usageTokens: 0, trailingTokens: 0, lastUsageIndex: -1 }),
|
estimateContextTokens: () => ({ tokens: 0, usageTokens: 0, trailingTokens: 0, lastUsageIndex: -1 }),
|
||||||
generateBranchSummary: async () => ({ summary: "", aborted: false, readFiles: [], modifiedFiles: [] }),
|
generateBranchSummary: async () => ({ summary: "", aborted: false, readFiles: [], modifiedFiles: [] }),
|
||||||
prepareCompaction: () => ({ dummy: true }),
|
prepareCompaction: () => ({ dummy: true }),
|
||||||
shouldCompact: () => false,
|
shouldCompact: (
|
||||||
|
contextTokens: number,
|
||||||
|
contextWindow: number,
|
||||||
|
settings: { enabled: boolean; reserveTokens: number },
|
||||||
|
) => settings.enabled && contextTokens > contextWindow - settings.reserveTokens,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
describe("AgentSession auto-compaction queue resume", () => {
|
describe("AgentSession auto-compaction queue resume", () => {
|
||||||
let session: AgentSession;
|
let session: AgentSession;
|
||||||
|
let sessionManager: SessionManager;
|
||||||
let tempDir: string;
|
let tempDir: string;
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
@@ -44,7 +55,7 @@ describe("AgentSession auto-compaction queue resume", () => {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const sessionManager = SessionManager.inMemory();
|
sessionManager = SessionManager.inMemory();
|
||||||
const settingsManager = SettingsManager.create(tempDir, tempDir);
|
const settingsManager = SettingsManager.create(tempDir, tempDir);
|
||||||
const authStorage = AuthStorage.create(join(tempDir, "auth.json"));
|
const authStorage = AuthStorage.create(join(tempDir, "auth.json"));
|
||||||
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
||||||
@@ -148,4 +159,61 @@ describe("AgentSession auto-compaction queue resume", () => {
|
|||||||
"Context overflow recovery failed after one compact-and-retry attempt. Try reducing context or switching to a larger-context model.",
|
"Context overflow recovery failed after one compact-and-retry attempt. Try reducing context or switching to a larger-context model.",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("should ignore stale pre-compaction assistant usage on pre-prompt compaction checks", async () => {
|
||||||
|
const model = session.model!;
|
||||||
|
const staleAssistantTimestamp = Date.now() - 10_000;
|
||||||
|
const staleAssistant: AssistantMessage = {
|
||||||
|
role: "assistant",
|
||||||
|
content: [{ type: "text", text: "large response before compaction" }],
|
||||||
|
api: model.api,
|
||||||
|
provider: model.provider,
|
||||||
|
model: model.id,
|
||||||
|
usage: {
|
||||||
|
input: 600_000,
|
||||||
|
output: 10_000,
|
||||||
|
cacheRead: 0,
|
||||||
|
cacheWrite: 0,
|
||||||
|
totalTokens: 610_000,
|
||||||
|
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||||
|
},
|
||||||
|
stopReason: "stop",
|
||||||
|
timestamp: staleAssistantTimestamp,
|
||||||
|
};
|
||||||
|
|
||||||
|
sessionManager.appendMessage({
|
||||||
|
role: "user",
|
||||||
|
content: [{ type: "text", text: "before compaction" }],
|
||||||
|
timestamp: staleAssistantTimestamp - 1000,
|
||||||
|
});
|
||||||
|
sessionManager.appendMessage(staleAssistant);
|
||||||
|
|
||||||
|
const firstKeptEntryId = sessionManager.getEntries()[0]!.id;
|
||||||
|
sessionManager.appendCompaction("summary", firstKeptEntryId, staleAssistant.usage.totalTokens, undefined, false);
|
||||||
|
|
||||||
|
sessionManager.appendMessage({
|
||||||
|
role: "user",
|
||||||
|
content: [{ type: "text", text: "session recovery payload" }],
|
||||||
|
timestamp: Date.now(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const runAutoCompactionSpy = vi
|
||||||
|
.spyOn(
|
||||||
|
session as unknown as {
|
||||||
|
_runAutoCompaction: (reason: "overflow" | "threshold", willRetry: boolean) => Promise<void>;
|
||||||
|
},
|
||||||
|
"_runAutoCompaction",
|
||||||
|
)
|
||||||
|
.mockResolvedValue();
|
||||||
|
|
||||||
|
const checkCompaction = (
|
||||||
|
session as unknown as {
|
||||||
|
_checkCompaction: (assistantMessage: AssistantMessage, skipAbortedCheck?: boolean) => Promise<void>;
|
||||||
|
}
|
||||||
|
)._checkCompaction.bind(session);
|
||||||
|
|
||||||
|
await checkCompaction(staleAssistant, false);
|
||||||
|
|
||||||
|
expect(runAutoCompactionSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user