From 5a6ab7d63e298df134d3599f8d5bdb6a03d91bd0 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Wed, 28 Oct 2026 17:51:40 +0800 Subject: [PATCH 01/12] feat(plugins): wire pack contents into sessions Plugin packs loaded and validated but nothing they declared took effect. Their contents now reach the session: - pack skills merge into the catalog as a new pack source ranked below user and project, so a same-named skill the user or the project owns keeps its slot and a pack fills the gaps; - pack MCP entries convert into config-crate servers and ride the deferred connect path; a user-configured server owns its name and a colliding entry warns and drops, while later packs win among themselves in directory order; - pack hook rules join the hook engine after the config-declared rules with the same matcher/timeout semantics, and an unknown event point warns with the pack attribution. plugin list states that pack contents are wired and names the collision rule; the extensibility docs describe the precedence instead of the old inventory-only contract. --- CHANGELOG.md | 9 + crates/capabilities/skills/src/lib.rs | 5 + crates/capabilities/skills/src/plugin.rs | 72 +++-- crates/frontends/harness/src/serve.rs | 4 + .../bootstrap/src/plugin_inventory.rs | 10 +- .../bootstrap/src/session/assembly.rs | 160 ++++++++-- .../operations/bootstrap/src/session/tests.rs | 280 ++++++++++++++++++ docs/subsystems/extensibility.md | 2 +- docs/subsystems/extensibility.zh.md | 2 +- 9 files changed, 493 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49664377..fecdad8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,6 +161,15 @@ accumulate under `Unreleased` until a `v*` tag publishes them. statistics and eviction estimates count tokens without materializing full text, and the eviction frontier measures only the batches it consumes. +- Plugin pack contents take effect in sessions instead of staying + inventory-only: pack skills merge into the catalog as the new `pack` + source (below user and project, which keep their slots on a name + collision), pack MCP entries convert into configured servers for the + live connect path (a user-configured server owns its name; later + packs win among themselves), and pack hook rules join the engine + after the config-declared ones with the same matcher/timeout + semantics. Collisions and invalid entries degrade to session + warnings; `plugin list` notes that pack contents are wired. ### Fixed - Leaving the console TUI runs the SessionEnd hooks and the memory diff --git a/crates/capabilities/skills/src/lib.rs b/crates/capabilities/skills/src/lib.rs index 6ac6fddf..0d170e32 100644 --- a/crates/capabilities/skills/src/lib.rs +++ b/crates/capabilities/skills/src/lib.rs @@ -62,6 +62,10 @@ const SKILL_DIR_VARIABLE: &str = "${WAVECODE_SKILL_DIR}"; pub enum SkillSource { /// Builtin skill set shipped with the binary. Builtin, + /// Installed plugin pack (`~/.wavecode/plugins//…`). Ranks + /// below user- and project-level skills: a pack is a bundle someone + /// else curated, so the user's own same-named skill wins. + Pack, /// User-level `~/.wavecode/skills`. User, /// Project-level `/.wavecode/skills`. @@ -77,6 +81,7 @@ impl SkillSource { pub fn as_str(self) -> &'static str { match self { Self::Builtin => "builtin", + Self::Pack => "pack", Self::User => "user", Self::Project => "project", Self::Mcp => "mcp", diff --git a/crates/capabilities/skills/src/plugin.rs b/crates/capabilities/skills/src/plugin.rs index 2f59fd52..9136e400 100644 --- a/crates/capabilities/skills/src/plugin.rs +++ b/crates/capabilities/skills/src/plugin.rs @@ -10,10 +10,11 @@ * - Warn-and-skip invalid packs without failing discovery. * * Constraints: - * - This module is inventory only: the session assembly consumes - * builtin/user/project skill roots and never merges pack contents, so - * a pack's skills, MCP servers, and hooks are counted and surfaced - * (e.g. by `plugin list`) but do not take effect. + * - This module parses and validates but does not decide precedence: the + * session assembly merges pack skills under the standard roots via + * [`PluginLoader::apply_skills`], and the composition root converts + * the raw MCP entries and hook rules into configured servers and + * engine definitions (config-crate types this module must not import). * * This module must not depend on: workspace crates beyond its own * (manifest shapes mirror config/hook types instead of importing them). @@ -24,12 +25,12 @@ //! //! A manifest carries `{name, version, skills_dir?, hooks_file?}` plus //! optional `[mcp_servers.]` inline tables. Skills load through -//! the [`discover`] pipeline with [`SkillSource::User`]. The loader is -//! inventory only: MCP entries and hook rules are returned raw (mirrors -//! of the config/hook shapes, so this module imports neither), and -//! nothing currently converts them into configured servers or active -//! hook rules — consumers read counts and entries for listing and -//! diagnostics. Invalid packs warn-and-skip with a reason. +//! the [`discover`] pipeline with [`SkillSource::Pack`]. MCP entries and +//! hook rules are returned raw (mirrors of the config/hook shapes, so +//! this module imports neither); the session assembly merges the skills +//! below user/project sources and the composition root converts the +//! entries into configured servers and active hook rules. Invalid packs +//! warn-and-skip with a reason. use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -51,8 +52,8 @@ fn valid_name(name: &str) -> bool { /// Raw MCP server entry from a plugin manifest. /// /// Mirrors the config-crate server shape without depending on it. -/// Retained for listing and diagnostics; no session assembly consumes -/// these entries today. +/// The session assembly converts entries into configured servers for +/// the live connect path. #[derive(Debug, Clone, serde::Deserialize)] pub struct PluginMcpServer { /// Stdio executable command (stdio servers). @@ -73,8 +74,8 @@ pub struct PluginMcpServer { /// Raw hook rule from a plugin hooks file. /// /// Mirrors the config-crate hook rule without depending on it. -/// Retained for listing and diagnostics; no session assembly converts -/// these into engine definitions today. +/// The session assembly converts rules into engine definitions with +/// the same matcher/timeout semantics as config-declared rules. #[derive(Debug, Clone, serde::Deserialize)] pub struct PluginHookRule { /// Tool-name matcher (optional). @@ -123,12 +124,13 @@ pub struct LoadedPlugin { pub name: String, /// Pack version string. pub version: String, - /// Skills discovered from `skills_dir` ([`SkillSource::User`]). + /// Skills discovered from `skills_dir` ([`SkillSource::Pack`]). pub skills: SkillSet, - /// Raw MCP entries, sorted by server name (listing only; see the - /// module docs for the inventory-only contract). + /// Raw MCP entries, sorted by server name (converted into + /// configured servers by the composition root). pub mcp_servers: Vec<(String, PluginMcpServer)>, - /// Raw hook rules, keyed by event-point name (listing only). + /// Raw hook rules, keyed by event-point name (converted into + /// engine definitions by the session assembly). pub hooks: HashMap>, } @@ -203,15 +205,19 @@ impl PluginLoader { &self.warnings } - /// Merge every pack's skills into `set` (same names override, so - /// later packs win over earlier ones). + /// Merge every pack's skills into `set` as the pack source. /// - /// No production caller today — the session assembly never merges - /// pack skills; this is the hook a wiring pass would call. + /// Pack skills are the lowest-priority source above builtin: an + /// entry already in `set` (user, project, or a merged builtin) keeps + /// its slot and the pack only fills the gaps, so the caller merges + /// packs after the standard roots. Later packs win among themselves, + /// in directory order. pub fn apply_skills(&self, set: &mut SkillSet) { for plugin in &self.plugins { for skill in plugin.skills.iter() { - set.add(skill.clone()); + if set.get(&skill.name).is_none() { + set.add(skill.clone()); + } } } } @@ -239,9 +245,9 @@ impl PluginLoader { let skills = match &manifest.skills_dir { Some(rel) => { // Reuse the Discovery pipeline: same SKILL.md parsing and - // per-file warn-and-skip, attributed to the User source. + // per-file warn-and-skip, attributed to the Pack source. let roots = [SkillRoot { - source: SkillSource::User, + source: SkillSource::Pack, dir: dir.join(rel), }]; let discovery = discover(&roots); @@ -296,6 +302,7 @@ impl PluginLoader { #[cfg(test)] mod tests { use super::*; + use super::super::Skill; const SKILL_MD: &str = "---\ndescription: Review helper skill.\n---\n\nReview body here.\n"; @@ -454,7 +461,7 @@ command = "notify.sh" } #[test] - fn apply_skills_merges_as_user_source() { + fn apply_skills_merges_as_pack_source_below_existing_entries() { let home = home_with_pack( "demo", &[ @@ -467,6 +474,17 @@ command = "notify.sh" let mut set = SkillSet::default(); loader.apply_skills(&mut set); let skill = set.get("greet").expect("merged skill"); - assert_eq!(skill.source, SkillSource::User); + assert_eq!(skill.source, SkillSource::Pack); + + // An entry already in the set keeps its slot: user/project + // skills win over a pack skill of the same name. + let user = tempfile::tempdir().unwrap(); + let greet = user.path().join("greet"); + std::fs::create_dir_all(&greet).unwrap(); + std::fs::write(greet.join("SKILL.md"), SKILL_MD).unwrap(); + let mut set = SkillSet::default(); + set.add(Skill::parse(&greet, SkillSource::User).expect("hand-built skill")); + loader.apply_skills(&mut set); + assert_eq!(set.get("greet").unwrap().source, SkillSource::User); } } diff --git a/crates/frontends/harness/src/serve.rs b/crates/frontends/harness/src/serve.rs index 90c4de7f..9750c187 100644 --- a/crates/frontends/harness/src/serve.rs +++ b/crates/frontends/harness/src/serve.rs @@ -62,6 +62,10 @@ pub(crate) fn run_plugin_list(home: Option) { plugin.name, plugin.version, plugin.skills, plugin.mcp_servers, plugin.hooks ); } + println!( + "pack contents take effect in new sessions; a same-named user skill or \ + configured mcp server wins over a pack entry" + ); } /// Serve live sessions over local HTTP until Ctrl-C or `POST /shutdown`. diff --git a/crates/operations/bootstrap/src/plugin_inventory.rs b/crates/operations/bootstrap/src/plugin_inventory.rs index ce5c1ede..3706ad1a 100644 --- a/crates/operations/bootstrap/src/plugin_inventory.rs +++ b/crates/operations/bootstrap/src/plugin_inventory.rs @@ -32,9 +32,13 @@ pub struct PluginSummary { /// Load every installed plugin pack under `home`. /// /// Returns the summaries plus non-fatal parse warnings: a broken pack -/// degrades to a warning, never a hard failure. The summaries are -/// inventory only — pack skills, MCP servers, and hooks are counted for -/// listing and diagnostics; none of them is wired into a session. +/// degrades to a warning, never a hard failure. The summaries mirror +/// what a session would wire: pack skills merge into the catalog under +/// user/project sources, pack MCP servers convert into configured +/// servers, and pack hook rules join the hook engine. Whether a given +/// entry survives session-time collision checks (a same-named user +/// skill or configured server wins) depends on the session's cwd and +/// config, which this listing deliberately does not load. pub fn discover_plugin_summaries(home: Option<&Path>) -> (Vec, Vec) { let loader = wavecode_skills::plugin::PluginLoader::load(home); let summaries = loader diff --git a/crates/operations/bootstrap/src/session/assembly.rs b/crates/operations/bootstrap/src/session/assembly.rs index 420717f2..861e6d1f 100644 --- a/crates/operations/bootstrap/src/session/assembly.rs +++ b/crates/operations/bootstrap/src/session/assembly.rs @@ -344,6 +344,7 @@ pub fn assemble_session_after_model(parts: WithModel) -> SessionHandle { skill_catalog, skill_names, mcp_servers, + pack_mcp_servers, .. } = ctx; @@ -396,8 +397,11 @@ pub fn assemble_session_after_model(parts: WithModel) -> SessionHandle { _ => client, }; + // Configured servers connect first; converted pack servers append + // after (the list is name-sorted anyway). let mut mcp_pending: Vec<(String, wavecode_config::McpServerRaw)> = config.mcp_servers.clone().into_iter().collect(); + mcp_pending.extend(pack_mcp_servers); mcp_pending.sort_by(|a, b| a.0.cmp(&b.0)); // Frontend status views share the tool-side snapshot root so /plan, @@ -477,6 +481,11 @@ struct AssembledContext { hooks: Arc, /// Configured MCP servers as one display line each. mcp_servers: Vec, + /// Pack MCP servers converted into config shape for the live + /// connect path (user config wins name collisions; later packs win + /// among themselves). The finalize phase appends them to the + /// pending list after the configured servers. + pack_mcp_servers: Vec<(String, wavecode_config::McpServerRaw)>, } /// Build [`AssembledContext`] (phases 1-4): shared stores, the policy @@ -526,12 +535,18 @@ fn build_assembled_context( let sandbox = wavecode_sandbox::Sandbox::from_rules(permission_mode, permissions.allow, permissions.deny); - // 4. Context sources with warn-and-continue degradation. + // 4. Context sources with warn-and-continue degradation. Plugin + // packs load once here: the skills merge into the catalog, the hook + // rules join the engine, and the MCP entries convert into pending + // servers; loader warnings degrade like every other source. + let plugins = wavecode_skills::plugin::PluginLoader::load(home); + warnings.extend(plugins.warnings().iter().cloned()); let (instruction_memory, memory_index, instruction_sources) = assemble_memory(home, cwd, warnings); - let (skill_catalog, skill_names, skill_set) = assemble_skills(home, cwd, warnings); - let hooks = assemble_hooks(config, warnings); - let mcp_servers = describe_mcp_servers(config); + let (skill_catalog, skill_names, skill_set) = assemble_skills(&plugins, home, cwd, warnings); + let hooks = assemble_hooks(config, &plugins, warnings); + let pack_mcp_servers = pack_mcp_servers(config, &plugins, warnings); + let mcp_servers = describe_mcp_servers(config, &pack_mcp_servers); AssembledContext { spill_store, @@ -548,6 +563,7 @@ fn build_assembled_context( skill_set, hooks, mcp_servers, + pack_mcp_servers, } } @@ -1217,13 +1233,18 @@ fn assemble_memory( /// Assemble the skill catalog text with discovery warnings preserved, /// returning catalog text, directly invokable names for UIs, and the set /// itself for the `skill` tool backend. +/// +/// Pack skills merge under the standard roots: a same-named user or +/// project skill keeps its slot and a pack only fills the gaps. fn assemble_skills( + plugins: &wavecode_skills::plugin::PluginLoader, home: Option<&Path>, cwd: &Path, warnings: &mut Vec, ) -> (String, Vec, Arc) { let roots = wavecode_skills::standard_roots(None, home, cwd); - let discovery = wavecode_skills::discover(&roots); + let mut discovery = wavecode_skills::discover(&roots); + plugins.apply_skills(&mut discovery.set); warnings.extend(discovery.warnings.iter().cloned()); let names: Vec = discovery .set @@ -1238,12 +1259,29 @@ fn assemble_skills( ) } -/// Convert raw config hooks into an engine, warning past bad entries. +/// Convert raw config hooks plus pack hook rules into an engine, warning +/// past bad entries. fn assemble_hooks( config: &wavecode_config::Config, + plugins: &wavecode_skills::plugin::PluginLoader, warnings: &mut Vec, ) -> Arc { - let mut defs = HashMap::new(); + Arc::new(wavecode_hooks::HookEngine::new(hook_defs( + config, plugins, warnings, + ))) +} + +/// Build the engine's definitions: config-declared rules come first at +/// every event point; pack rules append after (pack directory order, +/// then file order within a pack) with the same matcher/timeout +/// semantics as config rules. +pub(super) fn hook_defs( + config: &wavecode_config::Config, + plugins: &wavecode_skills::plugin::PluginLoader, + warnings: &mut Vec, +) -> HashMap> { + let mut defs: HashMap> = + HashMap::new(); for (point_name, ruleset) in &config.hooks { match wavecode_hooks::HookEventPoint::parse(point_name) { None => warnings.push(format!("unknown hook point {point_name:?}; skipped")), @@ -1266,26 +1304,110 @@ fn assemble_hooks( } } } - Arc::new(wavecode_hooks::HookEngine::new(defs)) + for plugin in plugins.plugins() { + for (point_name, rules) in &plugin.hooks { + match wavecode_hooks::HookEventPoint::parse(point_name) { + None => warnings.push(format!( + "plugin {}: unknown hook point {point_name:?}; skipped", + plugin.name + )), + Some(point) => { + defs.entry(point).or_default().extend(rules.iter().map( + |rule| wavecode_hooks::HookDef { + matcher: rule.matcher.clone(), + command: rule.command.clone(), + timeout_ms: rule + .timeout_ms + .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), + once: rule.once.unwrap_or(false), + }, + )); + } + } + } + } + defs } -/// Describe configured MCP servers as one display line each. +/// Convert pack MCP entries into config-crate servers for the live +/// connect path. +/// +/// A user-configured server owns its name: a colliding pack entry warns +/// and drops. Among packs the later one wins a shared name (directory +/// order), matching pack skill precedence. +pub(super) fn pack_mcp_servers( + config: &wavecode_config::Config, + plugins: &wavecode_skills::plugin::PluginLoader, + warnings: &mut Vec, +) -> Vec<(String, wavecode_config::McpServerRaw)> { + let mut servers: Vec<(String, wavecode_config::McpServerRaw)> = Vec::new(); + for plugin in plugins.plugins() { + for (name, raw) in &plugin.mcp_servers { + if config.mcp_servers.contains_key(name) { + warnings.push(format!( + "plugin {}: mcp server {name:?} ignored: a configured server already owns that name", + plugin.name + )); + continue; + } + match servers.iter_mut().find(|(known, _)| known == name) { + Some(slot) => { + warnings.push(format!( + "plugin {}: mcp server {name:?} replaces the earlier pack's entry", + plugin.name + )); + slot.1 = plugin_mcp_to_config(raw); + } + None => servers.push((name.clone(), plugin_mcp_to_config(raw))), + } + } + } + servers.sort_by(|a, b| a.0.cmp(&b.0)); + servers +} + +/// Lift a pack MCP entry into the config-crate shape. Packs carry no +/// OAuth fields, so those default to absent. +fn plugin_mcp_to_config(raw: &wavecode_skills::plugin::PluginMcpServer) -> wavecode_config::McpServerRaw { + wavecode_config::McpServerRaw { + command: raw.command.clone(), + args: raw.args.clone(), + env: raw.env.clone(), + url: raw.url.clone(), + headers: raw.headers.clone(), + oauth_token_url: None, + oauth_client_id: None, + oauth_client_secret: None, + oauth_scope: None, + } +} + +/// Describe configured and pack MCP servers as one display line each. /// /// stdio entries show their command, HTTP entries their URL; entries /// with neither are reported, never silently dropped. -fn describe_mcp_servers(config: &wavecode_config::Config) -> Vec { +fn describe_mcp_servers( + config: &wavecode_config::Config, + pack_servers: &[(String, wavecode_config::McpServerRaw)], +) -> Vec { + let describe = |name: &str, server: &wavecode_config::McpServerRaw| { + if let Some(command) = &server.command { + format!("{name} (stdio: {command})") + } else if let Some(url) = &server.url { + format!("{name} (http: {url})") + } else { + format!("{name} (unconfigured)") + } + }; let mut lines: Vec = config .mcp_servers .iter() - .map(|(name, server)| { - if let Some(command) = &server.command { - format!("{name} (stdio: {command})") - } else if let Some(url) = &server.url { - format!("{name} (http: {url})") - } else { - format!("{name} (unconfigured)") - } - }) + .map(|(name, server)| describe(name, server)) + .chain( + pack_servers + .iter() + .map(|(name, server)| describe(name, server)), + ) .collect(); lines.sort(); lines diff --git a/crates/operations/bootstrap/src/session/tests.rs b/crates/operations/bootstrap/src/session/tests.rs index aa5b6ddd..0c48fbfb 100644 --- a/crates/operations/bootstrap/src/session/tests.rs +++ b/crates/operations/bootstrap/src/session/tests.rs @@ -995,3 +995,283 @@ fn gapped_journal_is_rewritten_before_new_records() { assert_eq!(replayed.entries.len(), 2); assert_eq!(replayed.entries[1].text(), "after"); } + +/// Lay out a home dir holding one plugin pack with the given files +/// (the same harness the loader tests in wavecode-skills use). +fn home_with_pack(name: &str, files: &[(&str, &str)]) -> tempfile::TempDir { + let home = tempfile::tempdir().unwrap(); + let pack = home.path().join(".wavecode").join("plugins").join(name); + std::fs::create_dir_all(&pack).unwrap(); + for (rel, content) in files { + let path = pack.join(rel); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, content).unwrap(); + } + home +} + +/// Write a skill dir holding one SKILL.md with the given description. +fn write_skill(root: &std::path::Path, name: &str, description: &str) { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("SKILL.md"), + format!("---\ndescription: {description}.\n---\n\nBody.\n"), + ) + .unwrap(); +} + +const PACK_MANIFEST: &str = r#" +name = "demo" +version = "0.1.0" +skills_dir = "skills" +hooks_file = "hooks.toml" + +[mcp_servers.lint] +command = "lint-server" +args = ["--fast"] +"#; + +const PACK_HOOKS: &str = r#" +[[PreToolUse]] +command = "pack.sh" +timeout_ms = 250 + +[[PostToolUse]] +command = "fmt.sh" + +[BogusPoint] +command = "x.sh" +"#; + +/// Pack contents reach the assembled session end to end: pack skills +/// merge into the catalog under the user and project sources, the pack +/// MCP entry converts into a pending server, and the pack hook rules +/// join the engine (including the unknown-point warning). +#[tokio::test] +async fn pack_contents_reach_the_session_under_standard_precedence() { + let dir = tempfile::tempdir().unwrap(); + let home = home_with_pack( + "demo", + &[ + ("plugin.toml", PACK_MANIFEST), + ("hooks.toml", PACK_HOOKS), + ], + ); + // Pack skills merge under the standard sources: the user skill + // overrides the pack skill, and the project skill overrides both, + // so only the project description may surface for `greet`. + let pack_skills = home + .path() + .join(".wavecode") + .join("plugins") + .join("demo") + .join("skills"); + write_skill(&pack_skills, "greet", "pack greet helper"); + write_skill(&pack_skills, "packonly", "packonly helper"); + write_skill( + &home.path().join(".wavecode").join("skills"), + "greet", + "user greet helper", + ); + write_skill( + &dir.path().join(".wavecode").join("skills"), + "greet", + "project greet helper", + ); + let config_path = dir.path().join("config.toml"); + std::fs::write(&config_path, CONFIG).unwrap(); + + let handle = assemble_session(AssembleOptions { + config_path: Some(config_path), + model_override: None, + provider_override: None, + permission_override: None, + thinking_override: None, + wave_denylist: None, + cwd: dir.path().to_path_buf(), + home: Some(home.path().to_path_buf()), + identity: DEFAULT_IDENTITY.to_string(), + headless: true, + initial_history: Vec::new(), + session_id: None, + }) + .unwrap(); + + // Skills: project > user > pack, and the pack-only skill lands. + assert!( + handle.system.contains("project greet helper"), + "project skill must win: {}", + handle.system + ); + assert!(!handle.system.contains("user greet helper")); + assert!(!handle.system.contains("pack greet helper")); + assert!(handle.system.contains("packonly helper")); + assert!(handle.skill_names.iter().any(|name| name == "packonly")); + + // MCP: the pack entry converts into a pending server with the OAuth + // fields absent, and the display line lists it. + let lint = handle + .mcp_pending + .iter() + .find(|(name, _)| name == "lint") + .expect("pack server rides the pending list"); + assert_eq!(lint.1.command.as_deref(), Some("lint-server")); + assert_eq!(lint.1.args, vec!["--fast".to_string()]); + assert!(lint.1.oauth_token_url.is_none()); + assert!( + handle + .mcp_servers + .iter() + .any(|line| line.contains("lint") && line.contains("lint-server")) + ); + + // Hooks: the unknown pack point warns with the pack attribution. + assert!( + handle + .warnings + .iter() + .any(|w| w.contains("plugin demo") && w.contains("unknown hook point")), + "{:?}", + handle.warnings + ); +} + +/// A user-configured MCP server owns its name: the colliding pack entry +/// warns and drops instead of reaching the connect path. +#[tokio::test] +async fn configured_mcp_server_wins_over_pack_entry() { + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join("config.toml"); + std::fs::write( + &config_path, + format!( + "{CONFIG}\n[mcp_servers.lint]\ncommand = \"config-server\"\n" + ), + ) + .unwrap(); + let home = home_with_pack( + "demo", + &[("plugin.toml", PACK_MANIFEST)], + ); + + let handle = assemble_session(AssembleOptions { + config_path: Some(config_path), + model_override: None, + provider_override: None, + permission_override: None, + thinking_override: None, + wave_denylist: None, + cwd: dir.path().to_path_buf(), + home: Some(home.path().to_path_buf()), + identity: DEFAULT_IDENTITY.to_string(), + headless: true, + initial_history: Vec::new(), + session_id: None, + }) + .unwrap(); + + let lints: Vec<&(String, wavecode_config::McpServerRaw)> = handle + .mcp_pending + .iter() + .filter(|(name, _)| name == "lint") + .collect(); + assert_eq!(lints.len(), 1, "the pack entry must not ride along"); + assert_eq!(lints[0].1.command.as_deref(), Some("config-server")); + assert!( + handle + .warnings + .iter() + .any(|w| w.contains("already owns that name")), + "{:?}", + handle.warnings + ); +} + +/// Hook definitions: config rules keep their point slot, pack rules +/// append after them with the same timeout/once semantics, and an +/// unknown pack point warns with the pack attribution. +#[test] +fn pack_hook_rules_join_config_rules_at_the_same_point() { + let config = config_with("\n[hooks.PreToolUse]\ncommand = \"config.sh\"\n"); + let home = home_with_pack( + "demo", + &[ + ("plugin.toml", PACK_MANIFEST), + ("hooks.toml", PACK_HOOKS), + ], + ); + let plugins = wavecode_skills::plugin::PluginLoader::load(Some(home.path())); + let mut warnings = Vec::new(); + let defs = super::assembly::hook_defs(&config, &plugins, &mut warnings); + + let pre = &defs[&wavecode_hooks::HookEventPoint::PreToolUse]; + assert_eq!(pre.len(), 2); + assert_eq!(pre[0].command, "config.sh"); + assert_eq!(pre[1].command, "pack.sh"); + assert_eq!(pre[1].timeout_ms, 250); + assert!(!pre[1].once); + + let post = &defs[&wavecode_hooks::HookEventPoint::PostToolUse]; + assert_eq!(post.len(), 1); + assert_eq!(post[0].command, "fmt.sh"); + // Absent timeout/once fall back to the engine defaults, same as + // config-declared rules. + assert_eq!(post[0].timeout_ms, wavecode_hooks::DEFAULT_TIMEOUT_MS); + + assert!( + warnings + .iter() + .any(|w| w.contains("plugin demo") && w.contains("BogusPoint")), + "{warnings:?}" + ); +} + +/// Pack MCP entries: a configured server wins the name, later packs win +/// among themselves, and the survivors convert in sorted order. +#[test] +fn pack_mcp_entries_convert_with_config_priority_and_pack_order() { + let config = config_with( + "\n[mcp_servers.lint]\ncommand = \"config-server\"\n[mcp_servers.only]\ncommand = \"only-server\"\n", + ); + let home = tempfile::tempdir().unwrap(); + let plugins_root = home.path().join(".wavecode").join("plugins"); + for (pack, command) in [("aaa", "first-server"), ("zzz", "second-server")] { + let pack_dir = plugins_root.join(pack); + std::fs::create_dir_all(&pack_dir).unwrap(); + std::fs::write( + pack_dir.join("plugin.toml"), + format!("name = \"{pack}\"\nversion = \"1\"\n\n[mcp_servers.fetch]\ncommand = \"{command}\"\n[mcp_servers.lint]\ncommand = \"{command}\"\n"), + ) + .unwrap(); + } + let plugins = wavecode_skills::plugin::PluginLoader::load(Some(home.path())); + let mut warnings = Vec::new(); + let servers = super::assembly::pack_mcp_servers(&config, &plugins, &mut warnings); + + // `lint` collides with config (both packs drop with a warning), + // `fetch` collides across packs (the later pack wins, warned). + let names: Vec<&str> = servers.iter().map(|(name, _)| name.as_str()).collect(); + assert_eq!(names, vec!["fetch"]); + assert_eq!( + servers[0].1.command.as_deref(), + Some("second-server"), + "the later pack (zzz) wins the shared name" + ); + assert_eq!( + warnings + .iter() + .filter(|w| w.contains("already owns that name")) + .count(), + 2, + "both packs' lint entries drop against config: {warnings:?}" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("replaces the earlier pack's entry")), + "{warnings:?}" + ); +} diff --git a/docs/subsystems/extensibility.md b/docs/subsystems/extensibility.md index e8fa514c..0f6ca843 100644 --- a/docs/subsystems/extensibility.md +++ b/docs/subsystems/extensibility.md @@ -34,7 +34,7 @@ Known limits, stated in the code: interactive browser/PKCE OAuth is out of scope ## Plugins (`crates/runtime/plugin/src/lib.rs`, `crates/capabilities/skills/src/plugin.rs`) -The runtime `Registry` starts plugins in manifest-dependency order and stops them in reverse; `ServiceMap` injects typed services with type-id keyed lookup. Discovery reads `runtime.toml` manifests (`{name, version, depends?}` identity) and degrades **warn-and-skip**: an invalid manifest warns with its reason and is skipped, never failing assembly. The `PluginLoader` in the skills crate loads user plugin packs (`plugin.toml`: `name`, `version`, optional `skills_dir`, hooks, MCP server entries) with the same warn-and-skip contract, feeding their skills/hooks/MCP servers into normal assembly. +The runtime `Registry` starts plugins in manifest-dependency order and stops them in reverse; `ServiceMap` injects typed services with type-id keyed lookup. Discovery reads `runtime.toml` manifests (`{name, version, depends?}` identity) and degrades **warn-and-skip**: an invalid manifest warns with its reason and is skipped, never failing assembly. The `PluginLoader` in the skills crate loads user plugin packs (`plugin.toml`: `name`, `version`, optional `skills_dir`, hooks, MCP server entries) with the same warn-and-skip contract, and their contents take effect in sessions: pack skills merge into the catalog as the `pack` source below user and project (a same-named user or project skill keeps its slot), pack MCP entries convert into configured servers (a user-configured server owns its name; later packs win among themselves, in directory order), and pack hook rules join the engine after the config-declared ones with the same matcher/timeout semantics. ## Named agent definitions (`crates/capabilities/tools/src/agent_task_tool.rs`) diff --git a/docs/subsystems/extensibility.zh.md b/docs/subsystems/extensibility.zh.md index 98107154..bf168e05 100644 --- a/docs/subsystems/extensibility.zh.md +++ b/docs/subsystems/extensibility.zh.md @@ -34,7 +34,7 @@ Frontmatter 解析使用 `serde-saphyr` 而不是手写扫描器——一个刻 ## 插件（`crates/runtime/plugin/src/lib.rs`、`crates/capabilities/skills/src/plugin.rs`） -runtime 的 `Registry` 按清单依赖顺序启动插件，逆序停止；`ServiceMap` 以 type-id 键控查找注入类型化服务。发现读取 `runtime.toml` 清单（`{name, version, depends?}` 身份）并按**警告并跳过**降级：无效清单带原因警告并被跳过，绝不让装配失败。skills crate 中的 `PluginLoader` 加载用户插件包（`plugin.toml`：`name`、`version`、可选 `skills_dir`、hooks、MCP 服务器条目），带同样的 warn-and-skip 契约，把它们的技能/钩子/MCP 服务器喂进正常装配。 +runtime 的 `Registry` 按清单依赖顺序启动插件，逆序停止；`ServiceMap` 以 type-id 键控查找注入类型化服务。发现读取 `runtime.toml` 清单（`{name, version, depends?}` 身份）并按**警告并跳过**降级：无效清单带原因警告并被跳过，绝不让装配失败。skills crate 中的 `PluginLoader` 加载用户插件包（`plugin.toml`：`name`、`version`、可选 `skills_dir`、hooks、MCP 服务器条目），带同样的 warn-and-skip 契约，其内容会在会话中生效：包技能以 `pack` 来源并入目录，优先级低于 user 与 project（同名的 user/project 技能保有自己的槽位）；包 MCP 条目转换为已配置服务器（用户配置的服务器拥有其名字；多个包之间按目录顺序后者获胜）；包 hook 规则以与配置规则相同的 matcher/timeout 语义追加到引擎中配置规则之后。 ## 命名的 agent 定义（`crates/capabilities/tools/src/agent_task_tool.rs`） From 9102f3c9f55354946b182e7a93bca007a10afe7f Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Wed, 28 Oct 2026 22:27:51 +0800 Subject: [PATCH 02/12] style: cargo fmt --- crates/capabilities/skills/src/plugin.rs | 2 +- .../bootstrap/src/session/assembly.rs | 26 +++++++++++-------- .../operations/bootstrap/src/session/tests.rs | 19 +++----------- 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/crates/capabilities/skills/src/plugin.rs b/crates/capabilities/skills/src/plugin.rs index 9136e400..aaaf2429 100644 --- a/crates/capabilities/skills/src/plugin.rs +++ b/crates/capabilities/skills/src/plugin.rs @@ -301,8 +301,8 @@ impl PluginLoader { #[cfg(test)] mod tests { - use super::*; use super::super::Skill; + use super::*; const SKILL_MD: &str = "---\ndescription: Review helper skill.\n---\n\nReview body here.\n"; diff --git a/crates/operations/bootstrap/src/session/assembly.rs b/crates/operations/bootstrap/src/session/assembly.rs index 861e6d1f..ed3e6763 100644 --- a/crates/operations/bootstrap/src/session/assembly.rs +++ b/crates/operations/bootstrap/src/session/assembly.rs @@ -1312,16 +1312,18 @@ pub(super) fn hook_defs( plugin.name )), Some(point) => { - defs.entry(point).or_default().extend(rules.iter().map( - |rule| wavecode_hooks::HookDef { - matcher: rule.matcher.clone(), - command: rule.command.clone(), - timeout_ms: rule - .timeout_ms - .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), - once: rule.once.unwrap_or(false), - }, - )); + defs.entry(point) + .or_default() + .extend(rules.iter().map(|rule| { + wavecode_hooks::HookDef { + matcher: rule.matcher.clone(), + command: rule.command.clone(), + timeout_ms: rule + .timeout_ms + .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), + once: rule.once.unwrap_or(false), + } + })); } } } @@ -1368,7 +1370,9 @@ pub(super) fn pack_mcp_servers( /// Lift a pack MCP entry into the config-crate shape. Packs carry no /// OAuth fields, so those default to absent. -fn plugin_mcp_to_config(raw: &wavecode_skills::plugin::PluginMcpServer) -> wavecode_config::McpServerRaw { +fn plugin_mcp_to_config( + raw: &wavecode_skills::plugin::PluginMcpServer, +) -> wavecode_config::McpServerRaw { wavecode_config::McpServerRaw { command: raw.command.clone(), args: raw.args.clone(), diff --git a/crates/operations/bootstrap/src/session/tests.rs b/crates/operations/bootstrap/src/session/tests.rs index 0c48fbfb..dfaafea7 100644 --- a/crates/operations/bootstrap/src/session/tests.rs +++ b/crates/operations/bootstrap/src/session/tests.rs @@ -1055,10 +1055,7 @@ async fn pack_contents_reach_the_session_under_standard_precedence() { let dir = tempfile::tempdir().unwrap(); let home = home_with_pack( "demo", - &[ - ("plugin.toml", PACK_MANIFEST), - ("hooks.toml", PACK_HOOKS), - ], + &[("plugin.toml", PACK_MANIFEST), ("hooks.toml", PACK_HOOKS)], ); // Pack skills merge under the standard sources: the user skill // overrides the pack skill, and the project skill overrides both, @@ -1147,15 +1144,10 @@ async fn configured_mcp_server_wins_over_pack_entry() { let config_path = dir.path().join("config.toml"); std::fs::write( &config_path, - format!( - "{CONFIG}\n[mcp_servers.lint]\ncommand = \"config-server\"\n" - ), + format!("{CONFIG}\n[mcp_servers.lint]\ncommand = \"config-server\"\n"), ) .unwrap(); - let home = home_with_pack( - "demo", - &[("plugin.toml", PACK_MANIFEST)], - ); + let home = home_with_pack("demo", &[("plugin.toml", PACK_MANIFEST)]); let handle = assemble_session(AssembleOptions { config_path: Some(config_path), @@ -1198,10 +1190,7 @@ fn pack_hook_rules_join_config_rules_at_the_same_point() { let config = config_with("\n[hooks.PreToolUse]\ncommand = \"config.sh\"\n"); let home = home_with_pack( "demo", - &[ - ("plugin.toml", PACK_MANIFEST), - ("hooks.toml", PACK_HOOKS), - ], + &[("plugin.toml", PACK_MANIFEST), ("hooks.toml", PACK_HOOKS)], ); let plugins = wavecode_skills::plugin::PluginLoader::load(Some(home.path())); let mut warnings = Vec::new(); From d958f29655752f75294fa227e4708f6b95bb5003 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Thu, 29 Oct 2026 00:31:29 +0800 Subject: [PATCH 03/12] test: keep tainted config warnings out of assertion diagnostics The pack-wiring tests echoed the whole warnings vector in assert messages; the vector carries config-derived text (the fixture's inline provider key flows through assembly), which CodeQL's cleartext-logging rule reads as logging sensitive content. The assertions now report only the warnings count; the matched-substring checks are unchanged, so the coverage holds. --- .../operations/bootstrap/src/session/tests.rs | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/crates/operations/bootstrap/src/session/tests.rs b/crates/operations/bootstrap/src/session/tests.rs index dfaafea7..51fcc70a 100644 --- a/crates/operations/bootstrap/src/session/tests.rs +++ b/crates/operations/bootstrap/src/session/tests.rs @@ -1126,13 +1126,14 @@ async fn pack_contents_reach_the_session_under_standard_precedence() { ); // Hooks: the unknown pack point warns with the pack attribution. + let unknown_point = handle + .warnings + .iter() + .any(|w| w.contains("plugin demo") && w.contains("unknown hook point")); assert!( - handle - .warnings - .iter() - .any(|w| w.contains("plugin demo") && w.contains("unknown hook point")), - "{:?}", - handle.warnings + unknown_point, + "unknown-point warning missing (warnings_count={})", + handle.warnings.len() ); } @@ -1172,13 +1173,14 @@ async fn configured_mcp_server_wins_over_pack_entry() { .collect(); assert_eq!(lints.len(), 1, "the pack entry must not ride along"); assert_eq!(lints[0].1.command.as_deref(), Some("config-server")); + let name_owned = handle + .warnings + .iter() + .any(|w| w.contains("already owns that name")); assert!( - handle - .warnings - .iter() - .any(|w| w.contains("already owns that name")), - "{:?}", - handle.warnings + name_owned, + "collision warning missing (warnings_count={})", + handle.warnings.len() ); } @@ -1210,11 +1212,13 @@ fn pack_hook_rules_join_config_rules_at_the_same_point() { // config-declared rules. assert_eq!(post[0].timeout_ms, wavecode_hooks::DEFAULT_TIMEOUT_MS); + let unknown_point = warnings + .iter() + .any(|w| w.contains("plugin demo") && w.contains("BogusPoint")); assert!( - warnings - .iter() - .any(|w| w.contains("plugin demo") && w.contains("BogusPoint")), - "{warnings:?}" + unknown_point, + "unknown-point warning missing (warnings_count={})", + warnings.len() ); } @@ -1255,12 +1259,15 @@ fn pack_mcp_entries_convert_with_config_priority_and_pack_order() { .filter(|w| w.contains("already owns that name")) .count(), 2, - "both packs' lint entries drop against config: {warnings:?}" + "both packs' lint entries drop against config (warnings_count={})", + warnings.len() ); + let replaced = warnings + .iter() + .any(|w| w.contains("replaces the earlier pack's entry")); assert!( - warnings - .iter() - .any(|w| w.contains("replaces the earlier pack's entry")), - "{warnings:?}" + replaced, + "pack-vs-pack warning missing (warnings_count={})", + warnings.len() ); } From 4d37ab8858dba30ac5306a85c5ad2eb7e7960b6e Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Thu, 29 Oct 2026 00:45:16 +0800 Subject: [PATCH 04/12] fix: let a later pack displace an earlier pack's skill apply_skills' fill-the-gaps guard skipped a later pack's same-named skill, contradicting the documented precedence (later packs win among themselves, in directory order). A pack entry now yields to the later pack; user, project, and builtin entries keep their slots unchanged. A two-pack collision test pins the rule. --- crates/capabilities/skills/src/plugin.rs | 46 +++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/crates/capabilities/skills/src/plugin.rs b/crates/capabilities/skills/src/plugin.rs index aaaf2429..db3990d0 100644 --- a/crates/capabilities/skills/src/plugin.rs +++ b/crates/capabilities/skills/src/plugin.rs @@ -215,7 +215,14 @@ impl PluginLoader { pub fn apply_skills(&self, set: &mut SkillSet) { for plugin in &self.plugins { for skill in plugin.skills.iter() { - if set.get(&skill.name).is_none() { + // A non-pack entry (user, project, or builtin) keeps its + // slot; a pack entry yields to the later pack, in + // directory order. + let displaced = match set.get(&skill.name) { + Some(existing) => existing.source == SkillSource::Pack, + None => true, + }; + if displaced { set.add(skill.clone()); } } @@ -487,4 +494,41 @@ command = "notify.sh" loader.apply_skills(&mut set); assert_eq!(set.get("greet").unwrap().source, SkillSource::User); } + + /// Two packs declaring the same skill name: the later pack (directory + /// order) wins the slot, while a user skill still beats both. + #[test] + fn later_pack_wins_the_same_named_skill() { + let manifest = |name: &str| { + format!( + "name = \"{name}\" +version = \"1\" +skills_dir = \"skills\" +" + ) + }; + let home = home_with_pack("aaa", &[("plugin.toml", &manifest("aaa"))]); + let pack = home.path().join(".wavecode").join("plugins").join("bbb"); + std::fs::create_dir_all(pack.join("skills").join("greet")).unwrap(); + std::fs::write(pack.join("plugin.toml"), manifest("bbb")).unwrap(); + std::fs::write(pack.join("skills").join("greet").join("SKILL.md"), SKILL_MD).unwrap(); + let first = home.path().join(".wavecode").join("plugins").join("aaa"); + std::fs::create_dir_all(first.join("skills").join("greet")).unwrap(); + std::fs::write( + first.join("skills").join("greet").join("SKILL.md"), + SKILL_MD, + ) + .unwrap(); + let loader = PluginLoader::load(Some(home.path())); + assert_eq!(loader.plugins().len(), 2); + + let mut set = SkillSet::default(); + loader.apply_skills(&mut set); + assert_eq!(set.get("greet").unwrap().source, SkillSource::Pack); + + // The later pack's body displaced the first pack's entry (both + // packs carry the same SKILL.md fixture, so the slot survives the + // collision with one entry and no duplicate). + assert_eq!(set.len(), 1); + } } From e51b3ccc7b9aec8d38834168f5ab51e9f1d5a187 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Thu, 29 Oct 2026 05:25:16 +0800 Subject: [PATCH 05/12] test: keep config-derived values out of assertion messages CodeQL's cleartext-logging rule traces the fixture config (which carries a dummy inline provider key) through assembly into every handle-derived value, so echoing handle.system or even the warnings count in assert diagnostics trips it. The assertions keep their matched-substring checks; the messages are static text now. --- crates/operations/bootstrap/src/session/tests.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/crates/operations/bootstrap/src/session/tests.rs b/crates/operations/bootstrap/src/session/tests.rs index 51fcc70a..2e6f5ea4 100644 --- a/crates/operations/bootstrap/src/session/tests.rs +++ b/crates/operations/bootstrap/src/session/tests.rs @@ -1100,8 +1100,7 @@ async fn pack_contents_reach_the_session_under_standard_precedence() { // Skills: project > user > pack, and the pack-only skill lands. assert!( handle.system.contains("project greet helper"), - "project skill must win: {}", - handle.system + "the project skill must win the greet slot" ); assert!(!handle.system.contains("user greet helper")); assert!(!handle.system.contains("pack greet helper")); @@ -1130,11 +1129,7 @@ async fn pack_contents_reach_the_session_under_standard_precedence() { .warnings .iter() .any(|w| w.contains("plugin demo") && w.contains("unknown hook point")); - assert!( - unknown_point, - "unknown-point warning missing (warnings_count={})", - handle.warnings.len() - ); + assert!(unknown_point, "unknown-point warning missing"); } /// A user-configured MCP server owns its name: the colliding pack entry @@ -1177,11 +1172,7 @@ async fn configured_mcp_server_wins_over_pack_entry() { .warnings .iter() .any(|w| w.contains("already owns that name")); - assert!( - name_owned, - "collision warning missing (warnings_count={})", - handle.warnings.len() - ); + assert!(name_owned, "collision warning missing"); } /// Hook definitions: config rules keep their point slot, pack rules From 5f2ce8500f5feabb45435ec459491a37c930d230 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Thu, 29 Oct 2026 07:43:00 +0800 Subject: [PATCH 06/12] refactor: split hook_defs into config and pack passes The merged loop body flattened into three named helpers: the config pass, the pack append pass, and the shared rule conversion. The bumpy-road finding came from interleaved early-skip matches at two nesting levels; each pass is a straight scan now. --- .../bootstrap/src/session/assembly.rs | 78 ++++++++++++------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/crates/operations/bootstrap/src/session/assembly.rs b/crates/operations/bootstrap/src/session/assembly.rs index ed3e6763..273d746d 100644 --- a/crates/operations/bootstrap/src/session/assembly.rs +++ b/crates/operations/bootstrap/src/session/assembly.rs @@ -1271,6 +1271,9 @@ fn assemble_hooks( ))) } +/// Build the engine's definitions: config-declared rules come first at +/// every event point; pack rules append after (pack directory order, +/// then file order within a pack) with the same matcher/timeout /// Build the engine's definitions: config-declared rules come first at /// every event point; pack rules append after (pack directory order, /// then file order within a pack) with the same matcher/timeout @@ -1282,28 +1285,51 @@ pub(super) fn hook_defs( ) -> HashMap> { let mut defs: HashMap> = HashMap::new(); + config_hook_defs(config, &mut defs, warnings); + pack_hook_defs(plugins, &mut defs, warnings); + defs +} + +/// One config `HookRule` in engine form (absent timeout/once fall back +/// to the engine defaults). +fn config_rule_to_def(rule: &wavecode_config::HookRule) -> wavecode_hooks::HookDef { + wavecode_hooks::HookDef { + matcher: rule.matcher.clone(), + command: rule.command.clone(), + timeout_ms: rule + .timeout_ms + .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), + once: rule.once.unwrap_or(false), + } +} + +/// Insert the config-declared rules; an unknown event point warns and +/// skips (config keys are free-form TOML). +fn config_hook_defs( + config: &wavecode_config::Config, + defs: &mut HashMap>, + warnings: &mut Vec, +) { for (point_name, ruleset) in &config.hooks { match wavecode_hooks::HookEventPoint::parse(point_name) { None => warnings.push(format!("unknown hook point {point_name:?}; skipped")), Some(point) => { defs.insert( point, - ruleset - .rules() - .iter() - .map(|rule| wavecode_hooks::HookDef { - matcher: rule.matcher.clone(), - command: rule.command.clone(), - timeout_ms: rule - .timeout_ms - .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), - once: rule.once.unwrap_or(false), - }) - .collect(), + ruleset.rules().iter().map(config_rule_to_def).collect(), ); } } } +} + +/// Append the pack rules after the config rules at every event point +/// (an unknown pack point warns with the pack attribution). +fn pack_hook_defs( + plugins: &wavecode_skills::plugin::PluginLoader, + defs: &mut HashMap>, + warnings: &mut Vec, +) { for plugin in plugins.plugins() { for (point_name, rules) in &plugin.hooks { match wavecode_hooks::HookEventPoint::parse(point_name) { @@ -1311,24 +1337,22 @@ pub(super) fn hook_defs( "plugin {}: unknown hook point {point_name:?}; skipped", plugin.name )), - Some(point) => { - defs.entry(point) - .or_default() - .extend(rules.iter().map(|rule| { - wavecode_hooks::HookDef { - matcher: rule.matcher.clone(), - command: rule.command.clone(), - timeout_ms: rule - .timeout_ms - .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), - once: rule.once.unwrap_or(false), - } - })); - } + Some(point) => defs + .entry(point) + .or_default() + .extend(rules.iter().map(|rule| { + wavecode_hooks::HookDef { + matcher: rule.matcher.clone(), + command: rule.command.clone(), + timeout_ms: rule + .timeout_ms + .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), + once: rule.once.unwrap_or(false), + } + })), } } } - defs } /// Convert pack MCP entries into config-crate servers for the live From bfe809bf6df8bb8a0fdb2aca42d27d659ca28b53 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Sat, 31 Oct 2026 23:06:32 +0800 Subject: [PATCH 07/12] fix(skills): let pack skills displace builtins The merge preserved every non-pack entry, so a same-named builtin kept its slot even though SkillSource ranks pack above builtin (builtin < pack < user < project). A pack skill now displaces a builtin entry and still yields to user and project entries. --- crates/capabilities/skills/src/plugin.rs | 32 +++++++++++++++++------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/crates/capabilities/skills/src/plugin.rs b/crates/capabilities/skills/src/plugin.rs index db3990d0..0e5c68a5 100644 --- a/crates/capabilities/skills/src/plugin.rs +++ b/crates/capabilities/skills/src/plugin.rs @@ -207,19 +207,22 @@ impl PluginLoader { /// Merge every pack's skills into `set` as the pack source. /// - /// Pack skills are the lowest-priority source above builtin: an - /// entry already in `set` (user, project, or a merged builtin) keeps - /// its slot and the pack only fills the gaps, so the caller merges - /// packs after the standard roots. Later packs win among themselves, - /// in directory order. + /// Pack skills rank between builtin and the standard roots: they + /// displace a same-named builtin entry and yield to user and project + /// entries, which keep their slots, so the caller merges packs after + /// the standard roots. Later packs win among themselves, in + /// directory order. pub fn apply_skills(&self, set: &mut SkillSet) { for plugin in &self.plugins { for skill in plugin.skills.iter() { - // A non-pack entry (user, project, or builtin) keeps its - // slot; a pack entry yields to the later pack, in - // directory order. + // A user or project entry keeps its slot; a pack entry + // yields to the later pack, in directory order; a builtin + // entry yields to the pack (source order: builtin < pack + // < user < project). let displaced = match set.get(&skill.name) { - Some(existing) => existing.source == SkillSource::Pack, + Some(existing) => { + matches!(existing.source, SkillSource::Builtin | SkillSource::Pack) + } None => true, }; if displaced { @@ -493,6 +496,17 @@ command = "notify.sh" set.add(Skill::parse(&greet, SkillSource::User).expect("hand-built skill")); loader.apply_skills(&mut set); assert_eq!(set.get("greet").unwrap().source, SkillSource::User); + + // A builtin entry yields to the pack: pack ranks above builtin + // in the source order (builtin < pack < user < project). + let builtin = tempfile::tempdir().unwrap(); + let greet = builtin.path().join("greet"); + std::fs::create_dir_all(&greet).unwrap(); + std::fs::write(greet.join("SKILL.md"), SKILL_MD).unwrap(); + let mut set = SkillSet::default(); + set.add(Skill::parse(&greet, SkillSource::Builtin).expect("hand-built skill")); + loader.apply_skills(&mut set); + assert_eq!(set.get("greet").unwrap().source, SkillSource::Pack); } /// Two packs declaring the same skill name: the later pack (directory From a683a04ce7ef5bf1c8d9230888c0a558b91833f9 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Sat, 31 Oct 2026 23:06:38 +0800 Subject: [PATCH 08/12] fix(bootstrap): skip unusable pack content Pack hook rules with an empty command or a blank matcher joined the engine silently, and a pack MCP entry with neither command nor url displaced an earlier pack's valid same-named entry, leaving no server for that name once the connect path rejected it. Both surfaces now share the engine's unusable-entry rule (HookDef::unusable_reasons, extracted from HookEngine::validate so the diagnostics cannot drift) and the connect path's own conversion (McpServerConfig::from_raw): the entry warns with the pack attribution and is skipped. --- crates/capabilities/hooks/src/lib.rs | 42 +++++---- .../bootstrap/src/session/assembly.rs | 53 ++++++++--- .../operations/bootstrap/src/session/tests.rs | 89 +++++++++++++++++++ 3 files changed, 152 insertions(+), 32 deletions(-) diff --git a/crates/capabilities/hooks/src/lib.rs b/crates/capabilities/hooks/src/lib.rs index 5bba4d33..e70fb79e 100644 --- a/crates/capabilities/hooks/src/lib.rs +++ b/crates/capabilities/hooks/src/lib.rs @@ -171,6 +171,28 @@ impl HookDef { self.timeout_ms }) } + + /// Reasons this entry can never do anything useful at `point`: an + /// empty command, a blank matcher, or a matcher on a point that + /// carries no tool name. Empty when the entry is usable; a zero + /// timeout is not listed because it falls back to the default + /// rather than disabling the entry. [`HookEngine::validate`] and + /// the assembly layers share this rule, so a skipped entry and the + /// static diagnostics cannot drift apart. + pub fn unusable_reasons(&self, point: HookEventPoint) -> Vec<&'static str> { + let mut reasons = Vec::new(); + if self.command.trim().is_empty() { + reasons.push("has an empty command (never executes)"); + } + if let Some(matcher) = &self.matcher { + if matcher.trim().is_empty() { + reasons.push("matcher is blank (never matches)"); + } else if !is_tool_point(point) { + reasons.push("has a matcher but this point carries no tool name (never fires)"); + } + } + reasons + } } /// An event payload for one hook firing. @@ -293,11 +315,8 @@ impl HookEngine { tables.sort_by_key(|(p, _)| p.as_str()); for (point, defs) in tables { for (idx, def) in defs.iter().enumerate() { - if def.command.trim().is_empty() { - out.push(format!( - "[{}] entry #{idx} has an empty command (never executes)", - point.as_str() - )); + for reason in def.unusable_reasons(point) { + out.push(format!("[{}] entry #{idx} {reason}", point.as_str())); } if def.timeout_ms == 0 { out.push(format!( @@ -305,19 +324,6 @@ impl HookEngine { point.as_str() )); } - if let Some(matcher) = &def.matcher { - if matcher.trim().is_empty() { - out.push(format!( - "[{}] entry #{idx} matcher is blank (never matches)", - point.as_str() - )); - } else if !is_tool_point(point) { - out.push(format!( - "[{}] entry #{idx} has a matcher but this point carries no tool name (never fires)", - point.as_str() - )); - } - } } } out diff --git a/crates/operations/bootstrap/src/session/assembly.rs b/crates/operations/bootstrap/src/session/assembly.rs index 273d746d..7181c936 100644 --- a/crates/operations/bootstrap/src/session/assembly.rs +++ b/crates/operations/bootstrap/src/session/assembly.rs @@ -1271,9 +1271,6 @@ fn assemble_hooks( ))) } -/// Build the engine's definitions: config-declared rules come first at -/// every event point; pack rules append after (pack directory order, -/// then file order within a pack) with the same matcher/timeout /// Build the engine's definitions: config-declared rules come first at /// every event point; pack rules append after (pack directory order, /// then file order within a pack) with the same matcher/timeout @@ -1323,8 +1320,13 @@ fn config_hook_defs( } } -/// Append the pack rules after the config rules at every event point -/// (an unknown pack point warns with the pack attribution). +/// Append the pack rules after the config rules at every event point. +/// +/// An unknown pack point warns with the pack attribution; a rule the +/// engine can never fire (an empty command, a blank matcher, or a +/// matcher on a point that carries no tool name — the engine's own +/// [`wavecode_hooks::HookDef::unusable_reasons`] rule) warns with the +/// pack name and the reason, and is skipped. fn pack_hook_defs( plugins: &wavecode_skills::plugin::PluginLoader, defs: &mut HashMap>, @@ -1337,19 +1339,29 @@ fn pack_hook_defs( "plugin {}: unknown hook point {point_name:?}; skipped", plugin.name )), - Some(point) => defs - .entry(point) - .or_default() - .extend(rules.iter().map(|rule| { - wavecode_hooks::HookDef { + Some(point) => { + let slot = defs.entry(point).or_default(); + for (idx, rule) in rules.iter().enumerate() { + let def = wavecode_hooks::HookDef { matcher: rule.matcher.clone(), command: rule.command.clone(), timeout_ms: rule .timeout_ms .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), once: rule.once.unwrap_or(false), + }; + let reasons = def.unusable_reasons(point); + if reasons.is_empty() { + slot.push(def); + } else { + warnings.push(format!( + "plugin {}: hook {point_name} entry #{idx} skipped: {}", + plugin.name, + reasons.join("; ") + )); } - })), + } + } } } } @@ -1360,7 +1372,12 @@ fn pack_hook_defs( /// /// A user-configured server owns its name: a colliding pack entry warns /// and drops. Among packs the later one wins a shared name (directory -/// order), matching pack skill precedence. +/// order), matching pack skill precedence. Every converted entry goes +/// through the connect path's own conversion +/// ([`wavecode_mcp::McpServerConfig::from_raw`]) first: an entry that +/// could never connect warns with the pack name and the reason, and is +/// skipped — so it also cannot displace an earlier pack's valid entry +/// on a shared name. pub(super) fn pack_mcp_servers( config: &wavecode_config::Config, plugins: &wavecode_skills::plugin::PluginLoader, @@ -1376,15 +1393,23 @@ pub(super) fn pack_mcp_servers( )); continue; } + let server = plugin_mcp_to_config(raw); + if let Err(reason) = wavecode_mcp::McpServerConfig::from_raw(name, &server) { + warnings.push(format!( + "plugin {}: mcp server {name:?} skipped: {reason}", + plugin.name + )); + continue; + } match servers.iter_mut().find(|(known, _)| known == name) { Some(slot) => { warnings.push(format!( "plugin {}: mcp server {name:?} replaces the earlier pack's entry", plugin.name )); - slot.1 = plugin_mcp_to_config(raw); + slot.1 = server; } - None => servers.push((name.clone(), plugin_mcp_to_config(raw))), + None => servers.push((name.clone(), server)), } } } diff --git a/crates/operations/bootstrap/src/session/tests.rs b/crates/operations/bootstrap/src/session/tests.rs index 2e6f5ea4..09759af6 100644 --- a/crates/operations/bootstrap/src/session/tests.rs +++ b/crates/operations/bootstrap/src/session/tests.rs @@ -1262,3 +1262,92 @@ fn pack_mcp_entries_convert_with_config_priority_and_pack_order() { warnings.len() ); } + +/// A pack rule the engine can never fire (an empty command, a blank +/// matcher) warns with the pack attribution and is skipped, while the +/// usable rules from the same pack still load. +#[test] +fn unusable_pack_hook_rules_warn_and_skip() { + let config = config_with(""); + let pack_hooks = r#" +[[PreToolUse]] +command = "pack.sh" + +[[PreToolUse]] +command = " " + +[[PostToolUse]] +matcher = " " +command = "fmt.sh" +"#; + let home = home_with_pack( + "demo", + &[("plugin.toml", PACK_MANIFEST), ("hooks.toml", pack_hooks)], + ); + let plugins = wavecode_skills::plugin::PluginLoader::load(Some(home.path())); + let mut warnings = Vec::new(); + let defs = super::assembly::hook_defs(&config, &plugins, &mut warnings); + + // Only the usable rule reaches the engine; the unusable ones warn + // with the pack name and never join the definitions. + let pre = &defs[&wavecode_hooks::HookEventPoint::PreToolUse]; + assert_eq!(pre.len(), 1); + assert_eq!(pre[0].command, "pack.sh"); + let post = &defs[&wavecode_hooks::HookEventPoint::PostToolUse]; + assert!(post.is_empty(), "a blank matcher never fires: {post:?}"); + + let skipped = warnings + .iter() + .filter(|w| w.contains("plugin demo") && w.contains("skipped")) + .count(); + assert_eq!(skipped, 2, "both unusable rules warn: {warnings:?}"); +} + +/// An entry that could never connect (neither command nor url) warns +/// with the pack attribution and is skipped, so a later pack's invalid +/// entry cannot displace an earlier pack's valid one on a shared name. +#[test] +fn invalid_pack_mcp_entry_warns_and_skips() { + let config = config_with(""); + let home = tempfile::tempdir().unwrap(); + let plugins_root = home.path().join(".wavecode").join("plugins"); + let first = plugins_root.join("aaa"); + std::fs::create_dir_all(&first).unwrap(); + std::fs::write( + first.join("plugin.toml"), + "name = \"aaa\"\nversion = \"1\"\n\n[mcp_servers.fetch]\ncommand = \"first-server\"\n", + ) + .unwrap(); + let second = plugins_root.join("zzz"); + std::fs::create_dir_all(&second).unwrap(); + std::fs::write( + second.join("plugin.toml"), + "name = \"zzz\"\nversion = \"1\"\n\n[mcp_servers.fetch]\nargs = []\n", + ) + .unwrap(); + let plugins = wavecode_skills::plugin::PluginLoader::load(Some(home.path())); + let mut warnings = Vec::new(); + let servers = super::assembly::pack_mcp_servers(&config, &plugins, &mut warnings); + + let names: Vec<&str> = servers.iter().map(|(name, _)| name.as_str()).collect(); + assert_eq!(names, vec!["fetch"]); + assert_eq!( + servers[0].1.command.as_deref(), + Some("first-server"), + "the later pack's invalid entry cannot displace the valid one" + ); + assert!( + warnings.iter().any(|w| w.contains("plugin zzz") + && w.contains("skipped") + && w.contains("neither command nor url")), + "pack-attributed skip warning missing (warnings_count={})", + warnings.len() + ); + assert!( + !warnings + .iter() + .any(|w| w.contains("replaces the earlier pack's entry")), + "no replacement happened (warnings_count={})", + warnings.len() + ); +} From 370c6036b4ad4521d12c9b4d177454762d976cba Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Sat, 31 Oct 2026 23:06:46 +0800 Subject: [PATCH 09/12] docs(subsystems): sync the pack behavior notes The extensibility pages now describe what the code implements: a builtin skill yields to a pack skill, an MCP entry that could never connect warns with its reason and skips, and a hook rule the engine can never fire warns with its reason and skips. --- docs/subsystems/extensibility.md | 2 +- docs/subsystems/extensibility.zh.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/subsystems/extensibility.md b/docs/subsystems/extensibility.md index 0f6ca843..ae0f0a44 100644 --- a/docs/subsystems/extensibility.md +++ b/docs/subsystems/extensibility.md @@ -34,7 +34,7 @@ Known limits, stated in the code: interactive browser/PKCE OAuth is out of scope ## Plugins (`crates/runtime/plugin/src/lib.rs`, `crates/capabilities/skills/src/plugin.rs`) -The runtime `Registry` starts plugins in manifest-dependency order and stops them in reverse; `ServiceMap` injects typed services with type-id keyed lookup. Discovery reads `runtime.toml` manifests (`{name, version, depends?}` identity) and degrades **warn-and-skip**: an invalid manifest warns with its reason and is skipped, never failing assembly. The `PluginLoader` in the skills crate loads user plugin packs (`plugin.toml`: `name`, `version`, optional `skills_dir`, hooks, MCP server entries) with the same warn-and-skip contract, and their contents take effect in sessions: pack skills merge into the catalog as the `pack` source below user and project (a same-named user or project skill keeps its slot), pack MCP entries convert into configured servers (a user-configured server owns its name; later packs win among themselves, in directory order), and pack hook rules join the engine after the config-declared ones with the same matcher/timeout semantics. +The runtime `Registry` starts plugins in manifest-dependency order and stops them in reverse; `ServiceMap` injects typed services with type-id keyed lookup. Discovery reads `runtime.toml` manifests (`{name, version, depends?}` identity) and degrades **warn-and-skip**: an invalid manifest warns with its reason and is skipped, never failing assembly. The `PluginLoader` in the skills crate loads user plugin packs (`plugin.toml`: `name`, `version`, optional `skills_dir`, hooks, MCP server entries) with the same warn-and-skip contract, and their contents take effect in sessions: pack skills merge into the catalog as the `pack` source below user and project (a same-named user or project skill keeps its slot; a builtin one yields), pack MCP entries convert into configured servers (a user-configured server owns its name; later packs win among themselves, in directory order; an entry that could never connect warns with its reason and skips instead of displacing an earlier pack's valid entry), and pack hook rules join the engine after the config-declared ones with the same matcher/timeout semantics (a rule the engine can never fire — an empty command or a blank matcher — warns with its reason and skips). ## Named agent definitions (`crates/capabilities/tools/src/agent_task_tool.rs`) diff --git a/docs/subsystems/extensibility.zh.md b/docs/subsystems/extensibility.zh.md index bf168e05..e2feeb33 100644 --- a/docs/subsystems/extensibility.zh.md +++ b/docs/subsystems/extensibility.zh.md @@ -34,7 +34,7 @@ Frontmatter 解析使用 `serde-saphyr` 而不是手写扫描器——一个刻 ## 插件（`crates/runtime/plugin/src/lib.rs`、`crates/capabilities/skills/src/plugin.rs`） -runtime 的 `Registry` 按清单依赖顺序启动插件，逆序停止；`ServiceMap` 以 type-id 键控查找注入类型化服务。发现读取 `runtime.toml` 清单（`{name, version, depends?}` 身份）并按**警告并跳过**降级：无效清单带原因警告并被跳过，绝不让装配失败。skills crate 中的 `PluginLoader` 加载用户插件包（`plugin.toml`：`name`、`version`、可选 `skills_dir`、hooks、MCP 服务器条目），带同样的 warn-and-skip 契约，其内容会在会话中生效：包技能以 `pack` 来源并入目录，优先级低于 user 与 project（同名的 user/project 技能保有自己的槽位）；包 MCP 条目转换为已配置服务器（用户配置的服务器拥有其名字；多个包之间按目录顺序后者获胜）；包 hook 规则以与配置规则相同的 matcher/timeout 语义追加到引擎中配置规则之后。 +runtime 的 `Registry` 按清单依赖顺序启动插件，逆序停止；`ServiceMap` 以 type-id 键控查找注入类型化服务。发现读取 `runtime.toml` 清单（`{name, version, depends?}` 身份）并按**警告并跳过**降级：无效清单带原因警告并被跳过，绝不让装配失败。skills crate 中的 `PluginLoader` 加载用户插件包（`plugin.toml`：`name`、`version`、可选 `skills_dir`、hooks、MCP 服务器条目），带同样的 warn-and-skip 契约，其内容会在会话中生效：包技能以 `pack` 来源并入目录，优先级低于 user 与 project（同名的 user/project 技能保有自己的槽位；builtin 槽位让位给 pack）；包 MCP 条目转换为已配置服务器（用户配置的服务器拥有其名字；多个包之间按目录顺序后者获胜；永远无法连接的条目带原因警告并被跳过，不会挤掉先包的有效条目）；包 hook 规则以与配置规则相同的 matcher/timeout 语义追加到引擎中配置规则之后（引擎永远无法触发的规则——空 command 或空 matcher——带原因警告并被跳过）。 ## 命名的 agent 定义（`crates/capabilities/tools/src/agent_task_tool.rs`） From 53e883f89928f451a010ba5cc01600f910802490 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Sun, 1 Nov 2026 00:08:43 +0800 Subject: [PATCH 10/12] test(skills): assert the later pack's body wins Both packs carried the same SKILL.md fixture, so the source and length assertions passed even if the earlier pack had stayed. Distinct descriptions pin the merged entry to the later pack's body. --- crates/capabilities/skills/src/plugin.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/capabilities/skills/src/plugin.rs b/crates/capabilities/skills/src/plugin.rs index 0e5c68a5..c928bff8 100644 --- a/crates/capabilities/skills/src/plugin.rs +++ b/crates/capabilities/skills/src/plugin.rs @@ -522,15 +522,24 @@ skills_dir = \"skills\" ) }; let home = home_with_pack("aaa", &[("plugin.toml", &manifest("aaa"))]); + // Distinct SKILL.md fixtures: both packs carry the same skill + // name, so the merged entry's description is what tells the + // later pack's body apart from the first pack's. + let first_skill = "---\ndescription: First pack skill.\n---\n\nFirst pack body.\n"; + let later_skill = "---\ndescription: Later pack skill.\n---\n\nLater pack body.\n"; let pack = home.path().join(".wavecode").join("plugins").join("bbb"); std::fs::create_dir_all(pack.join("skills").join("greet")).unwrap(); std::fs::write(pack.join("plugin.toml"), manifest("bbb")).unwrap(); - std::fs::write(pack.join("skills").join("greet").join("SKILL.md"), SKILL_MD).unwrap(); + std::fs::write( + pack.join("skills").join("greet").join("SKILL.md"), + later_skill, + ) + .unwrap(); let first = home.path().join(".wavecode").join("plugins").join("aaa"); std::fs::create_dir_all(first.join("skills").join("greet")).unwrap(); std::fs::write( first.join("skills").join("greet").join("SKILL.md"), - SKILL_MD, + first_skill, ) .unwrap(); let loader = PluginLoader::load(Some(home.path())); @@ -539,10 +548,13 @@ skills_dir = \"skills\" let mut set = SkillSet::default(); loader.apply_skills(&mut set); assert_eq!(set.get("greet").unwrap().source, SkillSource::Pack); + assert_eq!( + set.get("greet").unwrap().meta.description, + "Later pack skill.", + "the later pack's body displaced the first pack's entry" + ); - // The later pack's body displaced the first pack's entry (both - // packs carry the same SKILL.md fixture, so the slot survives the - // collision with one entry and no duplicate). + // The collision leaves one entry, not a duplicate. assert_eq!(set.len(), 1); } } From b4bd184b885e57d197fa67b7bb9d191ee6186489 Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Sun, 1 Nov 2026 00:08:45 +0800 Subject: [PATCH 11/12] refactor(bootstrap): flatten the pack hook pass The per-rule validation loop nested four levels deep inside pack_hook_defs. File one point's rules in a dedicated helper, so the walk stays at the same depth as the config pass. --- .../bootstrap/src/session/assembly.rs | 65 ++++++++++++------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/crates/operations/bootstrap/src/session/assembly.rs b/crates/operations/bootstrap/src/session/assembly.rs index 7181c936..659ea267 100644 --- a/crates/operations/bootstrap/src/session/assembly.rs +++ b/crates/operations/bootstrap/src/session/assembly.rs @@ -1339,34 +1339,53 @@ fn pack_hook_defs( "plugin {}: unknown hook point {point_name:?}; skipped", plugin.name )), - Some(point) => { - let slot = defs.entry(point).or_default(); - for (idx, rule) in rules.iter().enumerate() { - let def = wavecode_hooks::HookDef { - matcher: rule.matcher.clone(), - command: rule.command.clone(), - timeout_ms: rule - .timeout_ms - .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), - once: rule.once.unwrap_or(false), - }; - let reasons = def.unusable_reasons(point); - if reasons.is_empty() { - slot.push(def); - } else { - warnings.push(format!( - "plugin {}: hook {point_name} entry #{idx} skipped: {}", - plugin.name, - reasons.join("; ") - )); - } - } - } + Some(point) => file_pack_rules( + &plugin.name, + point_name, + point, + rules, + defs.entry(point).or_default(), + warnings, + ), } } } } +/// File one pack point's rules into `slot` in rule order. +/// +/// A rule the engine can never fire +/// ([`wavecode_hooks::HookDef::unusable_reasons`]) warns with the pack +/// attribution and is skipped instead of joining `slot`. +fn file_pack_rules( + plugin_name: &str, + point_name: &str, + point: wavecode_hooks::HookEventPoint, + rules: &[wavecode_skills::plugin::PluginHookRule], + slot: &mut Vec, + warnings: &mut Vec, +) { + for (idx, rule) in rules.iter().enumerate() { + let def = wavecode_hooks::HookDef { + matcher: rule.matcher.clone(), + command: rule.command.clone(), + timeout_ms: rule + .timeout_ms + .unwrap_or(wavecode_hooks::DEFAULT_TIMEOUT_MS), + once: rule.once.unwrap_or(false), + }; + let reasons = def.unusable_reasons(point); + if reasons.is_empty() { + slot.push(def); + } else { + warnings.push(format!( + "plugin {plugin_name}: hook {point_name} entry #{idx} skipped: {}", + reasons.join("; ") + )); + } + } +} + /// Convert pack MCP entries into config-crate servers for the live /// connect path. /// From c172536c13decbc338dfd2a35d3d856229e6520d Mon Sep 17 00:00:00 2001 From: daftpunkwav Date: Sun, 1 Nov 2026 01:11:06 +0800 Subject: [PATCH 12/12] test(skills): also assert the merged skill body The collision test checked only the merged metadata, so a regression that kept the first pack's body while replacing its metadata would still pass. --- crates/capabilities/skills/src/plugin.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/capabilities/skills/src/plugin.rs b/crates/capabilities/skills/src/plugin.rs index c928bff8..cb43b7d1 100644 --- a/crates/capabilities/skills/src/plugin.rs +++ b/crates/capabilities/skills/src/plugin.rs @@ -548,10 +548,14 @@ skills_dir = \"skills\" let mut set = SkillSet::default(); loader.apply_skills(&mut set); assert_eq!(set.get("greet").unwrap().source, SkillSource::Pack); + let merged = set.get("greet").unwrap(); assert_eq!( - set.get("greet").unwrap().meta.description, - "Later pack skill.", - "the later pack's body displaced the first pack's entry" + merged.meta.description, "Later pack skill.", + "the later pack's metadata displaced the first pack's entry" + ); + assert_eq!( + merged.body, "Later pack body.", + "the merged entry carries the later pack's body, not just its metadata" ); // The collision leaves one entry, not a duplicate.