diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b27bace9a..b5f733bf60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- MCP protocol negotiation: every surface advertised the original 2024-11-05 + revision and the stdio client required an exact match, so newer servers + could not connect. The server and both clients now advertise 2025-06-18 + and negotiate over the supported set (2025-06-18, 2025-03-26, 2024-11-05) + — the server echoes the client's revision when it is supported and answers + with the latest otherwise, the stdio client accepts any supported revision, + and streamable HTTP sends the required `MCP-Protocol-Version` header on + every post-initialize request (#6280, first half). - Configured MCP servers now connect lazily instead of all at session boot. The pool owns a `connecting` set marked at spawn and cleared on resolution or abort, so "connecting" is no longer inferred as enabled-minus-connected. The @@ -114,6 +122,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `` suggestions stop nagging: a plugin id is now + injected at most once per engine lifetime, and a plugin whose name a + loaded skill already covers is never suggested — the local skill owns + the domain, so the nudge was noise. Dismissals still apply, and the + fragment stays append-only on the user turn (#6274). - A canceled automation run now settles with a transcript receipt that names the cancellation (by request, cancel timeout, or shutdown) instead of vanishing from the live band silently. The receipt wears attention ink and diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index fbdfca872f..13f4597caa 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -1036,14 +1036,20 @@ fn default_rpc_methods() -> Vec<&'static str> { ] } -const MCP_PROTOCOL_VERSION: &str = "2024-11-05"; +/// Latest dated MCP protocol revision this server implements. Codewhale's +/// MCP clients advertise the same revision at `initialize`. +pub(crate) const MCP_PROTOCOL_VERSION: &str = "2025-06-18"; +/// Dated MCP revisions accepted during protocol negotiation, newest first. +/// Servers answering an older supported revision get it echoed back. +pub(crate) const MCP_SUPPORTED_PROTOCOL_VERSIONS: &[&str] = + &[MCP_PROTOCOL_VERSION, "2025-03-26", "2024-11-05"]; const MCP_SERVER_NAME: &str = "codewhale-mcp-server"; -fn initialize_response(state: &StdioMcpState) -> Value { +fn initialize_response(state: &StdioMcpState, protocol_version: &str) -> Value { json!({ // Standard MCP initialize result. Keep the management metadata below // as additive compatibility fields for existing Codewhale clients. - "protocolVersion": MCP_PROTOCOL_VERSION, + "protocolVersion": protocol_version, "capabilities": { "tools": {}, "resources": {} @@ -1344,12 +1350,21 @@ fn dispatch_stdio_request( // Deserializing into a Map above is the object-shape check. The // proxy does not currently consume any client capability. let _client_capabilities = parsed.capabilities; + // Per spec, echo the requested revision when we support it; + // otherwise answer with the newest revision we do support and + // let the client decide whether to continue. + let negotiated = + if MCP_SUPPORTED_PROTOCOL_VERSIONS.contains(&parsed.protocol_version.as_str()) { + parsed.protocol_version + } else { + MCP_PROTOCOL_VERSION.to_string() + }; state.session_phase = McpSessionPhase::InitializeResponded; - Ok((initialize_response(state), false)) + Ok((initialize_response(state, &negotiated), false)) } // Pre-standard Codewhale management alias; it intentionally requires // no MCP initialize envelope. - "capabilities" => Ok((initialize_response(state), false)), + "capabilities" => Ok((initialize_response(state, MCP_PROTOCOL_VERSION), false)), "notifications/initialized" => { if state.session_phase != McpSessionPhase::InitializeResponded { return Err(JsonRpcError::invalid_request( @@ -2434,7 +2449,7 @@ mod tests { #[test] fn stdio_initialize_uses_standard_mcp_shape_and_codewhale_identity() { let state = build_stdio_state(Vec::new()); - let response = initialize_response(&state); + let response = initialize_response(&state, MCP_PROTOCOL_VERSION); assert_eq!(response["protocolVersion"], MCP_PROTOCOL_VERSION); assert_eq!(response["serverInfo"]["name"], MCP_SERVER_NAME); assert_eq!(response["serverInfo"]["version"], env!("CARGO_PKG_VERSION")); diff --git a/crates/mcp/src/stdio_client.rs b/crates/mcp/src/stdio_client.rs index 74debd82d1..fdef4d2a4d 100644 --- a/crates/mcp/src/stdio_client.rs +++ b/crates/mcp/src/stdio_client.rs @@ -26,12 +26,10 @@ use std::os::windows::process::CommandExt; use anyhow::{Context, Result, anyhow, bail}; use serde_json::{Value, json}; -use crate::{McpManagedClient, McpResourceDescriptor, McpServerConfig, McpToolDescriptor}; - -/// Protocol revision advertised during the handshake. Matches the revision the -/// TUI's MCP pool negotiates (`crates/tui/src/mcp.rs`), so a server that works -/// in the TUI works here. -const PROTOCOL_VERSION: &str = "2024-11-05"; +use crate::{ + MCP_PROTOCOL_VERSION, MCP_SUPPORTED_PROTOCOL_VERSIONS, McpManagedClient, McpResourceDescriptor, + McpServerConfig, McpToolDescriptor, +}; /// Budget for spawn + `initialize` + `notifications/initialized`. Generous /// because a first `npx`/`uvx` launch may download the server package. @@ -436,9 +434,12 @@ fn validate_initialize_result( .with_context(|| { format!("MCP server '{server_name}': initialize result omitted protocolVersion") })?; - if protocol_version != PROTOCOL_VERSION { + // Negotiation per spec: we advertise the newest revision and accept any + // dated revision we still implement; anything else ends the handshake. + if !MCP_SUPPORTED_PROTOCOL_VERSIONS.contains(&protocol_version) { bail!( - "MCP server '{server_name}': unsupported protocol version '{protocol_version}' (expected {PROTOCOL_VERSION})" + "MCP server '{server_name}': unsupported protocol version '{protocol_version}' (supported: {})", + MCP_SUPPORTED_PROTOCOL_VERSIONS.join(", ") ); } @@ -626,7 +627,7 @@ impl ChildProcessMcpClient { &server_name, "initialize", json!({ - "protocolVersion": PROTOCOL_VERSION, + "protocolVersion": MCP_PROTOCOL_VERSION, "clientInfo": { "name": "codewhale-mcp-server", "version": env!("CARGO_PKG_VERSION") @@ -1192,7 +1193,7 @@ mod tests { #[test] fn initialize_result_requires_supported_protocol_and_server_identity() { let valid = json!({ - "protocolVersion": PROTOCOL_VERSION, + "protocolVersion": MCP_PROTOCOL_VERSION, "serverInfo": {"name": "fixture", "version": "1"}, "capabilities": {"tools": {}, "resources": {}} }); @@ -1202,6 +1203,20 @@ mod tests { assert!(capabilities.tools); assert!(capabilities.resources); + // Negotiation accepts every dated revision still implemented, not only + // the newest one advertised at initialize. + for version in ["2025-03-26", "2024-11-05"] { + let older = json!({ + "protocolVersion": version, + "serverInfo": {"name": "fixture", "version": "1"}, + "capabilities": {"tools": {}} + }); + assert!( + validate_initialize_result("fixture", &older).is_ok(), + "supported revision {version} was rejected" + ); + } + for invalid in [ json!({}), json!({ @@ -1209,11 +1224,11 @@ mod tests { "serverInfo": {"name": "fixture", "version": "1"} }), json!({ - "protocolVersion": PROTOCOL_VERSION, + "protocolVersion": MCP_PROTOCOL_VERSION, "serverInfo": {"name": "fixture"} }), json!({ - "protocolVersion": PROTOCOL_VERSION, + "protocolVersion": MCP_PROTOCOL_VERSION, "serverInfo": {"name": "fixture", "version": "1"}, "capabilities": [] }), diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index abf942a085..bd0641021c 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -85,6 +85,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- MCP protocol negotiation: every surface advertised the original 2024-11-05 + revision and the stdio client required an exact match, so newer servers + could not connect. The server and both clients now advertise 2025-06-18 + and negotiate over the supported set (2025-06-18, 2025-03-26, 2024-11-05) + — the server echoes the client's revision when it is supported and answers + with the latest otherwise, the stdio client accepts any supported revision, + and streamable HTTP sends the required `MCP-Protocol-Version` header on + every post-initialize request (#6280, first half). - Configured MCP servers now connect lazily instead of all at session boot. The pool owns a `connecting` set marked at spawn and cleared on resolution or abort, so "connecting" is no longer inferred as enabled-minus-connected. The @@ -114,6 +122,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `` suggestions stop nagging: a plugin id is now + injected at most once per engine lifetime, and a plugin whose name a + loaded skill already covers is never suggested — the local skill owns + the domain, so the nudge was noise. Dismissals still apply, and the + fragment stays append-only on the user turn (#6274). - A canceled automation run now settles with a transcript receipt that names the cancellation (by request, cancel timeout, or shutdown) instead of vanishing from the live band silently. The receipt wears attention ink and diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 99895ae3b5..51ac1137d7 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -873,6 +873,12 @@ pub struct Engine { mcp_event_generation: u64, /// Workspace-scoped immutable plugin catalogue and authority receipts. plugin_registry: Arc, + /// Keeps the append-only `` fragment once-per- + /// Engine-lifetime per plugin id, and suppresses plugins whose name a + /// catalogue skill already covers (#6274). The skill-name snapshot is + /// taken at construction from the same catalogue the system prompt + /// indexes (see the gate's known-limitations note). + recommended_plugin_gate: StdMutex, api_provider: ApiProvider, /// Exact configured route key. Named custom providers share the `Custom` /// enum, so the enum alone cannot prove that the active client is current. @@ -1674,6 +1680,28 @@ impl Engine { // `run_turn` restarts it per turn; this initial value only matters // for hosts that inspect the engine before the first turn. let turn_wall_clock_budget = config.turn_wall_clock; + // Skill-name snapshot for the plugin-suggestion gate (#6274): the + // SAME catalogue the system prompt indexes (prompts.rs skills block — + // workspace roots + configured skills_dir + plugin-sourced skills), + // so suppression sees everything the session actually has. + let gate_skill_names: std::collections::BTreeSet = + crate::skills::discover_for_workspace_and_dir_with_mode_and_plugins( + &config.workspace, + &config.skills_dir, + crate::skills::SkillDiscoveryMode::from_codewhale_only( + config.skills_scan_codewhale_only, + ), + Some(plugin_registry.as_ref()), + ) + .list() + .iter() + .flat_map(|skill| { + std::iter::once(skill.name.clone()).chain(skill.aliases.iter().cloned()) + }) + .map(|name| name.trim().to_ascii_lowercase()) + .filter(|name| !name.is_empty()) + .collect(); + let engine = Engine { config, api_config: api_config.clone(), @@ -1698,6 +1726,11 @@ impl Engine { mcp_boot_generation: None, mcp_event_generation: 0, plugin_registry, + recommended_plugin_gate: StdMutex::new( + crate::plugins::recommend::RecommendedPluginGate::with_skill_names( + gate_skill_names, + ), + ), api_provider, api_provider_identity, api_provider_id, @@ -3708,13 +3741,20 @@ impl Engine { cache_control: None, }]; } - let recommended_plugins = crate::plugins::recommend::recommended_plugins_user_fragment( - &text, - self.plugin_registry.as_ref(), - &crate::plugins::recommend::load_marketplace_candidates( - self.plugin_registry.state_path(), - ), - ); + let recommended_plugins = { + let mut recommended_plugin_gate = self + .recommended_plugin_gate + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + crate::plugins::recommend::recommended_plugins_user_fragment( + &text, + self.plugin_registry.as_ref(), + &crate::plugins::recommend::load_marketplace_candidates( + self.plugin_registry.state_path(), + ), + &mut recommended_plugin_gate, + ) + }; let expanded = crate::image_attach::expand_attachment_blocks(&text); let mut content = Vec::with_capacity(3 + expanded.blocks.len()); content.push(ContentBlock::Text { diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index 20ec7baf35..7cf50fd024 100644 --- a/crates/tui/src/mcp.rs +++ b/crates/tui/src/mcp.rs @@ -45,6 +45,14 @@ use crate::utils::write_atomic; /// Bytes of a non-2xx response body to surface in connection errors. const ERROR_BODY_PREVIEW_BYTES: usize = 200; +/// Newest dated MCP protocol revision Codewhale advertises at `initialize` and +/// answers as an MCP server. Matches the shared MCP crate (`crates/mcp`). +pub(crate) const MCP_PROTOCOL_VERSION: &str = "2025-06-18"; +/// Dated MCP revisions accepted during negotiation, newest first. A peer +/// answering or requesting any of these continues the handshake. +pub(crate) const MCP_SUPPORTED_PROTOCOL_VERSIONS: &[&str] = + &[MCP_PROTOCOL_VERSION, "2025-03-26", "2024-11-05"]; + fn validate_mcp_config_path(path: &Path) -> Result<()> { if path.as_os_str().is_empty() { anyhow::bail!("MCP config path cannot be empty"); @@ -1425,6 +1433,12 @@ pub trait McpTransport: Send + Sync { async fn send(&mut self, msg: Vec) -> Result<()>; async fn recv(&mut self) -> Result>; + /// Record the protocol revision negotiated at `initialize`. Only the + /// Streamable HTTP transport uses it (the `MCP-Protocol-Version` header on + /// subsequent requests); stdio and legacy SSE have no header channel, so + /// the default is a no-op. + fn set_protocol_version(&mut self, _version: &str) {} + /// Synchronous, best-effort liveness probe consulted by /// [`McpConnection::is_ready`] so a crashed stdio child stops reading /// as "ready" before the next call fails (#6187). Must never block and @@ -1831,7 +1845,7 @@ impl McpConnection { "id": &init_id, "method": "initialize", "params": { - "protocolVersion": "2024-11-05", + "protocolVersion": MCP_PROTOCOL_VERSION, "clientInfo": { "name": "codewhale-tui", "version": env!("CARGO_PKG_VERSION") @@ -1846,11 +1860,30 @@ impl McpConnection { .await?; let response = self.recv(init_id).await?; - response_result( + let result = response_result( &response, "initialize", self.config.reviewed_plugin.is_some(), )?; + // Per spec, a server that cannot speak the advertised revision answers + // with one it does support. Accept any dated revision we still + // implement; anything else ends the handshake. + let negotiated = result + .and_then(|result| result.get("protocolVersion")) + .and_then(|version| version.as_str()) + .ok_or_else(|| { + anyhow::anyhow!( + "MCP server '{}' initialize result omitted protocolVersion", + self.name + ) + })?; + anyhow::ensure!( + MCP_SUPPORTED_PROTOCOL_VERSIONS.contains(&negotiated), + "MCP server '{}' negotiated unsupported protocol version '{negotiated}' (supported: {})", + self.name, + MCP_SUPPORTED_PROTOCOL_VERSIONS.join(", ") + ); + self.transport.set_protocol_version(negotiated); self.server_capabilities = McpServerCapabilities::from_initialize_response(&response); // Send initialized notification (no id, no response expected) diff --git a/crates/tui/src/mcp/http.rs b/crates/tui/src/mcp/http.rs index afdb576472..5d23973e85 100644 --- a/crates/tui/src/mcp/http.rs +++ b/crates/tui/src/mcp/http.rs @@ -250,6 +250,14 @@ impl HttpTransport { #[async_trait::async_trait] impl McpTransport for HttpTransport { + fn set_protocol_version(&mut self, version: &str) { + // Only Streamable HTTP carries the MCP-Protocol-Version header; the + // legacy SSE transport predates it and ignores the negotiation result. + if let HttpTransportMode::Streamable(transport) = &mut self.mode { + transport.set_protocol_version(version); + } + } + async fn send(&mut self, msg: Vec) -> Result<()> { match &mut self.mode { HttpTransportMode::Streamable(transport) => match transport.send(msg.clone()).await { diff --git a/crates/tui/src/mcp/streamable_http.rs b/crates/tui/src/mcp/streamable_http.rs index 05afdac61b..f8fea5e76f 100644 --- a/crates/tui/src/mcp/streamable_http.rs +++ b/crates/tui/src/mcp/streamable_http.rs @@ -21,6 +21,11 @@ pub(super) struct StreamableHttpTransport { /// request so the server can correlate messages within the same /// session. pub(super) session_id: Option, + /// Protocol revision negotiated at `initialize`. Attached as the + /// `MCP-Protocol-Version` header on every subsequent outbound request + /// per the Streamable HTTP spec (absent means the server assumes + /// the 2025-03-26 default, so the negotiated value is always sent). + protocol_version: Option, } #[derive(Debug)] @@ -38,9 +43,14 @@ impl StreamableHttpTransport { auth, pending_messages: VecDeque::new(), session_id: None, + protocol_version: None, } } + pub(super) fn set_protocol_version(&mut self, version: &str) { + self.protocol_version = Some(version.to_string()); + } + pub(super) async fn send( &mut self, msg: Vec, @@ -68,6 +78,11 @@ impl StreamableHttpTransport { if let Some(ref sid) = self.session_id { request = request.header("Mcp-Session-Id", sid.as_str()); } + // Per the Streamable HTTP spec, subsequent requests carry the + // negotiated revision; absent means the server assumes 2025-03-26. + if let Some(ref version) = self.protocol_version { + request = request.header("MCP-Protocol-Version", version.as_str()); + } let response = self .client .send(request.body(msg.clone())) diff --git a/crates/tui/src/mcp_server.rs b/crates/tui/src/mcp_server.rs index d0fb33279e..67668369aa 100644 --- a/crates/tui/src/mcp_server.rs +++ b/crates/tui/src/mcp_server.rs @@ -143,7 +143,14 @@ impl McpServer { let id = message.get("id").cloned(); match method { - "initialize" => respond(id.as_ref(), initialize_response()), + "initialize" => respond( + id.as_ref(), + initialize_response( + message + .pointer("/params/protocolVersion") + .and_then(Value::as_str), + ), + ), "tools/list" => respond(id.as_ref(), self.list_tools_response()), "tools/call" => { let params = message.get("params").cloned().unwrap_or_else(|| json!({})); @@ -312,9 +319,15 @@ fn tool_result_to_mcp(result: Result) -> Value { } } -fn initialize_response() -> Value { +fn initialize_response(requested: Option<&str>) -> Value { + // Per spec, echo the requested revision when we support it; otherwise + // answer with the newest revision we do support and let the client decide. + let negotiated = match requested { + Some(version) if crate::mcp::MCP_SUPPORTED_PROTOCOL_VERSIONS.contains(&version) => version, + _ => crate::mcp::MCP_PROTOCOL_VERSION, + }; json!({ - "protocolVersion": "2024-11-05", + "protocolVersion": negotiated, "serverInfo": { "name": "codewhale-mcp-server", "version": env!("CARGO_PKG_VERSION"), @@ -445,10 +458,30 @@ mod tests { #[test] fn initialize_uses_standard_mcp_shape_and_codewhale_identity() { - let response = initialize_response(); - assert_eq!(response["protocolVersion"], "2024-11-05"); + let response = initialize_response(Some(crate::mcp::MCP_PROTOCOL_VERSION)); + assert_eq!( + response["protocolVersion"], + crate::mcp::MCP_PROTOCOL_VERSION + ); assert_eq!(response["serverInfo"]["name"], "codewhale-mcp-server"); assert_eq!(response["serverInfo"]["version"], env!("CARGO_PKG_VERSION")); assert!(response["capabilities"]["tools"].is_object()); } + + #[test] + fn initialize_negotiates_supported_revisions() { + // A client asking for an older dated revision gets it echoed back; + // an unknown or missing revision answers with the newest supported. + for requested in ["2025-03-26", "2024-11-05"] { + let response = initialize_response(Some(requested)); + assert_eq!(response["protocolVersion"], requested); + } + for requested in [Some("2099-01-01"), None] { + let response = initialize_response(requested); + assert_eq!( + response["protocolVersion"], + crate::mcp::MCP_PROTOCOL_VERSION + ); + } + } } diff --git a/crates/tui/src/plugins/recommend.rs b/crates/tui/src/plugins/recommend.rs index e55058cc99..ffe3aa6ee4 100644 --- a/crates/tui/src/plugins/recommend.rs +++ b/crates/tui/src/plugins/recommend.rs @@ -259,6 +259,51 @@ pub fn match_plugin_for_draft_among( Some(matched) } +/// Per-Engine gate for the append-only `` fragment. +/// +/// A plugin id is suggested at most once per Engine lifetime, and a plugin +/// whose name (or alias) matches a skill in the session's catalogue is never +/// suggested — the skill already covers the domain, so the nudge is noise +/// (#6274). Dismissals continue to be honored through `Settings`. +/// +/// Known limitation: the skill-name set is snapshotted once at Engine +/// construction (from the same catalogue the system prompt indexes), so a +/// skill installed mid-session does not suppress its plugin twin until the +/// next Engine starts. +#[derive(Debug, Default)] +pub struct RecommendedPluginGate { + shown: BTreeSet, + skill_names: BTreeSet, +} + +impl RecommendedPluginGate { + /// Engine constructor input and test seam: suppress exactly these + /// skill names and aliases (case is normalized here, so callers may + /// pass them in any form). + #[must_use] + pub fn with_skill_names(skill_names: BTreeSet) -> Self { + Self { + shown: BTreeSet::new(), + skill_names: skill_names + .into_iter() + .map(|name| name.trim().to_ascii_lowercase()) + .filter(|name| !name.is_empty()) + .collect(), + } + } + + /// True when this plugin may be suggested now: not covered by a + /// catalogue skill and not already suggested in this Engine's lifetime. + /// First admission records the plugin id. + fn admits(&mut self, id: &str, name: &str) -> bool { + let name_key = name.trim().to_ascii_lowercase(); + if !name_key.is_empty() && self.skill_names.contains(&name_key) { + return false; + } + self.shown.insert(id.to_string()) + } +} + /// Append-only user-turn fragment. Never part of the pinned system prefix. /// Bounded, omitted when nothing matches. #[must_use] @@ -266,6 +311,7 @@ pub fn recommended_plugins_user_fragment( draft: &str, registry: &PluginRegistry, marketplace: &[MarketplaceCandidate], + gate: &mut RecommendedPluginGate, ) -> Option { // Called once when composing a user turn, never from the render loop. // Read the shared preference so headless and long-lived Engines also @@ -277,6 +323,11 @@ pub fn recommended_plugins_user_fragment( marketplace, &settings.dismissed_plugin_suggestions, )?; + // Once per Engine lifetime per plugin id, and never when a local skill + // already covers the plugin's domain (#6274). + if !gate.admits(&matched.id, &matched.name) { + return None; + } let mut listed = vec![matched]; listed.truncate(MAX_RECOMMENDED_PLUGINS); let body = listed @@ -650,14 +701,77 @@ mod tests { let registry = crate::plugins::PluginDiscoveryContext::capture_pre_dotenv() .registry_for_workspace(root.path()); - let fragment = - recommended_plugins_user_fragment("add supabase auth to login", ®istry, &[]) - .expect("idle plugin should produce a fragment"); + let fragment = recommended_plugins_user_fragment( + "add supabase auth to login", + ®istry, + &[], + &mut RecommendedPluginGate::default(), + ) + .expect("idle plugin should produce a fragment"); assert!(fragment.starts_with("")); assert!(fragment.contains("- supabase (")); assert!(fragment.contains("")); assert!( - recommended_plugins_user_fragment("fix the failing test", ®istry, &[]).is_none() + recommended_plugins_user_fragment( + "fix the failing test", + ®istry, + &[], + &mut RecommendedPluginGate::default(), + ) + .is_none() + ); + } + + #[test] + fn recommended_plugins_fragment_suggests_a_plugin_once_per_gate() { + let _lock = lock_test_env(); + let root = TempDir::new().unwrap(); + let _home = EnvVarGuard::set("CODEWHALE_HOME", root.path().join("home")); + write_keyword_bundle(root.path(), "supabase", "Hosted Postgres", &["supabase"]); + let registry = crate::plugins::PluginDiscoveryContext::capture_pre_dotenv() + .registry_for_workspace(root.path()); + + let mut gate = RecommendedPluginGate::default(); + let first = recommended_plugins_user_fragment( + "add supabase auth to login", + ®istry, + &[], + &mut gate, + ) + .expect("first matching turn suggests the plugin"); + assert!(first.contains("- supabase (")); + assert!( + recommended_plugins_user_fragment( + "add supabase auth to the signup flow", + ®istry, + &[], + &mut gate, + ) + .is_none(), + "a plugin id is suggested at most once per Engine lifetime (#6274)" + ); + } + + #[test] + fn recommended_plugins_fragment_suppressed_by_local_skill_name() { + let _lock = lock_test_env(); + let root = TempDir::new().unwrap(); + let _home = EnvVarGuard::set("CODEWHALE_HOME", root.path().join("home")); + write_keyword_bundle(root.path(), "supabase", "Hosted Postgres", &["supabase"]); + let registry = crate::plugins::PluginDiscoveryContext::capture_pre_dotenv() + .registry_for_workspace(root.path()); + + let skills: BTreeSet = ["Supabase".to_string()].into_iter().collect(); + let mut gate = RecommendedPluginGate::with_skill_names(skills); + assert!( + recommended_plugins_user_fragment( + "add supabase auth to login", + ®istry, + &[], + &mut gate, + ) + .is_none(), + "a loaded local skill covering the plugin name must suppress the suggestion (#6274)" ); } diff --git a/crates/tui/src/tui/plugin_suggestions.rs b/crates/tui/src/tui/plugin_suggestions.rs index 7e1e5f59cd..4dc6c5c2a8 100644 --- a/crates/tui/src/tui/plugin_suggestions.rs +++ b/crates/tui/src/tui/plugin_suggestions.rs @@ -510,6 +510,7 @@ mod tests { &app.input, restarted.plugin_registry.as_ref(), &[], + &mut crate::plugins::recommend::RecommendedPluginGate::default(), ) .is_none() ); diff --git a/docs/PLUGIN_AUTHORING.md b/docs/PLUGIN_AUTHORING.md index 84f31634f6..583df921a3 100644 --- a/docs/PLUGIN_AUTHORING.md +++ b/docs/PLUGIN_AUTHORING.md @@ -232,6 +232,27 @@ python3 scripts/convert-plugin.py --format dsh \ The DSH input may also be JSON, but must be the plain entry list, not a full profile or patch composition. Each row must name `@deepseek-ai/dsh-mcp-client`. +A real dsh bundle package — an npm package whose `package.json` declares +`dsh.bundle.patch` — converts directly with `--bundle`: + +```sh +python3 scripts/convert-plugin.py --format dsh \ + --bundle ./node_modules/@demo/tools-dsh --name migrated-dsh --output ./migrated-dsh +``` + +The converter reads the package's `cordis.patch.yml`, applies its `insert` and +keyed-override operations over an empty profile (matching `applyEntryPatches`), +and converts each resulting row. Rows it cannot represent — runtime plugins, +`dsh.client` UI code, `!!js` expressions outside the documented idioms, +conditional `disabled` flags — are listed in `CONVERSION.md` rather than +silently dropped. The `!!js` idioms it does lower: `process.execPath` (becomes +`node`), `process.env.NAME` and `process.env.NAME || 'literal'` (resolved +against this machine), and `` `${process.env.NAME}...` `` templates. An `args` +entry that resolves to a host file is snapshotted: its containing directory is +copied into `mcp/` and the resolution is recorded in the receipt. Rows +of `@deepseek-ai/dsh-skill-filesystem` contribute their `customSkillDirs` +children as skills when those directories live inside the package. + ### Local Node MCP servers For an already packaged Node MCP server, select its original process working diff --git a/scripts/convert-plugin.py b/scripts/convert-plugin.py index 8e4098abf4..14f183defe 100644 --- a/scripts/convert-plugin.py +++ b/scripts/convert-plugin.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Convert selected OpenCode/DSH data into a reviewable native plugin bundle. +"""Convert selected OpenCode/DSH data — or a dsh bundle package — into a reviewable native plugin bundle. This is an offline authoring tool, not a foreign plugin runtime or installer. The existing Codewhale /plugin install and hash-bound review remain authoritative. @@ -29,6 +29,10 @@ class ConversionError(ValueError): pass +class JsExpr(str): + """An unevaluated dsh `!!js` scalar captured for structural lowering; it is never executed.""" + + def require(condition, message): if not condition: raise ConversionError(message) @@ -56,7 +60,14 @@ def construct_mapping(self, node, deep=False): for k, v in node.value) -def data(text, *, json_only=False): +def js_scalar(loader, node): + return JsExpr(loader.construct_scalar(node)) + + +DataLoader.add_constructor("tag:yaml.org,2002:js", js_scalar) + + +def data(text, *, json_only=False, allow_js=False): """Closed data parsing: no YAML aliases/tags, duplicate keys or JS expressions.""" try: if json_only: @@ -65,40 +76,48 @@ def data(text, *, json_only=False): else: depth = 0 for event in yaml.parse(text): - require(not isinstance(event, yaml.AliasEvent) and not getattr(event, "tag", None), - "YAML aliases and explicit tags (including !!js) are unsupported.") + tag = getattr(event, "tag", None) + if isinstance(event, yaml.AliasEvent): + raise ConversionError("YAML aliases are unsupported.") + if tag is not None and not (allow_js and isinstance(event, yaml.ScalarEvent) + and tag == "tag:yaml.org,2002:js"): + raise ConversionError("YAML aliases and explicit tags (including !!js) are unsupported.") if isinstance(event, (yaml.MappingStartEvent, yaml.SequenceStartEvent)): depth += 1 require(depth <= 32, "Configuration nesting exceeds 32 levels.") elif isinstance(event, (yaml.MappingEndEvent, yaml.SequenceEndEvent)): depth -= 1 value = yaml.load(text, Loader=DataLoader) - check_data(value) + check_data(value, allow_js=allow_js) return value except (yaml.YAMLError, json.JSONDecodeError, RecursionError, TypeError): # Parser errors can contain source lines and credentials. Do not echo them. raise ConversionError("Cannot parse portable data; use JSON for OpenCode or plain YAML/JSON for DSH.") from None -def check_data(value, depth=0): +def check_data(value, depth=0, allow_js=False): require(depth <= 32, "Configuration nesting exceeds 32 levels.") if isinstance(value, dict): mapping(value) require("__jsExpr" not in value, "DSH executable expressions require a manual port.") for child in value.values(): - check_data(child, depth + 1) + check_data(child, depth + 1, allow_js) elif isinstance(value, list): for child in value: - check_data(child, depth + 1) + check_data(child, depth + 1, allow_js) else: - require(value is None or type(value) in (str, int, float, bool), "Unsupported data type.") + require(value is None or type(value) in (str, int, float, bool) + or (allow_js and isinstance(value, JsExpr)), "Unsupported data type.") def plain_path(path): """Reject links/reparse points in the supplied path, including ancestors.""" path = Path(os.path.abspath(path)) for entry in (path, *path.parents): - info = entry.lstat() + try: + info = entry.lstat() + except FileNotFoundError: + continue require(not stat.S_ISLNK(info.st_mode) and not (getattr(info, "st_file_attributes", 0) & 0x400), "Source and output paths must not contain links or reparse points.") @@ -372,20 +391,260 @@ def mcp_config(path, dialect, stdio_roots=None): return result, sorted(hosts), ignored +DSH_MCP_CLIENT = "@deepseek-ai/dsh-mcp-client" +DSH_SKILL_FILESYSTEM = "@deepseek-ai/dsh-skill-filesystem" +DSH_ENTRY = re.compile(r"(?:\./)?[A-Za-z0-9_][A-Za-z0-9_./-]*\.(?:mjs|js|cjs)") +DSH_ENV_NAME = r"[A-Za-z_][A-Za-z0-9_]*" + + +def js_literal(text, label): + """Resolve a quoted string or template literal inside a `!!js` idiom; env references + resolve from this machine because a bundle cannot name the original author's value.""" + text = text.strip() + if len(text) >= 2 and text[0] in "\"'" and text[-1] == text[0] and text[0] not in text[1:-1]: + return text[1:-1] + if text.startswith("`") and text.endswith("`") and len(text) >= 2: + body = text[1:-1] + resolved = re.sub(r"\$\{\s*process\.env\.(" + DSH_ENV_NAME + r")\s*\}", + lambda match: os.environ.get(match[1], ""), body) + missing = [name for name in re.findall(r"\$\{\s*process\.env\.(" + DSH_ENV_NAME + r")\s*\}", body) + if name not in os.environ] + if missing: + raise ConversionError(f"{label} references unset environment variable {missing[0]}.") + require("${" not in resolved, f"{label} interpolates an expression with no portable lowering.") + return resolved + raise ConversionError(f"{label} is not a quoted or template literal; author the value explicitly.") + + +def lower_js(value, label): + """Lower the documented `!!js` idioms to a literal string; every other expression refuses.""" + text = str(value).strip() + if text == "process.execPath": + return "node" + match = re.fullmatch(r"process\.env\.(" + DSH_ENV_NAME + r")", text) + if match: + resolved = os.environ.get(match[1]) + require(resolved is not None, f"{label} references environment variable {match[1]}, which is not set here.") + return resolved + match = re.fullmatch(r"process\.env\.(" + DSH_ENV_NAME + r")\s*\|\|\s*(.+)", text, re.DOTALL) + if match: + resolved = os.environ.get(match[1]) + if resolved is not None: + return resolved + return js_literal(match[2], f"{label} fallback") + if text.startswith("`"): + return js_literal(text, label) + raise ConversionError(f"{label} uses a `!!js` expression with no portable lowering; author the value explicitly.") + + +def free_of_js(value): + """True when no unevaluated `!!js` scalar survives inside the value.""" + if isinstance(value, JsExpr): + return False + if isinstance(value, dict): + return all(free_of_js(child) for child in value.values()) + if isinstance(value, list): + return all(free_of_js(child) for child in value) + return True + + +def evaluate_patches(patches, notes): + """Apply a dsh bundle patch list over an empty entry list (applyEntryPatches parity): + `insert` appends rows or appends into a group entry's config, keyed overrides + replace fields on an earlier inserted row. Skipped patches are recorded, never fatal.""" + entries, index = [], {} + def build_map(rows): + for row in rows: + if not isinstance(row, dict): + continue + identifier = row.get("id") + if isinstance(identifier, str): + index[identifier] = row + config = row.get("config") + if row.get("group") is True and isinstance(config, list): + build_map(config) + for order, patch in enumerate(patches): + require(isinstance(patch, dict), "Each dsh patch must be an object.") + insert, identifier = patch.get("insert"), patch.get("id") + if insert is not None: + require(isinstance(insert, list) and all(isinstance(row, dict) for row in insert), + "A dsh patch `insert` must be a list of entries.") + if identifier is None: + entries.extend(insert) + else: + target = index.get(identifier) + if target is None or target.get("group") is not True: + notes.append(f"patch {order + 1}: insert target `{identifier}` is missing or not a group; skipped") + continue + if not isinstance(target.get("config"), list): + target["config"] = [] + target["config"].extend(insert) + build_map(insert) + continue + if not isinstance(identifier, str): + notes.append(f"patch {order + 1}: non-insert patch without an `id`; skipped") + continue + target = index.get(identifier) + if target is None: + notes.append(f"patch {order + 1}: entry `{identifier}` was not inserted by an earlier layer; skipped") + continue + name = patch.get("name") + if name is not None and name != target.get("name"): + notes.append(f"patch {order + 1}: `name` does not match entry `{identifier}`; skipped") + continue + for key, value in patch.items(): + if key not in ("id", "name"): + target[key] = value + return entries + + +def load_dsh_bundle(path): + """Read a dsh bundle package directory: package.json → dsh.bundle.patch → evaluated rows.""" + bundle = plain_path(path) + require(bundle.is_dir(), "Select a dsh bundle package directory (a directory containing package.json).") + manifest = data(text_file(bundle / "package.json"), json_only=True) + mapping(manifest) + dsh = manifest.get("dsh") + require(isinstance(dsh, dict) and isinstance(dsh.get("bundle"), dict), + "Not a dsh bundle package: package.json lacks `dsh.bundle.patch`.") + notes = [] + if dsh.get("client") is not None: + notes.append("package declares `dsh.client`; the client UI half has no Codewhale equivalent and was not converted") + patch_rel = dsh["bundle"].get("patch") + require(isinstance(patch_rel, str) and bool(patch_rel), "`dsh.bundle.patch` must name a patch file.") + patch_path = plain_path(bundle / patch_rel) + require(patch_path.is_relative_to(bundle) and patch_path.is_file(), + "`dsh.bundle.patch` must resolve to a file inside the bundle directory.") + patches = data(text_file(patch_path), allow_js=True) + require(isinstance(patches, list), "A dsh bundle patch must be a patch list.") + return manifest, evaluate_patches(patches, notes), notes + + +def dsh_bundle_components(entries, bundle, explicit_roots): + """Convert evaluated dsh entries into Codewhale servers + skill sources. + Unconvertible rows are recorded as skipped diagnostics, never silently dropped.""" + servers, hosts, notes = {}, [], [] + implicit_roots = {} + skill_dirs = [] + + def label(row): + identifier = row.get("id") + return f"`{identifier}`" if isinstance(identifier, str) else "an unlabeled row" + + def mcp_row(row): + config = mapping(row.get("config")) + name = config.get("serverName") + require(isinstance(name, str) and re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9_-]{0,31}", name), + "dsh-mcp-client config needs a literal `serverName` of 1–32 letters/digits/_/-") + require(name not in servers, f"Duplicate MCP server name `{name}`; nothing was written for it.") + disabled = row.get("disabled", False) + require(type(disabled) is bool, "`disabled` must be a boolean; conditional rows need a manual port") + for field in ("command", "cwd", "url", "serverName"): + if isinstance(config.get(field), JsExpr): + config[field] = lower_js(config[field], f"`{field}` in `{name}`") + if isinstance(config.get("args"), list): + config["args"] = [lower_js(item, f"`args` in `{name}`") if isinstance(item, JsExpr) else item + for item in config["args"]] + arguments = config.get("args") or [] + root = explicit_roots.get(name) + if config.get("transport") == "stdio" and root is None and isinstance(arguments, list) and len(arguments) == 1: + arg = arguments[0] + if isinstance(arg, str) and not DSH_ENTRY.fullmatch(arg): + candidate = Path(arg) + if candidate.is_absolute(): + resolved = plain_path(candidate) + require(resolved.is_file(), f"`args` in `{name}` resolves to a host path that does not exist here") + require(DSH_ENTRY.fullmatch(resolved.name) is not None, + f"`args` in `{name}` resolves to a file that is not a .mjs/.js/.cjs entry") + implicit_roots[name] = resolved.parent + config["args"] = [resolved.name] + notes.append(f"`{name}`: `!!js`/`args` resolved to host path; copied {resolved.parent} as its source root") + root = resolved.parent + elif isinstance(arg, str) and ".." not in arg.split("/"): + candidate = bundle / arg + cwd = config.get("cwd", "") + if candidate.is_file(): + implicit_roots[name] = bundle + root = bundle + elif (isinstance(cwd, str) and cwd not in ("", ".") and ".." not in Path(cwd).parts + and not Path(cwd).is_absolute() and (bundle / cwd / arg).is_file()): + implicit_roots[name] = bundle / cwd + root = bundle / cwd + config["cwd"] = "." + require(free_of_js(config), f"an unevaluated `!!js` remains in `{name}`; author that field explicitly") + local = config.get("transport") == "stdio" + if local: + converted = stdio_server(config, "dsh", {}, name, root or implicit_roots.get(name)) + else: + converted, host = remote_server(config, "dsh", {}) + hosts.append(host) + if disabled: + converted["extensions"]["net.codewhale"]["disabled"] = True + servers[name] = converted + + def walk(rows): + for row in rows: + if not isinstance(row, dict): + continue + config = row.get("config") + if row.get("group") is True and isinstance(config, list): + walk(config) + continue + name = row.get("name") + if isinstance(row.get("disabled"), JsExpr): + notes.append(f"{label(row)} skipped: `disabled` is a `!!js` expression that cannot be evaluated offline") + continue + if name == DSH_MCP_CLIENT: + try: + mcp_row(row) + except ConversionError as reason: + notes.append(f"{label(row)} skipped: {reason}") + elif name == DSH_SKILL_FILESYSTEM: + dirs = config.get("customSkillDirs") if isinstance(config, dict) else None + if not isinstance(dirs, list) or not dirs: + notes.append(f"{label(row)} skipped: skill row has no `customSkillDirs` to import") + continue + for entry in dirs: + if isinstance(entry, JsExpr) or not isinstance(entry, str): + notes.append(f"{label(row)}: a `customSkillDirs` entry is not a literal path; skipped") + continue + candidate = Path(entry) + if candidate.is_absolute() or ".." in candidate.parts: + notes.append(f"{label(row)}: `customSkillDirs` entry `{entry}` is outside the bundle; " + "pass it explicitly with --skill") + continue + resolved = bundle / entry + if not resolved.is_dir(): + notes.append(f"{label(row)}: `customSkillDirs` entry `{entry}` does not exist in the bundle; skipped") + continue + skill_dirs.append(resolved) + else: + shown = name if isinstance(name, str) else "unlabeled" + notes.append(f"{label(row)} ({shown}) skipped: only dsh-mcp-client and dsh-skill-filesystem rows convert") + + walk(entries) + skills = [] + for directory in skill_dirs: + for child in sorted(directory.iterdir()): + if child.name.startswith("."): + continue + if (child / "SKILL.md").is_file() or child.suffix == ".md": + skills.append(child) + return servers, hosts, skills, implicit_roots, notes + + def convert(args): require(NAME.fullmatch(args.name) is not None and ".." not in args.name and "--" not in args.name, "Choose a native plugin name: 1–64 lowercase letters/digits with single internal dots or hyphens.") + bundle_arg = getattr(args, "bundle", None) + require(bundle_arg is None or args.format == "dsh", "--bundle reads a DeepSeek Harness bundle package; use --format dsh.") + require(not (bundle_arg is not None and args.config), "Select --bundle or --config, not both.") output = Path(os.path.abspath(args.output)) plain_path(output.parent) require(not os.path.lexists(output), "Output already exists; choose a fresh directory. Nothing was overwritten.") - files, skill_names = {}, set() - for path in args.skill: - source = plain_path(path) - require(source != output and source not in output.parents, "Output must be outside the selected skill.") - name, additions = skill_files(source, MAX_FILES - len(files), MAX_BYTES - sum(map(len, files.values()))) - require(name not in skill_names, "Duplicate skill name; no files were written.") - skill_names.add(name) - files.update(additions) + files, skill_names, notes = {}, set(), [] + skill_sources = [plain_path(path) for path in args.skill] + servers, hosts, ignored = {}, [], 0 roots = {} for specification in getattr(args, "stdio_root", []): name, separator, directory = specification.partition("=") @@ -394,15 +653,42 @@ def convert(args): require(source.is_dir(), "The selected stdio root must be a directory.") require(source != output and source not in output.parents, "Output must be outside the selected MCP source.") roots[name] = source - require(not roots or args.config, "--stdio-root requires a selected MCP configuration.") - servers, hosts, ignored = mcp_config(args.config, args.format, roots) if args.config else ({}, [], 0) + bundle_manifest = None + if bundle_arg is not None: + bundle_manifest, entries, bundle_notes = load_dsh_bundle(bundle_arg) + notes += bundle_notes + bundle = plain_path(bundle_arg) + require(bundle != output and bundle not in output.parents, "Output must be outside the selected bundle.") + servers, hosts, bundled_skills, implicit_roots, row_notes = dsh_bundle_components(entries, bundle, roots) + notes += row_notes + skill_sources += bundled_skills + implicit_roots.update(roots) + explicit_names = set(roots) + roots = implicit_roots + require(explicit_names <= set(servers), "Every --stdio-root must name a selected local MCP server.") + else: + require(not roots or args.config, "--stdio-root requires a selected MCP configuration.") + servers, hosts, ignored = mcp_config(args.config, args.format, roots) if args.config else ({}, [], 0) + for source in skill_sources: + require(source != output and source not in output.parents, "Output must be outside the selected skill.") + name, additions = skill_files(source, MAX_FILES - len(files), MAX_BYTES - sum(map(len, files.values()))) + require(name not in skill_names, "Duplicate skill name; no files were written.") + skill_names.add(name) + files.update(additions) for name, source in roots.items(): files.update(stdio_files(source, name, MAX_FILES - len(files), MAX_BYTES - sum(map(len, files.values())))) - require(files or servers, "No portable components selected. Use --skill and/or --config.") + require(files or servers, "No portable components selected. Use --skill, --config or --bundle.") manifest = {"$schema": "https://agent-plugins.org/schemas/plugin.json", "name": args.name} + if bundle_manifest is not None: + for field in ("version", "description"): + value = bundle_manifest.get(field) + if isinstance(value, str) and value.strip(): + manifest[field] = value + notes.insert(0, f"source package: {bundle_manifest.get('name', 'unnamed')}" + + (f"@{bundle_manifest['version']}" if isinstance(bundle_manifest.get('version'), str) else "")) extension = {} if hosts: - extension["capabilities"] = {"network_hosts": hosts} + extension["capabilities"] = {"network_hosts": sorted(set(hosts))} if roots: extension["when"] = {"binaries": ["node"]} if extension: @@ -410,9 +696,11 @@ def convert(args): files["plugin.json"] = (json.dumps(manifest, indent=2) + "\n").encode() if servers: files["mcp.json"] = (json.dumps({"mcpServers": servers}, indent=2) + "\n").encode() + notes_text = ("Bundle diagnostics:\n" + "\n".join(f"- {note}" for note in notes) + "\n\n") if notes else "" files["CONVERSION.md"] = (f"# Conversion receipt\n\nSource dialect: {args.format}.\n" f"Converted {len(skill_names)} selected Skills, {len(servers) - len(roots)} remote and {len(roots)} local MCP declarations.\n" f"Ignored {ignored} unrelated top-level application settings.\n\n" + + notes_text + "No source code, package manager, install hook, network request or credential lookup ran.\n" "Companion skill files were copied as data; review them before loading a skill.\n" "Selected Node source, dependencies and resources were copied as data into mcp/.\n" @@ -452,6 +740,8 @@ def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--format", choices=("opencode-v1", "opencode-v2", "dsh"), required=True) parser.add_argument("--config", type=Path, help="OpenCode JSON or static DSH Cordis YAML/JSON (optional)") + parser.add_argument("--bundle", type=Path, + help="dsh bundle package directory (package.json with dsh.bundle.patch); evaluates the patch layer") parser.add_argument("--skill", type=Path, action="append", default=[], help="Explicit skill directory or Markdown file; repeatable") parser.add_argument("--stdio-root", action="append", default=[], metavar="SERVER=DIRECTORY", help="Explicit packaged Node MCP working directory; repeat for each selected local server") diff --git a/scripts/test_convert_plugin.py b/scripts/test_convert_plugin.py index a12ea369da..6c6dfb4c21 100644 --- a/scripts/test_convert_plugin.py +++ b/scripts/test_convert_plugin.py @@ -65,8 +65,9 @@ def skill(self, name="safe-skill", *, metadata=None, body="Read the local refere (path / "SKILL.md").write_text(text, encoding="utf-8") return path - def args(self, *, config=None, skills=(), dialect="opencode-v1", output=None, name="converted-demo", stdio_roots=()): - return argparse.Namespace(config=config, skill=list(skills), format=dialect, + def args(self, *, config=None, bundle=None, skills=(), dialect="opencode-v1", output=None, + name="converted-demo", stdio_roots=()): + return argparse.Namespace(config=config, bundle=bundle, skill=list(skills), format=dialect, output=output or self.fresh("output"), name=name, stdio_root=list(stdio_roots)) def cli(self, args, *, env=None): @@ -74,6 +75,8 @@ def cli(self, args, *, env=None): "--name", args.name, "--output", str(args.output)] if args.config: command += ["--config", str(args.config)] + if args.bundle: + command += ["--bundle", str(args.bundle)] for skill in args.skill: command += ["--skill", str(skill)] for root in args.stdio_root: @@ -405,6 +408,125 @@ def test_dsh_patch_followed_by_disable_is_not_partially_imported(self): "config": {"url": "https://replacement.example.invalid/mcp"}}] self.refuse(self.args(config=self.config(patches), dialect="dsh")) + def bundle(self, patch_text=None, manifest=None, patch_name="cordis.patch.yml"): + directory = self.fresh("dsh-bundle") + directory.mkdir() + package = {"name": "@demo/tools-dsh", "version": "1.2.3", + "dsh": {"bundle": {"patch": f"./{patch_name}"}}, **(manifest or {})} + (directory / "package.json").write_text(json.dumps(package)) + if patch_text is not None: + (directory / patch_name).write_text(patch_text) + return directory + + def test_dsh_bundle_evaluates_patches_and_skips_foreign_rows(self): + bundle = self.bundle(yaml.safe_dump([ + {"insert": [ + {"id": "docs-entry", "name": "@deepseek-ai/dsh-mcp-client", "config": { + "serverName": "docs", "transport": "streamable-http", + "url": "https://docs.example.invalid/mcp", "toolCallTimeoutMs": 19000}}, + {"id": "skin", "name": "@deepseek-ai/dsh-client-ui-theme", "config": {"hue": 4}}, + ]}, + {"id": "docs-entry", "disabled": True}, + {"id": "ghost", "disabled": True}, + ])) + args = self.args(bundle=bundle, dialect="dsh") + result = self.cli(args) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(self.servers(args.output)["docs"], { + "type": "streamable-http", "url": "https://docs.example.invalid/mcp", + "extensions": {"net.codewhale": {"disabled": True, "execute_timeout": 19}}}) + receipt = (args.output / "CONVERSION.md").read_text() + self.assertIn("@demo/tools-dsh@1.2.3", receipt) + self.assertIn("skin", receipt) + self.assertIn("ghost", receipt) + + def test_dsh_bundle_lowers_js_command_and_host_path_arg(self): + bundle = self.bundle() + server_dir = self.node_source() + patch = ("- insert:\n - id: local-entry\n name: '@deepseek-ai/dsh-mcp-client'\n" + " config:\n serverName: localdocs\n transport: stdio\n" + " command: !!js process.execPath\n" + " args:\n - !!js process.env.CONVERT_TEST_UNSET_ENTRY_7391 || '" + + str(server_dir / "server.mjs") + "'\n") + (bundle / "cordis.patch.yml").write_text(patch) + args = self.args(bundle=bundle, dialect="dsh") + result = self.cli(args) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(self.servers(args.output)["localdocs"], { + "type": "stdio", "command": "node", "args": ["server.mjs"], "cwd": "mcp/localdocs", + "env": {}, "extensions": {"net.codewhale": {}}}) + self.assertEqual((args.output / "mcp/localdocs/server.mjs").read_bytes(), + (server_dir / "server.mjs").read_bytes()) + + def test_dsh_bundle_relative_entry_and_group_children_convert(self): + bundle = self.bundle() + (bundle / "mcp").mkdir() + (bundle / "mcp" / "server.mjs").write_text("// fixture\n") + patch = yaml.safe_dump([ + {"insert": [{"id": "grouped", "group": True, "config": []}]}, + {"id": "grouped", "insert": [ + {"id": "in-group", "name": "@deepseek-ai/dsh-mcp-client", "config": { + "serverName": "inner", "transport": "stdio", "command": "node", + "args": ["server.mjs"], "cwd": "mcp"}}]}, + ]) + (bundle / "cordis.patch.yml").write_text(patch) + args = self.args(bundle=bundle, dialect="dsh") + result = self.cli(args) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(self.servers(args.output)["inner"]["cwd"], "mcp/inner") + self.assertTrue((args.output / "mcp/inner/server.mjs").is_file()) + + def test_dsh_bundle_imports_custom_skill_dirs(self): + bundle = self.bundle() + skill = bundle / "pack-skills" / "guide" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("---\nname: guide\ndescription: Bundled skill\n---\nBody.\n") + patch = yaml.safe_dump([ + {"insert": [ + {"id": "skills-row", "name": "@deepseek-ai/dsh-skill-filesystem", + "config": {"customSkillDirs": ["pack-skills"]}}, + {"id": "mcp", "name": "@deepseek-ai/dsh-mcp-client", "config": { + "serverName": "docs", "transport": "streamable-http", + "url": "https://docs.example.invalid/mcp"}}, + ]}, + ]) + (bundle / "cordis.patch.yml").write_text(patch) + args = self.args(bundle=bundle, dialect="dsh") + self.assertEqual(converter.convert(args), (1, 1, 0)) + self.assertTrue((args.output / "skills/guide/SKILL.md").is_file()) + + def test_dsh_bundle_never_executes_js_and_records_unlowerable_rows(self): + sentinel = self.root / "expression-ran" + bundle = self.bundle() + patch = ("- insert:\n - id: bad\n name: '@deepseek-ai/dsh-mcp-client'\n" + " config:\n serverName: bad\n transport: streamable-http\n" + " url: !!js require('node:fs').writeFileSync('" + str(sentinel) + "', 'ran')\n" + " - id: ok\n name: '@deepseek-ai/dsh-mcp-client'\n" + " config:\n serverName: ok\n transport: streamable-http\n" + " url: https://ok.example.invalid/mcp\n") + (bundle / "cordis.patch.yml").write_text(patch) + args = self.args(bundle=bundle, dialect="dsh") + result = self.cli(args) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertFalse(sentinel.exists()) + self.assertEqual(sorted(self.servers(args.output)), ["ok"]) + self.assertIn("bad", (args.output / "CONVERSION.md").read_text()) + + def test_dsh_bundle_requires_manifest_patch_inside_package(self): + bundle = self.bundle() + self.refuse(self.args(bundle=bundle, dialect="dsh"), message="patch") + escaping = self.bundle(manifest={"dsh": {"bundle": {"patch": "../outside.yml"}}}) + self.refuse(self.args(bundle=escaping, dialect="dsh")) + plain = self.fresh("not-a-bundle") + plain.mkdir() + (plain / "package.json").write_text(json.dumps({"name": "plain"})) + self.refuse(self.args(bundle=plain, dialect="dsh"), message="dsh.bundle.patch") + self.refuse(self.args(bundle=self.bundle(), dialect="opencode-v1"), message="--format dsh") + combined = self.bundle() + (combined / "cordis.patch.yml").write_text("[]") + self.refuse(self.args(bundle=combined, config=self.config(self.dsh()), dialect="dsh"), + message="not both") + def test_cli_literal_credentials_and_interpolation_never_echo_or_publish(self): secret_file = self.write(CANARY, ".txt") cases = [self.v1(headers={"Authorization": "Bearer " + CANARY}), diff --git a/web/lib/changelog.generated.ts b/web/lib/changelog.generated.ts index 01d6cc98b2..7bf9b4d1bd 100644 --- a/web/lib/changelog.generated.ts +++ b/web/lib/changelog.generated.ts @@ -52,16 +52,18 @@ export const CHANGELOG: ChangelogRelease[] = [ { "heading": "Changed", "items": [ + "MCP protocol negotiation: every surface advertised the original 2024-11-05 revision and the stdio client required an exact match, so newer servers could not connect. The server and both clients now advertise 2025-06-18 and negotiate over the supported set (2025-06-18, 2025-03-26, 2024-11-05) — the server echoes the client's revision when it is supported and answers with the latest otherwise, the stdio client accepts any supported revision, and streamable HTTP sends the…", "Configured MCP servers now connect lazily instead of all at session boot. The pool owns a connecting set marked at spawn and cleared on resolution or abort, so \"connecting\" is no longer inferred as enabled-minus-connected. The boot pass scopes to the eager set — required servers plus those covered by tools.always_load / allowed_tools — and a turn naming an unstarted server spawns its connects alongside, under the existing five-second deadline. A configured-but-unstarted…", "The launch card's MCP problems row runs its own remedy. It already printed /mcp login or /mcp; it now joins the shared paint/click/keyboard ordering, so Up/Down lands on it and Enter or a click types the printed command into the composer for you to send. Typing beats copying: no clipboard dependency over SSH, and you see the command before a second Enter runs it (#6085).", "Computer Use is the only computer-use product in Extensions and /mcp recommendations. Cua is no longer suggested as a parallel desktop-control MCP; enable the first-party computer-use plugin instead. The bundled plugin is 0.4.0: Return/Enter from type, filtered and paginated get_app_state, focus/get_value, and strategy:\"app\" window-scoped clicks. Shared-desktop pointer gestures stay gated.", "The bundled first-party catalog pins marketplace revision ca6be22, so installing Computer Use from the Extensions listing fetches the same 0.4.0 source and the published notarized 0.4.0 Mac app." ], - "itemCount": 4 + "itemCount": 5 }, { "heading": "Fixed", "items": [ + " suggestions stop nagging: a plugin id is now injected at most once per engine lifetime, and a plugin whose name a loaded skill already covers is never suggested — the local skill owns the domain, so the nudge was noise. Dismissals still apply, and the fragment stays append-only on the user turn (#6274).", "A canceled automation run now settles with a transcript receipt that names the cancellation (by request, cancel timeout, or shutdown) instead of vanishing from the live band silently. The receipt wears attention ink and never lights the failure demand; the run record keeps the cancellation reason as its error detail. (#6162)", "A failed workflow run no longer settles silently: its terminal failure raises a sticky error toast naming the cause (dispatch, schema, or script errors), alongside the existing panel state (#5528).", "MCP OAuth re-login now forces the provider's consent screen: logout only clears the local token, so without a prompt the provider silently re-granted the same account/workspace and a re-login could never change it. /mcp logout and codewhale mcp logout also say plainly that they clear local credentials only (#6040).", @@ -73,7 +75,7 @@ export const CHANGELOG: ChangelogRelease[] = [ "/mcp no longer freezes the console while a turn is running: the panel opens immediately from the last known MCP snapshot with a receipt naming the wait, and live-pool mutations say their refresh is deferred instead of parking the UI event loop behind the running turn (#6159).", "MCP OAuth login no longer fails with \"Authorization server response missing required issuer\" against servers that implement RFC 9207, such as Cloudflare's mcp.cloudflare.com. The local callback listener now keeps the iss parameter from the redirect and hands it to the token exchange so the callback binds to the discovered issuer; servers that do not send iss keep working unchanged. (#6157)" ], - "itemCount": 10 + "itemCount": 11 } ] },