diff --git a/Package.swift b/Package.swift index 89d3924..0500bc9 100644 --- a/Package.swift +++ b/Package.swift @@ -18,6 +18,10 @@ let package = Package( .product(name: "ArgumentParser", package: "swift-argument-parser"), .product(name: "ZIPFoundation", package: "ZIPFoundation") ] + ), + .testTarget( + name: "pfwTests", + dependencies: ["pfw"] ) ], swiftLanguageModes: [.v6] diff --git a/Sources/pfw/Install.swift b/Sources/pfw/Install.swift index 84026ab..d474904 100644 --- a/Sources/pfw/Install.swift +++ b/Sources/pfw/Install.swift @@ -27,13 +27,37 @@ struct Install: AsyncParsableCommand { ) var tool: Tool = .codex - @Option(help: "Directory to install skills into.") + @Option(help: "Directory to install skills into. Use '.' for current directory.") var path: String? func run() async throws { try await install(shouldRetryAfterLogin: true) } + // MARK: - Testable Helper Functions + + static func isCurrentDirectoryPath(_ path: String?) -> Bool { + path == "." + } + + static func validateInstallPath(_ installPath: String, tool: Tool) -> Bool { + // Allow any path that contains /skills to support different directory naming conventions + // (e.g., .codex/skills, .claude/skills, .github/skills, .copilot/skills) + return installPath.contains("/skills") + } + + static func resolveInstallURL( + path: String?, + tool: Tool, + currentDirectory: String = FileManager.default.currentDirectoryPath + ) -> URL { + if isCurrentDirectoryPath(path) { + return URL(fileURLWithPath: currentDirectory) + } else { + return URL(fileURLWithPath: path ?? tool.defaultInstallPath.path) + } + } + private func install(shouldRetryAfterLogin: Bool) async throws { let token = try loadToken() let machine = try machine() @@ -68,9 +92,82 @@ struct Install: AsyncParsableCommand { let zipURL = URL.temporaryDirectory.appending(path: UUID().uuidString) try data.write(to: zipURL) - let installURL = URL(fileURLWithPath: path ?? tool.defaultInstallPath.path) - try? FileManager.default.removeItem(at: installURL) - try FileManager.default.unzipItem(at: zipURL, to: installURL) - print("Installed skills for \(tool.rawValue) into \(installURL.path)") + // Determine if installing to current directory + let isCurrentDirectory = Self.isCurrentDirectoryPath(path) + let installURL = Self.resolveInstallURL(path: path, tool: tool) + + // Verify the install path is in the expected location (only if custom path provided) + if path != nil { + let installPath = installURL.path + + guard Self.validateInstallPath(installPath, tool: tool) else { + print("Error: Install path is not in the expected location.") + print("") + print("The install path must contain '/skills' in it.") + print("") + print("Valid options:") + print(" 1. Use default path: pfw install --tool \(tool.rawValue)") + print(" Installs to: ~/.\(tool.rawValue)/skills/the-point-free-way") + print("") + print(" 2. Use custom path containing '/skills':") + print(" pfw install --tool \(tool.rawValue) --path /your/project/.config/skills") + print("") + print("Current install path: \(installPath)") + throw ExitCode.failure + } + + // Ask for confirmation when using custom path + print("You are about to install into:") + print(" \(installPath)") + print("\nThis will merge new skills with existing ones without removing current files.") + print("Continue? (yes/no): ", terminator: "") + + guard let response = readLine()?.lowercased(), + response == "yes" || response == "y" else { + print("Installation cancelled.") + throw ExitCode.success + } + } + + // Always merge - never remove existing files + // The zip contains a top-level "skills" folder, so we extract to a temp location + // and then move the contents of the skills folder to the target location + let tempExtractURL = URL.temporaryDirectory.appending(path: UUID().uuidString) + try FileManager.default.createDirectory(at: tempExtractURL, withIntermediateDirectories: true) + + try FileManager.default.unzipItem(at: zipURL, to: tempExtractURL) + + // The zip extracts to tempExtractURL/skills/, we want to move its contents to installURL + let extractedSkillsURL = tempExtractURL.appending(path: "skills") + + // Ensure target directory exists + try FileManager.default.createDirectory(at: installURL, withIntermediateDirectories: true) + + // Move each skill from the extracted location to the target + let skillDirs = try FileManager.default.contentsOfDirectory( + at: extractedSkillsURL, + includingPropertiesForKeys: nil + ) + + for skillURL in skillDirs { + let targetURL = installURL.appending(path: skillURL.lastPathComponent) + + // If the skill already exists, remove it first (we're replacing individual skills, not the whole directory) + if FileManager.default.fileExists(atPath: targetURL.path) { + try FileManager.default.removeItem(at: targetURL) + } + + try FileManager.default.moveItem(at: skillURL, to: targetURL) + } + + // Clean up temp directory + try? FileManager.default.removeItem(at: tempExtractURL) + try? FileManager.default.removeItem(at: zipURL) + + if isCurrentDirectory { + print("Successfully merged skills into \(installURL.path)") + } else { + print("Installed and merged skills for \(tool.rawValue) into \(installURL.path)") + } } } diff --git a/Tests/pfwTests/InstallTests.swift b/Tests/pfwTests/InstallTests.swift new file mode 100644 index 0000000..d335a60 --- /dev/null +++ b/Tests/pfwTests/InstallTests.swift @@ -0,0 +1,155 @@ +import Testing +import Foundation +@testable import pfw + +@Suite("Install Path Validation Tests") +struct InstallPathValidationTests { + + @Test("Valid Claude skills path is accepted") + func validClaudeSkillsPath() { + #expect(Install.validateInstallPath("/Users/test/.claude/skills", tool: .claude)) + #expect(Install.validateInstallPath("/project/.claude/skills/my-skills", tool: .claude)) + #expect(Install.validateInstallPath("/home/user/.claude/skills", tool: .claude)) + } + + @Test("Valid Codex skills path is accepted") + func validCodexSkillsPath() { + #expect(Install.validateInstallPath("/Users/test/.codex/skills", tool: .codex)) + #expect(Install.validateInstallPath("/project/.codex/skills/custom", tool: .codex)) + #expect(Install.validateInstallPath("/home/user/.codex/skills", tool: .codex)) + } + + @Test("Flexible directory naming is supported") + func flexibleDirectoryNaming() { + // Support different directory naming conventions (e.g., .github for copilot) + #expect(Install.validateInstallPath("/Users/test/.github/skills", tool: .claude)) + #expect(Install.validateInstallPath("/project/.config/skills", tool: .codex)) + #expect(Install.validateInstallPath("/home/user/my-project/skills", tool: .claude)) + } + + @Test("Invalid paths are rejected") + func invalidPathsRejected() { + // Paths without /skills + #expect(!Install.validateInstallPath("/tmp", tool: .claude)) + #expect(!Install.validateInstallPath("/Users/test/Downloads", tool: .claude)) + #expect(!Install.validateInstallPath("/project", tool: .codex)) + #expect(!Install.validateInstallPath("/home/user/.claude", tool: .claude)) + } + + @Test("Paths containing skills keyword are accepted") + func pathsContainingSkillsAccepted() { + // Any path with /skills is valid regardless of tool + #expect(Install.validateInstallPath("/Users/test/.claude/skills", tool: .codex)) + #expect(Install.validateInstallPath("/Users/test/.codex/skills", tool: .claude)) + #expect(Install.validateInstallPath("/home/user/skills", tool: .claude)) + } +} + +@Suite("Current Directory Detection Tests") +struct CurrentDirectoryDetectionTests { + + @Test("Dot is recognized as current directory") + func dotIsCurrentDirectory() { + #expect(Install.isCurrentDirectoryPath(".")) + } + + @Test("nil is not current directory") + func nilIsNotCurrentDirectory() { + #expect(!Install.isCurrentDirectoryPath(nil)) + } + + @Test("Absolute paths are not current directory") + func absolutePathsNotCurrentDirectory() { + #expect(!Install.isCurrentDirectoryPath("/tmp")) + #expect(!Install.isCurrentDirectoryPath("/Users/test/.claude/skills")) + #expect(!Install.isCurrentDirectoryPath("~/project")) + } + + @Test("Relative paths are not current directory") + func relativePathsNotCurrentDirectory() { + #expect(!Install.isCurrentDirectoryPath("./subdir")) + #expect(!Install.isCurrentDirectoryPath("../parent")) + } +} + +@Suite("Install URL Resolution Tests") +struct InstallURLResolutionTests { + + @Test("Resolves to current directory when path is dot") + func resolvesToCurrentDirectoryForDot() { + let currentDir = "/Users/test/.claude/skills" + let url = Install.resolveInstallURL(path: ".", tool: .claude, currentDirectory: currentDir) + #expect(url.path == currentDir) + } + + @Test("Resolves to custom path when provided") + func resolvesToCustomPath() { + let customPath = "/project/.claude/skills" + let url = Install.resolveInstallURL(path: customPath, tool: .claude) + #expect(url.path == customPath) + } + + @Test("Resolves to default path when path is nil for Claude") + func resolvesToDefaultPathForClaude() { + let url = Install.resolveInstallURL(path: nil, tool: .claude) + #expect(url.path.contains(".claude/skills/the-point-free-way")) + } + + @Test("Resolves to default path when path is nil for Codex") + func resolvesToDefaultPathForCodex() { + let url = Install.resolveInstallURL(path: nil, tool: .codex) + #expect(url.path.contains(".codex/skills/the-point-free-way")) + } +} + +@Suite("Tool Default Paths Tests") +struct ToolDefaultPathsTests { + + @Test("Claude default path contains correct pattern") + func claudeDefaultPath() { + let path = Install.Tool.claude.defaultInstallPath + #expect(path.path.contains(".claude/skills/the-point-free-way")) + } + + @Test("Codex default path contains correct pattern") + func codexDefaultPath() { + let path = Install.Tool.codex.defaultInstallPath + #expect(path.path.contains(".codex/skills/the-point-free-way")) + } +} + +@Suite("Edge Cases Tests") +struct EdgeCasesTests { + + @Test("Empty string path is not current directory") + func emptyStringNotCurrentDirectory() { + #expect(!Install.isCurrentDirectoryPath("")) + } + + @Test("Whitespace path is not current directory") + func whitespaceNotCurrentDirectory() { + #expect(!Install.isCurrentDirectoryPath(" ")) + #expect(!Install.isCurrentDirectoryPath(" ")) + } + + @Test("Skills keyword validation is case sensitive") + func skillsKeywordCaseSensitive() { + // Lowercase /skills should match + #expect(Install.validateInstallPath("/Users/test/.claude/skills", tool: .claude)) + // Capitalized /Skills or /SKILLS should not match (path is case-sensitive) + #expect(!Install.validateInstallPath("/Users/test/.claude/Skills", tool: .claude)) + #expect(!Install.validateInstallPath("/Users/test/.claude/SKILLS", tool: .claude)) + } + + @Test("Path with multiple dots") + func pathWithMultipleDots() { + #expect(Install.validateInstallPath("/Users/test.user/.claude/skills", tool: .claude)) + #expect(Install.validateInstallPath("/Users/test/.config/.claude/skills", tool: .claude)) + } + + @Test("Path with trailing slashes") + func pathWithTrailingSlashes() { + #expect(Install.validateInstallPath("/Users/test/.claude/skills/", tool: .claude)) + #expect(Install.validateInstallPath("/Users/test/.claude/skills//", tool: .claude)) + } +}