feat(cli): show author and source after run and install

crates/skilld-command/src/lib.rs

@@ -3,6 +3,7 @@ mod local_store;
 mod outdated;
 pub use outdated::{NoOutdatedProgress, OutdatedProgress, ancestor_roots};
 mod output;
+mod provenance;
 mod remote;
 mod run;
 
@@ -21,6 +22,8 @@ pub use local_store::{
     TargetInstall, TransactionGate,
 };
 pub use output::{CommandPlatform, OutputContext};
+pub use provenance::RemoteProvenance;
+use provenance::source_status_caution;
 pub use remote::{
     Cancellation, HeaderValue, HttpAdapter, HttpHeader, HttpMethod, HttpRequest, HttpResponse,
     NativeRemoteConfig, NeverCancelled, NoRemoteProgress, NoTokenProvider, PreparedRemoteSkill,
@@ -34,9 +37,9 @@ 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,
+    NotTrackedReason, 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};
@@ -201,12 +204,28 @@ enum ConfigCommand {
     List,
 }
 
+/// One Skill an install wrote, with the source the lockfile now records.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct InstalledSkill {
+    pub name: String,
+    pub source: LockedSource,
+    /// `verified`, `local`, or `unverified`.
+    pub source_status: &'static str,
+}
+
 pub trait Host {
     fn list(&self, scope: InstallScope) -> Result<Vec<String>, CommandError>;
 
-    fn install(&self, source: InstallSource, scope: InstallScope) -> Result<String, CommandError>;
+    fn install(
+        &self,
+        source: InstallSource,
+        scope: InstallScope,
+    ) -> Result<InstalledSkill, CommandError>;
 
-    fn install_request(&self, request: InstallRequest) -> Result<Vec<String>, CommandError> {
+    fn install_request(
+        &self,
+        request: InstallRequest,
+    ) -> Result<Vec<InstalledSkill>, CommandError> {
         if !request.targets.is_empty() || request.mode.is_some() {
             return Err(CommandError::unsupported_host(
                 "Agent target selection is unavailable on this host",
@@ -217,7 +236,7 @@ pub trait Host {
                 "lockfile restore is unavailable",
             ));
         };
-        self.install(source, request.scope).map(|name| vec![name])
+        self.install(source, request.scope).map(|skill| vec![skill])
     }
 
     fn run_skill(
@@ -825,18 +844,15 @@ fn dispatch<H: Host>(
                 .map(InstallMode::parse)
                 .transpose()
                 .map_err(CommandError::domain)?;
-            let names = host.install_request(InstallRequest {
+            let installed = host.install_request(InstallRequest {
                 operation,
                 scope,
                 targets,
                 mode,
             })?;
-            let mut lines = names
-                .into_iter()
-                .map(|name| Line::success(format!("Installed Skill {name}.")))
-                .collect::<Vec<_>>();
-            if direct {
-                lines.push(Line::hint("Review the unverified Skill before use."));
+            let mut lines = Vec::new();
+            for skill in &installed {
+                lines.extend(render_installed(skill)?);
             }
             Ok(CommandOutput::Screen(Screen::new(lines)))
         }
@@ -959,6 +975,8 @@ fn dispatch<H: Host>(
                         name: result.name,
                         selector: selector.to_string(),
                         description: result.description,
+                        owner: result.source.owner,
+                        repository: result.source.repository,
                         stargazer_count: result.stargazer_count,
                     })
                 })
@@ -996,15 +1014,48 @@ fn dispatch<H: Host>(
     }
 }
 
-fn render_view(view: SkillView) -> Result<Vec<Line>, CommandError> {
-    let source = match view.skill.source {
-        LockedSource::Local { path } => Line::field("Source", format!("local {path}")),
-        LockedSource::BundledSkilld => Line::field("Source", "skilld-maintained Skill"),
-        LockedSource::Remote { source, .. } => {
-            let selector = RemoteSelector::parse(&source).map_err(CommandError::remote)?;
-            Line::linked_field("Source", selector.canonical(), github_url(&selector)?)
+/// Say who published the Skill, where its bytes came from, and what the
+/// status means. Every status gets its meaning; a remote source gets the
+/// exact SKILL.md on GitHub.
+fn render_installed(skill: &InstalledSkill) -> Result<Vec<Line>, CommandError> {
+    let provenance = RemoteProvenance::from_locked(&skill.source)?;
+    let mut lines = vec![Line::success(format!("Installed Skill {}.", skill.name))];
+    if let Some(provenance) = &provenance {
+        lines.push(Line::item(provenance.headline(&skill.name)));
+    }
+    lines.push(source_line(&skill.source, provenance.as_ref()));
+    lines.push(Line::field("Source status", skill.source_status));
+    lines.extend(
+        source_status_caution(skill.source_status)
+            .lines()
+            .map(Line::hint),
+    );
+    if let Some(provenance) = &provenance {
+        lines.push(Line::linked_field(
+            "Read it first",
+            provenance.source_url.clone(),
+            provenance.source_url.clone(),
+        ));
+    }
+    Ok(lines)
+}
+
+/// The `Source` row. A remote source links to its exact SKILL.md when the
+/// terminal supports hyperlinks.
+fn source_line(source: &LockedSource, provenance: Option<&RemoteProvenance>) -> Line {
+    match (source, provenance) {
+        (LockedSource::Local { path }, _) => Line::field("Source", format!("local {path}")),
+        (LockedSource::BundledSkilld, _) => Line::field("Source", "skilld-maintained Skill"),
+        (LockedSource::Remote { source, .. }, Some(provenance)) => {
+            Line::linked_field("Source", source, provenance.source_url.clone())
         }
-    };
+        (LockedSource::Remote { source, .. }, None) => Line::field("Source", source),
+    }
+}
+
+fn render_view(view: SkillView) -> Result<Vec<Line>, CommandError> {
+    let provenance = RemoteProvenance::from_locked(&view.skill.source)?;
+    let source = source_line(&view.skill.source, provenance.as_ref());
     let targets = if view.skill.targets.is_empty() {
         "none".to_owned()
     } else {
@@ -1024,17 +1075,6 @@ fn render_view(view: SkillView) -> Result<Vec<Line>, CommandError> {
     ])
 }
 
-/// Build a GitHub repository URL from a parsed remote selector.
-fn github_url(selector: &RemoteSelector) -> Result<String, CommandError> {
-    let mut url = url::Url::parse("https://github.com/")
-        .map_err(|_| CommandError::service("the GitHub Repository URL could not be built"))?;
-    url.path_segments_mut()
-        .map_err(|_| CommandError::service("the GitHub Repository URL could not be built"))?
-        .push(&selector.source().owner)
-        .push(&selector.source().repository);
-    Ok(url.into())
-}
-
 fn scope(global: bool) -> InstallScope {
     if global {
         InstallScope::Global
@@ -1324,7 +1364,7 @@ impl LocalHost {
         scope: InstallScope,
         targets: &[TargetInstall],
         known: &[ResolvedTarget],
-    ) -> Result<String, CommandError> {
+    ) -> Result<InstalledSkill, CommandError> {
         let selector = skilld_core::RemoteSelector::parse(source).map_err(CommandError::remote)?;
         let prepared = self
             .remote_provider()?
@@ -1341,7 +1381,25 @@ impl LocalHost {
                 known,
             )
             .map_err(CommandError::store)?;
-        Ok(name.to_string())
+        self.installed(scope, &name, known)
+    }
+
+    /// Read back what the lockfile recorded for one installed Skill.
+    fn installed(
+        &self,
+        scope: InstallScope,
+        name: &skilld_core::SkillName,
+        known: &[ResolvedTarget],
+    ) -> Result<InstalledSkill, CommandError> {
+        let view = self
+            .store(scope)
+            .view(name, known)
+            .map_err(CommandError::store)?;
+        Ok(InstalledSkill {
+            name: view.name,
+            source: view.skill.source,
+            source_status: view.skill.source_status.as_str(),
+        })
     }
 
     fn run_remote(
@@ -1406,6 +1464,12 @@ impl LocalHost {
         ))
         .map_err(CommandError::remote)?
         .canonical();
+        let provenance = RemoteProvenance::new(
+            locked_selector.source().owner.as_str(),
+            locked_selector.source().repository.as_str(),
+            skill_path.as_str(),
+            revision.as_str(),
+        )?;
         let source_status = prepared.source_status.as_str();
         let (name, _, files) =
             skilld_core::prepare_unverified_files(prepared.files).map_err(CommandError::remote)?;
@@ -1414,6 +1478,7 @@ impl LocalHost {
             source: selector.canonical(),
             exact_source,
             direct,
+            provenance: Box::new(provenance),
         };
         if !wanted.is_empty() {
             return Ok(RunOutcome::Files {
@@ -1499,7 +1564,11 @@ impl LocalHost {
         })))
     }
 
-    fn restore(&self, request: &InstallRequest, direct: bool) -> Result<Vec<String>, CommandError> {
+    fn restore(
+        &self,
+        request: &InstallRequest,
+        direct: bool,
+    ) -> Result<Vec<InstalledSkill>, CommandError> {
         let (targets, known) = if request.targets.is_empty() {
             (None, self.known_targets(request.scope)?)
         } else {
@@ -1621,7 +1690,7 @@ impl LocalHost {
                         .map_err(CommandError::store)?;
                 }
             }
-            restored.push(name);
+            restored.push(self.installed(request.scope, &skill_name, &known)?);
         }
         Ok(restored)
     }
@@ -1633,7 +1702,11 @@ impl Host for LocalHost {
         self.store(scope).list(&known).map_err(CommandError::store)
     }
 
-    fn install(&self, source: InstallSource, scope: InstallScope) -> Result<String, CommandError> {
+    fn install(
+        &self,
+        source: InstallSource,
+        scope: InstallScope,
+    ) -> Result<InstalledSkill, CommandError> {
         self.install_request(InstallRequest {
             operation: InstallOperation::Install(source),
             scope,
@@ -1645,7 +1718,10 @@ impl Host for LocalHost {
         .ok_or_else(|| CommandError::service("Skill install returned no result"))
     }
 
-    fn install_request(&self, request: InstallRequest) -> Result<Vec<String>, CommandError> {
+    fn install_request(
+        &self,
+        request: InstallRequest,
+    ) -> Result<Vec<InstalledSkill>, CommandError> {
         let source = match request.operation.clone() {
             InstallOperation::Restore => return self.restore(&request, false),
             InstallOperation::DirectRestore => return self.restore(&request, true),
@@ -1655,17 +1731,17 @@ impl Host for LocalHost {
         match source {
             InstallSource::Remote(source) => self
                 .install_remote(&source, false, request.scope, &targets, &known)
-                .map(|name| vec![name]),
+                .map(|skill| vec![skill]),
             InstallSource::DirectRemote(source) => self
                 .install_remote(&source, true, request.scope, &targets, &known)
-                .map(|name| vec![name]),
+                .map(|skill| vec![skill]),
             source => {
                 let (source, locked_source) = self.resolve_source(source)?;
                 let name = self
                     .store(request.scope)
                     .install_from(&source, locked_source, &targets, &known)
                     .map_err(CommandError::store)?;
-                Ok(vec![name.to_string()])
+                Ok(vec![self.installed(request.scope, &name, &known)?])
             }
         }
     }
@@ -3101,20 +3177,31 @@ mod tests {
             &self,
             source: InstallSource,
             scope: InstallScope,
-        ) -> Result<String, CommandError> {
+        ) -> Result<InstalledSkill, CommandError> {
             assert_eq!(source, InstallSource::BundledSkilld);
             assert_eq!(scope, InstallScope::Global);
-            Ok("skilld".to_owned())
+            Ok(bundled_skilld())
         }
 
-        fn install_request(&self, request: InstallRequest) -> Result<Vec<String>, CommandError> {
+        fn install_request(
+            &self,
+            request: InstallRequest,
+        ) -> Result<Vec<InstalledSkill>, CommandError> {
             assert_eq!(
                 request.operation,
                 InstallOperation::Install(InstallSource::BundledSkilld)
             );
             assert_eq!(request.scope, InstallScope::Global);
             assert_eq!(request.targets, [AgentTargetId::Codex]);
-            Ok(vec!["skilld".to_owned()])
+            Ok(vec![bundled_skilld()])
+        }
+    }
+
+    fn bundled_skilld() -> InstalledSkill {
+        InstalledSkill {
+            name: "skilld".to_owned(),
+            source: LockedSource::BundledSkilld,
+            source_status: "local",
         }
     }
 
@@ -3165,7 +3252,12 @@ mod tests {
         assert_eq!(result.exit_code, 0);
         assert_eq!(
             String::from_utf8(stdout).unwrap(),
-            "Installed Skill skilld.\n"
+            concat!(
+                "Installed Skill skilld.\n",
+                "Source: skilld-maintained Skill\n",
+                "Source status: local\n",
+                "Read this Skill before you follow it.\n",
+            )
         );
         assert!(stderr.is_empty());
     }