Browse Source

fix(index): stop exporting minimumExpectedToolCount from package root

- Move minimumExpectedToolCount and its HEALTH_CHECK/BASELINE_TOOL_NAMES
  constants out of src/index.ts into a new internal src/health-check.ts,
  importing but not re-exporting them from the root module.
- OpenCode's legacy plugin loader iterates every named export of the
  package root and invokes each as a plugin factory with PluginInput.
  Exporting minimumExpectedToolCount meant it was called with
  PluginInput (not string[]) and its numeric return value pushed into
  the hooks array as if it were a Hooks object. The Array.isArray guard
  added in this branch prevents the resulting .filter() crash but does
  not stop the mis-invocation itself.
- Move the corresponding unit tests to src/health-check.test.ts.

Addresses triage feedback on #894:
https://github.com/alvinunreal/oh-my-opencode-slim/issues/894#issuecomment-5146763258

Signed-off-by: Major Hayden <major@mhtx.net>
Major Hayden 2 weeks ago
parent
commit
9e873a8598
4 changed files with 76 additions and 61 deletions
  1. 21 0
      src/health-check.test.ts
  2. 53 0
      src/health-check.ts
  3. 1 20
      src/index.test.ts
  4. 1 41
      src/index.ts

+ 21 - 0
src/health-check.test.ts

@@ -0,0 +1,21 @@
+import { describe, expect, test } from 'bun:test';
+import { minimumExpectedToolCount } from './health-check';
+
+describe('plugin health thresholds', () => {
+  test('accounts only for intentionally disabled baseline tools', () => {
+    expect(minimumExpectedToolCount()).toBe(5);
+    expect(minimumExpectedToolCount(['wait_for_user'])).toBe(4);
+    expect(minimumExpectedToolCount(['wait_for_user', 'wait_for_user'])).toBe(
+      4,
+    );
+    expect(minimumExpectedToolCount(['unknown_tool'])).toBe(5);
+  });
+
+  test('never throws when disabledTools is not an array', () => {
+    // Regression test: a malformed/non-array config.disabled_tools value
+    // must degrade to "nothing disabled" instead of crashing plugin init.
+    expect(minimumExpectedToolCount('' as any)).toBe(5);
+    expect(minimumExpectedToolCount(null as any)).toBe(5);
+    expect(minimumExpectedToolCount({} as any)).toBe(5);
+  });
+});

+ 53 - 0
src/health-check.ts

@@ -0,0 +1,53 @@
+/**
+ * Plugin init health-check thresholds and helpers.
+ *
+ * Deliberately NOT re-exported from the package root (`src/index.ts`).
+ * OpenCode's legacy plugin loader iterates every named export of the
+ * root module and invokes each as a plugin factory with `PluginInput`.
+ * A helper like `minimumExpectedToolCount` would then be called with a
+ * `PluginInput` object instead of `string[]`, and its numeric return
+ * value would be pushed into the hooks array as if it were a `Hooks`
+ * object. Keeping this module internal (imported by, but not
+ * re-exported from, `src/index.ts`) avoids that class of bug entirely;
+ * see https://github.com/alvinunreal/oh-my-opencode-slim/issues/894.
+ */
+
+/** Minimum expected registrations for a healthy plugin load. */
+export const HEALTH_CHECK = {
+  minAgents: 5,
+  // Default tool set when council and ACP agents are not configured:
+  // cancel_task, wait_for_user, webfetch, ast_grep_search, ast_grep_replace.
+  minTools: 5,
+  minMcps: 1,
+} as const;
+
+const BASELINE_TOOL_NAMES = new Set([
+  'cancel_task',
+  'wait_for_user',
+  'webfetch',
+  'ast_grep_search',
+  'ast_grep_replace',
+]);
+
+/**
+ * Compute the minimum tool count the health check should expect, accounting
+ * for baseline tools the user has intentionally disabled.
+ *
+ * @param disabledTools - Tool names disabled via config; non-array/malformed
+ *   values (which should never occur post-validation, but are not trusted at
+ *   runtime) are treated as "nothing disabled".
+ * @returns The adjusted minimum expected tool count
+ */
+export function minimumExpectedToolCount(
+  disabledTools: readonly string[] = [],
+): number {
+  // Config values come from user-edited JSON/JSONC (and can be re-derived
+  // via runtime preset switches); never trust the declared type at
+  // runtime. Fall back to "no disabled tools" instead of crashing plugin
+  // init if this isn't actually an array.
+  const safeDisabledTools = Array.isArray(disabledTools) ? disabledTools : [];
+  const disabledBaselineTools = new Set(
+    safeDisabledTools.filter((toolName) => BASELINE_TOOL_NAMES.has(toolName)),
+  );
+  return HEALTH_CHECK.minTools - disabledBaselineTools.size;
+}

+ 1 - 20
src/index.test.ts

@@ -1,24 +1,5 @@
 import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
-import plugin, { minimumExpectedToolCount } from './index';
-
-describe('plugin health thresholds', () => {
-  test('accounts only for intentionally disabled baseline tools', () => {
-    expect(minimumExpectedToolCount()).toBe(5);
-    expect(minimumExpectedToolCount(['wait_for_user'])).toBe(4);
-    expect(minimumExpectedToolCount(['wait_for_user', 'wait_for_user'])).toBe(
-      4,
-    );
-    expect(minimumExpectedToolCount(['unknown_tool'])).toBe(5);
-  });
-
-  test('never throws when disabledTools is not an array', () => {
-    // Regression test: a malformed/non-array config.disabled_tools value
-    // must degrade to "nothing disabled" instead of crashing plugin init.
-    expect(minimumExpectedToolCount('' as any)).toBe(5);
-    expect(minimumExpectedToolCount(null as any)).toBe(5);
-    expect(minimumExpectedToolCount({} as any)).toBe(5);
-  });
-});
+import plugin from './index';
 
 describe('plugin env disable', () => {
   let originalEnv: typeof process.env;

+ 1 - 41
src/index.ts

@@ -29,6 +29,7 @@ import {
   setActiveRuntimePreset,
 } from './config/runtime-preset';
 import { applyOrchestratorModelConfig } from './config/strip-orchestrator-model';
+import { HEALTH_CHECK, minimumExpectedToolCount } from './health-check';
 import {
   createApplyPatchHook,
   createAutoUpdateCheckerHook,
@@ -98,47 +99,6 @@ async function appLog(
   }
 }
 
-/** Minimum expected registrations for a healthy plugin load. */
-const HEALTH_CHECK = {
-  minAgents: 5,
-  // Default tool set when council and ACP agents are not configured:
-  // cancel_task, wait_for_user, webfetch, ast_grep_search, ast_grep_replace.
-  minTools: 5,
-  minMcps: 1,
-} as const;
-
-const BASELINE_TOOL_NAMES = new Set([
-  'cancel_task',
-  'wait_for_user',
-  'webfetch',
-  'ast_grep_search',
-  'ast_grep_replace',
-]);
-
-/**
- * Compute the minimum tool count the health check should expect, accounting
- * for baseline tools the user has intentionally disabled.
- *
- * @internal Exposed for deterministic health-threshold tests.
- * @param disabledTools - Tool names disabled via config; non-array/malformed
- *   values (which should never occur post-validation, but are not trusted at
- *   runtime) are treated as "nothing disabled".
- * @returns The adjusted minimum expected tool count
- */
-export function minimumExpectedToolCount(
-  disabledTools: readonly string[] = [],
-): number {
-  // Config values come from user-edited JSON/JSONC (and can be re-derived
-  // via runtime preset switches); never trust the declared type at
-  // runtime. Fall back to "no disabled tools" instead of crashing plugin
-  // init if this isn't actually an array.
-  const safeDisabledTools = Array.isArray(disabledTools) ? disabledTools : [];
-  const disabledBaselineTools = new Set(
-    safeDisabledTools.filter((toolName) => BASELINE_TOOL_NAMES.has(toolName)),
-  );
-  return HEALTH_CHECK.minTools - disabledBaselineTools.size;
-}
-
 /**
  * Probe jsdom at init time so the first webfetch call doesn't fail
  * silently. Logs a warning if jsdom can't be imported or instantiated,