From a8ae6ca2f8010f32a23439da8b7b6358380f0d12 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 5 Jun 2026 06:25:54 +0900 Subject: [PATCH 1/4] feat: add pod feature registry slice --- crates/pod/src/controller.rs | 12 +- crates/pod/src/feature.rs | 1424 ++++++++++++++++++++++++++++++++++ crates/pod/src/lib.rs | 1 + crates/pod/src/pod.rs | 10 + crates/tools/src/lib.rs | 24 +- 5 files changed, 1461 insertions(+), 10 deletions(-) create mode 100644 crates/pod/src/feature.rs diff --git a/crates/pod/src/controller.rs b/crates/pod/src/controller.rs index 5cd1907b..79046c6e 100644 --- a/crates/pod/src/controller.rs +++ b/crates/pod/src/controller.rs @@ -9,6 +9,7 @@ use session_store::Store; use tokio::sync::{broadcast, mpsc, oneshot}; use crate::discovery::{PodDiscovery, list_pods_tool, restore_pod_tool, send_to_peer_pod_tool}; +use crate::feature::{FeatureRegistryBuilder, builtin::task_feature}; use crate::ipc::alerter::Alerter; use crate::ipc::notify_buffer::NotifyBuffer; use crate::ipc::server::SocketServer; @@ -502,8 +503,6 @@ where let pod_store = pod.store().clone(); let self_parent_socket = pod.callback_socket().cloned(); - let worker = pod.worker_mut(); - // The Pod's SharedScope (already augmented with the bash-output // Read rule by the caller) is the single source of truth — every // ScopedFs (builtin tools, fs_view, compact worker) reads from it, @@ -515,14 +514,19 @@ where // a clone for the FS view we attach below, since the tools consume // `fs` itself. let fs_for_view = fs.clone(); - worker.register_tools(tools::builtin_tools( + pod.worker_mut().register_tools(tools::core_builtin_tools( fs, tracker.clone(), - task_store, bash_output_dir, web_config, )); + let mut feature_registry = FeatureRegistryBuilder::new(); + feature_registry.add_module(task_feature(task_store)); + let _feature_install_report = pod.install_features(feature_registry); + + let worker = pod.worker_mut(); + // Memory subsystem opt-in. When `[memory]` is present in the // manifest, register the memory-specific Read/Write/Edit tools that // target `/memory/` and `/knowledge/` with diff --git a/crates/pod/src/feature.rs b/crates/pod/src/feature.rs new file mode 100644 index 00000000..10520fb2 --- /dev/null +++ b/crates/pod/src/feature.rs @@ -0,0 +1,1424 @@ +//! Feature contribution registry for Pod-hosted builtin/plugin modules. +//! +//! This module defines the Pod-side feature boundary used to collect +//! descriptor metadata, capability requests, tool contributions, safe hook +//! contributions, background task declarations, and service declarations before +//! installing them into the existing Worker/HookRegistry host surfaces. +//! +//! The first implementation slice is intentionally host-mediated and +//! descriptor-first: tools are installed through the normal Worker tool path, +//! hooks are installed through [`crate::hook::HookRegistryBuilder`], while +//! service and background-task contributions are represented in descriptors and +//! install reports without starting an independent runtime lifecycle. + +use std::collections::{HashMap, HashSet}; +use std::fmt; +use std::sync::Arc; + +use llm_worker::Worker; +use llm_worker::llm_client::client::LlmClient; +use llm_worker::state::Mutable; +use llm_worker::tool::ToolDefinition; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +use crate::hook::{Hook, HookRegistryBuilder, OnTurnEnd, PostToolCall, PreLlmRequest, PreToolCall}; + +/// Stable source-qualified identifier for a feature module. +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] +pub struct FeatureId(String); + +impl FeatureId { + pub fn new(value: impl Into) -> Result { + let value = value.into(); + if value.trim().is_empty() { + return Err(FeatureInstallError::InvalidDescriptor( + "feature id must not be empty".into(), + )); + } + Ok(Self(value)) + } + + pub fn builtin(slug: impl AsRef) -> Self { + Self(format!("builtin:{}", slug.as_ref())) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl fmt::Display for FeatureId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +impl From for String { + fn from(value: FeatureId) -> Self { + value.0 + } +} + +/// Runtime/source class for a feature module. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FeatureRuntimeKind { + Builtin, + LuaProfile, + ExternalPlugin, +} + +/// Host capability requested by a feature before it contributes host-visible +/// behavior. Grants are additive and do not replace manifest/tool permission +/// checks. +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum HostCapability { + ContributeTool { name: String }, + ContributeHook { point: FeatureHookPoint }, + DeclareBackgroundTask { name: String }, + ProvideService { service: ServiceId }, + RequireService { service: ServiceId }, + EmitNotification, + EmitAlert, + EmitDiagnostic, +} + +/// A safe hook contribution point exposed to feature modules. +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FeatureHookPoint { + PreRequest, + PreToolCall, + ToolResult, + TurnEnd, +} + +/// Capability request declared by a feature descriptor. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct CapabilityRequest { + pub capability: HostCapability, + pub required: bool, + pub reason: String, +} + +impl CapabilityRequest { + pub fn required(capability: HostCapability, reason: impl Into) -> Self { + Self { + capability, + required: true, + reason: reason.into(), + } + } + + pub fn optional(capability: HostCapability, reason: impl Into) -> Self { + Self { + capability, + required: false, + reason: reason.into(), + } + } +} + +/// Capability grants resolved by the host for one feature installation. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct CapabilityGrantSet { + granted: HashSet, + denied: Vec, +} + +impl CapabilityGrantSet { + pub fn grant_all(requests: &[CapabilityRequest]) -> Self { + Self { + granted: requests + .iter() + .map(|request| request.capability.clone()) + .collect(), + denied: Vec::new(), + } + } + + pub fn empty() -> Self { + Self::default() + } + + pub fn contains(&self, capability: &HostCapability) -> bool { + self.granted.contains(capability) + } + + pub fn denied(&self) -> &[CapabilityDenial] { + &self.denied + } + + pub fn grant(&mut self, capability: HostCapability) { + self.granted.insert(capability); + } + + pub fn deny(&mut self, capability: HostCapability, reason: impl Into) { + self.granted.remove(&capability); + self.denied.push(CapabilityDenial { + capability, + reason: reason.into(), + }); + } +} + +/// Host-side denial of a requested feature capability. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct CapabilityDenial { + pub capability: HostCapability, + pub reason: String, +} + +/// Serializable declaration of a tool contribution. The executable factory is +/// carried by [`ToolContribution`] during installation. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ToolDeclaration { + pub name: String, + pub description: String, + pub required_capabilities: Vec, +} + +impl ToolDeclaration { + pub fn new(name: impl Into, description: impl Into) -> Self { + let name = name.into(); + Self { + required_capabilities: vec![HostCapability::ContributeTool { name: name.clone() }], + name, + description: description.into(), + } + } +} + +/// Executable tool contribution wrapper. +pub struct ToolContribution { + name: String, + definition: ToolDefinition, + required_capabilities: Vec, +} + +impl ToolContribution { + pub fn new(name: impl Into, definition: ToolDefinition) -> Self { + let name = name.into(); + Self { + required_capabilities: vec![HostCapability::ContributeTool { name: name.clone() }], + name, + definition, + } + } + + pub fn with_required_capabilities( + mut self, + required_capabilities: Vec, + ) -> Self { + self.required_capabilities = required_capabilities; + self + } + + pub fn name(&self) -> &str { + &self.name + } +} + +/// Serializable declaration of a hook contribution. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct HookDeclaration { + pub name: String, + pub point: FeatureHookPoint, + pub required_capabilities: Vec, +} + +impl HookDeclaration { + pub fn new(name: impl Into, point: FeatureHookPoint) -> Self { + Self { + name: name.into(), + required_capabilities: vec![HostCapability::ContributeHook { + point: point.clone(), + }], + point, + } + } +} + +/// Background task lifecycle phase represented by this registry slice. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum BackgroundTaskLifecycle { + DescriptorOnly, + HostManaged, +} + +/// Declaration for a feature-provided background task. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct BackgroundTaskDeclaration { + pub name: String, + pub description: String, + pub lifecycle: BackgroundTaskLifecycle, + pub required_capabilities: Vec, +} + +impl BackgroundTaskDeclaration { + pub fn descriptor_only(name: impl Into, description: impl Into) -> Self { + let name = name.into(); + Self { + required_capabilities: vec![HostCapability::DeclareBackgroundTask { + name: name.clone(), + }], + name, + description: description.into(), + lifecycle: BackgroundTaskLifecycle::DescriptorOnly, + } + } +} + +/// Source-qualified service identifier. +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] +pub struct ServiceId(String); + +impl ServiceId { + pub fn new(value: impl Into) -> Result { + let value = value.into(); + if value.trim().is_empty() { + return Err(FeatureInstallError::InvalidDescriptor( + "service id must not be empty".into(), + )); + } + Ok(Self(value)) + } + + pub fn builtin(slug: impl AsRef) -> Self { + Self(format!("builtin:{}", slug.as_ref())) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl fmt::Display for ServiceId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +/// Minimal version requirement placeholder for service resolution. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ServiceVersionReq { + pub requirement: String, +} + +impl ServiceVersionReq { + pub fn any() -> Self { + Self { + requirement: "*".into(), + } + } +} + +/// Feature-provided service declaration. This first slice records provider +/// metadata and supports requirement matching; it does not expose concrete +/// provider objects across feature boundaries. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ServiceDeclaration { + pub id: ServiceId, + pub version: String, + pub description: String, +} + +impl ServiceDeclaration { + pub fn new(id: ServiceId, version: impl Into, description: impl Into) -> Self { + Self { + id, + version: version.into(), + description: description.into(), + } + } +} + +/// Feature service requirement used for host-mediated dependency resolution. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ServiceRequirement { + pub id: ServiceId, + pub version: ServiceVersionReq, + pub required: bool, + pub reason: String, +} + +impl ServiceRequirement { + pub fn required(id: ServiceId, reason: impl Into) -> Self { + Self { + id, + version: ServiceVersionReq::any(), + required: true, + reason: reason.into(), + } + } + + pub fn optional(id: ServiceId, reason: impl Into) -> Self { + Self { + id, + version: ServiceVersionReq::any(), + required: false, + reason: reason.into(), + } + } +} + +/// Host-mediated service registry skeleton used during feature installation. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureServiceRegistry { + providers: HashMap, +} + +impl FeatureServiceRegistry { + pub fn providers(&self) -> &HashMap { + &self.providers + } + + pub fn provides(&self, id: &ServiceId) -> bool { + self.providers.contains_key(id) + } + + fn register_provider( + &mut self, + feature_id: FeatureId, + declaration: ServiceDeclaration, + ) -> Result<(), FeatureInstallError> { + if let Some(existing) = self.providers.get(&declaration.id) { + return Err(FeatureInstallError::DuplicateService { + service: declaration.id.to_string(), + first_feature: existing.feature_id.to_string(), + duplicate_feature: feature_id.to_string(), + }); + } + self.providers.insert( + declaration.id.clone(), + FeatureServiceProvider { + feature_id, + declaration, + }, + ); + Ok(()) + } +} + +/// Provider metadata for one service declaration. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureServiceProvider { + pub feature_id: FeatureId, + pub declaration: ServiceDeclaration, +} + +/// Feature descriptor advertised before installation. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureDescriptor { + pub id: FeatureId, + pub runtime: FeatureRuntimeKind, + pub display_name: String, + pub version: String, + pub description: String, + pub requested_capabilities: Vec, + pub tools: Vec, + pub hooks: Vec, + pub background_tasks: Vec, + pub provides_services: Vec, + pub requires_services: Vec, +} + +impl FeatureDescriptor { + pub fn builtin(id: impl AsRef, display_name: impl Into) -> Self { + Self { + id: FeatureId::builtin(id), + runtime: FeatureRuntimeKind::Builtin, + display_name: display_name.into(), + version: env!("CARGO_PKG_VERSION").into(), + description: String::new(), + requested_capabilities: Vec::new(), + tools: Vec::new(), + hooks: Vec::new(), + background_tasks: Vec::new(), + provides_services: Vec::new(), + requires_services: Vec::new(), + } + } + + pub fn with_description(mut self, description: impl Into) -> Self { + self.description = description.into(); + self + } + + pub fn with_capability(mut self, request: CapabilityRequest) -> Self { + self.requested_capabilities.push(request); + self + } + + pub fn with_tool(mut self, tool: ToolDeclaration) -> Self { + self.tools.push(tool); + self + } + + pub fn with_hook(mut self, hook: HookDeclaration) -> Self { + self.hooks.push(hook); + self + } + + pub fn with_background_task(mut self, task: BackgroundTaskDeclaration) -> Self { + self.background_tasks.push(task); + self + } + + pub fn with_provided_service(mut self, service: ServiceDeclaration) -> Self { + self.provides_services.push(service); + self + } + + pub fn with_service_requirement(mut self, requirement: ServiceRequirement) -> Self { + self.requires_services.push(requirement); + self + } +} + +/// Feature module contribution boundary. +pub trait FeatureModule: Send + Sync { + fn descriptor(&self) -> FeatureDescriptor; + fn install(&self, context: &mut FeatureInstallContext<'_>) -> Result<(), FeatureInstallError>; +} + +/// Severity for feature installation diagnostics. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FeatureDiagnosticSeverity { + Info, + Warning, + Error, +} + +/// Installation diagnostic emitted by the feature host or feature module. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureDiagnostic { + pub severity: FeatureDiagnosticSeverity, + pub message: String, +} + +impl FeatureDiagnostic { + pub fn info(message: impl Into) -> Self { + Self { + severity: FeatureDiagnosticSeverity::Info, + message: message.into(), + } + } + + pub fn warning(message: impl Into) -> Self { + Self { + severity: FeatureDiagnosticSeverity::Warning, + message: message.into(), + } + } + + pub fn error(message: impl Into) -> Self { + Self { + severity: FeatureDiagnosticSeverity::Error, + message: message.into(), + } + } +} + +/// Kind of contribution represented in install reports. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FeatureContributionKind { + Tool, + Hook, + BackgroundTask, + Service, + Notification, + Alert, + Diagnostic, +} + +/// A contribution intentionally skipped by the host. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct SkippedContribution { + pub kind: FeatureContributionKind, + pub name: String, + pub reason: String, +} + +/// Per-feature installation report. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureInstallReport { + pub feature_id: FeatureId, + pub runtime: FeatureRuntimeKind, + pub installed: bool, + pub granted_capabilities: CapabilityGrantSet, + pub installed_tools: Vec, + pub installed_hooks: Vec, + pub declared_background_tasks: Vec, + pub provided_services: Vec, + pub resolved_service_requirements: Vec, + pub skipped: Vec, + pub diagnostics: Vec, +} + +impl FeatureInstallReport { + fn new(descriptor: &FeatureDescriptor, granted_capabilities: CapabilityGrantSet) -> Self { + Self { + feature_id: descriptor.id.clone(), + runtime: descriptor.runtime.clone(), + installed: false, + granted_capabilities, + installed_tools: Vec::new(), + installed_hooks: Vec::new(), + declared_background_tasks: Vec::new(), + provided_services: Vec::new(), + resolved_service_requirements: Vec::new(), + skipped: Vec::new(), + diagnostics: Vec::new(), + } + } + + fn mark_skipped( + &mut self, + kind: FeatureContributionKind, + name: impl Into, + reason: impl Into, + ) { + self.skipped.push(SkippedContribution { + kind, + name: name.into(), + reason: reason.into(), + }); + } +} + +/// Model-visible durable notification sink skeleton. The first slice exposes +/// the boundary without implementing a new event channel. +pub struct FeatureNotificationSink<'a> { + report: &'a mut FeatureInstallReport, +} + +impl FeatureNotificationSink<'_> { + pub fn notify_model(&mut self, message: impl Into) { + let message = message.into(); + self.report.diagnostics.push(FeatureDiagnostic::warning(format!( + "model notification requested during feature installation but no durable Notify host is attached: {message}" + ))); + self.report.mark_skipped( + FeatureContributionKind::Notification, + "notify_model", + "durable Notify/SystemItem host is not connected during feature installation", + ); + } +} + +/// Transient human-facing alert sink skeleton. +pub struct FeatureAlertSink<'a> { + report: &'a mut FeatureInstallReport, +} + +impl FeatureAlertSink<'_> { + pub fn alert(&mut self, message: impl Into) { + let message = message.into(); + self.report + .diagnostics + .push(FeatureDiagnostic::info(format!("feature alert: {message}"))); + self.report.mark_skipped( + FeatureContributionKind::Alert, + "alert", + "transient alert host is not connected during feature installation", + ); + } +} + +/// Diagnostic sink available to feature installers. +pub struct FeatureDiagnosticSink<'a> { + report: &'a mut FeatureInstallReport, +} + +impl FeatureDiagnosticSink<'_> { + pub fn push(&mut self, diagnostic: FeatureDiagnostic) { + self.report.diagnostics.push(diagnostic); + } + + pub fn info(&mut self, message: impl Into) { + self.push(FeatureDiagnostic::info(message)); + } + + pub fn warning(&mut self, message: impl Into) { + self.push(FeatureDiagnostic::warning(message)); + } + + pub fn error(&mut self, message: impl Into) { + self.push(FeatureDiagnostic::error(message)); + } +} + +/// Tool contribution registrar exposed inside [`FeatureInstallContext`]. +pub struct ToolContributionRegistrar<'a> { + feature_id: &'a FeatureId, + grants: &'a CapabilityGrantSet, + pending_tools: &'a mut Vec, + installed_tool_names: &'a mut HashMap, + report: &'a mut FeatureInstallReport, +} + +impl ToolContributionRegistrar<'_> { + pub fn register(&mut self, contribution: ToolContribution) -> Result<(), FeatureInstallError> { + for capability in &contribution.required_capabilities { + if !self.grants.contains(capability) { + let reason = format!("required capability was not granted: {capability:?}"); + self.report.mark_skipped( + FeatureContributionKind::Tool, + contribution.name.clone(), + reason.clone(), + ); + return Err(FeatureInstallError::CapabilityDenied(reason)); + } + } + + if let Some(first) = self.installed_tool_names.get(&contribution.name) { + let error = FeatureInstallError::DuplicateToolName { + tool: contribution.name.clone(), + first_feature: first.to_string(), + duplicate_feature: self.feature_id.to_string(), + }; + self.report.mark_skipped( + FeatureContributionKind::Tool, + contribution.name.clone(), + error.to_string(), + ); + return Err(error); + } + + self.installed_tool_names + .insert(contribution.name.clone(), self.feature_id.clone()); + self.report.installed_tools.push(contribution.name); + self.pending_tools.push(contribution.definition); + Ok(()) + } +} + +/// Safe hook contribution registrar backed by [`HookRegistryBuilder`]. +pub struct HookContributionRegistrar<'a> { + grants: &'a CapabilityGrantSet, + hook_builder: &'a mut HookRegistryBuilder, + report: &'a mut FeatureInstallReport, +} + +impl HookContributionRegistrar<'_> { + pub fn add_pre_request( + &mut self, + name: impl Into, + hook: impl Hook + 'static, + ) -> Result<(), FeatureInstallError> { + let declaration = HookDeclaration::new(name, FeatureHookPoint::PreRequest); + self.require_hook_capability(&declaration)?; + self.hook_builder.add_pre_llm_request(hook); + self.report.installed_hooks.push(declaration); + Ok(()) + } + + pub fn add_pre_tool_call( + &mut self, + name: impl Into, + hook: impl Hook + 'static, + ) -> Result<(), FeatureInstallError> { + let declaration = HookDeclaration::new(name, FeatureHookPoint::PreToolCall); + self.require_hook_capability(&declaration)?; + self.hook_builder.add_pre_tool_call(hook); + self.report.installed_hooks.push(declaration); + Ok(()) + } + + pub fn add_tool_result( + &mut self, + name: impl Into, + hook: impl Hook + 'static, + ) -> Result<(), FeatureInstallError> { + let declaration = HookDeclaration::new(name, FeatureHookPoint::ToolResult); + self.require_hook_capability(&declaration)?; + self.hook_builder.add_post_tool_call(hook); + self.report.installed_hooks.push(declaration); + Ok(()) + } + + pub fn add_turn_end( + &mut self, + name: impl Into, + hook: impl Hook + 'static, + ) -> Result<(), FeatureInstallError> { + let declaration = HookDeclaration::new(name, FeatureHookPoint::TurnEnd); + self.require_hook_capability(&declaration)?; + self.hook_builder.add_on_turn_end(hook); + self.report.installed_hooks.push(declaration); + Ok(()) + } + + fn require_hook_capability( + &mut self, + declaration: &HookDeclaration, + ) -> Result<(), FeatureInstallError> { + for capability in &declaration.required_capabilities { + if !self.grants.contains(capability) { + let reason = format!("required capability was not granted: {capability:?}"); + self.report.mark_skipped( + FeatureContributionKind::Hook, + declaration.name.clone(), + reason.clone(), + ); + return Err(FeatureInstallError::CapabilityDenied(reason)); + } + } + Ok(()) + } +} + +/// Background task registrar for descriptor/report-only contributions. +pub struct BackgroundTaskRegistrar<'a> { + report: &'a mut FeatureInstallReport, +} + +impl BackgroundTaskRegistrar<'_> { + pub fn declare(&mut self, declaration: BackgroundTaskDeclaration) { + self.report.declared_background_tasks.push(declaration); + } +} + +/// Service registrar for descriptor/report-only provider metadata. +pub struct FeatureServiceRegistrar<'a> { + feature_id: &'a FeatureId, + service_registry: &'a mut FeatureServiceRegistry, + report: &'a mut FeatureInstallReport, +} + +impl FeatureServiceRegistrar<'_> { + pub fn provide(&mut self, declaration: ServiceDeclaration) -> Result<(), FeatureInstallError> { + self.service_registry + .register_provider(self.feature_id.clone(), declaration.clone())?; + self.report.provided_services.push(declaration); + Ok(()) + } +} + +/// Install-time context provided to a feature module. +pub struct FeatureInstallContext<'a> { + feature_id: &'a FeatureId, + grants: &'a CapabilityGrantSet, + pending_tools: &'a mut Vec, + installed_tool_names: &'a mut HashMap, + hook_builder: &'a mut HookRegistryBuilder, + service_registry: &'a mut FeatureServiceRegistry, + report: &'a mut FeatureInstallReport, +} + +impl FeatureInstallContext<'_> { + pub fn feature_id(&self) -> &FeatureId { + self.feature_id + } + + pub fn grants(&self) -> &CapabilityGrantSet { + self.grants + } + + pub fn tools(&mut self) -> ToolContributionRegistrar<'_> { + ToolContributionRegistrar { + feature_id: self.feature_id, + grants: self.grants, + pending_tools: self.pending_tools, + installed_tool_names: self.installed_tool_names, + report: self.report, + } + } + + pub fn hooks(&mut self) -> HookContributionRegistrar<'_> { + HookContributionRegistrar { + grants: self.grants, + hook_builder: self.hook_builder, + report: self.report, + } + } + + pub fn background_tasks(&mut self) -> BackgroundTaskRegistrar<'_> { + BackgroundTaskRegistrar { + report: self.report, + } + } + + pub fn services(&mut self) -> FeatureServiceRegistrar<'_> { + FeatureServiceRegistrar { + feature_id: self.feature_id, + service_registry: self.service_registry, + report: self.report, + } + } + + pub fn notifications(&mut self) -> FeatureNotificationSink<'_> { + FeatureNotificationSink { + report: self.report, + } + } + + pub fn alerts(&mut self) -> FeatureAlertSink<'_> { + FeatureAlertSink { + report: self.report, + } + } + + pub fn diagnostics(&mut self) -> FeatureDiagnosticSink<'_> { + FeatureDiagnosticSink { + report: self.report, + } + } +} + +/// Aggregate install output for a registry installation. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureRegistryInstallReport { + pub reports: Vec, + pub services: FeatureServiceRegistry, +} + +impl FeatureRegistryInstallReport { + pub fn installed_tool_names(&self) -> Vec { + self.reports + .iter() + .flat_map(|report| report.installed_tools.iter().cloned()) + .collect() + } +} + +/// Builder/installer for enabled feature modules. +#[derive(Default)] +pub struct FeatureRegistryBuilder { + modules: Vec>, +} + +impl FeatureRegistryBuilder { + pub fn new() -> Self { + Self::default() + } + + pub fn add_module(&mut self, module: M) -> &mut Self + where + M: FeatureModule + 'static, + { + self.modules.push(Arc::new(module)); + self + } + + pub fn with_module(mut self, module: M) -> Self + where + M: FeatureModule + 'static, + { + self.add_module(module); + self + } + + pub fn is_empty(&self) -> bool { + self.modules.is_empty() + } + + pub fn descriptors(&self) -> Vec { + self.modules + .iter() + .map(|module| module.descriptor()) + .collect() + } + + /// Install modules into the existing Worker tool path and hook builder. + pub(crate) fn install_into_worker( + self, + worker: &mut Worker, + hook_builder: &mut HookRegistryBuilder, + ) -> FeatureRegistryInstallReport { + let mut pending_tools = Vec::new(); + let report = self.install_into_pending(&mut pending_tools, hook_builder); + worker.register_tools(pending_tools); + report + } + + pub(crate) fn install_into_pending( + self, + pending_tools: &mut Vec, + hook_builder: &mut HookRegistryBuilder, + ) -> FeatureRegistryInstallReport { + let descriptors: Vec<_> = self + .modules + .iter() + .map(|module| module.descriptor()) + .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()) { + let grants = CapabilityGrantSet::grant_all(&descriptor.requested_capabilities); + let mut report = FeatureInstallReport::new(&descriptor, grants.clone()); + + if !seen_features.insert(descriptor.id.clone()) { + report.diagnostics.push(FeatureDiagnostic::error(format!( + "duplicate feature id: {}", + descriptor.id + ))); + report.mark_skipped( + FeatureContributionKind::Diagnostic, + descriptor.id.to_string(), + "duplicate feature id", + ); + reports.push(report); + continue; + } + + for capability in grants.denied() { + report.diagnostics.push(FeatureDiagnostic::warning(format!( + "capability denied: {:?}: {}", + capability.capability, capability.reason + ))); + } + + if let Some(reason) = missing_required_service(&descriptor, &service_registry) { + report + .diagnostics + .push(FeatureDiagnostic::error(reason.clone())); + report.mark_skipped( + FeatureContributionKind::Service, + descriptor.id.to_string(), + reason, + ); + reports.push(report); + continue; + } + + for requirement in descriptor.requires_services.iter().cloned() { + if service_registry.provides(&requirement.id) { + report.resolved_service_requirements.push(requirement); + } else if requirement.required { + // Already handled by missing_required_service. + } else { + report.diagnostics.push(FeatureDiagnostic::warning(format!( + "optional service requirement is not available: {}", + requirement.id + ))); + report.mark_skipped( + FeatureContributionKind::Service, + requirement.id.to_string(), + "optional service requirement is not available", + ); + } + } + + for background_task in descriptor.background_tasks.iter().cloned() { + report.declared_background_tasks.push(background_task); + } + + for service in descriptor.provides_services.iter().cloned() { + match service_registry.register_provider(descriptor.id.clone(), service.clone()) { + Ok(()) => report.provided_services.push(service), + Err(error) => { + report + .diagnostics + .push(FeatureDiagnostic::error(error.to_string())); + report.mark_skipped( + FeatureContributionKind::Service, + service.id.to_string(), + error.to_string(), + ); + } + } + } + + let install_result = { + let mut context = FeatureInstallContext { + feature_id: &descriptor.id, + grants: &grants, + pending_tools, + installed_tool_names: &mut installed_tool_names, + hook_builder, + service_registry: &mut service_registry, + report: &mut report, + }; + module.install(&mut context) + }; + + match install_result { + Ok(()) => report.installed = true, + Err(error) => { + report + .diagnostics + .push(FeatureDiagnostic::error(error.to_string())); + } + } + reports.push(report); + } + + FeatureRegistryInstallReport { + reports, + services: service_registry, + } + } +} + +fn missing_required_service( + descriptor: &FeatureDescriptor, + services: &FeatureServiceRegistry, +) -> Option { + descriptor + .requires_services + .iter() + .find(|requirement| requirement.required && !services.provides(&requirement.id)) + .map(|requirement| { + format!( + "required service requirement is not available: {}", + requirement.id + ) + }) +} + +/// Feature installation errors. +#[derive(Debug, Error)] +pub enum FeatureInstallError { + #[error("invalid feature descriptor: {0}")] + InvalidDescriptor(String), + #[error( + "duplicate tool contribution `{tool}` from feature `{duplicate_feature}`; first registered by `{first_feature}`" + )] + DuplicateToolName { + tool: String, + first_feature: String, + duplicate_feature: String, + }, + #[error( + "duplicate service declaration `{service}` from feature `{duplicate_feature}`; first provided by `{first_feature}`" + )] + DuplicateService { + service: String, + first_feature: String, + duplicate_feature: String, + }, + #[error("feature capability denied: {0}")] + CapabilityDenied(String), + #[error("feature install failed: {0}")] + Install(String), +} + +/// Builtin task tools feature used to prove existing builtin tool registration +/// through the feature registry without changing tool names, schemas, or +/// permission behavior. +pub mod builtin { + use super::*; + + pub fn task_feature(task_store: tools::TaskStore) -> impl FeatureModule { + TaskFeature { task_store } + } + + struct TaskFeature { + task_store: tools::TaskStore, + } + + impl FeatureModule for TaskFeature { + fn descriptor(&self) -> FeatureDescriptor { + FeatureDescriptor::builtin("task-tools", "Task tools") + .with_description("Session-lifetime task tracking builtin tools") + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "TaskCreate".into(), + }, + "register TaskCreate builtin tool", + )) + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "TaskUpdate".into(), + }, + "register TaskUpdate builtin tool", + )) + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "TaskGet".into(), + }, + "register TaskGet builtin tool", + )) + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "TaskList".into(), + }, + "register TaskList builtin tool", + )) + .with_tool(ToolDeclaration::new( + "TaskCreate", + "Create a session-lifetime user-visible task", + )) + .with_tool(ToolDeclaration::new( + "TaskUpdate", + "Update a session-lifetime user-visible task", + )) + .with_tool(ToolDeclaration::new( + "TaskGet", + "Get one session-lifetime user-visible task", + )) + .with_tool(ToolDeclaration::new( + "TaskList", + "List session-lifetime user-visible tasks", + )) + } + + fn install( + &self, + context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + for definition in tools::task_tools(self.task_store.clone()) { + let name = (definition)().0.name; + context + .tools() + .register(ToolContribution::new(name, definition))?; + } + Ok(()) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use async_trait::async_trait; + use futures::stream; + use llm_worker::llm_client::{ClientError, Request, ResponseStream}; + use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; + use serde_json::json; + + #[derive(Clone)] + struct DummyClient; + + #[async_trait] + impl LlmClient for DummyClient { + async fn stream(&self, _request: Request) -> Result { + Ok(Box::pin(stream::empty())) + } + + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } + } + + struct DummyTool; + + #[async_trait] + impl Tool for DummyTool { + async fn execute(&self, _input_json: &str) -> Result { + Ok(ToolOutput::from("ok".to_string())) + } + } + + fn dummy_tool(name: &'static str) -> ToolDefinition { + Arc::new(move || { + ( + ToolMeta::new(name) + .description("dummy") + .input_schema(json!({})), + Arc::new(DummyTool) as Arc, + ) + }) + } + + struct ToolFeature { + descriptor: FeatureDescriptor, + tool_name: &'static str, + } + + impl FeatureModule for ToolFeature { + fn descriptor(&self) -> FeatureDescriptor { + self.descriptor.clone() + } + + fn install( + &self, + context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + context.tools().register(ToolContribution::new( + self.tool_name, + dummy_tool(self.tool_name), + )) + } + } + + #[test] + fn descriptor_capabilities_and_install_report_are_recorded() { + let descriptor = FeatureDescriptor::builtin("dummy", "Dummy") + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "Dummy".into(), + }, + "test", + )) + .with_tool(ToolDeclaration::new("Dummy", "dummy tool")) + .with_background_task(BackgroundTaskDeclaration::descriptor_only( + "daily", + "descriptor-only background task", + )); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ToolFeature { + descriptor, + tool_name: "Dummy", + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert_eq!(pending_tools.len(), 1); + assert_eq!(report.reports.len(), 1); + let feature_report = &report.reports[0]; + assert!(feature_report.installed); + assert_eq!(feature_report.installed_tools, vec!["Dummy"]); + assert_eq!(feature_report.declared_background_tasks[0].name, "daily"); + assert!( + feature_report + .granted_capabilities + .contains(&HostCapability::ContributeTool { + name: "Dummy".into() + }) + ); + } + + #[test] + fn duplicate_tool_names_are_rejected() { + let descriptor_a = FeatureDescriptor::builtin("a", "A") + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "Duplicate".into(), + }, + "test duplicate handling", + )) + .with_tool(ToolDeclaration::new("Duplicate", "first tool")); + let descriptor_b = FeatureDescriptor::builtin("b", "B") + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "Duplicate".into(), + }, + "test duplicate handling", + )) + .with_tool(ToolDeclaration::new("Duplicate", "second tool")); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ToolFeature { + descriptor: descriptor_a, + tool_name: "Duplicate", + }) + .with_module(ToolFeature { + descriptor: descriptor_b, + tool_name: "Duplicate", + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert_eq!(pending_tools.len(), 1); + assert!(report.reports[0].installed); + assert!(!report.reports[1].installed); + assert!( + report.reports[1] + .diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains("duplicate tool contribution")) + ); + assert_eq!( + report.reports[1].skipped[0].kind, + FeatureContributionKind::Tool + ); + } + + struct ServiceFeature { + descriptor: FeatureDescriptor, + } + + impl FeatureModule for ServiceFeature { + fn descriptor(&self) -> FeatureDescriptor { + self.descriptor.clone() + } + + fn install( + &self, + _context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + Ok(()) + } + } + + #[test] + fn service_requirements_resolve_against_prior_providers() { + let service = ServiceId::builtin("demo-service"); + let provider = FeatureDescriptor::builtin("provider", "Provider").with_provided_service( + ServiceDeclaration::new(service.clone(), "1", "demo service"), + ); + let consumer = FeatureDescriptor::builtin("consumer", "Consumer") + .with_service_requirement(ServiceRequirement::required(service.clone(), "needs demo")); + let missing = FeatureDescriptor::builtin("missing", "Missing").with_service_requirement( + ServiceRequirement::required(ServiceId::builtin("missing-service"), "needs missing"), + ); + let optional = FeatureDescriptor::builtin("optional", "Optional").with_service_requirement( + ServiceRequirement::optional(ServiceId::builtin("optional-service"), "nice to have"), + ); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ServiceFeature { + descriptor: provider, + }) + .with_module(ServiceFeature { + descriptor: consumer, + }) + .with_module(ServiceFeature { + descriptor: missing, + }) + .with_module(ServiceFeature { + descriptor: optional, + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(report.services.provides(&service)); + assert!(report.reports[1].installed); + assert_eq!( + report.reports[1].resolved_service_requirements[0].id, + service + ); + assert!(!report.reports[2].installed); + assert!( + report.reports[2] + .diagnostics + .iter() + .any(|diagnostic| diagnostic.message.contains("required service requirement")) + ); + assert!(report.reports[3].installed); + assert_eq!( + report.reports[3].skipped[0].kind, + FeatureContributionKind::Service + ); + } + + #[test] + fn builtin_task_feature_installs_through_worker_tool_path() { + let task_store = tools::TaskStore::new(); + let mut worker = Worker::new(DummyClient); + let mut hook_builder = HookRegistryBuilder::default(); + let report = FeatureRegistryBuilder::new() + .with_module(builtin::task_feature(task_store)) + .install_into_worker(&mut worker, &mut hook_builder); + + worker.tool_server_handle().flush_pending(); + let names: Vec<_> = worker + .tool_server_handle() + .tool_definitions_sorted() + .into_iter() + .map(|tool| tool.name) + .collect(); + + assert_eq!( + names, + vec!["TaskCreate", "TaskGet", "TaskList", "TaskUpdate"] + ); + assert_eq!( + report.installed_tool_names(), + vec!["TaskCreate", "TaskList", "TaskGet", "TaskUpdate"] + ); + assert!(report.reports[0].installed); + } +} diff --git a/crates/pod/src/lib.rs b/crates/pod/src/lib.rs index aec13a77..c607e835 100644 --- a/crates/pod/src/lib.rs +++ b/crates/pod/src/lib.rs @@ -2,6 +2,7 @@ pub mod compact; pub mod controller; pub mod discovery; pub mod entrypoint; +pub mod feature; pub mod fs_view; pub mod hook; pub mod ipc; diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 3f5b2f85..8600b941 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -28,6 +28,7 @@ use manifest::{ use crate::compact::state::CompactState; use crate::compact::usage_tracker::UsageTracker; +use crate::feature::{FeatureRegistryBuilder, FeatureRegistryInstallReport}; use crate::hook::{ Hook, HookPreRequestAction, HookRegistryBuilder, OnAbort, OnPromptSubmit, OnTurnEnd, PostToolCall, PreLlmRequest, PreRequestInfo, PreToolCall, @@ -784,6 +785,15 @@ impl Pod { self.worker.as_mut().expect("worker taken during run") } + /// Install enabled feature modules into the Pod host surfaces. + pub fn install_features( + &mut self, + registry: FeatureRegistryBuilder, + ) -> FeatureRegistryInstallReport { + let worker = self.worker.as_mut().expect("worker taken during run"); + registry.install_into_worker(worker, &mut self.hook_builder) + } + /// Reference to the store. pub fn store(&self) -> &St { &self.store diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index 535953aa..7f76e4d2 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -43,8 +43,9 @@ pub use tracker::Tracker; pub use web::{web_fetch_tool, web_search_tool}; pub use write::write_tool; -/// Register all builtin tools, wiring them to a shared `ScopedFs` -/// (Pod-process lifetime) and `Tracker` (Pod-process lifetime). +/// Register core builtin tools that do not require Pod-local task state, +/// wiring them to a shared `ScopedFs` (Pod-process lifetime) and `Tracker` +/// (Pod-process lifetime). /// /// All returned factories share the same tracker instance so that /// `Read` / `Write` / `Edit` see a consistent history across tool @@ -54,14 +55,13 @@ pub use write::write_tool; /// caller is responsible for adding that path to the readable scope /// (see [`manifest::Scope::with_extra_read`]) so the agent can `Read` /// the saved files. -pub fn builtin_tools( +pub fn core_builtin_tools( fs: ScopedFs, tracker: Tracker, - task_store: TaskStore, bash_output_dir: std::path::PathBuf, web_config: Option, ) -> Vec { - let mut defs = vec![ + vec![ read_tool(fs.clone(), tracker.clone()), write_tool(fs.clone(), tracker.clone()), edit_tool(fs.clone(), tracker), @@ -70,7 +70,19 @@ pub fn builtin_tools( bash_tool(fs, bash_output_dir), web_search_tool(web::WebTools::new(web_config.clone())), web_fetch_tool(web::WebTools::new(web_config)), - ]; + ] +} + +/// Register all builtin tools, including task tools, for callers that are not +/// using the Pod feature registry path. +pub fn builtin_tools( + fs: ScopedFs, + tracker: Tracker, + task_store: TaskStore, + bash_output_dir: std::path::PathBuf, + web_config: Option, +) -> Vec { + let mut defs = core_builtin_tools(fs, tracker, bash_output_dir, web_config); defs.extend(task_tools(task_store)); defs } From 40701760ebbf131e0cb73155232f6d84b418befb Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 5 Jun 2026 06:41:06 +0900 Subject: [PATCH 2/4] fix: harden feature contribution gates --- crates/pod/src/feature.rs | 444 +++++++++++++++++++++++++++++++------- 1 file changed, 361 insertions(+), 83 deletions(-) diff --git a/crates/pod/src/feature.rs b/crates/pod/src/feature.rs index 10520fb2..2334ee56 100644 --- a/crates/pod/src/feature.rs +++ b/crates/pod/src/feature.rs @@ -592,14 +592,82 @@ impl FeatureInstallReport { } } +fn require_capability( + grants: &CapabilityGrantSet, + report: &mut FeatureInstallReport, + kind: FeatureContributionKind, + name: impl Into, + capability: &HostCapability, +) -> Result<(), FeatureInstallError> { + if grants.contains(capability) { + return Ok(()); + } + + let reason = format!("required capability was not granted: {capability:?}"); + report.mark_skipped(kind, name, reason.clone()); + Err(FeatureInstallError::CapabilityDenied(reason)) +} + +fn require_background_task_capability( + grants: &CapabilityGrantSet, + report: &mut FeatureInstallReport, + declaration: &BackgroundTaskDeclaration, +) -> Result<(), FeatureInstallError> { + let default_capability = HostCapability::DeclareBackgroundTask { + name: declaration.name.clone(), + }; + require_capability( + grants, + report, + FeatureContributionKind::BackgroundTask, + declaration.name.clone(), + &default_capability, + )?; + for capability in &declaration.required_capabilities { + require_capability( + grants, + report, + FeatureContributionKind::BackgroundTask, + declaration.name.clone(), + capability, + )?; + } + Ok(()) +} + +fn require_service_provider_capability( + grants: &CapabilityGrantSet, + report: &mut FeatureInstallReport, + declaration: &ServiceDeclaration, +) -> Result<(), FeatureInstallError> { + let capability = HostCapability::ProvideService { + service: declaration.id.clone(), + }; + require_capability( + grants, + report, + FeatureContributionKind::Service, + declaration.id.to_string(), + &capability, + ) +} + /// Model-visible durable notification sink skeleton. The first slice exposes /// the boundary without implementing a new event channel. pub struct FeatureNotificationSink<'a> { + grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl FeatureNotificationSink<'_> { - pub fn notify_model(&mut self, message: impl Into) { + pub fn notify_model(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { + require_capability( + self.grants, + self.report, + FeatureContributionKind::Notification, + "notify_model", + &HostCapability::EmitNotification, + )?; let message = message.into(); self.report.diagnostics.push(FeatureDiagnostic::warning(format!( "model notification requested during feature installation but no durable Notify host is attached: {message}" @@ -609,16 +677,25 @@ impl FeatureNotificationSink<'_> { "notify_model", "durable Notify/SystemItem host is not connected during feature installation", ); + Ok(()) } } /// Transient human-facing alert sink skeleton. pub struct FeatureAlertSink<'a> { + grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl FeatureAlertSink<'_> { - pub fn alert(&mut self, message: impl Into) { + pub fn alert(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { + require_capability( + self.grants, + self.report, + FeatureContributionKind::Alert, + "alert", + &HostCapability::EmitAlert, + )?; let message = message.into(); self.report .diagnostics @@ -628,29 +705,39 @@ impl FeatureAlertSink<'_> { "alert", "transient alert host is not connected during feature installation", ); + Ok(()) } } /// Diagnostic sink available to feature installers. pub struct FeatureDiagnosticSink<'a> { + grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl FeatureDiagnosticSink<'_> { - pub fn push(&mut self, diagnostic: FeatureDiagnostic) { + pub fn push(&mut self, diagnostic: FeatureDiagnostic) -> Result<(), FeatureInstallError> { + require_capability( + self.grants, + self.report, + FeatureContributionKind::Diagnostic, + "diagnostic", + &HostCapability::EmitDiagnostic, + )?; self.report.diagnostics.push(diagnostic); + Ok(()) } - pub fn info(&mut self, message: impl Into) { - self.push(FeatureDiagnostic::info(message)); + pub fn info(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { + self.push(FeatureDiagnostic::info(message)) } - pub fn warning(&mut self, message: impl Into) { - self.push(FeatureDiagnostic::warning(message)); + pub fn warning(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { + self.push(FeatureDiagnostic::warning(message)) } - pub fn error(&mut self, message: impl Into) { - self.push(FeatureDiagnostic::error(message)); + pub fn error(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { + self.push(FeatureDiagnostic::error(message)) } } @@ -665,35 +752,57 @@ pub struct ToolContributionRegistrar<'a> { impl ToolContributionRegistrar<'_> { pub fn register(&mut self, contribution: ToolContribution) -> Result<(), FeatureInstallError> { - for capability in &contribution.required_capabilities { - if !self.grants.contains(capability) { - let reason = format!("required capability was not granted: {capability:?}"); - self.report.mark_skipped( - FeatureContributionKind::Tool, - contribution.name.clone(), - reason.clone(), - ); - return Err(FeatureInstallError::CapabilityDenied(reason)); - } + let model_visible_name = (contribution.definition)().0.name; + if contribution.name != model_visible_name { + let error = FeatureInstallError::ToolNameMismatch { + declared: contribution.name, + model_visible: model_visible_name.clone(), + }; + self.report.mark_skipped( + FeatureContributionKind::Tool, + model_visible_name, + error.to_string(), + ); + return Err(error); } - if let Some(first) = self.installed_tool_names.get(&contribution.name) { + let tool_capability = HostCapability::ContributeTool { + name: model_visible_name.clone(), + }; + require_capability( + self.grants, + self.report, + FeatureContributionKind::Tool, + model_visible_name.clone(), + &tool_capability, + )?; + for capability in &contribution.required_capabilities { + require_capability( + self.grants, + self.report, + FeatureContributionKind::Tool, + model_visible_name.clone(), + capability, + )?; + } + + if let Some(first) = self.installed_tool_names.get(&model_visible_name) { let error = FeatureInstallError::DuplicateToolName { - tool: contribution.name.clone(), + tool: model_visible_name.clone(), first_feature: first.to_string(), duplicate_feature: self.feature_id.to_string(), }; self.report.mark_skipped( FeatureContributionKind::Tool, - contribution.name.clone(), + model_visible_name, error.to_string(), ); return Err(error); } self.installed_tool_names - .insert(contribution.name.clone(), self.feature_id.clone()); - self.report.installed_tools.push(contribution.name); + .insert(model_visible_name.clone(), self.feature_id.clone()); + self.report.installed_tools.push(model_visible_name); self.pending_tools.push(contribution.definition); Ok(()) } @@ -776,24 +885,32 @@ impl HookContributionRegistrar<'_> { /// Background task registrar for descriptor/report-only contributions. pub struct BackgroundTaskRegistrar<'a> { + grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl BackgroundTaskRegistrar<'_> { - pub fn declare(&mut self, declaration: BackgroundTaskDeclaration) { + pub fn declare( + &mut self, + declaration: BackgroundTaskDeclaration, + ) -> Result<(), FeatureInstallError> { + require_background_task_capability(self.grants, self.report, &declaration)?; self.report.declared_background_tasks.push(declaration); + Ok(()) } } /// Service registrar for descriptor/report-only provider metadata. pub struct FeatureServiceRegistrar<'a> { feature_id: &'a FeatureId, + grants: &'a CapabilityGrantSet, service_registry: &'a mut FeatureServiceRegistry, report: &'a mut FeatureInstallReport, } impl FeatureServiceRegistrar<'_> { pub fn provide(&mut self, declaration: ServiceDeclaration) -> Result<(), FeatureInstallError> { + require_service_provider_capability(self.grants, self.report, &declaration)?; self.service_registry .register_provider(self.feature_id.clone(), declaration.clone())?; self.report.provided_services.push(declaration); @@ -841,6 +958,7 @@ impl FeatureInstallContext<'_> { pub fn background_tasks(&mut self) -> BackgroundTaskRegistrar<'_> { BackgroundTaskRegistrar { + grants: self.grants, report: self.report, } } @@ -848,6 +966,7 @@ impl FeatureInstallContext<'_> { pub fn services(&mut self) -> FeatureServiceRegistrar<'_> { FeatureServiceRegistrar { feature_id: self.feature_id, + grants: self.grants, service_registry: self.service_registry, report: self.report, } @@ -855,18 +974,21 @@ impl FeatureInstallContext<'_> { pub fn notifications(&mut self) -> FeatureNotificationSink<'_> { FeatureNotificationSink { + grants: self.grants, report: self.report, } } pub fn alerts(&mut self) -> FeatureAlertSink<'_> { FeatureAlertSink { + grants: self.grants, report: self.report, } } pub fn diagnostics(&mut self) -> FeatureDiagnosticSink<'_> { FeatureDiagnosticSink { + grants: self.grants, report: self.report, } } @@ -978,24 +1100,47 @@ impl FeatureRegistryBuilder { ))); } - if let Some(reason) = missing_required_service(&descriptor, &service_registry) { - report - .diagnostics - .push(FeatureDiagnostic::error(reason.clone())); - report.mark_skipped( - FeatureContributionKind::Service, - descriptor.id.to_string(), - reason, - ); - reports.push(report); - continue; - } - + let mut required_service_failed = false; for requirement in descriptor.requires_services.iter().cloned() { + let capability = HostCapability::RequireService { + service: requirement.id.clone(), + }; + if let Err(error) = require_capability( + &grants, + &mut report, + FeatureContributionKind::Service, + requirement.id.to_string(), + &capability, + ) { + if requirement.required { + report + .diagnostics + .push(FeatureDiagnostic::error(error.to_string())); + required_service_failed = true; + } else { + report + .diagnostics + .push(FeatureDiagnostic::warning(error.to_string())); + } + continue; + } + if service_registry.provides(&requirement.id) { report.resolved_service_requirements.push(requirement); } else if requirement.required { - // Already handled by missing_required_service. + let reason = format!( + "required service requirement is not available: {}", + requirement.id + ); + report + .diagnostics + .push(FeatureDiagnostic::error(reason.clone())); + report.mark_skipped( + FeatureContributionKind::Service, + requirement.id.to_string(), + reason, + ); + required_service_failed = true; } else { report.diagnostics.push(FeatureDiagnostic::warning(format!( "optional service requirement is not available: {}", @@ -1008,24 +1153,40 @@ impl FeatureRegistryBuilder { ); } } + if required_service_failed { + reports.push(report); + continue; + } for background_task in descriptor.background_tasks.iter().cloned() { - report.declared_background_tasks.push(background_task); + match require_background_task_capability(&grants, &mut report, &background_task) { + Ok(()) => report.declared_background_tasks.push(background_task), + Err(error) => report + .diagnostics + .push(FeatureDiagnostic::warning(error.to_string())), + } } for service in descriptor.provides_services.iter().cloned() { - match service_registry.register_provider(descriptor.id.clone(), service.clone()) { - Ok(()) => report.provided_services.push(service), - Err(error) => { - report - .diagnostics - .push(FeatureDiagnostic::error(error.to_string())); - report.mark_skipped( - FeatureContributionKind::Service, - service.id.to_string(), - error.to_string(), - ); - } + match require_service_provider_capability(&grants, &mut report, &service) { + Ok(()) => match service_registry + .register_provider(descriptor.id.clone(), service.clone()) + { + Ok(()) => report.provided_services.push(service), + Err(error) => { + report + .diagnostics + .push(FeatureDiagnostic::error(error.to_string())); + report.mark_skipped( + FeatureContributionKind::Service, + service.id.to_string(), + error.to_string(), + ); + } + }, + Err(error) => report + .diagnostics + .push(FeatureDiagnostic::warning(error.to_string())), } } @@ -1060,22 +1221,6 @@ impl FeatureRegistryBuilder { } } -fn missing_required_service( - descriptor: &FeatureDescriptor, - services: &FeatureServiceRegistry, -) -> Option { - descriptor - .requires_services - .iter() - .find(|requirement| requirement.required && !services.provides(&requirement.id)) - .map(|requirement| { - format!( - "required service requirement is not available: {}", - requirement.id - ) - }) -} - /// Feature installation errors. #[derive(Debug, Error)] pub enum FeatureInstallError { @@ -1089,6 +1234,13 @@ pub enum FeatureInstallError { first_feature: String, duplicate_feature: String, }, + #[error( + "tool contribution declared name `{declared}` does not match model-visible tool name `{model_visible}`" + )] + ToolNameMismatch { + declared: String, + model_visible: String, + }, #[error( "duplicate service declaration `{service}` from feature `{duplicate_feature}`; first provided by `{first_feature}`" )] @@ -1223,7 +1375,8 @@ mod tests { struct ToolFeature { descriptor: FeatureDescriptor, - tool_name: &'static str, + contribution_name: &'static str, + model_visible_name: &'static str, } impl FeatureModule for ToolFeature { @@ -1236,8 +1389,8 @@ mod tests { context: &mut FeatureInstallContext<'_>, ) -> Result<(), FeatureInstallError> { context.tools().register(ToolContribution::new( - self.tool_name, - dummy_tool(self.tool_name), + self.contribution_name, + dummy_tool(self.model_visible_name), )) } } @@ -1251,6 +1404,12 @@ mod tests { }, "test", )) + .with_capability(CapabilityRequest::required( + HostCapability::DeclareBackgroundTask { + name: "daily".into(), + }, + "test background task declaration", + )) .with_tool(ToolDeclaration::new("Dummy", "dummy tool")) .with_background_task(BackgroundTaskDeclaration::descriptor_only( "daily", @@ -1261,7 +1420,8 @@ mod tests { let report = FeatureRegistryBuilder::new() .with_module(ToolFeature { descriptor, - tool_name: "Dummy", + contribution_name: "Dummy", + model_visible_name: "Dummy", }) .install_into_pending(&mut pending_tools, &mut hook_builder); @@ -1303,11 +1463,13 @@ mod tests { let report = FeatureRegistryBuilder::new() .with_module(ToolFeature { descriptor: descriptor_a, - tool_name: "Duplicate", + contribution_name: "Duplicate", + model_visible_name: "Duplicate", }) .with_module(ToolFeature { descriptor: descriptor_b, - tool_name: "Duplicate", + contribution_name: "Duplicate", + model_visible_name: "Duplicate", }) .install_into_pending(&mut pending_tools, &mut hook_builder); @@ -1326,6 +1488,36 @@ mod tests { ); } + #[test] + fn mismatched_tool_contribution_name_is_rejected_before_queueing() { + let descriptor = FeatureDescriptor::builtin("mismatch", "Mismatch") + .with_capability(CapabilityRequest::required( + HostCapability::ContributeTool { + name: "Actual".into(), + }, + "test mismatch handling", + )) + .with_tool(ToolDeclaration::new("Actual", "actual model-visible tool")); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ToolFeature { + descriptor, + contribution_name: "Declared", + model_visible_name: "Actual", + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(pending_tools.is_empty()); + assert!(!report.reports[0].installed); + assert!(report.reports[0].diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("does not match model-visible tool name") + })); + assert_eq!(report.reports[0].skipped[0].name, "Actual"); + } + struct ServiceFeature { descriptor: FeatureDescriptor, } @@ -1346,17 +1538,50 @@ mod tests { #[test] fn service_requirements_resolve_against_prior_providers() { let service = ServiceId::builtin("demo-service"); - let provider = FeatureDescriptor::builtin("provider", "Provider").with_provided_service( - ServiceDeclaration::new(service.clone(), "1", "demo service"), - ); + let provider = FeatureDescriptor::builtin("provider", "Provider") + .with_capability(CapabilityRequest::required( + HostCapability::ProvideService { + service: service.clone(), + }, + "provide demo service", + )) + .with_provided_service(ServiceDeclaration::new( + service.clone(), + "1", + "demo service", + )); let consumer = FeatureDescriptor::builtin("consumer", "Consumer") + .with_capability(CapabilityRequest::required( + HostCapability::RequireService { + service: service.clone(), + }, + "require demo service", + )) .with_service_requirement(ServiceRequirement::required(service.clone(), "needs demo")); - let missing = FeatureDescriptor::builtin("missing", "Missing").with_service_requirement( - ServiceRequirement::required(ServiceId::builtin("missing-service"), "needs missing"), - ); - let optional = FeatureDescriptor::builtin("optional", "Optional").with_service_requirement( - ServiceRequirement::optional(ServiceId::builtin("optional-service"), "nice to have"), - ); + let missing_service = ServiceId::builtin("missing-service"); + let missing = FeatureDescriptor::builtin("missing", "Missing") + .with_capability(CapabilityRequest::required( + HostCapability::RequireService { + service: missing_service.clone(), + }, + "require missing service", + )) + .with_service_requirement(ServiceRequirement::required( + missing_service, + "needs missing", + )); + let optional_service = ServiceId::builtin("optional-service"); + let optional = FeatureDescriptor::builtin("optional", "Optional") + .with_capability(CapabilityRequest::required( + HostCapability::RequireService { + service: optional_service.clone(), + }, + "optionally require service", + )) + .with_service_requirement(ServiceRequirement::optional( + optional_service, + "nice to have", + )); let mut hook_builder = HookRegistryBuilder::default(); let mut pending_tools = Vec::new(); let report = FeatureRegistryBuilder::new() @@ -1394,6 +1619,59 @@ mod tests { ); } + #[test] + fn background_task_declaration_without_capability_is_skipped() { + let descriptor = FeatureDescriptor::builtin("background-denied", "Background denied") + .with_background_task(BackgroundTaskDeclaration::descriptor_only( + "denied-task", + "should be skipped", + )); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ServiceFeature { descriptor }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(report.reports[0].installed); + assert!(report.reports[0].declared_background_tasks.is_empty()); + assert_eq!( + report.reports[0].skipped[0].kind, + FeatureContributionKind::BackgroundTask + ); + assert!(report.reports[0].diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("required capability was not granted") + })); + } + + #[test] + fn service_provider_without_capability_is_skipped() { + let service = ServiceId::builtin("denied-service"); + let descriptor = + FeatureDescriptor::builtin("service-denied", "Service denied").with_provided_service( + ServiceDeclaration::new(service.clone(), "1", "should be skipped"), + ); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ServiceFeature { descriptor }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(report.reports[0].installed); + assert!(!report.services.provides(&service)); + assert!(report.reports[0].provided_services.is_empty()); + assert_eq!( + report.reports[0].skipped[0].kind, + FeatureContributionKind::Service + ); + assert!(report.reports[0].diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("required capability was not granted") + })); + } + #[test] fn builtin_task_feature_installs_through_worker_tool_path() { let task_store = tools::TaskStore::new(); From 98bbd6f1853e7e3652a75af92dc1b381722e2c9c Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 5 Jun 2026 07:03:21 +0900 Subject: [PATCH 3/4] fix: align feature authority boundaries --- crates/pod/src/feature.rs | 575 ++++++++++++-------------------------- 1 file changed, 186 insertions(+), 389 deletions(-) diff --git a/crates/pod/src/feature.rs b/crates/pod/src/feature.rs index 2334ee56..32a10e1e 100644 --- a/crates/pod/src/feature.rs +++ b/crates/pod/src/feature.rs @@ -1,7 +1,7 @@ //! Feature contribution registry for Pod-hosted builtin/plugin modules. //! //! This module defines the Pod-side feature boundary used to collect -//! descriptor metadata, capability requests, tool contributions, safe hook +//! descriptor metadata, authority requests, tool contributions, safe hook //! contributions, background task declarations, and service declarations before //! installing them into the existing Worker/HookRegistry host surfaces. //! @@ -69,20 +69,23 @@ pub enum FeatureRuntimeKind { ExternalPlugin, } -/// Host capability requested by a feature before it contributes host-visible -/// behavior. Grants are additive and do not replace manifest/tool permission -/// checks. +/// Host authority requested by a feature for host-mediated operations that can +/// cross sandbox or model-context boundaries. +/// +/// Contribution declarations such as tools, hooks, background tasks, and +/// services are descriptor/package-approved host-visible contributions, not +/// sandbox authorities. Grants are additive and do not replace manifest/tool +/// permission checks. #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -pub enum HostCapability { - ContributeTool { name: String }, - ContributeHook { point: FeatureHookPoint }, - DeclareBackgroundTask { name: String }, - ProvideService { service: ServiceId }, - RequireService { service: ServiceId }, - EmitNotification, - EmitAlert, - EmitDiagnostic, +pub enum HostAuthority { + Filesystem, + Network, + SecretRef { id: String }, + ModelNotification, + PodManagement, + StateStore { name: String }, + ServiceAccess { service: ServiceId }, } /// A safe hook contribution point exposed to feature modules. @@ -95,45 +98,45 @@ pub enum FeatureHookPoint { TurnEnd, } -/// Capability request declared by a feature descriptor. +/// Authority request declared by a feature descriptor. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct CapabilityRequest { - pub capability: HostCapability, +pub struct AuthorityRequest { + pub authority: HostAuthority, pub required: bool, pub reason: String, } -impl CapabilityRequest { - pub fn required(capability: HostCapability, reason: impl Into) -> Self { +impl AuthorityRequest { + pub fn required(authority: HostAuthority, reason: impl Into) -> Self { Self { - capability, + authority, required: true, reason: reason.into(), } } - pub fn optional(capability: HostCapability, reason: impl Into) -> Self { + pub fn optional(authority: HostAuthority, reason: impl Into) -> Self { Self { - capability, + authority, required: false, reason: reason.into(), } } } -/// Capability grants resolved by the host for one feature installation. +/// Authority grants resolved by the host for one feature installation. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct CapabilityGrantSet { - granted: HashSet, - denied: Vec, +pub struct AuthorityGrantSet { + granted: HashSet, + denied: Vec, } -impl CapabilityGrantSet { - pub fn grant_all(requests: &[CapabilityRequest]) -> Self { +impl AuthorityGrantSet { + pub fn grant_all(requests: &[AuthorityRequest]) -> Self { Self { granted: requests .iter() - .map(|request| request.capability.clone()) + .map(|request| request.authority.clone()) .collect(), denied: Vec::new(), } @@ -143,31 +146,31 @@ impl CapabilityGrantSet { Self::default() } - pub fn contains(&self, capability: &HostCapability) -> bool { - self.granted.contains(capability) + pub fn contains(&self, authority: &HostAuthority) -> bool { + self.granted.contains(authority) } - pub fn denied(&self) -> &[CapabilityDenial] { + pub fn denied(&self) -> &[AuthorityDenial] { &self.denied } - pub fn grant(&mut self, capability: HostCapability) { - self.granted.insert(capability); + pub fn grant(&mut self, authority: HostAuthority) { + self.granted.insert(authority); } - pub fn deny(&mut self, capability: HostCapability, reason: impl Into) { - self.granted.remove(&capability); - self.denied.push(CapabilityDenial { - capability, + pub fn deny(&mut self, authority: HostAuthority, reason: impl Into) { + self.granted.remove(&authority); + self.denied.push(AuthorityDenial { + authority, reason: reason.into(), }); } } -/// Host-side denial of a requested feature capability. +/// Host-side denial of a requested feature authority. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct CapabilityDenial { - pub capability: HostCapability, +pub struct AuthorityDenial { + pub authority: HostAuthority, pub reason: String, } @@ -177,15 +180,12 @@ pub struct CapabilityDenial { pub struct ToolDeclaration { pub name: String, pub description: String, - pub required_capabilities: Vec, } impl ToolDeclaration { pub fn new(name: impl Into, description: impl Into) -> Self { - let name = name.into(); Self { - required_capabilities: vec![HostCapability::ContributeTool { name: name.clone() }], - name, + name: name.into(), description: description.into(), } } @@ -195,24 +195,20 @@ impl ToolDeclaration { pub struct ToolContribution { name: String, definition: ToolDefinition, - required_capabilities: Vec, + required_authorities: Vec, } impl ToolContribution { pub fn new(name: impl Into, definition: ToolDefinition) -> Self { - let name = name.into(); Self { - required_capabilities: vec![HostCapability::ContributeTool { name: name.clone() }], - name, + name: name.into(), definition, + required_authorities: Vec::new(), } } - pub fn with_required_capabilities( - mut self, - required_capabilities: Vec, - ) -> Self { - self.required_capabilities = required_capabilities; + pub fn with_required_authorities(mut self, required_authorities: Vec) -> Self { + self.required_authorities = required_authorities; self } @@ -226,16 +222,12 @@ impl ToolContribution { pub struct HookDeclaration { pub name: String, pub point: FeatureHookPoint, - pub required_capabilities: Vec, } impl HookDeclaration { pub fn new(name: impl Into, point: FeatureHookPoint) -> Self { Self { name: name.into(), - required_capabilities: vec![HostCapability::ContributeHook { - point: point.clone(), - }], point, } } @@ -255,17 +247,12 @@ pub struct BackgroundTaskDeclaration { pub name: String, pub description: String, pub lifecycle: BackgroundTaskLifecycle, - pub required_capabilities: Vec, } impl BackgroundTaskDeclaration { pub fn descriptor_only(name: impl Into, description: impl Into) -> Self { - let name = name.into(); Self { - required_capabilities: vec![HostCapability::DeclareBackgroundTask { - name: name.clone(), - }], - name, + name: name.into(), description: description.into(), lifecycle: BackgroundTaskLifecycle::DescriptorOnly, } @@ -418,7 +405,7 @@ pub struct FeatureDescriptor { pub display_name: String, pub version: String, pub description: String, - pub requested_capabilities: Vec, + pub requested_authorities: Vec, pub tools: Vec, pub hooks: Vec, pub background_tasks: Vec, @@ -434,7 +421,7 @@ impl FeatureDescriptor { display_name: display_name.into(), version: env!("CARGO_PKG_VERSION").into(), description: String::new(), - requested_capabilities: Vec::new(), + requested_authorities: Vec::new(), tools: Vec::new(), hooks: Vec::new(), background_tasks: Vec::new(), @@ -448,8 +435,8 @@ impl FeatureDescriptor { self } - pub fn with_capability(mut self, request: CapabilityRequest) -> Self { - self.requested_capabilities.push(request); + pub fn with_authority(mut self, request: AuthorityRequest) -> Self { + self.requested_authorities.push(request); self } @@ -551,7 +538,7 @@ pub struct FeatureInstallReport { pub feature_id: FeatureId, pub runtime: FeatureRuntimeKind, pub installed: bool, - pub granted_capabilities: CapabilityGrantSet, + pub granted_authorities: AuthorityGrantSet, pub installed_tools: Vec, pub installed_hooks: Vec, pub declared_background_tasks: Vec, @@ -562,12 +549,12 @@ pub struct FeatureInstallReport { } impl FeatureInstallReport { - fn new(descriptor: &FeatureDescriptor, granted_capabilities: CapabilityGrantSet) -> Self { + fn new(descriptor: &FeatureDescriptor, granted_authorities: AuthorityGrantSet) -> Self { Self { feature_id: descriptor.id.clone(), runtime: descriptor.runtime.clone(), installed: false, - granted_capabilities, + granted_authorities, installed_tools: Vec::new(), installed_hooks: Vec::new(), declared_background_tasks: Vec::new(), @@ -592,81 +579,37 @@ impl FeatureInstallReport { } } -fn require_capability( - grants: &CapabilityGrantSet, +fn require_authority( + grants: &AuthorityGrantSet, report: &mut FeatureInstallReport, kind: FeatureContributionKind, name: impl Into, - capability: &HostCapability, + authority: &HostAuthority, ) -> Result<(), FeatureInstallError> { - if grants.contains(capability) { + if grants.contains(authority) { return Ok(()); } - let reason = format!("required capability was not granted: {capability:?}"); + let reason = format!("required authority was not granted: {authority:?}"); report.mark_skipped(kind, name, reason.clone()); - Err(FeatureInstallError::CapabilityDenied(reason)) -} - -fn require_background_task_capability( - grants: &CapabilityGrantSet, - report: &mut FeatureInstallReport, - declaration: &BackgroundTaskDeclaration, -) -> Result<(), FeatureInstallError> { - let default_capability = HostCapability::DeclareBackgroundTask { - name: declaration.name.clone(), - }; - require_capability( - grants, - report, - FeatureContributionKind::BackgroundTask, - declaration.name.clone(), - &default_capability, - )?; - for capability in &declaration.required_capabilities { - require_capability( - grants, - report, - FeatureContributionKind::BackgroundTask, - declaration.name.clone(), - capability, - )?; - } - Ok(()) -} - -fn require_service_provider_capability( - grants: &CapabilityGrantSet, - report: &mut FeatureInstallReport, - declaration: &ServiceDeclaration, -) -> Result<(), FeatureInstallError> { - let capability = HostCapability::ProvideService { - service: declaration.id.clone(), - }; - require_capability( - grants, - report, - FeatureContributionKind::Service, - declaration.id.to_string(), - &capability, - ) + Err(FeatureInstallError::AuthorityDenied(reason)) } /// Model-visible durable notification sink skeleton. The first slice exposes /// the boundary without implementing a new event channel. pub struct FeatureNotificationSink<'a> { - grants: &'a CapabilityGrantSet, + grants: &'a AuthorityGrantSet, report: &'a mut FeatureInstallReport, } impl FeatureNotificationSink<'_> { pub fn notify_model(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { - require_capability( + require_authority( self.grants, self.report, FeatureContributionKind::Notification, "notify_model", - &HostCapability::EmitNotification, + &HostAuthority::ModelNotification, )?; let message = message.into(); self.report.diagnostics.push(FeatureDiagnostic::warning(format!( @@ -683,19 +626,11 @@ impl FeatureNotificationSink<'_> { /// Transient human-facing alert sink skeleton. pub struct FeatureAlertSink<'a> { - grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl FeatureAlertSink<'_> { - pub fn alert(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { - require_capability( - self.grants, - self.report, - FeatureContributionKind::Alert, - "alert", - &HostCapability::EmitAlert, - )?; + pub fn alert(&mut self, message: impl Into) { let message = message.into(); self.report .diagnostics @@ -705,46 +640,36 @@ impl FeatureAlertSink<'_> { "alert", "transient alert host is not connected during feature installation", ); - Ok(()) } } /// Diagnostic sink available to feature installers. pub struct FeatureDiagnosticSink<'a> { - grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl FeatureDiagnosticSink<'_> { - pub fn push(&mut self, diagnostic: FeatureDiagnostic) -> Result<(), FeatureInstallError> { - require_capability( - self.grants, - self.report, - FeatureContributionKind::Diagnostic, - "diagnostic", - &HostCapability::EmitDiagnostic, - )?; + pub fn push(&mut self, diagnostic: FeatureDiagnostic) { self.report.diagnostics.push(diagnostic); - Ok(()) } - pub fn info(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { - self.push(FeatureDiagnostic::info(message)) + pub fn info(&mut self, message: impl Into) { + self.push(FeatureDiagnostic::info(message)); } - pub fn warning(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { - self.push(FeatureDiagnostic::warning(message)) + pub fn warning(&mut self, message: impl Into) { + self.push(FeatureDiagnostic::warning(message)); } - pub fn error(&mut self, message: impl Into) -> Result<(), FeatureInstallError> { - self.push(FeatureDiagnostic::error(message)) + pub fn error(&mut self, message: impl Into) { + self.push(FeatureDiagnostic::error(message)); } } /// Tool contribution registrar exposed inside [`FeatureInstallContext`]. pub struct ToolContributionRegistrar<'a> { feature_id: &'a FeatureId, - grants: &'a CapabilityGrantSet, + grants: &'a AuthorityGrantSet, pending_tools: &'a mut Vec, installed_tool_names: &'a mut HashMap, report: &'a mut FeatureInstallReport, @@ -752,7 +677,8 @@ pub struct ToolContributionRegistrar<'a> { impl ToolContributionRegistrar<'_> { pub fn register(&mut self, contribution: ToolContribution) -> Result<(), FeatureInstallError> { - let model_visible_name = (contribution.definition)().0.name; + let (tool_meta, tool) = (contribution.definition)(); + let model_visible_name = tool_meta.name.clone(); if contribution.name != model_visible_name { let error = FeatureInstallError::ToolNameMismatch { declared: contribution.name, @@ -766,23 +692,13 @@ impl ToolContributionRegistrar<'_> { return Err(error); } - let tool_capability = HostCapability::ContributeTool { - name: model_visible_name.clone(), - }; - require_capability( - self.grants, - self.report, - FeatureContributionKind::Tool, - model_visible_name.clone(), - &tool_capability, - )?; - for capability in &contribution.required_capabilities { - require_capability( + for authority in &contribution.required_authorities { + require_authority( self.grants, self.report, FeatureContributionKind::Tool, model_visible_name.clone(), - capability, + authority, )?; } @@ -803,14 +719,14 @@ impl ToolContributionRegistrar<'_> { self.installed_tool_names .insert(model_visible_name.clone(), self.feature_id.clone()); self.report.installed_tools.push(model_visible_name); - self.pending_tools.push(contribution.definition); + self.pending_tools + .push(Arc::new(move || (tool_meta.clone(), Arc::clone(&tool)))); Ok(()) } } /// Safe hook contribution registrar backed by [`HookRegistryBuilder`]. pub struct HookContributionRegistrar<'a> { - grants: &'a CapabilityGrantSet, hook_builder: &'a mut HookRegistryBuilder, report: &'a mut FeatureInstallReport, } @@ -822,7 +738,6 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::PreRequest); - self.require_hook_capability(&declaration)?; self.hook_builder.add_pre_llm_request(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -834,7 +749,6 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::PreToolCall); - self.require_hook_capability(&declaration)?; self.hook_builder.add_pre_tool_call(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -846,7 +760,6 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::ToolResult); - self.require_hook_capability(&declaration)?; self.hook_builder.add_post_tool_call(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -858,59 +771,32 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::TurnEnd); - self.require_hook_capability(&declaration)?; self.hook_builder.add_on_turn_end(hook); self.report.installed_hooks.push(declaration); Ok(()) } - - fn require_hook_capability( - &mut self, - declaration: &HookDeclaration, - ) -> Result<(), FeatureInstallError> { - for capability in &declaration.required_capabilities { - if !self.grants.contains(capability) { - let reason = format!("required capability was not granted: {capability:?}"); - self.report.mark_skipped( - FeatureContributionKind::Hook, - declaration.name.clone(), - reason.clone(), - ); - return Err(FeatureInstallError::CapabilityDenied(reason)); - } - } - Ok(()) - } } /// Background task registrar for descriptor/report-only contributions. pub struct BackgroundTaskRegistrar<'a> { - grants: &'a CapabilityGrantSet, report: &'a mut FeatureInstallReport, } impl BackgroundTaskRegistrar<'_> { - pub fn declare( - &mut self, - declaration: BackgroundTaskDeclaration, - ) -> Result<(), FeatureInstallError> { - require_background_task_capability(self.grants, self.report, &declaration)?; + pub fn declare(&mut self, declaration: BackgroundTaskDeclaration) { self.report.declared_background_tasks.push(declaration); - Ok(()) } } /// Service registrar for descriptor/report-only provider metadata. pub struct FeatureServiceRegistrar<'a> { feature_id: &'a FeatureId, - grants: &'a CapabilityGrantSet, service_registry: &'a mut FeatureServiceRegistry, report: &'a mut FeatureInstallReport, } impl FeatureServiceRegistrar<'_> { pub fn provide(&mut self, declaration: ServiceDeclaration) -> Result<(), FeatureInstallError> { - require_service_provider_capability(self.grants, self.report, &declaration)?; self.service_registry .register_provider(self.feature_id.clone(), declaration.clone())?; self.report.provided_services.push(declaration); @@ -921,7 +807,7 @@ impl FeatureServiceRegistrar<'_> { /// Install-time context provided to a feature module. pub struct FeatureInstallContext<'a> { feature_id: &'a FeatureId, - grants: &'a CapabilityGrantSet, + grants: &'a AuthorityGrantSet, pending_tools: &'a mut Vec, installed_tool_names: &'a mut HashMap, hook_builder: &'a mut HookRegistryBuilder, @@ -934,7 +820,7 @@ impl FeatureInstallContext<'_> { self.feature_id } - pub fn grants(&self) -> &CapabilityGrantSet { + pub fn grants(&self) -> &AuthorityGrantSet { self.grants } @@ -950,7 +836,6 @@ impl FeatureInstallContext<'_> { pub fn hooks(&mut self) -> HookContributionRegistrar<'_> { HookContributionRegistrar { - grants: self.grants, hook_builder: self.hook_builder, report: self.report, } @@ -958,7 +843,6 @@ impl FeatureInstallContext<'_> { pub fn background_tasks(&mut self) -> BackgroundTaskRegistrar<'_> { BackgroundTaskRegistrar { - grants: self.grants, report: self.report, } } @@ -966,7 +850,6 @@ impl FeatureInstallContext<'_> { pub fn services(&mut self) -> FeatureServiceRegistrar<'_> { FeatureServiceRegistrar { feature_id: self.feature_id, - grants: self.grants, service_registry: self.service_registry, report: self.report, } @@ -981,14 +864,12 @@ impl FeatureInstallContext<'_> { pub fn alerts(&mut self) -> FeatureAlertSink<'_> { FeatureAlertSink { - grants: self.grants, report: self.report, } } pub fn diagnostics(&mut self) -> FeatureDiagnosticSink<'_> { FeatureDiagnosticSink { - grants: self.grants, report: self.report, } } @@ -1076,7 +957,7 @@ impl FeatureRegistryBuilder { let mut seen_features = HashSet::new(); for (module, descriptor) in self.modules.into_iter().zip(descriptors.into_iter()) { - let grants = CapabilityGrantSet::grant_all(&descriptor.requested_capabilities); + let grants = AuthorityGrantSet::grant_all(&descriptor.requested_authorities); let mut report = FeatureInstallReport::new(&descriptor, grants.clone()); if !seen_features.insert(descriptor.id.clone()) { @@ -1093,38 +974,15 @@ impl FeatureRegistryBuilder { continue; } - for capability in grants.denied() { + for authority in grants.denied() { report.diagnostics.push(FeatureDiagnostic::warning(format!( - "capability denied: {:?}: {}", - capability.capability, capability.reason + "authority denied: {:?}: {}", + authority.authority, authority.reason ))); } let mut required_service_failed = false; for requirement in descriptor.requires_services.iter().cloned() { - let capability = HostCapability::RequireService { - service: requirement.id.clone(), - }; - if let Err(error) = require_capability( - &grants, - &mut report, - FeatureContributionKind::Service, - requirement.id.to_string(), - &capability, - ) { - if requirement.required { - report - .diagnostics - .push(FeatureDiagnostic::error(error.to_string())); - required_service_failed = true; - } else { - report - .diagnostics - .push(FeatureDiagnostic::warning(error.to_string())); - } - continue; - } - if service_registry.provides(&requirement.id) { report.resolved_service_requirements.push(requirement); } else if requirement.required { @@ -1159,34 +1017,22 @@ impl FeatureRegistryBuilder { } for background_task in descriptor.background_tasks.iter().cloned() { - match require_background_task_capability(&grants, &mut report, &background_task) { - Ok(()) => report.declared_background_tasks.push(background_task), - Err(error) => report - .diagnostics - .push(FeatureDiagnostic::warning(error.to_string())), - } + report.declared_background_tasks.push(background_task); } for service in descriptor.provides_services.iter().cloned() { - match require_service_provider_capability(&grants, &mut report, &service) { - Ok(()) => match service_registry - .register_provider(descriptor.id.clone(), service.clone()) - { - Ok(()) => report.provided_services.push(service), - Err(error) => { - report - .diagnostics - .push(FeatureDiagnostic::error(error.to_string())); - report.mark_skipped( - FeatureContributionKind::Service, - service.id.to_string(), - error.to_string(), - ); - } - }, - Err(error) => report - .diagnostics - .push(FeatureDiagnostic::warning(error.to_string())), + match service_registry.register_provider(descriptor.id.clone(), service.clone()) { + Ok(()) => report.provided_services.push(service), + Err(error) => { + report + .diagnostics + .push(FeatureDiagnostic::error(error.to_string())); + report.mark_skipped( + FeatureContributionKind::Service, + service.id.to_string(), + error.to_string(), + ); + } } } @@ -1249,8 +1095,8 @@ pub enum FeatureInstallError { first_feature: String, duplicate_feature: String, }, - #[error("feature capability denied: {0}")] - CapabilityDenied(String), + #[error("feature authority denied: {0}")] + AuthorityDenied(String), #[error("feature install failed: {0}")] Install(String), } @@ -1273,30 +1119,6 @@ pub mod builtin { fn descriptor(&self) -> FeatureDescriptor { FeatureDescriptor::builtin("task-tools", "Task tools") .with_description("Session-lifetime task tracking builtin tools") - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "TaskCreate".into(), - }, - "register TaskCreate builtin tool", - )) - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "TaskUpdate".into(), - }, - "register TaskUpdate builtin tool", - )) - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "TaskGet".into(), - }, - "register TaskGet builtin tool", - )) - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "TaskList".into(), - }, - "register TaskList builtin tool", - )) .with_tool(ToolDeclaration::new( "TaskCreate", "Create a session-lifetime user-visible task", @@ -1338,6 +1160,7 @@ mod tests { use llm_worker::llm_client::{ClientError, Request, ResponseStream}; use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; use serde_json::json; + use std::sync::atomic::{AtomicUsize, Ordering}; #[derive(Clone)] struct DummyClient; @@ -1396,20 +1219,8 @@ mod tests { } #[test] - fn descriptor_capabilities_and_install_report_are_recorded() { + fn descriptor_authorities_and_install_report_are_recorded() { let descriptor = FeatureDescriptor::builtin("dummy", "Dummy") - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "Dummy".into(), - }, - "test", - )) - .with_capability(CapabilityRequest::required( - HostCapability::DeclareBackgroundTask { - name: "daily".into(), - }, - "test background task declaration", - )) .with_tool(ToolDeclaration::new("Dummy", "dummy tool")) .with_background_task(BackgroundTaskDeclaration::descriptor_only( "daily", @@ -1431,32 +1242,14 @@ mod tests { assert!(feature_report.installed); assert_eq!(feature_report.installed_tools, vec!["Dummy"]); assert_eq!(feature_report.declared_background_tasks[0].name, "daily"); - assert!( - feature_report - .granted_capabilities - .contains(&HostCapability::ContributeTool { - name: "Dummy".into() - }) - ); + assert!(feature_report.granted_authorities.denied().is_empty()); } #[test] fn duplicate_tool_names_are_rejected() { let descriptor_a = FeatureDescriptor::builtin("a", "A") - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "Duplicate".into(), - }, - "test duplicate handling", - )) .with_tool(ToolDeclaration::new("Duplicate", "first tool")); let descriptor_b = FeatureDescriptor::builtin("b", "B") - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "Duplicate".into(), - }, - "test duplicate handling", - )) .with_tool(ToolDeclaration::new("Duplicate", "second tool")); let mut hook_builder = HookRegistryBuilder::default(); let mut pending_tools = Vec::new(); @@ -1491,12 +1284,6 @@ mod tests { #[test] fn mismatched_tool_contribution_name_is_rejected_before_queueing() { let descriptor = FeatureDescriptor::builtin("mismatch", "Mismatch") - .with_capability(CapabilityRequest::required( - HostCapability::ContributeTool { - name: "Actual".into(), - }, - "test mismatch handling", - )) .with_tool(ToolDeclaration::new("Actual", "actual model-visible tool")); let mut hook_builder = HookRegistryBuilder::default(); let mut pending_tools = Vec::new(); @@ -1518,6 +1305,61 @@ mod tests { assert_eq!(report.reports[0].skipped[0].name, "Actual"); } + #[test] + fn stateful_tool_definition_is_materialized_once_for_report_and_worker() { + struct StatefulToolFeature { + calls: Arc, + } + + impl FeatureModule for StatefulToolFeature { + fn descriptor(&self) -> FeatureDescriptor { + FeatureDescriptor::builtin("stateful-tool", "Stateful tool") + .with_tool(ToolDeclaration::new("First", "stateful tool")) + } + + fn install( + &self, + context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + let calls = Arc::clone(&self.calls); + let definition: ToolDefinition = Arc::new(move || { + let call_index = calls.fetch_add(1, Ordering::SeqCst); + let name = if call_index == 0 { "First" } else { "Second" }; + ( + ToolMeta::new(name) + .description("stateful") + .input_schema(json!({})), + Arc::new(DummyTool) as Arc, + ) + }); + context + .tools() + .register(ToolContribution::new("First", definition)) + } + } + + let calls = Arc::new(AtomicUsize::new(0)); + let mut worker = Worker::new(DummyClient); + let mut hook_builder = HookRegistryBuilder::default(); + let report = FeatureRegistryBuilder::new() + .with_module(StatefulToolFeature { + calls: Arc::clone(&calls), + }) + .install_into_worker(&mut worker, &mut hook_builder); + + worker.tool_server_handle().flush_pending(); + let names: Vec<_> = worker + .tool_server_handle() + .tool_definitions_sorted() + .into_iter() + .map(|tool| tool.name) + .collect(); + + assert_eq!(report.installed_tool_names(), vec!["First"]); + assert_eq!(names, vec!["First"]); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } + struct ServiceFeature { descriptor: FeatureDescriptor, } @@ -1538,50 +1380,19 @@ mod tests { #[test] fn service_requirements_resolve_against_prior_providers() { let service = ServiceId::builtin("demo-service"); - let provider = FeatureDescriptor::builtin("provider", "Provider") - .with_capability(CapabilityRequest::required( - HostCapability::ProvideService { - service: service.clone(), - }, - "provide demo service", - )) - .with_provided_service(ServiceDeclaration::new( - service.clone(), - "1", - "demo service", - )); + let provider = FeatureDescriptor::builtin("provider", "Provider").with_provided_service( + ServiceDeclaration::new(service.clone(), "1", "demo service"), + ); let consumer = FeatureDescriptor::builtin("consumer", "Consumer") - .with_capability(CapabilityRequest::required( - HostCapability::RequireService { - service: service.clone(), - }, - "require demo service", - )) .with_service_requirement(ServiceRequirement::required(service.clone(), "needs demo")); let missing_service = ServiceId::builtin("missing-service"); - let missing = FeatureDescriptor::builtin("missing", "Missing") - .with_capability(CapabilityRequest::required( - HostCapability::RequireService { - service: missing_service.clone(), - }, - "require missing service", - )) - .with_service_requirement(ServiceRequirement::required( - missing_service, - "needs missing", - )); + let missing = FeatureDescriptor::builtin("missing", "Missing").with_service_requirement( + ServiceRequirement::required(missing_service, "needs missing"), + ); let optional_service = ServiceId::builtin("optional-service"); - let optional = FeatureDescriptor::builtin("optional", "Optional") - .with_capability(CapabilityRequest::required( - HostCapability::RequireService { - service: optional_service.clone(), - }, - "optionally require service", - )) - .with_service_requirement(ServiceRequirement::optional( - optional_service, - "nice to have", - )); + let optional = FeatureDescriptor::builtin("optional", "Optional").with_service_requirement( + ServiceRequirement::optional(optional_service, "nice to have"), + ); let mut hook_builder = HookRegistryBuilder::default(); let mut pending_tools = Vec::new(); let report = FeatureRegistryBuilder::new() @@ -1620,11 +1431,11 @@ mod tests { } #[test] - fn background_task_declaration_without_capability_is_skipped() { - let descriptor = FeatureDescriptor::builtin("background-denied", "Background denied") + fn background_task_declaration_is_not_sandbox_authority_gated() { + let descriptor = FeatureDescriptor::builtin("background", "Background") .with_background_task(BackgroundTaskDeclaration::descriptor_only( - "denied-task", - "should be skipped", + "declared-task", + "descriptor contribution", )); let mut hook_builder = HookRegistryBuilder::default(); let mut pending_tools = Vec::new(); @@ -1633,25 +1444,19 @@ mod tests { .install_into_pending(&mut pending_tools, &mut hook_builder); assert!(report.reports[0].installed); - assert!(report.reports[0].declared_background_tasks.is_empty()); assert_eq!( - report.reports[0].skipped[0].kind, - FeatureContributionKind::BackgroundTask + report.reports[0].declared_background_tasks[0].name, + "declared-task" ); - assert!(report.reports[0].diagnostics.iter().any(|diagnostic| { - diagnostic - .message - .contains("required capability was not granted") - })); + assert!(report.reports[0].skipped.is_empty()); } #[test] - fn service_provider_without_capability_is_skipped() { - let service = ServiceId::builtin("denied-service"); - let descriptor = - FeatureDescriptor::builtin("service-denied", "Service denied").with_provided_service( - ServiceDeclaration::new(service.clone(), "1", "should be skipped"), - ); + fn service_provider_declaration_is_not_sandbox_authority_gated() { + let service = ServiceId::builtin("declared-service"); + let descriptor = FeatureDescriptor::builtin("service", "Service").with_provided_service( + ServiceDeclaration::new(service.clone(), "1", "descriptor contribution"), + ); let mut hook_builder = HookRegistryBuilder::default(); let mut pending_tools = Vec::new(); let report = FeatureRegistryBuilder::new() @@ -1659,17 +1464,9 @@ mod tests { .install_into_pending(&mut pending_tools, &mut hook_builder); assert!(report.reports[0].installed); - assert!(!report.services.provides(&service)); - assert!(report.reports[0].provided_services.is_empty()); - assert_eq!( - report.reports[0].skipped[0].kind, - FeatureContributionKind::Service - ); - assert!(report.reports[0].diagnostics.iter().any(|diagnostic| { - diagnostic - .message - .contains("required capability was not granted") - })); + assert!(report.services.provides(&service)); + assert_eq!(report.reports[0].provided_services[0].id, service); + assert!(report.reports[0].skipped.is_empty()); } #[test] From 6fa08f891779e10e22a9bc35b018f629dc5ad8bb Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 5 Jun 2026 07:21:07 +0900 Subject: [PATCH 4/4] fix: enforce feature descriptor contributions --- crates/pod/src/feature.rs | 336 +++++++++++++++++++++++++++++++++++++- 1 file changed, 332 insertions(+), 4 deletions(-) diff --git a/crates/pod/src/feature.rs b/crates/pod/src/feature.rs index 32a10e1e..377ac7d9 100644 --- a/crates/pod/src/feature.rs +++ b/crates/pod/src/feature.rs @@ -579,6 +579,75 @@ impl FeatureInstallReport { } } +#[derive(Clone, Debug)] +struct FeatureContributionDeclarations { + tools: HashSet, + hooks: HashSet<(String, FeatureHookPoint)>, + background_tasks: HashSet, + provided_services: HashSet<(ServiceId, String)>, +} + +impl FeatureContributionDeclarations { + fn from_descriptor(descriptor: &FeatureDescriptor) -> Self { + Self { + tools: descriptor + .tools + .iter() + .map(|tool| tool.name.clone()) + .collect(), + hooks: descriptor + .hooks + .iter() + .map(|hook| (hook.name.clone(), hook.point.clone())) + .collect(), + background_tasks: descriptor + .background_tasks + .iter() + .map(|task| task.name.clone()) + .collect(), + provided_services: descriptor + .provides_services + .iter() + .map(|service| (service.id.clone(), service.version.clone())) + .collect(), + } + } + + fn contains_tool(&self, name: &str) -> bool { + self.tools.contains(name) + } + + fn contains_hook(&self, declaration: &HookDeclaration) -> bool { + self.hooks + .contains(&(declaration.name.clone(), declaration.point.clone())) + } + + fn contains_background_task(&self, declaration: &BackgroundTaskDeclaration) -> bool { + self.background_tasks.contains(&declaration.name) + } + + fn contains_provided_service(&self, declaration: &ServiceDeclaration) -> bool { + self.provided_services + .contains(&(declaration.id.clone(), declaration.version.clone())) + } +} + +fn reject_undeclared_contribution( + feature_id: &FeatureId, + report: &mut FeatureInstallReport, + kind: FeatureContributionKind, + name: impl Into, +) -> FeatureInstallError { + let name = name.into(); + let error = FeatureInstallError::UndeclaredContribution { + kind: kind.clone(), + name: name.clone(), + feature: feature_id.to_string(), + }; + report.mark_skipped(kind, name, error.to_string()); + error +} + fn require_authority( grants: &AuthorityGrantSet, report: &mut FeatureInstallReport, @@ -669,6 +738,7 @@ impl FeatureDiagnosticSink<'_> { /// Tool contribution registrar exposed inside [`FeatureInstallContext`]. pub struct ToolContributionRegistrar<'a> { feature_id: &'a FeatureId, + declarations: &'a FeatureContributionDeclarations, grants: &'a AuthorityGrantSet, pending_tools: &'a mut Vec, installed_tool_names: &'a mut HashMap, @@ -692,6 +762,15 @@ impl ToolContributionRegistrar<'_> { return Err(error); } + if !self.declarations.contains_tool(&model_visible_name) { + return Err(reject_undeclared_contribution( + self.feature_id, + self.report, + FeatureContributionKind::Tool, + model_visible_name, + )); + } + for authority in &contribution.required_authorities { require_authority( self.grants, @@ -727,17 +806,35 @@ impl ToolContributionRegistrar<'_> { /// Safe hook contribution registrar backed by [`HookRegistryBuilder`]. pub struct HookContributionRegistrar<'a> { + feature_id: &'a FeatureId, + declarations: &'a FeatureContributionDeclarations, hook_builder: &'a mut HookRegistryBuilder, report: &'a mut FeatureInstallReport, } impl HookContributionRegistrar<'_> { + fn require_declared( + &mut self, + declaration: &HookDeclaration, + ) -> Result<(), FeatureInstallError> { + if self.declarations.contains_hook(declaration) { + return Ok(()); + } + Err(reject_undeclared_contribution( + self.feature_id, + self.report, + FeatureContributionKind::Hook, + format!("{}:{:?}", declaration.name, declaration.point), + )) + } + pub fn add_pre_request( &mut self, name: impl Into, hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::PreRequest); + self.require_declared(&declaration)?; self.hook_builder.add_pre_llm_request(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -749,6 +846,7 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::PreToolCall); + self.require_declared(&declaration)?; self.hook_builder.add_pre_tool_call(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -760,6 +858,7 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::ToolResult); + self.require_declared(&declaration)?; self.hook_builder.add_post_tool_call(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -771,6 +870,7 @@ impl HookContributionRegistrar<'_> { hook: impl Hook + 'static, ) -> Result<(), FeatureInstallError> { let declaration = HookDeclaration::new(name, FeatureHookPoint::TurnEnd); + self.require_declared(&declaration)?; self.hook_builder.add_on_turn_end(hook); self.report.installed_hooks.push(declaration); Ok(()) @@ -779,24 +879,62 @@ impl HookContributionRegistrar<'_> { /// Background task registrar for descriptor/report-only contributions. pub struct BackgroundTaskRegistrar<'a> { + feature_id: &'a FeatureId, + declarations: &'a FeatureContributionDeclarations, report: &'a mut FeatureInstallReport, } impl BackgroundTaskRegistrar<'_> { - pub fn declare(&mut self, declaration: BackgroundTaskDeclaration) { - self.report.declared_background_tasks.push(declaration); + pub fn declare( + &mut self, + declaration: BackgroundTaskDeclaration, + ) -> Result<(), FeatureInstallError> { + if !self.declarations.contains_background_task(&declaration) { + return Err(reject_undeclared_contribution( + self.feature_id, + self.report, + FeatureContributionKind::BackgroundTask, + declaration.name, + )); + } + if !self + .report + .declared_background_tasks + .iter() + .any(|task| task.name == declaration.name) + { + self.report.declared_background_tasks.push(declaration); + } + Ok(()) } } /// Service registrar for descriptor/report-only provider metadata. pub struct FeatureServiceRegistrar<'a> { feature_id: &'a FeatureId, + declarations: &'a FeatureContributionDeclarations, service_registry: &'a mut FeatureServiceRegistry, report: &'a mut FeatureInstallReport, } impl FeatureServiceRegistrar<'_> { pub fn provide(&mut self, declaration: ServiceDeclaration) -> Result<(), FeatureInstallError> { + if !self.declarations.contains_provided_service(&declaration) { + return Err(reject_undeclared_contribution( + self.feature_id, + self.report, + FeatureContributionKind::Service, + declaration.id.to_string(), + )); + } + if self + .report + .provided_services + .iter() + .any(|service| service.id == declaration.id && service.version == declaration.version) + { + return Ok(()); + } self.service_registry .register_provider(self.feature_id.clone(), declaration.clone())?; self.report.provided_services.push(declaration); @@ -807,6 +945,7 @@ impl FeatureServiceRegistrar<'_> { /// Install-time context provided to a feature module. pub struct FeatureInstallContext<'a> { feature_id: &'a FeatureId, + declarations: &'a FeatureContributionDeclarations, grants: &'a AuthorityGrantSet, pending_tools: &'a mut Vec, installed_tool_names: &'a mut HashMap, @@ -827,6 +966,7 @@ impl FeatureInstallContext<'_> { pub fn tools(&mut self) -> ToolContributionRegistrar<'_> { ToolContributionRegistrar { feature_id: self.feature_id, + declarations: self.declarations, grants: self.grants, pending_tools: self.pending_tools, installed_tool_names: self.installed_tool_names, @@ -836,6 +976,8 @@ impl FeatureInstallContext<'_> { pub fn hooks(&mut self) -> HookContributionRegistrar<'_> { HookContributionRegistrar { + feature_id: self.feature_id, + declarations: self.declarations, hook_builder: self.hook_builder, report: self.report, } @@ -843,6 +985,8 @@ impl FeatureInstallContext<'_> { pub fn background_tasks(&mut self) -> BackgroundTaskRegistrar<'_> { BackgroundTaskRegistrar { + feature_id: self.feature_id, + declarations: self.declarations, report: self.report, } } @@ -850,6 +994,7 @@ impl FeatureInstallContext<'_> { pub fn services(&mut self) -> FeatureServiceRegistrar<'_> { FeatureServiceRegistrar { feature_id: self.feature_id, + declarations: self.declarations, service_registry: self.service_registry, report: self.report, } @@ -958,6 +1103,7 @@ impl FeatureRegistryBuilder { for (module, descriptor) in self.modules.into_iter().zip(descriptors.into_iter()) { let grants = AuthorityGrantSet::grant_all(&descriptor.requested_authorities); + let declarations = FeatureContributionDeclarations::from_descriptor(&descriptor); let mut report = FeatureInstallReport::new(&descriptor, grants.clone()); if !seen_features.insert(descriptor.id.clone()) { @@ -1039,6 +1185,7 @@ impl FeatureRegistryBuilder { let install_result = { let mut context = FeatureInstallContext { feature_id: &descriptor.id, + declarations: &declarations, grants: &grants, pending_tools, installed_tool_names: &mut installed_tool_names, @@ -1087,6 +1234,14 @@ pub enum FeatureInstallError { declared: String, model_visible: String, }, + #[error( + "undeclared {kind:?} contribution `{name}` from feature `{feature}` is not present in the approved feature descriptor" + )] + UndeclaredContribution { + kind: FeatureContributionKind, + name: String, + feature: String, + }, #[error( "duplicate service declaration `{service}` from feature `{duplicate_feature}`; first provided by `{first_feature}`" )] @@ -1141,8 +1296,11 @@ pub mod builtin { &self, context: &mut FeatureInstallContext<'_>, ) -> Result<(), FeatureInstallError> { - for definition in tools::task_tools(self.task_store.clone()) { - let name = (definition)().0.name; + let names = ["TaskCreate", "TaskList", "TaskGet", "TaskUpdate"]; + for (name, definition) in names + .into_iter() + .zip(tools::task_tools(self.task_store.clone())) + { context .tools() .register(ToolContribution::new(name, definition))?; @@ -1377,6 +1535,176 @@ mod tests { } } + struct DummyPreToolHook; + + #[async_trait] + impl Hook for DummyPreToolHook { + async fn call( + &self, + _input: &crate::hook::ToolCallSummary, + ) -> crate::hook::HookPreToolAction { + crate::hook::HookPreToolAction::Continue + } + } + + struct HookFeature { + descriptor: FeatureDescriptor, + hook_name: &'static str, + } + + impl FeatureModule for HookFeature { + fn descriptor(&self) -> FeatureDescriptor { + self.descriptor.clone() + } + + fn install( + &self, + context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + context + .hooks() + .add_pre_tool_call(self.hook_name, DummyPreToolHook) + } + } + + struct BackgroundFeature { + descriptor: FeatureDescriptor, + task_name: &'static str, + } + + impl FeatureModule for BackgroundFeature { + fn descriptor(&self) -> FeatureDescriptor { + self.descriptor.clone() + } + + fn install( + &self, + context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + context + .background_tasks() + .declare(BackgroundTaskDeclaration::descriptor_only( + self.task_name, + "runtime background task", + )) + } + } + + struct ServiceProviderFeature { + descriptor: FeatureDescriptor, + service: ServiceId, + } + + impl FeatureModule for ServiceProviderFeature { + fn descriptor(&self) -> FeatureDescriptor { + self.descriptor.clone() + } + + fn install( + &self, + context: &mut FeatureInstallContext<'_>, + ) -> Result<(), FeatureInstallError> { + context.services().provide(ServiceDeclaration::new( + self.service.clone(), + "1", + "runtime service provider", + )) + } + } + + #[test] + fn undeclared_tool_contribution_is_rejected() { + let descriptor = FeatureDescriptor::builtin("undeclared-tool", "Undeclared tool"); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ToolFeature { + descriptor, + contribution_name: "HiddenTool", + model_visible_name: "HiddenTool", + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(pending_tools.is_empty()); + assert!(!report.reports[0].installed); + assert_eq!( + report.reports[0].skipped[0].kind, + FeatureContributionKind::Tool + ); + assert_eq!(report.reports[0].skipped[0].name, "HiddenTool"); + assert!(report.reports[0].diagnostics.iter().any(|diagnostic| { + diagnostic + .message + .contains("is not present in the approved feature descriptor") + })); + } + + #[test] + fn undeclared_hook_contribution_is_rejected() { + let descriptor = FeatureDescriptor::builtin("undeclared-hook", "Undeclared hook"); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(HookFeature { + descriptor, + hook_name: "hidden-hook", + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(!report.reports[0].installed); + assert!(report.reports[0].installed_hooks.is_empty()); + assert_eq!( + report.reports[0].skipped[0].kind, + FeatureContributionKind::Hook + ); + assert!(report.reports[0].skipped[0].name.contains("hidden-hook")); + } + + #[test] + fn undeclared_background_task_contribution_is_rejected() { + let descriptor = + FeatureDescriptor::builtin("undeclared-background", "Undeclared background"); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(BackgroundFeature { + descriptor, + task_name: "hidden-task", + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(!report.reports[0].installed); + assert!(report.reports[0].declared_background_tasks.is_empty()); + assert_eq!( + report.reports[0].skipped[0].kind, + FeatureContributionKind::BackgroundTask + ); + assert_eq!(report.reports[0].skipped[0].name, "hidden-task"); + } + + #[test] + fn undeclared_service_provider_contribution_is_rejected() { + let service = ServiceId::builtin("hidden-service"); + let descriptor = FeatureDescriptor::builtin("undeclared-service", "Undeclared service"); + let mut hook_builder = HookRegistryBuilder::default(); + let mut pending_tools = Vec::new(); + let report = FeatureRegistryBuilder::new() + .with_module(ServiceProviderFeature { + descriptor, + service: service.clone(), + }) + .install_into_pending(&mut pending_tools, &mut hook_builder); + + assert!(!report.reports[0].installed); + assert!(!report.services.provides(&service)); + assert!(report.reports[0].provided_services.is_empty()); + assert_eq!( + report.reports[0].skipped[0].kind, + FeatureContributionKind::Service + ); + assert_eq!(report.reports[0].skipped[0].name, service.to_string()); + } + #[test] fn service_requirements_resolve_against_prior_providers() { let service = ServiceId::builtin("demo-service");