x: add skills spec for custom tool definitions

Add a JSON-based skills specification system that allows users to define custom tools loaded by the experimental agent loop. Skills are auto-discovered from standard locations (./ollama-skills.json, ~/.ollama/skills.json, ~/.config/ollama/skills.json). Features: - SkillSpec schema with parameters and executor configuration - Script executor that runs commands with JSON args via stdin - /skills reload command for runtime skill reloading - Comprehensive validation and error handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-04-17 15:53:27 +02:00 · 2026-01-09 01:39:27 -08:00
parent 301b8547da
commit b1d711f8cc
4 changed files with 761 additions and 0 deletions
--- a/x/cmd/run.go
+++ b/x/cmd/run.go
@@ -474,6 +474,15 @@ func GenerateInteractive(cmd *cobra.Command, modelName string, wordWrap bool, op
 	var toolRegistry *tools.Registry
 	if supportsTools {
 		toolRegistry = tools.DefaultRegistry()
+
+		// Load custom skills from skill files
+		loadedFiles, err := tools.LoadAllSkills(toolRegistry)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "\033[33mWarning: Error loading skills: %v\033[0m\n", err)
+		} else if len(loadedFiles) > 0 {
+			fmt.Fprintf(os.Stderr, "Loaded skills from: %s\n", strings.Join(loadedFiles, ", "))
+		}
+
 		fmt.Fprintf(os.Stderr, "Tools available: %s\n", strings.Join(toolRegistry.Names(), ", "))

 		// Check for OLLAMA_API_KEY for web search
@@ -517,13 +526,35 @@ func GenerateInteractive(cmd *cobra.Command, modelName string, wordWrap bool, op
 		case strings.HasPrefix(line, "/tools"):
 			showToolsStatus(toolRegistry, approval, supportsTools)
 			continue
+		case strings.HasPrefix(line, "/skills reload"):
+			if toolRegistry != nil {
+				loadedFiles, err := tools.LoadAllSkills(toolRegistry)
+				if err != nil {
+					fmt.Fprintf(os.Stderr, "\033[33mWarning: Error loading skills: %v\033[0m\n", err)
+				} else if len(loadedFiles) > 0 {
+					fmt.Fprintf(os.Stderr, "Reloaded skills from: %s\n", strings.Join(loadedFiles, ", "))
+					fmt.Fprintf(os.Stderr, "Tools available: %s\n", strings.Join(toolRegistry.Names(), ", "))
+				} else {
+					fmt.Fprintln(os.Stderr, "No skill files found")
+				}
+			} else {
+				fmt.Fprintln(os.Stderr, "Tools not available - model does not support tool calling")
+			}
+			continue
 		case strings.HasPrefix(line, "/help"), strings.HasPrefix(line, "/?"):
 			fmt.Fprintln(os.Stderr, "Available Commands:")
 			fmt.Fprintln(os.Stderr, "  /tools          Show available tools and approvals")
+			fmt.Fprintln(os.Stderr, "  /skills reload  Reload custom skills from skill files")
 			fmt.Fprintln(os.Stderr, "  /clear          Clear session context and approvals")
 			fmt.Fprintln(os.Stderr, "  /bye            Exit")
 			fmt.Fprintln(os.Stderr, "  /?, /help       Help for a command")
 			fmt.Fprintln(os.Stderr, "")
+			fmt.Fprintln(os.Stderr, "Custom Skills:")
+			fmt.Fprintln(os.Stderr, "  Skills are loaded from JSON files in these locations:")
+			fmt.Fprintln(os.Stderr, "    ./ollama-skills.json")
+			fmt.Fprintln(os.Stderr, "    ~/.ollama/skills.json")
+			fmt.Fprintln(os.Stderr, "    ~/.config/ollama/skills.json")
+			fmt.Fprintln(os.Stderr, "")
 			continue
 		case strings.HasPrefix(line, "/"):
 			fmt.Printf("Unknown command '%s'. Type /? for help\n", strings.Fields(line)[0])
--- a/x/tools/example-skills.json
+++ b/x/tools/example-skills.json
@@ -0,0 +1,44 @@
+{
+  "version": "1",
+  "skills": [
+    {
+      "name": "get_time",
+      "description": "Get the current date and time. Use this when the user asks about the current time or date.",
+      "parameters": [],
+      "executor": {
+        "type": "script",
+        "command": "date",
+        "timeout": 5
+      }
+    },
+    {
+      "name": "system_info",
+      "description": "Get system information including OS, hostname, and uptime.",
+      "parameters": [],
+      "executor": {
+        "type": "script",
+        "command": "sh",
+        "args": ["-c", "echo \"Hostname: $(hostname)\"; echo \"OS: $(uname -s)\"; echo \"Kernel: $(uname -r)\"; echo \"Uptime: $(uptime -p 2>/dev/null || uptime)\""],
+        "timeout": 10
+      }
+    },
+    {
+      "name": "python_eval",
+      "description": "Evaluate a Python expression. The expression is passed via stdin as JSON with an 'expression' field.",
+      "parameters": [
+        {
+          "name": "expression",
+          "type": "string",
+          "description": "The Python expression to evaluate",
+          "required": true
+        }
+      ],
+      "executor": {
+        "type": "script",
+        "command": "python3",
+        "args": ["-c", "import sys, json; data = json.load(sys.stdin); print(eval(data.get('expression', '')))"],
+        "timeout": 30
+      }
+    }
+  ]
+}
--- a/x/tools/skills.go
+++ b/x/tools/skills.go
@@ -0,0 +1,318 @@
+// Package tools provides built-in tool implementations for the agent loop.
+package tools
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/ollama/ollama/api"
+)
+
+// SkillSpec defines the specification for a custom skill.
+// Skills can be loaded from JSON files and registered with the tool registry.
+type SkillSpec struct {
+	// Name is the unique identifier for the skill
+	Name string `json:"name"`
+	// Description is a human-readable description shown to the LLM
+	Description string `json:"description"`
+	// Parameters defines the input schema for the skill
+	Parameters []SkillParameter `json:"parameters,omitempty"`
+	// Executor defines how the skill is executed
+	Executor SkillExecutor `json:"executor"`
+}
+
+// SkillParameter defines a single parameter for a skill.
+type SkillParameter struct {
+	// Name is the parameter name
+	Name string `json:"name"`
+	// Type is the JSON schema type (string, number, boolean, array, object)
+	Type string `json:"type"`
+	// Description explains what this parameter is for
+	Description string `json:"description"`
+	// Required indicates if this parameter must be provided
+	Required bool `json:"required"`
+}
+
+// SkillExecutor defines how to execute a skill.
+type SkillExecutor struct {
+	// Type is the executor type: "script", "http", or "builtin"
+	Type string `json:"type"`
+	// Command is the command to run for "script" type
+	// Arguments are passed as JSON via stdin, result is read from stdout
+	Command string `json:"command,omitempty"`
+	// Args are additional arguments appended to the command
+	Args []string `json:"args,omitempty"`
+	// Timeout is the maximum execution time in seconds (default: 60)
+	Timeout int `json:"timeout,omitempty"`
+	// URL is the endpoint for "http" type executors
+	URL string `json:"url,omitempty"`
+	// Method is the HTTP method (default: POST)
+	Method string `json:"method,omitempty"`
+}
+
+// SkillsFile represents a file containing skill definitions.
+type SkillsFile struct {
+	// Version is the spec version (currently "1")
+	Version string `json:"version"`
+	// Skills is the list of skill definitions
+	Skills []SkillSpec `json:"skills"`
+}
+
+// SkillTool wraps a SkillSpec to implement the Tool interface.
+type SkillTool struct {
+	spec SkillSpec
+}
+
+// NewSkillTool creates a Tool from a SkillSpec.
+func NewSkillTool(spec SkillSpec) *SkillTool {
+	return &SkillTool{spec: spec}
+}
+
+// Name returns the skill name.
+func (s *SkillTool) Name() string {
+	return s.spec.Name
+}
+
+// Description returns the skill description.
+func (s *SkillTool) Description() string {
+	return s.spec.Description
+}
+
+// Schema returns the tool's parameter schema for the LLM.
+func (s *SkillTool) Schema() api.ToolFunction {
+	props := api.NewToolPropertiesMap()
+	var required []string
+
+	for _, param := range s.spec.Parameters {
+		props.Set(param.Name, api.ToolProperty{
+			Type:        api.PropertyType{param.Type},
+			Description: param.Description,
+		})
+		if param.Required {
+			required = append(required, param.Name)
+		}
+	}
+
+	return api.ToolFunction{
+		Name:        s.spec.Name,
+		Description: s.spec.Description,
+		Parameters: api.ToolFunctionParameters{
+			Type:       "object",
+			Properties: props,
+			Required:   required,
+		},
+	}
+}
+
+// Execute runs the skill with the given arguments.
+func (s *SkillTool) Execute(args map[string]any) (string, error) {
+	switch s.spec.Executor.Type {
+	case "script":
+		return s.executeScript(args)
+	case "http":
+		return s.executeHTTP(args)
+	default:
+		return "", fmt.Errorf("unknown executor type: %s", s.spec.Executor.Type)
+	}
+}
+
+// executeScript runs a script-based skill.
+func (s *SkillTool) executeScript(args map[string]any) (string, error) {
+	if s.spec.Executor.Command == "" {
+		return "", fmt.Errorf("script executor requires command")
+	}
+
+	timeout := time.Duration(s.spec.Executor.Timeout) * time.Second
+	if timeout == 0 {
+		timeout = 60 * time.Second
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+
+	// Build command
+	cmdArgs := append([]string{}, s.spec.Executor.Args...)
+	cmd := exec.CommandContext(ctx, s.spec.Executor.Command, cmdArgs...)
+
+	// Pass arguments as JSON via stdin
+	inputJSON, err := json.Marshal(args)
+	if err != nil {
+		return "", fmt.Errorf("marshaling arguments: %w", err)
+	}
+	cmd.Stdin = bytes.NewReader(inputJSON)
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err = cmd.Run()
+
+	// Build output
+	var sb strings.Builder
+	if stdout.Len() > 0 {
+		output := stdout.String()
+		if len(output) > maxOutputSize {
+			output = output[:maxOutputSize] + "\n... (output truncated)"
+		}
+		sb.WriteString(output)
+	}
+
+	if stderr.Len() > 0 {
+		stderrOutput := stderr.String()
+		if len(stderrOutput) > maxOutputSize {
+			stderrOutput = stderrOutput[:maxOutputSize] + "\n... (stderr truncated)"
+		}
+		if sb.Len() > 0 {
+			sb.WriteString("\n")
+		}
+		sb.WriteString("stderr:\n")
+		sb.WriteString(stderrOutput)
+	}
+
+	if err != nil {
+		if ctx.Err() == context.DeadlineExceeded {
+			return sb.String() + fmt.Sprintf("\n\nError: command timed out after %d seconds", s.spec.Executor.Timeout), nil
+		}
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			return sb.String() + fmt.Sprintf("\n\nExit code: %d", exitErr.ExitCode()), nil
+		}
+		return sb.String(), fmt.Errorf("executing skill: %w", err)
+	}
+
+	if sb.Len() == 0 {
+		return "(no output)", nil
+	}
+
+	return sb.String(), nil
+}
+
+// executeHTTP runs an HTTP-based skill.
+func (s *SkillTool) executeHTTP(args map[string]any) (string, error) {
+	// HTTP executor is a placeholder for future implementation
+	return "", fmt.Errorf("http executor not yet implemented")
+}
+
+// LoadSkillsFile loads skill definitions from a JSON file.
+func LoadSkillsFile(path string) (*SkillsFile, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("reading skills file: %w", err)
+	}
+
+	var file SkillsFile
+	if err := json.Unmarshal(data, &file); err != nil {
+		return nil, fmt.Errorf("parsing skills file: %w", err)
+	}
+
+	if file.Version == "" {
+		file.Version = "1"
+	}
+
+	return &file, nil
+}
+
+// RegisterSkillsFromFile loads skills from a file and registers them with the registry.
+func RegisterSkillsFromFile(registry *Registry, path string) error {
+	file, err := LoadSkillsFile(path)
+	if err != nil {
+		return err
+	}
+
+	for _, spec := range file.Skills {
+		if err := validateSkillSpec(spec); err != nil {
+			return fmt.Errorf("invalid skill %q: %w", spec.Name, err)
+		}
+		registry.Register(NewSkillTool(spec))
+	}
+
+	return nil
+}
+
+// FindSkillsFiles searches for skill definition files in standard locations.
+// It looks for:
+// - ./ollama-skills.json (current directory)
+// - ~/.ollama/skills.json (user config)
+// - ~/.config/ollama/skills.json (XDG config)
+func FindSkillsFiles() []string {
+	var files []string
+
+	// Current directory
+	if _, err := os.Stat("ollama-skills.json"); err == nil {
+		files = append(files, "ollama-skills.json")
+	}
+
+	// Home directory
+	home, err := os.UserHomeDir()
+	if err == nil {
+		paths := []string{
+			filepath.Join(home, ".ollama", "skills.json"),
+			filepath.Join(home, ".config", "ollama", "skills.json"),
+		}
+		for _, p := range paths {
+			if _, err := os.Stat(p); err == nil {
+				files = append(files, p)
+			}
+		}
+	}
+
+	return files
+}
+
+// LoadAllSkills loads skills from all discovered skill files into the registry.
+func LoadAllSkills(registry *Registry) ([]string, error) {
+	files := FindSkillsFiles()
+	var loaded []string
+
+	for _, path := range files {
+		if err := RegisterSkillsFromFile(registry, path); err != nil {
+			return loaded, fmt.Errorf("loading %s: %w", path, err)
+		}
+		loaded = append(loaded, path)
+	}
+
+	return loaded, nil
+}
+
+// validateSkillSpec validates a skill specification.
+func validateSkillSpec(spec SkillSpec) error {
+	if spec.Name == "" {
+		return fmt.Errorf("name is required")
+	}
+	if spec.Description == "" {
+		return fmt.Errorf("description is required")
+	}
+	if spec.Executor.Type == "" {
+		return fmt.Errorf("executor.type is required")
+	}
+
+	switch spec.Executor.Type {
+	case "script":
+		if spec.Executor.Command == "" {
+			return fmt.Errorf("executor.command is required for script type")
+		}
+	case "http":
+		if spec.Executor.URL == "" {
+			return fmt.Errorf("executor.url is required for http type")
+		}
+	default:
+		return fmt.Errorf("unknown executor type: %s", spec.Executor.Type)
+	}
+
+	for _, param := range spec.Parameters {
+		if param.Name == "" {
+			return fmt.Errorf("parameter name is required")
+		}
+		if param.Type == "" {
+			return fmt.Errorf("parameter type is required for %s", param.Name)
+		}
+	}
+
+	return nil
+}
--- a/x/tools/skills_test.go
+++ b/x/tools/skills_test.go
@@ -0,0 +1,368 @@
+package tools
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestValidateSkillSpec(t *testing.T) {
+	tests := []struct {
+		name    string
+		spec    SkillSpec
+		wantErr bool
+		errMsg  string
+	}{
+		{
+			name: "valid script skill",
+			spec: SkillSpec{
+				Name:        "test_skill",
+				Description: "A test skill",
+				Parameters: []SkillParameter{
+					{Name: "input", Type: "string", Description: "Input value", Required: true},
+				},
+				Executor: SkillExecutor{
+					Type:    "script",
+					Command: "echo",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid http skill",
+			spec: SkillSpec{
+				Name:        "http_skill",
+				Description: "An HTTP skill",
+				Executor: SkillExecutor{
+					Type: "http",
+					URL:  "https://example.com/api",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "missing name",
+			spec: SkillSpec{
+				Description: "A skill without name",
+				Executor:    SkillExecutor{Type: "script", Command: "echo"},
+			},
+			wantErr: true,
+			errMsg:  "name is required",
+		},
+		{
+			name: "missing description",
+			spec: SkillSpec{
+				Name:     "no_desc",
+				Executor: SkillExecutor{Type: "script", Command: "echo"},
+			},
+			wantErr: true,
+			errMsg:  "description is required",
+		},
+		{
+			name: "missing executor type",
+			spec: SkillSpec{
+				Name:        "no_exec_type",
+				Description: "Missing executor type",
+				Executor:    SkillExecutor{Command: "echo"},
+			},
+			wantErr: true,
+			errMsg:  "executor.type is required",
+		},
+		{
+			name: "script missing command",
+			spec: SkillSpec{
+				Name:        "script_no_cmd",
+				Description: "Script without command",
+				Executor:    SkillExecutor{Type: "script"},
+			},
+			wantErr: true,
+			errMsg:  "executor.command is required",
+		},
+		{
+			name: "http missing url",
+			spec: SkillSpec{
+				Name:        "http_no_url",
+				Description: "HTTP without URL",
+				Executor:    SkillExecutor{Type: "http"},
+			},
+			wantErr: true,
+			errMsg:  "executor.url is required",
+		},
+		{
+			name: "unknown executor type",
+			spec: SkillSpec{
+				Name:        "unknown_type",
+				Description: "Unknown executor",
+				Executor:    SkillExecutor{Type: "invalid"},
+			},
+			wantErr: true,
+			errMsg:  "unknown executor type",
+		},
+		{
+			name: "parameter missing name",
+			spec: SkillSpec{
+				Name:        "param_no_name",
+				Description: "Parameter without name",
+				Parameters:  []SkillParameter{{Type: "string", Description: "desc"}},
+				Executor:    SkillExecutor{Type: "script", Command: "echo"},
+			},
+			wantErr: true,
+			errMsg:  "parameter name is required",
+		},
+		{
+			name: "parameter missing type",
+			spec: SkillSpec{
+				Name:        "param_no_type",
+				Description: "Parameter without type",
+				Parameters:  []SkillParameter{{Name: "foo", Description: "desc"}},
+				Executor:    SkillExecutor{Type: "script", Command: "echo"},
+			},
+			wantErr: true,
+			errMsg:  "parameter type is required",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateSkillSpec(tt.spec)
+			if tt.wantErr {
+				if err == nil {
+					t.Errorf("expected error containing %q, got nil", tt.errMsg)
+				} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
+					t.Errorf("expected error containing %q, got %q", tt.errMsg, err.Error())
+				}
+			} else if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+		})
+	}
+}
+
+func TestLoadSkillsFile(t *testing.T) {
+	// Create a temporary skills file
+	tmpDir := t.TempDir()
+	skillsPath := filepath.Join(tmpDir, "skills.json")
+
+	content := `{
+		"version": "1",
+		"skills": [
+			{
+				"name": "echo_skill",
+				"description": "Echoes the input",
+				"parameters": [
+					{"name": "message", "type": "string", "description": "Message to echo", "required": true}
+				],
+				"executor": {
+					"type": "script",
+					"command": "echo",
+					"timeout": 30
+				}
+			}
+		]
+	}`
+
+	if err := os.WriteFile(skillsPath, []byte(content), 0644); err != nil {
+		t.Fatalf("failed to write test file: %v", err)
+	}
+
+	file, err := LoadSkillsFile(skillsPath)
+	if err != nil {
+		t.Fatalf("LoadSkillsFile failed: %v", err)
+	}
+
+	if file.Version != "1" {
+		t.Errorf("expected version 1, got %s", file.Version)
+	}
+
+	if len(file.Skills) != 1 {
+		t.Fatalf("expected 1 skill, got %d", len(file.Skills))
+	}
+
+	skill := file.Skills[0]
+	if skill.Name != "echo_skill" {
+		t.Errorf("expected name 'echo_skill', got %s", skill.Name)
+	}
+	if skill.Executor.Timeout != 30 {
+		t.Errorf("expected timeout 30, got %d", skill.Executor.Timeout)
+	}
+	if len(skill.Parameters) != 1 {
+		t.Errorf("expected 1 parameter, got %d", len(skill.Parameters))
+	}
+}
+
+func TestRegisterSkillsFromFile(t *testing.T) {
+	tmpDir := t.TempDir()
+	skillsPath := filepath.Join(tmpDir, "skills.json")
+
+	content := `{
+		"version": "1",
+		"skills": [
+			{
+				"name": "skill_a",
+				"description": "Skill A",
+				"executor": {"type": "script", "command": "echo"}
+			},
+			{
+				"name": "skill_b",
+				"description": "Skill B",
+				"executor": {"type": "script", "command": "cat"}
+			}
+		]
+	}`
+
+	if err := os.WriteFile(skillsPath, []byte(content), 0644); err != nil {
+		t.Fatalf("failed to write test file: %v", err)
+	}
+
+	registry := NewRegistry()
+	if err := RegisterSkillsFromFile(registry, skillsPath); err != nil {
+		t.Fatalf("RegisterSkillsFromFile failed: %v", err)
+	}
+
+	if registry.Count() != 2 {
+		t.Errorf("expected 2 tools, got %d", registry.Count())
+	}
+
+	names := registry.Names()
+	if names[0] != "skill_a" || names[1] != "skill_b" {
+		t.Errorf("unexpected tool names: %v", names)
+	}
+}
+
+func TestSkillToolSchema(t *testing.T) {
+	spec := SkillSpec{
+		Name:        "test_tool",
+		Description: "A test tool",
+		Parameters: []SkillParameter{
+			{Name: "required_param", Type: "string", Description: "Required", Required: true},
+			{Name: "optional_param", Type: "number", Description: "Optional", Required: false},
+		},
+		Executor: SkillExecutor{Type: "script", Command: "echo"},
+	}
+
+	tool := NewSkillTool(spec)
+
+	if tool.Name() != "test_tool" {
+		t.Errorf("expected name 'test_tool', got %s", tool.Name())
+	}
+
+	if tool.Description() != "A test tool" {
+		t.Errorf("expected description 'A test tool', got %s", tool.Description())
+	}
+
+	schema := tool.Schema()
+	if schema.Name != "test_tool" {
+		t.Errorf("schema name mismatch")
+	}
+
+	if len(schema.Parameters.Required) != 1 {
+		t.Errorf("expected 1 required param, got %d", len(schema.Parameters.Required))
+	}
+	if schema.Parameters.Required[0] != "required_param" {
+		t.Errorf("wrong required param: %v", schema.Parameters.Required)
+	}
+}
+
+func TestSkillToolExecuteScript(t *testing.T) {
+	spec := SkillSpec{
+		Name:        "echo_test",
+		Description: "Echo test",
+		Executor: SkillExecutor{
+			Type:    "script",
+			Command: "cat",
+			Timeout: 5,
+		},
+	}
+
+	tool := NewSkillTool(spec)
+
+	// cat will read JSON from stdin and output it
+	result, err := tool.Execute(map[string]any{"message": "hello"})
+	if err != nil {
+		t.Fatalf("Execute failed: %v", err)
+	}
+
+	if !contains(result, "message") || !contains(result, "hello") {
+		t.Errorf("expected JSON output with 'message' and 'hello', got: %s", result)
+	}
+}
+
+func TestSkillToolExecuteHTTPNotImplemented(t *testing.T) {
+	spec := SkillSpec{
+		Name:        "http_test",
+		Description: "HTTP test",
+		Executor: SkillExecutor{
+			Type: "http",
+			URL:  "https://example.com",
+		},
+	}
+
+	tool := NewSkillTool(spec)
+	_, err := tool.Execute(map[string]any{})
+	if err == nil {
+		t.Error("expected error for http executor")
+	}
+	if !contains(err.Error(), "not yet implemented") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
+
+func TestLoadSkillsFileNotFound(t *testing.T) {
+	_, err := LoadSkillsFile("/nonexistent/path/skills.json")
+	if err == nil {
+		t.Error("expected error for nonexistent file")
+	}
+}
+
+func TestLoadSkillsFileInvalidJSON(t *testing.T) {
+	tmpDir := t.TempDir()
+	skillsPath := filepath.Join(tmpDir, "invalid.json")
+
+	if err := os.WriteFile(skillsPath, []byte("not valid json"), 0644); err != nil {
+		t.Fatalf("failed to write test file: %v", err)
+	}
+
+	_, err := LoadSkillsFile(skillsPath)
+	if err == nil {
+		t.Error("expected error for invalid JSON")
+	}
+}
+
+func TestRegisterSkillsFromFileInvalidSkill(t *testing.T) {
+	tmpDir := t.TempDir()
+	skillsPath := filepath.Join(tmpDir, "invalid_skill.json")
+
+	content := `{
+		"version": "1",
+		"skills": [
+			{
+				"name": "",
+				"description": "Missing name",
+				"executor": {"type": "script", "command": "echo"}
+			}
+		]
+	}`
+
+	if err := os.WriteFile(skillsPath, []byte(content), 0644); err != nil {
+		t.Fatalf("failed to write test file: %v", err)
+	}
+
+	registry := NewRegistry()
+	err := RegisterSkillsFromFile(registry, skillsPath)
+	if err == nil {
+		t.Error("expected error for invalid skill")
+	}
+}
+
+func contains(s, substr string) bool {
+	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr))
+}
+
+func containsHelper(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}