diff --git a/crates/pod/src/feature/plugin.rs b/crates/pod/src/feature/plugin.rs index dd2774cf..c8d58190 100644 --- a/crates/pod/src/feature/plugin.rs +++ b/crates/pod/src/feature/plugin.rs @@ -184,6 +184,7 @@ pub fn inspect_resolved_plugin_static(record: &ResolvedPluginRecord) -> PluginSt }) .collect(); + let duplicate_tool_names = duplicate_tool_names(record); let tools = record .manifest .tools @@ -192,9 +193,11 @@ pub fn inspect_resolved_plugin_static(record: &ResolvedPluginRecord) -> PluginSt let permission = PluginPermission::tool(&tool.name); let requested = permission_requested(record, &permission); let granted = grant_allows(record, &permission); - let diagnostic = authorize_plugin_tool(record, tool) - .err() - .map(|error| error.bounded_message()); + let mut diagnostics = validate_plugin_tool_definition(tool, &duplicate_tool_names); + if let Err(error) = authorize_plugin_tool(record, tool) { + diagnostics.push(error.bounded_message()); + } + let diagnostic = join_tool_diagnostics(diagnostics); PluginToolEligibility { name: tool.name.clone(), permission: permission.label(), @@ -230,6 +233,48 @@ fn grant_allows(record: &ResolvedPluginRecord, permission: &PluginPermission) -> .any(|granted| granted == permission) } +fn duplicate_tool_names(record: &ResolvedPluginRecord) -> HashSet { + let mut seen = HashSet::new(); + let mut duplicates = HashSet::new(); + for tool in &record.manifest.tools { + if !seen.insert(tool.name.clone()) { + duplicates.insert(tool.name.clone()); + } + } + duplicates +} + +fn validate_plugin_tool_definition( + tool: &PluginToolManifest, + duplicate_tool_names: &HashSet, +) -> Vec { + let mut diagnostics = Vec::new(); + if duplicate_tool_names.contains(&tool.name) { + diagnostics.push(format!( + "tool `{}` has duplicate name within plugin manifest", + tool.name + )); + } + if let Err(reason) = validate_tool_name(&tool.name) { + diagnostics.push(format!("tool `{}` has invalid name: {reason}", tool.name)); + } + if let Err(reason) = validate_input_schema(&tool.input_schema) { + diagnostics.push(format!( + "tool `{}` has invalid input_schema: {reason}", + tool.name + )); + } + diagnostics +} + +fn join_tool_diagnostics(diagnostics: Vec) -> Option { + if diagnostics.is_empty() { + None + } else { + Some(bounded_message(diagnostics.join("; "))) + } +} + impl FeatureModule for PluginToolFeature { fn descriptor(&self) -> FeatureDescriptor { let mut descriptor = @@ -1860,6 +1905,70 @@ input_schema = { type = "object", additionalProperties = true } ); } + #[test] + fn static_inspection_reports_invalid_tool_definition() { + let mut bad_schema = tool("Echo"); + bad_schema.input_schema = json!({"type":"string"}); + let mut record = record(vec![bad_schema]); + record.manifest.runtime = Some(PluginRuntimeManifest { + kind: "wasm".to_string(), + entry: "plugin.wasm".to_string(), + abi: Some("yoi-plugin-wasm-1".to_string()), + }); + + let inspection = inspect_resolved_plugin_static(&record); + + assert!(!inspection.statically_eligible()); + assert!(!inspection.tools[0].eligible); + let diagnostic = inspection.tools[0] + .diagnostic + .as_deref() + .unwrap_or_default(); + assert!(diagnostic.contains("invalid input_schema")); + assert!(diagnostic.contains("root schema type must be `object`")); + } + + #[test] + fn static_inspection_reports_invalid_and_duplicate_tool_names() { + let mut invalid = tool("Bad Tool"); + invalid.input_schema = json!({"type":"object"}); + let mut first_duplicate = tool("Echo"); + let mut second_duplicate = tool("Echo"); + first_duplicate.input_schema = json!({"type":"object"}); + second_duplicate.input_schema = json!({"type":"object"}); + let mut record = record(vec![invalid, first_duplicate, second_duplicate]); + record.manifest.runtime = Some(PluginRuntimeManifest { + kind: "wasm".to_string(), + entry: "plugin.wasm".to_string(), + abi: Some("yoi-plugin-wasm-1".to_string()), + }); + + let inspection = inspect_resolved_plugin_static(&record); + + assert!(!inspection.statically_eligible()); + assert!( + inspection.tools[0] + .diagnostic + .as_deref() + .unwrap_or_default() + .contains("invalid name") + ); + assert!( + inspection.tools[1] + .diagnostic + .as_deref() + .unwrap_or_default() + .contains("duplicate name") + ); + assert!( + inspection.tools[2] + .diagnostic + .as_deref() + .unwrap_or_default() + .contains("duplicate name") + ); + } + fn write_stored_zip(path: &Path, files: &[(&str, &[u8])]) { let mut out = Vec::new(); let mut central = Vec::new(); diff --git a/crates/yoi/src/plugin_cli.rs b/crates/yoi/src/plugin_cli.rs index 6382b1a8..26f540d9 100644 --- a/crates/yoi/src/plugin_cli.rs +++ b/crates/yoi/src/plugin_cli.rs @@ -989,6 +989,64 @@ mod tests { ); } + #[test] + fn invalid_tool_schema_and_name_are_rejected_in_json_and_human_output() { + let dir = tempdir().unwrap(); + let workspace = dir.path(); + let bad_schema_manifest = plugin_manifest("bad_schema", "Echo", "string", &["Echo"]); + let bad_name_manifest = plugin_manifest("bad_name", "Bad Tool", "object", &["Bad Tool"]); + let bad_schema_digest = + write_plugin_manifest(workspace, "bad_schema", &bad_schema_manifest); + let bad_name_digest = write_plugin_manifest(workspace, "bad_name", &bad_name_manifest); + let mut config = PluginConfig::default(); + config.enabled.push(enablement( + "project:bad_schema", + "0.1.0", + bad_schema_digest, + &["Echo"], + )); + config.enabled.push(enablement( + "project:bad_name", + "0.1.0", + bad_name_digest, + &["Bad Tool"], + )); + + let snapshot = inspect_snapshot(workspace, &config); + let bad_schema = select_item(&snapshot, "project:bad_schema").unwrap(); + let bad_name = select_item(&snapshot, "project:bad_name").unwrap(); + + assert_eq!(bad_schema.status, "rejected"); + assert_eq!(bad_name.status, "rejected"); + assert!(!bad_schema.tools[0].eligible); + assert!(!bad_name.tools[0].eligible); + + let list_json = serde_json::to_value(&snapshot).unwrap(); + assert!(list_json["items"].as_array().unwrap().iter().any(|item| { + item["reference"] == "project:bad_schema" + && item["status"] == "rejected" + && item["tools"][0]["diagnostic"] + .as_str() + .unwrap_or_default() + .contains("invalid input_schema") + })); + let show_json = serde_json::to_value(bad_name).unwrap(); + assert_eq!(show_json["status"], "rejected"); + assert!( + show_json["tools"][0]["diagnostic"] + .as_str() + .unwrap_or_default() + .contains("invalid name") + ); + + let list_output = render_list_snapshot_human(&snapshot).unwrap(); + assert!(list_output.contains("project:bad_schema [rejected]")); + assert!(list_output.contains("project:bad_name [rejected]")); + let show_output = render_item_human(bad_schema).unwrap(); + assert!(show_output.contains("invalid input_schema")); + assert!(show_output.contains("eligible=false")); + } + #[test] fn ambiguous_ref_is_bounded_error() { let snapshot = PluginInspectionSnapshot { @@ -1042,6 +1100,66 @@ mod tests { } } + fn enablement( + id: &str, + version: &str, + digest: String, + tool_permissions: &[&str], + ) -> PluginEnablementConfig { + let mut permissions = vec![PluginPermission::surface(PluginSurface::Tool)]; + permissions.extend( + tool_permissions + .iter() + .map(|tool_name| PluginPermission::tool(*tool_name)), + ); + PluginEnablementConfig { + id: id.to_string(), + digest: Some(digest.clone()), + version: Some(PluginExactVersion(version.to_string())), + surfaces: vec![PluginSurface::Tool], + grants: PluginGrantConfig { + id: Some(id.to_string()), + version: Some(PluginExactVersion(version.to_string())), + digest: Some(digest), + permissions, + }, + config: None, + } + } + + fn plugin_manifest( + id: &str, + tool_name: &str, + schema_type: &str, + permission_tools: &[&str], + ) -> String { + let permissions = permission_tools + .iter() + .map(|tool| format!(r#"{{ kind = "tool", name = "{tool}" }}"#)) + .collect::>() + .join(", "); + format!( + r#" +schema_version = 1 +id = "{id}" +name = "{id}" +version = "0.1.0" +surfaces = ["tool"] +permissions = [{{ kind = "surface", surface = "tool" }}, {permissions}] + +[runtime] +kind = "wasm" +entry = "plugin.wasm" +abi = "yoi-plugin-wasm-1" + +[[tools]] +name = "{tool_name}" +description = "Test tool" +input_schema = {{ type = "{schema_type}" }} +"# + ) + } + fn write_plugin_package(workspace: &Path, id: &str) -> String { let manifest = format!( r#"