Browse Source

fix(image-routing): address review findings

- Skip stripping image parts when no image could be saved (avoid losing
  bytes with a nudge that has no paths)
- Log the count of stripped parts, not saved files
- Correct image_routing docs to say image attachments (PDF not handled)
- Clarify README guidance: direct for multimodal, auto for non-multimodal
- Add disabled_mcps describe string and missing fallback docs rows
- Regenerate schema JSON
Michael Henke 1 month ago
parent
commit
31b73b7321
5 changed files with 21 additions and 5 deletions
  1. 1 1
      README.md
  2. 2 0
      docs/configuration.md
  3. 2 1
      oh-my-opencode-slim.schema.json
  4. 8 2
      src/config/schema.ts
  5. 8 1
      src/hooks/image-hook.ts

+ 1 - 1
README.md

@@ -614,7 +614,7 @@ rules.
 ### Observer: The Silent Witness
 
 > [!NOTE]
-> **Why a separate agent?** If your Orchestrator model is not multimodal, enable Observer to handle images, screenshots, PDFs, and other visual files. Observer is disabled by default and gives the Orchestrator a dedicated multimodal reader without forcing you to change your main reasoning model. Set `disabled_agents: []` and an `observer` model in your configuration. The bundled `opencode-go` install preset does this automatically because its GLM Orchestrator is not multimodal. If your Orchestrator *is* multimodal, set `image_routing: "auto"` instead — this routes attachments to `@observer` without intercepting them when the orchestrator can handle them directly.
+> **Why a separate agent?** If your Orchestrator model is not multimodal, enable Observer to handle images, screenshots, PDFs, and other visual files. Observer is disabled by default and gives the Orchestrator a dedicated multimodal reader without forcing you to change your main reasoning model. Set `disabled_agents: []` and an `observer` model in your configuration. The bundled `opencode-go` install preset does this automatically because its GLM Orchestrator is not multimodal. If your Orchestrator *is* multimodal, use the default `image_routing: "direct"` so attachments reach it directly. If it is *not* multimodal, set `image_routing: "auto"` to save attachments and route them to `@observer`.
 
 <table>
   <tr>

+ 2 - 0
docs/configuration.md

@@ -149,6 +149,8 @@ Presets can also be switched at runtime without restarting using the `/preset` c
 | `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.maxRetries` | number | `2` | Maximum failover attempts before giving up |
+| `fallback.runtimeOverride` | boolean | `true` | Allow per-call model overrides to bypass the fallback chain |
 | `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 |

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

@@ -196,7 +196,7 @@
       }
     },
     "image_routing": {
-      "description": "How image/PDF attachments are handled. \"direct\" (default): pass attachments to the orchestrator untouched; @observer stays available for manual delegation. \"auto\": save attachments to disk and nudge the orchestrator to delegate to @observer. Requires observer to be enabled (not in disabled_agents).",
+      "description": "How image attachments are handled. \"direct\" (default): pass attachments to the orchestrator untouched; @observer stays available for manual delegation. \"auto\": save attachments to disk and nudge the orchestrator to delegate to @observer. Requires observer to be enabled (not in disabled_agents).",
       "type": "string",
       "enum": [
         "auto",
@@ -204,6 +204,7 @@
       ]
     },
     "disabled_mcps": {
+      "description": "MCP server names to disable completely. Disabled servers are not started and cannot be used by agents.",
       "type": "array",
       "items": {
         "type": "string"

+ 8 - 2
src/config/schema.ts

@@ -332,14 +332,20 @@ export const PluginConfigSchema = z
       .enum(['auto', 'direct'])
       .optional()
       .describe(
-        'How image/PDF attachments are handled. ' +
+        'How image attachments are handled. ' +
           '"direct" (default): pass attachments to the orchestrator untouched; ' +
           '@observer stays available for manual delegation. ' +
           '"auto": save attachments to disk and nudge the orchestrator to ' +
           'delegate to @observer. Requires observer to be enabled ' +
           '(not in disabled_agents).',
       ),
-    disabled_mcps: z.array(z.string()).optional(),
+    disabled_mcps: z
+      .array(z.string())
+      .optional()
+      .describe(
+        'MCP server names to disable completely. Disabled servers are not ' +
+          'started and cannot be used by agents.',
+      ),
     disabled_tools: z
       .array(z.string())
       .optional()

+ 8 - 1
src/hooks/image-hook.ts

@@ -247,9 +247,16 @@ export function processImageAttachments(args: {
       savedPaths.length > 0 ? ` Saved to: ${savedPaths.join(', ')}` : '';
     log(`[image-hook] stripping image/file parts, saving to disk${pathsText}`);
     log(
-      `[image-routing] auto mode: intercepted ${savedPaths.length} image(s), delegating to @observer`,
+      `[image-routing] auto mode: intercepted ${imageParts.length} image(s), delegating to @observer`,
     );
 
+    // If no image could be saved, do not strip the parts: the orchestrator
+    // would receive a nudge with no usable path and the bytes would be lost.
+    if (savedPaths.length === 0) {
+      log('[image-hook] no images saved; leaving original parts in message');
+      return;
+    }
+
     msg.parts = msg.parts
       .filter((p) => !isImagePart(p as ImagePart))
       .concat([