diff --git a/src/agents/sisyphus-junior.ts b/src/agents/sisyphus-junior.ts index 671983a1..690b3eeb 100644 --- a/src/agents/sisyphus-junior.ts +++ b/src/agents/sisyphus-junior.ts @@ -84,13 +84,14 @@ export const SISYPHUS_JUNIOR_DEFAULTS = { } as const export function createSisyphusJuniorAgentWithOverrides( - override: AgentOverrideConfig | undefined + override: AgentOverrideConfig | undefined, + systemDefaultModel?: string ): AgentConfig { if (override?.disable) { override = undefined } - const model = override?.model ?? SISYPHUS_JUNIOR_DEFAULTS.model + const model = override?.model ?? systemDefaultModel ?? SISYPHUS_JUNIOR_DEFAULTS.model const temperature = override?.temperature ?? SISYPHUS_JUNIOR_DEFAULTS.temperature const promptAppend = override?.prompt_append diff --git a/src/agents/utils.ts b/src/agents/utils.ts index 808a6ef3..d831caa8 100644 --- a/src/agents/utils.ts +++ b/src/agents/utils.ts @@ -192,7 +192,7 @@ export function createBuiltinAgents( if (!disabledAgents.includes("orchestrator-sisyphus")) { const orchestratorOverride = agentOverrides["orchestrator-sisyphus"] - const orchestratorModel = orchestratorOverride?.model + const orchestratorModel = orchestratorOverride?.model ?? systemDefaultModel let orchestratorConfig = createOrchestratorSisyphusAgent({ model: orchestratorModel, availableAgents, diff --git a/src/features/task-toast-manager/index.ts b/src/features/task-toast-manager/index.ts index f779eee8..26d91af0 100644 --- a/src/features/task-toast-manager/index.ts +++ b/src/features/task-toast-manager/index.ts @@ -1,2 +1,2 @@ export { TaskToastManager, getTaskToastManager, initTaskToastManager } from "./manager" -export type { TrackedTask, TaskStatus, TaskToastOptions } from "./types" +export type { TrackedTask, TaskStatus, TaskToastOptions, ModelFallbackInfo } from "./types" diff --git a/src/features/task-toast-manager/manager.test.ts b/src/features/task-toast-manager/manager.test.ts index 1e813ba8..069f1851 100644 --- a/src/features/task-toast-manager/manager.test.ts +++ b/src/features/task-toast-manager/manager.test.ts @@ -142,4 +142,109 @@ describe("TaskToastManager", () => { expect(call.body.message).toContain("Running (1):") }) }) + + describe("model fallback info in toast message", () => { + test("should display warning when model falls back to category-default", () => { + // #given - a task with model fallback to category-default + const task = { + id: "task_1", + description: "Task with category default model", + agent: "Sisyphus-Junior", + isBackground: false, + modelInfo: { model: "google/gemini-3-pro-preview", type: "category-default" as const }, + } + + // #when - addTask is called + toastManager.addTask(task) + + // #then - toast should show warning with model info + expect(mockClient.tui.showToast).toHaveBeenCalled() + const call = mockClient.tui.showToast.mock.calls[0][0] + expect(call.body.message).toContain("⚠️") + expect(call.body.message).toContain("google/gemini-3-pro-preview") + expect(call.body.message).toContain("(category default)") + }) + + test("should display warning when model falls back to system-default", () => { + // #given - a task with model fallback to system-default + const task = { + id: "task_1b", + description: "Task with system default model", + agent: "Sisyphus-Junior", + isBackground: false, + modelInfo: { model: "anthropic/claude-sonnet-4-5", type: "system-default" as const }, + } + + // #when - addTask is called + toastManager.addTask(task) + + // #then - toast should show warning with model info + expect(mockClient.tui.showToast).toHaveBeenCalled() + const call = mockClient.tui.showToast.mock.calls[0][0] + expect(call.body.message).toContain("⚠️") + expect(call.body.message).toContain("anthropic/claude-sonnet-4-5") + expect(call.body.message).toContain("(system default)") + }) + + test("should display warning when model is inherited from parent", () => { + // #given - a task with inherited model + const task = { + id: "task_2", + description: "Task with inherited model", + agent: "Sisyphus-Junior", + isBackground: false, + modelInfo: { model: "cliproxy/claude-opus-4-5", type: "inherited" as const }, + } + + // #when - addTask is called + toastManager.addTask(task) + + // #then - toast should show warning with inherited model + expect(mockClient.tui.showToast).toHaveBeenCalled() + const call = mockClient.tui.showToast.mock.calls[0][0] + expect(call.body.message).toContain("⚠️") + expect(call.body.message).toContain("cliproxy/claude-opus-4-5") + expect(call.body.message).toContain("(inherited)") + }) + + test("should not display model info when user-defined", () => { + // #given - a task with user-defined model + const task = { + id: "task_3", + description: "Task with user model", + agent: "Sisyphus-Junior", + isBackground: false, + modelInfo: { model: "my-provider/my-model", type: "user-defined" as const }, + } + + // #when - addTask is called + toastManager.addTask(task) + + // #then - toast should NOT show model warning + expect(mockClient.tui.showToast).toHaveBeenCalled() + const call = mockClient.tui.showToast.mock.calls[0][0] + expect(call.body.message).not.toContain("⚠️ Model:") + expect(call.body.message).not.toContain("(inherited)") + expect(call.body.message).not.toContain("(category default)") + expect(call.body.message).not.toContain("(system default)") + }) + + test("should not display model info when not provided", () => { + // #given - a task without model info + const task = { + id: "task_4", + description: "Task without model info", + agent: "explore", + isBackground: true, + } + + // #when - addTask is called + toastManager.addTask(task) + + // #then - toast should NOT show model warning + expect(mockClient.tui.showToast).toHaveBeenCalled() + const call = mockClient.tui.showToast.mock.calls[0][0] + expect(call.body.message).not.toContain("⚠️ Model:") + }) + }) }) diff --git a/src/features/task-toast-manager/manager.ts b/src/features/task-toast-manager/manager.ts index 66a03b2a..5cb5a7b1 100644 --- a/src/features/task-toast-manager/manager.ts +++ b/src/features/task-toast-manager/manager.ts @@ -1,5 +1,5 @@ import type { PluginInput } from "@opencode-ai/plugin" -import type { TrackedTask, TaskStatus } from "./types" +import type { TrackedTask, TaskStatus, ModelFallbackInfo } from "./types" import type { ConcurrencyManager } from "../background-agent/concurrency" type OpencodeClient = PluginInput["client"] @@ -25,6 +25,7 @@ export class TaskToastManager { isBackground: boolean status?: TaskStatus skills?: string[] + modelInfo?: ModelFallbackInfo }): void { const trackedTask: TrackedTask = { id: task.id, @@ -34,6 +35,7 @@ export class TaskToastManager { startedAt: new Date(), isBackground: task.isBackground, skills: task.skills, + modelInfo: task.modelInfo, } this.tasks.set(task.id, trackedTask) @@ -105,6 +107,19 @@ export class TaskToastManager { const lines: string[] = [] + // Show model fallback warning for the new task if applicable + if (newTask.modelInfo && newTask.modelInfo.type !== "user-defined") { + const icon = "⚠️" + const suffixMap: Partial> = { + inherited: " (inherited)", + "category-default": " (category default)", + "system-default": " (system default)", + } + const suffix = suffixMap[newTask.modelInfo.type] ?? "" + lines.push(`${icon} Model: ${newTask.modelInfo.model}${suffix}`) + lines.push("") + } + if (running.length > 0) { lines.push(`Running (${running.length}):${concurrencyInfo}`) for (const task of running) { diff --git a/src/features/task-toast-manager/types.ts b/src/features/task-toast-manager/types.ts index de4aca0a..33d6f451 100644 --- a/src/features/task-toast-manager/types.ts +++ b/src/features/task-toast-manager/types.ts @@ -1,5 +1,10 @@ export type TaskStatus = "running" | "queued" | "completed" | "error" +export interface ModelFallbackInfo { + model: string + type: "user-defined" | "inherited" | "category-default" | "system-default" +} + export interface TrackedTask { id: string description: string @@ -8,6 +13,7 @@ export interface TrackedTask { startedAt: Date isBackground: boolean skills?: string[] + modelInfo?: ModelFallbackInfo } export interface TaskToastOptions { diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index 96ff156f..55c4f24e 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -154,7 +154,8 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { }; agentConfig["Sisyphus-Junior"] = createSisyphusJuniorAgentWithOverrides( - pluginConfig.agents?.["Sisyphus-Junior"] + pluginConfig.agents?.["Sisyphus-Junior"], + config.model as string | undefined ); if (builderEnabled) { diff --git a/src/tools/sisyphus-task/tools.test.ts b/src/tools/sisyphus-task/tools.test.ts index 58dfe66d..7b3cae68 100644 --- a/src/tools/sisyphus-task/tools.test.ts +++ b/src/tools/sisyphus-task/tools.test.ts @@ -4,8 +4,13 @@ import type { CategoryConfig } from "../../config/schema" function resolveCategoryConfig( categoryName: string, - userCategories?: Record -): { config: CategoryConfig; promptAppend: string } | null { + options: { + userCategories?: Record + 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] ?? "" @@ -14,10 +19,11 @@ function resolveCategoryConfig( return null } + const model = userConfig?.model ?? parentModelString ?? defaultConfig?.model ?? systemDefaultModel const config: CategoryConfig = { ...defaultConfig, ...userConfig, - model: userConfig?.model ?? defaultConfig?.model ?? "anthropic/claude-sonnet-4-5", + model, } let promptAppend = defaultPromptAppend @@ -27,7 +33,7 @@ function resolveCategoryConfig( : userConfig.prompt_append } - return { config, promptAppend } + return { config, promptAppend, model } } describe("sisyphus-task", () => { @@ -114,7 +120,7 @@ describe("sisyphus-task", () => { const categoryName = "unknown-category" // #when - const result = resolveCategoryConfig(categoryName) + const result = resolveCategoryConfig(categoryName, {}) // #then expect(result).toBeNull() @@ -125,7 +131,7 @@ describe("sisyphus-task", () => { const categoryName = "visual-engineering" // #when - const result = resolveCategoryConfig(categoryName) + const result = resolveCategoryConfig(categoryName, {}) // #then expect(result).not.toBeNull() @@ -141,7 +147,7 @@ describe("sisyphus-task", () => { } // #when - const result = resolveCategoryConfig(categoryName, userCategories) + const result = resolveCategoryConfig(categoryName, { userCategories }) // #then expect(result).not.toBeNull() @@ -159,7 +165,7 @@ describe("sisyphus-task", () => { } // #when - const result = resolveCategoryConfig(categoryName, userCategories) + const result = resolveCategoryConfig(categoryName, { userCategories }) // #then expect(result).not.toBeNull() @@ -179,7 +185,7 @@ describe("sisyphus-task", () => { } // #when - const result = resolveCategoryConfig(categoryName, userCategories) + const result = resolveCategoryConfig(categoryName, { userCategories }) // #then expect(result).not.toBeNull() @@ -199,12 +205,53 @@ describe("sisyphus-task", () => { } // #when - const result = resolveCategoryConfig(categoryName, userCategories) + const result = resolveCategoryConfig(categoryName, { userCategories }) // #then expect(result).not.toBeNull() expect(result!.config.temperature).toBe(0.3) }) + + test("parentModelString is used when no user model and takes precedence over default", () => { + // #given + const categoryName = "visual-engineering" + const parentModelString = "cliproxy/claude-opus-4-5" + + // #when + const result = resolveCategoryConfig(categoryName, { parentModelString }) + + // #then + expect(result).not.toBeNull() + expect(result!.config.model).toBe("cliproxy/claude-opus-4-5") + }) + + test("user model takes precedence over parentModelString", () => { + // #given + const categoryName = "visual-engineering" + const userCategories = { + "visual-engineering": { model: "my-provider/my-model" }, + } + const parentModelString = "cliproxy/claude-opus-4-5" + + // #when + const result = resolveCategoryConfig(categoryName, { userCategories, parentModelString }) + + // #then + expect(result).not.toBeNull() + expect(result!.config.model).toBe("my-provider/my-model") + }) + + test("default model is used when no user model and no parentModelString", () => { + // #given + const categoryName = "visual-engineering" + + // #when + const result = resolveCategoryConfig(categoryName, {}) + + // #then + expect(result).not.toBeNull() + expect(result!.config.model).toBe("google/gemini-3-pro-preview") + }) }) describe("category variant", () => { @@ -228,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: {} }), @@ -285,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: {} }), @@ -352,6 +401,7 @@ describe("sisyphus-task", () => { ], }), }, + config: { get: async () => ({}) }, app: { agents: async () => ({ data: [] }), }, @@ -409,6 +459,7 @@ describe("sisyphus-task", () => { data: [], }), }, + config: { get: async () => ({}) }, } const tool = createSisyphusTask({ @@ -460,6 +511,7 @@ describe("sisyphus-task", () => { messages: async () => ({ data: [] }), status: async () => ({ data: {} }), }, + config: { get: async () => ({}) }, app: { agents: async () => ({ data: [{ name: "ultrabrain", mode: "subagent" }] }), }, @@ -518,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" }] }), }, @@ -570,6 +623,7 @@ describe("sisyphus-task", () => { messages: async () => ({ data: [] }), status: async () => ({ data: {} }), }, + config: { get: async () => ({}) }, app: { agents: async () => ({ data: [{ name: "ultrabrain", mode: "subagent" }] }), }, @@ -624,6 +678,7 @@ describe("sisyphus-task", () => { }), status: async () => ({ data: {} }), }, + config: { get: async () => ({}) }, app: { agents: async () => ({ data: [] }) }, } @@ -665,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() @@ -710,4 +765,111 @@ describe("sisyphus-task", () => { expect(result).toContain("\n\n") }) }) + + describe("modelInfo detection via resolveCategoryConfig", () => { + 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 "category-default" + const categoryName = "ultrabrain" + const parentModelString = undefined + + // #when + const resolved = resolveCategoryConfig(categoryName, { parentModelString }) + + // #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 + expect(actualModel).toBe(defaultModel) + expect(actualModel).toBe("openai/gpt-5.2") + }) + + test("when parentModelString is used - modelInfo should report inherited", () => { + // #given + const categoryName = "ultrabrain" + const parentModelString = "cliproxy/claude-opus-4-5" + + // #when + const resolved = resolveCategoryConfig(categoryName, { parentModelString }) + + // #then - actualModel should be parentModelString, type should be "inherited" + expect(resolved).not.toBeNull() + const actualModel = resolved!.config.model + expect(actualModel).toBe(parentModelString) + }) + + test("when user defines model - modelInfo should report user-defined regardless of parentModelString", () => { + // #given + const categoryName = "ultrabrain" + const userCategories = { "ultrabrain": { model: "my-provider/custom-model" } } + const parentModelString = "cliproxy/claude-opus-4-5" + + // #when + const resolved = resolveCategoryConfig(categoryName, { userCategories, parentModelString }) + + // #then - actualModel should be userModel, type should be "user-defined" + expect(resolved).not.toBeNull() + const actualModel = resolved!.config.model + const userDefinedModel = userCategories[categoryName]?.model + expect(actualModel).toBe(userDefinedModel) + expect(actualModel).toBe("my-provider/custom-model") + }) + + test("detection logic: actualModel comparison correctly identifies source", () => { + // #given - This test verifies the fix for PR #770 bug + // The bug was: checking `if (parentModelString)` instead of `if (actualModel === parentModelString)` + const categoryName = "ultrabrain" + const parentModelString = "cliproxy/claude-opus-4-5" + const userCategories = { "ultrabrain": { model: "user/model" } } + + // #when - user model wins + const resolved = resolveCategoryConfig(categoryName, { userCategories, parentModelString }) + const actualModel = resolved!.config.model + const userDefinedModel = userCategories[categoryName]?.model + const defaultModel = DEFAULT_CATEGORIES[categoryName]?.model + + // #then - detection should compare against actual resolved model + const detectedType = actualModel === userDefinedModel + ? "user-defined" + : actualModel === parentModelString + ? "inherited" + : actualModel === defaultModel + ? "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 + 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 + + // #when + const resolved = resolveCategoryConfig(categoryName, { userCategories }) + + // #then - model should be undefined + expect(resolved).not.toBeNull() + expect(resolved!.model).toBeUndefined() + }) + }) }) diff --git a/src/tools/sisyphus-task/tools.ts b/src/tools/sisyphus-task/tools.ts index d4b72079..b8a519ef 100644 --- a/src/tools/sisyphus-task/tools.ts +++ b/src/tools/sisyphus-task/tools.ts @@ -9,6 +9,7 @@ import { findNearestMessageWithFields, findFirstMessageWithAgent, MESSAGE_STORAG import { resolveMultipleSkills } from "../../features/opencode-skill-loader/skill-content" import { createBuiltinSkills } from "../../features/builtin-skills/skills" import { getTaskToastManager } from "../../features/task-toast-manager" +import type { ModelFallbackInfo } from "../../features/task-toast-manager/types" import { subagentSessions, getSessionAgent } from "../../features/claude-code-session-state" import { log } from "../../shared/logger" @@ -60,8 +61,13 @@ type ToolContextWithMetadata = { function resolveCategoryConfig( categoryName: string, - userCategories?: CategoriesConfig -): { config: CategoryConfig; promptAppend: string } | null { + options: { + userCategories?: CategoriesConfig + 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] ?? "" @@ -70,10 +76,13 @@ function resolveCategoryConfig( return null } + // Model priority: user override > parent model (inherit) > category default > system default + // Parent model takes precedence over category default so custom providers work out-of-box + const model = userConfig?.model ?? parentModelString ?? defaultConfig?.model ?? systemDefaultModel const config: CategoryConfig = { ...defaultConfig, ...userConfig, - model: userConfig?.model ?? defaultConfig?.model ?? "anthropic/claude-sonnet-4-5", + model, } let promptAppend = defaultPromptAppend @@ -83,7 +92,7 @@ function resolveCategoryConfig( : userConfig.prompt_append } - return { config, promptAppend } + return { config, promptAppend, model } } export interface SisyphusTaskToolOptions { @@ -325,18 +334,66 @@ ${textContent || "(No text output)"}` return `❌ Invalid arguments: Must provide either category or subagent_type.` } + // Fetch OpenCode config at boundary to get system default model + let systemDefaultModel: string | undefined + try { + const openCodeConfig = await client.config.get() + systemDefaultModel = (openCodeConfig as { model?: string })?.model + } catch { + // Config fetch failed, proceed without system default + systemDefaultModel = undefined + } + let agentToUse: string let categoryModel: { providerID: string; modelID: string; variant?: string } | undefined let categoryPromptAppend: string | undefined + const parentModelString = parentModel + ? `${parentModel.providerID}/${parentModel.modelID}` + : undefined + + let modelInfo: ModelFallbackInfo | undefined + if (args.category) { - const resolved = resolveCategoryConfig(args.category, userCategories) + const resolved = resolveCategoryConfig(args.category, { + userCategories, + parentModelString, + systemDefaultModel, + }) if (!resolved) { return `❌ Unknown category: "${args.category}". Available: ${Object.keys({ ...DEFAULT_CATEGORIES, ...userCategories }).join(", ")}` } + // Determine model source by comparing against the actual resolved model + const actualModel = resolved.model + const userDefinedModel = userCategories?.[args.category]?.model + const categoryDefaultModel = DEFAULT_CATEGORIES[args.category]?.model + + if (!actualModel) { + return `❌ No model configured. Set a model in your OpenCode config, plugin config, or use a category with a default model.` + } + + if (!parseModelString(actualModel)) { + return `❌ Invalid model format "${actualModel}". Expected "provider/model" format (e.g., "anthropic/claude-sonnet-4-5").` + } + + switch (actualModel) { + case userDefinedModel: + modelInfo = { model: actualModel, type: "user-defined" } + break + case parentModelString: + modelInfo = { model: actualModel, type: "inherited" } + break + case categoryDefaultModel: + modelInfo = { model: actualModel, type: "category-default" } + break + case systemDefaultModel: + modelInfo = { model: actualModel, type: "system-default" } + break + } + agentToUse = SISYPHUS_JUNIOR_AGENT - const parsedModel = parseModelString(resolved.config.model) + const parsedModel = parseModelString(actualModel) categoryModel = parsedModel ? (resolved.config.variant ? { ...parsedModel, variant: resolved.config.variant } @@ -344,10 +401,11 @@ ${textContent || "(No text output)"}` : undefined categoryPromptAppend = resolved.promptAppend || undefined } else { - agentToUse = args.subagent_type!.trim() - if (!agentToUse) { + if (!args.subagent_type?.trim()) { return `❌ Agent name cannot be empty.` } + const agentName = args.subagent_type.trim() + agentToUse = agentName // Validate agent exists and is callable (not a primary agent) try { @@ -448,6 +506,7 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id agent: agentToUse, isBackground: false, skills: args.skills, + modelInfo, }) }