Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
107 changes: 102 additions & 5 deletions Sources/pfw/Install.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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")
Comment thread
ibrahimkteish marked this conversation as resolved.
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)")
}
}
}
155 changes: 155 additions & 0 deletions Tests/pfwTests/InstallTests.swift
Original file line number Diff line number Diff line change
@@ -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))
}
}