Address review feedback for fallback fixes

This commit is contained in:
VespianRex 2026-02-20 00:02:17 +02:00
parent f5f1d1d4c2
commit bf51919a79
11 changed files with 145 additions and 156 deletions

View File

@ -3011,6 +3011,9 @@
},
"disable_omo_env": {
"type": "boolean"
},
"model_fallback_title": {
"type": "boolean"
}
},
"additionalProperties": false

View File

@ -17,6 +17,10 @@ export const ExperimentalConfigSchema = z.object({
safe_hook_creation: z.boolean().optional(),
/** Disable auto-injected <omo-env> context in prompts (experimental) */
disable_omo_env: z.boolean().optional(),
/** Enable hashline_edit tool for improved file editing with hash-based line anchors */
hashline_edit: z.boolean().optional(),
/** Append fallback model info to session title when a runtime fallback occurs (default: false) */
model_fallback_title: z.boolean().optional(),
})
export type ExperimentalConfig = z.infer<typeof ExperimentalConfigSchema>

View File

@ -2920,6 +2920,39 @@ describe("BackgroundManager.handleEvent - session.deleted cascade", () => {
})
describe("BackgroundManager.handleEvent - session.error", () => {
const defaultRetryFallbackChain = [
{ providers: ["quotio"], model: "claude-opus-4-6", variant: "max" },
{ providers: ["quotio"], model: "gpt-5.3-codex", variant: "high" },
]
const stubProcessKey = (manager: BackgroundManager) => {
;(manager as unknown as { processKey: (key: string) => Promise<void> }).processKey = async () => {}
}
const createRetryTask = (manager: BackgroundManager, input: {
id: string
sessionID: string
description: string
concurrencyKey?: string
fallbackChain?: typeof defaultRetryFallbackChain
}) => {
const task = createMockTask({
id: input.id,
sessionID: input.sessionID,
parentSessionID: "parent-session",
parentMessageID: "msg-retry",
description: input.description,
agent: "sisyphus",
status: "running",
concurrencyKey: input.concurrencyKey,
model: { providerID: "quotio", modelID: "claude-opus-4-6-thinking" },
fallbackChain: input.fallbackChain ?? defaultRetryFallbackChain,
attemptCount: 0,
})
getTaskMap(manager).set(task.id, task)
return task
}
test("sets task to error, releases concurrency, and cleans up", async () => {
//#given
const manager = createBackgroundManager()
@ -3054,26 +3087,19 @@ describe("BackgroundManager.handleEvent - session.error", () => {
const concurrencyKey = "quotio/claude-opus-4-6-thinking"
await concurrencyManager.acquire(concurrencyKey)
;(manager as unknown as { processKey: (key: string) => Promise<void> }).processKey = async () => {}
stubProcessKey(manager)
const sessionID = "ses_error_retry"
const task = createMockTask({
const task = createRetryTask(manager, {
id: "task-session-error-retry",
sessionID,
parentSessionID: "parent-session",
parentMessageID: "msg-retry",
description: "task that should retry",
agent: "sisyphus",
status: "running",
concurrencyKey,
model: { providerID: "quotio", modelID: "claude-opus-4-6-thinking" },
fallbackChain: [
{ providers: ["quotio"], model: "claude-opus-4-6", variant: "max" },
{ providers: ["quotio"], model: "claude-opus-4-5" },
],
attemptCount: 0,
})
getTaskMap(manager).set(task.id, task)
//#when
manager.handleEvent({
@ -3107,25 +3133,14 @@ describe("BackgroundManager.handleEvent - session.error", () => {
test("retry path triggers on session.status retry events", async () => {
//#given
const manager = createBackgroundManager()
;(manager as unknown as { processKey: (key: string) => Promise<void> }).processKey = async () => {}
stubProcessKey(manager)
const sessionID = "ses_status_retry"
const task = createMockTask({
const task = createRetryTask(manager, {
id: "task-status-retry",
sessionID,
parentSessionID: "parent-session",
parentMessageID: "msg-status",
description: "task that should retry on status",
agent: "sisyphus",
status: "running",
model: { providerID: "quotio", modelID: "claude-opus-4-6-thinking" },
fallbackChain: [
{ providers: ["quotio"], model: "claude-opus-4-6", variant: "max" },
{ providers: ["quotio"], model: "gpt-5.3-codex", variant: "high" },
],
attemptCount: 0,
})
getTaskMap(manager).set(task.id, task)
//#when
manager.handleEvent({
@ -3154,25 +3169,14 @@ describe("BackgroundManager.handleEvent - session.error", () => {
test("retry path triggers on message.updated assistant error events", async () => {
//#given
const manager = createBackgroundManager()
;(manager as unknown as { processKey: (key: string) => Promise<void> }).processKey = async () => {}
stubProcessKey(manager)
const sessionID = "ses_message_updated_retry"
const task = createMockTask({
const task = createRetryTask(manager, {
id: "task-message-updated-retry",
sessionID,
parentSessionID: "parent-session",
parentMessageID: "msg-message-updated",
description: "task that should retry on message.updated",
agent: "sisyphus",
status: "running",
model: { providerID: "quotio", modelID: "claude-opus-4-6-thinking" },
fallbackChain: [
{ providers: ["quotio"], model: "claude-opus-4-6", variant: "max" },
{ providers: ["quotio"], model: "gpt-5.3-codex", variant: "high" },
],
attemptCount: 0,
})
getTaskMap(manager).set(task.id, task)
//#when
manager.handleEvent({

View File

@ -36,6 +36,20 @@ export type ModelFallbackState = {
*/
const pendingModelFallbacks = new Map<string, ModelFallbackState>()
const lastToastKey = new Map<string, string>()
const sessionFallbackChains = new Map<string, FallbackEntry[]>()
export function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void {
if (!sessionID) return
if (!fallbackChain || fallbackChain.length === 0) {
sessionFallbackChains.delete(sessionID)
return
}
sessionFallbackChains.set(sessionID, fallbackChain)
}
export function clearSessionFallbackChain(sessionID: string): void {
sessionFallbackChains.delete(sessionID)
}
/**
* Sets a pending model fallback for a session.
@ -49,12 +63,16 @@ export function setPendingModelFallback(
): boolean {
const agentKey = getAgentConfigKey(agentName)
const requirements = AGENT_MODEL_REQUIREMENTS[agentKey]
if (!requirements || !requirements.fallbackChain || requirements.fallbackChain.length === 0) {
const sessionFallback = sessionFallbackChains.get(sessionID)
const fallbackChain = sessionFallback && sessionFallback.length > 0
? sessionFallback
: requirements?.fallbackChain
if (!fallbackChain || fallbackChain.length === 0) {
log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")")
return false
}
const fallbackChain = requirements.fallbackChain
const existing = pendingModelFallbacks.get(sessionID)
if (existing) {

View File

@ -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: {

View File

@ -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)

View File

@ -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", () =>

View File

@ -20,6 +20,18 @@ function fb(providers: string[] | string, model: string, variant?: string): Fall
}
}
function dedupeChain(chain: FallbackEntry[]): FallbackEntry[] {
const seen = new Set<string>()
const result: FallbackEntry[] = []
for (const entry of chain) {
const key = `${entry.model}:${entry.variant ?? ""}`
if (seen.has(key)) continue
seen.add(key)
result.push(entry)
}
return result
}
// Provider preference rules:
// - Never use the paid `opencode` provider as an automatic fallback.
// - Prefer `quotio` when the same model exists across multiple providers.
@ -82,12 +94,12 @@ export const AGENT_MODEL_REQUIREMENTS: Record<string, ModelRequirement> = {
requiresAnyModel: true,
},
oracle: {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "gpt-5.3-codex", "high"),
fb("quotio", "claude-opus-4-6-thinking"),
fb("quotio", "claude-sonnet-4-5-thinking"),
...QUALITY_CODING_CHAIN,
],
]),
},
librarian: {
fallbackChain: [
@ -111,35 +123,35 @@ export const AGENT_MODEL_REQUIREMENTS: Record<string, ModelRequirement> = {
],
},
prometheus: {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "claude-opus-4-6-thinking"),
fb("quotio", "gpt-5.3-codex", "high"),
fb("quotio", "claude-sonnet-4-5-thinking"),
...QUALITY_CODING_CHAIN,
],
]),
},
metis: {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "claude-opus-4-6-thinking"),
fb("quotio", "gpt-5.3-codex", "high"),
fb("quotio", "claude-sonnet-4-5-thinking"),
...QUALITY_CODING_CHAIN,
],
]),
},
momus: {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "gpt-5.3-codex", "high"),
fb("quotio", "claude-opus-4-6-thinking"),
...QUALITY_CODING_CHAIN,
],
]),
},
atlas: {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "claude-sonnet-4-5-thinking"),
fb("quotio", "claude-opus-4-6-thinking"),
fb("quotio", "gpt-5.3-codex", "medium"),
...QUALITY_CODING_CHAIN,
],
]),
},
}
@ -155,13 +167,10 @@ export const CATEGORY_MODEL_REQUIREMENTS: Record<string, ModelRequirement> = {
],
},
ultrabrain: {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "gpt-5.3-codex", "high"),
fb("quotio", "claude-opus-4-6-thinking"),
fb("nvidia", "stepfun-ai/step-3.5-flash"),
fb("nvidia", "qwen/qwen3.5-397b-a17b"),
...QUALITY_CODING_CHAIN,
],
]),
},
deep: {
fallbackChain: [
@ -187,11 +196,11 @@ export const CATEGORY_MODEL_REQUIREMENTS: Record<string, ModelRequirement> = {
fallbackChain: SPEED_CHAIN,
},
"unspecified-high": {
fallbackChain: [
fallbackChain: dedupeChain([
fb("quotio", "claude-opus-4-6-thinking"),
fb("quotio", "gpt-5.3-codex", "high"),
...QUALITY_CODING_CHAIN,
],
]),
},
writing: {
fallbackChain: [

View File

@ -100,7 +100,7 @@ describe("executeSyncTask - cleanup on error paths", () => {
//#when - executeSyncTask with fetchSyncResult failing
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
sessionID: "parent-session",
}, "test-agent", undefined, undefined, undefined, deps)
}, "test-agent", undefined, undefined, undefined, undefined, deps)
//#then - should return error and cleanup resources
expect(result).toBe("Fetch failed")
@ -150,7 +150,7 @@ describe("executeSyncTask - cleanup on error paths", () => {
//#when - executeSyncTask with pollSyncSession failing
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
sessionID: "parent-session",
}, "test-agent", undefined, undefined, undefined, deps)
}, "test-agent", undefined, undefined, undefined, undefined, deps)
//#then - should return error and cleanup resources
expect(result).toBe("Poll error")
@ -200,7 +200,7 @@ describe("executeSyncTask - cleanup on error paths", () => {
//#when - executeSyncTask completes successfully
const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, {
sessionID: "parent-session",
}, "test-agent", undefined, undefined, undefined, deps)
}, "test-agent", undefined, undefined, undefined, undefined, deps)
//#then - should complete and cleanup resources
expect(result).toContain("Task completed")

View File

@ -8,6 +8,7 @@ import { log } from "../../shared/logger"
import { formatDuration } from "./time-formatter"
import { formatDetailedError } from "./error-formatting"
import { syncTaskDeps, type SyncTaskDeps } from "./sync-task-deps"
import { setSessionFallbackChain, clearSessionFallbackChain } from "../../hooks/model-fallback/hook"
export async function executeSyncTask(
args: DelegateTaskArgs,
@ -18,6 +19,7 @@ export async function executeSyncTask(
categoryModel: { providerID: string; modelID: string; variant?: string } | undefined,
systemContent: string | undefined,
modelInfo?: ModelFallbackInfo,
fallbackChain?: import("../../shared/model-requirements").FallbackEntry[],
deps: SyncTaskDeps = syncTaskDeps
): Promise<string> {
const { client, directory, onSyncSessionCreated } = executorCtx
@ -42,6 +44,7 @@ export async function executeSyncTask(
subagentSessions.add(sessionID)
syncSubagentSessions.add(sessionID)
setSessionAgent(sessionID, agentToUse)
setSessionFallbackChain(sessionID, fallbackChain)
if (onSyncSessionCreated) {
log("[task] Invoking onSyncSessionCreated callback", { sessionID, parentID: parentContext.sessionID })
@ -149,6 +152,7 @@ session_id: ${sessionID}
if (syncSessionID) {
subagentSessions.delete(syncSessionID)
syncSubagentSessions.delete(syncSessionID)
clearSessionFallbackChain(syncSessionID)
}
}
}

View File

@ -223,7 +223,7 @@ Prompts MUST be in English.`
return executeBackgroundTask(args, ctx, options, parentContext, agentToUse, categoryModel, systemContent, fallbackChain)
}
return executeSyncTask(args, ctx, options, parentContext, agentToUse, categoryModel, systemContent, modelInfo)
return executeSyncTask(args, ctx, options, parentContext, agentToUse, categoryModel, systemContent, modelInfo, fallbackChain)
},
})
}