Browse Source

Merge pull request #539 from adikpb/fallback-consolidation

cleanup: remove redundant fallback.chains config
Alvin 1 month ago
parent
commit
302d9da5f3

+ 1 - 2
docs/configuration.md

@@ -118,10 +118,9 @@ Presets can also be switched at runtime without restarting using the `/preset` c
 | `backgroundJobs.readContextMinLines` | integer | `10` | Minimum number of lines read from a file before it appears in reusable background-job context (0–1000) |
 | `backgroundJobs.readContextMaxFiles` | integer | `8` | Maximum number of recent read-context files shown per reusable child session (0–50) |
 | `disabled_mcps` | string[] | `[]` | MCP server IDs to disable globally |
-| `fallback.enabled` | boolean | `false` | Enable model failover on timeout/error |
+| `fallback.enabled` | boolean | `true` | Enable model failover on timeout/error |
 | `fallback.timeoutMs` | number | `15000` | Time before aborting and trying next model |
 | `fallback.retryDelayMs` | number | `500` | Delay between retry attempts |
-| `fallback.chains.<agent>` | string[] | — | Ordered fallback model IDs for an agent |
 | `fallback.retry_on_empty` | boolean | `true` | Treat silent empty provider responses (0 tokens) as failures and retry. Set `false` to accept empty responses |
 | `council.presets` | object | — | **Required if using council.** Named councillor presets |
 | `council.presets.<name>.<councillor>.model` | string | — | Councillor model |

+ 12 - 1
oh-my-opencode-slim.schema.json

@@ -337,12 +337,23 @@
           "default": true,
           "type": "boolean"
         },
+        "timeoutMs": {
+          "default": 15000,
+          "type": "number",
+          "minimum": 0
+        },
+        "retryDelayMs": {
+          "default": 500,
+          "type": "number",
+          "minimum": 0
+        },
         "retry_on_empty": {
           "default": true,
           "description": "When true (default), empty provider responses are treated as failures, triggering fallback/retry. Set to false to treat them as successes.",
           "type": "boolean"
         }
-      }
+      },
+      "additionalProperties": false
     },
     "council": {
       "type": "object",

+ 1 - 1
src/codemap.md

@@ -19,7 +19,7 @@
 - Startup:
   - `loadPluginConfig` builds effective config from user/project presets.
   - `createAgents` + `getAgentConfigs` construct final agent registry and resolved prompts.
-  - Runtime model chains are built from configured arrays plus fallback chains.
+  - Runtime model chains are built from `_modelArray` entries (when users configure `model` as an array in `agents.<name>`).
   - `SubagentDepthTracker`, shared `BackgroundJobBoard`, `MultiplexerSessionManager`, `CouncilManager`, `ForegroundFallbackManager`, and hook factories are initialized before registration.
 - Plugin registration: `index.ts` merges/overlays agent configs into OpenCode's config, registers tools (`council`, `webfetch`, `ast_grep_*`, todo tools), MCPs (`createBuiltinMcps`), and all hook handlers (`event`, `tool.execute.before/after`, `experimental.chat.system/messages.transform`, `command.execute.before`, etc.).
 - Runtime event flow (`event`): updates depth tree, multiplexer pane state, auto-update checks, interview/preset state, and task-session cleanup for deleted sessions.

+ 1 - 1
src/config/codemap.md

@@ -65,7 +65,7 @@ resolution, and helper APIs used by agents, council, and runtime subsystems.
   - `CouncilConfigSchema` now normalizes deprecated `master*` fields into
     `_legacyMasterModel` metadata for compatibility
   - supports presets + timeout/retry/execution mode.
-- Fallback config supports per-agent chain arrays and retry/backoff values.
+- Fallback config supports retry/backoff values and toggles.
 
 ## Control flow and dependencies
 

+ 0 - 34
src/config/fallback-chains.ts

@@ -1,34 +0,0 @@
-export function normalizeFallbackChainsForPreset(
-  chains: Record<string, string[] | undefined>,
-  presetName: string | null | undefined,
-): Record<string, string[]> {
-  const normalized: Record<string, string[]> = {};
-
-  for (const [rawKey, chainModels] of Object.entries(chains)) {
-    if (!chainModels?.length) continue;
-
-    const separatorIndex = rawKey.indexOf(':');
-    const hasPresetScope = separatorIndex !== -1;
-    const scopedPreset = hasPresetScope ? rawKey.slice(0, separatorIndex) : '';
-    const agentName = hasPresetScope
-      ? rawKey.slice(separatorIndex + 1)
-      : rawKey;
-
-    if (!agentName) continue;
-    if (hasPresetScope && scopedPreset !== presetName) continue;
-
-    const existing = normalized[agentName] ?? [];
-    const seen = new Set(existing);
-    for (const chainModel of chainModels) {
-      if (seen.has(chainModel)) continue;
-      seen.add(chainModel);
-      existing.push(chainModel);
-    }
-
-    if (existing.length > 0) {
-      normalized[agentName] = existing;
-    }
-  }
-
-  return normalized;
-}

+ 0 - 196
src/config/model-resolution.test.ts

@@ -1,6 +1,5 @@
 import { describe, expect, test } from 'bun:test';
 import type { ModelEntry } from '../config/schema';
-import { normalizeFallbackChainsForPreset } from './fallback-chains';
 
 /**
  * Test the model array resolution logic that runs in the config hook.
@@ -66,198 +65,3 @@ describe('model array resolution', () => {
     expect(result).toBeNull();
   });
 });
-
-/**
- * Tests for the fallback.chains merging logic that runs in the config hook.
- * Mirrors the effectiveArrays construction in src/index.ts.
- */
-describe('fallback.chains merging for foreground agents', () => {
-  /**
-   * Simulates the effectiveArrays construction + resolution from src/index.ts.
-   * Returns the resolved model string or null.
-   */
-  function resolveWithChains(opts: {
-    modelArray?: Array<{ id: string; variant?: string }>;
-    currentModel?: string;
-    chainModels?: string[];
-    preset?: string;
-    fallbackEnabled?: boolean;
-  }): string | null {
-    const {
-      modelArray,
-      currentModel,
-      chainModels,
-      preset,
-      fallbackEnabled = true,
-    } = opts;
-
-    // Build effectiveArrays (mirrors index.ts logic)
-    const effectiveArray: Array<{ id: string; variant?: string }> = modelArray
-      ? [...modelArray]
-      : [];
-
-    if (fallbackEnabled && chainModels && chainModels.length > 0) {
-      const normalized = normalizeFallbackChainsForPreset(
-        {
-          [currentModel ?? 'orchestrator']: chainModels,
-        },
-        preset,
-      );
-      const normalizedModels = Object.values(normalized)[0] ?? [];
-      if (effectiveArray.length === 0 && currentModel) {
-        effectiveArray.push({ id: currentModel });
-      }
-      const seen = new Set(effectiveArray.map((m) => m.id));
-      for (const chainModel of normalizedModels) {
-        if (!seen.has(chainModel)) {
-          seen.add(chainModel);
-          effectiveArray.push({ id: chainModel });
-        }
-      }
-    }
-
-    if (effectiveArray.length === 0) return null;
-
-    // Resolution: always use first model in effective array
-    return effectiveArray[0].id;
-  }
-
-  test('primary model wins regardless of provider config', () => {
-    const result = resolveWithChains({
-      currentModel: 'anthropic/claude-opus-4-5',
-      chainModels: ['openai/gpt-4o'],
-    });
-    expect(result).toBe('anthropic/claude-opus-4-5');
-  });
-
-  test('chain is ignored when fallback disabled', () => {
-    const result = resolveWithChains({
-      currentModel: 'anthropic/claude-opus-4-5',
-      chainModels: ['openai/gpt-4o'],
-      fallbackEnabled: false,
-    });
-    // chain not applied; no effectiveArray entry → falls through to null (no _modelArray either)
-    expect(result).toBeNull();
-  });
-
-  test('_modelArray entries take precedence and chain appends after', () => {
-    const result = resolveWithChains({
-      modelArray: [
-        { id: 'anthropic/claude-opus-4-5' },
-        { id: 'anthropic/claude-sonnet-4-5' },
-      ],
-      chainModels: ['openai/gpt-4o'],
-    });
-    // First entry in _modelArray wins; chain only used for runtime failover
-    expect(result).toBe('anthropic/claude-opus-4-5');
-  });
-
-  test('duplicate model ids across array and chain are deduplicated', () => {
-    const result = resolveWithChains({
-      modelArray: [
-        { id: 'anthropic/claude-opus-4-5' },
-        { id: 'openai/gpt-4o' },
-      ],
-      chainModels: ['openai/gpt-4o', 'google/gemini-pro'],
-    });
-    expect(result).toBe('anthropic/claude-opus-4-5');
-  });
-
-  test('no currentModel and no _modelArray with chain still resolves', () => {
-    const result = resolveWithChains({
-      chainModels: ['openai/gpt-4o', 'anthropic/claude-sonnet-4-5'],
-    });
-    expect(result).toBe('openai/gpt-4o');
-  });
-
-  test('built-in provider not skipped when other providers are configured', () => {
-    // Regression test: github-copilot is auto-loaded by opencode and doesn't
-    // need an entry in opencodeConfig.provider. The resolver must not skip
-    // it in favor of a configured provider later in the chain.
-    const result = resolveWithChains({
-      currentModel: 'github-copilot/claude-opus-4.6',
-      chainModels: [
-        'github-copilot/gemini-3.1-pro-preview',
-        'zai-coding-plan/glm-5',
-      ],
-    });
-    expect(result).toBe('github-copilot/claude-opus-4.6');
-  });
-
-  test('normalizes scoped fallback chains for the active preset', () => {
-    const result = normalizeFallbackChainsForPreset(
-      {
-        'gpt-plus-max:orchestrator': [
-          'openai/o3',
-          'anthropic/claude-sonnet-4-6',
-        ],
-        'gpt-plus-max:oracle': ['anthropic/claude-opus-4-5'],
-      },
-      'gpt-plus-max',
-    );
-
-    expect(result).toEqual({
-      orchestrator: ['openai/o3', 'anthropic/claude-sonnet-4-6'],
-      oracle: ['anthropic/claude-opus-4-5'],
-    });
-  });
-
-  test('ignores scoped fallback chains for other presets', () => {
-    const result = normalizeFallbackChainsForPreset(
-      {
-        'gpt-plus-max:orchestrator': ['openai/o3'],
-        orchestrator: ['anthropic/claude-sonnet-4-6'],
-      },
-      'other-preset',
-    );
-
-    expect(result).toEqual({
-      orchestrator: ['anthropic/claude-sonnet-4-6'],
-    });
-  });
-
-  test('does not emit scoped keys in normalized output', () => {
-    const result = normalizeFallbackChainsForPreset(
-      {
-        'gpt-plus-max:orchestrator': ['openai/o3'],
-      },
-      'gpt-plus-max',
-    );
-
-    expect(Object.keys(result).some((key) => key.includes(':'))).toBe(false);
-  });
-
-  test('ignores empty fallback chain agent names', () => {
-    const result = normalizeFallbackChainsForPreset(
-      {
-        '': ['openai/o3'],
-        'gpt-plus-max:': ['anthropic/claude-sonnet-4-6'],
-        orchestrator: ['ustc-deepseek/deepseek-v4-pro'],
-      },
-      'gpt-plus-max',
-    );
-
-    expect(result).toEqual({
-      orchestrator: ['ustc-deepseek/deepseek-v4-pro'],
-    });
-    expect(result).not.toHaveProperty('');
-  });
-
-  test('runtime preset takes precedence over config preset for scoped fallback chains', () => {
-    const configPreset = 'cheap';
-    const runtimePreset = 'powerful';
-    const activePreset = runtimePreset ?? configPreset ?? null;
-
-    const result = normalizeFallbackChainsForPreset(
-      {
-        'cheap:orchestrator': ['cheap/model'],
-        'powerful:orchestrator': ['powerful/model'],
-      },
-      activePreset,
-    );
-
-    expect(result).toEqual({
-      orchestrator: ['powerful/model'],
-    });
-  });
-});

+ 54 - 22
src/config/schema.ts

@@ -2,7 +2,7 @@ import { z } from 'zod';
 import { AGENT_ALIASES, ALL_AGENT_NAMES } from './constants';
 import { CouncilConfigSchema } from './council-schema';
 
-const FALLBACK_AGENT_NAMES = [
+const MANUAL_AGENT_NAMES = [
   'orchestrator',
   'oracle',
   'designer',
@@ -11,20 +11,49 @@ const FALLBACK_AGENT_NAMES = [
   'fixer',
 ] as const;
 
-const AgentModelChainSchema = z.array(z.string()).min(1);
+export const ProviderModelIdSchema = z
+  .string()
+  .regex(
+    /^[^/\s]+\/[^\s]+$/,
+    'Expected provider/model format (provider/.../model)',
+  );
 
-const FallbackChainsSchema = z
+export const ManualAgentPlanSchema = z
   .object({
-    orchestrator: AgentModelChainSchema.optional(),
-    oracle: AgentModelChainSchema.optional(),
-    designer: AgentModelChainSchema.optional(),
-    explorer: AgentModelChainSchema.optional(),
-    librarian: AgentModelChainSchema.optional(),
-    fixer: AgentModelChainSchema.optional(),
+    primary: ProviderModelIdSchema,
+    fallback1: ProviderModelIdSchema,
+    fallback2: ProviderModelIdSchema,
+    fallback3: ProviderModelIdSchema,
   })
-  .catchall(AgentModelChainSchema);
+  .superRefine((value, ctx) => {
+    const unique = new Set([
+      value.primary,
+      value.fallback1,
+      value.fallback2,
+      value.fallback3,
+    ]);
+    if (unique.size !== 4) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: 'primary and fallbacks must be unique per agent',
+      });
+    }
+  });
 
-export type FallbackAgentName = (typeof FALLBACK_AGENT_NAMES)[number];
+export const ManualPlanSchema = z
+  .object({
+    orchestrator: ManualAgentPlanSchema,
+    oracle: ManualAgentPlanSchema,
+    designer: ManualAgentPlanSchema,
+    explorer: ManualAgentPlanSchema,
+    librarian: ManualAgentPlanSchema,
+    fixer: ManualAgentPlanSchema,
+  })
+  .strict();
+
+export type ManualAgentName = (typeof MANUAL_AGENT_NAMES)[number];
+export type ManualAgentPlan = z.infer<typeof ManualAgentPlanSchema>;
+export type ManualPlan = z.infer<typeof ManualPlanSchema>;
 
 // Agent override configuration (distinct from SDK's AgentConfig)
 export const AgentOverrideConfigSchema = z
@@ -141,17 +170,20 @@ export const BackgroundJobsConfigSchema = z.object({
 
 export type BackgroundJobsConfig = z.infer<typeof BackgroundJobsConfigSchema>;
 
-export const FailoverConfigSchema = z.object({
-  enabled: z.boolean().default(true),
-  chains: FallbackChainsSchema.default({}),
-  retry_on_empty: z
-    .boolean()
-    .default(true)
-    .describe(
-      'When true (default), empty provider responses are treated as failures, ' +
-        'triggering fallback/retry. Set to false to treat them as successes.',
-    ),
-});
+export const FailoverConfigSchema = z
+  .object({
+    enabled: z.boolean().default(true),
+    timeoutMs: z.number().min(0).default(15000),
+    retryDelayMs: z.number().min(0).default(500),
+    retry_on_empty: z
+      .boolean()
+      .default(true)
+      .describe(
+        'When true (default), empty provider responses are treated as failures, ' +
+          'triggering fallback/retry. Set to false to treat them as successes.',
+      ),
+  })
+  .strict();
 
 export type FailoverConfig = z.infer<typeof FailoverConfigSchema>;
 

+ 1 - 1
src/hooks/foreground-fallback/index.ts

@@ -74,7 +74,7 @@ const REPROMPT_DELAY_MS = 500;
  * Manages runtime model fallback for foreground agent sessions.
  *
  * Constructed at plugin init with the ordered fallback chains for each agent
- * (built from _modelArray entries merged with fallback.chains config).
+ * (built from _modelArray entries in agents.<name>.model).
  */
 export class ForegroundFallbackManager {
   /** sessionID → last observed model string ("providerID/modelID") */

+ 11 - 90
src/index.ts

@@ -10,7 +10,6 @@ import {
 } from './config';
 import { parseList } from './config/agent-mcps';
 import { AGENT_ALIASES } from './config/constants';
-import { normalizeFallbackChainsForPreset } from './config/fallback-chains';
 import {
   getActiveRuntimePreset,
   getPreviousRuntimePreset,
@@ -178,48 +177,20 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     agentDefs = createAgents(config);
     agents = getAgentConfigs(config);
 
-    // Build a map of agent name → priority model array for runtime
-    // fallback. Populated when the user configures model as an array in
-    // their plugin config.
+    // Build model array map and runtime fallback chains from _modelArray
+    // entries (when the user configures model as an array in
+    // agents.<name>.model). A single pass populates both data structures.
     modelArrayMap = {} as Record<
       string,
       Array<{ id: string; variant?: string }>
     >;
-    for (const agentDef of agentDefs) {
-      if (agentDef._modelArray && agentDef._modelArray.length > 0) {
-        modelArrayMap[agentDef.name] = agentDef._modelArray;
-      }
-    }
-    // Build runtime fallback chains for all foreground agents. Each chain
-    // is an ordered list of model strings to try when the current model is
-    // rate-limited. Seeds from _modelArray entries (when the user
-    // configures model as an array), then appends fallback.chains entries.
     runtimeChains = {} as Record<string, string[]>;
     for (const agentDef of agentDefs) {
       if (agentDef._modelArray?.length) {
+        modelArrayMap[agentDef.name] = agentDef._modelArray;
         runtimeChains[agentDef.name] = agentDef._modelArray.map((m) => m.id);
       }
     }
-    const activePresetForFallback =
-      getActiveRuntimePreset() ?? config.preset ?? null;
-
-    if (config.fallback?.enabled !== false) {
-      const chains = normalizeFallbackChainsForPreset(
-        (config.fallback?.chains as Record<string, string[] | undefined>) ?? {},
-        activePresetForFallback,
-      );
-      for (const [agentName, chainModels] of Object.entries(chains)) {
-        const existing = runtimeChains[agentName] ?? [];
-        const seen = new Set(existing);
-        for (const m of chainModels) {
-          if (!seen.has(m)) {
-            seen.add(m);
-            existing.push(m);
-          }
-        }
-        runtimeChains[agentName] = existing;
-      }
-    }
 
     // Parse multiplexer config with defaults
     multiplexerConfig = {
@@ -446,74 +417,24 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       }
       const configAgent = opencodeConfig.agent as Record<string, unknown>;
 
-      // Model resolution for foreground agents: combine _modelArray
-      // entries with fallback.chains config, then pick the first model in
-      // the effective array for startup-time selection.
+      // Model resolution for foreground agents: use _modelArray entries
+      // to pick the first model for startup-time selection.
       //
       // Runtime failover on API errors (e.g. rate limits
       // mid-conversation) is handled separately by
       // ForegroundFallbackManager via the event hook.
-      const activePresetForFallback =
-        getActiveRuntimePreset() ?? config.preset ?? null;
-      const fallbackChainsEnabled = config.fallback?.enabled !== false;
-      const fallbackChains = fallbackChainsEnabled
-        ? normalizeFallbackChainsForPreset(
-            (config.fallback?.chains as Record<string, string[] | undefined>) ??
-              {},
-            activePresetForFallback,
-          )
-        : {};
-
-      // Build effective model arrays: seed from _modelArray, then append
-      // fallback.chains entries so the resolver considers the full chain
-      // when picking the best available provider at startup.
-      const effectiveArrays: Record<
-        string,
-        Array<{ id: string; variant?: string }>
-      > = {};
-
-      for (const [agentName, models] of Object.entries(modelArrayMap)) {
-        effectiveArrays[agentName] = [...models];
-      }
-
-      for (const [agentName, chainModels] of Object.entries(fallbackChains)) {
-        if (!chainModels || chainModels.length === 0) continue;
-
-        if (!effectiveArrays[agentName]) {
-          // Agent has no _modelArray — seed from its current string model
-          // so the fallback chain appends after it rather than replacing
-          // it.
-          const entry = configAgent[agentName] as
-            | Record<string, unknown>
-            | undefined;
-          const currentModel =
-            typeof entry?.model === 'string' ? entry.model : undefined;
-          effectiveArrays[agentName] = currentModel
-            ? [{ id: currentModel }]
-            : [];
-        }
-
-        const seen = new Set(effectiveArrays[agentName].map((m) => m.id));
-        for (const chainModel of chainModels) {
-          if (!seen.has(chainModel)) {
-            seen.add(chainModel);
-            effectiveArrays[agentName].push({ id: chainModel });
-          }
-        }
-      }
-
-      if (Object.keys(effectiveArrays).length > 0) {
-        for (const [agentName, modelArray] of Object.entries(effectiveArrays)) {
-          if (modelArray.length === 0) continue;
+      if (Object.keys(modelArrayMap).length > 0) {
+        for (const [agentName, models] of Object.entries(modelArrayMap)) {
+          if (models.length === 0) continue;
 
-          // Use the first model in the effective array. Not all providers
+          // Use the first model in the model array. Not all providers
           // require entries in opencodeConfig.provider — some are loaded
           // automatically by opencode (e.g. github-copilot, openrouter).
           // We cannot distinguish these from truly unconfigured providers
           // at config-hook time, so we cannot gate on the provider config
           // keys. Runtime failover is handled separately by
           // ForegroundFallbackManager.
-          const chosen = modelArray[0];
+          const chosen = models[0];
           const entry = configAgent[agentName] as
             | Record<string, unknown>
             | undefined;