diff --git a/Sources/CLI/cmd/agent/skills/AgentSkillsGetCommand.swift b/Sources/CLI/cmd/agent/skills/AgentSkillsGetCommand.swift index d20fa86..c775319 100644 --- a/Sources/CLI/cmd/agent/skills/AgentSkillsGetCommand.swift +++ b/Sources/CLI/cmd/agent/skills/AgentSkillsGetCommand.swift @@ -2,7 +2,7 @@ import ArgumentParser struct AgentSkillsGetCommand: ParsableCommand, GlobalOptionsProviding { #if DEBUG - typealias Deps = any TelemetryProvider + typealias Deps = any (TelemetryProvider & CommandOutputWriterProvider) #else typealias Deps = Dependencies #endif @@ -26,6 +26,6 @@ struct AgentSkillsGetCommand: ParsableCommand, GlobalOptionsProviding { guard let skill = BundledAgentSkills.skill(named: name) else { throw ValidationError("Unknown bundled Agent Skill '\(name)'.") } - print(skill.content) + deps.commandOutputWriter.write(skill.content) } } diff --git a/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift b/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift index dac3a2a..145dc69 100644 --- a/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift +++ b/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift @@ -2,7 +2,7 @@ import ArgumentParser struct AgentSkillsInstallCommand: ParsableCommand, GlobalOptionsProviding { #if DEBUG - typealias Deps = any AgentSkillInstallationServiceProvider + typealias Deps = any (AgentSkillInstallationServiceProvider & CommandOutputWriterProvider) #else typealias Deps = Dependencies #endif @@ -43,6 +43,6 @@ struct AgentSkillsInstallCommand: ParsableCommand, GlobalOptionsProviding { selection.selectedSkills(), root: selection.installationRoot(fileManager: deps.agentSkillFileManager()), dryRun: selection.dryRun, force: force ) - print(output) + deps.commandOutputWriter.write(output) } } diff --git a/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift b/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift index 574d2ad..4dbbff7 100644 --- a/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift +++ b/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift @@ -2,7 +2,7 @@ import ArgumentParser struct AgentSkillsUninstallCommand: ParsableCommand, GlobalOptionsProviding { #if DEBUG - typealias Deps = any AgentSkillInstallationServiceProvider + typealias Deps = any (AgentSkillInstallationServiceProvider & CommandOutputWriterProvider) #else typealias Deps = Dependencies #endif @@ -46,6 +46,6 @@ struct AgentSkillsUninstallCommand: ParsableCommand, GlobalOptionsProviding { selection.selectedSkills(), root: selection.installationRoot(fileManager: deps.agentSkillFileManager()), dryRun: selection.dryRun ) - print(output) + deps.commandOutputWriter.write(output) } } diff --git a/Sources/CLI/skills/AgentSkillFileSystem.swift b/Sources/CLI/skills/AgentSkillFileSystem.swift index 4b68774..894f143 100644 --- a/Sources/CLI/skills/AgentSkillFileSystem.swift +++ b/Sources/CLI/skills/AgentSkillFileSystem.swift @@ -1,5 +1,15 @@ import Foundation +extension FileManager { + func readSkillData(at url: URL) throws -> Data { + try Data(contentsOf: url) + } + + func writeSkillData(_ data: Data, to url: URL) throws { + try data.write(to: url, options: .atomic) + } +} + #if DEBUG protocol AgentSkillFileSystem { var currentDirectoryPath: String { get } @@ -12,6 +22,8 @@ import Foundation func removeItem(at url: URL) throws func contentsOfDirectory(atPath path: String) throws -> [String] func attributesOfItem(atPath path: String) throws -> [FileAttributeKey: Any] + func readSkillData(at url: URL) throws -> Data + func writeSkillData(_ data: Data, to url: URL) throws } extension FileManager: AgentSkillFileSystem {} diff --git a/Sources/CLI/skills/AgentSkillInstallationService.swift b/Sources/CLI/skills/AgentSkillInstallationService.swift index 7057e8d..c2c8b6c 100644 --- a/Sources/CLI/skills/AgentSkillInstallationService.swift +++ b/Sources/CLI/skills/AgentSkillInstallationService.swift @@ -119,9 +119,9 @@ struct DefaultAgentSkillInstallationService { if !dryRun { try fileManager.createDirectory( at: installation.directory, withIntermediateDirectories: true, attributes: nil) - try content.write(to: installation.file, options: .atomic) + try fileManager.writeSkillData(content, to: installation.file) let receipt = Receipt(name: installation.skill.name, content: content) - try JSONEncoder().encode(receipt).write(to: installation.receiptFile, options: .atomic) + try fileManager.writeSkillData(JSONEncoder().encode(receipt), to: installation.receiptFile) } logger.info(dryRun ? "Would install skill" : "Installed skill", metadata: metadata) return "\(dryRun ? "Would install" : "Installed"): \(installation.skill.name) at \(installation.file.path)" @@ -208,7 +208,7 @@ struct DefaultAgentSkillInstallationService { metadata: ["file": .string(url.lastPathComponent)]) throw ValidationError("Expected a regular file, not a symlink or directory, at '\(url.path)'.") } - return try Data(contentsOf: url) + return try fileManager.readSkillData(at: url) } private func fileType(_ url: URL) throws -> FileAttributeType? { diff --git a/Tests/CLITests/cmd/agent/skills/AgentSkillsCommandTests.swift b/Tests/CLITests/cmd/agent/skills/AgentSkillsCommandTests.swift index ccbfb00..8c98bc2 100644 --- a/Tests/CLITests/cmd/agent/skills/AgentSkillsCommandTests.swift +++ b/Tests/CLITests/cmd/agent/skills/AgentSkillsCommandTests.swift @@ -19,6 +19,20 @@ struct AgentSkillsCommandTests { #expect(deps.telemetry.commands == ["agent.skills.list"]) } + @Test("get writes bundled skill content through the injected output") + func writesSkillContent() throws { + // -- Arrange -- + let command = try AgentSkillsGetCommand.parse(["apple-docs"]) + let deps = SkillListDependencies() + + // -- Act -- + try command.run(deps: deps) + + // -- Assert -- + #expect(deps.output.lines == [BundledAgentSkills.skill(named: "apple-docs")?.content]) + #expect(deps.telemetry.commands == ["agent.skills.get"]) + } + @Test("registers the nested list command") func parsesListCommand() throws { // -- Arrange -- diff --git a/Tests/CLITests/cmd/agent/skills/AgentSkillsProjectInstallationTests.swift b/Tests/CLITests/cmd/agent/skills/AgentSkillsProjectInstallationTests.swift index e11c7a8..28f1482 100644 --- a/Tests/CLITests/cmd/agent/skills/AgentSkillsProjectInstallationTests.swift +++ b/Tests/CLITests/cmd/agent/skills/AgentSkillsProjectInstallationTests.swift @@ -22,6 +22,10 @@ struct AgentSkillsProjectInstallationTests { #expect( FileManager.default.fileExists( atPath: home.appendingPathComponent(".agents/skills/apple-docs/SKILL.md").path)) + #expect( + deps.output.lines == [ + "Installed: apple-docs at \(home.appendingPathComponent(".agents/skills/apple-docs/SKILL.md").path)" + ]) } @Test("default uninstall uses the file manager's home directory") @@ -43,6 +47,7 @@ struct AgentSkillsProjectInstallationTests { #expect( !FileManager.default.fileExists( atPath: home.appendingPathComponent(".agents/skills/apple-docs/SKILL.md").path)) + #expect(deps.output.lines.last == "Uninstalled: apple-docs") } @Test("project installation targets the Git root from a nested directory") @@ -182,8 +187,10 @@ struct AgentSkillsProjectInstallationTests { } } -private struct TestAgentSkillDependencies: AgentSkillInstallationServiceProvider { +private struct TestAgentSkillDependencies: AgentSkillInstallationServiceProvider, CommandOutputWriterProvider { let fileManager: FileManager + let output = RecordingCommandOutputWriter() + var commandOutputWriter: RecordingCommandOutputWriter { output } func agentSkillFileManager() -> FileManager { fileManager } diff --git a/Tests/CLITests/skills/AgentSkillFileSystemTests.swift b/Tests/CLITests/skills/AgentSkillFileSystemTests.swift new file mode 100644 index 0000000..726ef55 --- /dev/null +++ b/Tests/CLITests/skills/AgentSkillFileSystemTests.swift @@ -0,0 +1,87 @@ +import Foundation +import Logging +import Testing + +@testable import CLI + +@Suite("Agent skill file access") +struct AgentSkillFileSystemTests { + @Test("installation uses injected atomic file writes") + func usesInjectedWriter() throws { + // -- Arrange -- + let root = temporaryRoot() + defer { try? FileManager.default.removeItem(at: root) } + let skill = try #require(BundledAgentSkills.skill(named: "apple-docs")) + let service = DefaultAgentSkillInstallationService( + logger: Logger(label: "test"), fileManager: FailingDataAccessFileSystem(denied: .write)) + let file = root.appendingPathComponent("skills/apple-docs/SKILL.md") + + // -- Act -- + #expect(throws: DataAccessDenied.self) { + _ = try service.install([skill], root: root.path, dryRun: false, force: false) + } + + // -- Assert -- + #expect(!FileManager.default.fileExists(atPath: file.path)) + } + + @Test("uninstallation uses injected file reads before deleting") + func usesInjectedReader() throws { + // -- Arrange -- + let root = temporaryRoot() + defer { try? FileManager.default.removeItem(at: root) } + let skill = try #require(BundledAgentSkills.skill(named: "apple-docs")) + let installed = DefaultAgentSkillInstallationService(logger: Logger(label: "test")) + _ = try installed.install([skill], root: root.path, dryRun: false, force: false) + let service = DefaultAgentSkillInstallationService( + logger: Logger(label: "test"), fileManager: FailingDataAccessFileSystem(denied: .read)) + let file = root.appendingPathComponent("skills/apple-docs/SKILL.md") + + // -- Act -- + #expect(throws: DataAccessDenied.self) { + _ = try service.uninstall([skill], root: root.path, dryRun: false) + } + + // -- Assert -- + #expect(FileManager.default.fileExists(atPath: file.path)) + } + + private func temporaryRoot() -> URL { + FileManager.default.temporaryDirectory.resolvingSymlinksInPath() + .appendingPathComponent(UUID().uuidString) + } +} + +private enum DataAccessDenied: Error { + case read, write +} + +private struct FailingDataAccessFileSystem: AgentSkillFileSystem { + let denied: DataAccessDenied + private let fileManager = FileManager.default + + var currentDirectoryPath: String { fileManager.currentDirectoryPath } + var homeDirectoryForCurrentUser: URL { fileManager.homeDirectoryForCurrentUser } + func fileExists(atPath path: String) -> Bool { fileManager.fileExists(atPath: path) } + func createDirectory( + at url: URL, withIntermediateDirectories createIntermediates: Bool, attributes: [FileAttributeKey: Any]? + ) throws { + try fileManager.createDirectory( + at: url, withIntermediateDirectories: createIntermediates, attributes: attributes) + } + func removeItem(at url: URL) throws { try fileManager.removeItem(at: url) } + func contentsOfDirectory(atPath path: String) throws -> [String] { + try fileManager.contentsOfDirectory(atPath: path) + } + func attributesOfItem(atPath path: String) throws -> [FileAttributeKey: Any] { + try fileManager.attributesOfItem(atPath: path) + } + func readSkillData(at url: URL) throws -> Data { + if denied == .read { throw DataAccessDenied.read } + return try Data(contentsOf: url) + } + func writeSkillData(_ data: Data, to url: URL) throws { + if denied == .write { throw DataAccessDenied.write } + try data.write(to: url, options: .atomic) + } +} diff --git a/Tests/CLITests/telemetry/TelemetryTests.swift b/Tests/CLITests/telemetry/TelemetryTests.swift index 248c18d..583ab53 100644 --- a/Tests/CLITests/telemetry/TelemetryTests.swift +++ b/Tests/CLITests/telemetry/TelemetryTests.swift @@ -76,8 +76,9 @@ struct TelemetryTests { } } -private struct CommandDeps: TelemetryProvider { +private struct CommandDeps: TelemetryProvider, CommandOutputWriterProvider { let telemetry: CommandTelemetryRecorder + let commandOutputWriter = RecordingCommandOutputWriter() } private final class CommandTelemetryRecorder: Telemetry, @unchecked Sendable {