From fbe3c51fd4ee2732352b0ae81fb25561cbc0a281 Mon Sep 17 00:00:00 2001 From: Blomios Date: Fri, 31 Jul 2026 13:56:18 +0200 Subject: [PATCH] =?UTF-8?q?ticket115:=20snapshot=20runtime=20skills=20assi?= =?UTF-8?q?gn=C3=A9s=20+=20durcissement=20idea=5Fskill=5Fread?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/application/src/agent/lifecycle.rs | 273 +++++++---- crates/application/src/lib.rs | 8 +- .../application/src/orchestrator/service.rs | 5 +- crates/application/src/skill/mod.rs | 8 +- crates/application/src/skill/usecases.rs | 448 ++++++++++++++---- crates/backend/src/lib.rs | 11 +- crates/domain/src/lib.rs | 5 + crates/domain/src/orchestrator.rs | 68 ++- 8 files changed, 611 insertions(+), 215 deletions(-) diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index 9f8b474..eaf1aea 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -18,7 +18,7 @@ use domain::ports::{ AgentContextStore, AgentRuntime, AgentSessionFactory, ContextInjectionPlan, EventBus, FileSystem, FsError, IdGenerator, MemoryQuery, MemoryRecall, PermissionStore, PreparedContext, ProfileStore, ProjectStore, PtyPort, RemotePath, SecretStore, SessionPlan, SkillStore, - SpawnSpec, StoreError, StructuredProviderLaunchPolicy, SystemPermissionStore, + SpawnSpec, StructuredProviderLaunchPolicy, SystemPermissionStore, }; use domain::profile::{ McpConfigStrategy, OpenCodeProviderConfig, StructuredAdapter, CODEX_CODE_MODE_FEATURES_TABLE, @@ -29,9 +29,9 @@ use domain::{ bound_handoff_summary, Agent, AgentId, AgentManifest, AgentOrigin, AgentProfile, ContextInjection, ConversationId, ConversationParty, DomainEvent, EffectivePermissions, Handoff, HandoffStore, ManifestEntry, MarkdownDoc, MemoryIndexEntry, MemoryType, NetworkPolicy, - NodeId, PermissionProjector, Posture, ProfileId, Project, ProjectPath, ProjectedFile, - ProjectionContext, ProjectorKey, ProviderSessionStore, PtySize, SessionId, SessionKind, - SessionStatus, Skill, TerminalSession, HANDOFF_SUMMARY_MAX_CHARS, + NodeId, OrchestrationCapabilitySnapshot, PermissionProjector, Posture, ProfileId, Project, + ProjectPath, ProjectedFile, ProjectionContext, ProjectorKey, ProviderSessionStore, PtySize, + SessionId, SessionKind, SessionStatus, TerminalSession, HANDOFF_SUMMARY_MAX_CHARS, }; use domain::live_state::WorkStatus; @@ -42,6 +42,7 @@ use crate::model_server::{ EnsureLocalModelServer, EnsureLocalModelServerInput, ModelServerUseGuard, }; use crate::project::project_context_path; +use crate::skill::AssignedSkillResolver; use crate::terminal::{StructuredSessions, TerminalSessions}; use crate::workstate::GetLiveStateLean; @@ -117,6 +118,36 @@ pub struct InjectedLiveRow { pub intent: String, } +/// One assigned skill resolved for the effective context. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedAssignedSkill { + /// Agent-facing snapshot metadata. + pub snapshot: domain::AssignedSkillSnapshot, + /// Full Markdown body, used only on non-MCP profiles that cannot call + /// `idea_skill_read`. + pub content: MarkdownDoc, +} + +/// Effective model context resolved once per runtime launch and shared by every +/// provider path. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EffectiveAgentContext { + /// Raw persona Markdown from the agent `.md`. + pub persona: MarkdownDoc, + /// Shared project context. + pub project_context: String, + /// Provider-agnostic orchestration capabilities exposed to the agent. + pub capabilities: OrchestrationCapabilitySnapshot, + /// Resolved assigned skill bodies for fallback non-MCP injection. + pub assigned_skills: Vec, + /// Project-memory recall selected for this launch. + pub memory: Vec, + /// Optional conversation handoff for this launch. + pub handoff: Option, + /// Lean live-state rows for other agents. + pub live_rows: Vec, +} + /// Fournit le [`GetLiveStateLean`] **lié au project root** du lancement en cours /// (lot LS4), pour injecter l'aperçu `# État du projet` dans le contexte composé. /// @@ -1153,7 +1184,7 @@ pub struct LaunchAgent { runtime: Arc, fs: Arc, pty: Arc, - skills: Arc, + assigned_skills: Arc, sessions: Arc, events: Arc, ids: Arc, @@ -1243,7 +1274,7 @@ impl LaunchAgent { runtime, fs, pty, - skills, + assigned_skills: Arc::new(AssignedSkillResolver::new(skills)), sessions, events, ids, @@ -1381,31 +1412,44 @@ impl LaunchAgent { self } - /// Resolves the Markdown bodies of an agent's assigned skills, in the - /// **manifest order** (deterministic). A skill that no longer exists in its - /// store (deleted out from under the assignment) is silently skipped — a - /// dangling [`domain::SkillRef`] must not block a launch. - /// - /// # Errors - /// [`AppError::Store`] on any store failure other than a missing skill. - async fn resolve_skills( + /// Builds the effective runtime context once, before the provider split, so + /// PTY, structured/headless, hot relaunch and handoff rebuild paths consume the + /// same capability snapshot. + async fn build_effective_context( &self, + project: &Project, + manifest: &AgentManifest, agent: &Agent, - root: &ProjectPath, - ) -> Result, AppError> { - let mut out = Vec::with_capacity(agent.skills.len()); - for skill_ref in &agent.skills { - match self - .skills - .get(skill_ref.scope, root, skill_ref.skill_id) - .await - { - Ok(skill) => out.push(skill), - Err(StoreError::NotFound) => {} - Err(e) => return Err(e.into()), - } - } - Ok(out) + persona: MarkdownDoc, + cell_conversation_id: Option<&str>, + ) -> Result { + let assigned_skills: Vec = self + .assigned_skills + .resolve_for_agent_with_content(agent, &project.root) + .await? + .into_iter() + .map(|(snapshot, content)| ResolvedAssignedSkill { snapshot, content }) + .collect(); + let capabilities = OrchestrationCapabilitySnapshot::new( + assigned_skills.iter().map(|s| s.snapshot.clone()).collect(), + ); + let project_context = self.resolve_project_context(project).await?; + let memory = self.resolve_memory(&project.root, persona.as_str()).await; + let handoff = self + .resolve_handoff(&project.root, cell_conversation_id) + .await; + let live_rows = self + .resolve_live_state(&project.root, manifest, agent.id) + .await; + Ok(EffectiveAgentContext { + persona, + project_context, + capabilities, + assigned_skills, + memory, + handoff, + live_rows, + }) } /// Resolves the project's memory recall (index/hooks) to inject into the @@ -1742,25 +1786,18 @@ impl LaunchAgent { self.runtime .prepare_invocation(&profile, &prepared, &run_dir, &session_plan)?; - // 5. Resolve the agent's assigned skills (their `.md` bodies), then apply - // the injection plan side effects *before* spawning. - let skills = self.resolve_skills(&agent, &input.project.root).await?; - let project_context = self.resolve_project_context(&input.project).await?; - let memory = self - .resolve_memory(&input.project.root, content.as_str()) - .await; - // Reprise conversationnelle (lot P7) : best-effort, additif. Si la cellule a une - // conversation et qu'un handoff existe, son résumé est injecté dans le convention - // file (à côté de la mémoire projet). Indépendant du provider/resumable id. - let handoff = self - .resolve_handoff(&input.project.root, input.conversation_id.as_deref()) - .await; - // Aperçu live-state des autres agents (lot LS4) : best-effort, additif. Injecté - // comme section `# État du projet`. Ordre manifeste, self exclu, borné, vide ⇒ - // section omise. Indépendant du handoff/mémoire. - let live_rows = self - .resolve_live_state(&input.project.root, &manifest, agent.id) - .await; + // 5. Build the effective context once, then apply the injection side + // effects before spawning. The capability snapshot inside this context + // is also the authorization source for `idea_skill_read`. + let effective_context = self + .build_effective_context( + &input.project, + &manifest, + &agent, + content.clone(), + input.conversation_id.as_deref(), + ) + .await?; // Best-effort contextual embedder suggestion (LOT C3, §14.5.5): the agent // has just read the project memory, so this is the moment to check whether a // semantic embedder would now help. Fully isolated from the launch outcome — @@ -1776,12 +1813,7 @@ impl LaunchAgent { self.apply_injection( &input.project, &agent.context_path, - &content, - &project_context, - &skills, - &memory, - handoff.as_ref(), - &live_rows, + &effective_context, profile.mcp.is_some(), &mut spec, ) @@ -2338,12 +2370,7 @@ impl LaunchAgent { &self, project: &Project, context_rel_path: &str, - content: &MarkdownDoc, - project_context: &str, - skills: &[Skill], - memory: &[MemoryIndexEntry], - handoff: Option<&Handoff>, - live_rows: &[InjectedLiveRow], + effective: &EffectiveAgentContext, mcp_enabled: bool, spec: &mut SpawnSpec, ) -> Result<(), AppError> { @@ -2356,16 +2383,8 @@ impl LaunchAgent { // composed: an absolute project-root header (so the agent knows // where to operate, since its cwd is *not* the root), the agent's // persona `.md`, then the bodies of its assigned skills (§14.2). - let document = compose_convention_file( - project.root.as_str(), - project_context, - content.as_str(), - skills, - memory, - handoff, - live_rows, - mcp_enabled, - ); + let document = + compose_convention_file(project.root.as_str(), effective, mcp_enabled); let path = RemotePath::new(join(&spec.cwd, &target)); self.fs.write(&path, document.as_bytes()).await?; } @@ -3331,12 +3350,7 @@ fn append_block(input: &str, block: &str) -> String { #[must_use] pub(crate) fn compose_convention_file( project_root: &str, - project_context: &str, - agent_md: &str, - skills: &[Skill], - memory: &[MemoryIndexEntry], - handoff: Option<&Handoff>, - live_rows: &[InjectedLiveRow], + effective: &EffectiveAgentContext, mcp_enabled: bool, ) -> String { let mut out = String::new(); @@ -3432,49 +3446,55 @@ pub(crate) fn compose_convention_file( // 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() { + if mcp_enabled && !effective.capabilities.assigned_skills.is_empty() { out.push_str("# Skills disponibles\n\n"); + out.push_str("Snapshot version: "); + out.push_str(&effective.capabilities.version.to_string()); + out.push_str("\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 { + for skill in &effective.capabilities.assigned_skills { out.push_str("**"); out.push_str(&skill.name); out.push_str("** — "); - out.push_str(&skill.effective_description()); + out.push_str(&skill.description); out.push('\n'); } out.push_str("\n---\n\n"); } - if !project_context.trim().is_empty() { + if !effective.project_context.trim().is_empty() { out.push_str("# Contexte projet\n\n"); - out.push_str(project_context.trim()); + out.push_str(effective.project_context.trim()); out.push_str("\n\n---\n\n"); } - out.push_str(agent_md); + out.push_str(effective.persona.as_str()); // 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 { + if !effective.assigned_skills.is_empty() && !mcp_enabled { out.push_str("\n\n---\n\n# Skills\n"); - for skill in skills { + out.push_str("\nSnapshot version: "); + out.push_str(&effective.capabilities.version.to_string()); + out.push('\n'); + for skill in &effective.assigned_skills { out.push_str("\n## "); - out.push_str(&skill.name); + out.push_str(&skill.snapshot.name); out.push_str("\n\n"); - out.push_str(skill.content_md.as_str()); + out.push_str(skill.content.as_str()); out.push('\n'); } } - if !memory.is_empty() { + if !effective.memory.is_empty() { out.push_str("\n\n---\n\n# Mémoire projet\n\n"); - for entry in memory { + for entry in &effective.memory { out.push_str("- ["); out.push_str(&entry.title); out.push_str("]("); @@ -3493,14 +3513,14 @@ pub(crate) fn compose_convention_file( // ligne par agent (`- **Nom** — status · intent`), intent omis si vide. Jamais // d'UUID (ticket/lastDelegation), de progress ni de transcript. `live_rows` vide ⇒ // section entièrement omise (document octet-identique à sans-section). - if !live_rows.is_empty() { + if !effective.live_rows.is_empty() { out.push_str("\n\n---\n\n# État du projet\n\n"); out.push_str( "Aperçu de ce que font les autres agents en ce moment (last-writer-wins, \ non temps-réel). Pour le détail ou pour publier ton propre statut, utilise \ `idea_workstate_read` / `idea_workstate_set`.\n\n", ); - for row in live_rows { + for row in &effective.live_rows { out.push_str("- **"); out.push_str(&row.name); out.push_str("** — "); @@ -3515,7 +3535,7 @@ pub(crate) fn compose_convention_file( // Reprise conversationnelle (lot P7) : section finale, la plus situationnelle // (« où on en était »), placée après la mémoire projet. Omise sans handoff. - if let Some(handoff) = handoff { + if let Some(handoff) = effective.handoff.as_ref() { out.push_str("\n\n---\n\n# Reprise de la conversation\n\n"); if let Some(objective) = &handoff.objective { if !objective.trim().is_empty() { @@ -3612,6 +3632,46 @@ fn slugify(name: &str) -> String { #[cfg(test)] mod tests { use super::*; + use domain::Skill; + + #[allow(clippy::too_many_arguments)] + fn compose_convention_file( + project_root: &str, + project_context: &str, + agent_md: &str, + skills: &[domain::Skill], + memory: &[MemoryIndexEntry], + handoff: Option<&Handoff>, + live_rows: &[InjectedLiveRow], + mcp_enabled: bool, + ) -> String { + let assigned_skills: Vec = skills + .iter() + .enumerate() + .map(|(index, skill)| ResolvedAssignedSkill { + snapshot: domain::AssignedSkillSnapshot { + skill_id: skill.id, + scope: skill.scope, + name: skill.name.clone(), + description: skill.effective_description(), + assignment_index: index as u32, + }, + content: skill.content_md.clone(), + }) + .collect(); + let effective = EffectiveAgentContext { + persona: MarkdownDoc::new(agent_md), + project_context: project_context.to_owned(), + capabilities: OrchestrationCapabilitySnapshot::new( + assigned_skills.iter().map(|s| s.snapshot.clone()).collect(), + ), + assigned_skills, + memory: memory.to_vec(), + handoff: handoff.cloned(), + live_rows: live_rows.to_vec(), + }; + super::compose_convention_file(project_root, &effective, mcp_enabled) + } #[test] fn agent_run_dir_is_under_ideai_run_and_unique_per_agent() { @@ -3731,6 +3791,37 @@ mod tests { assert!(doc.contains("## review")); } + #[test] + fn compose_convention_file_exposes_assigned_skill_snapshot_version() { + let skill = Skill::new( + domain::SkillId::from_uuid(uuid::Uuid::from_u128(1)), + "review", + MarkdownDoc::new("REVIEW_BODY"), + domain::SkillScope::Global, + ) + .unwrap(); + + for mcp_enabled in [true, false] { + let doc = compose_convention_file( + "/root", + "", + "# Persona", + std::slice::from_ref(&skill), + &[], + None, + &[], + mcp_enabled, + ); + assert!( + doc.contains(&format!( + "Snapshot version: {}", + domain::ORCHESTRATION_CAPABILITY_SNAPSHOT_VERSION + )), + "assigned skill snapshot version must be visible (mcp_enabled={mcp_enabled})" + ); + } + } + #[test] fn compose_convention_file_skill_awareness_precedes_project_context() { let doc = compose_convention_file( diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 81e0d85..db81457 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -169,10 +169,10 @@ pub use project::{ }; pub use remote::{ConnectRemote, ConnectRemoteInput, ConnectRemoteOutput}; pub use skill::{ - AssignSkillToAgent, AssignSkillToAgentInput, CreateSkill, CreateSkillInput, CreateSkillOutput, - DeleteSkill, DeleteSkillInput, ListSkills, ListSkillsInput, ListSkillsOutput, ReadSkill, - ReadSkillInput, UnassignSkillFromAgent, UnassignSkillFromAgentInput, UpdateSkill, - UpdateSkillInput, UpdateSkillOutput, + AssignSkillToAgent, AssignSkillToAgentInput, AssignedSkillResolver, CreateSkill, + CreateSkillInput, CreateSkillOutput, DeleteSkill, DeleteSkillInput, ListSkills, + ListSkillsInput, ListSkillsOutput, ReadSkill, ReadSkillInput, UnassignSkillFromAgent, + UnassignSkillFromAgentInput, UpdateSkill, UpdateSkillInput, UpdateSkillOutput, }; pub use sprints::{ normalized_reorder, AssignTicketToSprint, AssignTicketToSprintInput, diff --git a/crates/application/src/orchestrator/service.rs b/crates/application/src/orchestrator/service.rs index e6f0239..55b9124 100644 --- a/crates/application/src/orchestrator/service.rs +++ b/crates/application/src/orchestrator/service.rs @@ -1353,7 +1353,7 @@ impl OrchestratorService { &self, project: &Project, name: String, - _requester: ConversationParty, + 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()) @@ -1361,7 +1361,8 @@ impl OrchestratorService { let md = read_skill .execute(ReadSkillInput { name: name.clone(), - project_root: project.root.clone(), + project: project.clone(), + requester, }) .await?; Ok(OrchestratorOutcome { diff --git a/crates/application/src/skill/mod.rs b/crates/application/src/skill/mod.rs index b5f06f1..051e25e 100644 --- a/crates/application/src/skill/mod.rs +++ b/crates/application/src/skill/mod.rs @@ -14,8 +14,8 @@ mod usecases; pub use usecases::{ - AssignSkillToAgent, AssignSkillToAgentInput, CreateSkill, CreateSkillInput, CreateSkillOutput, - DeleteSkill, DeleteSkillInput, ListSkills, ListSkillsInput, ListSkillsOutput, ReadSkill, - ReadSkillInput, UnassignSkillFromAgent, UnassignSkillFromAgentInput, UpdateSkill, - UpdateSkillInput, UpdateSkillOutput, + AssignSkillToAgent, AssignSkillToAgentInput, AssignedSkillResolver, CreateSkill, + CreateSkillInput, CreateSkillOutput, 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 71dd0ff..c388511 100644 --- a/crates/application/src/skill/usecases.rs +++ b/crates/application/src/skill/usecases.rs @@ -8,10 +8,11 @@ use std::sync::Arc; -use domain::ports::{AgentContextStore, EventBus, IdGenerator, SkillStore}; +use domain::ports::{AgentContextStore, EventBus, IdGenerator, SkillStore, StoreError}; use domain::{ - AgentId, AgentManifest, DomainEvent, MarkdownDoc, Project, ProjectPath, Skill, SkillId, - SkillRef, SkillScope, + Agent, AgentId, AgentManifest, AssignedSkillSnapshot, ConversationParty, DomainEvent, + MarkdownDoc, OrchestrationCapabilitySnapshot, Project, ProjectPath, Skill, SkillId, SkillRef, + SkillScope, }; use crate::error::AppError; @@ -213,70 +214,173 @@ 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 { +/// Resolves assigned skills into the runtime capability snapshot used by both +/// context rendering and `idea_skill_read` authorization. +pub struct AssignedSkillResolver { skills: Arc, } -impl ReadSkill { - /// Builds the use case from the existing skill store port. +impl AssignedSkillResolver { + /// Builds the resolver from the existing skill store port. #[must_use] pub fn new(skills: Arc) -> Self { Self { skills } } - /// Resolves `name` in `scope`, returning the single match. + /// Resolves the assigned skills for `agent` in manifest order. /// - /// `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( + /// Dangling assignments are skipped: a deleted skill must not block a launch, + /// and a skipped skill is therefore not readable via `idea_skill_read`. + /// + /// # Errors + /// [`AppError::Store`] on store failures other than a missing skill. + pub async fn resolve_for_agent( &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 - ))), + agent: &Agent, + root: &ProjectPath, + ) -> Result { + let resolved = self.resolve_for_agent_with_content(agent, root).await?; + Ok(OrchestrationCapabilitySnapshot::new( + resolved.into_iter().map(|(snapshot, _)| snapshot).collect(), + )) + } + + /// Resolves assigned skills and keeps their Markdown bodies for non-MCP + /// fallback context injection. + /// + /// # Errors + /// [`AppError::Store`] on store failures other than a missing skill. + pub async fn resolve_for_agent_with_content( + &self, + agent: &Agent, + root: &ProjectPath, + ) -> Result, AppError> { + let mut assigned = Vec::with_capacity(agent.skills.len()); + for (index, skill_ref) in agent.skills.iter().enumerate() { + match self + .skills + .get(skill_ref.scope, root, skill_ref.skill_id) + .await + { + Ok(skill) => { + let description = skill.effective_description(); + let snapshot = AssignedSkillSnapshot { + skill_id: skill.id, + scope: skill.scope, + name: skill.name, + description, + assignment_index: index as u32, + }; + assigned.push((snapshot, skill.content_md)); + } + Err(StoreError::NotFound) => {} + Err(e) => return Err(e.into()), + } + } + Ok(assigned) + } + + /// Reads the Markdown body for one skill by name, but only inside the supplied + /// assigned-skill snapshot. + /// + /// # Errors + /// - [`AppError::NotFound`] if `name` is not assigned in `snapshot`, + /// - [`AppError::Invalid`] if the assigned snapshot has ambiguous names, + /// - [`AppError::Store`] on a store failure. + pub async fn read_assigned_by_name( + &self, + snapshot: &OrchestrationCapabilitySnapshot, + root: &ProjectPath, + name: &str, + ) -> Result { + let mut matches = snapshot + .assigned_skills + .iter() + .filter(|s| s.name.eq_ignore_ascii_case(name)); + let selected = match (matches.next(), matches.next()) { + (None, _) => return Err(AppError::NotFound(format!("assigned skill `{name}`"))), + (Some(skill), None) => skill, + (Some(_), Some(_)) => { + return Err(AppError::Invalid(format!( + "assigned skill name `{name}` is ambiguous for this agent" + ))) + } + }; + let skill = self + .skills + .get(selected.scope, root, selected.skill_id) + .await?; + Ok(skill.content_md) + } +} + +/// Input for [`ReadSkill::execute`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReadSkillInput { + /// Skill display name to resolve (case-insensitive). + pub name: String, + /// Active project. + pub project: Project, + /// Current requester identity from the orchestration handshake. + pub requester: ConversationParty, +} + +/// Reads an assigned skill's Markdown body **by name** — the application side of +/// the MCP `idea_skill_read` tool. +/// +/// The same [`AssignedSkillResolver`] that builds the runtime capability +/// snapshot authorizes the read. A model can read only skills assigned to the +/// requesting agent in the current manifest. +pub struct ReadSkill { + contexts: Arc, + resolver: Arc, +} + +impl ReadSkill { + /// Builds the use case from existing ports. + #[must_use] + pub fn new(contexts: Arc, skills: Arc) -> Self { + Self { + contexts, + resolver: Arc::new(AssignedSkillResolver::new(skills)), } } - /// Resolves the skill by name and returns its Markdown body. + /// Builds the use case from a shared resolver. + #[must_use] + pub fn with_resolver( + contexts: Arc, + resolver: Arc, + ) -> Self { + Self { contexts, resolver } + } + + /// Resolves the requester's assigned snapshot and returns the skill 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. + /// - [`AppError::Invalid`] if requester is not an agent or the assigned name is ambiguous, + /// - [`AppError::NotFound`] if requester/skill is absent, + /// - [`AppError::Store`] on store failures. 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))) + let requester = input.requester.as_agent().ok_or_else(|| { + AppError::Invalid("idea_skill_read requires an agent requester".to_owned()) + })?; + let manifest = self.contexts.load_manifest(&input.project).await?; + let entry = manifest + .entries + .iter() + .find(|e| e.agent_id == requester) + .ok_or_else(|| AppError::NotFound(format!("agent {requester}")))?; + let agent = entry + .to_agent() + .map_err(|e| AppError::Invalid(e.to_string()))?; + let snapshot = self + .resolver + .resolve_for_agent(&agent, &input.project.root) + .await?; + self.resolver + .read_assigned_by_name(&snapshot, &input.project.root, &input.name) + .await } } @@ -432,7 +536,8 @@ mod tests { use std::sync::Mutex; use async_trait::async_trait; - use domain::ports::{SkillStore, StoreError}; + use domain::ports::{AgentContextStore, SkillStore, StoreError}; + use domain::remote::RemoteRef; /// In-memory [`SkillStore`] fake: skills are bucketed by scope, ignoring the /// project root (the [`ReadSkill`] resolution logic is root-agnostic — it only @@ -494,10 +599,88 @@ mod tests { } } + struct FakeAgentContextStore { + manifest: Mutex, + } + + impl FakeAgentContextStore { + fn new(manifest: AgentManifest) -> Self { + Self { + manifest: Mutex::new(manifest), + } + } + } + + #[async_trait] + impl AgentContextStore for FakeAgentContextStore { + async fn read_context( + &self, + _project: &Project, + _agent: &AgentId, + ) -> Result { + Ok(MarkdownDoc::new("# Persona")) + } + + async fn write_context( + &self, + _project: &Project, + _agent: &AgentId, + _md: &MarkdownDoc, + ) -> Result<(), StoreError> { + Ok(()) + } + + async fn load_manifest(&self, _project: &Project) -> Result { + Ok(self.manifest.lock().unwrap().clone()) + } + + async fn save_manifest( + &self, + _project: &Project, + manifest: &AgentManifest, + ) -> Result<(), StoreError> { + *self.manifest.lock().unwrap() = manifest.clone(); + Ok(()) + } + } + fn root() -> ProjectPath { ProjectPath::new("/proj").unwrap() } + fn project() -> Project { + Project::new( + domain::ProjectId::from_uuid(uuid::Uuid::from_u128(100)), + "Project", + root(), + RemoteRef::Local, + 0, + ) + .unwrap() + } + + fn agent_id(n: u128) -> AgentId { + AgentId::from_uuid(uuid::Uuid::from_u128(n)) + } + + fn profile_id(n: u128) -> domain::ProfileId { + domain::ProfileId::from_uuid(uuid::Uuid::from_u128(n)) + } + + fn manifest_for(agent_id: AgentId, skills: Vec) -> AgentManifest { + let agent = Agent::new( + agent_id, + "Dev", + "agents/dev.md", + profile_id(900), + domain::AgentOrigin::Scratch, + false, + ) + .unwrap() + .with_skills(skills); + AgentManifest::new(1, vec![domain::ManifestEntry::from_agent(&agent)]).unwrap() + } + fn skill(id: u128, name: &str, body: &str, scope: SkillScope) -> Skill { Skill::new( SkillId::from_uuid(uuid::Uuid::from_u128(id)), @@ -508,20 +691,35 @@ mod tests { .unwrap() } - fn read_skill_uc(store: Arc) -> ReadSkill { - ReadSkill::new(store) + fn read_skill_uc( + store: Arc, + manifest: AgentManifest, + ) -> (ReadSkill, Project, AgentId) { + let requester = manifest.entries[0].agent_id; + let contexts = Arc::new(FakeAgentContextStore::new(manifest)); + (ReadSkill::new(contexts, store), project(), requester) } #[tokio::test] - async fn read_skill_resolves_project_scope() { - // (a) skill present in the project scope ⇒ its body is returned. + async fn read_skill_returns_assigned_project_skill() { let store = Arc::new(FakeSkillStore::default()); store.push(skill(1, "deploy", "PROJECT_BODY", SkillScope::Project)); - let body = read_skill_uc(store) + let requester = agent_id(10); + let manifest = manifest_for( + requester, + vec![SkillRef { + skill_id: SkillId::from_uuid(uuid::Uuid::from_u128(1)), + scope: SkillScope::Project, + }], + ); + let (uc, project, requester) = read_skill_uc(store, manifest); + + let body = uc .execute(ReadSkillInput { name: "deploy".to_owned(), - project_root: root(), + project, + requester: ConversationParty::agent(requester), }) .await .unwrap(); @@ -529,15 +727,25 @@ mod tests { } #[tokio::test] - async fn read_skill_falls_back_to_global_scope() { - // (b) absent from project but present globally ⇒ resolved via global. + async fn read_skill_returns_assigned_global_skill() { let store = Arc::new(FakeSkillStore::default()); store.push(skill(1, "deploy", "GLOBAL_BODY", SkillScope::Global)); - let body = read_skill_uc(store) + let requester = agent_id(10); + let manifest = manifest_for( + requester, + vec![SkillRef { + skill_id: SkillId::from_uuid(uuid::Uuid::from_u128(1)), + scope: SkillScope::Global, + }], + ); + let (uc, project, requester) = read_skill_uc(store, manifest); + + let body = uc .execute(ReadSkillInput { name: "deploy".to_owned(), - project_root: root(), + project, + requester: ConversationParty::agent(requester), }) .await .unwrap(); @@ -545,45 +753,19 @@ mod tests { } #[tokio::test] - async fn read_skill_project_shadows_global() { - // (c) same name in both scopes ⇒ project wins. + async fn read_skill_refuses_unassigned_name_even_if_skill_exists() { let store = Arc::new(FakeSkillStore::default()); - store.push(skill(1, "deploy", "GLOBAL_BODY", SkillScope::Global)); - store.push(skill(2, "deploy", "PROJECT_BODY", SkillScope::Project)); + store.push(skill(1, "deploy", "PROJECT_BODY", SkillScope::Project)); - let body = read_skill_uc(store) + let requester = agent_id(10); + let manifest = manifest_for(requester, Vec::new()); + let (uc, project, requester) = read_skill_uc(store, manifest); + + let err = uc .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(), + project, + requester: ConversationParty::agent(requester), }) .await .unwrap_err(); @@ -591,16 +773,76 @@ mod tests { } #[tokio::test] - async fn read_skill_ambiguous_name_is_invalid() { - // (e) two skills share a name within a scope ⇒ Invalid (ambiguous). + async fn read_skill_is_case_insensitive_within_assigned_snapshot() { let store = Arc::new(FakeSkillStore::default()); - store.push(skill(1, "deploy", "ONE", SkillScope::Project)); - store.push(skill(2, "Deploy", "TWO", SkillScope::Project)); + store.push(skill(1, "Deploy", "BODY", SkillScope::Project)); - let err = read_skill_uc(store) + let requester = agent_id(10); + let manifest = manifest_for( + requester, + vec![SkillRef { + skill_id: SkillId::from_uuid(uuid::Uuid::from_u128(1)), + scope: SkillScope::Project, + }], + ); + let (uc, project, requester) = read_skill_uc(store, manifest); + + let body = uc .execute(ReadSkillInput { name: "deploy".to_owned(), - project_root: root(), + project, + requester: ConversationParty::agent(requester), + }) + .await + .unwrap(); + assert_eq!(body.as_str(), "BODY"); + } + + #[tokio::test] + async fn read_skill_requires_agent_requester() { + let store = Arc::new(FakeSkillStore::default()); + let requester = agent_id(10); + let manifest = manifest_for(requester, Vec::new()); + let (uc, project, _) = read_skill_uc(store, manifest); + + let err = uc + .execute(ReadSkillInput { + name: "ghost".to_owned(), + project, + requester: ConversationParty::User, + }) + .await + .unwrap_err(); + assert!(matches!(err, AppError::Invalid(_)), "got {err:?}"); + } + + #[tokio::test] + async fn read_skill_ambiguous_assigned_name_is_invalid() { + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "deploy", "ONE", SkillScope::Project)); + store.push(skill(2, "Deploy", "TWO", SkillScope::Global)); + + let requester = agent_id(10); + let manifest = manifest_for( + requester, + vec![ + SkillRef { + skill_id: SkillId::from_uuid(uuid::Uuid::from_u128(1)), + scope: SkillScope::Project, + }, + SkillRef { + skill_id: SkillId::from_uuid(uuid::Uuid::from_u128(2)), + scope: SkillScope::Global, + }, + ], + ); + let (uc, project, requester) = read_skill_uc(store, manifest); + + let err = uc + .execute(ReadSkillInput { + name: "deploy".to_owned(), + project, + requester: ConversationParty::agent(requester), }) .await .unwrap_err(); diff --git a/crates/backend/src/lib.rs b/crates/backend/src/lib.rs index 485f6d4..8160a49 100644 --- a/crates/backend/src/lib.rs +++ b/crates/backend/src/lib.rs @@ -2227,10 +2227,13 @@ impl BackendCore { 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))); + // Lecture d'un skill par nom pour l'outil MCP `idea_skill_read` : compose + // le SkillStore et le manifeste agent afin d'autoriser uniquement les + // skills assignés au requester courant. + let read_skill = Arc::new(ReadSkill::new( + Arc::clone(&contexts_port), + Arc::clone(&skill_store_port), + )); let assign_skill = Arc::new(AssignSkillToAgent::new( Arc::clone(&contexts_port), Arc::clone(&events_port), diff --git a/crates/domain/src/lib.rs b/crates/domain/src/lib.rs index 0bd19a7..58956b6 100644 --- a/crates/domain/src/lib.rs +++ b/crates/domain/src/lib.rs @@ -169,6 +169,11 @@ pub use memory_harvest::{ MAX_BLOCK_BYTES, MAX_DESCRIPTION_CHARS, }; +pub use orchestrator::{ + AssignedSkillSnapshot, OrchestrationCapabilitySnapshot, + ORCHESTRATION_CAPABILITY_SNAPSHOT_VERSION, +}; + pub use model_catalogue::{ evaluate_compatibility, CliVersion, CompatibilityMatrix, ModelCatalogSource, ModelCatalogueError, ModelCompatibility, diff --git a/crates/domain/src/orchestrator.rs b/crates/domain/src/orchestrator.rs index 7e1cf39..39fd121 100644 --- a/crates/domain/src/orchestrator.rs +++ b/crates/domain/src/orchestrator.rs @@ -14,11 +14,65 @@ use serde::{Deserialize, Serialize}; use crate::conversation::ConversationParty; -use crate::ids::{AgentId, NodeId}; +use crate::ids::{AgentId, NodeId, SkillId}; use crate::live_state::WorkStatus; use crate::mailbox::TicketId; use crate::skill::SkillScope; +/// Current schema version for orchestration capabilities injected into an +/// agent's effective runtime context. +pub const ORCHESTRATION_CAPABILITY_SNAPSHOT_VERSION: u32 = 1; + +/// Runtime snapshot of one skill assigned to an agent. +/// +/// This is the agent-facing capability contract: launch context rendering and +/// `idea_skill_read` authorization are both derived from these snapshots, not +/// from ad hoc skill-name lookups. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AssignedSkillSnapshot { + /// Stable skill id from the selected scope. + pub skill_id: SkillId, + /// Store scope used to resolve the skill body. + pub scope: SkillScope, + /// Display name shown to the model and accepted by `idea_skill_read`. + pub name: String, + /// One-line affordance description shown in the model context. + pub description: String, + /// Position in the agent assignment list, preserving manifest order. + pub assignment_index: u32, +} + +/// Provider-agnostic orchestration capability snapshot for one agent at runtime. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct OrchestrationCapabilitySnapshot { + /// Snapshot schema version. + pub version: u32, + /// Skills assigned to the agent and resolved for this runtime. + pub assigned_skills: Vec, +} + +impl OrchestrationCapabilitySnapshot { + /// Builds an empty snapshot at the current schema version. + #[must_use] + pub const fn empty() -> Self { + Self { + version: ORCHESTRATION_CAPABILITY_SNAPSHOT_VERSION, + assigned_skills: Vec::new(), + } + } + + /// Builds a snapshot from assigned skills. + #[must_use] + pub fn new(assigned_skills: Vec) -> Self { + Self { + version: ORCHESTRATION_CAPABILITY_SNAPSHOT_VERSION, + assigned_skills, + } + } +} + /// Errors raised while validating a raw [`OrchestratorRequest`]. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum OrchestratorError { @@ -293,15 +347,15 @@ 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). + /// Read a reusable skill's Markdown body **by name** (`idea_skill_read`). + /// The application layer authorizes the read against the requester's assigned + /// runtime skill snapshot; unassigned skills are not readable. 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. + /// The party that issued the read (handshake identity). Must be an agent + /// requester for authorization against assigned skills. requester: ConversationParty, }, /// Write a memory note under the [`crate::fileguard::FileGuard`] (cadrage C7).