Merge pull request #1029 from MyGO-Mujica/fix/normalize-disabled-config-keys

fix(config): normalize disabled_* keys before schema validation (#1027)

src/cli/doctor.test.ts

@@ -105,6 +105,63 @@ describe('runDoctorCheck', () => {
     expect(result.configs[1].path).toContain('.jsonc');
   });
 
+  test('string disabled_agents normalizes instead of failing schema validation', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const configDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(configDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(configDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        disabled_agents: 'explorer',
+        agents: { oracle: { model: 'test/model' } },
+      }),
+    );
+
+    const warnSpy = spyOn(console, 'warn').mockImplementation(() => {});
+    try {
+      const result = runDoctorCheck(projectDir);
+
+      // The string is normalized (matching the loader), so the config is
+      // valid rather than a false invalid-schema diagnosis
+      expect(result.ok).toBe(true);
+      expect(result.configs[1].ok).toBe(true);
+      expect(result.configs[1].error).toBeUndefined();
+      expect(result.configs[1].config?.disabled_agents).toEqual(['explorer']);
+      expect(result.configs[1].config?.agents?.oracle?.model).toBe(
+        'test/model',
+      );
+
+      // The normalization is reported to the user
+      const calls = warnSpy.mock.calls as string[][];
+      const message = calls.find((call) =>
+        (call[0] as string).includes('disabled_agents'),
+      )?.[0];
+      expect(message).toContain('should be an array; normalized');
+    } finally {
+      warnSpy.mockRestore();
+    }
+  });
+
+  test('array disabled_tools config passes unchanged', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const configDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(configDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(configDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        disabled_tools: ['webfetch'],
+        agents: { oracle: { model: 'test/model' } },
+      }),
+    );
+
+    const result = runDoctorCheck(projectDir);
+
+    expect(result.ok).toBe(true);
+    expect(result.configs[1].ok).toBe(true);
+    expect(result.configs[1].error).toBeUndefined();
+    expect(result.configs[1].config?.disabled_tools).toEqual(['webfetch']);
+  });
+
   test('invalid JSON returns not ok with invalid-json error', () => {
     const projectDir = path.join(tempDir, 'project');
     const configDir = path.join(projectDir, '.opencode');

src/cli/doctor.ts

@@ -1,6 +1,10 @@
 import * as fs from 'node:fs';
 import { z } from 'zod';
-import { findPluginConfigPaths, mergePluginConfigs } from '../config/loader';
+import {
+  findPluginConfigPaths,
+  mergePluginConfigs,
+  normalizeDisabledArrayKeys,
+} from '../config/loader';
 import { type PluginConfig, PluginConfigSchema } from '../config/schema';
 import { stripJsonComments } from './config-io';
 
@@ -78,6 +82,12 @@ function checkConfigFile(
 
     const content = fs.readFileSync(configPath, 'utf-8');
     const rawConfig = JSON.parse(stripJsonComments(content));
+    // Normalize disabled_* keys exactly like the loader does before schema
+    // validation, so a string value (e.g. "explorer") is not diagnosed as a
+    // false invalid-schema error. Report each normalization to the user.
+    normalizeDisabledArrayKeys(rawConfig, (message) => {
+      console.warn(`[oh-my-opencode-slim] ${message}`);
+    });
     const parseResult = PluginConfigSchema.safeParse(rawConfig);
 
     if (!parseResult.success) {

src/config/loader.test.ts

@@ -627,7 +627,7 @@ describe('onWarning callback', () => {
     expect(config.agents?.oracle?.model).toBe('model');
   });
 
-  test('rejects config with non-array disabled_tools (schema validation)', () => {
+  test('normalizes string disabled_tools instead of rejecting the config', () => {
     const projectDir = path.join(tempDir, 'project');
     const projectConfigDir = path.join(projectDir, '.opencode');
     fs.mkdirSync(projectConfigDir, { recursive: true });
@@ -644,14 +644,15 @@ describe('onWarning callback', () => {
       onWarning: (warning) => warnings.push(warning),
     });
 
-    // Schema validation rejects the entire file, so config is empty
-    expect(config).toEqual({});
+    // String is normalized to a single-element array, rest of config loads
+    expect(config.disabled_tools).toEqual(['not-an-array']);
+    expect(config.agents?.oracle?.model).toBe('test/model');
     expect(warnings).toHaveLength(1);
-    expect(warnings[0]?.kind).toBe('invalid-schema');
-    expect(warnings[0]?.message).toBe('Config does not match schema');
+    expect(warnings[0]?.kind).toBe('normalized');
+    expect(warnings[0]?.message).toContain('should be an array; normalized');
   });
 
-  test('rejects config with non-array disabled_agents (schema validation)', () => {
+  test('drops object disabled_agents instead of rejecting the config', () => {
     const projectDir = path.join(tempDir, 'project');
     const projectConfigDir = path.join(projectDir, '.opencode');
     fs.mkdirSync(projectConfigDir, { recursive: true });
@@ -667,12 +668,16 @@ describe('onWarning callback', () => {
       onWarning: (warning) => warnings.push(warning),
     });
 
-    expect(config).toEqual({});
+    // Non-array, non-string value is dropped; the config still loads
+    expect(config.disabled_agents).toBeUndefined();
     expect(warnings).toHaveLength(1);
-    expect(warnings[0]?.kind).toBe('invalid-schema');
+    expect(warnings[0]?.kind).toBe('normalized');
+    expect(warnings[0]?.message).toContain(
+      'must be an array; ignoring invalid value',
+    );
   });
 
-  test('rejects config with non-array disabled_mcps (schema validation)', () => {
+  test('drops number disabled_mcps instead of rejecting the config', () => {
     const projectDir = path.join(tempDir, 'project');
     const projectConfigDir = path.join(projectDir, '.opencode');
     fs.mkdirSync(projectConfigDir, { recursive: true });
@@ -688,12 +693,15 @@ describe('onWarning callback', () => {
       onWarning: (warning) => warnings.push(warning),
     });
 
-    expect(config).toEqual({});
+    expect(config.disabled_mcps).toBeUndefined();
     expect(warnings).toHaveLength(1);
-    expect(warnings[0]?.kind).toBe('invalid-schema');
+    expect(warnings[0]?.kind).toBe('normalized');
+    expect(warnings[0]?.message).toContain(
+      'must be an array; ignoring invalid value',
+    );
   });
 
-  test('rejects config with non-array disabled_skills (schema validation)', () => {
+  test('drops boolean disabled_skills instead of rejecting the config', () => {
     const projectDir = path.join(tempDir, 'project');
     const projectConfigDir = path.join(projectDir, '.opencode');
     fs.mkdirSync(projectConfigDir, { recursive: true });
@@ -709,9 +717,159 @@ describe('onWarning callback', () => {
       onWarning: (warning) => warnings.push(warning),
     });
 
-    expect(config).toEqual({});
+    expect(config.disabled_skills).toBeUndefined();
     expect(warnings).toHaveLength(1);
-    expect(warnings[0]?.kind).toBe('invalid-schema');
+    expect(warnings[0]?.kind).toBe('normalized');
+    expect(warnings[0]?.message).toContain(
+      'must be an array; ignoring invalid value',
+    );
+  });
+});
+
+describe('disabled_* key normalization', () => {
+  let tempDir: string;
+  let originalEnv: typeof process.env;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'disabled-normalize-'));
+    originalEnv = { ...process.env };
+    delete process.env.OPENCODE_CONFIG_DIR;
+    process.env.XDG_CONFIG_HOME = path.join(tempDir, 'user-config');
+  });
+
+  afterEach(() => {
+    fs.rmSync(tempDir, { recursive: true, force: true });
+    process.env = originalEnv;
+  });
+
+  test('normalizes string disabled_agents while preserving the rest of the config', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        disabled_agents: 'explorer',
+        autoUpdate: false,
+        agents: { oracle: { model: 'test/model' } },
+      }),
+    );
+
+    const warnings: ConfigLoadWarning[] = [];
+    const config = loadPluginConfig(projectDir, {
+      onWarning: (warning) => warnings.push(warning),
+    });
+
+    expect(config.disabled_agents).toEqual(['explorer']);
+    expect(config.autoUpdate).toBe(false);
+    expect(config.agents?.oracle?.model).toBe('test/model');
+    expect(warnings).toHaveLength(1);
+    expect(warnings[0]?.kind).toBe('normalized');
+    expect(warnings[0]?.message).toContain('should be an array; normalized');
+  });
+
+  test('leaves array disabled_agents unchanged', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        disabled_agents: ['explorer'],
+        agents: { oracle: { model: 'test/model' } },
+      }),
+    );
+
+    const warnings: ConfigLoadWarning[] = [];
+    const config = loadPluginConfig(projectDir, {
+      onWarning: (warning) => warnings.push(warning),
+    });
+
+    expect(config.disabled_agents).toEqual(['explorer']);
+    expect(config.agents?.oracle?.model).toBe('test/model');
+    expect(warnings).toHaveLength(0);
+  });
+
+  test('normalizes string disabled_tools while preserving presets and agents', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        disabled_tools: 'webfetch',
+        preset: 'fast',
+        presets: { fast: { oracle: { model: 'fast-model' } } },
+        agents: { oracle: { temperature: 0.9 } },
+      }),
+    );
+
+    const warnings: ConfigLoadWarning[] = [];
+    const config = loadPluginConfig(projectDir, {
+      onWarning: (warning) => warnings.push(warning),
+    });
+
+    expect(config.disabled_tools).toEqual(['webfetch']);
+    // Preset resolution still runs and merges with root agents
+    expect(config.agents?.oracle?.model).toBe('fast-model');
+    expect(config.agents?.oracle?.temperature).toBe(0.9);
+    expect(warnings).toHaveLength(1);
+    expect(warnings[0]?.kind).toBe('normalized');
+    expect(warnings[0]?.message).toContain('should be an array; normalized');
+  });
+
+  test('drops garbage disabled_* values while preserving the rest of the config', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        disabled_mcps: 123,
+        disabled_agents: { invalid: 'object' },
+        autoUpdate: false,
+        agents: { oracle: { model: 'test/model' } },
+      }),
+    );
+
+    const warnings: ConfigLoadWarning[] = [];
+    const config = loadPluginConfig(projectDir, {
+      onWarning: (warning) => warnings.push(warning),
+    });
+
+    expect(config.disabled_mcps).toBeUndefined();
+    expect(config.disabled_agents).toBeUndefined();
+    expect(config.autoUpdate).toBe(false);
+    expect(config.agents?.oracle?.model).toBe('test/model');
+    expect(warnings).toHaveLength(2);
+    for (const warning of warnings) {
+      expect(warning.kind).toBe('normalized');
+      expect(warning.message).toContain(
+        'must be an array; ignoring invalid value',
+      );
+    }
+  });
+
+  test('config without disabled_* keys is completely unaffected', () => {
+    const projectDir = path.join(tempDir, 'project');
+    const projectConfigDir = path.join(projectDir, '.opencode');
+    fs.mkdirSync(projectConfigDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(projectConfigDir, 'oh-my-opencode-slim.json'),
+      JSON.stringify({
+        autoUpdate: false,
+        agents: { oracle: { model: 'test/model' } },
+      }),
+    );
+
+    const warnings: ConfigLoadWarning[] = [];
+    const config = loadPluginConfig(projectDir, {
+      onWarning: (warning) => warnings.push(warning),
+    });
+
+    expect(config.autoUpdate).toBe(false);
+    expect(config.agents?.oracle?.model).toBe('test/model');
+    expect(warnings).toHaveLength(0);
   });
 });
 

src/config/loader.ts

@@ -19,7 +19,8 @@ export type ConfigLoadWarningKind =
   | 'invalid-schema'
   | 'read-error'
   | 'missing-preset'
-  | 'deprecated-key';
+  | 'deprecated-key'
+  | 'normalized';
 
 /**
  * A warning emitted while loading plugin configuration.
@@ -56,6 +57,61 @@ const INTERVIEW_CONFIG_KEYS = [
   'dashboard',
 ] as const;
 
+// Config keys that must be arrays. A string value (e.g. "explorer") is
+// normalized to a single-element array; any other non-array value is
+// dropped. Normalization happens before schema validation so a typo in one
+// key does not silently discard the user's entire config (issue #1027).
+const DISABLED_CONFIG_KEYS = [
+  'disabled_agents',
+  'disabled_tools',
+  'disabled_mcps',
+  'disabled_skills',
+] as const;
+
+/**
+ * Normalize disabled_* config keys in place so a non-array value does not
+ * reject the whole config object during schema validation. A string value
+ * (e.g. "explorer") becomes a single-element array so the user's disable
+ * intent survives; any other non-array value (number, boolean, object, ...)
+ * is dropped. Array and undefined values are left untouched. Each
+ * normalization is reported through `warn` (if provided) with a plain
+ * message; callers wrap it in their own warning channel (loader uses
+ * onWarning + console.warn, doctor just reports the message).
+ *
+ * @param rawConfig - Parsed config to normalize (mutated in place)
+ * @param warn - Optional callback invoked with each warning message
+ */
+export function normalizeDisabledArrayKeys(
+  rawConfig: unknown,
+  warn?: (message: string) => void,
+): void {
+  if (
+    typeof rawConfig !== 'object' ||
+    rawConfig === null ||
+    Array.isArray(rawConfig)
+  ) {
+    return;
+  }
+
+  const configRecord = rawConfig as Record<string, unknown>;
+  for (const key of DISABLED_CONFIG_KEYS) {
+    const value = configRecord[key];
+    if (value === undefined || Array.isArray(value)) {
+      continue;
+    }
+    if (typeof value === 'string') {
+      configRecord[key] = [value];
+      warn?.(
+        `Config key "${key}" should be an array; ` +
+          `normalized to ["${value}"].`,
+      );
+    } else {
+      delete configRecord[key];
+      warn?.(`Config key "${key}" must be an array; ignoring invalid value.`);
+    }
+  }
+}
+
 function retainExplicitInterviewFields(
   parsedConfig: PluginConfig,
   rawConfig: unknown,
@@ -202,6 +258,21 @@ function loadConfigFromPath(
       }
     }
 
+    // Normalize disabled_* config keys before schema validation so a
+    // non-array value does not reject the whole config object (which would
+    // silently discard every other user setting). Reported with the
+    // 'normalized' kind so TUI/doctor do not treat a fixed config as invalid.
+    normalizeDisabledArrayKeys(rawConfig, (message) => {
+      options?.onWarning?.({
+        path: configPath,
+        kind: 'normalized',
+        message,
+      });
+      if (!options?.silent) {
+        console.warn(`[oh-my-opencode-slim] ${message}`);
+      }
+    });
+
     const result = PluginConfigSchema.safeParse(rawConfig);
 
     if (!result.success) {
@@ -533,39 +604,6 @@ export function loadPluginConfig(
   // debounced toast in index.ts. Overriding to 'direct' here would prevent
   // processImageAttachments from returning true and suppress the toast.
 
-  // Normalize disabled_* config keys to ensure they are arrays or undefined.
-  // This loop is currently unreachable via the normal file-loading path:
-  // PluginConfigSchema.safeParse() rejects the WHOLE config object if any
-  // disabled_* field is non-array (no .catch() on these fields), so
-  // loadConfigFromPath returns null and the file falls back to {} BEFORE this
-  // loop ever runs. Retained only as defense-in-depth against a future schema
-  // relaxation (e.g. adding .catch() to these fields) or a construction path
-  // that bypasses safeParse entirely — not as a proven/tested fix for the
-  // originally reported crash (root cause not reproduced).
-  const ARRAY_CONFIG_KEYS = [
-    'disabled_agents',
-    'disabled_tools',
-    'disabled_mcps',
-    'disabled_skills',
-  ] as const;
-
-  const configPathForWarning = projectConfigPath ?? userConfigPath ?? '';
-  for (const key of ARRAY_CONFIG_KEYS) {
-    const value = config[key as keyof PluginConfig];
-    if (value !== undefined && !Array.isArray(value)) {
-      const message = `Config key "${key}" must be an array; ignoring invalid value.`;
-      options?.onWarning?.({
-        path: configPathForWarning,
-        kind: 'invalid-schema',
-        message,
-      });
-      if (!options?.silent) {
-        console.warn(`[oh-my-opencode-slim] ${message}`);
-      }
-      delete config[key as keyof PluginConfig];
-    }
-  }
-
   return config;
 }
 

src/tui.test.ts

@@ -149,6 +149,29 @@ describe('readConfigInvalid', () => {
     }
   });
 
+  test('returns false for config with normalized disabled_* string', () => {
+    const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'omos-tui-'));
+    try {
+      const projectDir = path.join(tempDir, 'project');
+      const configDir = path.join(projectDir, '.opencode');
+      fs.mkdirSync(configDir, { recursive: true });
+      fs.writeFileSync(
+        path.join(configDir, 'oh-my-opencode-slim.json'),
+        JSON.stringify({
+          disabled_agents: 'explorer',
+          agents: { oracle: { model: 'valid/model' } },
+        }),
+      );
+
+      // The string key is normalized to an array with a 'normalized' warning
+      // (not invalid-schema), so the config loads fine and the sidebar must
+      // NOT show "Config invalid".
+      expect(readConfigInvalid(projectDir)).toBe(false);
+    } finally {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+
   test('uses compact sidebar by default', () => {
     const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'omos-tui-'));
     try {