Просмотр исходного кода

fix(config): use normalized warning kind and share normalization with doctor (#1027)

mygo 1 месяц назад
Родитель
Сommit
e0a1dabb8c
5 измененных файлов с 155 добавлено и 48 удалено
  1. 57 0
      src/cli/doctor.test.ts
  2. 11 1
      src/cli/doctor.ts
  3. 7 4
      src/config/loader.test.ts
  4. 57 43
      src/config/loader.ts
  5. 23 0
      src/tui.test.ts

+ 57 - 0
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');

+ 11 - 1
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) {

+ 7 - 4
src/config/loader.test.ts

@@ -648,7 +648,7 @@ describe('onWarning callback', () => {
     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]?.kind).toBe('normalized');
     expect(warnings[0]?.message).toContain('should be an array; normalized');
   });
 
@@ -671,7 +671,7 @@ describe('onWarning callback', () => {
     // 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',
     );
@@ -695,7 +695,7 @@ describe('onWarning callback', () => {
 
     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',
     );
@@ -719,7 +719,7 @@ describe('onWarning callback', () => {
 
     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',
     );
@@ -764,6 +764,7 @@ describe('disabled_* key normalization', () => {
     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');
   });
 
@@ -813,6 +814,7 @@ describe('disabled_* key normalization', () => {
     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');
   });
 
@@ -841,6 +843,7 @@ describe('disabled_* key normalization', () => {
     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',
       );

+ 57 - 43
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.
@@ -67,6 +68,50 @@ const DISABLED_CONFIG_KEYS = [
   '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,
@@ -215,49 +260,18 @@ 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}`);
-          }
-        }
+    // 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);
 

+ 23 - 0
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 {