feat(cli): name each model reading in the approval message

GLOSSARY.md

@@ -23,6 +23,7 @@ Every public export, command, error, route, and document uses these terms.
 | Check result | `skilld.dev/api/v1` | published protocol | skilld CLI, developer | check result |
 | Linked file | `skilld.dev/api/v1` | published protocol | skilld CLI | linked file |
 | behavior | `packages/protocol/rules/skill-behaviors.json`, `skilld run` | published value | Agent, developer | behavior |
+| behavior reading | `behavior-review` check result, `BEHAVIOR_CONFIRMATION_REQUIRED` | published value | Agent, developer | model reading |
 | Source status | lockfile and protocol | published value | skilld CLI, CI | source status |
 | Skill page | `skilld.dev` and skilld CLI output | published value | Agent, developer | Skill page |
 | Update relation | skilld CLI JSON v1 | published value | Agent, developer, CI | update relation |
@@ -298,6 +299,16 @@ The digest reports changes in watched Repositories.
 
 **Casing:** `behavior` in prose, `Skill behaviors` as the output label, kebab-case ids such as `remote-code`.
 
+### behavior reading
+
+**Is:** a language model's reading of one `ask` behavior match, from the non-required `behavior-review` check result skilld.dev signs into an attestation. It names a verdict, `instruction`, `quoted example`, `prohibition`, `documentation`, or `unclear`, and a reason.
+
+**Use for:** the parenthesis after a match in `BEHAVIOR_CONFIRMATION_REQUIRED`, labelled `model reading`.
+
+**Never:** review result, scan, clearance, "safe", "false positive". A behavior reading never changes whether a behavior needs approval.
+
+**Casing:** `behavior reading` in prose, `model reading` as the output label.
+
 ### Source status
 
 **Is:** the recorded provenance state for an installed Skill.

README.md

@@ -280,6 +280,11 @@ skilld run 'github:OWNER/REPOSITORY/SKILL#commit:COMMIT' --allow remote-code
 The command pins the commit that skilld checked.
 A local or bundled Skill lists its behaviors and never stops.
 
+A Skill that skilld.dev delivered can carry a model reading for each match:
+what a language model read the line as, such as a quoted example in a security guide, and why.
+The message adds it after the match, as `SKILL.md:7 (model reading: quoted example. REASON)`.
+A model reading is no guarantee and changes no approval.
+
 `install`, `add`, and `update` stop the same way and write nothing.
 Run the same command again with the `--allow` ids the message names.
 `add` installs every other Skill the ref names.

crates/skilld-command/src/lib.rs

@@ -2134,17 +2134,12 @@ fn gate_behaviors<H: Host>(
                 .chain(held.iter().map(|behavior| behavior.id))
                 .collect::<Vec<_>>();
             let command = output::shell_command(&output::allow_argv(&skill.origin, &ids), platform);
-            let behaviors = held
-                .iter()
-                .map(|behavior| run::describe_behavior(behavior))
-                .collect::<Vec<_>>()
-                .join("; ");
+            let (behaviors, caveat) = describe_held(&held, &skill.behavior_readings);
             Err(CommandError::operation(
                 "BEHAVIOR_CONFIRMATION_REQUIRED",
                 format!(
-                    "The Skill {} needs the user's approval before it loads. skilld loaded nothing. Behaviors: {behaviors}. {} Show these behaviors to the user. If the user approves, run: {command}",
+                    "The Skill {} needs the user's approval before it loads. skilld loaded nothing. Behaviors: {behaviors}. {caveat} Show these behaviors to the user. If the user approves, run: {command}",
                     skill.name,
-                    run::BEHAVIOR_CAVEAT,
                 ),
             ))
         }
@@ -2170,6 +2165,7 @@ fn gate_new_behaviors(
     behaviors: &[skilld_core::Behavior],
     approved_before: &[skilld_core::Behavior],
     allowed: &[String],
+    readings: &[run::BehaviorReading],
 ) -> Result<(), CommandError> {
     let held = run::held_behaviors(behaviors, allowed, approved_before);
     if held.is_empty() {
@@ -2188,11 +2184,7 @@ fn gate_new_behaviors(
                 .chain(held.iter().map(|behavior| behavior.id))
                 .collect::<Vec<_>>()
                 .join(",");
-            let behaviors = held
-                .iter()
-                .map(|behavior| run::describe_behavior(behavior))
-                .collect::<Vec<_>>()
-                .join("; ");
+            let (behaviors, caveat) = describe_held(&held, readings);
             let opening = match change {
                 BehaviorChange::Install => {
                     format!("The Skill {skill} needs the user's approval before it installs.")
@@ -2204,21 +2196,40 @@ fn gate_new_behaviors(
             Err(CommandError::operation(
                 "BEHAVIOR_CONFIRMATION_REQUIRED",
                 format!(
-                    "{opening} skilld changed nothing. Behaviors: {behaviors}. {} Show these behaviors to the user. If the user approves, run the same command again with --allow {ids}",
-                    run::BEHAVIOR_CAVEAT,
+                    "{opening} skilld changed nothing. Behaviors: {behaviors}. {caveat} Show these behaviors to the user. If the user approves, run the same command again with --allow {ids}",
                 ),
             ))
         }
     }
 }
 
+/// The held behaviors as one line each, with any model readings, and the
+/// caveat that ends the list.
+fn describe_held(
+    held: &[&skilld_core::Behavior],
+    readings: &[run::BehaviorReading],
+) -> (String, String) {
+    let behaviors = held
+        .iter()
+        .map(|behavior| run::describe_read_behavior(behavior, readings))
+        .collect::<Vec<_>>()
+        .join("; ");
+    let caveat = if run::any_reading(held, readings) {
+        format!("{} {}", run::BEHAVIOR_CAVEAT, run::BEHAVIOR_READING_CAVEAT)
+    } else {
+        run::BEHAVIOR_CAVEAT.to_owned()
+    };
+    (behaviors, caveat)
+}
+
 /// Stop an update whose new version adds an ask behavior the installed copy lacks.
 fn gate_update_behaviors<H: Host>(
     host: &H,
     name: &str,
     installed: &Path,
     files: &[skilld_core::PreparedFile],
     allowed: &[String],
+    readings: &[run::BehaviorReading],
 ) -> Result<(), CommandError> {
     let (_, installed_files) = run::read_local(installed)?;
     gate_new_behaviors(
@@ -2228,6 +2239,7 @@ fn gate_update_behaviors<H: Host>(
         &skilld_core::detect_behaviors(files),
         &skilld_core::detect_behaviors(&installed_files),
         allowed,
+        readings,
     )
 }
 
@@ -2723,6 +2735,7 @@ impl LocalHost {
             &skilld_core::detect_behaviors(&prepared.files),
             &[],
             allowed,
+            &prepared.behavior_readings,
         )?;
         let name = self
             .store(scope)
@@ -2862,6 +2875,7 @@ impl LocalHost {
             source_status,
             revision: Some(revision.as_str().to_owned()),
             omitted_files: prepared.omitted_files,
+            behavior_readings: prepared.behavior_readings,
         })))
     }
 
@@ -2894,6 +2908,7 @@ impl LocalHost {
             source_status: "local",
             revision: None,
             omitted_files: Vec::new(),
+            behavior_readings: Vec::new(),
         })))
     }
 
@@ -2933,6 +2948,7 @@ impl LocalHost {
             source_status: "local",
             revision: None,
             omitted_files: Vec::new(),
+            behavior_readings: Vec::new(),
         })))
     }
 
@@ -3487,6 +3503,7 @@ impl Host for LocalHost {
                 &pending.view.canonical_path,
                 &prepared.files,
                 allowed,
+                &prepared.behavior_readings,
             )?;
             let targets = pending
                 .view
@@ -4053,6 +4070,7 @@ fn apply_update_selection(
             &pending.view.canonical_path,
             &prepared.files,
             allowed,
+            &prepared.behavior_readings,
         )?;
         let targets = pending
             .view

crates/skilld-command/src/remote.rs

@@ -19,9 +19,11 @@ use skilld_core::{
     verify_artifact, verify_attestation, verify_linked_file, verify_trusted_root,
     with_linked_files,
 };
-use skilld_ui::text::is_unsafe_terminal;
+use skilld_ui::text::{is_unsafe_terminal, sanitize};
 use url::Url;
 
+use crate::run::{BehaviorReading, BehaviorVerdict};
+
 mod api;
 
 pub use api::{ApiAnswer, ApiPage, BrowseQuery, BrowseSort, SkilldApi, TrendingWindow};
@@ -328,6 +330,9 @@ pub struct PreparedRemoteSkill {
     pub page_url: Option<String>,
     /// Files over a size limit that skilld.dev left out of the Artifact.
     pub omitted_files: Vec<OmittedFile>,
+    /// A language model's reading of each behavior match. Only an Artifact
+    /// skilld.dev delivered carries readings.
+    pub behavior_readings: Vec<BehaviorReading>,
 }
 
 impl PreparedRemoteSkill {
@@ -1994,6 +1999,7 @@ impl SkilldRemote {
             files,
             page_url: None,
             omitted_files: Vec::new(),
+            behavior_readings: Vec::new(),
         })
     }
 
@@ -3063,6 +3069,7 @@ impl RemoteProvider for SkilldRemote {
                 attestation_key_id: verified.attestation.signature.key_id.clone(),
             },
             omitted_files: omitted_files(&verified.attestation.check_results),
+            behavior_readings: behavior_readings(&verified.attestation.check_results),
             files: verified.files,
             page_url,
         })
@@ -4284,6 +4291,79 @@ fn omitted_file(finding: &str) -> OmittedFile {
     }
 }
 
+/// The signed check in which skilld.dev names a language model's reading of
+/// each behavior match. It is never required, and skilld never gates on it.
+///
+/// Each finding reads `PATH:LINE BEHAVIOR: VERDICT.`, then a reason. PATH
+/// holds no whitespace, so the first space ends the location.
+const BEHAVIOR_REVIEW_CHECK: &str = "behavior-review";
+
+/// The most readings skilld takes from one Artifact.
+const MAX_BEHAVIOR_READINGS: usize = 100;
+
+/// The longest reason skilld prints.
+const MAX_READING_REASON_CHARS: usize = 200;
+
+/// The readings the `behavior-review` check names. A finding in another
+/// shape gives none, and a match named twice reads as neither.
+fn behavior_readings(results: &[skilld_core::CheckResult]) -> Vec<BehaviorReading> {
+    let readings = results
+        .iter()
+        .filter(|result| result.name == BEHAVIOR_REVIEW_CHECK)
+        .flat_map(|result| &result.findings)
+        .take(MAX_BEHAVIOR_READINGS)
+        .filter_map(|finding| behavior_reading(finding))
+        .collect::<Vec<_>>();
+    let mut named = BTreeMap::<(String, usize, String), usize>::new();
+    for reading in &readings {
+        *named
+            .entry((reading.path.clone(), reading.line, reading.behavior.clone()))
+            .or_default() += 1;
+    }
+    readings
+        .into_iter()
+        .filter(|reading| {
+            named.get(&(reading.path.clone(), reading.line, reading.behavior.clone())) == Some(&1)
+        })
+        .collect()
+}
+
+/// Parse `PATH:LINE BEHAVIOR: VERDICT.` and an optional reason.
+fn behavior_reading(finding: &str) -> Option<BehaviorReading> {
+    let (location, rest) = finding.split_once(' ')?;
+    let (path, line) = location.rsplit_once(':')?;
+    let line = line.parse::<usize>().ok().filter(|line| *line > 0)?;
+    let (behavior, rest) = rest.split_once(": ")?;
+    if path.is_empty()
+        || behavior.is_empty()
+        || !behavior
+            .bytes()
+            .all(|byte| byte.is_ascii_lowercase() || byte == b'-')
+    {
+        return None;
+    }
+    let (verdict, reason) = match rest.split_once(". ") {
+        Some((verdict, reason)) => (verdict, Some(reason)),
+        None => (rest.strip_suffix('.')?, None),
+    };
+    let reason = reason
+        .map(|reason| {
+            sanitize(reason)
+                .trim()
+                .chars()
+                .take(MAX_READING_REASON_CHARS)
+                .collect::<String>()
+        })
+        .filter(|reason| !reason.is_empty());
+    Some(BehaviorReading {
+        path: path.to_owned(),
+        line,
+        behavior: behavior.to_owned(),
+        verdict: BehaviorVerdict::parse(verdict)?,
+        reason,
+    })
+}
+
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq)]
 #[serde(rename_all = "camelCase")]
 struct ArtifactDescriptor {