diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index e529d9dfc..b426de7eb 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -8,10 +8,11 @@ use crate::reasons::REASON_DENIED; use crate::reasons::REASON_NOT_ALLOWED; use crate::reasons::REASON_NOT_ALLOWED_LOCAL; use crate::state::NetworkProxyConstraints; -use crate::state::build_config_state; +use crate::state::build_default_config_state_and_reloader; use crate::state::validate_policy_against_constraints; use anyhow::Context; use anyhow::Result; +use async_trait::async_trait; use codex_utils_absolute_path::AbsolutePathBuf; use globset::GlobSet; use serde::Serialize; @@ -22,7 +23,6 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; -use std::time::SystemTime; use time::OffsetDateTime; use tokio::net::lookup_host; use tokio::sync::RwLock; @@ -110,27 +110,22 @@ pub(crate) struct ConfigState { pub(crate) allow_set: GlobSet, pub(crate) deny_set: GlobSet, pub(crate) constraints: NetworkProxyConstraints, - pub(crate) layer_mtimes: Vec, pub(crate) cfg_path: PathBuf, pub(crate) blocked: VecDeque, } -#[derive(Clone)] -pub(crate) struct LayerMtime { - pub(crate) path: PathBuf, - pub(crate) mtime: Option, +#[async_trait] +pub(crate) trait ConfigReloader: Send + Sync { + /// Return a freshly loaded state if a reload is needed; otherwise, return `None`. + async fn maybe_reload(&self) -> Result>; + + /// Force a reload, regardless of whether a change was detected. + async fn reload_now(&self) -> Result; } -impl LayerMtime { - pub(crate) fn new(path: PathBuf) -> Self { - let mtime = path.metadata().and_then(|m| m.modified()).ok(); - Self { path, mtime } - } -} - -#[derive(Clone)] pub struct NetworkProxyState { state: Arc>, + reloader: Arc, } impl std::fmt::Debug for NetworkProxyState { @@ -141,12 +136,26 @@ impl std::fmt::Debug for NetworkProxyState { } } +impl Clone for NetworkProxyState { + fn clone(&self) -> Self { + Self { + state: self.state.clone(), + reloader: self.reloader.clone(), + } + } +} + impl NetworkProxyState { pub async fn new() -> Result { - let cfg_state = build_config_state().await?; - Ok(Self { - state: Arc::new(RwLock::new(cfg_state)), - }) + let (cfg_state, reloader) = build_default_config_state_and_reloader().await?; + Ok(Self::with_reloader(cfg_state, Arc::new(reloader))) + } + + pub(crate) fn with_reloader(state: ConfigState, reloader: Arc) -> Self { + Self { + state: Arc::new(RwLock::new(state)), + reloader, + } } pub async fn current_cfg(&self) -> Result { @@ -178,7 +187,7 @@ impl NetworkProxyState { (guard.config.clone(), guard.cfg_path.clone()) }; - match build_config_state().await { + match self.reloader.reload_now().await { Ok(mut new_state) => { // Policy changes are operationally sensitive; logging diffs makes changes traceable // without needing to dump full config blobs (which can include unrelated settings). @@ -367,24 +376,22 @@ impl NetworkProxyState { } async fn reload_if_needed(&self) -> Result<()> { - let needs_reload = { - let guard = self.state.read().await; - guard.layer_mtimes.iter().any(|layer| { - let metadata = std::fs::metadata(&layer.path).ok(); - match (metadata.and_then(|m| m.modified().ok()), layer.mtime) { - (Some(new_mtime), Some(old_mtime)) => new_mtime > old_mtime, - (Some(_), None) => true, - (None, Some(_)) => true, - (None, None) => false, - } - }) - }; - - if !needs_reload { - return Ok(()); + match self.reloader.maybe_reload().await? { + None => Ok(()), + Some(mut new_state) => { + let (previous_cfg, blocked) = { + let guard = self.state.read().await; + (guard.config.clone(), guard.blocked.clone()) + }; + log_policy_changes(&previous_cfg, &new_state.config); + new_state.blocked = blocked; + let mut guard = self.state.write().await; + *guard = new_state; + let path = guard.cfg_path.display(); + info!("reloaded config from {path}"); + Ok(()) + } } - - self.force_reload().await } } @@ -497,13 +504,25 @@ pub(crate) fn network_proxy_state_for_policy( allow_set, deny_set, constraints: NetworkProxyConstraints::default(), - layer_mtimes: Vec::new(), cfg_path: PathBuf::from("/nonexistent/config.toml"), blocked: VecDeque::new(), }; - NetworkProxyState { - state: Arc::new(RwLock::new(state)), + NetworkProxyState::with_reloader(state, Arc::new(NoopReloader)) +} + +#[cfg(test)] +struct NoopReloader; + +#[cfg(test)] +#[async_trait] +impl ConfigReloader for NoopReloader { + async fn maybe_reload(&self) -> Result> { + Ok(None) + } + + async fn reload_now(&self) -> Result { + Err(anyhow::anyhow!("force reload is not supported in tests")) } } diff --git a/codex-rs/network-proxy/src/state.rs b/codex-rs/network-proxy/src/state.rs index 179d33dec..6b6517d13 100644 --- a/codex-rs/network-proxy/src/state.rs +++ b/codex-rs/network-proxy/src/state.rs @@ -2,10 +2,11 @@ use crate::config::NetworkMode; use crate::config::NetworkProxyConfig; use crate::policy::DomainPattern; use crate::policy::compile_globset; +use crate::runtime::ConfigReloader; use crate::runtime::ConfigState; -use crate::runtime::LayerMtime; use anyhow::Context; use anyhow::Result; +use async_trait::async_trait; use codex_app_server_protocol::ConfigLayerSource; use codex_core::config::CONFIG_TOML_FILE; use codex_core::config::ConstraintError; @@ -18,6 +19,7 @@ use codex_core::config_loader::RequirementSource; use codex_core::config_loader::load_config_layers_state; use serde::Deserialize; use std::collections::HashSet; +use tokio::sync::RwLock; pub use crate::runtime::BlockedRequest; pub use crate::runtime::BlockedRequestArgs; @@ -25,7 +27,13 @@ pub use crate::runtime::NetworkProxyState; #[cfg(test)] pub(crate) use crate::runtime::network_proxy_state_for_policy; -pub(crate) async fn build_config_state() -> Result { +pub(crate) async fn build_default_config_state_and_reloader() +-> Result<(ConfigState, MtimeConfigReloader)> { + let (state, layer_mtimes) = build_config_state_with_mtimes().await?; + Ok((state, MtimeConfigReloader::new(layer_mtimes))) +} + +async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec)> { // Load config through `codex-core` so we inherit the same layer ordering and semantics as the // rest of Codex (system/managed layers, user layers, session flags, etc.). let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; @@ -57,15 +65,17 @@ pub(crate) async fn build_config_state() -> Result { let layer_mtimes = collect_layer_mtimes(&config_layer_stack); let deny_set = compile_globset(&config.network.denied_domains)?; let allow_set = compile_globset(&config.network.allowed_domains)?; - Ok(ConfigState { - config, - allow_set, - deny_set, - constraints, + Ok(( + ConfigState { + config, + allow_set, + deny_set, + constraints, + cfg_path, + blocked: std::collections::VecDeque::new(), + }, layer_mtimes, - cfg_path, - blocked: std::collections::VecDeque::new(), - }) + )) } fn collect_layer_mtimes(stack: &ConfigLayerStack) -> Vec { @@ -90,6 +100,65 @@ fn collect_layer_mtimes(stack: &ConfigLayerStack) -> Vec { .collect() } +#[derive(Clone)] +struct LayerMtime { + path: std::path::PathBuf, + mtime: Option, +} + +impl LayerMtime { + fn new(path: std::path::PathBuf) -> Self { + let mtime = path.metadata().and_then(|m| m.modified()).ok(); + Self { path, mtime } + } +} + +pub(crate) struct MtimeConfigReloader { + layer_mtimes: RwLock>, +} + +impl MtimeConfigReloader { + fn new(layer_mtimes: Vec) -> Self { + Self { + layer_mtimes: RwLock::new(layer_mtimes), + } + } + + async fn needs_reload(&self) -> bool { + let guard = self.layer_mtimes.read().await; + guard.iter().any(|layer| { + let metadata = std::fs::metadata(&layer.path).ok(); + match (metadata.and_then(|m| m.modified().ok()), layer.mtime) { + (Some(new_mtime), Some(old_mtime)) => new_mtime > old_mtime, + (Some(_), None) => true, + (None, Some(_)) => true, + (None, None) => false, + } + }) + } +} + +#[async_trait] +impl ConfigReloader for MtimeConfigReloader { + async fn maybe_reload(&self) -> Result> { + if !self.needs_reload().await { + return Ok(None); + } + + let (state, layer_mtimes) = build_config_state_with_mtimes().await?; + let mut guard = self.layer_mtimes.write().await; + *guard = layer_mtimes; + Ok(Some(state)) + } + + async fn reload_now(&self) -> Result { + let (state, layer_mtimes) = build_config_state_with_mtimes().await?; + let mut guard = self.layer_mtimes.write().await; + *guard = layer_mtimes; + Ok(state) + } +} + #[derive(Debug, Default, Deserialize)] struct PartialConfig { #[serde(default)]