style: applied uniform style across the new rag and mcp hook implementation

src/config/agent.rs

@@ -356,9 +356,6 @@ impl Agent {
             }
         };
 
-        // Resolved from the config under construction: no context carries
-        // this agent yet, but its whitelist gate and hooks must still apply
-        // to the RAG builds below.
         let rag_sync_hooks = RagSyncHooks::resolve_for_agent(
             &app.hooks,
             &agent_config.global_hooks,

src/config/mcp_factory.rs

@@ -95,22 +95,11 @@ impl McpFactory {
     ) -> Result<Arc<ConnectedServer>> {
         let key = McpServerKey::from_spec(name, spec);
 
-        // Reuse of a live server fires no hooks; only a real spawn below
-        // reports anything. The live probe and the reconnect read share one
-        // lock section so a racing first connect that inserts between them
-        // cannot be misreported as a reconnect. The spawn await below stays
-        // outside the lock, so two racing first connects may still both
-        // spawn (no singleflight); the loser then fires a second plain
-        // `connected` (no RECONNECT flag) and its insert overwrites the
-        // winner's map entry.
         let reconnect = {
             let map = self.active.lock();
             if let Some(existing) = map.get(&key).and_then(|weak| weak.upgrade()) {
                 return Ok(existing);
             }
-            // The live probe failed, so an entry still present for this key
-            // is a dead weak: the key was connected earlier in this process
-            // and the spawn below is a reconnect.
             map.contains_key(&key)
         };
 

src/config/request_context.rs

@@ -54,6 +54,7 @@ use crate::hooks::{self, HookEvent};
 use anyhow::{Context, Error, Result, bail};
 use colored::Colorize;
 use gman::providers::SupportedProvider;
+use hooks::{McpServerHooks, RagSyncHooks};
 use indexmap::IndexMap;
 use indoc::formatdoc;
 use inquire::{Confirm, MultiSelect, Text, list_option::ListOption, validator::Validation};
@@ -4411,7 +4412,7 @@ impl RequestContext {
 
             if !server_ids.is_empty() {
                 let app_ref = &self.app;
-                let mcp_hooks = hooks::McpServerHooks::resolve(self);
+                let mcp_hooks = McpServerHooks::resolve(self);
                 let acquire_all = async {
                     let mut handles = Vec::new();
                     let mut auth_required = Vec::new();
@@ -5350,7 +5351,7 @@ impl RequestContext {
         let vault = self.app.vault.clone();
         let rag_cache = self.rag_cache();
         let working_mode = self.working_mode;
-        let sync_hooks = hooks::RagSyncHooks::resolve(self);
+        let sync_hooks = RagSyncHooks::resolve(self);
 
         let (rag, rag_key): (Arc<Rag>, Option<RagKey>) = match rag {
             None => {
@@ -5484,7 +5485,7 @@ impl RequestContext {
             false,
             &self.app.config,
             abort_signal,
-            hooks::RagSyncHooks::resolve(self),
+            RagSyncHooks::resolve(self),
         )
         .await?;
         self.rag = Some(Arc::new(rag));
@@ -5521,7 +5522,7 @@ impl RequestContext {
             true,
             &self.app.config,
             abort_signal,
-            hooks::RagSyncHooks::resolve(self),
+            RagSyncHooks::resolve(self),
         )
         .await?;
         self.rag = Some(Arc::new(rag));

src/function/agents.rs

@@ -16,6 +16,7 @@ use crate::graph;
 use crate::repl::DEFAULT_CONTINUATION_PROMPT;
 use anyhow::{Context, Result, anyhow, bail};
 use chrono::Utc;
+use hooks::McpServerHooks;
 use indexmap::IndexMap;
 use log::{debug, warn};
 use parking_lot::RwLock;
@@ -961,7 +962,7 @@ async fn populate_agent_mcp_runtime(ctx: &mut RequestContext, server_ids: &[Stri
         })
         .unwrap_or_default();
 
-    let mcp_hooks = hooks::McpServerHooks::resolve(ctx);
+    let mcp_hooks = McpServerHooks::resolve(ctx);
     for (id, spec) in server_specs {
         let handle = app
             .mcp_factory

src/hooks.rs

@@ -4,7 +4,7 @@ use crate::config::{
 };
 use crate::function::write_file_atomic;
 
-use anyhow::{Result, anyhow};
+use anyhow::{Error, Result, anyhow};
 use chrono::{SecondsFormat, Utc};
 use indexmap::IndexMap;
 use rand::distr::{Alphanumeric, SampleString};
@@ -223,12 +223,6 @@ pub struct ResolvedHook {
     pub cwd: PathBuf,
 }
 
-/// Pre-resolved `rag.sync.*` hooks plus the names `base_envs_parts` needs,
-/// carried by value into the context-free RAG build funnels (`Rag::init`,
-/// `Rag::init_with_config`, `Rag::refresh_document_paths`). Resolution
-/// happens where the caller's identity is known; dispatch happens inside the
-/// funnel, bracketing the one real sync it performs — so cache hits, pure
-/// loads, and attaches (which never enter a funnel) fire nothing.
 #[derive(Debug, Clone, Default)]
 pub struct RagSyncHooks {
     started: Vec<ResolvedHook>,
@@ -252,10 +246,6 @@ impl RagSyncHooks {
         }
     }
 
-    /// Resolution for `Agent::init`, which builds agent and graph-node RAGs
-    /// before any context carries the agent: the whitelist gate and the
-    /// agent's own hooks come straight from the config being constructed.
-    /// No session or role exists at that point.
     pub fn resolve_for_agent(
         global_hooks: &HooksMap,
         gate: &[String],
@@ -300,9 +290,7 @@ impl RagSyncHooks {
         );
     }
 
-    /// A user abort mid-sync counts as a failure: the funnel's sync came
-    /// back `Err` either way, and the knowledge base was not (re)built.
-    pub fn fire_failed(&self, rag_name: &str, rag_path: &str, error: &anyhow::Error) {
+    pub fn fire_failed(&self, rag_name: &str, rag_path: &str, error: &Error) {
         self.fire(
             HookEvent::RagSyncFailed,
             &self.failed,
@@ -337,10 +325,6 @@ impl RagSyncHooks {
     }
 }
 
-/// Pre-resolved `mcp.server.*` hooks plus the names `base_envs_parts` needs,
-/// carried into the context-free `McpFactory::acquire`. Dispatch happens at
-/// the spawn itself — one event per real spawn — so handing back an
-/// already-live server fires nothing.
 #[derive(Debug, Clone, Default)]
 pub struct McpServerHooks {
     connected: Vec<ResolvedHook>,
@@ -362,8 +346,6 @@ impl McpServerHooks {
         }
     }
 
-    /// `reconnect` marks a spawn for a server key that was live earlier in
-    /// this process; the variable is omitted entirely on a first connect.
     pub fn fire_connected(&self, server: &str, transport: &str, reconnect: bool) {
         let mut extras = vec![
             ("COYOTE_MCP_SERVER", server.to_string()),
@@ -375,15 +357,7 @@ impl McpServerHooks {
         self.fire(HookEvent::McpServerConnected, &self.connected, &extras);
     }
 
-    /// `auth_required` marks an `McpAuthRequired` failure; the variable is
-    /// omitted on every other error.
-    pub fn fire_failed(
-        &self,
-        server: &str,
-        transport: &str,
-        error: &anyhow::Error,
-        auth_required: bool,
-    ) {
+    pub fn fire_failed(&self, server: &str, transport: &str, error: &Error, auth_required: bool) {
         let mut extras = vec![
             ("COYOTE_MCP_SERVER", server.to_string()),
             ("COYOTE_MCP_TRANSPORT", transport.to_string()),
@@ -710,12 +684,6 @@ pub(crate) fn base_envs_parts(
     envs
 }
 
-/// `COYOTE_ROLE` extras for the `session.*` and `turn.*` fire sites — the
-/// only two families that carry the variable. Resolved at fire time, because
-/// roles change mid-session (`.role`, `.exit role`, temp roles): the role
-/// held directly on the context wins, then the name of a role a session has
-/// absorbed. Derived roles have no name; resolution then falls through to
-/// the session-held role, if any.
 pub fn role_extras(ctx: &RequestContext) -> Vec<(&'static str, String)> {
     ctx.role
         .as_ref()

src/mcp/mod.rs

@@ -401,10 +401,6 @@ impl McpRegistry {
         Ok(())
     }
 
-    /// Boot-time connects fire no `mcp.server.*` hooks: this runs from
-    /// `AppState::new`, before any `RequestContext` exists to resolve hook
-    /// scopes against. Hook-observed connects go through
-    /// `McpFactory::acquire`.
     async fn start_server(&self, id: String) -> Result<Option<(String, Arc<ConnectedServer>)>> {
         let spec = self
             .config

src/rag/mod.rs

@@ -167,8 +167,7 @@ impl Rag {
             sync_hooks.fire_failed(&rag.name, &rag.path, &err);
             return Err(err);
         }
-        // rag.sync.completed promises a saved knowledge base, so it fires
-        // only after a successful save; a save error is a sync failure.
+
         match rag.save() {
             Ok(true) => println!("✓ Saved RAG to '{}'.", save_path.display()),
             Ok(false) => {}
@@ -332,8 +331,7 @@ impl Rag {
             sync_hooks.fire_failed(&rag.name, &rag.path, &err);
             return Err(err);
         }
-        // rag.sync.completed promises a saved knowledge base, so it fires
-        // only after a successful save; a save error is a sync failure.
+
         match rag.save() {
             Ok(true) => println!("✓ Saved RAG to '{}'.", save_path.display()),
             Ok(false) => {}
@@ -686,8 +684,7 @@ impl Rag {
             sync_hooks.fire_failed(&self.name, &self.path, &err);
             return Err(err);
         }
-        // rag.sync.completed promises a saved knowledge base, so it fires
-        // only after a successful save; a save error is a sync failure.
+
         match self.save() {
             Ok(true) => println!("✓ Saved rag to '{}'.", self.path),
             Ok(false) => {}