From b020e1b75e5f25bba13243bce92fc83cb1db919a Mon Sep 17 00:00:00 2001
From: Alex Clarke <alex.j.tusa@gmail.com>
Date: Tue, 15 Sep 2026 16:06:03 -0600
Subject: [PATCH] fix(TASK-033): cover hooks env overrides with tests and
 debug-log malformed hook config

---
 src/config/agent.rs      | 45 +++++++++++++++++++++++++++++++++----
 src/config/app_config.rs | 48 ++++++++++++++++++++++++++++++++++++----
 src/config/role.rs       |  8 ++++++-
 3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/src/config/agent.rs b/src/config/agent.rs
index aa54c3d..6db3e90 100644
--- a/src/config/agent.rs
+++ b/src/config/agent.rs
@@ -972,10 +972,13 @@ impl AgentConfig {
         {
             self.global_tools = v;
         }
-        if let Ok(v) = env::var(with_prefix("global_hooks"))
-            && let Ok(v) = serde_json::from_str(&v)
-        {
-            self.global_hooks = v;
+        if let Ok(v) = env::var(with_prefix("global_hooks")) {
+            match serde_json::from_str(&v) {
+                Ok(v) => self.global_hooks = v,
+                Err(err) => {
+                    debug!("Ignoring malformed global_hooks env override for agent '{name}': {err}")
+                }
+            }
         }
         if let Ok(v) = env::var(with_prefix("mcp_servers"))
             && let Ok(v) = serde_json::from_str(&v)
@@ -1373,6 +1376,40 @@ global_hooks:
         assert_eq!(reparsed.global_hooks, config.global_hooks);
     }
 
+    #[test]
+    fn load_envs_overrides_global_hooks() {
+        let yaml = "name: hooks-env-probe\ninstructions: hi\nglobal_hooks:\n  - initial.hook\n";
+        let mut config: AgentConfig = serde_yaml::from_str(yaml).unwrap();
+        let env_name = normalize_env_name("hooks-env-probe_global_hooks");
+        let prev = env::var_os(&env_name);
+
+        unsafe {
+            env::set_var(
+                &env_name,
+                r#"["tool.started.notify","turn.completed.webhook"]"#,
+            )
+        };
+        config.load_envs(&AppConfig::default());
+        assert_eq!(
+            config.global_hooks,
+            vec!["tool.started.notify", "turn.completed.webhook"]
+        );
+
+        unsafe { env::set_var(&env_name, "not json") };
+        config.load_envs(&AppConfig::default());
+        assert_eq!(
+            config.global_hooks,
+            vec!["tool.started.notify", "turn.completed.webhook"]
+        );
+
+        unsafe {
+            match prev {
+                Some(v) => env::set_var(&env_name, v),
+                None => env::remove_var(&env_name),
+            }
+        }
+    }
+
     #[test]
     fn from_graph_carries_hooks_and_global_hooks() {
         let yaml = "\
diff --git a/src/config/app_config.rs b/src/config/app_config.rs
index 5113979..0827edb 100644
--- a/src/config/app_config.rs
+++ b/src/config/app_config.rs
@@ -580,10 +580,13 @@ impl AppConfig {
             self.enabled_mcp_servers = v.map(|raw| super::csv_to_vec(&raw));
         }
 
-        if let Ok(v) = env::var(get_env_name("hooks"))
-            && let Ok(v) = serde_json::from_str(&v)
-        {
-            self.hooks = v;
+        if let Ok(v) = env::var(get_env_name("hooks")) {
+            match serde_json::from_str(&v) {
+                Ok(v) => self.hooks = v,
+                Err(err) => {
+                    debug!("Ignoring malformed hooks env override: {err}")
+                }
+            }
         }
 
         if let Some(v) = super::read_env_value::<String>(&get_env_name("repl_prelude")) {
@@ -836,6 +839,43 @@ mod tests {
         assert!(app.hooks.is_empty());
     }
 
+    #[test]
+    #[serial_test::serial]
+    fn load_envs_overrides_hooks() {
+        let env_name = get_env_name("hooks");
+        let prev = env::var_os(&env_name);
+
+        let mut hooks = crate::hooks::HooksMap::new();
+        hooks.insert(
+            "turn.completed".to_string(),
+            vec![crate::hooks::HookDef {
+                name: "notify".to_string(),
+                command: "./hooks/notify.sh".to_string(),
+            }],
+        );
+
+        let mut app = AppConfig::default();
+
+        unsafe { env::set_var(&env_name, serde_json::to_string(&hooks).unwrap()) };
+        app.load_envs();
+        assert_eq!(app.hooks, hooks);
+
+        unsafe { env::set_var(&env_name, "[not json") };
+        app.load_envs();
+        assert_eq!(app.hooks, hooks);
+
+        unsafe { env::remove_var(&env_name) };
+        app.load_envs();
+        assert_eq!(app.hooks, hooks);
+
+        unsafe {
+            match prev {
+                Some(v) => env::set_var(&env_name, v),
+                None => env::remove_var(&env_name),
+            }
+        }
+    }
+
     #[test]
     fn from_config_copies_mapping_fields() {
         let mut cfg = Config {
diff --git a/src/config/role.rs b/src/config/role.rs
index ebb7775..01423c0 100644
--- a/src/config/role.rs
+++ b/src/config/role.rs
@@ -558,7 +558,13 @@ fn parse_mcp_tools_map(value: &Value) -> Option<IndexMap<String, Vec<String>>> {
 }
 
 fn parse_hooks_map(value: &Value) -> Option<HooksMap> {
-    serde_json::from_value(value.clone()).ok()
+    match serde_json::from_value(value.clone()) {
+        Ok(map) => Some(map),
+        Err(err) => {
+            debug!("Ignoring malformed hooks configuration: {err}");
+            None
+        }
+    }
 }
 
 fn parse_structure_prompt(prompt: &str) -> (&str, Vec<(&str, &str)>) {