From fcae8860445221503876ee68f72b25e6419edbcd Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 00:54:32 +0900 Subject: [PATCH 1/8] ticket: accept plugin tool surface work --- .../artifacts/orchestration-plan.jsonl | 1 + .yoi/tickets/00001KV5W3PHA/item.md | 4 +- .yoi/tickets/00001KV5W3PHA/thread.md | 84 +++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 .yoi/tickets/00001KV5W3PHA/artifacts/orchestration-plan.jsonl diff --git a/.yoi/tickets/00001KV5W3PHA/artifacts/orchestration-plan.jsonl b/.yoi/tickets/00001KV5W3PHA/artifacts/orchestration-plan.jsonl new file mode 100644 index 00000000..1b12b5d3 --- /dev/null +++ b/.yoi/tickets/00001KV5W3PHA/artifacts/orchestration-plan.jsonl @@ -0,0 +1 @@ +{"id":"orch-plan-20260615-155415-1","ticket_id":"00001KV5W3PHA","kind":"accepted_plan","accepted_plan":{"summary":"Accept Plugin Tool surface registration boundary work. Register enabled Plugin Tool schemas into ToolRegistry with origin metadata, duplicate/schema diagnostics, feature/profile hiding, and runtime-missing safe errors without implementing WASM execution.","branch":"impl/00001KV5W3PHA-plugin-tool-surface","worktree":"/home/hare/Projects/yoi/.worktree/00001KV5W3PHA-plugin-tool-surface","role_plan":"Orchestrator creates dedicated implementation worktree and spawns Coder with write scope limited to that worktree. Reviewer will run read-only after implementation report. Dependency `00001KV5R5V2S` is done and current workspace is clean."},"author":"yoi-orchestrator","at":"2026-06-15T15:54:15Z"} diff --git a/.yoi/tickets/00001KV5W3PHA/item.md b/.yoi/tickets/00001KV5W3PHA/item.md index 8cd46dd1..97f24d80 100644 --- a/.yoi/tickets/00001KV5W3PHA/item.md +++ b/.yoi/tickets/00001KV5W3PHA/item.md @@ -1,8 +1,8 @@ --- title: 'Plugin: register enabled Tool surface from packages' -state: 'queued' +state: 'inprogress' created_at: '2026-06-15T14:48:59Z' -updated_at: '2026-06-15T15:53:32Z' +updated_at: '2026-06-15T15:54:24Z' assignee: null readiness: 'implementation_ready' risk_flags: ['plugin', 'tool-registry', 'model-visible-schema', 'capability-boundary', 'profile-config'] diff --git a/.yoi/tickets/00001KV5W3PHA/thread.md b/.yoi/tickets/00001KV5W3PHA/thread.md index d90a2992..c0d04e20 100644 --- a/.yoi/tickets/00001KV5W3PHA/thread.md +++ b/.yoi/tickets/00001KV5W3PHA/thread.md @@ -13,4 +13,88 @@ LocalTicketBackend によって作成されました。 Ticket を `workspace-panel` が queued にしました。 +--- + + + +## Decision + +Routing decision: implementation_ready + +Reason: +- Panel Queue により routing が明示的に許可され、Ticket は `queued`。 +- Ticket body / thread / relations / OrchestrationPlan / Orchestrator workspace state を確認した。 +- Outgoing dependency `00001KV5R5V2S` は `done` で、typed relation blocker は解消済み。 +- 本 Ticket は resolved Plugin metadata を入力にした Tool surface registration boundary の実装であり、Plugin code execution / WASM runtime / permission grant enforcement は non-goal として明確。 +- capability-boundary / model-visible-schema / tool-registry risk は高いが、acceptance criteria と fail-closed invariants が具体的で、残る不確実性は typed metadata / registry integration tactic に閉じている。 + +Evidence checked: +- Ticket body/thread: requirements、acceptance criteria、non-goals、related work を確認。 +- Ticket relations: depends_on `00001KV5R5V2S` は done。incoming dependency from runtime Ticket `00001KV5W3PHW` は本 Ticket の blocker ではない。 +- OrchestrationPlan: 既存 record なし。 +- Orchestrator workspace: `/home/hare/Projects/yoi/.worktree/orchestration` は clean、`1fdb4cd6` 上。 +- Visible Pods: implementation child Pod なし。 + +IntentPacket: + +Intent: +- Enabled Plugin package の manifest Tool surface を読み取り、既存 `ToolRegistry` の model-visible schema 経路に安全に登録できる registration boundary を作る。ただし Tool call execution / WASM runtime はまだ実装しない。 + +Binding decisions / invariants: +- Discovery-only package は Tool schema surface に出さない。explicit enablement が必要。 +- Tool registration は existing `ToolRegistry` 原則に従い、feature/profile config disabled なら model-visible schema から消える。 +- Plugin Tool metadata に origin（plugin id/ref、source、digest、version/api、surface）を保持する。 +- Duplicate Tool name は fail closed。builtin Tool / other Plugin Tool との衝突を曖昧に勝たせない。 +- Invalid/unsupported input schema は fail closed。 +- Runtime executor missing の Tool call は panic せず安全な unavailable/runtime-missing error を返す。 +- Plugin code execution / WASM runtime / host API / permission grant enforcement / Service/Ingress/MCP bridge は non-goal。 +- Permission declarations/grants を実効権限として扱わない。registration-time shape checks だけ。 + +Requirements / acceptance criteria: +- Enabled Plugin package の Tool definition が `ToolRegistry` に登録され、model-visible tools に現れる。 +- Enablement がない Plugin package の Tool は model-visible tools に現れない。 +- Duplicate Tool name / builtin collision は登録されず diagnostic で理由が分かる。 +- Invalid input schema は登録されず diagnostic で理由が分かる。 +- Registered Plugin Tool metadata から plugin origin / digest / source を追跡できる。 +- Feature/profile flag により Plugin Tool surface を非表示にできる。 +- Tool call が未実行状態でも panic せず unavailable/runtime-missing error。 +- Tests cover enabled registration、no enablement inactive、duplicate Plugin Tool name、builtin collision、invalid schema、origin metadata、disabled feature/profile surface hiding。 + +Implementation latitude: +- Plugin manifest Tool definition schema の最小 shape は既存 design / resolver の strict parser に合わせてよい。 +- Existing ToolRegistry contribution path に合わせた adapter / feature contribution module を追加してよい。 +- Runtime-missing handler は minimal stub でよいが、model-visible registration と call failure behavior は明確にテストする。 +- Diagnostics の storage/reporting surface は existing resolver diagnostics と整合させる。 + +Escalate if: +- ToolRegistry API が origin metadata を保持できず public API redesign が必要。 +- Feature/profile config で Plugin Tool surface を安全に hide できない。 +- JSON schema validation requires broader schema engine choice。 +- Runtime-missing Tool stub が model/provider protocol に悪影響を与える。 +- Permission grants / WASM executor / host API を実装しないと registration boundary が成立しない。 + +Validation: +- focused plugin/tool-registry tests。 +- relevant `cargo test` / `cargo check`。 +- `cargo fmt --check`。 +- `git diff --check`。 +- `nix build .#yoi` if dependencies, Cargo.lock, package resources, or packaging correctness change。 + +Critical risks / reviewer focus: +- model-visible schema gating。 +- duplicate/builtin collision fail closed。 +- no enablement → no schema。 +- origin metadata traceability。 +- no execution/registration side effects beyond schema contribution。 +- unavailable runtime call safety。 +- permission declaration vs effective grant separation。 + +--- + + + +## State changed + +Routing decision と accepted implementation plan を記録済み。dependency `00001KV5R5V2S` は done で relation blocker は解消済み。Orchestrator workspace は clean。implementation side effects の前に `queued -> inprogress` acceptance を記録する。 + --- From 05a9c522178243337d0b299280065c2b6dd9b9b2 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:18:54 +0900 Subject: [PATCH 2/8] feat: register plugin tool surfaces --- crates/llm-worker/src/tool.rs | 34 +++ crates/manifest/src/config.rs | 8 + crates/manifest/src/lib.rs | 3 + crates/manifest/src/plugin.rs | 38 ++- crates/pod/src/controller.rs | 118 ++++---- crates/pod/src/feature.rs | 25 +- crates/pod/src/feature/plugin.rs | 490 +++++++++++++++++++++++++++++++ crates/pod/src/pod.rs | 3 +- 8 files changed, 660 insertions(+), 59 deletions(-) create mode 100644 crates/pod/src/feature/plugin.rs diff --git a/crates/llm-worker/src/tool.rs b/crates/llm-worker/src/tool.rs index 231c645d..e90ffb12 100644 --- a/crates/llm-worker/src/tool.rs +++ b/crates/llm-worker/src/tool.rs @@ -127,6 +127,31 @@ impl From for ToolOutput { // ToolMeta - Immutable Meta Information // ============================================================================= +/// Origin metadata for a registered tool. +/// +/// This metadata is intentionally not part of the provider-facing tool schema. +/// It lets host layers audit where a model-visible tool definition came from +/// while keeping execution and permission semantics in the normal Worker path. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ToolOrigin { + /// Origin kind, for example `plugin` or `builtin`. + pub kind: String, + /// Package-local plugin id. + pub plugin_id: String, + /// Source-qualified plugin/package reference when `kind == "plugin"`. + pub plugin_ref: String, + /// Plugin source such as `user`, `project`, or `builtin`. + pub source: String, + /// Resolved package digest. + pub digest: String, + /// Resolved package version. + pub package_version: String, + /// Plugin API/schema version declared by the package. + pub package_api_version: u32, + /// Surface that contributed this tool. Plugin tools use `tool`. + pub surface: String, +} + /// Tool meta information (fixed at registration, immutable) /// /// Generated from `ToolDefinition` factory and does not change after registration with Worker. @@ -139,6 +164,8 @@ pub struct ToolMeta { pub description: String, /// JSON Schema for arguments pub input_schema: Value, + /// Optional host-side origin metadata. This is not exposed to the LLM. + pub origin: Option, } impl ToolMeta { @@ -148,6 +175,7 @@ impl ToolMeta { name: name.into(), description: String::new(), input_schema: Value::Object(Default::default()), + origin: None, } } @@ -162,6 +190,12 @@ impl ToolMeta { self.input_schema = schema; self } + + /// Set host-side origin metadata. + pub fn origin(mut self, origin: ToolOrigin) -> Self { + self.origin = Some(origin); + self + } } // ============================================================================= diff --git a/crates/manifest/src/config.rs b/crates/manifest/src/config.rs index bea5ae8c..1137e71c 100644 --- a/crates/manifest/src/config.rs +++ b/crates/manifest/src/config.rs @@ -84,6 +84,8 @@ pub struct FeatureConfigPartial { pub ticket: Option, #[serde(default)] pub ticket_orchestration: Option, + #[serde(default)] + pub plugins: Option, } impl FeatureConfigPartial { @@ -99,6 +101,7 @@ impl FeatureConfigPartial { other.ticket_orchestration, FeatureFlagConfigPartial::merge, ), + plugins: merge_option(self.plugins, other.plugins, FeatureFlagConfigPartial::merge), } } } @@ -152,6 +155,10 @@ impl From for FeatureConfig { .ticket_orchestration .map(FeatureFlagConfig::from) .unwrap_or_default(), + plugins: value + .plugins + .map(FeatureFlagConfig::from) + .unwrap_or_default(), } } } @@ -199,6 +206,7 @@ impl From for FeatureConfigPartial { pods: Some(value.pods.into()), ticket: Some(value.ticket.into()), ticket_orchestration: Some(value.ticket_orchestration.into()), + plugins: Some(value.plugins.into()), } } } diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 707253dd..19319f95 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -107,6 +107,8 @@ pub struct FeatureConfig { pub ticket: TicketFeatureConfig, #[serde(default)] pub ticket_orchestration: FeatureFlagConfig, + #[serde(default)] + pub plugins: FeatureFlagConfig, } impl Default for FeatureConfig { @@ -118,6 +120,7 @@ impl Default for FeatureConfig { pods: FeatureFlagConfig::disabled(), ticket: TicketFeatureConfig::default(), ticket_orchestration: FeatureFlagConfig::disabled(), + plugins: FeatureFlagConfig::disabled(), } } } diff --git a/crates/manifest/src/plugin.rs b/crates/manifest/src/plugin.rs index 431d919a..d4460e8e 100644 --- a/crates/manifest/src/plugin.rs +++ b/crates/manifest/src/plugin.rs @@ -188,14 +188,19 @@ pub struct PluginPackageManifest { pub runtime: Option, #[serde(default)] pub hooks: Vec, + #[serde(default)] + pub tools: Vec, } impl PluginPackageManifest { - fn declared_surfaces(&self) -> BTreeSet { + pub fn declared_surfaces(&self) -> BTreeSet { let mut surfaces: BTreeSet<_> = self.surfaces.iter().copied().collect(); if !self.hooks.is_empty() { surfaces.insert(PluginSurface::Hook); } + if !self.tools.is_empty() { + surfaces.insert(PluginSurface::Tool); + } if self.runtime.is_some() { surfaces.insert(PluginSurface::Wasm); } @@ -218,6 +223,14 @@ pub struct PluginHookManifest { pub file: String, } +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct PluginToolManifest { + pub name: String, + pub description: String, + pub input_schema: serde_json::Value, +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct PluginDiscoveryLimits { pub max_packages_per_store: usize, @@ -1653,6 +1666,29 @@ file = "hooks/summary.md" ); } + #[test] + fn package_manifest_tool_surface_shape_is_accepted() { + let manifest: PluginPackageManifest = toml::from_str( + r#" +schema_version = 1 +id = "example.tool" +name = "Example Tool" +version = "0.1.0" + +[[tools]] +name = "ExampleTool" +description = "Runs a package-defined tool." +input_schema = { type = "object", properties = { query = { type = "string" } }, required = ["query"], additionalProperties = false } +"#, + ) + .unwrap(); + + assert_eq!(manifest.tools.len(), 1); + assert!(manifest.declared_surfaces().contains(&PluginSurface::Tool)); + assert_eq!(manifest.tools[0].name, "ExampleTool"); + assert_eq!(manifest.tools[0].input_schema["type"], "object"); + } + #[test] fn malformed_manifest_multibyte_diagnostic_is_bounded_and_redacted() { let temp = TempDir::new().unwrap(); diff --git a/crates/pod/src/controller.rs b/crates/pod/src/controller.rs index fbb09bf2..cc8b5b06 100644 --- a/crates/pod/src/controller.rs +++ b/crates/pod/src/controller.rs @@ -634,66 +634,74 @@ where ), ); } - let _feature_install_report = pod.install_features(feature_registry); - - let worker = pod.worker_mut(); - - // Memory tools require both explicit feature exposure and memory storage - // configuration. This keeps resident-memory config separate from the - // model-visible Memory*/Knowledge* tool surface. - if feature_config.memory.enabled { - let mem = memory_config.as_ref().ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "[feature.memory].enabled = true requires a [memory] configuration section", - ) - })?; - let layout = memory::WorkspaceLayout::resolve(mem, &workspace_root); - let query_cfg = memory::tool::QueryConfig::from(mem); - worker.register_tool(memory::tool::read_tool_with_usage( - layout.clone(), - session_id_for_usage, - )); - worker.register_tool(memory::tool::write_tool(layout.clone())); - worker.register_tool(memory::tool::edit_tool(layout.clone())); - worker.register_tool(memory::tool::delete_tool(layout.clone())); - worker.register_tool(memory::tool::memory_query_tool(layout.clone(), query_cfg)); - worker.register_tool(memory::tool::knowledge_query_tool(layout, query_cfg)); + for module in crate::feature::plugin::plugin_tool_features_if_enabled( + feature_config.plugins.enabled, + &pod.manifest().plugins, + ) { + feature_registry = feature_registry.with_module(module); } - // Pod-orchestration tools (SpawnPod + the four comm tools) share - // the Pod-scoped `SpawnedPodRegistry` (also consumed by the main - // loop's `PodEvent` handler). Expose them only behind the explicit - // profile feature and require delegation authority up front so enabling - // the surface cannot imply broad child scope by accident. - if feature_config.pods.enabled { - if spawner_manifest.delegation_scope.allow.is_empty() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "[feature.pods].enabled = true requires non-empty [[delegation_scope.allow]]", + { + let worker = pod.worker_mut(); + + // Memory tools require both explicit feature exposure and memory storage + // configuration. This keeps resident-memory config separate from the + // model-visible Memory*/Knowledge* tool surface. + if feature_config.memory.enabled { + let mem = memory_config.as_ref().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "[feature.memory].enabled = true requires a [memory] configuration section", + ) + })?; + let layout = memory::WorkspaceLayout::resolve(mem, &workspace_root); + let query_cfg = memory::tool::QueryConfig::from(mem); + worker.register_tool(memory::tool::read_tool_with_usage( + layout.clone(), + session_id_for_usage, )); + worker.register_tool(memory::tool::write_tool(layout.clone())); + worker.register_tool(memory::tool::edit_tool(layout.clone())); + worker.register_tool(memory::tool::delete_tool(layout.clone())); + worker.register_tool(memory::tool::memory_query_tool(layout.clone(), query_cfg)); + worker.register_tool(memory::tool::knowledge_query_tool(layout, query_cfg)); + } + + // Pod-orchestration tools (SpawnPod + the four comm tools) share + // the Pod-scoped `SpawnedPodRegistry` (also consumed by the main + // loop's `PodEvent` handler). Expose them only behind the explicit + // profile feature and require delegation authority up front so enabling + // the surface cannot imply broad child scope by accident. + if feature_config.pods.enabled { + if spawner_manifest.delegation_scope.allow.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "[feature.pods].enabled = true requires non-empty [[delegation_scope.allow]]", + )); + } + worker.register_tool(spawn_pod_tool( + spawner_name.clone(), + spawner_socket, + runtime_base.clone(), + workspace_root.clone(), + cwd.clone(), + spawned_registry.clone(), + self_parent_socket, + spawner_manifest, + scope_handle, + prompts, + )); + worker.register_tool(send_to_pod_tool(spawned_registry.clone())); + worker.register_tool(read_pod_output_tool(spawned_registry.clone())); + worker.register_tool(stop_pod_tool(spawned_registry.clone())); + let discovery = + PodDiscovery::new(pod_store, spawner_name, runtime_base, cwd, spawned_registry); + worker.register_tool(list_pods_tool(discovery.clone())); + worker.register_tool(restore_pod_tool(discovery.clone())); + worker.register_tool(send_to_peer_pod_tool(discovery)); } - worker.register_tool(spawn_pod_tool( - spawner_name.clone(), - spawner_socket, - runtime_base.clone(), - workspace_root.clone(), - cwd.clone(), - spawned_registry.clone(), - self_parent_socket, - spawner_manifest, - scope_handle, - prompts, - )); - worker.register_tool(send_to_pod_tool(spawned_registry.clone())); - worker.register_tool(read_pod_output_tool(spawned_registry.clone())); - worker.register_tool(stop_pod_tool(spawned_registry.clone())); - let discovery = - PodDiscovery::new(pod_store, spawner_name, runtime_base, cwd, spawned_registry); - worker.register_tool(list_pods_tool(discovery.clone())); - worker.register_tool(restore_pod_tool(discovery.clone())); - worker.register_tool(send_to_peer_pod_tool(discovery)); } + let _feature_install_report = pod.install_features(feature_registry); pod.attach_tracker(tracker); Ok(fs_for_view) } diff --git a/crates/pod/src/feature.rs b/crates/pod/src/feature.rs index b6bbf8fb..924065c5 100644 --- a/crates/pod/src/feature.rs +++ b/crates/pod/src/feature.rs @@ -1290,15 +1290,36 @@ impl FeatureRegistryBuilder { hook_builder: &mut HookRegistryBuilder, ) -> FeatureRegistryInstallReport { let mut pending_tools = Vec::new(); - let report = self.install_into_pending(&mut pending_tools, hook_builder); + worker.tool_server_handle().flush_pending(); + let registered_tool_names = worker + .tool_server_handle() + .tool_definitions_sorted() + .into_iter() + .map(|definition| (definition.name, FeatureId::builtin("preexisting-tool"))) + .collect(); + let report = self.install_into_pending_with_registered( + &mut pending_tools, + hook_builder, + registered_tool_names, + ); worker.register_tools(pending_tools); report } + #[allow(dead_code)] pub(crate) fn install_into_pending( self, pending_tools: &mut Vec, hook_builder: &mut HookRegistryBuilder, + ) -> FeatureRegistryInstallReport { + self.install_into_pending_with_registered(pending_tools, hook_builder, HashMap::new()) + } + + fn install_into_pending_with_registered( + self, + pending_tools: &mut Vec, + hook_builder: &mut HookRegistryBuilder, + mut installed_tool_names: HashMap, ) -> FeatureRegistryInstallReport { let descriptors: Vec<_> = self .modules @@ -1307,7 +1328,6 @@ impl FeatureRegistryBuilder { .collect(); let mut service_registry = FeatureServiceRegistry::default(); let mut reports = Vec::with_capacity(self.modules.len()); - let mut installed_tool_names = HashMap::new(); let mut seen_features = HashSet::new(); for (module, descriptor) in self.modules.into_iter().zip(descriptors.into_iter()) { @@ -1455,6 +1475,7 @@ pub enum FeatureInstallError { } pub mod builtin; +pub mod plugin; #[cfg(test)] mod tests { diff --git a/crates/pod/src/feature/plugin.rs b/crates/pod/src/feature/plugin.rs new file mode 100644 index 00000000..6133289c --- /dev/null +++ b/crates/pod/src/feature/plugin.rs @@ -0,0 +1,490 @@ +//! Plugin package contributions for model-visible Tool schemas. +//! +//! This module registers *enabled* plugin package tool surface definitions as +//! unavailable Tool stubs. It deliberately does not execute plugin code or grant +//! plugin permissions; the runtime/WASM executor belongs to a later boundary. + +use std::collections::HashSet; +use std::sync::Arc; + +use async_trait::async_trait; +use llm_worker::tool::{ + Tool, ToolDefinition, ToolError, ToolExecutionContext, ToolMeta, ToolOrigin, ToolOutput, +}; +use manifest::plugin::{PluginConfig, PluginSurface, ResolvedPluginRecord}; +use serde_json::Value; + +use super::{ + FeatureDescriptor, FeatureId, FeatureInstallContext, FeatureInstallError, FeatureModule, + FeatureRuntimeKind, ToolContribution, ToolDeclaration, +}; + +/// Build Feature modules for enabled plugin packages when the profile exposes +/// the plugin Tool surface feature. +pub fn plugin_tool_features_if_enabled( + feature_enabled: bool, + config: &PluginConfig, +) -> Vec { + if !feature_enabled { + return Vec::new(); + } + plugin_tool_features(config) +} + +/// Build Feature modules for enabled plugin packages that declare Tool surfaces. +pub fn plugin_tool_features(config: &PluginConfig) -> Vec { + config + .resolved + .iter() + .filter(|record| record.enabled_surfaces.contains(&PluginSurface::Tool)) + .filter(|record| !record.manifest.tools.is_empty()) + .cloned() + .map(PluginToolFeature::new) + .collect() +} + +#[derive(Clone, Debug)] +pub struct PluginToolFeature { + record: ResolvedPluginRecord, + feature_id: FeatureId, +} + +impl PluginToolFeature { + pub fn new(record: ResolvedPluginRecord) -> Self { + let feature_id = FeatureId::new(format!("plugin:{}:tool", record.identity)) + .expect("source-qualified plugin identity yields non-empty feature id"); + Self { record, feature_id } + } + + pub fn origin(&self) -> ToolOrigin { + ToolOrigin { + kind: "plugin".into(), + plugin_id: self.record.manifest.id.clone(), + plugin_ref: self.record.identity.to_string(), + source: self.record.identity.source.to_string(), + digest: self.record.digest.clone(), + package_version: self.record.version.clone(), + package_api_version: self.record.manifest.schema_version, + surface: "tool".into(), + } + } +} + +impl FeatureModule for PluginToolFeature { + fn descriptor(&self) -> FeatureDescriptor { + let mut descriptor = + FeatureDescriptor { + id: self.feature_id.clone(), + runtime: FeatureRuntimeKind::ExternalPlugin, + display_name: self.record.manifest.name.clone(), + version: self.record.manifest.version.clone(), + description: self.record.manifest.description.clone().unwrap_or_else(|| { + format!("Plugin tool surface from {}", self.record.identity) + }), + tools: Vec::new(), + hooks: Vec::new(), + background_tasks: Vec::new(), + provides_services: Vec::new(), + requires_services: Vec::new(), + protocol_providers: Vec::new(), + }; + for tool in &self.record.manifest.tools { + descriptor = descriptor.with_tool(ToolDeclaration::new( + tool.name.clone(), + tool.description.clone(), + )); + } + descriptor + } + + fn install(&self, context: &mut FeatureInstallContext<'_>) -> Result<(), FeatureInstallError> { + validate_declared_tool_names(&self.record)?; + let origin = self.origin(); + for tool in &self.record.manifest.tools { + validate_tool_name(&tool.name).map_err(|reason| { + FeatureInstallError::Install(format!( + "plugin `{}` tool `{}` has invalid name: {reason}", + self.record.identity, tool.name + )) + })?; + validate_input_schema(&tool.input_schema).map_err(|reason| { + FeatureInstallError::Install(format!( + "plugin `{}` tool `{}` has invalid input_schema: {reason}", + self.record.identity, tool.name + )) + })?; + context.tools().register(ToolContribution::new( + tool.name.clone(), + plugin_runtime_missing_definition( + tool.name.clone(), + tool.description.clone(), + tool.input_schema.clone(), + origin.clone(), + ), + ))?; + } + Ok(()) + } +} + +fn plugin_runtime_missing_definition( + name: String, + description: String, + input_schema: Value, + origin: ToolOrigin, +) -> ToolDefinition { + Arc::new(move || { + ( + ToolMeta::new(name.clone()) + .description(description.clone()) + .input_schema(input_schema.clone()) + .origin(origin.clone()), + Arc::new(PluginRuntimeMissingTool { + name: name.clone(), + origin: origin.clone(), + }) as Arc, + ) + }) +} + +struct PluginRuntimeMissingTool { + name: String, + origin: ToolOrigin, +} + +#[async_trait] +impl Tool for PluginRuntimeMissingTool { + async fn execute( + &self, + _input_json: &str, + _ctx: ToolExecutionContext, + ) -> Result { + Err(ToolError::ExecutionFailed(format!( + "plugin tool runtime missing/unavailable for `{}` from `{}` (digest {}, package {} api {})", + self.name, + self.origin.plugin_ref, + self.origin.digest, + self.origin.package_version, + self.origin.package_api_version + ))) + } +} + +fn validate_declared_tool_names(record: &ResolvedPluginRecord) -> Result<(), FeatureInstallError> { + let mut seen = HashSet::new(); + for tool in &record.manifest.tools { + if !seen.insert(tool.name.as_str()) { + return Err(FeatureInstallError::DuplicateToolName { + tool: tool.name.clone(), + first_feature: format!("{} (same plugin package)", record.identity), + duplicate_feature: record.identity.to_string(), + }); + } + } + Ok(()) +} + +fn validate_tool_name(name: &str) -> Result<(), &'static str> { + if name.is_empty() { + return Err("name must not be empty"); + } + if name.len() > 128 { + return Err("name is longer than 128 bytes"); + } + if name.chars().any(|c| c.is_control() || c.is_whitespace()) { + return Err("name must not contain whitespace or control characters"); + } + Ok(()) +} + +fn validate_input_schema(schema: &Value) -> Result<(), String> { + let Value::Object(root) = schema else { + return Err("root schema must be a JSON object".into()); + }; + match root.get("type") { + Some(Value::String(value)) if value == "object" => {} + Some(_) => return Err("root schema type must be `object`".into()), + None => return Err("root schema must declare type = `object`".into()), + } + if let Some(properties) = root.get("properties") { + if !properties.is_object() { + return Err("properties must be a JSON object".into()); + } + } + if let Some(required) = root.get("required") { + let Some(required) = required.as_array() else { + return Err("required must be an array".into()); + }; + if !required.iter().all(Value::is_string) { + return Err("required entries must be strings".into()); + } + } + if let Some(additional) = root.get("additionalProperties") { + if !(additional.is_boolean() || additional.is_object()) { + return Err("additionalProperties must be boolean or object".into()); + } + } + reject_unsupported_keywords(schema) +} + +fn reject_unsupported_keywords(schema: &Value) -> Result<(), String> { + match schema { + Value::Object(map) => { + for (key, value) in map { + if matches!( + key.as_str(), + "$ref" + | "$dynamicRef" + | "oneOf" + | "anyOf" + | "allOf" + | "not" + | "patternProperties" + | "dependentSchemas" + | "dependencies" + ) { + return Err(format!("unsupported schema keyword `{key}`")); + } + reject_unsupported_keywords(value)?; + } + Ok(()) + } + Value::Array(values) => { + for value in values { + reject_unsupported_keywords(value)?; + } + Ok(()) + } + _ => Ok(()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use manifest::plugin::{PluginPackageManifest, SourceQualifiedPluginId}; + use serde_json::json; + + fn tool(name: &str) -> manifest::plugin::PluginToolManifest { + manifest::plugin::PluginToolManifest { + name: name.into(), + description: format!("{name} tool"), + input_schema: json!({"type":"object","properties":{},"additionalProperties":false}), + } + } + + fn record(tools: Vec) -> ResolvedPluginRecord { + record_with_identity("project:example", tools) + } + + fn record_with_identity( + identity: &str, + tools: Vec, + ) -> ResolvedPluginRecord { + let parsed_identity = SourceQualifiedPluginId::parse(identity).unwrap(); + ResolvedPluginRecord { + identity: parsed_identity.clone(), + source: parsed_identity.source, + package_path: std::path::PathBuf::from("/tmp/example.zip"), + package_label: "example.zip".into(), + digest: "sha256:abc".into(), + version: "0.1.0".into(), + manifest: PluginPackageManifest { + schema_version: 1, + id: "example".into(), + name: "Example".into(), + version: "0.1.0".into(), + description: None, + surfaces: vec![PluginSurface::Tool], + runtime: None, + hooks: Vec::new(), + tools, + }, + enabled_surfaces: vec![PluginSurface::Tool], + grants: manifest::plugin::PluginGrantConfig::default(), + config: None, + } + } + + fn skipped_count(report: &super::super::FeatureRegistryInstallReport) -> usize { + report + .reports + .iter() + .map(|feature_report| feature_report.skipped.len()) + .sum() + } + + fn has_diagnostic(report: &super::super::FeatureRegistryInstallReport, needle: &str) -> bool { + report.reports.iter().any(|feature_report| { + feature_report + .diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains(needle)) + }) + } + + #[test] + fn rejects_invalid_root_schema() { + let schema = json!({"type":"string"}); + assert!( + validate_input_schema(&schema) + .unwrap_err() + .contains("type must be `object`") + ); + } + + #[test] + fn rejects_unsupported_schema_keyword() { + let schema = json!({"type":"object","oneOf":[]}); + assert!( + validate_input_schema(&schema) + .unwrap_err() + .contains("unsupported schema keyword") + ); + } + + #[test] + fn accepts_object_tool_schema() { + validate_input_schema(&json!({ + "type":"object", + "properties":{"query":{"type":"string"}}, + "required":["query"], + "additionalProperties":false + })) + .unwrap(); + } + + #[test] + fn origin_retains_plugin_metadata() { + let feature = PluginToolFeature::new(record(Vec::new())); + let origin = feature.origin(); + assert_eq!(origin.kind, "plugin"); + assert_eq!(origin.plugin_id, "example"); + assert_eq!(origin.plugin_ref, "project:example"); + assert_eq!(origin.source, "project"); + assert_eq!(origin.digest, "sha256:abc"); + assert_eq!(origin.package_version, "0.1.0"); + assert_eq!(origin.package_api_version, 1); + assert_eq!(origin.surface, "tool"); + } + + #[test] + fn enabled_plugin_tool_registers_model_visible_schema_and_origin() { + let mut pending = Vec::new(); + let mut hooks = crate::hook::HookRegistryBuilder::new(); + let report = super::super::FeatureRegistryBuilder::default() + .with_module(PluginToolFeature::new(record(vec![tool("PluginSearch")]))) + .install_into_pending(&mut pending, &mut hooks); + + assert!( + report + .reports + .iter() + .all(|feature_report| feature_report.diagnostics.is_empty()), + "{:#?}", + report.reports + ); + assert_eq!(report.installed_tool_names(), vec!["PluginSearch"]); + assert_eq!(pending.len(), 1); + let (meta, _) = pending[0](); + assert_eq!(meta.name, "PluginSearch"); + assert_eq!(meta.input_schema["type"], "object"); + let origin = meta.origin.expect("plugin origin metadata"); + assert_eq!(origin.plugin_ref, "project:example"); + assert_eq!(origin.digest, "sha256:abc"); + assert_eq!(origin.source, "project"); + assert_eq!(origin.surface, "tool"); + } + + #[test] + fn package_without_enabled_tool_surface_registers_no_schema() { + let mut config = PluginConfig::default(); + let mut disabled = record(vec![tool("PluginSearch")]); + disabled.enabled_surfaces.clear(); + config.resolved.push(disabled); + + assert!(plugin_tool_features(&config).is_empty()); + } + + #[test] + fn disabled_profile_feature_registers_no_schema() { + let mut config = PluginConfig::default(); + config.resolved.push(record(vec![tool("PluginSearch")])); + + assert!(plugin_tool_features_if_enabled(false, &config).is_empty()); + assert_eq!(plugin_tool_features_if_enabled(true, &config).len(), 1); + } + + #[test] + fn duplicate_plugin_tool_names_are_rejected_with_diagnostic() { + let mut pending = Vec::new(); + let mut hooks = crate::hook::HookRegistryBuilder::new(); + let report = super::super::FeatureRegistryBuilder::default() + .with_module(PluginToolFeature::new(record(vec![tool("PluginSearch")]))) + .with_module(PluginToolFeature::new(record_with_identity( + "project:other", + vec![tool("PluginSearch")], + ))) + .install_into_pending(&mut pending, &mut hooks); + + assert_eq!(pending.len(), 1); + assert_eq!(skipped_count(&report), 1); + assert!(has_diagnostic(&report, "duplicate tool contribution")); + } + + #[test] + fn builtin_tool_name_collision_is_rejected_with_diagnostic() { + let mut pending = Vec::new(); + let mut hooks = crate::hook::HookRegistryBuilder::new(); + let mut registered = std::collections::HashMap::new(); + registered.insert("Read".to_string(), FeatureId::builtin("preexisting-tool")); + + let report = super::super::FeatureRegistryBuilder::default() + .with_module(PluginToolFeature::new(record(vec![tool("Read")]))) + .install_into_pending_with_registered(&mut pending, &mut hooks, registered); + + assert!(pending.is_empty()); + assert_eq!(skipped_count(&report), 1); + assert!(has_diagnostic(&report, "duplicate tool contribution")); + } + + #[test] + fn invalid_input_schema_is_rejected_with_diagnostic() { + let mut invalid = tool("BadSchema"); + invalid.input_schema = json!({"type":"object","$ref":"#/defs/input"}); + let mut pending = Vec::new(); + let mut hooks = crate::hook::HookRegistryBuilder::new(); + + let report = super::super::FeatureRegistryBuilder::default() + .with_module(PluginToolFeature::new(record(vec![invalid]))) + .install_into_pending(&mut pending, &mut hooks); + + assert!(pending.is_empty()); + assert!(has_diagnostic(&report, "invalid input_schema")); + } + + #[tokio::test] + async fn registered_tool_executes_as_runtime_missing_error() { + let mut pending = Vec::new(); + let mut hooks = crate::hook::HookRegistryBuilder::new(); + let report = super::super::FeatureRegistryBuilder::default() + .with_module(PluginToolFeature::new(record(vec![tool("PluginSearch")]))) + .install_into_pending(&mut pending, &mut hooks); + assert!( + report + .reports + .iter() + .all(|feature_report| feature_report.diagnostics.is_empty()), + "{:#?}", + report.reports + ); + + let (_, tool) = pending[0](); + let error = tool + .execute("{}", ToolExecutionContext::default()) + .await + .unwrap_err(); + assert!(error.to_string().contains("runtime missing/unavailable")); + assert!(error.to_string().contains("project:example")); + } +} diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index fe155b63..b34845b7 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -824,7 +824,6 @@ impl Pod { registry: FeatureRegistryBuilder, ) -> FeatureRegistryInstallReport { let worker = self.worker.as_mut().expect("worker taken during run"); - let report = registry.install_into_worker(worker, &mut self.hook_builder); let active_workflow_committer = self.log_writer.clone().map(|writer| { Arc::new(move |entry| writer.commit_log_entry(entry)) as active_workflow::LogEntryCommitter @@ -833,6 +832,7 @@ impl Pod { self.active_workflows.clone(), active_workflow_committer, )); + let report = registry.install_into_worker(worker, &mut self.hook_builder); report } @@ -5353,6 +5353,7 @@ permission = "read" surfaces: vec![manifest::plugin::PluginSurface::Hook], runtime: None, hooks: vec![], + tools: vec![], }, enabled_surfaces: vec![manifest::plugin::PluginSurface::Hook], grants: manifest::plugin::PluginGrantConfig::default(), From f262815990842e39eefbbc26b32f4a1c9e69f560 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:20:13 +0900 Subject: [PATCH 3/8] ticket: record plugin tool surface implementation --- .yoi/tickets/00001KV5W3PHA/item.md | 2 +- .yoi/tickets/00001KV5W3PHA/thread.md | 71 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/.yoi/tickets/00001KV5W3PHA/item.md b/.yoi/tickets/00001KV5W3PHA/item.md index 97f24d80..814dd04b 100644 --- a/.yoi/tickets/00001KV5W3PHA/item.md +++ b/.yoi/tickets/00001KV5W3PHA/item.md @@ -2,7 +2,7 @@ title: 'Plugin: register enabled Tool surface from packages' state: 'inprogress' created_at: '2026-06-15T14:48:59Z' -updated_at: '2026-06-15T15:54:24Z' +updated_at: '2026-06-15T16:20:05Z' assignee: null readiness: 'implementation_ready' risk_flags: ['plugin', 'tool-registry', 'model-visible-schema', 'capability-boundary', 'profile-config'] diff --git a/.yoi/tickets/00001KV5W3PHA/thread.md b/.yoi/tickets/00001KV5W3PHA/thread.md index c0d04e20..fe2600f5 100644 --- a/.yoi/tickets/00001KV5W3PHA/thread.md +++ b/.yoi/tickets/00001KV5W3PHA/thread.md @@ -98,3 +98,74 @@ Critical risks / reviewer focus: Routing decision と accepted implementation plan を記録済み。dependency `00001KV5R5V2S` は done で relation blocker は解消済み。Orchestrator workspace は clean。implementation side effects の前に `queued -> inprogress` acceptance を記録する。 --- + + + +## Implementation report + +Implementation report from Coder Pod `yoi-coder-00001KV5W3PHA`: + +Commit: +- `05a9c522 feat: register plugin tool surfaces` + +Changed files: +- `crates/manifest/src/plugin.rs` +- `crates/manifest/src/lib.rs` +- `crates/manifest/src/config.rs` +- `crates/llm-worker/src/tool.rs` +- `crates/pod/src/feature.rs` +- `crates/pod/src/feature/plugin.rs` +- `crates/pod/src/controller.rs` +- `crates/pod/src/pod.rs` + +Implemented behavior: +- Added minimal Plugin package `tools` manifest surface parsing: + - `name` + - `description` + - `input_schema` +- Added `feature.plugins` profile/manifest gate, default disabled. +- Added Plugin Tool feature adapter: + - only enabled resolved Plugin packages with enabled Tool surface register schemas; + - package discovery alone does not register Tools; + - disabled profile feature suppresses Plugin Tool schema surface. +- Registered Plugin Tool definitions into existing ToolRegistry/model-visible schema path. +- Added host-side `ToolOrigin` metadata on `ToolMeta`: + - plugin id/ref + - source + - digest + - package version/API + - `surface = "tool"` +- Added runtime-missing stub executor: + - Tool schema is visible when registered; + - actual calls return safe unavailable/runtime-missing error; + - no Plugin code execution or WASM runtime added. +- Added registration-time fail-closed checks: + - duplicate Plugin Tool names; + - collision with already registered builtin Tool names; + - invalid/unsupported input schemas. +- Adjusted feature install ordering so builtin/pending tools are flushed before Plugin feature registration, allowing Plugin collisions against already registered Tool names to be diagnosed instead of reaching later duplicate panic. + +Diagnostics behavior: +- Duplicate Tool contributions produce feature install diagnostics and skip duplicate registration. +- Invalid Plugin input schema produces feature install diagnostics and registers no stub for that invalid Tool. +- Runtime calls before executor support return safe unavailable/runtime-missing `ToolError`. + +Validation reported by coder: +- Passed: `cargo test -p manifest plugin --no-default-features` +- Passed: `cargo test -p pod plugin::tests --no-default-features` +- Passed: `cargo check -p pod -p manifest -p llm-worker` +- Passed: `cargo fmt --check` +- Passed: `git diff --check` +- Attempted broader validation: `cargo test -p pod --no-default-features` + - Failed in existing prompt text assertions unrelated to Plugin Tool surface implementation: + - `prompt::tests::default_subagent_prompt_matches_resource` + - `prompt::tests::subagent_prompt_treats_paths_as_data` + +Not run: +- `nix build .#yoi` — no dependency, `Cargo.lock`, resource, or packaging changes. + +Residual risks / blockers: +- Plugin executor is intentionally runtime-missing stub; actual WASM/runtime execution remains for later Ticket. +- Input schema validation is intentionally a narrow model-visible shape check, not a full JSON Schema engine. Unsupported composition/reference keywords are rejected fail-closed. + +--- From d9b986853f781678bf8f1ace8bab6d4b58188a58 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:26:44 +0900 Subject: [PATCH 4/8] ticket: request plugin tool schema changes --- .yoi/tickets/00001KV5W3PHA/item.md | 2 +- .yoi/tickets/00001KV5W3PHA/thread.md | 60 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/.yoi/tickets/00001KV5W3PHA/item.md b/.yoi/tickets/00001KV5W3PHA/item.md index 814dd04b..7fb78c30 100644 --- a/.yoi/tickets/00001KV5W3PHA/item.md +++ b/.yoi/tickets/00001KV5W3PHA/item.md @@ -2,7 +2,7 @@ title: 'Plugin: register enabled Tool surface from packages' state: 'inprogress' created_at: '2026-06-15T14:48:59Z' -updated_at: '2026-06-15T16:20:05Z' +updated_at: '2026-06-15T16:26:34Z' assignee: null readiness: 'implementation_ready' risk_flags: ['plugin', 'tool-registry', 'model-visible-schema', 'capability-boundary', 'profile-config'] diff --git a/.yoi/tickets/00001KV5W3PHA/thread.md b/.yoi/tickets/00001KV5W3PHA/thread.md index fe2600f5..c8bb05dc 100644 --- a/.yoi/tickets/00001KV5W3PHA/thread.md +++ b/.yoi/tickets/00001KV5W3PHA/thread.md @@ -169,3 +169,63 @@ Residual risks / blockers: - Input schema validation is intentionally a narrow model-visible shape check, not a full JSON Schema engine. Unsupported composition/reference keywords are rejected fail-closed. --- + + + +## Review: request changes + +Review result: request_changes + +Blocker: +- Invalid nested Plugin Tool schemas can still become model-visible. + +Evidence: +- `crates/pod/src/feature/plugin.rs` validates root-level schema shape and recursively rejects only selected unsupported keywords. +- It does not validate that nested schema nodes are valid JSON Schema objects. +- Example invalid schema that appears to pass current validation: + +```toml +input_schema = { + type = "object", + properties = { query = "not-a-schema" }, + required = ["query"], + additionalProperties = false +} +``` + +Why this is a blocker: +- `properties.query = "not-a-schema"` is not a valid schema node. +- Current checks can allow it into ToolRegistry / model-visible tool schema path. +- This violates the acceptance criteria that invalid input schema is rejected with diagnostic and the critical focus on model-visible schema gating. + +Required changes: +- Strengthen Plugin Tool schema validation before registration so invalid nested schemas fail closed with diagnostics. +- Add regression tests showing nested invalid schema nodes do not register a Plugin Tool. +- Prefer defining/enforcing an explicit supported subset recursively, for example: + - validate property schemas; + - validate allowed `type` values; + - validate `required` string/unique array; + - validate or reject `items` / `additionalProperties` recursively; + - reject unknown/problematic keywords unless explicitly supported. + +Positive evidence checked: +- Plugin tool registration is feature-gated by `feature_config.plugins.enabled`. +- Discovery-only packages do not register tools because registration uses resolved enabled Plugin records/surfaces. +- Registered Plugin Tools preserve `ToolOrigin::Plugin` metadata. +- Builtin collisions and duplicate Plugin Tool names are diagnosed/rejected. +- Runtime execution is a safe unavailable/runtime-missing stub. +- No Plugin code execution, WASM runtime, host API, Service/Ingress/MCP bridge, or permission grant enforcement was added. +- `ToolOrigin` is not exposed directly to model-visible schema. + +Validation performed by reviewer: +- Passed: `git diff --check fcae8860..05a9c522` +- Passed: `cargo fmt --check` +- Worktree remained clean. + +Validation not run: +- `cargo test` / `cargo check` were not rerun because reviewer scope was read-only. + +Conclusion: +- Changes requested. Do not integrate until nested schema validation is fail-closed and covered. + +--- From 3413bae7d7e0db23e4595e254b56ea0607968a24 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:30:00 +0900 Subject: [PATCH 5/8] fix: reject nested plugin tool schema errors --- crates/pod/src/feature/plugin.rs | 297 +++++++++++++++++++++++++------ 1 file changed, 239 insertions(+), 58 deletions(-) diff --git a/crates/pod/src/feature/plugin.rs b/crates/pod/src/feature/plugin.rs index 6133289c..f6fc75f6 100644 --- a/crates/pod/src/feature/plugin.rs +++ b/crates/pod/src/feature/plugin.rs @@ -197,68 +197,177 @@ fn validate_tool_name(name: &str) -> Result<(), &'static str> { Ok(()) } -fn validate_input_schema(schema: &Value) -> Result<(), String> { - let Value::Object(root) = schema else { - return Err("root schema must be a JSON object".into()); - }; - match root.get("type") { - Some(Value::String(value)) if value == "object" => {} - Some(_) => return Err("root schema type must be `object`".into()), - None => return Err("root schema must declare type = `object`".into()), - } - if let Some(properties) = root.get("properties") { - if !properties.is_object() { - return Err("properties must be a JSON object".into()); - } - } - if let Some(required) = root.get("required") { - let Some(required) = required.as_array() else { - return Err("required must be an array".into()); - }; - if !required.iter().all(Value::is_string) { - return Err("required entries must be strings".into()); - } - } - if let Some(additional) = root.get("additionalProperties") { - if !(additional.is_boolean() || additional.is_object()) { - return Err("additionalProperties must be boolean or object".into()); - } - } - reject_unsupported_keywords(schema) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum SupportedSchemaType { + Object, + Array, + String, + Number, + Integer, + Boolean, + Null, } -fn reject_unsupported_keywords(schema: &Value) -> Result<(), String> { - match schema { - Value::Object(map) => { - for (key, value) in map { - if matches!( - key.as_str(), - "$ref" - | "$dynamicRef" - | "oneOf" - | "anyOf" - | "allOf" - | "not" - | "patternProperties" - | "dependentSchemas" - | "dependencies" - ) { - return Err(format!("unsupported schema keyword `{key}`")); - } - reject_unsupported_keywords(value)?; - } - Ok(()) +impl SupportedSchemaType { + fn parse(value: &str) -> Option { + match value { + "object" => Some(Self::Object), + "array" => Some(Self::Array), + "string" => Some(Self::String), + "number" => Some(Self::Number), + "integer" => Some(Self::Integer), + "boolean" => Some(Self::Boolean), + "null" => Some(Self::Null), + _ => None, } - Value::Array(values) => { - for value in values { - reject_unsupported_keywords(value)?; - } - Ok(()) - } - _ => Ok(()), } } +fn validate_input_schema(schema: &Value) -> Result<(), String> { + let ty = validate_schema_node(schema, "$", true)?; + if ty != SupportedSchemaType::Object { + return Err("root schema type must be `object`".into()); + } + Ok(()) +} + +fn validate_schema_node( + schema: &Value, + path: &str, + root: bool, +) -> Result { + let Value::Object(map) = schema else { + return Err(format!("{path}: schema node must be a JSON object")); + }; + + for key in map.keys() { + if !is_supported_schema_keyword(key) { + return Err(format!("{path}: unsupported schema keyword `{key}`")); + } + } + + let ty = match map.get("type") { + Some(Value::String(value)) => SupportedSchemaType::parse(value) + .ok_or_else(|| format!("{path}: unsupported schema type `{value}`"))?, + Some(_) => return Err(format!("{path}: type must be a string")), + None if root => return Err("root schema must declare type = `object`".into()), + None => return Err(format!("{path}: schema node must declare type")), + }; + + if let Some(title) = map.get("title") { + if !title.is_string() { + return Err(format!("{path}: title must be a string")); + } + } + if let Some(description) = map.get("description") { + if !description.is_string() { + return Err(format!("{path}: description must be a string")); + } + } + + let properties = map.get("properties"); + if let Some(properties) = properties { + if ty != SupportedSchemaType::Object { + return Err(format!( + "{path}: properties is only supported for object schemas" + )); + } + let Some(properties) = properties.as_object() else { + return Err(format!("{path}: properties must be a JSON object")); + }; + for (name, child_schema) in properties { + validate_schema_node(child_schema, &format!("{path}.properties.{name}"), false)?; + } + } + + if let Some(required) = map.get("required") { + if ty != SupportedSchemaType::Object { + return Err(format!( + "{path}: required is only supported for object schemas" + )); + } + let Some(required) = required.as_array() else { + return Err(format!("{path}: required must be an array")); + }; + let mut seen = HashSet::new(); + for entry in required { + let Some(name) = entry.as_str() else { + return Err(format!("{path}: required entries must be strings")); + }; + if !seen.insert(name) { + return Err(format!("{path}: required entries must be unique")); + } + if let Some(properties) = properties.and_then(Value::as_object) { + if !properties.contains_key(name) { + return Err(format!( + "{path}: required entry `{name}` is not declared in properties" + )); + } + } + } + } + + if let Some(additional) = map.get("additionalProperties") { + if ty != SupportedSchemaType::Object { + return Err(format!( + "{path}: additionalProperties is only supported for object schemas" + )); + } + match additional { + Value::Bool(_) => {} + Value::Object(_) => { + validate_schema_node(additional, &format!("{path}.additionalProperties"), false)?; + } + _ => { + return Err(format!( + "{path}: additionalProperties must be boolean or schema object" + )); + } + } + } + + if let Some(items) = map.get("items") { + if ty != SupportedSchemaType::Array { + return Err(format!("{path}: items is only supported for array schemas")); + } + validate_schema_node(items, &format!("{path}.items"), false)?; + } + + if let Some(enum_values) = map.get("enum") { + let Some(enum_values) = enum_values.as_array() else { + return Err(format!("{path}: enum must be an array")); + }; + if enum_values.is_empty() { + return Err(format!("{path}: enum must not be empty")); + } + for (index, value) in enum_values.iter().enumerate() { + if enum_values + .iter() + .skip(index + 1) + .any(|other| other == value) + { + return Err(format!("{path}: enum entries must be unique")); + } + } + } + + Ok(ty) +} + +fn is_supported_schema_keyword(key: &str) -> bool { + matches!( + key, + "type" + | "title" + | "description" + | "properties" + | "required" + | "additionalProperties" + | "items" + | "enum" + ) +} + #[cfg(test)] mod tests { use super::*; @@ -343,13 +452,64 @@ mod tests { ); } + #[test] + fn rejects_invalid_nested_property_schema_node() { + let schema = json!({ + "type":"object", + "properties":{"query":"not-a-schema"}, + "required":["query"], + "additionalProperties":false + }); + let error = validate_input_schema(&schema).unwrap_err(); + assert!(error.contains("$.properties.query")); + assert!(error.contains("schema node must be a JSON object")); + } + + #[test] + fn rejects_invalid_recursive_schema_members() { + let duplicate_required = json!({ + "type":"object", + "properties":{"query":{"type":"string"}}, + "required":["query", "query"] + }); + assert!( + validate_input_schema(&duplicate_required) + .unwrap_err() + .contains("required entries must be unique") + ); + + let invalid_items = json!({ + "type":"object", + "properties":{"values":{"type":"array", "items":"not-a-schema"}} + }); + assert!( + validate_input_schema(&invalid_items) + .unwrap_err() + .contains("$.properties.values.items") + ); + + let invalid_additional = json!({ + "type":"object", + "additionalProperties":{"type":"unsupported"} + }); + assert!( + validate_input_schema(&invalid_additional) + .unwrap_err() + .contains("unsupported schema type") + ); + } + #[test] fn accepts_object_tool_schema() { validate_input_schema(&json!({ "type":"object", - "properties":{"query":{"type":"string"}}, + "properties":{ + "query":{"type":"string", "description":"Search text"}, + "limit":{"type":"integer", "enum":[1, 5, 10]}, + "tags":{"type":"array", "items":{"type":"string"}} + }, "required":["query"], - "additionalProperties":false + "additionalProperties":{"type":"string"} })) .unwrap(); } @@ -463,6 +623,27 @@ mod tests { assert!(has_diagnostic(&report, "invalid input_schema")); } + #[test] + fn nested_invalid_input_schema_does_not_register_plugin_tool() { + let mut invalid = tool("BadNestedSchema"); + invalid.input_schema = json!({ + "type":"object", + "properties":{"query":"not-a-schema"}, + "required":["query"], + "additionalProperties":false + }); + let mut pending = Vec::new(); + let mut hooks = crate::hook::HookRegistryBuilder::new(); + + let report = super::super::FeatureRegistryBuilder::default() + .with_module(PluginToolFeature::new(record(vec![invalid]))) + .install_into_pending(&mut pending, &mut hooks); + + assert!(pending.is_empty()); + assert!(has_diagnostic(&report, "invalid input_schema")); + assert!(has_diagnostic(&report, "$.properties.query")); + } + #[tokio::test] async fn registered_tool_executes_as_runtime_missing_error() { let mut pending = Vec::new(); From fb44159261274cd4017069a41bf5edaa714e754b Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:31:00 +0900 Subject: [PATCH 6/8] ticket: record plugin tool schema fix --- .yoi/tickets/00001KV5W3PHA/item.md | 2 +- .yoi/tickets/00001KV5W3PHA/thread.md | 64 ++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/.yoi/tickets/00001KV5W3PHA/item.md b/.yoi/tickets/00001KV5W3PHA/item.md index 7fb78c30..d9837ce2 100644 --- a/.yoi/tickets/00001KV5W3PHA/item.md +++ b/.yoi/tickets/00001KV5W3PHA/item.md @@ -2,7 +2,7 @@ title: 'Plugin: register enabled Tool surface from packages' state: 'inprogress' created_at: '2026-06-15T14:48:59Z' -updated_at: '2026-06-15T16:26:34Z' +updated_at: '2026-06-15T16:30:49Z' assignee: null readiness: 'implementation_ready' risk_flags: ['plugin', 'tool-registry', 'model-visible-schema', 'capability-boundary', 'profile-config'] diff --git a/.yoi/tickets/00001KV5W3PHA/thread.md b/.yoi/tickets/00001KV5W3PHA/thread.md index c8bb05dc..32c2f94b 100644 --- a/.yoi/tickets/00001KV5W3PHA/thread.md +++ b/.yoi/tickets/00001KV5W3PHA/thread.md @@ -229,3 +229,67 @@ Conclusion: - Changes requested. Do not integrate until nested schema validation is fail-closed and covered. --- + + + +## Implementation report + +Follow-up implementation report after reviewer request_changes: + +New commit: +- `3413bae7 fix: reject nested plugin tool schema errors` + +Changed file: +- `crates/pod/src/feature/plugin.rs` + +Schema validation behavior updated: +- Replaced shallow root-only validation with recursive validation of an explicit supported subset. +- Every schema node must be a JSON object with supported string `type`. +- Supported `type` values: + - `object` + - `array` + - `string` + - `number` + - `integer` + - `boolean` + - `null` +- Supported keywords: + - `type` + - `title` + - `description` + - `properties` + - `required` + - `additionalProperties` + - `items` + - `enum` + +Recursive/fail-closed behavior: +- `properties.*` values are recursively validated as schema objects. +- `items` is only allowed on array schemas and recursively validates a schema object. +- `additionalProperties` is only allowed on object schemas and must be boolean or recursively valid schema object. +- Non-object nested schema nodes reject registration. +- Unknown/problematic keywords reject registration. +- Unsupported `type` values reject registration. +- `required` must be an array of unique strings. +- `required` entries must be declared in `properties` when `properties` exists. +- `enum` must be non-empty array with unique entries. + +Regression tests added: +- Direct validation rejects: + - `properties.query = "not-a-schema"` + - invalid recursive `items` + - invalid recursive `additionalProperties` + - duplicate `required` entries +- Registration test confirms nested invalid schema does not register a Plugin Tool and emits an `invalid input_schema` diagnostic with nested path. + +Validation reported by coder: +- Passed: `cargo test -p pod plugin::tests --no-default-features` +- Passed: `cargo test -p manifest plugin --no-default-features` +- Passed: `cargo check -p pod -p manifest -p llm-worker` +- Passed: `cargo fmt --check` +- Passed: `git diff --check` + +Repository status: +- Child implementation worktree clean after follow-up commit. + +--- From fb39bf38a163bafcc9afb014f70c1f86d5286440 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:38:46 +0900 Subject: [PATCH 7/8] ticket: approve plugin tool schema fix --- .yoi/tickets/00001KV5W3PHA/item.md | 2 +- .yoi/tickets/00001KV5W3PHA/thread.md | 61 ++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/.yoi/tickets/00001KV5W3PHA/item.md b/.yoi/tickets/00001KV5W3PHA/item.md index d9837ce2..391ea65d 100644 --- a/.yoi/tickets/00001KV5W3PHA/item.md +++ b/.yoi/tickets/00001KV5W3PHA/item.md @@ -2,7 +2,7 @@ title: 'Plugin: register enabled Tool surface from packages' state: 'inprogress' created_at: '2026-06-15T14:48:59Z' -updated_at: '2026-06-15T16:30:49Z' +updated_at: '2026-06-15T16:38:37Z' assignee: null readiness: 'implementation_ready' risk_flags: ['plugin', 'tool-registry', 'model-visible-schema', 'capability-boundary', 'profile-config'] diff --git a/.yoi/tickets/00001KV5W3PHA/thread.md b/.yoi/tickets/00001KV5W3PHA/thread.md index 32c2f94b..9491f7e6 100644 --- a/.yoi/tickets/00001KV5W3PHA/thread.md +++ b/.yoi/tickets/00001KV5W3PHA/thread.md @@ -293,3 +293,64 @@ Repository status: - Child implementation worktree clean after follow-up commit. --- + + + +## Review: approve + +Review result: approve + +Second-pass blocker verification: resolved. + +Evidence checked: +- Child worktree: + - `/home/hare/Projects/yoi/.worktree/00001KV5W3PHA-plugin-tool-surface` + - branch `impl/00001KV5W3PHA-plugin-tool-surface` + - HEAD `3413bae7 fix: reject nested plugin tool schema errors` +- Fix diff `05a9c522..3413bae7`. +- Relevant implementation: + - `crates/pod/src/feature/plugin.rs` + - `crates/pod/src/feature.rs` + - `crates/pod/src/controller.rs` + - `crates/llm-worker/src/tool.rs` + - `crates/llm-worker/src/tool_server.rs` + - `crates/manifest/src/plugin.rs` + +Blocker-specific findings: +- Nested invalid schemas are now rejected recursively. +- `validate_schema_node` requires every schema node to be a JSON object with supported string `type`. +- `properties` child schemas are recursively validated. +- `items` is only accepted on array schemas and recursively validated. +- `additionalProperties` is only accepted on object schemas and must be boolean or recursively valid schema object. +- `required` must be a unique string array and, when `properties` exists, entries must be declared. +- `enum` must be non-empty and unique. +- Unknown keywords are rejected before registration. +- The prior concrete failure case `properties.query = "not-a-schema"` now fails with path-bearing diagnostic such as `$.properties.query`. +- Regression test `nested_invalid_input_schema_does_not_register_plugin_tool` asserts pending tool registration remains empty and diagnostics include `invalid input_schema`. +- Invalid schemas return `FeatureInstallError::InvalidToolManifest`, recorded as install diagnostic by feature registry. +- No evidence found that invalid nested schemas become model-visible after the fix. + +Overall acceptance spot-check: +- Enabled Plugin Tool definitions are wired through normal `FeatureRegistryBuilder` / `ToolRegistry` path. +- Discovery-only or non-tool packages remain inactive through manifest surface filtering. +- Duplicate/builtin collisions are handled by feature registry duplicate checks against registered/pending tool names. +- `ToolOrigin` metadata is preserved on `ToolMeta`. +- Feature/profile disabled plugin support hides Plugin Tool modules through `plugin_tool_features_if_enabled`. +- Runtime-missing calls use `PluginRuntimeMissingTool` and return safe unavailable error. +- No Plugin code execution, WASM runtime, host API, Service/Ingress/MCP bridge, or permission grant enforcement was added. + +Validation performed by reviewer: +- Passed: `git diff --check fcae8860..HEAD` +- Passed: `cargo fmt --check` + +Validation not rerun by reviewer: +- `cargo test` / `cargo check` were not rerun because reviewer scope was read-only. Coder-reported passing results were inspected as evidence. + +Risk assessment: +- Residual risk is mainly integration-level because reviewer did not run cargo tests. +- The fix is focused and directly covers the prior blocker with recursive validation and regression coverage. + +Conclusion: +- Approved. No remaining blocker found. + +--- From 0da2b5db7b9ec088e94f70251e2766aeda445140 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 16 Jun 2026 01:40:07 +0900 Subject: [PATCH 8/8] ticket: complete plugin tool surface --- .yoi/tickets/00001KV5W3PHA/item.md | 4 +-- .yoi/tickets/00001KV5W3PHA/thread.md | 46 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/.yoi/tickets/00001KV5W3PHA/item.md b/.yoi/tickets/00001KV5W3PHA/item.md index 391ea65d..7dd25711 100644 --- a/.yoi/tickets/00001KV5W3PHA/item.md +++ b/.yoi/tickets/00001KV5W3PHA/item.md @@ -1,8 +1,8 @@ --- title: 'Plugin: register enabled Tool surface from packages' -state: 'inprogress' +state: 'done' created_at: '2026-06-15T14:48:59Z' -updated_at: '2026-06-15T16:38:37Z' +updated_at: '2026-06-15T16:39:58Z' assignee: null readiness: 'implementation_ready' risk_flags: ['plugin', 'tool-registry', 'model-visible-schema', 'capability-boundary', 'profile-config'] diff --git a/.yoi/tickets/00001KV5W3PHA/thread.md b/.yoi/tickets/00001KV5W3PHA/thread.md index 9491f7e6..6f0aadf6 100644 --- a/.yoi/tickets/00001KV5W3PHA/thread.md +++ b/.yoi/tickets/00001KV5W3PHA/thread.md @@ -354,3 +354,49 @@ Conclusion: - Approved. No remaining blocker found. --- + + + +## Implementation report + +Integration report: + +Merged implementation branch into orchestration branch: +- branch: `impl/00001KV5W3PHA-plugin-tool-surface` +- implementation commits: + - `05a9c522 feat: register plugin tool surfaces` + - `3413bae7 fix: reject nested plugin tool schema errors` +- merge commit: `204d0d02 merge: plugin tool surface registration` + +Reviewer result: +- Initial review requested changes for nested invalid Plugin Tool schema validation. +- Second-pass review `approve` recorded from `yoi-reviewer-00001KV5W3PHA-r2`. + +Validation in Orchestrator worktree `/home/hare/Projects/yoi/.worktree/orchestration`: +- Passed: `cargo test -p pod plugin::tests --no-default-features` +- Passed: `cargo test -p manifest plugin --no-default-features` + - 15 passed +- Passed: `cargo check -p pod -p manifest -p llm-worker` +- Passed: `cargo fmt --check` +- Passed: `git diff --check` + +Notes: +- Broader `cargo test -p pod --no-default-features` was not rerun because coder had already reported existing unrelated prompt text assertion failures outside this change path. +- Plugin Tool registration is feature-gated and enablement-gated. +- Runtime execution remains an unavailable/runtime-missing stub; no Plugin code execution or WASM runtime was added. +- Recursive schema validation now rejects nested invalid schema nodes before model-visible registration. +- Orchestrator worktree is clean after validation. + +Cleanup planned: +- Stop related coder/reviewer Pods. +- Remove only child implementation worktree/branch for this Ticket. + +--- + + + +## State changed + +Reviewer approved after requested schema validation fix, implementation branch merged into the orchestration branch, and focused validation passed in the Orchestrator worktree. Marking Ticket done in the orchestration branch. + +---