refactor: Move createTool from SkillManager to skillTool.ts

lewis617 · lewis617 · commit bcb83e9cda7b · 2025-10-30T12:16:21.000+08:00
diff --git a/packages/agent-sdk/src/managers/skillManager.ts b/packages/agent-sdk/src/managers/skillManager.ts
@@ -11,7 +11,6 @@ import type {
   SkillInvocationContext,
   Logger,
 } from "../types.js";
-import type { ToolPlugin, ToolResult } from "../tools/types.js";
 import { parseSkillFile, formatSkillError } from "../utils/skillParser.js";
 
 /**
@@ -76,6 +75,13 @@ export class SkillManager {
     }
   }
 
+  /**
+   * Check if the skill manager is initialized
+   */
+  isInitialized(): boolean {
+    return this.initialized;
+  }
+
   /**
    * Get all available skills metadata
    */
@@ -233,101 +239,6 @@ export class SkillManager {
     return directories;
   }
 
-  /**
-   * Create a tool plugin for registering with ToolManager
-   */
-  createTool(): ToolPlugin {
-    // Initialize skill manager asynchronously
-    let initializationPromise: Promise<void> | null = null;
-
-    const ensureInitialized = async (): Promise<void> => {
-      if (!initializationPromise) {
-        initializationPromise = this.initialize();
-      }
-      await initializationPromise;
-    };
-
-    const getToolDescription = (): string => {
-      if (!this.initialized) {
-        return "Invoke a Wave skill by name. Skills are user-defined automation templates that can be personal or project-specific. Skills will be loaded during initialization.";
-      }
-
-      const availableSkills = this.getAvailableSkills();
-
-      if (availableSkills.length === 0) {
-        return "Invoke a Wave skill by name. Skills are user-defined automation templates that can be personal or project-specific. No skills are currently available.";
-      }
-
-      const skillList = availableSkills
-        .map(
-          (skill) =>
-            `• **${skill.name}** (${skill.type}): ${skill.description}`,
-        )
-        .join("\n");
-
-      return `Invoke a Wave skill by name. Skills are user-defined automation templates that can be personal or project-specific.\n\nAvailable skills:\n${skillList}`;
-    };
-
-    return {
-      name: "skill",
-      config: {
-        type: "function",
-        function: {
-          name: "skill",
-          description: getToolDescription(),
-          parameters: {
-            type: "object",
-            properties: {
-              skill_name: {
-                type: "string",
-                description: "Name of the skill to invoke",
-                enum: this.initialized
-                  ? this.getAvailableSkills().map((skill) => skill.name)
-                  : [],
-              },
-            },
-            required: ["skill_name"],
-          },
-        },
-      },
-      execute: async (args: Record<string, unknown>): Promise<ToolResult> => {
-        try {
-          // Ensure skill manager is initialized
-          await ensureInitialized();
-
-          // Validate arguments
-          const skillName = args.skill_name as string;
-          if (!skillName || typeof skillName !== "string") {
-            return {
-              success: false,
-              content: "",
-              error: "skill_name parameter is required and must be a string",
-            };
-          }
-
-          // Execute the skill
-          const result = await this.executeSkill({ skill_name: skillName });
-
-          return {
-            success: true,
-            content: result.content,
-            shortResult: `Invoked skill: ${skillName}`,
-          };
-        } catch (error) {
-          return {
-            success: false,
-            content: "",
-            error: error instanceof Error ? error.message : String(error),
-          };
-        }
-      },
-      formatCompactParams: (params: Record<string, unknown>) => {
-        const skillName = params.skill_name as string;
-        return skillName || "unknown-skill";
-      },
-    };
-  }
-
   /**
    * Execute a skill by name
    */
diff --git a/packages/agent-sdk/src/managers/toolManager.ts b/packages/agent-sdk/src/managers/toolManager.ts
@@ -10,6 +10,7 @@ import { grepTool } from "../tools/grepTool.js";
 import { lsTool } from "../tools/lsTool.js";
 import { readTool } from "../tools/readTool.js";
 import { SkillManager } from "./skillManager.js";
+import { createSkillTool } from "../tools/skillTool.js";
 import { McpManager } from "./mcpManager.js";
 import { ChatCompletionFunctionTool } from "openai/resources.js";
 import type { Logger } from "../types.js";
@@ -48,7 +49,7 @@ class ToolManager {
       grepTool,
       lsTool,
       readTool,
-      new SkillManager({ logger: this.logger }).createTool(),
+      createSkillTool(new SkillManager({ logger: this.logger })),
     ];
 
     for (const tool of builtInTools) {
diff --git a/packages/agent-sdk/src/tools/skillTool.ts b/packages/agent-sdk/src/tools/skillTool.ts
@@ -0,0 +1,98 @@
+import type { ToolPlugin, ToolResult } from "./types.js";
+import type { SkillManager } from "../managers/skillManager.js";
+
+/**
+ * Create a skill tool plugin that uses the provided SkillManager
+ */
+export function createSkillTool(skillManager: SkillManager): ToolPlugin {
+  // Initialize skill manager asynchronously
+  let initializationPromise: Promise<void> | null = null;
+
+  const ensureInitialized = async (): Promise<void> => {
+    if (!initializationPromise) {
+      initializationPromise = skillManager.initialize();
+    }
+    await initializationPromise;
+  };
+
+  const getToolDescription = (): string => {
+    if (!skillManager.isInitialized()) {
+      return "Invoke a Wave skill by name. Skills are user-defined automation templates that can be personal or project-specific. Skills will be loaded during initialization.";
+    }
+
+    const availableSkills = skillManager.getAvailableSkills();
+
+    if (availableSkills.length === 0) {
+      return "Invoke a Wave skill by name. Skills are user-defined automation templates that can be personal or project-specific. No skills are currently available.";
+    }
+
+    const skillList = availableSkills
+      .map(
+        (skill) => `• **${skill.name}** (${skill.type}): ${skill.description}`,
+      )
+      .join("\n");
+
+    return `Invoke a Wave skill by name. Skills are user-defined automation templates that can be personal or project-specific.\n\nAvailable skills:\n${skillList}`;
+  };
+
+  return {
+    name: "skill",
+    config: {
+      type: "function",
+      function: {
+        name: "skill",
+        description: getToolDescription(),
+        parameters: {
+          type: "object",
+          properties: {
+            skill_name: {
+              type: "string",
+              description: "Name of the skill to invoke",
+              enum: skillManager.isInitialized()
+                ? skillManager.getAvailableSkills().map((skill) => skill.name)
+                : [],
+            },
+          },
+          required: ["skill_name"],
+        },
+      },
+    },
+    execute: async (args: Record<string, unknown>): Promise<ToolResult> => {
+      try {
+        // Ensure skill manager is initialized
+        await ensureInitialized();
+
+        // Validate arguments
+        const skillName = args.skill_name as string;
+        if (!skillName || typeof skillName !== "string") {
+          return {
+            success: false,
+            content: "",
+            error: "skill_name parameter is required and must be a string",
+          };
+        }
+
+        // Execute the skill
+        const result = await skillManager.executeSkill({
+          skill_name: skillName,
+        });
+
+        return {
+          success: true,
+          content: result.content,
+          shortResult: `Invoked skill: ${skillName}`,
+        };
+      } catch (error) {
+        return {
+          success: false,
+          content: "",
+          error: error instanceof Error ? error.message : String(error),
+        };
+      }
+    },
+    formatCompactParams: (params: Record<string, unknown>) => {
+      const skillName = params.skill_name as string;
+      return skillName || "unknown-skill";
+    },
+  };
+}
diff --git a/packages/agent-sdk/tests/managers/skillManager.test.ts b/packages/agent-sdk/tests/managers/skillManager.test.ts
@@ -40,6 +40,10 @@ describe("SkillManager", () => {
   });
 
   describe("initialization checks", () => {
+    it("should return false when not initialized", () => {
+      expect(skillManager.isInitialized()).toBe(false);
+    });
+
     it("should throw error if not initialized when getting skills", () => {
       expect(() => skillManager.getAvailableSkills()).toThrow(
         "SkillManager not initialized. Call initialize() first.",
@@ -53,89 +57,6 @@ describe("SkillManager", () => {
     });
   });
 
-  describe("createTool", () => {
-    it("should create a tool plugin with correct structure when not initialized", () => {
-      const tool = skillManager.createTool();
-
-      expect(tool.name).toBe("skill");
-      expect(tool.config).toBeDefined();
-      expect(tool.config.type).toBe("function");
-      expect(tool.config.function.name).toBe("skill");
-      expect(typeof tool.execute).toBe("function");
-      expect(typeof tool.formatCompactParams).toBe("function");
-
-      // When not initialized, enum should be empty
-      const params = tool.config.function?.parameters as Record<
-        string,
-        unknown
-      >;
-      const properties = params?.properties as Record<string, unknown>;
-      const skillName = properties?.skill_name as Record<string, unknown>;
-      expect(skillName?.enum).toEqual([]);
-    });
-
-    it("should format compact params correctly", () => {
-      const tool = skillManager.createTool();
-      const context = { workdir: "/test" };
-
-      expect(
-        tool.formatCompactParams?.({ skill_name: "test-skill" }, context),
-      ).toBe("test-skill");
-      expect(tool.formatCompactParams?.({}, context)).toBe("unknown-skill");
-    });
-
-    it("should handle tool execution when not initialized", async () => {
-      // Mock initialization to succeed
-      vi.spyOn(skillManager, "initialize").mockResolvedValue(undefined);
-      vi.spyOn(skillManager, "executeSkill").mockResolvedValue({
-        content: "Test result",
-        context: { skillName: "test-skill" },
-      });
-
-      const tool = skillManager.createTool();
-      const context = { workdir: "/test" };
-      const result = await tool.execute?.(
-        { skill_name: "test-skill" },
-        context,
-      );
-
-      expect(result.success).toBe(true);
-      expect(result.content).toBe("Test result");
-      expect(result.shortResult).toBe("Invoked skill: test-skill");
-    });
-
-    it("should validate skill_name parameter", async () => {
-      const tool = skillManager.createTool();
-      const context = { workdir: "/test" };
-
-      // Mock initialization
-      vi.spyOn(skillManager, "initialize").mockResolvedValue(undefined);
-
-      const result = await tool.execute?.({}, context);
-
-      expect(result.success).toBe(false);
-      expect(result.error).toBe(
-        "skill_name parameter is required and must be a string",
-      );
-    });
-
-    it("should handle initialization errors", async () => {
-      vi.spyOn(skillManager, "initialize").mockRejectedValue(
-        new Error("Init failed"),
-      );
-
-      const tool = skillManager.createTool();
-      const context = { workdir: "/test" };
-      const result = await tool.execute?.(
-        { skill_name: "test-skill" },
-        context,
-      );
-
-      expect(result.success).toBe(false);
-      expect(result.error).toBe("Init failed");
-    });
-  });
-
   describe("executeSkill", () => {
     beforeEach(() => {
       // Mock initialization to make other methods available
diff --git a/packages/agent-sdk/tests/tools/skillTool.test.ts b/packages/agent-sdk/tests/tools/skillTool.test.ts
