diff --git a/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift b/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift index 145dc69..860b2fa 100644 --- a/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift +++ b/Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift @@ -39,10 +39,14 @@ struct AgentSkillsInstallCommand: ParsableCommand, GlobalOptionsProviding { } func run(deps: Deps) throws { - let output = try deps.agentSkillInstallationService().install( - selection.selectedSkills(), root: selection.installationRoot(fileManager: deps.agentSkillFileManager()), - dryRun: selection.dryRun, force: force - ) - deps.commandOutputWriter.write(output) + do { + let outcomes = try deps.agentSkillInstallationService().install( + selection.selectedSkills(), root: selection.installationRoot(fileManager: deps.agentSkillFileManager()), + dryRun: selection.dryRun, force: force + ) + deps.commandOutputWriter.write(AgentSkillInstallationRenderer().render(outcomes)) + } catch let error as AgentSkillInstallationError { + throw ValidationError(error.localizedDescription) + } } } diff --git a/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift b/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift index 4dbbff7..09a1817 100644 --- a/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift +++ b/Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift @@ -42,10 +42,14 @@ struct AgentSkillsUninstallCommand: ParsableCommand, GlobalOptionsProviding { } func run(deps: Deps) throws { - let output = try deps.agentSkillInstallationService().uninstall( - selection.selectedSkills(), root: selection.installationRoot(fileManager: deps.agentSkillFileManager()), - dryRun: selection.dryRun - ) - deps.commandOutputWriter.write(output) + do { + let outcomes = try deps.agentSkillInstallationService().uninstall( + selection.selectedSkills(), root: selection.installationRoot(fileManager: deps.agentSkillFileManager()), + dryRun: selection.dryRun + ) + deps.commandOutputWriter.write(AgentSkillInstallationRenderer().render(outcomes)) + } catch let error as AgentSkillInstallationError { + throw ValidationError(error.localizedDescription) + } } } diff --git a/Sources/CLI/main/AgentSkillErrorPresentation.swift b/Sources/CLI/main/AgentSkillErrorPresentation.swift new file mode 100644 index 0000000..5f2c8f3 --- /dev/null +++ b/Sources/CLI/main/AgentSkillErrorPresentation.swift @@ -0,0 +1,20 @@ +import Foundation + +extension AgentSkillInstallationError: LocalizedError { + var errorDescription: String? { + switch self { + case .emptyRoot: return "The installation directory must not be empty." + case .unmanagedSkill(let name, let removing): + return "Refusing to \(removing ? "remove" : "overwrite") unmanaged skill '\(name)'." + case .modifiedSkill(let name, let removing): + return removing + ? "Skill '\(name)' was edited. Back up and restore it before uninstalling." + : "Skill '\(name)' differs. Use --force to replace the managed file." + case .invalidReceipt(let url): return "Invalid apple-docs installation receipt at '\(url.path)'." + case .invalidDirectory(let url): + return "Expected a directory, not a symlink or other file, at '\(url.path)'." + case .invalidFile(let url): + return "Expected a regular file, not a symlink or directory, at '\(url.path)'." + } + } +} diff --git a/Sources/CLI/renderer/AgentSkillInstallationRenderer.swift b/Sources/CLI/renderer/AgentSkillInstallationRenderer.swift new file mode 100644 index 0000000..ee6602b --- /dev/null +++ b/Sources/CLI/renderer/AgentSkillInstallationRenderer.swift @@ -0,0 +1,16 @@ +struct AgentSkillInstallationRenderer { + func render(_ outcomes: [AgentSkillInstallationOutcome]) -> String { + outcomes.map { outcome in + switch outcome.status { + case .installed, .updated: + return "Installed: \(outcome.name) at \(outcome.file.path)" + case .wouldInstall, .wouldUpdate: + return "Would install: \(outcome.name) at \(outcome.file.path)" + case .unchanged: return "Unchanged: \(outcome.name)" + case .uninstalled: return "Uninstalled: \(outcome.name)" + case .wouldUninstall: return "Would uninstall: \(outcome.name)" + case .notInstalled: return "Not installed: \(outcome.name)" + } + }.joined(separator: "\n") + } +} diff --git a/Sources/CLI/skills/AgentSkillInstallationOutcome.swift b/Sources/CLI/skills/AgentSkillInstallationOutcome.swift new file mode 100644 index 0000000..0892ccc --- /dev/null +++ b/Sources/CLI/skills/AgentSkillInstallationOutcome.swift @@ -0,0 +1,21 @@ +import Foundation + +struct AgentSkillInstallationOutcome: Equatable, Sendable { + enum Status: Equatable, Sendable { + case installed, updated, unchanged, uninstalled, notInstalled + case wouldInstall, wouldUpdate, wouldUninstall + } + + let name: String + let file: URL + let status: Status +} + +enum AgentSkillInstallationError: Error, Equatable { + case emptyRoot + case unmanagedSkill(name: String, removing: Bool) + case modifiedSkill(name: String, removing: Bool) + case invalidReceipt(URL) + case invalidDirectory(URL) + case invalidFile(URL) +} diff --git a/Sources/CLI/skills/AgentSkillInstallationService.swift b/Sources/CLI/skills/AgentSkillInstallationService.swift index c2c8b6c..536d5fa 100644 --- a/Sources/CLI/skills/AgentSkillInstallationService.swift +++ b/Sources/CLI/skills/AgentSkillInstallationService.swift @@ -1,4 +1,3 @@ -import ArgumentParser import Foundation import Logging @@ -29,7 +28,7 @@ struct DefaultAgentSkillInstallationService { func install( _ skills: [BundledAgentSkill], root: String, dryRun: Bool, force: Bool - ) throws -> String { + ) throws -> [AgentSkillInstallationOutcome] { logger.debug( "Installing agent skills", metadata: [ @@ -44,23 +43,21 @@ struct DefaultAgentSkillInstallationService { logger.warning( "Refusing to overwrite unmanaged skill", metadata: ["skill": .string(installation.skill.name)]) - throw ValidationError("Refusing to overwrite unmanaged skill '\(installation.skill.name)'.") + throw AgentSkillInstallationError.unmanagedSkill(name: installation.skill.name, removing: false) } if content != Data(installation.skill.content.utf8), !force { logger.warning( "Managed skill differs, force required", metadata: ["skill": .string(installation.skill.name)]) - throw ValidationError( - "Skill '\(installation.skill.name)' differs. Use --force to replace the managed file." - ) + throw AgentSkillInstallationError.modifiedSkill(name: installation.skill.name, removing: false) } } } logger.debug( "Skill installation preflight completed", metadata: ["count": .stringConvertible(installations.count)]) - return try installations.map { try install($0, dryRun: dryRun, force: force) }.joined(separator: "\n") + return try installations.map { try install($0, dryRun: dryRun, force: force) } } catch { - if !(error is ValidationError) { + if !(error is AgentSkillInstallationError) { logger.error( "Skill installation failed", metadata: ["error_type": .string(String(reflecting: type(of: error)))]) } @@ -68,7 +65,9 @@ struct DefaultAgentSkillInstallationService { } } - func uninstall(_ skills: [BundledAgentSkill], root: String, dryRun: Bool) throws -> String { + func uninstall( + _ skills: [BundledAgentSkill], root: String, dryRun: Bool + ) throws -> [AgentSkillInstallationOutcome] { logger.debug( "Uninstalling agent skills", metadata: [ @@ -81,22 +80,20 @@ struct DefaultAgentSkillInstallationService { guard let receipt = installation.receipt else { logger.warning( "Refusing to remove unmanaged skill", metadata: ["skill": .string(installation.skill.name)]) - throw ValidationError("Refusing to remove unmanaged skill '\(installation.skill.name)'.") + throw AgentSkillInstallationError.unmanagedSkill(name: installation.skill.name, removing: true) } guard content == receipt.content else { logger.warning( "Refusing to remove edited skill", metadata: ["skill": .string(installation.skill.name)]) - throw ValidationError( - "Skill '\(installation.skill.name)' was edited. Back up and restore it before uninstalling." - ) + throw AgentSkillInstallationError.modifiedSkill(name: installation.skill.name, removing: true) } } } logger.debug( "Skill removal preflight completed", metadata: ["count": .stringConvertible(installations.count)]) - return try installations.map { try uninstall($0, dryRun: dryRun) }.joined(separator: "\n") + return try installations.map { try uninstall($0, dryRun: dryRun) } } catch { - if !(error is ValidationError) { + if !(error is AgentSkillInstallationError) { logger.error( "Skill removal failed", metadata: ["error_type": .string(String(reflecting: type(of: error)))]) } @@ -104,12 +101,15 @@ struct DefaultAgentSkillInstallationService { } } - private func install(_ installation: Installation, dryRun: Bool, force: Bool) throws -> String { + private func install( + _ installation: Installation, dryRun: Bool, force: Bool + ) throws -> AgentSkillInstallationOutcome { let metadata: Logger.Metadata = ["skill": .string(installation.skill.name)] let content = Data(installation.skill.content.utf8) if installation.content == content, installation.receipt?.content == content { logger.debug("Skill installation unchanged", metadata: metadata) - return "Unchanged: \(installation.skill.name)" + return AgentSkillInstallationOutcome( + name: installation.skill.name, file: installation.file, status: .unchanged) } if force, let previousContent = installation.content, previousContent != content { logger.notice( @@ -124,14 +124,18 @@ struct DefaultAgentSkillInstallationService { 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)" + let updating = installation.content != nil + return AgentSkillInstallationOutcome( + name: installation.skill.name, file: installation.file, + status: dryRun ? (updating ? .wouldUpdate : .wouldInstall) : (updating ? .updated : .installed)) } - private func uninstall(_ installation: Installation, dryRun: Bool) throws -> String { + private func uninstall(_ installation: Installation, dryRun: Bool) throws -> AgentSkillInstallationOutcome { let metadata: Logger.Metadata = ["skill": .string(installation.skill.name)] guard installation.receipt != nil else { logger.debug("Skill not installed", metadata: metadata) - return "Not installed: \(installation.skill.name)" + return AgentSkillInstallationOutcome( + name: installation.skill.name, file: installation.file, status: .notInstalled) } if !dryRun { if installation.content != nil { @@ -146,14 +150,15 @@ struct DefaultAgentSkillInstallationService { } } logger.info(dryRun ? "Would uninstall skill" : "Uninstalled skill", metadata: metadata) - return "\(dryRun ? "Would uninstall" : "Uninstalled"): \(installation.skill.name)" + return AgentSkillInstallationOutcome( + name: installation.skill.name, file: installation.file, status: dryRun ? .wouldUninstall : .uninstalled) } private func inspect(_ skills: [BundledAgentSkill], root: String) throws -> [Installation] { logger.trace("Inspecting skill installations", metadata: ["count": .stringConvertible(skills.count)]) guard !root.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { logger.warning("Empty skill installation root") - throw ValidationError("The installation directory must not be empty.") + throw AgentSkillInstallationError.emptyRoot } let rootURL = URL(fileURLWithPath: (root as NSString).expandingTildeInPath).standardizedFileURL return try skills.map { skill in @@ -170,7 +175,7 @@ struct DefaultAgentSkillInstallationService { decoded.name == skill.name else { logger.warning("Invalid skill installation receipt", metadata: ["skill": .string(skill.name)]) - throw ValidationError("Invalid apple-docs installation receipt at '\(receiptFile.path)'.") + throw AgentSkillInstallationError.invalidReceipt(receiptFile) } logger.trace("Validated skill installation receipt", metadata: ["skill": .string(skill.name)]) receipt = decoded @@ -191,7 +196,7 @@ struct DefaultAgentSkillInstallationService { while true { if let type = try fileType(ancestor), type != .typeDirectory { logger.warning("Refusing symlink or non-directory in skill installation path") - throw ValidationError("Expected a directory, not a symlink or other file, at '\(ancestor.path)'.") + throw AgentSkillInstallationError.invalidDirectory(ancestor) } // Ancestors above the user-selected root may be OS aliases such as /var on macOS. if ancestor.path == root.path { return } @@ -206,7 +211,7 @@ struct DefaultAgentSkillInstallationService { logger.warning( "Refusing symlink or non-regular skill installation file", metadata: ["file": .string(url.lastPathComponent)]) - throw ValidationError("Expected a regular file, not a symlink or directory, at '\(url.path)'.") + throw AgentSkillInstallationError.invalidFile(url) } return try fileManager.readSkillData(at: url) } @@ -224,8 +229,10 @@ struct DefaultAgentSkillInstallationService { #if DEBUG protocol AgentSkillInstallationService { - func install(_ skills: [BundledAgentSkill], root: String, dryRun: Bool, force: Bool) throws -> String - func uninstall(_ skills: [BundledAgentSkill], root: String, dryRun: Bool) throws -> String + func install(_ skills: [BundledAgentSkill], root: String, dryRun: Bool, force: Bool) throws + -> [AgentSkillInstallationOutcome] + func uninstall(_ skills: [BundledAgentSkill], root: String, dryRun: Bool) throws + -> [AgentSkillInstallationOutcome] } extension DefaultAgentSkillInstallationService: AgentSkillInstallationService {} diff --git a/Tests/CLITests/skills/AgentSkillInstallationOutcomeTests.swift b/Tests/CLITests/skills/AgentSkillInstallationOutcomeTests.swift new file mode 100644 index 0000000..12b98ab --- /dev/null +++ b/Tests/CLITests/skills/AgentSkillInstallationOutcomeTests.swift @@ -0,0 +1,52 @@ +import Foundation +import Logging +import Testing + +@testable import CLI + +@Suite("Skill installation outcomes") +struct AgentSkillInstallationOutcomeTests { + @Test("returns structured lifecycle outcomes without formatting CLI text") + func returnsLifecycleOutcomes() throws { + // -- Arrange -- + let root = FileManager.default.temporaryDirectory.resolvingSymlinksInPath() + .appendingPathComponent(UUID().uuidString) + defer { try? FileManager.default.removeItem(at: root) } + let skill = try #require(BundledAgentSkills.skill(named: "apple-docs")) + let service = DefaultAgentSkillInstallationService( + logger: Logger(label: "test") { _ in SwiftLogNoOpLogHandler() }) + + // -- Act -- + let preview = try service.install([skill], root: root.path, dryRun: true, force: false) + let rootExistsAfterPreview = FileManager.default.fileExists(atPath: root.path) + let installed = try service.install([skill], root: root.path, dryRun: false, force: false) + let unchanged = try service.install([skill], root: root.path, dryRun: false, force: false) + let removed = try service.uninstall([skill], root: root.path, dryRun: false) + let absent = try service.uninstall([skill], root: root.path, dryRun: false) + + // -- Assert -- + #expect(preview.map(\.status) == [.wouldInstall]) + #expect(installed.map(\.status) == [.installed]) + #expect(installed.map(\.name) == ["apple-docs"]) + #expect(installed.first?.file == root.appendingPathComponent("skills/apple-docs/SKILL.md")) + #expect(unchanged.map(\.status) == [.unchanged]) + #expect(removed.map(\.status) == [.uninstalled]) + #expect(absent.map(\.status) == [.notInstalled]) + #expect(!rootExistsAfterPreview) + #expect(!FileManager.default.fileExists(atPath: root.appendingPathComponent("skills/apple-docs/SKILL.md").path)) + } + + @Test("reports an empty installation root as a domain error") + func rejectsEmptyRoot() throws { + // -- Arrange -- + let skill = try #require(BundledAgentSkills.skill(named: "apple-docs")) + let service = DefaultAgentSkillInstallationService( + logger: Logger(label: "test") { _ in SwiftLogNoOpLogHandler() }) + + // -- Act -- + let install = { try service.install([skill], root: " ", dryRun: false, force: false) } + + // -- Assert -- + #expect(throws: AgentSkillInstallationError.emptyRoot) { try install() } + } +} diff --git a/Tests/CLITests/skills/AgentSkillInstallationServiceTests.swift b/Tests/CLITests/skills/AgentSkillInstallationServiceTests.swift index 0d7a658..b19f962 100644 --- a/Tests/CLITests/skills/AgentSkillInstallationServiceTests.swift +++ b/Tests/CLITests/skills/AgentSkillInstallationServiceTests.swift @@ -1,4 +1,3 @@ -import ArgumentParser import Foundation import Logging import Testing @@ -83,7 +82,7 @@ struct DefaultAgentSkillInstallationServiceTests { try write("private user skill", to: skillFile(root)) // -- Act -- - #expect(throws: ValidationError.self) { + #expect(throws: AgentSkillInstallationError.self) { if uninstall { _ = try installer.uninstall([skill], root: root.path, dryRun: false) } else { @@ -140,7 +139,7 @@ struct DefaultAgentSkillInstallationServiceTests { let previousEventCount = recorder.events.count // -- Act -- - #expect(throws: ValidationError.self) { + #expect(throws: AgentSkillInstallationError.self) { try installer.uninstall([skill], root: root.path, dryRun: false) }