fix(session): preserve overflow error when no recovery runs
Split active-context vs persisted-history removal in #checkCompaction so the persisted assistant error stays on the branch unless context promotion or compaction is actually scheduled. Fixes #3747
This commit is contained in:
@@ -9482,11 +9482,18 @@ export class AgentSession {
|
||||
const errorIsFromBeforeCompaction =
|
||||
compactionEntry !== null && assistantMessage.timestamp < new Date(compactionEntry.timestamp).getTime();
|
||||
if (sameModel && !errorIsFromBeforeCompaction && AIError.isContextOverflow(assistantMessage, contextWindow)) {
|
||||
await this.#discardRecoverableAssistantTurn(assistantMessage);
|
||||
// Clear the failed turn from active context so the retry (or the next
|
||||
// user prompt) does not replay it. The persisted branch entry stays
|
||||
// for now: when no recovery path runs, the user-facing transcript
|
||||
// MUST keep the only assistant message explaining why the turn
|
||||
// stopped. The branch entry is dropped further down, but only on the
|
||||
// paths that actually schedule a retry/compaction.
|
||||
this.#removeAssistantMessageFromActiveContext(assistantMessage);
|
||||
|
||||
// Try context promotion first - switch to a larger model and retry without compacting
|
||||
const promoted = await this.#tryContextPromotion(assistantMessage);
|
||||
if (promoted) {
|
||||
await this.#dropPersistedAssistantTurn(assistantMessage);
|
||||
// Retry on the promoted (larger) model without compacting
|
||||
this.#scheduleAgentContinue({ delayMs: 100, generation });
|
||||
return COMPACTION_CHECK_CONTINUATION;
|
||||
@@ -9495,6 +9502,7 @@ export class AgentSession {
|
||||
// No promotion target available fall through to compaction
|
||||
const compactionSettings = this.settings.getGroup("compaction");
|
||||
if (compactionSettings.enabled && compactionSettings.strategy !== "off") {
|
||||
await this.#dropPersistedAssistantTurn(assistantMessage);
|
||||
return await this.#runAutoCompaction("overflow", true, false, allowDefer, { autoContinue });
|
||||
}
|
||||
return COMPACTION_CHECK_NONE;
|
||||
@@ -9507,10 +9515,14 @@ export class AgentSession {
|
||||
// otherwise compaction/handoff. Unlike overflow, the *input* is fine, so we
|
||||
// allow the handoff strategy to actually run.
|
||||
if (sameModel && !errorIsFromBeforeCompaction && assistantMessage.stopReason === "length") {
|
||||
await this.#discardRecoverableAssistantTurn(assistantMessage);
|
||||
// Same active-context vs persisted-history split as the overflow path
|
||||
// above: clear the dead turn from agent state so it cannot be replayed,
|
||||
// but keep it on the branch unless promotion or compaction actually runs.
|
||||
this.#removeAssistantMessageFromActiveContext(assistantMessage);
|
||||
|
||||
const promoted = await this.#tryContextPromotion(assistantMessage);
|
||||
if (promoted) {
|
||||
await this.#dropPersistedAssistantTurn(assistantMessage);
|
||||
logger.debug("Context promotion triggered by response.incomplete (length stop)", {
|
||||
from: `${assistantMessage.provider}/${assistantMessage.model}`,
|
||||
});
|
||||
@@ -9520,6 +9532,7 @@ export class AgentSession {
|
||||
|
||||
const incompleteCompactionSettings = this.settings.getGroup("compaction");
|
||||
if (incompleteCompactionSettings.enabled && incompleteCompactionSettings.strategy !== "off") {
|
||||
await this.#dropPersistedAssistantTurn(assistantMessage);
|
||||
logger.debug("Compaction triggered by response.incomplete (length stop, no promotion target)", {
|
||||
model: `${assistantMessage.provider}/${assistantMessage.model}`,
|
||||
strategy: incompleteCompactionSettings.strategy,
|
||||
@@ -9789,7 +9802,16 @@ export class AgentSession {
|
||||
}
|
||||
}
|
||||
|
||||
async #discardRecoverableAssistantTurn(assistantMessage: AssistantMessage): Promise<void> {
|
||||
/**
|
||||
* Drop a recoverable assistant turn from the persisted session branch once a
|
||||
* recovery path (context promotion or compaction) is committed. Waits for the
|
||||
* in-flight `message_end` persistence slot first so the branch entry exists
|
||||
* before we reparent past it. Active context removal is the caller's
|
||||
* responsibility — recovery paths clear it eagerly so the retry never
|
||||
* replays the failed turn, while no-recovery paths leave the persisted entry
|
||||
* (and the user-visible transcript line) in place.
|
||||
*/
|
||||
async #dropPersistedAssistantTurn(assistantMessage: AssistantMessage): Promise<void> {
|
||||
await this.#waitForSessionMessagePersistence(assistantMessage);
|
||||
this.#discardAssistantTurn(assistantMessage);
|
||||
}
|
||||
|
||||
@@ -413,6 +413,36 @@ describe("AgentSession auto-compaction progress guard", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the visible overflow error when no recovery path is available", async () => {
|
||||
// When promotion is off and compaction is disabled there is no retry to run;
|
||||
// the persisted assistant error MUST stay on the branch so the user (and the
|
||||
// reloaded transcript) keeps the only explanation of why the turn stopped.
|
||||
session.settings.set("contextPromotion.enabled", false);
|
||||
session.settings.set("compaction.enabled", false);
|
||||
|
||||
const promptSpy = vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
|
||||
const continueSpy = vi.spyOn(session.agent, "continue").mockResolvedValue();
|
||||
|
||||
const assistantMsg = overflowAssistant();
|
||||
session.agent.emitExternalEvent({ type: "message_end", message: assistantMsg });
|
||||
session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMsg] });
|
||||
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(promptSpy).not.toHaveBeenCalled();
|
||||
expect(continueSpy).not.toHaveBeenCalled();
|
||||
expect(sessionManager.getBranch()).toContainEqual(
|
||||
expect.objectContaining({
|
||||
type: "message",
|
||||
message: expect.objectContaining({
|
||||
role: "assistant",
|
||||
stopReason: "error",
|
||||
errorMessage: assistantMsg.errorMessage,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("retries a small-window overflow when the default reserve exceeds the model window", async () => {
|
||||
// Bundled 4k/8k models can be smaller than the default absolute reserve
|
||||
// (16,384). Retry fit must clamp that reserve; otherwise the budget goes
|
||||
|
||||
Reference in New Issue
Block a user