fix: properly handle symbolic links to directories

- Check if symbolic links point to files before processing
- Prevent attempting to read directories as files
- Add test coverage for symlinks pointing to directories

Author: daniel-lxs

src/services/command/__tests__/symlink-commands.spec.ts

@@ -35,7 +35,14 @@ description: Deploys the application
 
 Deploy instructions.`
 
-			mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true })
+			mockFs.stat = vi
+				.fn()
+				// First call for directory check
+				.mockResolvedValueOnce({ isDirectory: () => true } as any)
+				// Subsequent calls for symlink target checks
+				.mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) // setup.md symlink points to file
+				.mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) // build.md symlink points to file
+
 			mockFs.readdir = vi.fn().mockResolvedValue([
 				{ name: "setup.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link
 				{ name: "deploy.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file
@@ -86,10 +93,17 @@ description: Linked command
 
 This command is accessed via symbolic link.`
 
-			mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true })
+			// Mock directory stat
+			mockFs.stat = vi
+				.fn()
+				// First call for directory check
+				.mockResolvedValueOnce({ isDirectory: () => true } as any)
+				// Second call for symlink target check (linked.md points to a file)
+				.mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any)
+
 			mockFs.readdir = vi.fn().mockResolvedValue([
 				{ name: "test.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file
-				{ name: "linked.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link
+				{ name: "linked.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link to file
 				{ name: "not-markdown.txt", isFile: () => true, isSymbolicLink: () => false }, // Should be ignored
 				{ name: "symlink.txt", isFile: () => false, isSymbolicLink: () => true }, // Non-markdown symlink, should be ignored
 			])
@@ -118,15 +132,16 @@ This command is accessed via symbolic link.`
 
 This is a valid command.`
 
-			mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true })
+			mockFs.stat = vi
+				.fn()
+				.mockResolvedValueOnce({ isDirectory: () => true } as any) // Directory check
+				.mockRejectedValueOnce(new Error("ENOENT: no such file or directory")) // Broken symlink
+
 			mockFs.readdir = vi.fn().mockResolvedValue([
 				{ name: "valid.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file
 				{ name: "broken-link.md", isFile: () => false, isSymbolicLink: () => true }, // Broken symbolic link
 			])
-			mockFs.readFile = vi
-				.fn()
-				.mockResolvedValueOnce(validContent)
-				.mockRejectedValueOnce(new Error("ENOENT: no such file or directory")) // Broken link
+			mockFs.readFile = vi.fn().mockResolvedValueOnce(validContent)
 
 			const result = await getCommands("/test/cwd")
 
@@ -139,6 +154,35 @@ This is a valid command.`
 			)
 		})
 
+		it("should ignore symbolic links pointing to directories", async () => {
+			const validContent = `# Valid Command
+
+This is a valid command.`
+
+			mockFs.stat = vi
+				.fn()
+				.mockResolvedValueOnce({ isDirectory: () => true } as any) // Commands directory check
+				.mockResolvedValueOnce({ isFile: () => false, isDirectory: () => true } as any) // dir-link.md points to a directory
+
+			mockFs.readdir = vi.fn().mockResolvedValue([
+				{ name: "valid.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file
+				{ name: "dir-link.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link to directory
+			])
+			mockFs.readFile = vi.fn().mockResolvedValueOnce(validContent)
+
+			const result = await getCommands("/test/cwd")
+
+			// Should only load the valid command, ignoring the symlink to directory
+			expect(result).toHaveLength(1)
+			expect(result[0]).toEqual(
+				expect.objectContaining({
+					name: "valid",
+				}),
+			)
+			// readFile should only be called once (for the valid file)
+			expect(mockFs.readFile).toHaveBeenCalledTimes(1)
+		})
+
 		it("should prioritize project symbolic links over global commands", async () => {
 			const projectSymlinkContent = `---
 description: Project symbolic link command
@@ -157,7 +201,11 @@ description: Global command
 Global command content.`
 
 			// Mock both directories
-			mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true })
+			mockFs.stat = vi
+				.fn()
+				.mockResolvedValueOnce({ isDirectory: () => true } as any) // Global directory
+				.mockResolvedValueOnce({ isDirectory: () => true } as any) // Project directory
+				.mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) // Project symlink target
 
 			// First call for global directory scan, second for project directory scan
 			mockFs.readdir = vi

src/services/command/commands.ts

@@ -136,9 +136,28 @@ async function scanCommandDirectory(
 		const entries = await fs.readdir(dirPath, { withFileTypes: true })
 
 		for (const entry of entries) {
-			// Check for both regular files and symbolic links
-			if ((entry.isFile() || entry.isSymbolicLink()) && isMarkdownFile(entry.name)) {
+			// Only process markdown files
+			if (isMarkdownFile(entry.name)) {
 				const filePath = path.join(dirPath, entry.name)
+
+				// Check if entry is a valid file or a symbolic link pointing to a file
+				let isValidFile = false
+				if (entry.isFile()) {
+					isValidFile = true
+				} else if (entry.isSymbolicLink()) {
+					try {
+						const stats = await fs.stat(filePath) // follows the symlink
+						isValidFile = stats.isFile()
+					} catch {
+						// Broken symlink or points to non-existent target
+						isValidFile = false
+					}
+				}
+
+				if (!isValidFile) {
+					continue
+				}
+
 				const commandName = getCommandNameFromFile(entry.name)
 
 				try {