fix(skills): wire two_stage_matching and confusability_threshold at startup; migrate legacy bundled skills (#2410)

AgentBuilder gains with_two_stage_matching and with_confusability_threshold builder
methods. Both are wired in runner.rs and daemon.rs alongside with_disambiguation_threshold,
so config values are applied at startup rather than only after hot-reload. Closes #2404.

provision_bundled_skills now detects legacy bundled skills (dirs without a .bundled marker
whose SKILL.md content matches the embedded version) and re-provisions them, adding the
marker and updated category field without overwriting user-modified skills. Closes #2403.

Author: bug-ops

CHANGELOG.md

@@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 - fix(classifiers): sha2 0.11 hex formatting — replace `format!("{:x}", ...)` with `hex::encode(...)` in `verify_sha256` and its test helper (#2401)
 - fix(deps): bump sha2 0.10→0.11, ordered-float 5.1→5.3, proptest 1.10→1.11, toml 1.0→1.1, uuid 1.22→1.23 (#2401)
+- fix(skills): `two_stage_matching` and `confusability_threshold` config fields are now applied at agent startup; `AgentBuilder` gains `with_two_stage_matching` and `with_confusability_threshold` builder methods wired in `runner.rs` and `daemon.rs` (closes #2404)
+- fix(skills): bundled skills provisioned before the `.bundled` marker system are now migrated on upgrade — `provision_bundled_skills` re-provisions skills whose `SKILL.md` matches the embedded version, restoring the `category` field without overwriting user-modified skills (closes #2403)
 - fix(memory): correct ESCAPE clause in spreading activation BFS alias query — `ESCAPE '\\\\'` (2 chars) changed to `ESCAPE '\\'` (1 char) as required by SQLite (closes #2393)
 - fix(llm): call `save_bandit_state()` in `save_router_state()` to persist PILOT LinUCB bandit state across restarts (closes #2394)
 - fix(classifiers): use Metal/CUDA device when available in candle classifiers — falls back to CPU (#2396)

crates/zeph-core/src/agent/builder.rs

@@ -263,6 +263,18 @@ impl<C: Channel> Agent<C> {
         self
     }
 
+    #[must_use]
+    pub fn with_two_stage_matching(mut self, enabled: bool) -> Self {
+        self.skill_state.two_stage_matching = enabled;
+        self
+    }
+
+    #[must_use]
+    pub fn with_confusability_threshold(mut self, threshold: f32) -> Self {
+        self.skill_state.confusability_threshold = threshold.clamp(0.0, 1.0);
+        self
+    }
+
     #[must_use]
     pub fn with_skill_prompt_mode(mut self, mode: crate::config::SkillPromptMode) -> Self {
         self.skill_state.prompt_mode = mode;
@@ -1632,4 +1644,40 @@ mod tests {
             "apply_session_config must not create anomaly_detector when disabled"
         );
     }
+
+    #[test]
+    fn with_two_stage_matching_sets_flag() {
+        let agent = make_agent().with_two_stage_matching(true);
+        assert!(
+            agent.skill_state.two_stage_matching,
+            "with_two_stage_matching(true) must enable two_stage_matching"
+        );
+
+        let agent = make_agent().with_two_stage_matching(false);
+        assert!(
+            !agent.skill_state.two_stage_matching,
+            "with_two_stage_matching(false) must disable two_stage_matching"
+        );
+    }
+
+    #[test]
+    fn with_confusability_threshold_sets_and_clamps() {
+        let agent = make_agent().with_confusability_threshold(0.85);
+        assert!(
+            (agent.skill_state.confusability_threshold - 0.85).abs() < f32::EPSILON,
+            "with_confusability_threshold must store the provided value"
+        );
+
+        let agent = make_agent().with_confusability_threshold(1.5);
+        assert_eq!(
+            agent.skill_state.confusability_threshold, 1.0,
+            "with_confusability_threshold must clamp values above 1.0"
+        );
+
+        let agent = make_agent().with_confusability_threshold(-0.1);
+        assert_eq!(
+            agent.skill_state.confusability_threshold, 0.0,
+            "with_confusability_threshold must clamp values below 0.0"
+        );
+    }
 }

crates/zeph-skills/src/bundled.rs

@@ -99,9 +99,28 @@ pub fn provision_bundled_skills(managed_dir: &Path) -> Result<ProvisionReport, s
         // Skill dir exists — check marker.
         match read_marker_version(&marker_path) {
             MarkerState::NoMarker => {
-                // User-owned skill — never overwrite.
-                debug!(skill = %skill_name, "skipping user-owned skill (no .bundled marker)");
-                report.skipped.push(skill_name);
+                // Check if this is a legacy bundled skill (provisioned before the
+                // .bundled marker system): compare on-disk SKILL.md to embedded.
+                if is_legacy_bundled(&target_dir, &skill_name) {
+                    match write_skill(skill_dir, &target_dir, &marker_path, &embedded_version) {
+                        Ok(()) => {
+                            info!(
+                                skill = %skill_name,
+                                to = %embedded_version,
+                                "migrated legacy bundled skill (added .bundled marker)"
+                            );
+                            report.updated.push(skill_name);
+                        }
+                        Err(e) => {
+                            warn!(skill = %skill_name, error = %e, "failed to migrate legacy bundled skill");
+                            report.failed.push((skill_name, e.to_string()));
+                        }
+                    }
+                } else {
+                    // User-owned skill — never overwrite.
+                    debug!(skill = %skill_name, "skipping user-owned skill (no .bundled marker)");
+                    report.skipped.push(skill_name);
+                }
             }
             MarkerState::CorruptMarker => {
                 warn!(
@@ -255,6 +274,26 @@ fn extract_embedded_version(skill_dir: &include_dir::Dir<'_>) -> String {
     parse_frontmatter_version(content).unwrap_or_else(|| "1.0".to_owned())
 }
 
+/// Check whether an on-disk skill dir (without a `.bundled` marker) matches the
+/// embedded version — indicating it was provisioned before the marker system.
+///
+/// Returns `true` only when the on-disk `SKILL.md` content (trimmed) equals the
+/// embedded `SKILL.md` content (trimmed). A mismatch means the user modified the
+/// file, so we treat the skill as user-owned.
+fn is_legacy_bundled(target_dir: &Path, skill_name: &str) -> bool {
+    let embedded_path = format!("{skill_name}/SKILL.md");
+    let Some(embedded_file) = BUNDLED_SKILLS_DIR.get_file(&embedded_path) else {
+        return false;
+    };
+    let Ok(embedded_content) = std::str::from_utf8(embedded_file.contents()) else {
+        return false;
+    };
+    match fs::read_to_string(target_dir.join("SKILL.md")) {
+        Ok(on_disk) => on_disk.trim() == embedded_content.trim(),
+        Err(_) => false,
+    }
+}
+
 /// Parse the `version:` key from the `metadata:` block in SKILL.md frontmatter.
 ///
 /// Frontmatter is delimited by `---` lines. Within `metadata:`, lines of the
@@ -349,6 +388,109 @@ mod tests {
         ));
     }
 
+    /// `is_legacy_bundled` returns true when on-disk SKILL.md matches embedded content (trimmed).
+    #[test]
+    fn is_legacy_bundled_matches_identical_content() {
+        let tmp = TempDir::new().unwrap();
+        let skill_dir = tmp.path().join("test-skill");
+        fs::create_dir_all(&skill_dir).unwrap();
+
+        // Write the same content that would be in an embedded skill.
+        let skill_md_content = make_skill_md("1.0");
+        fs::write(skill_dir.join("SKILL.md"), &skill_md_content).unwrap();
+
+        // We can't directly call is_legacy_bundled with a real include_dir::Dir,
+        // so we test the provision path: provision to empty dir first (installs),
+        // then remove the .bundled marker and re-provision — the skill must be
+        // migrated (moved to updated), not skipped.
+        let managed = TempDir::new().unwrap();
+        let report1 = provision_bundled_skills(managed.path()).expect("first provision");
+        assert!(report1.failed.is_empty());
+
+        // Remove all .bundled markers to simulate pre-marker state.
+        for name in &report1.installed {
+            let marker = managed.path().join(name).join(".bundled");
+            if marker.exists() {
+                fs::remove_file(&marker).unwrap();
+            }
+        }
+
+        // Re-provision: skills whose SKILL.md matches embedded → migrate (updated).
+        // Skills whose SKILL.md was modified → skip.
+        let report2 = provision_bundled_skills(managed.path()).expect("second provision");
+        assert!(
+            report2.failed.is_empty(),
+            "no failures on re-provision: {:?}",
+            report2.failed
+        );
+        // All skills without markers should be migrated (updated), none skipped.
+        assert!(
+            report2.installed.is_empty(),
+            "no new installs expected on re-provision"
+        );
+        assert!(
+            report2.skipped.is_empty(),
+            "no skills should be skipped when content matches embedded"
+        );
+        assert!(
+            !report2.updated.is_empty(),
+            "all skills without marker must be migrated to updated"
+        );
+
+        // After migration, each skill must have a .bundled marker.
+        for name in &report2.updated {
+            let marker = managed.path().join(name).join(".bundled");
+            assert!(
+                marker.exists(),
+                "{name}: .bundled marker missing after migration"
+            );
+        }
+    }
+
+    /// `is_legacy_bundled` returns false when on-disk SKILL.md differs from embedded.
+    #[test]
+    fn is_legacy_bundled_skips_modified_skill() {
+        let managed = TempDir::new().unwrap();
+        let report1 = provision_bundled_skills(managed.path()).expect("first provision");
+        assert!(report1.failed.is_empty());
+        assert!(!report1.installed.is_empty());
+
+        // Remove .bundled markers AND modify SKILL.md to simulate user edits.
+        for name in &report1.installed {
+            let skill_dir = managed.path().join(name);
+            let marker = skill_dir.join(".bundled");
+            if marker.exists() {
+                fs::remove_file(&marker).unwrap();
+            }
+            let skill_md = skill_dir.join("SKILL.md");
+            if skill_md.exists() {
+                let mut content = fs::read_to_string(&skill_md).unwrap();
+                content.push_str("\n# user modification\n");
+                fs::write(&skill_md, content).unwrap();
+            }
+        }
+
+        let report2 = provision_bundled_skills(managed.path()).expect("second provision");
+        assert!(
+            report2.failed.is_empty(),
+            "no failures: {:?}",
+            report2.failed
+        );
+        // All modified skills must be skipped (treated as user-owned).
+        assert!(
+            report2.updated.is_empty(),
+            "modified skills must not be updated"
+        );
+        assert!(
+            report2.installed.is_empty(),
+            "no re-installs expected when dir exists"
+        );
+        assert!(
+            !report2.skipped.is_empty(),
+            "modified skills must be skipped"
+        );
+    }
+
     /// Provision to an empty managed dir: all bundled skills are installed and
     /// each gets a `.bundled` marker file containing the skill version.
     #[test]

src/daemon.rs

@@ -374,6 +374,8 @@ pub(crate) async fn run_daemon(
         )
         .apply_session_config(session_config)
         .with_disambiguation_threshold(config.skills.disambiguation_threshold)
+        .with_two_stage_matching(config.skills.two_stage_matching)
+        .with_confusability_threshold(config.skills.confusability_threshold)
         .with_skill_reload(skill_paths, reload_rx)
         .with_managed_skills_dir(zeph_core::bootstrap::managed_skills_dir())
         .with_memory(

src/runner.rs

@@ -976,6 +976,8 @@ pub(crate) async fn run(cli: Cli) -> anyhow::Result<()> {
         zeph_core::config::ProviderEntry::effective_name,
     ))
     .with_disambiguation_threshold(config.skills.disambiguation_threshold)
+    .with_two_stage_matching(config.skills.two_stage_matching)
+    .with_confusability_threshold(config.skills.confusability_threshold)
     .with_skill_reload(skill_paths.clone(), reload_rx)
     .with_managed_skills_dir(zeph_core::bootstrap::managed_skills_dir())
     .with_trust_config(config.skills.trust.clone())