fix(test): improve coverage for TelemetryService and worktree-service

packages/core/src/custom-tools/__tests__/custom-tool-registry.spec.ts

@@ -212,4 +212,144 @@ describe("CustomToolRegistry", () => {
 			expect(registry.list()).toEqual([])
 		})
 	})
+
+	describe("getAllSerialized", () => {
+		it("should return serialized versions of all tools", () => {
+			const tool1: CustomToolDefinition = {
+				name: "serialized_tool_1",
+				description: "First serialized tool",
+				execute: async () => "result1",
+			}
+			const tool2: CustomToolDefinition = {
+				name: "serialized_tool_2",
+				description: "Second serialized tool",
+				execute: async () => "result2",
+			}
+
+			registry.register(tool1)
+			registry.register(tool2)
+
+			const serialized = registry.getAllSerialized()
+
+			expect(serialized).toHaveLength(2)
+			const first = serialized[0]!
+			const second = serialized[1]!
+			expect(first.name).toBe("serialized_tool_1")
+			expect(first.description).toBe("First serialized tool")
+			expect(second.name).toBe("serialized_tool_2")
+		})
+
+		it("should return empty array when no tools registered", () => {
+			const serialized = registry.getAllSerialized()
+			expect(serialized).toEqual([])
+		})
+	})
+
+	describe("setExtensionPath / getExtensionPath", () => {
+		it("should return undefined by default", () => {
+			expect(registry.getExtensionPath()).toBeUndefined()
+		})
+
+		it("should set and retrieve extension path", () => {
+			const testPath = "/path/to/extension"
+
+			registry.setExtensionPath(testPath)
+
+			expect(registry.getExtensionPath()).toBe(testPath)
+		})
+
+		it("should update extension path when called multiple times", () => {
+			registry.setExtensionPath("/first/path")
+			expect(registry.getExtensionPath()).toBe("/first/path")
+
+			registry.setExtensionPath("/second/path")
+			expect(registry.getExtensionPath()).toBe("/second/path")
+		})
+	})
+
+	describe("clearCache", () => {
+		it("should clear the in-memory TypeScript cache", () => {
+			const testRegistry = new CustomToolRegistry({ cacheDir: "/tmp/test-cache-clear" })
+
+			expect(testRegistry).toBeDefined()
+			testRegistry.clearCache()
+		})
+
+		it("should handle clearCache on a registry with tools", () => {
+			const testRegistry = new CustomToolRegistry({ cacheDir: "/tmp/test-cache-with-tools" })
+
+			testRegistry.register({ name: "cache_test_tool", description: "Test", execute: async () => "result" })
+
+			expect(testRegistry.size).toBe(1)
+
+			testRegistry.clearCache()
+
+			expect(testRegistry.size).toBe(1)
+			expect(testRegistry.has("cache_test_tool")).toBe(true)
+		})
+	})
+
+	describe("validate edge cases", () => {
+		it("should reject null value via register", () => {
+			// @ts-expect-error - testing invalid input
+			expect(() => registry.register(null)).toThrow()
+		})
+
+		it("should reject undefined value via register", () => {
+			// @ts-expect-error - testing invalid input
+			expect(() => registry.register(undefined)).toThrow()
+		})
+
+		it("should reject object without execute function", () => {
+			const noExecuteTool = { name: "no_execute", description: "Missing execute" } as CustomToolDefinition
+			expect(() => registry.register(noExecuteTool)).toThrow(/Invalid tool definition/)
+		})
+
+		it("should reject non-string description", () => {
+			const badDescTool = {
+				name: "bad_desc_tool",
+				description: 123,
+				execute: async () => "result",
+			} as unknown as CustomToolDefinition
+			expect(() => registry.register(badDescTool)).toThrow(/description/)
+		})
+
+		it("should reject object with non-string name", () => {
+			const badNameTool = {
+				name: 456,
+				description: "Has description",
+				execute: async () => "result",
+			} as unknown as CustomToolDefinition
+			expect(() => registry.register(badNameTool)).toThrow(/name/)
+		})
+
+		it("should reject tool with missing description", () => {
+			const minimalTool = {
+				name: "minimal_tool",
+				execute: async () => "result",
+			} as unknown as CustomToolDefinition
+			expect(() => registry.register(minimalTool)).toThrow(/description/)
+		})
+
+		it("should reject tool with execute that is not a function", () => {
+			const badExecTool = {
+				name: "bad_exec_tool",
+				description: "Bad exec type",
+				execute: "not-a-function" as unknown as CustomToolDefinition["execute"],
+			}
+			expect(() => registry.register(badExecTool)).toThrow(/Invalid tool definition/)
+		})
+
+		it("should accept valid tool with all fields including parameters", () => {
+			const fullTool: CustomToolDefinition = {
+				name: "full_tool",
+				description: "Full featured tool",
+				parameters: z.object({ input: z.string() }),
+				execute: async (args) => `Processed: ${args.input}`,
+			}
+
+			expect(() => registry.register(fullTool)).not.toThrow()
+			expect(registry.has("full_tool")).toBe(true)
+		})
+	})
 })

packages/core/src/worktree/__tests__/worktree-service.spec.ts

@@ -1,5 +1,14 @@
 import * as path from "path"
 
+// Mock child_process BEFORE importing WorktreeService so that promisify(exec) and
+// promisify(execFile) capture the mocked versions at module load time.
+vi.mock("child_process", () => {
+	const exec = vi.fn()
+	const execFile = vi.fn()
+	return { exec, execFile }
+})
+
+import type { Worktree } from "../types.js"
 import { WorktreeService } from "../worktree-service.js"
 
 describe("WorktreeService", () => {
@@ -10,7 +19,6 @@ describe("WorktreeService", () => {
 			service = new WorktreeService()
 		})
 
-		// Access private method for testing
 		const callNormalizePath = (service: WorktreeService, p: string): string => {
 			// @ts-expect-error - accessing private method for testing
 			return service.normalizePath(p)
@@ -23,13 +31,10 @@ describe("WorktreeService", () => {
 
 		it("should normalize paths with multiple trailing slashes", () => {
 			const result = callNormalizePath(service, "/home/user/project///")
-			// path.normalize already handles multiple slashes
 			expect(result).toBe(path.normalize("/home/user/project"))
 		})
 
 		it("should preserve root path /", () => {
-			// This is a critical test - the old regex would turn "/" into ""
-			// On Windows, path.normalize("/") returns "\", on Unix it returns "/"
 			const result = callNormalizePath(service, "/")
 			expect(result).toBe(path.sep)
 		})
@@ -50,9 +55,7 @@ describe("WorktreeService", () => {
 		})
 
 		it("should handle Windows-style paths on non-Windows", () => {
-			// path.normalize will convert separators appropriately
 			const result = callNormalizePath(service, "C:\\Users\\test\\project")
-			// On Unix, this stays as-is; on Windows it would normalize
 			expect(result).toBeTruthy()
 		})
 	})
@@ -64,12 +67,7 @@ describe("WorktreeService", () => {
 			service = new WorktreeService()
 		})
 
-		// Access private method for testing
-		const callParseWorktreeOutput = (
-			service: WorktreeService,
-			output: string,
-			currentCwd: string,
-		): ReturnType<WorktreeService["parseWorktreeOutput"]> => {
+		const callParseWorktreeOutput = (service: WorktreeService, output: string, currentCwd: string): Worktree[] => {
 			// @ts-expect-error - accessing private method for testing
 			return service.parseWorktreeOutput(output, currentCwd)
 		}
@@ -115,7 +113,7 @@ detached
 			})
 		})
 
-		it("should handle locked worktrees", () => {
+		it("should handle locked worktrees with reason", () => {
 			const output = `worktree /home/user/repo-locked
 HEAD abc123def456
 branch refs/heads/locked-branch
@@ -142,5 +140,103 @@ bare
 				isBare: true,
 			})
 		})
+
+		it("should handle empty output", () => {
+			const result = callParseWorktreeOutput(service, "", "/home/user/other")
+			expect(result).toHaveLength(0)
+		})
+
+		it("should handle whitespace-only output", () => {
+			const result = callParseWorktreeOutput(service, "   \n\n  ", "/home/user/other")
+			expect(result).toHaveLength(0)
+		})
+
+		it("should parse worktrees with no branch field", () => {
+			const output = `worktree /home/user/repo-no-branch
+HEAD abc123def456
+`
+			const result = callParseWorktreeOutput(service, output, "/home/user/other")
+			expect(result).toHaveLength(1)
+			expect(result[0]).toMatchObject({
+				path: "/home/user/repo-no-branch",
+				branch: "",
+			})
+		})
+
+		it("should parse worktrees with locked reason field", () => {
+			const output = `worktree /home/user/repo-locked2
+HEAD abc123def456
+branch refs/heads/test
+locked another reason
+`
+			const result = callParseWorktreeOutput(service, output, "/home/user/other")
+			expect(result).toHaveLength(1)
+			expect(result[0]).toMatchObject({
+				isLocked: true,
+				lockReason: "another reason",
+			})
+		})
+
+		it("should handle multiple worktrees with mixed states", () => {
+			const output = `worktree /home/user/repo-main
+HEAD abc123def456
+branch refs/heads/main
+
+worktree /home/user/repo-detached-2
+HEAD def456abc789
+detached
+
+worktree /home/user/repo-locked-3
+HEAD 789abc123def
+branch refs/heads/locked-branch
+locked lock reason
+`
+			const result = callParseWorktreeOutput(service, output, "/home/user/repo-main")
+			expect(result).toHaveLength(3)
+			expect(result[0]).toMatchObject({ isCurrent: true, branch: "main" })
+			expect(result[1]).toMatchObject({ isDetached: true, branch: "" })
+			expect(result[2]).toMatchObject({ isLocked: true, lockReason: "lock reason" })
+		})
+
+		it("should handle Windows-style paths in worktree output", () => {
+			const output = `worktree C:\\Users\\test\\repo
+HEAD abc123def456
+branch refs/heads/main
+`
+			const result = callParseWorktreeOutput(service, output, "C:\\Users\\test\\repo")
+			expect(result).toHaveLength(1)
+			expect(result[0]).toMatchObject({
+				path: "C:\\Users\\test\\repo",
+				branch: "main",
+			})
+		})
+
+		it("should handle worktree with trailing whitespace in fields", () => {
+			const output = `worktree /home/user/repo  
+HEAD abc123def456 
+branch refs/heads/main 
+`
+			const result = callParseWorktreeOutput(service, output, "/home/user/repo")
+			expect(result).toHaveLength(1)
+			expect(result[0]).toMatchObject({
+				path: "/home/user/repo",
+				commitHash: "abc123def456",
+				branch: "main",
+			})
+		})
+
+		it("should handle worktree with only whitespace between entries", () => {
+			const output = `worktree /home/user/repo1
+HEAD abc123
+
+
+
+worktree /home/user/repo2
+DEF456
+branch refs/heads/test
+`
+			const result = callParseWorktreeOutput(service, output, "/home/user/repo1")
+			expect(result).toHaveLength(2)
+		})
 	})
 })