Browse Source

Merge remote-tracking branch 'origin/pr-723-image-routing' into omos/pr-723-image-routing

# Conflicts:
#	src/config/constants.ts
#	src/hooks/image-hook.test.ts
#	src/index.ts
Alvin Unreal 4 weeks ago
parent
commit
d3e23e257e

+ 2 - 2
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.
+> **Why a separate agent?** If your Orchestrator model is not multimodal, enable Observer to handle images, screenshots, 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. Omitting `image_routing` preserves existing conditional Observer behavior. Set `image_routing: "auto"` only when Observer is enabled, or `"direct"` to always pass image attachments to the Orchestrator.
 
 <table>
   <tr>
@@ -628,7 +628,7 @@ rules.
 
 - Images, screenshots, diagrams → `read` tool (native image support)
 - PDFs and binary documents → `read` tool (text + structure extraction)
-- **Disabled by default** - enable with `"disabled_agents": []` and configure a vision-capable model; installing with `--preset=opencode-go` enables it with `opencode-go/kimi-k2.6`
+- **Disabled by default** - enable with `"disabled_agents": []` and configure a vision-capable model; installing with `--preset=opencode-go` enables it with `opencode-go/kimi-k2.6`. Image attachments route to Observer by default when it is enabled; set `"image_routing": "direct"` to keep them on the Orchestrator.
 
     </td>
   </tr>

+ 3 - 0
docs/configuration.md

@@ -133,6 +133,7 @@ Presets can also be switched at runtime without restarting using the `/preset` c
 | `acpAgents.<name>.permissionMode` | string | `ask` | How ACP permission requests are handled: `ask`, `allow`, or `reject` |
 | `acpAgents.<name>.timeoutMs` | integer | `0` | Timeout for a single ACP run in milliseconds. `0` disables the timeout so external agents can run indefinitely. Finite values can be up to `2147483647`ms (~24.8 days) |
 | `disabled_agents` | string[] | `["observer"]` | Agent names to disable globally. Set to `[]` to enable Observer; this is global, not per-preset |
+| `image_routing` | `"auto"` \| `"direct"` | omitted (legacy conditional) | Optional. When omitted, images are intercepted only when Observer is enabled, preserving existing behavior. Explicit `"auto"` requires Observer enabled and saves image attachments to disk before nudging delegation to @observer. `"direct"`: always pass images to the orchestrator. |
 | `autoUpdate` | boolean | `true` | Automatically install plugin updates in the background; set to `false` for notification-only mode |
 | `multiplexer.type` | string | `"none"` | Multiplexer mode: `auto`, `tmux`, `zellij`, `herdr`, or `none` |
 | `multiplexer.layout` | string | `"main-vertical"` | Layout preset: `main-vertical`, `main-horizontal`, `tiled`, `even-horizontal`, `even-vertical`. Tmux applies full layouts; Zellij and Herdr map `main-vertical` to right and `main-horizontal` to down |
@@ -148,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 |

+ 9 - 0
oh-my-opencode-slim.schema.json

@@ -195,7 +195,16 @@
         "type": "string"
       }
     },
+    "image_routing": {
+      "description": "How image attachments are handled. When omitted, preserves legacy conditional behavior: intercept attachments only when observer is enabled. \"auto\": requires observer to be enabled and saves attachments to disk before nudging delegation to @observer. \"direct\": always passes attachments to the orchestrator untouched.",
+      "type": "string",
+      "enum": [
+        "auto",
+        "direct"
+      ]
+    },
     "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"

+ 11 - 0
src/config/constants.ts

@@ -94,3 +94,14 @@ export const DEFAULT_DISABLED_AGENTS: string[] = ['observer'];
 export const DEFAULT_MAX_SESSIONS_PER_AGENT = 2;
 export const DEFAULT_READ_CONTEXT_MIN_LINES = 10;
 export const DEFAULT_READ_CONTEXT_MAX_FILES = 8;
+
+export type ImageRouting = 'auto' | 'direct';
+
+/** Default image routing mode, preserving Observer's existing behavior. */
+export const DEFAULT_IMAGE_ROUTING: ImageRouting = 'auto';
+
+export function resolveImageRouting(
+  imageRouting: ImageRouting | undefined,
+): ImageRouting {
+  return imageRouting ?? DEFAULT_IMAGE_ROUTING;
+}

+ 58 - 0
src/config/loader.test.ts

@@ -66,6 +66,64 @@ describe('loadPluginConfig', () => {
     expect(config.autoUpdate).toBe(false);
   });
 
+  test('validates auto image routing after project enables Observer', () => {
+    const userConfigPath = path.join(userConfigDir, 'opencode');
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(userConfigPath, { recursive: true });
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(userConfigPath, 'oh-my-opencode-slim.json'),
+      JSON.stringify({ image_routing: 'auto' }),
+    );
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({ disabled_agents: [] }),
+    );
+
+    const config = loadPluginConfig(projectDir, { silent: true });
+    expect(config.image_routing).toBe('auto');
+    expect(config.disabled_agents).toEqual([]);
+  });
+
+  test('validates auto image routing after project enables it', () => {
+    const userConfigPath = path.join(userConfigDir, 'opencode');
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(userConfigPath, { recursive: true });
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(userConfigPath, 'oh-my-opencode-slim.json'),
+      JSON.stringify({ disabled_agents: [] }),
+    );
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({ image_routing: 'auto' }),
+    );
+
+    const config = loadPluginConfig(projectDir, { silent: true });
+    expect(config.image_routing).toBe('auto');
+    expect(config.disabled_agents).toEqual([]);
+  });
+
+  test('rejects auto image routing when final config disables Observer', () => {
+    const userConfigPath = path.join(userConfigDir, 'opencode');
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(userConfigPath, { recursive: true });
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(userConfigPath, 'oh-my-opencode-slim.json'),
+      JSON.stringify({ image_routing: 'auto' }),
+    );
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({ disabled_agents: ['observer'] }),
+    );
+
+    expect(loadPluginConfig(projectDir, { silent: true })).toEqual({});
+  });
+
   test('ignores invalid config (schema violation or malformed JSON)', () => {
     const projectDir = path.join(tempDir, 'project');
     const projectConfigDir = path.join(projectDir, '.opencode');

+ 35 - 0
src/config/loader.ts

@@ -2,6 +2,7 @@ import * as fs from 'node:fs';
 import * as path from 'node:path';
 import { stripJsonComments } from '../cli/config-io';
 import { getConfigSearchDirs } from '../cli/paths';
+import { DEFAULT_DISABLED_AGENTS } from './constants';
 import { type PluginConfig, PluginConfigSchema } from './schema';
 
 /**
@@ -157,6 +158,30 @@ function findConfigPathInDirs(
   return null;
 }
 
+function validateFinalImageRouting(
+  config: PluginConfig,
+  configPath: string,
+  options?: LoadPluginConfigOptions,
+): boolean {
+  if (config.image_routing !== 'auto') return true;
+
+  const disabledAgents = config.disabled_agents ?? DEFAULT_DISABLED_AGENTS;
+  if (!disabledAgents.includes('observer')) return true;
+
+  const message =
+    'image_routing "auto" requires observer to be enabled. ' +
+    'Remove "observer" from disabled_agents.';
+  options?.onWarning?.({
+    path: configPath,
+    kind: 'invalid-schema',
+    message,
+  });
+  if (!options?.silent) {
+    console.warn(`[oh-my-opencode-slim] Invalid config: ${message}`);
+  }
+  return false;
+}
+
 /**
  * Find plugin config paths (user and project) for a given directory.
  * User config uses getConfigSearchDirs() for lookup.
@@ -333,6 +358,16 @@ export function loadPluginConfig(
     };
   }
 
+  if (
+    !validateFinalImageRouting(
+      config,
+      projectConfigPath ?? userConfigPath ?? '',
+      options,
+    )
+  ) {
+    return {};
+  }
+
   return config;
 }
 

+ 41 - 0
src/config/schema.test.ts

@@ -0,0 +1,41 @@
+import { describe, expect, it } from 'bun:test';
+import { PluginConfigSchema } from './schema';
+
+describe('PluginConfigSchema image_routing', () => {
+  it('accepts image_routing: direct with observer disabled', () => {
+    const result = PluginConfigSchema.safeParse({
+      disabled_agents: ['observer'],
+      image_routing: 'direct',
+    });
+    expect(result.success).toBe(true);
+  });
+
+  it('accepts image_routing: auto with observer enabled', () => {
+    const result = PluginConfigSchema.safeParse({
+      disabled_agents: [],
+      image_routing: 'auto',
+    });
+    expect(result.success).toBe(true);
+  });
+
+  it('accepts image_routing: auto with observer disabled until layers merge', () => {
+    const result = PluginConfigSchema.safeParse({
+      disabled_agents: ['observer'],
+      image_routing: 'auto',
+    });
+    expect(result.success).toBe(true);
+  });
+
+  it('leaves image_routing undefined when omitted (default applied downstream)', () => {
+    const result = PluginConfigSchema.safeParse({});
+    expect(result.success).toBe(true);
+    if (result.success) {
+      expect(result.data.image_routing).toBeUndefined();
+    }
+  });
+
+  it('accepts image_routing: auto when disabled_agents is omitted', () => {
+    const result = PluginConfigSchema.safeParse({ image_routing: 'auto' });
+    expect(result.success).toBe(true);
+  });
+});

+ 18 - 1
src/config/schema.ts

@@ -327,7 +327,24 @@ export const PluginConfigSchema = z
           'Orchestrator and council internal agents (councillor) cannot be disabled. ' +
           "By default, 'observer' is disabled. Remove it from this list and configure a vision-capable model to enable.",
       ),
-    disabled_mcps: z.array(z.string()).optional(),
+    image_routing: z
+      .enum(['auto', 'direct'])
+      .optional()
+      .describe(
+        'How image attachments are handled. ' +
+          'When omitted, preserves legacy conditional behavior: intercept ' +
+          'attachments only when observer is enabled. "auto": requires ' +
+          'observer to be enabled and saves attachments to disk before ' +
+          'nudging delegation to @observer. "direct": always passes ' +
+          'attachments to the orchestrator untouched.',
+      ),
+    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()

+ 139 - 14
src/hooks/image-hook.test.ts

@@ -8,10 +8,12 @@ import {
 } from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-
+import { resolveImageRouting } from '../config/constants';
 import { processImageAttachments } from './image-hook';
+import type { MessageWithParts } from './types';
 
 const TEST_DIR = path.join(os.tmpdir(), `image-hook-test-${process.pid}`);
+const IMG = { type: 'image', url: 'data:image/png;base64,AAAA' };
 
 function makeTestDir(name: string): { workDir: string; saveDir: string } {
   const workDir = path.join(TEST_DIR, name);
@@ -21,32 +23,37 @@ function makeTestDir(name: string): { workDir: string; saveDir: string } {
 }
 
 function makeOldFile(dir: string, name: string): string {
-  const fp = path.join(dir, name);
-  writeFileSync(fp, 'data');
+  const filePath = path.join(dir, name);
+  writeFileSync(filePath, 'data');
   const past = new Date(Date.now() - 2 * 60 * 60 * 1000);
-  utimesSync(fp, past, past);
-  return fp;
+  utimesSync(filePath, past, past);
+  return filePath;
 }
 
-describe('image-hook catch logging', () => {
-  afterAll(() => {
-    rmSync(TEST_DIR, { recursive: true, force: true });
-  });
+function makeUserMsg(parts: MessageWithParts['parts']): MessageWithParts {
+  return { info: { role: 'user', sessionID: 's1' }, parts };
+}
+
+function imagePartCount(message: MessageWithParts): number {
+  return message.parts.filter((part) => part.type === 'image').length;
+}
+
+afterAll(() => {
+  rmSync(TEST_DIR, { recursive: true, force: true });
+});
 
+describe('image-hook catch logging', () => {
   it('survives file cleanup failure without throwing', () => {
     const { workDir, saveDir } = makeTestDir('cleanup-fail-1');
-
     makeOldFile(saveDir, 'old-image.png');
-
-    // Make the directory read-only to cause unlinkSync to fail
     chmodSync(saveDir, 0o555);
 
     try {
-      // Must not throw despite failed cleanup
       expect(() => {
         processImageAttachments({
           messages: [],
           workDir,
+          imageRouting: 'auto',
           disabledAgents: new Set<string>(),
           log: () => {},
         });
@@ -61,7 +68,6 @@ describe('image-hook catch logging', () => {
     const sessionDir = path.join(saveDir, 'ses-abc');
     mkdirSync(sessionDir, { recursive: true });
     makeOldFile(sessionDir, 'img.png');
-
     chmodSync(sessionDir, 0o555);
 
     try {
@@ -69,6 +75,7 @@ describe('image-hook catch logging', () => {
         processImageAttachments({
           messages: [],
           workDir,
+          imageRouting: 'auto',
           disabledAgents: new Set<string>(),
           log: () => {},
         });
@@ -78,3 +85,121 @@ describe('image-hook catch logging', () => {
     }
   });
 });
+
+describe('processImageAttachments image routing', () => {
+  it('direct mode leaves image parts untouched', () => {
+    const message = makeUserMsg([IMG]);
+    processImageAttachments({
+      messages: [message],
+      workDir: path.join(TEST_DIR, 'direct'),
+      imageRouting: 'direct',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(imagePartCount(message)).toBe(1);
+  });
+
+  it('auto mode saves image parts and adds an @observer nudge', () => {
+    const message = makeUserMsg([IMG]);
+    processImageAttachments({
+      messages: [message],
+      workDir: path.join(TEST_DIR, 'auto'),
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(imagePartCount(message)).toBe(0);
+    const textParts = message.parts.filter((part) => part.type === 'text');
+    expect(textParts).toHaveLength(1);
+    expect(textParts[0]?.text).toContain('@observer');
+  });
+
+  it('resolves omitted image routing to auto and intercepts for Observer', () => {
+    const message = makeUserMsg([IMG]);
+    processImageAttachments({
+      messages: [message],
+      workDir: path.join(TEST_DIR, 'omitted-routing'),
+      imageRouting: resolveImageRouting(undefined),
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(imagePartCount(message)).toBe(0);
+    expect(message.parts.some((part) => part.type === 'text')).toBe(true);
+  });
+
+  it('keeps images when auto mode has observer disabled', () => {
+    const message = makeUserMsg([IMG]);
+    processImageAttachments({
+      messages: [message],
+      workDir: path.join(TEST_DIR, 'disabled'),
+      imageRouting: 'auto',
+      disabledAgents: new Set(['observer']),
+      log: () => {},
+    });
+    expect(imagePartCount(message)).toBe(1);
+  });
+
+  it('keeps images when auto mode cannot save them', () => {
+    const message = makeUserMsg([
+      { type: 'image', url: 'https://example.com/image.png' },
+    ]);
+    processImageAttachments({
+      messages: [message],
+      workDir: path.join(TEST_DIR, 'unsaved'),
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(imagePartCount(message)).toBe(1);
+    expect(message.parts).toHaveLength(1);
+  });
+
+  it('strips only attachments saved successfully', () => {
+    const message = makeUserMsg([
+      IMG,
+      { type: 'image', url: 'https://example.com/image.png' },
+    ]);
+    processImageAttachments({
+      messages: [message],
+      workDir: path.join(TEST_DIR, 'mixed'),
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(imagePartCount(message)).toBe(1);
+    expect(message.parts.some((part) => part.type === 'text')).toBe(true);
+  });
+
+  it('continues after an earlier message cannot save its images', () => {
+    const failed = makeUserMsg([
+      { type: 'image', url: 'https://example.com/image.png' },
+    ]);
+    const saved = makeUserMsg([IMG]);
+    processImageAttachments({
+      messages: [failed, saved],
+      workDir: path.join(TEST_DIR, 'multiple'),
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(imagePartCount(failed)).toBe(1);
+    expect(imagePartCount(saved)).toBe(0);
+  });
+
+  it('ignores non-user messages and non-image parts', () => {
+    const userText = makeUserMsg([{ type: 'text', text: 'hello' }]);
+    const assistant = {
+      info: { role: 'assistant', sessionID: 's1' },
+      parts: [{ type: 'text', text: 'hi' }],
+    } as unknown as MessageWithParts;
+    processImageAttachments({
+      messages: [userText, assistant],
+      workDir: path.join(TEST_DIR, 'non-image'),
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(userText.parts).toHaveLength(1);
+    expect(assistant.parts).toHaveLength(1);
+  });
+});

+ 25 - 4
src/hooks/image-hook.ts

@@ -166,11 +166,18 @@ function writeUniqueFile(
 export function processImageAttachments(args: {
   messages: MessageWithParts[];
   workDir: string;
+  imageRouting: 'auto' | 'direct';
   disabledAgents: Set<string>;
   log: (msg: string) => void;
 }): void {
-  const { messages, workDir, disabledAgents, log } = args;
+  const { messages, workDir, imageRouting, disabledAgents, log } = args;
 
+  // direct mode: never intercept attachments; the orchestrator handles them
+  // inline. @observer remains available for manual delegation.
+  if (imageRouting === 'direct') return;
+
+  // auto mode: observer must be enabled (enforced at config load). Retain
+  // this guard as defense-in-depth in case validation is bypassed.
   const observerEnabled = !disabledAgents.has('observer');
   if (!observerEnabled) return;
 
@@ -219,6 +226,7 @@ export function processImageAttachments(args: {
 
     // Save each image to .opencode/images/ and collect paths
     const savedPaths: string[] = [];
+    const savedImageParts = new Set<ImagePart>();
     for (const p of imageParts) {
       const url = p.url as string | undefined;
       const filename =
@@ -241,17 +249,30 @@ export function processImageAttachments(args: {
             : extFromMime(decoded.mime);
           const name = `${baseName}-${hash}${ext}`;
           const filePath = writeUniqueFile(targetDir, name, decoded.data, log);
-          if (filePath) savedPaths.push(filePath);
+          if (filePath) {
+            savedPaths.push(filePath);
+            savedImageParts.add(p);
+          }
         }
       }
     }
 
     const pathsText =
       savedPaths.length > 0 ? ` Saved to: ${savedPaths.join(', ')}` : '';
-    log(`[image-hook] stripping image/file parts, saving to disk${pathsText}`);
+    log(`[image-hook] saved image/file parts to disk${pathsText}`);
+    log(
+      `[image-routing] auto mode: intercepted ${savedImageParts.size} 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');
+      continue;
+    }
 
     msg.parts = msg.parts
-      .filter((p) => !isImagePart(p as ImagePart))
+      .filter((p) => !savedImageParts.has(p as ImagePart))
       .concat([
         {
           type: 'text',

+ 2 - 0
src/index.ts

@@ -15,6 +15,7 @@ import {
   DEFAULT_MAX_SESSIONS_PER_AGENT,
   DEFAULT_READ_CONTEXT_MAX_FILES,
   DEFAULT_READ_CONTEXT_MIN_LINES,
+  resolveImageRouting,
 } from './config/constants';
 import {
   getActiveRuntimePreset,
@@ -1142,6 +1143,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       processImageAttachments({
         messages: typedOutput.messages,
         workDir: ctx.directory,
+        imageRouting: resolveImageRouting(config.image_routing),
         disabledAgents,
         log,
       });