Преглед изворни кода

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

mygo пре 1 месец
родитељ
комит
c437f6a97f
2 измењених фајлова са 222 додато и 43 уклоњено
  1. 165 10
      src/config/loader.test.ts
  2. 57 33
      src/config/loader.ts

+ 165 - 10
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]?.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]?.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]?.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,156 @@ 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]?.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]?.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]?.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.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);
   });
 });
 

+ 57 - 33
src/config/loader.ts

@@ -56,6 +56,17 @@ 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;
+
 function retainExplicitInterviewFields(
   parsedConfig: PluginConfig,
   rawConfig: unknown,
@@ -202,6 +213,52 @@ 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). A string value keeps the
+    // user's disable intent as a single-element array; any other non-array
+    // value (number, boolean, object, ...) is dropped.
+    if (
+      typeof rawConfig === 'object' &&
+      rawConfig !== null &&
+      !Array.isArray(rawConfig)
+    ) {
+      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];
+          const normalizedMsg =
+            `Config key "${key}" should be an array; ` +
+            `normalized to ["${value}"].`;
+          options?.onWarning?.({
+            path: configPath,
+            kind: 'invalid-schema',
+            message: normalizedMsg,
+          });
+          if (!options?.silent) {
+            console.warn(`[oh-my-opencode-slim] ${normalizedMsg}`);
+          }
+        } else {
+          delete configRecord[key];
+          const droppedMsg =
+            `Config key "${key}" must be an array; ` +
+            `ignoring invalid value.`;
+          options?.onWarning?.({
+            path: configPath,
+            kind: 'invalid-schema',
+            message: droppedMsg,
+          });
+          if (!options?.silent) {
+            console.warn(`[oh-my-opencode-slim] ${droppedMsg}`);
+          }
+        }
+      }
+    }
+
     const result = PluginConfigSchema.safeParse(rawConfig);
 
     if (!result.success) {
@@ -533,39 +590,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;
 }