diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 29de2a63..c53c2c0f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1928,7 +1928,7 @@ Nouvelles commandes (jumelles des commandes PTY existantes ; réutilisent `resol - **Vivant de bout en bout** : `apply_mcp_config` matérialise `.mcp.json` dans le **run dir isolé** de l'agent **AVANT** le split structuré/PTY (`crates/application/src/agent/lifecycle.rs` ~1094) ⇒ Claude/Codex le lisent **nativement**. La déclaration porte l'**exe réel** injecté par `McpRuntime` (`$APPIMAGE` sinon `current_exe`, `crates/app-tauri/src/mcp_endpoint.rs:139` `idea_exe_path`), l'**endpoint loopback** du projet, le `--project` et le `--requester` (agent réel, fin du `"mcp"` figé). - **Endpoint loopback** = **UDS** (Linux/macOS) / **named pipe** (Windows), **zéro port réseau** ; source de vérité unique `mcp_endpoint(project_id)` (`mcp_endpoint.rs`), bindé à l'open / fermé au close (`crates/app-tauri/src/state.rs` `ensure_mcp_server`/`bind_endpoint`). **Fix D1** : cadavre `.sock` (run SIGKILL) **unlinké avant bind** (`state.rs` ~1019-1033) — sinon `EADDRINUSE`. - **Serveur** `McpServer` (`crates/infrastructure/src/orchestrator/mcp/server.rs`) = **jumeau du `FsOrchestratorWatcher`** : autre porte sur le **même** `OrchestratorService::dispatch`, `serve(conn)` **par pair**. Transport `StdioTransport` (JSON Lines stdin/stdout du pont) + `MemoryTransport` (tests, sans socket ni process). Pont = sous-commande `mcp-server` du binaire app-tauri (`mcp_bridge.rs`). -- **Outils** (`mcp/tools.rs`) — **catalogue de 14**, tous mappés 1:1 vers `OrchestratorCommand` et servis par le **même** `dispatch` (les trois portes : fichier, MCP, UI) : `idea_list_agents`, `idea_ask_agent`, `idea_reply`, `idea_launch_agent`, `idea_stop_agent`, `idea_update_context`, `idea_create_skill` (7 base) · `idea_context_read`, `idea_context_propose`, `idea_memory_read`, `idea_memory_write` (4 FileGuard C7) · `idea_skill_read` (skill-awareness) · `idea_workstate_read`, `idea_workstate_set` (LS4 ; `set` n'écrit que la ligne de l'agent courant, via l'identité handshake). +- **Outils** (`mcp/tools.rs`) — **catalogue de 14**, tous mappés 1:1 vers `OrchestratorCommand` et servis par le **même** `dispatch` (les trois portes : fichier, MCP, UI) : `idea_list_agents`, `idea_ask_agent`, `idea_reply`, `idea_launch_agent`, `idea_stop_agent`, `idea_update_context`, `idea_create_skill` (7 base) · `idea_context_read`, `idea_context_propose`, `idea_memory_read`, `idea_memory_write` (4 FileGuard C7) · `idea_skill_read` (skill-awareness ; lecture canonique par ref stable, lookup par nom compatible mais erreur typée `ambiguousSkillName` avec candidates en cas de doublon) · `idea_workstate_read`, `idea_workstate_set` (LS4 ; `set` n'écrit que la ligne de l'agent courant, via l'identité handshake). ### 18.6 Invariant « 1 agent = 1 session vivante » (livré) - Registres `TerminalSessions` + `StructuredSessions` (`crates/application/src/terminal/registry.rs`) agrégés en `LiveSessions` (PTY+structuré). `session_for_agent` (singulier, **non ambigu**) **+** `sessions_for_agent` (pluriel). Garde reattach `Rebind`/`Refuse`/`Idempotent` dans `LaunchAgent` (`lifecycle.rs`). Ancienne ambiguïté `session-registry-agent-ambiguity` = **fermée par construction**. @@ -2418,3 +2418,9 @@ Ok(declared_main || declared_icon || rel.as_str().starts_with("assets/")) 4. Preuve requise : test de purge (installer, écrire via `ctx.storage`, désinstaller, vérifier `plugins/data//` disparu) et absence de tout chemin `.ideai/...` dans les exemples SDK par défaut. *Document maintenu par l'Agent Architecture — base du jalon « cadrage architecture » avant tout code applicatif.* + +### 22.3 #350 — Projection des autorisations agent vers les runtimes plugin + +`PluginMcpServerSpec` transporte l'allowlist exacte `authorizedTools`. Le resolver projet regroupe les `PluginToolRef` par `(pluginId, serverId)` sans perdre leur `toolName` ni leur ordre d'assignation. Pour Codex, chaque table plugin générée écrit `enabled_tools` avec cette seule allowlist et `default_tools_approval_mode = "approve"` : une permission déjà accordée par le projet ne déclenche pas de seconde approbation interactive, tandis qu'un tool non assigné n'entre pas dans la surface appelable. + +Les skills assignés exposent parallèlement une ref stable dans le contexte agent. Une contribution plugin utilise `plugin::`. `idea_skill_read(ref=...)` est canonique ; `name` reste compatible, mais plusieurs correspondances produisent l'erreur structurée `ambiguousSkillName` et la liste `candidates` des refs utilisables. diff --git a/agents-dev/L12-skills.md b/agents-dev/L12-skills.md index 54702119..249e6cac 100644 --- a/agents-dev/L12-skills.md +++ b/agents-dev/L12-skills.md @@ -44,7 +44,7 @@ Modéliser les **Skills** : workflows réutilisables (équivalent universel des ### ✅ Injection dans le convention file (vert, fil L6) - `LaunchAgent` reçoit le port `SkillStore` ; `resolve_skills` lit les `.md` des skills assignés (ordre manifest, déterministe ; skill supprimé = `SkillRef` pendant → ignoré sans bloquer le lancement). -- `compose_convention_file` étendu : section `# Skills` (sous-titres `## `) après le persona ; omise si aucun skill. **3 tests** unitaires + e2e (`agent_lifecycle.rs` : injection ordonnée, ref pendant tolérée). +- `compose_convention_file` étendu : section `# Skills` avant le persona ; omise si aucun skill. Chaque entrée expose une ref stable (`skill::` ou `plugin::`). `idea_skill_read(ref=…)` est la voie canonique ; le lookup historique par `name` reste accepté seulement s'il est non ambigu et renvoie `ambiguousSkillName` avec les refs candidates sinon. **3 tests** unitaires + e2e (`agent_lifecycle.rs` : injection ordonnée, ref pendant tolérée). - **Composition root** (`app-tauri/state.rs`) : `FsSkillStore` construit (app-data global), injecté dans `LaunchAgent`. ### ✅ IPC `app-tauri` (vert) diff --git a/crates/app-tauri/tests/dto_agents.rs b/crates/app-tauri/tests/dto_agents.rs index fd4368a6..219604b3 100644 --- a/crates/app-tauri/tests/dto_agents.rs +++ b/crates/app-tauri/tests/dto_agents.rs @@ -79,6 +79,7 @@ fn agent_list_dto_is_transparent_array() { capabilities: vec![ListedAgentCapabilities { agent_id: first.id, capabilities: vec![AgentCapability { + reference: "skill:project:00000000-0000-0000-0000-000000000001".to_owned(), name: "review".to_owned(), description: "Reviews changes".to_owned(), kind: SkillKind::Reference, @@ -91,6 +92,10 @@ fn agent_list_dto_is_transparent_array() { let arr = v.as_array().expect("transparent array"); assert_eq!(arr.len(), 2); assert_eq!(arr[0]["name"], "My Agent"); + assert_eq!( + arr[0]["capabilities"][0]["reference"], + "skill:project:00000000-0000-0000-0000-000000000001" + ); assert_eq!(arr[0]["capabilities"][0]["name"], "review"); assert_eq!(arr[0]["capabilities"][0]["description"], "Reviews changes"); assert_eq!(arr[0]["capabilities"][0]["kind"], "reference"); diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index 2d6715ca..47798a37 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -3801,7 +3801,7 @@ fn codex_plugin_mcp_declaration(plugin_servers: &[PluginMcpServerSpec]) -> Strin for server in plugin_servers { rendered.push('\n'); rendered.push_str(&format!( - "[mcp_servers.{}]\ncommand = {}\nargs = [{}]\nenv = {}\ncwd = {}\n", + "[mcp_servers.{}]\ncommand = {}\nargs = [{}]\nenv = {}\ncwd = {}\nenabled_tools = [{}]\ndefault_tools_approval_mode = \"approve\"\n", toml_string(&server.client_name()), toml_string(&server.command), server @@ -3812,6 +3812,12 @@ fn codex_plugin_mcp_declaration(plugin_servers: &[PluginMcpServerSpec]) -> Strin .join(", "), toml_inline_string_map(&server.env), toml_string(&server.cwd), + server + .authorized_tools + .iter() + .map(|tool| toml_string(tool)) + .collect::>() + .join(", "), )); } rendered @@ -4468,7 +4474,7 @@ pub(crate) fn compose_convention_file( 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 \ + `idea_skill_read(ref=…)` ; crée-en un avec `idea_create_skill`. Ne réinvente \ pas un workflow déjà couvert par un skill.\n\n", ); } else { @@ -4517,7 +4523,7 @@ pub(crate) fn compose_convention_file( if mcp_enabled { 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 \ + détail, appelle l'outil `idea_skill_read(ref=…)` avec la ref affichée — n'improvise pas un \ workflow déjà couvert par un skill, charge-le.\n\n", ); } else { @@ -4531,7 +4537,9 @@ pub(crate) fn compose_convention_file( for skill in &effective.agent_capabilities { out.push_str("**"); out.push_str(&skill.name); - out.push_str("** — "); + out.push_str("** (`"); + out.push_str(&skill.reference); + out.push_str("`) — "); out.push_str(&skill.description); out.push_str(" ("); out.push_str(match skill.kind { @@ -4943,8 +4951,10 @@ mod tests { ); // Both skill affordances are present, but bodies are not dumped. - assert!(doc.contains("**refactor** — REFAC_BODY (workflow)")); - assert!(doc.contains("**review** — REVIEW_BODY (workflow)")); + assert!(doc.contains("**refactor** (`skill:global:")); + assert!(doc.contains("`) — REFAC_BODY (workflow)")); + assert!(doc.contains("**review** (`skill:global:")); + assert!(doc.contains("`) — REVIEW_BODY (workflow)")); assert!(!doc.contains("\n\nREFAC_BODY")); assert!(!doc.contains("\n\nREVIEW_BODY")); let awareness_at = doc.find("**Skills IdeA**").unwrap(); @@ -5118,8 +5128,10 @@ mod tests { "points to the file-protocol read path" ); } - assert!(doc.contains("**refactor** — Refactors code (workflow)")); - assert!(doc.contains("**review** — Review skill (workflow)")); + assert!(doc.contains("**refactor** (`skill:global:")); + assert!(doc.contains("`) — Refactors code (workflow)")); + assert!(doc.contains("**review** (`skill:global:")); + assert!(doc.contains("`) — Review skill (workflow)")); assert!(!doc.contains("REFAC_BODY"), "no full body is injected"); assert!(!doc.contains("REVIEW_BODY"), "no full body is injected"); assert!(!doc.contains("## refactor")); @@ -5541,6 +5553,7 @@ mod tests { env: vec![("UNITY_SOCKET".to_owned(), "/tmp/unity.sock".to_owned())], cwd: "/plugins/unity".to_owned(), transport: "stdio".to_owned(), + authorized_tools: vec!["unity_detect_project".to_owned(), "unity_build".to_owned()], } } @@ -5565,6 +5578,9 @@ mod tests { ); assert!(codex.contains(&format!("[mcp_servers.{}]", toml_string(&name)))); assert!(codex.contains("UNITY_SOCKET")); + assert!(codex.contains("enabled_tools = [\"unity_detect_project\", \"unity_build\"]")); + assert!(!codex.contains("unity_delete_project")); + assert!(codex.contains("default_tools_approval_mode = \"approve\"")); let opencode = opencode_mcp_servers( vec!["idea".into(), "mcp-server".into()], diff --git a/crates/application/src/error.rs b/crates/application/src/error.rs index f1591ed9..5c20a2dc 100644 --- a/crates/application/src/error.rs +++ b/crates/application/src/error.rs @@ -27,6 +27,15 @@ pub enum AppError { #[error("invalid input: {0}")] Invalid(String), + /// A legacy skill-name lookup matched more than one assigned capability. + #[error("skill name `{name}` is ambiguous; use one of these refs: {candidates:?}")] + AmbiguousSkillName { + /// Name supplied by the caller. + name: String, + /// Stable refs of every matching assigned skill. + candidates: Vec, + }, + /// An optimistic-concurrency `if_match` did not match the resource's current /// version. Carries the current version so the caller can retry. #[error("concurrency conflict: {0}")] @@ -150,6 +159,7 @@ impl AppError { match self { Self::NotFound(_) => "NOT_FOUND", Self::Invalid(_) => "INVALID", + Self::AmbiguousSkillName { .. } => "ambiguousSkillName", Self::Conflict(_) => "CONFLICT", Self::FileSystem(_) => "FILESYSTEM", Self::Store(_) => "STORE", diff --git a/crates/application/src/orchestrator/service.rs b/crates/application/src/orchestrator/service.rs index 4984c1f9..e1a38271 100644 --- a/crates/application/src/orchestrator/service.rs +++ b/crates/application/src/orchestrator/service.rs @@ -1186,8 +1186,8 @@ 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 + /// Use case de lecture d'un skill par ref stable ou nom compatible + /// (`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>, @@ -2396,9 +2396,11 @@ 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::ReadSkill { + reference, + name, + requester, + } => self.read_skill(project, reference, name, requester).await, OrchestratorCommand::WriteMemory { slug, content, @@ -2997,14 +2999,15 @@ impl OrchestratorService { }) } - /// `skill.read` → resolves a skill by name (project scope first, then global) + /// `skill.read` → resolves an assigned skill by stable ref or legacy name /// 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, + reference: Option, + name: Option, requester: ConversationParty, ) -> Result { let read_skill = self.read_skill.as_deref().ok_or_else(|| { @@ -3012,13 +3015,17 @@ impl OrchestratorService { })?; let md = read_skill .execute(ReadSkillInput { + reference: reference.clone(), name: name.clone(), project: project.clone(), requester, }) .await?; Ok(OrchestratorOutcome { - detail: format!("read skill {name}"), + detail: format!( + "read skill {}", + reference.as_deref().or(name.as_deref()).unwrap_or_default() + ), reply: Some(md.into_string()), }) } diff --git a/crates/application/src/plugin/mod.rs b/crates/application/src/plugin/mod.rs index 0ca31cdc..7cc52c91 100644 --- a/crates/application/src/plugin/mod.rs +++ b/crates/application/src/plugin/mod.rs @@ -3649,6 +3649,7 @@ async fn active_mcp_specs( } else { format!("{}/{}", plugin_root.trim_end_matches(['/', '\\']), command) }; + let authorized_tools = server.tools.iter().map(|tool| tool.name.clone()).collect(); specs.push(PluginMcpServerSpec { identity: format!( "plugin:{}:{}", @@ -3680,6 +3681,7 @@ async fn active_mcp_specs( app_data_dir.as_deref(), ), transport: server.transport, + authorized_tools, }); } } diff --git a/crates/application/src/project_plugin.rs b/crates/application/src/project_plugin.rs index 02d81330..28ea08d8 100644 --- a/crates/application/src/project_plugin.rs +++ b/crates/application/src/project_plugin.rs @@ -1,6 +1,5 @@ //! Project-scoped plugin activation and per-agent capability assignments. -use std::collections::HashSet; use std::sync::Arc; use domain::{ @@ -144,12 +143,16 @@ impl ProjectPluginResolver { .await .map_err(package_error)?; let app_data_dir = self.packages.app_data_dir_label(); - let mut seen = HashSet::new(); + let mut spec_indexes = std::collections::HashMap::new(); let mut specs = Vec::new(); for reference in &assignments.tools { let key = (reference.plugin_id.clone(), reference.server_id.clone()); - if !seen.insert(key) { + if let Some(index) = spec_indexes.get(&key).copied() { + let spec: &mut PluginMcpServerSpec = &mut specs[index]; + if !spec.authorized_tools.contains(&reference.tool_name) { + spec.authorized_tools.push(reference.tool_name.clone()); + } continue; } let Some(manifest) = self @@ -180,6 +183,7 @@ impl ProjectPluginResolver { } else { format!("{}/{}", package_root.trim_end_matches(['/', '\\']), command) }; + let index = specs.len(); specs.push(PluginMcpServerSpec { identity: format!( "plugin:{}:{}", @@ -211,7 +215,9 @@ impl ProjectPluginResolver { app_data_dir.as_deref(), ), transport: server.transport, + authorized_tools: vec![reference.tool_name.clone()], }); + spec_indexes.insert(key, index); } Ok(specs) } @@ -896,8 +902,13 @@ mod tests { server_id: PluginMcpServerId::new("unity-editor").unwrap(), tool_name: "unity_detect_project".to_owned(), }; + let second_tool = PluginToolRef { + plugin_id: plugin_id(), + server_id: PluginMcpServerId::new("unity-editor").unwrap(), + tool_name: "unity_build".to_owned(), + }; let resolver = resolver( - settings(true, vec![tool]), + settings(true, vec![tool.clone(), second_tool, tool]), registry(PluginLifecycleState::Enabled), manifest(true), ); @@ -923,6 +934,10 @@ mod tests { vec![("PLUGIN_ROOT".to_owned(), "/plugins/unity".to_owned())] ); assert_eq!(server.cwd, "/plugins/unity/work"); + assert_eq!( + server.authorized_tools, + vec!["unity_detect_project".to_owned(), "unity_build".to_owned()] + ); } #[tokio::test] diff --git a/crates/application/src/skill/usecases.rs b/crates/application/src/skill/usecases.rs index 3cc2c653..85486fcd 100644 --- a/crates/application/src/skill/usecases.rs +++ b/crates/application/src/skill/usecases.rs @@ -223,6 +223,9 @@ impl DeleteSkill { #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub struct AgentCapability { + /// Stable reference accepted by `idea_skill_read(ref=...)`. + #[serde(default)] + pub reference: String, /// Capability display name. pub name: String, /// One-line affordance description. @@ -234,6 +237,7 @@ pub struct AgentCapability { impl From<&AssignedSkillSnapshot> for AgentCapability { fn from(skill: &AssignedSkillSnapshot) -> Self { Self { + reference: skill.stable_ref(), name: skill.name.clone(), description: skill.description.clone(), kind: skill.kind, @@ -415,6 +419,22 @@ impl AssignedSkillResolver { Ok(assigned) } + /// Reads the Markdown body for one skill by stable ref, but only inside the supplied + /// assigned-skill snapshot. + pub async fn read_assigned_by_ref( + &self, + snapshot: &OrchestrationCapabilitySnapshot, + root: &ProjectPath, + reference: &str, + ) -> Result { + let selected = snapshot + .assigned_skills + .iter() + .find(|skill| skill.stable_ref() == reference) + .ok_or_else(|| AppError::NotFound(format!("assigned skill ref `{reference}`")))?; + self.read_assigned_snapshot(selected, root).await + } + /// Reads the Markdown body for one skill by name, but only inside the supplied /// assigned-skill snapshot. /// @@ -435,27 +455,41 @@ impl AssignedSkillResolver { 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" - ))) + (Some(first), Some(second)) => { + let mut candidates = vec![first.stable_ref(), second.stable_ref()]; + candidates.extend(matches.map(AssignedSkillSnapshot::stable_ref)); + return Err(AppError::AmbiguousSkillName { + name: name.to_owned(), + candidates, + }); } }; + self.read_assigned_snapshot(selected, root).await + } + + async fn read_assigned_snapshot( + &self, + selected: &AssignedSkillSnapshot, + root: &ProjectPath, + ) -> Result { if let Some(plugin) = &selected.plugin { - let resolver = self - .plugin_skills - .as_ref() - .ok_or_else(|| AppError::NotFound(format!("assigned plugin skill `{name}`")))?; + let resolver = self.plugin_skills.as_ref().ok_or_else(|| { + AppError::NotFound(format!("assigned plugin skill `{}`", selected.stable_ref())) + })?; let manifest = resolver .enabled_manifest(root, &plugin.plugin_id) .await? - .ok_or_else(|| AppError::NotFound(format!("assigned plugin skill `{name}`")))?; + .ok_or_else(|| { + AppError::NotFound(format!("assigned plugin skill `{}`", selected.stable_ref())) + })?; let contribution = manifest .contributes .skills .iter() .find(|s| s.id == plugin.skill_id) - .ok_or_else(|| AppError::NotFound(format!("assigned plugin skill `{name}`")))?; + .ok_or_else(|| { + AppError::NotFound(format!("assigned plugin skill `{}`", selected.stable_ref())) + })?; return resolver .read_skill_body(&plugin.plugin_id, &contribution.path) .await; @@ -471,8 +505,10 @@ impl AssignedSkillResolver { /// Input for [`ReadSkill::execute`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ReadSkillInput { - /// Skill display name to resolve (case-insensitive). - pub name: String, + /// Stable assigned-skill reference. Takes precedence over `name`. + pub reference: Option, + /// Legacy skill display name to resolve (case-insensitive). + pub name: Option, /// Active project. pub project: Project, /// Current requester identity from the orchestration handshake. @@ -532,9 +568,19 @@ impl ReadSkill { .resolver .resolve_for_agent(&agent, &input.project.root) .await?; - self.resolver - .read_assigned_by_name(&snapshot, &input.project.root, &input.name) - .await + if let Some(reference) = input.reference.as_deref() { + self.resolver + .read_assigned_by_ref(&snapshot, &input.project.root, reference) + .await + } else if let Some(name) = input.name.as_deref() { + self.resolver + .read_assigned_by_name(&snapshot, &input.project.root, name) + .await + } else { + Err(AppError::Invalid( + "idea_skill_read requires `ref` or `name`".to_owned(), + )) + } } } @@ -929,7 +975,35 @@ mod tests { let body = uc .execute(ReadSkillInput { - name: "deploy".to_owned(), + reference: None, + name: Some("deploy".to_owned()), + project, + requester: ConversationParty::agent(requester), + }) + .await + .unwrap(); + assert_eq!(body.as_str(), "PROJECT_BODY"); + } + + #[tokio::test] + async fn read_skill_returns_assigned_skill_by_stable_ref() { + let store = Arc::new(FakeSkillStore::default()); + store.push(skill(1, "deploy", "PROJECT_BODY", SkillScope::Project)); + + 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 { + reference: Some("skill:project:00000000-0000-0000-0000-000000000001".to_owned()), + name: None, project, requester: ConversationParty::agent(requester), }) @@ -955,7 +1029,8 @@ mod tests { let body = uc .execute(ReadSkillInput { - name: "deploy".to_owned(), + reference: None, + name: Some("deploy".to_owned()), project, requester: ConversationParty::agent(requester), }) @@ -975,7 +1050,8 @@ mod tests { let err = uc .execute(ReadSkillInput { - name: "deploy".to_owned(), + reference: None, + name: Some("deploy".to_owned()), project, requester: ConversationParty::agent(requester), }) @@ -1001,7 +1077,8 @@ mod tests { let body = uc .execute(ReadSkillInput { - name: "deploy".to_owned(), + reference: None, + name: Some("deploy".to_owned()), project, requester: ConversationParty::agent(requester), }) @@ -1019,7 +1096,8 @@ mod tests { let err = uc .execute(ReadSkillInput { - name: "ghost".to_owned(), + reference: None, + name: Some("ghost".to_owned()), project, requester: ConversationParty::User, }) @@ -1052,12 +1130,69 @@ mod tests { let err = uc .execute(ReadSkillInput { - name: "deploy".to_owned(), + reference: None, + name: Some("deploy".to_owned()), project, requester: ConversationParty::agent(requester), }) .await .unwrap_err(); - assert!(matches!(err, AppError::Invalid(_)), "got {err:?}"); + assert!( + matches!( + err, + AppError::AmbiguousSkillName { ref name, ref candidates } + if name == "deploy" + && *candidates == vec![ + "skill:project:00000000-0000-0000-0000-000000000001".to_owned(), + "skill:global:00000000-0000-0000-0000-000000000002".to_owned(), + ] + ), + "got {err:?}" + ); + } + + #[tokio::test] + async fn duplicate_plugin_skill_name_reports_stable_plugin_refs() { + let resolver = AssignedSkillResolver::new(Arc::new(FakeSkillStore::default())); + let snapshot = OrchestrationCapabilitySnapshot::new(vec![ + AssignedSkillSnapshot { + skill_id: SkillId::from_uuid(uuid::Uuid::nil()), + scope: SkillScope::Project, + plugin: Some(domain::orchestrator::AssignedPluginSkillSource { + plugin_id: domain::PluginId::new("dev.idea.android").unwrap(), + skill_id: domain::PluginSkillId::new("build-install").unwrap(), + }), + name: "android-build-install".to_owned(), + description: "Android".to_owned(), + kind: SkillKind::Workflow, + assignment_index: 0, + }, + AssignedSkillSnapshot { + skill_id: SkillId::from_uuid(uuid::Uuid::nil()), + scope: SkillScope::Project, + plugin: Some(domain::orchestrator::AssignedPluginSkillSource { + plugin_id: domain::PluginId::new("dev.idea.unity").unwrap(), + skill_id: domain::PluginSkillId::new("android-build-install").unwrap(), + }), + name: "android-build-install".to_owned(), + description: "Unity Android".to_owned(), + kind: SkillKind::Workflow, + assignment_index: 1, + }, + ]); + + let error = resolver + .read_assigned_by_name(&snapshot, &project().root, "android-build-install") + .await + .unwrap_err(); + assert_eq!(error.code(), "ambiguousSkillName"); + assert!(matches!( + error, + AppError::AmbiguousSkillName { ref candidates, .. } + if *candidates == vec![ + "plugin:dev.idea.android:build-install".to_owned(), + "plugin:dev.idea.unity:android-build-install".to_owned(), + ] + )); } } diff --git a/crates/backend/src/dto.rs b/crates/backend/src/dto.rs index 0c5d5659..a7d14ef3 100644 --- a/crates/backend/src/dto.rs +++ b/crates/backend/src/dto.rs @@ -2979,6 +2979,8 @@ use domain::{ #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] pub struct AgentCapabilityDto { + /// Stable reference accepted by `idea_skill_read(ref=...)`. + pub reference: String, /// Capability display name. pub name: String, /// One-line affordance description. @@ -2990,6 +2992,7 @@ pub struct AgentCapabilityDto { impl From for AgentCapabilityDto { fn from(value: AgentCapability) -> Self { Self { + reference: value.reference, name: value.name, description: value.description, kind: value.kind, diff --git a/crates/domain/src/orchestrator.rs b/crates/domain/src/orchestrator.rs index 82ea31b3..9260ec7c 100644 --- a/crates/domain/src/orchestrator.rs +++ b/crates/domain/src/orchestrator.rs @@ -92,6 +92,25 @@ impl OrchestrationCapabilitySnapshot { } } +impl AssignedSkillSnapshot { + /// Stable reference accepted by `idea_skill_read(ref=...)`. + #[must_use] + pub fn stable_ref(&self) -> String { + if let Some(plugin) = &self.plugin { + return format!( + "plugin:{}:{}", + plugin.plugin_id.as_str(), + plugin.skill_id.as_str() + ); + } + let scope = match self.scope { + SkillScope::Global => "global", + SkillScope::Project => "project", + }; + format!("skill:{scope}:{}", self.skill_id) + } +} + /// Errors raised while validating a raw [`OrchestratorRequest`]. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum OrchestratorError { @@ -164,6 +183,10 @@ pub struct OrchestratorRequest { /// Target agent display name (required by every v1 action). #[serde(default, skip_serializing_if = "Option::is_none")] pub name: Option, + /// Stable assigned-skill reference for `skill.read`. Preferred over `name`; + /// kept separate so legacy name-based callers remain compatible. + #[serde(default, rename = "ref", skip_serializing_if = "Option::is_none")] + pub skill_ref: Option, /// V2 target agent display name (`agent.run`/`agent.stop`). #[serde(default, skip_serializing_if = "Option::is_none")] pub target_agent: Option, @@ -411,13 +434,15 @@ pub enum OrchestratorCommand { /// The reading party (handshake identity). requester: ConversationParty, }, - /// Read a reusable skill's Markdown body **by name** (`idea_skill_read`). + /// Read a reusable skill's Markdown body by stable ref, or by legacy name. /// 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, + /// Stable reference (`plugin::` for plugin skills). + reference: Option, + /// Legacy display-name lookup (case-insensitive). + name: Option, /// The party that issued the read (handshake identity). Must be an agent /// requester for authorization against assigned skills. requester: ConversationParty, @@ -651,10 +676,29 @@ impl OrchestratorRequest { slug: self.optional_slug(), requester: self.requester_party(), }), - "skill.read" => Ok(OrchestratorCommand::ReadSkill { - name: self.require_name(action)?, - requester: self.requester_party(), - }), + "skill.read" => { + let reference = self + .skill_ref + .as_deref() + .map(str::trim) + .filter(|v| !v.is_empty()); + let name = self + .name + .as_deref() + .map(str::trim) + .filter(|v| !v.is_empty()); + if reference.is_none() && name.is_none() { + return Err(OrchestratorError::MissingField { + action: action.to_owned(), + field: "ref or name".to_owned(), + }); + } + Ok(OrchestratorCommand::ReadSkill { + reference: reference.map(str::to_owned), + name: name.map(str::to_owned), + 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/plugin.rs b/crates/domain/src/plugin.rs index 938795d6..3a7a287d 100644 --- a/crates/domain/src/plugin.rs +++ b/crates/domain/src/plugin.rs @@ -924,6 +924,12 @@ pub struct PluginMcpServerSpec { pub cwd: String, /// Transport. pub transport: String, + /// Exact MCP tool names authorized for the target agent. + /// + /// This allowlist is carried with the launch spec so provider adapters never + /// have to infer authorization from the server's advertised catalogue. + #[serde(default)] + pub authorized_tools: Vec, } impl PluginMcpServerSpec { @@ -1044,6 +1050,7 @@ mod tests { env: Vec::new(), cwd: String::new(), transport: "stdio".to_owned(), + authorized_tools: Vec::new(), } } diff --git a/crates/infrastructure/src/orchestrator/mcp/server.rs b/crates/infrastructure/src/orchestrator/mcp/server.rs index 1f6690ee..640155cc 100644 --- a/crates/infrastructure/src/orchestrator/mcp/server.rs +++ b/crates/infrastructure/src/orchestrator/mcp/server.rs @@ -759,7 +759,17 @@ impl McpServer { // (`isError: true`) rather than a protocol error: the agent can read // it and decide, and the connection stays healthy. Err(e) => { - let detail = e.to_string(); + let detail = match &e { + application::AppError::AmbiguousSkillName { candidates, .. } => { + serde_json::json!({ + "code": e.code(), + "message": e.to_string(), + "candidates": candidates, + }) + .to_string() + } + _ => e.to_string(), + }; application::diag!( "[mcp] tools_call end tool={name} requester={requester_label} \ target={arg_target} ok=false is_error=true result_len={} elapsed_ms={}", diff --git a/crates/infrastructure/src/orchestrator/mcp/tools.rs b/crates/infrastructure/src/orchestrator/mcp/tools.rs index 14122413..9d32427a 100644 --- a/crates/infrastructure/src/orchestrator/mcp/tools.rs +++ b/crates/infrastructure/src/orchestrator/mcp/tools.rs @@ -521,16 +521,21 @@ pub fn catalogue() -> Vec { }, ToolDef { name: "idea_skill_read", - description: "Read the full body of an IdeA skill assigned to you, by name — call this \ + description: "Read the full body of an IdeA skill assigned to you, preferably by its stable ref. \ + Name lookup remains available for compatibility and fails when ambiguous — call this \ to actually execute/follow a skill listed in your « Skills disponibles » \ - section. Resolves project scope first, then global. Returns the skill's \ + section. Returns the skill's \ Markdown inline.", input_schema: json!({ "type": "object", "properties": { - "name": { "type": "string", "description": "Skill name (as listed in your context)." } + "ref": { "type": "string", "description": "Stable skill ref (for example plugin::)." }, + "name": { "type": "string", "description": "Legacy skill name lookup (must be unambiguous)." } }, - "required": ["name"], + "anyOf": [ + { "required": ["ref"] }, + { "required": ["name"] } + ], "additionalProperties": false }), }, @@ -767,6 +772,7 @@ pub fn map_tool_call( // 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()), + skill_ref: s("ref"), name: s("name"), ..base() }, @@ -809,6 +815,7 @@ fn base() -> OrchestratorRequest { request_type: None, requested_by: None, name: None, + skill_ref: None, target_agent: None, profile: None, context: None, @@ -1450,6 +1457,48 @@ mod tests { assert!(matches!(err, Err(ToolMapError::Invalid(_)))); } + #[test] + fn skill_read_prefers_stable_ref_and_keeps_name_compatibility() { + let requester = domain::ConversationParty::agent(domain::AgentId::from_uuid( + uuid::Uuid::parse_str(REQ).unwrap(), + )); + assert_eq!( + map_tool_call( + "idea_skill_read", + &json!({ "ref": "plugin:dev.idea.android:build-install" }), + REQ, + ) + .unwrap(), + OrchestratorCommand::ReadSkill { + reference: Some("plugin:dev.idea.android:build-install".to_owned()), + name: None, + requester: requester.clone(), + } + ); + assert_eq!( + map_tool_call("idea_skill_read", &json!({ "name": "build" }), REQ).unwrap(), + OrchestratorCommand::ReadSkill { + reference: None, + name: Some("build".to_owned()), + requester, + } + ); + assert!(matches!( + map_tool_call("idea_skill_read", &json!({}), REQ), + Err(ToolMapError::Invalid(_)) + )); + + let tool = catalogue() + .into_iter() + .find(|tool| tool.name == "idea_skill_read") + .unwrap(); + assert!(tool.input_schema["properties"].get("ref").is_some()); + assert_eq!( + tool.input_schema["anyOf"], + json!([{ "required": ["ref"] }, { "required": ["name"] }]) + ); + } + #[test] fn ask_agent_is_an_inline_reply_tool_but_reply_is_not_exposed() { assert!(tool_returns_reply("idea_ask_agent")); diff --git a/crates/infrastructure/src/plugin/mod.rs b/crates/infrastructure/src/plugin/mod.rs index 2491dd63..000335f0 100644 --- a/crates/infrastructure/src/plugin/mod.rs +++ b/crates/infrastructure/src/plugin/mod.rs @@ -890,6 +890,7 @@ mod tests { env: vec![("PLUGIN_ROOT".to_owned(), "/plugin".to_owned())], cwd: "/plugin".to_owned(), transport: "stdio".to_owned(), + authorized_tools: Vec::new(), } } diff --git a/frontend/src/features/agents/CustomAgentChatView.test.tsx b/frontend/src/features/agents/CustomAgentChatView.test.tsx index 8a644259..480ef894 100644 --- a/frontend/src/features/agents/CustomAgentChatView.test.tsx +++ b/frontend/src/features/agents/CustomAgentChatView.test.tsx @@ -12,6 +12,7 @@ import { hasProjectCliInteraction, } from "@/features/projects/projectCliInteractionActivity"; import { publishProjectIconChanged } from "@/features/projects/projectIcons"; +import { publishProjectThemeChanged } from "@/features/projects/projectTheme"; import { PluginCommandRegistry, PluginLayoutRegistry, @@ -213,9 +214,17 @@ describe("CustomAgentChatView", () => { expect(watermark.getAttribute("aria-hidden")).toBe("true"); expect((watermark.firstElementChild as HTMLElement | null)?.style.width).toBe("92%"); expect((watermark.firstElementChild as HTMLElement | null)?.style.height).toBe("92%"); - expect(watermark.textContent).toBe("🐍"); + expect(watermark.textContent).toBe("🐍\uFE0E"); + expect(watermark.dataset.colorMode).toBe("monochrome"); publishProjectIconChanged({ projectId: "project-1", icon: { set: "emoji", name: "star" } }); - await waitFor(() => expect(watermark.textContent).toBe("⭐")); + await waitFor(() => expect(watermark.textContent).toBe("⭐\uFE0E")); + publishProjectThemeChanged({ projectId: "project-1", themeColor: "#7c3aed" }); + await waitFor(() => { + expect(watermark.dataset.colorMode).toBe("project"); + expect(watermark.style.color).toBe("rgb(124, 58, 237)"); + }); + expect(screen.getByTestId("custom-agent-chat-scroll").className).toContain("z-10"); + expect(screen.getByTestId("custom-agent-chat-composer").className).toContain("bg-raised/40"); }); it("settles an isolated conversation id without recovered turns as an actionable unavailable state (#307/#314)", async () => { diff --git a/frontend/src/features/agents/CustomAgentChatView.tsx b/frontend/src/features/agents/CustomAgentChatView.tsx index ee89d07c..ceb45662 100644 --- a/frontend/src/features/agents/CustomAgentChatView.tsx +++ b/frontend/src/features/agents/CustomAgentChatView.tsx @@ -45,6 +45,7 @@ import { } from "@/features/projects/projectCliInteractionActivity"; import { Button, EntryTimestamp, Spinner, cn, normalizeTimestamp } from "@/shared"; import { onProjectIconChanged, ProjectIcon } from "@/features/projects/projectIcons"; +import { onProjectThemeChanged, validThemeColor } from "@/features/projects/projectTheme"; import type { ChatAttachmentInput, ChatDraftScope, @@ -1012,6 +1013,7 @@ export function CustomAgentChatView({ const [stopTerminalOverride, setStopTerminalOverride] = useState(false); const [error, setError] = useState(null); const [projectIcon, setProjectIcon] = useState(null); + const [projectThemeColor, setProjectThemeColor] = useState(null); const scrollRef = useRef(null); const composerRef = useRef(null); const chatRootRef = useRef(null); @@ -1062,12 +1064,24 @@ export function CustomAgentChatView({ useEffect(() => { let cancelled = false; void project?.readProjectSettings(projectId).then((settings) => { - if (!cancelled) setProjectIcon(settings.icon ?? null); + if (!cancelled) { + setProjectIcon(settings.icon ?? null); + setProjectThemeColor(validThemeColor(settings.themeColor)); + } }).catch(() => undefined); - const unsubscribe = onProjectIconChanged((change) => { + const unsubscribeIcon = onProjectIconChanged((change) => { if (change.projectId === projectId) setProjectIcon(change.icon); }); - return () => { cancelled = true; unsubscribe(); }; + const unsubscribeTheme = onProjectThemeChanged((change) => { + if (change.projectId === projectId) { + setProjectThemeColor(validThemeColor(change.themeColor)); + } + }); + return () => { + cancelled = true; + unsubscribeIcon(); + unsubscribeTheme(); + }; }, [project, projectId]); const clampComposerHeight = useCallback( @@ -2494,9 +2508,14 @@ export function CustomAgentChatView({ >