diff --git a/src/config/agent.rs b/src/config/agent.rs index 0451902..24b7417 100644 --- a/src/config/agent.rs +++ b/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, diff --git a/src/config/mcp_factory.rs b/src/config/mcp_factory.rs index 5153d68..c09f5f4 100644 --- a/src/config/mcp_factory.rs +++ b/src/config/mcp_factory.rs @@ -95,22 +95,11 @@ impl McpFactory { ) -> Result> { 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) }; diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 271fd0d..d974b16 100644 --- a/src/config/request_context.rs +++ b/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, Option) = 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)); diff --git a/src/function/agents.rs b/src/function/agents.rs index 24b2063..3083d24 100644 --- a/src/function/agents.rs +++ b/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 diff --git a/src/hooks.rs b/src/hooks.rs index eaa5c65..f0cf11a 100644 --- a/src/hooks.rs +++ b/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, @@ -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, @@ -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() diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index fe3059a..72f3930 100644 --- a/src/mcp/mod.rs +++ b/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)>> { let spec = self .config diff --git a/src/rag/mod.rs b/src/rag/mod.rs index 129962d..f88cca8 100644 --- a/src/rag/mod.rs +++ b/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) => {}