From 50a30f94ec63674eebb0e738bf6bd330b19e4d51 Mon Sep 17 00:00:00 2001 From: prolls Date: Fri, 4 Sep 2026 09:42:03 +0900 Subject: [PATCH 1/6] fix(coding-agent): skip context-incompatible fallbacks Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- .../coding-agent/src/core/agent-session.ts | 288 +++++++++++------- packages/coding-agent/src/core/changes.md | 25 ++ .../src/core/extensions/changes.md | 28 ++ .../coding-agent/src/core/extensions/types.ts | 23 ++ .../src/core/retry-fallback/AGENTS.md | 10 +- .../src/core/retry-fallback/controller.ts | 210 +++++++++++-- ...try-fallback-context-compatibility.test.ts | 234 ++++++++++++++ 7 files changed, 682 insertions(+), 136 deletions(-) create mode 100644 packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 2025c08b55..9e79410b67 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -174,7 +174,7 @@ import { createProviderTimeoutRetryPlan, runBoundedRetryContinuation } from "./p import type { ResourceExtensionPaths, ResourceLoader } from "./resource-loader.ts"; import { isBillingErrorMessage } from "./retry-fallback/billing.ts"; import { formatSelector } from "./retry-fallback/chains.ts"; -import { RetryFallbackController } from "./retry-fallback/controller.ts"; +import { type CandidateUsability, RetryFallbackController } from "./retry-fallback/controller.ts"; import { SelectorCooldowns } from "./retry-fallback/cooldown.ts"; import { classifyRateLimitedWait, @@ -1222,6 +1222,11 @@ export class AgentSession { registry: this._modelRegistry, cooldowns: this._selectorCooldowns, logger: fallbackLogger, + isCandidateUsable: (model) => this._assessFallbackCandidate(model), + // Only a budget refusal is a verdict about the candidate. Anything else that + // escapes the switch (an extension defect) must not be mistaken for a spent rung. + classifySwitchFailure: (error) => + error instanceof ModelUsabilityBudgetError ? { projection: error.projection } : undefined, switchModel: async (model, thinking, reason) => { await this._switchActiveModel(model, { persistDefault: false, @@ -4567,6 +4572,23 @@ export class AgentSession { if (!projection.usable) throw new ModelUsabilityBudgetError(projection); } + /** + * Capacity verdict for one fallback chain rung, using the same projection that + * gates explicit model selection. Switching into a window that cannot hold the + * live conversation only trades one dead lane for another, so the walk skips it. + */ + private _assessFallbackCandidate(model: Model): CandidateUsability { + if (model.contextWindow <= 0) return true; + const projection = projectModelUsabilityBudget({ + model, + systemPrompt: this.agent.state.systemPrompt, + tools: this.agent.state.tools, + liveContextTokens: this._getDownswitchLiveContextTokens(model), + compaction: this.settingsManager.getCompactionSettings(), + }); + return { usable: projection.usable, projection }; + } + private _getDownswitchLiveContextTokens(model: Model): number { const currentModel = this.model; if (!currentModel) return 0; @@ -4658,68 +4680,137 @@ export class AgentSession { }, ): Promise { const previousModel = this.model; - if ( + const invalidatesCompaction = opts.invalidateCompaction && (this._modelSelectionChangesContext(previousModel, model) || previousModel?.provider !== model.provider || - previousModel?.id !== model.id) - ) { - this._invalidateCompactionForModelSelection(); - } + previousModel?.id !== model.id); const thinking = this._getThinkingForModelSwitch(model, opts.ephemeralThinkingLevel); const liveContextTokens = this._getDownswitchLiveContextTokens(model); + const ephemeralThinking = opts.ephemeralThinkingLevel !== undefined; + const previous = { + model: previousModel, + systemPrompt: this.agent.state.systemPrompt, + thinkingLevel: this.agent.state.thinkingLevel, + thinkingSelection: this.agent.state.thinkingSelection, + tier: this._currentServiceTier, + fastMode: this.isFastModeActive(), + abortServerSideFallback: this.agent.abortServerSideFallback, + }; + + // Provisional state only. `model_select` handlers must build prompts and + // toolsets against the target model and its thinking level, but the switch is + // not real until the post-handler budget assert clears it: until then nothing + // is announced, persisted, or invalidated, so a rejected target leaves no trace. this.agent.state.model = model; this.agent.abortServerSideFallback = this.settingsManager.getAbortServerSideFallback() && this._retryFallback.hasConfiguredChain(); - if (opts.appendSessionEntry) { - this.sessionManager.appendModelChange( - model.provider, - model.id, - opts.entryReason, - previousModel?.provider, - previousModel?.id, - ); - } - if (opts.persistDefault) { - this.settingsManager.setDefaultModelAndProvider(model.provider, model.id); - } - const scopedMatch = this._scopedModels.find((sm) => modelsAreEqual(sm.model, model)); - const previousTier = this._currentServiceTier; - const previousFastMode = this.isFastModeActive(); this._currentServiceTier = this._resolveServiceTier(model, scopedMatch?.serviceTier); + this._applyProvisionalThinkingLevel(thinking, ephemeralThinking); + + // Runs only once the target has cleared the post-`model_select` budget assert. + // Order matches an undeferred switch exactly: the durable record lands before any + // outward notification, so no observer can see a model that was not recorded. + const commit = (): void => { + if (invalidatesCompaction) this._invalidateCompactionForModelSelection(); + if (opts.appendSessionEntry) { + this.sessionManager.appendModelChange( + model.provider, + model.id, + opts.entryReason, + previousModel?.provider, + previousModel?.id, + ); + } + if (opts.persistDefault) { + this.settingsManager.setDefaultModelAndProvider(model.provider, model.id); + } + // Rewind the provisional level so the real setter observes the same before/after + // pair it would have seen without deferral, and therefore emits the same events, + // session entry, and fallback bookkeeping as an undeferred switch. + this.agent.state.thinkingLevel = previous.thinkingLevel; + this.agent.state.thinkingSelection = previous.thinkingSelection; + if (ephemeralThinking) this._applyEphemeralThinkingLevel(thinking.level); + else this._setThinkingLevel(thinking.level, false, thinking.selection); + this._emitHighReasoningWarningIfNeeded(); + // Post-switch: the level reported here is the one actually in force (clamped, or restored + // from this model's memory), not the level requested for the previous model. + this._emit({ + type: "model_changed", + model, + thinkingLevel: this.thinkingLevel, + source: opts.modelSelectSource, + }); + this._emitServiceTierChangeIfNeeded(previous.tier, previous.fastMode); + }; - if (opts.ephemeralThinkingLevel !== undefined) { - this._applyEphemeralThinkingLevel(thinking.level); - } else { - this._setThinkingLevel(thinking.level, false, thinking.selection); + if (!opts.emitModelSelect) { + commit(); + return undefined; } - - this._emitHighReasoningWarningIfNeeded(); - // Post-switch: the level reported here is the one actually in force (clamped, or restored - // from this model's memory), not the level requested for the previous model. - this._emit({ - type: "model_changed", - model, - thinkingLevel: this.thinkingLevel, - source: opts.modelSelectSource, - }); - this._emitServiceTierChangeIfNeeded(previousTier, previousFastMode); - - if (!opts.emitModelSelect) return undefined; - const previousSystemPrompt = this.agent.state.systemPrompt; try { const systemPromptChange = await this._emitModelSelect(model, previousModel, opts.modelSelectSource); this.assertModelUsable(model, liveContextTokens); + commit(); return systemPromptChange; } catch (error) { - if (previousModel) this.agent.state.model = previousModel; - else delete (this.agent.state as { model?: Model }).model; - this.agent.state.systemPrompt = previousSystemPrompt; + await this._rollbackProvisionalModelSwitch(model, previous); throw error; } } + /** + * Put the switch's thinking level in force without announcing it. The level has to + * be live while `model_select` runs so handlers see the level that would actually + * be used, but a rejected switch must leave no event or session entry behind. + * `_getThinkingForModelSwitch` already clamped against this model's supported + * levels, which is the same set `_setThinkingLevel` would clamp against. + */ + private _applyProvisionalThinkingLevel( + thinking: { level: ThinkingLevel; selection?: ThinkingSelection }, + ephemeral: boolean, + ): void { + this.agent.state.thinkingLevel = thinking.level; + this.agent.state.thinkingSelection = ephemeral ? undefined : thinking.selection; + } + + /** + * Undo a provisional switch whose target was rejected after `model_select` ran. + * Nothing was announced, persisted, or invalidated, so this restores session-owned + * state silently. Prompt and tool state belong to the extensions that swapped it, + * so it is restored by re-running `model_select` for the previous model rather + * than by snapshotting state this session does not own. + */ + private async _rollbackProvisionalModelSwitch( + rejectedModel: Model, + previous: { + model: Model | undefined; + systemPrompt: string; + thinkingLevel: ThinkingLevel; + thinkingSelection: ThinkingSelection | undefined; + tier: ServiceTier | undefined; + abortServerSideFallback: boolean | undefined; + }, + ): Promise { + if (previous.model) this.agent.state.model = previous.model; + else delete (this.agent.state as { model?: Model }).model; + this.agent.state.systemPrompt = previous.systemPrompt; + this.agent.state.thinkingLevel = previous.thinkingLevel; + this.agent.state.thinkingSelection = previous.thinkingSelection; + this._currentServiceTier = previous.tier; + this.agent.abortServerSideFallback = previous.abortServerSideFallback; + if (!previous.model) return; + try { + await this._emitModelSelect(previous.model, rejectedModel, "restore"); + } catch (error) { + // A failing resync must not replace the rejection the caller is about to + // report, and must not leave the prompt owned by the rejected model. + this.agent.state.systemPrompt = previous.systemPrompt; + console.error("model switch rollback could not resync model_select state", error); + } + } + private _applyEphemeralThinkingLevel(level: ThinkingLevel): void { const previousLevel = this.agent.state.thinkingLevel; this.agent.state.thinkingLevel = level; @@ -7400,6 +7491,32 @@ export class AgentSession { ); } + /** + * Report that the active model's chain has no usable rung left. The session event + * keeps its long-standing shape for the TUI and RPC hosts; the extension event + * additionally carries who ran out and why, so an extension that can route the + * work to another model has enough to decide. Both are emitted from this single + * place so the two views can never disagree across the retry branches. + */ + private async _emitRetryFallbackExhausted(lastError: string): Promise { + const chainKey = this._retryFallback.exhaustedChainKey; + if (!chainKey) return; + this._emit({ type: "retry_fallback_exhausted", chainKey, lastError }); + const exhaustion = this._retryFallback.exhaustion; + // Detail is only trustworthy when it describes the chain being reported. + const detail = exhaustion?.chainKey === chainKey ? exhaustion : undefined; + const model = this.model; + await this._extensionRunner.emit({ + type: "retry_fallback_exhausted", + sessionId: this.sessionId, + chainKey, + from: detail?.from ?? (model ? `${model.provider}/${model.id}` : ""), + lastError, + exhaustionReason: detail?.reason ?? "candidates-exhausted", + rejectedCandidates: detail?.rejectedCandidates ?? [], + }); + } + private _getProviderRetryDelayMs(errorMessage: string): number | undefined { const markerMs = parseRetryAfterMsMarker(errorMessage); if (markerMs !== undefined) return markerMs; @@ -7447,26 +7564,19 @@ export class AgentSession { * in the final error. Returns the in-turn retry delay, or undefined after * emitting the terminal auto_retry_end. */ - private _degradeRateLimitedWithoutFallback( + private async _degradeRateLimitedWithoutFallback( tier: HintTier, hintMs: number | undefined, message: AssistantMessage, errorMessage: string, - ): number | undefined { + ): Promise { const settings = this.settingsManager.getRetrySettings(); // Budget checks use the resolved profile (same value as settings.maxRetries // for providers without a declared profile). const turnMaxRetries = this._resolveRetryProfile().turn.maxRetries; const hintSettings = this.settingsManager.getHintPolicySettings(); - const finishTurn = (attempt: number, finalError: string | undefined) => { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + const finishTurn = async (attempt: number, finalError: string | undefined): Promise => { + await this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7486,7 +7596,7 @@ export class AgentSession { ); if (degraded.kind === "fail") { const waitSeconds = Math.ceil(degraded.hintMs / 1000); - finishTurn( + await finishTurn( this._retryAttempt, `Provider requested a ${waitSeconds}s wait before retrying and no usable fallback model is available. ${message.errorMessage ?? ""}`, ); @@ -7494,7 +7604,7 @@ export class AgentSession { } this._retryAttempt++; if (this._retryAttempt > turnMaxRetries) { - finishTurn(this._retryAttempt - 1, message.errorMessage); + await finishTurn(this._retryAttempt - 1, message.errorMessage); return undefined; } return degraded.delayMs; @@ -7559,14 +7669,7 @@ export class AgentSession { errorMessage, }); if (!switchedFallback) { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + await this._emitRetryFallbackExhausted(errorMessage); this._resolveRetry(); return "not-handled"; } @@ -7591,14 +7694,7 @@ export class AgentSession { } switchedFallback = await this._retryFallback.tryFallback("refusal", {}); if (!switchedFallback) { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + await this._emitRetryFallbackExhausted(errorMessage); if (this._retryAttempt > 0) { this._emit({ type: "auto_retry_end", @@ -7644,14 +7740,7 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + await this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7676,7 +7765,12 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - const degradedDelayMs = this._degradeRateLimitedWithoutFallback(tier, hintMs, message, errorMessage); + const degradedDelayMs = await this._degradeRateLimitedWithoutFallback( + tier, + hintMs, + message, + errorMessage, + ); if (degradedDelayMs === undefined) return "not-handled"; hintTierDelayMs = degradedDelayMs; } @@ -7691,14 +7785,7 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + await this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7736,14 +7823,7 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + await this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7773,7 +7853,12 @@ export class AgentSession { this._armProbeBackForDemotedSelector(selector, remainingHintMs); } } else { - const degradedDelayMs = this._degradeRateLimitedWithoutFallback(tier, hintMs, message, errorMessage); + const degradedDelayMs = await this._degradeRateLimitedWithoutFallback( + tier, + hintMs, + message, + errorMessage, + ); if (degradedDelayMs === undefined) return "not-handled"; hintTierDelayMs = degradedDelayMs; } @@ -7791,14 +7876,7 @@ export class AgentSession { // The new model receives a fresh retry budget; the failed model does not. this._retryAttempt = 1; } else { - const exhaustedChainKey = this._retryFallback.exhaustedChainKey; - if (exhaustedChainKey) { - this._emit({ - type: "retry_fallback_exhausted", - chainKey: exhaustedChainKey, - lastError: errorMessage, - }); - } + await this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index b512899e71..cc65b2d1e8 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -1,5 +1,30 @@ # changes +## 2026-09-03 - Skip context-incompatible fallback rungs and make the model switch transactional + +### What changed + +- `packages/coding-agent/src/core/retry-fallback/controller.ts`: `RetryFallbackControllerDeps` gained the injected `isCandidateUsable` capacity preflight and the `classifySwitchFailure` seam. `nextCandidate` now skips a rung whose window cannot hold the live conversation (`context-unusable`) and keeps walking; `tryFallback` walks the remaining rungs when applying a model is refused on capacity grounds after `model_select` ran, and rethrows any failure the classifier does not recognize so one broken extension cannot spend the whole chain. A turn-scoped, selector-keyed rejection ledger backs the new `exhaustion` accessor (`chainKey`, `from`, `reason`, `rejectedCandidates`); `exhaustedChainKey` is unchanged. +- `packages/coding-agent/src/core/agent-session.ts`: `_switchActiveModel` is now two-phase. Model, thinking level, service tier, and the server-side-fallback flag are applied provisionally so `model_select` handlers build against the target, but compaction invalidation, `model_changed`, `thinking_level_changed`/`thinking_level_select`, the service-tier event, the high-reasoning warning, `appendModelChange`, and the persisted default all wait until the post-`model_select` `assertModelUsable` clears. A rejected target is rolled back silently and extension-owned prompt/tool state is resynced by re-running `model_select` for the previous model. The seven duplicated exhaustion emits collapse into `_emitRetryFallbackExhausted`, which emits the unchanged session event plus the new extension event. +- `packages/coding-agent/src/core/extensions/types.ts`: new `RetryFallbackExhaustedEvent` (`sessionId`, `chainKey`, `from`, `lastError`, `exhaustionReason`, `rejectedCandidates`) in the `ExtensionEvent` union with a `pi.on("retry_fallback_exhausted", ...)` overload. It flows through the generic runner `emit` and returns no result: it is notification-only. + +### Why + +- The chain walk had no capacity dimension at all, so a 1M-window primary could fall back onto a 200K rung that cannot hold the conversation, trading one dead lane for another. +- The switch wrote its `model_change` entry before the budget assert, so a rejected target left an unpaired `reason: "fallback"` entry that session restore replays as a fallback window that was never entered, while thinking level and extension-swapped toolsets stayed on the rejected model. +- `ModelUsabilityBudgetError` escaping `tryFallback` reached `_processAgentEvent`, whose rejection is swallowed by the agent event queue, so `_resolveRetry()` never ran and `prompt()` hung on `waitForRetry()`. +- `retry_fallback_exhausted` existed only as an `AgentSessionEvent`, so an extension that could route the work to another model had no typed way to learn the parent model's chain was spent. + +### Why an extension could not handle it + +- Candidate selection, the model-switch commit boundary, and retry dispatch are private `AgentSession`/controller lifecycle state; extensions observe `model_select` only after the core has already mutated and persisted the switch, and cannot atomically defer or undo it. + +### Expected merge conflict zones + +- HIGH: `packages/coding-agent/src/core/agent-session.ts` around `_switchActiveModel` and the `_handleRetryableError` exhaustion branches. +- MEDIUM: `packages/coding-agent/src/core/retry-fallback/controller.ts` around `nextCandidate`, `tryFallback`, and the deps interface. +- LOW: `packages/coding-agent/src/core/extensions/types.ts` around the event union and the `on` overloads. + ## 2026-09-03 - Make eval-only tool routing unconditional and registry-aware ### What changed diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index a7c90e01f7..63875d773d 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -1,5 +1,33 @@ # Core Extensions Changes +## Expose context-incompatible fallback exhaustion (2026-09-03) + +### What changed + +- `packages/coding-agent/src/core/extensions/types.ts` adds the notification-only + `RetryFallbackExhaustedEvent` and the matching + `pi.on("retry_fallback_exhausted", ...)` overload. The payload names the session, + active selector, exhausted chain, terminal provider error, exhaustion reason, and + the rejected candidate budget projections. + +### Why + +- Retry fallback previously exposed exhaustion only to session listeners used by the + TUI and RPC hosts. An extension capable of moving an oversized parent turn into a + fresh child context could not distinguish a context-incompatible chain from ordinary + exhaustion or select the rejected candidate safely. + +### Why an extension could not handle it + +- The event describes a decision made inside the private retry-fallback controller. + Extensions cannot observe candidate budget rejections until the host publishes the + typed result. + +### Expected merge conflict zones + +- LOW: the event interface, `ExtensionEvent` union, and `ExtensionAPI.on` overload in + `packages/coding-agent/src/core/extensions/types.ts`. + ## Expose the extension event bus for session activity signals (2026-08-31) diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index be2a23f4f6..f51748d15c 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -58,6 +58,7 @@ import type { KeybindingsManager } from "../keybindings.ts"; import type { CustomMessage } from "../messages.ts"; import type { ModelRegistry } from "../model-registry.ts"; import type { InitialModelProvenance, ScopedModel } from "../model-resolver.ts"; +import type { FallbackExhaustionReason, FallbackRejectedCandidate } from "../retry-fallback/controller.ts"; import type { BranchSummaryEntry, CompactionEntry, @@ -1156,6 +1157,26 @@ export interface ThinkingLevelSelectEvent { previousLevel: ThinkingLevel; } +/** + * Fired when the fallback chain for the active model runs out of usable rungs, so + * the turn is about to fail on the parent model. Notification-only: an extension + * that can route the work elsewhere (a subagent on another model) uses this to + * decide, and `rejectedCandidates` tells it whether the chain was spent or merely + * too small for the current conversation. + */ +export interface RetryFallbackExhaustedEvent { + type: "retry_fallback_exhausted"; + sessionId: string; + /** Chain whose rungs are spent, in canonical `provider/model[:thinking]` form. */ + chainKey: string; + /** Selector that was active when the chain ran out, as `provider/model`. */ + from: string; + /** Terminal provider error that ended the walk. */ + lastError: string; + exhaustionReason: FallbackExhaustionReason; + rejectedCandidates: readonly FallbackRejectedCandidate[]; +} + // ============================================================================ // User Bash Events // ============================================================================ @@ -1434,6 +1455,7 @@ export type ExtensionEvent = | ModelSelectEvent | SystemPromptChangeEvent | ThinkingLevelSelectEvent + | RetryFallbackExhaustedEvent | UserBashEvent | InputEvent | InputDispositionEvent @@ -1667,6 +1689,7 @@ export interface ExtensionAPI { on(event: "model_select", handler: ExtensionHandler): void; on(event: "system_prompt_change", handler: ExtensionHandler): void; on(event: "thinking_level_select", handler: ExtensionHandler): void; + on(event: "retry_fallback_exhausted", handler: ExtensionHandler): void; on(event: "tool_call", handler: ExtensionHandler): void; on(event: "tool_result", handler: ExtensionHandler): void; on(event: "user_bash", handler: ExtensionHandler): void; diff --git a/packages/coding-agent/src/core/retry-fallback/AGENTS.md b/packages/coding-agent/src/core/retry-fallback/AGENTS.md index e422ac99c2..c8a66cc869 100644 --- a/packages/coding-agent/src/core/retry-fallback/AGENTS.md +++ b/packages/coding-agent/src/core/retry-fallback/AGENTS.md @@ -6,7 +6,7 @@ Model fallback chains and hint-aware 429 retry policy for `agent-session.ts`. Pu | File | Role | |---|---| -| `controller.ts` | `RetryFallbackController`: turn-scoped tried-selector set, `ActiveFallbackState`, `tryFallback` / `maybeRestorePrimary(revertPolicy)` / `notifyCompactionApplied` / `clearForManualModelChange`, content-keyed memo of canonicalized chains | +| `controller.ts` | `RetryFallbackController`: turn-scoped tried-selector set and rejection ledger, `ActiveFallbackState`, `tryFallback` / `maybeRestorePrimary(revertPolicy)` / `notifyCompactionApplied` / `clearForManualModelChange`, injected `isCandidateUsable` capacity preflight + `classifySwitchFailure` seam, `exhaustion` summary, content-keyed memo of canonicalized chains | | `chains.ts` | Selector parse/format, chain-key resolution, `canonicalizeFallbackChains` (bare-selector expansion + registry eligibility) | | `expansion.ts` | Bare-selector family expansion; OpenRouter denylist; OAuth-first auth tiers; `PROVIDER_PRECEDENCE` tie-break | | `hint-policy.ts` | Pure 429 hint tiers (`no-hint-fast-fallback` / `tier1-in-turn` / `tier2-fallback-probe-back` / `tier3-fallback-only`) + probe schedule math | @@ -22,6 +22,7 @@ Model fallback chains and hint-aware 429 retry policy for `agent-session.ts`. Pu | Task | File | |---|---| | Change when fallback fires or reverts | `controller.ts` | +| Change which candidates are context-compatible | `controller.ts` (`isCandidateUsable` wiring lives in `core/agent-session.ts`) | | Change selector syntax / chain canonicalization | `chains.ts` | | Change which providers a bare selector expands to | `expansion.ts` | | Tune 429 wait/probe behavior | `hint-policy.ts`, `probe-scheduler.ts` | @@ -31,6 +32,8 @@ Model fallback chains and hint-aware 429 retry policy for `agent-session.ts`. Pu ## CONVENTIONS - Everything time- or randomness-dependent is injected (`now`, `random`, `setTimeout`/`clearTimeout`) — tests drive it deterministically with fake timers; never read `Date.now()` directly here. +- Injected probes are total. `isCandidateUsable` and `classifySwitchFailure` are optional, but when supplied a failure inside them propagates: a projection that cannot be computed is a defect, not a verdict about the candidate. +- A rung is skipped only on a verdict about that rung (`context-unusable`, cooldown, auth, …). Any other failure from `switchModel` is rethrown — swallowing it would spend the whole chain on one broken extension. - Cooldowns are runtime-only and deliberately never persisted to settings or session files. - Billing-class errors pin the fallback candidate as the session model and NEVER release; refusal pins release when a senpi-owned compaction successfully applies (context changed => one fresh primary attempt); `transient`/`hard-error` fallbacks revert per `fallbackRevertPolicy` (`cooldown-expiry` | `never`). - `canonicalizeFallbackChains` is memoized on chains content — provider-error handling calls it several times per error. @@ -44,8 +47,11 @@ Model fallback chains and hint-aware 429 retry policy for `agent-session.ts`. Pu - Two in-flight probes, or arming a second plan without superseding — `probe-scheduler.ts` owns exactly one plan per session. - Writing raw provider errors/headers to `fallback.log` — always go through `log.ts` scrubbing. - Treating a billing error as transient — retrying the same account never recovers it. +- Falling back onto a model whose context window cannot hold the live conversation — that trades one dead lane for another; the capacity preflight skips it and the walk continues. +- Letting a rejected switch leave a committed `model_change` entry: `agent-session.ts` only persists and announces after the post-`model_select` budget assert clears. ## NOTES -- Tests: `test/suite/retry-fallback-*.test.ts` (20 files) — engine, chains, expansion eligibility, cooldown, hint tiers, probe scheduler, billing swap, revert, validate, log. +- Tests: `test/suite/retry-fallback-*.test.ts` (21 files) — engine, chains, expansion eligibility, cooldown, hint tiers, probe scheduler, billing swap, revert, validate, log, context compatibility. +- Exhaustion is reported once by `agent-session.ts` `_emitRetryFallbackExhausted`: the unchanged `retry_fallback_exhausted` session event for TUI/RPC hosts, plus the typed extension event carrying `sessionId`, `from`, `exhaustionReason`, and `rejectedCandidates` so an extension can delegate. - `streamRetryTimeoutMs` reconciles to `max(cap, streamStartTimeoutMs)` so a granted stream-start budget is never cut short; `0` disables. diff --git a/packages/coding-agent/src/core/retry-fallback/controller.ts b/packages/coding-agent/src/core/retry-fallback/controller.ts index 19bbd00e77..0ac337a2f5 100644 --- a/packages/coding-agent/src/core/retry-fallback/controller.ts +++ b/packages/coding-agent/src/core/retry-fallback/controller.ts @@ -1,5 +1,6 @@ import type { ThinkingLevel } from "@earendil-works/pi-agent-core"; import { type Api, clampThinkingLevel, type Model } from "@earendil-works/pi-ai"; +import type { ModelUsabilityBudgetProjection } from "../extensions/builtin/compaction/model-usability-budget.ts"; import { baseSelector, candidatesAfter, @@ -28,6 +29,47 @@ export interface ActiveFallbackState { type FallbackReason = "transient" | "refusal" | "hard-error" | "billing"; +/** + * Why a chain rung never became the active model. Every reason here is a verdict + * about the rung itself, so the walk may move on; a failure that is not one of + * these is a defect and propagates instead of silently burning candidates. + */ +export type FallbackRejectionReason = + | "unknown" + | "self" + | "tried" + | "suppressed" + | "unauthenticated" + | "context-unusable"; + +export interface FallbackRejectedCandidate { + readonly selector: string; + readonly reason: FallbackRejectionReason; + /** Budget arithmetic behind a `context-unusable` verdict, when it was available. */ + readonly projection?: ModelUsabilityBudgetProjection; + /** Failure text when the verdict came from a rejected switch rather than the preflight. */ + readonly error?: string; +} + +/** + * Verdict from the injected capacity probe. The bare boolean keeps simple + * callers (and array-backed test doubles) ergonomic; the object form carries the + * projection so an exhaustion consumer can explain the refusal in tokens. + */ +export type CandidateUsability = + | boolean + | { readonly usable: boolean; readonly projection?: ModelUsabilityBudgetProjection }; + +export type FallbackExhaustionReason = "candidates-exhausted" | "no-context-compatible-candidate"; + +export interface FallbackExhaustion { + readonly chainKey: string; + /** Selector that was active when the walk ran out of rungs. */ + readonly from: string; + readonly reason: FallbackExhaustionReason; + readonly rejectedCandidates: readonly FallbackRejectedCandidate[]; +} + interface FallbackSettings { modelFallback: boolean; chains: Readonly>; @@ -45,6 +87,22 @@ export interface RetryFallbackControllerDeps { }; cooldowns: SelectorCooldowns; logger: FallbackLogger; + /** + * Capacity preflight for one candidate against the live conversation. Only a + * definitive `false` removes the rung; omitting the probe entirely leaves the + * chain exactly as wide as it is without one. The probe must be total - a failure + * inside it propagates rather than being read as "candidate is fine". + */ + isCandidateUsable?(model: Model): CandidateUsability; + /** + * Classifies a failure thrown by {@link switchModel}. A returned projection means + * the target was refused on capacity grounds after `model_select` ran, which the + * preflight cannot see; the walk then treats the rung as spent. Returning + * `undefined` means the failure is not a verdict about this candidate (an + * extension defect, an aborted switch), and the controller rethrows it rather + * than quietly consuming the rest of the chain. + */ + classifySwitchFailure?(error: unknown): { projection?: ModelUsabilityBudgetProjection } | undefined; switchModel(model: Model, thinking: ThinkingLevel, reason: "fallback" | "fallback-revert"): Promise; emit( event: @@ -64,8 +122,13 @@ export interface RetryFallbackControllerDeps { export class RetryFallbackController { private readonly deps: RetryFallbackControllerDeps; private readonly triedSelectors = new Set(); + // Turn-scoped rejection ledger, keyed by selector so a rung re-walked on a later + // error is recorded once. First write wins: the reason that first removed a rung + // explains it better than the "tried" skip a subsequent walk would overwrite it with. + private readonly rejectedCandidates = new Map(); private state: ActiveFallbackState | undefined; private lastExhaustedChainKey: string | undefined; + private lastExhaustion: FallbackExhaustion | undefined; // Content-keyed memo of canonicalizeFallbackChains. Provider-error handling calls // canTryFallback/nextCandidate several times per error; without this each call // re-expands bare selectors and re-probes registry eligibility over the full @@ -87,9 +150,20 @@ export class RetryFallbackController { return this.lastExhaustedChainKey; } + /** + * Structured detail for the chain reported by {@link exhaustedChainKey}. Kept + * separate from that getter so the existing string-only consumers keep working + * and a caller that stubs one never silently reads stale detail from the other. + */ + get exhaustion(): FallbackExhaustion | undefined { + return this.lastExhaustion; + } + resetTurn(): void { this.triedSelectors.clear(); + this.rejectedCandidates.clear(); this.lastExhaustedChainKey = undefined; + this.lastExhaustion = undefined; } clear(): void { @@ -200,33 +274,60 @@ export class RetryFallbackController { failure: { errorMessage?: string; retryAfterMs?: number }, ): Promise { const current = this.deps.getCurrentSelector(); - const candidate = this.nextCandidate(); - if (!current || !candidate) return false; + if (!current) return false; + let candidate = this.nextCandidate(); + if (!candidate) return false; const currentBase = formatSelector(current.model); if (reason === "transient" || reason === "hard-error" || reason === "billing") { this.deps.cooldowns.note(currentBase, failure); this.deps.logger.info("cooldown_noted", { selector: currentBase, errorMessage: failure.errorMessage }); } - const thinking = this.selectThinking(candidate.selector, candidate.model, current.thinkingLevel); - await this.deps.switchModel(candidate.model, thinking, "fallback"); - const from = formatSelector(current.model); - const to = formatSelector(candidate.model); - const prior = this.state; - const pinnedByRefusal = prior?.pinnedByRefusal === true || reason === "refusal"; - const pinnedByBilling = prior?.pinnedByBilling === true || reason === "billing"; - this.state = { - chainKey: candidate.chainKey, - originalSelector: prior?.originalSelector ?? from, - originalThinkingLevel: prior?.originalThinkingLevel ?? current.thinkingLevel, - lastAppliedThinkingLevel: thinking, - pinnedByRefusal, - pinnedByBilling, - pinned: pinnedByRefusal || pinnedByBilling, - }; - this.deps.logger.info("fallback_applied", { from, to, chainKey: candidate.chainKey, reason }); - this.deps.emit({ type: "retry_fallback_applied", from, to, chainKey: candidate.chainKey, reason }); - return true; + // Applying a model can fail after the point where a capacity preflight can see + // it: a `model_select` handler may grow the system prompt or toolset past the + // target's budget. The switch owner restores itself before rethrowing, so the + // walk simply moves to the next rung. Termination is guaranteed - every visited + // rung is added to `triedSelectors`, which `nextCandidate` skips. + while (candidate) { + const thinking = this.selectThinking(candidate.selector, candidate.model, current.thinkingLevel); + try { + await this.deps.switchModel(candidate.model, thinking, "fallback"); + } catch (error) { + const rejection = this.deps.classifySwitchFailure?.(error); + // Not a capacity verdict about this rung: surface it. Swallowing arbitrary + // failures here would spend the whole chain on one broken extension. + if (!rejection) throw error; + const selector = baseSelector(candidate.selector); + const errorMessage = error instanceof Error ? error.message : String(error); + this.recordRejection({ + selector, + reason: "context-unusable", + ...(rejection.projection === undefined ? {} : { projection: rejection.projection }), + error: errorMessage, + }); + this.deps.logger.warn("fallback_switch_rejected", { candidate: selector, errorMessage }); + candidate = this.nextCandidate(); + continue; + } + const from = formatSelector(current.model); + const to = formatSelector(candidate.model); + const prior = this.state; + const pinnedByRefusal = prior?.pinnedByRefusal === true || reason === "refusal"; + const pinnedByBilling = prior?.pinnedByBilling === true || reason === "billing"; + this.state = { + chainKey: candidate.chainKey, + originalSelector: prior?.originalSelector ?? from, + originalThinkingLevel: prior?.originalThinkingLevel ?? current.thinkingLevel, + lastAppliedThinkingLevel: thinking, + pinnedByRefusal, + pinnedByBilling, + pinned: pinnedByRefusal || pinnedByBilling, + }; + this.deps.logger.info("fallback_applied", { from, to, chainKey: candidate.chainKey, reason }); + this.deps.emit({ type: "retry_fallback_applied", from, to, chainKey: candidate.chainKey, reason }); + return true; + } + return false; } private nextCandidate( @@ -253,39 +354,73 @@ export class RetryFallbackController { for (const raw of candidatesAfter(entries, formatSelector(current.model, current.thinkingLevel))) { const selector = parseFallbackSelector(raw, this.deps.registry); if (!selector) { - this.skip(raw, "unknown"); + this.skip(raw, raw, "unknown", reserve); continue; } if (selector.provider === current.model.provider && selector.id === current.model.id) { - this.skip(raw, "self"); + this.skip(raw, baseSelector(selector), "self", reserve); continue; } const base = baseSelector(selector); if (this.triedSelectors.has(base)) { - this.skip(raw, "tried"); + this.skip(raw, base, "tried", reserve); continue; } if (this.deps.cooldowns.isSuppressed(base)) { - this.skip(raw, "suppressed"); + this.skip(raw, base, "suppressed", reserve); continue; } if (!this.deps.isAuthAvailable(selector.provider)) { - this.skip(raw, "unauthenticated"); + this.skip(raw, base, "unauthenticated", reserve); continue; } const model = this.deps.registry.find(selector.provider, selector.id); if (!model) { - this.skip(raw, "unknown"); + this.skip(raw, base, "unknown", reserve); + continue; + } + // Capacity preflight: a rung whose window cannot hold the live conversation + // would only trade one dead lane for another, so keep walking the chain. + const usability = this.assessUsability(model); + if (usability && !usability.usable) { + this.skip(raw, base, "context-unusable", reserve, { projection: usability.projection }); continue; } if (reserve) this.triedSelectors.add(base); return { chainKey, selector, model }; } this.lastExhaustedChainKey = chainKey; - if (reserve) this.deps.logger.info("candidates_exhausted", { chainKey }); + const rejectedCandidates = [...this.rejectedCandidates.values()]; + this.lastExhaustion = { + chainKey, + from: formatSelector(current.model), + reason: rejectedCandidates.some((rejected) => rejected.reason === "context-unusable") + ? "no-context-compatible-candidate" + : "candidates-exhausted", + rejectedCandidates, + }; + if (reserve) this.deps.logger.info("candidates_exhausted", { chainKey, reason: this.lastExhaustion.reason }); return undefined; } + /** + * Normalizes the injected probe's verdict. The probe is total: only its absence is + * handled here, and a failure inside it propagates, because a projection that + * cannot be computed is a defect rather than a verdict about this candidate. + */ + private assessUsability( + model: Model, + ): { usable: boolean; projection?: ModelUsabilityBudgetProjection } | undefined { + if (!this.deps.isCandidateUsable) return undefined; + const verdict = this.deps.isCandidateUsable(model); + return typeof verdict === "boolean" ? { usable: verdict } : verdict; + } + + private recordRejection(rejected: FallbackRejectedCandidate): void { + if (this.rejectedCandidates.has(rejected.selector)) return; + this.rejectedCandidates.set(rejected.selector, rejected); + } + private selectThinking( selector: FallbackSelector, model: Model, @@ -298,7 +433,24 @@ export class RetryFallbackController { return clampThinkingLevel(model, requested); } - private skip(candidate: string, skipReason: string): void { + /** + * Single funnel for every reason a rung is passed over. Only reserving walks feed + * the ledger: `canTryFallback` re-walks the same chain several times per provider + * error, and those probes must not multiply the reported rejections. + */ + private skip( + candidate: string, + selector: string, + skipReason: FallbackRejectionReason, + reserve: boolean, + detail?: { projection?: ModelUsabilityBudgetProjection }, + ): void { this.deps.logger.debug("candidate_skipped", { candidate, skipReason }); + if (!reserve) return; + this.recordRejection( + detail?.projection === undefined + ? { selector, reason: skipReason } + : { selector, reason: skipReason, projection: detail.projection }, + ); } } diff --git a/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts b/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts new file mode 100644 index 0000000000..0cf813d28d --- /dev/null +++ b/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts @@ -0,0 +1,234 @@ +import type { AgentTool, ThinkingLevel } from "@earendil-works/pi-agent-core"; +import type { Api, Model } from "@earendil-works/pi-ai"; +import { fauxAssistantMessage, getModel } from "@earendil-works/pi-ai/compat"; +import { Type } from "typebox"; +import { afterEach, describe, expect, it } from "vitest"; +import { RetryFallbackController } from "../../src/core/retry-fallback/controller.ts"; +import { SelectorCooldowns } from "../../src/core/retry-fallback/cooldown.ts"; +import { createHarness, type Harness } from "./harness.ts"; + +type SwitchRecord = { + readonly model: string; + readonly thinking: ThinkingLevel; +}; + +function seedLiveContext(harness: Harness, tokens: number): void { + const timestamp = Date.now(); + const primary = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "continue the interrupted task" }], + timestamp: timestamp - 1, + }); + harness.sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "progress before provider failure" }], + api: primary.api, + provider: primary.provider, + model: primary.id, + stopReason: "stop", + usage: { + input: tokens - 1_000, + output: 1_000, + cacheRead: 0, + cacheWrite: 0, + totalTokens: tokens, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; +} + +function model(id: string): Model { + return { + ...getModel("openai", "gpt-5.4"), + provider: "faux", + id, + }; +} + +describe("retry fallback context compatibility", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("skips a context-incompatible rung and switches to the next compatible model", async () => { + // given + const primary = model("primary"); + const incompatible = model("incompatible"); + const compatible = model("compatible"); + const models = [primary, incompatible, compatible]; + const switches: SwitchRecord[] = []; + let current = { model: primary, thinkingLevel: "high" as ThinkingLevel }; + const deps = { + getSettings: () => ({ + modelFallback: true, + chains: { "faux/primary": ["faux/incompatible", "faux/compatible"] }, + }), + registry: { + find: (provider: string, id: string) => + models.find((candidate) => candidate.provider === provider && candidate.id === id), + getAll: () => models, + }, + cooldowns: new SelectorCooldowns(() => 0), + logger: { debug: () => {}, info: () => {}, warn: () => {} }, + isCandidateUsable: (candidate: Model) => candidate.id !== "incompatible", + switchModel: async (candidate: Model, thinking: ThinkingLevel) => { + switches.push({ model: candidate.id, thinking }); + current = { model: candidate, thinkingLevel: thinking }; + }, + emit: () => {}, + getCurrentSelector: () => current, + isAuthAvailable: () => true, + }; + const controller = new RetryFallbackController(deps); + + // when + const switched = await controller.tryFallback("hard-error", { errorMessage: "upstream unavailable" }); + + // then + expect(switched).toBe(true); + expect(switches).toEqual([{ model: "compatible", thinking: "high" }]); + }); + + it("rolls back a post-model-select budget rejection without persisting a fallback switch", async () => { + // given + const primaryTool: AgentTool = { + name: "primary_tool", + label: "Primary", + description: "Primary model tool.", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "primary" }], details: {} }), + }; + const fallbackTool: AgentTool = { + name: "fallback_tool", + label: "Fallback", + description: "Fallback model tool.", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "fallback" }], details: {} }), + }; + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 80_000, maxTokens: 4_000 }, + ], + systemPrompt: "primary prompt", + tools: [primaryTool, fallbackTool], + initialActiveToolNames: ["primary_tool"], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + extensionFactories: [ + (pi) => { + pi.on("model_select", (event) => { + if (event.model.id === "faux-2") { + pi.setActiveTools(["fallback_tool"]); + return { systemPrompt: "oversized ".repeat(40_000), systemPromptName: "oversized" }; + } + pi.setActiveTools(["primary_tool"]); + return { systemPrompt: "primary prompt", systemPromptName: "primary" }; + }); + }, + ], + }); + harnesses.push(harness); + const internals = harness.session as unknown as { + _handleRetryableError: ( + message: ReturnType, + options: { hardErrorFallback: boolean }, + ) => Promise; + }; + + // when + const result = await internals + ._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + { hardErrorFallback: true }, + ) + .then( + (value) => ({ kind: "returned" as const, value }), + (error: unknown) => ({ kind: "threw" as const, error }), + ); + + // then + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "model_change")).toEqual([]); + expect(harness.session.model?.id).toBe("faux-1"); + expect(harness.session.systemPrompt).toBe("primary prompt"); + expect(harness.session.getActiveToolNames()).toEqual(["primary_tool"]); + expect(result).toEqual({ kind: "returned", value: "not-handled" }); + }); + + it("emits one extension-visible exhaustion event when every fallback is context-incompatible", async () => { + // given + const extensionEvents: unknown[] = []; + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 80_000, maxTokens: 4_000 }, + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + extensionFactories: [ + (pi) => { + const register = Reflect.get(pi, "on"); + if (typeof register !== "function") throw new Error("missing extension event registration"); + Reflect.apply(register, pi, [ + "retry_fallback_exhausted", + (event: unknown) => extensionEvents.push(event), + ]); + }, + ], + }); + harnesses.push(harness); + seedLiveContext(harness, 90_000); + const internals = harness.session as unknown as { + _handleRetryableError: ( + message: ReturnType, + options: { hardErrorFallback: boolean }, + ) => Promise; + }; + + // when + await internals + ._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + { hardErrorFallback: true }, + ) + .catch(() => undefined); + + // then + expect(extensionEvents).toMatchObject([ + { + type: "retry_fallback_exhausted", + sessionId: harness.session.sessionId, + chainKey: "faux/faux-1", + from: "faux/faux-1", + exhaustionReason: "no-context-compatible-candidate", + rejectedCandidates: [ + { + selector: "faux/faux-2", + reason: "context-unusable", + projection: { usable: false }, + }, + ], + }, + ]); + expect(harness.eventsOfType("retry_fallback_exhausted")).toMatchObject([ + { chainKey: "faux/faux-1", lastError: "upstream unavailable" }, + ]); + }); +}); From 5e6e1d69d4146dbde197486806b19148ef19ced8 Mon Sep 17 00:00:00 2001 From: prolls Date: Fri, 4 Sep 2026 10:10:32 +0900 Subject: [PATCH 2/6] fix(coding-agent): harden fallback exhaustion recovery Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 7 + packages/coding-agent/docs/extensions.md | 31 +++ .../coding-agent/src/core/agent-session.ts | 213 ++++++++++++++--- packages/coding-agent/src/core/changes.md | 1 + .../src/core/retry-fallback/controller.ts | 25 +- ...try-fallback-context-compatibility.test.ts | 8 +- ...etry-fallback-exhaustion-isolation.test.ts | 222 ++++++++++++++++++ ...etry-fallback-exhaustion-lifecycle.test.ts | 88 +++++++ 8 files changed, 540 insertions(+), 55 deletions(-) create mode 100644 packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts create mode 100644 packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fad325cff2..7aa27e3038 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,10 +6,17 @@ ### Added +- Extensions can subscribe to `retry_fallback_exhausted` to receive bounded, + structured diagnostics when no configured fallback can hold the live + conversation, enabling fresh-context delegation without parsing TUI errors. + ### Changed ### Fixed +- Retry fallback now skips context-incompatible model rungs, continues to later + candidates, settles when none fit, and never persists a rejected automatic + fallback switch or leaks its prompt/tool state into the parent session. - TTSR now interrupts a single streamed assistant message that repeats the same paragraph three times (a within-message narration loop such as re-announcing the same "now writing the DAG cell" step for minutes without ever issuing the tool call): the collapse guard gains a paragraph-repeat mechanism, truncates the message from the first repeat, and injects the usual recovery nudge. Scalar runs, short periods, and line cycles were the only mechanisms before, and blank lines reset line-cycle tracking, so paragraph-level loops streamed unchecked until the user aborted. Tool-argument streams are not affected. ### Removed diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 2619b937e3..21a7caba84 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -825,6 +825,37 @@ pi.on("thinking_level_select", async (event, ctx) => { Use this to update extension UI when `pi.setThinkingLevel()`, model changes, or built-in thinking-level controls change the active thinking level. +#### retry_fallback_exhausted + +Fired when retry fallback has a configured chain but no remaining rung can +continue the parent turn. `exhaustionReason` distinguishes a spent chain from +one whose candidates cannot hold the live conversation. + +```typescript +pi.on("retry_fallback_exhausted", (event, ctx) => { + if (event.exhaustionReason !== "no-context-compatible-candidate") return; + + const candidate = event.rejectedCandidates.find( + (rejected) => rejected.reason === "context-unusable", + ); + ctx.ui.notify( + `Fresh-context recovery is available through ${candidate?.selector ?? event.chainKey}`, + "warning", + ); +}); +``` + +The payload includes `sessionId`, `chainKey`, `from`, `lastError`, +`exhaustionReason`, and `rejectedCandidates`. Diagnostics delivered to +extensions are bounded to 8,192 error characters and 16 rejected candidates. +The original session event remains available to TUI/RPC listeners with its +existing `{ chainKey, lastError }` shape. + +This hook is notification-only. Senpi starts handlers without waiting for them, +so a slow or non-settling recovery extension cannot block retry cleanup. An +extension that delegates should enforce its own exactly-once ownership and +return quickly after starting background work. + ### Tool Events #### tool_call diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 9e79410b67..75c2e07c83 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -220,6 +220,8 @@ function evalHelperCall(name: string): string { return `tool.${name}({ ... })`; } const TURN_RETRY_SUPPRESSION_PREFIX = "senpi:no-turn-retry:"; +const MAX_FALLBACK_EXHAUSTION_ERROR_CHARS = 8_192; +const MAX_FALLBACK_EXHAUSTION_CANDIDATES = 16; const DEFERRED_RETRY_QUEUE_OWNERS = new WeakSet(); // ============================================================================ @@ -1069,6 +1071,7 @@ export class AgentSession { private readonly _assistantsPendingAtCompaction = new WeakSet(); private readonly _postCompactionUsageExemptAssistants = new WeakSet(); private _messageRevision = 0; + private _provisionalModelSelectDepth = 0; // Branch summarization state private _branchSummaryAbortController: AbortController | undefined = undefined; @@ -3219,7 +3222,7 @@ export class AgentSession { // binding itself, not to a mid-session change. The session was already // committed to the client, so cancelling or invalidating work it started // against that session would be spurious. - if (this._extensionBindingPromptReadiness === undefined) { + if (this._extensionBindingPromptReadiness === undefined && this._provisionalModelSelectDepth === 0) { this.abortCompaction(); this._incrementMessageRevision(); } @@ -4513,6 +4516,7 @@ export class AgentSession { nextModel: Model, previousModel: Model | undefined, source: ModelSelectSource, + options: { deferSystemPromptAnnouncement?: boolean } = {}, ): Promise { this.syncPromptCacheSafeWaitEnv(); if (!this._modelSelectionChangesContext(previousModel, nextModel)) return undefined; @@ -4546,9 +4550,16 @@ export class AgentSession { if (result.systemPromptName) { event.systemPromptName = result.systemPromptName; } + if (options.deferSystemPromptAnnouncement) { + return event; + } + await this._announceSystemPromptChange(event); + return event; + } + + private async _announceSystemPromptChange(event: SystemPromptChangeEvent): Promise { await this._extensionRunner.emit(event); this._emit(event); - return event; } /** @@ -4679,6 +4690,9 @@ export class AgentSession { ephemeralThinkingLevel?: ThinkingLevel; }, ): Promise { + if (opts.entryReason !== "fallback") { + return this._switchActiveModelCommittedFirst(model, opts); + } const previousModel = this.model; const invalidatesCompaction = opts.invalidateCompaction && @@ -4691,6 +4705,10 @@ export class AgentSession { const previous = { model: previousModel, systemPrompt: this.agent.state.systemPrompt, + baseSystemPrompt: this._baseSystemPrompt, + tools: [...this.agent.state.tools], + requestedActiveToolNames: this._requestedActiveToolNames ? [...this._requestedActiveToolNames] : undefined, + withheldEvalOnlyToolNames: [...this._withheldEvalOnlyToolNames], thinkingLevel: this.agent.state.thinkingLevel, thinkingSelection: this.agent.state.thinkingSelection, tier: this._currentServiceTier, @@ -4750,9 +4768,20 @@ export class AgentSession { return undefined; } try { - const systemPromptChange = await this._emitModelSelect(model, previousModel, opts.modelSelectSource); + this._provisionalModelSelectDepth++; + let systemPromptChange: SystemPromptChangeEvent | undefined; + try { + systemPromptChange = await this._emitModelSelect(model, previousModel, opts.modelSelectSource, { + deferSystemPromptAnnouncement: true, + }); + } finally { + this._provisionalModelSelectDepth--; + } this.assertModelUsable(model, liveContextTokens); commit(); + if (systemPromptChange) { + await this._announceSystemPromptChange(systemPromptChange); + } return systemPromptChange; } catch (error) { await this._rollbackProvisionalModelSwitch(model, previous); @@ -4760,6 +4789,84 @@ export class AgentSession { } } + /** + * Preserve the established event order for manual selection, cycling, restore, + * and fallback revert. Only automatic fallback admission needs the provisional + * transaction because it may walk to another rung after a context rejection. + */ + private async _switchActiveModelCommittedFirst( + model: Model, + opts: { + persistDefault: boolean; + appendSessionEntry: boolean; + entryReason?: "fallback" | "fallback-revert"; + emitModelSelect: boolean; + modelSelectSource: ModelSelectSource; + invalidateCompaction: boolean; + ephemeralThinkingLevel?: ThinkingLevel; + }, + ): Promise { + const previousModel = this.model; + if ( + opts.invalidateCompaction && + (this._modelSelectionChangesContext(previousModel, model) || + previousModel?.provider !== model.provider || + previousModel?.id !== model.id) + ) { + this._invalidateCompactionForModelSelection(); + } + const thinking = this._getThinkingForModelSwitch(model, opts.ephemeralThinkingLevel); + const liveContextTokens = this._getDownswitchLiveContextTokens(model); + this.agent.state.model = model; + this.agent.abortServerSideFallback = + this.settingsManager.getAbortServerSideFallback() && this._retryFallback.hasConfiguredChain(); + if (opts.appendSessionEntry) { + this.sessionManager.appendModelChange( + model.provider, + model.id, + opts.entryReason, + previousModel?.provider, + previousModel?.id, + ); + } + if (opts.persistDefault) { + this.settingsManager.setDefaultModelAndProvider(model.provider, model.id); + } + + const scopedMatch = this._scopedModels.find((sm) => modelsAreEqual(sm.model, model)); + const previousTier = this._currentServiceTier; + const previousFastMode = this.isFastModeActive(); + this._currentServiceTier = this._resolveServiceTier(model, scopedMatch?.serviceTier); + + if (opts.ephemeralThinkingLevel !== undefined) { + this._applyEphemeralThinkingLevel(thinking.level); + } else { + this._setThinkingLevel(thinking.level, false, thinking.selection); + } + + this._emitHighReasoningWarningIfNeeded(); + this._emit({ + type: "model_changed", + model, + thinkingLevel: this.thinkingLevel, + source: opts.modelSelectSource, + }); + this._emitServiceTierChangeIfNeeded(previousTier, previousFastMode); + + if (!opts.emitModelSelect) return undefined; + const previousSystemPrompt = this.agent.state.systemPrompt; + try { + const systemPromptChange = await this._emitModelSelect(model, previousModel, opts.modelSelectSource); + this.assertModelUsable(model, liveContextTokens); + return systemPromptChange; + } catch (error) { + if (previousModel) this.agent.state.model = previousModel; + else delete (this.agent.state as { model?: Model }).model; + this.agent.state.systemPrompt = previousSystemPrompt; + throw error; + } + } + /** * Put the switch's thinking level in force without announcing it. The level has to * be live while `model_select` runs so handlers see the level that would actually @@ -4787,27 +4894,53 @@ export class AgentSession { previous: { model: Model | undefined; systemPrompt: string; + baseSystemPrompt: string; + tools: AgentTool[]; + requestedActiveToolNames: string[] | undefined; + withheldEvalOnlyToolNames: string[]; thinkingLevel: ThinkingLevel; thinkingSelection: ThinkingSelection | undefined; tier: ServiceTier | undefined; abortServerSideFallback: boolean | undefined; }, ): Promise { - if (previous.model) this.agent.state.model = previous.model; - else delete (this.agent.state as { model?: Model }).model; - this.agent.state.systemPrompt = previous.systemPrompt; - this.agent.state.thinkingLevel = previous.thinkingLevel; - this.agent.state.thinkingSelection = previous.thinkingSelection; - this._currentServiceTier = previous.tier; - this.agent.abortServerSideFallback = previous.abortServerSideFallback; + const restoreSessionState = (): void => { + if (previous.model) this.agent.state.model = previous.model; + else delete (this.agent.state as { model?: Model }).model; + this.agent.state.systemPrompt = previous.systemPrompt; + this._baseSystemPrompt = previous.baseSystemPrompt; + this.agent.state.tools = [...previous.tools]; + this._requestedActiveToolNames = previous.requestedActiveToolNames + ? [...previous.requestedActiveToolNames] + : undefined; + this._withheldEvalOnlyToolNames.clear(); + for (const name of previous.withheldEvalOnlyToolNames) { + this._withheldEvalOnlyToolNames.add(name); + } + this.agent.state.thinkingLevel = previous.thinkingLevel; + this.agent.state.thinkingSelection = previous.thinkingSelection; + this._currentServiceTier = previous.tier; + this.agent.abortServerSideFallback = previous.abortServerSideFallback; + }; + + restoreSessionState(); if (!previous.model) return; try { - await this._emitModelSelect(previous.model, rejectedModel, "restore"); + this._provisionalModelSelectDepth++; + try { + await this._emitModelSelect(previous.model, rejectedModel, "restore", { + deferSystemPromptAnnouncement: true, + }); + } finally { + this._provisionalModelSelectDepth--; + } } catch (error) { - // A failing resync must not replace the rejection the caller is about to - // report, and must not leave the prompt owned by the rejected model. - this.agent.state.systemPrompt = previous.systemPrompt; console.error("model switch rollback could not resync model_select state", error); + } finally { + // Extension resynchronization is best-effort. Session-owned state is restored + // directly both before and after it so a handler that mutates and then fails + // cannot leak the rejected model's prompt or tools into the parent session. + restoreSessionState(); } } @@ -7479,16 +7612,17 @@ export class AgentSession { } private _isHardErrorFallbackEligible(message: AssistantMessage): boolean { - return ( + const eligibleError = !message.errorMessage?.startsWith(TURN_RETRY_SUPPRESSION_PREFIX) && message.stopReason === "error" && !isContextOverflow(message, this.model?.contextWindow ?? 0) && !this._isCursorPayloadOverflow(message) && !isCursorZeroTokenResourceExhausted(message) && !isClassifierRefusal(message) && - !message.content.some((content) => content.type === "toolCall") && - this._retryFallback.canTryFallback() - ); + !message.content.some((content) => content.type === "toolCall"); + if (!eligibleError) return false; + if (this._retryFallback.canTryFallback()) return true; + return this._retryFallback.exhaustion?.reason === "no-context-compatible-candidate"; } /** @@ -7498,7 +7632,7 @@ export class AgentSession { * work to another model has enough to decide. Both are emitted from this single * place so the two views can never disagree across the retry branches. */ - private async _emitRetryFallbackExhausted(lastError: string): Promise { + private _emitRetryFallbackExhausted(lastError: string): void { const chainKey = this._retryFallback.exhaustedChainKey; if (!chainKey) return; this._emit({ type: "retry_fallback_exhausted", chainKey, lastError }); @@ -7506,15 +7640,21 @@ export class AgentSession { // Detail is only trustworthy when it describes the chain being reported. const detail = exhaustion?.chainKey === chainKey ? exhaustion : undefined; const model = this.model; - await this._extensionRunner.emit({ - type: "retry_fallback_exhausted", - sessionId: this.sessionId, - chainKey, - from: detail?.from ?? (model ? `${model.provider}/${model.id}` : ""), - lastError, - exhaustionReason: detail?.reason ?? "candidates-exhausted", - rejectedCandidates: detail?.rejectedCandidates ?? [], - }); + void this._extensionRunner + .emit({ + type: "retry_fallback_exhausted", + sessionId: this.sessionId, + chainKey, + from: detail?.from ?? (model ? `${model.provider}/${model.id}` : ""), + lastError: lastError.slice(0, MAX_FALLBACK_EXHAUSTION_ERROR_CHARS), + exhaustionReason: detail?.reason ?? "candidates-exhausted", + rejectedCandidates: (detail?.rejectedCandidates ?? []).slice(0, MAX_FALLBACK_EXHAUSTION_CANDIDATES), + }) + .catch((error: unknown) => { + this._sessionLogger.warn("retry_fallback_exhaustion_extension_failed", { + error: error instanceof Error ? error.message : String(error), + }); + }); } private _getProviderRetryDelayMs(errorMessage: string): number | undefined { @@ -7575,8 +7715,8 @@ export class AgentSession { // for providers without a declared profile). const turnMaxRetries = this._resolveRetryProfile().turn.maxRetries; const hintSettings = this.settingsManager.getHintPolicySettings(); - const finishTurn = async (attempt: number, finalError: string | undefined): Promise => { - await this._emitRetryFallbackExhausted(errorMessage); + const finishTurn = (attempt: number, finalError: string | undefined): void => { + this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7669,7 +7809,7 @@ export class AgentSession { errorMessage, }); if (!switchedFallback) { - await this._emitRetryFallbackExhausted(errorMessage); + this._emitRetryFallbackExhausted(errorMessage); this._resolveRetry(); return "not-handled"; } @@ -7694,7 +7834,7 @@ export class AgentSession { } switchedFallback = await this._retryFallback.tryFallback("refusal", {}); if (!switchedFallback) { - await this._emitRetryFallbackExhausted(errorMessage); + this._emitRetryFallbackExhausted(errorMessage); if (this._retryAttempt > 0) { this._emit({ type: "auto_retry_end", @@ -7740,7 +7880,7 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - await this._emitRetryFallbackExhausted(errorMessage); + this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7785,7 +7925,7 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - await this._emitRetryFallbackExhausted(errorMessage); + this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7823,7 +7963,7 @@ export class AgentSession { if (switchedFallback) { this._retryAttempt = 1; } else { - await this._emitRetryFallbackExhausted(errorMessage); + this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7876,7 +8016,7 @@ export class AgentSession { // The new model receives a fresh retry budget; the failed model does not. this._retryAttempt = 1; } else { - await this._emitRetryFallbackExhausted(errorMessage); + this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, @@ -7915,6 +8055,7 @@ export class AgentSession { } } if (!switchedFallback) { + this._emitRetryFallbackExhausted(errorMessage); this._emit({ type: "auto_retry_end", success: false, diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index cc65b2d1e8..398ca6663e 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -7,6 +7,7 @@ - `packages/coding-agent/src/core/retry-fallback/controller.ts`: `RetryFallbackControllerDeps` gained the injected `isCandidateUsable` capacity preflight and the `classifySwitchFailure` seam. `nextCandidate` now skips a rung whose window cannot hold the live conversation (`context-unusable`) and keeps walking; `tryFallback` walks the remaining rungs when applying a model is refused on capacity grounds after `model_select` ran, and rethrows any failure the classifier does not recognize so one broken extension cannot spend the whole chain. A turn-scoped, selector-keyed rejection ledger backs the new `exhaustion` accessor (`chainKey`, `from`, `reason`, `rejectedCandidates`); `exhaustedChainKey` is unchanged. - `packages/coding-agent/src/core/agent-session.ts`: `_switchActiveModel` is now two-phase. Model, thinking level, service tier, and the server-side-fallback flag are applied provisionally so `model_select` handlers build against the target, but compaction invalidation, `model_changed`, `thinking_level_changed`/`thinking_level_select`, the service-tier event, the high-reasoning warning, `appendModelChange`, and the persisted default all wait until the post-`model_select` `assertModelUsable` clears. A rejected target is rolled back silently and extension-owned prompt/tool state is resynced by re-running `model_select` for the previous model. The seven duplicated exhaustion emits collapse into `_emitRetryFallbackExhausted`, which emits the unchanged session event plus the new extension event. - `packages/coding-agent/src/core/extensions/types.ts`: new `RetryFallbackExhaustedEvent` (`sessionId`, `chainKey`, `from`, `lastError`, `exhaustionReason`, `rejectedCandidates`) in the `ExtensionEvent` union with a `pi.on("retry_fallback_exhausted", ...)` overload. It flows through the generic runner `emit` and returns no result: it is notification-only. +- Review hardening keeps the two-phase transaction scoped to automatic fallback while manual, cycle, restore, and fallback-revert switches retain their established committed-first event order. Rejected automatic fallbacks directly restore prompt, base prompt, active tools, requested/withheld tool names, model, thinking, tier, and server-fallback state; best-effort extension resynchronization cannot overwrite that snapshot. Exhaustion notifications do not block retry settlement and bound extension diagnostics to 8,192 error characters and 16 candidates. ### Why diff --git a/packages/coding-agent/src/core/retry-fallback/controller.ts b/packages/coding-agent/src/core/retry-fallback/controller.ts index 0ac337a2f5..3bbde8e4b9 100644 --- a/packages/coding-agent/src/core/retry-fallback/controller.ts +++ b/packages/coding-agent/src/core/retry-fallback/controller.ts @@ -51,14 +51,9 @@ export interface FallbackRejectedCandidate { readonly error?: string; } -/** - * Verdict from the injected capacity probe. The bare boolean keeps simple - * callers (and array-backed test doubles) ergonomic; the object form carries the - * projection so an exhaustion consumer can explain the refusal in tokens. - */ export type CandidateUsability = - | boolean - | { readonly usable: boolean; readonly projection?: ModelUsabilityBudgetProjection }; + | { readonly usable: true } + | { readonly usable: false; readonly projection?: ModelUsabilityBudgetProjection }; export type FallbackExhaustionReason = "candidates-exhausted" | "no-context-compatible-candidate"; @@ -408,12 +403,9 @@ export class RetryFallbackController { * handled here, and a failure inside it propagates, because a projection that * cannot be computed is a defect rather than a verdict about this candidate. */ - private assessUsability( - model: Model, - ): { usable: boolean; projection?: ModelUsabilityBudgetProjection } | undefined { + private assessUsability(model: Model): CandidateUsability | undefined { if (!this.deps.isCandidateUsable) return undefined; - const verdict = this.deps.isCandidateUsable(model); - return typeof verdict === "boolean" ? { usable: verdict } : verdict; + return this.deps.isCandidateUsable(model); } private recordRejection(rejected: FallbackRejectedCandidate): void { @@ -434,9 +426,10 @@ export class RetryFallbackController { } /** - * Single funnel for every reason a rung is passed over. Only reserving walks feed - * the ledger: `canTryFallback` re-walks the same chain several times per provider - * error, and those probes must not multiply the reported rejections. + * Single funnel for every reason a rung is passed over. Context incompatibility + * is retained during admission probes so the session can route the otherwise + * terminal error through the extension-visible exhaustion path. Other reasons + * only enter the ledger during the reserving walk. */ private skip( candidate: string, @@ -446,7 +439,7 @@ export class RetryFallbackController { detail?: { projection?: ModelUsabilityBudgetProjection }, ): void { this.deps.logger.debug("candidate_skipped", { candidate, skipReason }); - if (!reserve) return; + if (!reserve && skipReason !== "context-unusable") return; this.recordRejection( detail?.projection === undefined ? { selector, reason: skipReason } diff --git a/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts b/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts index 0cf813d28d..060da1a085 100644 --- a/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts @@ -3,7 +3,7 @@ import type { Api, Model } from "@earendil-works/pi-ai"; import { fauxAssistantMessage, getModel } from "@earendil-works/pi-ai/compat"; import { Type } from "typebox"; import { afterEach, describe, expect, it } from "vitest"; -import { RetryFallbackController } from "../../src/core/retry-fallback/controller.ts"; +import { type CandidateUsability, RetryFallbackController } from "../../src/core/retry-fallback/controller.ts"; import { SelectorCooldowns } from "../../src/core/retry-fallback/cooldown.ts"; import { createHarness, type Harness } from "./harness.ts"; @@ -75,7 +75,8 @@ describe("retry fallback context compatibility", () => { }, cooldowns: new SelectorCooldowns(() => 0), logger: { debug: () => {}, info: () => {}, warn: () => {} }, - isCandidateUsable: (candidate: Model) => candidate.id !== "incompatible", + isCandidateUsable: (candidate: Model): CandidateUsability => + candidate.id === "incompatible" ? { usable: false } : { usable: true }, switchModel: async (candidate: Model, thinking: ThinkingLevel) => { switches.push({ model: candidate.id, thinking }); current = { model: candidate, thinkingLevel: thinking }; @@ -140,6 +141,7 @@ describe("retry fallback context compatibility", () => { ], }); harnesses.push(harness); + const originalSystemPrompt = harness.session.systemPrompt; const internals = harness.session as unknown as { _handleRetryableError: ( message: ReturnType, @@ -161,7 +163,7 @@ describe("retry fallback context compatibility", () => { // then expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "model_change")).toEqual([]); expect(harness.session.model?.id).toBe("faux-1"); - expect(harness.session.systemPrompt).toBe("primary prompt"); + expect(harness.session.systemPrompt).toBe(originalSystemPrompt); expect(harness.session.getActiveToolNames()).toEqual(["primary_tool"]); expect(result).toEqual({ kind: "returned", value: "not-handled" }); }); diff --git a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts new file mode 100644 index 0000000000..727c21a6e8 --- /dev/null +++ b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts @@ -0,0 +1,222 @@ +import type { AgentTool } from "@earendil-works/pi-agent-core"; +import { fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; +import { Type } from "typebox"; +import { afterEach, describe, expect, it } from "vitest"; +import { createHarness, type Harness } from "./harness.ts"; + +type RetryInternals = { + _handleRetryableError: ( + message: ReturnType, + options: { hardErrorFallback: boolean }, + ) => Promise; +}; + +function retryInternals(harness: Harness): RetryInternals { + return harness.session as unknown as RetryInternals; +} + +function seedLiveContext(harness: Harness, tokens: number): void { + const timestamp = Date.now(); + const primary = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "continue the interrupted task" }], + timestamp: timestamp - 1, + }); + harness.sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "progress before provider failure" }], + api: primary.api, + provider: primary.provider, + model: primary.id, + stopReason: "stop", + usage: { + input: tokens - 1_000, + output: 1_000, + cacheRead: 0, + cacheWrite: 0, + totalTokens: tokens, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; +} + +function registerUnknownEvent(pi: object, eventName: string, handler: (event: unknown) => unknown): void { + const register = Reflect.get(pi, "on"); + if (typeof register !== "function") throw new Error("missing extension event registration"); + Reflect.apply(register, pi, [eventName, handler]); +} + +function objectValue(value: unknown): Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error("expected object value"); + } + return Object.fromEntries(Object.entries(value)); +} + +describe("retry fallback exhaustion isolation", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("settles the retry while a notification-only exhaustion handler remains pending", async () => { + // given + let notifyStarted: (() => void) | undefined; + const started = new Promise((resolve) => { + notifyStarted = resolve; + }); + let releaseNotify: (() => void) | undefined; + const pending = new Promise((resolve) => { + releaseNotify = resolve; + }); + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 80_000, maxTokens: 4_000 }, + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + extensionFactories: [ + (pi) => { + registerUnknownEvent(pi, "retry_fallback_exhausted", () => { + notifyStarted?.(); + return pending; + }); + }, + ], + }); + harnesses.push(harness); + seedLiveContext(harness, 90_000); + let settled = false; + const retry = retryInternals(harness) + ._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + { hardErrorFallback: true }, + ) + .then((result) => { + settled = true; + return result; + }); + + // when + await started; + await Promise.resolve(); + + // then + try { + expect(settled).toBe(true); + } finally { + releaseNotify?.(); + await retry; + } + }); + + it("restores the original toolset when model-select handlers mutate and throw", async () => { + // given + const primaryTool: AgentTool = { + name: "primary_tool", + label: "Primary", + description: "Primary model tool.", + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "primary" }], details: {} }), + }; + const fallbackTool: AgentTool = { + name: "fallback_tool", + label: "Fallback", + description: "oversized ".repeat(40_000), + parameters: Type.Object({}), + execute: async () => ({ content: [{ type: "text", text: "fallback" }], details: {} }), + }; + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 80_000, maxTokens: 4_000 }, + ], + systemPrompt: "primary prompt", + tools: [primaryTool, fallbackTool], + initialActiveToolNames: ["primary_tool"], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + extensionFactories: [ + (pi) => { + pi.on("model_select", (event) => { + if (event.model.id === "faux-2") pi.setActiveTools(["fallback_tool"]); + throw new Error(`model-select failure for ${event.model.id}`); + }); + }, + ], + }); + harnesses.push(harness); + const originalSystemPrompt = harness.session.systemPrompt; + + // when + await retryInternals(harness)._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + { hardErrorFallback: true }, + ); + + // then + expect(harness.session.model?.id).toBe("faux-1"); + expect(harness.session.systemPrompt).toBe(originalSystemPrompt); + expect(harness.session.getActiveToolNames()).toEqual(["primary_tool"]); + expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "model_change")).toEqual([]); + }); + + it("bounds the extension exhaustion diagnostics", async () => { + // given + const extensionEvents: unknown[] = []; + const fallbackIds = Array.from({ length: 24 }, (_, index) => `fallback-${index + 1}`); + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + ...fallbackIds.map((id) => ({ id, contextWindow: 80_000, maxTokens: 4_000 })), + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": fallbackIds.map((id) => `faux/${id}`) }, + }, + }, + extensionFactories: [ + (pi) => { + registerUnknownEvent(pi, "retry_fallback_exhausted", (event) => extensionEvents.push(event)); + }, + ], + }); + harnesses.push(harness); + seedLiveContext(harness, 90_000); + + // when + await retryInternals(harness)._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "x".repeat(20_000) }), + { hardErrorFallback: true }, + ); + const event = objectValue(extensionEvents[0]); + const rejected = event["rejectedCandidates"]; + + // then + expect(extensionEvents).toHaveLength(1); + expect(typeof event["lastError"]).toBe("string"); + expect(String(event["lastError"]).length).toBeLessThanOrEqual(8_192); + expect(Array.isArray(rejected)).toBe(true); + expect(Array.isArray(rejected) ? rejected.length : Number.POSITIVE_INFINITY).toBeLessThanOrEqual(16); + }); +}); diff --git a/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts b/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts new file mode 100644 index 0000000000..c9c627cc24 --- /dev/null +++ b/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts @@ -0,0 +1,88 @@ +import { fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; +import { afterEach, describe, expect, it } from "vitest"; +import { createHarness, type Harness } from "./harness.ts"; + +function seedLiveContext(harness: Harness, tokens: number): void { + const timestamp = Date.now(); + const primary = harness.getModel(); + harness.sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "work already in progress" }], + timestamp: timestamp - 1, + }); + harness.sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "text", text: "saved progress" }], + api: primary.api, + provider: primary.provider, + model: primary.id, + stopReason: "stop", + usage: { + input: tokens - 1_000, + output: 1_000, + cacheRead: 0, + cacheWrite: 0, + totalTokens: tokens, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp, + }); + harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; +} + +function registerUnknownEvent(pi: object, eventName: string, handler: (event: unknown) => unknown): void { + const register = Reflect.get(pi, "on"); + if (typeof register !== "function") throw new Error("missing extension event registration"); + Reflect.apply(register, pi, [eventName, handler]); +} + +describe("retry fallback exhaustion lifecycle", () => { + const harnesses: Harness[] = []; + + afterEach(() => { + while (harnesses.length > 0) harnesses.pop()?.cleanup(); + }); + + it("publishes context exhaustion from the real prompt lifecycle without calling the rejected model", async () => { + // given + const extensionEvents: unknown[] = []; + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 80_000, maxTokens: 4_000 }, + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + extensionFactories: [ + (pi) => { + registerUnknownEvent(pi, "retry_fallback_exhausted", (event) => extensionEvents.push(event)); + }, + ], + }); + harnesses.push(harness); + seedLiveContext(harness, 90_000); + harness.setResponses([ + fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "billing error: insufficient_quota", + }), + ]); + + // when + await harness.session.prompt("continue"); + await Promise.resolve(); + + // then + expect(harness.faux.getCallLog().map((call) => call.modelId)).toEqual(["faux-1"]); + expect(extensionEvents).toHaveLength(1); + expect(harness.eventsOfType("retry_fallback_exhausted")).toMatchObject([ + { chainKey: "faux/faux-1", lastError: "billing error: insufficient_quota" }, + ]); + }); +}); From ff26c610ad81e6c0d8e2340f5c64e8ce9674b5f9 Mon Sep 17 00:00:00 2001 From: prolls Date: Fri, 4 Sep 2026 10:23:47 +0900 Subject: [PATCH 3/6] fix(coding-agent): preserve fallback commit consistency Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- .../coding-agent/src/core/agent-session.ts | 72 +++++++++--------- packages/coding-agent/src/core/changes.md | 1 + .../src/core/extensions/changes.md | 5 +- .../coding-agent/src/core/extensions/index.ts | 1 + .../coding-agent/src/core/session-manager.ts | 9 ++- packages/coding-agent/src/index.ts | 1 + .../session-manager/tree-traversal.test.ts | 16 ++++ ...try-fallback-context-compatibility.test.ts | 49 +++++++++++-- ...etry-fallback-exhaustion-isolation.test.ts | 73 +++++++++++++------ 9 files changed, 161 insertions(+), 66 deletions(-) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 75c2e07c83..647c511df6 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -4589,7 +4589,7 @@ export class AgentSession { * live conversation only trades one dead lane for another, so the walk skips it. */ private _assessFallbackCandidate(model: Model): CandidateUsability { - if (model.contextWindow <= 0) return true; + if (model.contextWindow <= 0) return { usable: true }; const projection = projectModelUsabilityBudget({ model, systemPrompt: this.agent.state.systemPrompt, @@ -4727,20 +4727,8 @@ export class AgentSession { this._currentServiceTier = this._resolveServiceTier(model, scopedMatch?.serviceTier); this._applyProvisionalThinkingLevel(thinking, ephemeralThinking); - // Runs only once the target has cleared the post-`model_select` budget assert. - // Order matches an undeferred switch exactly: the durable record lands before any - // outward notification, so no observer can see a model that was not recorded. - const commit = (): void => { + const commitAfterPersistence = (): void => { if (invalidatesCompaction) this._invalidateCompactionForModelSelection(); - if (opts.appendSessionEntry) { - this.sessionManager.appendModelChange( - model.provider, - model.id, - opts.entryReason, - previousModel?.provider, - previousModel?.id, - ); - } if (opts.persistDefault) { this.settingsManager.setDefaultModelAndProvider(model.provider, model.id); } @@ -4763,30 +4751,46 @@ export class AgentSession { this._emitServiceTierChangeIfNeeded(previous.tier, previous.fastMode); }; - if (!opts.emitModelSelect) { - commit(); - return undefined; - } - try { - this._provisionalModelSelectDepth++; - let systemPromptChange: SystemPromptChangeEvent | undefined; + let systemPromptChange: SystemPromptChangeEvent | undefined; + if (opts.emitModelSelect) { try { - systemPromptChange = await this._emitModelSelect(model, previousModel, opts.modelSelectSource, { - deferSystemPromptAnnouncement: true, - }); - } finally { - this._provisionalModelSelectDepth--; + this._provisionalModelSelectDepth++; + try { + systemPromptChange = await this._emitModelSelect(model, previousModel, opts.modelSelectSource, { + deferSystemPromptAnnouncement: true, + }); + } finally { + this._provisionalModelSelectDepth--; + } + this.assertModelUsable(model, liveContextTokens); + } catch (error) { + await this._rollbackProvisionalModelSwitch(model, previous); + throw error; } - this.assertModelUsable(model, liveContextTokens); - commit(); - if (systemPromptChange) { - await this._announceSystemPromptChange(systemPromptChange); + } + + // Persistence is the commit boundary. A failed append is still provisional + // and rolls back; once it succeeds, later observer failures must leave the + // runtime on the model recorded in session history. + if (opts.appendSessionEntry) { + try { + this.sessionManager.appendModelChange( + model.provider, + model.id, + opts.entryReason, + previousModel?.provider, + previousModel?.id, + ); + } catch (error) { + await this._rollbackProvisionalModelSwitch(model, previous); + throw error; } - return systemPromptChange; - } catch (error) { - await this._rollbackProvisionalModelSwitch(model, previous); - throw error; } + commitAfterPersistence(); + if (systemPromptChange) { + await this._announceSystemPromptChange(systemPromptChange); + } + return systemPromptChange; } /** diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index 398ca6663e..af2f0dca08 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -8,6 +8,7 @@ - `packages/coding-agent/src/core/agent-session.ts`: `_switchActiveModel` is now two-phase. Model, thinking level, service tier, and the server-side-fallback flag are applied provisionally so `model_select` handlers build against the target, but compaction invalidation, `model_changed`, `thinking_level_changed`/`thinking_level_select`, the service-tier event, the high-reasoning warning, `appendModelChange`, and the persisted default all wait until the post-`model_select` `assertModelUsable` clears. A rejected target is rolled back silently and extension-owned prompt/tool state is resynced by re-running `model_select` for the previous model. The seven duplicated exhaustion emits collapse into `_emitRetryFallbackExhausted`, which emits the unchanged session event plus the new extension event. - `packages/coding-agent/src/core/extensions/types.ts`: new `RetryFallbackExhaustedEvent` (`sessionId`, `chainKey`, `from`, `lastError`, `exhaustionReason`, `rejectedCandidates`) in the `ExtensionEvent` union with a `pi.on("retry_fallback_exhausted", ...)` overload. It flows through the generic runner `emit` and returns no result: it is notification-only. - Review hardening keeps the two-phase transaction scoped to automatic fallback while manual, cycle, restore, and fallback-revert switches retain their established committed-first event order. Rejected automatic fallbacks directly restore prompt, base prompt, active tools, requested/withheld tool names, model, thinking, tier, and server-fallback state; best-effort extension resynchronization cannot overwrite that snapshot. Exhaustion notifications do not block retry settlement and bound extension diagnostics to 8,192 error characters and 16 candidates. +- Persistence now defines the automatic-switch commit boundary: pre-commit failures roll back, while post-commit observer failures leave runtime state aligned with the durable `model_change`. `SessionManager` persists an entry before exposing it through in-memory indexes, so a filesystem failure cannot leave a phantom resident entry. ### Why diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index 63875d773d..ff42b6fc82 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -8,7 +8,8 @@ `RetryFallbackExhaustedEvent` and the matching `pi.on("retry_fallback_exhausted", ...)` overload. The payload names the session, active selector, exhausted chain, terminal provider error, exhaustion reason, and - the rejected candidate budget projections. + the rejected candidate budget projections. The event type is re-exported from + both the extension barrel and the package root for extension authors. ### Why @@ -27,6 +28,8 @@ - LOW: the event interface, `ExtensionEvent` union, and `ExtensionAPI.on` overload in `packages/coding-agent/src/core/extensions/types.ts`. +- LOW: public type export lists in `packages/coding-agent/src/core/extensions/index.ts` + and `packages/coding-agent/src/index.ts`. ## Expose the extension event bus for session activity signals (2026-08-31) diff --git a/packages/coding-agent/src/core/extensions/index.ts b/packages/coding-agent/src/core/extensions/index.ts index 75ef51484c..60c7829847 100644 --- a/packages/coding-agent/src/core/extensions/index.ts +++ b/packages/coding-agent/src/core/extensions/index.ts @@ -143,6 +143,7 @@ export type { // Events - Resources ResourcesDiscoverEvent, ResourcesDiscoverResult, + RetryFallbackExhaustedEvent, SendMessageHandler, SendUserMessageHandler, SessionBeforeCompactEvent, diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index b751702606..2b4cc7a786 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -1031,7 +1031,9 @@ export class SessionManager { if (!this.persist || !this.sessionFile) return; const persistedEntry = this.residentStore.materialize(entry); - const hasAssistant = this.fileEntries.some((e) => e.type === "message" && e.message.role === "assistant"); + const hasAssistant = + (entry.type === "message" && entry.message.role === "assistant") || + this.fileEntries.some((e) => e.type === "message" && e.message.role === "assistant"); if (!hasAssistant) { if (this.flushed) { appendFileSync(this.sessionFile, `${JSON.stringify(persistedEntry)}\n`); @@ -1048,6 +1050,7 @@ export class SessionManager { for (const e of this.fileEntries) { writeFileSync(fd, `${JSON.stringify(this.residentStore.materialize(e))}\n`); } + writeFileSync(fd, `${JSON.stringify(persistedEntry)}\n`); } finally { closeSync(fd); } @@ -1059,13 +1062,15 @@ export class SessionManager { private _appendEntry(entry: SessionEntry): void { const residentEntry = this.residentStore.externalize(entry); + // Persist first so a filesystem failure cannot expose an entry through the + // in-memory indexes while the durable session lacks it. + this._persist(residentEntry); 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++; - this._persist(residentEntry); } /** diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index c18fd4726b..2d08df323e 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -128,6 +128,7 @@ export type { RegisteredCommand, RegisteredTool, ResolvedCommand, + RetryFallbackExhaustedEvent, SessionBeforeCompactEvent, SessionBeforeForkEvent, SessionBeforeSwitchEvent, diff --git a/packages/coding-agent/test/session-manager/tree-traversal.test.ts b/packages/coding-agent/test/session-manager/tree-traversal.test.ts index 123dc65cd6..85004e9167 100644 --- a/packages/coding-agent/test/session-manager/tree-traversal.test.ts +++ b/packages/coding-agent/test/session-manager/tree-traversal.test.ts @@ -66,6 +66,22 @@ describe("SessionManager append and tree traversal", () => { expect(entries[2].parentId).toBe(modelId); }); + it("does not expose an entry when persistence fails", () => { + // given + const session = SessionManager.inMemory(); + const entriesBefore = session.getEntries(); + Reflect.set(session, "_persist", () => { + throw new Error("disk full"); + }); + + // when + const append = () => session.appendModelChange("openai", "gpt-4"); + + // then + expect(append).toThrow("disk full"); + expect(session.getEntries()).toEqual(entriesBefore); + }); + it("appendCompaction integrates into tree", () => { const session = SessionManager.inMemory(); diff --git a/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts b/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts index 060da1a085..b8b54a0c4e 100644 --- a/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-context-compatibility.test.ts @@ -5,6 +5,7 @@ import { Type } from "typebox"; import { afterEach, describe, expect, it } from "vitest"; import { type CandidateUsability, RetryFallbackController } from "../../src/core/retry-fallback/controller.ts"; import { SelectorCooldowns } from "../../src/core/retry-fallback/cooldown.ts"; +import type { RetryFallbackExhaustedEvent } from "../../src/index.ts"; import { createHarness, type Harness } from "./harness.ts"; type SwitchRecord = { @@ -95,6 +96,43 @@ describe("retry fallback context compatibility", () => { expect(switches).toEqual([{ model: "compatible", thinking: "high" }]); }); + it("accepts a fallback with an unknown context window", async () => { + // given + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 0, maxTokens: 4_000 }, + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + }); + harnesses.push(harness); + const internals = harness.session as unknown as { + _handleRetryableError: ( + message: ReturnType, + options: { hardErrorFallback: boolean }, + ) => Promise; + }; + + // when + await internals._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + { hardErrorFallback: true }, + ); + + // then + expect(harness.session.model?.id).toBe("faux-2"); + expect(harness.sessionManager.getEntries()).toContainEqual( + expect.objectContaining({ type: "model_change", modelId: "faux-2", reason: "fallback" }), + ); + }); + it("rolls back a post-model-select budget rejection without persisting a fallback switch", async () => { // given const primaryTool: AgentTool = { @@ -170,7 +208,7 @@ describe("retry fallback context compatibility", () => { it("emits one extension-visible exhaustion event when every fallback is context-incompatible", async () => { // given - const extensionEvents: unknown[] = []; + const extensionEvents: RetryFallbackExhaustedEvent[] = []; const harness = await createHarness({ models: [ { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, @@ -186,12 +224,9 @@ describe("retry fallback context compatibility", () => { }, extensionFactories: [ (pi) => { - const register = Reflect.get(pi, "on"); - if (typeof register !== "function") throw new Error("missing extension event registration"); - Reflect.apply(register, pi, [ - "retry_fallback_exhausted", - (event: unknown) => extensionEvents.push(event), - ]); + pi.on("retry_fallback_exhausted", (event) => { + extensionEvents.push(event); + }); }, ], }); diff --git a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts index 727c21a6e8..c95e42b150 100644 --- a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts @@ -2,6 +2,7 @@ import type { AgentTool } from "@earendil-works/pi-agent-core"; import { fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; import { Type } from "typebox"; import { afterEach, describe, expect, it } from "vitest"; +import type { RetryFallbackExhaustedEvent } from "../../src/index.ts"; import { createHarness, type Harness } from "./harness.ts"; type RetryInternals = { @@ -43,19 +44,6 @@ function seedLiveContext(harness: Harness, tokens: number): void { harness.session.agent.state.messages = harness.sessionManager.buildSessionContext().messages; } -function registerUnknownEvent(pi: object, eventName: string, handler: (event: unknown) => unknown): void { - const register = Reflect.get(pi, "on"); - if (typeof register !== "function") throw new Error("missing extension event registration"); - Reflect.apply(register, pi, [eventName, handler]); -} - -function objectValue(value: unknown): Record { - if (typeof value !== "object" || value === null || Array.isArray(value)) { - throw new Error("expected object value"); - } - return Object.fromEntries(Object.entries(value)); -} - describe("retry fallback exhaustion isolation", () => { const harnesses: Harness[] = []; @@ -88,7 +76,7 @@ describe("retry fallback exhaustion isolation", () => { }, extensionFactories: [ (pi) => { - registerUnknownEvent(pi, "retry_fallback_exhausted", () => { + pi.on("retry_fallback_exhausted", () => { notifyStarted?.(); return pending; }); @@ -178,9 +166,51 @@ describe("retry fallback exhaustion isolation", () => { expect(harness.sessionManager.getEntries().filter((entry) => entry.type === "model_change")).toEqual([]); }); + it("keeps a persisted fallback active when a post-commit observer throws", async () => { + // given + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 200_000, maxTokens: 4_000 }, + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + }); + harnesses.push(harness); + harness.session.subscribe((event) => { + if (event.type === "model_changed" && event.model.id === "faux-2") { + throw new Error("observer failed after commit"); + } + }); + + // when + const result = await retryInternals(harness) + ._handleRetryableError( + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + { hardErrorFallback: true }, + ) + .then( + () => "returned" as const, + (error: unknown) => (error instanceof Error ? error.message : String(error)), + ); + + // then + expect(result).toBe("observer failed after commit"); + expect(harness.session.model?.id).toBe("faux-2"); + expect(harness.sessionManager.getEntries()).toContainEqual( + expect.objectContaining({ type: "model_change", modelId: "faux-2", reason: "fallback" }), + ); + }); + it("bounds the extension exhaustion diagnostics", async () => { // given - const extensionEvents: unknown[] = []; + const extensionEvents: RetryFallbackExhaustedEvent[] = []; const fallbackIds = Array.from({ length: 24 }, (_, index) => `fallback-${index + 1}`); const harness = await createHarness({ models: [ @@ -197,7 +227,9 @@ describe("retry fallback exhaustion isolation", () => { }, extensionFactories: [ (pi) => { - registerUnknownEvent(pi, "retry_fallback_exhausted", (event) => extensionEvents.push(event)); + pi.on("retry_fallback_exhausted", (event) => { + extensionEvents.push(event); + }); }, ], }); @@ -209,14 +241,11 @@ describe("retry fallback exhaustion isolation", () => { fauxAssistantMessage("", { stopReason: "error", errorMessage: "x".repeat(20_000) }), { hardErrorFallback: true }, ); - const event = objectValue(extensionEvents[0]); - const rejected = event["rejectedCandidates"]; + const event = extensionEvents[0]; // then expect(extensionEvents).toHaveLength(1); - expect(typeof event["lastError"]).toBe("string"); - expect(String(event["lastError"]).length).toBeLessThanOrEqual(8_192); - expect(Array.isArray(rejected)).toBe(true); - expect(Array.isArray(rejected) ? rejected.length : Number.POSITIVE_INFINITY).toBeLessThanOrEqual(16); + expect(event?.lastError.length).toBeLessThanOrEqual(8_192); + expect(event?.rejectedCandidates.length).toBeLessThanOrEqual(16); }); }); From 5a816a1b50635d8175fbc608a94ef2338e77f559 Mon Sep 17 00:00:00 2001 From: prolls Date: Fri, 4 Sep 2026 10:44:40 +0900 Subject: [PATCH 4/6] fix(coding-agent): isolate fallback observers and persistence Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/docs/extensions.md | 4 +- .../coding-agent/src/core/agent-session.ts | 37 +++++++++-- packages/coding-agent/src/core/changes.md | 1 + .../coding-agent/src/core/session-manager.ts | 37 +++++++++-- .../session-manager/tree-traversal.test.ts | 35 ++++++++++ ...etry-fallback-exhaustion-isolation.test.ts | 65 ++++++++++++++++++- 6 files changed, 164 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 21a7caba84..035681f0f5 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -847,7 +847,9 @@ pi.on("retry_fallback_exhausted", (event, ctx) => { The payload includes `sessionId`, `chainKey`, `from`, `lastError`, `exhaustionReason`, and `rejectedCandidates`. Diagnostics delivered to -extensions are bounded to 8,192 error characters and 16 rejected candidates. +extensions are bounded to 8,192 terminal-error characters, 16 rejected +candidates, 512 characters per selector-bearing field, and 2,048 characters +per candidate error. The original session event remains available to TUI/RPC listeners with its existing `{ chainKey, lastError }` shape. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 66a03b5458..95d4c6ff52 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -222,6 +222,8 @@ function evalHelperCall(name: string): string { const TURN_RETRY_SUPPRESSION_PREFIX = "senpi:no-turn-retry:"; const MAX_FALLBACK_EXHAUSTION_ERROR_CHARS = 8_192; const MAX_FALLBACK_EXHAUSTION_CANDIDATES = 16; +const MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS = 512; +const MAX_FALLBACK_EXHAUSTION_CANDIDATE_ERROR_CHARS = 2_048; const DEFERRED_RETRY_QUEUE_OWNERS = new WeakSet(); // ============================================================================ @@ -1642,7 +1644,14 @@ export class AgentSession { private _emit(event: AgentSessionEvent): void { this._logSessionEvent(event); for (const l of this._eventListeners) { - l(event); + try { + l(event); + } catch (error) { + this._sessionLogger.warn("session_event_listener_failed", { + kind: event.type, + error: error instanceof Error ? error.message : String(error), + }); + } } } @@ -7679,15 +7688,35 @@ export class AgentSession { // Detail is only trustworthy when it describes the chain being reported. const detail = exhaustion?.chainKey === chainKey ? exhaustion : undefined; const model = this.model; + const rejectedCandidates = (detail?.rejectedCandidates ?? []) + .slice(0, MAX_FALLBACK_EXHAUSTION_CANDIDATES) + .map((candidate) => ({ + ...candidate, + selector: candidate.selector.slice(0, MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS), + ...(candidate.error === undefined + ? {} + : { error: candidate.error.slice(0, MAX_FALLBACK_EXHAUSTION_CANDIDATE_ERROR_CHARS) }), + ...(candidate.projection === undefined + ? {} + : { + projection: { + ...candidate.projection, + model: candidate.projection.model.slice(0, MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS), + }, + }), + })); void this._extensionRunner .emit({ type: "retry_fallback_exhausted", sessionId: this.sessionId, - chainKey, - from: detail?.from ?? (model ? `${model.provider}/${model.id}` : ""), + chainKey: chainKey.slice(0, MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS), + from: (detail?.from ?? (model ? `${model.provider}/${model.id}` : "")).slice( + 0, + MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS, + ), lastError: lastError.slice(0, MAX_FALLBACK_EXHAUSTION_ERROR_CHARS), exhaustionReason: detail?.reason ?? "candidates-exhausted", - rejectedCandidates: (detail?.rejectedCandidates ?? []).slice(0, MAX_FALLBACK_EXHAUSTION_CANDIDATES), + rejectedCandidates, }) .catch((error: unknown) => { this._sessionLogger.warn("retry_fallback_exhaustion_extension_failed", { diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index f211540ad5..459fb7d98c 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -10,6 +10,7 @@ - Review hardening keeps the two-phase transaction scoped to automatic fallback while manual, cycle, restore, and fallback-revert switches retain their established committed-first event order. Rejected automatic fallbacks directly restore prompt, base prompt, active tools, requested/withheld tool names, model, thinking, tier, and server-fallback state; best-effort extension resynchronization cannot overwrite that snapshot. Exhaustion notifications do not block retry settlement and bound extension diagnostics to 8,192 error characters and 16 candidates. - Persistence now defines the automatic-switch commit boundary: pre-commit failures roll back, while post-commit observer failures leave runtime state aligned with the durable `model_change`. `SessionManager` persists an entry before exposing it through in-memory indexes, so a filesystem failure cannot leave a phantom resident entry. - Post-commit switch and fallback-event observer failures are logged and isolated so controller bookkeeping still records the applied rung. The retry-handler ownership boundary converts any remaining internal failure into a terminal `not-handled` outcome and resolves the retry promise, preventing a failed persistence or extension path from wedging `prompt()`. +- Session event listeners are failure-isolated per listener so an observer cannot abort core `agent_end` retry handling. Initial session-file publication now writes a private temporary file and atomically renames it, while failed append writes truncate back to the observed pre-append size before the in-memory entry is exposed. Extension exhaustion payloads cap every selector/model/error string as well as candidate count. ### Why diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index 2b4cc7a786..fe5afbd96c 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -9,7 +9,10 @@ import { openSync, readdirSync, readSync, + renameSync, + rmSync, statSync, + truncateSync, writeFileSync, } from "fs"; import { readdir } from "fs/promises"; @@ -1045,18 +1048,38 @@ export class SessionManager { } if (!this.flushed) { - const fd = openSync(this.sessionFile, "wx"); + const temporaryFile = `${this.sessionFile}.${process.pid}.${randomUUID()}.tmp`; try { - for (const e of this.fileEntries) { - writeFileSync(fd, `${JSON.stringify(this.residentStore.materialize(e))}\n`); + const fd = openSync(temporaryFile, "wx", 0o600); + try { + for (const e of this.fileEntries) { + writeFileSync(fd, `${JSON.stringify(this.residentStore.materialize(e))}\n`); + } + writeFileSync(fd, `${JSON.stringify(persistedEntry)}\n`); + } finally { + closeSync(fd); } - writeFileSync(fd, `${JSON.stringify(persistedEntry)}\n`); - } finally { - closeSync(fd); + renameSync(temporaryFile, this.sessionFile); + } catch (error) { + rmSync(temporaryFile, { force: true }); + throw error; } this.flushed = true; } else { - appendFileSync(this.sessionFile, `${JSON.stringify(persistedEntry)}\n`); + const originalSize = statSync(this.sessionFile).size; + try { + appendFileSync(this.sessionFile, `${JSON.stringify(persistedEntry)}\n`); + } catch (error) { + try { + truncateSync(this.sessionFile, originalSize); + } catch (rollbackError) { + throw new AggregateError( + [error, rollbackError], + `Session append failed and ${this.sessionFile} could not be restored`, + ); + } + throw error; + } } } diff --git a/packages/coding-agent/test/session-manager/tree-traversal.test.ts b/packages/coding-agent/test/session-manager/tree-traversal.test.ts index 85004e9167..d4a37da5f3 100644 --- a/packages/coding-agent/test/session-manager/tree-traversal.test.ts +++ b/packages/coding-agent/test/session-manager/tree-traversal.test.ts @@ -611,4 +611,39 @@ describe("createBranchedSession", () => { rmSync(tempDir, { recursive: true, force: true }); } }); + + it("does not publish a partial initial session file when serialization fails", () => { + // given + const tempDir = join(tmpdir(), `session-atomic-flush-${Date.now()}`); + mkdirSync(tempDir, { recursive: true }); + + try { + const session = SessionManager.create(tempDir, tempDir); + const sessionFile = session.getSessionFile(); + if (!sessionFile) throw new Error("missing session file path"); + session.appendMessage(userMsg("deferred")); + const residentStore = Reflect.get(session, "residentStore"); + const materialize = Reflect.get(residentStore, "materialize"); + if (typeof materialize !== "function") throw new Error("missing resident materializer"); + Reflect.set(residentStore, "materialize", (entry: { readonly type: string }) => { + const materialized = Reflect.apply(materialize, residentStore, [entry]); + if (entry.type !== "message" || typeof materialized !== "object" || materialized === null) { + return materialized; + } + const message = Reflect.get(materialized, "message"); + return typeof message === "object" && message !== null && Reflect.get(message, "role") === "assistant" + ? { ...Object.fromEntries(Object.entries(materialized)), invalid: 1n } + : materialized; + }); + + // when + const flush = () => session.appendMessage(assistantMsg("flush")); + + // then + expect(flush).toThrow(); + expect(existsSync(sessionFile)).toBe(false); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); }); diff --git a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts index 54d7a2dedf..0ef354facc 100644 --- a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts @@ -224,6 +224,56 @@ describe("retry fallback exhaustion isolation", () => { expect(lastMessage.content).toEqual([{ type: "text", text: "fallback answer" }]); }); + it("completes a fallback turn when an agent-end observer throws before retry handling", async () => { + // given + const harness = await createHarness({ + models: [ + { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: "faux-2", contextWindow: 200_000, maxTokens: 4_000 }, + ], + settings: { + retry: { + enabled: true, + maxRetries: 0, + baseDelayMs: 1, + fallbackChains: { "faux/faux-1": ["faux/faux-2"] }, + }, + }, + }); + harnesses.push(harness); + harness.setResponses([ + fauxAssistantMessage("", { stopReason: "error", errorMessage: "upstream unavailable" }), + fauxAssistantMessage("fallback after observer"), + ]); + let rejectedAgentEnd = false; + harness.session.subscribe((event) => { + if (event.type === "agent_end" && !rejectedAgentEnd) { + rejectedAgentEnd = true; + throw new Error("agent-end observer failed"); + } + }); + + // when + let timeout: ReturnType | undefined; + const prompt = harness.session.prompt("continue despite observer"); + try { + await Promise.race([ + prompt, + new Promise((_, reject) => { + timeout = setTimeout(() => reject(new Error("prompt remained blocked after observer failure")), 500); + }), + ]); + } finally { + if (timeout) clearTimeout(timeout); + if (harness.session.isRetrying) harness.session.abortRetry(); + } + + // then + expect(rejectedAgentEnd).toBe(true); + expect(harness.session.isRetrying).toBe(false); + expect(harness.session.model?.id).toBe("faux-2"); + }); + it("settles the public prompt lifecycle when fallback persistence fails", async () => { // given const harness = await createHarness({ @@ -261,10 +311,11 @@ describe("retry fallback exhaustion isolation", () => { it("bounds the extension exhaustion diagnostics", async () => { // given const extensionEvents: RetryFallbackExhaustedEvent[] = []; - const fallbackIds = Array.from({ length: 24 }, (_, index) => `fallback-${index + 1}`); + const primaryId = `primary-${"p".repeat(2_000)}`; + const fallbackIds = Array.from({ length: 24 }, (_, index) => `fallback-${index + 1}-${"f".repeat(2_000)}`); const harness = await createHarness({ models: [ - { id: "faux-1", contextWindow: 1_000_000, maxTokens: 4_000 }, + { id: primaryId, contextWindow: 1_000_000, maxTokens: 4_000 }, ...fallbackIds.map((id) => ({ id, contextWindow: 80_000, maxTokens: 4_000 })), ], settings: { @@ -272,7 +323,7 @@ describe("retry fallback exhaustion isolation", () => { enabled: true, maxRetries: 0, baseDelayMs: 1, - fallbackChains: { "faux/faux-1": fallbackIds.map((id) => `faux/${id}`) }, + fallbackChains: { [`faux/${primaryId}`]: fallbackIds.map((id) => `faux/${id}`) }, }, }, extensionFactories: [ @@ -295,7 +346,15 @@ describe("retry fallback exhaustion isolation", () => { // then expect(extensionEvents).toHaveLength(1); + expect(event?.chainKey.length).toBeLessThanOrEqual(512); + expect(event?.from.length).toBeLessThanOrEqual(512); expect(event?.lastError.length).toBeLessThanOrEqual(8_192); expect(event?.rejectedCandidates.length).toBeLessThanOrEqual(16); + for (const rejected of event?.rejectedCandidates ?? []) { + expect(rejected.selector.length).toBeLessThanOrEqual(512); + expect(rejected.error?.length ?? 0).toBeLessThanOrEqual(2_048); + expect(rejected.projection?.model.length ?? 0).toBeLessThanOrEqual(512); + } + expect(Buffer.byteLength(JSON.stringify(event))).toBeLessThanOrEqual(64 * 1_024); }); }); From 62854b8e9c4f0bd6cb17f72c8b58e783aa6a6ab0 Mon Sep 17 00:00:00 2001 From: prolls Date: Fri, 4 Sep 2026 10:50:22 +0900 Subject: [PATCH 5/6] fix(coding-agent): cap fallback events by utf8 size Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/docs/extensions.md | 6 +- .../coding-agent/src/core/agent-session.ts | 66 ++++++++++++------- ...etry-fallback-exhaustion-isolation.test.ts | 18 ++--- 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 035681f0f5..fa61cc054b 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -847,9 +847,9 @@ pi.on("retry_fallback_exhausted", (event, ctx) => { The payload includes `sessionId`, `chainKey`, `from`, `lastError`, `exhaustionReason`, and `rejectedCandidates`. Diagnostics delivered to -extensions are bounded to 8,192 terminal-error characters, 16 rejected -candidates, 512 characters per selector-bearing field, and 2,048 characters -per candidate error. +extensions are bounded to 64 KiB of serialized UTF-8: at most 8,192 bytes for +the terminal error, 16 rejected candidates, 512 bytes per selector-bearing +field, and 2,048 bytes per candidate error. The original session event remains available to TUI/RPC listeners with its existing `{ chainKey, lastError }` shape. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 95d4c6ff52..e2c2935a85 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -132,6 +132,7 @@ import { type MessageStartEvent, type MessageUpdateEvent, type ReplacedSessionContext, + type RetryFallbackExhaustedEvent, type SessionBeforeCompactResult, type SessionBeforeTreeResult, type SessionCompactFailedEvent, @@ -220,12 +221,26 @@ function evalHelperCall(name: string): string { return `tool.${name}({ ... })`; } const TURN_RETRY_SUPPRESSION_PREFIX = "senpi:no-turn-retry:"; -const MAX_FALLBACK_EXHAUSTION_ERROR_CHARS = 8_192; +const MAX_FALLBACK_EXHAUSTION_ERROR_BYTES = 8_192; const MAX_FALLBACK_EXHAUSTION_CANDIDATES = 16; -const MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS = 512; -const MAX_FALLBACK_EXHAUSTION_CANDIDATE_ERROR_CHARS = 2_048; +const MAX_FALLBACK_EXHAUSTION_SELECTOR_BYTES = 512; +const MAX_FALLBACK_EXHAUSTION_CANDIDATE_ERROR_BYTES = 2_048; +const MAX_FALLBACK_EXHAUSTION_PAYLOAD_BYTES = 64 * 1_024; const DEFERRED_RETRY_QUEUE_OWNERS = new WeakSet(); +function truncateUtf8(text: string, maxBytes: number): string { + if (Buffer.byteLength(text) <= maxBytes) return text; + let bytes = 0; + let result = ""; + for (const character of text) { + const characterBytes = Buffer.byteLength(character); + if (bytes + characterBytes > maxBytes) break; + result += character; + bytes += characterBytes; + } + return result; +} + // ============================================================================ // Skill Invocation Formatting and Parsing // ============================================================================ @@ -7692,37 +7707,42 @@ export class AgentSession { .slice(0, MAX_FALLBACK_EXHAUSTION_CANDIDATES) .map((candidate) => ({ ...candidate, - selector: candidate.selector.slice(0, MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS), + selector: truncateUtf8(candidate.selector, MAX_FALLBACK_EXHAUSTION_SELECTOR_BYTES), ...(candidate.error === undefined ? {} - : { error: candidate.error.slice(0, MAX_FALLBACK_EXHAUSTION_CANDIDATE_ERROR_CHARS) }), + : { error: truncateUtf8(candidate.error, MAX_FALLBACK_EXHAUSTION_CANDIDATE_ERROR_BYTES) }), ...(candidate.projection === undefined ? {} : { projection: { ...candidate.projection, - model: candidate.projection.model.slice(0, MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS), + model: truncateUtf8(candidate.projection.model, MAX_FALLBACK_EXHAUSTION_SELECTOR_BYTES), }, }), })); - void this._extensionRunner - .emit({ - type: "retry_fallback_exhausted", - sessionId: this.sessionId, - chainKey: chainKey.slice(0, MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS), - from: (detail?.from ?? (model ? `${model.provider}/${model.id}` : "")).slice( - 0, - MAX_FALLBACK_EXHAUSTION_SELECTOR_CHARS, - ), - lastError: lastError.slice(0, MAX_FALLBACK_EXHAUSTION_ERROR_CHARS), - exhaustionReason: detail?.reason ?? "candidates-exhausted", - rejectedCandidates, - }) - .catch((error: unknown) => { - this._sessionLogger.warn("retry_fallback_exhaustion_extension_failed", { - error: error instanceof Error ? error.message : String(error), - }); + const extensionEvent = { + type: "retry_fallback_exhausted", + sessionId: this.sessionId, + chainKey: truncateUtf8(chainKey, MAX_FALLBACK_EXHAUSTION_SELECTOR_BYTES), + from: truncateUtf8( + detail?.from ?? (model ? `${model.provider}/${model.id}` : ""), + MAX_FALLBACK_EXHAUSTION_SELECTOR_BYTES, + ), + lastError: truncateUtf8(lastError, MAX_FALLBACK_EXHAUSTION_ERROR_BYTES), + exhaustionReason: detail?.reason ?? "candidates-exhausted", + rejectedCandidates, + } satisfies RetryFallbackExhaustedEvent; + while ( + Buffer.byteLength(JSON.stringify(extensionEvent)) > MAX_FALLBACK_EXHAUSTION_PAYLOAD_BYTES && + rejectedCandidates.length > 0 + ) { + rejectedCandidates.pop(); + } + void this._extensionRunner.emit(extensionEvent).catch((error: unknown) => { + this._sessionLogger.warn("retry_fallback_exhaustion_extension_failed", { + error: error instanceof Error ? error.message : String(error), }); + }); } private _getProviderRetryDelayMs(errorMessage: string): number | undefined { diff --git a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts index 0ef354facc..a8281c3039 100644 --- a/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-exhaustion-isolation.test.ts @@ -311,8 +311,8 @@ describe("retry fallback exhaustion isolation", () => { it("bounds the extension exhaustion diagnostics", async () => { // given const extensionEvents: RetryFallbackExhaustedEvent[] = []; - const primaryId = `primary-${"p".repeat(2_000)}`; - const fallbackIds = Array.from({ length: 24 }, (_, index) => `fallback-${index + 1}-${"f".repeat(2_000)}`); + const primaryId = `primary-${"주".repeat(2_000)}`; + const fallbackIds = Array.from({ length: 24 }, (_, index) => `fallback-${index + 1}-${"후".repeat(2_000)}`); const harness = await createHarness({ models: [ { id: primaryId, contextWindow: 1_000_000, maxTokens: 4_000 }, @@ -339,21 +339,21 @@ describe("retry fallback exhaustion isolation", () => { // when await retryInternals(harness)._handleRetryableError( - fauxAssistantMessage("", { stopReason: "error", errorMessage: "x".repeat(20_000) }), + fauxAssistantMessage("", { stopReason: "error", errorMessage: "오류".repeat(20_000) }), { hardErrorFallback: true }, ); const event = extensionEvents[0]; // then expect(extensionEvents).toHaveLength(1); - expect(event?.chainKey.length).toBeLessThanOrEqual(512); - expect(event?.from.length).toBeLessThanOrEqual(512); - expect(event?.lastError.length).toBeLessThanOrEqual(8_192); + expect(Buffer.byteLength(event?.chainKey ?? "")).toBeLessThanOrEqual(512); + expect(Buffer.byteLength(event?.from ?? "")).toBeLessThanOrEqual(512); + expect(Buffer.byteLength(event?.lastError ?? "")).toBeLessThanOrEqual(8_192); expect(event?.rejectedCandidates.length).toBeLessThanOrEqual(16); for (const rejected of event?.rejectedCandidates ?? []) { - expect(rejected.selector.length).toBeLessThanOrEqual(512); - expect(rejected.error?.length ?? 0).toBeLessThanOrEqual(2_048); - expect(rejected.projection?.model.length ?? 0).toBeLessThanOrEqual(512); + expect(Buffer.byteLength(rejected.selector)).toBeLessThanOrEqual(512); + expect(Buffer.byteLength(rejected.error ?? "")).toBeLessThanOrEqual(2_048); + expect(Buffer.byteLength(rejected.projection?.model ?? "")).toBeLessThanOrEqual(512); } expect(Buffer.byteLength(JSON.stringify(event))).toBeLessThanOrEqual(64 * 1_024); }); From 759544e9bfff0dafae19aee5771b2506d5c2b7be Mon Sep 17 00:00:00 2001 From: prolls Date: Fri, 4 Sep 2026 12:43:25 +0900 Subject: [PATCH 6/6] fix(retry-fallback): authenticate exhaustion errors Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai --- packages/coding-agent/CHANGELOG.md | 3 ++- packages/coding-agent/docs/extensions.md | 3 +++ packages/coding-agent/src/core/agent-session.ts | 3 ++- packages/coding-agent/src/core/changes.md | 2 +- packages/coding-agent/src/core/extensions/changes.md | 5 +++-- packages/coding-agent/src/core/extensions/types.ts | 2 ++ .../test/suite/retry-fallback-exhaustion-lifecycle.test.ts | 7 +++++++ 7 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 813cc7f709..25a47e74be 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,7 +8,8 @@ - Extensions can subscribe to `retry_fallback_exhausted` to receive bounded, structured diagnostics when no configured fallback can hold the live - conversation, enabling fresh-context delegation without parsing TUI errors. + conversation, including a full-error SHA-256 correlation digest, enabling + fresh-context delegation without parsing TUI errors. - New `gpt-6-astra` prompt preset, written from scratch against the GPT-6 Astra prompting guide: every `gpt-6-astra` model id (bare, `-fast`, dated snapshots, provider-prefixed, Bedrock `openai.gpt-6-astra`, display name "GPT-6 Astra") now gets a full-core system prompt with an initiative section (bias to action, approval as the last step on a concrete result), explicit instruction precedence for skills and project files, an asynchronous-work section mapping Astra's async-tool training onto background sessions, monitors, child tasks, and detached eval cells (end the turn to wait; no wait tool), calibrated test-first verification, and an engineer-prose writing style with the guide's slop-phrase ban. `promptPreset: "gpt-6-astra"` forces it. ### Changed diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 022b42521d..3671fade8f 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -846,10 +846,13 @@ pi.on("retry_fallback_exhausted", (event, ctx) => { ``` The payload includes `sessionId`, `chainKey`, `from`, `lastError`, +`lastErrorSha256` (the full pre-truncation error digest), `exhaustionReason`, and `rejectedCandidates`. Diagnostics delivered to extensions are bounded to 64 KiB of serialized UTF-8: at most 8,192 bytes for the terminal error, 16 rejected candidates, 512 bytes per session or selector-bearing field, and 2,048 bytes per candidate error. +Consumers can correlate the bounded `lastError` to a persisted failed message +by hashing that message's complete error and comparing `lastErrorSha256`. The original session event remains available to TUI/RPC listeners with its existing `{ chainKey, lastError }` shape. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index de46b899e9..26d6614be2 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -13,7 +13,7 @@ * Modes use this class and add their own I/O layer on top. */ -import { randomUUID } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { rm } from "node:fs/promises"; import { basename, dirname } from "node:path"; @@ -7729,6 +7729,7 @@ export class AgentSession { MAX_FALLBACK_EXHAUSTION_SELECTOR_BYTES, ), lastError: truncateUtf8(lastError, MAX_FALLBACK_EXHAUSTION_ERROR_BYTES), + lastErrorSha256: createHash("sha256").update(lastError).digest("hex"), exhaustionReason: detail?.reason ?? "candidates-exhausted", rejectedCandidates, } satisfies RetryFallbackExhaustedEvent; diff --git a/packages/coding-agent/src/core/changes.md b/packages/coding-agent/src/core/changes.md index b05848b150..f6a644343e 100644 --- a/packages/coding-agent/src/core/changes.md +++ b/packages/coding-agent/src/core/changes.md @@ -6,7 +6,7 @@ - `packages/coding-agent/src/core/retry-fallback/controller.ts`: `RetryFallbackControllerDeps` gained the injected `isCandidateUsable` capacity preflight and the `classifySwitchFailure` seam. `nextCandidate` now skips a rung whose window cannot hold the live conversation (`context-unusable`) and keeps walking; `tryFallback` walks the remaining rungs when applying a model is refused on capacity grounds after `model_select` ran, and rethrows any failure the classifier does not recognize so one broken extension cannot spend the whole chain. A turn-scoped, selector-keyed rejection ledger backs the new `exhaustion` accessor (`chainKey`, `from`, `reason`, `rejectedCandidates`); `exhaustedChainKey` is unchanged. - `packages/coding-agent/src/core/agent-session.ts`: `_switchActiveModel` is now two-phase. Model, thinking level, service tier, and the server-side-fallback flag are applied provisionally so `model_select` handlers build against the target, but compaction invalidation, `model_changed`, `thinking_level_changed`/`thinking_level_select`, the service-tier event, the high-reasoning warning, `appendModelChange`, and the persisted default all wait until the post-`model_select` `assertModelUsable` clears. A rejected target is rolled back silently and extension-owned prompt/tool state is resynced by re-running `model_select` for the previous model. The seven duplicated exhaustion emits collapse into `_emitRetryFallbackExhausted`, which emits the unchanged session event plus the new extension event. -- `packages/coding-agent/src/core/extensions/types.ts`: new `RetryFallbackExhaustedEvent` (`sessionId`, `chainKey`, `from`, `lastError`, `exhaustionReason`, `rejectedCandidates`) in the `ExtensionEvent` union with a `pi.on("retry_fallback_exhausted", ...)` overload. It flows through the generic runner `emit` and returns no result: it is notification-only. +- `packages/coding-agent/src/core/extensions/types.ts`: new `RetryFallbackExhaustedEvent` (`sessionId`, `chainKey`, `from`, `lastError`, `lastErrorSha256`, `exhaustionReason`, `rejectedCandidates`) in the `ExtensionEvent` union with a `pi.on("retry_fallback_exhausted", ...)` overload. The digest correlates the bounded diagnostic to the complete failed error. It flows through the generic runner `emit` and returns no result: it is notification-only. - Review hardening keeps the two-phase transaction scoped to automatic fallback while manual, cycle, restore, and fallback-revert switches retain their established committed-first event order. Rejected automatic fallbacks directly restore prompt, base prompt, active tools, requested/withheld tool names, model, thinking, tier, and server-fallback state; best-effort extension resynchronization cannot overwrite that snapshot. Exhaustion notifications do not block retry settlement and bound extension diagnostics to 8,192 error characters and 16 candidates. - Persistence now defines the automatic-switch commit boundary: pre-commit failures roll back, while post-commit observer failures leave runtime state aligned with the durable `model_change`. `SessionManager` persists an entry before exposing it through in-memory indexes, so a filesystem failure cannot leave a phantom resident entry. - Post-commit switch and fallback-event observer failures are logged and isolated so controller bookkeeping still records the applied rung. The retry-handler ownership boundary converts any remaining internal failure into a terminal `not-handled` outcome and resolves the retry promise, preventing a failed persistence or extension path from wedging `prompt()`. diff --git a/packages/coding-agent/src/core/extensions/changes.md b/packages/coding-agent/src/core/extensions/changes.md index ff42b6fc82..faec5356e1 100644 --- a/packages/coding-agent/src/core/extensions/changes.md +++ b/packages/coding-agent/src/core/extensions/changes.md @@ -7,8 +7,9 @@ - `packages/coding-agent/src/core/extensions/types.ts` adds the notification-only `RetryFallbackExhaustedEvent` and the matching `pi.on("retry_fallback_exhausted", ...)` overload. The payload names the session, - active selector, exhausted chain, terminal provider error, exhaustion reason, and - the rejected candidate budget projections. The event type is re-exported from + active selector, exhausted chain, bounded terminal provider error plus its + full-error SHA-256 correlation digest, exhaustion reason, and the rejected + candidate budget projections. The event type is re-exported from both the extension barrel and the package root for extension authors. ### Why diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index f51748d15c..07abc49fb2 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -1173,6 +1173,8 @@ export interface RetryFallbackExhaustedEvent { from: string; /** Terminal provider error that ended the walk. */ lastError: string; + /** SHA-256 of the complete terminal error, before `lastError` byte bounding. */ + lastErrorSha256: string; exhaustionReason: FallbackExhaustionReason; rejectedCandidates: readonly FallbackRejectedCandidate[]; } diff --git a/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts b/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts index c9c627cc24..3440ec10d6 100644 --- a/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts +++ b/packages/coding-agent/test/suite/retry-fallback-exhaustion-lifecycle.test.ts @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { fauxAssistantMessage } from "@earendil-works/pi-ai/compat"; import { afterEach, describe, expect, it } from "vitest"; import { createHarness, type Harness } from "./harness.ts"; @@ -81,6 +82,12 @@ describe("retry fallback exhaustion lifecycle", () => { // then expect(harness.faux.getCallLog().map((call) => call.modelId)).toEqual(["faux-1"]); expect(extensionEvents).toHaveLength(1); + expect(extensionEvents).toMatchObject([ + { + lastError: "billing error: insufficient_quota", + lastErrorSha256: createHash("sha256").update("billing error: insufficient_quota").digest("hex"), + }, + ]); expect(harness.eventsOfType("retry_fallback_exhausted")).toMatchObject([ { chainKey: "faux/faux-1", lastError: "billing error: insufficient_quota" }, ]);