From 354ce0a861aceaf5271f8758a503c76ba7bd1d6c Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 2 Oct 2026 10:20:46 +0200 Subject: [PATCH] ref(cli): Complete skill command I/O injection Send remaining skill command output through the shared writer and move skill file reads and atomic writes behind the injected filesystem. Add focused command and filesystem tests for those boundaries. --- .../agent/skills/AgentSkillsGetCommand.swift | 4 +- .../skills/AgentSkillsInstallCommand.swift | 4 +- .../skills/AgentSkillsUninstallCommand.swift | 4 +- Sources/CLI/skills/AgentSkillFileSystem.swift | 12 +++ .../AgentSkillInstallationService.swift | 6 +- .../skills/AgentSkillsCommandTests.swift | 14 +++ .../AgentSkillsProjectInstallationTests.swift | 9 +- .../skills/AgentSkillFileSystemTests.swift | 87 +++++++++++++++++++ Tests/CLITests/telemetry/TelemetryTests.swift | 3 +- 9 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 Tests/CLITests/skills/AgentSkillFileSystemTests.swift 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 {