refactor(sisyphus-task): use dynamic model fallback from OpenCode config
- Remove hardcoded "anthropic/claude-sonnet-4-5" fallback - Fetch systemDefaultModel from client.config.get() at tool boundary - Add 'category-default' and 'system-default' fallback types - Use switch(actualModel) for cleaner type detection - Add guard clauses and fail-loud validation for invalid models - Wrap config fetch in try/catch for graceful degradation - Update toast messages with typed suffixMap
This commit is contained in:
@@ -4,9 +4,13 @@ import type { CategoryConfig } from "../../config/schema"
|
||||
|
||||
function resolveCategoryConfig(
|
||||
categoryName: string,
|
||||
userCategories?: Record<string, CategoryConfig>,
|
||||
parentModelString?: string
|
||||
): { config: CategoryConfig; promptAppend: string } | null {
|
||||
options: {
|
||||
userCategories?: Record<string, CategoryConfig>
|
||||
parentModelString?: string
|
||||
systemDefaultModel?: string
|
||||
}
|
||||
): { config: CategoryConfig; promptAppend: string; model: string | undefined } | null {
|
||||
const { userCategories, parentModelString, systemDefaultModel } = options
|
||||
const defaultConfig = DEFAULT_CATEGORIES[categoryName]
|
||||
const userConfig = userCategories?.[categoryName]
|
||||
const defaultPromptAppend = CATEGORY_PROMPT_APPENDS[categoryName] ?? ""
|
||||
@@ -15,10 +19,11 @@ function resolveCategoryConfig(
|
||||
return null
|
||||
}
|
||||
|
||||
const model = userConfig?.model ?? parentModelString ?? defaultConfig?.model ?? systemDefaultModel
|
||||
const config: CategoryConfig = {
|
||||
...defaultConfig,
|
||||
...userConfig,
|
||||
model: userConfig?.model ?? parentModelString ?? defaultConfig?.model ?? "anthropic/claude-sonnet-4-5",
|
||||
model,
|
||||
}
|
||||
|
||||
let promptAppend = defaultPromptAppend
|
||||
@@ -28,7 +33,7 @@ function resolveCategoryConfig(
|
||||
: userConfig.prompt_append
|
||||
}
|
||||
|
||||
return { config, promptAppend }
|
||||
return { config, promptAppend, model }
|
||||
}
|
||||
|
||||
describe("sisyphus-task", () => {
|
||||
@@ -115,7 +120,7 @@ describe("sisyphus-task", () => {
|
||||
const categoryName = "unknown-category"
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName)
|
||||
const result = resolveCategoryConfig(categoryName, {})
|
||||
|
||||
// #then
|
||||
expect(result).toBeNull()
|
||||
@@ -126,7 +131,7 @@ describe("sisyphus-task", () => {
|
||||
const categoryName = "visual-engineering"
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName)
|
||||
const result = resolveCategoryConfig(categoryName, {})
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -142,7 +147,7 @@ describe("sisyphus-task", () => {
|
||||
}
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, userCategories)
|
||||
const result = resolveCategoryConfig(categoryName, { userCategories })
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -160,7 +165,7 @@ describe("sisyphus-task", () => {
|
||||
}
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, userCategories)
|
||||
const result = resolveCategoryConfig(categoryName, { userCategories })
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -180,7 +185,7 @@ describe("sisyphus-task", () => {
|
||||
}
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, userCategories)
|
||||
const result = resolveCategoryConfig(categoryName, { userCategories })
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -200,7 +205,7 @@ describe("sisyphus-task", () => {
|
||||
}
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, userCategories)
|
||||
const result = resolveCategoryConfig(categoryName, { userCategories })
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -213,7 +218,7 @@ describe("sisyphus-task", () => {
|
||||
const parentModelString = "cliproxy/claude-opus-4-5"
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, undefined, parentModelString)
|
||||
const result = resolveCategoryConfig(categoryName, { parentModelString })
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -229,7 +234,7 @@ describe("sisyphus-task", () => {
|
||||
const parentModelString = "cliproxy/claude-opus-4-5"
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, userCategories, parentModelString)
|
||||
const result = resolveCategoryConfig(categoryName, { userCategories, parentModelString })
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -241,7 +246,7 @@ describe("sisyphus-task", () => {
|
||||
const categoryName = "visual-engineering"
|
||||
|
||||
// #when
|
||||
const result = resolveCategoryConfig(categoryName, undefined, undefined)
|
||||
const result = resolveCategoryConfig(categoryName, {})
|
||||
|
||||
// #then
|
||||
expect(result).not.toBeNull()
|
||||
@@ -270,6 +275,7 @@ describe("sisyphus-task", () => {
|
||||
|
||||
const mockClient = {
|
||||
app: { agents: async () => ({ data: [] }) },
|
||||
config: { get: async () => ({}) },
|
||||
session: {
|
||||
create: async () => ({ data: { id: "test-session" } }),
|
||||
prompt: async () => ({ data: {} }),
|
||||
@@ -327,6 +333,7 @@ describe("sisyphus-task", () => {
|
||||
const mockManager = { launch: async () => ({}) }
|
||||
const mockClient = {
|
||||
app: { agents: async () => ({ data: [] }) },
|
||||
config: { get: async () => ({}) },
|
||||
session: {
|
||||
create: async () => ({ data: { id: "test-session" } }),
|
||||
prompt: async () => ({ data: {} }),
|
||||
@@ -394,6 +401,7 @@ describe("sisyphus-task", () => {
|
||||
],
|
||||
}),
|
||||
},
|
||||
config: { get: async () => ({}) },
|
||||
app: {
|
||||
agents: async () => ({ data: [] }),
|
||||
},
|
||||
@@ -451,6 +459,7 @@ describe("sisyphus-task", () => {
|
||||
data: [],
|
||||
}),
|
||||
},
|
||||
config: { get: async () => ({}) },
|
||||
}
|
||||
|
||||
const tool = createSisyphusTask({
|
||||
@@ -502,6 +511,7 @@ describe("sisyphus-task", () => {
|
||||
messages: async () => ({ data: [] }),
|
||||
status: async () => ({ data: {} }),
|
||||
},
|
||||
config: { get: async () => ({}) },
|
||||
app: {
|
||||
agents: async () => ({ data: [{ name: "ultrabrain", mode: "subagent" }] }),
|
||||
},
|
||||
@@ -560,6 +570,7 @@ describe("sisyphus-task", () => {
|
||||
}),
|
||||
status: async () => ({ data: { "ses_sync_success": { type: "idle" } } }),
|
||||
},
|
||||
config: { get: async () => ({}) },
|
||||
app: {
|
||||
agents: async () => ({ data: [{ name: "ultrabrain", mode: "subagent" }] }),
|
||||
},
|
||||
@@ -612,6 +623,7 @@ describe("sisyphus-task", () => {
|
||||
messages: async () => ({ data: [] }),
|
||||
status: async () => ({ data: {} }),
|
||||
},
|
||||
config: { get: async () => ({}) },
|
||||
app: {
|
||||
agents: async () => ({ data: [{ name: "ultrabrain", mode: "subagent" }] }),
|
||||
},
|
||||
@@ -666,6 +678,7 @@ describe("sisyphus-task", () => {
|
||||
}),
|
||||
status: async () => ({ data: {} }),
|
||||
},
|
||||
config: { get: async () => ({}) },
|
||||
app: { agents: async () => ({ data: [] }) },
|
||||
}
|
||||
|
||||
@@ -707,7 +720,7 @@ describe("sisyphus-task", () => {
|
||||
const { buildSystemContent } = require("./tools")
|
||||
|
||||
// #when
|
||||
const result = buildSystemContent({ skills: undefined, categoryPromptAppend: undefined })
|
||||
const result = buildSystemContent({ skillContent: undefined, categoryPromptAppend: undefined })
|
||||
|
||||
// #then
|
||||
expect(result).toBeUndefined()
|
||||
@@ -754,18 +767,18 @@ describe("sisyphus-task", () => {
|
||||
})
|
||||
|
||||
describe("modelInfo detection via resolveCategoryConfig", () => {
|
||||
test("when parentModelString exists but default model wins - modelInfo should report default", () => {
|
||||
test("when parentModelString exists but default model wins - modelInfo should report category-default", () => {
|
||||
// #given - Bug scenario: parentModelString is passed but userModel is undefined,
|
||||
// and the resolution order is: userModel ?? parentModelString ?? defaultModel
|
||||
// If parentModelString matches the resolved model, it's "inherited"
|
||||
// If defaultModel matches, it's "default"
|
||||
// If defaultModel matches, it's "category-default"
|
||||
const categoryName = "ultrabrain"
|
||||
const parentModelString = undefined
|
||||
|
||||
// #when
|
||||
const resolved = resolveCategoryConfig(categoryName, undefined, parentModelString)
|
||||
const resolved = resolveCategoryConfig(categoryName, { parentModelString })
|
||||
|
||||
// #then - actualModel should be defaultModel, type should be "default"
|
||||
// #then - actualModel should be defaultModel, type should be "category-default"
|
||||
expect(resolved).not.toBeNull()
|
||||
const actualModel = resolved!.config.model
|
||||
const defaultModel = DEFAULT_CATEGORIES[categoryName]?.model
|
||||
@@ -779,7 +792,7 @@ describe("sisyphus-task", () => {
|
||||
const parentModelString = "cliproxy/claude-opus-4-5"
|
||||
|
||||
// #when
|
||||
const resolved = resolveCategoryConfig(categoryName, undefined, parentModelString)
|
||||
const resolved = resolveCategoryConfig(categoryName, { parentModelString })
|
||||
|
||||
// #then - actualModel should be parentModelString, type should be "inherited"
|
||||
expect(resolved).not.toBeNull()
|
||||
@@ -794,7 +807,7 @@ describe("sisyphus-task", () => {
|
||||
const parentModelString = "cliproxy/claude-opus-4-5"
|
||||
|
||||
// #when
|
||||
const resolved = resolveCategoryConfig(categoryName, userCategories, parentModelString)
|
||||
const resolved = resolveCategoryConfig(categoryName, { userCategories, parentModelString })
|
||||
|
||||
// #then - actualModel should be userModel, type should be "user-defined"
|
||||
expect(resolved).not.toBeNull()
|
||||
@@ -812,7 +825,7 @@ describe("sisyphus-task", () => {
|
||||
const userCategories = { "ultrabrain": { model: "user/model" } }
|
||||
|
||||
// #when - user model wins
|
||||
const resolved = resolveCategoryConfig(categoryName, userCategories, parentModelString)
|
||||
const resolved = resolveCategoryConfig(categoryName, { userCategories, parentModelString })
|
||||
const actualModel = resolved!.config.model
|
||||
const userDefinedModel = userCategories[categoryName]?.model
|
||||
const defaultModel = DEFAULT_CATEGORIES[categoryName]?.model
|
||||
@@ -823,11 +836,40 @@ describe("sisyphus-task", () => {
|
||||
: actualModel === parentModelString
|
||||
? "inherited"
|
||||
: actualModel === defaultModel
|
||||
? "default"
|
||||
? "category-default"
|
||||
: undefined
|
||||
|
||||
expect(detectedType).toBe("user-defined")
|
||||
expect(actualModel).not.toBe(parentModelString)
|
||||
})
|
||||
|
||||
test("systemDefaultModel is used when no other model is available", () => {
|
||||
// #given - custom category with no model, but systemDefaultModel is set
|
||||
const categoryName = "my-custom"
|
||||
// Using type assertion since we're testing fallback behavior for categories without model
|
||||
const userCategories = { "my-custom": { temperature: 0.5 } } as unknown as Record<string, CategoryConfig>
|
||||
const systemDefaultModel = "anthropic/claude-sonnet-4-5"
|
||||
|
||||
// #when
|
||||
const resolved = resolveCategoryConfig(categoryName, { userCategories, systemDefaultModel })
|
||||
|
||||
// #then - actualModel should be systemDefaultModel
|
||||
expect(resolved).not.toBeNull()
|
||||
expect(resolved!.model).toBe(systemDefaultModel)
|
||||
})
|
||||
|
||||
test("model is undefined when no model available anywhere", () => {
|
||||
// #given - custom category with no model, no systemDefaultModel
|
||||
const categoryName = "my-custom"
|
||||
// Using type assertion since we're testing fallback behavior for categories without model
|
||||
const userCategories = { "my-custom": { temperature: 0.5 } } as unknown as Record<string, CategoryConfig>
|
||||
|
||||
// #when
|
||||
const resolved = resolveCategoryConfig(categoryName, { userCategories })
|
||||
|
||||
// #then - model should be undefined
|
||||
expect(resolved).not.toBeNull()
|
||||
expect(resolved!.model).toBeUndefined()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user