From 7650ca492fe61a646a3145e1e495f5890aca110a Mon Sep 17 00:00:00 2001 From: minpeter Date: Thu, 23 Jul 2026 22:02:02 +0900 Subject: [PATCH 01/10] fix(coding-agent): stabilize compaction lifecycle --- .../coding-agent/src/core/agent-session.ts | 209 +++++++++++++++--- packages/coding-agent/src/core/changes.md | 18 ++ .../src/core/compaction/changes.md | 16 ++ .../src/core/compaction/lifecycle.ts | 107 +++++++++ .../extensions/builtin/compaction/changes.md | 16 ++ .../extensions/builtin/compaction/index.ts | 7 +- .../src/core/extensions/changes.md | 13 ++ .../coding-agent/src/core/extensions/types.ts | 2 + .../test/compaction/lifecycle.test.ts | 136 ++++++++++++ .../agent-session-apply-compaction.test.ts | 6 +- .../compaction-feedback-lifecycle.test.ts | 118 ++++++++++ .../compaction-current-model-state.test.ts | 109 +++++++++ .../pre-prompt-compaction-no-continue.test.ts | 78 +++++++ 13 files changed, 802 insertions(+), 33 deletions(-) create mode 100644 packages/coding-agent/src/core/compaction/lifecycle.ts create mode 100644 packages/coding-agent/test/compaction/lifecycle.test.ts create mode 100644 packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts create mode 100644 packages/coding-agent/test/suite/regressions/compaction-current-model-state.test.ts diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index b6436535e..81cb710dc 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -68,6 +68,13 @@ import { prepareCompaction, shouldCompact, } from "./compaction/index.ts"; +import { + beginCompactionOperation, + type CompactionLifecycleState, + finishCompactionOperation, + initialCompactionLifecycleState, + promoteCompactionOperation, +} from "./compaction/lifecycle.ts"; import { DEFAULT_THINKING_LEVEL } from "./defaults.ts"; import { type BuildDynamicSystemPromptOptions, buildDynamicSystemPrompt } from "./dynamic-prompt/index.ts"; import { exportSessionToHtml, type ToolHtmlRenderer } from "./export-html/index.ts"; @@ -462,6 +469,9 @@ export class AgentSession { // Compaction state private _compactionAbortController: AbortController | undefined = undefined; private _autoCompactionAbortController: AbortController | undefined = undefined; + private _compactionOperationController: AbortController | undefined = undefined; + private _compactionFeedbackController: AbortController | undefined = undefined; + private _compactionState: CompactionLifecycleState = initialCompactionLifecycleState(); private readonly _sessionWorkBarrier = new SessionWorkBarrier(); private _overflowRecoveryAttempted = false; private _messageRevision = 0; @@ -1510,12 +1520,17 @@ export class AgentSession { /** Whether compaction or branch summarization is currently running */ get isCompacting(): boolean { return ( + this._compactionState.status === "running" || this._autoCompactionAbortController !== undefined || this._compactionAbortController !== undefined || this._branchSummaryAbortController !== undefined ); } + get compactionState(): Readonly { + return this._compactionState; + } + /** All messages including custom types like BashExecutionMessage */ get messages(): AgentMessage[] { return this.agent.state.messages; @@ -2677,7 +2692,8 @@ export class AgentSession { async compact(customInstructions?: string): Promise { this._disconnectFromAgent(); await this.abort(); - this._compactionAbortController = new AbortController(); + const controller = new AbortController(); + this._compactionAbortController = controller; this._emit({ type: "compaction_start", reason: "manual" }); try { @@ -2702,7 +2718,9 @@ export class AgentSession { }); throw error; } finally { - this._compactionAbortController = undefined; + if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + this._compactionAbortController = undefined; + } this._reconnectToAgent(); } } @@ -2720,6 +2738,8 @@ export class AgentSession { this._compactionAbortController = new AbortController(); this._emit({ type: "compaction_start", reason: options.reason }); } + const controller = this._compactionAbortController; + if (!controller) return { applied: false, reason: "rejected" }; try { const execution = await this._executeCompaction({ @@ -2744,23 +2764,38 @@ export class AgentSession { }); return { applied: false, reason: "rejected" }; } finally { - this._compactionAbortController = undefined; + if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + this._compactionAbortController = undefined; + } + if (this._compactionFeedbackController === controller && this._compactionState.status !== "running") { + this._compactionFeedbackController = undefined; + } } } private _beginExtensionCompactionFeedback(reason: CompactionReason): AbortSignal { - if (!this._compactionAbortController) { - this._compactionAbortController = new AbortController(); + let controller = + this._compactionOperationController ?? this._compactionAbortController ?? this._autoCompactionAbortController; + if (!controller) { + controller = new AbortController(); + this._compactionAbortController = controller; + this._compactionFeedbackController = controller; this._emit({ type: "compaction_start", reason }); } - return this._compactionAbortController.signal; + const model = this.model; + if (model) { + this._beginCompactionOperation(randomUUID(), reason, model, controller, "feedback"); + } + return controller.signal; } private _updateExtensionCompactionFeedback(options: { reason: CompactionReason; + signal?: AbortSignal; delta?: string; text?: string; }): void { + if (options.signal && this._compactionOperationController?.signal !== options.signal) return; if (!this._compactionAbortController && !this._autoCompactionAbortController) return; this._emit({ type: "compaction_progress", @@ -2772,11 +2807,14 @@ export class AgentSession { private _endExtensionCompactionFeedback(options: { reason: CompactionReason; + signal?: AbortSignal; aborted?: boolean; errorMessage?: string; }): void { - const controller = this._compactionAbortController; + const controller = + this._compactionOperationController ?? this._compactionAbortController ?? this._autoCompactionAbortController; if (!controller) return; + if (options.signal && controller.signal !== options.signal) return; const aborted = options.aborted ?? controller.signal.aborted; this._emit({ type: "compaction_end", @@ -2786,26 +2824,38 @@ export class AgentSession { willRetry: false, errorMessage: aborted ? undefined : options.errorMessage, }); - this._compactionAbortController = undefined; + if ( + this._compactionState.status === "running" && + this._compactionOperationController === controller && + this._compactionState.stage === "feedback" + ) { + this._finishCompactionOperation(this._compactionState.operationId, aborted ? "aborted" : "failed", { + errorMessage: aborted ? "Compaction cancelled" : (options.errorMessage ?? "Compaction did not apply"), + }); + } + if (this._compactionFeedbackController === controller) { + this._compactionFeedbackController = undefined; + if (this._compactionAbortController === controller) { + this._compactionAbortController = undefined; + } + } } private async _executeCompaction(request: CompactionExecutionRequest): Promise { + const model = this.model; + if (!model) throw new Error(formatNoModelSelectedMessage()); + const thinkingLevel = this.thinkingLevel; + const controller = this._compactionAbortController ?? this._autoCompactionAbortController; + if (!controller) throw new Error("Compaction abort controller unavailable"); + const requestId = randomUUID(); + const operationId = this._beginCompactionOperation(requestId, request.reason, model, controller, "execution"); const finishCompactionWork = this._sessionWorkBarrier.begin(); const agentMessagesAtStart = request.agentMessagesAtStart ?? this.agent.state.messages.slice(); + const signal = controller.signal; try { - if (!this.model) { - throw new Error(formatNoModelSelectedMessage()); - } - - const requestId = randomUUID(); const pathEntries = this.sessionManager.getBranch(); const settings = this.settingsManager.getCompactionSettings(); - const signal = this._compactionAbortController?.signal ?? this._autoCompactionAbortController?.signal; - if (!signal) { - throw new Error("Compaction abort controller unavailable"); - } - let compactionResult = request.precomputed; let fromExtension = request.precomputed !== undefined; @@ -2833,6 +2883,10 @@ export class AgentSession { })) as SessionBeforeCompactResult | undefined; if (extensionResult?.cancel) { + this._finishCompactionOperation(operationId, "failed", { + rejectionCause: extensionResult.rejectionCause ?? "cancelled-by-extension", + errorMessage: extensionResult.reason, + }); return await this._rejectCompaction( request, requestId, @@ -2849,16 +2903,16 @@ export class AgentSession { } if (!compactionResult) { - const { apiKey, headers, extraBody, env } = await this._getCompactionRequestAuth(this.model); + const { apiKey, headers, extraBody, env } = await this._getCompactionRequestAuth(model); compactionResult = await compact( preparation, - this.model, + model, apiKey, headers, request.customInstructions, signal, extraBody, - this.thinkingLevel, + thinkingLevel, this.agent.streamFn, env, this.agent.transformContext, @@ -2869,8 +2923,14 @@ export class AgentSession { if (signal.aborted) { throw new Error("Compaction cancelled"); } + if (!this._isCurrentCompactionOperation(operationId, controller)) { + throw new DOMException("Compaction superseded", "AbortError"); + } - if (this._wouldCompactionOverflow(pathEntries, compactionResult, fromExtension)) { + if (this._wouldCompactionOverflow(pathEntries, compactionResult, fromExtension, model)) { + this._finishCompactionOperation(operationId, "failed", { + rejectionCause: "would-overflow", + }); return await this._rejectCompaction(request, requestId, "would-overflow", false); } @@ -2897,6 +2957,7 @@ export class AgentSession { this.agent.state.messages = [...sessionContext.messages, ...messagesAppendedDuringCompaction]; compactionResult.estimatedTokensAfter = estimateMessagesTokens(sessionContext.messages); this._incrementMessageRevision(); + this._finishCompactionOperation(operationId, "completed"); await this._extensionRunner.emit({ type: "session_compact", @@ -2919,18 +2980,80 @@ export class AgentSession { }); return { accepted: true, requestId, result: compactionResult, compactionEntry: savedEntry, fromExtension }; + } catch (error) { + const aborted = signal.aborted || (error instanceof Error && error.name === "AbortError"); + this._finishCompactionOperation(operationId, aborted ? "aborted" : "failed", { + errorMessage: error instanceof Error ? error.message : String(error), + }); + throw error; } finally { finishCompactionWork(); } } + private _beginCompactionOperation( + operationId: string, + reason: CompactionReason, + model: Model, + controller: AbortController, + stage: "feedback" | "execution", + ): string { + if (this._compactionState.status === "running") { + if (this._compactionOperationController === controller) { + const currentOperationId = this._compactionState.operationId; + if (stage === "execution") { + this._compactionState = promoteCompactionOperation(this._compactionState, currentOperationId); + } + return currentOperationId; + } + this._compactionOperationController?.abort(); + } + this._compactionOperationController = controller; + this._compactionState = beginCompactionOperation(this._compactionState, { + operationId, + stage, + reason, + model: { provider: model.provider, id: model.id }, + startedRevision: this._messageRevision, + }); + return operationId; + } + + private _isCurrentCompactionOperation(operationId: string, controller: AbortController): boolean { + return ( + this._compactionState.status === "running" && + this._compactionState.operationId === operationId && + this._compactionOperationController === controller && + !controller.signal.aborted + ); + } + + private _finishCompactionOperation( + operationId: string, + status: "completed" | "failed" | "aborted", + options: { rejectionCause?: CompactionRejectionCause; errorMessage?: string } = {}, + ): void { + const previous = this._compactionState; + const next = finishCompactionOperation(previous, { + operationId, + status, + endedRevision: this._messageRevision, + ...options, + }); + this._compactionState = next; + if (previous.status === "running" && previous.operationId === operationId && next !== previous) { + this._compactionOperationController = undefined; + } + } + private _wouldCompactionOverflow( pathEntries: SessionEntry[], compactionResult: CompactionResult, fromExtension: boolean, + model: Model, ): boolean { const currentLeaf = pathEntries[pathEntries.length - 1]; - if (!currentLeaf || !this.model) return false; + if (!currentLeaf) return false; const simulatedCompactionEntry: CompactionEntry = { type: "compaction", @@ -2950,7 +3073,7 @@ export class AgentSession { ).messages; const contextTokens = estimateMessagesTokens(filterContextExcludedMessages(simulatedMessages)); const settings = this.settingsManager.getCompactionSettings(); - return contextTokens > this.model.contextWindow - settings.reserveTokens; + return contextTokens > model.contextWindow - settings.reserveTokens; } private async _rejectCompaction( @@ -2998,8 +3121,21 @@ export class AgentSession { * Cancel in-progress compaction (manual or auto). */ abortCompaction(): void { + const feedbackController = this._compactionFeedbackController; this._compactionAbortController?.abort(); this._autoCompactionAbortController?.abort(); + this._compactionOperationController?.abort(); + if (this._compactionState.status === "running") { + this._finishCompactionOperation(this._compactionState.operationId, "aborted", { + errorMessage: "Compaction cancelled", + }); + } + if (feedbackController && this._compactionFeedbackController === feedbackController) { + this._compactionFeedbackController = undefined; + if (this._compactionAbortController === feedbackController) { + this._compactionAbortController = undefined; + } + } } /** @@ -3094,7 +3230,16 @@ export class AgentSession { this._incrementMessageRevision(); } if (inlineReason) { - return await this._runPrePromptCompaction(assistantMessage, skipAbortedCheck, "overflow", willRetry); + const compacted = await this._runPrePromptCompaction( + assistantMessage, + skipAbortedCheck, + "overflow", + willRetry, + ); + if (!compacted) { + throw new RequiredCompactionError(); + } + return true; } else { return await this._runAutoCompaction("overflow", willRetry); } @@ -3147,7 +3292,8 @@ export class AgentSession { willRetry = false, ): Promise { this._emit({ type: "compaction_start", reason }); - this._compactionAbortController = new AbortController(); + const controller = new AbortController(); + this._compactionAbortController = controller; try { const execution = await this._executeCompaction({ @@ -3177,7 +3323,9 @@ export class AgentSession { }); return false; } finally { - this._compactionAbortController = undefined; + if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + this._compactionAbortController = undefined; + } } } @@ -3577,7 +3725,8 @@ export class AgentSession { void (async () => { this._disconnectFromAgent(); await this.abort(); - this._compactionAbortController = new AbortController(); + const controller = new AbortController(); + this._compactionAbortController = controller; this._emit({ type: "compaction_start", reason: "extension" }); try { @@ -3606,7 +3755,9 @@ export class AgentSession { const err = error instanceof Error ? error : new Error(String(error)); options?.onError?.(err); } finally { - this._compactionAbortController = undefined; + if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + this._compactionAbortController = undefined; + } this._reconnectToAgent(); } })(); diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index ae1932ea8..6c89331a9 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,5 +1,23 @@ # changes +## Session-owned compaction lifecycle (2026-07-23) + +### What changed + +- `agent-session.ts` now owns a monotonic compaction operation state, snapshots the active model and controller at + operation start, rejects stale completion/feedback, and retains the terminal result until another operation begins. +- Provider-confirmed overflow remains fail-closed when required pre-prompt compaction fails, even when the local token + estimate is below the configured threshold. + +### Why extension system couldn't handle this alone + +- Model selection, durable session append, provider-overflow recovery, controller ownership, and prompt admission are + private `AgentSession` lifecycle boundaries. + +### Expected merge conflict zones + +- HIGH: `agent-session.ts` compaction execution, pre-prompt recovery, abort handling, and extension context bindings. + ## Streaming steer/followUp submissions bypass the session-work barrier (2026-07-21) ### What changed diff --git a/packages/coding-agent/src/core/compaction/changes.md b/packages/coding-agent/src/core/compaction/changes.md index ceb2e6e78..3568fe34b 100644 --- a/packages/coding-agent/src/core/compaction/changes.md +++ b/packages/coding-agent/src/core/compaction/changes.md @@ -1,5 +1,21 @@ # changes.md — compaction +## Operation lifecycle reducer (2026-07-23) + +### What changed + +- `lifecycle.ts` adds the pure `idle` / `running` / `completed` / `failed` / `aborted` transition model used by + `AgentSession`, including monotonic generations, feedback-to-execution promotion, and stale terminal-event rejection. + +### Why + +- Compaction completion must remain observable after controllers are released, while delayed work from an older + generation must not overwrite the active operation. + +### Expected merge conflict zones + +- NONE: `lifecycle.ts` is a new fork-owned module. + ## Summarization stream idle watchdog (2026-07-21) ### What changed diff --git a/packages/coding-agent/src/core/compaction/lifecycle.ts b/packages/coding-agent/src/core/compaction/lifecycle.ts new file mode 100644 index 000000000..79ba3fe42 --- /dev/null +++ b/packages/coding-agent/src/core/compaction/lifecycle.ts @@ -0,0 +1,107 @@ +import type { CompactionReason, CompactionRejectionCause } from "../extensions/types.ts"; + +export interface CompactionModelRef { + readonly provider: string; + readonly id: string; +} + +interface CompactionOperation { + readonly generation: number; + readonly operationId: string; + readonly stage: "feedback" | "execution"; + readonly reason: CompactionReason; + readonly model: CompactionModelRef; + readonly startedRevision: number; +} + +export type CompactionLifecycleState = + | { readonly status: "idle"; readonly generation: 0 } + | (CompactionOperation & { readonly status: "running" }) + | (CompactionOperation & { + readonly status: "completed"; + readonly endedRevision: number; + }) + | (CompactionOperation & { + readonly status: "failed"; + readonly endedRevision: number; + readonly rejectionCause?: CompactionRejectionCause; + readonly errorMessage?: string; + }) + | (CompactionOperation & { + readonly status: "aborted"; + readonly endedRevision: number; + readonly errorMessage?: string; + }); + +export interface BeginCompactionOperation { + readonly operationId: string; + readonly stage: "feedback" | "execution"; + readonly reason: CompactionReason; + readonly model: CompactionModelRef; + readonly startedRevision: number; +} + +export interface FinishCompactionOperation { + readonly operationId: string; + readonly status: "completed" | "failed" | "aborted"; + readonly endedRevision: number; + readonly rejectionCause?: CompactionRejectionCause; + readonly errorMessage?: string; +} + +export function initialCompactionLifecycleState(): CompactionLifecycleState { + return { status: "idle", generation: 0 }; +} + +export function beginCompactionOperation( + state: CompactionLifecycleState, + operation: BeginCompactionOperation, +): CompactionLifecycleState { + return { + status: "running", + generation: state.generation + 1, + ...operation, + }; +} + +export function promoteCompactionOperation( + state: CompactionLifecycleState, + operationId: string, +): CompactionLifecycleState { + if (state.status !== "running" || state.operationId !== operationId || state.stage === "execution") return state; + return { ...state, stage: "execution" }; +} + +export function finishCompactionOperation( + state: CompactionLifecycleState, + event: FinishCompactionOperation, +): CompactionLifecycleState { + if (state.status !== "running" || state.operationId !== event.operationId) return state; + + const operation: CompactionOperation = { + generation: state.generation, + operationId: state.operationId, + stage: state.stage, + reason: state.reason, + model: state.model, + startedRevision: state.startedRevision, + }; + if (event.status === "completed") { + return { ...operation, status: "completed", endedRevision: event.endedRevision }; + } + if (event.status === "aborted") { + return { + ...operation, + status: "aborted", + endedRevision: event.endedRevision, + errorMessage: event.errorMessage, + }; + } + return { + ...operation, + status: "failed", + endedRevision: event.endedRevision, + rejectionCause: event.rejectionCause, + errorMessage: event.errorMessage, + }; +} diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md index 396f63170..e55b76cb1 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md @@ -1,5 +1,21 @@ # Builtin compaction extension changes +## Session-owned compaction completion state (2026-07-23) + +- AgentSession now records compaction as `idle`, `running`, `completed`, `failed`, or `aborted` with a monotonic + generation and operation identity. +- Compaction snapshots the current AgentSession model at operation start. If main-thread retry fallback selected a + different model, that active model performs compaction; there is no compaction-specific fallback policy. +- Extension feedback starts the same operation before summary generation and carries its abort signal through + progress, application, and terminal feedback. +- Stale or duplicate terminal events cannot overwrite a newer compaction operation. +- Durable append is guarded by the current operation and controller identity. +- Required compaction remains fail-closed when generation or application fails, including provider-confirmed overflow + that the local token estimate places below the configured threshold. + +Expected upstream conflict zones: `agent-session.ts` around compaction execution, abort handling, and status access; +`core/compaction/lifecycle.ts`. + ## Sanitize Anthropic tool pairs on direct summarization requests (2026-07-23) - `speculative.ts`: local compaction summarization now applies the existing Anthropic payload sanitizer at the direct diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts index d4c5e2e49..70bbbca6a 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts @@ -112,7 +112,7 @@ function endCompactionFeedback( result: SpeculativeCompactionResult, ): void { if (shouldEndFeedback(result)) { - ctx.endCompaction?.({ reason: "extension", aborted: signal?.aborted }); + ctx.endCompaction?.({ reason: "extension", signal, aborted: signal?.aborted }); } } @@ -301,7 +301,7 @@ export default function compactionExtension(pi: ExtensionAPI): void { let compaction: CompactionResult | undefined; try { compaction = await runExtensionCompaction(ctx, snapshot, feedbackSignal, (delta) => - ctx.updateCompaction?.({ reason: "extension", delta }), + ctx.updateCompaction?.({ reason: "extension", signal: feedbackSignal, delta }), ); } catch (error) { // Auto-path parity: summary-generation failures (missing credentials, @@ -317,6 +317,7 @@ export default function compactionExtension(pi: ExtensionAPI): void { const message = error instanceof Error ? error.message : String(error); ctx.endCompaction?.({ reason: "extension", + signal: feedbackSignal, aborted: feedbackSignal?.aborted, errorMessage: `Compaction failed: ${message}`, }); @@ -369,7 +370,7 @@ export default function compactionExtension(pi: ExtensionAPI): void { let compaction: CompactionResult | undefined; try { compaction = await runExtensionCompaction(ctx, snapshot, event.signal, (delta) => - ctx.updateCompaction?.({ reason: event.reason, delta }), + ctx.updateCompaction?.({ reason: event.reason, signal: event.signal, delta }), ); } catch (error) { // Surface the real provider failure (e.g. a policy refusal or rate diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index fa35b5781..43fc6de55 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -1,5 +1,18 @@ # Core Extensions Changes +## 2026-07-23 - Compaction feedback operation handles + +### What changed + +- `ExtensionContext` compaction feedback actions now return and accept an optional operation `AbortSignal`, allowing + progress and terminal feedback from superseded generations to be ignored without breaking existing extensions. +- The builtin compaction extension threads that signal through local and remote summary generation and application. + +### Why + +Asynchronous summary feedback can arrive after a newer compaction begins; operation identity prevents stale progress +or completion from mutating the current session lifecycle. + ## 2026-07-22 - Config-reload rejection loop breaker ### What changed diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index 3ad34d225..5b71325f3 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -341,12 +341,14 @@ export interface BeginCompactionOptions { export interface UpdateCompactionOptions { reason: CompactionReason; + signal?: AbortSignal; delta?: string; text?: string; } export interface EndCompactionOptions { reason: CompactionReason; + signal?: AbortSignal; aborted?: boolean; errorMessage?: string; } diff --git a/packages/coding-agent/test/compaction/lifecycle.test.ts b/packages/coding-agent/test/compaction/lifecycle.test.ts new file mode 100644 index 000000000..3063aa7ae --- /dev/null +++ b/packages/coding-agent/test/compaction/lifecycle.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from "vitest"; +import { + beginCompactionOperation, + finishCompactionOperation, + initialCompactionLifecycleState, + promoteCompactionOperation, +} from "../../src/core/compaction/lifecycle.ts"; + +function begin(operationId: string, generation = 0) { + return beginCompactionOperation( + generation === 0 + ? initialCompactionLifecycleState() + : { + status: "completed", + generation, + operationId: `previous-${generation}`, + stage: "execution", + reason: "threshold", + model: { provider: "faux", id: "previous" }, + startedRevision: generation, + endedRevision: generation, + }, + { + operationId, + stage: "execution", + reason: "threshold", + model: { provider: "faux", id: "active" }, + startedRevision: generation + 1, + }, + ); +} + +describe("compaction lifecycle", () => { + it("retains completed state until the next operation begins", () => { + const running = begin("operation-1"); + const completed = finishCompactionOperation(running, { + operationId: "operation-1", + status: "completed", + endedRevision: 2, + }); + + expect(completed).toMatchObject({ + status: "completed", + generation: 1, + operationId: "operation-1", + model: { provider: "faux", id: "active" }, + endedRevision: 2, + }); + }); + + it("ignores stale completion from a superseded operation", () => { + const first = begin("operation-1"); + const second = beginCompactionOperation(first, { + operationId: "operation-2", + stage: "execution", + reason: "pre_prompt", + model: { provider: "faux", id: "fallback" }, + startedRevision: 3, + }); + const afterStaleCompletion = finishCompactionOperation(second, { + operationId: "operation-1", + status: "completed", + endedRevision: 4, + }); + + expect(afterStaleCompletion).toBe(second); + expect(afterStaleCompletion).toMatchObject({ + status: "running", + generation: 2, + operationId: "operation-2", + model: { provider: "faux", id: "fallback" }, + }); + }); + + it("promotes feedback generation into the same execution operation", () => { + const feedback = beginCompactionOperation(initialCompactionLifecycleState(), { + operationId: "operation-1", + stage: "feedback", + reason: "extension", + model: { provider: "faux", id: "fallback" }, + startedRevision: 2, + }); + const execution = promoteCompactionOperation(feedback, "operation-1"); + + expect(execution).toMatchObject({ + status: "running", + generation: 1, + operationId: "operation-1", + stage: "execution", + }); + }); + + it("records failure and abort as distinct terminal states", () => { + const failed = finishCompactionOperation(begin("failed-operation"), { + operationId: "failed-operation", + status: "failed", + endedRevision: 5, + rejectionCause: "would-overflow", + errorMessage: "summary exceeds budget", + }); + const aborted = finishCompactionOperation(begin("aborted-operation", 1), { + operationId: "aborted-operation", + status: "aborted", + endedRevision: 6, + errorMessage: "Compaction cancelled", + }); + + expect(failed).toMatchObject({ + status: "failed", + rejectionCause: "would-overflow", + errorMessage: "summary exceeds budget", + }); + expect(aborted).toMatchObject({ + status: "aborted", + generation: 2, + errorMessage: "Compaction cancelled", + }); + }); + + it("ignores duplicate terminal events", () => { + const completed = finishCompactionOperation(begin("operation-1"), { + operationId: "operation-1", + status: "completed", + endedRevision: 2, + }); + const duplicate = finishCompactionOperation(completed, { + operationId: "operation-1", + status: "failed", + endedRevision: 3, + errorMessage: "late failure", + }); + + expect(duplicate).toBe(completed); + expect(duplicate.status).toBe("completed"); + }); +}); diff --git a/packages/coding-agent/test/suite/agent-session-apply-compaction.test.ts b/packages/coding-agent/test/suite/agent-session-apply-compaction.test.ts index 048849816..bd4cc8bb5 100644 --- a/packages/coding-agent/test/suite/agent-session-apply-compaction.test.ts +++ b/packages/coding-agent/test/suite/agent-session-apply-compaction.test.ts @@ -125,6 +125,8 @@ describe("AgentSession applyCompaction", () => { it("emits one compaction start while an extension prepares and applies a summary", async () => { // given + const observedCompactionStates: string[] = []; + let harness: Harness; const extension = (pi: ExtensionAPI): void => { pi.on("before_agent_start", async (_event, ctx) => { const entries = ctx.sessionManager.getEntries(); @@ -134,6 +136,7 @@ describe("AgentSession applyCompaction", () => { } ctx.beginCompaction?.({ reason: "extension" }); + observedCompactionStates.push(harness.session.compactionState.status); await ctx.applyCompaction( { summary: "extension feedback summary", @@ -145,7 +148,7 @@ describe("AgentSession applyCompaction", () => { return undefined; }); }; - const harness = await createHarness({ extensionFactories: [extension] }); + harness = await createHarness({ extensionFactories: [extension] }); harnesses.push(harness); harness.setResponses([fauxAssistantMessage("one"), fauxAssistantMessage("two")]); await harness.session.prompt("one"); @@ -165,5 +168,6 @@ describe("AgentSession applyCompaction", () => { reason: "extension", aborted: false, }); + expect(observedCompactionStates).toEqual(["running"]); }); }); diff --git a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts new file mode 100644 index 000000000..3b5a7bbad --- /dev/null +++ b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts @@ -0,0 +1,118 @@ +import { afterEach, describe, expect, it } from "vitest"; +import type { CompactionReason } from "../../src/core/extensions/types.ts"; +import { createHarness, type Harness } from "./harness.ts"; + +type BeginFeedback = (reason: CompactionReason) => AbortSignal; +type UpdateFeedback = (options: { + reason: CompactionReason; + signal?: AbortSignal; + delta?: string; + text?: string; +}) => void; +type EndFeedback = (options: { + reason: CompactionReason; + signal?: AbortSignal; + aborted?: boolean; + errorMessage?: string; +}) => void; + +describe("compaction feedback lifecycle", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("ignores stale feedback completion after a newer operation begins", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const begin = Reflect.get(harness.session, "_beginExtensionCompactionFeedback"); + const update = Reflect.get(harness.session, "_updateExtensionCompactionFeedback"); + const end = Reflect.get(harness.session, "_endExtensionCompactionFeedback"); + if (typeof begin !== "function" || typeof update !== "function" || typeof end !== "function") { + throw new Error("Compaction feedback lifecycle methods unavailable"); + } + + const oldSignal = (begin as BeginFeedback).call(harness.session, "extension"); + (end as EndFeedback).call(harness.session, { + reason: "extension", + signal: oldSignal, + aborted: true, + }); + const freshSignal = (begin as BeginFeedback).call(harness.session, "extension"); + const freshState = harness.session.compactionState; + const progressBeforeStaleUpdate = harness.eventsOfType("compaction_progress").length; + (update as UpdateFeedback).call(harness.session, { + reason: "extension", + signal: oldSignal, + delta: "late progress", + }); + (end as EndFeedback).call(harness.session, { + reason: "extension", + signal: oldSignal, + errorMessage: "late failure", + }); + + expect(harness.session.compactionState).toBe(freshState); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + expect(harness.session.isCompacting).toBe(true); + expect(harness.eventsOfType("compaction_progress")).toHaveLength(progressBeforeStaleUpdate); + + (update as UpdateFeedback).call(harness.session, { + reason: "extension", + signal: freshSignal, + delta: "current progress", + }); + expect(harness.eventsOfType("compaction_progress").at(-1)).toMatchObject({ + reason: "extension", + delta: "current progress", + }); + + (end as EndFeedback).call(harness.session, { + reason: "extension", + signal: freshSignal, + errorMessage: "current failure", + }); + expect(harness.session.compactionState).toMatchObject({ + status: "failed", + generation: 2, + errorMessage: "current failure", + }); + expect(harness.session.isCompacting).toBe(false); + }); + + it("clears feedback ownership on abort before a later operation begins", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const begin = Reflect.get(harness.session, "_beginExtensionCompactionFeedback"); + const end = Reflect.get(harness.session, "_endExtensionCompactionFeedback"); + if (typeof begin !== "function" || typeof end !== "function") { + throw new Error("Compaction feedback lifecycle methods unavailable"); + } + + const abortedSignal = (begin as BeginFeedback).call(harness.session, "extension"); + harness.session.abortCompaction(); + + expect(abortedSignal.aborted).toBe(true); + expect(harness.session.compactionState.status).toBe("aborted"); + expect(harness.session.isCompacting).toBe(false); + + const freshSignal = (begin as BeginFeedback).call(harness.session, "extension"); + expect(freshSignal.aborted).toBe(false); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + + (end as EndFeedback).call(harness.session, { + reason: "extension", + signal: freshSignal, + errorMessage: "current failure", + }); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/compaction-current-model-state.test.ts b/packages/coding-agent/test/suite/regressions/compaction-current-model-state.test.ts new file mode 100644 index 000000000..227441b9a --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/compaction-current-model-state.test.ts @@ -0,0 +1,109 @@ +import { fauxAssistantMessage } from "@earendil-works/pi-ai"; +import type { Context, FauxResponseFactory } from "@earendil-works/pi-ai/compat"; +import { afterEach, describe, expect, it } from "vitest"; +import compactionExtension from "../../../src/core/extensions/builtin/compaction/index.ts"; +import { createHarness, getAssistantTexts, getMessageText, type Harness } from "../harness.ts"; + +function lastUserText(context: Context): string { + for (let index = context.messages.length - 1; index >= 0; index--) { + const message = context.messages[index]; + if (message?.role === "user") return getMessageText(message); + } + return ""; +} + +function seedLargeContext(harness: Harness): void { + const model = harness.getModel("faux-1"); + if (!model) throw new Error("Primary model was not registered"); + const now = Date.now(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "history ".repeat(22_000) }], + timestamp: now - 1000, + }); + harness.sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "result ".repeat(200) }], + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 19_900, + output: 100, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 20_000, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: now - 500, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; +} + +describe("Regression: compaction state during model fallback", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("uses the active main-thread fallback model for compaction", async () => { + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 50_000, maxTokens: 2048 }, + { id: "faux-2", contextWindow: 50_000, maxTokens: 2048 }, + ], + settings: { + compaction: { + enabled: true, + reserveTokens: 5000, + keepRecentTokens: 32, + speculativeEnabled: false, + }, + retry: { + enabled: true, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + extensionFactories: [compactionExtension], + }); + harnesses.push(harness); + seedLargeContext(harness); + + const response: FauxResponseFactory = async (context, _options, _state, model) => { + const prompt = lastUserText(context); + if (prompt === "trigger fallback") { + if (model.id === "faux-1") { + return fauxAssistantMessage("", { stopReason: "error", errorMessage: "overloaded_error" }); + } + return fauxAssistantMessage("fallback answer"); + } + if (prompt === "prompt after compaction") return fauxAssistantMessage("next answer"); + return fauxAssistantMessage("compacted on active fallback"); + }; + harness.setResponses([response, response, response, response]); + + await harness.session.prompt("trigger fallback"); + const compactionModels = harness.faux + .getCallLog() + .filter((entry) => !["trigger fallback", "prompt after compaction"].includes(lastUserText(entry.context))) + .map((entry) => entry.modelId); + + expect(harness.eventsOfType("retry_fallback_applied")).toMatchObject([ + { from: "faux/faux-1", to: "faux/faux-2" }, + ]); + expect(compactionModels).toEqual(["faux-2"]); + expect(harness.session.model?.id).toBe("faux-2"); + expect(Reflect.get(harness.session, "compactionState")).toMatchObject({ + status: "completed", + generation: 1, + model: { provider: "faux", id: "faux-2" }, + }); + + await harness.session.prompt("prompt after compaction"); + expect(getAssistantTexts(harness)).toContain("next answer"); + expect(harness.session.model?.id).toBe("faux-2"); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts index b087568ea..7670ad346 100644 --- a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts +++ b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts @@ -73,6 +73,71 @@ describe("pre-prompt compaction regression", () => { expect(harness.faux.state.callCount).toBe(1); }); + it("blocks the next provider call when required overflow compaction is rejected below the local threshold", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "forced rejection", + })); + }, + ], + }); + harnesses.push(harness); + + const now = Date.now(); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "earlier prompt" }], + timestamp: now - 3000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("earlier response", { timestamp: now - 2000 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(50), + }); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "previous prompt" }], + timestamp: now - 1000, + }); + const overflowAssistant: AssistantMessage = { + ...fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "context_length_exceeded", + timestamp: now - 500, + }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(100), + }; + harness.sessionManager.appendMessage(overflowAssistant); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([fauxAssistantMessage("must not reach provider")]); + + await expect(harness.session.prompt("next prompt")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + + expect(harness.faux.state.callCount).toBe(0); + expect(getUserTexts(harness)).not.toContain("next prompt"); + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "overflow", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + }); + it("compacts upstream model alias overflow before a dot retry", async () => { const harness = await createHarness({ api: "openai-responses", @@ -97,6 +162,18 @@ describe("pre-prompt compaction regression", () => { const now = Date.now(); const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "earlier prompt" }], + timestamp: now - 3000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("earlier response", { timestamp: now - 2000 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(50), + }); harness.sessionManager.appendMessage({ role: "user", content: [{ type: "text", text: "read /tmp/h2.jpg" }], @@ -126,6 +203,7 @@ describe("pre-prompt compaction regression", () => { reason: "overflow", aborted: false, willRetry: true, + accepted: true, }); expect(getUserTexts(harness)).toContain("."); expect(harness.faux.state.callCount).toBe(1); From 93c25f3d9909792d908d2efe194597dea6f34b3d Mon Sep 17 00:00:00 2001 From: minpeter Date: Thu, 23 Jul 2026 22:22:43 +0900 Subject: [PATCH 02/10] fix(coding-agent): retain rejected overflow context --- .../coding-agent/src/core/agent-session.ts | 25 +++++++++---------- packages/coding-agent/src/core/changes.md | 3 ++- .../extensions/builtin/compaction/changes.md | 3 ++- .../pre-prompt-compaction-no-continue.test.ts | 5 ++++ 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 81cb710dc..ed3c09f24 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -3225,24 +3225,23 @@ export class AgentSession { // Remove the error message from agent state (it IS saved to session for history, // but we don't want it in context for the retry) const messages = this.agent.state.messages; + let removedOverflowAssistant = false; if (messages.length > 0 && messages[messages.length - 1].role === "assistant") { this.agent.state.messages = messages.slice(0, -1); + removedOverflowAssistant = true; this._incrementMessageRevision(); } - if (inlineReason) { - const compacted = await this._runPrePromptCompaction( - assistantMessage, - skipAbortedCheck, - "overflow", - willRetry, - ); - if (!compacted) { - throw new RequiredCompactionError(); - } - return true; - } else { - return await this._runAutoCompaction("overflow", willRetry); + const compacted = inlineReason + ? await this._runPrePromptCompaction(assistantMessage, skipAbortedCheck, "overflow", willRetry) + : await this._runAutoCompaction("overflow", willRetry); + if (!compacted && removedOverflowAssistant) { + this.agent.state.messages = this.sessionManager.buildSessionContext().messages; + this._incrementMessageRevision(); + } + if (!compacted && inlineReason) { + throw new RequiredCompactionError(); } + return compacted; } // Case 2: Threshold - context is getting large diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 6c89331a9..166e5320a 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -7,7 +7,8 @@ - `agent-session.ts` now owns a monotonic compaction operation state, snapshots the active model and controller at operation start, rejects stale completion/feedback, and retains the terminal result until another operation begins. - Provider-confirmed overflow remains fail-closed when required pre-prompt compaction fails, even when the local token - estimate is below the configured threshold. + estimate is below the configured threshold; failed recovery restores the overflow context so later prompts cannot + bypass the same requirement. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md index e55b76cb1..d76f41d38 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md @@ -11,7 +11,8 @@ - Stale or duplicate terminal events cannot overwrite a newer compaction operation. - Durable append is guarded by the current operation and controller identity. - Required compaction remains fail-closed when generation or application fails, including provider-confirmed overflow - that the local token estimate places below the configured threshold. + that the local token estimate places below the configured threshold; rejected recovery restores the overflow + context so a later prompt cannot bypass the same requirement. Expected upstream conflict zones: `agent-session.ts` around compaction execution, abort handling, and status access; `core/compaction/lifecycle.ts`. diff --git a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts index 7670ad346..bef47473a 100644 --- a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts +++ b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts @@ -126,9 +126,14 @@ describe("pre-prompt compaction regression", () => { await expect(harness.session.prompt("next prompt")).rejects.toThrow( "Context remains above the compaction threshold because compaction did not complete", ); + await expect(harness.session.prompt("retry prompt")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); expect(harness.faux.state.callCount).toBe(0); expect(getUserTexts(harness)).not.toContain("next prompt"); + expect(getUserTexts(harness)).not.toContain("retry prompt"); + expect(harness.eventsOfType("compaction_end").filter((event) => event.accepted === false)).toHaveLength(2); expect(harness.eventsOfType("compaction_end")).toContainEqual( expect.objectContaining({ reason: "overflow", From 5dfafe4629457f107488cb726f30f353826d87bf Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 24 Jul 2026 02:33:58 +0900 Subject: [PATCH 03/10] fix(coding-agent): close compaction lifecycle races --- .../coding-agent/src/core/agent-session.ts | 397 ++++++++++-------- packages/coding-agent/src/core/changes.md | 14 +- .../src/core/compaction/changes.md | 24 ++ .../src/core/compaction/compaction.ts | 14 +- .../src/core/compaction/lifecycle.ts | 71 +++- .../src/core/extensions/changes.md | 3 + .../src/core/extensions/runner.ts | 8 +- .../coding-agent/src/core/extensions/types.ts | 7 +- .../compaction-feedback-lifecycle.test.ts | 175 +++++++- ...mpaction-generation-stale-revision.test.ts | 110 +++++ .../pre-prompt-compaction-no-continue.test.ts | 126 ++++++ 11 files changed, 771 insertions(+), 178 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index ed3c09f24..463289e2f 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -68,13 +68,7 @@ import { prepareCompaction, shouldCompact, } from "./compaction/index.ts"; -import { - beginCompactionOperation, - type CompactionLifecycleState, - finishCompactionOperation, - initialCompactionLifecycleState, - promoteCompactionOperation, -} from "./compaction/lifecycle.ts"; +import { CompactionLifecycleCoordinator, type CompactionLifecycleState } from "./compaction/lifecycle.ts"; import { DEFAULT_THINKING_LEVEL } from "./defaults.ts"; import { type BuildDynamicSystemPromptOptions, buildDynamicSystemPrompt } from "./dynamic-prompt/index.ts"; import { exportSessionToHtml, type ToolHtmlRenderer } from "./export-html/index.ts"; @@ -317,6 +311,8 @@ function describeCompactionRejection(cause: CompactionRejectionCause): string { return "Compaction rejected: the compaction circuit breaker is open after repeated failures. Wait for the cooldown and retry."; case "per-turn-cap": return "Compaction rejected: per-turn compaction cap reached for this turn."; + case "stale-revision": + return "Compaction rejected: the session changed while the summary was being prepared. Retry compaction against the latest context."; } } @@ -455,6 +451,13 @@ export class AgentSession { private _unsubscribeAgent?: () => void; private _eventListeners: AgentSessionEventListener[] = []; private _agentEventQueue: Promise = Promise.resolve(); + /** + * Exact message objects whose message_end persistence is still queued. + * Agent core appends messages to agent.state.messages before emitting + * message_end; until that event settles on _agentEventQueue, compaction must + * treat these identities as pending persistence, never as stale or droppable. + */ + private readonly _messageEndsAwaitingPersistence = new Set(); private _isAgentRunActive = false; private _idleWaitPromise: Promise | undefined; private _resolveIdleWait: (() => void) | undefined; @@ -469,11 +472,10 @@ export class AgentSession { // Compaction state private _compactionAbortController: AbortController | undefined = undefined; private _autoCompactionAbortController: AbortController | undefined = undefined; - private _compactionOperationController: AbortController | undefined = undefined; - private _compactionFeedbackController: AbortController | undefined = undefined; - private _compactionState: CompactionLifecycleState = initialCompactionLifecycleState(); + private readonly _compactionLifecycle = new CompactionLifecycleCoordinator(); private readonly _sessionWorkBarrier = new SessionWorkBarrier(); private _overflowRecoveryAttempted = false; + private _requiredCompactionAdmissionError: RequiredCompactionError | undefined; private _messageRevision = 0; // Branch summarization state @@ -756,22 +758,24 @@ export class AgentSession { ? async (_turn: PrepareNextTurnContext, signal?: AbortSignal) => await this.agent.prepareNextTurn?.(signal) : undefined); this.agent.prepareNextTurnWithContext = async (turn, signal) => { - let messages = turn.context.messages; - if (turn.toolResults.length > 0) { + let compacted = false; + // Enforce compaction only when this prepare precedes an actual provider + // admission: a tool continuation or queued steer/follow-up messages. A + // completed turn with no continuation keeps pre-PR timing, while the + // prior prepare callback and context refresh below still run every turn. + if (turn.toolResults.length > 0 || this.agent.hasQueuedMessages()) { await this._agentEventQueue; - const compacted = await this._checkCompaction(turn.message, true, "threshold"); - if (compacted) { - messages = this.agent.state.messages.slice(); - } else { - const settings = this.settingsManager.getCompactionSettings(); - const contextTokens = estimateContextTokens( - filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), - ).tokens; - if (settings.enabled && this.model && shouldCompact(contextTokens, this.model.contextWindow, settings)) { - throw new RequiredCompactionError(); + try { + compacted = await this._enforceCompactionBeforeProvider(turn.message, true, "threshold"); + } catch (error) { + if (error instanceof RequiredCompactionError && this.agent.hasQueuedMessages()) { + this._requiredCompactionAdmissionError = error; } + throw error; } } + const messages = compacted ? this.agent.state.messages.slice() : turn.context.messages; + const postCompactionTurn = { ...turn, context: { ...turn.context, messages }, @@ -879,9 +883,29 @@ export class AgentSession { private async _promptAgent(messages: AgentMessage | AgentMessage[]): Promise { this._isAgentRunActive = true; + this._requiredCompactionAdmissionError = undefined; try { await this.agent.prompt(messages); + const requiredCompactionError = this._requiredCompactionAdmissionError; + this._requiredCompactionAdmissionError = undefined; + if (requiredCompactionError) { + throw requiredCompactionError; + } } catch (error) { + if ( + error instanceof Error && + error.message === + "Agent is already processing a prompt. Use steer() or followUp() to queue messages, or wait for completion." + ) { + const queuedMessages = Array.isArray(messages) ? messages : [messages]; + for (const message of queuedMessages) this.agent.steer(message); + const userMessage = queuedMessages.find((message) => message.role === "user"); + if (userMessage?.role === "user") { + this._steeringMessages.push(this._extractUserMessageText(userMessage.content)); + this._emitQueueUpdate(); + } + return; + } await this._emitAgentSettled(); throw error; } @@ -899,10 +923,24 @@ export class AgentSession { // and waitForRetry() can miss the in-flight retry. this._createRetryPromiseForAgentEnd(event); - this._agentEventQueue = this._agentEventQueue.then( + // The message object is already in agent.state.messages when message_end + // fires; track its exact identity until this event's queued processing + // settles so compaction can distinguish pending persistence from stale state. + const pendingMessage = event.type === "message_end" ? event.message : undefined; + if (pendingMessage !== undefined) { + this._messageEndsAwaitingPersistence.add(pendingMessage); + } + + const processing = this._agentEventQueue.then( () => this._processAgentEvent(event), () => this._processAgentEvent(event), ); + this._agentEventQueue = + pendingMessage !== undefined + ? processing.finally(() => { + this._messageEndsAwaitingPersistence.delete(pendingMessage); + }) + : processing; // Keep queue alive if an event handler fails this._agentEventQueue.catch(() => {}); @@ -1520,7 +1558,7 @@ export class AgentSession { /** Whether compaction or branch summarization is currently running */ get isCompacting(): boolean { return ( - this._compactionState.status === "running" || + this._compactionLifecycle.state.status === "running" || this._autoCompactionAbortController !== undefined || this._compactionAbortController !== undefined || this._branchSummaryAbortController !== undefined @@ -1528,7 +1566,7 @@ export class AgentSession { } get compactionState(): Readonly { - return this._compactionState; + return this._compactionLifecycle.state; } /** All messages including custom types like BashExecutionMessage */ @@ -1847,21 +1885,8 @@ export class AgentSession { throw new Error(formatNoApiKeyFoundMessage(this.model.provider)); } - // Check if we need to compact before sending (catches aborted responses). // The user's new prompt is sent below, so do not call agent.continue() here. - const lastAssistant = this._findLastAssistantMessage(); - if (lastAssistant) { - const compacted = await this._checkCompaction(lastAssistant, false, "pre_prompt"); - if (!compacted && !this._isAssistantFromBeforeLatestCompaction(lastAssistant)) { - const settings = this.settingsManager.getCompactionSettings(); - const contextTokens = estimateContextTokens( - filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), - ).tokens; - if (settings.enabled && this.model && shouldCompact(contextTokens, this.model.contextWindow, settings)) { - throw new RequiredCompactionError(); - } - } - } + await this._enforceCompactionBeforeProvider(this._findLastAssistantMessage(), false, "pre_prompt"); // Build messages array (custom message if any, then user message) messages = []; @@ -2199,6 +2224,7 @@ export class AgentSession { this.agent.steer(appMessage); } } else if (options?.triggerTurn) { + await this._enforceCompactionBeforeProvider(this._findLastAssistantMessage(), false, "pre_prompt"); await this._promptAgent(appMessage); } else { this.agent.state.messages.push(appMessage); @@ -2718,7 +2744,7 @@ export class AgentSession { }); throw error; } finally { - if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + if (this._compactionAbortController === controller && this._compactionLifecycle.state.status !== "running") { this._compactionAbortController = undefined; } this._reconnectToAgent(); @@ -2764,28 +2790,27 @@ export class AgentSession { }); return { applied: false, reason: "rejected" }; } finally { - if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + if (this._compactionAbortController === controller && this._compactionLifecycle.state.status !== "running") { this._compactionAbortController = undefined; } - if (this._compactionFeedbackController === controller && this._compactionState.status !== "running") { - this._compactionFeedbackController = undefined; - } } } private _beginExtensionCompactionFeedback(reason: CompactionReason): AbortSignal { - let controller = - this._compactionOperationController ?? this._compactionAbortController ?? this._autoCompactionAbortController; - if (!controller) { - controller = new AbortController(); - this._compactionAbortController = controller; - this._compactionFeedbackController = controller; - this._emit({ type: "compaction_start", reason }); - } + const controller = new AbortController(); + this._compactionAbortController = controller; const model = this.model; - if (model) { - this._beginCompactionOperation(randomUUID(), reason, model, controller, "feedback"); - } + this._compactionLifecycle.begin( + { + operationId: randomUUID(), + stage: "feedback", + reason, + model: model ? { provider: model.provider, id: model.id } : undefined, + startedRevision: this._messageRevision, + }, + controller, + ); + this._emit({ type: "compaction_start", reason }); return controller.signal; } @@ -2795,8 +2820,7 @@ export class AgentSession { delta?: string; text?: string; }): void { - if (options.signal && this._compactionOperationController?.signal !== options.signal) return; - if (!this._compactionAbortController && !this._autoCompactionAbortController) return; + if (!options.signal || !this._compactionLifecycle.hasCurrentSignal(options.signal)) return; this._emit({ type: "compaction_progress", reason: options.reason, @@ -2811,11 +2835,18 @@ export class AgentSession { aborted?: boolean; errorMessage?: string; }): void { - const controller = - this._compactionOperationController ?? this._compactionAbortController ?? this._autoCompactionAbortController; - if (!controller) return; - if (options.signal && controller.signal !== options.signal) return; - const aborted = options.aborted ?? controller.signal.aborted; + if (!options.signal || !this._compactionLifecycle.hasCurrentSignal(options.signal)) return; + const operation = this._compactionLifecycle.state; + if (operation.status !== "running" || operation.stage !== "feedback") return; + const aborted = options.aborted ?? options.signal.aborted; + this._compactionLifecycle.finish({ + operationId: operation.operationId, + status: aborted ? "aborted" : "failed", + endedRevision: this._messageRevision, + ...(aborted + ? { errorMessage: "Compaction cancelled" } + : { errorMessage: options.errorMessage ?? "Compaction did not apply" }), + }); this._emit({ type: "compaction_end", reason: options.reason, @@ -2824,20 +2855,8 @@ export class AgentSession { willRetry: false, errorMessage: aborted ? undefined : options.errorMessage, }); - if ( - this._compactionState.status === "running" && - this._compactionOperationController === controller && - this._compactionState.stage === "feedback" - ) { - this._finishCompactionOperation(this._compactionState.operationId, aborted ? "aborted" : "failed", { - errorMessage: aborted ? "Compaction cancelled" : (options.errorMessage ?? "Compaction did not apply"), - }); - } - if (this._compactionFeedbackController === controller) { - this._compactionFeedbackController = undefined; - if (this._compactionAbortController === controller) { - this._compactionAbortController = undefined; - } + if (this._compactionAbortController?.signal === options.signal) { + this._compactionAbortController = undefined; } } @@ -2848,7 +2867,16 @@ export class AgentSession { const controller = this._compactionAbortController ?? this._autoCompactionAbortController; if (!controller) throw new Error("Compaction abort controller unavailable"); const requestId = randomUUID(); - const operationId = this._beginCompactionOperation(requestId, request.reason, model, controller, "execution"); + const operationId = this._compactionLifecycle.begin( + { + operationId: requestId, + stage: "execution", + reason: request.reason, + model: { provider: model.provider, id: model.id }, + startedRevision: this._messageRevision, + }, + controller, + ); const finishCompactionWork = this._sessionWorkBarrier.begin(); const agentMessagesAtStart = request.agentMessagesAtStart ?? this.agent.state.messages.slice(); const signal = controller.signal; @@ -2860,7 +2888,7 @@ export class AgentSession { let fromExtension = request.precomputed !== undefined; if (!compactionResult) { - const preparation = prepareCompaction(pathEntries, settings); + const preparation = prepareCompaction(pathEntries, settings, request.reason === "overflow"); if (!preparation) { const lastEntry = pathEntries[pathEntries.length - 1]; @@ -2883,7 +2911,10 @@ export class AgentSession { })) as SessionBeforeCompactResult | undefined; if (extensionResult?.cancel) { - this._finishCompactionOperation(operationId, "failed", { + this._compactionLifecycle.finish({ + operationId, + status: "failed", + endedRevision: this._messageRevision, rejectionCause: extensionResult.rejectionCause ?? "cancelled-by-extension", errorMessage: extensionResult.reason, }); @@ -2923,12 +2954,43 @@ export class AgentSession { if (signal.aborted) { throw new Error("Compaction cancelled"); } - if (!this._isCurrentCompactionOperation(operationId, controller)) { + if (!this._compactionLifecycle.isCurrent(operationId, controller)) { throw new DOMException("Compaction superseded", "AbortError"); } + const lifecycleState = this._compactionLifecycle.state; + const currentMessagesAtCheck = this.agent.state.messages; + const startPrefixIntact = agentMessagesAtStart.every( + (message, index) => currentMessagesAtCheck[index] === message, + ); + // Appends after the start snapshot are fresh only while they are exact + // message_end identities still awaiting persistence. Any revision change, + // other append, replacement, or reorder still makes this compaction stale. + const onlyPendingPersistenceAppends = + startPrefixIntact && + currentMessagesAtCheck + .slice(agentMessagesAtStart.length) + .every((message) => this._messageEndsAwaitingPersistence.has(message)); + const sourceChanged = + lifecycleState.status !== "running" || + lifecycleState.operationId !== operationId || + lifecycleState.startedRevision !== this._messageRevision || + !onlyPendingPersistenceAppends; + if (sourceChanged) { + this._compactionLifecycle.finish({ + operationId, + status: "failed", + endedRevision: this._messageRevision, + rejectionCause: "stale-revision", + }); + return await this._rejectCompaction(request, requestId, "stale-revision", false); + } + if (this._wouldCompactionOverflow(pathEntries, compactionResult, fromExtension, model)) { - this._finishCompactionOperation(operationId, "failed", { + this._compactionLifecycle.finish({ + operationId, + status: "failed", + endedRevision: this._messageRevision, rejectionCause: "would-overflow", }); return await this._rejectCompaction(request, requestId, "would-overflow", false); @@ -2954,19 +3016,24 @@ export class AgentSession { const messagesAppendedDuringCompaction = hasUnchangedPrefix ? currentAgentMessages.slice(agentMessagesAtStart.length) : []; - this.agent.state.messages = [...sessionContext.messages, ...messagesAppendedDuringCompaction]; + // Preserve an identity-deduped, agent-ordered union of the append-during- + // compaction suffix and exact messages still awaiting persistence. Their + // queued message_end owns exactly-once persistence, so they are kept in + // agent state only and never persisted here. + const preservedIdentities = new Set(messagesAppendedDuringCompaction); + for (const message of currentAgentMessages) { + if (this._messageEndsAwaitingPersistence.has(message)) { + preservedIdentities.add(message); + } + } + const preservedPendingMessages = currentAgentMessages.filter((message) => preservedIdentities.delete(message)); + this.agent.state.messages = [...sessionContext.messages, ...preservedPendingMessages]; compactionResult.estimatedTokensAfter = estimateMessagesTokens(sessionContext.messages); this._incrementMessageRevision(); - this._finishCompactionOperation(operationId, "completed"); - - await this._extensionRunner.emit({ - type: "session_compact", - reason: request.reason, - requestId, - accepted: true, - compactionEntry: savedEntry, - fromExtension, - willRetry: request.willRetry, + this._compactionLifecycle.finish({ + operationId, + status: "completed", + endedRevision: this._messageRevision, }); this._emit({ @@ -2979,10 +3046,23 @@ export class AgentSession { accepted: true, }); + await this._extensionRunner.emit({ + type: "session_compact", + reason: request.reason, + requestId, + accepted: true, + compactionEntry: savedEntry, + fromExtension, + willRetry: request.willRetry, + }); + return { accepted: true, requestId, result: compactionResult, compactionEntry: savedEntry, fromExtension }; } catch (error) { const aborted = signal.aborted || (error instanceof Error && error.name === "AbortError"); - this._finishCompactionOperation(operationId, aborted ? "aborted" : "failed", { + this._compactionLifecycle.finish({ + operationId, + status: aborted ? "aborted" : "failed", + endedRevision: this._messageRevision, errorMessage: error instanceof Error ? error.message : String(error), }); throw error; @@ -2991,61 +3071,6 @@ export class AgentSession { } } - private _beginCompactionOperation( - operationId: string, - reason: CompactionReason, - model: Model, - controller: AbortController, - stage: "feedback" | "execution", - ): string { - if (this._compactionState.status === "running") { - if (this._compactionOperationController === controller) { - const currentOperationId = this._compactionState.operationId; - if (stage === "execution") { - this._compactionState = promoteCompactionOperation(this._compactionState, currentOperationId); - } - return currentOperationId; - } - this._compactionOperationController?.abort(); - } - this._compactionOperationController = controller; - this._compactionState = beginCompactionOperation(this._compactionState, { - operationId, - stage, - reason, - model: { provider: model.provider, id: model.id }, - startedRevision: this._messageRevision, - }); - return operationId; - } - - private _isCurrentCompactionOperation(operationId: string, controller: AbortController): boolean { - return ( - this._compactionState.status === "running" && - this._compactionState.operationId === operationId && - this._compactionOperationController === controller && - !controller.signal.aborted - ); - } - - private _finishCompactionOperation( - operationId: string, - status: "completed" | "failed" | "aborted", - options: { rejectionCause?: CompactionRejectionCause; errorMessage?: string } = {}, - ): void { - const previous = this._compactionState; - const next = finishCompactionOperation(previous, { - operationId, - status, - endedRevision: this._messageRevision, - ...options, - }); - this._compactionState = next; - if (previous.status === "running" && previous.operationId === operationId && next !== previous) { - this._compactionOperationController = undefined; - } - } - private _wouldCompactionOverflow( pathEntries: SessionEntry[], compactionResult: CompactionResult, @@ -3076,6 +3101,25 @@ export class AgentSession { return contextTokens > model.contextWindow - settings.reserveTokens; } + /** + * Replaces agent state with the canonical session context while keeping exact + * message objects whose message_end persistence is still queued. Identities + * already present in the session context are kept from the context only, so + * nothing is duplicated; the queued message_end still owns exactly-once + * persistence for the rest. + */ + private _restoreAgentMessagesFromSession(): void { + const sessionMessages = this.sessionManager.buildSessionContext().messages; + const seen = new Set(sessionMessages); + const pendingMessages: AgentMessage[] = []; + for (const message of this.agent.state.messages) { + if (seen.has(message) || !this._messageEndsAwaitingPersistence.has(message)) continue; + seen.add(message); + pendingMessages.push(message); + } + this.agent.state.messages = [...sessionMessages, ...pendingMessages]; + } + private async _rejectCompaction( request: CompactionExecutionRequest, requestId: string, @@ -3121,21 +3165,19 @@ export class AgentSession { * Cancel in-progress compaction (manual or auto). */ abortCompaction(): void { - const feedbackController = this._compactionFeedbackController; + const feedbackOperation = this._compactionLifecycle.abort(this._messageRevision); this._compactionAbortController?.abort(); this._autoCompactionAbortController?.abort(); - this._compactionOperationController?.abort(); - if (this._compactionState.status === "running") { - this._finishCompactionOperation(this._compactionState.operationId, "aborted", { - errorMessage: "Compaction cancelled", + if (feedbackOperation?.stage === "feedback") { + this._compactionAbortController = undefined; + this._emit({ + type: "compaction_end", + reason: feedbackOperation.reason, + result: undefined, + aborted: true, + willRetry: false, }); } - if (feedbackController && this._compactionFeedbackController === feedbackController) { - this._compactionFeedbackController = undefined; - if (this._compactionAbortController === feedbackController) { - this._compactionAbortController = undefined; - } - } } /** @@ -3156,6 +3198,28 @@ export class AgentSession { * @param assistantMessage The assistant message to check * @param skipAbortedCheck If false, include aborted messages (for pre-prompt check). Default: true */ + private async _enforceCompactionBeforeProvider( + assistantMessage: AssistantMessage | undefined, + skipAbortedCheck: boolean, + inlineReason: "pre_prompt" | "threshold", + ): Promise { + const compacted = assistantMessage + ? await this._checkCompaction(assistantMessage, skipAbortedCheck, inlineReason) + : false; + if (compacted || (assistantMessage && this._isAssistantFromBeforeLatestCompaction(assistantMessage))) { + return compacted; + } + + const settings = this.settingsManager.getCompactionSettings(); + const contextTokens = estimateContextTokens( + filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), + ).tokens; + if (settings.enabled && this.model && shouldCompact(contextTokens, this.model.contextWindow, settings)) { + throw new RequiredCompactionError(); + } + return false; + } + private async _checkCompaction( assistantMessage: AssistantMessage, skipAbortedCheck = true, @@ -3235,7 +3299,7 @@ export class AgentSession { ? await this._runPrePromptCompaction(assistantMessage, skipAbortedCheck, "overflow", willRetry) : await this._runAutoCompaction("overflow", willRetry); if (!compacted && removedOverflowAssistant) { - this.agent.state.messages = this.sessionManager.buildSessionContext().messages; + this._restoreAgentMessagesFromSession(); this._incrementMessageRevision(); } if (!compacted && inlineReason) { @@ -3322,7 +3386,7 @@ export class AgentSession { }); return false; } finally { - if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + if (this._compactionAbortController === controller && this._compactionLifecycle.state.status !== "running") { this._compactionAbortController = undefined; } } @@ -3395,6 +3459,7 @@ export class AgentSession { const preparation = prepareCompaction( this.sessionManager.getBranch(), this.settingsManager.getCompactionSettings(), + reason === "overflow", ); if (!preparation) { if (reason === "overflow") this._overflowRecoveryAttempted = false; @@ -3754,7 +3819,10 @@ export class AgentSession { const err = error instanceof Error ? error : new Error(String(error)); options?.onError?.(err); } finally { - if (this._compactionAbortController === controller && this._compactionState.status !== "running") { + if ( + this._compactionAbortController === controller && + this._compactionLifecycle.state.status !== "running" + ) { this._compactionAbortController = undefined; } this._reconnectToAgent(); @@ -4555,9 +4623,8 @@ export class AgentSession { this.sessionManager.appendLabelChange(targetId, label); } - // Update agent state - const sessionContext = this.sessionManager.buildSessionContext(); - this.agent.state.messages = sessionContext.messages; + // Update agent state (preserving exact messages still awaiting persistence) + this._restoreAgentMessagesFromSession(); this._incrementMessageRevision(); // Emit session_tree event diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 166e5320a..0013a0fd2 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -4,11 +4,15 @@ ### What changed -- `agent-session.ts` now owns a monotonic compaction operation state, snapshots the active model and controller at - operation start, rejects stale completion/feedback, and retains the terminal result until another operation begins. -- Provider-confirmed overflow remains fail-closed when required pre-prompt compaction fails, even when the local token - estimate is below the configured threshold; failed recovery restores the overflow context so later prompts cannot - bypass the same requirement. +- `agent-session.ts` now holds a monotonic compaction lifecycle coordinator that snapshots the active model and + controller at operation start, rejects stale completion/feedback, and retains the terminal result until another + operation begins. Feedback-only aborts publish one terminal event, and accepted completions publish their terminal + event before `session_compact` handlers can begin a fresh operation. +- Durable append now rejects a generation whose message revision or agent-message snapshot changed during preparation + or summary generation (`stale-revision`), preserving intervening context without duplicate replay. +- Required compaction uses one provider-admission gate for normal prompts, extension-triggered turns, and every next + turn. Provider-confirmed overflow remains fail-closed even when the local token estimate is below the configured + threshold; failed recovery restores the overflow context so later prompts cannot bypass the same requirement. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/compaction/changes.md b/packages/coding-agent/src/core/compaction/changes.md index 3568fe34b..ffedb6b3c 100644 --- a/packages/coding-agent/src/core/compaction/changes.md +++ b/packages/coding-agent/src/core/compaction/changes.md @@ -1,5 +1,29 @@ # changes.md — compaction +## Lifecycle ownership and required-admission safety (2026-07-23) + +### What changed + +- `lifecycle.ts` now owns the active compaction controller together with reducer transitions, so feedback from an older + generation cannot progress or terminate a newer one. Feedback-only cancellation emits one terminal + `compaction_end`, and accepted compactions emit their terminal event before `session_compact` handlers can start + another generation. +- Extension contexts retain the signal returned by `beginCompaction()` and supply it to legacy `updateCompaction()` / + `endCompaction()` calls that omit one. Core accepts feedback mutations only from the current signal. +- Provider admissions now share one required-compaction gate for prompt preflight, extension-triggered turns, and + next turns. Provider-confirmed overflow can force a split-turn preparation when keeping the only oversized prompt + would otherwise leave no compactable source. +- Compaction rejects stale source snapshots with `stale-revision` before the durable entry append. + +### Why + +A late extension completion could overwrite fresh feedback, and some continuation routes skipped required compaction. +Compacting a source that changed during summary generation could also append a stale checkpoint over intervening work. + +### Expected merge conflict zones + +- LOW: `lifecycle.ts` and the compaction admission calls in `agent-session.ts`. + ## Operation lifecycle reducer (2026-07-23) ### What changed diff --git a/packages/coding-agent/src/core/compaction/compaction.ts b/packages/coding-agent/src/core/compaction/compaction.ts index d282254d7..ba7ccdea0 100644 --- a/packages/coding-agent/src/core/compaction/compaction.ts +++ b/packages/coding-agent/src/core/compaction/compaction.ts @@ -752,6 +752,7 @@ export interface CompactionPreparation { export function prepareCompaction( pathEntries: SessionEntry[], settings: CompactionSettings, + forceProgress = false, ): CompactionPreparation | undefined { if (pathEntries.length > 0 && pathEntries[pathEntries.length - 1].type === "compaction") { return undefined; @@ -779,7 +780,18 @@ export function prepareCompaction( filterContextExcludedMessages(buildSessionContext(pathEntries).messages), ).tokens; - const cutPoint = findCutPoint(pathEntries, boundaryStart, boundaryEnd, settings.keepRecentTokens); + let cutPoint = findCutPoint(pathEntries, boundaryStart, boundaryEnd, settings.keepRecentTokens); + if (forceProgress && cutPoint.firstKeptEntryIndex === boundaryStart) { + const nextCutPoint = findValidCutPoints(pathEntries, boundaryStart + 1, boundaryEnd)[0]; + if (nextCutPoint !== undefined) { + const turnStartIndex = findTurnStartIndex(pathEntries, nextCutPoint, boundaryStart); + cutPoint = { + firstKeptEntryIndex: nextCutPoint, + turnStartIndex, + isSplitTurn: turnStartIndex !== -1, + }; + } + } // Get UUID of first kept entry const firstKeptEntry = pathEntries[cutPoint.firstKeptEntryIndex]; diff --git a/packages/coding-agent/src/core/compaction/lifecycle.ts b/packages/coding-agent/src/core/compaction/lifecycle.ts index 79ba3fe42..277399ed7 100644 --- a/packages/coding-agent/src/core/compaction/lifecycle.ts +++ b/packages/coding-agent/src/core/compaction/lifecycle.ts @@ -10,7 +10,7 @@ interface CompactionOperation { readonly operationId: string; readonly stage: "feedback" | "execution"; readonly reason: CompactionReason; - readonly model: CompactionModelRef; + readonly model?: CompactionModelRef; readonly startedRevision: number; } @@ -37,7 +37,7 @@ export interface BeginCompactionOperation { readonly operationId: string; readonly stage: "feedback" | "execution"; readonly reason: CompactionReason; - readonly model: CompactionModelRef; + readonly model?: CompactionModelRef; readonly startedRevision: number; } @@ -105,3 +105,70 @@ export function finishCompactionOperation( errorMessage: event.errorMessage, }; } + +/** + * Couples lifecycle reducer transitions with the controller that owns the + * active generation. AgentSession owns event emission and durable work; this + * coordinator only protects state transitions from stale callers. + */ +export class CompactionLifecycleCoordinator { + private _state: CompactionLifecycleState = initialCompactionLifecycleState(); + private _controller: AbortController | undefined; + + get state(): CompactionLifecycleState { + return this._state; + } + + begin(operation: BeginCompactionOperation, controller: AbortController): string { + if (this._state.status === "running") { + if (this._controller === controller) { + const runningOperationId = this._state.operationId; + if (operation.stage === "execution") { + this._state = promoteCompactionOperation(this._state, runningOperationId); + } + return runningOperationId; + } + this._controller?.abort(); + } + + this._controller = controller; + this._state = beginCompactionOperation(this._state, operation); + return operation.operationId; + } + + isCurrent(operationId: string, controller: AbortController): boolean { + return ( + this._state.status === "running" && + this._state.operationId === operationId && + this._controller === controller && + !controller.signal.aborted + ); + } + + hasCurrentSignal(signal: AbortSignal): boolean { + return this._state.status === "running" && this._controller?.signal === signal && !signal.aborted; + } + + finish(event: FinishCompactionOperation): boolean { + const next = finishCompactionOperation(this._state, event); + if (next === this._state) return false; + this._state = next; + this._controller = undefined; + return true; + } + + abort( + endedRevision: number, + ): { readonly reason: CompactionReason; readonly stage: "feedback" | "execution" } | undefined { + if (this._state.status !== "running") return undefined; + const { operationId, reason, stage } = this._state; + this._controller?.abort(); + this.finish({ + operationId, + status: "aborted", + endedRevision, + errorMessage: "Compaction cancelled", + }); + return { reason, stage }; + } +} diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index 43fc6de55..a2177cbe9 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -6,6 +6,9 @@ - `ExtensionContext` compaction feedback actions now return and accept an optional operation `AbortSignal`, allowing progress and terminal feedback from superseded generations to be ignored without breaking existing extensions. + A context remembers its `beginCompaction()` signal and supplies it to legacy `updateCompaction()` and + `endCompaction()` calls that omit the signal, so an old context cannot end a newer operation. +- `stale-revision` is a structured compaction rejection cause for a source that changed before durable append. - The builtin compaction extension threads that signal through local and remote summary generation and application. ### Why diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts index ae4f92f3f..df86c332f 100644 --- a/packages/coding-agent/src/core/extensions/runner.ts +++ b/packages/coding-agent/src/core/extensions/runner.ts @@ -902,6 +902,7 @@ export class ExtensionRunner { const runner = this; const getModel = this.getModel; const getServiceTier = this.getServiceTier; + let compactionSignal: AbortSignal | undefined; return { get ui() { runner.assertActive(); @@ -993,15 +994,16 @@ export class ExtensionRunner { }, beginCompaction: (options) => { runner.assertActive(); - return runner.beginCompactionFn?.(options); + compactionSignal = runner.beginCompactionFn?.(options); + return compactionSignal; }, updateCompaction: (options) => { runner.assertActive(); - runner.updateCompactionFn?.(options); + runner.updateCompactionFn?.({ ...options, signal: options.signal ?? compactionSignal }); }, endCompaction: (options) => { runner.assertActive(); - runner.endCompactionFn?.(options); + runner.endCompactionFn?.({ ...options, signal: options.signal ?? compactionSignal }); }, getMessageRevision: () => { runner.assertActive(); diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index 5b71325f3..779402181 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -91,7 +91,12 @@ export type { AgentToolResult, AgentToolUpdateCallback, ToolExecutionMode }; export type ServiceTier = "auto" | "flex" | "priority"; // biome-ignore format: keep literal union alias consistent with nearby ServiceTier style. export type CompactionReason = "manual" | "threshold" | "overflow" | "pre_prompt" | "branch" | "extension"; -export type CompactionRejectionCause = "cancelled-by-extension" | "would-overflow" | "circuit-breaker" | "per-turn-cap"; +export type CompactionRejectionCause = + | "cancelled-by-extension" + | "would-overflow" + | "circuit-breaker" + | "per-turn-cap" + | "stale-revision"; // ============================================================================ // UI Context diff --git a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts index 3b5a7bbad..12e765f09 100644 --- a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts +++ b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts @@ -1,5 +1,7 @@ +import { fauxAssistantMessage } from "@earendil-works/pi-ai"; import { afterEach, describe, expect, it } from "vitest"; -import type { CompactionReason } from "../../src/core/extensions/types.ts"; +import type { ExtensionAPI } from "../../src/core/extensions/index.ts"; +import type { CompactionReason, ExtensionContext } from "../../src/core/extensions/types.ts"; import { createHarness, type Harness } from "./harness.ts"; type BeginFeedback = (reason: CompactionReason) => AbortSignal; @@ -16,6 +18,42 @@ type EndFeedback = (options: { errorMessage?: string; }) => void; +interface PostApplyFeedbackCapture { + firstContext?: ExtensionContext; + firstSignal?: AbortSignal; + secondSignal?: AbortSignal; +} + +/** + * Drives the real extension surface: feedback begun in before_agent_start is + * applied, and the accepted session_compact handler immediately begins the + * next feedback operation (the builtin speculative path does this). + */ +function createPostApplyFeedbackExtension(capture: PostApplyFeedbackCapture) { + return (pi: ExtensionAPI): void => { + pi.on("before_agent_start", async (_event, ctx) => { + if (capture.firstSignal) return undefined; + const firstEntry = ctx.sessionManager.getEntries()[0]; + if (!firstEntry) return undefined; + capture.firstSignal = ctx.beginCompaction?.({ reason: "extension" }); + capture.firstContext = ctx; + await ctx.applyCompaction( + { + summary: "applied before the next feedback operation", + firstKeptEntryId: firstEntry.id, + tokensBefore: 42, + }, + { reason: "extension", expectedRevision: ctx.getMessageRevision() }, + ); + return undefined; + }); + pi.on("session_compact", (event, ctx) => { + if (!event.accepted) return; + capture.secondSignal = ctx.beginCompaction?.({ reason: "extension" }); + }); + }; +} + describe("compaction feedback lifecycle", () => { const harnesses: Harness[] = []; @@ -115,4 +153,139 @@ describe("compaction feedback lifecycle", () => { errorMessage: "current failure", }); }); + + it("emits exactly one aborted compaction_end when abortCompaction cancels feedback-only compaction", async () => { + const harness = await createHarness(); + harnesses.push(harness); + const begin = Reflect.get(harness.session, "_beginExtensionCompactionFeedback"); + const end = Reflect.get(harness.session, "_endExtensionCompactionFeedback"); + if (typeof begin !== "function" || typeof end !== "function") { + throw new Error("Compaction feedback lifecycle methods unavailable"); + } + + const signal = (begin as BeginFeedback).call(harness.session, "extension"); + expect(harness.eventsOfType("compaction_start")).toHaveLength(1); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 1, + stage: "feedback", + }); + + harness.session.abortCompaction(); + + expect(signal.aborted).toBe(true); + expect(harness.session.compactionState).toMatchObject({ status: "aborted", generation: 1 }); + expect(harness.session.isCompacting).toBe(false); + expect(harness.eventsOfType("compaction_end")).toEqual([ + expect.objectContaining({ reason: "extension", aborted: true }), + ]); + + // The owning extension's late end after the abort must not emit a second public end. + (end as EndFeedback).call(harness.session, { reason: "extension", signal, aborted: true }); + expect(harness.eventsOfType("compaction_end")).toHaveLength(1); + }); + + it("gives feedback begun from session_compact a distinct controller and its own compaction_start", async () => { + const capture: PostApplyFeedbackCapture = {}; + const harness = await createHarness({ extensionFactories: [createPostApplyFeedbackExtension(capture)] }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("one"), fauxAssistantMessage("two")]); + await harness.session.prompt("one"); + await harness.session.prompt("two"); + + expect(capture.secondSignal).toBeDefined(); + expect(capture.secondSignal?.aborted).toBe(false); + expect(capture.secondSignal).not.toBe(capture.firstSignal); + expect(harness.eventsOfType("compaction_start")).toHaveLength(2); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + expect(harness.session.isCompacting).toBe(true); + }); + + it("keeps post-apply feedback running when the superseded operation ends", async () => { + const capture: PostApplyFeedbackCapture = {}; + const harness = await createHarness({ extensionFactories: [createPostApplyFeedbackExtension(capture)] }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("one"), fauxAssistantMessage("two")]); + await harness.session.prompt("one"); + await harness.session.prompt("two"); + + expect(capture.secondSignal).toBeDefined(); + expect(harness.session.compactionState).toMatchObject({ status: "running", generation: 2 }); + const endCountBeforeLateEnd = harness.eventsOfType("compaction_end").length; + + // The old operation's terminal end must not terminate the newer feedback operation. + capture.firstContext?.endCompaction?.({ + reason: "extension", + signal: capture.firstSignal, + errorMessage: "superseded operation ended late", + }); + + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + expect(harness.session.isCompacting).toBe(true); + expect(harness.eventsOfType("compaction_end")).toHaveLength(endCountBeforeLateEnd); + }); + + it("binds an omitted endCompaction signal to the operation begun by the same context", async () => { + const contexts: ExtensionContext[] = []; + const harness = await createHarness({ + extensionFactories: [ + (pi: ExtensionAPI) => { + pi.on("agent_settled", (_event, ctx) => { + contexts.push(ctx); + }); + }, + ], + }); + harnesses.push(harness); + const runner = harness.getExtensionRunner(); + + await runner.emit({ type: "agent_settled" }); + const legacyContext = contexts[0]; + if (!legacyContext) throw new Error("Expected legacy extension context"); + const legacySignal = legacyContext.beginCompaction?.({ reason: "extension" }); + expect(legacySignal?.aborted).toBe(false); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 1, + stage: "feedback", + }); + + legacyContext.endCompaction?.({ + reason: "extension", + signal: legacySignal, + errorMessage: "legacy work finished", + }); + expect(harness.session.compactionState).toMatchObject({ status: "failed", generation: 1 }); + expect(harness.eventsOfType("compaction_end")).toHaveLength(1); + + await runner.emit({ type: "agent_settled" }); + const newerContext = contexts[1]; + if (!newerContext) throw new Error("Expected newer extension context"); + const newerSignal = newerContext.beginCompaction?.({ reason: "extension" }); + expect(newerSignal?.aborted).toBe(false); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + + // Legacy context ends without an explicit signal; it must not terminate the newer operation. + legacyContext.endCompaction?.({ reason: "extension" }); + + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + expect(harness.session.isCompacting).toBe(true); + expect(harness.eventsOfType("compaction_end")).toHaveLength(1); + }); }); diff --git a/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts b/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts new file mode 100644 index 000000000..617b26086 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts @@ -0,0 +1,110 @@ +import { fauxAssistantMessage } from "@earendil-works/pi-ai"; +import { afterEach, describe, expect, it } from "vitest"; +import type { ExtensionAPI } from "../../../src/core/extensions/index.ts"; +import { createHarness, getMessageText, type Harness } from "../harness.ts"; + +type Deferred = { + readonly promise: Promise; + readonly resolve: () => void; +}; + +function createDeferred(): Deferred { + let resolve: (() => void) | undefined; + const promise = new Promise((next) => { + resolve = next; + }); + if (!resolve) throw new Error("Deferred resolver was not initialized"); + return { promise, resolve }; +} + +function compactionEntryCount(harness: Harness): number { + return harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction").length; +} + +function agentMessagesContaining(harness: Harness, text: string): number { + return harness.session.messages.filter((message) => getMessageText(message).includes(text)).length; +} + +async function appendMidCompactionMessage(harness: Harness): Promise { + await harness.session.sendCustomMessage({ + customType: "mid-compaction-note", + content: "arrived mid-compaction", + display: true, + }); +} + +describe("Regression: stale compaction generation after a revision change", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) { + harnesses.pop()?.cleanup(); + } + }); + + it("rejects stale compaction when the session changes during extension preparation", async () => { + const preparationStarted = createDeferred(); + const releasePreparation = createDeferred(); + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 128_000, maxTokens: 64 }], + settings: { compaction: { enabled: true, reserveTokens: 16_384, keepRecentTokens: 1 } }, + extensionFactories: [ + (pi: ExtensionAPI) => { + pi.on("session_before_compact", async (event) => { + preparationStarted.resolve(); + await releasePreparation.promise; + return { + compaction: { + summary: "summary generated from a stale branch", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + }, + }; + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("initial assistant")]); + await harness.session.prompt("initial prompt ".repeat(40)); + + const compactPromise = harness.session.compact(); + await preparationStarted.promise; + await appendMidCompactionMessage(harness); + releasePreparation.resolve(); + + await expect(compactPromise).rejects.toThrow(); + expect(compactionEntryCount(harness)).toBe(0); + expect(agentMessagesContaining(harness, "arrived mid-compaction")).toBe(1); + expect(harness.eventsOfType("compaction_end").filter((event) => event.accepted === true)).toHaveLength(0); + }); + + it("rejects stale compaction when the session changes during summary generation", async () => { + const generationStarted = createDeferred(); + const releaseGeneration = createDeferred(); + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 128_000, maxTokens: 64 }], + settings: { compaction: { enabled: true, reserveTokens: 16_384, keepRecentTokens: 1 } }, + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage("initial assistant"), + async () => { + generationStarted.resolve(); + await releaseGeneration.promise; + return fauxAssistantMessage("provider generated summary"); + }, + ]); + await harness.session.prompt("initial prompt ".repeat(40)); + + const compactPromise = harness.session.compact(); + await generationStarted.promise; + await appendMidCompactionMessage(harness); + releaseGeneration.resolve(); + + await expect(compactPromise).rejects.toThrow(); + expect(compactionEntryCount(harness)).toBe(0); + expect(agentMessagesContaining(harness, "arrived mid-compaction")).toBe(1); + expect(harness.eventsOfType("compaction_end").filter((event) => event.accepted === true)).toHaveLength(0); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts index bef47473a..7e2758ffe 100644 --- a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts +++ b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts @@ -13,6 +13,15 @@ function createUsage(totalTokens: number) { }; } +function createDeferred(): { promise: Promise; resolve: () => void } { + let resolve: (() => void) | undefined; + const promise = new Promise((next) => { + resolve = next; + }); + if (!resolve) throw new Error("Deferred resolver was not initialized"); + return { promise, resolve }; +} + describe("pre-prompt compaction regression", () => { const harnesses: Harness[] = []; @@ -213,4 +222,121 @@ describe("pre-prompt compaction regression", () => { expect(getUserTexts(harness)).toContain("."); expect(harness.faux.state.callCount).toBe(1); }); + + it("blocks a queued steer continuation when overflow compaction is rejected", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "forced rejection", + })); + }, + ], + }); + harnesses.push(harness); + + const firstTurnStarted = createDeferred(); + const releaseFirstTurn = createDeferred(); + harness.setResponses([ + async () => { + firstTurnStarted.resolve(); + await releaseFirstTurn.promise; + // Provider-confirmed overflow: native length stop with zero output and + // a full context window (the 40_000-char prompt fills the 10_000 window). + return fauxAssistantMessage("", { stopReason: "length" }); + }, + fauxAssistantMessage("must not reach provider"), + ]); + + const promptPromise = harness.session.prompt("x".repeat(40_000)); + await firstTurnStarted.promise; + const steerPromise = harness.session.prompt("steered follow-up", { streamingBehavior: "steer" }); + releaseFirstTurn.resolve(); + + // The steered continuation must reject at turn admission instead of + // reaching the provider with the still-overflowing context. + await expect(promptPromise).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + await steerPromise; + expect(harness.faux.state.callCount).toBe(1); + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "overflow", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + }); + + it("blocks sendCustomMessage triggerTurn when overflow compaction is rejected below the local threshold", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "forced rejection", + })); + }, + ], + }); + harnesses.push(harness); + + const now = Date.now(); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "earlier prompt" }], + timestamp: now - 3000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("earlier response", { timestamp: now - 2000 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(50), + }); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "previous prompt" }], + timestamp: now - 1000, + }); + const overflowAssistant: AssistantMessage = { + ...fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "context_length_exceeded", + timestamp: now - 500, + }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(100), + }; + harness.sessionManager.appendMessage(overflowAssistant); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([fauxAssistantMessage("must not reach provider")]); + + await expect( + harness.session.sendCustomMessage( + { customType: "extension-note", content: "trigger a turn", display: true }, + { triggerTurn: true }, + ), + ).rejects.toThrow("Context remains above the compaction threshold because compaction did not complete"); + + expect(harness.faux.state.callCount).toBe(0); + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "overflow", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + }); }); From 2092861f6f081f19fcb55570aed9a39b39be1d85 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 24 Jul 2026 03:28:40 +0900 Subject: [PATCH 04/10] fix(coding-agent): gate overflow continuations --- .../coding-agent/src/core/agent-session.ts | 209 +++++++++++++----- packages/coding-agent/src/core/changes.md | 4 +- .../src/core/compaction/changes.md | 5 +- .../src/core/extensions/changes.md | 5 +- .../src/core/extensions/runner.ts | 55 ++--- .../compaction-feedback-lifecycle.test.ts | 131 +++++++++++ .../pre-prompt-compaction-no-continue.test.ts | 130 +++++++++++ 7 files changed, 451 insertions(+), 88 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 463289e2f..a7b80bbe7 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -330,6 +330,42 @@ class CompactionRejectedError extends Error { } } +class CompactionCancelledError extends Error { + constructor() { + super("Compaction cancelled"); + this.name = "CompactionCancelledError"; + } +} + +/** + * An execution failure annotated with whether this operation still owns its + * terminal transition. Callers must not publish a terminal event for an + * operation that a newer compaction generation has superseded. + */ +class CompactionExecutionError extends Error { + readonly ownsTerminalTransition: boolean; + readonly aborted: boolean; + + constructor(error: unknown, ownsTerminalTransition: boolean, aborted: boolean) { + super(error instanceof Error ? error.message : String(error)); + this.name = "CompactionExecutionError"; + this.ownsTerminalTransition = ownsTerminalTransition; + this.aborted = aborted; + } +} + +function compactionExecutionOwnsTerminalTransition(error: unknown): boolean { + return !(error instanceof CompactionExecutionError) || error.ownsTerminalTransition; +} + +function isCompactionExecutionAborted(error: unknown): boolean { + return ( + (error instanceof CompactionExecutionError && error.aborted) || + error instanceof CompactionCancelledError || + (error instanceof Error && error.name === "AbortError") + ); +} + class RequiredCompactionError extends Error { constructor() { super("Context remains above the compaction threshold because compaction did not complete"); @@ -763,17 +799,28 @@ export class AgentSession { // admission: a tool continuation or queued steer/follow-up messages. A // completed turn with no continuation keeps pre-PR timing, while the // prior prepare callback and context refresh below still run every turn. - if (turn.toolResults.length > 0 || this.agent.hasQueuedMessages()) { + const compactBeforeNextAdmission = async (): Promise => { + if (turn.toolResults.length === 0 && !this.agent.hasQueuedMessages()) { + return false; + } await this._agentEventQueue; + // A queue can be cleared while waiting for persistence. Re-sample it + // immediately before compaction so a completed turn never compacts + // merely because it once had a possible continuation. + if (turn.toolResults.length === 0 && !this.agent.hasQueuedMessages()) { + return false; + } try { - compacted = await this._enforceCompactionBeforeProvider(turn.message, true, "threshold"); + return await this._enforceCompactionBeforeProvider(turn.message, true, "threshold"); } catch (error) { if (error instanceof RequiredCompactionError && this.agent.hasQueuedMessages()) { this._requiredCompactionAdmissionError = error; } throw error; } - } + }; + + compacted = await compactBeforeNextAdmission(); const messages = compacted ? this.agent.state.messages.slice() : turn.context.messages; const postCompactionTurn = { @@ -782,16 +829,21 @@ export class AgentSession { }; const previousSnapshot = await previousPrepareNextTurnWithContext?.(postCompactionTurn, signal); const previousContext = previousSnapshot?.context ?? postCompactionTurn.context; + // The previous callback may await while agent_end extensions enqueue + // continuation work. Re-sample after it returns so that work cannot + // slip through with the stale provider snapshot it observed on entry. + if (!compacted) { + compacted = await compactBeforeNextAdmission(); + } return { ...previousSnapshot, context: { ...previousContext, - systemPrompt: this._systemPromptOverride ?? this._baseSystemPrompt, - tools: this.agent.state.tools.slice(), + messages: compacted ? this.agent.state.messages.slice() : previousContext.messages, }, - model: this.agent.state.model, - thinkingLevel: this.agent.state.thinkingLevel, + model: previousSnapshot?.model ?? this.agent.state.model, + thinkingLevel: previousSnapshot?.thinkingLevel ?? this.agent.state.thinkingLevel, }; }; } @@ -886,6 +938,10 @@ export class AgentSession { this._requiredCompactionAdmissionError = undefined; try { await this.agent.prompt(messages); + // AgentSession's subscriber intentionally queues event work instead of + // blocking Agent core. Wait for this run's queued recovery decision + // before reporting prompt completion to the caller. + await this._agentEventQueue; const requiredCompactionError = this._requiredCompactionAdmissionError; this._requiredCompactionAdmissionError = undefined; if (requiredCompactionError) { @@ -916,6 +972,19 @@ export class AgentSession { /** Internal handler for agent events - shared by subscribe and reconnect */ private _handleAgentEvent = (event: AgentEvent): void => { + // Agent core drains native steer/follow-up queues immediately after its + // final agent_end. This subscriber intentionally processes its own event + // queue asynchronously, so a later recovery rejection cannot abort that + // drain in time. agent_end itself is still an awaited synchronous boundary + // before that drain: transfer provider-confirmed overflow ownership there, + // retaining queues until AgentSession accepts recovery. + if (event.type === "agent_end") { + const lastAssistant = this._findLastAssistantInMessages(event.messages); + if (lastAssistant && this._requiresOverflowRecovery(lastAssistant)) { + this.agent.abort(); + } + } + // Create retry promise synchronously before queueing async processing. // Agent.emit() calls this handler synchronously, and prompt() calls waitForRetry() // as soon as agent.prompt() resolves. If _retryPromise is created only inside @@ -979,6 +1048,16 @@ export class AgentSession { return undefined; } + private _requiresOverflowRecovery(message: AssistantMessage): boolean { + const model = this.model; + return ( + model !== undefined && + (message.stopReason === "error" || message.stopReason === "length") && + isContextOverflow(message, model.contextWindow) && + isSameOverflowSource(message, model, this._modelRuntime.getCompatibilityRequestConfig(model).upstreamModelId) + ); + } + private _agentEndAllowsQueuedContinuation(messages: AgentMessage[]): boolean { let lastAssistantIndex = -1; for (let index = messages.length - 1; index >= 0; index--) { @@ -998,6 +1077,9 @@ export class AgentSession { if (lastAssistant.stopReason === "aborted" || lastAssistant.stopReason === "error") { return false; } + if (this._requiresOverflowRecovery(lastAssistant)) { + return false; + } for (let index = lastAssistantIndex + 1; index < messages.length; index++) { const message = messages[index]; @@ -1149,6 +1231,7 @@ export class AgentSession { if (event.type === "agent_end" && this._lastAssistantMessage) { const msg = this._lastAssistantMessage; this._lastAssistantMessage = undefined; + const requiredOverflowRecovery = this._requiresOverflowRecovery(msg); // Retry transient failures normally and eligible hard errors only through a fallback. const retryableError = this._isRetryableError(msg); @@ -1162,6 +1245,9 @@ export class AgentSession { this._resolveRetry(); launchedContinuation = await this._checkCompaction(msg); + if (requiredOverflowRecovery && !launchedContinuation) { + this._requiredCompactionAdmissionError = new RequiredCompactionError(); + } } if (event.type === "agent_end") { @@ -2732,8 +2818,11 @@ export class AgentSession { if (error instanceof CompactionRejectedError) { throw new Error(error.message); } + if (!compactionExecutionOwnsTerminalTransition(error)) { + throw error; + } const message = error instanceof Error ? error.message : String(error); - const aborted = message === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError"); + const aborted = isCompactionExecutionAborted(error); this._emit({ type: "compaction_end", reason: "manual", @@ -2778,8 +2867,11 @@ export class AgentSession { } return { applied: true, reason: "ok" }; } catch (error) { + if (!compactionExecutionOwnsTerminalTransition(error)) { + return { applied: false, reason: "rejected" }; + } const message = error instanceof Error ? error.message : String(error); - const aborted = message === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError"); + const aborted = isCompactionExecutionAborted(error); this._emit({ type: "compaction_end", reason: options.reason, @@ -2910,17 +3002,15 @@ export class AgentSession { signal, })) as SessionBeforeCompactResult | undefined; + if (!this._compactionLifecycle.isCurrent(operationId, controller)) { + throw new CompactionCancelledError(); + } + if (extensionResult?.cancel) { - this._compactionLifecycle.finish({ - operationId, - status: "failed", - endedRevision: this._messageRevision, - rejectionCause: extensionResult.rejectionCause ?? "cancelled-by-extension", - errorMessage: extensionResult.reason, - }); return await this._rejectCompaction( request, requestId, + operationId, extensionResult.rejectionCause ?? "cancelled-by-extension", true, extensionResult.reason, @@ -2952,10 +3042,10 @@ export class AgentSession { } if (signal.aborted) { - throw new Error("Compaction cancelled"); + throw new CompactionCancelledError(); } if (!this._compactionLifecycle.isCurrent(operationId, controller)) { - throw new DOMException("Compaction superseded", "AbortError"); + throw new CompactionCancelledError(); } const lifecycleState = this._compactionLifecycle.state; @@ -2977,23 +3067,11 @@ export class AgentSession { lifecycleState.startedRevision !== this._messageRevision || !onlyPendingPersistenceAppends; if (sourceChanged) { - this._compactionLifecycle.finish({ - operationId, - status: "failed", - endedRevision: this._messageRevision, - rejectionCause: "stale-revision", - }); - return await this._rejectCompaction(request, requestId, "stale-revision", false); + return await this._rejectCompaction(request, requestId, operationId, "stale-revision", false); } if (this._wouldCompactionOverflow(pathEntries, compactionResult, fromExtension, model)) { - this._compactionLifecycle.finish({ - operationId, - status: "failed", - endedRevision: this._messageRevision, - rejectionCause: "would-overflow", - }); - return await this._rejectCompaction(request, requestId, "would-overflow", false); + return await this._rejectCompaction(request, requestId, operationId, "would-overflow", false); } const compactionEntryId = this.sessionManager.appendCompaction( @@ -3030,11 +3108,15 @@ export class AgentSession { this.agent.state.messages = [...sessionContext.messages, ...preservedPendingMessages]; compactionResult.estimatedTokensAfter = estimateMessagesTokens(sessionContext.messages); this._incrementMessageRevision(); - this._compactionLifecycle.finish({ - operationId, - status: "completed", - endedRevision: this._messageRevision, - }); + if ( + !this._compactionLifecycle.finish({ + operationId, + status: "completed", + endedRevision: this._messageRevision, + }) + ) { + throw new CompactionCancelledError(); + } this._emit({ type: "compaction_end", @@ -3058,14 +3140,21 @@ export class AgentSession { return { accepted: true, requestId, result: compactionResult, compactionEntry: savedEntry, fromExtension }; } catch (error) { - const aborted = signal.aborted || (error instanceof Error && error.name === "AbortError"); - this._compactionLifecycle.finish({ - operationId, - status: aborted ? "aborted" : "failed", - endedRevision: this._messageRevision, - errorMessage: error instanceof Error ? error.message : String(error), - }); - throw error; + if (error instanceof CompactionExecutionError) { + throw error; + } + const lifecycleState = this._compactionLifecycle.state; + const ownsTerminalTransition = lifecycleState.status !== "idle" && lifecycleState.operationId === operationId; + const aborted = signal.aborted || isCompactionExecutionAborted(error); + if (ownsTerminalTransition && lifecycleState.status === "running") { + this._compactionLifecycle.finish({ + operationId, + status: aborted ? "aborted" : "failed", + endedRevision: this._messageRevision, + errorMessage: error instanceof Error ? error.message : String(error), + }); + } + throw new CompactionExecutionError(error, ownsTerminalTransition, aborted); } finally { finishCompactionWork(); } @@ -3123,6 +3212,7 @@ export class AgentSession { private async _rejectCompaction( request: CompactionExecutionRequest, requestId: string, + operationId: string, rejectionCause: CompactionRejectionCause, aborted: boolean, extensionReason?: string, @@ -3138,6 +3228,17 @@ export class AgentSession { ? `Compaction rejected: ${trimmedExtensionReason}` : describeCompactionRejection(rejectionCause); const errorMessage = aborted && !trimmedExtensionReason ? undefined : detailedMessage; + if ( + !this._compactionLifecycle.finish({ + operationId, + status: "failed", + endedRevision: this._messageRevision, + rejectionCause, + ...(trimmedExtensionReason !== undefined ? { errorMessage: trimmedExtensionReason } : {}), + }) + ) { + throw new CompactionCancelledError(); + } this._emit({ type: "compaction_end", reason: request.reason, @@ -3370,12 +3471,14 @@ export class AgentSession { } return execution.accepted; } catch (error) { + if (!compactionExecutionOwnsTerminalTransition(error)) { + return false; + } if (isContextOverflow(lastAssistantMessage, this.model?.contextWindow ?? 0)) { this._overflowRecoveryAttempted = false; } const errorMessage = error instanceof Error ? error.message : "compaction failed"; - const aborted = - errorMessage === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError"); + const aborted = isCompactionExecutionAborted(error); this._emit({ type: "compaction_end", reason, @@ -3502,10 +3605,12 @@ export class AgentSession { return false; } catch (error) { + if (!compactionExecutionOwnsTerminalTransition(error)) { + return false; + } if (reason === "overflow") this._overflowRecoveryAttempted = false; const errorMessage = error instanceof Error ? error.message : "compaction failed"; - const aborted = - errorMessage === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError"); + const aborted = isCompactionExecutionAborted(error); this._emit({ type: "compaction_end", reason, @@ -3805,9 +3910,11 @@ export class AgentSession { options?.onError?.(new CompactionRejectedError(execution.rejectionCause)); } } catch (error) { + if (!compactionExecutionOwnsTerminalTransition(error)) { + return; + } const message = error instanceof Error ? error.message : String(error); - const aborted = - message === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError"); + const aborted = isCompactionExecutionAborted(error); this._emit({ type: "compaction_end", reason: "extension", diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 0013a0fd2..85c2beb76 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -12,7 +12,9 @@ or summary generation (`stale-revision`), preserving intervening context without duplicate replay. - Required compaction uses one provider-admission gate for normal prompts, extension-triggered turns, and every next turn. Provider-confirmed overflow remains fail-closed even when the local token estimate is below the configured - threshold; failed recovery restores the overflow context so later prompts cannot bypass the same requirement. + threshold; `agent_end` synchronously transfers overflow continuation ownership to `AgentSession` before agent-core + can drain native queues, and failed recovery restores the overflow context so later prompts cannot bypass the same + requirement. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/compaction/changes.md b/packages/coding-agent/src/core/compaction/changes.md index ffedb6b3c..c758c92ff 100644 --- a/packages/coding-agent/src/core/compaction/changes.md +++ b/packages/coding-agent/src/core/compaction/changes.md @@ -11,8 +11,9 @@ - Extension contexts retain the signal returned by `beginCompaction()` and supply it to legacy `updateCompaction()` / `endCompaction()` calls that omit one. Core accepts feedback mutations only from the current signal. - Provider admissions now share one required-compaction gate for prompt preflight, extension-triggered turns, and - next turns. Provider-confirmed overflow can force a split-turn preparation when keeping the only oversized prompt - would otherwise leave no compactable source. + next turns. Provider-confirmed overflow synchronously stops agent-core's post-`agent_end` queue drain so only an + accepted `AgentSession` recovery may resume queued work, and it can force a split-turn preparation when keeping the + only oversized prompt would otherwise leave no compactable source. - Compaction rejects stale source snapshots with `stale-revision` before the durable entry append. ### Why diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index a2177cbe9..ccf4c0cd4 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -6,8 +6,9 @@ - `ExtensionContext` compaction feedback actions now return and accept an optional operation `AbortSignal`, allowing progress and terminal feedback from superseded generations to be ignored without breaking existing extensions. - A context remembers its `beginCompaction()` signal and supplies it to legacy `updateCompaction()` and - `endCompaction()` calls that omit the signal, so an old context cannot end a newer operation. + Each handler invocation receives an isolated context that remembers its own `beginCompaction()` signal and supplies + it to legacy `updateCompaction()` and `endCompaction()` calls that omit the signal, so another handler in the same + event emission cannot rebind an old completion to a newer operation. - `stale-revision` is a structured compaction rejection cause for a source that changed before durable append. - The builtin compaction extension threads that signal through local and remote summary generation and application. diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts index df86c332f..c6613ca10 100644 --- a/packages/coding-agent/src/core/extensions/runner.ts +++ b/packages/coding-agent/src/core/extensions/runner.ts @@ -1077,7 +1077,6 @@ export class ExtensionRunner { } async emit(event: TEvent): Promise> { - const ctx = this.createContext(); let result: SessionBeforeEventResult | undefined; for (const ext of this.extensions) { @@ -1086,7 +1085,7 @@ export class ExtensionRunner { for (const handler of handlers) { try { - const handlerResult = await handler(event, ctx); + const handlerResult = await handler(event, this.createContext()); if (this.isSessionBeforeEvent(event) && handlerResult) { result = handlerResult as SessionBeforeEventResult; @@ -1111,7 +1110,6 @@ export class ExtensionRunner { } async emitModelSelect(event: ModelSelectEvent): Promise { - const ctx = this.createContext(); let result: ModelSelectEventResult | undefined; for (const ext of this.extensions) { @@ -1124,7 +1122,7 @@ export class ExtensionRunner { // the active toolset (gpt-apply-patch) must let later handlers // (prompt-preset) rebuild from the post-swap tools in the same emission. const liveEvent: ModelSelectEvent = { ...event, systemPromptOptions: this.getSystemPromptOptionsFn() }; - const handlerResult = await handler(liveEvent, ctx); + const handlerResult = await handler(liveEvent, this.createContext()); if (handlerResult) { const nextResult = handlerResult as ModelSelectEventResult; if (nextResult.systemPrompt !== undefined || nextResult.systemPromptName !== undefined) { @@ -1159,7 +1157,6 @@ export class ExtensionRunner { } async emitMessageEnd(event: MessageEndEvent): Promise { - const ctx = this.createContext(); let currentMessage = event.message; let modified = false; @@ -1170,7 +1167,9 @@ export class ExtensionRunner { for (const handler of handlers) { try { const currentEvent: MessageEndEvent = { ...event, message: currentMessage }; - const handlerResult = (await handler(currentEvent, ctx)) as MessageEndEventResult | undefined; + const handlerResult = (await handler(currentEvent, this.createContext())) as + | MessageEndEventResult + | undefined; if (!handlerResult?.message) continue; if (handlerResult.message.role !== currentMessage.role) { @@ -1201,7 +1200,6 @@ export class ExtensionRunner { } async emitToolResult(event: ToolResultEvent): Promise { - const ctx = this.createContext(); const currentEvent: ToolResultEvent = { ...event }; let modified = false; @@ -1210,7 +1208,7 @@ export class ExtensionRunner { if (!handlers || handlers.length === 0) continue; for (const handler of handlers) { - const hookRun = this.beginToolHookRun(ctx, { + const hookRun = this.beginToolHookRun(this.createContext(), { hookName: "PostToolUse", toolName: event.toolName, toolCallId: event.toolCallId, @@ -1265,7 +1263,6 @@ export class ExtensionRunner { } async emitToolCall(event: ToolCallEvent): Promise { - const ctx = this.createContext(); let result: ToolCallEventResult | undefined; for (const ext of this.extensions) { @@ -1273,7 +1270,7 @@ export class ExtensionRunner { if (!handlers || handlers.length === 0) continue; for (const handler of handlers) { - const hookRun = this.beginToolHookRun(ctx, { + const hookRun = this.beginToolHookRun(this.createContext(), { hookName: "PreToolUse", toolName: event.toolName, toolCallId: event.toolCallId, @@ -1305,15 +1302,13 @@ export class ExtensionRunner { } async emitUserBash(event: UserBashEvent): Promise { - const ctx = this.createContext(); - for (const ext of this.extensions) { const handlers = ext.handlers.get("user_bash"); if (!handlers || handlers.length === 0) continue; for (const handler of handlers) { try { - const handlerResult = await handler(event, ctx); + const handlerResult = await handler(event, this.createContext()); if (handlerResult) { return handlerResult as UserBashEventResult; } @@ -1334,7 +1329,6 @@ export class ExtensionRunner { } async emitContext(messages: AgentMessage[]): Promise { - const ctx = this.createContext(); let currentMessages = cloneJsonValue(messages); for (const ext of this.extensions) { @@ -1344,7 +1338,7 @@ export class ExtensionRunner { for (const handler of handlers) { try { const event: ContextEvent = { type: "context", messages: currentMessages }; - const handlerResult = await handler(event, ctx); + const handlerResult = await handler(event, this.createContext()); if (handlerResult && (handlerResult as ContextEventResult).messages) { currentMessages = (handlerResult as ContextEventResult).messages!; @@ -1366,7 +1360,6 @@ export class ExtensionRunner { } async emitBeforeProviderRequest(payload: unknown): Promise { - const ctx = this.createContext(); let currentPayload = payload; for (const ext of this.extensions) { @@ -1379,7 +1372,7 @@ export class ExtensionRunner { type: "before_provider_request", payload: currentPayload, }; - const handlerResult = await handler(event, ctx); + const handlerResult = await handler(event, this.createContext()); if (handlerResult !== undefined) { currentPayload = handlerResult; } @@ -1400,8 +1393,6 @@ export class ExtensionRunner { } async emitBeforeProviderHeaders(headers: ProviderHeaders): Promise { - const ctx = this.createContext(); - for (const ext of this.extensions) { const handlers = ext.handlers.get("before_provider_headers"); if (!handlers || handlers.length === 0) continue; @@ -1413,7 +1404,7 @@ export class ExtensionRunner { type: "before_provider_headers", headers, }; - await handler(event, ctx); + await handler(event, this.createContext()); } catch (err) { const message = err instanceof Error ? err.message : String(err); const stack = err instanceof Error ? err.stack : undefined; @@ -1437,14 +1428,6 @@ export class ExtensionRunner { systemPromptOptions: BuildSystemPromptOptions, ): Promise { let currentSystemPrompt = systemPrompt; - const ctx = Object.defineProperties( - {}, - Object.getOwnPropertyDescriptors(this.createContext()), - ) as ExtensionContext; - ctx.getSystemPrompt = () => { - this.assertActive(); - return currentSystemPrompt; - }; const messages: NonNullable[] = []; let systemPromptModified = false; @@ -1454,6 +1437,16 @@ export class ExtensionRunner { for (const handler of handlers) { try { + // Keep guarded context getters lazy while giving each handler its + // own legacy omitted-signal ownership slot. + const ctx = Object.defineProperties( + {}, + Object.getOwnPropertyDescriptors(this.createContext()), + ) as ExtensionContext; + ctx.getSystemPrompt = () => { + this.assertActive(); + return currentSystemPrompt; + }; const event: BeforeAgentStartEvent = { type: "before_agent_start", prompt, @@ -1505,7 +1498,6 @@ export class ExtensionRunner { themePaths: Array<{ path: string; extensionPath: string }>; hookPaths: Array<{ path: string; extensionPath: string }>; }> { - const ctx = this.createContext(); const skillPaths: Array<{ path: string; extensionPath: string }> = []; const promptPaths: Array<{ path: string; extensionPath: string }> = []; const themePaths: Array<{ path: string; extensionPath: string }> = []; @@ -1518,7 +1510,7 @@ export class ExtensionRunner { for (const handler of handlers) { try { const event: ResourcesDiscoverEvent = { type: "resources_discover", cwd, reason }; - const handlerResult = await handler(event, ctx); + const handlerResult = await handler(event, this.createContext()); const result = handlerResult as ResourcesDiscoverResult | undefined; if (result?.skillPaths?.length) { @@ -1556,7 +1548,6 @@ export class ExtensionRunner { source: InputSource, streamingBehavior?: "steer" | "followUp", ): Promise { - const ctx = this.createContext(); let currentText = text; let currentImages = images; @@ -1570,7 +1561,7 @@ export class ExtensionRunner { source, streamingBehavior, }; - const result = (await handler(event, ctx)) as InputEventResult | undefined; + const result = (await handler(event, this.createContext())) as InputEventResult | undefined; if (result?.action === "handled") return result; if (result?.action === "transform") { currentText = result.text; diff --git a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts index 12e765f09..600a1fc44 100644 --- a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts +++ b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts @@ -24,6 +24,25 @@ interface PostApplyFeedbackCapture { secondSignal?: AbortSignal; } +function createDeferred(): { promise: Promise; resolve: () => void } { + let resolve: (() => void) | undefined; + const promise = new Promise((next) => { + resolve = next; + }); + if (!resolve) throw new Error("Deferred resolver was not initialized"); + return { promise, resolve }; +} + +function createManualCompactionResult(harness: Harness, summary: string) { + const firstEntry = harness.sessionManager.getEntries()[0]; + if (!firstEntry) throw new Error("Expected a session entry before compaction"); + return { + summary, + firstKeptEntryId: firstEntry.id, + tokensBefore: 42, + }; +} + /** * Drives the real extension surface: feedback begun in before_agent_start is * applied, and the accepted session_compact handler immediately begins the @@ -288,4 +307,116 @@ describe("compaction feedback lifecycle", () => { expect(harness.session.isCompacting).toBe(true); expect(harness.eventsOfType("compaction_end")).toHaveLength(1); }); + + it("does not emit an execution terminal from A after B supersedes it", async () => { + const firstExecutionStarted = createDeferred(); + const releaseFirstExecution = createDeferred(); + const secondExecutionStarted = createDeferred(); + const releaseSecondExecution = createDeferred(); + let executionCount = 0; + const harness = await createHarness({ + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => { + executionCount++; + if (executionCount === 1) { + firstExecutionStarted.resolve(); + await releaseFirstExecution.promise; + } else { + secondExecutionStarted.resolve(); + await releaseSecondExecution.promise; + } + return { compaction: createManualCompactionResult(harness, `manual summary ${executionCount}`) }; + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("seed response")]); + await harness.session.prompt("seed context ".repeat(40)); + harness.events.length = 0; + + const operationA = harness.session.compact(); + await firstExecutionStarted.promise; + const operationB = harness.session.compact(); + await secondExecutionStarted.promise; + + try { + releaseFirstExecution.resolve(); + await expect(operationA).rejects.toThrow("Compaction cancelled"); + expect( + harness.events + .filter((event) => event.type === "compaction_start" || event.type === "compaction_end") + .map((event) => event.type), + ).toEqual(["compaction_start", "compaction_start"]); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "execution", + }); + expect(harness.session.isCompacting).toBe(true); + } finally { + releaseSecondExecution.resolve(); + await expect(operationB).resolves.toBeDefined(); + } + }); + + it.each([ + ["two extensions", true], + ["two handlers in one extension", false], + ])("keeps feedback ownership isolated across %s in one emission", async (_label, splitExtensions) => { + let firstContext: ExtensionContext | undefined; + let secondContext: ExtensionContext | undefined; + let firstSignal: AbortSignal | undefined; + let secondSignal: AbortSignal | undefined; + const firstHandler = (_event: { type: "agent_settled" }, ctx: ExtensionContext) => { + firstContext = ctx; + firstSignal = ctx.beginCompaction?.({ reason: "extension" }); + }; + const secondHandler = (_event: { type: "agent_settled" }, ctx: ExtensionContext) => { + secondContext = ctx; + secondSignal = ctx.beginCompaction?.({ reason: "extension" }); + }; + const harness = await createHarness({ + extensionFactories: splitExtensions + ? [(pi) => pi.on("agent_settled", firstHandler), (pi) => pi.on("agent_settled", secondHandler)] + : [ + (pi) => { + pi.on("agent_settled", firstHandler); + pi.on("agent_settled", secondHandler); + }, + ], + }); + harnesses.push(harness); + + await harness.getExtensionRunner().emit({ type: "agent_settled" }); + if (!firstContext || !secondContext || !firstSignal || !secondSignal) { + throw new Error("Expected both handlers to retain their feedback context"); + } + expect(firstSignal.aborted).toBe(true); + expect(secondSignal.aborted).toBe(false); + expect(harness.session.compactionState).toMatchObject({ status: "running", generation: 2 }); + + try { + firstContext.endCompaction?.({ reason: "extension", errorMessage: "A ended late" }); + expect(harness.session.compactionState).toMatchObject({ + status: "running", + generation: 2, + stage: "feedback", + }); + expect(harness.eventsOfType("compaction_end")).toHaveLength(0); + } finally { + secondContext.endCompaction?.({ reason: "extension", errorMessage: "B ended itself" }); + } + + expect(harness.session.compactionState).toMatchObject({ + status: "failed", + generation: 2, + errorMessage: "B ended itself", + }); + expect(harness.eventsOfType("compaction_end")).toEqual([ + expect.objectContaining({ reason: "extension", errorMessage: "B ended itself" }), + ]); + }); }); diff --git a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts index 7e2758ffe..45eabe14b 100644 --- a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts +++ b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts @@ -273,6 +273,136 @@ describe("pre-prompt compaction regression", () => { ); }); + it("retains native steer and follow-up queues after an error-terminal overflow recovery is rejected", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "required recovery rejected", + })); + }, + ], + }); + harnesses.push(harness); + + const providerStarted = createDeferred(); + const releaseProvider = createDeferred(); + harness.setResponses([ + async () => { + providerStarted.resolve(); + await releaseProvider.promise; + return fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "context_length_exceeded", + }); + }, + fauxAssistantMessage("must not reach provider"), + ]); + + const initialPrompt = harness.session.prompt("x".repeat(40_000)); + await providerStarted.promise; + await harness.session.prompt("retain native steer", { streamingBehavior: "steer" }); + await harness.session.followUp("retain native follow-up"); + releaseProvider.resolve(); + + await expect(initialPrompt).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getSteeringMessages()).toEqual(["retain native steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["retain native follow-up"]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + + await expect(harness.session.prompt("later normal admission")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + await expect( + harness.session.sendCustomMessage( + { customType: "extension-note", content: "later custom admission", display: true }, + { triggerTurn: true }, + ), + ).rejects.toThrow("Context remains above the compaction threshold because compaction did not complete"); + + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getSteeringMessages()).toEqual(["retain native steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["retain native follow-up"]); + }); + + it("does not admit agent_end queues that arrive after the first next-turn compaction sample", async () => { + const firstPrepareSampled = createDeferred(); + const releaseFirstPrepare = createDeferred(); + let prepareCount = 0; + let queuedAtAgentEnd = false; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + prepareNextTurnWithContext: async () => { + prepareCount++; + if (prepareCount === 1) { + firstPrepareSampled.resolve(); + await releaseFirstPrepare.promise; + } + return undefined; + }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "late queue recovery rejected", + })); + }, + (pi) => { + pi.on("agent_end", () => { + if (queuedAtAgentEnd) return; + queuedAtAgentEnd = true; + pi.sendUserMessage("late native follow-up", { deliverAs: "followUp" }); + pi.sendUserMessage("late native steer", { deliverAs: "steer" }); + }); + }, + ], + }); + harnesses.push(harness); + + const model = harness.getModel(); + harness.setResponses([ + { + ...fauxAssistantMessage("", { stopReason: "length" }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(10_000), + }, + fauxAssistantMessage("must not reach provider"), + fauxAssistantMessage("must not reach provider either"), + ]); + + const prompt = harness.session.prompt("x".repeat(40_000)); + await firstPrepareSampled.promise; + expect(harness.session.pendingMessageCount).toBe(0); + releaseFirstPrepare.resolve(); + await expect(prompt).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + await harness.session.waitForSettledSessionWork(); + + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "overflow", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getSteeringMessages()).toEqual(["late native steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["late native follow-up"]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + }); + it("blocks sendCustomMessage triggerTurn when overflow compaction is rejected below the local threshold", async () => { const harness = await createHarness({ models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], From aadbe3515970c449d93b507fe65072370942f552 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 24 Jul 2026 04:28:50 +0900 Subject: [PATCH 05/10] fix(coding-agent): retain required compaction queues --- .../coding-agent/src/core/agent-session.ts | 83 +++++- packages/coding-agent/src/core/changes.md | 8 +- .../src/core/compaction/changes.md | 6 +- .../pre-prompt-compaction-no-continue.test.ts | 256 +++++++++++++++++- 4 files changed, 332 insertions(+), 21 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index a7b80bbe7..779dcd4e1 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -841,9 +841,11 @@ export class AgentSession { context: { ...previousContext, messages: compacted ? this.agent.state.messages.slice() : previousContext.messages, + systemPrompt: this._systemPromptOverride ?? this._baseSystemPrompt, + tools: this.agent.state.tools.slice(), }, - model: previousSnapshot?.model ?? this.agent.state.model, - thinkingLevel: previousSnapshot?.thinkingLevel ?? this.agent.state.thinkingLevel, + model: this.agent.state.model, + thinkingLevel: this.agent.state.thinkingLevel, }; }; } @@ -976,11 +978,11 @@ export class AgentSession { // final agent_end. This subscriber intentionally processes its own event // queue asynchronously, so a later recovery rejection cannot abort that // drain in time. agent_end itself is still an awaited synchronous boundary - // before that drain: transfer provider-confirmed overflow ownership there, - // retaining queues until AgentSession accepts recovery. + // before that drain: transfer every required overflow or threshold + // compaction to AgentSession, retaining queues until recovery is accepted. if (event.type === "agent_end") { const lastAssistant = this._findLastAssistantInMessages(event.messages); - if (lastAssistant && this._requiresOverflowRecovery(lastAssistant)) { + if (lastAssistant && this._getRequiredAutoCompactionReason(lastAssistant)) { this.agent.abort(); } } @@ -1048,14 +1050,59 @@ export class AgentSession { return undefined; } - private _requiresOverflowRecovery(message: AssistantMessage): boolean { + /** + * Synchronously mirror the auto-compaction decision that _checkCompaction() + * will make after agent_end. Agent core drains queues before that async work + * runs, so only this preflight can transfer required admissions safely. + */ + private _getRequiredAutoCompactionReason(message: AssistantMessage): "overflow" | "threshold" | undefined { + const settings = this.settingsManager.getCompactionSettings(); + if (!settings.enabled || message.stopReason === "aborted") { + return undefined; + } + const model = this.model; - return ( - model !== undefined && - (message.stopReason === "error" || message.stopReason === "length") && - isContextOverflow(message, model.contextWindow) && - isSameOverflowSource(message, model, this._modelRuntime.getCompatibilityRequestConfig(model).upstreamModelId) + if (!model || this._isAssistantFromBeforeLatestCompaction(message)) { + return undefined; + } + + const sameModel = isSameOverflowSource( + message, + model, + this._modelRuntime.getCompatibilityRequestConfig(model).upstreamModelId, ); + const contextUsage = this.getContextUsage(); + const currentContextNeedsCompaction = + contextUsage !== undefined && + contextUsage.tokens !== null && + shouldCompact(contextUsage.tokens, contextUsage.contextWindow, settings); + if (isContextOverflow(message, model.contextWindow) && (sameModel || currentContextNeedsCompaction)) { + return "overflow"; + } + + let contextTokens: number; + const directContextTokens = message.usage ? calculateContextTokens(message.usage) : 0; + if (message.stopReason !== "error" && directContextTokens !== 0) { + contextTokens = directContextTokens; + } else { + const messages = filterContextExcludedMessages(this.agent.state.messages); + const estimate = estimateContextTokens(messages); + if (estimate.lastUsageIndex === null) { + return undefined; + } + const compactionEntry = getLatestCompactionEntry(this.sessionManager.getBranch()); + const usageMessage = messages[estimate.lastUsageIndex]; + if ( + compactionEntry && + usageMessage?.role === "assistant" && + usageMessage.timestamp <= new Date(compactionEntry.timestamp).getTime() + ) { + return undefined; + } + contextTokens = estimate.tokens; + } + + return shouldCompact(contextTokens, model.contextWindow, settings) ? "threshold" : undefined; } private _agentEndAllowsQueuedContinuation(messages: AgentMessage[]): boolean { @@ -1077,7 +1124,7 @@ export class AgentSession { if (lastAssistant.stopReason === "aborted" || lastAssistant.stopReason === "error") { return false; } - if (this._requiresOverflowRecovery(lastAssistant)) { + if (this._getRequiredAutoCompactionReason(lastAssistant)) { return false; } @@ -1231,7 +1278,7 @@ export class AgentSession { if (event.type === "agent_end" && this._lastAssistantMessage) { const msg = this._lastAssistantMessage; this._lastAssistantMessage = undefined; - const requiredOverflowRecovery = this._requiresOverflowRecovery(msg); + const requiredAutoCompaction = this._getRequiredAutoCompactionReason(msg); // Retry transient failures normally and eligible hard errors only through a fallback. const retryableError = this._isRetryableError(msg); @@ -1245,7 +1292,15 @@ export class AgentSession { this._resolveRetry(); launchedContinuation = await this._checkCompaction(msg); - if (requiredOverflowRecovery && !launchedContinuation) { + // _runAutoCompaction() returns false both when recovery was rejected and + // when an accepted compaction had no queue to continue. Re-sample after + // it settles so only the still-required (rejected) case fails admission. + if ( + requiredAutoCompaction && + !launchedContinuation && + this.agent.hasQueuedMessages() && + this._getRequiredAutoCompactionReason(msg) !== undefined + ) { this._requiredCompactionAdmissionError = new RequiredCompactionError(); } } diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 85c2beb76..e26bcf802 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -12,9 +12,11 @@ or summary generation (`stale-revision`), preserving intervening context without duplicate replay. - Required compaction uses one provider-admission gate for normal prompts, extension-triggered turns, and every next turn. Provider-confirmed overflow remains fail-closed even when the local token estimate is below the configured - threshold; `agent_end` synchronously transfers overflow continuation ownership to `AgentSession` before agent-core - can drain native queues, and failed recovery restores the overflow context so later prompts cannot bypass the same - requirement. + threshold; `agent_end` synchronously transfers both silent-overflow and threshold-compaction continuation ownership + to `AgentSession` before agent-core can drain native queues, and failed recovery restores the overflow context so + later prompts cannot bypass the same requirement. +- Next-turn snapshots reapply the live active tools and effective per-run system prompt after asynchronous preparation, + so a tool removed during the turn is neither advertised nor executable by the following provider request. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/compaction/changes.md b/packages/coding-agent/src/core/compaction/changes.md index c758c92ff..1661c54ad 100644 --- a/packages/coding-agent/src/core/compaction/changes.md +++ b/packages/coding-agent/src/core/compaction/changes.md @@ -11,9 +11,9 @@ - Extension contexts retain the signal returned by `beginCompaction()` and supply it to legacy `updateCompaction()` / `endCompaction()` calls that omit one. Core accepts feedback mutations only from the current signal. - Provider admissions now share one required-compaction gate for prompt preflight, extension-triggered turns, and - next turns. Provider-confirmed overflow synchronously stops agent-core's post-`agent_end` queue drain so only an - accepted `AgentSession` recovery may resume queued work, and it can force a split-turn preparation when keeping the - only oversized prompt would otherwise leave no compactable source. + next turns. Silent provider overflow and threshold-required compaction synchronously stop agent-core's + post-`agent_end` queue drain so only an accepted `AgentSession` recovery may resume queued work, and overflow can + force a split-turn preparation when keeping the only oversized prompt would otherwise leave no compactable source. - Compaction rejects stale source snapshots with `stale-revision` before the durable entry append. ### Why diff --git a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts index 45eabe14b..daaa385b2 100644 --- a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts +++ b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts @@ -1,6 +1,6 @@ import { type AssistantMessage, fauxAssistantMessage } from "@earendil-works/pi-ai"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { createHarness, getUserTexts, type Harness } from "../harness.ts"; +import { createHarness, getAssistantTexts, getUserTexts, type Harness } from "../harness.ts"; function createUsage(totalTokens: number) { return { @@ -469,4 +469,258 @@ describe("pre-prompt compaction regression", () => { }), ); }); + + it("retains native steer and follow-up queues when a silent successful overflow recovery is rejected", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "silent overflow recovery rejected", + })); + }, + (pi) => { + let queuedAtAgentEnd = false; + pi.on("agent_end", () => { + if (queuedAtAgentEnd) return; + queuedAtAgentEnd = true; + pi.sendUserMessage("retain silent steer", { deliverAs: "steer" }); + pi.sendUserMessage("retain silent follow-up", { deliverAs: "followUp" }); + }); + }, + ], + }); + harnesses.push(harness); + + const now = Date.now(); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "earlier prompt" }], + timestamp: now - 2000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("earlier response", { timestamp: now - 1000 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(50), + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + // Silent successful overflow (z.ai style): the provider answers with + // stopReason "stop" while the faux-simulated usage.input + cacheRead + // exceeds the 10_000 context window (the 44_000-char prompt fills it), + // so isContextOverflow() is true without any error or length signal. + fauxAssistantMessage("silent overflow answer", { stopReason: "stop" }), + fauxAssistantMessage("must not reach provider"), + fauxAssistantMessage("must not reach provider either"), + ]); + + // The follow-up fix owns whether the already-completed prompt itself + // rejects; the fail-closed admission contract below is what this RED pins. + await harness.session.prompt("x".repeat(44_000)).then( + () => undefined, + () => undefined, + ); + await harness.session.waitForSettledSessionWork(); + + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "overflow", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getSteeringMessages()).toEqual(["retain silent steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["retain silent follow-up"]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + + await expect(harness.session.prompt("later normal admission")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + await expect( + harness.session.sendCustomMessage( + { customType: "extension-note", content: "later custom admission", display: true }, + { triggerTurn: true }, + ), + ).rejects.toThrow("Context remains above the compaction threshold because compaction did not complete"); + + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getSteeringMessages()).toEqual(["retain silent steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["retain silent follow-up"]); + }); + + it("drains retained agent_end queues only after an accepted silent-overflow compaction", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 0 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => ({ + compaction: { + summary: "silent overflow summary", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })); + }, + (pi) => { + let queuedAtAgentEnd = false; + pi.on("agent_end", () => { + if (queuedAtAgentEnd) return; + queuedAtAgentEnd = true; + pi.sendUserMessage("silent steer", { deliverAs: "steer" }); + pi.sendUserMessage("silent follow-up", { deliverAs: "followUp" }); + }); + }, + ], + }); + harnesses.push(harness); + + let callCountAtAcceptedCompaction: number | undefined; + harness.session.subscribe((event) => { + if (event.type === "compaction_end" && event.accepted === true) { + callCountAtAcceptedCompaction = harness.faux.state.callCount; + } + }); + + const now = Date.now(); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "earlier prompt" }], + timestamp: now - 2000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("earlier response", { timestamp: now - 1000 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(50), + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + fauxAssistantMessage("silent overflow answer", { stopReason: "stop" }), + fauxAssistantMessage("silent steer answer"), + fauxAssistantMessage("silent follow-up answer"), + ]); + + await expect(harness.session.prompt("x".repeat(44_000))).resolves.toBeUndefined(); + await harness.session.waitForSettledSessionWork(); + + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ reason: "overflow", accepted: true }), + ); + // Compaction completed while only the overflow turn had reached the + // provider, so the retained queues drained strictly after recovery. + expect(callCountAtAcceptedCompaction).toBe(1); + expect(harness.faux.state.callCount).toBe(3); + expect(harness.session.getSteeringMessages()).toEqual([]); + expect(harness.session.getFollowUpMessages()).toEqual([]); + expect(getAssistantTexts(harness)).toContain("silent steer answer"); + expect(getAssistantTexts(harness)).toContain("silent follow-up answer"); + }); + + it("does not continue agent_end queues above the compaction threshold when late recovery is rejected", async () => { + const firstPrepareSampled = createDeferred(); + const releaseFirstPrepare = createDeferred(); + let prepareCount = 0; + let queuedAtAgentEnd = false; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 13_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 3_000 } }, + prepareNextTurnWithContext: async () => { + prepareCount++; + if (prepareCount === 1) { + firstPrepareSampled.resolve(); + await releaseFirstPrepare.promise; + } + return undefined; + }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "late threshold recovery rejected", + })); + }, + (pi) => { + pi.on("agent_end", () => { + if (queuedAtAgentEnd) return; + queuedAtAgentEnd = true; + pi.sendUserMessage("late threshold follow-up", { deliverAs: "followUp" }); + pi.sendUserMessage("late threshold steer", { deliverAs: "steer" }); + }); + }, + ], + }); + harnesses.push(harness); + + const now = Date.now(); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "earlier prompt" }], + timestamp: now - 2000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("earlier response", { timestamp: now - 1000 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(50), + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + // Faux-simulated usage lands above the configured threshold + // (13_000 - 3_000 = 10_000) without any provider-confirmed overflow + // signal: the 38_000-char prompt yields ~12_100 input tokens (default + // tool schemas included), below the 13_000 context window. + fauxAssistantMessage("threshold answer", { stopReason: "stop" }), + fauxAssistantMessage("must not reach provider"), + fauxAssistantMessage("must not reach provider either"), + ]); + + const prompt = harness.session.prompt("x".repeat(38_000)); + await firstPrepareSampled.promise; + expect(harness.session.pendingMessageCount).toBe(0); + releaseFirstPrepare.resolve(); + // The follow-up fix owns whether the already-completed prompt itself + // rejects; the fail-closed admission contract below is what this RED pins. + await prompt.then( + () => undefined, + () => undefined, + ); + await harness.session.waitForSettledSessionWork(); + + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "threshold", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getSteeringMessages()).toEqual(["late threshold steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["late threshold follow-up"]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + + await expect(harness.session.prompt("later normal admission")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + await expect( + harness.session.sendCustomMessage( + { customType: "extension-note", content: "later custom admission", display: true }, + { triggerTurn: true }, + ), + ).rejects.toThrow("Context remains above the compaction threshold because compaction did not complete"); + expect(harness.faux.state.callCount).toBe(1); + }); }); From 04bdc6b51d9363376c8312688eb21a85b048264e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 24 Jul 2026 05:21:34 +0900 Subject: [PATCH 06/10] fix(coding-agent): preserve compaction boundaries --- packages/agent/src/agent.ts | 19 +- packages/agent/src/changes.md | 22 ++ packages/agent/test/agent.test.ts | 36 ++++ .../coding-agent/src/core/agent-session.ts | 137 ++++++++---- packages/coding-agent/src/core/changes.md | 5 + .../extensions/builtin/compaction/changes.md | 11 + .../extensions/builtin/compaction/index.ts | 22 +- .../builtin/compaction/speculative.ts | 4 +- .../src/core/extensions/changes.md | 4 +- .../src/core/extensions/runner.ts | 2 +- .../coding-agent/src/core/extensions/types.ts | 2 + packages/coding-agent/test/compaction.test.ts | 61 +++++- .../suite/agent-session-compaction.test.ts | 203 +++++++++++++++++- .../compaction-feedback-lifecycle.test.ts | 63 ++++++ ...2-extension-active-tools-next-turn.test.ts | 56 +++++ .../pre-prompt-compaction-no-continue.test.ts | 188 ++++++++++++++++ 16 files changed, 781 insertions(+), 54 deletions(-) diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index 18043a1a7..89d935f4c 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -171,6 +171,7 @@ type ActiveRun = { promise: Promise; resolve: () => void; abortController: AbortController; + suppressQueuedMessageDrain: boolean; }; /** @@ -324,6 +325,16 @@ export class Agent { this.activeRun?.abortController.abort(); } + /** + * Keep queued steering and follow-up messages for an external owner after + * this run reaches agent_end, without changing the active abort signal. + */ + suppressQueuedMessageDrain(): void { + if (this.activeRun) { + this.activeRun.suppressQueuedMessageDrain = true; + } + } + /** * Resolve when the current run and all awaited event listeners have finished. * @@ -505,7 +516,7 @@ export class Agent { const promise = new Promise((resolve) => { resolvePromise = resolve; }); - this.activeRun = { promise, resolve: resolvePromise, abortController }; + this.activeRun = { promise, resolve: resolvePromise, abortController, suppressQueuedMessageDrain: false }; this._state.isStreaming = true; this._state.streamingMessage = undefined; @@ -513,7 +524,11 @@ export class Agent { try { await executor(abortController.signal); - while (!abortController.signal.aborted && this.hasQueuedMessages()) { + while ( + !abortController.signal.aborted && + !this.activeRun?.suppressQueuedMessageDrain && + this.hasQueuedMessages() + ) { await this.runQueuedMessagesAfterAgentEnd(abortController.signal); } } catch (error) { diff --git a/packages/agent/src/changes.md b/packages/agent/src/changes.md index f9e158460..7988783a1 100644 --- a/packages/agent/src/changes.md +++ b/packages/agent/src/changes.md @@ -1,5 +1,27 @@ # Changes +## 2026-07-23 - Session-owned post-agent_end queue drain suppression + +### What changed and why + +- `Agent` now exposes `suppressQueuedMessageDrain()` for the active run. It stops only the lifecycle-owned + post-`agent_end` steering/follow-up drain, retaining both queues without aborting the run signal. +- The coding-agent compaction admission gate uses this ownership transfer for required recovery. Real user aborts + continue to abort the active signal and retain the normal terminal semantics. + +### Files modified + +- `agent.ts` +- `../test/agent.test.ts` + +### Why the extension system could not handle this + +- Native queue draining and active-run signal ownership occur inside `Agent` after event subscribers return. + +### Expected merge conflict zones on next upstream sync + +- MEDIUM: `agent.ts` active-run lifecycle and post-`agent_end` queue draining. + ## 2026-07-23 - uuidv7 concurrency refutation + immutable launch profile ### What changed and why diff --git a/packages/agent/test/agent.test.ts b/packages/agent/test/agent.test.ts index fac0f2be7..ba8ef6d22 100644 --- a/packages/agent/test/agent.test.ts +++ b/packages/agent/test/agent.test.ts @@ -547,6 +547,42 @@ describe("Agent", () => { expect(() => agent.abort()).not.toThrow(); }); + it("retains agent_end queues without aborting when drain suppression is requested", async () => { + let providerCalls = 0; + let agentEndSignal: AbortSignal | undefined; + const agent = new Agent({ + streamFn: () => { + providerCalls++; + const stream = new MockAssistantStream(); + queueMicrotask(() => { + stream.push({ + type: "done", + reason: "stop", + message: createAssistantMessage(`response ${providerCalls}`), + }); + }); + return stream; + }, + }); + agent.subscribe((event, signal) => { + if (event.type !== "agent_end" || providerCalls !== 1) return; + agentEndSignal = signal; + agent.followUp({ role: "user", content: "deferred follow-up", timestamp: Date.now() }); + agent.suppressQueuedMessageDrain(); + }); + + await agent.prompt("first prompt"); + + expect(agentEndSignal?.aborted).toBe(false); + expect(agent.hasQueuedMessages()).toBe(true); + expect(providerCalls).toBe(1); + + await agent.continue(); + + expect(agent.hasQueuedMessages()).toBe(false); + expect(providerCalls).toBe(2); + }); + it("should throw when prompt() called while streaming", async () => { let abortSignal: AbortSignal | undefined; const agent = new Agent({ diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 779dcd4e1..9816f7d0d 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -528,6 +528,7 @@ export class AgentSession { private _retryResolve: (() => void) | undefined = undefined; private _userAbortPromise: Promise | undefined = undefined; private _suppressQueuedContinuationAfterUserAbort = false; + private _extensionEventSignal: AbortSignal | undefined = undefined; // Bash execution state private _bashAbortController: AbortController | undefined = undefined; @@ -794,7 +795,6 @@ export class AgentSession { ? async (_turn: PrepareNextTurnContext, signal?: AbortSignal) => await this.agent.prepareNextTurn?.(signal) : undefined); this.agent.prepareNextTurnWithContext = async (turn, signal) => { - let compacted = false; // Enforce compaction only when this prepare precedes an actual provider // admission: a tool continuation or queued steer/follow-up messages. A // completed turn with no continuation keeps pre-PR timing, while the @@ -820,8 +820,8 @@ export class AgentSession { } }; - compacted = await compactBeforeNextAdmission(); - const messages = compacted ? this.agent.state.messages.slice() : turn.context.messages; + const compactedBeforeCallback = await compactBeforeNextAdmission(); + const messages = compactedBeforeCallback ? this.agent.state.messages.slice() : turn.context.messages; const postCompactionTurn = { ...turn, @@ -832,15 +832,16 @@ export class AgentSession { // The previous callback may await while agent_end extensions enqueue // continuation work. Re-sample after it returns so that work cannot // slip through with the stale provider snapshot it observed on entry. - if (!compacted) { - compacted = await compactBeforeNextAdmission(); + let compactedAfterCallback = false; + if (!compactedBeforeCallback) { + compactedAfterCallback = await compactBeforeNextAdmission(); } return { ...previousSnapshot, context: { ...previousContext, - messages: compacted ? this.agent.state.messages.slice() : previousContext.messages, + messages: compactedAfterCallback ? this.agent.state.messages.slice() : previousContext.messages, systemPrompt: this._systemPromptOverride ?? this._baseSystemPrompt, tools: this.agent.state.tools.slice(), }, @@ -973,7 +974,7 @@ export class AgentSession { private _lastAssistantMessage: AssistantMessage | undefined = undefined; /** Internal handler for agent events - shared by subscribe and reconnect */ - private _handleAgentEvent = (event: AgentEvent): void => { + private _handleAgentEvent = (event: AgentEvent, signal: AbortSignal): void => { // Agent core drains native steer/follow-up queues immediately after its // final agent_end. This subscriber intentionally processes its own event // queue asynchronously, so a later recovery rejection cannot abort that @@ -983,7 +984,7 @@ export class AgentSession { if (event.type === "agent_end") { const lastAssistant = this._findLastAssistantInMessages(event.messages); if (lastAssistant && this._getRequiredAutoCompactionReason(lastAssistant)) { - this.agent.abort(); + this.agent.suppressQueuedMessageDrain(); } } @@ -1003,8 +1004,8 @@ export class AgentSession { } const processing = this._agentEventQueue.then( - () => this._processAgentEvent(event), - () => this._processAgentEvent(event), + () => this._processAgentEvent(event, signal), + () => this._processAgentEvent(event, signal), ); this._agentEventQueue = pendingMessage !== undefined @@ -1179,7 +1180,7 @@ export class AgentSession { return providerDelayMs <= this.settingsManager.getProviderRetrySettings().maxRetryDelayMs; } - private async _processAgentEvent(event: AgentEvent): Promise { + private async _processAgentEvent(event: AgentEvent, signal: AbortSignal): Promise { // When a user message starts, check if it's from either queue and remove it BEFORE emitting // This ensures the UI sees the updated queue state if (event.type === "message_start" && event.message.role === "user") { @@ -1203,8 +1204,14 @@ export class AgentSession { } } - // Emit to extensions first - await this._emitExtensionEvent(event); + // Emit to extensions first. Agent event persistence is intentionally + // asynchronous, so retain the source run signal while dispatching. + this._extensionEventSignal = signal; + try { + await this._emitExtensionEvent(event); + } finally { + this._extensionEventSignal = undefined; + } // Notify all listeners this._emit( @@ -1282,26 +1289,50 @@ export class AgentSession { // Retry transient failures normally and eligible hard errors only through a fallback. const retryableError = this._isRetryableError(msg); + const hardErrorFallbackEligible = this._isHardErrorFallbackEligible(msg); + const retryCanAdmitProvider = + this.settingsManager.getRetrySettings().enabled && (retryableError || hardErrorFallbackEligible); + let retryCompactionRejected = false; + let compactedBeforeRetry = false; + if (retryCanAdmitProvider && requiredAutoCompaction) { + try { + compactedBeforeRetry = await this._enforceCompactionBeforeProvider(msg, true, "threshold", true); + } catch (error) { + if (error instanceof RequiredCompactionError) { + retryCompactionRejected = true; + } else { + throw error; + } + } + } + let didRetry = false; - if (retryableError) { - didRetry = await this._handleRetryableError(msg); - } else if (this._isHardErrorFallbackEligible(msg)) { - didRetry = await this._handleRetryableError(msg, { hardErrorFallback: true }); + if (!retryCompactionRejected) { + if (retryableError) { + didRetry = await this._handleRetryableError(msg); + } else if (hardErrorFallbackEligible) { + didRetry = await this._handleRetryableError(msg, { hardErrorFallback: true }); + } } if (didRetry) return; // Retry was initiated, don't proceed to compaction this._resolveRetry(); - launchedContinuation = await this._checkCompaction(msg); - // _runAutoCompaction() returns false both when recovery was rejected and - // when an accepted compaction had no queue to continue. Re-sample after - // it settles so only the still-required (rejected) case fails admission. - if ( - requiredAutoCompaction && - !launchedContinuation && - this.agent.hasQueuedMessages() && - this._getRequiredAutoCompactionReason(msg) !== undefined - ) { - this._requiredCompactionAdmissionError = new RequiredCompactionError(); + if (compactedBeforeRetry && this.agent.hasQueuedMessages()) { + this._scheduleContinuationAfterCurrentEvent(); + launchedContinuation = true; + } else if (!retryCompactionRejected) { + launchedContinuation = await this._checkCompaction(msg); + // _runAutoCompaction() returns false both when recovery was rejected and + // when an accepted compaction had no queue to continue. Re-sample after + // it settles so only the still-required (rejected) case fails admission. + if ( + requiredAutoCompaction && + !launchedContinuation && + this.agent.hasQueuedMessages() && + this._getRequiredAutoCompactionReason(msg) !== undefined + ) { + this._requiredCompactionAdmissionError = new RequiredCompactionError(); + } } } @@ -1689,11 +1720,18 @@ export class AgentSession { validToolNames.push(name); } } + const activeToolNamesChanged = + validToolNames.length !== this.agent.state.tools.length || + validToolNames.some((name, index) => name !== this.agent.state.tools[index]?.name); this.agent.state.tools = tools; // Rebuild base system prompt with new tool set this._baseSystemPrompt = this._rebuildSystemPrompt(validToolNames); this.agent.state.systemPrompt = this._systemPromptOverride ?? this._baseSystemPrompt; + if (activeToolNamesChanged) { + this.abortCompaction(); + this._incrementMessageRevision(); + } } /** Whether compaction or branch summarization is currently running */ @@ -2899,6 +2937,9 @@ export class AgentSession { precomputed: CompactionResult, options: ApplyCompactionOptions, ): Promise { + if (options.signal !== undefined && options.signal !== this._compactionAbortController?.signal) { + return { applied: false, reason: "stale" }; + } if (options.expectedRevision !== undefined && options.expectedRevision !== this._messageRevision) { return { applied: false, reason: "stale" }; } @@ -3358,11 +3399,12 @@ export class AgentSession { assistantMessage: AssistantMessage | undefined, skipAbortedCheck: boolean, inlineReason: "pre_prompt" | "threshold", + retryAfterCompaction = false, ): Promise { const compacted = assistantMessage - ? await this._checkCompaction(assistantMessage, skipAbortedCheck, inlineReason) + ? await this._checkCompaction(assistantMessage, skipAbortedCheck, inlineReason, retryAfterCompaction) : false; - if (compacted || (assistantMessage && this._isAssistantFromBeforeLatestCompaction(assistantMessage))) { + if (compacted) { return compacted; } @@ -3370,16 +3412,34 @@ export class AgentSession { const contextTokens = estimateContextTokens( filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), ).tokens; - if (settings.enabled && this.model && shouldCompact(contextTokens, this.model.contextWindow, settings)) { - throw new RequiredCompactionError(); + if (!settings.enabled || !this.model || !shouldCompact(contextTokens, this.model.contextWindow, settings)) { + return false; } - return false; + + const latestCompaction = getLatestCompactionEntry(this.sessionManager.getBranch()); + const assistantBeforeLatestCompaction = + assistantMessage !== undefined && this._isAssistantFromBeforeLatestCompaction(assistantMessage); + const hasPostCompactionCustomState = + latestCompaction !== null && + this.agent.state.messages.some( + (message) => + message.role === "custom" && message.timestamp >= new Date(latestCompaction.timestamp).getTime(), + ); + if (assistantBeforeLatestCompaction && !hasPostCompactionCustomState) { + return false; + } + if (assistantBeforeLatestCompaction && assistantMessage) { + const compacted = await this._runPrePromptCompaction(assistantMessage, skipAbortedCheck, inlineReason); + if (compacted) return true; + } + throw new RequiredCompactionError(); } private async _checkCompaction( assistantMessage: AssistantMessage, skipAbortedCheck = true, inlineReason?: "pre_prompt" | "threshold", + retryAfterCompaction = false, ): Promise { const settings = this.settingsManager.getCompactionSettings(); if (!settings.enabled) return false; @@ -3418,7 +3478,7 @@ export class AgentSession { contextUsage.tokens !== null && shouldCompact(contextUsage.tokens, contextUsage.contextWindow, settings); if (isContextOverflow(assistantMessage, contextWindow) && (sameModel || currentContextNeedsCompaction)) { - const willRetry = assistantMessage.stopReason !== "stop"; + const willRetry = retryAfterCompaction || assistantMessage.stopReason !== "stop"; if (!willRetry) { return await this._runAutoCompaction("overflow", false); @@ -3496,7 +3556,12 @@ export class AgentSession { } if (shouldCompact(contextTokens, contextWindow, settings)) { if (inlineReason) { - return await this._runPrePromptCompaction(assistantMessage, skipAbortedCheck, inlineReason); + return await this._runPrePromptCompaction( + assistantMessage, + skipAbortedCheck, + inlineReason, + retryAfterCompaction, + ); } else { return await this._runAutoCompaction("threshold", false); } @@ -3888,7 +3953,7 @@ export class AgentSession { getServiceTier: () => this.serviceTier, isIdle: () => this.isIdle, isProjectTrusted: () => this.settingsManager.isProjectTrusted(), - getSignal: () => this.agent.signal, + getSignal: () => this._extensionEventSignal ?? this.agent.signal, abort: () => { if (this._extensionAbortHandler) { this._extensionAbortHandler(); diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index e26bcf802..e0bd01c46 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -17,6 +17,11 @@ later prompts cannot bypass the same requirement. - Next-turn snapshots reapply the live active tools and effective per-run system prompt after asynchronous preparation, so a tool removed during the turn is neither advertised nor executable by the following provider request. +- Required ownership now suppresses only agent-core's post-`agent_end` queue drain, not the run abort signal. Deferred + extension dispatch retains the real source signal, so compaction ownership does not masquerade as user cancellation. +- Retry and fallback admission resolve required compaction first; rejected recovery retains native queues without + dispatching a provider retry. Active-tool changes advance the context revision and abort active core compaction so + summaries prepared against a prior tool set cannot apply. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md index d76f41d38..56f02498f 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md @@ -1,5 +1,16 @@ # Builtin compaction extension changes +## Active-tool-only summarization requests (2026-07-23) + +- `index.ts`: direct local summarization requests now map the current active tool names to registered definitions. + Inactive registered tools, including inactive MCP catalog entries, no longer consume remote compaction payload + budget or appear as callable tools to the summarizer. +- Applied speculative summaries carry their handler's feedback signal, allowing core to reject a superseded apply + before durable session mutation. + +Expected upstream conflict zones: `builtin/compaction/index.ts` tool snapshot construction and +`builtin/compaction/speculative.ts` apply path. + ## Session-owned compaction completion state (2026-07-23) - AgentSession now records compaction as `idle`, `running`, `completed`, `failed`, or `aborted` with a monotonic diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts index 70bbbca6a..fd3cf495c 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts @@ -166,13 +166,13 @@ export default function compactionExtension(pi: ExtensionAPI): void { const pendingMetadata = new Map(); function getSummarizationTools(): Tool[] { - if (typeof pi.getAllTools !== "function") return []; + if (typeof pi.getAllTools !== "function" || typeof pi.getActiveTools !== "function") return []; try { - return pi.getAllTools().map((tool) => ({ - name: tool.name, - description: tool.description, - parameters: tool.parameters, - })); + const definitionsByName = new Map(pi.getAllTools().map((tool) => [tool.name, tool])); + return pi.getActiveTools().flatMap((name) => { + const tool = definitionsByName.get(name); + return tool ? [{ name: tool.name, description: tool.description, parameters: tool.parameters }] : []; + }); } catch { // Tool registry not bound yet (extension still loading); the summary // request simply goes out without tool definitions. @@ -254,6 +254,7 @@ export default function compactionExtension(pi: ExtensionAPI): void { remoteSnapshot, () => speculativeGeneration, remoteCompaction, + feedbackSignal, ); endCompactionFeedback(ctx, feedbackSignal, result); return result; @@ -275,6 +276,7 @@ export default function compactionExtension(pi: ExtensionAPI): void { pendingJob.snapshot, () => speculativeGeneration, compaction, + feedbackSignal, ); if (result.applied || result.reason === "stale") { speculativeJob = undefined; @@ -310,7 +312,13 @@ export default function compactionExtension(pi: ExtensionAPI): void { // the session_before_compact route. if (!(error instanceof SummaryGenerationError)) throw error; } - const result = await applyGeneratedCompaction(ctx, snapshot, () => speculativeGeneration, compaction); + const result = await applyGeneratedCompaction( + ctx, + snapshot, + () => speculativeGeneration, + compaction, + feedbackSignal, + ); endCompactionFeedback(ctx, feedbackSignal, result); return result; } catch (error) { diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts index 5e029e9b0..81e4bc0c7 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts @@ -49,7 +49,7 @@ export interface SpeculativeCompactionContext { getSystemPrompt?(): string; applyCompaction( precomputed: CompactionResult, - options: { reason: "extension"; expectedRevision: number }, + options: { reason: "extension"; expectedRevision: number; signal?: AbortSignal }, ): Promise; } @@ -466,6 +466,7 @@ export async function applyGeneratedCompaction( snapshot: SpeculativeCompactionSnapshot | undefined, getCurrentGeneration: () => number, compaction: CompactionResult | undefined, + signal?: AbortSignal, ): Promise { if (!snapshot || !compaction) return { applied: false, reason: "unavailable" }; @@ -476,6 +477,7 @@ export async function applyGeneratedCompaction( return await context.applyCompaction(compaction, { reason: "extension", expectedRevision: snapshot.expectedRevision, + signal, }); } diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index ccf4c0cd4..8035c37c6 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -7,8 +7,8 @@ - `ExtensionContext` compaction feedback actions now return and accept an optional operation `AbortSignal`, allowing progress and terminal feedback from superseded generations to be ignored without breaking existing extensions. Each handler invocation receives an isolated context that remembers its own `beginCompaction()` signal and supplies - it to legacy `updateCompaction()` and `endCompaction()` calls that omit the signal, so another handler in the same - event emission cannot rebind an old completion to a newer operation. + it to legacy `updateCompaction()`, `endCompaction()`, and `applyCompaction()` calls that omit the signal, so another + handler in the same event emission cannot rebind an old completion or durable apply to a newer operation. - `stale-revision` is a structured compaction rejection cause for a source that changed before durable append. - The builtin compaction extension threads that signal through local and remote summary generation and application. diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts index c6613ca10..35aaf0d41 100644 --- a/packages/coding-agent/src/core/extensions/runner.ts +++ b/packages/coding-agent/src/core/extensions/runner.ts @@ -1011,7 +1011,7 @@ export class ExtensionRunner { }, applyCompaction: (precomputed, options) => { runner.assertActive(); - return runner.applyCompactionFn(precomputed, options); + return runner.applyCompactionFn(precomputed, { ...options, signal: options.signal ?? compactionSignal }); }, getSystemPrompt: () => { runner.assertActive(); diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index 779402181..bf9759999 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -336,6 +336,8 @@ export interface CompactOptions { export interface ApplyCompactionOptions { reason: CompactionReason; expectedRevision?: number; + /** The feedback operation that owns this apply, when one was begun. */ + signal?: AbortSignal; } export type ApplyCompactionResult = { applied: true; reason: "ok" } | { applied: false; reason: "stale" | "rejected" }; diff --git a/packages/coding-agent/test/compaction.test.ts b/packages/coding-agent/test/compaction.test.ts index b17954c65..21ea742ab 100644 --- a/packages/coding-agent/test/compaction.test.ts +++ b/packages/coding-agent/test/compaction.test.ts @@ -1,4 +1,5 @@ import type { AgentMessage } from "@earendil-works/pi-agent-core"; +import type { Tool } from "@earendil-works/pi-ai"; import type { AssistantMessage, Context, Model, StreamOptions, Usage } from "@earendil-works/pi-ai/compat"; import { readFileSync } from "fs"; import { join } from "path"; @@ -200,18 +201,22 @@ interface CapturedCompactionHandlers { modelSelect: ModelSelectHandler; } -function captureBeforeAgentStartHandler(): BeforeAgentStartHandler { +function captureBeforeAgentStartHandler(options?: { + activeTools?: string[]; + allTools?: Tool[]; +}): BeforeAgentStartHandler { let handler: BeforeAgentStartHandler | undefined; - const api: ExtensionAPI = Object.assign(Object.create(null), { + const api = Object.assign(Object.create(null), { on: (event: string, currentHandler: BeforeAgentStartHandler) => { if (event === "before_agent_start") { handler = currentHandler; } }, appendEntry: vi.fn(), - getActiveTools: () => [], + getActiveTools: () => options?.activeTools ?? [], + getAllTools: () => options?.allTools ?? [], getThinkingLevel: () => "off" as const, - }); + }) as ExtensionAPI; compactionExtension(api); if (!handler) { @@ -932,6 +937,54 @@ describe("builtin compaction extension threshold regressions", () => { expect(order).toEqual(["auth-start", "apply-called", "hook-returned"]); }); + it("sends only active registered and MCP tools to blocking compaction summarization", async () => { + const handler = captureBeforeAgentStartHandler({ + activeTools: ["registered_active"], + allTools: [ + { + name: "registered_active", + description: "Active registered tool", + parameters: { type: "object", properties: {} }, + }, + { + name: "mcp__inactive_server__query", + description: "Inactive MCP tool", + parameters: { type: "object", properties: {} }, + }, + ], + }); + const model = createAnthropicModel("claude-tools", 200_000); + const branchEntries = [ + createMessageEntry(createUserMessage("first request")), + createMessageEntry(createAssistantMessage("first answer", createMockUsage(4_000, 500))), + createMessageEntry(createUserMessage("second request")), + createMessageEntry(createAssistantMessage("second answer", createMockUsage(5_000, 500))), + ]; + const providerToolNames: string[][] = []; + completeMock.mockImplementation(async (_model: Model, context: Context) => { + providerToolNames.push((context.tools ?? []).map((tool) => tool.name)); + return createAssistantMessage("summary constrained to active tools"); + }); + const ctx = createExtensionContext({ + model, + sessionManager: Object.assign(Object.create(null), { + getEntries: () => branchEntries, + getBranch: () => branchEntries, + }) as ExtensionContext["sessionManager"], + modelRegistry: Object.assign(Object.create(null), { + getApiKeyAndHeaders: async () => ({ ok: true as const, apiKey: "test-key" }), + }) as ExtensionContext["modelRegistry"], + getContextUsage: () => ({ tokens: 190_000, contextWindow: 200_000, percent: 95 }), + getCompactionSettings: () => ({ ...DEFAULT_COMPACTION_SETTINGS, keepRecentTokens: 1 }), + beginCompaction: () => new AbortController().signal, + applyCompaction: async () => ({ applied: true, reason: "ok" }), + }); + + await handler({ type: "before_agent_start", systemPrompt: "system" }, ctx); + + expect(providerToolNames).toEqual([["registered_active"]]); + }); + it("starts compaction feedback before blocking extension summary generation", async () => { // given const handler = captureBeforeAgentStartHandler(); diff --git a/packages/coding-agent/test/suite/agent-session-compaction.test.ts b/packages/coding-agent/test/suite/agent-session-compaction.test.ts index 6bd08bd11..ab66b1c16 100644 --- a/packages/coding-agent/test/suite/agent-session-compaction.test.ts +++ b/packages/coding-agent/test/suite/agent-session-compaction.test.ts @@ -1,4 +1,4 @@ -import type { AgentMessage } from "@earendil-works/pi-agent-core"; +import type { AgentMessage, PrepareNextTurnContext } from "@earendil-works/pi-agent-core"; import { type AssistantMessage, createAssistantMessageEventStream, @@ -73,6 +73,49 @@ function createUsage(totalTokens: number) { }; } +function createDeferred(): { promise: Promise; resolve: () => void } { + let resolve: (() => void) | undefined; + const promise = new Promise((next) => { + resolve = next; + }); + if (!resolve) throw new Error("Deferred resolver was not initialized"); + return { promise, resolve }; +} + +function bypassFirstPrePromptCompaction(harness: Harness): void { + const original = Reflect.get(harness.session, "_enforceCompactionBeforeProvider"); + if (typeof original !== "function") { + throw new Error("AgentSession._enforceCompactionBeforeProvider is not available for retry characterization"); + } + let bypassed = false; + Reflect.set(harness.session, "_enforceCompactionBeforeProvider", async (...args: unknown[]) => { + if (!bypassed) { + bypassed = true; + return false; + } + return await original.apply(harness.session, args); + }); +} + +function seedSuccessfulContextAboveThreshold(harness: Harness): void { + const model = harness.getModel(); + const timestamp = Date.now() - 1_000; + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "successful context seed" }], + timestamp: timestamp - 1, + }); + harness.sessionManager.appendMessage( + createAssistant(harness, { + text: "successful response before retryable failure", + stopReason: "stop", + totalTokens: (model.contextWindow ?? 10_000) - 999, + timestamp, + }), + ); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; +} + function createAssistant( harness: Harness, options: { @@ -605,6 +648,70 @@ describe("AgentSession compaction characterization", () => { expect(callbackContexts[0]).not.toContain("callback prior context "); }); + it("retains a constructor next-turn context transform in the provider request after compaction", async () => { + const largeToolResult = "transformed callback tool output ".repeat(300); + const compactionSummary = "transform-before-next-turn summary"; + const injectedMarker = "INJECTED_NEXT_TURN_CONTEXT"; + const callbackInputs: string[] = []; + let continuationRequest = ""; + const prepareNextTurnWithContext = vi.fn(async (turn: PrepareNextTurnContext) => { + callbackInputs.push(JSON.stringify(turn.context.messages)); + return { + context: { + ...turn.context, + messages: [ + { + role: "user" as const, + content: [{ type: "text" as const, text: injectedMarker }], + timestamp: Date.now(), + }, + ...turn.context.messages.filter((message) => message.role !== "compactionSummary").reverse(), + ], + }, + }; + }); + const harness = await createHarness({ + settings: { + compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 }, + retry: { enabled: false }, + }, + models: [{ id: "faux-1", contextWindow: 5_000 }], + extensionFactories: [createResultToolExtension(largeToolResult, compactionSummary)], + prepareNextTurnWithContext, + }); + harnesses.push(harness); + const timestamp = Date.now() - 2_000; + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "transform prior context ".repeat(220) }], + timestamp, + }); + harness.sessionManager.appendMessage( + createAssistant(harness, { + text: "prior response", + stopReason: "stop", + totalTokens: 700, + timestamp: timestamp + 1_000, + }), + ); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + fauxAssistantMessage(fauxToolCall("large_result", {}), { stopReason: "toolUse" }), + (context) => { + continuationRequest = JSON.stringify(context.messages); + return fauxAssistantMessage("done after transformed callback"); + }, + ]); + + await harness.session.prompt("run the transformed callback result tool"); + + expect(prepareNextTurnWithContext).toHaveBeenCalled(); + expect(callbackInputs).toContainEqual(expect.stringContaining(compactionSummary)); + expect(continuationRequest).toContain(injectedMarker); + expect(continuationRequest).not.toContain(compactionSummary); + expect(continuationRequest.indexOf(injectedMarker)).toBeLessThan(continuationRequest.indexOf(largeToolResult)); + }); + it("applies the provider context transform to inline compaction summarization", async () => { // given const sensitiveToolOutput = "SENSITIVE_TOOL_OUTPUT"; @@ -970,6 +1077,100 @@ describe("AgentSession compaction characterization", () => { expect(continueSpy).toHaveBeenCalledTimes(1); }); + it("compacts a retryable zero-usage error before retrying when the prior context is above threshold", async () => { + let acceptedCompactionsAtRetryProviderCall = 0; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { + compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 }, + retry: { enabled: true, maxRetries: 1, baseDelayMs: 0 }, + }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => ({ + compaction: { + summary: "retry threshold summary", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })); + }, + ], + }); + harnesses.push(harness); + seedSuccessfulContextAboveThreshold(harness); + bypassFirstPrePromptCompaction(harness); + harness.setResponses([ + createAssistant(harness, { stopReason: "error", errorMessage: "overloaded_error", totalTokens: 0 }), + () => { + acceptedCompactionsAtRetryProviderCall = harness + .eventsOfType("compaction_end") + .filter((event) => event.reason === "threshold" && event.accepted).length; + return fauxAssistantMessage("retry succeeded after compaction"); + }, + ]); + + await harness.session.prompt("trigger zero-usage retryable failure"); + + expect(harness.faux.state.callCount).toBe(2); + expect(acceptedCompactionsAtRetryProviderCall).toBe(1); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction")).toHaveLength(1); + }); + + it("retains queues and skips the retry provider call when required retry compaction is rejected", async () => { + const providerStarted = createDeferred(); + const releaseError = createDeferred(); + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 10_000, maxTokens: 1_000 }], + settings: { + compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 }, + retry: { enabled: true, maxRetries: 1, baseDelayMs: 0 }, + }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "retry compaction rejected", + })); + }, + ], + }); + harnesses.push(harness); + seedSuccessfulContextAboveThreshold(harness); + bypassFirstPrePromptCompaction(harness); + harness.setResponses([ + async () => { + providerStarted.resolve(); + await releaseError.promise; + return createAssistant(harness, { + stopReason: "error", + errorMessage: "overloaded_error", + totalTokens: 0, + }); + }, + fauxAssistantMessage("retry provider must not run"), + ]); + + const prompt = harness.session.prompt("trigger rejected retry compaction"); + await providerStarted.promise; + await harness.session.followUp("retain retry follow-up"); + releaseError.resolve(); + await prompt; + + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "threshold", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + expect(harness.faux.state.callCount).toBe(1); + expect(harness.session.getFollowUpMessages()).toEqual(["retain retry follow-up"]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + }); + it("does not retry overflow recovery more than once", async () => { const harness = await createHarness(); harnesses.push(harness); diff --git a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts index 600a1fc44..958cc3ef9 100644 --- a/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts +++ b/packages/coding-agent/test/suite/compaction-feedback-lifecycle.test.ts @@ -419,4 +419,67 @@ describe("compaction feedback lifecycle", () => { expect.objectContaining({ reason: "extension", errorMessage: "B ended itself" }), ]); }); + + it("rejects A's stale apply after B supersedes feedback from the same emission", async () => { + let captureContexts = false; + let contextA: ExtensionContext | undefined; + let contextB: ExtensionContext | undefined; + let signalA: AbortSignal | undefined; + let signalB: AbortSignal | undefined; + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.on("agent_settled", (_event, ctx) => { + if (!captureContexts) return; + contextA = ctx; + signalA = ctx.beginCompaction?.({ reason: "extension" }); + }); + pi.on("agent_settled", (_event, ctx) => { + if (!captureContexts) return; + contextB = ctx; + signalB = ctx.beginCompaction?.({ reason: "extension" }); + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("seed response")]); + await harness.session.prompt("seed context"); + + const firstEntry = harness.sessionManager.getEntries()[0]; + if (!firstEntry) throw new Error("Expected a persisted entry for precomputed compaction"); + const expectedRevision = harness.session.getMessageRevision(); + const precomputed = { + summary: "summary prepared by A before B superseded it", + firstKeptEntryId: firstEntry.id, + tokensBefore: 42, + }; + captureContexts = true; + await harness.getExtensionRunner().emit({ type: "agent_settled" }); + if (!contextA || !contextB || !signalA || !signalB) { + throw new Error("Expected both same-emission extension contexts to begin feedback"); + } + + try { + expect(signalA.aborted).toBe(true); + expect(signalB.aborted).toBe(false); + const staleResult = await contextA.applyCompaction(precomputed, { + reason: "extension", + expectedRevision, + }); + expect(staleResult).toEqual({ applied: false, reason: "stale" }); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction")).toHaveLength(0); + expect(harness.session.compactionState).toMatchObject({ status: "running", generation: 2, stage: "feedback" }); + + const freshResult = await contextB.applyCompaction(precomputed, { + reason: "extension", + expectedRevision, + }); + expect(freshResult).toEqual({ applied: true, reason: "ok" }); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction")).toHaveLength(1); + expect(harness.session.compactionState).toMatchObject({ status: "completed" }); + } finally { + contextB.endCompaction?.({ reason: "extension", signal: signalB }); + } + }); }); diff --git a/packages/coding-agent/test/suite/regressions/6162-extension-active-tools-next-turn.test.ts b/packages/coding-agent/test/suite/regressions/6162-extension-active-tools-next-turn.test.ts index cbe86a392..eafec40b9 100644 --- a/packages/coding-agent/test/suite/regressions/6162-extension-active-tools-next-turn.test.ts +++ b/packages/coding-agent/test/suite/regressions/6162-extension-active-tools-next-turn.test.ts @@ -1,6 +1,7 @@ import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai"; import { Type } from "typebox"; import { describe, expect, it } from "vitest"; +import type { ExtensionContext } from "../../../src/core/extensions/types.ts"; import type { ExtensionFactory } from "../../../src/index.ts"; import { createHarness } from "../harness.ts"; @@ -189,4 +190,59 @@ describe("extension active tools next-turn refresh", () => { harness.cleanup(); } }); + + it("stale-rejects an in-flight extension compaction after a tool is revoked", async () => { + let capturedContext: ExtensionContext | undefined; + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.registerTool({ + name: "active_compaction_tool", + label: "Active Compaction Tool", + description: "Remains active while compaction is prepared", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "active" }], details: {} }), + }); + pi.registerTool({ + name: "mcp__revoked_server__tool", + label: "Revoked MCP Tool", + description: "Is revoked before its summary can apply", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "revoked" }], details: {} }), + }); + pi.on("agent_settled", (_event, ctx) => { + capturedContext = ctx; + }); + }, + ], + }); + + try { + harness.session.setActiveToolsByName(["active_compaction_tool", "mcp__revoked_server__tool"]); + harness.setResponses([fauxAssistantMessage("seed response")]); + await harness.session.prompt("seed in-flight compaction context"); + if (!capturedContext) throw new Error("Expected an ExtensionContext from agent_settled"); + const firstEntry = harness.sessionManager.getEntries()[0]; + if (!firstEntry) throw new Error("Expected a persisted source entry"); + const expectedRevision = capturedContext.getMessageRevision(); + const signal = capturedContext.beginCompaction?.({ reason: "extension" }); + if (!signal) throw new Error("Expected compaction feedback signal"); + + harness.session.setActiveToolsByName(["active_compaction_tool"]); + const result = await capturedContext.applyCompaction( + { + summary: "summary prepared with mcp__revoked_server__tool", + firstKeptEntryId: firstEntry.id, + tokensBefore: 42, + }, + { reason: "extension", expectedRevision }, + ); + + expect(result).toEqual({ applied: false, reason: "stale" }); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction")).toHaveLength(0); + } finally { + capturedContext?.endCompaction?.({ reason: "extension" }); + harness.cleanup(); + } + }); }); diff --git a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts index daaa385b2..9ddaed336 100644 --- a/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts +++ b/packages/coding-agent/test/suite/regressions/pre-prompt-compaction-no-continue.test.ts @@ -723,4 +723,192 @@ describe("pre-prompt compaction regression", () => { ).rejects.toThrow("Context remains above the compaction threshold because compaction did not complete"); expect(harness.faux.state.callCount).toBe(1); }); + + it.each([ + { + label: "silent overflow", + contextWindow: 10_000, + reserveTokens: 0, + prompt: "x".repeat(44_000), + reason: "overflow", + }, + { + label: "threshold", + contextWindow: 13_000, + reserveTokens: 3_000, + prompt: "x".repeat(38_000), + reason: "threshold", + }, + ])("keeps the agent signal live for a signal-aware $label agent_end continuation", async (scenario) => { + const observedAbortStates: boolean[] = []; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: scenario.contextWindow, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: scenario.reserveTokens } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => ({ + compaction: { + summary: `${scenario.label} signal summary`, + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })); + }, + (pi) => { + let queued = false; + pi.on("agent_end", (_event, ctx) => { + observedAbortStates.push(ctx.signal?.aborted ?? false); + if (queued || ctx.signal?.aborted) return; + queued = true; + pi.sendUserMessage("signal-aware continuation", { deliverAs: "followUp" }); + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage(`${scenario.label} answer`), + fauxAssistantMessage("continuation answer"), + ]); + + await harness.session.prompt(scenario.prompt); + await harness.session.waitForSettledSessionWork(); + + expect(observedAbortStates[0]).toBe(false); + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ reason: scenario.reason, accepted: true }), + ); + expect(harness.faux.state.callCount).toBe(2); + expect(getUserTexts(harness)).toContain("signal-aware continuation"); + expect(getAssistantTexts(harness)).toContain("continuation answer"); + }); + + it("still exposes genuine user cancellation to an agent_end handler", async () => { + const providerStarted = createDeferred(); + const releaseProvider = createDeferred(); + const observedAbortStates: boolean[] = []; + const harness = await createHarness({ + extensionFactories: [ + (pi) => { + pi.on("agent_end", (_event, ctx) => { + observedAbortStates.push(ctx.signal?.aborted ?? false); + }); + }, + ], + }); + harnesses.push(harness); + harness.setResponses([ + async () => { + providerStarted.resolve(); + await releaseProvider.promise; + return fauxAssistantMessage("cancelled response"); + }, + ]); + + const prompt = harness.session.prompt("wait for cancellation"); + await providerStarted.promise; + const abort = harness.session.abort(); + releaseProvider.resolve(); + await abort; + await prompt; + await harness.session.waitForSettledSessionWork(); + + expect(observedAbortStates).toContain(true); + }); + + it("rejects both normal and custom admissions after an oversized custom message follows accepted compaction", async () => { + let compactionRequests = 0; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 5_000, maxTokens: 1_000 }], + settings: { compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 } }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async () => { + compactionRequests++; + return { + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "reject follow-up compaction", + }; + }); + }, + ], + }); + harnesses.push(harness); + + const now = Date.now(); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "context before compaction" }], + timestamp: now - 1_000, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("retained assistant", { timestamp: now - 500 }), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(100), + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + const retainedAssistant = harness.sessionManager.getEntries().at(-1); + if (retainedAssistant?.type !== "message") { + throw new Error("Expected an assistant entry to retain after compaction"); + } + + await expect( + harness.session.applyCompaction( + { + summary: "accepted baseline summary", + firstKeptEntryId: retainedAssistant.id, + tokensBefore: 100, + }, + { reason: "extension", expectedRevision: harness.session.getMessageRevision() }, + ), + ).resolves.toEqual({ applied: true, reason: "ok" }); + + const oversizedCustomContent = "oversized custom state ".repeat(2_000); + await harness.session.sendCustomMessage({ + customType: "oversized-post-compaction-state", + content: oversizedCustomContent, + display: true, + }); + harness.setResponses([ + fauxAssistantMessage("normal provider must not run"), + fauxAssistantMessage("custom provider must not run"), + ]); + + const normalError = await harness.session.prompt("normal admission").then( + () => undefined, + (error: unknown) => error, + ); + const customError = await harness.session + .sendCustomMessage( + { customType: "custom-trigger", content: "custom admission", display: true }, + { triggerTurn: true }, + ) + .then( + () => undefined, + (error: unknown) => error, + ); + + expect(normalError).toBeInstanceOf(Error); + expect((normalError as Error).message).toContain( + "Context remains above the compaction threshold because compaction did not complete", + ); + expect(customError).toBeInstanceOf(Error); + expect((customError as Error).message).toContain( + "Context remains above the compaction threshold because compaction did not complete", + ); + expect(compactionRequests).toBe(2); + expect(harness.faux.state.callCount).toBe(0); + expect(harness.session.messages).toContainEqual( + expect.objectContaining({ + role: "custom", + customType: "oversized-post-compaction-state", + content: oversizedCustomContent, + }), + ); + }); }); From d5a5d5aa9a5dbb706b8ec8585e77cc6ff41825d7 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 24 Jul 2026 06:34:45 +0900 Subject: [PATCH 07/10] fix(coding-agent): synchronize fallback compaction state --- .../coding-agent/src/core/agent-session.ts | 255 ++++++++++++++---- packages/coding-agent/src/core/changes.md | 5 + .../src/core/compaction/changes.md | 4 + .../src/core/compaction/compaction.ts | 6 +- .../src/core/extensions/changes.md | 2 + .../coding-agent/src/core/extensions/types.ts | 2 +- .../coding-agent/src/core/session-manager.ts | 32 ++- .../src/modes/interactive/changes.md | 17 ++ .../src/modes/interactive/interactive-mode.ts | 7 +- .../test/interactive-mode-compaction.test.ts | 4 +- ...mpaction-generation-stale-revision.test.ts | 92 +++++++ ...ost-compaction-queued-input-resume.test.ts | 146 +++++++++- .../test/suite/retry-fallback-engine.test.ts | 235 ++++++++++++---- 13 files changed, 697 insertions(+), 110 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 9816f7d0d..dc7853418 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -110,6 +110,7 @@ import type { ApplyCompactionResult, CompactionReason, CompactionRejectionCause, + ModelSelectSource, } from "./extensions/types.ts"; import { type BashExecutionMessage, type CustomMessage, filterContextExcludedMessages } from "./messages.ts"; import { ModelRegistry } from "./model-registry.ts"; @@ -278,6 +279,7 @@ interface CompactionExecutionRequest { skipAbortedCheck?: boolean; lastAssistantMessage?: AgentMessage; precomputed?: CompactionResult; + allowSummaryOnly?: boolean; agentMessagesAtStart?: readonly AgentMessage[]; } @@ -512,6 +514,12 @@ export class AgentSession { private readonly _sessionWorkBarrier = new SessionWorkBarrier(); private _overflowRecoveryAttempted = false; private _requiredCompactionAdmissionError: RequiredCompactionError | undefined; + // A retry continuation immediately follows an accepted compaction. Its first + // response must not retrigger threshold compaction from stale provider usage. + private _skipNextPostRetryCompactionCheck = false; + private _blockedPostCompactionAssistant: { assistant: AssistantMessage; revision: number } | undefined; + private _skipNextPostCompactionAssistantCheck = false; + private readonly _assistantsPendingAtCompaction = new WeakSet(); private _messageRevision = 0; // Branch summarization state @@ -612,8 +620,9 @@ export class AgentSession { persistDefault: false, appendSessionEntry: true, entryReason: reason, - emitModelSelect: reason === "fallback-revert", - invalidateCompaction: false, + emitModelSelect: true, + modelSelectSource: reason, + invalidateCompaction: true, ephemeralThinkingLevel: thinking, }); }, @@ -872,6 +881,7 @@ export class AgentSession { private _incrementMessageRevision(): void { this._messageRevision++; + this._blockedPostCompactionAssistant = undefined; } getMessageRevision(): number { @@ -1057,6 +1067,7 @@ export class AgentSession { * runs, so only this preflight can transfer required admissions safely. */ private _getRequiredAutoCompactionReason(message: AssistantMessage): "overflow" | "threshold" | undefined { + if (this._skipNextPostRetryCompactionCheck) return undefined; const settings = this.settingsManager.getCompactionSettings(); if (!settings.enabled || message.stopReason === "aborted") { return undefined; @@ -1096,7 +1107,7 @@ export class AgentSession { if ( compactionEntry && usageMessage?.role === "assistant" && - usageMessage.timestamp <= new Date(compactionEntry.timestamp).getTime() + this._isAssistantFromBeforeLatestCompaction(usageMessage) ) { return undefined; } @@ -1271,8 +1282,9 @@ export class AgentSession { } } - // Check auto-retry and auto-compaction after agent completes + // Check auto-retry and auto-compaction after agent completes. let launchedContinuation = false; + let retryContinuationBlocked = false; const userAbortSuppressedQueuedContinuation = event.type === "agent_end" && this._suppressQueuedContinuationAfterUserAbort; if (userAbortSuppressedQueuedContinuation) { @@ -1285,60 +1297,65 @@ export class AgentSession { if (event.type === "agent_end" && this._lastAssistantMessage) { const msg = this._lastAssistantMessage; this._lastAssistantMessage = undefined; - const requiredAutoCompaction = this._getRequiredAutoCompactionReason(msg); + const skipPostRetryCompaction = this._skipNextPostRetryCompactionCheck; + this._skipNextPostRetryCompactionCheck = false; + const requiredAutoCompaction = skipPostRetryCompaction + ? undefined + : this._getRequiredAutoCompactionReason(msg); // Retry transient failures normally and eligible hard errors only through a fallback. const retryableError = this._isRetryableError(msg); const hardErrorFallbackEligible = this._isHardErrorFallbackEligible(msg); const retryCanAdmitProvider = this.settingsManager.getRetrySettings().enabled && (retryableError || hardErrorFallbackEligible); - let retryCompactionRejected = false; let compactedBeforeRetry = false; if (retryCanAdmitProvider && requiredAutoCompaction) { - try { - compactedBeforeRetry = await this._enforceCompactionBeforeProvider(msg, true, "threshold", true); - } catch (error) { - if (error instanceof RequiredCompactionError) { - retryCompactionRejected = true; - } else { - throw error; - } - } + this._retireFailedRetryAssistant(msg); + compactedBeforeRetry = await this._runPrePromptCompaction(msg, true, "threshold", true); + retryContinuationBlocked = !compactedBeforeRetry; } - let didRetry = false; - if (!retryCompactionRejected) { + let retryOutcome: "continued" | "blocked" | "not-handled" = "not-handled"; + if (!retryContinuationBlocked) { if (retryableError) { - didRetry = await this._handleRetryableError(msg); + retryOutcome = await this._handleRetryableError(msg); } else if (hardErrorFallbackEligible) { - didRetry = await this._handleRetryableError(msg, { hardErrorFallback: true }); + retryOutcome = await this._handleRetryableError(msg, { hardErrorFallback: true }); } } - if (didRetry) return; // Retry was initiated, don't proceed to compaction + if (retryOutcome === "continued") return; this._resolveRetry(); - if (compactedBeforeRetry && this.agent.hasQueuedMessages()) { - this._scheduleContinuationAfterCurrentEvent(); - launchedContinuation = true; - } else if (!retryCompactionRejected) { - launchedContinuation = await this._checkCompaction(msg); - // _runAutoCompaction() returns false both when recovery was rejected and - // when an accepted compaction had no queue to continue. Re-sample after - // it settles so only the still-required (rejected) case fails admission. - if ( - requiredAutoCompaction && - !launchedContinuation && - this.agent.hasQueuedMessages() && - this._getRequiredAutoCompactionReason(msg) !== undefined - ) { - this._requiredCompactionAdmissionError = new RequiredCompactionError(); + retryContinuationBlocked ||= retryOutcome === "blocked"; + if (!retryContinuationBlocked && !skipPostRetryCompaction) { + if (compactedBeforeRetry && this.agent.hasQueuedMessages()) { + this._scheduleContinuationAfterCurrentEvent(); + launchedContinuation = true; + } else { + launchedContinuation = await this._checkCompaction(msg); + // _runAutoCompaction() returns false both when recovery was rejected and + // when an accepted compaction had no queue to continue. Re-sample after + // it settles so only the still-required (rejected) case fails admission. + if ( + requiredAutoCompaction && + !launchedContinuation && + this.agent.hasQueuedMessages() && + this._getRequiredAutoCompactionReason(msg) !== undefined + ) { + this._requiredCompactionAdmissionError = new RequiredCompactionError(); + } } } } if (event.type === "agent_end") { this._flushPendingBashMessages(); - if (!launchedContinuation && allowsQueuedContinuation && this.agent.hasQueuedMessages()) { + if ( + !launchedContinuation && + !retryContinuationBlocked && + allowsQueuedContinuation && + this.agent.hasQueuedMessages() + ) { this._scheduleContinuationAfterCurrentEvent(); launchedContinuation = true; } @@ -1378,9 +1395,47 @@ export class AgentSession { return undefined; } + /** + * Retry failures stay in append-only history but must not be retained in the + * active context branch. Otherwise split-turn compaction keeps the failed + * assistant response verbatim and cannot make progress before a retry. + */ + private _retireFailedRetryAssistant(message: AssistantMessage): void { + const position = this.sessionManager.getMessageEntryPosition(message); + if (!position || this.sessionManager.getLeafId() !== position.entryId) return; + const entry = this.sessionManager.getEntry(position.entryId); + if (entry?.type !== "message") return; + + if (entry.parentId === null) { + this.sessionManager.resetLeaf(); + } else { + this.sessionManager.branch(entry.parentId); + } + const messageIndex = this.agent.state.messages.lastIndexOf(message); + if (messageIndex !== -1) { + this.agent.state.messages = this.agent.state.messages.slice(0, messageIndex); + } + this._incrementMessageRevision(); + } + private _isAssistantFromBeforeLatestCompaction(assistantMessage: AssistantMessage): boolean { const compactionEntry = getLatestCompactionEntry(this.sessionManager.getBranch()); - return compactionEntry !== null && assistantMessage.timestamp <= new Date(compactionEntry.timestamp).getTime(); + if (compactionEntry === null) return false; + + // An agent message_end can still be awaiting persistence while compaction + // commits. It is necessarily a post-boundary message, even when a provider + // supplied an older payload timestamp. + if (this._messageEndsAwaitingPersistence.has(assistantMessage)) return false; + + const messagePosition = this.sessionManager.getMessageEntryPosition(assistantMessage); + const compactionOrder = this.sessionManager.getEntryOrder(compactionEntry.id); + if (messagePosition !== undefined && compactionOrder !== undefined) { + return messagePosition.order <= compactionOrder; + } + + // Reloaded/reconstructed messages have no runtime identity. Retain the + // historical timestamp heuristic only for that compatibility path. + return assistantMessage.timestamp <= new Date(compactionEntry.timestamp).getTime(); } private _replaceMessageInPlace(target: AgentMessage, replacement: AgentMessage): void { @@ -2538,7 +2593,7 @@ export class AgentSession { private async _emitModelSelect( nextModel: Model, previousModel: Model | undefined, - source: "set" | "cycle" | "restore", + source: ModelSelectSource, ): Promise { if (!this._modelSelectionChangesContext(previousModel, nextModel)) return undefined; const result = await this._extensionRunner.emitModelSelect({ @@ -2613,6 +2668,7 @@ export class AgentSession { persistDefault: updateGlobalDefaults, appendSessionEntry: true, emitModelSelect: true, + modelSelectSource: "set", invalidateCompaction: true, }); } @@ -2633,6 +2689,7 @@ export class AgentSession { appendSessionEntry: boolean; entryReason?: "fallback" | "fallback-revert"; emitModelSelect: boolean; + modelSelectSource: ModelSelectSource; invalidateCompaction: boolean; ephemeralThinkingLevel?: ThinkingLevel; }, @@ -2668,7 +2725,7 @@ export class AgentSession { } if (!opts.emitModelSelect) return undefined; - return await this._emitModelSelect(model, previousModel, "set"); + return await this._emitModelSelect(model, previousModel, opts.modelSelectSource); } private _applyEphemeralThinkingLevel(level: ThinkingLevel): void { @@ -3076,7 +3133,12 @@ export class AgentSession { let fromExtension = request.precomputed !== undefined; if (!compactionResult) { - const preparation = prepareCompaction(pathEntries, settings, request.reason === "overflow"); + const preparation = prepareCompaction( + pathEntries, + settings, + request.reason === "overflow", + request.allowSummaryOnly, + ); if (!preparation) { const lastEntry = pathEntries[pathEntries.length - 1]; @@ -3201,6 +3263,12 @@ export class AgentSession { } } const preservedPendingMessages = currentAgentMessages.filter((message) => preservedIdentities.delete(message)); + this._skipNextPostCompactionAssistantCheck = true; + for (const message of preservedPendingMessages) { + if (message.role === "assistant") { + this._assistantsPendingAtCompaction.add(message); + } + } this.agent.state.messages = [...sessionContext.messages, ...preservedPendingMessages]; compactionResult.estimatedTokensAfter = estimateMessagesTokens(sessionContext.messages); this._incrementMessageRevision(); @@ -3401,6 +3469,20 @@ export class AgentSession { inlineReason: "pre_prompt" | "threshold", retryAfterCompaction = false, ): Promise { + const blockedAdmission = this._blockedPostCompactionAssistant; + if ( + blockedAdmission !== undefined && + blockedAdmission.assistant === assistantMessage && + blockedAdmission.revision === this._messageRevision + ) { + throw new RequiredCompactionError(); + } + + const settings = this.settingsManager.getCompactionSettings(); + const model = this.model; + const contextTokens = estimateContextTokens( + filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), + ).tokens; const compacted = assistantMessage ? await this._checkCompaction(assistantMessage, skipAbortedCheck, inlineReason, retryAfterCompaction) : false; @@ -3408,11 +3490,7 @@ export class AgentSession { return compacted; } - const settings = this.settingsManager.getCompactionSettings(); - const contextTokens = estimateContextTokens( - filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), - ).tokens; - if (!settings.enabled || !this.model || !shouldCompact(contextTokens, this.model.contextWindow, settings)) { + if (!settings.enabled || !model || !shouldCompact(contextTokens, model.contextWindow, settings)) { return false; } @@ -3468,6 +3546,15 @@ export class AgentSession { if (this._isAssistantFromBeforeLatestCompaction(assistantMessage)) { return false; } + if (this._skipNextPostCompactionAssistantCheck) { + this._skipNextPostCompactionAssistantCheck = false; + // The first ordinary post-compaction response can still report stale + // provider usage. An active overflow recovery is different: its retry + // response must be checked so the one-retry cap can terminate it. + if (!this._assistantsPendingAtCompaction.has(assistantMessage) && !this._overflowRecoveryAttempted) { + return false; + } + } // Case 1: Overflow - LLM returned context overflow error. // If the saved assistant provider differs from the currently selected provider alias, @@ -3481,7 +3568,18 @@ export class AgentSession { const willRetry = retryAfterCompaction || assistantMessage.stopReason !== "stop"; if (!willRetry) { - return await this._runAutoCompaction("overflow", false); + const compacted = await this._runAutoCompaction("overflow", false); + if ( + !compacted && + this._compactionLifecycle.state.status === "failed" && + getLatestCompactionEntry(this.sessionManager.getBranch()) !== null + ) { + this._blockedPostCompactionAssistant = { + assistant: assistantMessage, + revision: this._messageRevision, + }; + } + return compacted; } if (this._overflowRecoveryAttempted) { @@ -3547,7 +3645,7 @@ export class AgentSession { if ( compactionEntry && usageMsg.role === "assistant" && - (usageMsg as AssistantMessage).timestamp <= new Date(compactionEntry.timestamp).getTime() + this._isAssistantFromBeforeLatestCompaction(usageMsg) ) { return false; } @@ -3563,7 +3661,18 @@ export class AgentSession { retryAfterCompaction, ); } else { - return await this._runAutoCompaction("threshold", false); + const compacted = await this._runAutoCompaction("threshold", false); + if ( + !compacted && + this._compactionLifecycle.state.status === "failed" && + getLatestCompactionEntry(this.sessionManager.getBranch()) !== null + ) { + this._blockedPostCompactionAssistant = { + assistant: assistantMessage, + revision: this._messageRevision, + }; + } + return compacted; } } return false; @@ -3574,6 +3683,7 @@ export class AgentSession { skipAbortedCheck: boolean, reason: "pre_prompt" | "overflow" | "threshold" = "pre_prompt", willRetry = false, + allowSummaryOnly = false, ): Promise { this._emit({ type: "compaction_start", reason }); const controller = new AbortController(); @@ -3585,6 +3695,7 @@ export class AgentSession { willRetry, lastAssistantMessage, skipAbortedCheck, + allowSummaryOnly, }); if (!execution.accepted && isContextOverflow(lastAssistantMessage, this.model?.contextWindow ?? 0)) { this._overflowRecoveryAttempted = false; @@ -4350,16 +4461,16 @@ export class AgentSession { /** * Handle retryable errors with exponential backoff. - * @returns true if retry was initiated, false if max retries exceeded or disabled + * @returns whether retry continuation started, was blocked by compaction, or was not handled */ private async _handleRetryableError( message: AssistantMessage, options: { hardErrorFallback?: boolean } = {}, - ): Promise { + ): Promise<"continued" | "blocked" | "not-handled"> { const settings = this.settingsManager.getRetrySettings(); if (!settings.enabled) { this._resolveRetry(); - return false; + return "not-handled"; } // Retry promise is created synchronously in _handleAgentEvent for agent_end. @@ -4379,7 +4490,7 @@ export class AgentSession { switchedFallback = await this._retryFallback.tryFallback("hard-error", { errorMessage }); if (!switchedFallback) { this._resolveRetry(); - return false; + return "not-handled"; } // The fallback starts fresh; the failed model's transient attempts do not carry over. this._retryAttempt = 1; @@ -4388,7 +4499,7 @@ export class AgentSession { // same-model retries or the transient over-budget fallback escape hatch. if (this._retryAttempt + 1 > settings.maxRetries) { this._resolveRetry(); - return false; + return "not-handled"; } switchedFallback = await this._retryFallback.tryFallback("refusal", {}); if (!switchedFallback) { @@ -4397,7 +4508,7 @@ export class AgentSession { this._emit({ type: "retry_fallback_exhausted", chainKey: exhaustedChainKey, lastError: errorMessage }); } this._resolveRetry(); - return false; + return "not-handled"; } this._retryAttempt++; } else { @@ -4427,7 +4538,7 @@ export class AgentSession { }); this._retryAttempt = 0; this._resolveRetry(); - return false; + return "not-handled"; } } } @@ -4443,7 +4554,7 @@ export class AgentSession { }); this._retryAttempt = 0; this._resolveRetry(); - return false; + return "not-handled"; } if (!switchedFallback) { @@ -4487,7 +4598,7 @@ export class AgentSession { finalError: "Retry cancelled", }); this._resolveRetry(); - return false; + return "not-handled"; } this._retryAbortController = undefined; @@ -4495,6 +4606,34 @@ export class AgentSession { // the retry continue on the restored primary instead of the fallback model. await this._maybeRestoreFallbackPrimary(); + // Model fallback (or reversion) can select a smaller context window after + // the prior retry checks. Revalidate canonical session context immediately + // before the continuation so a rejected compaction never admits that model. + const model = this.model; + const compactionSettings = this.settingsManager.getCompactionSettings(); + const contextTokens = estimateContextTokens( + filterContextExcludedMessages(this.sessionManager.buildSessionContext().messages), + ).tokens; + if ( + compactionSettings.enabled && + model && + shouldCompact(contextTokens, model.contextWindow, compactionSettings) + ) { + if (!(await this._runPrePromptCompaction(message, true, "threshold", true, true))) { + const attempt = this._retryAttempt; + this._retryAttempt = 0; + this._emit({ + type: "auto_retry_end", + success: false, + attempt, + finalError: new RequiredCompactionError().message, + }); + this._resolveRetry(); + return "blocked"; + } + this._skipNextPostRetryCompactionCheck = true; + } + // Retry via continue() - use setTimeout to break out of event handler chain setTimeout(() => { this.agent.continue().catch(() => { @@ -4502,7 +4641,7 @@ export class AgentSession { }); }, 0); - return true; + return "continued"; } /** diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index e0bd01c46..f934a4fa5 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -22,6 +22,11 @@ - Retry and fallback admission resolve required compaction first; rejected recovery retains native queues without dispatching a provider retry. Active-tool changes advance the context revision and abort active core compaction so summaries prepared against a prior tool set cannot apply. +- Fallback apply/revert transitions emit typed model-selection events, rebuild model-scoped tools and prompts, abort + compaction prepared for the prior model, and re-run required compaction against the selected model's context window + before retrying. +- Message objects are associated with their persisted session-entry order. Compaction-boundary checks use that order + (and treat pending `message_end` persistence as post-boundary) instead of relying only on payload timestamps. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/compaction/changes.md b/packages/coding-agent/src/core/compaction/changes.md index 1661c54ad..d432dd283 100644 --- a/packages/coding-agent/src/core/compaction/changes.md +++ b/packages/coding-agent/src/core/compaction/changes.md @@ -15,6 +15,10 @@ post-`agent_end` queue drain so only an accepted `AgentSession` recovery may resume queued work, and overflow can force a split-turn preparation when keeping the only oversized prompt would otherwise leave no compactable source. - Compaction rejects stale source snapshots with `stale-revision` before the durable entry append. +- Retry fallback model changes invalidate prior-model compaction and re-check the selected model's context window. + Summary-only re-compaction is allowed only for this retry boundary. +- Assistant history is classified around the latest compaction by persisted branch order; an older payload timestamp + cannot hide a message whose entry was appended after the compaction boundary. ### Why diff --git a/packages/coding-agent/src/core/compaction/compaction.ts b/packages/coding-agent/src/core/compaction/compaction.ts index ba7ccdea0..001b29de3 100644 --- a/packages/coding-agent/src/core/compaction/compaction.ts +++ b/packages/coding-agent/src/core/compaction/compaction.ts @@ -753,6 +753,7 @@ export function prepareCompaction( pathEntries: SessionEntry[], settings: CompactionSettings, forceProgress = false, + allowSummaryOnly = false, ): CompactionPreparation | undefined { if (pathEntries.length > 0 && pathEntries[pathEntries.length - 1].type === "compaction") { return undefined; @@ -818,7 +819,10 @@ export function prepareCompaction( } } - if (messagesToSummarize.length === 0 && turnPrefixMessages.length === 0) { + // A model switch can make an existing summary too large even when no new + // messages were added. The retry fallback path explicitly opts into + // regenerating that summary for its selected model's smaller context window. + if (messagesToSummarize.length === 0 && turnPrefixMessages.length === 0 && (!previousSummary || !allowSummaryOnly)) { return undefined; } diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index 8035c37c6..a2efd1240 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -11,6 +11,8 @@ handler in the same event emission cannot rebind an old completion or durable apply to a newer operation. - `stale-revision` is a structured compaction rejection cause for a source that changed before durable append. - The builtin compaction extension threads that signal through local and remote summary generation and application. +- `model_select` sources now distinguish fallback apply and fallback revert transitions, allowing model-scoped + extensions to update prompts and active tools before the retry request. ### Why diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index bf9759999..b1f55aa0b 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -923,7 +923,7 @@ export interface ToolExecutionEndEvent { // Model Events // ============================================================================ -export type ModelSelectSource = "set" | "cycle" | "restore"; +export type ModelSelectSource = "set" | "cycle" | "restore" | "fallback" | "fallback-revert"; /** Fired when a new model is selected */ export interface ModelSelectEvent { diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 85bffed52..a950bdf3c 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -486,6 +486,10 @@ export function buildContextEntries( let foundFirstKept = false; for (let i = 0; i < compactionIdx; i++) { const entry = path[i]; + // The latest summary supersedes every older compaction summary. Older + // entries selected by firstKeptEntryId remain verbatim, but nesting a + // prior summary here would double-count it in the model context. + if (entry.type === "compaction") continue; if (entry.id === compaction.firstKeptEntryId) { foundFirstKept = true; } @@ -878,6 +882,12 @@ export class SessionManager { private flushed: boolean = false; private fileEntries: FileEntry[] = []; private byId: Map = new Map(); + // Runtime-only identity tracking lets AgentSession compare a live message to + // a compaction boundary by append order rather than provider timestamps. + // Reconstructed messages intentionally have no entry identity and use the + // timestamp fallback in AgentSession. + private entryOrdersById: Map = new Map(); + private messageEntryPositions = new WeakMap(); private labelsById: Map = new Map(); private labelTimestampsById: Map = new Map(); private leafId: string | null = null; @@ -978,6 +988,8 @@ export class SessionManager { this.fileEntries = [header]; this.residentStore.clear(); this.byId.clear(); + this.entryOrdersById.clear(); + this.messageEntryPositions = new WeakMap(); this.labelsById.clear(); this.labelTimestampsById.clear(); this.leafId = null; @@ -1002,6 +1014,8 @@ export class SessionManager { private _buildIndex(): void { this.byId.clear(); + this.entryOrdersById.clear(); + this.messageEntryPositions = new WeakMap(); this.labelsById.clear(); this.labelTimestampsById.clear(); this.leafId = null; @@ -1014,9 +1028,10 @@ export class SessionManager { cost: 0, latestCacheHitRate: undefined, }; - for (const entry of this.fileEntries) { + for (const [order, entry] of this.fileEntries.entries()) { if (entry.type === "session") continue; this.byId.set(entry.id, entry); + this.entryOrdersById.set(entry.id, order); this.leafId = entry.id; this._accumulateUsage(entry); if (entry.type === "session_info") { @@ -1109,6 +1124,7 @@ export class SessionManager { const residentEntry = this.residentStore.externalize(entry); this.fileEntries.push(residentEntry); this.byId.set(residentEntry.id, residentEntry); + this.entryOrdersById.set(residentEntry.id, this.fileEntries.length - 1); this.leafId = residentEntry.id; this._accumulateUsage(residentEntry); this.mutationCount++; @@ -1158,9 +1174,23 @@ export class SessionManager { message, }; this._appendEntry(entry); + const order = this.entryOrdersById.get(entry.id); + if (order !== undefined) { + this.messageEntryPositions.set(message, { entryId: entry.id, order }); + } return entry.id; } + /** Runtime append position for this exact persisted message object, if known. */ + getMessageEntryPosition(message: AgentMessage): Readonly<{ entryId: string; order: number }> | undefined { + return this.messageEntryPositions.get(message); + } + + /** Runtime/file append position for an entry, used with getMessageEntryPosition(). */ + getEntryOrder(entryId: string): number | undefined { + return this.entryOrdersById.get(entryId); + } + /** Append a thinking level change as child of current leaf, then advance leaf. Returns entry id. */ appendThinkingLevelChange(thinkingLevel: string): string { const entry: ThinkingLevelChangeEntry = { diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index 8db8b087c..396dd1fcd 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -1,5 +1,22 @@ # changes +## accepted-only compaction queue transfer (2026-07-24) + +### What changed + +- `interactive-mode.ts`: input queued while compaction owns the editor is automatically transferred only after an + accepted compaction result. Rejected, failed, or aborted compaction retains the input in the editor-owned queue + instead of resubmitting it through the unchanged required-compaction gate and recursively starting compaction. + +### Why + +- Rejection and cancellation do not create a new admissible context. Automatically replaying the same prompt caused an + unbounded compaction-start/rejection/restore loop. + +### Expected merge conflict zones + +- LOW: `interactive-mode.ts` `compaction_end` handling around `flushCompactionQueue()`. + ## per-section thinking duration headers (2026-07-22) ### What changed diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 31640688d..25c73f56a 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -3625,7 +3625,12 @@ export class InteractiveMode { this.chatContainer.addChild(new Text(theme.fg("error", message), 1, 0)); } } - void this.flushCompactionQueue({ willRetry: event.willRetry }); + // Only an accepted compaction transfers editor-owned input to the + // session. Rejections and aborts retain the draft for an explicit + // user retry while the UI cleanup above still always runs. + if (event.accepted === true || event.result !== undefined) { + void this.flushCompactionQueue({ willRetry: event.willRetry }); + } this.ui.requestRender(); break; } diff --git a/packages/coding-agent/test/interactive-mode-compaction.test.ts b/packages/coding-agent/test/interactive-mode-compaction.test.ts index cb182601f..9d84a6c57 100644 --- a/packages/coding-agent/test/interactive-mode-compaction.test.ts +++ b/packages/coding-agent/test/interactive-mode-compaction.test.ts @@ -200,7 +200,9 @@ describe("InteractiveMode compaction events", () => { ...fakeThis.showStatus.mock.calls.map((call) => String(call[0])), ].join("\n"); expect(feedback).toMatch(/would.?overflow|overflow|rejected/i); - expect(fakeThis.flushCompactionQueue).toHaveBeenCalledWith({ willRetry: false }); + // Rejected compaction retains editor-owned input instead of submitting it + // through a recursive post-compaction prompt path. + expect(fakeThis.flushCompactionQueue).not.toHaveBeenCalled(); }); test("sanitizes a detached continuation launch failure before rendering", async () => { diff --git a/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts b/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts index 617b26086..cf57e310b 100644 --- a/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts +++ b/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts @@ -107,4 +107,96 @@ describe("Regression: stale compaction generation after a revision change", () = expect(agentMessagesContaining(harness, "arrived mid-compaction")).toBe(1); expect(harness.eventsOfType("compaction_end").filter((event) => event.accepted === true)).toHaveLength(0); }); + + it("keeps a delayed assistant message_end post-compaction despite its earlier payload timestamp", async () => { + const assistantEndStarted = createDeferred(); + const releaseAssistantEnd = createDeferred(); + const payloadTimestamp = Date.now() - 10_000; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 1_000, maxTokens: 64 }], + settings: { compaction: { enabled: true, reserveTokens: 0, keepRecentTokens: 1 } }, + extensionFactories: [ + (pi: ExtensionAPI) => { + pi.on("message_end", async (event) => { + if ( + event.message.role !== "assistant" || + !getMessageText(event.message).includes("delayed assistant payload") + ) + return; + assistantEndStarted.resolve(); + await releaseAssistantEnd.promise; + }); + pi.on("session_before_compact", () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "subsequent admission must remain blocked", + })); + }, + ], + }); + harnesses.push(harness); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "seed pending persistence boundary" }], + timestamp: payloadTimestamp - 1, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + { + ...fauxAssistantMessage("delayed assistant payload"), + timestamp: payloadTimestamp, + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 1_200, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 1_200, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + }, + fauxAssistantMessage("must not reach the next provider admission"), + ]); + const delayedPrompt = harness.session.prompt("produce a delayed assistant"); + void delayedPrompt.catch(() => undefined); + await assistantEndStarted.promise; + const firstEntry = harness.sessionManager.getEntries()[0]; + if (!firstEntry) throw new Error("Expected a persisted entry before compaction"); + + const applied = await harness.session.applyCompaction( + { + summary: "summary that fits the original context window", + firstKeptEntryId: firstEntry.id, + tokensBefore: 42, + }, + { reason: "extension" }, + ); + expect(applied).toEqual({ applied: true, reason: "ok" }); + expect(agentMessagesContaining(harness, "delayed assistant payload")).toBe(1); + + releaseAssistantEnd.resolve(); + await delayedPrompt; + const branch = harness.sessionManager.getBranch(); + const compactionIndex = branch.findIndex((entry) => entry.type === "compaction"); + const delayedAssistantIndex = branch.findIndex( + (entry) => entry.type === "message" && getMessageText(entry.message).includes("delayed assistant payload"), + ); + expect(compactionIndex).toBeGreaterThanOrEqual(0); + expect(delayedAssistantIndex).toBeGreaterThan(compactionIndex); + const delayedAssistant = branch[delayedAssistantIndex]; + if (delayedAssistant?.type !== "message" || delayedAssistant.message.role !== "assistant") { + throw new Error("Expected the delayed assistant entry after compaction"); + } + expect(delayedAssistant.message.timestamp).toBe(payloadTimestamp); + expect(new Date(branch[compactionIndex]!.timestamp).getTime()).toBeGreaterThan(payloadTimestamp); + + await expect(harness.session.prompt("subsequent admission")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + expect(harness.faux.state.callCount).toBe(1); + expect(harness.eventsOfType("compaction_start")).toHaveLength(2); + }); }); diff --git a/packages/coding-agent/test/suite/regressions/post-compaction-queued-input-resume.test.ts b/packages/coding-agent/test/suite/regressions/post-compaction-queued-input-resume.test.ts index b62e25f9b..32a366605 100644 --- a/packages/coding-agent/test/suite/regressions/post-compaction-queued-input-resume.test.ts +++ b/packages/coding-agent/test/suite/regressions/post-compaction-queued-input-resume.test.ts @@ -1,8 +1,9 @@ import { readFileSync } from "node:fs"; import { type AssistantMessage, type FauxResponseFactory, fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, beforeAll, describe, expect, it } from "vitest"; import type { ExtensionAPI } from "../../../src/core/extensions/index.ts"; import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts"; +import { initTheme } from "../../../src/modes/interactive/theme/theme.ts"; import { createHarness, getUserTexts, type Harness } from "../harness.ts"; type Deferred = { @@ -49,6 +50,19 @@ function getFlushCompactionQueue() { Promise.resolve(flush.call(context, options)); } +function getHandleEvent() { + const handleEvent = Reflect.get(InteractiveMode.prototype, "handleEvent"); + if (typeof handleEvent !== "function") throw new Error("Expected InteractiveMode.handleEvent"); + return (context: object, event: object): Promise => Promise.resolve(handleEvent.call(context, event)); +} + +function getRunAutoCompaction(harness: Harness) { + const runAutoCompaction = Reflect.get(harness.session, "_runAutoCompaction"); + if (typeof runAutoCompaction !== "function") throw new Error("Expected AgentSession._runAutoCompaction"); + return (reason: "overflow" | "threshold", willRetry: boolean): Promise => + Promise.resolve(runAutoCompaction.call(harness.session, reason, willRetry)); +} + function createTuiQueueContext(harness: Harness) { return { compactionQueuedMessages: [] as QueuedMessage[], @@ -63,6 +77,49 @@ function createTuiQueueContext(harness: Harness) { }; } +function createTuiCompactionEventContext(harness: Harness) { + const context = createTuiQueueContext(harness) as ReturnType & { + isInitialized: boolean; + footer: { invalidate: () => void }; + autoCompactionEscapeHandler: (() => void) | undefined; + autoCompactionProgressText: string; + defaultEditor: { onEscape?: () => void }; + statusContainer: { clear: () => void }; + chatContainer: { clear: () => void; addChild: () => void }; + clearStatusIndicator: () => void; + rebuildChatFromMessages: () => void; + addMessageToChat: () => void; + showStatus: () => void; + ui: { requestRender: () => void; terminal: { setProgress: () => void } }; + settingsManager: { getShowTerminalProgress: () => boolean }; + flushCompactionQueue: (options: { willRetry: boolean }) => Promise; + flushes: Promise[]; + }; + const flushes: Promise[] = []; + Object.assign(context, { + isInitialized: true, + footer: { invalidate: () => {} }, + autoCompactionEscapeHandler: undefined, + autoCompactionProgressText: "", + defaultEditor: {}, + statusContainer: { clear: () => {} }, + chatContainer: { clear: () => {}, addChild: () => {} }, + clearStatusIndicator: () => {}, + rebuildChatFromMessages: () => {}, + addMessageToChat: () => {}, + showStatus: () => {}, + ui: { requestRender: () => {}, terminal: { setProgress: () => {} } }, + settingsManager: { getShowTerminalProgress: () => false }, + flushes, + flushCompactionQueue(options: { willRetry: boolean }) { + const flush = getFlushCompactionQueue()(context, options); + flushes.push(flush); + return flush; + }, + }); + return context; +} + async function submitLikeTui(harness: Harness, context: ReturnType, text: string) { if (harness.session.isCompacting) { context.compactionQueuedMessages.push({ text, mode: "steer" }); @@ -90,6 +147,10 @@ function createOverflowResponse(harness: Harness): AssistantMessage { } describe("post-compaction queued input recovery", () => { + beforeAll(() => { + initTheme("dark"); + }); + const harnesses: Harness[] = []; afterEach(() => { @@ -222,4 +283,87 @@ describe("post-compaction queued input recovery", () => { expect(countInPersistedSession(harness, OMO_MARKER)).toBe(1); expect(countInPersistedSession(harness, GOAL_MARKER)).toBe(1); }); + + it.each([ + ["rejected overflow", "overflow" as const, true, "reject" as const], + ["rejected threshold", "threshold" as const, false, "reject" as const], + ["feedback-only abort", undefined, false, "abort" as const], + ])("keeps queued TUI input owned by the editor after a %s compaction_end", async (_label, reason, willRetry, outcome) => { + const marker = `[TUI queue remains ${outcome}]`; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 128_000, maxTokens: 64 }], + settings: { compaction: { enabled: true, reserveTokens: 16_384, keepRecentTokens: 1 } }, + extensionFactories: + outcome === "reject" + ? [ + (pi: ExtensionAPI) => { + pi.on("session_before_compact", () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "test rejection", + })); + }, + ] + : [], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("seed handled"), fauxAssistantMessage("must not execute")]); + await harness.session.prompt("seed context ".repeat(40)); + const providerCallsBeforeCompaction = harness.faux.state.callCount; + const context = createTuiCompactionEventContext(harness); + context.compactionQueuedMessages.push({ text: marker, mode: "steer" }); + harness.session.subscribe((event) => { + if (event.type === "compaction_end") { + void getHandleEvent()(context, event); + } + }); + + if (outcome === "abort") { + const beginFeedback = Reflect.get(harness.session, "_beginExtensionCompactionFeedback"); + const endFeedback = Reflect.get(harness.session, "_endExtensionCompactionFeedback"); + if (typeof beginFeedback !== "function" || typeof endFeedback !== "function") { + throw new Error("Expected extension compaction feedback lifecycle methods"); + } + const signal = beginFeedback.call(harness.session, "extension") as AbortSignal; + endFeedback.call(harness.session, { reason: "extension", signal, aborted: true }); + } else { + await getRunAutoCompaction(harness)(reason!, willRetry); + } + await Promise.all(context.flushes); + await harness.session.waitForSettledSessionWork(); + + expect(context.compactionQueuedMessages).toEqual([{ text: marker, mode: "steer" }]); + expect(context.compactionInFlightMessages).toEqual([]); + expect(getUserTexts(harness)).not.toContain(marker); + expect(harness.faux.state.callCount).toBe(providerCallsBeforeCompaction); + expect(harness.eventsOfType("compaction_start")).toHaveLength(1); + }); + + it("flushes accepted compaction input once through the real compaction_end handler", async () => { + const marker = "[TUI queue flushes once]"; + const harness = await createHarness({ + models: [{ id: "faux-1", contextWindow: 128_000, maxTokens: 64 }], + settings: { compaction: { enabled: true, reserveTokens: 16_384, keepRecentTokens: 1 } }, + extensionFactories: [createAcceptedCompactionExtension()], + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("seed handled"), fauxAssistantMessage("queued marker handled")]); + await harness.session.prompt("seed context ".repeat(40)); + const context = createTuiCompactionEventContext(harness); + context.compactionQueuedMessages.push({ text: marker, mode: "steer" }); + harness.session.subscribe((event) => { + if (event.type === "compaction_end") { + void getHandleEvent()(context, event); + } + }); + + await getRunAutoCompaction(harness)("threshold", false); + await Promise.all(context.flushes); + await harness.session.waitForSettledSessionWork(); + + expect(context.compactionQueuedMessages).toEqual([]); + expect(context.compactionInFlightMessages).toEqual([]); + expect(getUserTexts(harness).filter((text) => text === marker)).toHaveLength(1); + expect(harness.faux.state.callCount).toBe(2); + }); }); diff --git a/packages/coding-agent/test/suite/retry-fallback-engine.test.ts b/packages/coding-agent/test/suite/retry-fallback-engine.test.ts index 854b869a5..d71574b2c 100644 --- a/packages/coding-agent/test/suite/retry-fallback-engine.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-engine.test.ts @@ -94,79 +94,222 @@ describe("retry fallback engine", () => { expect(harness.eventsOfType("agent_end").map((event) => event.willRetry)).toEqual([true, false]); }); - it("removes only the failed assistant while preserving state across a fallback switch", async () => { - const snapshotTool: AgentTool = { - name: "snapshot", - label: "Snapshot", - description: "Provides stable tool state for fallback assertions.", + it("rebuilds model-scoped prompt and tools through an explicit fallback model_select", async () => { + const primaryPrivilegedTool: AgentTool = { + name: "primary_privileged", + label: "Primary Privileged", + description: "Must never remain active after a fallback switch.", parameters: Type.Object({}), - execute: async () => ({ - content: [{ type: "text", text: "snapshot" }], - details: {}, - }), + execute: async () => ({ content: [{ type: "text", text: "primary" }], details: {} }), + }; + const fallbackPresetTool: AgentTool = { + name: "fallback_preset", + label: "Fallback Preset", + description: "Only available through the fallback model preset.", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "fallback" }], details: {} }), }; + const modelSelectSources: string[] = []; const harness = await createHarness({ models: [{ id: "faux-1" }, { id: "faux-2" }], - tools: [snapshotTool], - settings: { - retry: { - enabled: true, - baseDelayMs: 1, - fallbackChains: { [primary]: [fallback] }, + tools: [primaryPrivilegedTool, fallbackPresetTool], + initialActiveToolNames: ["primary_privileged"], + settings: { retry: { enabled: true, baseDelayMs: 1, fallbackChains: { [primary]: [fallback] } } }, + extensionFactories: [ + (pi) => { + pi.on("model_select", (event) => { + modelSelectSources.push(`${event.previousModel?.id ?? "none"}->${event.model.id}:${event.source}`); + if (event.model.id !== "faux-2") return undefined; + pi.setActiveTools(["fallback_preset"]); + return { systemPrompt: "fallback preset system prompt", systemPromptName: "fallback-preset" }; + }); }, - }, + ], }); harnesses.push(harness); - harness.setResponses([ - fauxAssistantMessage("first"), - fauxAssistantMessage("", { - stopReason: "error", - errorMessage: "overloaded_error", - }), - fauxAssistantMessage("recovered"), - ]); + harness.setResponses([fauxAssistantMessage("first")]); await harness.session.prompt("first turn"); let stateBeforeFailedAssistant: typeof harness.session.state.messages | undefined; - let systemPromptBeforeFailedAssistant: string | undefined; - let toolsBeforeFailedAssistant: unknown; let fallbackRequestMessages: unknown; let stateAtFallbackRequest: typeof harness.session.state.messages | undefined; + let fallbackRequestSystemPrompt: string | undefined; + let fallbackRequestToolNames: string[] | undefined; harness.session.subscribe((event) => { - if (event.type === "auto_retry_start") { + if (event.type === "auto_retry_start") stateBeforeFailedAssistant = structuredClone(harness.session.state.messages); - systemPromptBeforeFailedAssistant = harness.session.state.systemPrompt; - toolsBeforeFailedAssistant = JSON.parse(JSON.stringify(harness.session.state.tools)); - } }); harness.setResponses([ - fauxAssistantMessage("", { - stopReason: "error", - errorMessage: "overloaded_error", - }), + fauxAssistantMessage("", { stopReason: "error", errorMessage: "overloaded_error" }), (context) => { fallbackRequestMessages = structuredClone(context.messages); stateAtFallbackRequest = structuredClone(harness.session.state.messages); + fallbackRequestSystemPrompt = context.systemPrompt; + fallbackRequestToolNames = (context.tools ?? []).map((tool) => tool.name); return fauxAssistantMessage("recovered"); }, ]); await harness.session.prompt("second turn"); - if ( - !stateBeforeFailedAssistant || - systemPromptBeforeFailedAssistant === undefined || - !toolsBeforeFailedAssistant - ) { - throw new Error("Missing pre-error fallback snapshot"); - } + if (!stateBeforeFailedAssistant) throw new Error("Missing pre-error fallback snapshot"); expect(stateAtFallbackRequest).toEqual(stateBeforeFailedAssistant.slice(0, -1)); expect(fallbackRequestMessages).toEqual(stateBeforeFailedAssistant.slice(0, -1)); - expect(harness.session.state.systemPrompt).toBe(systemPromptBeforeFailedAssistant); - expect(JSON.stringify(harness.session.state.systemPrompt)).toBe( - JSON.stringify(systemPromptBeforeFailedAssistant), - ); - expect(JSON.stringify(harness.session.state.tools)).toBe(JSON.stringify(toolsBeforeFailedAssistant)); + expect(modelSelectSources).toEqual(["faux-1->faux-2:fallback"]); + expect(fallbackRequestSystemPrompt).toBe("fallback preset system prompt"); + expect(fallbackRequestToolNames).toEqual(["fallback_preset"]); + expect(harness.session.systemPrompt).toBe("fallback preset system prompt"); + expect(harness.session.getActiveToolNames()).toEqual(["fallback_preset"]); + expect(harness.session.getActiveToolNames()).not.toContain("primary_privileged"); + }); + + it("invalidates an in-flight compaction when retry fallback changes the model", async () => { + const harness = await createHarness({ + models: [{ id: "faux-1" }, { id: "faux-2" }], + settings: { retry: { enabled: true, baseDelayMs: 1, fallbackChains: { [primary]: [fallback] } } }, + }); + harnesses.push(harness); + harness.setResponses([fauxAssistantMessage("seed")]); + await harness.session.prompt("seed fallback compaction state"); + const firstEntry = harness.sessionManager.getEntries()[0]; + if (!firstEntry) throw new Error("Expected a persisted seed entry"); + const beginFeedback = Reflect.get(harness.session, "_beginExtensionCompactionFeedback"); + if (typeof beginFeedback !== "function") throw new Error("Expected extension compaction feedback lifecycle"); + const signal = beginFeedback.call(harness.session, "extension") as AbortSignal; + let oldApply: Promise | undefined; + harness.session.subscribe((event) => { + if (event.type !== "auto_retry_start" || event.delayMs !== 0) return; + oldApply = harness.session.applyCompaction( + { summary: "must not apply after fallback selection", firstKeptEntryId: firstEntry.id, tokensBefore: 42 }, + { reason: "extension", signal }, + ); + }); + harness.setResponses([ + fauxAssistantMessage("", { stopReason: "error", errorMessage: "overloaded_error" }), + fauxAssistantMessage("fallback answer"), + ]); + + await harness.session.prompt("trigger fallback while compaction is pending"); + + if (!oldApply) throw new Error("Expected fallback retry to attempt the stale apply"); + await expect(oldApply).resolves.toEqual({ applied: false, reason: "stale" }); + expect(signal.aborted).toBe(true); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "compaction")).toHaveLength(0); + }); + + it.each([ + ["rejects", true], + ["accepts", false], + ])("revalidates the smaller fallback context window immediately before a retry (%s second compaction)", async (_label, rejectSecondCompaction) => { + let compactionCount = 0; + let releasePrimaryError: (() => void) | undefined; + const primaryErrorReady = new Promise((resolve) => { + releasePrimaryError = resolve; + }); + let primaryProviderStarted: (() => void) | undefined; + const primaryStarted = new Promise((resolve) => { + primaryProviderStarted = resolve; + }); + const fallbackCompactionEndsAtCall: number[] = []; + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000, maxTokens: 64 }, + { id: "faux-2", contextWindow: 100, maxTokens: 64 }, + ], + settings: { + compaction: { enabled: true, reserveTokens: 0, keepRecentTokens: 0 }, + retry: { enabled: true, baseDelayMs: 1, fallbackChains: { [primary]: [fallback] } }, + }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", (event) => { + compactionCount++; + if (compactionCount === 2 && rejectSecondCompaction) { + return { + cancel: true, + rejectionCause: "cancelled-by-extension" as const, + reason: "fallback window requires a rejected second compaction", + }; + } + return { + compaction: { + summary: compactionCount === 1 ? "p".repeat(480) : "fallback summary", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + }, + }; + }); + }, + ], + }); + harnesses.push(harness); + const primaryModel = harness.getModel("faux-1"); + if (!primaryModel) throw new Error("Expected primary fallback model"); + const historyTimestamp = Date.now() - 1_000; + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "history before retry fallback" }], + timestamp: historyTimestamp, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("history response", { timestamp: historyTimestamp + 1 }), + api: primaryModel.api, + provider: primaryModel.provider, + model: primaryModel.id, + usage: { + input: 900, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 900, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + async (_context, _options, _state, model) => { + if (model.id === "faux-1") { + primaryProviderStarted?.(); + await primaryErrorReady; + return fauxAssistantMessage("context ".repeat(900), { + stopReason: "error", + errorMessage: "overloaded_error", + }); + } + fallbackCompactionEndsAtCall.push( + harness.eventsOfType("compaction_end").filter((event) => event.accepted === true).length, + ); + return fauxAssistantMessage("fallback answer"); + }, + (_context, _options, _state, model) => { + fallbackCompactionEndsAtCall.push( + harness.eventsOfType("compaction_end").filter((event) => event.accepted === true).length, + ); + expect(model.id).toBe("faux-2"); + return fauxAssistantMessage("fallback answer"); + }, + fauxAssistantMessage("queued continuation answer"), + ]); + + const prompt = harness.session.prompt("trigger fallback window revalidation"); + await primaryStarted; + if (rejectSecondCompaction) { + await harness.session.followUp("queued until fallback context is safe"); + } + releasePrimaryError?.(); + await prompt; + + expect(harness.eventsOfType("retry_fallback_applied")).toMatchObject([{ from: primary, to: fallback }]); + if (rejectSecondCompaction) { + expect(fallbackCompactionEndsAtCall).toEqual([]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + expect(harness.eventsOfType("compaction_start")).toHaveLength(2); + expect(harness.eventsOfType("compaction_end").at(-1)).toMatchObject({ accepted: false }); + } else { + expect(fallbackCompactionEndsAtCall).toEqual([2]); + expect(harness.eventsOfType("compaction_start")).toHaveLength(2); + expect(harness.eventsOfType("compaction_end").filter((event) => event.accepted === true)).toHaveLength(2); + } }); it("submits a complete fallback request rather than reusing primary continuation state", async () => { From bf4c8c2cd9ba308e0f05bed8418287a5b8d4f480 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 24 Jul 2026 07:43:23 +0900 Subject: [PATCH 08/10] fix(coding-agent): preserve resumed compaction contracts --- .../coding-agent/src/core/agent-session.ts | 26 ++- packages/coding-agent/src/core/changes.md | 4 + .../coding-agent/src/core/session-manager.ts | 29 ++- .../src/modes/interactive/changes.md | 2 + .../src/modes/interactive/interactive-mode.ts | 21 +- .../test/interactive-mode-compaction.test.ts | 94 ++++++++ .../suite/agent-session-compaction.test.ts | 98 ++++++++ ...mpaction-generation-stale-revision.test.ts | 209 +++++++++++++++++- 8 files changed, 465 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index dc7853418..8d91d7b92 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -836,8 +836,8 @@ export class AgentSession { ...turn, context: { ...turn.context, messages }, }; - const previousSnapshot = await previousPrepareNextTurnWithContext?.(postCompactionTurn, signal); - const previousContext = previousSnapshot?.context ?? postCompactionTurn.context; + let previousSnapshot = await previousPrepareNextTurnWithContext?.(postCompactionTurn, signal); + let previousContext = previousSnapshot?.context ?? postCompactionTurn.context; // The previous callback may await while agent_end extensions enqueue // continuation work. Re-sample after it returns so that work cannot // slip through with the stale provider snapshot it observed on entry. @@ -845,12 +845,32 @@ export class AgentSession { if (!compactedBeforeCallback) { compactedAfterCallback = await compactBeforeNextAdmission(); } + if (compactedAfterCallback) { + // The callback's first result describes the stale pre-compaction + // context. Reapply it once to the compacted context so any host + // transformation reaches the provider request. Do not re-sample after + // this invocation: one replay is the bounded admission path. + const postLateCompactionTurn = { + ...turn, + context: { ...turn.context, messages: this.agent.state.messages.slice() }, + }; + const postLateCompactionSnapshot = await previousPrepareNextTurnWithContext?.( + postLateCompactionTurn, + signal, + ); + previousSnapshot = { + ...previousSnapshot, + ...postLateCompactionSnapshot, + context: postLateCompactionSnapshot?.context ?? postLateCompactionTurn.context, + }; + previousContext = previousSnapshot.context ?? postLateCompactionTurn.context; + } return { ...previousSnapshot, context: { ...previousContext, - messages: compactedAfterCallback ? this.agent.state.messages.slice() : previousContext.messages, + messages: previousContext.messages, systemPrompt: this._systemPromptOverride ?? this._baseSystemPrompt, tools: this.agent.state.tools.slice(), }, diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index f934a4fa5..586ee2f68 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -27,6 +27,10 @@ before retrying. - Message objects are associated with their persisted session-entry order. Compaction-boundary checks use that order (and treat pending `message_end` persistence as post-boundary) instead of relying only on payload timestamps. +- Session reload materialization restores those message-to-entry associations, so older payload timestamps cannot + bypass post-compaction admission after reopening a session. +- When a late queue triggers compaction after a host `prepareNextTurnWithContext` callback, the callback is replayed + once against the compacted context so its message filtering/injection contract reaches the provider request. ### Why extension system couldn't handle this alone diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index a950bdf3c..5b1235721 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -882,10 +882,10 @@ export class SessionManager { private flushed: boolean = false; private fileEntries: FileEntry[] = []; private byId: Map = new Map(); - // Runtime-only identity tracking lets AgentSession compare a live message to - // a compaction boundary by append order rather than provider timestamps. - // Reconstructed messages intentionally have no entry identity and use the - // timestamp fallback in AgentSession. + // Runtime-only identity tracking lets AgentSession compare messages to a + // compaction boundary by append order rather than provider timestamps. + // Materialized persisted messages are bound as they are read; only pending + // messages that have not reached a session entry use AgentSession's fallback. private entryOrdersById: Map = new Map(); private messageEntryPositions = new WeakMap(); private labelsById: Map = new Map(); @@ -1131,6 +1131,17 @@ export class SessionManager { this._persist(residentEntry); } + private _materializeEntry(entry: SessionEntry): SessionEntry { + const materialized = this.residentStore.materialize(entry); + if (materialized.type === "message") { + const order = this.entryOrdersById.get(materialized.id); + if (order !== undefined) { + this.messageEntryPositions.set(materialized.message, { entryId: materialized.id, order }); + } + } + return materialized; + } + /** * Fold one entry into the running usage totals. Totals iterate ALL entries * (not branch-scoped), matching the footer hot path's historical semantics. @@ -1325,12 +1336,12 @@ export class SessionManager { getLeafEntry(): SessionEntry | undefined { const entry = this.leafId ? this.byId.get(this.leafId) : undefined; - return entry ? this.residentStore.materialize(entry) : undefined; + return entry ? this._materializeEntry(entry) : undefined; } getEntry(id: string): SessionEntry | undefined { const entry = this.byId.get(id); - return entry ? this.residentStore.materialize(entry) : undefined; + return entry ? this._materializeEntry(entry) : undefined; } /** @@ -1340,7 +1351,7 @@ export class SessionManager { const children: SessionEntry[] = []; for (const entry of this.byId.values()) { if (entry.parentId === parentId) { - children.push(this.residentStore.materialize(entry)); + children.push(this._materializeEntry(entry)); } } return children; @@ -1401,7 +1412,7 @@ export class SessionManager { const startId = fromId ?? this.leafId; let current = startId ? this.byId.get(startId) : undefined; while (current) { - path.unshift(this.residentStore.materialize(current)); + path.unshift(this._materializeEntry(current)); current = current.parentId ? this.byId.get(current.parentId) : undefined; } if (fromId === undefined) { @@ -1481,7 +1492,7 @@ export class SessionManager { } const entries = this.fileEntries .filter((e): e is SessionEntry => e.type !== "session") - .map((entry) => this.residentStore.materialize(entry)); + .map((entry) => this._materializeEntry(entry)); this.entriesCache = { mutation: this.mutationCount, entries }; return entries; } diff --git a/packages/coding-agent/src/modes/interactive/changes.md b/packages/coding-agent/src/modes/interactive/changes.md index 396dd1fcd..a060c17e9 100644 --- a/packages/coding-agent/src/modes/interactive/changes.md +++ b/packages/coding-agent/src/modes/interactive/changes.md @@ -7,6 +7,8 @@ - `interactive-mode.ts`: input queued while compaction owns the editor is automatically transferred only after an accepted compaction result. Rejected, failed, or aborted compaction retains the input in the editor-owned queue instead of resubmitting it through the unchanged required-compaction gate and recursively starting compaction. +- Consecutive `compaction_start` events share one Escape override. The original editor handler is preserved through + supersession and restored exactly once on terminal cleanup, session rebind, invalidation, or TUI stop. ### Why diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 25c73f56a..13c2fa665 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -418,6 +418,13 @@ export interface InteractiveModeOptions { } export class InteractiveMode { + private static restoreCompactionEscapeOverride(host: InteractiveMode): void { + if (!host.compactionEscapeOverrideActive) return; + host.defaultEditor.onEscape = host.autoCompactionEscapeHandler; + host.autoCompactionEscapeHandler = undefined; + host.compactionEscapeOverrideActive = false; + } + private runtimeHost: AgentSessionRuntime; private options: InteractiveModeOptions; private ui: TUI; @@ -512,6 +519,7 @@ export class InteractiveMode { // Auto-compaction state private autoCompactionEscapeHandler?: () => void; + private compactionEscapeOverrideActive = false; private autoCompactionProgressText = ""; // Auto-retry state @@ -572,6 +580,7 @@ export class InteractiveMode { this.options = options; this.autoTrustOnReloadCwd = options.autoTrustOnReloadCwd; this.runtimeHost.setBeforeSessionInvalidate(() => { + InteractiveMode.restoreCompactionEscapeOverride(this); this.resetExtensionUI(); }); this.runtimeHost.setRebindSession(async () => { @@ -1895,6 +1904,7 @@ export class InteractiveMode { } private async rebindCurrentSession(options: { renderBeforeBind?: boolean } = {}): Promise { + InteractiveMode.restoreCompactionEscapeOverride(this); this.unsubscribe?.(); this.unsubscribe = undefined; this.applyRuntimeSettings(); @@ -3541,7 +3551,10 @@ export class InteractiveMode { this.ui.terminal.setProgress(true); } // Keep editor active; submissions are queued during compaction. - this.autoCompactionEscapeHandler = this.defaultEditor.onEscape; + if (!this.compactionEscapeOverrideActive) { + this.autoCompactionEscapeHandler = this.defaultEditor.onEscape; + this.compactionEscapeOverrideActive = true; + } this.defaultEditor.onEscape = () => { this.session.abortCompaction(); }; @@ -3574,10 +3587,7 @@ export class InteractiveMode { if (this.settingsManager.getShowTerminalProgress()) { this.ui.terminal.setProgress(false); } - if (this.autoCompactionEscapeHandler) { - this.defaultEditor.onEscape = this.autoCompactionEscapeHandler; - this.autoCompactionEscapeHandler = undefined; - } + InteractiveMode.restoreCompactionEscapeOverride(this); this.clearStatusIndicator("compaction"); this.autoCompactionProgressText = ""; if (event.aborted) { @@ -6681,6 +6691,7 @@ export class InteractiveMode { } stop(): void { + InteractiveMode.restoreCompactionEscapeOverride(this); this.streamingReveal.stop(); this.toolResultReveal.stop(); if (this.settingsManager.getShowTerminalProgress()) { diff --git a/packages/coding-agent/test/interactive-mode-compaction.test.ts b/packages/coding-agent/test/interactive-mode-compaction.test.ts index 9d84a6c57..1efad274e 100644 --- a/packages/coding-agent/test/interactive-mode-compaction.test.ts +++ b/packages/coding-agent/test/interactive-mode-compaction.test.ts @@ -299,4 +299,98 @@ describe("InteractiveMode compaction events", () => { expect(fakeThis.compactionQueuedMessages).toEqual([]); expect(fakeThis.showError).not.toHaveBeenCalled(); }); + + test("restores the normal escape handler after a superseded compaction sequence", async () => { + const statusContainer = new Container(); + const abortCompaction = vi.fn(); + const abortAndFireQueuedMessages = vi.fn().mockResolvedValue(undefined); + const fakeThis = { + isInitialized: true, + footer: { invalidate: vi.fn() }, + autoCompactionEscapeHandler: undefined as (() => void) | undefined, + autoCompactionProgressText: "", + retryEscapeHandler: undefined as (() => void) | undefined, + activeStatusIndicator: undefined as { dispose(): void } | undefined, + defaultEditor: { + onEscape: undefined as (() => void) | undefined, + onAction: vi.fn(), + onCtrlD: undefined as unknown, + onChange: undefined as unknown, + onPasteImage: undefined as unknown, + }, + editor: { getText: () => "", setText: vi.fn() }, + session: { + isStreaming: true, + retryAttempt: 0, + isBashRunning: false, + abortBash: vi.fn(), + abortCompaction, + }, + abortAndFireQueuedMessages, + isBashMode: false, + lastEscapeTime: 0, + statusContainer, + chatContainer: { clear: vi.fn(), addChild: vi.fn() }, + rebuildChatFromMessages: vi.fn(), + addMessageToChat: vi.fn(), + showError: vi.fn(), + showWarning: vi.fn(), + showStatus: vi.fn(), + clearStatusIndicator: vi.fn(), + updateEditorBorderColor: vi.fn(), + showTreeSelector: vi.fn(), + showUserMessageSelector: vi.fn(), + flushCompactionQueue: vi.fn().mockResolvedValue(undefined), + settingsManager: { getShowTerminalProgress: () => false, getDoubleEscapeAction: () => "none" }, + ui: { requestRender: vi.fn(), terminal: { setProgress: vi.fn() }, onDebug: undefined as unknown }, + }; + + // Install the real normal Escape handler, then drive the real event handler + // through a supersession sequence: compaction A starts, compaction B starts + // before A ends (A is superseded and never emits compaction_end), then B ends. + const setupKeyHandlers = Reflect.get(InteractiveMode.prototype, "setupKeyHandlers") as ( + this: typeof fakeThis, + ) => void; + setupKeyHandlers.call(fakeThis); + const normalEscapeHandler = fakeThis.defaultEditor.onEscape; + expect(typeof normalEscapeHandler).toBe("function"); + + const handleEvent = Reflect.get(InteractiveMode.prototype, "handleEvent") as ( + this: typeof fakeThis, + event: + | { type: "compaction_start"; reason: "extension" } + | { + type: "compaction_end"; + reason: "extension"; + result: { tokensBefore: number; summary: string } | undefined; + aborted: boolean; + willRetry: boolean; + accepted: boolean; + }, + ) => Promise; + + await handleEvent.call(fakeThis, { type: "compaction_start", reason: "extension" }); + await handleEvent.call(fakeThis, { type: "compaction_start", reason: "extension" }); + await handleEvent.call(fakeThis, { + type: "compaction_end", + reason: "extension", + result: { tokensBefore: 42, summary: "summary" }, + aborted: false, + willRetry: false, + accepted: true, + }); + + // The compaction escape override must be fully unwound back to the handler + // that was installed before compaction A started, not to compaction A's stale + // abort closure captured when compaction B superseded it. + expect(fakeThis.autoCompactionEscapeHandler).toBeUndefined(); + expect(fakeThis.defaultEditor.onEscape).toBe(normalEscapeHandler); + + // Escape during streaming/retry must run the normal cancellation path. + fakeThis.defaultEditor.onEscape?.(); + expect(abortAndFireQueuedMessages).toHaveBeenCalledTimes(1); + expect(abortCompaction).not.toHaveBeenCalled(); + + fakeThis.activeStatusIndicator?.dispose(); + }); }); diff --git a/packages/coding-agent/test/suite/agent-session-compaction.test.ts b/packages/coding-agent/test/suite/agent-session-compaction.test.ts index ab66b1c16..bc129c2b1 100644 --- a/packages/coding-agent/test/suite/agent-session-compaction.test.ts +++ b/packages/coding-agent/test/suite/agent-session-compaction.test.ts @@ -712,6 +712,104 @@ describe("AgentSession compaction characterization", () => { expect(continuationRequest.indexOf(injectedMarker)).toBeLessThan(continuationRequest.indexOf(largeToolResult)); }); + it("reapplies a late constructor next-turn transform to the post-compaction provider request", async () => { + // given a constructor callback that suspends mid-turn and transforms the + // context (inject + redact + reorder), and a queue that arrives while the + // callback is suspended so the second admission sample compacts. + const callbackStarted = createDeferred(); + const releaseCallback = createDeferred(); + const injectedMarker = "INJECTED_LATE_CALLBACK_CONTEXT"; + const compactionSummary = "late callback compaction summary"; + const queuedText = "queued while the next-turn callback is suspended"; + let continuationRequest = ""; + const prepareNextTurnWithContext = vi.fn(async (turn: PrepareNextTurnContext) => { + callbackStarted.resolve(); + await releaseCallback.promise; + return { + context: { + ...turn.context, + messages: [ + { + role: "user" as const, + content: [{ type: "text" as const, text: injectedMarker }], + timestamp: Date.now(), + }, + ...turn.context.messages.filter((message) => message.role !== "compactionSummary").reverse(), + ], + }, + }; + }); + const harness = await createHarness({ + settings: { + compaction: { enabled: true, keepRecentTokens: 1, reserveTokens: 1_000 }, + retry: { enabled: false }, + }, + models: [{ id: "faux-1", contextWindow: 5_000 }], + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", async (event) => ({ + compaction: { + summary: compactionSummary, + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + details: {}, + }, + })); + }, + ], + prepareNextTurnWithContext, + }); + harnesses.push(harness); + const seedTimestamp = Date.now() - 2_000; + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "late callback prior context ".repeat(220) }], + timestamp: seedTimestamp, + }); + harness.sessionManager.appendMessage( + createAssistant(harness, { + text: "prior response", + stopReason: "stop", + totalTokens: 700, + timestamp: seedTimestamp + 1_000, + }), + ); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + const model = harness.getModel(); + harness.setResponses([ + { + ...fauxAssistantMessage("first response before the queued admission"), + api: model.api, + provider: model.provider, + model: model.id, + usage: createUsage(4_500), + }, + (context) => { + continuationRequest = JSON.stringify(context.messages); + return fauxAssistantMessage("response after the queued admission"); + }, + ]); + + // when the first turn completes, the callback suspends, a queue arrives, and + // the second admission sample compacts before the continuation request. + const promptPromise = harness.session.prompt("trigger the late next-turn callback"); + void promptPromise.catch(() => undefined); + await callbackStarted.promise; + await harness.session.steer(queuedText); + releaseCallback.resolve(); + await promptPromise; + + // then the queued admission did compact exactly once, and the provider + // request for the drained queue still respects the host transformation + // (reapplied on the post-compaction context), not the raw compacted state. + expect(prepareNextTurnWithContext).toHaveBeenCalled(); + expect(harness.eventsOfType("compaction_start")).toHaveLength(1); + expect(continuationRequest).toContain(injectedMarker); + expect(continuationRequest).not.toContain(compactionSummary); + expect(continuationRequest.indexOf(injectedMarker)).toBeGreaterThanOrEqual(0); + expect(continuationRequest.indexOf(injectedMarker)).toBeLessThan(continuationRequest.indexOf(queuedText)); + }); + it("applies the provider context transform to inline compaction summarization", async () => { // given const sensitiveToolOutput = "SENSITIVE_TOOL_OUTPUT"; diff --git a/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts b/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts index cf57e310b..a0a0b804c 100644 --- a/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts +++ b/packages/coding-agent/test/suite/regressions/compaction-generation-stale-revision.test.ts @@ -1,6 +1,19 @@ +import { join } from "node:path"; +import { Agent } from "@earendil-works/pi-agent-core"; import { fauxAssistantMessage } from "@earendil-works/pi-ai"; import { afterEach, describe, expect, it } from "vitest"; -import type { ExtensionAPI } from "../../../src/core/extensions/index.ts"; +import { AgentSession, type AgentSessionEvent } from "../../../src/core/agent-session.ts"; +import type { ExtensionAPI, ExtensionRunner } from "../../../src/core/extensions/index.ts"; +import { convertToLlm } from "../../../src/core/messages.ts"; +import { SessionManager } from "../../../src/core/session-manager.ts"; +import { type Settings, SettingsManager } from "../../../src/core/settings-manager.ts"; +import type { InlineExtension } from "../../../src/index.ts"; +import { createInMemoryModelRegistry, getModelRuntime } from "../../model-runtime-test-utils.ts"; +import { + type CreateTestExtensionsResultInput, + createTestExtensionsResult, + createTestResourceLoader, +} from "../../utilities.ts"; import { createHarness, getMessageText, type Harness } from "../harness.ts"; type Deferred = { @@ -25,6 +38,79 @@ function agentMessagesContaining(harness: Harness, text: string): number { return harness.session.messages.filter((message) => getMessageText(message).includes(text)).length; } +interface ReloadedHarness { + session: AgentSession; + sessionManager: SessionManager; + events: AgentSessionEvent[]; +} + +/** + * Rebuild the full SessionManager/AgentSession stack from the source harness's + * persisted session file, mirroring a close/reopen cycle. Reloaded messages are + * new object identities with no runtime position bookkeeping, exactly like a + * process restart. + */ +async function reloadHarnessFromSessionFile( + source: Harness, + options: { + settings?: Partial; + extensionFactories?: Array; + } = {}, +): Promise { + const sessionFile = source.sessionManager.getSessionFile(); + if (!sessionFile) throw new Error("Expected the source harness to persist its session file"); + const sessionManager = SessionManager.open(sessionFile); + const settingsManager = SettingsManager.inMemory(options.settings); + const model = source.getModel(); + const modelRegistry = await createInMemoryModelRegistry(source.authStorage); + modelRegistry.registerProvider(model.provider, { + baseUrl: model.baseUrl, + apiKey: "faux-key", + api: source.faux.api, + models: source.models.map((registeredModel) => ({ + id: registeredModel.id, + name: registeredModel.name, + api: registeredModel.api, + reasoning: registeredModel.reasoning, + input: registeredModel.input, + cost: registeredModel.cost, + contextWindow: registeredModel.contextWindow, + maxTokens: registeredModel.maxTokens, + baseUrl: registeredModel.baseUrl, + })), + }); + const extensionRunnerRef: { current?: ExtensionRunner } = {}; + const agent = new Agent({ + getApiKey: () => "faux-key", + initialState: { + model, + systemPrompt: "You are a test assistant.", + tools: [], + }, + convertToLlm, + }); + const extensionsResult = options.extensionFactories + ? await createTestExtensionsResult(options.extensionFactories, source.tempDir) + : undefined; + const resourceLoader = createTestResourceLoader(extensionsResult ? { extensionsResult } : undefined); + const session = new AgentSession({ + agent, + sessionManager, + settingsManager, + cwd: source.tempDir, + agentDir: join(source.tempDir, "agent-reload"), + modelRuntime: getModelRuntime(modelRegistry), + resourceLoader, + extensionRunnerRef, + }); + const events: AgentSessionEvent[] = []; + session.subscribe((event) => { + events.push(event); + }); + session.agent.state.messages = sessionManager.buildSessionContext().messages; + return { session, sessionManager, events }; +} + async function appendMidCompactionMessage(harness: Harness): Promise { await harness.session.sendCustomMessage({ customType: "mid-compaction-note", @@ -199,4 +285,125 @@ describe("Regression: stale compaction generation after a revision change", () = expect(harness.faux.state.callCount).toBe(1); expect(harness.eventsOfType("compaction_start")).toHaveLength(2); }); + + it("keeps a reloaded delayed assistant post-compaction without falling back to payload timestamps", async () => { + const assistantEndStarted = createDeferred(); + const releaseAssistantEnd = createDeferred(); + const payloadTimestamp = Date.now() - 10_000; + const harness = await createHarness({ + persistSession: true, + models: [{ id: "faux-1", contextWindow: 1_000, maxTokens: 64 }], + settings: { compaction: { enabled: true, reserveTokens: 0, keepRecentTokens: 1 } }, + extensionFactories: [ + (pi: ExtensionAPI) => { + pi.on("message_end", async (event) => { + if ( + event.message.role !== "assistant" || + !getMessageText(event.message).includes("delayed assistant payload") + ) + return; + assistantEndStarted.resolve(); + await releaseAssistantEnd.promise; + }); + pi.on("session_before_compact", () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "post-reload admission must compact or block", + })); + }, + ], + }); + harnesses.push(harness); + const model = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "seed pending persistence boundary" }], + timestamp: payloadTimestamp - 1, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + harness.setResponses([ + { + ...fauxAssistantMessage("delayed assistant payload"), + timestamp: payloadTimestamp, + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 1_200, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 1_200, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + }, + fauxAssistantMessage("reloaded session must not reach the provider"), + ]); + const delayedPrompt = harness.session.prompt("produce a delayed assistant"); + void delayedPrompt.catch(() => undefined); + await assistantEndStarted.promise; + const firstEntry = harness.sessionManager.getEntries()[0]; + if (!firstEntry) throw new Error("Expected a persisted entry before compaction"); + + const applied = await harness.session.applyCompaction( + { + summary: "summary that fits the original context window", + firstKeptEntryId: firstEntry.id, + tokensBefore: 42, + }, + { reason: "extension" }, + ); + expect(applied).toEqual({ applied: true, reason: "ok" }); + + releaseAssistantEnd.resolve(); + await delayedPrompt; + + // Close the live session, then reopen the persisted file through a fresh + // SessionManager/AgentSession stack, exactly like a process restart. + harness.session.dispose(); + const reloaded = await reloadHarnessFromSessionFile(harness, { + settings: { compaction: { enabled: true, reserveTokens: 0, keepRecentTokens: 1 } }, + extensionFactories: [ + (pi: ExtensionAPI) => { + pi.on("session_before_compact", () => ({ + cancel: true, + rejectionCause: "cancelled-by-extension", + reason: "post-reload admission must compact or block", + })); + }, + ], + }); + + try { + const branch = reloaded.sessionManager.getBranch(); + const compactionIndex = branch.findIndex((entry) => entry.type === "compaction"); + const delayedAssistantIndex = branch.findIndex( + (entry) => entry.type === "message" && getMessageText(entry.message).includes("delayed assistant payload"), + ); + expect(compactionIndex).toBeGreaterThanOrEqual(0); + expect(delayedAssistantIndex).toBeGreaterThan(compactionIndex); + const compactionEntry = branch[compactionIndex]!; + const delayedAssistantEntry = branch[delayedAssistantIndex]!; + if (delayedAssistantEntry.type !== "message" || delayedAssistantEntry.message.role !== "assistant") { + throw new Error("Expected the delayed assistant entry after compaction"); + } + // The persisted branch orders the assistant after the compaction entry even + // though its provider-supplied payload timestamp is older. + expect(delayedAssistantEntry.message.timestamp).toBe(payloadTimestamp); + expect(new Date(compactionEntry.timestamp).getTime()).toBeGreaterThan(payloadTimestamp); + + // The reloaded context is still above threshold (usage 1200 > window 1000), + // so the next admission must compact (cancelled here) or block entirely. It + // must not classify the assistant as pre-compaction via the timestamp + // fallback and wave the prompt through to the provider. + const providerCallsBefore = harness.faux.state.callCount; + await expect(reloaded.session.prompt("subsequent admission after reload")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + expect(harness.faux.state.callCount).toBe(providerCallsBefore); + expect(reloaded.events.filter((event) => event.type === "compaction_start")).toHaveLength(1); + } finally { + reloaded.session.dispose(); + } + }); }); From 6b0d03d214dc049f1fca0d075b64cb3d7f94ad01 Mon Sep 17 00:00:00 2001 From: minpeter Date: Fri, 24 Jul 2026 19:49:07 +0900 Subject: [PATCH 09/10] fix(coding-agent): preserve fallback overflow ownership --- .../coding-agent/src/core/agent-session.ts | 55 ++++--- packages/coding-agent/src/core/changes.md | 6 +- .../test/suite/retry-fallback-engine.test.ts | 145 ++++++++++++++++++ 3 files changed, 184 insertions(+), 22 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 8d91d7b92..0e8287fb2 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -477,6 +477,8 @@ const THINKING_LEVELS_WITH_MAX: ThinkingLevel[] = ["off", "minimal", "low", "med // AgentSession Class // ============================================================================ +type PostRetryCompactionState = { kind: "none" } | { kind: "skip-next-threshold"; owner: "retry-fallback-window" }; + export class AgentSession { readonly agent: Agent; readonly sessionManager: SessionManager; @@ -516,7 +518,7 @@ export class AgentSession { private _requiredCompactionAdmissionError: RequiredCompactionError | undefined; // A retry continuation immediately follows an accepted compaction. Its first // response must not retrigger threshold compaction from stale provider usage. - private _skipNextPostRetryCompactionCheck = false; + private _postRetryCompactionState: PostRetryCompactionState = { kind: "none" }; private _blockedPostCompactionAssistant: { assistant: AssistantMessage; revision: number } | undefined; private _skipNextPostCompactionAssistantCheck = false; private readonly _assistantsPendingAtCompaction = new WeakSet(); @@ -1086,8 +1088,16 @@ export class AgentSession { * will make after agent_end. Agent core drains queues before that async work * runs, so only this preflight can transfer required admissions safely. */ - private _getRequiredAutoCompactionReason(message: AssistantMessage): "overflow" | "threshold" | undefined { - if (this._skipNextPostRetryCompactionCheck) return undefined; + private _consumePostRetryThresholdSkip(): boolean { + const suppressThreshold = this._postRetryCompactionState.kind === "skip-next-threshold"; + this._postRetryCompactionState = { kind: "none" }; + return suppressThreshold; + } + + private _getRequiredAutoCompactionReason( + message: AssistantMessage, + suppressThreshold = this._postRetryCompactionState.kind === "skip-next-threshold", + ): "overflow" | "threshold" | undefined { const settings = this.settingsManager.getCompactionSettings(); if (!settings.enabled || message.stopReason === "aborted") { return undefined; @@ -1108,9 +1118,12 @@ export class AgentSession { contextUsage !== undefined && contextUsage.tokens !== null && shouldCompact(contextUsage.tokens, contextUsage.contextWindow, settings); - if (isContextOverflow(message, model.contextWindow) && (sameModel || currentContextNeedsCompaction)) { + const contextOverflow = + isContextOverflow(message, model.contextWindow) && (sameModel || currentContextNeedsCompaction); + if (contextOverflow && (message.stopReason === "error" || !suppressThreshold)) { return "overflow"; } + if (suppressThreshold) return undefined; let contextTokens: number; const directContextTokens = message.usage ? calculateContextTokens(message.usage) : 0; @@ -1317,11 +1330,8 @@ export class AgentSession { if (event.type === "agent_end" && this._lastAssistantMessage) { const msg = this._lastAssistantMessage; this._lastAssistantMessage = undefined; - const skipPostRetryCompaction = this._skipNextPostRetryCompactionCheck; - this._skipNextPostRetryCompactionCheck = false; - const requiredAutoCompaction = skipPostRetryCompaction - ? undefined - : this._getRequiredAutoCompactionReason(msg); + const suppressPostRetryThreshold = this._consumePostRetryThresholdSkip(); + const requiredAutoCompaction = this._getRequiredAutoCompactionReason(msg, suppressPostRetryThreshold); // Retry transient failures normally and eligible hard errors only through a fallback. const retryableError = this._isRetryableError(msg); @@ -1331,7 +1341,7 @@ export class AgentSession { let compactedBeforeRetry = false; if (retryCanAdmitProvider && requiredAutoCompaction) { this._retireFailedRetryAssistant(msg); - compactedBeforeRetry = await this._runPrePromptCompaction(msg, true, "threshold", true); + compactedBeforeRetry = await this._runPrePromptCompaction(msg, true, requiredAutoCompaction, true); retryContinuationBlocked = !compactedBeforeRetry; } @@ -1347,7 +1357,8 @@ export class AgentSession { this._resolveRetry(); retryContinuationBlocked ||= retryOutcome === "blocked"; - if (!retryContinuationBlocked && !skipPostRetryCompaction) { + const shouldRunPostRetryCompaction = !suppressPostRetryThreshold || requiredAutoCompaction === "overflow"; + if (!retryContinuationBlocked && shouldRunPostRetryCompaction) { if (compactedBeforeRetry && this.agent.hasQueuedMessages()) { this._scheduleContinuationAfterCurrentEvent(); launchedContinuation = true; @@ -3566,14 +3577,14 @@ export class AgentSession { if (this._isAssistantFromBeforeLatestCompaction(assistantMessage)) { return false; } + let suppressPostCompactionThreshold = false; if (this._skipNextPostCompactionAssistantCheck) { this._skipNextPostCompactionAssistantCheck = false; // The first ordinary post-compaction response can still report stale - // provider usage. An active overflow recovery is different: its retry - // response must be checked so the one-retry cap can terminate it. - if (!this._assistantsPendingAtCompaction.has(assistantMessage) && !this._overflowRecoveryAttempted) { - return false; - } + // provider usage. This exemption applies only to threshold estimates; + // provider-confirmed overflow remains required. + suppressPostCompactionThreshold = + !this._assistantsPendingAtCompaction.has(assistantMessage) && !this._overflowRecoveryAttempted; } // Case 1: Overflow - LLM returned context overflow error. @@ -3584,7 +3595,11 @@ export class AgentSession { contextUsage !== undefined && contextUsage.tokens !== null && shouldCompact(contextUsage.tokens, contextUsage.contextWindow, settings); - if (isContextOverflow(assistantMessage, contextWindow) && (sameModel || currentContextNeedsCompaction)) { + const contextOverflow = + isContextOverflow(assistantMessage, contextWindow) && (sameModel || currentContextNeedsCompaction); + const providerConfirmedOverflow = assistantMessage.stopReason === "error" && contextOverflow; + if (suppressPostCompactionThreshold && !providerConfirmedOverflow) return false; + if (contextOverflow) { const willRetry = retryAfterCompaction || assistantMessage.stopReason !== "stop"; if (!willRetry) { @@ -3641,7 +3656,6 @@ export class AgentSession { } return compacted; } - // Case 2: Threshold - context is getting large // For error messages or all-zero usage messages, estimate from the last valid response. // This ensures sessions that hit persistent API errors (e.g. 529) or malformed zero-usage @@ -4651,7 +4665,10 @@ export class AgentSession { this._resolveRetry(); return "blocked"; } - this._skipNextPostRetryCompactionCheck = true; + this._postRetryCompactionState = { + kind: "skip-next-threshold", + owner: "retry-fallback-window", + }; } // Retry via continue() - use setTimeout to break out of event handler chain diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 586ee2f68..09e7e3ead 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -12,9 +12,9 @@ or summary generation (`stale-revision`), preserving intervening context without duplicate replay. - Required compaction uses one provider-admission gate for normal prompts, extension-triggered turns, and every next turn. Provider-confirmed overflow remains fail-closed even when the local token estimate is below the configured - threshold; `agent_end` synchronously transfers both silent-overflow and threshold-compaction continuation ownership - to `AgentSession` before agent-core can drain native queues, and failed recovery restores the overflow context so - later prompts cannot bypass the same requirement. + threshold or stale threshold usage is suppressed after compaction/retry; `agent_end` synchronously transfers both + silent-overflow and threshold-compaction continuation ownership to `AgentSession` before agent-core can drain native + queues, and failed recovery restores the overflow context so later prompts cannot bypass the same requirement. - Next-turn snapshots reapply the live active tools and effective per-run system prompt after asynchronous preparation, so a tool removed during the turn is neither advertised nor executable by the following provider request. - Required ownership now suppresses only agent-core's post-`agent_end` queue drain, not the run abort signal. Deferred diff --git a/packages/coding-agent/test/suite/retry-fallback-engine.test.ts b/packages/coding-agent/test/suite/retry-fallback-engine.test.ts index d71574b2c..c7fa17fb3 100644 --- a/packages/coding-agent/test/suite/retry-fallback-engine.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-engine.test.ts @@ -57,6 +57,15 @@ function retryTranscript(events: Harness["events"]): EventTranscriptEntry[] { }); } +function createDeferred(): { promise: Promise; resolve: () => void } { + let resolve: (() => void) | undefined; + const promise = new Promise((next) => { + resolve = next; + }); + if (!resolve) throw new Error("Deferred resolver was not initialized"); + return { promise, resolve }; +} + describe("retry fallback engine", () => { const harnesses: Harness[] = []; afterEach(() => { @@ -312,6 +321,142 @@ describe("retry fallback engine", () => { } }); + it("owns a provider-confirmed fallback retry overflow despite the post-retry compaction skip", async () => { + // Composition: a retryable primary error selects the smaller fallback window, + // the required fallback-window compaction succeeds (arming + // _skipNextPostRetryCompactionCheck), and the fallback retry itself then + // returns provider-confirmed overflow. The skip flag may still suppress a + // threshold-only stale-usage check, but it must never suppress overflow + // ownership: the overflow needs its own required compaction (rejected here, + // so recovery fails closed) while queued steer/follow-up messages are + // retained instead of draining into the provider. + let compactionRequests = 0; + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000, maxTokens: 64 }, + { id: "faux-2", contextWindow: 100, maxTokens: 64 }, + ], + settings: { + compaction: { enabled: true, reserveTokens: 0, keepRecentTokens: 0 }, + retry: { + enabled: true, + baseDelayMs: 1, + fallbackChains: { [primary]: [fallback] }, + fallbackRevertPolicy: "never", + }, + }, + extensionFactories: [ + (pi) => { + pi.on("session_before_compact", (event) => { + compactionRequests++; + if (compactionRequests > 2) { + return { + cancel: true, + rejectionCause: "cancelled-by-extension" as const, + reason: "fallback overflow recovery rejected", + }; + } + return { + compaction: { + summary: compactionRequests === 1 ? "p".repeat(480) : "fallback summary", + firstKeptEntryId: event.preparation.firstKeptEntryId, + tokensBefore: event.preparation.tokensBefore, + }, + }; + }); + }, + ], + }); + harnesses.push(harness); + const primaryModel = harness.getModel("faux-1"); + if (!primaryModel) throw new Error("Expected primary fallback model"); + const historyTimestamp = Date.now() - 1_000; + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "history before retry fallback" }], + timestamp: historyTimestamp, + }); + harness.sessionManager.appendMessage({ + ...fauxAssistantMessage("history response", { timestamp: historyTimestamp + 1 }), + api: primaryModel.api, + provider: primaryModel.provider, + model: primaryModel.id, + usage: { + input: 900, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 900, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; + + const fallbackStarted = createDeferred(); + const releaseFallback = createDeferred(); + harness.setResponses([ + async (_context, _options, _state, model) => { + expect(model.id).toBe("faux-1"); + return fauxAssistantMessage("context ".repeat(900), { + stopReason: "error", + errorMessage: "overloaded_error", + }); + }, + async (_context, _options, _state, model) => { + expect(model.id).toBe("faux-2"); + fallbackStarted.resolve(); + await releaseFallback.promise; + // Provider-confirmed overflow from the fallback model itself. + return fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "context_length_exceeded", + }); + }, + fauxAssistantMessage("must not reach provider"), + fauxAssistantMessage("must not reach provider either"), + ]); + + const prompt = harness.session.prompt("trigger fallback overflow ownership"); + await fallbackStarted.promise; + await harness.session.prompt("retain fallback steer", { streamingBehavior: "steer" }); + await harness.session.followUp("retain fallback follow-up"); + releaseFallback.resolve(); + // The follow-up fix owns whether the already-completed prompt itself + // rejects; the fail-closed admission contract below is what this RED pins. + await prompt.then( + () => undefined, + () => undefined, + ); + await harness.session.waitForSettledSessionWork(); + + expect(harness.eventsOfType("retry_fallback_applied")).toMatchObject([{ from: primary, to: fallback }]); + // The required fallback-window compaction succeeded before the retry, + // which is what arms the post-retry skip flag under test. + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ reason: "threshold", accepted: true }), + ); + // RED: the skip flag must not suppress overflow ownership of the fallback + // retry response, so a second required compaction runs and fails closed. + expect(harness.eventsOfType("compaction_end")).toContainEqual( + expect.objectContaining({ + reason: "overflow", + accepted: false, + rejectionCause: "cancelled-by-extension", + }), + ); + expect(harness.session.getSteeringMessages()).toEqual(["retain fallback steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["retain fallback follow-up"]); + expect(harness.session.agent.hasQueuedMessages()).toBe(true); + expect(harness.faux.state.callCount).toBe(2); + + await expect(harness.session.prompt("later normal admission")).rejects.toThrow( + "Context remains above the compaction threshold because compaction did not complete", + ); + expect(harness.faux.state.callCount).toBe(2); + expect(harness.session.getSteeringMessages()).toEqual(["retain fallback steer"]); + expect(harness.session.getFollowUpMessages()).toEqual(["retain fallback follow-up"]); + }); + it("submits a complete fallback request rather than reusing primary continuation state", async () => { const harness = await createHarness({ models: [{ id: "faux-1" }, { id: "faux-2" }], From 354927892f9e04267cffe03feb5813a0f428a71b Mon Sep 17 00:00:00 2001 From: minpeter Date: Fri, 24 Jul 2026 20:31:42 +0900 Subject: [PATCH 10/10] fix(coding-agent): preserve compaction hook diagnostics --- .../coding-agent/src/core/agent-session.ts | 20 +++++++++++++------ packages/coding-agent/src/core/changes.md | 5 +++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 0e8287fb2..2e0f9a469 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -3242,18 +3242,26 @@ export class AgentSession { const startPrefixIntact = agentMessagesAtStart.every( (message, index) => currentMessagesAtCheck[index] === message, ); + const appendedMessages = currentMessagesAtCheck.slice(agentMessagesAtStart.length); // Appends after the start snapshot are fresh only while they are exact - // message_end identities still awaiting persistence. Any revision change, - // other append, replacement, or reorder still makes this compaction stale. + // message_end identities awaiting persistence or hidden lifecycle-hook + // diagnostics. Any other append, replacement, or reorder remains stale. const onlyPendingPersistenceAppends = startPrefixIntact && - currentMessagesAtCheck - .slice(agentMessagesAtStart.length) - .every((message) => this._messageEndsAwaitingPersistence.has(message)); + appendedMessages.every( + (message) => + this._messageEndsAwaitingPersistence.has(message) || + (message.role === "custom" && message.customType === "senpi.hook" && message.display === false), + ); + const startedRevision = + lifecycleState.status === "running" ? lifecycleState.startedRevision : this._messageRevision; + const revisionDelta = this._messageRevision - startedRevision; + const revisionStillOwned = + revisionDelta === 0 || (onlyPendingPersistenceAppends && revisionDelta === appendedMessages.length); const sourceChanged = lifecycleState.status !== "running" || lifecycleState.operationId !== operationId || - lifecycleState.startedRevision !== this._messageRevision || + !revisionStillOwned || !onlyPendingPersistenceAppends; if (sourceChanged) { return await this._rejectCompaction(request, requestId, operationId, "stale-revision", false); diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 09e7e3ead..007a7e3c5 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -8,8 +8,9 @@ controller at operation start, rejects stale completion/feedback, and retains the terminal result until another operation begins. Feedback-only aborts publish one terminal event, and accepted completions publish their terminal event before `session_compact` handlers can begin a fresh operation. -- Durable append now rejects a generation whose message revision or agent-message snapshot changed during preparation - or summary generation (`stale-revision`), preserving intervening context without duplicate replay. +- Durable append recognizes hidden `PreCompact` hook diagnostics as operation-owned appends outside the summary while + later revision or agent-message changes still reject as `stale-revision`, preserving intervening context without + duplicate replay. - Required compaction uses one provider-admission gate for normal prompts, extension-triggered turns, and every next turn. Provider-confirmed overflow remains fail-closed even when the local token estimate is below the configured threshold or stale threshold usage is suppressed after compaction/retry; `agent_end` synchronously transfers both