refactor(session): fold the pending-fallback surface into servingModel

Final review found `pendingRetryFallbackModel` unreachable. `servingModel`
returns `undefined` only when the session has no model at all, and the
pending getter required one, so the badge term guarding on it could never
fire. Its case — a fallback armed before anything has served — is already
answered by `servingModel`'s bootstrap, which names the current model and
flags it as fallback-routed. Removed, the same duplicate-surface cleanup
that removed `retryFallbackModel`.

Attribution now anchors on the session id rather than the session file. An
unpersisted session has no file, so two `undefined`s compared equal and
stale attribution survived `/new` and branch switches there; every real
switch mints a new id, persisted or not.

The cooldown-expiry restore keeps `#fallbackRouted` when the stored primary
selector cannot be parsed. Nothing is restored on that path, so the session
is still running on the fallback and its remaining turns are still fallback
work; clearing the flag reported them as the configured primary.

`executor-prewalk`'s fake session predates this work and never set
`servingModel`, so the prewalk hand-off stopped advancing the reported
model once the executor began reading attribution from the session. It now
mirrors the hand-off the way the other executor fixtures do.
This commit is contained in:
enieuwy
2026-08-08 11:47:07 +08:00
parent a06a7f5ef1
commit a1e60c3450
6 changed files with 39 additions and 40 deletions
@@ -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)}`;
}
@@ -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;
@@ -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;
}
@@ -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 });
@@ -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 () => {
@@ -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" });
}