From 5e4102566cd7a53c3cc7fed49ee2a85e90211595 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 02:35:46 +0900 Subject: [PATCH] refactor(model-fallback): fully encapsulate session state in factory closure Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/create-hooks.ts | 4 + src/create-managers.ts | 5 + src/create-tools.ts | 2 +- src/hooks/index.ts | 8 +- .../model-fallback/controller-accessor.ts | 30 +++++ src/hooks/model-fallback/hook.test.ts | 77 +++++++----- src/hooks/model-fallback/hook.ts | 117 ++++++++++++------ src/hooks/model-fallback/index.ts | 2 + src/index.ts | 1 + src/plugin/event.model-fallback-2941.test.ts | 21 ++-- src/plugin/event.model-fallback.test.ts | 13 +- src/plugin/event.test.ts | 3 +- src/plugin/event.ts | 31 +++-- .../fallback.cliproxyapi-matrix.test.ts | 2 - src/plugin/hooks/create-core-hooks.ts | 5 +- src/plugin/hooks/create-session-hooks.ts | 5 +- src/plugin/tool-registry.ts | 4 +- src/tools/call-omo-agent/sync-executor.ts | 9 +- src/tools/call-omo-agent/tools.ts | 39 +++++- src/tools/delegate-task/background-task.ts | 7 +- src/tools/delegate-task/executor-types.ts | 2 + src/tools/delegate-task/sync-task.ts | 5 +- src/tools/delegate-task/types.ts | 2 + 23 files changed, 271 insertions(+), 123 deletions(-) create mode 100644 src/hooks/model-fallback/controller-accessor.ts create mode 100644 src/hooks/model-fallback/index.ts diff --git a/src/create-hooks.ts b/src/create-hooks.ts index 0e40ad480..436f8e2b9 100644 --- a/src/create-hooks.ts +++ b/src/create-hooks.ts @@ -2,6 +2,7 @@ import type { AvailableSkill } from "./agents/dynamic-agent-prompt-builder" import type { HookName, OhMyOpenCodeConfig } from "./config" import type { LoadedSkill } from "./features/opencode-skill-loader/types" import type { BackgroundManager } from "./features/background-agent" +import type { ModelFallbackControllerAccessor } from "./hooks/model-fallback" import type { PluginContext } from "./plugin/types" import type { ModelCacheState } from "./plugin-state" @@ -36,6 +37,7 @@ export function createHooks(args: { pluginConfig: OhMyOpenCodeConfig modelCacheState: ModelCacheState backgroundManager: BackgroundManager + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor isHookEnabled: (hookName: HookName) => boolean safeHookEnabled: boolean mergedSkills: LoadedSkill[] @@ -46,6 +48,7 @@ export function createHooks(args: { pluginConfig, modelCacheState, backgroundManager, + modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, mergedSkills, @@ -56,6 +59,7 @@ export function createHooks(args: { ctx, pluginConfig, modelCacheState, + modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, }) diff --git a/src/create-managers.ts b/src/create-managers.ts index d40896343..9c0013fd4 100644 --- a/src/create-managers.ts +++ b/src/create-managers.ts @@ -5,6 +5,7 @@ import type { PluginContext, TmuxConfig } from "./plugin/types" import type { SubagentSessionCreatedEvent } from "./features/background-agent" import { BackgroundManager } from "./features/background-agent" import { SkillMcpManager } from "./features/skill-mcp-manager" +import { createModelFallbackControllerAccessor } from "./hooks/model-fallback" import { initTaskToastManager } from "./features/task-toast-manager" import { TmuxSessionManager } from "./features/tmux-subagent" import * as openclawRuntimeDispatch from "./openclaw/runtime-dispatch" @@ -12,6 +13,7 @@ import { registerManagerForCleanup } from "./features/background-agent/process-c import { createConfigHandler } from "./plugin-handlers" import { log } from "./shared" import { markServerRunningInProcess } from "./shared/tmux/tmux-utils/server-health" +import type { ModelFallbackControllerAccessor } from "./hooks/model-fallback" type CreateManagersDeps = { BackgroundManagerClass: typeof BackgroundManager @@ -38,6 +40,7 @@ export type Managers = { backgroundManager: BackgroundManager skillMcpManager: SkillMcpManager configHandler: ReturnType + modelFallbackControllerAccessor: ModelFallbackControllerAccessor } export function createManagers(args: { @@ -119,11 +122,13 @@ export function createManagers(args: { pluginConfig, modelCacheState, }) + const modelFallbackControllerAccessor = createModelFallbackControllerAccessor() return { tmuxSessionManager, backgroundManager, skillMcpManager, configHandler, + modelFallbackControllerAccessor, } } diff --git a/src/create-tools.ts b/src/create-tools.ts index 5ac5a7e2f..6a9bc3941 100644 --- a/src/create-tools.ts +++ b/src/create-tools.ts @@ -22,7 +22,7 @@ type CreateToolsResult = { export async function createTools(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig - managers: Pick + managers: Pick }): Promise { const { ctx, pluginConfig, managers } = args diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 051cbd12a..8fd15af2f 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -14,7 +14,13 @@ export { createEmptyTaskResponseDetectorHook } from "./empty-task-response-detec export { createAnthropicContextWindowLimitRecoveryHook, type AnthropicContextWindowLimitRecoveryOptions } from "./anthropic-context-window-limit-recovery"; export { createThinkModeHook } from "./think-mode"; -export { createModelFallbackHook, setPendingModelFallback, clearPendingModelFallback, type ModelFallbackState } from "./model-fallback/hook"; +export { + createModelFallbackHook, + setPendingModelFallback, + clearPendingModelFallback, + type ModelFallbackHook, + type ModelFallbackState, +} from "./model-fallback/hook"; export { createClaudeCodeHooksHook } from "./claude-code-hooks"; export { createRulesInjectorHook } from "./rules-injector"; export { createBackgroundNotificationHook } from "./background-notification" diff --git a/src/hooks/model-fallback/controller-accessor.ts b/src/hooks/model-fallback/controller-accessor.ts new file mode 100644 index 000000000..281ae9931 --- /dev/null +++ b/src/hooks/model-fallback/controller-accessor.ts @@ -0,0 +1,30 @@ +import type { FallbackEntry } from "../../shared/model-requirements" +import type { ModelFallbackStateController } from "./fallback-state-controller" + +export type ModelFallbackControllerAccessor = { + register: (controller: ModelFallbackStateController) => void + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + clearSessionFallbackChain: (sessionID: string) => void +} + +export function createModelFallbackControllerAccessor(): ModelFallbackControllerAccessor { + let controller: ModelFallbackStateController | null = null + + function register(nextController: ModelFallbackStateController): void { + controller = nextController + } + + function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { + controller?.setSessionFallbackChain(sessionID, fallbackChain) + } + + function clearSessionFallbackChain(sessionID: string): void { + controller?.clearSessionFallbackChain(sessionID) + } + + return { + register, + setSessionFallbackChain, + clearSessionFallbackChain, + } +} diff --git a/src/hooks/model-fallback/hook.test.ts b/src/hooks/model-fallback/hook.test.ts index 14b3ff6bb..de9e66fd7 100644 --- a/src/hooks/model-fallback/hook.test.ts +++ b/src/hooks/model-fallback/hook.test.ts @@ -70,22 +70,23 @@ const { setPendingModelFallback, } = await importFreshModelFallbackHookModule() +type ModelFallbackHook = ReturnType + describe("model fallback hook", () => { + let modelFallback: ModelFallbackHook + beforeEach(() => { + modelFallback = createModelFallbackHook() readConnectedProvidersCacheMock.mockReturnValue(null) readProviderModelsCacheMock.mockReturnValue(null) readConnectedProvidersCacheMock.mockClear() readProviderModelsCacheMock.mockClear() selectFallbackProviderMock.mockClear() - - clearPendingModelFallback("ses_model_fallback_main") - clearPendingModelFallback("ses_model_fallback_ghcp") - clearPendingModelFallback("ses_model_fallback_google") }) test("applies pending fallback on chat.message by overriding model", async () => { //#given - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -93,6 +94,7 @@ describe("model fallback hook", () => { } const set = setPendingModelFallback( + modelFallback, "ses_model_fallback_main", "Sisyphus - Ultraworker", "anthropic", @@ -123,7 +125,7 @@ describe("model fallback hook", () => { test("preserves fallback progression across repeated session.error retries", async () => { //#given - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -132,7 +134,7 @@ describe("model fallback hook", () => { const sessionID = "ses_model_fallback_main" expect( - setPendingModelFallback(sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7-thinking"), + setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7-thinking"), ).toBe(true) const firstOutput = { @@ -154,7 +156,7 @@ describe("model fallback hook", () => { //#when - second error re-arms fallback and should advance to next entry expect( - setPendingModelFallback(sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"), + setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"), ).toBe(true) const secondOutput = { @@ -176,16 +178,18 @@ describe("model fallback hook", () => { test("does not re-arm fallback when one is already pending", () => { //#given const sessionID = "ses_model_fallback_pending_guard" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) //#when const firstSet = setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7-thinking", ) const secondSet = setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", @@ -195,28 +199,29 @@ describe("model fallback hook", () => { //#then expect(firstSet).toBe(true) expect(secondSet).toBe(false) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("skips no-op fallback entries that resolve to same provider/model", async () => { //#given const sessionID = "ses_model_fallback_noop_skip" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, ) => Promise } - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["anthropic"], model: "claude-opus-4-7" }, { providers: ["opencode"], model: "kimi-k2.5-free" }, ]) expect( setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", @@ -239,28 +244,29 @@ describe("model fallback hook", () => { providerID: "opencode", modelID: "kimi-k2.5-free", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("skips no-op fallback entries even when variant differs", async () => { //#given const sessionID = "ses_model_fallback_noop_variant_skip" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, ) => Promise } - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["quotio"], model: "claude-opus-4-7", variant: "max" }, { providers: ["quotio"], model: "gpt-5.2" }, ]) expect( setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "quotio", @@ -285,28 +291,29 @@ describe("model fallback hook", () => { modelID: "gpt-5.2", }) expect(output.message["variant"]).toBeUndefined() - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("uses connected preferred provider when fallback entry providers are disconnected", async () => { //#given const sessionID = "ses_model_fallback_preferred_provider" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) readConnectedProvidersCacheMock.mockReturnValue(["provider-x"]) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, ) => Promise } - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["provider-y"], model: "fallback-model" }, ]) expect( setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Ultraworker", "provider-x", @@ -329,17 +336,18 @@ describe("model fallback hook", () => { providerID: "provider-x", modelID: "fallback-model", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("does not fall back to hardcoded agent chain when session explicitly stores no fallback chain [regression #2941]", () => { //#given const sessionID = "ses_model_fallback_explicit_none" - clearPendingModelFallback(sessionID) - setSessionFallbackChain(sessionID, undefined) + clearPendingModelFallback(modelFallback, sessionID) + setSessionFallbackChain(modelFallback, sessionID, undefined) //#when const set = setPendingModelFallback( + modelFallback, sessionID, "Sisyphus - Junior", "anthropic", @@ -348,7 +356,7 @@ describe("model fallback hook", () => { //#then expect(set).toBe(false) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("shows toast when fallback is applied", async () => { @@ -366,6 +374,7 @@ describe("model fallback hook", () => { } const set = setPendingModelFallback( + hook, "ses_model_fallback_toast", "Sisyphus - Ultraworker", "anthropic", @@ -392,9 +401,9 @@ describe("model fallback hook", () => { test("transforms model names for github-copilot provider via fallback chain", async () => { //#given const sessionID = "ses_model_fallback_ghcp" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -402,11 +411,12 @@ describe("model fallback hook", () => { } // Set a custom fallback chain that routes through github-copilot - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["github-copilot"], model: "claude-sonnet-4-6" }, ]) const set = setPendingModelFallback( + modelFallback, sessionID, "Atlas - Plan Executor", "github-copilot", @@ -430,15 +440,15 @@ describe("model fallback hook", () => { modelID: "claude-sonnet-4.6", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) test("preserves canonical google preview model names via fallback chain", async () => { //#given const sessionID = "ses_model_fallback_google" - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) - const hook = createModelFallbackHook() as unknown as { + const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, output: { message: Record; parts: Array<{ type: string; text?: string }> }, @@ -446,11 +456,12 @@ describe("model fallback hook", () => { } // Set a custom fallback chain that routes through google - setSessionFallbackChain(sessionID, [ + setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["google"], model: "gemini-3.1-pro-preview" }, ]) const set = setPendingModelFallback( + modelFallback, sessionID, "Oracle", "google", @@ -474,7 +485,7 @@ describe("model fallback hook", () => { modelID: "gemini-3.1-pro-preview", }) - clearPendingModelFallback(sessionID) + clearPendingModelFallback(modelFallback, sessionID) }) }) diff --git a/src/hooks/model-fallback/hook.ts b/src/hooks/model-fallback/hook.ts index 191a58e3a..fee130ed8 100644 --- a/src/hooks/model-fallback/hook.ts +++ b/src/hooks/model-fallback/hook.ts @@ -5,6 +5,7 @@ import { createModelFallbackStateController, type ModelFallbackStateController, } from "./fallback-state-controller" +import type { ModelFallbackControllerAccessor } from "./controller-accessor" type FallbackToast = (input: { title: string @@ -28,26 +29,45 @@ export type ModelFallbackState = { pending: boolean } -const modelFallbackControllerRef: { current?: ModelFallbackStateController } = {} +type ModelFallbackControllerWithState = Pick< + ModelFallbackStateController, + | "lastToastKey" + | "setSessionFallbackChain" + | "clearSessionFallbackChain" + | "setPendingModelFallback" + | "getNextFallback" + | "clearPendingModelFallback" + | "hasPendingModelFallback" + | "getFallbackState" + | "reset" +> -function getOrCreateModelFallbackController(): ModelFallbackStateController { - if (!modelFallbackControllerRef.current) { - createModelFallbackHook() - } - - const controller = modelFallbackControllerRef.current - if (!controller) { - throw new Error("Model fallback controller should be initialized") - } - return controller +export type ModelFallbackHook = ModelFallbackControllerWithState & { + "chat.message": ( + input: ChatMessageInput, + output: ChatMessageHandlerOutput, + ) => Promise } -export function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { - getOrCreateModelFallbackController().setSessionFallbackChain(sessionID, fallbackChain) +type ModelFallbackHookArgs = { + toast?: FallbackToast + onApplied?: FallbackCallback + controllerAccessor?: ModelFallbackControllerAccessor } -export function clearSessionFallbackChain(sessionID: string): void { - getOrCreateModelFallbackController().clearSessionFallbackChain(sessionID) +export function setSessionFallbackChain( + controller: Pick, + sessionID: string, + fallbackChain: FallbackEntry[] | undefined, +): void { + controller.setSessionFallbackChain(sessionID, fallbackChain) +} + +export function clearSessionFallbackChain( + controller: Pick, + sessionID: string, +): void { + controller.clearSessionFallbackChain(sessionID) } /** @@ -55,12 +75,13 @@ export function clearSessionFallbackChain(sessionID: string): void { * Called when a model error is detected in session.error handler. */ export function setPendingModelFallback( + controller: Pick, sessionID: string, agentName: string, currentProviderID: string, currentModelID: string, ): boolean { - return getOrCreateModelFallbackController().setPendingModelFallback( + return controller.setPendingModelFallback( sessionID, agentName, currentProviderID, @@ -73,54 +94,71 @@ export function setPendingModelFallback( * Increments attemptCount each time called. */ export function getNextFallback( + controller: Pick, sessionID: string, ): { providerID: string; modelID: string; variant?: string } | null { - return getOrCreateModelFallbackController().getNextFallback(sessionID) + return controller.getNextFallback(sessionID) } /** * Clears the pending fallback for a session. * Called after fallback is successfully applied. */ -export function clearPendingModelFallback(sessionID: string): void { - getOrCreateModelFallbackController().clearPendingModelFallback(sessionID) +export function clearPendingModelFallback( + controller: Pick, + sessionID: string, +): void { + controller.clearPendingModelFallback(sessionID) } /** * Checks if there's a pending fallback for a session. */ -export function hasPendingModelFallback(sessionID: string): boolean { - return getOrCreateModelFallbackController().hasPendingModelFallback(sessionID) +export function hasPendingModelFallback( + controller: Pick, + sessionID: string, +): boolean { + return controller.hasPendingModelFallback(sessionID) } /** * Gets the current fallback state for a session (for debugging). */ -export function getFallbackState(sessionID: string): ModelFallbackState | undefined { - return getOrCreateModelFallbackController().getFallbackState(sessionID) +export function getFallbackState( + controller: Pick, + sessionID: string, +): ModelFallbackState | undefined { + return controller.getFallbackState(sessionID) } /** * Creates a chat.message hook that applies model fallbacks when pending. */ -export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplied?: FallbackCallback }) { - if (!modelFallbackControllerRef.current) { - const pendingModelFallbacks = new Map() - const lastToastKey = new Map() - const sessionFallbackChains = new Map() +export function createModelFallbackHook(args?: ModelFallbackHookArgs): ModelFallbackHook { + const pendingModelFallbacks = new Map() + const lastToastKey = new Map() + const sessionFallbackChains = new Map() + const controller = createModelFallbackStateController({ + pendingModelFallbacks, + lastToastKey, + sessionFallbackChains, + }) - modelFallbackControllerRef.current = createModelFallbackStateController({ - pendingModelFallbacks, - lastToastKey, - sessionFallbackChains, - }) - } + args?.controllerAccessor?.register(controller) - const controller = getOrCreateModelFallbackController() const toast = args?.toast const onApplied = args?.onApplied return { + lastToastKey: controller.lastToastKey, + setSessionFallbackChain: controller.setSessionFallbackChain, + clearSessionFallbackChain: controller.clearSessionFallbackChain, + setPendingModelFallback: controller.setPendingModelFallback, + getNextFallback: controller.getNextFallback, + clearPendingModelFallback: controller.clearPendingModelFallback, + hasPendingModelFallback: controller.hasPendingModelFallback, + getFallbackState: controller.getFallbackState, + reset: controller.reset, "chat.message": async ( input: ChatMessageInput, output: ChatMessageHandlerOutput, @@ -128,7 +166,7 @@ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplie const { sessionID } = input if (!sessionID) return - const fallback = getNextFallback(sessionID) + const fallback = getNextFallback(controller, sessionID) if (!fallback) return await applyFallbackToChatMessage({ @@ -144,9 +182,8 @@ export function createModelFallbackHook(args?: { toast?: FallbackToast; onApplie } /** - * Resets all module-global state for testing. - * Clears pending fallbacks, toast keys, and session chains. + * Resets hook-owned state for testing. */ -export function _resetForTesting(): void { - getOrCreateModelFallbackController().reset() +export function _resetForTesting(controller?: Pick): void { + controller?.reset() } diff --git a/src/hooks/model-fallback/index.ts b/src/hooks/model-fallback/index.ts new file mode 100644 index 000000000..08e4c0dd7 --- /dev/null +++ b/src/hooks/model-fallback/index.ts @@ -0,0 +1,2 @@ +export { createModelFallbackControllerAccessor } from "./controller-accessor" +export type { ModelFallbackControllerAccessor } from "./controller-accessor" diff --git a/src/index.ts b/src/index.ts index b41f611e1..2c24e003e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -91,6 +91,7 @@ const serverPlugin: Plugin = async (input, _options): Promise => { pluginConfig, modelCacheState, backgroundManager: managers.backgroundManager, + modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, mergedSkills: toolsResult.mergedSkills, diff --git a/src/plugin/event.model-fallback-2941.test.ts b/src/plugin/event.model-fallback-2941.test.ts index 46765a5d9..2b97d2cb7 100644 --- a/src/plugin/event.model-fallback-2941.test.ts +++ b/src/plugin/event.model-fallback-2941.test.ts @@ -65,13 +65,13 @@ function createChatMessageHandlerHooks(modelFallback: ReturnType void } | undefined let readProviderModelsCacheSpy: { mockRestore: () => void } | undefined -afterEach(() => { - readConnectedProvidersCacheSpy?.mockRestore() - readProviderModelsCacheSpy?.mockRestore() - readConnectedProvidersCacheSpy = undefined - readProviderModelsCacheSpy = undefined - _resetForTesting() -}) + afterEach(() => { + readConnectedProvidersCacheSpy?.mockRestore() + readProviderModelsCacheSpy?.mockRestore() + readConnectedProvidersCacheSpy = undefined + readProviderModelsCacheSpy = undefined + _resetForTesting() + }) describe("createEventHandler - category runtime fallback suppression", () => { test("does not arm retry fallback when category session explicitly stores no fallback chain [regression #2941]", async () => { @@ -83,11 +83,10 @@ describe("createEventHandler - category runtime fallback suppression", () => { readConnectedProvidersCacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(null) readProviderModelsCacheSpy = spyOn(connectedProvidersCache, "readProviderModelsCache").mockReturnValue(null) - clearPendingModelFallback(sessionID) - setSessionAgent(sessionID, "sisyphus-junior") - setSessionFallbackChain(sessionID, undefined) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) + setSessionAgent(sessionID, "sisyphus-junior") + setSessionFallbackChain(modelFallback, sessionID, undefined) const eventHandler = createEventHandler({ ctx: asEventHandlerContext({ directory: "/tmp", diff --git a/src/plugin/event.model-fallback.test.ts b/src/plugin/event.model-fallback.test.ts index 3e82817ff..967608f09 100644 --- a/src/plugin/event.model-fallback.test.ts +++ b/src/plugin/event.model-fallback.test.ts @@ -142,9 +142,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_fallback" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const { handler, abortCalls, promptCalls } = createHandler({ hooks: { modelFallback } }) @@ -232,8 +231,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_dedup" setMainSession(sessionID) - clearPendingModelFallback(sessionID) const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const { handler, abortCalls, promptCalls } = createHandler({ hooks: { modelFallback } }) await handler({ @@ -293,8 +292,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_runtime_enabled" setMainSession(sessionID) - clearPendingModelFallback(sessionID) const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const runtimeFallback = { event: async () => {}, "chat.message": async () => {}, @@ -346,9 +345,8 @@ describe("createEventHandler - model fallback", () => { //#given const sessionID = "ses_status_retry_user_fallback" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const pluginConfig = { agents: { sisyphus: { @@ -446,9 +444,8 @@ describe("createEventHandler - model fallback", () => { const toastCalls: string[] = [] const sessionID = "ses_main_fallback_chain" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) setupConnectedProviderCacheMocks() const eventHandler = createEventHandler({ diff --git a/src/plugin/event.test.ts b/src/plugin/event.test.ts index cb87efff4..ea880c145 100644 --- a/src/plugin/event.test.ts +++ b/src/plugin/event.test.ts @@ -761,11 +761,10 @@ describe("createEventHandler - retry dedupe lifecycle", () => { //#given const sessionID = "ses_retry_recovery_rearm" setMainSession(sessionID) - clearPendingModelFallback(sessionID) - const abortCalls: string[] = [] const promptCalls: string[] = [] const modelFallback = createModelFallbackHook() + clearPendingModelFallback(modelFallback, sessionID) const eventHandler = createEventHandler({ ctx: asEventHandlerContext({ diff --git a/src/plugin/event.ts b/src/plugin/event.ts index 6d70d7951..5a5f177b6 100644 --- a/src/plugin/event.ts +++ b/src/plugin/event.ts @@ -15,6 +15,7 @@ import { clearSessionFallbackChain, setSessionFallbackChain, setPendingModelFallback, + type ModelFallbackHook, } from "../hooks/model-fallback/hook"; import { getRawFallbackModels } from "../hooks/runtime-fallback/fallback-models"; import { @@ -111,6 +112,7 @@ function extractProviderModelFromErrorMessage(message: string): { providerID?: s return {}; } function applyUserConfiguredFallbackChain( + modelFallback: Pick | null | undefined, sessionID: string, agentName: string, currentProviderID: string, @@ -123,7 +125,9 @@ function applyUserConfiguredFallbackChain( const fallbackChain = buildFallbackChainFromModels(rawFallbackModels, currentProviderID); if (fallbackChain && fallbackChain.length > 0) { - setSessionFallbackChain(sessionID, fallbackChain); + if (modelFallback) { + setSessionFallbackChain(modelFallback, sessionID, fallbackChain); + } } } @@ -170,6 +174,7 @@ export function createEventHandler(args: { const isModelFallbackEnabled = hooks.modelFallback !== null && hooks.modelFallback !== undefined; + const modelFallback = hooks.modelFallback; // Avoid triggering multiple abort+continue cycles for the same failing assistant message. const lastHandledModelErrorMessageID = new Map(); @@ -408,8 +413,10 @@ export function createEventHandler(args: { lastHandledModelErrorMessageID.delete(sessionInfo.id); lastHandledRetryStatusKey.delete(sessionInfo.id); lastKnownModelBySession.delete(sessionInfo.id); - clearPendingModelFallback(sessionInfo.id); - clearSessionFallbackChain(sessionInfo.id); + if (modelFallback) { + clearPendingModelFallback(modelFallback, sessionInfo.id); + clearSessionFallbackChain(modelFallback, sessionInfo.id); + } resetMessageCursor(sessionInfo.id); clearBackgroundOutputConsumptionsForParentSession(sessionInfo.id); clearBackgroundOutputConsumptionsForTaskSession(sessionInfo.id); @@ -517,9 +524,11 @@ export function createEventHandler(args: { ); const rawModel = (info?.modelID as string | undefined) ?? "claude-opus-4-7"; const currentModel = normalizeFallbackModelID(rawModel); - applyUserConfiguredFallbackChain(sessionID, agentName, currentProvider, args.pluginConfig); + applyUserConfiguredFallbackChain(modelFallback, sessionID, agentName, currentProvider, args.pluginConfig); - const setFallback = setPendingModelFallback(sessionID, agentName, currentProvider, currentModel); + const setFallback = modelFallback + ? setPendingModelFallback(modelFallback, sessionID, agentName, currentProvider, currentModel) + : false; if ( setFallback && @@ -580,9 +589,11 @@ export function createEventHandler(args: { const currentProvider = resolveFallbackProviderID(sessionID, parsed.providerID); let currentModel = parsed.modelID ?? lastKnown?.modelID ?? "claude-opus-4-7"; currentModel = normalizeFallbackModelID(currentModel); - applyUserConfiguredFallbackChain(sessionID, agentName, currentProvider, args.pluginConfig); + applyUserConfiguredFallbackChain(modelFallback, sessionID, agentName, currentProvider, args.pluginConfig); - const setFallback = setPendingModelFallback(sessionID, agentName, currentProvider, currentModel); + const setFallback = modelFallback + ? setPendingModelFallback(modelFallback, sessionID, agentName, currentProvider, currentModel) + : false; if ( setFallback && @@ -666,9 +677,11 @@ export function createEventHandler(args: { ); let currentModel = (props?.modelID as string) || parsed.modelID || "claude-opus-4-7"; currentModel = normalizeFallbackModelID(currentModel); - applyUserConfiguredFallbackChain(sessionID, agentName, currentProvider, args.pluginConfig); + applyUserConfiguredFallbackChain(modelFallback, sessionID, agentName, currentProvider, args.pluginConfig); - const setFallback = setPendingModelFallback(sessionID, agentName, currentProvider, currentModel); + const setFallback = modelFallback + ? setPendingModelFallback(modelFallback, sessionID, agentName, currentProvider, currentModel) + : false; if ( setFallback && diff --git a/src/plugin/fallback.cliproxyapi-matrix.test.ts b/src/plugin/fallback.cliproxyapi-matrix.test.ts index d5e810745..3d1b5fd4c 100644 --- a/src/plugin/fallback.cliproxyapi-matrix.test.ts +++ b/src/plugin/fallback.cliproxyapi-matrix.test.ts @@ -9,7 +9,6 @@ import { createModelFallbackHook } from "../hooks/model-fallback/hook" import { createRuntimeFallbackHook } from "../hooks/runtime-fallback" import type { RuntimeFallbackPluginInput } from "../hooks/runtime-fallback/types" import { _resetForTesting } from "../features/claude-code-session-state" -import { _resetForTesting as _resetModelFallbackForTesting } from "../hooks/model-fallback/hook" import { SessionCategoryRegistry } from "../shared/session-category-registry" import * as connectedProvidersCache from "../shared/connected-providers-cache" @@ -369,7 +368,6 @@ function setupConnectedProviderCacheMocks(): void { afterEach(() => { _resetForTesting() - _resetModelFallbackForTesting() SessionCategoryRegistry.clear() }) diff --git a/src/plugin/hooks/create-core-hooks.ts b/src/plugin/hooks/create-core-hooks.ts index 4da2b5085..5a36aa026 100644 --- a/src/plugin/hooks/create-core-hooks.ts +++ b/src/plugin/hooks/create-core-hooks.ts @@ -1,4 +1,5 @@ import type { HookName, OhMyOpenCodeConfig } from "../../config" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { PluginContext } from "../types" import type { ModelCacheState } from "../../plugin-state" @@ -10,15 +11,17 @@ export function createCoreHooks(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig modelCacheState: ModelCacheState + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor isHookEnabled: (hookName: HookName) => boolean safeHookEnabled: boolean }) { - const { ctx, pluginConfig, modelCacheState, isHookEnabled, safeHookEnabled } = args + const { ctx, pluginConfig, modelCacheState, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args const session = createSessionHooks({ ctx, pluginConfig, modelCacheState, + modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled, }) diff --git a/src/plugin/hooks/create-session-hooks.ts b/src/plugin/hooks/create-session-hooks.ts index af87bd366..9d437bc75 100644 --- a/src/plugin/hooks/create-session-hooks.ts +++ b/src/plugin/hooks/create-session-hooks.ts @@ -1,4 +1,5 @@ import type { OhMyOpenCodeConfig, HookName } from "../../config" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { ModelCacheState } from "../../plugin-state" import type { PluginContext } from "../types" @@ -69,10 +70,11 @@ export function createSessionHooks(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig modelCacheState: ModelCacheState + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor isHookEnabled: (hookName: HookName) => boolean safeHookEnabled: boolean }): SessionHooks { - const { ctx, pluginConfig, modelCacheState, isHookEnabled, safeHookEnabled } = args + const { ctx, pluginConfig, modelCacheState, modelFallbackControllerAccessor, isHookEnabled, safeHookEnabled } = args const safeHook = (hookName: HookName, factory: () => T): T | null => safeCreateHook(hookName, factory, { enabled: safeHookEnabled }) @@ -171,6 +173,7 @@ export function createSessionHooks(args: { .catch(() => {}) }, onApplied: enableFallbackTitle ? updateFallbackTitle : undefined, + controllerAccessor: modelFallbackControllerAccessor, })) : null diff --git a/src/plugin/tool-registry.ts b/src/plugin/tool-registry.ts index 6d04e7e1c..a3e46185a 100644 --- a/src/plugin/tool-registry.ts +++ b/src/plugin/tool-registry.ts @@ -144,7 +144,7 @@ export function trimToolsToCap(filteredTools: ToolsRecord, maxTools: number): vo export function createToolRegistry(args: { ctx: PluginContext pluginConfig: OhMyOpenCodeConfig - managers: Pick + managers: Pick skillContext: SkillContext availableCategories: AvailableCategory[] interactiveBashEnabled?: boolean @@ -170,6 +170,7 @@ export function createToolRegistry(args: { pluginConfig.disabled_agents ?? [], pluginConfig.agents, pluginConfig.categories, + managers.modelFallbackControllerAccessor, ) const isMultimodalLookerEnabled = !(pluginConfig.disabled_agents ?? []).some( @@ -191,6 +192,7 @@ export function createToolRegistry(args: { availableSkills: skillContext.availableSkills, sisyphusAgentConfig: pluginConfig.sisyphus_agent, syncPollTimeoutMs: pluginConfig.background_task?.syncPollTimeoutMs, + modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor, onSyncSessionCreated: async (event) => { log("[index] onSyncSessionCreated callback", { sessionID: event.sessionID, diff --git a/src/tools/call-omo-agent/sync-executor.ts b/src/tools/call-omo-agent/sync-executor.ts index 23089d8ea..56e22a80a 100644 --- a/src/tools/call-omo-agent/sync-executor.ts +++ b/src/tools/call-omo-agent/sync-executor.ts @@ -1,7 +1,6 @@ import type { CallOmoAgentArgs } from "./types" import type { PluginInput } from "@opencode-ai/plugin" import { subagentSessions, syncSubagentSessions } from "../../features/claude-code-session-state" -import { clearSessionFallbackChain, setSessionFallbackChain } from "../../hooks/model-fallback/hook" import { getAgentToolRestrictions, log } from "../../shared" import { applySessionPromptParams } from "../../shared/session-prompt-params-helpers" import type { DelegatedModelConfig } from "../../shared/model-resolution-types" @@ -19,8 +18,8 @@ type ExecuteSyncDeps = { createOrGetSession: typeof createOrGetSession waitForCompletion: typeof waitForCompletion processMessages: typeof processMessages - setSessionFallbackChain: typeof setSessionFallbackChain - clearSessionFallbackChain: typeof clearSessionFallbackChain + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + clearSessionFallbackChain: (sessionID: string) => void } type SpawnReservation = { @@ -32,8 +31,8 @@ const defaultDeps: ExecuteSyncDeps = { createOrGetSession, waitForCompletion, processMessages, - setSessionFallbackChain, - clearSessionFallbackChain, + setSessionFallbackChain: () => {}, + clearSessionFallbackChain: () => {}, } function buildPromptGenerationParams(model: DelegatedModelConfig | undefined): Record { diff --git a/src/tools/call-omo-agent/tools.ts b/src/tools/call-omo-agent/tools.ts index 839f5abe8..51ea8730c 100644 --- a/src/tools/call-omo-agent/tools.ts +++ b/src/tools/call-omo-agent/tools.ts @@ -2,6 +2,7 @@ import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin import { ALLOWED_AGENTS, CALL_OMO_AGENT_DESCRIPTION } from "./constants" import type { CallOmoAgentArgs, ToolContextWithMetadata } from "./types" import type { BackgroundManager } from "../../features/background-agent" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { CategoriesConfig, AgentOverrides } from "../../config/schema" import type { DelegatedModelConfig } from "../../shared/model-resolution-types" import type { FallbackEntry } from "../../shared/model-requirements" @@ -15,6 +16,23 @@ import { parseModelString } from "../../shared" import { executeBackground } from "./background-executor" import { executeSync } from "./sync-executor" import { resolveCallableAgents } from "./agent-resolver" +import { createOrGetSession } from "./session-creator" +import { processMessages } from "./message-processor" +import { waitForCompletion } from "./completion-poller" + +function createSyncExecutorDeps(modelFallbackControllerAccessor?: ModelFallbackControllerAccessor) { + return { + createOrGetSession, + waitForCompletion, + processMessages, + setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => { + modelFallbackControllerAccessor?.setSessionFallbackChain(sessionID, fallbackChain) + }, + clearSessionFallbackChain: (sessionID: string) => { + modelFallbackControllerAccessor?.clearSessionFallbackChain(sessionID) + }, + } +} function resolveModelAndFallbackChain(args: { subagentType: string @@ -82,6 +100,7 @@ export function createCallOmoAgent( disabledAgents: string[] = [], agentOverrides?: AgentOverrides, userCategories?: CategoriesConfig, + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor, ): ToolDefinition { const agentDescriptions = ALLOWED_AGENTS.map( (name) => `- ${name}: Specialized agent for ${name} tasks`, @@ -158,14 +177,30 @@ export function createCallOmoAgent( let spawnReservation: Awaited> | undefined try { spawnReservation = await backgroundManager.reserveSubagentSpawn(toolCtx.sessionID) - return await executeSync(args, toolCtx, ctx, undefined, fallbackChain, spawnReservation, resolvedModel) + return await executeSync( + args, + toolCtx, + ctx, + createSyncExecutorDeps(modelFallbackControllerAccessor), + fallbackChain, + spawnReservation, + resolvedModel, + ) } catch (error) { spawnReservation?.rollback() return `Error: ${error instanceof Error ? error.message : String(error)}` } } - return await executeSync(args, toolCtx, ctx, undefined, fallbackChain, undefined, resolvedModel) + return await executeSync( + args, + toolCtx, + ctx, + createSyncExecutorDeps(modelFallbackControllerAccessor), + fallbackChain, + undefined, + resolvedModel, + ) }, }); } diff --git a/src/tools/delegate-task/background-task.ts b/src/tools/delegate-task/background-task.ts index 6d982992b..d5c4adf5d 100644 --- a/src/tools/delegate-task/background-task.ts +++ b/src/tools/delegate-task/background-task.ts @@ -8,7 +8,6 @@ import { formatDetailedError } from "./error-formatting" import { getSessionTools } from "../../shared/session-tools-store" import { SessionCategoryRegistry } from "../../shared/session-category-registry" import { QUESTION_DENIED_SESSION_PERMISSION } from "../../shared/question-denied-session-permission" -import { setSessionFallbackChain } from "../../hooks/model-fallback/hook" import { stripAgentListSortPrefix } from "../../shared/agent-display-names" import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-metadata-contract" import { resolveMetadataModel } from "./resolve-metadata-model" @@ -19,6 +18,7 @@ function continueSessionSetup(args: { timing: ReturnType fallbackChain?: FallbackEntry[] category?: string + modelFallbackControllerAccessor?: ExecutorContext["modelFallbackControllerAccessor"] }): void { if (!args.fallbackChain && !args.category) { return @@ -41,7 +41,7 @@ function continueSessionSetup(args: { continue } - setSessionFallbackChain(sessionId, args.fallbackChain) + args.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionId, args.fallbackChain) if (args.category) { SessionCategoryRegistry.register(sessionId, args.category) } @@ -106,6 +106,7 @@ export async function executeBackgroundTask( timing, fallbackChain, category: args.category, + modelFallbackControllerAccessor: executorCtx.modelFallbackControllerAccessor, }) break } @@ -113,7 +114,7 @@ export async function executeBackgroundTask( } if (sessionId) { - setSessionFallbackChain(sessionId, fallbackChain) + executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionId, fallbackChain) } if (args.category && sessionId) { SessionCategoryRegistry.register(sessionId, args.category) diff --git a/src/tools/delegate-task/executor-types.ts b/src/tools/delegate-task/executor-types.ts index bfa7fc70b..8b430c9ce 100644 --- a/src/tools/delegate-task/executor-types.ts +++ b/src/tools/delegate-task/executor-types.ts @@ -1,5 +1,6 @@ import type { BackgroundManager } from "../../features/background-agent" import type { CategoriesConfig, GitMasterConfig, BrowserAutomationProvider, AgentOverrides, SisyphusAgentConfig } from "../../config/schema" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { OpencodeClient } from "./types" export interface ExecutorContext { @@ -12,6 +13,7 @@ export interface ExecutorContext { browserProvider?: BrowserAutomationProvider agentOverrides?: AgentOverrides sisyphusAgentConfig?: SisyphusAgentConfig + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor onSyncSessionCreated?: (event: { sessionID: string; parentID: string; title: string }) => Promise syncPollTimeoutMs?: number } diff --git a/src/tools/delegate-task/sync-task.ts b/src/tools/delegate-task/sync-task.ts index 111371a51..034c0e199 100644 --- a/src/tools/delegate-task/sync-task.ts +++ b/src/tools/delegate-task/sync-task.ts @@ -9,7 +9,6 @@ import { SessionCategoryRegistry } from "../../shared/session-category-registry" 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" import { retrySyncPromptWithFallbacks } from "./sync-task-fallback" import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-metadata-contract" import { resolveMetadataModel } from "./resolve-metadata-model" @@ -81,7 +80,7 @@ export async function executeSyncTask( subagentSessions.add(sessionID) syncSubagentSessions.add(sessionID) setSessionAgent(sessionID, agentToUse) - setSessionFallbackChain(sessionID, fallbackChain) + executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionID, fallbackChain) if (args.category) { SessionCategoryRegistry.register(sessionID, args.category) @@ -237,7 +236,7 @@ ${buildTaskMetadataBlock({ if (syncSessionID) { subagentSessions.delete(syncSessionID) syncSubagentSessions.delete(syncSessionID) - clearSessionFallbackChain(syncSessionID) + executorCtx.modelFallbackControllerAccessor?.clearSessionFallbackChain(syncSessionID) SessionCategoryRegistry.remove(syncSessionID) } } diff --git a/src/tools/delegate-task/types.ts b/src/tools/delegate-task/types.ts index 987e821a2..9eff782ce 100644 --- a/src/tools/delegate-task/types.ts +++ b/src/tools/delegate-task/types.ts @@ -1,6 +1,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import type { BackgroundManager } from "../../features/background-agent" import type { CategoriesConfig, GitMasterConfig, BrowserAutomationProvider, AgentOverrides, SisyphusAgentConfig } from "../../config/schema" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import type { AvailableCategory, AvailableSkill, @@ -68,6 +69,7 @@ export interface DelegateTaskToolOptions { availableSkills?: AvailableSkill[] agentOverrides?: AgentOverrides sisyphusAgentConfig?: SisyphusAgentConfig + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor onSyncSessionCreated?: (event: SyncSessionCreatedEvent) => Promise syncPollTimeoutMs?: number }