diff --git a/CLAUDE.md b/CLAUDE.md index 50b74c21..e9761f15 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ cargo clippy --workspace --all-targets -- -D warnings ## Product boundary -The native `skilld` CLI searches, installs, lists, views, removes, upgrades, and verifies Skills. +The native `skilld` CLI searches, installs, lists, views, removes, updates, and verifies Skills. It also manages account authentication and Agent target configuration. The skilld CLI contains no Skill generation logic or Agent runtime. diff --git a/GLOSSARY.md b/GLOSSARY.md index 55e20f4f..b08d58f8 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -19,6 +19,7 @@ Every public export, command, error, route, and document uses these terms. | Artifact attestation | `skilld.dev/api/v1` | published protocol | skilld CLI | attestation | | Check result | `skilld.dev/api/v1` | published protocol | skilld CLI, developer | check result | | Source status | lockfile and protocol | published value | skilld CLI, CI | source status | +| Update relation | skilld CLI JSON v1 | published value | Agent, developer, CI | update relation | | Agent target | skilld CLI | published configuration | Agent | Agent target | | Identifier | Term | @@ -28,7 +29,8 @@ Every public export, command, error, route, and document uses these terms. | `skilld list` | installed Skills | | `skilld view` | Skill details | | `skilld remove` | Skill removal | -| `skilld upgrade` | Skill upgrade | +| `skilld update` | Skill update | +| `skilld update --check --json` | update relation check | | `skilld verify` | source verification | | `skilld install skilld --global` | global skilld Skill install | | `skilld auth login` | account login | @@ -94,7 +96,7 @@ None recorded. ### skilld CLI -**Is:** the Rust command line interface that searches, installs, upgrades, and removes Skills. +**Is:** the Rust command line interface that searches, installs, updates, and removes Skills. **Use for:** the command product and its manager logic. @@ -182,6 +184,16 @@ None recorded. **Casing:** `Source status` in headings, `sourceStatus` in identifiers. +### Update relation + +**Is:** the Git relationship between an installed Skill commit and its current source commit. + +**Use for:** `current`, `available`, `behind`, `diverged`, `pinned`, `notTracked`, or `unavailable` JSON values. + +**Never:** upgrade status, version status, release status. + +**Casing:** `Update relation` in headings, `relation` in JSON. + ### Agent target **Is:** an Agent installation destination managed by skilld. diff --git a/README.md b/README.md index bc0f864f..a67dc745 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,10 @@ skilld view vue # Keep Skills current skilld verify vue -skilld upgrade vue +skilld update vue + +# Check update relations for an Agent or CI +skilld update --check --json # Remove a Skill skilld remove vue diff --git a/crates/skilld-command/src/lib.rs b/crates/skilld-command/src/lib.rs index 0f74a879..3df7b2a7 100644 --- a/crates/skilld-command/src/lib.rs +++ b/crates/skilld-command/src/lib.rs @@ -24,11 +24,16 @@ pub use remote::{ RemoteSourceState, SecretValue, SkilldRemote, Sleeper, ThreadSleeper, TokenProvider, }; use skilld_core::{ - AGENT_TARGETS, AgentTargetId, DomainError, GlobalTargetPath, InstallMode, InstallRequest, - InstallScope, InstallSource, LockedSource, VERSION, select_target_ids, + AGENT_TARGETS, AgentTargetId, CommitSha, DomainError, GlobalTargetPath, InstallMode, + InstallRequest, InstallScope, InstallSource, LockedSource, NotTrackedReason, SourceRef, + UpdateCheckV1, UpdateFailure, UpdateLatestCommit, UpdateModelError, UpdatePlan, UpdatePlanItem, + UpdateRelation, VERSION, select_target_ids, }; -use output::{OutputMode, SearchItem, SearchOutcome, render_error, render_search, resolve_mode}; +use output::{ + OutputMode, SearchItem, SearchOutcome, render_error, render_search, render_update_check, + resolve_mode, +}; #[derive(Debug, Parser)] #[command( @@ -81,8 +86,13 @@ enum Command { #[arg(long)] global: bool, }, - /// Upgrade installed Skills. - Upgrade { skill: Option }, + /// Update installed Skills. + Update { + skill: Option, + /// Check update relations without changing files. + #[arg(long)] + check: bool, + }, /// Verify a Skill source. Verify { skill: Option }, /// Manage account authentication. @@ -152,9 +162,15 @@ pub trait Host { )) } - fn upgrade(&self, _name: Option<&str>) -> Result, CommandError> { + fn update(&self, _name: Option<&str>) -> Result, CommandError> { + Err(CommandError::unsupported_host( + "Skill update is unavailable on this host", + )) + } + + fn update_check(&self, _name: Option<&str>) -> Result { Err(CommandError::unsupported_host( - "Skill upgrade is unavailable on this host", + "Skill update checks are unavailable on this host", )) } @@ -297,6 +313,7 @@ pub struct CommandResult { enum CommandOutput { Lines(Vec), Search(SearchOutcome), + UpdateCheck(UpdateCheckV1), } pub fn run(args: I, host: &H, stdout: &mut O, stderr: &mut E) -> CommandResult @@ -374,10 +391,19 @@ where }; let mode = resolve_mode(cli.json, cli.plain, context); - if mode == OutputMode::JsonV1 && !matches!(&cli.command, Command::Search { .. }) { + if matches!(&cli.command, Command::Update { check: true, .. }) && mode != OutputMode::JsonV1 { + let error = CommandError::usage("UNSUPPORTED_OUTPUT", "Skill update checks need --json"); + if stderr.write_all(&render_error(&error, mode)).is_err() { + return CommandResult { exit_code: 2 }; + } + return CommandResult { exit_code: 2 }; + } + let supports_json = matches!(&cli.command, Command::Search { .. }) + || matches!(&cli.command, Command::Update { check: true, .. }); + if mode == OutputMode::JsonV1 && !supports_json { let error = CommandError::usage( "UNSUPPORTED_OUTPUT", - "JSON output is available for Skill search only", + "JSON output is available for Skill search and update checks", ); if stderr.write_all(&render_error(&error, mode)).is_err() { return CommandResult { exit_code: 2 }; @@ -407,6 +433,19 @@ where } } }, + Ok(CommandOutput::UpdateCheck(outcome)) => match render_update_check(&outcome, mode) { + Ok(bytes) => write_success(&bytes, mode, stdout, stderr), + Err(error) => { + if stderr.write_all(&render_error(&error, mode)).is_err() { + return CommandResult { + exit_code: error.exit_code(), + }; + } + CommandResult { + exit_code: error.exit_code(), + } + } + }, Err(error) => { if stderr.write_all(&render_error(&error, mode)).is_err() { return CommandResult { @@ -438,7 +477,7 @@ fn requested_output(args: &[OsString]) -> (bool, bool) { fn display_path(args: &[OsString]) -> String { let commands = [ - "search", "install", "list", "view", "remove", "upgrade", "verify", "auth", "config", + "search", "install", "list", "view", "remove", "update", "verify", "auth", "config", ]; let mut path = vec!["skilld"]; if let Some(command) = args @@ -617,7 +656,14 @@ fn dispatch(command: Command, host: &H) -> Result host.upgrade(skill.as_deref()).map(CommandOutput::Lines), + Command::Update { skill, check } => { + if check { + host.update_check(skill.as_deref()) + .map(CommandOutput::UpdateCheck) + } else { + host.update(skill.as_deref()).map(CommandOutput::Lines) + } + } Command::Verify { skill } => host.verify(skill.as_deref()).map(CommandOutput::Lines), } } @@ -893,6 +939,144 @@ impl LocalHost { }) } + fn update_relation( + &self, + skill: &skilld_core::LockedSkill, + ) -> Result { + let (source, locked_commit_sha) = match &skill.source { + LockedSource::Local { .. } => { + return Ok(UpdateRelation::NotTracked { + reason: NotTrackedReason::Local, + }); + } + LockedSource::BundledSkilld => { + return Ok(UpdateRelation::NotTracked { + reason: NotTrackedReason::Bundled, + }); + } + LockedSource::Remote { + source, commit_sha, .. + } => ( + source, + CommitSha::parse(commit_sha.clone()).map_err(update_model_error)?, + ), + }; + + let selector = match skilld_core::RemoteSelector::parse(source) { + Ok(selector) => selector, + Err(error) => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + error.code, + error.message, + )); + } + }; + if let Some(SourceRef::Commit { value }) = &selector.source().r#ref { + let pinned_commit_sha = match CommitSha::parse(value.clone()) { + Ok(commit_sha) => commit_sha, + Err(error) => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + "INVALID_SOURCE", + error.to_string(), + )); + } + }; + if pinned_commit_sha != locked_commit_sha { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Known { + commit_sha: pinned_commit_sha, + }, + "INVALID_LOCKFILE", + "the locked commit differs from its source selector", + )); + } + return Ok(UpdateRelation::Pinned { + commit_sha: locked_commit_sha, + }); + } + + let artifact_id = match &skill.source_status { + skilld_core::SourceStatus::Verified { artifact_id, .. } => artifact_id, + skilld_core::SourceStatus::Unverified { .. } => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + "UNVERIFIED_SOURCE", + "run an explicit --direct install to update this Skill", + )); + } + skilld_core::SourceStatus::Local { .. } => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + "INVALID_LOCKFILE", + "the remote Skill has a local source status", + )); + } + }; + let provider = match self.remote_provider() { + Ok(provider) => provider, + Err(error) => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + error.code, + error.message, + )); + } + }; + let state = match provider.source_state(&selector, artifact_id, locked_commit_sha.as_str()) + { + Ok(state) => state, + Err(error) => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + error.code, + error.message, + )); + } + }; + match state { + RemoteSourceState::Current => Ok(UpdateRelation::Current { + commit_sha: locked_commit_sha, + }), + RemoteSourceState::Stale { + current_commit_sha, .. + } => { + let latest_commit_sha = match CommitSha::parse(current_commit_sha) { + Ok(commit_sha) => commit_sha, + Err(error) => { + return Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Unknown, + "INVALID_RESPONSE", + error.to_string(), + )); + } + }; + if latest_commit_sha == locked_commit_sha { + return Ok(UpdateRelation::Current { + commit_sha: locked_commit_sha, + }); + } + Ok(unavailable_update( + locked_commit_sha, + UpdateLatestCommit::Known { + commit_sha: latest_commit_sha, + }, + "COMPARISON_UNAVAILABLE", + "skilld.dev does not provide Git comparison data", + )) + } + } + } + fn install_remote( &self, source: &str, @@ -1172,12 +1356,12 @@ impl Host for LocalHost { Ok(lines) } - fn upgrade(&self, requested: Option<&str>) -> Result, CommandError> { + fn update(&self, requested: Option<&str>) -> Result, CommandError> { let scope = InstallScope::Project; let known = self.known_targets(scope)?; let store = self.store(scope); let names = selected_names(&store, &known, requested)?; - let mut upgraded = Vec::new(); + let mut updated = Vec::new(); for name in names { let skill_name = skilld_core::SkillName::parse(name.clone()).map_err(CommandError::domain)?; @@ -1208,7 +1392,7 @@ impl Host for LocalHost { if staged_name != skill_name { return Err(CommandError::operation( "SOURCE_MISMATCH", - format!("the upgraded Skill name changed from {name}"), + format!("the updated Skill name changed from {name}"), )); } let targets = view @@ -1240,9 +1424,27 @@ impl Host for LocalHost { &known, ) .map_err(CommandError::store)?; - upgraded.push(format!("Upgraded Skill {name}.")); + updated.push(format!("Updated Skill {name}.")); } - Ok(upgraded) + Ok(updated) + } + + fn update_check(&self, requested: Option<&str>) -> Result { + let scope = InstallScope::Project; + let known = self.known_targets(scope)?; + let store = self.store(scope); + let names = selected_names(&store, &known, requested)?; + let mut items = Vec::with_capacity(names.len()); + for name in names { + let skill_name = skilld_core::SkillName::parse(name).map_err(CommandError::domain)?; + let view = store + .view(&skill_name, &known) + .map_err(CommandError::store)?; + let relation = self.update_relation(&view.skill)?; + items.push(UpdatePlanItem::new(skill_name, relation)); + } + let plan = UpdatePlan::new(items).map_err(update_model_error)?; + Ok(UpdateCheckV1::new(plan)) } } @@ -1312,6 +1514,23 @@ fn selected_names( } } +fn unavailable_update( + locked_commit_sha: CommitSha, + latest_commit: UpdateLatestCommit, + code: impl Into, + message: impl Into, +) -> UpdateRelation { + UpdateRelation::Unavailable { + locked_commit_sha, + latest_commit, + failure: UpdateFailure::new(code, message), + } +} + +fn update_model_error(error: UpdateModelError) -> CommandError { + CommandError::operation("INVALID_LOCKFILE", error.to_string()) +} + fn detects_environment(agent: AgentTargetId, environment: &DetectionEnvironment) -> bool { match agent { AgentTargetId::ClaudeCode => [ @@ -1469,12 +1688,31 @@ mod tests { assert_eq!( command_names(), [ - "search", "install", "list", "view", "remove", "upgrade", "verify", "auth", - "config" + "search", "install", "list", "view", "remove", "update", "verify", "auth", "config" ] ); } + #[test] + fn upgrade_is_not_a_command_alias() { + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let result = run( + ["skilld", "upgrade"], + &RecordingHost, + &mut stdout, + &mut stderr, + ); + + assert_eq!(result.exit_code, 2); + assert!(stdout.is_empty()); + assert!( + String::from_utf8(stderr) + .unwrap() + .contains("unrecognized subcommand 'upgrade'") + ); + } + #[test] fn global_skilld_install_uses_the_target_contract() { let mut stdout = Vec::new(); diff --git a/crates/skilld-command/src/output.rs b/crates/skilld-command/src/output.rs index 0b2559ad..767b27ec 100644 --- a/crates/skilld-command/src/output.rs +++ b/crates/skilld-command/src/output.rs @@ -1,5 +1,6 @@ use clap::error::ErrorKind; use serde::Serialize; +use skilld_core::UpdateCheckV1; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use crate::{CommandError, CommandErrorKind}; @@ -76,22 +77,15 @@ pub(crate) fn render_search( match mode { OutputMode::Human { width, color } => Ok(render_human(outcome, width, color).into_bytes()), OutputMode::Plain => Ok(render_plain(outcome).into_bytes()), - OutputMode::JsonV1 => serde_json::to_vec(&JsonSuccess { - schema_version: JSON_SCHEMA_VERSION, - tag: "Success", - command: "search", - data: JsonSearchData { + OutputMode::JsonV1 => render_json_success( + "search", + JsonSearchData { query: &outcome.query, items: &outcome.items, total: outcome.total, }, - notices: Vec::new(), - }) - .map(|mut bytes| { - bytes.push(b'\n'); - bytes - }) - .map_err(|_| CommandError::service("Skill search output could not be encoded")), + "Skill search output could not be encoded", + ), } } @@ -107,6 +101,31 @@ pub(crate) fn render_display(kind: ErrorKind, path: &str, text: &str) -> Vec } else { ("help", JsonDisplayData::Help { path, text }) }; + render_json_success(command, data, "display output could not be encoded") + .unwrap_or_else(|_| b"OUTPUT_RENDER_FAILED: display output could not be encoded\n".to_vec()) +} + +pub(crate) fn render_update_check( + outcome: &UpdateCheckV1, + mode: OutputMode, +) -> Result, CommandError> { + if mode != OutputMode::JsonV1 { + return Err(CommandError::service( + "Skill update check output needs JSON mode", + )); + } + render_json_success( + "update", + outcome, + "Skill update check output could not be encoded", + ) +} + +fn render_json_success( + command: &'static str, + data: T, + encoding_error: &'static str, +) -> Result, CommandError> { serde_json::to_vec(&JsonSuccess { schema_version: JSON_SCHEMA_VERSION, tag: "Success", @@ -118,7 +137,7 @@ pub(crate) fn render_display(kind: ErrorKind, path: &str, text: &str) -> Vec bytes.push(b'\n'); bytes }) - .unwrap_or_else(|_| b"OUTPUT_RENDER_FAILED: display output could not be encoded\n".to_vec()) + .map_err(|_| CommandError::service(encoding_error)) } pub(crate) fn render_error(error: &CommandError, mode: OutputMode) -> Vec { diff --git a/crates/skilld-command/tests/remote.rs b/crates/skilld-command/tests/remote.rs index 73a7abec..ffed92b9 100644 --- a/crates/skilld-command/tests/remote.rs +++ b/crates/skilld-command/tests/remote.rs @@ -18,6 +18,7 @@ use skilld_core::{ CheckResult, InstallMode, InstallRequest, InstallScope, InstallSource, LockedSource, PreparedFile, RemoteError, RemoteSelector, RepositoryVisibility, ResolvedSource, SearchResponse, SignatureAlgorithm, SourceProvider, SourceStatus, TrustedRootPin, + UpdateCheckV1, UpdateLatestCommit, UpdateRelation, }; const ROOT_DOMAIN: &[u8] = b"skilld-trusted-key-v1\0"; @@ -843,7 +844,7 @@ fn verify_reports_changed_bytes_and_stale_sources() { } #[test] -fn remote_install_verify_and_failed_upgrade_use_the_normal_transaction() { +fn remote_install_verify_and_failed_update_use_the_normal_transaction() { let temporary = tempfile::tempdir().unwrap(); let project = temporary.path().join("project"); let data = temporary.path().join("data"); @@ -868,7 +869,7 @@ fn remote_install_verify_and_failed_upgrade_use_the_normal_transaction() { *provider.content.lock().unwrap() = b"---\nname: example\ndescription: second\n---\n".to_vec(); *provider.fail_prepare.lock().unwrap() = true; - let error = host.upgrade(Some("example")).unwrap_err(); + let error = host.update(Some("example")).unwrap_err(); assert_eq!(error.code, "CHECK_BLOCKED"); assert_eq!( @@ -877,6 +878,64 @@ fn remote_install_verify_and_failed_upgrade_use_the_normal_transaction() { ); } +#[test] +fn update_check_keeps_an_uncompared_commit_unavailable() { + let temporary = tempfile::tempdir().unwrap(); + let project = temporary.path().join("project"); + fs::create_dir_all(&project).unwrap(); + let provider = provider("---\nname: example\ndescription: first\n---\n"); + let host = LocalHost::new(project, temporary.path().join("data")) + .with_remote_provider(provider.clone()); + host.install_request(InstallRequest { + source: Some(InstallSource::Remote( + "skilld:skilld-dev/skills/example".to_owned(), + )), + scope: InstallScope::Project, + targets: vec![AgentTargetId::Codex], + mode: Some(InstallMode::Copy), + }) + .unwrap(); + *provider.stale.lock().unwrap() = true; + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + let result = run( + ["skilld", "update", "example", "--check", "--json"], + &host, + &mut stdout, + &mut stderr, + ); + let mut global_stdout = Vec::new(); + let mut global_stderr = Vec::new(); + let global_result = run( + ["skilld", "--json", "update", "example", "--check"], + &host, + &mut global_stdout, + &mut global_stderr, + ); + let outcome: serde_json::Value = serde_json::from_slice(&stdout).unwrap(); + let check: UpdateCheckV1 = serde_json::from_value(outcome["data"].clone()).unwrap(); + + assert_eq!(result.exit_code, 0); + assert!(stderr.is_empty()); + assert_eq!(global_result.exit_code, 0); + assert!(global_stderr.is_empty()); + assert_eq!(global_stdout, stdout); + assert_eq!(outcome["schemaVersion"], 1); + assert_eq!(outcome["_tag"], "Success"); + assert_eq!(outcome["command"], "update"); + assert_eq!(outcome["notices"], serde_json::json!([])); + assert!(matches!( + check.items()[0].relation(), + UpdateRelation::Unavailable { + latest_commit: UpdateLatestCommit::Known { commit_sha }, + failure, + .. + } if commit_sha.as_str() == "ffffffffffffffffffffffffffffffffffffffff" + && failure.code == "COMPARISON_UNAVAILABLE" + )); +} + #[test] fn cli_direct_install_marks_review_as_required() { let temporary = tempfile::tempdir().unwrap(); diff --git a/crates/skilld-core/src/lib.rs b/crates/skilld-core/src/lib.rs index ebaf30cd..ef8f7aea 100644 --- a/crates/skilld-core/src/lib.rs +++ b/crates/skilld-core/src/lib.rs @@ -1,6 +1,7 @@ mod lock; mod remote; mod target; +mod update; use std::fmt; use std::path::{Path, PathBuf}; @@ -20,6 +21,10 @@ use serde::{Deserialize, Serialize}; pub use target::{ AGENT_TARGETS, AgentTarget, AgentTargetId, GlobalTargetPath, TargetSelection, select_target_ids, }; +pub use update::{ + CommitSha, NotTrackedReason, UpdateCheckV1, UpdateFailure, UpdateLatestCommit, + UpdateModelError, UpdatePlan, UpdatePlanItem, UpdateRelation, classify_update_comparison, +}; pub const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -48,7 +53,8 @@ impl InstallMode { } } -#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Debug, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize)] +#[serde(try_from = "String", into = "String")] pub struct SkillName(String); impl SkillName { @@ -91,6 +97,20 @@ impl fmt::Display for SkillName { } } +impl TryFrom for SkillName { + type Error = DomainError; + + fn try_from(value: String) -> Result { + Self::parse(value) + } +} + +impl From for String { + fn from(value: SkillName) -> Self { + value.0 + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum InstallScope { Project, diff --git a/crates/skilld-core/src/update.rs b/crates/skilld-core/src/update.rs new file mode 100644 index 00000000..1c8609d1 --- /dev/null +++ b/crates/skilld-core/src/update.rs @@ -0,0 +1,247 @@ +use std::fmt; +use std::num::NonZeroU64; + +use serde::{Deserialize, Deserializer, Serialize}; + +use crate::SkillName; + +#[derive(Clone, Debug, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize)] +#[serde(try_from = "String", into = "String")] +pub struct CommitSha(String); + +impl CommitSha { + pub fn parse(value: impl Into) -> Result { + let value = value.into(); + let valid = value.len() == 40 + && value + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)); + valid + .then_some(Self(value.clone())) + .ok_or(UpdateModelError::InvalidCommitSha(value)) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl TryFrom for CommitSha { + type Error = UpdateModelError; + + fn try_from(value: String) -> Result { + Self::parse(value) + } +} + +impl From for String { + fn from(value: CommitSha) -> Self { + value.0 + } +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct UpdateFailure { + pub code: String, + pub message: String, +} + +impl UpdateFailure { + pub fn new(code: impl Into, message: impl Into) -> Self { + Self { + code: code.into(), + message: message.into(), + } + } +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde( + deny_unknown_fields, + tag = "_tag", + rename_all = "camelCase", + rename_all_fields = "camelCase" +)] +pub enum UpdateLatestCommit { + Known { commit_sha: CommitSha }, + Unknown, +} + +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub enum NotTrackedReason { + Local, + Bundled, +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde( + deny_unknown_fields, + tag = "_tag", + rename_all = "camelCase", + rename_all_fields = "camelCase" +)] +pub enum UpdateRelation { + Current { + commit_sha: CommitSha, + }, + Available { + locked_commit_sha: CommitSha, + latest_commit_sha: CommitSha, + ahead_by: NonZeroU64, + }, + Behind { + locked_commit_sha: CommitSha, + latest_commit_sha: CommitSha, + behind_by: NonZeroU64, + }, + Diverged { + locked_commit_sha: CommitSha, + latest_commit_sha: CommitSha, + ahead_by: NonZeroU64, + behind_by: NonZeroU64, + }, + Pinned { + commit_sha: CommitSha, + }, + NotTracked { + reason: NotTrackedReason, + }, + Unavailable { + locked_commit_sha: CommitSha, + latest_commit: UpdateLatestCommit, + failure: UpdateFailure, + }, +} + +pub fn classify_update_comparison( + locked_commit_sha: CommitSha, + latest_commit_sha: CommitSha, + ahead_by: u64, + behind_by: u64, +) -> Result { + match (locked_commit_sha == latest_commit_sha, ahead_by, behind_by) { + (true, 0, 0) => Ok(UpdateRelation::Current { + commit_sha: locked_commit_sha, + }), + (false, ahead_by, 0) if ahead_by > 0 => Ok(UpdateRelation::Available { + locked_commit_sha, + latest_commit_sha, + ahead_by: NonZeroU64::new(ahead_by).expect("ahead count is non-zero"), + }), + (false, 0, behind_by) if behind_by > 0 => Ok(UpdateRelation::Behind { + locked_commit_sha, + latest_commit_sha, + behind_by: NonZeroU64::new(behind_by).expect("behind count is non-zero"), + }), + (false, ahead_by, behind_by) if ahead_by > 0 && behind_by > 0 => { + Ok(UpdateRelation::Diverged { + locked_commit_sha, + latest_commit_sha, + ahead_by: NonZeroU64::new(ahead_by).expect("ahead count is non-zero"), + behind_by: NonZeroU64::new(behind_by).expect("behind count is non-zero"), + }) + } + _ => Err(UpdateModelError::InvalidComparison), + } +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct UpdatePlanItem { + name: SkillName, + relation: UpdateRelation, +} + +impl UpdatePlanItem { + pub fn new(name: SkillName, relation: UpdateRelation) -> Self { + Self { name, relation } + } + + pub const fn name(&self) -> &SkillName { + &self.name + } + + pub const fn relation(&self) -> &UpdateRelation { + &self.relation + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct UpdatePlan { + items: Vec, +} + +impl UpdatePlan { + pub fn new(mut items: Vec) -> Result { + items.sort_by(|left, right| left.name.cmp(&right.name)); + if let Some(duplicate) = items + .windows(2) + .find(|pair| pair[0].name == pair[1].name) + .map(|pair| pair[0].name.to_string()) + { + return Err(UpdateModelError::DuplicateSkill(duplicate)); + } + Ok(Self { items }) + } + + pub fn items(&self) -> &[UpdatePlanItem] { + &self.items + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct UpdateCheckV1 { + items: Vec, +} + +impl UpdateCheckV1 { + pub fn new(plan: UpdatePlan) -> Self { + Self { items: plan.items } + } + + pub fn items(&self) -> &[UpdatePlanItem] { + &self.items + } +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +struct UpdateCheckV1Wire { + items: Vec, +} + +impl<'de> Deserialize<'de> for UpdateCheckV1 { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let wire = UpdateCheckV1Wire::deserialize(deserializer)?; + UpdatePlan::new(wire.items) + .map(Self::new) + .map_err(serde::de::Error::custom) + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum UpdateModelError { + DuplicateSkill(String), + InvalidCommitSha(String), + InvalidComparison, +} + +impl fmt::Display for UpdateModelError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::DuplicateSkill(name) => { + write!(formatter, "duplicate Skill in update plan: {name}") + } + Self::InvalidCommitSha(_) => formatter.write_str("invalid Git commit in update plan"), + Self::InvalidComparison => formatter.write_str("invalid Git update comparison"), + } + } +} + +impl std::error::Error for UpdateModelError {} diff --git a/crates/skilld-core/tests/update.rs b/crates/skilld-core/tests/update.rs new file mode 100644 index 00000000..d75ca05e --- /dev/null +++ b/crates/skilld-core/tests/update.rs @@ -0,0 +1,122 @@ +use std::num::NonZeroU64; + +use skilld_core::{ + CommitSha, SkillName, UpdateCheckV1, UpdateLatestCommit, UpdateModelError, UpdatePlan, + UpdatePlanItem, UpdateRelation, classify_update_comparison, +}; + +fn sha(value: char) -> CommitSha { + CommitSha::parse(value.to_string().repeat(40)).unwrap() +} + +#[test] +fn comparison_counts_classify_every_git_relation() { + let locked = sha('1'); + let latest = sha('2'); + + assert_eq!( + classify_update_comparison(locked.clone(), locked.clone(), 0, 0).unwrap(), + UpdateRelation::Current { + commit_sha: locked.clone(), + } + ); + assert_eq!( + classify_update_comparison(locked.clone(), latest.clone(), 3, 0).unwrap(), + UpdateRelation::Available { + locked_commit_sha: locked.clone(), + latest_commit_sha: latest.clone(), + ahead_by: NonZeroU64::new(3).unwrap(), + } + ); + assert_eq!( + classify_update_comparison(locked.clone(), latest.clone(), 0, 2).unwrap(), + UpdateRelation::Behind { + locked_commit_sha: locked.clone(), + latest_commit_sha: latest.clone(), + behind_by: NonZeroU64::new(2).unwrap(), + } + ); + assert_eq!( + classify_update_comparison(locked.clone(), latest.clone(), 4, 2).unwrap(), + UpdateRelation::Diverged { + locked_commit_sha: locked, + latest_commit_sha: latest, + ahead_by: NonZeroU64::new(4).unwrap(), + behind_by: NonZeroU64::new(2).unwrap(), + } + ); +} + +#[test] +fn impossible_comparison_counts_are_rejected() { + assert_eq!( + classify_update_comparison(sha('1'), sha('2'), 0, 0), + Err(UpdateModelError::InvalidComparison) + ); + assert_eq!( + classify_update_comparison(sha('1'), sha('1'), 1, 0), + Err(UpdateModelError::InvalidComparison) + ); +} + +#[test] +fn update_plan_sorts_skills_and_rejects_duplicates() { + let first = UpdatePlanItem::new( + SkillName::parse("zeta").unwrap(), + UpdateRelation::Pinned { + commit_sha: sha('1'), + }, + ); + let second = UpdatePlanItem::new( + SkillName::parse("alpha").unwrap(), + UpdateRelation::Current { + commit_sha: sha('2'), + }, + ); + let plan = UpdatePlan::new(vec![first, second]).unwrap(); + + assert_eq!( + plan.items() + .iter() + .map(|item| item.name().as_str()) + .collect::>(), + ["alpha", "zeta"] + ); + assert_eq!( + UpdatePlan::new(vec![ + UpdatePlanItem::new( + SkillName::parse("alpha").unwrap(), + UpdateRelation::Pinned { + commit_sha: sha('1'), + }, + ), + UpdatePlanItem::new( + SkillName::parse("alpha").unwrap(), + UpdateRelation::Current { + commit_sha: sha('2'), + }, + ), + ]), + Err(UpdateModelError::DuplicateSkill("alpha".to_owned())) + ); +} + +#[test] +fn v1_check_fixture_covers_the_published_relations() { + let bytes = include_bytes!("../../../tests/fixtures/v3-rust/v1/update-check.json"); + let fixture: serde_json::Value = serde_json::from_slice(bytes).unwrap(); + let check: UpdateCheckV1 = serde_json::from_value(fixture["data"].clone()).unwrap(); + + assert_eq!(fixture["schemaVersion"], 1); + assert_eq!(fixture["_tag"], "Success"); + assert_eq!(fixture["command"], "update"); + assert_eq!(check.items().len(), 7); + assert!(matches!( + check.items()[6].relation(), + UpdateRelation::Unavailable { + latest_commit: UpdateLatestCommit::Known { .. }, + .. + } + )); + assert_eq!(serde_json::to_value(check).unwrap(), fixture["data"]); +} diff --git a/docs/migrate-v2-to-v3.md b/docs/migrate-v2-to-v3.md index 98af919d..edc036b5 100644 --- a/docs/migrate-v2-to-v3.md +++ b/docs/migrate-v2-to-v3.md @@ -111,7 +111,7 @@ It cannot restore a v2 lockfile. | v2 command | v3 replacement | | --- | --- | | `skilld add ` | Run `skilld search`, then `skilld install ` | -| `skilld update [name]` | `skilld upgrade [name]` | +| `skilld update [name]` | `skilld update [name]` | | `skilld info` | `skilld list`, then `skilld view ` | | `skilld login` | `skilld auth login` | | `skilld whoami` | `skilld auth status` | diff --git a/skills/skilld/SKILL.md b/skills/skilld/SKILL.md index 5a9351e8..2022b177 100644 --- a/skills/skilld/SKILL.md +++ b/skills/skilld/SKILL.md @@ -1,6 +1,6 @@ --- name: skilld -description: Search, view, install, upgrade, verify, and remove Skills with skilld CLI, including private repository access. +description: Search, view, install, update, verify, and remove Skills with skilld CLI, including private repository access. --- # Use skilld CLI @@ -64,11 +64,18 @@ Use `view` to show one Skill's path, source status, and Agent targets. ## Maintain installed Skills ```sh -skilld upgrade +skilld update +skilld update --check --json skilld verify skilld remove ``` -Use `upgrade` to install a newer Artifact. +Use `update --check --json` to inspect update relations without changing files. +Read each `data.items[].relation._tag` before changing files. +Use `update ` only when the relation is `available`. +Treat `current`, `pinned`, and `notTracked` as no action. +If the relation is `behind` or `diverged`, ask before changing files. +If the relation is `unavailable`, report `failure.code` and `failure.message`. +Treat `unavailable` as unknown. Do not infer a newer commit. Use `verify` to check the installed bytes and source status. Use `remove` only when the request names the Skill to remove. diff --git a/tests/fixtures/v3-rust/v1/update-check.json b/tests/fixtures/v3-rust/v1/update-check.json new file mode 100644 index 00000000..3e27f108 --- /dev/null +++ b/tests/fixtures/v3-rust/v1/update-check.json @@ -0,0 +1,74 @@ +{ + "schemaVersion": 1, + "_tag": "Success", + "command": "update", + "data": { + "items": [ + { + "name": "available-skill", + "relation": { + "_tag": "available", + "lockedCommitSha": "1111111111111111111111111111111111111111", + "latestCommitSha": "2222222222222222222222222222222222222222", + "aheadBy": 3 + } + }, + { + "name": "behind-skill", + "relation": { + "_tag": "behind", + "lockedCommitSha": "2222222222222222222222222222222222222222", + "latestCommitSha": "1111111111111111111111111111111111111111", + "behindBy": 2 + } + }, + { + "name": "current-skill", + "relation": { + "_tag": "current", + "commitSha": "1111111111111111111111111111111111111111" + } + }, + { + "name": "diverged-skill", + "relation": { + "_tag": "diverged", + "lockedCommitSha": "1111111111111111111111111111111111111111", + "latestCommitSha": "2222222222222222222222222222222222222222", + "aheadBy": 4, + "behindBy": 2 + } + }, + { + "name": "local-skill", + "relation": { + "_tag": "notTracked", + "reason": "local" + } + }, + { + "name": "pinned-skill", + "relation": { + "_tag": "pinned", + "commitSha": "1111111111111111111111111111111111111111" + } + }, + { + "name": "unavailable-skill", + "relation": { + "_tag": "unavailable", + "lockedCommitSha": "1111111111111111111111111111111111111111", + "latestCommit": { + "_tag": "known", + "commitSha": "2222222222222222222222222222222222222222" + }, + "failure": { + "code": "COMPARISON_UNAVAILABLE", + "message": "Git comparison is unavailable." + } + } + } + ] + }, + "notices": [] +}