diff --git a/GLOSSARY.md b/GLOSSARY.md index 72fffd4c..b6bc4b2b 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -22,6 +22,9 @@ Every public export, command, error, route, and document uses these terms. | 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 | +| Curator | skilld.dev | published route | skilld.dev, skilld CLI | curator | +| Collection | skilld.dev | published route | skilld.dev, skilld CLI | collection | +| Multi-skill ref | `skilld run`, `skilld add` | published argument | developer, Agent | ref | | Identifier | Term | | --- | --- | @@ -29,6 +32,8 @@ Every public export, command, error, route, and document uses these terms. | `skilld run` | transient Skill load | | `skilld run --file` | supporting file read | | `skilld install` | Skill install | +| `skilld add` | multi-skill install | +| `skilld run gh:OWNER/REPOSITORY` | Skill index | | `skilld list` | installed Skills | | `skilld view` | Skill details | | `skilld remove` | Skill removal | @@ -222,6 +227,36 @@ The Rust type for the second is `TransientSkill`, never `SkillRun`. **Casing:** `Agent target` in prose, `AgentTarget` in types. +### Curator + +**Is:** a skilld.dev account that publishes collections at `/@LOGIN`. + +**Use for:** the `@LOGIN` ref and curator profile pages. + +**Never:** author, publisher, maintainer in this sense. + +**Casing:** `Curator` in headings, `curator` in sentences. + +### Collection + +**Is:** a curator's ordered, named list of Skills at `/@LOGIN/SLUG`. + +**Use for:** the `@LOGIN/SLUG` ref and collection pages. + +**Never:** pack, bundle, list, set. + +**Casing:** `Collection` in headings, `collection` in sentences. + +### Multi-skill ref + +**Is:** one `skilld run` or `skilld add` argument that names more than one Skill: `gh:OWNER/REPOSITORY`, `@LOGIN`, or `@LOGIN/SLUG`. + +**Use for:** the argument grammar and its index output. + +**Never:** bulk selector, group source, target. + +**Casing:** `multi-skill ref` in prose, `MultiSkillRef` in Rust. + ### Outdated Skill report **Is:** the per Skill status produced by `skilld outdated`. diff --git a/README.md b/README.md index ad1ee879..f959ee5c 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,14 @@ skilld run skilld:skilld-dev/skills/vue --revision --file references/ap # Install a Skill in the current project skilld install skilld:skilld-dev/skills/vue +# List every Skill a Repository, curator, or collection names +skilld run gh:anthropics/skills +skilld run @harlan-zw +skilld run @harlan-zw/agent-workflow-stack + +# Install every Skill one of those refs names +skilld add @harlan-zw/agent-workflow-stack + # Inspect installed Skills skilld list skilld view vue @@ -114,6 +122,10 @@ Use `--mode copy` or `--mode symlink` to control target writes. Run `skilld install` without a source to restore the lockfile state. +`skilld run` with a Repository, curator, or collection ref prints an index. +The index has one line per Skill with its run command. It loads no Skill. +`skilld add` installs every Skill the same ref names. It accepts the `skilld install` flags. + ## Artifact delivery The skilld CLI resolves remote Skills through the skilld.dev API. diff --git a/crates/skilld-command/src/lib.rs b/crates/skilld-command/src/lib.rs index c2e1b083..9797c8c7 100644 --- a/crates/skilld-command/src/lib.rs +++ b/crates/skilld-command/src/lib.rs @@ -34,9 +34,10 @@ pub use run::{ use skilld_core::{ AGENT_TARGETS, AgentTargetId, CommitHistory, CommitSha, DomainError, GlobalTargetPath, InstallMode, InstallOperation, InstallRequest, InstallScope, InstallSource, LockedSource, - NotTrackedReason, RemoteSelector, SourceRef, UpdateFailure, UpdateLatestCommit, - UpdateModelError, UpdatePlan, UpdatePlanItem, UpdatePlanV1, UpdateRelation, UpdateRetryAfter, - VERSION, classify_update_comparison, parse_agent_targets, select_target_ids, + MultiSkillRef, NotTrackedReason, RemoteSelector, SkillListing, SkillRef, SourceRef, + UpdateFailure, UpdateLatestCommit, UpdateModelError, UpdatePlan, UpdatePlanItem, UpdatePlanV1, + UpdateRelation, UpdateRetryAfter, VERSION, classify_update_comparison, parse_agent_targets, + select_target_ids, }; use skilld_ui::text::is_unsafe_terminal; use skilld_ui::{Detail, Line, Marker, Screen}; @@ -103,13 +104,45 @@ enum Command { )] direct: bool, }, + /// Install every Skill a Repository, curator, or collection names. + #[command( + long_about = "Install every Skill a Repository, curator, or collection names.\n\nGive REF as:\n gh:OWNER/REPOSITORY\n Install every Skill the Repository carries.\n @LOGIN\n Install every Skill the curator's collections name.\n @LOGIN/SLUG\n Install every Skill one collection names.\n Any SOURCE skilld install accepts\n Install that one Skill.\n\nEach Skill installs through the same hosted Artifact path skilld install uses.\nRun skilld run REF first to see the Skills a ref names.", + after_long_help = "Examples:\n npx skilld add gh:skilld-dev/skills\n npx skilld add @harlan-zw/nuxt --agent codex\n npx skilld add skilld:skilld-dev/skills/vue --global" + )] + Add { + /// The Repository, curator, collection, or Skill source to install. + #[arg(value_name = "REF")] + reference: String, + #[arg( + long, + long_help = "Install to your account-level Agent targets. The default is the current project." + )] + global: bool, + #[arg( + long = "agent", + value_name = "AGENT", + long_help = "Select an Agent target. Repeat --agent to select several.\nDefault: every Agent target skilld detects. If skilld detects none, it uses agent.targets." + )] + agents: Vec, + #[arg( + long, + value_name = "MODE", + long_help = "Choose how each Agent target receives the Skill.\nValues: copy, symlink. The default comes from install.mode." + )] + mode: Option, + #[arg( + long, + long_help = "Fetch a public GitHub Repository without going through skilld.dev.\nOnly one Skill source accepts --direct. A direct install records the unverified source status." + )] + direct: bool, + }, /// Load a Skill for this session without installing it. #[command( - long_about = "Load a Skill for this session without installing it.\n\nskilld run prints SKILL.md so the calling Agent follows it now.\nA remote run retains no Skill files. It creates no lockfile entry, Agent target,\nor project file.\n\nskilld names the supporting files and prints none of them.\nUse --file to read one. Remote file reads also require the returned --revision.\nUse skilld install to put supporting files on disk.\n\nGive SOURCE in the same forms skilld install accepts.", - after_long_help = "Examples:\n npx skilld run skilld:skilld-dev/skills/find-skill\n skilld run ./skills/my-skill --file references/api.md\n skilld run github:skilld-dev/skilld/skills/skilld --direct\n skilld run ./skills/my-skill" + long_about = "Load a Skill for this session without installing it.\n\nskilld run prints SKILL.md so the calling Agent follows it now.\nA remote run retains no Skill files. It creates no lockfile entry, Agent target,\nor project file.\n\nskilld names the supporting files and prints none of them.\nUse --file to read one. Remote file reads also require the returned --revision.\nUse skilld install to put supporting files on disk.\n\nGive SOURCE in the same forms skilld install accepts.\n\nGive a ref that names several Skills to list them instead:\n gh:OWNER/REPOSITORY, @LOGIN, or @LOGIN/SLUG\nskilld prints one line per Skill with its run command and loads none of them.", + after_long_help = "Examples:\n npx skilld run skilld:skilld-dev/skills/find-skill\n npx skilld run gh:skilld-dev/skills\n npx skilld run @harlan-zw/nuxt\n skilld run ./skills/my-skill --file references/api.md\n skilld run github:skilld-dev/skilld/skills/skilld --direct\n skilld run ./skills/my-skill" )] Run { - /// The Skill source to load. + /// The Skill source to load, or a ref that names several Skills. #[arg(value_name = "SOURCE")] source: String, #[arg( @@ -231,6 +264,12 @@ pub trait Host { )) } + fn list_skills(&self, _reference: &MultiSkillRef) -> Result { + Err(CommandError::unsupported_host( + "Skill listings are unavailable on this host", + )) + } + fn view(&self, _name: &str, _scope: InstallScope) -> Result { Err(CommandError::unsupported_host( "Skill details are unavailable on this host", @@ -391,6 +430,15 @@ impl CommandError { ) } + fn direct_multi_skill_ref(reference: &MultiSkillRef) -> Self { + Self::usage( + "DIRECT_SOURCE_REQUIRED", + format!( + "--direct needs one Skill source. {reference} names several Skills. Remove --direct, then run the same command again." + ), + ) + } + fn remote_file_revision() -> Self { Self::input( "Remote --file reads require --revision. Run the Skill without --file first. Then repeat this run with the returned revision.", @@ -712,7 +760,8 @@ fn requested_output(args: &[OsString]) -> (bool, bool) { fn display_path(args: &[OsString]) -> String { let commands = [ - "search", "install", "run", "list", "view", "remove", "update", "verify", "auth", "config", + "search", "install", "add", "run", "list", "view", "remove", "update", "verify", "auth", + "config", ]; let mut path = vec!["skilld"]; if let Some(command) = args @@ -785,60 +834,60 @@ fn dispatch( platform: CommandPlatform, ) -> Result { match command { - Command::Install { - source, + Command::Add { + reference, global, agents, mode, direct, } => { - let scope = scope(global); - let operation = match source { - Some(source) => match (direct, InstallSource::parse(&source)) { - (true, InstallSource::Remote(source)) => { - InstallOperation::Install(InstallSource::DirectRemote(source)) - } - (true, InstallSource::DirectRemote(source)) => { - InstallOperation::Install(InstallSource::DirectRemote(source)) - } - (true, InstallSource::Local(_)) => { - return Err(CommandError::direct_local_install_source()); - } - (true, InstallSource::BundledSkilld) => { - return Err(CommandError::direct_bundled_install_source()); + let reference = SkillRef::parse(&reference).map_err(CommandError::remote)?; + let options = InstallOptions::parse(global, &agents, mode.as_deref())?; + match reference { + SkillRef::Skill(source) => install(host, Some(source), options, direct), + SkillRef::Many(reference) if direct => { + Err(CommandError::direct_multi_skill_ref(&reference)) + } + SkillRef::Many(reference) => { + let listing = list_skills(host, &reference)?; + let mut lines = Vec::with_capacity(listing.items.len()); + for (index, item) in listing.items.iter().enumerate() { + let names = host + .install_request(InstallRequest { + operation: InstallOperation::Install(InstallSource::Remote( + item.selector(), + )), + scope: options.scope, + targets: options.targets.clone(), + mode: options.mode, + }) + .map_err(|error| CommandError { + message: format!( + "{}. skilld installed {index} of {} Skills from {reference} before this failure.", + error.message.trim_end_matches('.'), + listing.items.len() + ), + ..error + })?; + lines.extend( + names + .into_iter() + .map(|name| Line::success(format!("Installed Skill {name}."))), + ); } - (false, source) => InstallOperation::Install(source), - }, - None if direct => InstallOperation::DirectRestore, - None => InstallOperation::Restore, - }; - if operation == InstallOperation::Install(InstallSource::BundledSkilld) - && scope != InstallScope::Global - { - return Err(CommandError::input( - "install the skilld-maintained Skill with --global", - )); - } - let targets = parse_agent_targets(&agents).map_err(CommandError::domain)?; - let mode = mode - .as_deref() - .map(InstallMode::parse) - .transpose() - .map_err(CommandError::domain)?; - let names = host.install_request(InstallRequest { - operation, - scope, - targets, - mode, - })?; - let mut lines = names - .into_iter() - .map(|name| Line::success(format!("Installed Skill {name}."))) - .collect::>(); - if direct { - lines.push(Line::hint("Review the unverified Skill before use.")); + Ok(CommandOutput::Screen(Screen::new(lines))) + } } - Ok(CommandOutput::Screen(Screen::new(lines))) + } + Command::Install { + source, + global, + agents, + mode, + direct, + } => { + let options = InstallOptions::parse(global, &agents, mode.as_deref())?; + install(host, source, options, direct) } Command::Run { source, @@ -846,6 +895,22 @@ fn dispatch( revision, direct, } => { + let reference = SkillRef::parse(&source).map_err(CommandError::remote)?; + let source = match reference { + SkillRef::Skill(source) => source, + SkillRef::Many(reference) => { + if direct { + return Err(CommandError::direct_multi_skill_ref(&reference)); + } + if !files.is_empty() || revision.is_some() { + return Err(CommandError::input(format!( + "--file and --revision need one Skill source. {reference} names several Skills. Run skilld run {reference} to list them." + ))); + } + return list_skills(host, &reference) + .map(|listing| CommandOutput::Run(RunOutcome::Index(listing))); + } + }; run::reject_duplicate_files(&files)?; let revision = revision.map(CommitSha::parse).transpose().map_err(|_| { CommandError::input("--revision must use 40 lowercase hexadecimal characters") @@ -996,6 +1061,92 @@ fn dispatch( } } +/// The install flags `skilld install` and `skilld add` share, parsed once. +#[derive(Clone, Debug, Eq, PartialEq)] +struct InstallOptions { + scope: InstallScope, + targets: Vec, + mode: Option, +} + +impl InstallOptions { + fn parse(global: bool, agents: &[String], mode: Option<&str>) -> Result { + let targets = parse_agent_targets(agents).map_err(CommandError::domain)?; + let mode = mode + .map(InstallMode::parse) + .transpose() + .map_err(CommandError::domain)?; + Ok(Self { + scope: scope(global), + targets, + mode, + }) + } +} + +/// Install one Skill source, or restore the lockfile when SOURCE is absent. +fn install( + host: &H, + source: Option, + options: InstallOptions, + direct: bool, +) -> Result { + let scope = options.scope; + let operation = match source { + Some(source) => match (direct, InstallSource::parse(&source)) { + (true, InstallSource::Remote(source)) => { + InstallOperation::Install(InstallSource::DirectRemote(source)) + } + (true, InstallSource::DirectRemote(source)) => { + InstallOperation::Install(InstallSource::DirectRemote(source)) + } + (true, InstallSource::Local(_)) => { + return Err(CommandError::direct_local_install_source()); + } + (true, InstallSource::BundledSkilld) => { + return Err(CommandError::direct_bundled_install_source()); + } + (false, source) => InstallOperation::Install(source), + }, + None if direct => InstallOperation::DirectRestore, + None => InstallOperation::Restore, + }; + if operation == InstallOperation::Install(InstallSource::BundledSkilld) + && scope != InstallScope::Global + { + return Err(CommandError::input( + "install the skilld-maintained Skill with --global", + )); + } + let names = host.install_request(InstallRequest { + operation, + scope, + targets: options.targets, + mode: options.mode, + })?; + let mut lines = names + .into_iter() + .map(|name| Line::success(format!("Installed Skill {name}."))) + .collect::>(); + if direct { + lines.push(Line::hint("Review the unverified Skill before use.")); + } + Ok(CommandOutput::Screen(Screen::new(lines))) +} + +/// List the Skills a multi-skill ref names. An empty listing is a failure: +/// the caller asked for Skills and got none to act on. +fn list_skills(host: &H, reference: &MultiSkillRef) -> Result { + let listing = host.list_skills(reference)?; + if listing.items.is_empty() { + return Err(CommandError::operation( + "SOURCE_NOT_FOUND", + format!("{reference} names no Skills that skilld.dev lists"), + )); + } + Ok(listing) +} + fn render_view(view: SkillView) -> Result, CommandError> { let source = match view.skill.source { LockedSource::Local { path } => Line::field("Source", format!("local {path}")), @@ -1697,6 +1848,12 @@ impl Host for LocalHost { } } + fn list_skills(&self, reference: &MultiSkillRef) -> Result { + self.remote_provider()? + .list_skills(reference) + .map_err(CommandError::remote) + } + fn view(&self, name: &str, scope: InstallScope) -> Result { let known = self.known_targets(scope)?; let name = skilld_core::SkillName::parse(name.to_owned()).map_err(CommandError::domain)?; @@ -3118,12 +3275,231 @@ mod tests { } } + /// Lists two Skills for any multi-skill ref and records every install. + struct ListingHost { + installs: std::sync::Mutex>, + empty: bool, + } + + impl ListingHost { + fn new() -> Self { + Self { + installs: std::sync::Mutex::new(vec![]), + empty: false, + } + } + + fn requests(&self) -> Vec { + self.installs.lock().unwrap().clone() + } + } + + impl Host for ListingHost { + fn list(&self, _scope: InstallScope) -> Result, CommandError> { + Ok(vec![]) + } + + fn install( + &self, + _source: InstallSource, + _scope: InstallScope, + ) -> Result { + unreachable!("add installs through install_request") + } + + fn install_request(&self, request: InstallRequest) -> Result, CommandError> { + let InstallOperation::Install(InstallSource::Remote(source)) = &request.operation + else { + panic!("expected a hosted install: {:?}", request.operation); + }; + let name = source.rsplit('/').next().unwrap().to_owned(); + self.installs.lock().unwrap().push(request); + Ok(vec![name]) + } + + fn list_skills(&self, reference: &MultiSkillRef) -> Result { + let items = if self.empty { + vec![] + } else { + vec![ + skilld_core::ListedSkill { + name: "vue".to_owned(), + owner: "skilld-dev".to_owned(), + repository: "skills".to_owned(), + description: Some("Build Vue interfaces.".to_owned()), + }, + skilld_core::ListedSkill { + name: "nuxt".to_owned(), + owner: "skilld-dev".to_owned(), + repository: "skills".to_owned(), + description: None, + }, + ] + }; + Ok(SkillListing { + reference: reference.clone(), + items, + }) + } + } + + fn run_plain(host: &impl Host, args: &[&str]) -> (u8, String, String) { + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let result = run(args.iter().copied(), host, &mut stdout, &mut stderr); + ( + result.exit_code, + String::from_utf8(stdout).unwrap(), + String::from_utf8(stderr).unwrap(), + ) + } + + #[test] + fn run_prints_an_index_for_a_multi_skill_ref() { + let host = ListingHost::new(); + let (exit, stdout, stderr) = run_plain(&host, &["skilld", "run", "@harlan-zw/nuxt"]); + + assert_eq!(exit, 0, "{stderr}"); + assert_eq!( + stdout, + "vue\tskilld-dev/skills\tBuild Vue interfaces.\tnpx skilld run skilld:skilld-dev/skills/vue\n\ + nuxt\tskilld-dev/skills\t\tnpx skilld run skilld:skilld-dev/skills/nuxt\n" + ); + assert!(host.requests().is_empty(), "run must install nothing"); + } + + #[test] + fn run_index_json_names_every_skill_and_its_run_command() { + let host = ListingHost::new(); + let (exit, stdout, _) = + run_plain(&host, &["skilld", "run", "gh:skilld-dev/skills", "--json"]); + + assert_eq!(exit, 0); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!(json["command"], "run"); + assert_eq!(json["data"]["_tag"], "index"); + assert_eq!(json["data"]["kind"], "repository"); + assert_eq!(json["data"]["reference"], "gh:skilld-dev/skills"); + assert_eq!(json["data"]["wroteSkillFiles"], false); + assert_eq!(json["data"]["total"], 2); + assert_eq!( + json["data"]["items"][0]["selector"], + "skilld:skilld-dev/skills/vue" + ); + assert_eq!( + json["data"]["items"][0]["runArgv"], + serde_json::json!(["skilld", "run", "skilld:skilld-dev/skills/vue", "--json"]) + ); + assert_eq!( + json["data"]["items"][1]["description"], + serde_json::Value::Null + ); + assert_eq!( + json["data"]["addArgv"], + serde_json::json!(["skilld", "add", "gh:skilld-dev/skills"]) + ); + } + + #[test] + fn run_rejects_file_direct_and_empty_listings_for_multi_skill_refs() { + let host = ListingHost::new(); + let (exit, stdout, stderr) = run_plain(&host, &["skilld", "run", "@harlan-zw", "--direct"]); + assert_eq!((exit, stdout.as_str()), (2, "")); + assert!(stderr.starts_with("DIRECT_SOURCE_REQUIRED:"), "{stderr}"); + + let (exit, _, stderr) = run_plain( + &host, + &["skilld", "run", "@harlan-zw", "--file", "references/api.md"], + ); + assert_eq!(exit, 2); + assert!( + stderr.contains("--file and --revision need one Skill source"), + "{stderr}" + ); + + let empty = ListingHost { + empty: true, + ..ListingHost::new() + }; + let (exit, stdout, stderr) = run_plain(&empty, &["skilld", "run", "gh:skilld-dev/empty"]); + assert_eq!((exit, stdout.as_str()), (1, "")); + assert_eq!( + stderr, + "SOURCE_NOT_FOUND: gh:skilld-dev/empty names no Skills that skilld.dev lists\n" + ); + } + + #[test] + fn add_installs_every_listed_skill_with_the_shared_install_flags() { + let host = ListingHost::new(); + let (exit, stdout, stderr) = run_plain( + &host, + &[ + "skilld", + "add", + "gh:skilld-dev/skills", + "--global", + "--agent", + "codex", + "--mode", + "symlink", + ], + ); + + assert_eq!(exit, 0, "{stderr}"); + assert_eq!(stdout, "Installed Skill vue.\nInstalled Skill nuxt.\n"); + let requests = host.requests(); + assert_eq!(requests.len(), 2); + for (request, selector) in requests.iter().zip([ + "skilld:skilld-dev/skills/vue", + "skilld:skilld-dev/skills/nuxt", + ]) { + assert_eq!( + request.operation, + InstallOperation::Install(InstallSource::Remote(selector.to_owned())) + ); + assert_eq!(request.scope, InstallScope::Global); + assert_eq!(request.targets, [AgentTargetId::Codex]); + assert_eq!(request.mode, Some(InstallMode::Symlink)); + } + } + + #[test] + fn add_with_one_skill_source_installs_like_install() { + let host = ListingHost::new(); + let (exit, stdout, stderr) = + run_plain(&host, &["skilld", "add", "skilld:skilld-dev/skills/vue"]); + + assert_eq!(exit, 0, "{stderr}"); + assert_eq!(stdout, "Installed Skill vue.\n"); + assert_eq!( + host.requests()[0].operation, + InstallOperation::Install(InstallSource::Remote( + "skilld:skilld-dev/skills/vue".to_owned() + )) + ); + } + + #[test] + fn add_rejects_ambiguous_refs_before_any_request() { + let host = ListingHost::new(); + let (exit, stdout, stderr) = + run_plain(&host, &["skilld", "add", "gh:skilld-dev/skills/vue"]); + + assert_eq!((exit, stdout.as_str()), (2, "")); + assert!( + stderr.contains("gh:OWNER/REPOSITORY names every Skill in a Repository"), + "{stderr}" + ); + assert!(host.requests().is_empty()); + } + #[test] fn public_command_vocabulary_matches_v3() { assert_eq!( command_names(), [ - "search", "install", "run", "list", "view", "remove", "update", "verify", + "search", "install", "add", "run", "list", "view", "remove", "update", "verify", "outdated", "auth", "config" ] ); diff --git a/crates/skilld-command/src/output.rs b/crates/skilld-command/src/output.rs index deaa9e67..d23d1817 100644 --- a/crates/skilld-command/src/output.rs +++ b/crates/skilld-command/src/output.rs @@ -1,6 +1,6 @@ use clap::error::ErrorKind; use serde::Serialize; -use skilld_core::UpdatePlanV1; +use skilld_core::{ListedSkill, SkillListing, UpdatePlanV1}; use skilld_ui::text::{grouped_number, is_unsafe_terminal, sanitize, width, wrap}; use skilld_ui::{Role, paint}; @@ -422,6 +422,17 @@ pub(crate) fn render_run(outcome: &RunOutcome, mode: OutputMode) -> Result render_json_success( + "run", + index_json(listing), + "Skill run output could not be encoded", + ), + (RunOutcome::Index(listing), OutputMode::Plain { .. }) => { + Ok(render_index_plain(listing).into_bytes()) + } + (RunOutcome::Index(listing), OutputMode::Human { color, .. }) => { + Ok(render_index(listing, color).into_bytes()) + } (RunOutcome::Load(skill), _) => { Ok(render_load(skill, colored(mode), command_platform(mode)).into_bytes()) } @@ -457,6 +468,106 @@ const fn command_platform(mode: OutputMode) -> CommandPlatform { } } +/// One line per Skill: the run command, then the description. +fn render_index(listing: &SkillListing, color: bool) -> String { + let reference = sanitize(&listing.reference.canonical()); + let count = listing.items.len(); + let noun = if count == 1 { "Skill" } else { "Skills" }; + let mut out = String::new(); + out.push_str(&paint( + &format!("{reference} names {count} {noun}. skilld loaded none of them."), + Role::Emphasis, + color, + )); + out.push_str("\n\n"); + for item in &listing.items { + out.push_str(&paint(&run_command(item), Role::Emphasis, color)); + if let Some(description) = &item.description { + out.push_str(" "); + out.push_str(&paint(&sanitize(description), Role::Dim, color)); + } + out.push('\n'); + } + out.push('\n'); + out.push_str("Run one command above to load that Skill for this session.\n"); + out.push_str(&format!( + "Install every Skill listed here with {}.\n", + paint( + &format!("npx skilld add {reference}"), + Role::Emphasis, + color + ) + )); + out +} + +/// Tab separated: name, owner/repository, description, run command. +fn render_index_plain(listing: &SkillListing) -> String { + let mut out = String::new(); + for item in &listing.items { + out.push_str(&escape_plain(&item.name)); + out.push('\t'); + out.push_str(&escape_plain(&format!( + "{}/{}", + item.owner, item.repository + ))); + out.push('\t'); + out.push_str(&escape_plain( + item.description.as_deref().unwrap_or_default(), + )); + out.push('\t'); + out.push_str(&escape_plain(&run_command(item))); + out.push('\n'); + } + out +} + +fn run_command(item: &ListedSkill) -> String { + format!("npx skilld run {}", item.selector()) +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct JsonIndexedSkill { + name: String, + owner: String, + repository: String, + description: Option, + selector: String, + run_argv: Vec, +} + +fn index_json(listing: &SkillListing) -> JsonRunData { + JsonRunData::Index { + reference: listing.reference.canonical(), + kind: listing.reference.kind(), + wrote_skill_files: false, + items: listing + .items + .iter() + .map(|item| JsonIndexedSkill { + name: item.name.clone(), + owner: item.owner.clone(), + repository: item.repository.clone(), + description: item.description.clone(), + selector: item.selector(), + run_argv: vec![ + "skilld".to_owned(), + "run".to_owned(), + item.selector(), + "--json".to_owned(), + ], + }) + .collect(), + total: listing.items.len(), + add_argv: vec![ + "skilld".to_owned(), + "add".to_owned(), + listing.reference.canonical(), + ], + } +} + fn render_load(skill: &TransientSkill, color: bool, platform: CommandPlatform) -> String { let mut out = String::new(); out.push_str(&format!( @@ -837,6 +948,14 @@ enum JsonRunData { wrote_skill_files: bool, files: Vec, }, + Index { + reference: String, + kind: &'static str, + wrote_skill_files: bool, + items: Vec, + total: usize, + add_argv: Vec, + }, } #[derive(Serialize)] diff --git a/crates/skilld-command/src/remote.rs b/crates/skilld-command/src/remote.rs index 73764498..a9c71785 100644 --- a/crates/skilld-command/src/remote.rs +++ b/crates/skilld-command/src/remote.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::fmt; use std::sync::Arc; #[cfg(not(target_os = "wasi"))] @@ -10,11 +10,11 @@ use base64::{Engine as _, engine::general_purpose::STANDARD}; use serde::{Deserialize, Serialize}; use serde_json::json; use skilld_core::{ - ArtifactAttestation, CommitAuthor, CommitSha, CommitSummary, LockedSource, PreparedFile, - RemoteError, RemoteSelector, RepositoryVisibility, SearchResponse, SourceRef, SourceRequest, - SourceSelector, SourceStatus, TrustedRoot, TrustedRootPin, VerifiedTrustedRoot, - parse_search_response, prepare_unverified_files, verify_artifact, verify_attestation, - verify_trusted_root, + ArtifactAttestation, CommitAuthor, CommitSha, CommitSummary, ListedSkill, LockedSource, + MultiSkillRef, PreparedFile, RemoteError, RemoteSelector, RepositoryVisibility, SearchResponse, + SkillListing, SourceRef, SourceRequest, SourceSelector, SourceStatus, TrustedRoot, + TrustedRootPin, VerifiedTrustedRoot, parse_search_response, prepare_unverified_files, + verify_artifact, verify_attestation, verify_trusted_root, }; use skilld_ui::text::is_unsafe_terminal; use url::Url; @@ -22,6 +22,16 @@ use url::Url; const JSON_LIMIT: usize = 8 * 1024 * 1024; const UPDATE_RESPONSE_LIMIT: usize = 64 * 1024 * 1024; const SEARCH_LIMIT: usize = 1024 * 1024; +const LISTING_LIMIT: usize = 4 * 1024 * 1024; +const LISTING_PAGE: usize = 200; +/// Most pages one owner index may span. `pages` is server supplied, so this +/// bounds the loop when the response lies; a partial or empty page also ends +/// paging early. +const MAX_LISTING_PAGES: usize = 25; +/// Most server supplied listing rows skilld trusts per response, mirroring +/// `MAX_LISTING_PAGES`: the curator collection list and each collection's +/// entry list are capped here so a malformed response cannot fan out. +const MAX_LISTING_ENTRIES: usize = MAX_LISTING_PAGES; const ARTIFACT_LIMIT: usize = 64 * 1024 * 1024; const DIRECT_BLOB_LIMIT: usize = 12 * 1024 * 1024; const MAX_REDIRECTS: usize = 3; @@ -380,6 +390,15 @@ pub trait RemoteProvider: Send + Sync { &self, comparisons: &[RemoteUpdateComparison], ) -> Result, RemoteError>; + + /// List every Skill a Repository, curator, or collection names. + fn list_skills(&self, reference: &MultiSkillRef) -> Result { + let _ = reference; + Err(RemoteError::new( + "NOT_IMPLEMENTED", + "this remote provider lists no Skills", + )) + } } #[derive(Clone)] @@ -1485,7 +1504,274 @@ impl SkilldRemote { } } +/// One entry a collection names, before the Skills behind it are known. +#[derive(Clone, Debug, Eq, PartialEq)] +struct CollectionEntry { + owner: String, + repository: String, + /// `None` names every Skill the Repository carries. + name: Option, + /// The curator's one-line reason for including it. + reason: Option, +} + +/// `GET /api/skills?owner=` rows. The site returns more fields; skilld reads these. +#[derive(Deserialize)] +struct RegistrySkillPage { + items: Vec, + total: Option, + pages: Option, +} + +#[derive(Deserialize)] +struct RegistrySkillRow { + name: String, + owner: String, + repo: String, + description: Option, +} + +/// `GET /api/curators/{login}`: the protocol `CuratorPayload` shape. +#[derive(Deserialize)] +struct CuratorPayload { + collections: Vec, +} + +#[derive(Deserialize)] +struct CollectionSummary { + slug: String, +} + +/// `GET /api/collections/by-author/{login}/{slug}`. The site returns more +/// fields; skilld reads the resolved Skill rows. +#[derive(Deserialize)] +struct CollectionDetail { + skills: Vec, +} + +#[derive(Deserialize)] +struct CollectionSkillRow { + owner: String, + repo: String, + name: Option, + reason: Option, +} + +impl SkilldRemote { + fn service_json Deserialize<'de>>(&self, url: Url) -> Result { + let request = HttpRequest { + method: HttpMethod::Get, + url: url.into(), + headers: vec![], + body: vec![], + response_limit: LISTING_LIMIT, + }; + let response = self.execute(request, AllowedOrigin::Service(self.endpoint.clone()))?; + parse_json(&response.body) + } + + /// Every indexed Skill one GitHub account owns. + fn owner_skills(&self, owner: &str) -> Result, RemoteError> { + let first = self.owner_skills_page(owner, 1)?; + let pages = first + .pages + .or_else(|| first.total.map(|total| total.div_ceil(LISTING_PAGE))) + .unwrap_or(1) + .min(MAX_LISTING_PAGES); + let mut rows = first.items; + for page in 2..=pages { + let next = self.owner_skills_page(owner, page)?; + let received = next.items.len(); + rows.extend(next.items); + if received < LISTING_PAGE { + break; + } + } + let mut seen = BTreeSet::new(); + Ok(rows + .into_iter() + .filter(|row| row.owner.eq_ignore_ascii_case(owner)) + .filter_map(|row| { + listed_skill(row.owner, row.repo, row.name, row.description.as_deref()) + }) + .filter(|skill| seen.insert(skill.selector())) + .collect()) + } + + fn owner_skills_page( + &self, + owner: &str, + page: usize, + ) -> Result { + let mut url = self.service_url("/api/skills")?; + { + let mut pairs = url.query_pairs_mut(); + pairs.append_pair("owner", owner); + pairs.append_pair("limit", &LISTING_PAGE.to_string()); + if page > 1 { + pairs.append_pair("page", &page.to_string()); + } + } + self.service_json(url) + } + + /// Every Skill one Repository carries, by name. The owner index fetch is + /// memoized per Repository in `memo` for one listing, so repeated + /// collection entries naming the same Repository cost one fetch. + fn repository_skills( + &self, + owner: &str, + repository: &str, + memo: &mut HashMap<(String, String), Vec>, + ) -> Result, RemoteError> { + let key = (owner.to_ascii_lowercase(), repository.to_ascii_lowercase()); + if let Some(items) = memo.get(&key) { + return Ok(items.clone()); + } + let mut items = self + .owner_skills(owner)? + .into_iter() + .filter(|skill| skill.repository.eq_ignore_ascii_case(repository)) + .collect::>(); + items.sort_by(|left, right| left.name.cmp(&right.name)); + memo.insert(key, items.clone()); + Ok(items) + } + + fn collection_entries( + &self, + login: &str, + slug: &str, + ) -> Result, RemoteError> { + let path = format!( + "/api/collections/by-author/{}/{}", + path_segment(login), + path_segment(slug) + ); + let detail: CollectionDetail = + self.service_json(self.service_url(&path)?) + .map_err(|error| { + not_found_as_source( + error, + format!("skilld.dev has no collection @{login}/{slug}"), + ) + })?; + Ok(detail + .skills + .into_iter() + .map(|row| CollectionEntry { + owner: row.owner, + repository: row.repo, + name: row.name, + reason: row.reason, + }) + .take(MAX_LISTING_ENTRIES) + .collect()) + } + + fn curator_slugs(&self, login: &str) -> Result, RemoteError> { + let path = format!("/api/curators/{}", path_segment(login)); + let curator: CuratorPayload = + self.service_json(self.service_url(&path)?) + .map_err(|error| { + not_found_as_source(error, format!("skilld.dev has no curator @{login}")) + })?; + Ok(curator + .collections + .into_iter() + .map(|collection| collection.slug) + .collect()) + } + + /// Turn collection entries into listed Skills, in collection order. + /// An entry that names one Skill lists it with the curator's reason. + /// An entry that names a Repository lists every Skill it carries. + fn expand_entries( + &self, + entries: Vec, + memo: &mut HashMap<(String, String), Vec>, + ) -> Result, RemoteError> { + let mut seen = BTreeSet::new(); + let mut items = Vec::new(); + for entry in entries { + let expanded = match entry.name { + Some(name) => { + listed_skill(entry.owner, entry.repository, name, entry.reason.as_deref()) + .into_iter() + .collect() + } + None => self.repository_skills(&entry.owner, &entry.repository, memo)?, + }; + for skill in expanded { + if seen.insert(skill.selector()) { + items.push(skill); + } + } + } + Ok(items) + } +} + +/// Build one listed Skill from untrusted registry fields. +/// +/// A row outside the selector contract has no `skilld:` selector, so skilld +/// could not run or install it. Listing it would print a command that fails, +/// so the row is dropped. +fn listed_skill( + owner: String, + repository: String, + name: String, + description: Option<&str>, +) -> Option { + let skill = ListedSkill { + owner, + repository, + name, + description: description + .map(|value| sanitize_line(value, 500, "")) + .filter(|value| !value.is_empty()), + }; + RemoteSelector::parse(&skill.selector()) + .is_ok() + .then_some(skill) +} + +fn not_found_as_source(error: RemoteError, message: String) -> RemoteError { + if error.code == "SERVICE_UNAVAILABLE" && error.message.ends_with("HTTP 404") { + RemoteError::new("SOURCE_NOT_FOUND", message) + } else { + error + } +} + impl RemoteProvider for SkilldRemote { + fn list_skills(&self, reference: &MultiSkillRef) -> Result { + let mut memo = HashMap::new(); + let items = match reference { + MultiSkillRef::Repository { owner, repository } => { + self.repository_skills(owner, repository, &mut memo)? + } + MultiSkillRef::Collection { login, slug } => { + self.expand_entries(self.collection_entries(login, slug)?, &mut memo)? + } + MultiSkillRef::Curator { login } => { + let mut entries = Vec::new(); + for slug in self + .curator_slugs(login)? + .into_iter() + .take(MAX_LISTING_ENTRIES) + { + entries.extend(self.collection_entries(login, &slug)?); + } + self.expand_entries(entries, &mut memo)? + } + }; + Ok(SkillListing { + reference: reference.clone(), + items, + }) + } + fn search(&self, query: &str, limit: u8) -> Result { let query = query.trim(); if query.is_empty() || query.len() > 200 || !(1..=50).contains(&limit) { diff --git a/crates/skilld-command/src/run.rs b/crates/skilld-command/src/run.rs index 9166d4c1..1e8ecfcb 100644 --- a/crates/skilld-command/src/run.rs +++ b/crates/skilld-command/src/run.rs @@ -11,7 +11,7 @@ use std::fs::{self, File}; use std::io::Read; use std::path::{Path, PathBuf}; -use skilld_core::PreparedFile; +use skilld_core::{PreparedFile, SkillListing}; use skilld_ui::text::is_unsafe_terminal; use crate::CommandError; @@ -109,6 +109,8 @@ pub enum FileContent { #[derive(Clone, Debug, Eq, PartialEq)] pub enum RunOutcome { Load(Box), + /// A multi-skill ref: skilld names the Skills and loads none of them. + Index(SkillListing), Files { skill: String, origin: SkillOrigin, diff --git a/crates/skilld-command/tests/add.rs b/crates/skilld-command/tests/add.rs new file mode 100644 index 00000000..f283e694 --- /dev/null +++ b/crates/skilld-command/tests/add.rs @@ -0,0 +1,161 @@ +//! `skilld add` installs every Skill a multi-skill ref names. + +use std::fs; +use std::sync::Arc; + +use sha2::{Digest, Sha256}; +use skilld_command::{ + CommandPlatform, DetectionEnvironment, Host, LocalHost, OutputContext, PreparedRemoteSkill, + RemoteLatestCommit, RemoteProvider, RemoteSourceState, RemoteUpdateComparison, + RemoteUpdateResult, run_with_output, +}; +use skilld_core::{ + CommitSha, InstallScope, ListedSkill, LockedSource, MultiSkillRef, PreparedFile, RemoteError, + RemoteSelector, SearchResponse, SkillListing, SourceSelector, SourceStatus, +}; + +/// Lists two Skills for one Repository and prepares whichever one is asked for. +struct ListingRemote; + +impl RemoteProvider for ListingRemote { + fn list_skills(&self, reference: &MultiSkillRef) -> Result { + assert_eq!( + *reference, + MultiSkillRef::Repository { + owner: "vuejs".to_owned(), + repository: "core".to_owned(), + } + ); + Ok(SkillListing { + reference: reference.clone(), + items: ["vue", "nuxt"] + .into_iter() + .map(|name| ListedSkill { + name: name.to_owned(), + owner: "vuejs".to_owned(), + repository: "core".to_owned(), + description: None, + }) + .collect(), + }) + } + + fn search(&self, _query: &str, _limit: u8) -> Result { + unimplemented!("search is outside an add") + } + + fn prepare( + &self, + selector: &RemoteSelector, + direct: bool, + ) -> Result { + assert!(!direct, "add uses hosted delivery"); + let SourceSelector::NamedSkill { name } = &selector.source().selector else { + panic!("expected a named Skill selector: {selector}"); + }; + let file = PreparedFile { + path: "SKILL.md".to_owned(), + mode: 0o644, + bytes: format!("---\nname: {name}\ndescription: Use {name}.\n---\n\n# {name}\n") + .into_bytes(), + }; + let digest = installed_digest(&file); + Ok(PreparedRemoteSkill { + files: vec![file], + locked_source: LockedSource::Remote { + source: selector.canonical(), + commit_sha: "a".repeat(40), + skill_path: format!("skills/{name}"), + }, + source_status: SourceStatus::Unverified { + content_sha256: digest.clone(), + installed_sha256: digest, + }, + }) + } + + fn prepare_exact( + &self, + _selector: &RemoteSelector, + _expected_commit: &CommitSha, + _direct: bool, + ) -> Result { + unimplemented!("exact preparation is outside an add") + } + + fn source_state( + &self, + _selector: &RemoteSelector, + _artifact_id: &str, + _commit_sha: &str, + ) -> Result { + unimplemented!("source state is outside an add") + } + + fn latest_commit( + &self, + _selector: &RemoteSelector, + _direct: bool, + ) -> Result { + unimplemented!("latest commit is outside an add") + } + + fn compare_updates( + &self, + _comparisons: &[RemoteUpdateComparison], + ) -> Result, RemoteError> { + unimplemented!("update comparison is outside an add") + } +} + +fn installed_digest(file: &PreparedFile) -> String { + let mut hasher = Sha256::new(); + hasher.update((file.path.len() as u64).to_be_bytes()); + hasher.update(file.path.as_bytes()); + hasher.update((file.bytes.len() as u64).to_be_bytes()); + hasher.update(&file.bytes); + hasher + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect() +} + +#[test] +fn add_installs_every_skill_the_repository_ref_names() { + let temporary = tempfile::tempdir().unwrap(); + let project = temporary.path().join("project"); + fs::create_dir_all(&project).unwrap(); + let host = LocalHost::new(project.clone(), temporary.path().join("global")) + .with_detection_environment(DetectionEnvironment::new(["CLAUDE_CODE".to_owned()])) + .with_remote_provider(Arc::new(ListingRemote)); + + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let result = run_with_output( + ["skilld", "add", "gh:vuejs/core"], + &host, + OutputContext::Plain { + platform: CommandPlatform::Unix, + }, + &mut stdout, + &mut stderr, + ); + + assert_eq!(result.exit_code, 0, "{}", String::from_utf8_lossy(&stderr)); + assert_eq!( + String::from_utf8(stdout).unwrap(), + "Installed Skill vue.\nInstalled Skill nuxt.\n" + ); + assert_eq!(host.list(InstallScope::Project).unwrap(), ["nuxt", "vue"]); + for name in ["vue", "nuxt"] { + assert!( + project + .join(".claude/skills") + .join(name) + .join("SKILL.md") + .exists(), + "{name} reached the Agent target" + ); + } +} diff --git a/crates/skilld-command/tests/remote.rs b/crates/skilld-command/tests/remote.rs index ca8064f4..54f43e6d 100644 --- a/crates/skilld-command/tests/remote.rs +++ b/crates/skilld-command/tests/remote.rs @@ -18,9 +18,10 @@ use skilld_command::{ use skilld_core::{ AgentTargetId, ArtifactAttestation, ArtifactFile, AttestationSignature, CheckOutcome, CheckResult, CommitAuthor, CommitSha, CommitSummary, InstallMode, InstallOperation, - InstallRequest, InstallScope, InstallSource, LockedSource, PreparedFile, RemoteError, - RemoteSelector, RepositoryVisibility, ResolvedSource, SearchResponse, SignatureAlgorithm, - SourceProvider, SourceStatus, TrustedRootPin, UpdatePlanItem, UpdatePlanV1, UpdateRelation, + InstallRequest, InstallScope, InstallSource, ListedSkill, LockedSource, MultiSkillRef, + PreparedFile, RemoteError, RemoteSelector, RepositoryVisibility, ResolvedSource, + SearchResponse, SignatureAlgorithm, SourceProvider, SourceStatus, TrustedRootPin, + UpdatePlanItem, UpdatePlanV1, UpdateRelation, }; const ROOT_DOMAIN: &[u8] = b"skilld-trusted-key-v1\0"; @@ -388,6 +389,426 @@ fn search_remote(http: Arc) -> SkilldRemote { .with_sleeper(Arc::new(NoSleep)) } +fn listed(owner: &str, repository: &str, name: &str, description: Option<&str>) -> ListedSkill { + ListedSkill { + name: name.to_owned(), + owner: owner.to_owned(), + repository: repository.to_owned(), + description: description.map(str::to_owned), + } +} + +fn registry_page(rows: &[(&str, &str, &str, Option<&str>)]) -> HttpResponse { + let items = rows + .iter() + .map(|(owner, repo, name, description)| { + json!({ + "name": name, + "owner": owner, + "repo": repo, + "description": description, + "stars": 12, + "registryPath": format!("/gh/{owner}/{repo}/{name}"), + }) + }) + .collect::>(); + response( + 200, + serde_json::to_vec(&json!({ "items": items, "total": items.len(), "page": 1 })).unwrap(), + ) +} + +fn request_paths(http: &FakeHttp) -> Vec { + http.requests + .lock() + .unwrap() + .iter() + .map(|request| { + request + .url + .trim_start_matches("http://127.0.0.1:8787") + .to_owned() + }) + .collect() +} + +#[test] +fn a_repository_ref_lists_that_repository_from_the_owner_index() { + let http = Arc::new(FakeHttp::with([registry_page(&[ + ( + "vuejs", + "core", + "vue", + Some("Build Vue interfaces.\nSecond line."), + ), + ("vuejs", "core", "Not A Skill", Some("dropped: no selector")), + ("vuejs", "router", "vue-router", None), + ("vuejs", "core", "composition", None), + ])])); + let remote = search_remote(http.clone()); + + let listing = remote + .list_skills(&MultiSkillRef::Repository { + owner: "vuejs".to_owned(), + repository: "core".to_owned(), + }) + .unwrap(); + + assert_eq!( + listing.items, + [ + listed("vuejs", "core", "composition", None), + listed("vuejs", "core", "vue", Some("Build Vue interfaces.")), + ] + ); + assert_eq!(request_paths(&http), ["/api/skills?owner=vuejs&limit=200"]); +} + +#[test] +fn an_owner_index_past_page_one_is_merged_into_the_listing() { + let page_one = response( + 200, + serde_json::to_vec(&json!({ + "items": [{ + "name": "padding", + "owner": "big", + "repo": "other", + "description": null, + "stars": 1, + "registryPath": "/gh/big/other/padding", + }], + "total": 300, + "page": 1, + "pages": 2, + })) + .unwrap(), + ); + let page_two = registry_page(&[ + ("big", "wanted", "zulu", None), + ("big", "wanted", "alpha", Some("On page two.")), + ]); + let http = Arc::new(FakeHttp::with([page_one, page_two])); + let remote = search_remote(http.clone()); + + let listing = remote + .list_skills(&MultiSkillRef::Repository { + owner: "big".to_owned(), + repository: "wanted".to_owned(), + }) + .unwrap(); + + assert_eq!( + listing.items, + [ + listed("big", "wanted", "alpha", Some("On page two.")), + listed("big", "wanted", "zulu", None), + ] + ); + assert_eq!( + request_paths(&http), + [ + "/api/skills?owner=big&limit=200", + "/api/skills?owner=big&limit=200&page=2", + ] + ); +} + +/// Serves the same full page for every owner index request, whatever `page` +/// names, so only a client side page cap can end the listing. +#[derive(Default)] +struct RunawayOwnerIndexHttp { + requests: Mutex>, +} + +const RUNAWAY_PAGE_ROWS: usize = 200; +const RUNAWAY_REQUEST_LIMIT: usize = 25; + +impl HttpAdapter for RunawayOwnerIndexHttp { + fn send( + &self, + request: &HttpRequest, + _cancellation: &dyn Cancellation, + _timeout: Option, + ) -> Result { + let mut requests = self.requests.lock().unwrap(); + requests.push(request.url.to_string()); + assert!( + requests.len() <= RUNAWAY_REQUEST_LIMIT, + "the owner index kept paging: {} requests issued", + requests.len() + ); + let mut page = json!({ + "items": (0..RUNAWAY_PAGE_ROWS) + .map(|_| json!({ + "name": "wanted", + "owner": "big", + "repo": "wanted", + "description": null, + "stars": 1, + "registryPath": "/gh/big/wanted/wanted", + })) + .collect::>(), + "total": 20_000_000, + }); + if requests.len() == 1 { + page["pages"] = json!(u64::MAX); + } + Ok(response(200, serde_json::to_vec(&page).unwrap())) + } +} + +#[test] +fn a_runaway_owner_index_stops_paging_and_lists_each_skill_once() { + let http = Arc::new(RunawayOwnerIndexHttp::default()); + let remote = SkilldRemote::new( + http.clone(), + Arc::new(NoTokenProvider), + NativeRemoteConfig::Unconfigured, + ) + .with_endpoint("http://127.0.0.1:8787") + .unwrap() + .with_sleeper(Arc::new(NoSleep)); + + let listing = remote + .list_skills(&MultiSkillRef::Repository { + owner: "big".to_owned(), + repository: "wanted".to_owned(), + }) + .unwrap(); + + assert_eq!(listing.items, [listed("big", "wanted", "wanted", None)]); + assert_eq!(http.requests.lock().unwrap().len(), RUNAWAY_REQUEST_LIMIT); +} + +#[test] +fn a_collection_ref_lists_its_skills_in_order_and_expands_repository_entries() { + let http = Arc::new(FakeHttp::with([ + response( + 200, + serde_json::to_vec(&json!({ + "authorLogin": "harlan-zw", + "slug": "nuxt", + "skills": [ + { "position": 0, "owner": "vuejs", "repo": "core", "name": "vue", "reason": "The reactive core." }, + { "position": 1, "owner": "nuxt", "repo": "skills", "name": null, "reason": null }, + { "position": 2, "owner": "vuejs", "repo": "core", "name": "vue", "reason": "Listed twice." }, + ] + })) + .unwrap(), + ), + registry_page(&[ + ("nuxt", "skills", "nuxt", Some("Build Nuxt apps.")), + ("nuxt", "other", "ignored", None), + ]), + ])); + let remote = search_remote(http.clone()); + + let listing = remote + .list_skills(&MultiSkillRef::Collection { + login: "harlan-zw".to_owned(), + slug: "nuxt".to_owned(), + }) + .unwrap(); + + assert_eq!( + listing.items, + [ + listed("vuejs", "core", "vue", Some("The reactive core.")), + listed("nuxt", "skills", "nuxt", Some("Build Nuxt apps.")), + ] + ); + assert_eq!( + request_paths(&http), + [ + "/api/collections/by-author/harlan-zw/nuxt", + "/api/skills?owner=nuxt&limit=200", + ] + ); +} + +#[test] +fn a_curator_ref_lists_every_collection_once() { + let collection = |name: &str| { + response( + 200, + serde_json::to_vec(&json!({ + "skills": [ + { "position": 0, "owner": "vuejs", "repo": "core", "name": name, "reason": null }, + { "position": 1, "owner": "vuejs", "repo": "core", "name": "shared", "reason": null }, + ] + })) + .unwrap(), + ) + }; + let http = Arc::new(FakeHttp::with([ + response( + 200, + serde_json::to_vec(&json!({ + "login": "harlan-zw", + "collections": [ + { "slug": "vue", "name": "Vue", "itemCount": 2 }, + { "slug": "nuxt", "name": "Nuxt", "itemCount": 2 }, + ] + })) + .unwrap(), + ), + collection("vue"), + collection("nuxt"), + ])); + let remote = search_remote(http.clone()); + + let listing = remote + .list_skills(&MultiSkillRef::Curator { + login: "harlan-zw".to_owned(), + }) + .unwrap(); + + assert_eq!( + listing.items, + [ + listed("vuejs", "core", "vue", None), + listed("vuejs", "core", "shared", None), + listed("vuejs", "core", "nuxt", None), + ] + ); + assert_eq!( + request_paths(&http), + [ + "/api/curators/harlan-zw", + "/api/collections/by-author/harlan-zw/vue", + "/api/collections/by-author/harlan-zw/nuxt", + ] + ); +} + +/// Serves a curator payload with 100 collections, answers every collection +/// detail with one name-less Repository entry, and serves a runaway owner +/// index to every owner, so only client side caps and memoization can end +/// the listing. +#[derive(Default)] +struct RunawayCuratorHttp { + requests: Mutex>, +} + +const RUNAWAY_CURATOR_COLLECTIONS: usize = 100; +const RUNAWAY_CURATOR_PAGE_CAP: usize = 25; + +/// One curator payload, the capped collection details, and one memoized +/// owner index fetch for the repeated entry spanning the capped pages. +const RUNAWAY_CURATOR_REQUEST_LIMIT: usize = 1 + 2 * RUNAWAY_CURATOR_PAGE_CAP; + +impl HttpAdapter for RunawayCuratorHttp { + fn send( + &self, + request: &HttpRequest, + _cancellation: &dyn Cancellation, + _timeout: Option, + ) -> Result { + let mut requests = self.requests.lock().unwrap(); + requests.push(request.url.clone()); + assert!( + requests.len() <= RUNAWAY_CURATOR_REQUEST_LIMIT, + "the curator listing kept issuing requests: {} requests issued", + requests.len() + ); + if request.url.contains("/api/curators/") { + return Ok(response( + 200, + serde_json::to_vec(&json!({ + "login": "curator", + "collections": (0..RUNAWAY_CURATOR_COLLECTIONS) + .map(|index| { + json!({ + "slug": format!("collection-{index}"), + "name": format!("Collection {index}"), + "itemCount": 1, + }) + }) + .collect::>(), + })) + .unwrap(), + )); + } + if request.url.contains("/api/collections/by-author/") { + return Ok(response( + 200, + serde_json::to_vec(&json!({ + "skills": [{ + "position": 0, + "owner": "big", + "repo": "wanted", + "name": null, + "reason": null, + }] + })) + .unwrap(), + )); + } + let page = json!({ + "items": (0..RUNAWAY_PAGE_ROWS) + .map(|_| json!({ + "name": "wanted", + "owner": "big", + "repo": "wanted", + "description": null, + "stars": 1, + "registryPath": "/gh/big/wanted/wanted", + })) + .collect::>(), + "total": 20_000_000, + "pages": u64::MAX, + }); + Ok(response(200, serde_json::to_vec(&page).unwrap())) + } +} + +#[test] +fn a_runaway_curator_stops_fanning_out_and_fetches_one_repository_once() { + let http = Arc::new(RunawayCuratorHttp::default()); + let remote = SkilldRemote::new( + http.clone(), + Arc::new(NoTokenProvider), + NativeRemoteConfig::Unconfigured, + ) + .with_endpoint("http://127.0.0.1:8787") + .unwrap() + .with_sleeper(Arc::new(NoSleep)); + + let listing = remote + .list_skills(&MultiSkillRef::Curator { + login: "curator".to_owned(), + }) + .unwrap(); + + assert_eq!(listing.items, [listed("big", "wanted", "wanted", None)]); + assert_eq!( + http.requests.lock().unwrap().len(), + RUNAWAY_CURATOR_REQUEST_LIMIT + ); +} + +#[test] +fn a_missing_collection_is_a_source_not_found_error() { + let http = Arc::new(FakeHttp::with([response( + 404, + br#"{"error":true,"statusCode":404,"message":"Collection not found"}"#.to_vec(), + )])); + let remote = search_remote(http); + + let error = remote + .list_skills(&MultiSkillRef::Collection { + login: "harlan-zw".to_owned(), + slug: "missing".to_owned(), + }) + .unwrap_err(); + + assert_eq!(error.code, "SOURCE_NOT_FOUND"); + assert_eq!( + error.message, + "skilld.dev has no collection @harlan-zw/missing" + ); +} + fn skilld_selector() -> RemoteSelector { RemoteSelector::parse("skilld:skilld-dev/skilld/skilld").unwrap() } diff --git a/crates/skilld-command/tests/run.rs b/crates/skilld-command/tests/run.rs index cab56dee..26d39e79 100644 --- a/crates/skilld-command/tests/run.rs +++ b/crates/skilld-command/tests/run.rs @@ -201,7 +201,7 @@ fn load(host: &LocalHost) -> Box { .unwrap() { RunOutcome::Load(skill) => skill, - RunOutcome::Files { .. } => panic!("expected a Skill load"), + RunOutcome::Files { .. } | RunOutcome::Index(_) => panic!("expected a Skill load"), } } @@ -219,7 +219,7 @@ fn pull(host: &LocalHost, wanted: &[&str]) -> Vec { .unwrap() { RunOutcome::Files { files, .. } => files, - RunOutcome::Load(_) => panic!("expected supporting files"), + RunOutcome::Load(_) | RunOutcome::Index(_) => panic!("expected supporting files"), } } diff --git a/crates/skilld-core/src/lib.rs b/crates/skilld-core/src/lib.rs index c02369b9..b376f252 100644 --- a/crates/skilld-core/src/lib.rs +++ b/crates/skilld-core/src/lib.rs @@ -1,4 +1,5 @@ mod lock; +mod reference; mod remote; mod target; mod update; @@ -9,6 +10,7 @@ use std::path::{Path, PathBuf}; pub use lock::{ LockDocument, LockedSkill, LockedSource, LockedTarget, SOURCE_STATUSES, SourceStatus, }; +pub use reference::{ListedSkill, MultiSkillRef, SkillListing, SkillRef}; pub use remote::{ ArtifactAttestation, ArtifactFile, AttestationSignature, CheckOutcome, CheckResult, PreparedFile, RemoteError, RemoteSelector, RepositoryVisibility, ResolvedSource, diff --git a/crates/skilld-core/src/reference.rs b/crates/skilld-core/src/reference.rs new file mode 100644 index 00000000..9207ce6d --- /dev/null +++ b/crates/skilld-core/src/reference.rs @@ -0,0 +1,267 @@ +//! Skill references for `skilld run` and `skilld add`. +//! +//! One argument names either one Skill or a set of Skills. The set forms are +//! the refs skilld.dev prints: a Repository, a curator, or a collection. +//! Parsing happens once here. Every later step trusts the tagged value. + +use std::fmt; + +use crate::remote::{RemoteError, valid_owner, valid_repository}; + +/// One parsed `skilld run` or `skilld add` argument. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SkillRef { + /// One Skill, in any form `skilld install` accepts. + Skill(String), + /// A ref that names more than one Skill. + Many(MultiSkillRef), +} + +/// A ref that names a set of Skills. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum MultiSkillRef { + /// Every Skill one GitHub Repository carries: `gh:OWNER/REPOSITORY`. + Repository { owner: String, repository: String }, + /// Every Skill one curator's collections name: `@LOGIN`. + Curator { login: String }, + /// Every Skill one collection names: `@LOGIN/SLUG`. + Collection { login: String, slug: String }, +} + +/// One Skill a multi-skill ref names, as skilld.dev lists it. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ListedSkill { + pub name: String, + pub owner: String, + pub repository: String, + pub description: Option, +} + +impl ListedSkill { + /// The hosted selector `skilld run` and `skilld install` accept. + pub fn selector(&self) -> String { + format!("skilld:{}/{}/{}", self.owner, self.repository, self.name) + } +} + +/// Every Skill one multi-skill ref names. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SkillListing { + pub reference: MultiSkillRef, + pub items: Vec, +} + +impl SkillRef { + /// Sort one argument into a single Skill source or a multi-skill ref. + /// + /// A single Skill source passes through untouched. `skilld install` parses + /// it later, with its own rules for local paths and remote selectors. + pub fn parse(value: &str) -> Result { + let value = value.trim(); + if let Some(rest) = value.strip_prefix('@') { + return parse_handle(rest).map(Self::Many); + } + if let Some(rest) = value.strip_prefix("gh:") { + return parse_repository(rest, true).map(Self::Many); + } + if let Some(rest) = value.strip_prefix("github:") + && rest.matches('/').count() == 1 + { + return parse_repository(rest, false).map(Self::Many); + } + if value.starts_with("npm:") { + return Err(invalid( + "npm: references are not supported. Use gh:OWNER/REPOSITORY for a Repository, or skilld:OWNER/REPOSITORY/SKILL for one Skill.", + )); + } + Ok(Self::Skill(value.to_owned())) + } +} + +impl MultiSkillRef { + /// The kind word skilld prints and emits in JSON. + pub const fn kind(&self) -> &'static str { + match self { + Self::Repository { .. } => "repository", + Self::Curator { .. } => "curator", + Self::Collection { .. } => "collection", + } + } + + /// The shortest form that parses back to this ref. + pub fn canonical(&self) -> String { + match self { + Self::Repository { owner, repository } => format!("gh:{owner}/{repository}"), + Self::Curator { login } => format!("@{login}"), + Self::Collection { login, slug } => format!("@{login}/{slug}"), + } + } +} + +impl fmt::Display for MultiSkillRef { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(&self.canonical()) + } +} + +fn parse_handle(rest: &str) -> Result { + const GUIDANCE: &str = "Use @LOGIN for a curator, or @LOGIN/SLUG for one collection."; + let mut parts = rest.split('/'); + let login = parts.next().unwrap_or_default(); + if !valid_owner(login) { + return Err(invalid(format!("the curator login is invalid. {GUIDANCE}"))); + } + match (parts.next(), parts.next()) { + (None, None) => Ok(MultiSkillRef::Curator { + login: login.to_owned(), + }), + (Some(slug), None) if valid_slug(slug) => Ok(MultiSkillRef::Collection { + login: login.to_owned(), + slug: slug.to_owned(), + }), + (Some(_), None) => Err(invalid(format!( + "the collection slug is invalid. {GUIDANCE}" + ))), + _ => Err(invalid(format!( + "a collection reference has one slug. {GUIDANCE}" + ))), + } +} + +fn parse_repository(rest: &str, short_prefix: bool) -> Result { + const GUIDANCE: &str = + "Use gh:OWNER/REPOSITORY for a Repository, or skilld:OWNER/REPOSITORY/SKILL for one Skill."; + if rest.contains('#') { + return Err(invalid(format!( + "a Repository reference takes no #reference. {GUIDANCE}" + ))); + } + let mut parts = rest.split('/'); + let owner = parts.next().unwrap_or_default(); + let repository = parts + .next() + .map(|value| value.trim_end_matches(".git")) + .unwrap_or_default(); + if parts.next().is_some() { + let prefix = if short_prefix { "gh:" } else { "github:" }; + return Err(invalid(format!( + "{prefix}OWNER/REPOSITORY names every Skill in a Repository. {GUIDANCE}" + ))); + } + if !valid_owner(owner) || !valid_repository(repository) { + return Err(invalid(format!( + "the GitHub Repository owner or name is invalid. {GUIDANCE}" + ))); + } + Ok(MultiSkillRef::Repository { + owner: owner.to_owned(), + repository: repository.to_owned(), + }) +} + +fn valid_slug(value: &str) -> bool { + !value.is_empty() + && value.len() <= 100 + && !value.contains("..") + && value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.')) +} + +fn invalid(message: impl Into) -> RemoteError { + RemoteError::new("INVALID_SOURCE", message) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn many(value: &str) -> MultiSkillRef { + match SkillRef::parse(value).unwrap() { + SkillRef::Many(reference) => reference, + SkillRef::Skill(skill) => panic!("{value} parsed as one Skill: {skill}"), + } + } + + #[test] + fn parses_every_multi_skill_form() { + let repository = MultiSkillRef::Repository { + owner: "skilld-dev".to_owned(), + repository: "skills".to_owned(), + }; + assert_eq!(many("gh:skilld-dev/skills"), repository); + assert_eq!(many("gh:skilld-dev/skills.git"), repository); + assert_eq!(many("github:skilld-dev/skills"), repository); + assert_eq!( + many("@harlan-zw"), + MultiSkillRef::Curator { + login: "harlan-zw".to_owned() + } + ); + assert_eq!( + many("@harlan-zw/nuxt"), + MultiSkillRef::Collection { + login: "harlan-zw".to_owned(), + slug: "nuxt".to_owned(), + } + ); + } + + #[test] + fn single_skill_forms_pass_through_unchanged() { + for value in [ + "skilld:skilld-dev/skills/vue", + "github:skilld-dev/skilld/skills/skilld", + "github:skilld-dev/skilld/skills/skilld#branch:main", + "https://github.com/skilld-dev/skilld/tree/main/skills/skilld", + "./skills/vue", + "/srv/skills/vue", + "skilld", + ] { + assert_eq!( + SkillRef::parse(value).unwrap(), + SkillRef::Skill(value.to_owned()) + ); + } + } + + #[test] + fn rejects_ambiguous_and_unsupported_forms() { + let cases = [ + ( + "gh:skilld-dev/skills/vue", + "names every Skill in a Repository", + ), + ("gh:skilld-dev", "owner or name is invalid"), + ("gh:skilld-dev/skills#branch:main", "takes no #reference"), + ( + "github:skilld-dev/skills#branch:main", + "takes no #reference", + ), + ("@", "curator login is invalid"), + ("@harlan-zw/nuxt/extra", "one slug"), + ("@harlan-zw/", "collection slug is invalid"), + ("@-bad", "curator login is invalid"), + ("npm:vue", "npm: references are not supported"), + ("gh:skilld-dev/skills\u{1b}[0m", "owner or name is invalid"), + ("@harlan\u{202e}zw", "curator login is invalid"), + ]; + for (value, expected) in cases { + let error = SkillRef::parse(value).unwrap_err(); + assert_eq!(error.code, "INVALID_SOURCE", "{value}"); + assert!( + error.message.contains(expected), + "{value}: {}", + error.message + ); + } + } + + #[test] + fn canonical_forms_round_trip() { + for value in ["gh:skilld-dev/skills", "@harlan-zw", "@harlan-zw/nuxt"] { + assert_eq!(many(value).canonical(), value); + } + assert_eq!(many("github:a/b").canonical(), "gh:a/b"); + } +} diff --git a/crates/skilld-core/src/remote.rs b/crates/skilld-core/src/remote.rs index 87e748b8..a7942691 100644 --- a/crates/skilld-core/src/remote.rs +++ b/crates/skilld-core/src/remote.rs @@ -221,22 +221,28 @@ fn split_three(value: &str) -> Result<(&str, &str, &str), RemoteError> { } } -fn validate_source_request(source: &SourceRequest) -> Result<(), RemoteError> { - let valid_owner = !source.owner.is_empty() - && source.owner.len() <= 39 - && !source.owner.starts_with('-') - && !source.owner.ends_with('-') - && source - .owner +/// Whether a value is a valid GitHub account login. +pub(crate) fn valid_owner(value: &str) -> bool { + !value.is_empty() + && value.len() <= 39 + && !value.starts_with('-') + && !value.ends_with('-') + && value .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-'); - let valid_repository = !source.repository.is_empty() - && source.repository.len() <= 100 - && source - .repository + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-') +} + +/// Whether a value is a valid GitHub Repository name. +pub(crate) fn valid_repository(value: &str) -> bool { + !value.is_empty() + && value.len() <= 100 + && value .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.')); - if !valid_owner || !valid_repository { + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.')) +} + +fn validate_source_request(source: &SourceRequest) -> Result<(), RemoteError> { + if !valid_owner(&source.owner) || !valid_repository(&source.repository) { return Err(RemoteError::new( "INVALID_SOURCE", "the GitHub Repository owner or name is invalid", @@ -1098,7 +1104,7 @@ fn validate_relative_path(value: &str, maximum: usize) -> Result<(), RemoteError } } -fn is_unsafe_terminal(character: char) -> bool { +pub(crate) fn is_unsafe_terminal(character: char) -> bool { let code = u32::from(character); character.is_control() || matches!( diff --git a/docs/migrate-v2-to-v3.md b/docs/migrate-v2-to-v3.md index cb3e17d3..16e32529 100644 --- a/docs/migrate-v2-to-v3.md +++ b/docs/migrate-v2-to-v3.md @@ -128,6 +128,8 @@ It cannot restore a v2 lockfile. | v2 command | v3 replacement | | --- | --- | | `skilld add ` | Run `skilld search`, then `skilld run ` to use it once, or `skilld install ` to keep it | +| `skilld add gh:OWNER/REPOSITORY`, `skilld add @LOGIN`, `skilld add @LOGIN/SLUG` | Unchanged. `skilld run` with the same ref lists the Skills first | +| `skilld add npm:PACKAGE` | Not supported. Run `skilld search `, then use the selector | | `skilld update [name]` | `skilld update [name]` | | `skilld info` | `skilld list`, then `skilld view ` | | `skilld login` | `skilld auth login` | diff --git a/skills/skilld/SKILL.md b/skills/skilld/SKILL.md index f82d648e..18bbc086 100644 --- a/skills/skilld/SKILL.md +++ b/skills/skilld/SKILL.md @@ -1,6 +1,6 @@ --- name: skilld -description: Operate skilld CLI for Skill discovery, use, installation, inspection, updates, authentication, configuration, restoration, and removal. +description: Operate skilld CLI for Skill discovery, use, installation, inspection, updates, authentication, configuration, restoration, and removal, including Repository, curator, and collection refs. --- # Use skilld CLI @@ -75,6 +75,25 @@ A `verified` status covers where the Skill came from. It does not cover what the instructions ask you to do. If the status is `unverified`, tell the user before you follow the Skill. +## List the Skills a Repository, curator, or collection names + +skilld.dev prints refs that name several Skills: + +- `gh:OWNER/REPOSITORY` names every Skill in one Repository. +- `@LOGIN` names every Skill in one curator's collections. +- `@LOGIN/SLUG` names every Skill in one collection. + +Run one of these refs to list its Skills: + +```sh +skilld run @LOGIN/SLUG --json +``` + +The command prints an index and loads no Skill. +Read `data.items` for each Skill's `name`, `owner`, `repository`, `description`, and `selector`. +Run the `data.items[].runArgv` array to load one Skill. +Pick the Skills the current task needs. Do not run every Skill in the index. + ## Choose the source Prefer the exact `skilld:` selector returned by Skill search. @@ -129,6 +148,18 @@ Install this skilld-maintained Skill globally: skilld install skilld --global ``` +Install every Skill a Repository, curator, or collection names: + +```sh +skilld add gh:OWNER/REPOSITORY +skilld add @LOGIN/SLUG --global +``` + +`skilld add` accepts `--global`, `--agent`, and `--mode` like `skilld install`. +It prints one `Installed Skill` line per Skill. +Run `skilld run` with the same ref first, then confirm the list with the user. +`skilld add` with one Skill selector installs that Skill like `skilld install`. + Always use the source selector shown by `skilld search`. After installation, report the Skill name, scope, Agent targets, and source status.