ticket115: snapshot runtime skills assignés + durcissement idea_skill_read

This commit is contained in:
2026-07-31 13:56:18 +02:00
parent fe5fe7cb70
commit fbe3c51fd4
8 changed files with 611 additions and 215 deletions

View File

@ -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,
};

View File

@ -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<dyn SkillStore>,
}
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<dyn SkillStore>) -> 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<Option<Skill>, 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<OrchestrationCapabilitySnapshot, AppError> {
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<Vec<(AssignedSkillSnapshot, MarkdownDoc)>, 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<MarkdownDoc, AppError> {
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<dyn AgentContextStore>,
resolver: Arc<AssignedSkillResolver>,
}
impl ReadSkill {
/// Builds the use case from existing ports.
#[must_use]
pub fn new(contexts: Arc<dyn AgentContextStore>, skills: Arc<dyn SkillStore>) -> 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<dyn AgentContextStore>,
resolver: Arc<AssignedSkillResolver>,
) -> 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<MarkdownDoc, AppError> {
// 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<AgentManifest>,
}
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<MarkdownDoc, StoreError> {
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<AgentManifest, StoreError> {
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<SkillRef>) -> 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<FakeSkillStore>) -> ReadSkill {
ReadSkill::new(store)
fn read_skill_uc(
store: Arc<FakeSkillStore>,
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();