Skip to content
Merged
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
14 changes: 9 additions & 5 deletions Sources/CLI/cmd/agent/skills/AgentSkillsInstallCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
14 changes: 9 additions & 5 deletions Sources/CLI/cmd/agent/skills/AgentSkillsUninstallCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
20 changes: 20 additions & 0 deletions Sources/CLI/main/AgentSkillErrorPresentation.swift
Original file line number Diff line number Diff line change
@@ -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)'."
}
}
}
16 changes: 16 additions & 0 deletions Sources/CLI/renderer/AgentSkillInstallationRenderer.swift
Original file line number Diff line number Diff line change
@@ -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")
}
}
21 changes: 21 additions & 0 deletions Sources/CLI/skills/AgentSkillInstallationOutcome.swift
Original file line number Diff line number Diff line change
@@ -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)
}
61 changes: 34 additions & 27 deletions Sources/CLI/skills/AgentSkillInstallationService.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import ArgumentParser
import Foundation
import Logging

Expand Down Expand Up @@ -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: [
Expand All @@ -44,31 +43,31 @@ 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)))])
}
throw error
}
}

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: [
Expand All @@ -81,35 +80,36 @@ 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)))])
}
throw error
}
}

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(
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 }
Expand All @@ -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)
}
Expand All @@ -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 {}
Expand Down
52 changes: 52 additions & 0 deletions Tests/CLITests/skills/AgentSkillInstallationOutcomeTests.swift
Original file line number Diff line number Diff line change
@@ -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() }
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import ArgumentParser
import Foundation
import Logging
import Testing
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
Loading