diff --git a/assets/oh-my-opencode.schema.json b/assets/oh-my-opencode.schema.json index b215a7c81d..15ff28371d 100644 --- a/assets/oh-my-opencode.schema.json +++ b/assets/oh-my-opencode.schema.json @@ -102,6 +102,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -228,6 +244,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -354,6 +386,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -480,6 +528,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -606,6 +670,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -732,6 +812,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -858,6 +954,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -984,6 +1096,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1110,6 +1238,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1236,6 +1380,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1362,6 +1522,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1488,6 +1664,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1614,6 +1806,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1740,6 +1948,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1866,6 +2090,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, @@ -1999,6 +2239,22 @@ "model": { "type": "string" }, + "fallback": { + "type": "array", + "items": { + "type": "string" + } + }, + "fallbackDelayMs": { + "type": "number", + "minimum": 100, + "maximum": 30000 + }, + "fallbackRetryCount": { + "type": "number", + "minimum": 1, + "maximum": 5 + }, "variant": { "type": "string" }, diff --git a/pr_791_body.md b/pr_791_body.md new file mode 100644 index 0000000000..90cba8aac7 --- /dev/null +++ b/pr_791_body.md @@ -0,0 +1,104 @@ +## Summary + +Adds an automatic model fallback system that gracefully handles API failures by retrying with configured fallback models. + +## Features + +### Automatic Fallback on Transient Errors + +When a model fails due to transient errors, the system automatically retries with fallback models: + +- **Rate limits** (`rate_limit`, HTTP 429) +- **Service unavailable** (HTTP 503, 502) +- **Capacity issues** (`overloaded`, `capacity`, `unavailable`) +- **Network errors** (`timeout`, `ECONNREFUSED`, `ENOTFOUND`) + +Non-model errors (authentication failures, invalid input) are NOT retried. + +### Configuration + +```json +{ + "agents": { + "oracle": { + "model": "openai/gpt-5.2", + "fallback": ["deepseek/deepseek-r1", "anthropic/claude-opus-4-5"], + "fallbackDelayMs": 1000, + "fallbackRetryCount": 3 + } + } +} +``` + +| Option | Default | Description | +|--------|---------|-------------| +| `fallback` | `[]` | Array of fallback models to try | +| `fallbackDelayMs` | `1000` | Delay between retry attempts (100-30000ms) | +| `fallbackRetryCount` | chain length | Max models to try | + +### User Notification + +Users are notified when fallback occurs in the task completion message: + +**Normal completion:** +``` +Task completed in 5.2s. + +Agent: Sisyphus-Junior (category: general) +Model: deepseek/deepseek-v3-2 +Session ID: ses_abc123 +``` + +**Fallback occurred:** +``` +Task completed in 8.1s. + +Agent: Sisyphus-Junior (category: general) +Model: anthropic/claude-haiku-4-5 +⚠️ Model fallback occurred: Primary model failed, used anthropic/claude-haiku-4-5 instead. + Failed models: deepseek/deepseek-v3-2 +Session ID: ses_abc123 +``` + +## Implementation + +### New Module: `src/features/model-fallback/` + +Core utilities for model fallback: +- `parseModelString()` - Parse "provider/model" format +- `buildModelChain()` - Build ordered list of models to try +- `isModelError()` - Detect retryable errors +- `withModelFallback()` - Generic retry wrapper +- `formatRetryErrors()` - Format error list for display + +### Integration with sisyphus-task + +- Updated `resolveCategoryConfig()` to return `modelChain` and `retryConfig` +- Inline retry logic with configurable delay and max attempts +- Special handling: agent configuration errors do NOT trigger fallback +- Model used and fallback status included in completion message + +### Bug Fixes + +- P1: Fixed `maxAttempts: 0` when no category (now defaults to 1) +- P2: Preserve variant on primary model when building modelChain +- P2: Clean up `subagentSessions` on early returns (memory leak) +- P2: Ensure maxAttempts >= 1 even with empty modelChain +- P2: Updated warning text to use generic model descriptions + +## Testing + +- **36 unit tests** covering all model-fallback functions +- All tests passing ✅ +- HTTP transport tests also pass +- Build and typecheck pass + +## Breaking Changes + +None. The feature is backward compatible. Existing configurations without fallback fields continue to work as before. + +## Related + +Fixes PR #785 authorship issue + +PR #790 includes the HTTP transport feature from PR #773 as well. \ No newline at end of file diff --git a/src/config/schema.ts b/src/config/schema.ts index d5fad7e014..5b5e9d6226 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -96,12 +96,12 @@ export const BuiltinCommandNameSchema = z.enum([ ]) export const AgentOverrideConfigSchema = z.object({ - /** @deprecated Use `category` instead. Model is inherited from category defaults. */ model: z.string().optional(), + fallback: z.array(z.string()).optional(), + fallbackDelayMs: z.number().min(100).max(30000).optional(), + fallbackRetryCount: z.number().min(1).max(5).optional(), variant: z.string().optional(), - /** Category name to inherit model and other settings from CategoryConfig */ category: z.string().optional(), - /** Skill names to inject into agent prompt */ skills: z.array(z.string()).optional(), temperature: z.number().min(0).max(2).optional(), top_p: z.number().min(0).max(1).optional(), @@ -155,6 +155,9 @@ export const SisyphusAgentConfigSchema = z.object({ export const CategoryConfigSchema = z.object({ model: z.string(), + fallback: z.array(z.string()).optional(), + fallbackDelayMs: z.number().min(100).max(30000).optional(), + fallbackRetryCount: z.number().min(1).max(5).optional(), variant: z.string().optional(), temperature: z.number().min(0).max(2).optional(), top_p: z.number().min(0).max(1).optional(), diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 5860258fa4..b13aebbebd 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -66,6 +66,7 @@ export class BackgroundManager { log("[background-agent] launch() called with:", { agent: input.agent, model: input.model, + modelChainLength: input.modelChain?.length, description: input.description, parentSessionID: input.parentSessionID, }) @@ -125,6 +126,7 @@ export class BackgroundManager { parentModel: input.parentModel, parentAgent: input.parentAgent, model: input.model, + modelChain: input.modelChain, concurrencyKey, } @@ -153,17 +155,19 @@ export class BackgroundManager { sessionID, agent: input.agent, model: input.model, + modelChainLength: input.modelChain?.length, hasSkillContent: !!input.skillContent, promptLength: input.prompt.length, }) // Use prompt() instead of promptAsync() to properly initialize agent loop (fire-and-forget) - // Include model if caller provided one (e.g., from Sisyphus category configs) + // Include model or modelChain if caller provided one (e.g., from Sisyphus category configs) this.client.session.prompt({ path: { id: sessionID }, body: { agent: input.agent, ...(input.model ? { model: input.model } : {}), + ...(input.modelChain && input.modelChain.length > 0 ? { modelChain: input.modelChain } : {}), system: input.skillContent, tools: { task: false, @@ -178,7 +182,7 @@ export class BackgroundManager { if (existingTask) { existingTask.status = "error" const errorMessage = error instanceof Error ? error.message : String(error) - if (errorMessage.includes("agent.name") || errorMessage.includes("undefined")) { + if (errorMessage.includes("agent.name")) { existingTask.error = `Agent "${input.agent}" not found. Make sure the agent is registered in your opencode.json or provided by a plugin.` } else { existingTask.error = errorMessage diff --git a/src/features/background-agent/types.ts b/src/features/background-agent/types.ts index 8c384211b4..67d1f81f6b 100644 --- a/src/features/background-agent/types.ts +++ b/src/features/background-agent/types.ts @@ -28,6 +28,7 @@ export interface BackgroundTask { progress?: TaskProgress parentModel?: { providerID: string; modelID: string } model?: { providerID: string; modelID: string; variant?: string } + modelChain?: Array<{ providerID: string; modelID: string; variant?: string }> /** Agent name used for concurrency tracking */ concurrencyKey?: string /** Parent session's agent name for notification */ @@ -47,6 +48,7 @@ export interface LaunchInput { parentModel?: { providerID: string; modelID: string } parentAgent?: string model?: { providerID: string; modelID: string; variant?: string } + modelChain?: Array<{ providerID: string; modelID: string; variant?: string }> skills?: string[] skillContent?: string } diff --git a/src/features/model-fallback/index.test.ts b/src/features/model-fallback/index.test.ts new file mode 100644 index 0000000000..041ac86188 --- /dev/null +++ b/src/features/model-fallback/index.test.ts @@ -0,0 +1,498 @@ +import { describe, test, expect, mock } from "bun:test" +import { + parseModelString, + modelSpecToString, + buildModelChain, + isModelError, + withModelFallback, + formatRetryErrors, + type ModelSpec, + type RetryResult, +} from "./index" + +describe("model-fallback", () => { + describe("parseModelString", () => { + test("parses valid provider/model string", () => { + // #given + const modelString = "anthropic/claude-opus-4-5" + + // #when + const result = parseModelString(modelString) + + // #then + expect(result).toBeDefined() + expect(result?.providerID).toBe("anthropic") + expect(result?.modelID).toBe("claude-opus-4-5") + }) + + test("parses model string with nested path", () => { + // #given + const modelString = "a-llm-nextologies/anthropic/claude-opus-4-5" + + // #when + const result = parseModelString(modelString) + + // #then + expect(result).toBeDefined() + expect(result?.providerID).toBe("a-llm-nextologies") + expect(result?.modelID).toBe("anthropic/claude-opus-4-5") + }) + + test("returns undefined for single segment string", () => { + // #given + const modelString = "claude-opus" + + // #when + const result = parseModelString(modelString) + + // #then + expect(result).toBeUndefined() + }) + + test("returns undefined for empty string", () => { + // #given + const modelString = "" + + // #when + const result = parseModelString(modelString) + + // #then + expect(result).toBeUndefined() + }) + }) + + describe("modelSpecToString", () => { + test("converts ModelSpec to string", () => { + // #given + const spec: ModelSpec = { + providerID: "anthropic", + modelID: "claude-opus-4-5", + } + + // #when + const result = modelSpecToString(spec) + + // #then + expect(result).toBe("anthropic/claude-opus-4-5") + }) + + test("handles nested modelID", () => { + // #given + const spec: ModelSpec = { + providerID: "a-llm-nextologies", + modelID: "anthropic/claude-opus-4-5", + } + + // #when + const result = modelSpecToString(spec) + + // #then + expect(result).toBe("a-llm-nextologies/anthropic/claude-opus-4-5") + }) + }) + + describe("buildModelChain", () => { + test("builds chain with primary model only", () => { + // #given + const primary = "anthropic/claude-opus-4-5" + + // #when + const chain = buildModelChain(primary) + + // #then + expect(chain).toHaveLength(1) + expect(chain[0].providerID).toBe("anthropic") + expect(chain[0].modelID).toBe("claude-opus-4-5") + }) + + test("builds chain with primary and fallback models", () => { + // #given + const primary = "anthropic/claude-opus-4-5" + const fallback = ["anthropic/claude-sonnet-4-5", "openai/gpt-5.2"] + + // #when + const chain = buildModelChain(primary, fallback) + + // #then + expect(chain).toHaveLength(3) + expect(chain[0].providerID).toBe("anthropic") + expect(chain[0].modelID).toBe("claude-opus-4-5") + expect(chain[1].providerID).toBe("anthropic") + expect(chain[1].modelID).toBe("claude-sonnet-4-5") + expect(chain[2].providerID).toBe("openai") + expect(chain[2].modelID).toBe("gpt-5.2") + }) + + test("skips invalid model strings in fallback", () => { + // #given + const primary = "anthropic/claude-opus-4-5" + const fallback = ["invalid-model", "openai/gpt-5.2"] + + // #when + const chain = buildModelChain(primary, fallback) + + // #then + expect(chain).toHaveLength(2) + expect(chain[0].modelID).toBe("claude-opus-4-5") + expect(chain[1].modelID).toBe("gpt-5.2") + }) + + test("returns empty chain for invalid primary", () => { + // #given + const primary = "invalid" + + // #when + const chain = buildModelChain(primary) + + // #then + expect(chain).toHaveLength(0) + }) + + test("handles empty fallback array", () => { + // #given + const primary = "anthropic/claude-opus-4-5" + const fallback: string[] = [] + + // #when + const chain = buildModelChain(primary, fallback) + + // #then + expect(chain).toHaveLength(1) + }) + + test("handles undefined fallback", () => { + // #given + const primary = "anthropic/claude-opus-4-5" + + // #when + const chain = buildModelChain(primary, undefined) + + // #then + expect(chain).toHaveLength(1) + }) + }) + + describe("isModelError", () => { + test("returns true for rate limit error", () => { + // #given + const error = new Error("Rate limit exceeded") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for 429 error", () => { + // #given + const error = new Error("Request failed with status 429") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for 503 error", () => { + // #given + const error = new Error("Service unavailable: 503") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for 502 error", () => { + // #given + const error = new Error("Bad gateway: 502") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for timeout error", () => { + // #given + const error = new Error("Request timeout") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for unavailable error", () => { + // #given + const error = new Error("Model is unavailable") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for overloaded error", () => { + // #given + const error = new Error("Server is overloaded") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for capacity error", () => { + // #given + const error = new Error("No capacity available") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for ECONNREFUSED error", () => { + // #given + const error = new Error("connect ECONNREFUSED") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns true for ENOTFOUND error", () => { + // #given + const error = new Error("getaddrinfo ENOTFOUND") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(true) + }) + + test("returns false for non-model errors", () => { + // #given + const error = new Error("Invalid input parameter") + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(false) + }) + + test("returns false for non-Error objects", () => { + // #given + const error = "string error" + + // #when + const result = isModelError(error) + + // #then + expect(result).toBe(false) + }) + + test("returns false for null", () => { + // #given / #when + const result = isModelError(null) + + // #then + expect(result).toBe(false) + }) + }) + + describe("withModelFallback", () => { + test("succeeds on first model", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5", ["openai/gpt-5.2"]) + const operation = mock(async (model: ModelSpec) => `success:${model.modelID}`) + + // #when + const result = await withModelFallback(chain, operation) + + // #then + expect(result.success).toBe(true) + expect(result.result).toBe("success:claude-opus-4-5") + expect(result.attempts).toBe(1) + expect(result.errors).toHaveLength(0) + expect(operation).toHaveBeenCalledTimes(1) + }) + + test("falls back on model error", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5", ["openai/gpt-5.2"]) + let callCount = 0 + const operation = mock(async (model: ModelSpec) => { + callCount++ + if (callCount === 1) { + throw new Error("Rate limit exceeded") + } + return `success:${model.modelID}` + }) + + // #when + const result = await withModelFallback(chain, operation, { retryConfig: { delayMs: 10 } }) + + // #then + expect(result.success).toBe(true) + expect(result.result).toBe("success:gpt-5.2") + expect(result.attempts).toBe(2) + expect(result.errors).toHaveLength(1) + expect(result.errors[0].error).toContain("Rate limit") + }) + + test("does not retry on non-model errors", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5", ["openai/gpt-5.2"]) + const operation = mock(async (_model: ModelSpec) => { + throw new Error("Invalid prompt format") + }) + + // #when + const result = await withModelFallback(chain, operation) + + // #then + expect(result.success).toBe(false) + expect(result.attempts).toBe(1) + expect(result.errors).toHaveLength(1) + expect(operation).toHaveBeenCalledTimes(1) + }) + + test("exhausts all models on persistent failures", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5", [ + "openai/gpt-5.2", + "google/gemini-3-flash", + ]) + const operation = mock(async (_model: ModelSpec) => { + throw new Error("Service unavailable") + }) + + // #when + const result = await withModelFallback(chain, operation, { retryConfig: { delayMs: 10 } }) + + // #then + expect(result.success).toBe(false) + expect(result.attempts).toBe(3) + expect(result.errors).toHaveLength(3) + expect(operation).toHaveBeenCalledTimes(3) + }) + + test("respects maxAttempts config", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5", [ + "openai/gpt-5.2", + "google/gemini-3-flash", + ]) + const operation = mock(async (_model: ModelSpec) => { + throw new Error("Rate limit exceeded") + }) + + // #when + const result = await withModelFallback(chain, operation, { + retryConfig: { maxAttempts: 2, delayMs: 10 }, + }) + + // #then + expect(result.success).toBe(false) + expect(result.attempts).toBe(2) + expect(operation).toHaveBeenCalledTimes(2) + }) + + test("returns error for empty model chain", async () => { + // #given + const chain: ModelSpec[] = [] + const operation = mock(async (_model: ModelSpec) => "success") + + // #when + const result = await withModelFallback(chain, operation) + + // #then + expect(result.success).toBe(false) + expect(result.attempts).toBe(0) + expect(result.errors).toHaveLength(1) + expect(result.errors[0].error).toContain("No models configured") + expect(operation).not.toHaveBeenCalled() + }) + + test("records usedModel on success", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5") + const operation = mock(async (_model: ModelSpec) => "result") + + // #when + const result = await withModelFallback(chain, operation) + + // #then + expect(result.usedModel).toBeDefined() + expect(result.usedModel?.modelID).toBe("claude-opus-4-5") + }) + + test("records usedModel on failure", async () => { + // #given + const chain = buildModelChain("anthropic/claude-opus-4-5") + const operation = mock(async (_model: ModelSpec) => { + throw new Error("Some error") + }) + + // #when + const result = await withModelFallback(chain, operation) + + // #then + expect(result.usedModel).toBeDefined() + expect(result.usedModel?.modelID).toBe("claude-opus-4-5") + }) + }) + + describe("formatRetryErrors", () => { + test("returns 'No errors' for empty array", () => { + // #given + const errors: Array<{ model: string; error: string }> = [] + + // #when + const result = formatRetryErrors(errors) + + // #then + expect(result).toBe("No errors") + }) + + test("formats single error inline", () => { + // #given + const errors = [{ model: "anthropic/claude-opus-4-5", error: "Rate limit exceeded" }] + + // #when + const result = formatRetryErrors(errors) + + // #then + expect(result).toBe("anthropic/claude-opus-4-5: Rate limit exceeded") + }) + + test("formats multiple errors as numbered list", () => { + // #given + const errors = [ + { model: "anthropic/claude-opus-4-5", error: "Rate limit exceeded" }, + { model: "openai/gpt-5.2", error: "Service unavailable" }, + ] + + // #when + const result = formatRetryErrors(errors) + + // #then + expect(result).toContain("1. anthropic/claude-opus-4-5: Rate limit exceeded") + expect(result).toContain("2. openai/gpt-5.2: Service unavailable") + }) + }) +}) diff --git a/src/features/model-fallback/index.ts b/src/features/model-fallback/index.ts new file mode 100644 index 0000000000..b7bd40723c --- /dev/null +++ b/src/features/model-fallback/index.ts @@ -0,0 +1,121 @@ +import { log } from "../../shared/logger" + +export interface ModelSpec { + providerID: string + modelID: string + variant?: string +} + +export function parseModelString(model: string): ModelSpec | undefined { + const parts = model.split("/") + if (parts.length >= 2) { + const providerID = parts[0].trim() + const modelID = parts.slice(1).join("/").trim() + if (providerID.length === 0 || modelID.length === 0) { + return undefined + } + return { providerID, modelID } + } + return undefined +} + +export function modelSpecToString(spec: ModelSpec): string { + return `${spec.providerID}/${spec.modelID}` +} + +export function buildModelChain(primary: string, fallback?: string[]): ModelSpec[] { + const chain: ModelSpec[] = [] + const primarySpec = parseModelString(primary) + if (primarySpec) chain.push(primarySpec) + if (fallback) { + for (const fb of fallback) { + const spec = parseModelString(fb) + if (spec) chain.push(spec) + } + } + return chain +} + +export function isModelError(error: unknown): boolean { + if (!(error instanceof Error)) return false + const msg = error.message.toLowerCase() + return ( + msg.includes("rate limit") || + msg.includes("rate_limit") || + msg.includes("429") || + msg.includes("503") || + msg.includes("502") || + msg.includes("unavailable") || + msg.includes("overloaded") || + msg.includes("capacity") || + msg.includes("timeout") || + msg.includes("econnrefused") || + msg.includes("enotfound") + ) +} + +export interface RetryResult { + success: boolean + result?: T + usedModel?: ModelSpec + attempts: number + errors: Array<{ model: string; error: string }> +} + +export interface RetryConfig { + delayMs?: number + maxAttempts?: number +} + +export async function withModelFallback( + modelChain: ModelSpec[], + operation: (model: ModelSpec) => Promise, + options?: { retryConfig?: RetryConfig; logPrefix?: string } +): Promise> { + const maxAttempts = Math.max(1, options?.retryConfig?.maxAttempts ?? modelChain.length) + const delayMs = options?.retryConfig?.delayMs ?? 1000 + const prefix = options?.logPrefix ?? "[model-fallback]" + const errors: Array<{ model: string; error: string }> = [] + + if (modelChain.length === 0) { + return { success: false, attempts: 0, errors: [{ model: "none", error: "No models configured" }] } + } + if (modelChain.length < maxAttempts) { + log(`${prefix} Warning: maxAttempts (${maxAttempts}) exceeds available models (${modelChain.length})`, { level: "warn" }) + } + + for (let i = 0; i < Math.min(maxAttempts, modelChain.length); i++) { + const model = modelChain[i] + const modelStr = modelSpecToString(model) + + try { + if (i > 0) { + log(`${prefix} Trying fallback model: ${modelStr}`, { attempt: i + 1, maxAttempts }) + } + const result = await operation(model) + if (i > 0) { + log(`${prefix} Fallback succeeded: ${modelStr}`, { attempt: i + 1 }) + } + return { success: true, result, usedModel: model, attempts: i + 1, errors } + } catch (error) { + const errorMsg = error instanceof Error ? error.message : String(error) + errors.push({ model: modelStr, error: errorMsg }) + log(`${prefix} Model ${modelStr} failed: ${errorMsg}`, { attempt: i + 1 }) + + const shouldRetry = isModelError(error) && i < Math.min(maxAttempts, modelChain.length) - 1 + if (!shouldRetry) { + return { success: false, usedModel: model, attempts: i + 1, errors } + } + + await new Promise(resolve => setTimeout(resolve, delayMs)) + } + } + + return { success: false, attempts: Math.min(maxAttempts, modelChain.length), errors } +} + +export function formatRetryErrors(errors: Array<{ model: string; error: string }>): string { + if (errors.length === 0) return "No errors" + if (errors.length === 1) return `${errors[0].model}: ${errors[0].error}` + return errors.map((e, i) => ` ${i + 1}. ${e.model}: ${e.error}`).join("\n") +} diff --git a/src/features/skill-mcp-manager/manager.test.ts b/src/features/skill-mcp-manager/manager.test.ts index 2313e22ee5..510f6d4022 100644 --- a/src/features/skill-mcp-manager/manager.test.ts +++ b/src/features/skill-mcp-manager/manager.test.ts @@ -3,11 +3,29 @@ import { SkillMcpManager } from "./manager" import type { SkillMcpClientInfo, SkillMcpServerContext } from "./types" import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types" +// Mock the MCP SDK transports to avoid network calls +const mockHttpConnect = mock(() => Promise.reject(new Error("Mocked HTTP connection failure"))) +const mockHttpClose = mock(() => Promise.resolve()) + +mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ + StreamableHTTPClientTransport: class MockStreamableHTTPClientTransport { + constructor(public url: URL, public options?: { requestInit?: RequestInit }) {} + async start() { + await mockHttpConnect() + } + async close() { + await mockHttpClose() + } + }, +})) + describe("SkillMcpManager", () => { let manager: SkillMcpManager beforeEach(() => { manager = new SkillMcpManager() + mockHttpConnect.mockClear() + mockHttpClose.mockClear() }) afterEach(async () => { @@ -15,34 +33,296 @@ describe("SkillMcpManager", () => { }) describe("getOrCreateClient", () => { - it("throws error when command is missing", async () => { - // #given - const info: SkillMcpClientInfo = { - serverName: "test-server", - skillName: "test-skill", - sessionID: "session-1", - } - const config: ClaudeCodeMcpServer = {} + describe("configuration validation", () => { + it("throws error when neither url nor command is provided", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "test-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = {} - // #when / #then - await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( - /missing required 'command' field/ - ) + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /no valid connection configuration/ + ) + }) + + it("includes both HTTP and stdio examples in error message", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "my-mcp", + skillName: "data-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = {} + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /HTTP[\s\S]*Stdio/ + ) + }) + + it("includes server and skill names in error message", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "custom-server", + skillName: "custom-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = {} + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /custom-server[\s\S]*custom-skill/ + ) + }) }) - it("includes helpful error message with example when command is missing", async () => { - // #given - const info: SkillMcpClientInfo = { - serverName: "my-mcp", - skillName: "data-skill", - sessionID: "session-1", - } - const config: ClaudeCodeMcpServer = {} + describe("connection type detection", () => { + it("detects HTTP connection from explicit type='http'", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "http-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + type: "http", + url: "https://example.com/mcp", + } - // #when / #then - await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( - /my-mcp[\s\S]*data-skill[\s\S]*Example/ - ) + // #when / #then - should fail at connection, not config validation + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect/ + ) + }) + + it("detects HTTP connection from explicit type='sse'", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "sse-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + type: "sse", + url: "https://example.com/mcp", + } + + // #when / #then - should fail at connection, not config validation + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect/ + ) + }) + + it("detects HTTP connection from url field when type is not specified", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "inferred-http", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://example.com/mcp", + } + + // #when / #then - should fail at connection, not config validation + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect[\s\S]*URL/ + ) + }) + + it("detects stdio connection from explicit type='stdio'", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "stdio-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + type: "stdio", + command: "node", + args: ["-e", "process.exit(0)"], + } + + // #when / #then - should fail at connection, not config validation + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect[\s\S]*Command/ + ) + }) + + it("detects stdio connection from command field when type is not specified", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "inferred-stdio", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + command: "node", + args: ["-e", "process.exit(0)"], + } + + // #when / #then - should fail at connection, not config validation + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect[\s\S]*Command/ + ) + }) + + it("prefers explicit type over inferred type", async () => { + // #given - has both url and command, but type is explicitly stdio + const info: SkillMcpClientInfo = { + serverName: "mixed-config", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + type: "stdio", + url: "https://example.com/mcp", // should be ignored + command: "node", + args: ["-e", "process.exit(0)"], + } + + // #when / #then - should use stdio (show Command in error, not URL) + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Command: node/ + ) + }) + }) + + describe("HTTP connection", () => { + it("throws error for invalid URL", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "bad-url-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + type: "http", + url: "not-a-valid-url", + } + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /invalid URL/ + ) + }) + + it("includes URL in HTTP connection error", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "http-error-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://nonexistent.example.com/mcp", + } + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /https:\/\/nonexistent\.example\.com\/mcp/ + ) + }) + + it("includes helpful hints for HTTP connection failures", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "hint-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://nonexistent.example.com/mcp", + } + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Hints[\s\S]*Verify the URL[\s\S]*authentication headers[\s\S]*MCP over HTTP/ + ) + }) + + it("calls mocked transport connect for HTTP connections", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "mock-test-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://example.com/mcp", + } + + // #when + try { + await manager.getOrCreateClient(info, config) + } catch { + // Expected to fail + } + + // #then - verify mock was called (transport was instantiated) + // The connection attempt happens through the Client.connect() which + // internally calls transport.start() + expect(mockHttpConnect).toHaveBeenCalled() + }) + }) + + describe("stdio connection (backward compatibility)", () => { + it("throws error when command is missing for stdio type", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "missing-command", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + type: "stdio", + // command is missing + } + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /missing 'command' field/ + ) + }) + + it("includes command in stdio connection error", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "test-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + command: "nonexistent-command-xyz", + args: ["--foo"], + } + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /nonexistent-command-xyz --foo/ + ) + }) + + it("includes helpful hints for stdio connection failures", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "test-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + command: "nonexistent-command", + } + + // #when / #then + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Hints[\s\S]*PATH[\s\S]*package exists/ + ) + }) }) }) @@ -156,4 +436,46 @@ describe("SkillMcpManager", () => { } }) }) + + describe("HTTP headers handling", () => { + it("accepts configuration with headers", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "auth-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://example.com/mcp", + headers: { + Authorization: "Bearer test-token", + "X-Custom-Header": "custom-value", + }, + } + + // #when / #then - should fail at connection, not config validation + // Headers are passed through to the transport + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect/ + ) + }) + + it("works without headers (optional)", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "no-auth-server", + skillName: "test-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://example.com/mcp", + // no headers + } + + // #when / #then - should fail at connection, not config validation + await expect(manager.getOrCreateClient(info, config)).rejects.toThrow( + /Failed to connect/ + ) + }) + }) }) diff --git a/src/features/skill-mcp-manager/manager.ts b/src/features/skill-mcp-manager/manager.ts index 089e186eeb..7741b8ee4f 100644 --- a/src/features/skill-mcp-manager/manager.ts +++ b/src/features/skill-mcp-manager/manager.ts @@ -1,16 +1,60 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js" import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js" +import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js" import type { Tool, Resource, Prompt } from "@modelcontextprotocol/sdk/types.js" import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types" import { expandEnvVarsInObject } from "../claude-code-mcp-loader/env-expander" import { createCleanMcpEnvironment } from "./env-cleaner" import type { SkillMcpClientInfo, SkillMcpServerContext } from "./types" -interface ManagedClient { +/** + * Connection type for a managed MCP client. + * - "stdio": Local process via stdin/stdout + * - "http": Remote server via HTTP (Streamable HTTP transport) + */ +type ConnectionType = "stdio" | "http" + +interface ManagedClientBase { client: Client - transport: StdioClientTransport skillName: string lastUsedAt: number + connectionType: ConnectionType +} + +interface ManagedStdioClient extends ManagedClientBase { + connectionType: "stdio" + transport: StdioClientTransport +} + +interface ManagedHttpClient extends ManagedClientBase { + connectionType: "http" + transport: StreamableHTTPClientTransport +} + +type ManagedClient = ManagedStdioClient | ManagedHttpClient + +/** + * Determines connection type from MCP server configuration. + * Priority: explicit type field > url presence > command presence + */ +function getConnectionType(config: ClaudeCodeMcpServer): ConnectionType | null { + // Explicit type takes priority + if (config.type === "http" || config.type === "sse") { + return "http" + } + if (config.type === "stdio") { + return "stdio" + } + + // Infer from available fields + if (config.url) { + return "http" + } + if (config.command) { + return "stdio" + } + + return null } export class SkillMcpManager { @@ -98,18 +142,125 @@ export class SkillMcpManager { private async createClient( info: SkillMcpClientInfo, config: ClaudeCodeMcpServer + ): Promise { + const connectionType = getConnectionType(config) + + if (!connectionType) { + throw new Error( + `MCP server "${info.serverName}" has no valid connection configuration.\n\n` + + `The MCP configuration in skill "${info.skillName}" must specify either:\n` + + ` - A URL for HTTP connection (remote MCP server)\n` + + ` - A command for stdio connection (local MCP process)\n\n` + + `Examples:\n` + + ` HTTP:\n` + + ` mcp:\n` + + ` ${info.serverName}:\n` + + ` url: https://mcp.example.com/mcp\n` + + ` headers:\n` + + ` Authorization: Bearer \${API_KEY}\n\n` + + ` Stdio:\n` + + ` mcp:\n` + + ` ${info.serverName}:\n` + + ` command: npx\n` + + ` args: [-y, @some/mcp-server]` + ) + } + + if (connectionType === "http") { + return this.createHttpClient(info, config) + } else { + return this.createStdioClient(info, config) + } + } + + /** + * Create an HTTP-based MCP client using StreamableHTTPClientTransport. + * Supports remote MCP servers with optional authentication headers. + */ + private async createHttpClient( + info: SkillMcpClientInfo, + config: ClaudeCodeMcpServer + ): Promise { + const key = this.getClientKey(info) + + if (!config.url) { + throw new Error( + `MCP server "${info.serverName}" is configured for HTTP but missing 'url' field.` + ) + } + + let url: URL + try { + url = new URL(config.url) + } catch { + throw new Error( + `MCP server "${info.serverName}" has invalid URL: ${config.url}\n\n` + + `Expected a valid URL like: https://mcp.example.com/mcp` + ) + } + + this.registerProcessCleanup() + + // Build request init with headers if provided + const requestInit: RequestInit = {} + if (config.headers && Object.keys(config.headers).length > 0) { + requestInit.headers = config.headers + } + + const transport = new StreamableHTTPClientTransport(url, { + requestInit: Object.keys(requestInit).length > 0 ? requestInit : undefined, + }) + + const client = new Client( + { name: `skill-mcp-${info.skillName}-${info.serverName}`, version: "1.0.0" }, + { capabilities: {} } + ) + + try { + await client.connect(transport) + } catch (error) { + try { + await transport.close() + } catch { + // Transport may already be closed + } + const errorMessage = error instanceof Error ? error.message : String(error) + throw new Error( + `Failed to connect to MCP server "${info.serverName}".\n\n` + + `URL: ${config.url}\n` + + `Reason: ${errorMessage}\n\n` + + `Hints:\n` + + ` - Verify the URL is correct and the server is running\n` + + ` - Check if authentication headers are required\n` + + ` - Ensure the server supports MCP over HTTP` + ) + } + + const managedClient: ManagedHttpClient = { + client, + transport, + skillName: info.skillName, + lastUsedAt: Date.now(), + connectionType: "http", + } + this.clients.set(key, managedClient) + this.startCleanupTimer() + return client + } + + /** + * Create a stdio-based MCP client using StdioClientTransport. + * Spawns a local process and communicates via stdin/stdout. + */ + private async createStdioClient( + info: SkillMcpClientInfo, + config: ClaudeCodeMcpServer ): Promise { const key = this.getClientKey(info) if (!config.command) { throw new Error( - `MCP server "${info.serverName}" is missing required 'command' field.\n\n` + - `The MCP configuration in skill "${info.skillName}" must specify a command to execute.\n\n` + - `Example:\n` + - ` mcp:\n` + - ` ${info.serverName}:\n` + - ` command: npx\n` + - ` args: [-y, @some/mcp-server]` + `MCP server "${info.serverName}" is configured for stdio but missing 'command' field.` ) } @@ -153,7 +304,14 @@ export class SkillMcpManager { ) } - this.clients.set(key, { client, transport, skillName: info.skillName, lastUsedAt: Date.now() }) + const managedClient: ManagedStdioClient = { + client, + transport, + skillName: info.skillName, + lastUsedAt: Date.now(), + connectionType: "stdio", + } + this.clients.set(key, managedClient) this.startCleanupTimer() return client } diff --git a/src/tools/sisyphus-task/tools.ts b/src/tools/sisyphus-task/tools.ts index b8a519ef9c..91df9d4fc3 100644 --- a/src/tools/sisyphus-task/tools.ts +++ b/src/tools/sisyphus-task/tools.ts @@ -12,6 +12,7 @@ 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" +import { isModelError, type ModelSpec, type RetryConfig, buildModelChain } from "../../features/model-fallback" type OpencodeClient = PluginInput["client"] @@ -59,6 +60,13 @@ type ToolContextWithMetadata = { metadata?: (input: { title?: string; metadata?: Record }) => void } +interface ResolvedCategoryConfig { + config: CategoryConfig + promptAppend: string + modelChain: ModelSpec[] + retryConfig: RetryConfig +} + function resolveCategoryConfig( categoryName: string, options: { @@ -66,7 +74,7 @@ function resolveCategoryConfig( parentModelString?: string systemDefaultModel?: string } -): { config: CategoryConfig; promptAppend: string; model: string | undefined } | null { +): { config: CategoryConfig; promptAppend: string; model: string | undefined; modelChain: ModelSpec[]; retryConfig: RetryConfig } | null { const { userCategories, parentModelString, systemDefaultModel } = options const defaultConfig = DEFAULT_CATEGORIES[categoryName] const userConfig = userCategories?.[categoryName] @@ -85,6 +93,19 @@ function resolveCategoryConfig( model, } + const fallbackList = userConfig?.fallback ?? defaultConfig?.fallback + let modelChain = buildModelChain(config.model, fallbackList) + + // Preserve variant on primary model if configured + if (config.variant && modelChain.length > 0) { + modelChain = [{ ...modelChain[0], variant: config.variant }, ...modelChain.slice(1)] + } + + const retryConfig: RetryConfig = { + delayMs: userConfig?.fallbackDelayMs ?? defaultConfig?.fallbackDelayMs ?? 1000, + maxAttempts: userConfig?.fallbackRetryCount ?? defaultConfig?.fallbackRetryCount ?? Math.max(modelChain.length, 1), + } + let promptAppend = defaultPromptAppend if (userConfig?.prompt_append) { promptAppend = defaultPromptAppend @@ -92,7 +113,7 @@ function resolveCategoryConfig( : userConfig.prompt_append } - return { config, promptAppend, model } + return { config, promptAppend, model, modelChain, retryConfig } } export interface SisyphusTaskToolOptions { @@ -347,6 +368,8 @@ ${textContent || "(No text output)"}` let agentToUse: string let categoryModel: { providerID: string; modelID: string; variant?: string } | undefined let categoryPromptAppend: string | undefined + let modelChain: ModelSpec[] = [] + let retryConfig: RetryConfig = { delayMs: 1000, maxAttempts: 1 } const parentModelString = parentModel ? `${parentModel.providerID}/${parentModel.modelID}` @@ -400,6 +423,8 @@ ${textContent || "(No text output)"}` : parsedModel) : undefined categoryPromptAppend = resolved.promptAppend || undefined + modelChain = resolved.modelChain + retryConfig = resolved.retryConfig } else { if (!args.subagent_type?.trim()) { return `❌ Agent name cannot be empty.` @@ -445,6 +470,7 @@ ${textContent || "(No text output)"}` parentModel, parentAgent, model: categoryModel, + modelChain, skills: args.skills, skillContent: systemContent, }) @@ -515,30 +541,76 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id metadata: { sessionId: sessionID, category: args.category, sync: true }, }) - try { - await client.session.prompt({ - path: { id: sessionID }, - body: { - agent: agentToUse, - system: systemContent, - tools: { - task: false, - sisyphus_task: false, - call_omo_agent: true, + const modelsToTry = modelChain.length > 0 ? modelChain : (categoryModel ? [categoryModel] : []) + const maxAttempts = retryConfig.maxAttempts ?? modelsToTry.length + const delayMs = retryConfig.delayMs ?? 1000 + let promptSuccess = false + let usedModel: ModelSpec | undefined = categoryModel + const failedModels: Array<{ model: string; error: string }> = [] + + for (let attempt = 0; attempt < Math.min(Math.max(modelsToTry.length, 1), maxAttempts); attempt++) { + const currentModel = modelsToTry[attempt] + const modelStr = currentModel ? `${currentModel.providerID}/${currentModel.modelID}` : "default" + + try { + if (attempt > 0) { + log(`[sisyphus_task] Retrying with fallback model: ${modelStr}`, { attempt: attempt + 1, sessionID }) + } + + await client.session.prompt({ + path: { id: sessionID }, + body: { + agent: agentToUse, + system: systemContent, + tools: { + task: false, + sisyphus_task: false, + call_omo_agent: true, + }, + parts: [{ type: "text", text: args.prompt }], + ...(currentModel ? { model: currentModel } : {}), }, - parts: [{ type: "text", text: args.prompt }], - ...(categoryModel ? { model: categoryModel } : {}), - }, - }) - } catch (promptError) { + }) + promptSuccess = true + usedModel = currentModel + if (attempt > 0) { + log(`[sisyphus_task] Fallback succeeded with: ${modelStr}`, { attempt: attempt + 1 }) + } + break + } catch (promptError) { + const errorMessage = promptError instanceof Error ? promptError.message : String(promptError) + failedModels.push({ model: modelStr, error: errorMessage }) + + // Only abort on agent configuration errors, not transient errors + if (errorMessage.includes("agent.name")) { + if (toastManager && taskId !== undefined) { + toastManager.removeTask(taskId) + } + subagentSessions.delete(sessionID) + return `❌ Agent "${agentToUse}" not found. Make sure the agent is registered in your opencode.json or provided by a plugin.\n\nSession ID: ${sessionID}` + } + + const attemptsRemaining = Math.min(modelsToTry.length, maxAttempts) - attempt - 1 + if (!isModelError(promptError) || attemptsRemaining <= 0) { + if (toastManager && taskId !== undefined) { + toastManager.removeTask(taskId) + } + subagentSessions.delete(sessionID) + const allErrors = failedModels.map((f, i) => ` ${i + 1}. ${f.model}: ${f.error}`).join("\n") + return `❌ Failed to send prompt (tried ${failedModels.length} model${failedModels.length > 1 ? "s" : ""}):\n${allErrors}\n\nSession ID: ${sessionID}` + } + + log(`[sisyphus_task] Model ${modelStr} failed, trying fallback...`, { error: errorMessage }) + await new Promise(resolve => setTimeout(resolve, delayMs)) + } + } + + if (!promptSuccess) { if (toastManager && taskId !== undefined) { toastManager.removeTask(taskId) } - const errorMessage = promptError instanceof Error ? promptError.message : String(promptError) - if (errorMessage.includes("agent.name") || errorMessage.includes("undefined")) { - return `❌ Agent "${agentToUse}" not found. Make sure the agent is registered in your opencode.json or provided by a plugin.\n\nSession ID: ${sessionID}` - } - return `❌ Failed to send prompt: ${errorMessage}\n\nSession ID: ${sessionID}` + subagentSessions.delete(sessionID) + return `❌ All models failed.\n\nSession ID: ${sessionID}` } // Poll for session completion with stability detection @@ -644,9 +716,15 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id subagentSessions.delete(sessionID) + const usedModelStr = usedModel ? `${usedModel.providerID}/${usedModel.modelID}` : "default" + const fallbackNote = failedModels.length > 0 + ? `\n⚠️ Model fallback occurred: Primary model failed, used ${usedModelStr} instead.\n Failed models: ${failedModels.map(f => f.model).join(", ")}` + : "" + return `Task completed in ${duration}. Agent: ${agentToUse}${args.category ? ` (category: ${args.category})` : ""} +Model: ${usedModelStr}${fallbackNote} Session ID: ${sessionID} ---