fix(TASK-033): cover hooks env overrides with tests and debug-log malformed hook config
This commit is contained in:
3 files changed
+92
-9
No files matched your search
+41
-4
@@ -972,10 +972,13 @@ impl AgentConfig {
|
|||||||
{
|
{
|
||||||
self.global_tools = v;
|
self.global_tools = v;
|
||||||
}
|
}
|
||||||
if let Ok(v) = env::var(with_prefix("global_hooks"))
|
if let Ok(v) = env::var(with_prefix("global_hooks")) {
|
||||||
&& let Ok(v) = serde_json::from_str(&v)
|
match serde_json::from_str(&v) {
|
||||||
{
|
Ok(v) => self.global_hooks = 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"))
|
if let Ok(v) = env::var(with_prefix("mcp_servers"))
|
||||||
&& let Ok(v) = serde_json::from_str(&v)
|
&& let Ok(v) = serde_json::from_str(&v)
|
||||||
@@ -1373,6 +1376,40 @@ global_hooks:
|
|||||||
assert_eq!(reparsed.global_hooks, config.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]
|
#[test]
|
||||||
fn from_graph_carries_hooks_and_global_hooks() {
|
fn from_graph_carries_hooks_and_global_hooks() {
|
||||||
let yaml = "\
|
let yaml = "\
|
||||||
|
|||||||
@@ -580,10 +580,13 @@ impl AppConfig {
|
|||||||
self.enabled_mcp_servers = v.map(|raw| super::csv_to_vec(&raw));
|
self.enabled_mcp_servers = v.map(|raw| super::csv_to_vec(&raw));
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Ok(v) = env::var(get_env_name("hooks"))
|
if let Ok(v) = env::var(get_env_name("hooks")) {
|
||||||
&& let Ok(v) = serde_json::from_str(&v)
|
match serde_json::from_str(&v) {
|
||||||
{
|
Ok(v) => self.hooks = 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")) {
|
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());
|
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]
|
#[test]
|
||||||
fn from_config_copies_mapping_fields() {
|
fn from_config_copies_mapping_fields() {
|
||||||
let mut cfg = Config {
|
let mut cfg = Config {
|
||||||
|
|||||||
+7
-1
@@ -558,7 +558,13 @@ fn parse_mcp_tools_map(value: &Value) -> Option<IndexMap<String, Vec<String>>> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn parse_hooks_map(value: &Value) -> Option<HooksMap> {
|
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)>) {
|
fn parse_structure_prompt(prompt: &str) -> (&str, Vec<(&str, &str)>) {
|
||||||
|
|||||||
Reference in new issue
Block a user