Address review feedback for fallback fixes

src/plugin/event.model-fallback.test.ts

@@ -6,15 +6,9 @@ import { _resetForTesting, setMainSession } from "../features/claude-code-sessio
 import { createModelFallbackHook, clearPendingModelFallback } from "../hooks/model-fallback/hook"
 
 describe("createEventHandler - model fallback", () => {
-  afterEach(() => {
-    _resetForTesting()
-  })
-
-  test("triggers retry prompt for assistant message.updated APIError payloads (headless resume)", async () => {
-    //#given
+  const createHandler = (args?: { hooks?: any }) => {
     const abortCalls: string[] = []
     const promptCalls: string[] = []
-    const sessionID = "ses_message_updated_fallback"
 
     const handler = createEventHandler({
       ctx: {
@@ -46,9 +40,21 @@ describe("createEventHandler - model fallback", () => {
           disconnectSession: async () => {},
         },
       } as any,
-      hooks: {} as any,
+      hooks: args?.hooks ?? ({} as any),
     })
 
+    return { handler, abortCalls, promptCalls }
+  }
+
+  afterEach(() => {
+    _resetForTesting()
+  })
+
+  test("triggers retry prompt for assistant message.updated APIError payloads (headless resume)", async () => {
+    //#given
+    const sessionID = "ses_message_updated_fallback"
+    const { handler, abortCalls, promptCalls } = createHandler()
+
     //#when
     await handler({
       event: {
@@ -87,43 +93,9 @@ describe("createEventHandler - model fallback", () => {
 
   test("triggers retry prompt for nested model error payloads", async () => {
     //#given
-    const abortCalls: string[] = []
-    const promptCalls: string[] = []
     const sessionID = "ses_main_fallback_nested"
     setMainSession(sessionID)
-
-    const handler = createEventHandler({
-      ctx: {
-        directory: "/tmp",
-        client: {
-          session: {
-            abort: async ({ path }: { path: { id: string } }) => {
-              abortCalls.push(path.id)
-              return {}
-            },
-            prompt: async ({ path }: { path: { id: string } }) => {
-              promptCalls.push(path.id)
-              return {}
-            },
-          },
-        },
-      } as any,
-      pluginConfig: {} as any,
-      firstMessageVariantGate: {
-        markSessionCreated: () => {},
-        clear: () => {},
-      },
-      managers: {
-        tmuxSessionManager: {
-          onSessionCreated: async () => {},
-          onSessionDeleted: async () => {},
-        },
-        skillMcpManager: {
-          disconnectSession: async () => {},
-        },
-      } as any,
-      hooks: {} as any,
-    })
+    const { handler, abortCalls, promptCalls } = createHandler()
 
     //#when
     await handler({
@@ -151,48 +123,13 @@ describe("createEventHandler - model fallback", () => {
 
   test("triggers retry prompt on session.status retry events and applies fallback", async () => {
     //#given
-    const abortCalls: string[] = []
-    const promptCalls: string[] = []
     const sessionID = "ses_status_retry_fallback"
     setMainSession(sessionID)
     clearPendingModelFallback(sessionID)
 
     const modelFallback = createModelFallbackHook()
 
-    const handler = createEventHandler({
-      ctx: {
-        directory: "/tmp",
-        client: {
-          session: {
-            abort: async ({ path }: { path: { id: string } }) => {
-              abortCalls.push(path.id)
-              return {}
-            },
-            prompt: async ({ path }: { path: { id: string } }) => {
-              promptCalls.push(path.id)
-              return {}
-            },
-          },
-        },
-      } as any,
-      pluginConfig: {} as any,
-      firstMessageVariantGate: {
-        markSessionCreated: () => {},
-        clear: () => {},
-      },
-      managers: {
-        tmuxSessionManager: {
-          onSessionCreated: async () => {},
-          onSessionDeleted: async () => {},
-        },
-        skillMcpManager: {
-          disconnectSession: async () => {},
-        },
-      } as any,
-      hooks: {
-        modelFallback,
-      } as any,
-    })
+    const { handler, abortCalls, promptCalls } = createHandler({ hooks: { modelFallback } })
 
     const chatMessageHandler = createChatMessageHandler({
       ctx: {

src/plugin/event.ts

@@ -13,7 +13,7 @@ import {
 import { resetMessageCursor } from "../shared"
 import { lspManager } from "../tools"
 import { shouldRetryError } from "../shared/model-error-classifier"
-import { clearPendingModelFallback, setPendingModelFallback } from "../hooks/model-fallback/hook"
+import { clearPendingModelFallback, clearSessionFallbackChain, setPendingModelFallback } from "../hooks/model-fallback/hook"
 import { clearSessionModel, setSessionModel } from "../shared/session-model-state"
 
 import type { CreatedHooks } from "../create-hooks"
@@ -135,11 +135,11 @@ export function createEventHandler(args: {
   const DEDUP_WINDOW_MS = 500
 
   const shouldAutoRetrySession = (sessionID: string): boolean => {
+    if (syncSubagentSessions.has(sessionID)) return true
     const mainSessionID = getMainSessionID()
     if (mainSessionID) return sessionID === mainSessionID
     // Headless runs (or resumed sessions) may not emit session.created, so mainSessionID can be unset.
     // In that case, treat any non-subagent session as the "main" interactive session.
-    if (syncSubagentSessions.has(sessionID)) return true
     return !subagentSessions.has(sessionID)
   }
 
@@ -213,6 +213,7 @@ export function createEventHandler(args: {
         lastHandledRetryStatusKey.delete(sessionInfo.id)
         lastKnownModelBySession.delete(sessionInfo.id)
         clearPendingModelFallback(sessionInfo.id)
+        clearSessionFallbackChain(sessionInfo.id)
         resetMessageCursor(sessionInfo.id)
         firstMessageVariantGate.clear(sessionInfo.id)
         clearSessionModel(sessionInfo.id)

src/plugin/hooks/create-session-hooks.ts

@@ -105,6 +105,8 @@ export function createSessionHooks(args: {
     ? safeHook("think-mode", () => createThinkModeHook())
     : null
 
+  const enableFallbackTitle = pluginConfig.experimental?.model_fallback_title ?? false
+  const fallbackTitleMaxEntries = 200
   const fallbackTitleState = new Map<string, { baseTitle?: string; lastKey?: string }>()
   const updateFallbackTitle = async (input: {
     sessionID: string
@@ -112,6 +114,7 @@ export function createSessionHooks(args: {
     modelID: string
     variant?: string
   }) => {
+    if (!enableFallbackTitle) return
     const key = `${input.providerID}/${input.modelID}${input.variant ? `:${input.variant}` : ""}`
     const existing = fallbackTitleState.get(input.sessionID) ?? {}
     if (existing.lastKey === key) return
@@ -142,26 +145,32 @@ export function createSessionHooks(args: {
 
     existing.lastKey = key
     fallbackTitleState.set(input.sessionID, existing)
+    if (fallbackTitleState.size > fallbackTitleMaxEntries) {
+      const oldestKey = fallbackTitleState.keys().next().value
+      if (oldestKey) fallbackTitleState.delete(oldestKey)
+    }
   }
 
-  // Model fallback hook - always enabled (no feature flag)
+  // Model fallback hook (configurable via disabled_hooks)
   // This handles automatic model switching when model errors occur
-  const modelFallback = safeHook("model-fallback", () =>
-    createModelFallbackHook({
-      toast: async ({ title, message, variant, duration }) => {
-        await ctx.client.tui
-          .showToast({
-            body: {
-              title,
-              message,
-              variant: variant ?? "warning",
-              duration: duration ?? 5000,
-            },
-          })
-          .catch(() => {})
-      },
-      onApplied: updateFallbackTitle,
-    }))
+  const modelFallback = isHookEnabled("model-fallback")
+    ? safeHook("model-fallback", () =>
+      createModelFallbackHook({
+        toast: async ({ title, message, variant, duration }) => {
+          await ctx.client.tui
+            .showToast({
+              body: {
+                title,
+                message,
+                variant: variant ?? "warning",
+                duration: duration ?? 5000,
+              },
+            })
+            .catch(() => {})
+        },
+        onApplied: enableFallbackTitle ? updateFallbackTitle : undefined,
+      }))
+    : null
 
   const anthropicContextWindowLimitRecovery = isHookEnabled("anthropic-context-window-limit-recovery")
     ? safeHook("anthropic-context-window-limit-recovery", () =>