diff --git a/.ideai/memory/feature-agent-skill-awareness-design.md b/.ideai/memory/feature-agent-skill-awareness-design.md new file mode 100644 index 0000000..f53b632 --- /dev/null +++ b/.ideai/memory/feature-agent-skill-awareness-design.md @@ -0,0 +1,50 @@ +--- +name: feature-agent-skill-awareness-design +description: Design valide + decoupage de la feature « surfacer les skills assignes a un agent a la maniere MCP » (manifeste + idea_skill_read), a reprendre apres relance IdeA. +metadata: + type: project +--- +# Feature : skills connus de l'agent « a la MCP » + +Demarree le 2026-06-17. Branche Git **`feature/agent-skill-awareness`** (basee sur `develop` @ 8452333, qui contient deja le fix context-guard). + +## ETAT 2026-06-17 (apres-midi) : slice backend T1->T5 FAIT, VERT, COMMITTE. +DevBackend a livre T1->T5, QA a couvert (23 tests, 1212 passed). Commits sur `feature/agent-skill-awareness` : +- `ab34363` feat(skill-awareness): T1->T5 (manifeste # Skills disponibles + outil MCP idea_skill_read, description+effective_description, use case ReadSkill, cablage state.rs, dump legacy conserve en mode sans-MCP). +- `1a10d67` fix(test): compteur d'outils MCP 11->12 (une 2e assertion codee en dur dans `crates/app-tauri/src/state.rs` ~l.2813 que QA avait ratee ; QA n'avait corrige que `infrastructure/tests/mcp_server.rs`). +- `e93a2c1` = cherry-pick du fix cold-start (cf. Bug 8 dans [[mcp-bridge-and-delegation-runtime-notes]]) — present aussi sur la branche dediee. +`cargo test --workspace` VERT sur l'arbre combine. **L'orchestrateur a committe lui-meme** (l'agent Git etait injoignable a cause du Bug 8) — A FAIRE RELIRE/REBASER PAR GIT une fois le canal restaure. + +**RESTE :** T6 (champ description dans creation/edition skill cote front) + T7 (e2e : rebuild AppImage avec le fix Bug 8, relance, agent neuf + skill assigne -> voit le manifeste + appelle idea_skill_read). Puis Git decide les merges (feature->develop ; fix cold-start->develop+main). + +## AJOUT 2026-06-17 (soir) : brief « capacites IdeA » INCONDITIONNEL — VERT, COMMITTE (566bff4) +Besoin utilisateur elargi : un agent neuf ignorait TOUT le perimetre IdeA (pas que les skills). Cas vecu : « agremente le contexte du projet » -> l'agent reinvente son propre systeme de contexte car le bloc `# Orchestration IdeA` ne parlait QUE de delegation. Fix (GO Architect, meme fonction pure `compose_convention_file` lifecycle.rs, zero port/entite) : ajout d'un brief « capacites IdeA » TOUJOURS present (decrit la CAPACITE, jamais le contenu -> survit a memoire/contexte vides), dans les 2 surfaces, AVANT le persona : +- mode MCP : nomme les outils contexte (`idea_context_read`/`idea_context_propose` avec semantique single-writer global HONNETE : proposition sans target = enregistree pour validation, PAS appliquee ; `idea_update_context` pour le .md d'un agent), memoire partagee (`idea_memory_read`/`idea_memory_write`), skills (`idea_skill_read`/`idea_create_skill`). Martele « le contexte projet d'IdeA EST le contexte, n'improvise jamais ton propre fichier ». +- mode non-MCP : MEMES concepts via fichiers `.ideai/` UNIQUEMENT (CONTEXT.md, memory/+MEMORY.md, skills .md), AUCUN nom d'outil `idea_*` (cloisonnement strict des 2 surfaces — sinon casse `mcp_prose_*`/`non_mcp_prose_*`). +QA : 5 tests dedies (brief inconditionnel des 3 capacites a zero contenu ; single-writer honnete ; non-MCP sans outil ; cloisonnement ; ordre avant persona). `cargo test -p application` = 0 failed (lib 52, agent_lifecycle 59). 1 assert existant adapte (`mcp_mode_without_skills_omits_section`). Commit atomique **566bff4** sur `feature/agent-skill-awareness` (Git a laisse les .ideai/ runtime hors commit). Merge feature->develop differe par Git jusqu'a T6+T7 (unite d'integration unique). + +## Historique +Cycle initialement stoppe avant le code : la delegation MCP a wedge (Bug 7, puis Bug 8 residuel cf. [[mcp-bridge-and-delegation-runtime-notes]]). + +## Besoin +Un agent neuf dans un projet neuf ignore les skills qui lui sont assignes (cas vecu : `build-appimage` assigne mais ignore, tout reinvente). Objectif : l'agent est **automatiquement au courant** de ses skills, comme il l'est des outils MCP, sans que l'utilisateur ait a le lui dire. + +## Decision produit (utilisateur, 2026-06-17) : approche « Manifeste + idea_skill_read » +Diagnostic Architect : les skills sont DEJA injectes, mais (A) en VRAC (corps complet) en fin de `CLAUDE.md` -> lus comme de la doc, ignores ; (B) seulement au (re)lancement. On recadre « a la MCP » : affordances nommees+decrites en tete de contexte + chargement du corps a la demande. + +- Bloc **« # Skills disponibles »** injecte JUSTE APRES le bloc « # Orchestration IdeA » (haute altitude), listant `**** — ` ; prose imperative + renvoi a `idea_skill_read(name=…)`. Omis si zero skill. +- Nouvel outil MCP **`idea_skill_read(name)`** read-only, miroir exact de `idea_context_read`/`idea_memory_read` ; resout par nom (scope projet puis global), renvoie `content_md` inline ; erreur typee si introuvable/ambigu. +- Champ **`description: Option`** sur l'entite `Skill` + helper `effective_description()` (fallback : 1ere ligne non vide du `content_md`, nettoyee du `#`). `#[serde(default)]` sur l'index (retro-compat `index.json` legacy obligatoire). +- Mode **sans MCP** (`profile.mcp` absent) : conserver l'ancien dump du corps complet (decision 4.2(b), zero regression). + +## Decoupage (commits atomiques par tache verte, decides par Git) +- **T1 domaine** `crates/domain/src/skill.rs` : champ `description` + `effective_description()`. +- **T2 store** `crates/infrastructure/src/store/skill.rs` (+ application/skill) : `description` dans IndexEntry (`serde(default)`), propage via `CreateSkill`. +- **T3 convention-file** `crates/application/src/agent/lifecycle.rs` : section « # Skills disponibles » dans `compose_convention_file` (~l.2568, juste apres bloc Orchestration ~l.2586) ; alleger `resolve_skills` (~l.1202) pour n'utiliser que name+description (index, pas le corps) ; garder dump corps si non-MCP. +- **T4 outil MCP** `crates/infrastructure/src/orchestrator/mcp/tools.rs` + commande `OrchestratorCommand::ReadSkill { name, requester }` + methode `read_skill` dans `OrchestratorService` + petit use case `ReadSkill` (compose le port `SkillStore` existant, AUCUN nouveau port). +- **T5 cablage** `crates/app-tauri/src/state.rs` : injecter `ReadSkill` dans `OrchestratorService` (builder additif, comme le fix context-guard) + re-exports application (orchestrator/mod.rs + lib.rs). +- **T6 front (differable)** `frontend` : champ description dans creation/edition de skill. +- **T7 e2e** : rebuild AppImage + relance, agent neuf + skill assigne -> voit le manifeste + appelle `idea_skill_read`. + +## Pieges (Architect) +Retro-compat `index.json` (serde default) ; resolution de nom ambigue (projet d'abord, erreur typee sinon) ; un skill assigne en cours de session reste invisible jusqu'au relaunch (limite inherente, comme MCP) ; pas de nouveau port, domaine = +1 champ, composition root seul point de cablage. GO Architect. 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..ac7034f 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 @@ -3043,6 +3050,8 @@ mod mcp_serve_peer_tests { "idea_context_propose", "idea_memory_read", "idea_memory_write", + // Skill-awareness : lecture à la demande du corps d'un skill. + "idea_skill_read", ] { assert!( names.contains(&expected), @@ -3051,8 +3060,8 @@ mod mcp_serve_peer_tests { } 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 + 4 FileGuard C7 + idea_skill_read); got {names:?}" ); drop(client); // EOF ⇒ serve loop ends diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index c038434..c755a19 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 @@ -2615,6 +2624,33 @@ pub(crate) fn compose_convention_file( résultat. Ne réponds **jamais** uniquement en texte : seul `idea_reply` \ débloque l'agent qui t'a sollicité.\n\n", ); + // Capacités IdeA (feature skill-awareness) : briefing à HAUTE ALTITUDE, + // télégraphique, injecté à CHAQUE lancement (coût token → pas d'exemples ni de + // schémas). On expose la CAPACITÉ (jamais le contenu) : contexte projet géré par + // IdeA, mémoire durable partagée, skills. But : un agent neuf ne réinvente pas son + // propre contexte/mémoire/workflow alors qu'IdeA les fournit déjà. + out.push_str( + "**Contexte projet** : le contexte projet d'IdeA EST le contexte du projet — \ + n'improvise JAMAIS ton propre fichier de contexte/config. Lis-le avec \ + `idea_context_read` (sans `target`). Pour le faire évoluer, soumets une \ + proposition via `idea_context_propose` : le contexte projet global est \ + single-writer (réservé à l'orchestrateur), donc une proposition sans `target` \ + n'est PAS appliquée directement, elle est enregistrée pour validation. Pour \ + écrire le `.md` d'un agent précis : `idea_update_context` (ou \ + `idea_context_propose` avec `target`).\n\n", + ); + out.push_str( + "**Mémoire durable projet** : partagée entre TOUS les agents du projet. Lis-la \ + avec `idea_memory_read` (sans `slug` = index agrégé, ou un slug pour une note), \ + écris une note stable avec `idea_memory_write`. N'invente pas ton propre \ + stockage de mémoire.\n\n", + ); + out.push_str( + "**Skills** : workflows réutilisables. Ceux qui te sont assignés te sont listés \ + plus haut dans ce fichier ; charge le détail d'un skill avec \ + `idea_skill_read(name=…)` ; crée-en un avec `idea_create_skill`. Ne réinvente \ + pas un workflow déjà couvert par un skill.\n\n", + ); } else { out.push_str( "Pour déléguer une tâche à un autre agent, n'utilise jamais les subagents \ @@ -2622,11 +2658,51 @@ pub(crate) fn compose_convention_file( `.ideai/requests//` ; IdeA lancera ou réattachera l'agent cible \ avec son propre AI Profile, son contexte et sa mémoire.\n\n", ); + // Capacités IdeA, surface SANS MCP : mêmes concepts décrits UNIQUEMENT via les + // fichiers/dossiers `.ideai/` — aucun nom d'outil `idea_*` ici (cloisonnement + // strict des deux surfaces). Télégraphique, capacité (jamais contenu). + out.push_str( + "Le contexte projet partagé vit dans `.ideai/CONTEXT.md` (géré par IdeA) — ne \ + crée pas ton propre fichier de contexte/config, c'est CELUI-LÀ le contexte du \ + projet.\n\n", + ); + out.push_str( + "La mémoire durable partagée entre tous les agents vit dans `.ideai/memory/` \ + (notes `.md` + index `MEMORY.md`). N'invente pas ton propre stockage de \ + mémoire.\n\n", + ); + out.push_str( + "Les skills sont des workflows réutilisables, stockés comme des `.md` gérés par \ + IdeA. Ne réinvente pas un workflow déjà couvert par un skill.\n\n", + ); } out.push_str(skill_awareness(mcp_enabled)); 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 +2711,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 +3024,70 @@ 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` affordance section at all. + // NB : l'outil `idea_skill_read` reste évoqué par le briefing « capacités IdeA » + // (toujours présent), donc on n'asserte plus son absence — seulement celle de la + // section d'affordances, qui, elle, dépend bien des skills assignés. + let doc = compose_convention_file("/root", "", "# Persona", &[], &[], None, true); + assert!(!doc.contains("# Skills disponibles")); + } + /// Builds a memory index entry for the convention-file composition tests. fn mem(slug_str: &str, title: &str, hook: &str, kind: MemoryType) -> MemoryIndexEntry { MemoryIndexEntry { @@ -3107,6 +3251,158 @@ mod tests { ); } + #[test] + fn compose_convention_file_mcp_briefs_all_three_capabilities_even_when_empty() { + // Cœur du besoin (feature skill-awareness) : le briefing « capacités IdeA » + // est INCONDITIONNEL. Projet/agent neuf et vide (zéro skill, zéro mémoire, + // contexte vide) ⇒ les 3 capacités (contexte, mémoire, skills) DOIVENT quand + // même être décrites, avec les noms des outils MCP correspondants. + let doc = compose_convention_file("/root", "", "# Persona", &[], &[], None, true); + + // Capacité contexte projet. + assert!( + doc.contains("idea_context_read"), + "brief must surface idea_context_read even with empty content" + ); + assert!(doc.contains("idea_context_propose")); + assert!( + doc.contains("idea_update_context"), + "brief must surface idea_update_context for per-agent .md writes" + ); + // Capacité mémoire durable partagée. + assert!(doc.contains("idea_memory_read")); + assert!(doc.contains("idea_memory_write")); + // Capacité skills. + assert!(doc.contains("idea_skill_read")); + assert!(doc.contains("idea_create_skill")); + } + + #[test] + fn compose_convention_file_mcp_context_brief_is_single_writer_honest() { + // La prose contexte ne doit PAS laisser croire qu'une proposition globale est + // appliquée directement. On asserte sur la nuance réellement écrite par le dev : + // « single-writer » + le fait qu'une proposition sans `target` est enregistrée + // « pour validation » (et donc « n'est PAS appliquée directement »). + let doc = compose_convention_file("/root", "", "# Persona", &[], &[], None, true); + + assert!( + doc.contains("single-writer"), + "context brief must name the single-writer constraint" + ); + assert!( + doc.contains("validation"), + "context brief must say a proposal is recorded for validation" + ); + assert!( + doc.contains("n'est PAS appliquée directement"), + "context brief must be honest that a global proposal isn't auto-applied" + ); + } + + #[test] + fn compose_convention_file_non_mcp_briefs_capabilities_via_ideai_files_no_tools() { + // Surface SANS MCP : mêmes 3 capacités décrites UNIQUEMENT via les fichiers + // `.ideai/` — et AUCUN nom d'outil `idea_*` (cloisonnement strict des surfaces). + let doc = compose_convention_file("/root", "", "# Persona", &[], &[], None, false); + + // Capacités décrites par leurs fichiers/dossiers. + assert!( + doc.contains(".ideai/CONTEXT.md"), + "non-MCP brief must point to the .ideai/CONTEXT.md context file" + ); + assert!( + doc.contains(".ideai/memory/"), + "non-MCP brief must point to the .ideai/memory/ store" + ); + assert!( + doc.contains("MEMORY.md"), + "non-MCP brief must name the MEMORY.md index" + ); + + // AUCUN nom d'outil `idea_*` ne fuit (asserts forts sur les noms d'OUTILS, pas + // sur la sous-chaîne brute `idea` qui matcherait `.ideai/`). + assert!( + !doc.contains("idea_context"), + "no idea_context* tool in file surface" + ); + assert!( + !doc.contains("idea_memory"), + "no idea_memory* tool in file surface" + ); + assert!( + !doc.contains("idea_skill"), + "no idea_skill* tool in file surface" + ); + assert!( + !doc.contains("idea_create"), + "no idea_create* tool in file surface" + ); + assert!( + !doc.contains("idea_ask_agent"), + "no idea_ask_agent in file surface" + ); + assert!( + !doc.contains("idea_update_context"), + "no idea_update_context in file surface" + ); + } + + #[test] + fn compose_convention_file_surfaces_stay_cloistered_on_capabilities() { + // Cloisonnement : la prose MCP ne mentionne pas le protocole fichier + // `.ideai/requests`, et la prose fichier ne mentionne aucun nom d'outil. + let mcp = compose_convention_file("/root", "", "# Persona", &[], &[], None, true); + let file = compose_convention_file("/root", "", "# Persona", &[], &[], None, false); + + assert!( + !mcp.contains(".ideai/requests"), + "MCP capability brief must not leak the file delegation protocol" + ); + // Aucun nom d'outil `idea_*` dans la surface fichier (recoupe le test dédié, + // gardé vert ici sur l'ensemble des outils des 3 capacités + délégation). + for tool in [ + "idea_context_read", + "idea_context_propose", + "idea_update_context", + "idea_memory_read", + "idea_memory_write", + "idea_skill_read", + "idea_create_skill", + "idea_ask_agent", + "idea_reply", + ] { + assert!( + !file.contains(tool), + "non-MCP surface must not mention the tool {tool}" + ); + } + } + + #[test] + fn compose_convention_file_capability_brief_precedes_the_persona_in_both_modes() { + // Ordre : le briefing capacités apparaît AVANT le persona dans les 2 modes. + // On ancre le brief sur un marqueur stable propre à chaque surface. + let mcp = compose_convention_file("/root", "", "# Persona", &[], &[], None, true); + let mcp_brief_at = mcp + .find("idea_context_read") + .expect("MCP capability brief present"); + let mcp_persona_at = mcp.find("# Persona").expect("persona present"); + assert!( + mcp_brief_at < mcp_persona_at, + "MCP capability brief must come before the persona" + ); + + let file = compose_convention_file("/root", "", "# Persona", &[], &[], None, false); + let file_brief_at = file + .find(".ideai/CONTEXT.md") + .expect("file capability brief present"); + let file_persona_at = file.find("# Persona").expect("persona present"); + assert!( + file_brief_at < file_persona_at, + "file capability brief must come before the persona" + ); + } + #[test] fn mcp_declaration_with_runtime_points_the_bridge_at_the_real_endpoint() { // B-0 — a PTY-launched MCP-capable CLI (Claude/Codex REPL) auto-discovers the 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.