feat(cli): link every remote Skill to its exact source (#130)

Author: harlan-zw

crates/skilld-command/src/lib.rs

@@ -4,6 +4,7 @@ use skilld_core::{ListedSkill, SkillListing, UpdatePlanV1};
 use skilld_ui::text::{grouped_number, is_unsafe_terminal, sanitize, width, wrap};
 use skilld_ui::{Role, paint};
 
+use crate::provenance::{RemoteProvenance, source_status_caution};
 use crate::run::{FileContent, PulledFile, RunOutcome, SkillOrigin, TransientSkill};
 use crate::{CommandError, CommandErrorKind};
 
@@ -115,9 +116,18 @@ pub(crate) struct SearchItem {
     pub name: String,
     pub selector: String,
     pub description: Option<String>,
+    pub owner: String,
+    pub repository: String,
     pub stargazer_count: u64,
 }
 
+impl SearchItem {
+    /// `owner/repository`, the way GitHub names it.
+    fn slug(&self) -> String {
+        format!("{}/{}", self.owner, self.repository)
+    }
+}
+
 pub(crate) fn render_search(
     outcome: &SearchOutcome,
     mode: OutputMode,
@@ -247,6 +257,8 @@ fn render_plain(outcome: &SearchOutcome) -> String {
         output.push('\t');
         output.push_str(&escape_plain(&item.selector));
         output.push('\t');
+        output.push_str(&escape_plain(&item.slug()));
+        output.push('\t');
         output.push_str(&item.stargazer_count.to_string());
         output.push('\t');
         output.push_str(&escape_plain(
@@ -296,23 +308,32 @@ fn render_human(
     for item in &outcome.items {
         output.push('\n');
         let name = sanitize(&item.name);
+        let slug = sanitize(&item.slug());
         let stars = format!("{} stars", grouped_number(item.stargazer_count));
-        if 2 + width(&name) + 2 + width(&stars) <= columns {
-            let gap = columns - 2 - width(&name) - width(&stars);
+        let meta = format!(
+            "{} · {}",
+            paint(&slug, Role::Dim, color),
+            paint(&stars, Role::Warn, color)
+        );
+        let meta_width = width(&slug) + 3 + width(&stars);
+        if 2 + width(&name) + 2 + meta_width <= columns {
+            let gap = columns - 2 - width(&name) - meta_width;
             output.push_str("  ");
             output.push_str(&paint(&name, Role::Emphasis, color));
             output.push_str(&" ".repeat(gap));
-            output.push_str(&paint(&stars, Role::Warn, color));
+            output.push_str(&meta);
             output.push('\n');
         } else {
             for line in wrap(&name, columns.saturating_sub(2)) {
                 output.push_str("  ");
                 output.push_str(&paint(&line, Role::Emphasis, color));
                 output.push('\n');
             }
-            output.push_str("  ");
-            output.push_str(&paint(&stars, Role::Warn, color));
-            output.push('\n');
+            for line in wrap(&format!("{slug} · {stars}"), columns.saturating_sub(2)) {
+                output.push_str("  ");
+                output.push_str(&paint(&line, Role::Dim, color));
+                output.push('\n');
+            }
         }
 
         if let Some(description) = &item.description {
@@ -588,9 +609,17 @@ fn render_load(skill: &TransientSkill, color: bool, platform: CommandPlatform) -
             out.push_str("skilld wrote no Skill files.\n");
             out.push_str(&field("Source", "skilld-maintained Skill", color));
         }
-        SkillOrigin::Remote { source, .. } => {
+        SkillOrigin::Remote {
+            source, provenance, ..
+        } => {
             out.push_str("skilld retained no Skill files.\n");
             out.push_str("It created no lockfile entry, Agent target, or project file.\n");
+            out.push_str(&paint(
+                &sanitize(&provenance.headline(&skill.name)),
+                Role::Emphasis,
+                color,
+            ));
+            out.push('\n');
             out.push_str(&field("Source", source, color));
         }
         SkillOrigin::Local { root } => {
@@ -603,6 +632,7 @@ fn render_load(skill: &TransientSkill, color: bool, platform: CommandPlatform) -
     }
     out.push_str(&field("Source status", skill.source_status, color));
     out.push_str(source_status_caution(skill.source_status));
+    out.push_str(&read_it_first(&skill.origin, color));
 
     out.push('\n');
     out.push_str(&paint("--- SKILL.md ---", Role::Dim, color));
@@ -699,6 +729,7 @@ fn render_files(
     }
     out.push_str(&field("Source status", source_status, color));
     out.push_str(source_status_caution(source_status));
+    out.push_str(&read_it_first(origin, color));
     out.push('\n');
     for file in files {
         let path = sanitize(&file.path);
@@ -849,17 +880,14 @@ fn shell_quote(argument: &str, platform: CommandPlatform) -> String {
     }
 }
 
-/// State what the status covers, on every status.
-///
-/// A verified Artifact proves where the bytes came from. It says nothing about
-/// what the instructions ask an Agent to do, and the output must not imply it.
-fn source_status_caution(status: &str) -> &'static str {
-    match status {
-        "verified" => {
-            "skilld checked where this Skill came from, not what it asks you to do.\nRead it before you follow it.\n"
+/// Point at the exact SKILL.md on GitHub. Local and bundled Skills sit on
+/// disk already, so they get no link.
+fn read_it_first(origin: &SkillOrigin, color: bool) -> String {
+    match origin {
+        SkillOrigin::Remote { provenance, .. } => {
+            field("Read it first", &provenance.source_url, color)
         }
-        "unverified" => "skilld did not check this source. Read this Skill before you follow it.\n",
-        _ => "Read this Skill before you follow it.\n",
+        SkillOrigin::Bundled | SkillOrigin::Local { .. } => String::new(),
     }
 }
 
@@ -884,19 +912,51 @@ fn safe_terminal_text(value: &str) -> String {
 
 #[derive(Serialize)]
 #[serde(tag = "_tag", rename_all = "lowercase")]
+#[serde(rename_all_fields = "camelCase")]
 enum JsonOrigin {
-    Bundled { source: &'static str },
-    Remote { source: String, direct: bool },
-    Local { root: String },
+    Bundled {
+        source: &'static str,
+    },
+    Remote {
+        source: String,
+        direct: bool,
+        owner: String,
+        repository: String,
+        skill_path: String,
+        commit: String,
+        source_url: String,
+    },
+    Local {
+        root: String,
+    },
 }
 
 fn origin_json(origin: &SkillOrigin) -> JsonOrigin {
     match origin {
         SkillOrigin::Bundled => JsonOrigin::Bundled { source: "skilld" },
-        SkillOrigin::Remote { source, direct, .. } => JsonOrigin::Remote {
-            source: source.clone(),
-            direct: *direct,
-        },
+        SkillOrigin::Remote {
+            source,
+            direct,
+            provenance,
+            ..
+        } => {
+            let RemoteProvenance {
+                owner,
+                repository,
+                skill_path,
+                commit_sha,
+                source_url,
+            } = provenance.as_ref();
+            JsonOrigin::Remote {
+                source: source.clone(),
+                direct: *direct,
+                owner: owner.clone(),
+                repository: repository.clone(),
+                skill_path: skill_path.clone(),
+                commit: commit_sha.clone(),
+                source_url: source_url.clone(),
+            }
+        }
         SkillOrigin::Local { root } => JsonOrigin::Local {
             root: root.display().to_string(),
         },

crates/skilld-command/src/output.rs

@@ -0,0 +1,115 @@
+//! Where a remote Skill came from: the human, the Repository, and the exact file.
+//!
+//! Every surface that shows a remote Skill points at the SKILL.md the author
+//! committed. The lockfile records the Repository, path, and commit; this module
+//! turns those into one line a person can read and one URL they can open.
+
+use skilld_core::{LockedSource, RemoteSelector};
+
+use crate::CommandError;
+
+/// The Repository, path, and commit that one remote Skill was read from.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct RemoteProvenance {
+    pub owner: String,
+    pub repository: String,
+    pub skill_path: String,
+    pub commit_sha: String,
+    /// The SKILL.md file at the exact commit, on github.com.
+    pub source_url: String,
+}
+
+impl RemoteProvenance {
+    pub fn new(
+        owner: impl Into<String>,
+        repository: impl Into<String>,
+        skill_path: impl Into<String>,
+        commit_sha: impl Into<String>,
+    ) -> Result<Self, CommandError> {
+        let owner = owner.into();
+        let repository = repository.into();
+        let skill_path = skill_path.into();
+        let commit_sha = commit_sha.into();
+        let source_url = source_url(&owner, &repository, &skill_path, &commit_sha)?;
+        Ok(Self {
+            owner,
+            repository,
+            skill_path,
+            commit_sha,
+            source_url,
+        })
+    }
+
+    /// Read the provenance a lockfile entry recorded. Local and bundled
+    /// Skills have no remote source, so they carry none.
+    pub fn from_locked(source: &LockedSource) -> Result<Option<Self>, CommandError> {
+        let LockedSource::Remote {
+            source,
+            commit_sha,
+            skill_path,
+        } = source
+        else {
+            return Ok(None);
+        };
+        let selector = RemoteSelector::parse(source).map_err(CommandError::remote)?;
+        Self::new(
+            selector.source().owner.as_str(),
+            selector.source().repository.as_str(),
+            skill_path.as_str(),
+            commit_sha.as_str(),
+        )
+        .map(Some)
+    }
+
+    /// `owner/repository`, the way GitHub names it.
+    pub fn slug(&self) -> String {
+        format!("{}/{}", self.owner, self.repository)
+    }
+
+    /// The first seven characters of the commit, for reading. The URL keeps the full commit.
+    pub fn short_commit(&self) -> &str {
+        self.commit_sha.get(..7).unwrap_or(&self.commit_sha)
+    }
+
+    /// One line: the Skill, the human who published it, and the commit.
+    pub fn headline(&self, name: &str) -> String {
+        format!("{name} · {} @ {}", self.slug(), self.short_commit())
+    }
+}
+
+/// State what the status covers, on every status.
+///
+/// A verified Artifact proves where the bytes came from. It says nothing about
+/// what the instructions ask an Agent to do, and the output must not imply it.
+pub fn source_status_caution(status: &str) -> &'static str {
+    match status {
+        "verified" => {
+            "skilld checked where this Skill came from, not what it asks you to do.\nRead it before you follow it.\n"
+        }
+        "unverified" => "skilld did not check this source. Read this Skill before you follow it.\n",
+        _ => "Read this Skill before you follow it.\n",
+    }
+}
+
+fn source_url(
+    owner: &str,
+    repository: &str,
+    skill_path: &str,
+    commit_sha: &str,
+) -> Result<String, CommandError> {
+    let failed = || CommandError::service("the GitHub Skill URL could not be built");
+    let mut url = url::Url::parse("https://github.com/").map_err(|_| failed())?;
+    {
+        let mut segments = url.path_segments_mut().map_err(|_| failed())?;
+        segments
+            .push(owner)
+            .push(repository)
+            .push("blob")
+            .push(commit_sha);
+        for segment in skill_path.split('/').filter(|segment| !segment.is_empty()) {
+            segments.push(segment);
+        }
+        segments.push(crate::run::INSTRUCTIONS_FILE);
+    }
+    Ok(url.into())
+}

crates/skilld-command/src/provenance.rs

@@ -15,6 +15,7 @@ use skilld_core::{PreparedFile, SkillListing};
 use skilld_ui::text::is_unsafe_terminal;
 
 use crate::CommandError;
+use crate::provenance::RemoteProvenance;
 
 /// The instructions file every Skill carries.
 pub const INSTRUCTIONS_FILE: &str = "SKILL.md";
@@ -37,6 +38,8 @@ pub enum SkillOrigin {
         source: String,
         exact_source: String,
         direct: bool,
+        /// The Repository, path, and commit the bytes came from.
+        provenance: Box<RemoteProvenance>,
     },
 }
 

crates/skilld-command/src/run.rs

@@ -145,7 +145,20 @@ fn add_installs_every_skill_the_repository_ref_names() {
     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"
+        concat!(
+            "Installed Skill vue.\n",
+            "vue · vuejs/core @ aaaaaaa\n",
+            "Source: skilld:vuejs/core/vue\n",
+            "Source status: unverified\n",
+            "skilld did not check this source. Read this Skill before you follow it.\n",
+            "Read it first: https://github.com/vuejs/core/blob/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/skills/vue/SKILL.md\n",
+            "Installed Skill nuxt.\n",
+            "nuxt · vuejs/core @ aaaaaaa\n",
+            "Source: skilld:vuejs/core/nuxt\n",
+            "Source status: unverified\n",
+            "skilld did not check this source. Read this Skill before you follow it.\n",
+            "Read it first: https://github.com/vuejs/core/blob/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/skills/nuxt/SKILL.md\n",
+        )
     );
     assert_eq!(host.list(InstallScope::Project).unwrap(), ["nuxt", "vue"]);
     for name in ["vue", "nuxt"] {

crates/skilld-command/tests/add.rs

@@ -71,7 +71,15 @@ fn every_project_signal_selects_the_matching_agent_target() {
             })
             .unwrap();
 
-        assert_eq!(names, ["example"], "{}", agent.as_str());
+        assert_eq!(
+            names
+                .iter()
+                .map(|skill| skill.name.as_str())
+                .collect::<Vec<_>>(),
+            ["example"],
+            "{}",
+            agent.as_str()
+        );
         assert!(
             project.join(skills_dir).join("example/SKILL.md").exists(),
             "{}",
@@ -215,6 +223,7 @@ fn a_first_party_skills_directory_alone_does_not_select_openclaw() {
         })
         .unwrap();
 
+    let names: Vec<&str> = names.iter().map(|skill| skill.name.as_str()).collect();
     assert_eq!(names, ["example"]);
     let skills = fs::read_dir(project.join("skills"))
         .unwrap()

crates/skilld-command/tests/agent_targets.rs

@@ -436,7 +436,13 @@ fn view_and_outdated_preserve_valid_metacharacters_as_quoted_data() {
     assert_eq!(view.exit_code, 0);
     assert!(stderr.is_empty());
     assert!(stdout.contains(source));
-    assert!(stdout.contains("\u{1b}]8;;https://github.com/skilld-dev/skills\u{1b}\\"));
+    assert!(
+        stdout.contains(concat!(
+            "\u{1b}]8;;https://github.com/skilld-dev/skills/blob/",
+            "0123456789abcdef0123456789abcdef01234567/skills/o'hare$(%22quoted%22)/SKILL.md\u{1b}\\"
+        )),
+        "{stdout:?}"
+    );
 
     let mut stdout = Vec::new();
     let mut stderr = Vec::new();