diff --git a/src/server/rpc/tools_ns.rs b/src/server/rpc/tools_ns.rs index c638c04..008742d 100644 --- a/src/server/rpc/tools_ns.rs +++ b/src/server/rpc/tools_ns.rs @@ -1,8 +1,12 @@ //! `grob/tools/*` namespace: tool layer inspection and management. use super::auth::{require_role, CallerIdentity}; -use super::types::{Role, StatusResponse}; -use crate::server::AppState; +use super::types::{rpc_err, Role, StatusResponse, ERR_INTERNAL}; +use crate::features::tool_layer::config::InjectRule; +use crate::providers::ProviderRegistry; +use crate::routing::classify::Router; +use crate::server::{AppState, ReloadableState}; +use jsonrpsee::types::error::INVALID_PARAMS_CODE; use jsonrpsee::types::ErrorObjectOwned; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -45,40 +49,137 @@ pub async fn list( Ok(tools) } -/// Enables a tool by name (in-memory only). +/// Enables a tool by name (adds an inject rule, in-memory only). +/// +/// Appends `InjectRule { tool, if_absent: true }` to +/// `config.tool_layer.inject`, mirroring TOML-declared rules. Disk +/// persistence is out of scope (#228); the change reverts on next reload. +/// +/// # Errors +/// +/// Returns `ERR_FORBIDDEN` when the caller is below `Admin`. +/// Returns `INVALID_PARAMS_CODE` when `tool` is empty or already enabled. +/// Returns `ERR_INTERNAL` when the registry rebuild or atomic swap fails. pub async fn enable( state: &Arc, caller: &CallerIdentity, - _tool: &str, + tool: &str, ) -> Result { require_role(caller, Role::Admin)?; - - // TODO(#228): Implement runtime tool enable with config mutation. - let _ = state; - + let mut new_config = state.snapshot().config.clone(); + apply_enable(&mut new_config, tool)?; + swap_state(state, new_config, caller, &format!("enable tool '{tool}'"))?; Ok(StatusResponse { status: "ok".into(), - message: Some("Tool enabled (in-memory — reload to persist)".into()), + message: Some(format!( + "Tool '{tool}' enabled (in-memory only — change reverts on next disk reload)" + )), }) } -/// Disables a tool by name (in-memory only). +/// Disables a tool by name (removes the inject rule, in-memory only). +/// +/// All matching `InjectRule` entries are removed from +/// `config.tool_layer.inject`. Aliases that map TO the disabled tool are +/// kept in place so client requests still resolve through them. Disk +/// persistence is out of scope (#228); the change reverts on next reload. +/// +/// # Errors +/// +/// Returns `ERR_FORBIDDEN` when the caller is below `Admin`. +/// Returns `INVALID_PARAMS_CODE` when `tool` is empty or not currently enabled. +/// Returns `ERR_INTERNAL` when the registry rebuild or atomic swap fails. pub async fn disable( state: &Arc, caller: &CallerIdentity, - _tool: &str, + tool: &str, ) -> Result { require_role(caller, Role::Admin)?; - - // TODO(#228): Implement runtime tool disable with config mutation. - let _ = state; - + let mut new_config = state.snapshot().config.clone(); + apply_disable(&mut new_config, tool)?; + swap_state(state, new_config, caller, &format!("disable tool '{tool}'"))?; Ok(StatusResponse { status: "ok".into(), - message: Some("Tool disabled (in-memory — reload to persist)".into()), + message: Some(format!( + "Tool '{tool}' disabled (in-memory only — change reverts on next disk reload)" + )), }) } +/// Pure mutation helper for `enable`. Validates the tool name and pushes +/// the new `InjectRule` into the supplied config. Separated from the +/// async handler so the validation logic is unit-testable without an +/// `AppState`. +fn apply_enable( + config: &mut crate::models::config::AppConfig, + tool: &str, +) -> Result<(), ErrorObjectOwned> { + if tool.trim().is_empty() { + return Err(rpc_err(INVALID_PARAMS_CODE, "tool name cannot be empty")); + } + if config.tool_layer.inject.iter().any(|r| r.tool == tool) { + return Err(rpc_err( + INVALID_PARAMS_CODE, + format!("tool '{tool}' is already enabled"), + )); + } + config.tool_layer.inject.push(InjectRule { + tool: tool.to_string(), + if_absent: true, + }); + Ok(()) +} + +/// Pure mutation helper for `disable`. Removes any inject rule matching +/// `tool` from the supplied config. Separated for unit-testability. +fn apply_disable( + config: &mut crate::models::config::AppConfig, + tool: &str, +) -> Result<(), ErrorObjectOwned> { + if tool.trim().is_empty() { + return Err(rpc_err(INVALID_PARAMS_CODE, "tool name cannot be empty")); + } + let before = config.tool_layer.inject.len(); + config.tool_layer.inject.retain(|r| r.tool != tool); + if config.tool_layer.inject.len() == before { + return Err(rpc_err( + INVALID_PARAMS_CODE, + format!("tool '{tool}' is not enabled"), + )); + } + Ok(()) +} + +/// Rebuilds reloadable state from a mutated config and atomically swaps +/// it. Mirrors the helper used by `config_ns::set` and `server_ns::reload_config`. +/// In-memory only — see [`tools/enable`] / [`tools/disable`] for rationale. +fn swap_state( + state: &Arc, + new_config: crate::models::config::AppConfig, + caller: &CallerIdentity, + action: &str, +) -> Result<(), ErrorObjectOwned> { + let new_router = Router::new(new_config.clone()); + let new_registry = ProviderRegistry::from_configs_with_models( + &new_config.providers, + Some(state.token_store.clone()), + &new_config.models, + &new_config.server.timeouts, + ) + .map(Arc::new) + .map_err(|e| rpc_err(ERR_INTERNAL, format!("Failed to rebuild providers: {e}")))?; + + let new_inner = Arc::new(ReloadableState::new(new_config, new_router, new_registry)); + *state.inner.write().unwrap_or_else(|e| e.into_inner()) = new_inner; + + tracing::info!( + caller_ip = %caller.ip, + action = action, + "RPC tools/* applied (in-memory only)" + ); + Ok(()) +} + /// Returns the full tool catalog (inject rules + aliases + capabilities). pub async fn catalog( state: &Arc, @@ -137,6 +238,95 @@ pub async fn catalog( #[cfg(test)] mod tests { use super::*; + use crate::models::config::AppConfig; + use crate::server::rpc::types::ERR_FORBIDDEN; + + /// Minimal AppConfig for tool_layer mutation tests. + fn fixture_config() -> AppConfig { + let toml = r#" +[router] +default = "claude-sonnet-4-6" + +[cache] +enabled = false +max_capacity = 100 +ttl_secs = 60 +max_entry_bytes = 8192 + +[tool_layer] +enabled = true + +[[tool_layer.inject]] +tool = "bash" +if_absent = true +"#; + toml::from_str(toml).expect("valid test TOML") + } + + #[test] + fn enable_appends_inject_rule() { + let mut config = fixture_config(); + apply_enable(&mut config, "grep").expect("enable should succeed"); + assert_eq!(config.tool_layer.inject.len(), 2); + let last = config.tool_layer.inject.last().unwrap(); + assert_eq!(last.tool, "grep"); + assert!(last.if_absent); + } + + #[test] + fn enable_rejects_empty_tool_name() { + let mut config = fixture_config(); + let err = apply_enable(&mut config, " ").unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + } + + #[test] + fn enable_rejects_duplicate() { + let mut config = fixture_config(); + // `bash` is already in the fixture; enabling it again must error. + let err = apply_enable(&mut config, "bash").unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + assert!(err.message().contains("already enabled")); + // Config must not have grown. + assert_eq!(config.tool_layer.inject.len(), 1); + } + + #[test] + fn disable_removes_inject_rule() { + let mut config = fixture_config(); + apply_disable(&mut config, "bash").expect("disable should succeed"); + assert!(config.tool_layer.inject.is_empty()); + } + + #[test] + fn disable_rejects_not_enabled() { + let mut config = fixture_config(); + let err = apply_disable(&mut config, "grep").unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + assert!(err.message().contains("not enabled")); + } + + #[test] + fn disable_rejects_empty_tool_name() { + let mut config = fixture_config(); + let err = apply_disable(&mut config, "").unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + } + + #[test] + fn require_role_denies_observer_for_admin_methods() { + // Documents the role gate at the top of `enable` / `disable`. The + // role check uses the same primitive as `config_ns::set` and is + // covered end-to-end by the auth.rs tests; this case asserts the + // contract surface for `tools/*`. + let observer = CallerIdentity { + role: Role::Observer, + ip: "10.0.0.1".into(), + tenant_id: String::new(), + }; + let err = require_role(&observer, Role::Admin).unwrap_err(); + assert_eq!(err.code(), ERR_FORBIDDEN); + } #[test] fn tool_info_serialization() {