fix(config): hide legacy internal hook handlers

2026-04-20 13:13:06 +00:00 · 2026-04-04 04:20:29 +09:00
parent d9af49a7af
commit 4265a59892
15 changed files with 186 additions and 215 deletions
--- a/docs/.generated/config-baseline.core.json
+++ b/docs/.generated/config-baseline.core.json
@@ -12279,7 +12279,7 @@
        "advanced"
      ],
      "label": "Internal Hooks Enabled",
-      "help": "Enables processing for internal hook handlers and configured entries in the internal hook runtime. Keep disabled unless internal hook handlers are intentionally configured.",
+      "help": "Enables processing for internal hooks and configured entries in the internal hook runtime. Keep disabled unless internal hooks are intentionally configured.",
      "hasChildren": false
    },
    {
@@ -12345,72 +12345,6 @@
      "tags": [],
      "hasChildren": false
    },
-    {
-      "path": "hooks.internal.handlers",
-      "kind": "core",
-      "type": "array",
-      "required": false,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Handlers",
-      "help": "List of internal event handlers mapping event names to modules and optional exports. Keep handler definitions explicit so event-to-code routing is auditable.",
-      "hasChildren": true
-    },
-    {
-      "path": "hooks.internal.handlers.*",
-      "kind": "core",
-      "type": "object",
-      "required": false,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [],
-      "hasChildren": true
-    },
-    {
-      "path": "hooks.internal.handlers.*.event",
-      "kind": "core",
-      "type": "string",
-      "required": true,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Event",
-      "help": "Internal event name that triggers this handler module when emitted by the runtime. Use stable event naming conventions to avoid accidental overlap across handlers.",
-      "hasChildren": false
-    },
-    {
-      "path": "hooks.internal.handlers.*.export",
-      "kind": "core",
-      "type": "string",
-      "required": false,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Export",
-      "help": "Optional named export for the internal hook handler function when module default export is not used. Set this when one module ships multiple handler entrypoints.",
-      "hasChildren": false
-    },
-    {
-      "path": "hooks.internal.handlers.*.module",
-      "kind": "core",
-      "type": "string",
-      "required": true,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Module",
-      "help": "Safe relative module path for the internal hook handler implementation loaded at runtime. Keep module files in reviewed directories and avoid dynamic path composition.",
-      "hasChildren": false
-    },
    {
      "path": "hooks.internal.installs",
      "kind": "core",
--- a/docs/.generated/config-baseline.json
+++ b/docs/.generated/config-baseline.json
@@ -12278,7 +12278,7 @@
        "advanced"
      ],
      "label": "Internal Hooks Enabled",
-      "help": "Enables processing for internal hook handlers and configured entries in the internal hook runtime. Keep disabled unless internal hook handlers are intentionally configured.",
+      "help": "Enables processing for internal hooks and configured entries in the internal hook runtime. Keep disabled unless internal hooks are intentionally configured.",
      "hasChildren": false
    },
    {
@@ -12344,72 +12344,6 @@
      "tags": [],
      "hasChildren": false
    },
-    {
-      "path": "hooks.internal.handlers",
-      "kind": "core",
-      "type": "array",
-      "required": false,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Handlers",
-      "help": "List of internal event handlers mapping event names to modules and optional exports. Keep handler definitions explicit so event-to-code routing is auditable.",
-      "hasChildren": true
-    },
-    {
-      "path": "hooks.internal.handlers.*",
-      "kind": "core",
-      "type": "object",
-      "required": false,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [],
-      "hasChildren": true
-    },
-    {
-      "path": "hooks.internal.handlers.*.event",
-      "kind": "core",
-      "type": "string",
-      "required": true,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Event",
-      "help": "Internal event name that triggers this handler module when emitted by the runtime. Use stable event naming conventions to avoid accidental overlap across handlers.",
-      "hasChildren": false
-    },
-    {
-      "path": "hooks.internal.handlers.*.export",
-      "kind": "core",
-      "type": "string",
-      "required": false,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Export",
-      "help": "Optional named export for the internal hook handler function when module default export is not used. Set this when one module ships multiple handler entrypoints.",
-      "hasChildren": false
-    },
-    {
-      "path": "hooks.internal.handlers.*.module",
-      "kind": "core",
-      "type": "string",
-      "required": true,
-      "deprecated": false,
-      "sensitive": false,
-      "tags": [
-        "advanced"
-      ],
-      "label": "Internal Hook Module",
-      "help": "Safe relative module path for the internal hook handler implementation loaded at runtime. Keep module files in reviewed directories and avoid dynamic path composition.",
-      "hasChildren": false
-    },
    {
      "path": "hooks.internal.installs",
      "kind": "core",
--- a/src/commands/doctor-config-flow.test.ts
+++ b/src/commands/doctor-config-flow.test.ts
@@ -1430,6 +1430,40 @@ describe("doctor config flow", () => {
    }
  });

+  it("warns clearly about legacy hooks.internal.handlers and requires manual migration", async () => {
+    const noteSpy = vi.spyOn(noteModule, "note").mockImplementation(() => {});
+    try {
+      await runDoctorConfigWithInput({
+        config: {
+          hooks: {
+            internal: {
+              handlers: [{ event: "command:new", module: "hooks/legacy-handler.js" }],
+            },
+          },
+        },
+        run: loadAndMaybeMigrateDoctorConfig,
+      });
+
+      expect(
+        noteSpy.mock.calls.some(
+          ([message, title]) =>
+            title === "Legacy config keys detected" &&
+            String(message).includes("hooks.internal.handlers:") &&
+            String(message).includes("HOOK.md + handler.js"),
+        ),
+      ).toBe(true);
+      expect(
+        noteSpy.mock.calls.some(
+          ([message, title]) =>
+            title === "Legacy config keys detected" &&
+            String(message).includes("does not rewrite this shape automatically"),
+        ),
+      ).toBe(true);
+    } finally {
+      noteSpy.mockRestore();
+    }
+  });
+
  it("warns clearly about legacy thread binding ttlHours config and points to doctor --fix", async () => {
    const noteSpy = vi.spyOn(noteModule, "note").mockImplementation(() => {});
    try {
--- a/src/commands/doctor-config-flow.ts
+++ b/src/commands/doctor-config-flow.ts
@@ -31,6 +31,12 @@ import {
 } from "./doctor/shared/mutable-allowlist.js";
 import { collectDoctorPreviewWarnings } from "./doctor/shared/preview-warnings.js";

+function hasLegacyInternalHookHandlers(raw: unknown): boolean {
+  const handlers = (raw as { hooks?: { internal?: { handlers?: unknown } } })?.hooks?.internal
+    ?.handlers;
+  return Array.isArray(handlers) && handlers.length > 0;
+}
+
 export async function loadAndMaybeMigrateDoctorConfig(params: {
  options: DoctorOptions;
  confirm: (p: { message: string; initialValue: boolean }) => Promise<boolean>;
@@ -58,6 +64,16 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
  if (legacyStep.changeLines.length > 0) {
    note(legacyStep.changeLines.join("\n"), "Doctor changes");
  }
+  if (hasLegacyInternalHookHandlers(snapshot.parsed)) {
+    note(
+      [
+        "- hooks.internal.handlers: legacy inline hook modules are no longer part of the public config surface.",
+        "- Migrate each entry to a managed or workspace hook directory with HOOK.md + handler.js, then enable it through hooks.internal.entries.<hookKey> as needed.",
+        "- openclaw doctor --fix does not rewrite this shape automatically.",
+      ].join("\n"),
+      "Legacy config keys detected",
+    );
+  }

  const normalized = normalizeCompatibilityConfigValues(candidate);
  if (normalized.changes.length > 0) {
--- a/src/config/doc-baseline.integration.test.ts
+++ b/src/config/doc-baseline.integration.test.ts
@@ -114,6 +114,13 @@ describe("config doc baseline integration", () => {
    expect(tokenEntry?.tags).toContain("security");
  });

+  it("omits legacy hooks.internal.handlers from the generated baseline", async () => {
+    const byPath = await getSharedByPath();
+
+    expect(byPath.get("hooks.internal.handlers")).toBeUndefined();
+    expect(byPath.get("hooks.internal.handlers.*.module")).toBeUndefined();
+  });
+
  it("uses human-readable channel metadata for top-level channel sections", async () => {
    const byPath = await getSharedByPath();

--- a/src/config/schema-base.ts
+++ b/src/config/schema-base.ts
@@ -19,6 +19,8 @@ type JsonSchemaObject = Record<string, unknown> & {
  additionalProperties?: JsonSchemaObject | boolean;
 };

+const LEGACY_HIDDEN_PUBLIC_PATHS = ["hooks.internal.handlers"] as const;
+
 const asJsonSchemaObject = (value: unknown): JsonSchemaObject | null =>
  asSchemaObject<JsonSchemaObject>(value);

@@ -52,6 +54,50 @@ function stripChannelSchema(schema: ConfigSchema): ConfigSchema {
  return next;
 }

+function stripObjectPropertyPath(schema: ConfigSchema, path: readonly string[]): void {
+  const root = asJsonSchemaObject(schema);
+  if (!root || path.length === 0) {
+    return;
+  }
+
+  let current: JsonSchemaObject | null = root;
+  for (const segment of path.slice(0, -1)) {
+    current = asJsonSchemaObject(current?.properties?.[segment]);
+    if (!current) {
+      return;
+    }
+  }
+
+  const key = path[path.length - 1];
+  if (!current?.properties || !key) {
+    return;
+  }
+  delete current.properties[key];
+  if (Array.isArray(current.required)) {
+    current.required = current.required.filter((entry) => entry !== key);
+  }
+}
+
+function stripLegacyCompatSchemaPaths(schema: ConfigSchema): ConfigSchema {
+  const next = cloneSchema(schema);
+  for (const path of LEGACY_HIDDEN_PUBLIC_PATHS) {
+    stripObjectPropertyPath(next, path.split("."));
+  }
+  return next;
+}
+
+function stripLegacyCompatHints(hints: ConfigUiHints): ConfigUiHints {
+  const next: ConfigUiHints = { ...hints };
+  for (const path of LEGACY_HIDDEN_PUBLIC_PATHS) {
+    for (const key of Object.keys(next)) {
+      if (key === path || key.startsWith(`${path}.`) || key.startsWith(`${path}[`)) {
+        delete next[key];
+      }
+    }
+  }
+  return next;
+}
+
 let baseConfigSchemaStablePayload: BaseConfigSchemaStablePayload | null = null;

 function computeBaseConfigSchemaStablePayload(): BaseConfigSchemaStablePayload {
@@ -74,8 +120,10 @@ function computeBaseConfigSchemaStablePayload(): BaseConfigSchemaStablePayload {
    isSensitiveUrlConfigPath,
  );
  const stablePayload = {
-    schema: stripChannelSchema(schema),
-    uiHints: applyDerivedTags(applySensitiveUrlHints(baseHints, sensitiveUrlPaths)),
+    schema: stripLegacyCompatSchemaPaths(stripChannelSchema(schema)),
+    uiHints: stripLegacyCompatHints(
+      applyDerivedTags(applySensitiveUrlHints(baseHints, sensitiveUrlPaths)),
+    ),
    version: VERSION,
  } satisfies BaseConfigSchemaStablePayload;
  baseConfigSchemaStablePayload = stablePayload;
--- a/src/config/schema.base.generated.test.ts
+++ b/src/config/schema.base.generated.test.ts
@@ -20,4 +20,24 @@ describe("generated base config schema", () => {
      SENSITIVE_URL_HINT_TAG,
    );
  });
+
+  it("omits legacy hooks.internal.handlers from the public schema payload", () => {
+    const hooksInternalProperties = (
+      GENERATED_BASE_CONFIG_SCHEMA.schema as {
+        properties?: {
+          hooks?: {
+            properties?: {
+              internal?: {
+                properties?: Record<string, unknown>;
+              };
+            };
+          };
+        };
+      }
+    ).properties?.hooks?.properties?.internal?.properties;
+    const uiHints = GENERATED_BASE_CONFIG_SCHEMA.uiHints as Record<string, unknown>;
+
+    expect(hooksInternalProperties?.handlers).toBeUndefined();
+    expect(uiHints["hooks.internal.handlers"]).toBeUndefined();
+  });
 });
--- a/src/config/schema.base.generated.ts
+++ b/src/config/schema.base.generated.ts
@@ -18009,25 +18009,6 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
              enabled: {
                type: "boolean",
              },
-              handlers: {
-                type: "array",
-                items: {
-                  type: "object",
-                  properties: {
-                    event: {
-                      type: "string",
-                    },
-                    module: {
-                      type: "string",
-                    },
-                    export: {
-                      type: "string",
-                    },
-                  },
-                  required: ["event", "module"],
-                  additionalProperties: false,
-                },
-              },
              entries: {
                type: "object",
                propertyNames: {
@@ -23568,27 +23549,7 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
    },
    "hooks.internal.enabled": {
      label: "Internal Hooks Enabled",
-      help: "Enables processing for internal hook handlers and configured entries in the internal hook runtime. Keep disabled unless internal hook handlers are intentionally configured.",
-      tags: ["advanced"],
-    },
-    "hooks.internal.handlers": {
-      label: "Internal Hook Handlers",
-      help: "List of internal event handlers mapping event names to modules and optional exports. Keep handler definitions explicit so event-to-code routing is auditable.",
-      tags: ["advanced"],
-    },
-    "hooks.internal.handlers[].event": {
-      label: "Internal Hook Event",
-      help: "Internal event name that triggers this handler module when emitted by the runtime. Use stable event naming conventions to avoid accidental overlap across handlers.",
-      tags: ["advanced"],
-    },
-    "hooks.internal.handlers[].module": {
-      label: "Internal Hook Module",
-      help: "Safe relative module path for the internal hook handler implementation loaded at runtime. Keep module files in reviewed directories and avoid dynamic path composition.",
-      tags: ["advanced"],
-    },
-    "hooks.internal.handlers[].export": {
-      label: "Internal Hook Export",
-      help: "Optional named export for the internal hook handler function when module default export is not used. Set this when one module ships multiple handler entrypoints.",
+      help: "Enables processing for internal hooks and configured entries in the internal hook runtime. Keep disabled unless internal hooks are intentionally configured.",
      tags: ["advanced"],
    },
    "hooks.internal.entries": {
--- a/src/config/schema.help.quality.test.ts
+++ b/src/config/schema.help.quality.test.ts
@@ -233,9 +233,6 @@ const TARGET_KEYS = [
  "hooks.gmail.tailscale.mode",
  "hooks.gmail.thinking",
  "hooks.internal",
-  "hooks.internal.handlers",
-  "hooks.internal.handlers[].event",
-  "hooks.internal.handlers[].module",
  "hooks.internal.load.extraDirs",
  "messages",
  "messages.messagePrefix",
--- a/src/config/schema.help.ts
+++ b/src/config/schema.help.ts
@@ -1422,15 +1422,7 @@ export const FIELD_HELP: Record<string, string> = {
  "hooks.internal":
    "Internal hook runtime settings for bundled/custom event handlers loaded from module paths. Use this for trusted in-process automations and keep handler loading tightly scoped.",
  "hooks.internal.enabled":
-    "Enables processing for internal hook handlers and configured entries in the internal hook runtime. Keep disabled unless internal hook handlers are intentionally configured.",
-  "hooks.internal.handlers":
-    "List of internal event handlers mapping event names to modules and optional exports. Keep handler definitions explicit so event-to-code routing is auditable.",
-  "hooks.internal.handlers[].event":
-    "Internal event name that triggers this handler module when emitted by the runtime. Use stable event naming conventions to avoid accidental overlap across handlers.",
-  "hooks.internal.handlers[].module":
-    "Safe relative module path for the internal hook handler implementation loaded at runtime. Keep module files in reviewed directories and avoid dynamic path composition.",
-  "hooks.internal.handlers[].export":
-    "Optional named export for the internal hook handler function when module default export is not used. Set this when one module ships multiple handler entrypoints.",
+    "Enables processing for internal hooks and configured entries in the internal hook runtime. Keep disabled unless internal hooks are intentionally configured.",
  "hooks.internal.entries":
    "Configured internal hook entry records used to register concrete runtime handlers and metadata. Keep entries explicit and versioned so production behavior is auditable.",
  "hooks.internal.load":
--- a/src/config/schema.labels.ts
+++ b/src/config/schema.labels.ts
@@ -690,10 +690,6 @@ export const FIELD_LABELS: Record<string, string> = {
  "hooks.gmail.thinking": "Gmail Hook Thinking Override",
  "hooks.internal": "Internal Hooks",
  "hooks.internal.enabled": "Internal Hooks Enabled",
-  "hooks.internal.handlers": "Internal Hook Handlers",
-  "hooks.internal.handlers[].event": "Internal Hook Event",
-  "hooks.internal.handlers[].module": "Internal Hook Module",
-  "hooks.internal.handlers[].export": "Internal Hook Export",
  "hooks.internal.entries": "Internal Hook Entries",
  "hooks.internal.load": "Internal Hook Loader",
  "hooks.internal.load.extraDirs": "Internal Hook Extra Directories",
--- a/src/config/types.hooks.ts
+++ b/src/config/types.hooks.ts
@@ -66,15 +66,6 @@ export type HooksGmailConfig = {
  thinking?: "off" | "minimal" | "low" | "medium" | "high";
 };

-export type InternalHookHandlerConfig = {
-  /** Event key to listen for (e.g., 'command:new', 'message:received', 'message:transcribed', 'session:start') */
-  event: string;
-  /** Path to handler module (workspace-relative) */
-  module: string;
-  /** Export name from module (default: 'default') */
-  export?: string;
-};
-
 export type HookConfig = {
  enabled?: boolean;
  env?: Record<string, string>;
@@ -88,8 +79,6 @@ export type HookInstallRecord = InstallRecordBase & {
 export type InternalHooksConfig = {
  /** Enable hooks system */
  enabled?: boolean;
-  /** Legacy: List of internal hook handlers to register (still supported) */
-  handlers?: InternalHookHandlerConfig[];
  /** Per-hook configuration overrides */
  entries?: Record<string, HookConfig>;
  /** Load configuration */
--- a/src/hooks/legacy-config.ts
+++ b/src/hooks/legacy-config.ts
@@ -0,0 +1,18 @@
+type LegacyInternalHookHandler = {
+  event: string;
+  module: string;
+  export?: string;
+};
+
+type LegacyInternalHooksCarrier = {
+  hooks?: {
+    internal?: {
+      handlers?: LegacyInternalHookHandler[];
+    };
+  };
+};
+
+export function getLegacyInternalHookHandlers(config: unknown): LegacyInternalHookHandler[] {
+  const handlers = (config as LegacyInternalHooksCarrier)?.hooks?.internal?.handlers;
+  return Array.isArray(handlers) ? handlers : [];
+}
--- a/src/hooks/loader.test.ts
+++ b/src/hooks/loader.test.ts
@@ -82,14 +82,36 @@ describe("loader", () => {
    return handlerPath;
  }

+  function withLegacyInternalHookHandlers(
+    config: OpenClawConfig,
+    handlers?: Array<{ event: string; module: string; export?: string }>,
+  ): OpenClawConfig {
+    if (!handlers) {
+      return config;
+    }
+    return {
+      ...config,
+      hooks: {
+        ...config.hooks,
+        internal: {
+          ...config.hooks?.internal,
+          handlers,
+        },
+      },
+    } as OpenClawConfig;
+  }
+
  function createEnabledHooksConfig(
    handlers?: Array<{ event: string; module: string; export?: string }>,
  ): OpenClawConfig {
-    return {
-      hooks: {
-        internal: handlers ? { enabled: true, handlers } : { enabled: true },
+    return withLegacyInternalHookHandlers(
+      {
+        hooks: {
+          internal: { enabled: true },
+        },
      },
-    };
+      handlers,
+    );
  }

  afterEach(async () => {
@@ -130,14 +152,16 @@ describe("loader", () => {
            },
          },
        } satisfies OpenClawConfig,
-        {
-          hooks: {
-            internal: {
-              enabled: false,
-              handlers: [],
+        withLegacyInternalHookHandlers(
+          {
+            hooks: {
+              internal: {
+                enabled: false,
+              },
            },
-          },
-        } satisfies OpenClawConfig,
+          } satisfies OpenClawConfig,
+          [],
+        ),
      ]) {
        const count = await loadInternalHooks(cfg, tmpDir);
        expect(count).toBe(0);
--- a/src/hooks/loader.ts
+++ b/src/hooks/loader.ts
@@ -15,6 +15,7 @@ import { shouldIncludeHook } from "./config.js";
 import { buildImportUrl } from "./import-url.js";
 import type { InternalHookHandler } from "./internal-hooks.js";
 import { registerInternalHook } from "./internal-hooks.js";
+import { getLegacyInternalHookHandlers } from "./legacy-config.js";
 import { resolveFunctionModuleExport } from "./module-loader.js";
 import { loadWorkspaceHookEntries } from "./workspace.js";

@@ -153,7 +154,7 @@ export async function loadInternalHooks(
  }

  // 2. Load legacy config handlers (backwards compatibility)
-  const handlers = cfg.hooks?.internal?.handlers ?? [];
+  const handlers = getLegacyInternalHookHandlers(cfg);
  for (const handlerConfig of handlers) {
    try {
      // Legacy handler paths: keep them workspace-relative.