feat(skills): retract uninstalled skills from mid-session catalogue (#3745)

src/openhuman/agent/harness/session/builder/setters.rs

@@ -589,6 +589,7 @@ impl AgentBuilder {
             pending_mcp_announcement: Vec::new(),
             announced_skills: std::collections::HashSet::new(),
             pending_skill_announcement: Vec::new(),
+            pending_skill_retraction: Vec::new(),
             archivist_hook: self.archivist_hook,
             synthesized_tool_names: std::collections::HashSet::new(),
             pending_synthesized_tools_mask: std::collections::HashSet::new(),

src/openhuman/agent/harness/session/tests.rs

@@ -533,6 +533,112 @@ fn refresh_workflows_picks_up_skill_installed_on_disk() {
     );
 }
 
+#[test]
+fn refresh_workflows_retracts_skill_removed_from_disk() {
+    use crate::openhuman::workflows::ops_types::{SKILL_MD, TRUST_MARKER};
+
+    let ws = tempfile::TempDir::new().expect("temp workspace");
+    let wsp = ws.path().to_path_buf();
+    std::fs::create_dir_all(wsp.join(".openhuman")).unwrap();
+    std::fs::write(wsp.join(".openhuman").join(TRUST_MARKER), "").unwrap();
+
+    // Write a skill to disk.
+    let skill_dir = wsp
+        .join(".openhuman")
+        .join("skills")
+        .join("zz-retract-test");
+    std::fs::create_dir_all(&skill_dir).unwrap();
+    std::fs::write(
+        skill_dir.join(SKILL_MD),
+        "---\nname: zz-retract-test\ndescription: a retraction test skill\n---\n# body\n",
+    )
+    .unwrap();
+
+    let memory_cfg = crate::openhuman::config::MemoryConfig {
+        backend: "none".into(),
+        ..crate::openhuman::config::MemoryConfig::default()
+    };
+    let mem: Arc<dyn Memory> =
+        Arc::from(crate::openhuman::memory_store::create_memory(&memory_cfg, &wsp).unwrap());
+    let provider = Box::new(MockProvider {
+        responses: Mutex::new(vec![]),
+    });
+    let mut agent = Agent::builder()
+        .provider(provider)
+        .tools(vec![Box::new(MockTool)])
+        .memory(mem)
+        .tool_dispatcher(Box::new(NativeToolDispatcher))
+        .workspace_dir(wsp.clone())
+        .build()
+        .expect("agent build should succeed");
+
+    // First refresh: picks up the installed skill.
+    assert!(agent.refresh_workflows("test-install"));
+    assert!(
+        agent
+            .test_workflow_ids()
+            .iter()
+            .any(|id| id == "zz-retract-test"),
+        "skill should be in catalogue after first refresh"
+    );
+    assert!(
+        agent
+            .test_pending_skill_announcement()
+            .iter()
+            .any(|id| id == "zz-retract-test"),
+        "skill should be parked for announcement"
+    );
+    // Now remove the skill from disk.
+    std::fs::remove_dir_all(&skill_dir).unwrap();
+
+    // Second refresh: detects the removal, parks the retraction.
+    assert!(
+        agent.refresh_workflows("test-remove"),
+        "removing a skill should change the set"
+    );
+    assert!(
+        !agent
+            .test_workflow_ids()
+            .iter()
+            .any(|id| id == "zz-retract-test"),
+        "skill should be gone from catalogue after removal"
+    );
+    assert!(
+        agent
+            .test_pending_skill_retraction()
+            .iter()
+            .any(|id| id == "zz-retract-test"),
+        "removed skill should be parked for retraction"
+    );
+    // Retraction should have cleared it from announced_skills; re-install will
+    // be announced fresh (not silently re-added). Verify by re-adding the skill
+    // and confirming it gets announced again.
+    std::fs::create_dir_all(&skill_dir).unwrap();
+    std::fs::write(
+        skill_dir.join(SKILL_MD),
+        "---\nname: zz-retract-test\ndescription: a retraction test skill\n---\n# body\n",
+    )
+    .unwrap();
+    assert!(agent.refresh_workflows("test-reinstall"));
+    assert!(
+        agent
+            .test_pending_skill_announcement()
+            .iter()
+            .any(|id| id == "zz-retract-test"),
+        "re-installed skill should be announced again after retraction cleared it from announced set"
+    );
+    // Re-install must also cancel the still-pending retraction so the user turn
+    // never carries a contradictory "installed" + "retracted" pair for the same
+    // skill.
+    assert!(
+        !agent
+            .test_pending_skill_retraction()
+            .iter()
+            .any(|id| id == "zz-retract-test"),
+        "re-install should cancel the pending retraction for the same skill"
+    );
+}
+
 #[tokio::test]
 async fn turn_without_tools_returns_text() {
     let workspace = tempfile::TempDir::new().expect("temp workspace");

src/openhuman/agent/harness/session/turn/core.rs

@@ -5,7 +5,7 @@ use super::super::turn_engine_adapter::{AgentCheckpoint, AgentObserver, AgentToo
 use super::super::types::Agent;
 use super::{
     integration_announcement_note, mcp_announcement_note, newly_connected_slugs,
-    skill_announcement_note,
+    skill_announcement_note, skill_retraction_note,
 };
 use crate::openhuman::agent::harness;
 use crate::openhuman::agent::harness::definition::TriggerMemoryAgent;
@@ -388,6 +388,16 @@ impl Agent {
             None => enriched,
         };
 
+        // Same one-shot treatment for skills uninstalled mid-session (parked by
+        // `refresh_workflows`). The model must know the skill is gone so it does
+        // not attempt `run_skill` on a removed entry. Rides the user turn for
+        // the same KV-cache reason as the install note above.
+        let pending_retracted = std::mem::take(&mut self.pending_skill_retraction);
+        let enriched = match skill_retraction_note(&pending_retracted) {
+            Some(note) => format!("{note}\n\n{enriched}"),
+            None => enriched,
+        };
+
         // Pin the main agent to its configured model for the lifetime of
         // the session. Per-turn classification used to run here, but it
         // would flip `effective_model` mid-conversation (e.g. reasoning →

src/openhuman/agent/harness/session/turn/mod.rs

@@ -136,6 +136,22 @@ They are in your `## Installed Skills` list — run one with `run_skill` immedia
     ))
 }
 
+/// One-shot note prepended to the next user turn when skills are uninstalled
+/// mid-session. Symmetric to [`skill_announcement_note`]: tells the model the
+/// listed skills are no longer present and `run_skill` will fail for them, so
+/// it does not attempt to invoke them. Rides the user turn (not the system
+/// prompt) to keep the KV-cache prefix stable.
+pub(super) fn skill_retraction_note(skill_ids: &[String]) -> Option<String> {
+    if skill_ids.is_empty() {
+        return None;
+    }
+    Some(format!(
+        "[skills retracted] These skill(s) were uninstalled during this conversation and are no longer available: {}. \
+Do not attempt to run them with `run_skill` — they have been removed. Tell the user to reinstall if they want to use them again.",
+        skill_ids.join(", ")
+    ))
+}
+
 /// Wrapper around
 /// [`crate::openhuman::memory_tree::tree_runtime::store::collect_root_summaries_with_caps`]
 /// that takes user-resolved per-namespace and total caps. The actual

src/openhuman/agent/harness/session/turn/tools.rs

@@ -391,27 +391,49 @@ impl Agent {
             return false;
         }
         // Newly-present skills (on disk now, absent from the prior snapshot),
-        // announced at most once this session. Removals are detected by the
-        // id-set diff above but are NOT yet retracted from the frozen
-        // catalogue (symmetric to the integration/MCP announcements, which
-        // also only announce additions) — tracked in
-        // tinyhumansai/openhuman#3738.
+        // announced at most once this session.
         let newly: Vec<String> = latest_ids
             .difference(&current_ids)
             .filter(|id| self.announced_skills.insert((*id).clone()))
             .cloned()
             .collect();
+        // Skills removed from disk since the last snapshot: retract them so the
+        // model stops routing `run_skill` calls to skills that no longer exist.
+        // The frozen `## Installed Skills` system-prompt block cannot be updated
+        // mid-session (KV-cache stability), so the retraction note on the user
+        // turn is the only signal the model gets — mirrors the install path.
+        // Clear from `announced_skills` so a re-install later is announced fresh.
+        let removed: Vec<String> = current_ids.difference(&latest_ids).cloned().collect();
+        for id in &removed {
+            self.announced_skills.remove(id);
+        }
         log::info!(
-            "[agent_loop] installed-skills set changed ({trigger}): {} -> {} skills; updating tracked set + parking announcement (system-prompt catalogue is frozen mid-session; the user-turn note surfaces the change)",
+            "[agent_loop] installed-skills set changed ({trigger}): {} -> {} skills (new={} removed={}); updating tracked set + parking notes (system-prompt catalogue frozen for KV cache)",
             self.workflows.len(),
-            latest.len()
+            latest.len(),
+            newly.len(),
+            removed.len(),
         );
         self.workflows = latest;
         for id in newly {
+            // A re-install after a still-pending retraction cancels the
+            // retraction: the skill is present again, so drop the stale "gone"
+            // note and announce it instead.
+            self.pending_skill_retraction.retain(|p| p != &id);
             if !self.pending_skill_announcement.contains(&id) {
                 self.pending_skill_announcement.push(id);
             }
         }
+        for id in removed {
+            // If the skill was installed and uninstalled before its
+            // announcement ever surfaced, the model never saw it as available —
+            // drop the pending announcement so we don't emit a contradictory
+            // "installed" + "retracted" pair on the same user turn.
+            self.pending_skill_announcement.retain(|p| p != &id);
+            if !self.pending_skill_retraction.contains(&id) {
+                self.pending_skill_retraction.push(id);
+            }
+        }
         true
     }
 
@@ -439,6 +461,13 @@ impl Agent {
         &self.pending_skill_announcement
     }
 
+    /// Test-only: skill ids parked for the next-turn `[skills retracted]`
+    /// retraction note by `refresh_workflows`.
+    #[cfg(test)]
+    pub(in super::super) fn test_pending_skill_retraction(&self) -> &[String] {
+        &self.pending_skill_retraction
+    }
+
     /// Test-only: inject a specific skill-events receiver (e.g. one whose
     /// sender has been dropped) so `drain_skill_events`' `Closed` arm is
     /// reachable without the global bus singleton.

src/openhuman/agent/harness/session/turn_tests.rs

@@ -1889,3 +1889,26 @@ fn skill_announcement_note_mentions_ids_and_run_skill() {
         "note must steer the model to run_skill: {note}"
     );
 }
+
+#[test]
+fn skill_retraction_note_empty_yields_none() {
+    assert!(super::skill_retraction_note(&[]).is_none());
+}
+
+#[test]
+fn skill_retraction_note_names_removed_skills_and_warns_against_run_skill() {
+    let note =
+        super::skill_retraction_note(&["ascii-art".to_string(), "github-issues".to_string()])
+            .expect("non-empty input should yield a note");
+    assert!(note.contains("[skills retracted]"));
+    assert!(note.contains("ascii-art"));
+    assert!(note.contains("github-issues"));
+    assert!(
+        note.contains("run_skill"),
+        "note must mention run_skill so the model knows not to invoke it: {note}"
+    );
+    assert!(
+        !note.contains("[skills update]"),
+        "retraction note must not look like an install announcement: {note}"
+    );
+}

src/openhuman/agent/harness/session/types.rs

@@ -245,6 +245,13 @@ pub struct Agent {
     /// next user message is built so the note rides the user turn (NOT the
     /// system prompt) and the KV-cache prefix stays byte-identical.
     pub(super) pending_skill_announcement: Vec<String>,
+    /// Skill ids removed mid-session (uninstalled after session build) that
+    /// still need retracting on the next user message. Symmetric to
+    /// [`Self::pending_skill_announcement`]: parked by `refresh_workflows`,
+    /// rendered + cleared when the next user message is built so the retraction
+    /// note rides the user turn (NOT the system prompt) and the KV-cache prefix
+    /// stays byte-identical.
+    pub(super) pending_skill_retraction: Vec<String>,
     /// Skill ids already surfaced to the model as installed this session, so
     /// each newly-installed skill is announced exactly once and never
     /// re-announced per turn. Seeded from the session-build catalogue.