fix(agents): quarantine unreadable extension tools

vincentkoc · vincentkoc · commit 179ab368b746 · 2026-06-02T12:13:09.000+02:00
diff --git a/src/agents/sessions/extensions/loader.test.ts b/src/agents/sessions/extensions/loader.test.ts
@@ -3,6 +3,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
 import { loadExtensions } from "./loader.js";
+import type { ExtensionContext } from "./types.js";
 
 const tempDirs: string[] = [];
 
@@ -43,4 +44,81 @@ export default async function(api) {
     expect(result.extensions).toHaveLength(1);
     expect(result.extensions[0]?.commands.has("sdk-subpath-probe")).toBe(true);
   });
+
+  it("skips unreadable registered tools without failing the extension", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "openclaw-extension-bad-tool-"));
+    tempDirs.push(dir);
+    const extensionPath = join(dir, "extension.ts");
+    await writeFile(
+      extensionPath,
+      `
+export default async function(api) {
+  const unreadableNameTool = {
+    get name() {
+      throw new Error("boom name");
+    },
+    label: "Bad Name",
+    description: "Breaks while reading the name.",
+    parameters: { type: "object", properties: {} },
+    async execute() {
+      return { content: [{ type: "text", text: "bad" }], details: {} };
+    },
+  };
+  const unreadableParametersTool = {
+    name: "bad_parameters",
+    label: "Bad Parameters",
+    description: "Breaks while reading the schema.",
+    get parameters() {
+      throw new Error("boom params");
+    },
+    async execute() {
+      return { content: [{ type: "text", text: "bad" }], details: {} };
+    },
+  };
+  const healthyTool = {
+    name: "healthy_lookup",
+    label: "Healthy Lookup",
+    description: "Uses state from the original tool object.",
+    parameters: { type: "object", properties: {} },
+    calls: 0,
+    async execute() {
+      this.calls += 1;
+      return { content: [{ type: "text", text: String(this.calls) }], details: {} };
+    },
+  };
+  api.registerTool(unreadableNameTool);
+  api.registerTool(unreadableParametersTool);
+  api.registerTool(healthyTool);
+  api.registerCommand("after-bad-tool", {
+    description: "probe",
+    handler() {},
+  });
+}
+`,
+    );
+
+    const result = await loadExtensions([extensionPath], dir);
+
+    expect(result.errors).toEqual([
+      { path: extensionPath, error: "skipped extension tool registration: boom name" },
+      {
+        path: extensionPath,
+        error: 'skipped extension tool registration for "bad_parameters": boom params',
+      },
+    ]);
+    expect(result.extensions).toHaveLength(1);
+    expect(Array.from(result.extensions[0]?.tools.keys() ?? [])).toEqual(["healthy_lookup"]);
+    expect(result.extensions[0]?.commands.has("after-bad-tool")).toBe(true);
+
+    const definition = result.extensions[0]?.tools.get("healthy_lookup")?.definition;
+    const toolResult = await definition?.execute(
+      "call-1",
+      {},
+      undefined,
+      undefined,
+      undefined as unknown as ExtensionContext,
+    );
+
+    expect(toolResult?.content).toEqual([{ type: "text", text: "1" }]);
+  });
 });
diff --git a/src/agents/sessions/extensions/loader.ts b/src/agents/sessions/extensions/loader.ts
@@ -214,6 +214,7 @@ function createExtensionAPI(
   runtime: ExtensionRuntime,
   cwd: string,
   eventBus: EventBus,
+  onRegistrationError?: (error: string) => void,
 ): ExtensionAPI {
   const api = {
     // Registration methods - write to extension
@@ -226,8 +227,13 @@ function createExtensionAPI(
 
     registerTool(tool: ToolDefinition): void {
       runtime.assertActive();
-      extension.tools.set(tool.name, {
-        definition: tool,
+      const snapshot = snapshotExtensionToolDefinition(tool);
+      if ("error" in snapshot) {
+        onRegistrationError?.(snapshot.error);
+        return;
+      }
+      extension.tools.set(snapshot.definition.name, {
+        definition: snapshot.definition,
         sourceInfo: extension.sourceInfo,
       });
       runtime.refreshTools();
@@ -365,6 +371,75 @@ function createExtensionAPI(
   return api;
 }
 
+function snapshotExtensionToolDefinition(
+  definition: ToolDefinition,
+): { definition: ToolDefinition } | { error: string } {
+  let toolName: string | undefined;
+  try {
+    if (!definition || typeof definition !== "object") {
+      return { error: "skipped extension tool registration: tool definition is not an object" };
+    }
+    const name = Reflect.get(definition, "name");
+    toolName = typeof name === "string" ? name : undefined;
+    const execute = Reflect.get(definition, "execute");
+    if (!toolName) {
+      return { error: "skipped extension tool registration: tool name is missing or invalid" };
+    }
+    if (typeof execute !== "function") {
+      return {
+        error: `skipped extension tool registration for "${toolName}": execute is missing or invalid`,
+      };
+    }
+    const promptGuidelines = Reflect.get(definition, "promptGuidelines");
+    const prepareArguments = Reflect.get(definition, "prepareArguments");
+    const renderCall = Reflect.get(definition, "renderCall");
+    const renderResult = Reflect.get(definition, "renderResult");
+    const executeWithReceiver = ((...args: Parameters<ToolDefinition["execute"]>) =>
+      Reflect.apply(execute, definition, args)) as ToolDefinition["execute"];
+    return {
+      definition: {
+        name: toolName,
+        label: Reflect.get(definition, "label"),
+        description: Reflect.get(definition, "description"),
+        promptSnippet: Reflect.get(definition, "promptSnippet"),
+        promptGuidelines: Array.isArray(promptGuidelines)
+          ? [...promptGuidelines]
+          : promptGuidelines,
+        parameters: Reflect.get(definition, "parameters"),
+        renderShell: Reflect.get(definition, "renderShell"),
+        prepareArguments:
+          typeof prepareArguments === "function"
+            ? (((...args: Parameters<NonNullable<ToolDefinition["prepareArguments"]>>) =>
+                Reflect.apply(
+                  prepareArguments,
+                  definition,
+                  args,
+                )) as ToolDefinition["prepareArguments"])
+            : prepareArguments,
+        executionMode: Reflect.get(definition, "executionMode"),
+        execute: executeWithReceiver,
+        renderCall:
+          typeof renderCall === "function"
+            ? (((...args: Parameters<NonNullable<ToolDefinition["renderCall"]>>) =>
+                Reflect.apply(renderCall, definition, args)) as ToolDefinition["renderCall"])
+            : renderCall,
+        renderResult:
+          typeof renderResult === "function"
+            ? (((...args: Parameters<NonNullable<ToolDefinition["renderResult"]>>) =>
+                Reflect.apply(renderResult, definition, args)) as ToolDefinition["renderResult"])
+            : renderResult,
+      },
+    };
+  } catch (error) {
+    const message = error instanceof Error ? error.message : String(error);
+    return {
+      error: toolName
+        ? `skipped extension tool registration for "${toolName}": ${message}`
+        : `skipped extension tool registration: ${message}`,
+    };
+  }
+}
+
 function resolveExtensionFactory(module: unknown): ExtensionFactory | undefined {
   const candidate =
     typeof module === "object" && module !== null && "default" in module
@@ -487,26 +562,34 @@ async function loadExtension(
   cwd: string,
   eventBus: EventBus,
   runtime: ExtensionRuntime,
-): Promise<{ extension: Extension | null; error: string | null }> {
+): Promise<{ extension: Extension | null; error: string | null; registrationErrors: string[] }> {
   const resolvedPath = resolvePath(extensionPath, cwd);
+  const registrationErrors: string[] = [];
 
   try {
     const factory = await loadExtensionModule(resolvedPath);
     if (!factory) {
       return {
         extension: null,
         error: `Extension does not export a valid factory function: ${extensionPath}`,
+        registrationErrors,
       };
     }
 
     const extension = createExtension(extensionPath, resolvedPath);
-    const api = createExtensionAPI(extension, runtime, cwd, eventBus);
+    const api = createExtensionAPI(extension, runtime, cwd, eventBus, (error) => {
+      registrationErrors.push(error);
+    });
     await factory(api);
 
-    return { extension, error: null };
+    return { extension, error: null, registrationErrors };
   } catch (err) {
     const message = err instanceof Error ? err.message : String(err);
-    return { extension: null, error: `Failed to load extension: ${message}` };
+    return {
+      extension: null,
+      error: `Failed to load extension: ${message}`,
+      registrationErrors,
+    };
   }
 }
 
@@ -519,9 +602,12 @@ export async function loadExtensionFromFactory(
   eventBus: EventBus,
   runtime: ExtensionRuntime,
   extensionPath = "<inline>",
+  registrationErrors?: string[],
 ): Promise<Extension> {
   const extension = createExtension(extensionPath, extensionPath);
-  const api = createExtensionAPI(extension, runtime, cwd, eventBus);
+  const api = createExtensionAPI(extension, runtime, cwd, eventBus, (error) => {
+    registrationErrors?.push(error);
+  });
   await factory(api);
   return extension;
 }
@@ -540,16 +626,27 @@ export async function loadExtensions(
   const runtime = createExtensionRuntime();
 
   for (const extPath of paths) {
-    const { extension, error } = await loadExtension(extPath, cwd, resolvedEventBus, runtime);
+    const { extension, error, registrationErrors } = await loadExtension(
+      extPath,
+      cwd,
+      resolvedEventBus,
+      runtime,
+    );
 
     if (error) {
       errors.push({ path: extPath, error });
+      for (const registrationError of registrationErrors) {
+        errors.push({ path: extPath, error: registrationError });
+      }
       continue;
     }
 
     if (extension) {
       extensions.push(extension);
     }
+    for (const registrationError of registrationErrors) {
+      errors.push({ path: extPath, error: registrationError });
+    }
   }
 
   return {
diff --git a/src/agents/sessions/resource-loader.ts b/src/agents/sessions/resource-loader.ts
@@ -880,17 +880,25 @@ export class DefaultResourceLoader implements ResourceLoader {
 
     for (const [index, factory] of this.extensionFactories.entries()) {
       const extensionPath = `<inline:${index + 1}>`;
+      const registrationErrors: string[] = [];
       try {
         const extension = await loadExtensionFromFactory(
           factory,
           this.cwd,
           this.eventBus,
           runtime,
           extensionPath,
+          registrationErrors,
         );
         extensions.push(extension);
+        for (const registrationError of registrationErrors) {
+          errors.push({ path: extensionPath, error: registrationError });
+        }
       } catch (error) {
         const message = error instanceof Error ? error.message : "failed to load extension";
+        for (const registrationError of registrationErrors) {
+          errors.push({ path: extensionPath, error: registrationError });
+        }
         errors.push({ path: extensionPath, error: message });
       }
     }
