chore: refactor network-proxy so that ConfigReloader is injectable behavior (#11114)

Currently, `codex-network-proxy` depends on `codex-core`, but this
should be the other way around. As a first step, refactor out
`ConfigReloader`, which should make it easier to move
`codex-rs/network-proxy/src/state.rs` to `codex-core` in a subsequent
commit.

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<LayerMtime>,
     pub(crate) cfg_path: PathBuf,
     pub(crate) blocked: VecDeque<BlockedRequest>,
 }
 
-#[derive(Clone)]
-pub(crate) struct LayerMtime {
-    pub(crate) path: PathBuf,
-    pub(crate) mtime: Option<SystemTime>,
+#[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<Option<ConfigState>>;
+
+    /// Force a reload, regardless of whether a change was detected.
+    async fn reload_now(&self) -> Result<ConfigState>;
 }
 
-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<RwLock<ConfigState>>,
+    reloader: Arc<dyn ConfigReloader>,
 }
 
 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<Self> {
-        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<dyn ConfigReloader>) -> Self {
+        Self {
+            state: Arc::new(RwLock::new(state)),
+            reloader,
+        }
     }
 
     pub async fn current_cfg(&self) -> Result<NetworkProxyConfig> {
@@ -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<Option<ConfigState>> {
+        Ok(None)
+    }
+
+    async fn reload_now(&self) -> Result<ConfigState> {
+        Err(anyhow::anyhow!("force reload is not supported in tests"))
     }
 }
 

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<ConfigState> {
+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<LayerMtime>)> {
     // 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<ConfigState> {
     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<LayerMtime> {
@@ -90,6 +100,65 @@ fn collect_layer_mtimes(stack: &ConfigLayerStack) -> Vec<LayerMtime> {
         .collect()
 }
 
+#[derive(Clone)]
+struct LayerMtime {
+    path: std::path::PathBuf,
+    mtime: Option<std::time::SystemTime>,
+}
+
+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<Vec<LayerMtime>>,
+}
+
+impl MtimeConfigReloader {
+    fn new(layer_mtimes: Vec<LayerMtime>) -> 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<Option<ConfigState>> {
+        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<ConfigState> {
+        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)]