diff --git a/packages/coding-agent/src/modes/components/agent-hub-renderer.ts b/packages/coding-agent/src/modes/components/agent-hub-renderer.ts index a97e7caf3..4e345b09f 100644 --- a/packages/coding-agent/src/modes/components/agent-hub-renderer.ts +++ b/packages/coding-agent/src/modes/components/agent-hub-renderer.ts @@ -109,11 +109,7 @@ export function modelBadge(ref: AgentRef, observed: ObservableSession | undefine const fallbackSelector = (serving?.isFallback ? serving.selector : undefined) ?? (progress?.resolvedModelIsFallback ? progress.resolvedModel : undefined) ?? - (ref.history?.resolvedModelIsFallback ? ref.history.resolvedModel : undefined) ?? - // Nothing has served at all, so there is no earlier work to miscredit and - // the armed candidate is all there is to report — but it is still a - // fallback, and a bare model badge would hide that. - (serving ? undefined : ref.session?.pendingRetryFallbackModel); + (ref.history?.resolvedModelIsFallback ? ref.history.resolvedModel : undefined); if (fallbackSelector) { return `${theme.fg("warning", "fallback →")} ${formatResolvedModelBadge(fallbackSelector, true, liveThinkingLevel)}`; } diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 3a8650256..827bdc8e0 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -4003,11 +4003,6 @@ export class AgentSession { return this.#recovery.servingModel; } - /** Selector of a fallback that is armed but has not produced a turn yet. */ - get pendingRetryFallbackModel(): string | undefined { - return this.#recovery.pendingRetryFallbackModel; - } - /** Install the interactive decision surface for reserve-triggered model changes. */ setUsageFallbackConfirmer(confirmer: UsageFallbackConfirmer | undefined): void { this.#usageFallbackConfirmer = confirmer; diff --git a/packages/coding-agent/src/session/turn-recovery.ts b/packages/coding-agent/src/session/turn-recovery.ts index 0049f243a..0cc0fba4b 100644 --- a/packages/coding-agent/src/session/turn-recovery.ts +++ b/packages/coding-agent/src/session/turn-recovery.ts @@ -189,14 +189,20 @@ export class TurnRecovery { #emptyStopRetryCount = 0; #unexpectedStopRetryCount = 0; #acceptTerminalEmptyStopForPrompt = false; + // Three fields sit near the word "serve" and are deliberately distinct: + // `#activeRetryFallback.served` gates the one-shot `retry_fallback_succeeded` + // event for the current arm, `#fallbackRouted` says how the CURRENT model was + // reached, and `#lastServed` is the session's attribution. A fallback flipping + // to served does not by itself move attribution — only a settled turn does. /** * Attribution of the newest turn that produced output, tagged with the - * transcript it belongs to. Anchoring rather than resetting follows - * `#ensurePersistedMessageKeys`: switching sessions in place changes the - * session file, so the stale attribution drops itself and no mutation call - * site has to remember to clear it. + * session it belongs to. Anchoring rather than resetting follows + * `#ensurePersistedMessageKeys`: every real switch mints a new session id, so + * stale attribution drops itself and no mutation call site has to remember to + * clear it. The id — not the file — is the anchor because an unpersisted + * session has no file, and comparing two `undefined`s would never invalidate. */ - #lastServed: { attribution: ServingModel; sessionFile: string | undefined } | undefined; + #lastServed: { attribution: ServingModel; sessionId: string } | undefined; /** * Whether the current model was reached by fallback routing rather than by * the configured primary. Tracked separately from {@link #activeRetryFallback} @@ -245,7 +251,7 @@ export class TurnRecovery { */ get servingModel(): ServingModel | undefined { const served = this.#lastServed; - if (served && served.sessionFile === this.#host.sessionManager.getSessionFile()) return served.attribution; + if (served && served.sessionId === this.#host.sessionManager.getSessionId()) return served.attribution; const model = this.#host.model(); if (!model) return undefined; // Polled per streaming event and per render, so the pre-first-turn window @@ -263,19 +269,6 @@ export class TurnRecovery { return value; } - /** - * Selector of a fallback that is armed but has not served yet. - * - * Nothing has been attributed to it, so it is not the serving model — but - * when the session has produced no output at all there is no earlier work to - * miscredit, and naming it is the only honest thing an observer can show. - */ - get pendingRetryFallbackModel(): string | undefined { - const model = this.#host.model(); - if (!model || !this.#activeRetryFallback || this.#activeRetryFallback.served) return undefined; - return formatRetryFallbackSelector(model, this.#host.thinkingLevel()); - } - /** Resets per-prompt recovery counters and terminal-stop acceptance. */ resetForNewPrompt(): void { this.#emptyStopRetryCount = 0; @@ -304,7 +297,7 @@ export class TurnRecovery { selector: formatRetryFallbackSelector(model, this.#host.thinkingLevel()), isFallback: this.#fallbackRouted, }, - sessionFile: this.#host.sessionManager.getSessionFile(), + sessionId: this.#host.sessionManager.getSessionId(), }; } // Independent of the retry saga below: a usage-aware fallback is applied @@ -1531,7 +1524,12 @@ export class TurnRecovery { } = this.#activeRetryFallback; const originalSelector = parseRetryFallbackSelector(originalSelectorRaw, this.#host.modelRegistry); if (!originalSelector) { - this.clearActiveRetryFallback(); + // Defensive: the stored selector is always produced by + // `formatRetryFallbackSelector`, so it should never fail to parse. If it + // somehow does, nothing is restored and the session keeps running on the + // fallback — so drop the chain record but NOT `#fallbackRouted`, whose + // clearing would report the fallback's remaining turns as the primary. + this.#activeRetryFallback = undefined; return false; } diff --git a/packages/coding-agent/test/agent-hub-ordering.test.ts b/packages/coding-agent/test/agent-hub-ordering.test.ts index df029726e..6aea29399 100644 --- a/packages/coding-agent/test/agent-hub-ordering.test.ts +++ b/packages/coding-agent/test/agent-hub-ordering.test.ts @@ -477,8 +477,9 @@ describe("Agent hub row ordering", () => { const session = { model: { id: "gpt-5.6-sol", thinking: true }, thinkingLevel: "high", - servingModel: undefined, - pendingRetryFallbackModel: "openai-codex/gpt-5.6-sol", + // Nothing served, so the session names what it currently points at — + // still flagged as fallback-routed. + servingModel: { selector: "openai-codex/gpt-5.6-sol", isFallback: true }, } as unknown as AgentSession; agents.register({ id: "UnprovenAgent", displayName: "Unproven Agent", kind: "sub", session }); diff --git a/packages/coding-agent/test/agent-session-retry-fallback.test.ts b/packages/coding-agent/test/agent-session-retry-fallback.test.ts index 191a17af7..29ff8297b 100644 --- a/packages/coding-agent/test/agent-session-retry-fallback.test.ts +++ b/packages/coding-agent/test/agent-session-retry-fallback.test.ts @@ -4241,10 +4241,11 @@ describe("AgentSession retry fallback", () => { selector: `${primaryModel.provider}/${primaryModel.id}`, isFallback: false, }); - // Attribution belongs to the transcript it was earned in. Switching the - // session in place drops it rather than carrying another session's model - // across, leaving only what this session currently points at. - vi.spyOn(session.sessionManager, "getSessionFile").mockReturnValue("/tmp/some-other-session.jsonl"); + // Attribution belongs to the session it was earned in. Every real switch + // mints a new session id — including for an unpersisted session, which has + // no file to compare — so the stale value drops itself, leaving only what + // this session currently points at. + vi.spyOn(session.sessionManager, "getSessionId").mockReturnValue("some-other-session"); expect(session.servingModel).toEqual({ selector: `${fallbackModel.provider}/${fallbackModel.id}`, isFallback: true, @@ -4274,6 +4275,11 @@ describe("AgentSession retry fallback", () => { modelRegistry, }); + // Observers poll this per streaming event and per render. Before anything + // has served the answer is computed rather than stored, so that is the + // window where a fresh allocation per call would show up. + expect(session.servingModel).toBe(session.servingModel); + await session.prompt("Fail over to a working fallback"); await session.waitForIdle(); @@ -4282,9 +4288,6 @@ describe("AgentSession retry fallback", () => { selector: `${fallbackModel.provider}/${fallbackModel.id}`, isFallback: true, }); - // Observers poll this per streaming event and per render, so a steady - // session must not allocate a fresh answer each time. - expect(session.servingModel).toBe(session.servingModel); }); it("keeps attribution on a served fallback while the next candidate is unproven", async () => { diff --git a/packages/coding-agent/test/task/executor-prewalk.test.ts b/packages/coding-agent/test/task/executor-prewalk.test.ts index 6f72d4830..998c35935 100644 --- a/packages/coding-agent/test/task/executor-prewalk.test.ts +++ b/packages/coding-agent/test/task/executor-prewalk.test.ts @@ -30,10 +30,15 @@ function yieldEmittingSession( ): AgentSession { const listeners: Array<(event: AgentSessionEvent) => void> = []; let activeTools = initialTools; + // `servingModel` mirrors the real session: attribution names the model that + // produced output, so a prewalk hand-off moves it along with `model`. + const serving = (model: Model | undefined): { selector: string; isFallback: boolean } | undefined => + model ? { selector: `${model.provider}/${model.id}`, isFallback: false } : undefined; const session = { state: { messages: [] }, agent: { state: { systemPrompt: ["test"] } }, model: modelSwitch?.from, + servingModel: serving(modelSwitch?.from), extensionRunner: undefined, sessionManager: { appendSessionInit: () => {} }, getActiveToolNames: () => activeTools, @@ -52,6 +57,7 @@ function yieldEmittingSession( prompt: async (_text: string, _options?: PromptOptions) => { if (modelSwitch) { session.model = modelSwitch.to; + session.servingModel = serving(modelSwitch.to); for (const listener of listeners) { listener({ type: "notice", level: "info", message: "Prewalk switched", source: "prewalk" }); }