test(config,skills,backup,mcp): wave 2 integration tests

Add Wave 2 package coverage:
- config/config_extras_test.go: NormalizeAgentID, ExpandHome, ResolveAgent, env overlays
- skills/search_bm25_test.go: BM25 index/search/ranking
- skills/loader_test.go: SKILL.md loader, frontmatter, dedup, versioning
- backup/fs_archive_test.go: ArchiveDirectory, skip fn, tar prefix
- backup/manifest_dsn_test.go: SanitizeDSN, ParseDSN, WritePgpass, CleanEnv
- mcp/util_bm25_test.go: pool, manager, bridge BM25, env resolution

internal/backup/fs_archive_test.go

@@ -0,0 +1,225 @@
+package backup
+
+import (
+	"archive/tar"
+	"bytes"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// readTarEntries reads all entries from a tar archive into a map[name]content.
+func readTarEntries(t *testing.T, data []byte) map[string]string {
+	t.Helper()
+	tr := tar.NewReader(bytes.NewReader(data))
+	entries := make(map[string]string)
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("tar read error: %v", err)
+		}
+		content, _ := io.ReadAll(tr)
+		entries[hdr.Name] = string(content)
+	}
+	return entries
+}
+
+// --- ArchiveDirectory ---
+
+func TestArchiveDirectory_EmptyDir(t *testing.T) {
+	src := t.TempDir()
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+
+	files, bytes, err := ArchiveDirectory(tw, src, "workspace", nil)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("ArchiveDirectory on empty dir: %v", err)
+	}
+	if files != 0 {
+		t.Errorf("expected 0 files, got %d", files)
+	}
+	if bytes != 0 {
+		t.Errorf("expected 0 bytes, got %d", bytes)
+	}
+}
+
+func TestArchiveDirectory_SingleFile(t *testing.T) {
+	src := t.TempDir()
+	content := "hello world content"
+	os.WriteFile(filepath.Join(src, "hello.txt"), []byte(content), 0644)
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	files, totalBytes, err := ArchiveDirectory(tw, src, "ws", nil)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if files != 1 {
+		t.Errorf("expected 1 file, got %d", files)
+	}
+	if totalBytes != int64(len(content)) {
+		t.Errorf("expected %d bytes, got %d", len(content), totalBytes)
+	}
+
+	entries := readTarEntries(t, buf.Bytes())
+	if _, ok := entries["ws/hello.txt"]; !ok {
+		t.Errorf("expected ws/hello.txt in tar, got keys: %v", keys(entries))
+	}
+	if entries["ws/hello.txt"] != content {
+		t.Errorf("content mismatch: got %q", entries["ws/hello.txt"])
+	}
+}
+
+func TestArchiveDirectory_NestedDirs(t *testing.T) {
+	src := t.TempDir()
+	os.MkdirAll(filepath.Join(src, "subdir"), 0755)
+	os.WriteFile(filepath.Join(src, "top.txt"), []byte("top"), 0644)
+	os.WriteFile(filepath.Join(src, "subdir", "nested.txt"), []byte("nested"), 0644)
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	files, _, err := ArchiveDirectory(tw, src, "data", nil)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if files != 2 {
+		t.Errorf("expected 2 files, got %d", files)
+	}
+	entries := readTarEntries(t, buf.Bytes())
+	if _, ok := entries["data/top.txt"]; !ok {
+		t.Errorf("missing data/top.txt, got: %v", keys(entries))
+	}
+	if _, ok := entries["data/subdir/nested.txt"]; !ok {
+		t.Errorf("missing data/subdir/nested.txt, got: %v", keys(entries))
+	}
+}
+
+func TestArchiveDirectory_SkipsHiddenFiles(t *testing.T) {
+	src := t.TempDir()
+	os.WriteFile(filepath.Join(src, ".hidden"), []byte("secret"), 0644)
+	os.WriteFile(filepath.Join(src, "visible.txt"), []byte("visible"), 0644)
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	files, _, err := ArchiveDirectory(tw, src, "ws", nil)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if files != 1 {
+		t.Errorf("expected 1 file (hidden skipped), got %d", files)
+	}
+	entries := readTarEntries(t, buf.Bytes())
+	if _, ok := entries["ws/.hidden"]; ok {
+		t.Error("hidden file should not be in archive")
+	}
+}
+
+func TestArchiveDirectory_SkipsHiddenDirs(t *testing.T) {
+	src := t.TempDir()
+	os.MkdirAll(filepath.Join(src, ".git"), 0755)
+	os.WriteFile(filepath.Join(src, ".git", "config"), []byte("git stuff"), 0644)
+	os.WriteFile(filepath.Join(src, "README.txt"), []byte("readme"), 0644)
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	files, _, err := ArchiveDirectory(tw, src, "ws", nil)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if files != 1 {
+		t.Errorf("expected 1 file (.git dir skipped), got %d", files)
+	}
+	entries := readTarEntries(t, buf.Bytes())
+	for k := range entries {
+		if strings.Contains(k, ".git") {
+			t.Errorf("hidden dir contents should not be archived: %q", k)
+		}
+	}
+}
+
+func TestArchiveDirectory_SkipsTmpDirs(t *testing.T) {
+	src := t.TempDir()
+	os.MkdirAll(filepath.Join(src, "tmp"), 0755)
+	os.WriteFile(filepath.Join(src, "tmp", "temp.txt"), []byte("temp"), 0644)
+	os.WriteFile(filepath.Join(src, "real.txt"), []byte("real"), 0644)
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	files, _, err := ArchiveDirectory(tw, src, "ws", nil)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if files != 1 {
+		t.Errorf("expected 1 file (tmp dir skipped), got %d", files)
+	}
+}
+
+func TestArchiveDirectory_CustomSkipFn(t *testing.T) {
+	src := t.TempDir()
+	os.WriteFile(filepath.Join(src, "keep.txt"), []byte("keep"), 0644)
+	os.WriteFile(filepath.Join(src, "skip.log"), []byte("skip me"), 0644)
+
+	skipLogs := func(path string) bool {
+		return strings.HasSuffix(path, ".log")
+	}
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	files, _, err := ArchiveDirectory(tw, src, "ws", skipLogs)
+	tw.Close()
+
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if files != 1 {
+		t.Errorf("expected 1 file (.log skipped), got %d", files)
+	}
+	entries := readTarEntries(t, buf.Bytes())
+	if _, ok := entries["ws/keep.txt"]; !ok {
+		t.Error("keep.txt should be in archive")
+	}
+	if _, ok := entries["ws/skip.log"]; ok {
+		t.Error("skip.log should not be in archive")
+	}
+}
+
+func TestArchiveDirectory_TarPrefix(t *testing.T) {
+	src := t.TempDir()
+	os.WriteFile(filepath.Join(src, "file.txt"), []byte("x"), 0644)
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	ArchiveDirectory(tw, src, "custom/prefix", nil)
+	tw.Close()
+
+	entries := readTarEntries(t, buf.Bytes())
+	if _, ok := entries["custom/prefix/file.txt"]; !ok {
+		t.Errorf("expected custom/prefix/file.txt in archive, got: %v", keys(entries))
+	}
+}
+
+// keys returns sorted map keys for test output readability.
+func keys(m map[string]string) []string {
+	out := make([]string, 0, len(m))
+	for k := range m {
+		out = append(out, k)
+	}
+	return out
+}

internal/backup/manifest_dsn_test.go

@@ -0,0 +1,201 @@
+package backup
+
+import (
+	"archive/tar"
+	"compress/gzip"
+	"context"
+	"encoding/json"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// --- Backup.Run (filesystem-only, no DB) ---
+
+func TestBackupRun_FilesystemOnly(t *testing.T) {
+	// Setup source dirs
+	ws := t.TempDir()
+	dataDir := t.TempDir()
+	outDir := t.TempDir()
+
+	os.WriteFile(filepath.Join(ws, "agent.md"), []byte("agent content"), 0644)
+	os.WriteFile(filepath.Join(dataDir, "config.json"), []byte(`{"key":"val"}`), 0644)
+
+	outPath := filepath.Join(outDir, "backup.tar.gz")
+
+	opts := Options{
+		DataDir:       dataDir,
+		WorkspacePath: ws,
+		OutputPath:    outPath,
+		CreatedBy:     "test",
+		GoclawVersion: "test-1.0",
+		SchemaVersion: 42,
+		ExcludeDB:     true,
+	}
+
+	manifest, err := Run(context.Background(), opts)
+	if err != nil {
+		t.Fatalf("Run error: %v", err)
+	}
+
+	// Verify manifest fields
+	if manifest.Version != 1 {
+		t.Errorf("manifest.Version: got %d", manifest.Version)
+	}
+	if manifest.Format != "goclaw-system-backup" {
+		t.Errorf("manifest.Format: got %q", manifest.Format)
+	}
+	if manifest.CreatedBy != "test" {
+		t.Errorf("manifest.CreatedBy: got %q", manifest.CreatedBy)
+	}
+	if manifest.SchemaVersion != 42 {
+		t.Errorf("manifest.SchemaVersion: got %d", manifest.SchemaVersion)
+	}
+	if manifest.PgDumpVersion != "excluded" {
+		t.Errorf("ExcludeDB=true should set PgDumpVersion=excluded, got %q", manifest.PgDumpVersion)
+	}
+	if manifest.Stats.FilesystemFiles < 2 {
+		t.Errorf("expected ≥2 filesystem files, got %d", manifest.Stats.FilesystemFiles)
+	}
+
+	// Verify archive exists and is valid gzip+tar
+	f, err := os.Open(outPath)
+	if err != nil {
+		t.Fatalf("open archive: %v", err)
+	}
+	defer f.Close()
+
+	gr, err := gzip.NewReader(f)
+	if err != nil {
+		t.Fatalf("gzip reader: %v", err)
+	}
+	defer gr.Close()
+
+	tr := tar.NewReader(gr)
+	var foundManifest, foundWorkspace, foundData bool
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("tar read: %v", err)
+		}
+		switch {
+		case hdr.Name == "manifest.json":
+			foundManifest = true
+			data, _ := io.ReadAll(tr)
+			var m BackupManifest
+			if err := json.Unmarshal(data, &m); err != nil {
+				t.Errorf("manifest.json parse error: %v", err)
+			}
+			if m.Format != "goclaw-system-backup" {
+				t.Errorf("manifest in archive: wrong format %q", m.Format)
+			}
+		case strings.HasPrefix(hdr.Name, "workspace/"):
+			foundWorkspace = true
+		case strings.HasPrefix(hdr.Name, "data/"):
+			foundData = true
+		}
+	}
+
+	if !foundManifest {
+		t.Error("archive should contain manifest.json")
+	}
+	if !foundWorkspace {
+		t.Error("archive should contain workspace/ entries")
+	}
+	if !foundData {
+		t.Error("archive should contain data/ entries")
+	}
+}
+
+func TestBackupRun_ExcludeFiles(t *testing.T) {
+	outDir := t.TempDir()
+	outPath := filepath.Join(outDir, "backup.tar.gz")
+
+	opts := Options{
+		OutputPath:    outPath,
+		CreatedBy:     "test",
+		GoclawVersion: "1.0",
+		ExcludeDB:     true,
+		ExcludeFiles:  true,
+	}
+
+	manifest, err := Run(context.Background(), opts)
+	if err != nil {
+		t.Fatalf("Run error: %v", err)
+	}
+
+	if manifest.Stats.FilesystemFiles != 0 {
+		t.Errorf("ExcludeFiles=true: expected 0 filesystem files, got %d", manifest.Stats.FilesystemFiles)
+	}
+}
+
+func TestBackupRun_ProgressCallback(t *testing.T) {
+	outDir := t.TempDir()
+	outPath := filepath.Join(outDir, "backup.tar.gz")
+
+	var phases []string
+	opts := Options{
+		OutputPath:   outPath,
+		CreatedBy:    "test",
+		ExcludeDB:    true,
+		ExcludeFiles: true,
+		ProgressFn: func(phase, detail string) {
+			phases = append(phases, phase)
+		},
+	}
+
+	_, err := Run(context.Background(), opts)
+	if err != nil {
+		t.Fatalf("Run error: %v", err)
+	}
+
+	// Should have at least a "done" phase
+	var hasDone bool
+	for _, p := range phases {
+		if p == "done" {
+			hasDone = true
+		}
+	}
+	if !hasDone {
+		t.Errorf("expected 'done' phase in progress, got %v", phases)
+	}
+}
+
+func TestBackupRun_InvalidOutputPath(t *testing.T) {
+	opts := Options{
+		OutputPath: "/nonexistent/deep/path/backup.tar.gz",
+		ExcludeDB:  true,
+	}
+	_, err := Run(context.Background(), opts)
+	if err == nil {
+		t.Error("expected error for invalid output path")
+	}
+}
+
+func TestBackupRun_TotalBytesSum(t *testing.T) {
+	ws := t.TempDir()
+	outDir := t.TempDir()
+	os.WriteFile(filepath.Join(ws, "file.txt"), []byte("hello"), 0644)
+
+	opts := Options{
+		WorkspacePath: ws,
+		OutputPath:    filepath.Join(outDir, "backup.tar.gz"),
+		CreatedBy:     "test",
+		ExcludeDB:     true,
+	}
+
+	manifest, err := Run(context.Background(), opts)
+	if err != nil {
+		t.Fatalf("Run error: %v", err)
+	}
+
+	if manifest.Stats.TotalBytes != manifest.Stats.DatabaseSizeBytes+manifest.Stats.FilesystemBytes {
+		t.Errorf("TotalBytes should equal DB+FS: %d != %d+%d",
+			manifest.Stats.TotalBytes, manifest.Stats.DatabaseSizeBytes, manifest.Stats.FilesystemBytes)
+	}
+}

internal/config/config_extras_test.go

@@ -0,0 +1,472 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// --- NormalizeAgentID ---
+
+func TestNormalizeAgentID(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"default", "default"},
+		{"my-agent", "my-agent"},
+		{"MY_AGENT", "my_agent"},
+		{"Hello World", "hello-world"},
+		{"agent@123", "agent-123"},
+		{"  trimmed  ", "trimmed"},
+		{"", "default"},
+		{"   ", "default"},
+		// leading dashes stripped (falls through best-effort path)
+		{"---leading", "leading"},
+		// trailing dashes stripped via best-effort path (already valid regex allows trailing dashes
+		// only when input passes validIDRe; raw "trailing---" DOES match so it's returned as-is)
+		{"trailing---", "trailing---"},
+		// special chars become dashes, then trailing dash stripped
+		{"agent!@#$%", "agent"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := NormalizeAgentID(tt.input)
+			if got != tt.want {
+				t.Errorf("NormalizeAgentID(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestNormalizeAgentID_Long(t *testing.T) {
+	long := strings.Repeat("a", 100)
+	got := NormalizeAgentID(long)
+	if len(got) > 64 {
+		t.Errorf("NormalizeAgentID truncation failed: len=%d", len(got))
+	}
+}
+
+// --- ExpandHome / ContractHome ---
+
+func TestExpandHome(t *testing.T) {
+	home, _ := os.UserHomeDir()
+
+	got := ExpandHome("~/foo/bar")
+	if !strings.HasPrefix(got, home) {
+		t.Errorf("ExpandHome(~/foo/bar) = %q, expected to start with %q", got, home)
+	}
+
+	// Non-tilde path is unchanged
+	abs := "/absolute/path"
+	if ExpandHome(abs) != abs {
+		t.Errorf("ExpandHome(%q) should be unchanged", abs)
+	}
+
+	// Empty string
+	if ExpandHome("") != "" {
+		t.Error("ExpandHome('') should return ''")
+	}
+}
+
+func TestContractHome(t *testing.T) {
+	home, _ := os.UserHomeDir()
+
+	contracted := ContractHome(filepath.Join(home, "projects", "goclaw"))
+	if !strings.HasPrefix(contracted, "~") {
+		t.Errorf("ContractHome should start with ~, got %q", contracted)
+	}
+
+	// Non-home path is unchanged
+	abs := "/some/other/path"
+	if ContractHome(abs) != abs {
+		t.Errorf("ContractHome(%q) should be unchanged", abs)
+	}
+
+	// Empty
+	if ContractHome("") != "" {
+		t.Error("ContractHome('') should return ''")
+	}
+}
+
+// --- ResolveAgent ---
+
+func TestResolveAgent_Default(t *testing.T) {
+	cfg := Default()
+	d := cfg.ResolveAgent("nonexistent")
+
+	if d.Provider != "anthropic" {
+		t.Errorf("expected default provider, got %q", d.Provider)
+	}
+	if d.MaxTokens != DefaultMaxTokens {
+		t.Errorf("expected default max tokens, got %d", d.MaxTokens)
+	}
+}
+
+func TestResolveAgent_Override(t *testing.T) {
+	cfg := Default()
+	cfg.Agents.List = map[string]AgentSpec{
+		"myagent": {
+			Provider:  "openai",
+			Model:     "gpt-4o",
+			MaxTokens: 4096,
+			AgentType: "predefined",
+		},
+	}
+
+	d := cfg.ResolveAgent("myagent")
+	if d.Provider != "openai" {
+		t.Errorf("expected openai, got %q", d.Provider)
+	}
+	if d.Model != "gpt-4o" {
+		t.Errorf("expected gpt-4o, got %q", d.Model)
+	}
+	if d.MaxTokens != 4096 {
+		t.Errorf("expected 4096, got %d", d.MaxTokens)
+	}
+	if d.AgentType != "predefined" {
+		t.Errorf("expected predefined, got %q", d.AgentType)
+	}
+	// Temperature should still be from defaults (not overridden)
+	if d.Temperature != DefaultTemperature {
+		t.Errorf("unset Temperature should inherit default, got %v", d.Temperature)
+	}
+}
+
+// --- ResolveDefaultAgentID ---
+
+func TestResolveDefaultAgentID_NoDefault(t *testing.T) {
+	cfg := Default()
+	got := cfg.ResolveDefaultAgentID()
+	if got != DefaultAgentID {
+		t.Errorf("expected %q, got %q", DefaultAgentID, got)
+	}
+}
+
+func TestResolveDefaultAgentID_Explicit(t *testing.T) {
+	cfg := Default()
+	cfg.Agents.List = map[string]AgentSpec{
+		"agent-a": {Default: false},
+		"agent-b": {Default: true},
+	}
+	got := cfg.ResolveDefaultAgentID()
+	if got != "agent-b" {
+		t.Errorf("expected agent-b, got %q", got)
+	}
+}
+
+// --- ResolveDisplayName ---
+
+func TestResolveDisplayName_Default(t *testing.T) {
+	cfg := Default()
+	got := cfg.ResolveDisplayName("nonexistent")
+	if got != "GoClaw" {
+		t.Errorf("expected GoClaw fallback, got %q", got)
+	}
+}
+
+func TestResolveDisplayName_Set(t *testing.T) {
+	cfg := Default()
+	cfg.Agents.List = map[string]AgentSpec{
+		"myagent": {DisplayName: "My Custom Agent"},
+	}
+	got := cfg.ResolveDisplayName("myagent")
+	if got != "My Custom Agent" {
+		t.Errorf("expected 'My Custom Agent', got %q", got)
+	}
+}
+
+// --- Hash ---
+
+func TestConfigHash(t *testing.T) {
+	cfg1 := Default()
+	cfg2 := Default()
+
+	h1 := cfg1.Hash()
+	h2 := cfg2.Hash()
+
+	if h1 != h2 {
+		t.Errorf("identical configs should have same hash: %q vs %q", h1, h2)
+	}
+
+	// Changing a field should change the hash
+	cfg2.Gateway.Port = 9999
+	h3 := cfg2.Hash()
+	if h1 == h3 {
+		t.Error("modified config should have different hash")
+	}
+}
+
+// --- ReplaceFrom ---
+
+func TestReplaceFrom(t *testing.T) {
+	dst := Default()
+	src := Default()
+	src.Gateway.Port = 12345
+	src.DataDir = "/custom/data"
+
+	dst.ReplaceFrom(src)
+
+	if dst.Gateway.Port != 12345 {
+		t.Errorf("ReplaceFrom should copy port, got %d", dst.Gateway.Port)
+	}
+	if dst.DataDir != "/custom/data" {
+		t.Errorf("ReplaceFrom should copy DataDir, got %q", dst.DataDir)
+	}
+}
+
+// --- CronConfig helpers ---
+
+func TestCronConfig_JobTimeoutDuration_Default(t *testing.T) {
+	cc := CronConfig{}
+	got := cc.JobTimeoutDuration()
+	if got != DefaultJobTimeout {
+		t.Errorf("expected default timeout %v, got %v", DefaultJobTimeout, got)
+	}
+}
+
+func TestCronConfig_JobTimeoutDuration_Custom(t *testing.T) {
+	cc := CronConfig{JobTimeout: "5m"}
+	got := cc.JobTimeoutDuration()
+	if got != 5*time.Minute {
+		t.Errorf("expected 5m, got %v", got)
+	}
+}
+
+func TestCronConfig_JobTimeoutDuration_Invalid(t *testing.T) {
+	cc := CronConfig{JobTimeout: "not-a-duration"}
+	got := cc.JobTimeoutDuration()
+	if got != DefaultJobTimeout {
+		t.Errorf("invalid duration should fall back to default, got %v", got)
+	}
+}
+
+func TestCronConfig_ToRetryConfig_Defaults(t *testing.T) {
+	cc := CronConfig{}
+	got := cc.ToRetryConfig()
+	// Should apply cron default retry config
+	if got.MaxRetries <= 0 {
+		t.Errorf("default MaxRetries should be positive, got %d", got.MaxRetries)
+	}
+}
+
+func TestCronConfig_ToRetryConfig_Custom(t *testing.T) {
+	cc := CronConfig{
+		MaxRetries:     5,
+		RetryBaseDelay: "1s",
+		RetryMaxDelay:  "60s",
+	}
+	got := cc.ToRetryConfig()
+	if got.MaxRetries != 5 {
+		t.Errorf("expected MaxRetries=5, got %d", got.MaxRetries)
+	}
+	if got.BaseDelay != time.Second {
+		t.Errorf("expected BaseDelay=1s, got %v", got.BaseDelay)
+	}
+	if got.MaxDelay != 60*time.Second {
+		t.Errorf("expected MaxDelay=60s, got %v", got.MaxDelay)
+	}
+}
+
+// --- ApplySystemConfigs ---
+
+func TestApplySystemConfigs(t *testing.T) {
+	cfg := Default()
+	cfg.ApplySystemConfigs(map[string]string{
+		"agent.default_provider":   "openai",
+		"agent.default_model":      "gpt-4o",
+		"agent.context_window":     "100000",
+		"gateway.rate_limit_rpm":   "60",
+		"gateway.max_message_chars": "50000",
+	})
+
+	if cfg.Agents.Defaults.Provider != "openai" {
+		t.Errorf("provider: got %q", cfg.Agents.Defaults.Provider)
+	}
+	if cfg.Agents.Defaults.Model != "gpt-4o" {
+		t.Errorf("model: got %q", cfg.Agents.Defaults.Model)
+	}
+	if cfg.Agents.Defaults.ContextWindow != 100000 {
+		t.Errorf("context_window: got %d", cfg.Agents.Defaults.ContextWindow)
+	}
+	if cfg.Gateway.RateLimitRPM != 60 {
+		t.Errorf("rate_limit_rpm: got %d", cfg.Gateway.RateLimitRPM)
+	}
+}
+
+// --- Save / Load round-trip ---
+
+func TestSaveLoad_RoundTrip(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.json")
+
+	cfg := Default()
+	cfg.Gateway.Port = 7654
+	cfg.DataDir = "/test/data"
+
+	if err := Save(path, cfg); err != nil {
+		t.Fatalf("Save error: %v", err)
+	}
+
+	loaded, err := Load(path)
+	if err != nil {
+		t.Fatalf("Load error: %v", err)
+	}
+
+	if loaded.Gateway.Port != 7654 {
+		t.Errorf("port round-trip: got %d", loaded.Gateway.Port)
+	}
+	if loaded.DataDir != "/test/data" {
+		t.Errorf("DataDir round-trip: got %q", loaded.DataDir)
+	}
+}
+
+// --- Env fallback: GOCLAW_PROVIDER / GOCLAW_MODEL only used when config is empty ---
+
+func TestLoad_EnvFallback_OnlyWhenEmpty(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "cfg.json5")
+	os.WriteFile(path, []byte(`{"agents":{"defaults":{"provider":"anthropic","model":"claude-3-5"}}}`), 0644)
+
+	t.Setenv("GOCLAW_PROVIDER", "openai")
+	t.Setenv("GOCLAW_MODEL", "gpt-4o")
+
+	cfg, err := Load(path)
+	if err != nil {
+		t.Fatalf("load error: %v", err)
+	}
+	// File has provider=anthropic → env fallback should NOT override it
+	if cfg.Agents.Defaults.Provider != "anthropic" {
+		t.Errorf("file provider should win over env fallback: got %q", cfg.Agents.Defaults.Provider)
+	}
+}
+
+// --- Env override: GOCLAW_DATA_DIR ---
+
+func TestLoad_DataDirEnvOverride(t *testing.T) {
+	t.Setenv("GOCLAW_DATA_DIR", "/env/data")
+	cfg, err := Load("/nonexistent/path")
+	if err != nil {
+		t.Fatalf("load error: %v", err)
+	}
+	if cfg.DataDir != "/env/data" {
+		t.Errorf("GOCLAW_DATA_DIR not applied: got %q", cfg.DataDir)
+	}
+}
+
+// --- Telemetry env overrides ---
+
+func TestLoad_TelemetryEnvOverrides(t *testing.T) {
+	t.Setenv("GOCLAW_TELEMETRY_ENABLED", "true")
+	t.Setenv("GOCLAW_TELEMETRY_ENDPOINT", "localhost:4317")
+	t.Setenv("GOCLAW_TELEMETRY_PROTOCOL", "grpc")
+
+	cfg, err := Load("/nonexistent/path")
+	if err != nil {
+		t.Fatalf("load error: %v", err)
+	}
+	if !cfg.Telemetry.Enabled {
+		t.Error("GOCLAW_TELEMETRY_ENABLED=true should enable telemetry")
+	}
+	if cfg.Telemetry.Endpoint != "localhost:4317" {
+		t.Errorf("telemetry endpoint: got %q", cfg.Telemetry.Endpoint)
+	}
+}
+
+// --- Sandbox env overrides ---
+
+func TestLoad_SandboxEnvOverrides(t *testing.T) {
+	t.Setenv("GOCLAW_SANDBOX_MODE", "all")
+	t.Setenv("GOCLAW_SANDBOX_IMAGE", "my-sandbox:latest")
+	t.Setenv("GOCLAW_SANDBOX_MEMORY_MB", "1024")
+
+	cfg, err := Load("/nonexistent/path")
+	if err != nil {
+		t.Fatalf("load error: %v", err)
+	}
+	if cfg.Agents.Defaults.Sandbox == nil {
+		t.Fatal("sandbox config should not be nil after env override")
+	}
+	if cfg.Agents.Defaults.Sandbox.Mode != "all" {
+		t.Errorf("sandbox mode: got %q", cfg.Agents.Defaults.Sandbox.Mode)
+	}
+	if cfg.Agents.Defaults.Sandbox.Image != "my-sandbox:latest" {
+		t.Errorf("sandbox image: got %q", cfg.Agents.Defaults.Sandbox.Image)
+	}
+	if cfg.Agents.Defaults.Sandbox.MemoryMB != 1024 {
+		t.Errorf("sandbox memory: got %d", cfg.Agents.Defaults.Sandbox.MemoryMB)
+	}
+}
+
+// --- SandboxConfig.ToSandboxConfig ---
+
+func TestSandboxConfig_ToSandboxConfig_Nil(t *testing.T) {
+	var sc *SandboxConfig
+	got := sc.ToSandboxConfig()
+	// Should return defaults without panic
+	if got.TimeoutSec <= 0 {
+		t.Errorf("expected positive timeout, got %d", got.TimeoutSec)
+	}
+}
+
+func TestSandboxConfig_ToSandboxConfig_AllModes(t *testing.T) {
+	tests := []struct {
+		input string
+	}{
+		{"all"},
+		{"non-main"},
+		{"off"},
+		{"unknown"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			sc := &SandboxConfig{Mode: tt.input, Image: "test:latest", MemoryMB: 256, CPUs: 2.0}
+			got := sc.ToSandboxConfig()
+			if tt.input == "all" || tt.input == "non-main" {
+				// just ensure no panic
+				_ = got
+			}
+			if got.Image != "test:latest" {
+				t.Errorf("image not propagated: got %q", got.Image)
+			}
+			if got.MemoryMB != 256 {
+				t.Errorf("memory not propagated: got %d", got.MemoryMB)
+			}
+			if got.CPUs != 2.0 {
+				t.Errorf("cpus not propagated: got %v", got.CPUs)
+			}
+		})
+	}
+}
+
+// --- Channel auto-enable ---
+
+func TestLoad_ChannelAutoEnable_Telegram(t *testing.T) {
+	t.Setenv("GOCLAW_TELEGRAM_TOKEN", "bot123:abc")
+
+	cfg, err := Load("/nonexistent/path")
+	if err != nil {
+		t.Fatalf("load error: %v", err)
+	}
+	if !cfg.Channels.Telegram.Enabled {
+		t.Error("Telegram should be auto-enabled when token is set")
+	}
+	if cfg.Channels.Telegram.Token != "bot123:abc" {
+		t.Errorf("telegram token: got %q", cfg.Channels.Telegram.Token)
+	}
+}
+
+func TestLoad_ChannelAutoEnable_Slack(t *testing.T) {
+	t.Setenv("GOCLAW_SLACK_BOT_TOKEN", "xoxb-bot")
+	t.Setenv("GOCLAW_SLACK_APP_TOKEN", "xapp-app")
+
+	cfg, err := Load("/nonexistent/path")
+	if err != nil {
+		t.Fatalf("load error: %v", err)
+	}
+	if !cfg.Channels.Slack.Enabled {
+		t.Error("Slack should be auto-enabled when both tokens are set")
+	}
+}

internal/mcp/util_bm25_test.go

@@ -0,0 +1,462 @@
+package mcp
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/uuid"
+)
+
+// --- mapToEnvSlice ---
+
+func TestMapToEnvSlice_Empty(t *testing.T) {
+	got := mapToEnvSlice(nil)
+	if got != nil {
+		t.Errorf("nil map should return nil, got %v", got)
+	}
+	got = mapToEnvSlice(map[string]string{})
+	if got != nil {
+		t.Errorf("empty map should return nil, got %v", got)
+	}
+}
+
+func TestMapToEnvSlice_Singles(t *testing.T) {
+	got := mapToEnvSlice(map[string]string{"FOO": "bar"})
+	if len(got) != 1 {
+		t.Fatalf("expected 1 entry, got %d", len(got))
+	}
+	if got[0] != "FOO=bar" {
+		t.Errorf("expected FOO=bar, got %q", got[0])
+	}
+}
+
+func TestMapToEnvSlice_MultipleEntries(t *testing.T) {
+	in := map[string]string{"A": "1", "B": "2", "C": "3"}
+	got := mapToEnvSlice(in)
+	if len(got) != 3 {
+		t.Fatalf("expected 3 entries, got %d", len(got))
+	}
+	// All should be in KEY=VALUE format
+	set := make(map[string]bool)
+	for _, e := range got {
+		set[e] = true
+	}
+	for _, want := range []string{"A=1", "B=2", "C=3"} {
+		if !set[want] {
+			t.Errorf("missing %q in output %v", want, got)
+		}
+	}
+}
+
+// --- toSet ---
+
+func TestToSet_Nil(t *testing.T) {
+	got := toSet(nil)
+	if got != nil {
+		t.Errorf("nil slice should return nil, got %v", got)
+	}
+}
+
+func TestToSet_Empty(t *testing.T) {
+	got := toSet([]string{})
+	if got != nil {
+		t.Errorf("empty slice should return nil, got %v", got)
+	}
+}
+
+func TestToSet_Values(t *testing.T) {
+	got := toSet([]string{"a", "b", "c"})
+	if len(got) != 3 {
+		t.Fatalf("expected 3 entries, got %d", len(got))
+	}
+	for _, v := range []string{"a", "b", "c"} {
+		if _, ok := got[v]; !ok {
+			t.Errorf("missing %q in set", v)
+		}
+	}
+}
+
+func TestToSet_Deduplication(t *testing.T) {
+	got := toSet([]string{"a", "a", "b"})
+	if len(got) != 2 {
+		t.Errorf("expected 2 unique entries, got %d", len(got))
+	}
+}
+
+// --- joinErrors ---
+
+func TestJoinErrors_Empty(t *testing.T) {
+	got := joinErrors(nil)
+	if got != "" {
+		t.Errorf("empty slice should return empty string, got %q", got)
+	}
+}
+
+func TestJoinErrors_Single(t *testing.T) {
+	got := joinErrors([]string{"error one"})
+	if got != "error one" {
+		t.Errorf("single error: got %q", got)
+	}
+}
+
+func TestJoinErrors_Multiple(t *testing.T) {
+	got := joinErrors([]string{"err1", "err2", "err3"})
+	want := "err1; err2; err3"
+	if got != want {
+		t.Errorf("joinErrors: got %q, want %q", got, want)
+	}
+}
+
+// --- jsonBytesToStringSlice ---
+
+func TestJSONBytesToStringSlice_Valid(t *testing.T) {
+	data, _ := json.Marshal([]string{"a", "b", "c"})
+	got := jsonBytesToStringSlice(data)
+	if len(got) != 3 || got[0] != "a" || got[1] != "b" || got[2] != "c" {
+		t.Errorf("got %v", got)
+	}
+}
+
+func TestJSONBytesToStringSlice_Empty(t *testing.T) {
+	got := jsonBytesToStringSlice(nil)
+	if got != nil {
+		t.Errorf("nil input should return nil, got %v", got)
+	}
+	got = jsonBytesToStringSlice([]byte{})
+	if got != nil {
+		t.Errorf("empty input should return nil, got %v", got)
+	}
+}
+
+func TestJSONBytesToStringSlice_Invalid(t *testing.T) {
+	got := jsonBytesToStringSlice([]byte(`{not valid json`))
+	if got != nil {
+		t.Errorf("invalid JSON should return nil, got %v", got)
+	}
+}
+
+// --- jsonBytesToStringMap ---
+
+func TestJSONBytesToStringMap_Valid(t *testing.T) {
+	data, _ := json.Marshal(map[string]string{"key": "val", "foo": "bar"})
+	got := jsonBytesToStringMap(data)
+	if len(got) != 2 {
+		t.Fatalf("expected 2 entries, got %d", len(got))
+	}
+	if got["key"] != "val" {
+		t.Errorf("key: got %q", got["key"])
+	}
+	if got["foo"] != "bar" {
+		t.Errorf("foo: got %q", got["foo"])
+	}
+}
+
+func TestJSONBytesToStringMap_Empty(t *testing.T) {
+	got := jsonBytesToStringMap(nil)
+	if got != nil {
+		t.Errorf("nil input should return nil, got %v", got)
+	}
+}
+
+func TestJSONBytesToStringMap_Invalid(t *testing.T) {
+	got := jsonBytesToStringMap([]byte(`[not a map]`))
+	if got != nil {
+		t.Errorf("invalid JSON should return nil, got %v", got)
+	}
+}
+
+// resolveEnvVars tests are in manager_env_test.go (covers nil, env:, missing cases).
+
+// --- requireUserCreds ---
+
+func TestRequireUserCreds_Nil(t *testing.T) {
+	if requireUserCreds(nil) {
+		t.Error("nil settings should not require user creds")
+	}
+}
+
+func TestRequireUserCreds_Empty(t *testing.T) {
+	if requireUserCreds(json.RawMessage(`{}`)) {
+		t.Error("empty settings should not require user creds")
+	}
+}
+
+func TestRequireUserCreds_True(t *testing.T) {
+	settings := json.RawMessage(`{"require_user_credentials": true}`)
+	if !requireUserCreds(settings) {
+		t.Error("should require user credentials")
+	}
+}
+
+func TestRequireUserCreds_False(t *testing.T) {
+	settings := json.RawMessage(`{"require_user_credentials": false}`)
+	if requireUserCreds(settings) {
+		t.Error("should not require user credentials when false")
+	}
+}
+
+func TestRequireUserCreds_InvalidJSON(t *testing.T) {
+	// Invalid JSON → returns false (safe default)
+	if requireUserCreds(json.RawMessage(`{invalid}`)) {
+		t.Error("invalid JSON should return false")
+	}
+}
+
+// --- mcpBM25Index ---
+
+func TestMCPBM25Index_EmptyIndex(t *testing.T) {
+	idx := newMCPBM25Index()
+	results := idx.search("anything", 5)
+	if len(results) != 0 {
+		t.Errorf("empty index should return no results, got %d", len(results))
+	}
+	if idx.docCount() != 0 {
+		t.Errorf("empty index docCount should be 0, got %d", idx.docCount())
+	}
+}
+
+func TestMCPBM25Index_Build_SingleTool(t *testing.T) {
+	idx := newMCPBM25Index()
+	bt := makeBridgeToolWithDesc("myserver", "search_web", "Search the web for information")
+	idx.build([]*BridgeTool{bt})
+
+	if idx.docCount() != 1 {
+		t.Errorf("expected 1 doc, got %d", idx.docCount())
+	}
+}
+
+func TestMCPBM25Index_Search_ExactMatch(t *testing.T) {
+	idx := newMCPBM25Index()
+	tools := []*BridgeTool{
+		makeBridgeToolWithDesc("server1", "search_web", "Search the web using DuckDuckGo"),
+		makeBridgeToolWithDesc("server2", "run_code", "Execute Python code in sandbox"),
+		makeBridgeToolWithDesc("server3", "read_file", "Read file from filesystem"),
+	}
+	idx.build(tools)
+
+	results := idx.search("web search", 5)
+	if len(results) == 0 {
+		t.Fatal("expected results for 'web search'")
+	}
+	if results[0].ServerName != "server1" {
+		t.Errorf("expected server1 first, got %q", results[0].ServerName)
+	}
+}
+
+func TestMCPBM25Index_Search_ZeroResults(t *testing.T) {
+	idx := newMCPBM25Index()
+	idx.build([]*BridgeTool{
+		makeBridgeToolWithDesc("s", "tool", "does something"),
+	})
+	results := idx.search("xyzzy_unknown_query_abc", 5)
+	if len(results) != 0 {
+		t.Errorf("unknown query should return 0 results, got %d", len(results))
+	}
+}
+
+func TestMCPBM25Index_Search_EmptyQuery(t *testing.T) {
+	idx := newMCPBM25Index()
+	idx.build([]*BridgeTool{
+		makeBridgeToolWithDesc("s", "tool", "description"),
+	})
+	results := idx.search("", 5)
+	if len(results) != 0 {
+		t.Errorf("empty query should return 0 results, got %d", len(results))
+	}
+}
+
+func TestMCPBM25Index_Search_MaxResultsRespected(t *testing.T) {
+	idx := newMCPBM25Index()
+	tools := make([]*BridgeTool, 10)
+	for i := range tools {
+		tools[i] = makeBridgeToolWithDesc("server", "search_tool", "search web data tool")
+	}
+	idx.build(tools)
+
+	results := idx.search("search", 3)
+	if len(results) > 3 {
+		t.Errorf("maxResults=3 exceeded, got %d", len(results))
+	}
+}
+
+func TestMCPBM25Index_Search_ResultFields(t *testing.T) {
+	idx := newMCPBM25Index()
+	bt := makeBridgeToolWithDesc("my-server", "query_db", "Query the database")
+	bt.registeredName = "mcp_my_server__query_db"
+	idx.build([]*BridgeTool{bt})
+
+	results := idx.search("query database", 5)
+	if len(results) == 0 {
+		t.Fatal("expected results")
+	}
+	r := results[0]
+	if r.OriginalName != "query_db" {
+		t.Errorf("OriginalName: got %q", r.OriginalName)
+	}
+	if r.ServerName != "my-server" {
+		t.Errorf("ServerName: got %q", r.ServerName)
+	}
+	if r.RegisteredName != "mcp_my_server__query_db" {
+		t.Errorf("RegisteredName: got %q", r.RegisteredName)
+	}
+}
+
+func TestMCPBM25Index_Rebuild(t *testing.T) {
+	idx := newMCPBM25Index()
+	idx.build([]*BridgeTool{
+		makeBridgeToolWithDesc("s", "old_tool", "old description search"),
+	})
+
+	// Rebuild with new tools
+	idx.build([]*BridgeTool{
+		makeBridgeToolWithDesc("s", "new_tool", "new description query"),
+	})
+
+	results := idx.search("old", 5)
+	if len(results) != 0 {
+		t.Errorf("after rebuild, old tools should be gone, got %d", len(results))
+	}
+	results = idx.search("new", 5)
+	if len(results) == 0 {
+		t.Error("after rebuild, new tools should be searchable")
+	}
+}
+
+// --- tokenizeMCP ---
+
+func TestTokenizeMCP_BasicTokenization(t *testing.T) {
+	tests := []struct {
+		input  string
+		expect []string
+	}{
+		{"search web", []string{"search", "web"}},
+		{"Search Web", []string{"search", "web"}},         // lowercased
+		{"query_db tool", []string{"query", "db", "tool"}}, // underscore as separator
+		{"", nil},
+		{"a b c", nil}, // single-char tokens filtered
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := tokenizeMCP(tt.input)
+			if len(got) != len(tt.expect) {
+				t.Errorf("tokenizeMCP(%q) = %v, want %v", tt.input, got, tt.expect)
+				return
+			}
+			for i, tok := range got {
+				if tok != tt.expect[i] {
+					t.Errorf("tokenizeMCP(%q)[%d] = %q, want %q", tt.input, i, tok, tt.expect[i])
+				}
+			}
+		})
+	}
+}
+
+// --- Pool key helpers ---
+
+func TestPoolKey(t *testing.T) {
+	id := uuid.MustParse("00000000-0000-0000-0000-000000000001")
+	key := poolKey(id, "my-server")
+	if key != "00000000-0000-0000-0000-000000000001/my-server" {
+		t.Errorf("poolKey: got %q", key)
+	}
+}
+
+func TestUserPoolKey(t *testing.T) {
+	id := uuid.MustParse("00000000-0000-0000-0000-000000000001")
+	key := UserPoolKey(id, "server", "user123")
+	if key != "00000000-0000-0000-0000-000000000001/server/user:user123" {
+		t.Errorf("UserPoolKey: got %q", key)
+	}
+}
+
+// --- Pool: NewPool / Stop ---
+
+func TestNewPool_Defaults(t *testing.T) {
+	p := NewPool(PoolConfig{})
+	// Should start without panic and stop cleanly
+	t.Cleanup(func() {
+		p.Stop()
+	})
+	// Default config applied
+	if p.cfg.MaxSize != 200 {
+		t.Errorf("default MaxSize: got %d", p.cfg.MaxSize)
+	}
+	if p.cfg.MaxIdle != 20 {
+		t.Errorf("default MaxIdle: got %d", p.cfg.MaxIdle)
+	}
+}
+
+func TestPool_Release_NonExistent(t *testing.T) {
+	p := NewPool(PoolConfig{MaxSize: 5})
+	t.Cleanup(func() { p.Stop() })
+	// Release on non-existent key should not panic
+	p.Release("nonexistent/key")
+}
+
+func TestPool_ReleaseUser_NonExistent(t *testing.T) {
+	p := NewPool(PoolConfig{MaxSize: 5})
+	t.Cleanup(func() { p.Stop() })
+	p.ReleaseUser("nonexistent/key")
+}
+
+func TestPool_Stop_EmptyPool(t *testing.T) {
+	p := NewPool(PoolConfig{MaxSize: 5})
+	// Stop on empty pool should not panic
+	p.Stop()
+}
+
+// --- Manager: NewManager / Stop ---
+
+func TestNewManager_Empty(t *testing.T) {
+	m := NewManager(nil)
+	if m == nil {
+		t.Fatal("NewManager should not return nil")
+	}
+	if m.servers == nil {
+		t.Error("servers map should be initialized")
+	}
+}
+
+func TestManager_Stop_Empty(t *testing.T) {
+	m := NewManager(nil)
+	// Stop on empty manager should not panic
+	m.Stop()
+}
+
+func TestManager_ServerStatus_Empty(t *testing.T) {
+	m := NewManager(nil)
+	statuses := m.ServerStatus()
+	if len(statuses) != 0 {
+		t.Errorf("empty manager should have 0 server statuses, got %d", len(statuses))
+	}
+}
+
+func TestManager_IsSearchMode_Default(t *testing.T) {
+	m := NewManager(nil)
+	if m.IsSearchMode() {
+		t.Error("new manager should not be in search mode")
+	}
+}
+
+func TestManager_DeferredToolInfos_Empty(t *testing.T) {
+	m := NewManager(nil)
+	tools := m.DeferredToolInfos()
+	if len(tools) != 0 {
+		t.Errorf("empty manager should have 0 deferred tools, got %d", len(tools))
+	}
+}
+
+// --- helpers ---
+
+// makeBridgeTool builds a minimal BridgeTool for testing.
+func makeBridgeToolWithDesc(serverName, toolName, description string) *BridgeTool {
+	return &BridgeTool{
+		serverName:     serverName,
+		toolName:       toolName,
+		registeredName: "mcp_" + serverName + "__" + toolName,
+		description:    description,
+		inputSchema:    map[string]any{"type": "object", "properties": map[string]any{}},
+		requiredSet:    map[string]bool{},
+	}
+}
+

internal/skills/loader_test.go

@@ -0,0 +1,588 @@
+package skills
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// makeSkillDir creates a skill directory with a SKILL.md file.
+func makeSkillDir(t *testing.T, parent, slug, content string) string {
+	t.Helper()
+	dir := filepath.Join(parent, slug)
+	if err := os.MkdirAll(dir, 0755); err != nil {
+		t.Fatalf("makeSkillDir: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0644); err != nil {
+		t.Fatalf("makeSkillDir write: %v", err)
+	}
+	return dir
+}
+
+// --- ListSkills ---
+
+func TestLoader_ListSkills_ZeroSkills(t *testing.T) {
+	dir := t.TempDir()
+	l := NewLoader(dir, "", "")
+
+	skills := l.ListSkills(context.Background())
+	if len(skills) != 0 {
+		t.Errorf("empty workspace should have 0 skills, got %d", len(skills))
+	}
+}
+
+func TestLoader_ListSkills_SingleSkill(t *testing.T) {
+	ws := t.TempDir()
+	skillsDir := filepath.Join(ws, "skills")
+	makeSkillDir(t, skillsDir, "my-tool", "---\nname: My Tool\ndescription: Does stuff\n---\n# My Tool\n")
+
+	l := NewLoader(ws, "", "")
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 skill, got %d", len(skills))
+	}
+	if skills[0].Slug != "my-tool" {
+		t.Errorf("slug: got %q", skills[0].Slug)
+	}
+	if skills[0].Name != "My Tool" {
+		t.Errorf("name: got %q", skills[0].Name)
+	}
+	if skills[0].Description != "Does stuff" {
+		t.Errorf("description: got %q", skills[0].Description)
+	}
+	if skills[0].Source != "workspace" {
+		t.Errorf("source: got %q", skills[0].Source)
+	}
+}
+
+func TestLoader_ListSkills_MultipleSkills(t *testing.T) {
+	ws := t.TempDir()
+	skillsDir := filepath.Join(ws, "skills")
+	makeSkillDir(t, skillsDir, "skill-a", "---\nname: Skill A\n---\n")
+	makeSkillDir(t, skillsDir, "skill-b", "---\nname: Skill B\n---\n")
+	makeSkillDir(t, skillsDir, "skill-c", "---\nname: Skill C\n---\n")
+
+	l := NewLoader(ws, "", "")
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 3 {
+		t.Errorf("expected 3 skills, got %d", len(skills))
+	}
+}
+
+func TestLoader_ListSkills_IgnoresNonDirs(t *testing.T) {
+	ws := t.TempDir()
+	skillsDir := filepath.Join(ws, "skills")
+	os.MkdirAll(skillsDir, 0755)
+
+	// Create a plain file (not a dir) — should be ignored
+	os.WriteFile(filepath.Join(skillsDir, "not-a-dir.md"), []byte("content"), 0644)
+	// Create a valid skill dir
+	makeSkillDir(t, skillsDir, "valid-skill", "---\nname: Valid\n---\n")
+
+	l := NewLoader(ws, "", "")
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 1 {
+		t.Errorf("expected 1 skill (files ignored), got %d", len(skills))
+	}
+}
+
+func TestLoader_ListSkills_IgnoresDirWithoutSKILLmd(t *testing.T) {
+	ws := t.TempDir()
+	skillsDir := filepath.Join(ws, "skills")
+	os.MkdirAll(skillsDir, 0755)
+
+	// A dir without SKILL.md should be ignored
+	os.MkdirAll(filepath.Join(skillsDir, "no-skill-md"), 0755)
+	makeSkillDir(t, skillsDir, "real-skill", "---\nname: Real\n---\n")
+
+	l := NewLoader(ws, "", "")
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 1 {
+		t.Errorf("expected 1 skill, got %d: %+v", len(skills), skills)
+	}
+}
+
+func TestLoader_ListSkills_PriorityWorkspaceOverGlobal(t *testing.T) {
+	ws := t.TempDir()
+	global := t.TempDir()
+
+	// Same slug in both workspace and global
+	makeSkillDir(t, filepath.Join(ws, "skills"), "shared-skill", "---\nname: From Workspace\n---\n")
+	makeSkillDir(t, global, "shared-skill", "---\nname: From Global\n---\n")
+
+	l := NewLoader(ws, global, "")
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 1 {
+		t.Errorf("expected 1 skill (deduplication), got %d", len(skills))
+	}
+	if skills[0].Name != "From Workspace" {
+		t.Errorf("workspace should take priority, got %q", skills[0].Name)
+	}
+}
+
+func TestLoader_ListSkills_GlobalSkills(t *testing.T) {
+	global := t.TempDir()
+	makeSkillDir(t, global, "global-skill", "---\nname: Global\ndescription: global tool\n---\n")
+
+	l := NewLoader("", global, "")
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 global skill, got %d", len(skills))
+	}
+	if skills[0].Source != "global" {
+		t.Errorf("source: got %q, want global", skills[0].Source)
+	}
+}
+
+func TestLoader_ListSkills_BuiltinSkills(t *testing.T) {
+	builtin := t.TempDir()
+	makeSkillDir(t, builtin, "builtin-skill", "---\nname: Builtin\n---\n")
+
+	l := NewLoader("", "", builtin)
+	skills := l.ListSkills(context.Background())
+
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 builtin skill, got %d", len(skills))
+	}
+	if skills[0].Source != "builtin" {
+		t.Errorf("source: got %q, want builtin", skills[0].Source)
+	}
+}
+
+// --- LoadSkill ---
+
+func TestLoader_LoadSkill_Found(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "---\nname: My Skill\n---\n# Content here\nDo something useful.")
+
+	l := NewLoader(ws, "", "")
+	content, ok := l.LoadSkill(context.Background(), "my-skill")
+
+	if !ok {
+		t.Fatal("expected skill to be found")
+	}
+	if !strings.Contains(content, "Content here") {
+		t.Errorf("expected content body, got %q", content)
+	}
+	// Frontmatter should be stripped
+	if strings.Contains(content, "---") {
+		t.Errorf("frontmatter should be stripped, got %q", content)
+	}
+}
+
+func TestLoader_LoadSkill_NotFound(t *testing.T) {
+	l := NewLoader("", "", "")
+	_, ok := l.LoadSkill(context.Background(), "nonexistent")
+	if ok {
+		t.Error("nonexistent skill should return false")
+	}
+}
+
+func TestLoader_LoadSkill_BaseDirPlaceholder(t *testing.T) {
+	ws := t.TempDir()
+	skillDir := makeSkillDir(t, filepath.Join(ws, "skills"), "my-skill",
+		"---\nname: My Skill\n---\nScript at: {baseDir}/run.sh")
+
+	l := NewLoader(ws, "", "")
+	content, ok := l.LoadSkill(context.Background(), "my-skill")
+
+	if !ok {
+		t.Fatal("expected skill to be found")
+	}
+	if strings.Contains(content, "{baseDir}") {
+		t.Errorf("{baseDir} should be replaced, got %q", content)
+	}
+	if !strings.Contains(content, skillDir) {
+		t.Errorf("expected skill dir %q in content, got %q", skillDir, content)
+	}
+}
+
+// --- LoadForContext ---
+
+func TestLoader_LoadForContext_Empty(t *testing.T) {
+	l := NewLoader("", "", "")
+	result := l.LoadForContext(context.Background(), nil)
+	if result != "" {
+		t.Errorf("empty loader should return empty string, got %q", result)
+	}
+}
+
+func TestLoader_LoadForContext_AllSkills(t *testing.T) {
+	ws := t.TempDir()
+	// Use slug == name so LoadSkill lookup by name succeeds (LoadForContext passes s.Name to LoadSkill).
+	makeSkillDir(t, filepath.Join(ws, "skills"), "skill-a", "---\nname: skill-a\ndescription: Tool A\n---\nContent A")
+	makeSkillDir(t, filepath.Join(ws, "skills"), "skill-b", "---\nname: skill-b\ndescription: Tool B\n---\nContent B")
+
+	l := NewLoader(ws, "", "")
+	result := l.LoadForContext(context.Background(), nil)
+
+	if !strings.Contains(result, "Available Skills") {
+		t.Errorf("expected '## Available Skills' header, got %q", result)
+	}
+	if !strings.Contains(result, "skill-a") {
+		t.Errorf("expected skill-a in output")
+	}
+	if !strings.Contains(result, "skill-b") {
+		t.Errorf("expected skill-b in output")
+	}
+}
+
+func TestLoader_LoadForContext_AllowList(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "skill-a", "---\nname: skill-a\n---\nContent A")
+	makeSkillDir(t, filepath.Join(ws, "skills"), "skill-b", "---\nname: skill-b\n---\nContent B")
+
+	l := NewLoader(ws, "", "")
+	// allowList uses skill names (same as slug when name==slug in frontmatter)
+	result := l.LoadForContext(context.Background(), []string{"skill-a"})
+
+	if !strings.Contains(result, "skill-a") {
+		t.Error("expected skill-a in output")
+	}
+	if strings.Contains(result, "skill-b") {
+		t.Error("skill-b should not appear when not in allowList")
+	}
+}
+
+// --- BuildSummary ---
+
+func TestLoader_BuildSummary_Empty(t *testing.T) {
+	l := NewLoader("", "", "")
+	result := l.BuildSummary(context.Background(), nil)
+	if result != "" {
+		t.Errorf("empty loader BuildSummary should return empty, got %q", result)
+	}
+}
+
+func TestLoader_BuildSummary_XMLFormat(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "web-search",
+		"---\nname: Web Search\ndescription: Search the web\n---\n")
+
+	l := NewLoader(ws, "", "")
+	result := l.BuildSummary(context.Background(), nil)
+
+	if !strings.Contains(result, "<available_skills>") {
+		t.Error("expected <available_skills> root element")
+	}
+	if !strings.Contains(result, "<skill>") {
+		t.Error("expected <skill> element")
+	}
+	if !strings.Contains(result, "Web Search") {
+		t.Error("expected skill name in summary")
+	}
+	if !strings.Contains(result, "</available_skills>") {
+		t.Error("expected closing tag")
+	}
+}
+
+func TestLoader_BuildSummary_XMLEscaping(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "xml-skill",
+		"---\nname: \"Tool <with> &special& chars\"\ndescription: \"A & B < C > D\"\n---\n")
+
+	l := NewLoader(ws, "", "")
+	result := l.BuildSummary(context.Background(), nil)
+
+	// Raw < and & should be escaped in XML output
+	if strings.Contains(result, "<with>") {
+		t.Error("< should be escaped in XML output")
+	}
+	if strings.Contains(result, "&special&") {
+		t.Error("& should be escaped in XML output")
+	}
+}
+
+// --- FilterSkills ---
+
+func TestLoader_FilterSkills_NilAllowList(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "a", "---\nname: A\n---\n")
+	makeSkillDir(t, filepath.Join(ws, "skills"), "b", "---\nname: B\n---\n")
+
+	l := NewLoader(ws, "", "")
+	result := l.FilterSkills(context.Background(), nil)
+	if len(result) != 2 {
+		t.Errorf("nil allowList should return all skills, got %d", len(result))
+	}
+}
+
+func TestLoader_FilterSkills_EmptyAllowList(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "a", "---\nname: A\n---\n")
+
+	l := NewLoader(ws, "", "")
+	result := l.FilterSkills(context.Background(), []string{})
+	if len(result) != 0 {
+		t.Errorf("empty allowList should return 0 skills, got %d", len(result))
+	}
+}
+
+func TestLoader_FilterSkills_SpecificSkill(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "skill-a", "---\nname: A\n---\n")
+	makeSkillDir(t, filepath.Join(ws, "skills"), "skill-b", "---\nname: B\n---\n")
+
+	l := NewLoader(ws, "", "")
+	result := l.FilterSkills(context.Background(), []string{"skill-a"})
+	if len(result) != 1 {
+		t.Fatalf("expected 1 skill, got %d", len(result))
+	}
+	if result[0].Slug != "skill-a" {
+		t.Errorf("expected skill-a, got %q", result[0].Slug)
+	}
+}
+
+// --- GetSkill ---
+
+func TestLoader_GetSkill_Found(t *testing.T) {
+	ws := t.TempDir()
+	makeSkillDir(t, filepath.Join(ws, "skills"), "my-skill", "---\nname: My Skill\n---\n")
+
+	l := NewLoader(ws, "", "")
+	info, ok := l.GetSkill(context.Background(), "my-skill")
+	if !ok {
+		t.Fatal("expected skill to be found")
+	}
+	if info.Slug != "my-skill" {
+		t.Errorf("slug: got %q", info.Slug)
+	}
+}
+
+func TestLoader_GetSkill_NotFound(t *testing.T) {
+	l := NewLoader("", "", "")
+	_, ok := l.GetSkill(context.Background(), "nonexistent")
+	if ok {
+		t.Error("nonexistent skill should return false")
+	}
+}
+
+// --- Version / BumpVersion ---
+
+func TestLoader_Version(t *testing.T) {
+	l := NewLoader("", "", "")
+	v1 := l.Version()
+
+	l.BumpVersion()
+	v2 := l.Version()
+
+	if v2 <= v1 {
+		t.Errorf("BumpVersion should increase version: %d -> %d", v1, v2)
+	}
+}
+
+// --- Frontmatter parsing ---
+
+func TestExtractFrontmatter_Valid(t *testing.T) {
+	content := "---\nname: Test Tool\ndescription: Does something\n---\n# Body"
+	fm := extractFrontmatter(content)
+	if !strings.Contains(fm, "name: Test Tool") {
+		t.Errorf("expected frontmatter content, got %q", fm)
+	}
+}
+
+func TestExtractFrontmatter_Missing(t *testing.T) {
+	content := "# Just a body\nNo frontmatter here."
+	fm := extractFrontmatter(content)
+	if fm != "" {
+		t.Errorf("expected empty frontmatter, got %q", fm)
+	}
+}
+
+func TestStripFrontmatter(t *testing.T) {
+	content := "---\nname: Test\n---\n# Body\nContent here."
+	result := stripFrontmatter(content)
+	if strings.Contains(result, "---") {
+		t.Errorf("frontmatter should be stripped, got %q", result)
+	}
+	if !strings.Contains(result, "Body") {
+		t.Errorf("body should remain, got %q", result)
+	}
+}
+
+func TestStripFrontmatter_NonePresent(t *testing.T) {
+	content := "# Just body"
+	result := stripFrontmatter(content)
+	if result != content {
+		t.Errorf("no frontmatter: content should be unchanged, got %q", result)
+	}
+}
+
+// --- parseSimpleYAML ---
+
+func TestParseSimpleYAML_BasicKV(t *testing.T) {
+	yaml := "name: My Tool\ndescription: Does something\n"
+	kv := parseSimpleYAML(yaml)
+
+	if kv["name"] != "My Tool" {
+		t.Errorf("name: got %q", kv["name"])
+	}
+	if kv["description"] != "Does something" {
+		t.Errorf("description: got %q", kv["description"])
+	}
+}
+
+func TestParseSimpleYAML_QuotedValues(t *testing.T) {
+	yaml := "name: \"Quoted Name\"\ndescription: 'Single quoted'\n"
+	kv := parseSimpleYAML(yaml)
+
+	if kv["name"] != "Quoted Name" {
+		t.Errorf("name: got %q", kv["name"])
+	}
+	if kv["description"] != "Single quoted" {
+		t.Errorf("description: got %q", kv["description"])
+	}
+}
+
+func TestParseSimpleYAML_Empty(t *testing.T) {
+	kv := parseSimpleYAML("")
+	if len(kv) != 0 {
+		t.Errorf("empty yaml should return empty map, got %v", kv)
+	}
+}
+
+func TestParseSimpleYAML_CommentLines(t *testing.T) {
+	yaml := "# This is a comment\nname: Tool\n"
+	kv := parseSimpleYAML(yaml)
+	if kv["name"] != "Tool" {
+		t.Errorf("expected name=Tool after comment, got %q", kv["name"])
+	}
+	if _, ok := kv["# This is a comment"]; ok {
+		t.Error("comment line should not be a key")
+	}
+}
+
+// --- parseMetadata ---
+
+func TestParseMetadata_ValidFrontmatter(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "SKILL.md")
+	os.WriteFile(path, []byte("---\nname: Test Skill\ndescription: A test skill\n---\n# Body"), 0644)
+
+	meta := parseMetadata(path)
+	if meta == nil {
+		t.Fatal("expected non-nil metadata")
+	}
+	if meta.Name != "Test Skill" {
+		t.Errorf("name: got %q", meta.Name)
+	}
+	if meta.Description != "A test skill" {
+		t.Errorf("description: got %q", meta.Description)
+	}
+}
+
+func TestParseMetadata_NoFrontmatter(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "SKILL.md")
+	os.WriteFile(path, []byte("# Body without frontmatter"), 0644)
+
+	meta := parseMetadata(path)
+	if meta == nil {
+		t.Fatal("expected non-nil metadata even without frontmatter")
+	}
+	// Name should fall back to directory name
+	if meta.Name == "" {
+		t.Error("name should fall back to directory name")
+	}
+}
+
+func TestParseMetadata_FileNotFound(t *testing.T) {
+	meta := parseMetadata("/nonexistent/path/SKILL.md")
+	if meta != nil {
+		t.Errorf("nonexistent file should return nil metadata, got %+v", meta)
+	}
+}
+
+// --- normalizeLineEndings ---
+
+func TestNormalizeLineEndings(t *testing.T) {
+	crlf := "line1\r\nline2\r\nline3"
+	got := normalizeLineEndings(crlf)
+	if strings.Contains(got, "\r") {
+		t.Errorf("normalizeLineEndings should remove \\r, got %q", got)
+	}
+	if got != "line1\nline2\nline3" {
+		t.Errorf("expected unix line endings, got %q", got)
+	}
+}
+
+// --- Managed skills versioning ---
+
+func TestLoader_ManagedSkills_LatestVersion(t *testing.T) {
+	managed := t.TempDir()
+
+	// Create versioned structure: managed/my-skill/1/SKILL.md, managed/my-skill/2/SKILL.md
+	os.MkdirAll(filepath.Join(managed, "my-skill", "1"), 0755)
+	os.WriteFile(filepath.Join(managed, "my-skill", "1", "SKILL.md"),
+		[]byte("---\nname: My Skill v1\n---\nVersion 1"), 0644)
+	os.MkdirAll(filepath.Join(managed, "my-skill", "2"), 0755)
+	os.WriteFile(filepath.Join(managed, "my-skill", "2", "SKILL.md"),
+		[]byte("---\nname: My Skill v2\n---\nVersion 2"), 0644)
+
+	l := NewLoader("", "", "")
+	l.SetManagedDir(managed)
+
+	skills := l.ListSkills(context.Background())
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 managed skill, got %d", len(skills))
+	}
+	// Should pick v2 (highest)
+	if skills[0].Name != "My Skill v2" {
+		t.Errorf("expected latest version (v2), got %q", skills[0].Name)
+	}
+	if skills[0].Source != "managed" {
+		t.Errorf("source should be managed, got %q", skills[0].Source)
+	}
+}
+
+func TestLoader_ManagedSkills_WorkspaceTakesPriority(t *testing.T) {
+	ws := t.TempDir()
+	managed := t.TempDir()
+
+	// Same slug in both workspace and managed
+	makeSkillDir(t, filepath.Join(ws, "skills"), "shared-skill", "---\nname: Workspace Version\n---\n")
+	os.MkdirAll(filepath.Join(managed, "shared-skill", "1"), 0755)
+	os.WriteFile(filepath.Join(managed, "shared-skill", "1", "SKILL.md"),
+		[]byte("---\nname: Managed Version\n---\n"), 0644)
+
+	l := NewLoader(ws, "", "")
+	l.SetManagedDir(managed)
+
+	skills := l.ListSkills(context.Background())
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 skill (dedup), got %d", len(skills))
+	}
+	if skills[0].Name != "Workspace Version" {
+		t.Errorf("workspace should take priority over managed, got %q", skills[0].Name)
+	}
+}
+
+// --- Dirs ---
+
+func TestLoader_Dirs(t *testing.T) {
+	ws := t.TempDir()
+	global := t.TempDir()
+	builtin := t.TempDir()
+
+	l := NewLoader(ws, global, builtin)
+	dirs := l.Dirs()
+
+	// Should include workspace skills dir, global, and builtin
+	if len(dirs) == 0 {
+		t.Error("expected non-empty dirs list")
+	}
+	// All returned dirs should be non-empty strings
+	for _, d := range dirs {
+		if d == "" {
+			t.Error("dirs should not contain empty strings")
+		}
+	}
+}

internal/skills/search_bm25_test.go

@@ -0,0 +1,253 @@
+package skills
+
+import (
+	"testing"
+)
+
+// --- tokenize ---
+
+func TestTokenize(t *testing.T) {
+	tests := []struct {
+		input  string
+		expect []string
+	}{
+		{"hello world", []string{"hello", "world"}},
+		{"Hello World", []string{"hello", "world"}},
+		{"web search tool", []string{"web", "search", "tool"}},
+		{"BM25 search", []string{"bm25", "search"}},
+		// punctuation stripped
+		{"hello, world!", []string{"hello", "world"}},
+		// single-char tokens filtered
+		{"a b c hello", []string{"hello"}},
+		// digits kept
+		{"tool123 search", []string{"tool123", "search"}},
+		// empty
+		{"", nil},
+		// only single chars
+		{"a b c", nil},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := tokenize(tt.input)
+			if len(got) != len(tt.expect) {
+				t.Errorf("tokenize(%q): got %v, want %v", tt.input, got, tt.expect)
+				return
+			}
+			for i, tok := range got {
+				if tok != tt.expect[i] {
+					t.Errorf("tokenize(%q)[%d] = %q, want %q", tt.input, i, tok, tt.expect[i])
+				}
+			}
+		})
+	}
+}
+
+// --- BM25 Index: Build + Search ---
+
+func TestIndex_EmptyIndex_ReturnsNil(t *testing.T) {
+	idx := NewIndex()
+	idx.Build(nil)
+
+	results := idx.Search("anything", 5)
+	if len(results) != 0 {
+		t.Errorf("empty index should return no results, got %d", len(results))
+	}
+}
+
+func TestIndex_EmptyQuery_ReturnsNil(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "web-search", Slug: "web-search", Description: "Search the web"},
+	})
+
+	results := idx.Search("", 5)
+	if len(results) != 0 {
+		t.Errorf("empty query should return no results, got %d", len(results))
+	}
+}
+
+func TestIndex_ExactMatch(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "web-search", Slug: "web-search", Description: "Search the web using DuckDuckGo"},
+		{Name: "code-runner", Slug: "code-runner", Description: "Execute code in sandbox"},
+		{Name: "memory-store", Slug: "memory-store", Description: "Store and recall memories"},
+	})
+
+	results := idx.Search("web search", 5)
+	if len(results) == 0 {
+		t.Fatal("expected at least 1 result for 'web search'")
+	}
+	// web-search should rank first
+	if results[0].Slug != "web-search" {
+		t.Errorf("expected web-search first, got %q", results[0].Slug)
+	}
+}
+
+func TestIndex_PartialMatch(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "code-runner", Slug: "code-runner", Description: "Execute code in sandbox"},
+		{Name: "web-search", Slug: "web-search", Description: "Search the web"},
+	})
+
+	results := idx.Search("code", 5)
+	if len(results) == 0 {
+		t.Fatal("expected result for partial query 'code'")
+	}
+	if results[0].Slug != "code-runner" {
+		t.Errorf("expected code-runner first, got %q", results[0].Slug)
+	}
+}
+
+func TestIndex_ZeroResults_UnknownQuery(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "web-search", Slug: "web-search", Description: "Search the web"},
+	})
+
+	results := idx.Search("xyzzy totally unknown term", 5)
+	if len(results) != 0 {
+		t.Errorf("unknown query should return 0 results, got %d", len(results))
+	}
+}
+
+func TestIndex_RelevanceOrdering(t *testing.T) {
+	// skill-b matches "search" more specifically
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "general-tool", Slug: "general", Description: "A general purpose tool for various tasks"},
+		{Name: "search-engine", Slug: "searcher", Description: "Search search engine for web search queries"},
+		{Name: "code-runner", Slug: "runner", Description: "Run code in a container"},
+	})
+
+	results := idx.Search("search", 5)
+	if len(results) < 1 {
+		t.Fatal("expected results for 'search'")
+	}
+	// searcher should rank higher than general due to more 'search' occurrences
+	if results[0].Slug != "searcher" {
+		t.Errorf("expected searcher to rank first, got %q (score=%f)", results[0].Slug, results[0].Score)
+	}
+}
+
+func TestIndex_MaxResults_Respected(t *testing.T) {
+	idx := NewIndex()
+	skills := make([]Info, 10)
+	for i := range skills {
+		skills[i] = Info{
+			Name:        "tool",
+			Slug:        "tool",
+			Description: "search tool helper",
+		}
+	}
+	idx.Build(skills)
+
+	results := idx.Search("search", 3)
+	if len(results) > 3 {
+		t.Errorf("maxResults=3 should cap output, got %d", len(results))
+	}
+}
+
+func TestIndex_DefaultMaxResults(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "a", Slug: "a", Description: "search helper tool"},
+	})
+	// maxResults <= 0 → defaults to 5
+	results := idx.Search("search", 0)
+	// Should not panic and should return results
+	_ = results
+}
+
+func TestIndex_ScoreField_Populated(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "web", Slug: "web", Description: "Web search tool"},
+	})
+
+	results := idx.Search("web search", 5)
+	if len(results) == 0 {
+		t.Fatal("expected results")
+	}
+	if results[0].Score <= 0 {
+		t.Errorf("score should be positive, got %f", results[0].Score)
+	}
+}
+
+func TestIndex_ResultFields_Mapped(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{
+			Name:        "My Tool",
+			Slug:        "my-tool",
+			Description: "Does something useful",
+			Path:        "/path/to/SKILL.md",
+			BaseDir:     "/path/to",
+			Source:      "workspace",
+		},
+	})
+
+	results := idx.Search("useful", 5)
+	if len(results) == 0 {
+		t.Fatal("expected results")
+	}
+	r := results[0]
+	if r.Name != "My Tool" {
+		t.Errorf("Name: got %q", r.Name)
+	}
+	if r.Slug != "my-tool" {
+		t.Errorf("Slug: got %q", r.Slug)
+	}
+	if r.Description != "Does something useful" {
+		t.Errorf("Description: got %q", r.Description)
+	}
+	if r.Location != "/path/to/SKILL.md" {
+		t.Errorf("Location: got %q", r.Location)
+	}
+	if r.Source != "workspace" {
+		t.Errorf("Source: got %q", r.Source)
+	}
+}
+
+func TestIndex_Rebuild_Replaces(t *testing.T) {
+	idx := NewIndex()
+	idx.Build([]Info{
+		{Name: "old", Slug: "old", Description: "old skill"},
+	})
+
+	// Rebuild with completely different skills
+	idx.Build([]Info{
+		{Name: "new", Slug: "new", Description: "new skill"},
+	})
+
+	results := idx.Search("old", 5)
+	if len(results) != 0 {
+		t.Errorf("after rebuild, old skills should be gone, got %d results", len(results))
+	}
+
+	results = idx.Search("new", 5)
+	if len(results) == 0 {
+		t.Error("after rebuild, new skills should be searchable")
+	}
+}
+
+// --- sortScored ---
+
+func TestSortScored(t *testing.T) {
+	results := []scored{
+		{score: 1.5},
+		{score: 3.0},
+		{score: 0.5},
+		{score: 2.0},
+	}
+	sortScored(results)
+
+	for i := 1; i < len(results); i++ {
+		if results[i].score > results[i-1].score {
+			t.Errorf("not sorted descending: results[%d].score=%f > results[%d].score=%f",
+				i, results[i].score, i-1, results[i-1].score)
+		}
+	}
+}