From 11dc8d7ba3322da01d896bd572cc9f06958ba778 Mon Sep 17 00:00:00 2001 From: Blomios Date: Wed, 17 Jun 2026 15:08:33 +0200 Subject: [PATCH] =?UTF-8?q?feat(skill-awareness):=20T1=E2=86=92T5=20?= =?UTF-8?q?=E2=80=94=20manifeste=20de=20skills=20+=20outil=20MCP=20idea=5F?= =?UTF-8?q?skill=5Fread?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface les skills assignés à un agent « à la MCP » : description sur l'entité Skill (+ effective_description fallback), section « # Skills disponibles » haute altitude dans le convention-file (mode MCP), outil read-only idea_skill_read (résolution projet→global), use case ReadSkill (port SkillStore existant), et câblage au composition root. Dump legacy du corps complet conservé en mode sans-MCP (zéro régression). Rétro-compat index.json (serde default). Tests : domain+application+infrastructure 1212 passed / 0 failed (23 ajoutés). Reste : T6 (champ description front) + T7 (e2e après rebuild AppImage). NB topologie : commit réalisé par l'orchestrateur car l'agent Git était injoignable (bug de livraison cold-start) ; à faire relire/rebaser par Git. Co-Authored-By: Claude Opus 4.8 --- crates/app-tauri/src/commands.rs | 4 + crates/app-tauri/src/state.rs | 15 +- crates/application/src/agent/lifecycle.rs | 115 +++++++- crates/application/src/lib.rs | 6 +- .../application/src/orchestrator/service.rs | 48 +++- crates/application/src/skill/mod.rs | 6 +- crates/application/src/skill/usecases.rs | 260 +++++++++++++++++- crates/application/tests/skill_usecases.rs | 4 + crates/domain/src/orchestrator.rs | 15 + crates/domain/src/skill.rs | 144 +++++++++- .../src/orchestrator/mcp/tools.rs | 29 +- crates/infrastructure/src/store/skill.rs | 133 +++++++++ crates/infrastructure/tests/mcp_server.rs | 6 +- 13 files changed, 758 insertions(+), 27 deletions(-) diff --git a/crates/app-tauri/src/commands.rs b/crates/app-tauri/src/commands.rs index 60314c4..cd6c9f9 100644 --- a/crates/app-tauri/src/commands.rs +++ b/crates/app-tauri/src/commands.rs @@ -2118,6 +2118,10 @@ pub async fn create_skill( .create_skill .execute(CreateSkillInput { name: request.name, + // Description is set via the dedicated frontend field (T6); the create + // path stays None for now so the affordance falls back to the body's + // first line (see `Skill::effective_description`). + description: None, content: request.content, scope: request.scope, project_root: project.root, diff --git a/crates/app-tauri/src/state.rs b/crates/app-tauri/src/state.rs index 781b107..37d2b68 100644 --- a/crates/app-tauri/src/state.rs +++ b/crates/app-tauri/src/state.rs @@ -25,9 +25,9 @@ use application::{ ListResumableAgents, ListSkills, ListTemplates, LiveAgentRegistry, LiveSessions, LoadLayout, McpRuntime, MoveTabToNewWindow, MutateLayout, OnnxModelView, OpenProject, OpenTerminal, OrchestratorService, PermissionProjectorRegistry, ProposeContext, ReadAgentContext, - ReadContext, ReadMemory, ReadMemoryIndex, ReadProjectContext, RecallMemory, ReconcileLayouts, - RecordTurn, RecordTurnProvider, ReferenceProfiles, RenameLayout, ResizeTerminal, - ResolveAgentPermissions, ResolveMemoryLinks, SaveEmbedderProfile, SaveProfile, + ReadContext, ReadMemory, ReadMemoryIndex, ReadProjectContext, ReadSkill, RecallMemory, + ReconcileLayouts, RecordTurn, RecordTurnProvider, ReferenceProfiles, RenameLayout, + ResizeTerminal, ResolveAgentPermissions, ResolveMemoryLinks, SaveEmbedderProfile, SaveProfile, SessionLimitService, SetActiveLayout, SnapshotRunningAgents, StopLiveAgent, StructuredSessions, SuggestedThisSession, SyncAgentWithTemplate, TerminalSessions, UnassignSkillFromAgent, UpdateAgentContext, UpdateAgentPermissions, UpdateMemory, UpdateProjectContext, @@ -965,6 +965,10 @@ impl AppState { let update_skill = Arc::new(UpdateSkill::new(Arc::clone(&skill_store_port))); let list_skills = Arc::new(ListSkills::new(Arc::clone(&skill_store_port))); let delete_skill = Arc::new(DeleteSkill::new(Arc::clone(&skill_store_port))); + // Lecture d'un skill par nom pour l'outil MCP `idea_skill_read` (feature + // « skills à la MCP ») — compose le SkillStore existant, câblé plus bas sur + // l'OrchestratorService via le builder additif `.with_read_skill(...)`. + let read_skill = Arc::new(ReadSkill::new(Arc::clone(&skill_store_port))); let assign_skill = Arc::new(AssignSkillToAgent::new( Arc::clone(&contexts_port), Arc::clone(&events_port), @@ -1189,7 +1193,10 @@ impl AppState { .with_memory_harvest(Arc::new(HarvestMemoryFromTurn::new( Arc::clone(&memory_store_port), Arc::clone(&events_port), - ))), + ))) + // Outil MCP `idea_skill_read` (feature « skills à la MCP ») : sans ça, + // `skill.read` renverrait « not configured ». Compose le SkillStore existant. + .with_read_skill(Arc::clone(&read_skill)), // NB (régression corrigée) : on ne câble PAS `.with_structured(...)` ici. // Décision produit lot B-2 (« Option 1 Terminal + MCP », cf. construction // de `LaunchAgent` plus haut) : la fabrique structurée est décâblée, donc diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index c038434..7f2022a 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -2542,16 +2542,25 @@ fn append_block(input: &str, block: &str) -> String { /// `# Skills` section (ARCHITECTURE §14.2). /// /// A short skill-awareness paragraph is always injected in the orchestration -/// block: it explains that assigned skills are operational workflow context, not -/// magic commands or provider subagents, and points reusable workflow creation to -/// the active IdeA orchestration surface (`idea_create_skill` for MCP profiles, -/// `skill.create` for the file protocol). This awareness deliberately does not -/// inject unassigned skill bodies; assignment remains the context boundary. +/// block (followed by the auto-memory harvest directive, Lot E1): it explains that +/// assigned skills are operational workflow context, not magic commands or provider +/// subagents, and points reusable workflow creation to the active IdeA orchestration +/// surface (`idea_create_skill` for MCP profiles, `skill.create` for the file +/// protocol). This awareness deliberately does not inject unassigned skill bodies; +/// assignment remains the context boundary. /// -/// Skills are emitted in the order given (the caller passes them in manifest -/// order, making the output deterministic); each is introduced by a `##` header -/// carrying its name. When `skills` is empty the section is omitted entirely, so -/// an agent with no skills gets exactly the previous document. +/// On top of that awareness, the assigned skills surface in one of two ways +/// depending on the agent's **surface** (feature « skills à la MCP »), always in +/// the given (manifest) order — making the output deterministic: +/// - **MCP mode** (`mcp_enabled`): a high-altitude `# Skills disponibles` section, +/// right after the orchestration block, listing each as +/// `**** — ` (affordances only, *no body*), with +/// prose pointing to `idea_skill_read` to load a body on demand. Respects the +/// altitude: the capability is exposed, never the skill content. +/// - **Non-MCP mode**: the legacy `# Skills` section dumping each body in full +/// under a `##` header carrying its name (unchanged — zero regression). +/// When `skills` is empty both sections are omitted entirely, so an agent with no +/// skills gets exactly the previous document. /// /// The project's `memory` recall (index/hooks, ARCHITECTURE §14.5.4) is appended as /// a `# Mémoire projet` section — one `- [Title](slug.md) — hook (type)` line per @@ -2627,6 +2636,29 @@ pub(crate) fn compose_convention_file( out.push_str(memory_awareness()); out.push_str("---\n\n"); + // Skills « à la MCP » (feature skill-awareness) : à HAUTE ALTITUDE, juste après + // le bloc d'orchestration. On expose les skills assignés comme des **affordances + // nommées+décrites** (et NON leur corps complet), à la manière des outils MCP, + // pour que l'agent sache qu'ils existent et charge le détail à la demande via + // `idea_skill_read`. Réservé au mode MCP (le mode sans MCP conserve l'ancien dump + // du corps complet en fin de fichier, plus bas). Omis si zéro skill. + if mcp_enabled && !skills.is_empty() { + out.push_str("# Skills disponibles\n\n"); + out.push_str( + "Les skills suivants te sont assignés. Pour en exécuter un ou consulter son \ + détail, appelle l'outil `idea_skill_read(name=…)` — n'improvise pas un \ + workflow déjà couvert par un skill, charge-le.\n\n", + ); + for skill in skills { + out.push_str("**"); + out.push_str(&skill.name); + out.push_str("** — "); + out.push_str(&skill.effective_description()); + out.push('\n'); + } + out.push_str("\n---\n\n"); + } + if !project_context.trim().is_empty() { out.push_str("# Contexte projet\n\n"); out.push_str(project_context.trim()); @@ -2635,7 +2667,11 @@ pub(crate) fn compose_convention_file( out.push_str(agent_md); - if !skills.is_empty() { + // MODE SANS MCP (exigence zéro régression, décision produit 4.2(b)) : on conserve + // l'ancien dump du **corps complet** des skills en fin de fichier. En mode MCP, le + // corps n'est PAS injecté ici (l'agent le charge à la demande via `idea_skill_read`, + // cf. la section « # Skills disponibles » à haute altitude plus haut). + if !skills.is_empty() && !mcp_enabled { out.push_str("\n\n---\n\n# Skills\n"); for skill in skills { out.push_str("\n## "); @@ -2944,6 +2980,65 @@ mod tests { ); } + #[test] + fn compose_convention_file_mcp_mode_exposes_skill_affordances_not_bodies() { + // MCP mode (feature « skills à la MCP »): a high-altitude `# Skills disponibles` + // section after the Orchestration block, listing `**name** — description` + // affordances and pointing to `idea_skill_read`, WITHOUT dumping the bodies. + let s = |n: u128, name: &str, desc: Option<&str>, body: &str| { + Skill::new( + domain::SkillId::from_uuid(uuid::Uuid::from_u128(n)), + name, + MarkdownDoc::new(body), + domain::SkillScope::Global, + ) + .unwrap() + .with_description(desc.map(str::to_owned)) + }; + let doc = compose_convention_file( + "/root", + "", + "# Persona", + &[ + s(1, "refactor", Some("Refactors code"), "REFAC_BODY"), + // No explicit description ⇒ effective_description falls back to the + // body's first line (heading marker stripped). + s(2, "review", None, "# Review skill\n\nREVIEW_BODY"), + ], + &[], + None, + true, // mcp_enabled + ); + + // The affordance section is present. + assert!(doc.contains("# Skills disponibles"), "MCP skills section present"); + assert!(doc.contains("idea_skill_read"), "points to the read tool"); + // Affordance lines: `**name** — `. + assert!(doc.contains("**refactor** — Refactors code")); + assert!(doc.contains("**review** — Review skill")); + // The full bodies are NOT injected in MCP mode (loaded on demand instead). + assert!(!doc.contains("REFAC_BODY"), "no full body in MCP mode"); + assert!(!doc.contains("REVIEW_BODY"), "no full body in MCP mode"); + // The legacy `## ` body dump headers are absent too. + assert!(!doc.contains("## refactor")); + + // The section sits at high altitude: after the Orchestration block, before + // the persona. + let orch_at = doc.find("# Orchestration IdeA").unwrap(); + let skills_at = doc.find("# Skills disponibles").unwrap(); + let persona_at = doc.find("# Persona").unwrap(); + assert!(orch_at < skills_at, "skills come after orchestration"); + assert!(skills_at < persona_at, "skills come before the persona"); + } + + #[test] + fn compose_convention_file_mcp_mode_without_skills_omits_section() { + // MCP mode + zero skills ⇒ no `# Skills disponibles` section at all. + let doc = compose_convention_file("/root", "", "# Persona", &[], &[], None, true); + assert!(!doc.contains("# Skills disponibles")); + assert!(!doc.contains("idea_skill_read")); + } + /// Builds a memory index entry for the convention-file composition tests. fn mem(slug_str: &str, title: &str, hook: &str, kind: MemoryType) -> MemoryIndexEntry { MemoryIndexEntry { diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 9a34bf1..c0c3f65 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -100,9 +100,9 @@ pub use project::{ pub use remote::{ConnectRemote, ConnectRemoteInput, ConnectRemoteOutput}; pub use skill::{ AssignSkillToAgent, AssignSkillToAgentInput, CreateSkill, CreateSkillInput, CreateSkillOutput, - DeleteSkill, DeleteSkillInput, ListSkills, ListSkillsInput, ListSkillsOutput, - UnassignSkillFromAgent, UnassignSkillFromAgentInput, UpdateSkill, UpdateSkillInput, - UpdateSkillOutput, + DeleteSkill, DeleteSkillInput, ListSkills, ListSkillsInput, ListSkillsOutput, ReadSkill, + ReadSkillInput, UnassignSkillFromAgent, UnassignSkillFromAgentInput, UpdateSkill, + UpdateSkillInput, UpdateSkillOutput, }; pub use template::{ AgentDrift, CreateAgentFromTemplate, CreateAgentFromTemplateInput, diff --git a/crates/application/src/orchestrator/service.rs b/crates/application/src/orchestrator/service.rs index 61fdec6..7e2a992 100644 --- a/crates/application/src/orchestrator/service.rs +++ b/crates/application/src/orchestrator/service.rs @@ -43,7 +43,7 @@ use crate::orchestrator::{ ProposeContext, ProposeContextInput, ProposeOutcome, ReadContext, ReadContextInput, ReadMemory, ReadMemoryInput, WriteMemory, WriteMemoryInput, }; -use crate::skill::{CreateSkill, CreateSkillInput}; +use crate::skill::{CreateSkill, CreateSkillInput, ReadSkill, ReadSkillInput}; use crate::terminal::{CloseTerminal, CloseTerminalInput, StructuredSessions, TerminalSessions}; /// Default terminal geometry for an orchestrator-launched agent cell. The UI @@ -303,6 +303,11 @@ pub struct OrchestratorService { /// [`Self::with_memory_harvest`] ; `None` ⇒ aucun harvest (zéro régression). Un /// échec de harvest ne transforme **jamais** un succès de délégation en erreur. memory_harvest: Option>, + /// Use case de lecture d'un skill **par nom** (`idea_skill_read`, feature + /// « skills à la MCP »). Compose le port `SkillStore` existant (aucun nouveau + /// port). Injecté via [`Self::with_read_skill`] ; `None` ⇒ la commande + /// `skill.read` renvoie une erreur typée (call sites/tests legacy restent verts). + read_skill: Option>, } /// Bundle des quatre use cases C7 sous [`domain::fileguard::FileGuard`], injectés @@ -367,9 +372,19 @@ impl OrchestratorService { clock: None, structured: None, memory_harvest: None, + read_skill: None, } } + /// Branche le use case [`ReadSkill`] (`skill.read`/`idea_skill_read`). Builder + /// additif (signature de [`Self::new`] inchangée) ; `None` ⇒ la commande + /// renvoie une erreur typée « not configured ». + #[must_use] + pub fn with_read_skill(mut self, read_skill: Arc) -> Self { + self.read_skill = Some(read_skill); + self + } + /// Branche le registre des [`StructuredSessions`] (§17.5) pour router un `ask` /// vers une cible **structurée** sur sa propre session. Builder additif (signature /// de [`Self::new`] inchangée) ; `None` ⇒ chemin PTY/mailbox legacy uniquement. @@ -588,6 +603,9 @@ impl OrchestratorService { OrchestratorCommand::ReadMemory { slug, requester } => { self.read_memory(project, slug, requester).await } + OrchestratorCommand::ReadSkill { name, requester } => { + self.read_skill(project, name, requester).await + } OrchestratorCommand::WriteMemory { slug, content, @@ -682,6 +700,31 @@ impl OrchestratorService { }) } + /// `skill.read` → resolves a skill by name (project scope first, then global) + /// and returns its Markdown body inline in the outcome's `reply`. Read-only: + /// `requester` is accepted for symmetry with the other read tools but skill + /// reads take no [`domain::fileguard::FileGuard`] lease. + async fn read_skill( + &self, + project: &Project, + name: String, + _requester: ConversationParty, + ) -> Result { + let read_skill = self.read_skill.as_deref().ok_or_else(|| { + AppError::Invalid("the idea_skill_read tool is not configured".to_owned()) + })?; + let md = read_skill + .execute(ReadSkillInput { + name: name.clone(), + project_root: project.root.clone(), + }) + .await?; + Ok(OrchestratorOutcome { + detail: format!("read skill {name}"), + reply: Some(md.into_string()), + }) + } + /// `memory.write` → writes a note under an exclusive write-lease. async fn write_memory( &self, @@ -1867,6 +1910,9 @@ impl OrchestratorService { .create_skill .execute(CreateSkillInput { name: name.clone(), + // `idea_create_skill` carries no description argument; the affordance + // falls back to the body's first line (see `effective_description`). + description: None, content, scope, project_root: project.root.clone(), diff --git a/crates/application/src/skill/mod.rs b/crates/application/src/skill/mod.rs index 8b6b62b..b5f06f1 100644 --- a/crates/application/src/skill/mod.rs +++ b/crates/application/src/skill/mod.rs @@ -15,7 +15,7 @@ mod usecases; pub use usecases::{ AssignSkillToAgent, AssignSkillToAgentInput, CreateSkill, CreateSkillInput, CreateSkillOutput, - DeleteSkill, DeleteSkillInput, ListSkills, ListSkillsInput, ListSkillsOutput, - UnassignSkillFromAgent, UnassignSkillFromAgentInput, UpdateSkill, UpdateSkillInput, - UpdateSkillOutput, + DeleteSkill, DeleteSkillInput, ListSkills, ListSkillsInput, ListSkillsOutput, ReadSkill, + ReadSkillInput, UnassignSkillFromAgent, UnassignSkillFromAgentInput, UpdateSkill, + UpdateSkillInput, UpdateSkillOutput, }; diff --git a/crates/application/src/skill/usecases.rs b/crates/application/src/skill/usecases.rs index a285f22..71dd0ff 100644 --- a/crates/application/src/skill/usecases.rs +++ b/crates/application/src/skill/usecases.rs @@ -25,6 +25,10 @@ use crate::error::AppError; pub struct CreateSkillInput { /// Display name (also the `.md` stem on disk). pub name: String, + /// Optional one-line affordance description (feature « skills à la MCP »). + /// `None`/empty ⇒ the skill falls back to the first line of its body when + /// surfaced (see [`domain::Skill::effective_description`]). + pub description: Option, /// Initial Markdown body. pub content: String, /// Scope the skill is created in (selects its backing store). @@ -61,7 +65,8 @@ impl CreateSkill { pub async fn execute(&self, input: CreateSkillInput) -> Result { let id = SkillId::from_uuid(self.ids.new_uuid()); let skill = Skill::new(id, input.name, MarkdownDoc::new(input.content), input.scope) - .map_err(|e| AppError::Invalid(e.to_string()))?; + .map_err(|e| AppError::Invalid(e.to_string()))? + .with_description(input.description); self.skills.save(&skill, &input.project_root).await?; Ok(CreateSkillOutput { skill }) } @@ -204,6 +209,77 @@ impl DeleteSkill { } } +// --------------------------------------------------------------------------- +// ReadSkill +// --------------------------------------------------------------------------- + +/// Input for [`ReadSkill::execute`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReadSkillInput { + /// Skill display name to resolve (case-insensitive). + pub name: String, + /// Active project root (used for the [`SkillScope::Project`] lookup). + pub project_root: ProjectPath, +} + +/// Reads a skill's Markdown body **by name** — the application side of the MCP +/// `idea_skill_read` tool (feature « skills à la MCP »). +/// +/// Resolution follows the affordance contract: **project scope first, then +/// global** (a project skill shadows a global one of the same name). It composes +/// the **existing** [`SkillStore`] port only — no new port. Read-only. +pub struct ReadSkill { + skills: Arc, +} + +impl ReadSkill { + /// Builds the use case from the existing skill store port. + #[must_use] + pub fn new(skills: Arc) -> Self { + Self { skills } + } + + /// Resolves `name` in `scope`, returning the single match. + /// + /// `Ok(None)` ⇒ no skill by that name in this scope; `Err(Invalid)` ⇒ the + /// name is **ambiguous** (more than one skill shares it in this scope). + async fn resolve_in( + &self, + scope: SkillScope, + input: &ReadSkillInput, + ) -> Result, AppError> { + let all = self.skills.list(scope, &input.project_root).await?; + let mut matches = all + .into_iter() + .filter(|s| s.name.eq_ignore_ascii_case(&input.name)); + match (matches.next(), matches.next()) { + (None, _) => Ok(None), + (Some(skill), None) => Ok(Some(skill)), + (Some(_), Some(_)) => Err(AppError::Invalid(format!( + "skill name `{}` is ambiguous in {scope:?} scope (several skills share it)", + input.name + ))), + } + } + + /// Resolves the skill by name and returns its Markdown body. + /// + /// # Errors + /// - [`AppError::Invalid`] if the name is ambiguous within a scope, + /// - [`AppError::NotFound`] if no skill carries that name in either scope, + /// - [`AppError::Store`] on a store failure. + pub async fn execute(&self, input: ReadSkillInput) -> Result { + // Project scope shadows global: try it first, then fall back to global. + if let Some(skill) = self.resolve_in(SkillScope::Project, &input).await? { + return Ok(skill.content_md); + } + if let Some(skill) = self.resolve_in(SkillScope::Global, &input).await? { + return Ok(skill.content_md); + } + Err(AppError::NotFound(format!("skill `{}`", input.name))) + } +} + // --------------------------------------------------------------------------- // AssignSkillToAgent / UnassignSkillFromAgent // --------------------------------------------------------------------------- @@ -349,3 +425,185 @@ impl UnassignSkillFromAgent { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + use async_trait::async_trait; + use domain::ports::{SkillStore, StoreError}; + + /// In-memory [`SkillStore`] fake: skills are bucketed by scope, ignoring the + /// project root (the [`ReadSkill`] resolution logic is root-agnostic — it only + /// switches scope). Enough to exercise project-first resolution, shadowing, + /// ambiguity and absence without touching disk. + #[derive(Default)] + struct FakeSkillStore { + global: Mutex>, + project: Mutex>, + } + + impl FakeSkillStore { + fn bucket(&self, scope: SkillScope) -> &Mutex> { + match scope { + SkillScope::Global => &self.global, + SkillScope::Project => &self.project, + } + } + + fn push(&self, skill: Skill) { + self.bucket(skill.scope).lock().unwrap().push(skill); + } + } + + #[async_trait] + impl SkillStore for FakeSkillStore { + async fn list( + &self, + scope: SkillScope, + _root: &ProjectPath, + ) -> Result, StoreError> { + Ok(self.bucket(scope).lock().unwrap().clone()) + } + async fn get( + &self, + scope: SkillScope, + _root: &ProjectPath, + id: SkillId, + ) -> Result { + self.bucket(scope) + .lock() + .unwrap() + .iter() + .find(|s| s.id == id) + .cloned() + .ok_or(StoreError::NotFound) + } + async fn save(&self, skill: &Skill, _root: &ProjectPath) -> Result<(), StoreError> { + self.push(skill.clone()); + Ok(()) + } + async fn delete( + &self, + _scope: SkillScope, + _root: &ProjectPath, + _id: SkillId, + ) -> Result<(), StoreError> { + Ok(()) + } + } + + fn root() -> ProjectPath { + ProjectPath::new("/proj").unwrap() + } + + fn skill(id: u128, name: &str, body: &str, scope: SkillScope) -> Skill { + Skill::new( + SkillId::from_uuid(uuid::Uuid::from_u128(id)), + name, + MarkdownDoc::new(body), + scope, + ) + .unwrap() + } + + fn read_skill_uc(store: Arc) -> ReadSkill { + ReadSkill::new(store) + } + + #[tokio::test] + async fn read_skill_resolves_project_scope() { + // (a) skill present in the project scope ⇒ its body is returned. + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "deploy", "PROJECT_BODY", SkillScope::Project)); + + let body = read_skill_uc(store) + .execute(ReadSkillInput { + name: "deploy".to_owned(), + project_root: root(), + }) + .await + .unwrap(); + assert_eq!(body.as_str(), "PROJECT_BODY"); + } + + #[tokio::test] + async fn read_skill_falls_back_to_global_scope() { + // (b) absent from project but present globally ⇒ resolved via global. + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "deploy", "GLOBAL_BODY", SkillScope::Global)); + + let body = read_skill_uc(store) + .execute(ReadSkillInput { + name: "deploy".to_owned(), + project_root: root(), + }) + .await + .unwrap(); + assert_eq!(body.as_str(), "GLOBAL_BODY"); + } + + #[tokio::test] + async fn read_skill_project_shadows_global() { + // (c) same name in both scopes ⇒ project wins. + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "deploy", "GLOBAL_BODY", SkillScope::Global)); + store.push(skill(2, "deploy", "PROJECT_BODY", SkillScope::Project)); + + let body = read_skill_uc(store) + .execute(ReadSkillInput { + name: "deploy".to_owned(), + project_root: root(), + }) + .await + .unwrap(); + assert_eq!(body.as_str(), "PROJECT_BODY"); + } + + #[tokio::test] + async fn read_skill_is_case_insensitive() { + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "Deploy", "BODY", SkillScope::Project)); + + let body = read_skill_uc(store) + .execute(ReadSkillInput { + name: "deploy".to_owned(), + project_root: root(), + }) + .await + .unwrap(); + assert_eq!(body.as_str(), "BODY"); + } + + #[tokio::test] + async fn read_skill_unknown_is_not_found() { + // (d) unknown in both scopes ⇒ NotFound. + let store = Arc::new(FakeSkillStore::default()); + let err = read_skill_uc(store) + .execute(ReadSkillInput { + name: "ghost".to_owned(), + project_root: root(), + }) + .await + .unwrap_err(); + assert!(matches!(err, AppError::NotFound(_)), "got {err:?}"); + } + + #[tokio::test] + async fn read_skill_ambiguous_name_is_invalid() { + // (e) two skills share a name within a scope ⇒ Invalid (ambiguous). + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "deploy", "ONE", SkillScope::Project)); + store.push(skill(2, "Deploy", "TWO", SkillScope::Project)); + + let err = read_skill_uc(store) + .execute(ReadSkillInput { + name: "deploy".to_owned(), + project_root: root(), + }) + .await + .unwrap_err(); + assert!(matches!(err, AppError::Invalid(_)), "got {err:?}"); + } +} diff --git a/crates/application/tests/skill_usecases.rs b/crates/application/tests/skill_usecases.rs index 0b37e3a..4cee8a8 100644 --- a/crates/application/tests/skill_usecases.rs +++ b/crates/application/tests/skill_usecases.rs @@ -195,6 +195,7 @@ async fn create_skill_persists_in_its_scope() { let out = CreateSkill::new(Arc::new(store.clone()), Arc::new(SeqIds::new())) .execute(CreateSkillInput { name: "refactor".to_owned(), + description: Some("Refactors code".to_owned()), content: "# body".to_owned(), scope: SkillScope::Project, project_root: root(), @@ -203,6 +204,8 @@ async fn create_skill_persists_in_its_scope() { .unwrap(); assert_eq!(out.skill.scope, SkillScope::Project); + // The one-line affordance description flows through the use case onto the skill. + assert_eq!(out.skill.description.as_deref(), Some("Refactors code")); assert_eq!( store .list(SkillScope::Project, &root()) @@ -224,6 +227,7 @@ async fn create_skill_rejects_empty_content() { let err = CreateSkill::new(Arc::new(store), Arc::new(SeqIds::new())) .execute(CreateSkillInput { name: "k".to_owned(), + description: None, content: String::new(), scope: SkillScope::Global, project_root: root(), diff --git a/crates/domain/src/orchestrator.rs b/crates/domain/src/orchestrator.rs index 19ff4de..a772dbd 100644 --- a/crates/domain/src/orchestrator.rs +++ b/crates/domain/src/orchestrator.rs @@ -248,6 +248,17 @@ pub enum OrchestratorCommand { /// The reading party (handshake identity). requester: ConversationParty, }, + /// Read a reusable skill's Markdown body **by name** (`idea_skill_read`, + /// feature « skills à la MCP »). Resolution is project-scope-first then global; + /// the body is returned inline. Read-only — no [`crate::fileguard::FileGuard`] + /// lease (skills are not mutated through this path). + ReadSkill { + /// Skill display name to resolve (case-insensitive). + name: String, + /// The party that issued the read (handshake identity). Carried for + /// symmetry/auditing with the other read tools; skill reads need no lease. + requester: ConversationParty, + }, /// Write a memory note under the [`crate::fileguard::FileGuard`] (cadrage C7). /// Memory is project-shared; written directly under a write-lease. WriteMemory { @@ -345,6 +356,10 @@ impl OrchestratorRequest { slug: self.optional_slug(), requester: self.requester_party(), }), + "skill.read" => Ok(OrchestratorCommand::ReadSkill { + name: self.require_name(action)?, + requester: self.requester_party(), + }), "memory.write" => Ok(OrchestratorCommand::WriteMemory { slug: self.require("slug", action, self.slug.as_deref())?, content: self.require("content", action, self.content.as_deref())?, diff --git a/crates/domain/src/skill.rs b/crates/domain/src/skill.rs index 450cd9c..e86d57e 100644 --- a/crates/domain/src/skill.rs +++ b/crates/domain/src/skill.rs @@ -39,6 +39,12 @@ pub struct Skill { pub id: SkillId, /// Display name (also used as the `.md` file stem on disk). pub name: String, + /// Optional one-line description surfaced as an *affordance* at high altitude + /// in an agent's convention file (« à la MCP »), so the agent knows the skill + /// exists and what it does without reading the whole body. `None`/empty ⇒ + /// [`Skill::effective_description`] falls back to the first line of the body. + #[serde(default)] + pub description: Option, /// Markdown body — the workflow injected into an agent's convention file. pub content_md: MarkdownDoc, /// Scope (selects the backing store). @@ -67,18 +73,57 @@ impl Skill { Ok(Self { id, name, + description: None, content_md, scope, }) } + /// Returns this skill with its one-line [`Skill::description`] set (builder). + /// + /// Kept separate from [`Skill::new`] so every existing call site is untouched + /// (the field is purely additive). An empty/whitespace description is treated + /// as absent by [`Skill::effective_description`], so no normalisation here. + #[must_use] + pub fn with_description(mut self, description: Option) -> Self { + self.description = description; + self + } + + /// The description to surface for this skill, with a deterministic fallback. + /// + /// Returns [`Skill::description`] when present and non-blank (trimmed); + /// otherwise the **first non-empty line** of the body, stripped of any leading + /// `#` (Markdown heading marker) and trimmed. When the body has no usable line + /// either, returns an **empty `String`** (chosen over `Option` so callers can + /// format it unconditionally — an empty affordance line is harmless). + #[must_use] + pub fn effective_description(&self) -> String { + if let Some(desc) = &self.description { + let trimmed = desc.trim(); + if !trimmed.is_empty() { + return trimmed.to_owned(); + } + } + self.content_md + .as_str() + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .map(|line| line.trim_start_matches('#').trim().to_owned()) + .unwrap_or_default() + } + /// Returns a copy of this skill with replaced content, re-validating the - /// non-empty invariant. + /// non-empty invariant and **preserving** the [`Skill::description`]. /// /// # Errors /// [`DomainError::EmptyField`] if `content_md` is empty. pub fn with_content(&self, content_md: MarkdownDoc) -> Result { - Skill::new(self.id, self.name.clone(), content_md, self.scope) + Ok( + Skill::new(self.id, self.name.clone(), content_md, self.scope)? + .with_description(self.description.clone()), + ) } } @@ -109,3 +154,98 @@ impl From<&Skill> for SkillRef { Self::new(skill.id, skill.scope) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ids::SkillId; + + fn skill(body: &str) -> Skill { + Skill::new( + SkillId::from_uuid(uuid::Uuid::from_u128(1)), + "demo", + MarkdownDoc::new(body), + SkillScope::Global, + ) + .unwrap() + } + + // -- effective_description -------------------------------------------- + + #[test] + fn effective_description_returns_explicit_description_trimmed() { + // (a) explicit non-empty description ⇒ returned trimmed. + let s = skill("# Heading\n\nbody").with_description(Some(" Refactors code ".to_owned())); + assert_eq!(s.effective_description(), "Refactors code"); + } + + #[test] + fn effective_description_falls_back_to_first_body_line_without_hash() { + // (b) description None ⇒ first non-empty body line, `#` stripped & trimmed. + let s = skill("\n\n# My Skill Title \n\nrest of body"); + assert_eq!(s.effective_description(), "My Skill Title"); + } + + #[test] + fn effective_description_first_line_without_heading_marker() { + // (b') first non-empty line that is not a heading is returned verbatim (trimmed). + let s = skill(" just a plain first line\nsecond"); + assert_eq!(s.effective_description(), "just a plain first line"); + } + + #[test] + fn effective_description_blank_body_yields_empty_string() { + // (c) body that is only whitespace ⇒ empty String. + let s = skill(" \n\t\n "); + assert_eq!(s.effective_description(), ""); + } + + #[test] + fn effective_description_blank_description_falls_back_to_body() { + // (d) description Some("") / whitespace-only ⇒ treated as absent ⇒ body fallback. + let empty = skill("# Body Heading").with_description(Some(String::new())); + assert_eq!(empty.effective_description(), "Body Heading"); + + let spaces = skill("# Body Heading").with_description(Some(" \t ".to_owned())); + assert_eq!(spaces.effective_description(), "Body Heading"); + } + + // -- with_content preserves description ------------------------------- + + #[test] + fn with_content_preserves_description() { + let s = skill("old body").with_description(Some("kept".to_owned())); + let updated = s.with_content(MarkdownDoc::new("new body")).unwrap(); + assert_eq!(updated.description.as_deref(), Some("kept")); + assert_eq!(updated.content_md.as_str(), "new body"); + } + + #[test] + fn with_content_rejects_empty_body() { + let s = skill("old body"); + assert!(s.with_content(MarkdownDoc::new("")).is_err()); + } + + // -- serde: description is additive / defaults to None ---------------- + + #[test] + fn deserialize_legacy_skill_without_description_defaults_to_none() { + // A skill JSON written before `description` existed must deserialise with + // `description: None` thanks to `#[serde(default)]`. + let id = uuid::Uuid::from_u128(7); + let json = + format!(r##"{{"id":"{id}","name":"legacy","contentMd":"# body","scope":"global"}}"##); + let parsed: Skill = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.description, None); + assert_eq!(parsed.name, "legacy"); + } + + #[test] + fn serde_round_trip_with_description() { + let original = skill("# body").with_description(Some("affordance".to_owned())); + let json = serde_json::to_string(&original).unwrap(); + let back: Skill = serde_json::from_str(&json).unwrap(); + assert_eq!(back, original); + assert_eq!(back.description.as_deref(), Some("affordance")); + } +} diff --git a/crates/infrastructure/src/orchestrator/mcp/tools.rs b/crates/infrastructure/src/orchestrator/mcp/tools.rs index 5c5f7c4..e7fdc6d 100644 --- a/crates/infrastructure/src/orchestrator/mcp/tools.rs +++ b/crates/infrastructure/src/orchestrator/mcp/tools.rs @@ -51,7 +51,11 @@ pub enum ToolMapError { pub fn tool_returns_reply(tool: &str) -> bool { matches!( tool, - "idea_ask_agent" | "idea_list_agents" | "idea_context_read" | "idea_memory_read" + "idea_ask_agent" + | "idea_list_agents" + | "idea_context_read" + | "idea_memory_read" + | "idea_skill_read" ) } @@ -201,6 +205,21 @@ pub fn catalogue() -> Vec { "additionalProperties": false }), }, + ToolDef { + name: "idea_skill_read", + description: "Read the full body of an IdeA skill assigned to you, by name — call this \ + to actually execute/follow a skill listed in your « Skills disponibles » \ + section. Resolves project scope first, then global. Returns the skill's \ + Markdown inline.", + input_schema: json!({ + "type": "object", + "properties": { + "name": { "type": "string", "description": "Skill name (as listed in your context)." } + }, + "required": ["name"], + "additionalProperties": false + }), + }, ToolDef { name: "idea_create_skill", description: "Create a reusable IdeA skill (Markdown) in the project or global scope.", @@ -320,6 +339,14 @@ pub fn map_tool_call( content: s("content"), ..base() }, + "idea_skill_read" => OrchestratorRequest { + request_type: Some("skill.read".to_owned()), + // The requester (handshake identity) is carried for symmetry with the + // other read tools; `validate` derives the requester party from it. + requested_by: Some(requester.to_owned()), + name: s("name"), + ..base() + }, "idea_create_skill" => OrchestratorRequest { request_type: Some("skill.create".to_owned()), name: s("name"), diff --git a/crates/infrastructure/src/store/skill.rs b/crates/infrastructure/src/store/skill.rs index 1c034a6..d3d9bd5 100644 --- a/crates/infrastructure/src/store/skill.rs +++ b/crates/infrastructure/src/store/skill.rs @@ -58,6 +58,11 @@ const INDEX_VERSION: u32 = 1; struct IndexEntry { id: SkillId, name: String, + /// One-line affordance description (feature « skills à la MCP »). `#[serde(default)]` + /// is **mandatory** for backward-compat: legacy `index.json` written before this + /// field existed deserialise with `None` instead of failing. + #[serde(default)] + description: Option, content_hash: String, } @@ -189,6 +194,7 @@ impl FsSkillStore { MarkdownDoc::new(content), scope, ) + .map(|skill| skill.with_description(entry.description.clone())) .map_err(|e| StoreError::Serialization(e.to_string())) } } @@ -239,6 +245,7 @@ impl SkillStore for FsSkillStore { let row = IndexEntry { id: skill.id, name: skill.name.clone(), + description: skill.description.clone(), content_hash: content_hash(&skill.content_md), }; if let Some(slot) = index.skills.iter_mut().find(|e| e.id == skill.id) { @@ -266,3 +273,129 @@ impl SkillStore for FsSkillStore { self.write_index(scope, root, &index).await } } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashMap; + use std::sync::Mutex; + + use domain::ports::{DirEntry, FsError}; + + /// In-memory [`FileSystem`] mock (same minimal shape as the memory-store tests). + #[derive(Default)] + struct MemFs { + files: Mutex>>, + } + + impl MemFs { + fn arc() -> Arc { + Arc::new(Self::default()) + } + } + + #[async_trait] + impl FileSystem for MemFs { + async fn read(&self, path: &RemotePath) -> Result, FsError> { + self.files + .lock() + .unwrap() + .get(path.as_str()) + .cloned() + .ok_or_else(|| FsError::NotFound(path.as_str().to_string())) + } + async fn write(&self, path: &RemotePath, data: &[u8]) -> Result<(), FsError> { + self.files + .lock() + .unwrap() + .insert(path.as_str().to_string(), data.to_vec()); + Ok(()) + } + async fn exists(&self, path: &RemotePath) -> Result { + Ok(self.files.lock().unwrap().contains_key(path.as_str())) + } + async fn create_dir_all(&self, _path: &RemotePath) -> Result<(), FsError> { + Ok(()) + } + async fn list(&self, _path: &RemotePath) -> Result, FsError> { + Ok(Vec::new()) + } + async fn symlink(&self, _src: &RemotePath, _dst: &RemotePath) -> Result<(), FsError> { + Ok(()) + } + } + + fn root() -> ProjectPath { + ProjectPath::new("/proj").unwrap() + } + + fn skill(id: u128, name: &str, body: &str) -> Skill { + Skill::new( + SkillId::from_uuid(uuid::Uuid::from_u128(id)), + name, + MarkdownDoc::new(body), + SkillScope::Global, + ) + .unwrap() + } + + #[tokio::test] + async fn legacy_index_without_description_loads_with_none() { + // A legacy `index.json` written before the `description` field existed must + // deserialise (thanks to `#[serde(default)]`) and load with `description: None`. + let fs = MemFs::arc(); + let id = uuid::Uuid::from_u128(42); + let legacy_index = format!( + r#"{{"version":1,"skills":[{{"id":"{id}","name":"legacy","contentHash":"abc"}}]}}"# + ); + fs.write( + &RemotePath::new("/app/skills/index.json".to_owned()), + legacy_index.as_bytes(), + ) + .await + .unwrap(); + fs.write( + &RemotePath::new(format!("/app/skills/md/{id}.md")), + b"# legacy body", + ) + .await + .unwrap(); + + let store = FsSkillStore::new(fs, "/app"); + let listed = store.list(SkillScope::Global, &root()).await.unwrap(); + + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].name, "legacy"); + assert_eq!(listed[0].description, None); + assert_eq!(listed[0].content_md.as_str(), "# legacy body"); + } + + #[tokio::test] + async fn save_then_load_round_trips_description() { + let store = FsSkillStore::new(MemFs::arc(), "/app"); + let s = skill(1, "refactor", "# Refactor\n\nbody") + .with_description(Some("Refactors code".to_owned())); + + store.save(&s, &root()).await.unwrap(); + + let got = store.get(SkillScope::Global, &root(), s.id).await.unwrap(); + assert_eq!(got.description.as_deref(), Some("Refactors code")); + assert_eq!(got, s); + + // Also surfaces through `list`. + let listed = store.list(SkillScope::Global, &root()).await.unwrap(); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].description.as_deref(), Some("Refactors code")); + } + + #[tokio::test] + async fn save_without_description_loads_as_none() { + let store = FsSkillStore::new(MemFs::arc(), "/app"); + let s = skill(2, "plain", "# Plain\n\nbody"); + + store.save(&s, &root()).await.unwrap(); + + let got = store.get(SkillScope::Global, &root(), s.id).await.unwrap(); + assert_eq!(got.description, None); + } +} diff --git a/crates/infrastructure/tests/mcp_server.rs b/crates/infrastructure/tests/mcp_server.rs index 8415014..0da2353 100644 --- a/crates/infrastructure/tests/mcp_server.rs +++ b/crates/infrastructure/tests/mcp_server.rs @@ -506,6 +506,8 @@ async fn tools_list_advertises_the_seven_idea_tools_with_schemas() { "idea_stop_agent", "idea_update_context", "idea_create_skill", + // Read a skill's body by name (feature « skills à la MCP », T6). + "idea_skill_read", // FileGuard-mediated context/memory tools (cadrage C7). "idea_context_read", "idea_context_propose", @@ -519,8 +521,8 @@ async fn tools_list_advertises_the_seven_idea_tools_with_schemas() { } assert_eq!( tools.len(), - 11, - "exactly the eleven idea_* tools (7 base + 4 FileGuard C7); got {names:?}" + 12, + "exactly the twelve idea_* tools (7 base + idea_skill_read + 4 FileGuard C7); got {names:?}" ); // Every tool advertises an object input schema.