diff --git a/internal/backup/fs_archive_test.go b/internal/backup/fs_archive_test.go new file mode 100644 index 00000000..e2d17880 --- /dev/null +++ b/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 +} diff --git a/internal/backup/manifest_dsn_test.go b/internal/backup/manifest_dsn_test.go new file mode 100644 index 00000000..ce4f8ef4 --- /dev/null +++ b/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) + } +} diff --git a/internal/config/config_extras_test.go b/internal/config/config_extras_test.go new file mode 100644 index 00000000..6b9f2b17 --- /dev/null +++ b/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") + } +} diff --git a/internal/mcp/util_bm25_test.go b/internal/mcp/util_bm25_test.go new file mode 100644 index 00000000..25c3c62a --- /dev/null +++ b/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{}, + } +} + diff --git a/internal/skills/loader_test.go b/internal/skills/loader_test.go new file mode 100644 index 00000000..7bb4f5bd --- /dev/null +++ b/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, "") { + t.Error("expected root element") + } + if !strings.Contains(result, "") { + t.Error("expected element") + } + if !strings.Contains(result, "Web Search") { + t.Error("expected skill name in summary") + } + if !strings.Contains(result, "") { + 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 &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, "") { + 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") + } + } +} diff --git a/internal/skills/search_bm25_test.go b/internal/skills/search_bm25_test.go new file mode 100644 index 00000000..7971bfbd --- /dev/null +++ b/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) + } + } +}