fix(model-fallback): clone session fallback chains
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -39,7 +39,7 @@ export function createModelFallbackStateController(input: {
|
|||||||
|
|
||||||
function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void {
|
function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void {
|
||||||
if (!sessionID) return
|
if (!sessionID) return
|
||||||
sessionFallbackChains.set(sessionID, fallbackChain?.length ? fallbackChain : [])
|
sessionFallbackChains.set(sessionID, fallbackChain?.length ? [...fallbackChain] : [])
|
||||||
}
|
}
|
||||||
|
|
||||||
function clearSessionFallbackChain(sessionID: string): void {
|
function clearSessionFallbackChain(sessionID: string): void {
|
||||||
@@ -47,7 +47,8 @@ export function createModelFallbackStateController(input: {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function getSessionFallbackChain(sessionID: string): FallbackEntry[] | undefined {
|
function getSessionFallbackChain(sessionID: string): FallbackEntry[] | undefined {
|
||||||
return sessionFallbackChains.get(sessionID)
|
const fallbackChain = sessionFallbackChains.get(sessionID)
|
||||||
|
return fallbackChain ? [...fallbackChain] : undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
function setPendingModelFallback(
|
function setPendingModelFallback(
|
||||||
@@ -61,7 +62,7 @@ export function createModelFallbackStateController(input: {
|
|||||||
const fallbackChain = sessionFallbackChains.get(sessionID) ?? requirements?.fallbackChain
|
const fallbackChain = sessionFallbackChains.get(sessionID) ?? requirements?.fallbackChain
|
||||||
|
|
||||||
if (!fallbackChain?.length) {
|
if (!fallbackChain?.length) {
|
||||||
log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")")
|
log(`[model-fallback] No fallback chain for agent: ${agentName} (key: ${agentKey})`)
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -74,12 +75,12 @@ export function createModelFallbackStateController(input: {
|
|||||||
attemptCount: 0,
|
attemptCount: 0,
|
||||||
pending: true,
|
pending: true,
|
||||||
})
|
})
|
||||||
log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName)
|
log(`[model-fallback] Set pending fallback for session: ${sessionID}, agent: ${agentName}`)
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
if (existing.pending) {
|
if (existing.pending) {
|
||||||
log("[model-fallback] Pending fallback already armed for session: " + sessionID)
|
log(`[model-fallback] Pending fallback already armed for session: ${sessionID}`)
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -87,10 +88,10 @@ export function createModelFallbackStateController(input: {
|
|||||||
existing.modelID = currentModelID
|
existing.modelID = currentModelID
|
||||||
existing.pending = true
|
existing.pending = true
|
||||||
if (existing.attemptCount >= existing.fallbackChain.length) {
|
if (existing.attemptCount >= existing.fallbackChain.length) {
|
||||||
log("[model-fallback] Fallback chain exhausted for session: " + sessionID)
|
log(`[model-fallback] Fallback chain exhausted for session: ${sessionID}`)
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
log("[model-fallback] Re-armed pending fallback for session: " + sessionID)
|
log(`[model-fallback] Re-armed pending fallback for session: ${sessionID}`)
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -101,7 +102,7 @@ export function createModelFallbackStateController(input: {
|
|||||||
const fallback = getNextReachableFallback(sessionID, state)
|
const fallback = getNextReachableFallback(sessionID, state)
|
||||||
if (fallback) return fallback
|
if (fallback) return fallback
|
||||||
|
|
||||||
log("[model-fallback] No more fallbacks for session: " + sessionID)
|
log(`[model-fallback] No more fallbacks for session: ${sessionID}`)
|
||||||
pendingModelFallbacks.delete(sessionID)
|
pendingModelFallbacks.delete(sessionID)
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -66,6 +66,7 @@ async function importFreshModelFallbackHookModule() {
|
|||||||
const {
|
const {
|
||||||
clearPendingModelFallback,
|
clearPendingModelFallback,
|
||||||
createModelFallbackHook,
|
createModelFallbackHook,
|
||||||
|
getSessionFallbackChain,
|
||||||
setSessionFallbackChain,
|
setSessionFallbackChain,
|
||||||
setPendingModelFallback,
|
setPendingModelFallback,
|
||||||
} = await importFreshModelFallbackHookModule()
|
} = await importFreshModelFallbackHookModule()
|
||||||
@@ -85,7 +86,6 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("applies pending fallback on chat.message by overriding model", async () => {
|
test("applies pending fallback on chat.message by overriding model", async () => {
|
||||||
//#given
|
|
||||||
const hook = modelFallback as unknown as {
|
const hook = modelFallback as unknown as {
|
||||||
"chat.message"?: (
|
"chat.message"?: (
|
||||||
input: { sessionID: string },
|
input: { sessionID: string },
|
||||||
@@ -110,13 +110,11 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.(
|
await hook["chat.message"]?.(
|
||||||
{ sessionID: "ses_model_fallback_main" },
|
{ sessionID: "ses_model_fallback_main" },
|
||||||
output,
|
output,
|
||||||
)
|
)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(output.message["model"]).toEqual({
|
expect(output.message["model"]).toEqual({
|
||||||
providerID: "anthropic",
|
providerID: "anthropic",
|
||||||
modelID: "claude-opus-4-7",
|
modelID: "claude-opus-4-7",
|
||||||
@@ -124,7 +122,6 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("preserves fallback progression across repeated session.error retries", async () => {
|
test("preserves fallback progression across repeated session.error retries", async () => {
|
||||||
//#given
|
|
||||||
const hook = modelFallback as unknown as {
|
const hook = modelFallback as unknown as {
|
||||||
"chat.message"?: (
|
"chat.message"?: (
|
||||||
input: { sessionID: string },
|
input: { sessionID: string },
|
||||||
@@ -145,16 +142,13 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when - first retry is applied
|
|
||||||
await hook["chat.message"]?.({ sessionID }, firstOutput)
|
await hook["chat.message"]?.({ sessionID }, firstOutput)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(firstOutput.message["model"]).toEqual({
|
expect(firstOutput.message["model"]).toEqual({
|
||||||
providerID: "anthropic",
|
providerID: "anthropic",
|
||||||
modelID: "claude-opus-4-7",
|
modelID: "claude-opus-4-7",
|
||||||
})
|
})
|
||||||
|
|
||||||
//#when - second error re-arms fallback and should advance to next entry
|
|
||||||
expect(
|
expect(
|
||||||
setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"),
|
setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"),
|
||||||
).toBe(true)
|
).toBe(true)
|
||||||
@@ -167,7 +161,6 @@ describe("model fallback hook", () => {
|
|||||||
}
|
}
|
||||||
await hook["chat.message"]?.({ sessionID }, secondOutput)
|
await hook["chat.message"]?.({ sessionID }, secondOutput)
|
||||||
|
|
||||||
//#then - chain should progress to entry[1], not repeat entry[0]
|
|
||||||
expect(secondOutput.message["model"]).toEqual({
|
expect(secondOutput.message["model"]).toEqual({
|
||||||
providerID: "opencode-go",
|
providerID: "opencode-go",
|
||||||
modelID: "kimi-k2.5",
|
modelID: "kimi-k2.5",
|
||||||
@@ -176,11 +169,9 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("does not re-arm fallback when one is already pending", () => {
|
test("does not re-arm fallback when one is already pending", () => {
|
||||||
//#given
|
|
||||||
const sessionID = "ses_model_fallback_pending_guard"
|
const sessionID = "ses_model_fallback_pending_guard"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
|
|
||||||
//#when
|
|
||||||
const firstSet = setPendingModelFallback(
|
const firstSet = setPendingModelFallback(
|
||||||
modelFallback,
|
modelFallback,
|
||||||
sessionID,
|
sessionID,
|
||||||
@@ -196,14 +187,28 @@ describe("model fallback hook", () => {
|
|||||||
"claude-opus-4-7-thinking",
|
"claude-opus-4-7-thinking",
|
||||||
)
|
)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(firstSet).toBe(true)
|
expect(firstSet).toBe(true)
|
||||||
expect(secondSet).toBe(false)
|
expect(secondSet).toBe(false)
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("isolates stored fallback chains from caller mutations on set and get", () => {
|
||||||
|
const sessionID = "ses_model_fallback_defensive_copy"
|
||||||
|
const originalChain = [
|
||||||
|
{ providers: ["anthropic"], model: "claude-opus-4-7" },
|
||||||
|
]
|
||||||
|
|
||||||
|
setSessionFallbackChain(modelFallback, sessionID, originalChain)
|
||||||
|
originalChain.push({ providers: ["google"], model: "gemini-2.5-pro" })
|
||||||
|
const retrieved = getSessionFallbackChain(modelFallback, sessionID)
|
||||||
|
retrieved?.push({ providers: ["openai"], model: "gpt-5.4" })
|
||||||
|
|
||||||
|
expect(getSessionFallbackChain(modelFallback, sessionID)).toEqual([
|
||||||
|
{ providers: ["anthropic"], model: "claude-opus-4-7" },
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
test("skips no-op fallback entries that resolve to same provider/model", async () => {
|
test("skips no-op fallback entries that resolve to same provider/model", async () => {
|
||||||
//#given
|
|
||||||
const sessionID = "ses_model_fallback_noop_skip"
|
const sessionID = "ses_model_fallback_noop_skip"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
|
|
||||||
@@ -236,10 +241,8 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.({ sessionID }, output)
|
await hook["chat.message"]?.({ sessionID }, output)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(output.message["model"]).toEqual({
|
expect(output.message["model"]).toEqual({
|
||||||
providerID: "opencode",
|
providerID: "opencode",
|
||||||
modelID: "kimi-k2.5-free",
|
modelID: "kimi-k2.5-free",
|
||||||
@@ -248,7 +251,6 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("skips no-op fallback entries even when variant differs", async () => {
|
test("skips no-op fallback entries even when variant differs", async () => {
|
||||||
//#given
|
|
||||||
const sessionID = "ses_model_fallback_noop_variant_skip"
|
const sessionID = "ses_model_fallback_noop_variant_skip"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
|
|
||||||
@@ -282,10 +284,8 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.({ sessionID }, output)
|
await hook["chat.message"]?.({ sessionID }, output)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(output.message["model"]).toEqual({
|
expect(output.message["model"]).toEqual({
|
||||||
providerID: "quotio",
|
providerID: "quotio",
|
||||||
modelID: "gpt-5.2",
|
modelID: "gpt-5.2",
|
||||||
@@ -295,7 +295,6 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("uses connected preferred provider when fallback entry providers are disconnected", async () => {
|
test("uses connected preferred provider when fallback entry providers are disconnected", async () => {
|
||||||
//#given
|
|
||||||
const sessionID = "ses_model_fallback_preferred_provider"
|
const sessionID = "ses_model_fallback_preferred_provider"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
readConnectedProvidersCacheMock.mockReturnValue(["provider-x"])
|
readConnectedProvidersCacheMock.mockReturnValue(["provider-x"])
|
||||||
@@ -328,10 +327,8 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.({ sessionID }, output)
|
await hook["chat.message"]?.({ sessionID }, output)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(output.message["model"]).toEqual({
|
expect(output.message["model"]).toEqual({
|
||||||
providerID: "provider-x",
|
providerID: "provider-x",
|
||||||
modelID: "fallback-model",
|
modelID: "fallback-model",
|
||||||
@@ -340,12 +337,10 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("does not fall back to hardcoded agent chain when session explicitly stores no fallback chain [regression #2941]", () => {
|
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"
|
const sessionID = "ses_model_fallback_explicit_none"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
setSessionFallbackChain(modelFallback, sessionID, undefined)
|
setSessionFallbackChain(modelFallback, sessionID, undefined)
|
||||||
|
|
||||||
//#when
|
|
||||||
const set = setPendingModelFallback(
|
const set = setPendingModelFallback(
|
||||||
modelFallback,
|
modelFallback,
|
||||||
sessionID,
|
sessionID,
|
||||||
@@ -354,13 +349,11 @@ describe("model fallback hook", () => {
|
|||||||
"claude-sonnet-4-6",
|
"claude-sonnet-4-6",
|
||||||
)
|
)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(set).toBe(false)
|
expect(set).toBe(false)
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
})
|
})
|
||||||
|
|
||||||
test("shows toast when fallback is applied", async () => {
|
test("shows toast when fallback is applied", async () => {
|
||||||
//#given
|
|
||||||
const toastCalls: Array<{ title: string; message: string }> = []
|
const toastCalls: Array<{ title: string; message: string }> = []
|
||||||
const hook = createModelFallbackHook({
|
const hook = createModelFallbackHook({
|
||||||
toast: async ({ title, message }) => {
|
toast: async ({ title, message }) => {
|
||||||
@@ -390,16 +383,13 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.({ sessionID: "ses_model_fallback_toast" }, output)
|
await hook["chat.message"]?.({ sessionID: "ses_model_fallback_toast" }, output)
|
||||||
|
|
||||||
//#then
|
|
||||||
expect(toastCalls.length).toBe(1)
|
expect(toastCalls.length).toBe(1)
|
||||||
expect(toastCalls[0]?.title).toBe("Model fallback")
|
expect(toastCalls[0]?.title).toBe("Model fallback")
|
||||||
})
|
})
|
||||||
|
|
||||||
test("transforms model names for github-copilot provider via fallback chain", async () => {
|
test("transforms model names for github-copilot provider via fallback chain", async () => {
|
||||||
//#given
|
|
||||||
const sessionID = "ses_model_fallback_ghcp"
|
const sessionID = "ses_model_fallback_ghcp"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
|
|
||||||
@@ -410,7 +400,6 @@ describe("model fallback hook", () => {
|
|||||||
) => Promise<void>
|
) => Promise<void>
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set a custom fallback chain that routes through github-copilot
|
|
||||||
setSessionFallbackChain(modelFallback, sessionID, [
|
setSessionFallbackChain(modelFallback, sessionID, [
|
||||||
{ providers: ["github-copilot"], model: "claude-sonnet-4-6" },
|
{ providers: ["github-copilot"], model: "claude-sonnet-4-6" },
|
||||||
])
|
])
|
||||||
@@ -431,10 +420,8 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.({ sessionID }, output)
|
await hook["chat.message"]?.({ sessionID }, output)
|
||||||
|
|
||||||
//#then - model name should be transformed from hyphen to dot notation
|
|
||||||
expect(output.message["model"]).toEqual({
|
expect(output.message["model"]).toEqual({
|
||||||
providerID: "github-copilot",
|
providerID: "github-copilot",
|
||||||
modelID: "claude-sonnet-4.6",
|
modelID: "claude-sonnet-4.6",
|
||||||
@@ -444,7 +431,6 @@ describe("model fallback hook", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("preserves canonical google preview model names via fallback chain", async () => {
|
test("preserves canonical google preview model names via fallback chain", async () => {
|
||||||
//#given
|
|
||||||
const sessionID = "ses_model_fallback_google"
|
const sessionID = "ses_model_fallback_google"
|
||||||
clearPendingModelFallback(modelFallback, sessionID)
|
clearPendingModelFallback(modelFallback, sessionID)
|
||||||
|
|
||||||
@@ -455,7 +441,6 @@ describe("model fallback hook", () => {
|
|||||||
) => Promise<void>
|
) => Promise<void>
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set a custom fallback chain that routes through google
|
|
||||||
setSessionFallbackChain(modelFallback, sessionID, [
|
setSessionFallbackChain(modelFallback, sessionID, [
|
||||||
{ providers: ["google"], model: "gemini-3.1-pro-preview" },
|
{ providers: ["google"], model: "gemini-3.1-pro-preview" },
|
||||||
])
|
])
|
||||||
@@ -476,10 +461,8 @@ describe("model fallback hook", () => {
|
|||||||
parts: [{ type: "text", text: "continue" }],
|
parts: [{ type: "text", text: "continue" }],
|
||||||
}
|
}
|
||||||
|
|
||||||
//#when
|
|
||||||
await hook["chat.message"]?.({ sessionID }, output)
|
await hook["chat.message"]?.({ sessionID }, output)
|
||||||
|
|
||||||
//#then: model name should remain gemini-3.1-pro-preview because no google transform exists for this ID
|
|
||||||
expect(output.message["model"]).toEqual({
|
expect(output.message["model"]).toEqual({
|
||||||
providerID: "google",
|
providerID: "google",
|
||||||
modelID: "gemini-3.1-pro-preview",
|
modelID: "gemini-3.1-pro-preview",
|
||||||
|
|||||||
Reference in New Issue
Block a user