From 934a40c7d9606a79bce472560fa665705293162f Mon Sep 17 00:00:00 2001 From: Abhinav Date: Wed, 13 May 2026 02:58:58 -0400 Subject: [PATCH] Use root repo hooks in linked worktrees (#21969) # Why Linked worktrees currently load their own project hook declarations, so the same repo can present different hook definitions depending on which checkout is active. https://github.com/openai/codex/pull/21762 tried to share trust by giving matching worktree hooks a shared synthetic key, but review pointed out that divergent worktree hook definitions would then fight over one `trusted_hash`. Instead of introducing a second trust model, this makes linked worktrees use the root checkout as the single source of truth for project hook declarations. Worktree-local project config can still diverge for unrelated settings, but project hooks now keep one real source path and one trust state per repo. # What - Teach project config loading to remember the matching root-checkout `.codex/` folder for actual linked-worktree project layers. - Keep ordinary project config sourced from the worktree, but replace project hook declarations with the root checkout's matching layer before hook discovery runs, including linked-worktree layers with `.codex/` but no local `config.toml`. - Make hook discovery use that authoritative hook folder for both `hooks.json` and TOML hook source paths, so linked worktrees produce the same hook key and trust state as the root checkout. - Cover the linked-worktree path plus regressions for missing worktree `config.toml` and nested non-worktree project roots. --- codex-rs/app-server/README.md | 2 + .../app-server/tests/suite/v2/hooks_list.rs | 105 ++++++ codex-rs/config/src/loader/mod.rs | 125 ++++++- codex-rs/config/src/state.rs | 24 ++ .../core/src/config/config_loader_tests.rs | 343 ++++++++++++++++-- codex-rs/hooks/src/engine/discovery.rs | 7 +- 6 files changed, 567 insertions(+), 39 deletions(-) diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 57a133dbd..788fc9e7c 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1546,6 +1546,8 @@ To enable or disable a skill by name: Use `hooks/list` to fetch discovered hooks for one or more `cwds`. Each result is evaluated with that `cwd`'s effective config, so feature gates and discovered config layers can differ within a single response. +For linked Git worktrees, project hook declarations come from the matching `.codex/` folders in the root checkout rather than from divergent hook declarations stored only in the linked worktree. This keeps each repo on one authoritative project-hook definition and one trust state. + Hooks are returned even when disabled so clients can render and re-enable them. User-controlled state lives under `hooks.state`. Managed hooks are non-configurable, and user entries for managed hook keys are ignored during loading. For unmanaged hooks, `currentHash` and `trustStatus` describe whether the current definition is first-seen, approved, or changed since approval. Only trusted unmanaged hooks become runnable. Hook keys combine the source identity with a trailing event/group/handler selector that is currently positional. diff --git a/codex-rs/app-server/tests/suite/v2/hooks_list.rs b/codex-rs/app-server/tests/suite/v2/hooks_list.rs index f3ce4cbf2..15675257f 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -107,6 +107,29 @@ enabled = true Ok(()) } +fn write_project_hook_config(dot_codex_folder: &std::path::Path, command: &str) -> Result<()> { + std::fs::create_dir_all(dot_codex_folder)?; + std::fs::write( + dot_codex_folder.join("config.toml"), + format!( + r#"[features] +hooks = true + +[hooks] + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "{command}" +timeout = 5 +"# + ), + )?; + Ok(()) +} + #[tokio::test] async fn hooks_list_shows_discovered_hook() -> Result<()> { let codex_home = TempDir::new()?; @@ -368,6 +391,88 @@ timeout = 5 Ok(()) } +#[tokio::test] +async fn hooks_list_uses_root_repo_hooks_for_linked_worktrees() -> Result<()> { + let codex_home = TempDir::new()?; + let workspace = TempDir::new()?; + let repo_root = workspace.path().join("repo"); + let worktree_root = workspace.path().join("worktree"); + let worktree_git_dir = repo_root.join(".git/worktrees/feature-x"); + + std::fs::create_dir_all(&worktree_git_dir)?; + std::fs::create_dir_all(&worktree_root)?; + std::fs::write( + worktree_root.join(".git"), + format!("gitdir: {}\n", worktree_git_dir.display()), + )?; + write_project_hook_config(&repo_root.join(".codex"), "echo root hook")?; + write_project_hook_config(&worktree_root.join(".codex"), "echo worktree hook")?; + set_project_trust_level(codex_home.path(), &repo_root, TrustLevel::Trusted)?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let list_id = mcp + .send_hooks_list_request(HooksListParams { + cwds: vec![repo_root.clone(), worktree_root.clone()], + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(list_id)), + ) + .await??; + let HooksListResponse { data } = to_response(response)?; + let repo_hook = data[0].hooks[0].clone(); + let worktree_hook = data[1].hooks[0].clone(); + let repo_config_path = + AbsolutePathBuf::from_absolute_path(repo_root.join(".codex/config.toml"))?; + + assert_eq!(repo_hook.command.as_deref(), Some("echo root hook")); + assert_eq!(worktree_hook.command.as_deref(), Some("echo root hook")); + assert_eq!(repo_hook.key, worktree_hook.key); + assert_eq!(repo_hook.source_path, repo_config_path); + assert_eq!(worktree_hook.source_path, repo_config_path); + + let write_id = mcp + .send_config_batch_write_request(ConfigBatchWriteParams { + edits: vec![ConfigEdit { + key_path: "hooks.state".to_string(), + value: serde_json::json!({ + repo_hook.key.clone(): { + "trusted_hash": repo_hook.current_hash.clone() + } + }), + merge_strategy: MergeStrategy::Upsert, + }], + file_path: None, + expected_version: None, + reload_user_config: true, + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(write_id)), + ) + .await??; + let _: codex_app_server_protocol::ConfigWriteResponse = to_response(response)?; + + let list_id = mcp + .send_hooks_list_request(HooksListParams { + cwds: vec![worktree_root], + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(list_id)), + ) + .await??; + let HooksListResponse { data } = to_response(response)?; + assert_eq!(data[0].hooks[0].trust_status, HookTrustStatus::Trusted); + + Ok(()) +} + #[tokio::test] async fn config_batch_write_toggles_user_hook() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index e9f819bcf..6e9869146 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -627,6 +627,8 @@ struct ProjectTrustContext { project_root: AbsolutePathBuf, project_root_key: String, project_root_lookup_keys: Vec, + checkout_root: Option, + repo_root: Option, repo_root_key: Option, repo_root_lookup_keys: Option>, projects_trust: std::collections::HashMap, @@ -712,22 +714,36 @@ impl ProjectTrustContext { )), } } + + fn root_checkout_hooks_folder_for_dir(&self, dir: &AbsolutePathBuf) -> Option { + let checkout_root = self.checkout_root.as_ref()?; + let repo_root = self.repo_root.as_ref()?; + // Regular checkouts resolve both paths to the same root; linked worktrees do not. + if checkout_root == repo_root { + return None; + } + + let relative_dir = dir.as_path().strip_prefix(checkout_root.as_path()).ok()?; + Some(repo_root.join(relative_dir).join(".codex")) + } } fn project_layer_entry( dot_codex_folder: &AbsolutePathBuf, config: TomlValue, disabled_reason: Option, + hooks_config_folder_override: Option, ) -> ConfigLayerEntry { let source = ConfigLayerSource::Project { dot_codex_folder: dot_codex_folder.clone(), }; - if let Some(reason) = disabled_reason { + let entry = if let Some(reason) = disabled_reason { ConfigLayerEntry::new_disabled(source, config, reason) } else { ConfigLayerEntry::new(source, config) - } + }; + entry.with_hooks_config_folder_override(hooks_config_folder_override) } fn sanitize_project_config(config: &mut TomlValue) -> Vec { @@ -786,6 +802,7 @@ async fn project_trust_context( .first() .cloned() .unwrap_or_else(|| project_trust_key(project_root.as_path())); + let checkout_root = find_git_checkout_root(fs, cwd).await; let repo_root = resolve_root_git_project_for_trust(fs, cwd).await; let repo_root_lookup_keys = repo_root .as_ref() @@ -803,6 +820,8 @@ async fn project_trust_context( project_root, project_root_key, project_root_lookup_keys, + checkout_root, + repo_root, repo_root_key, repo_root_lookup_keys, projects_trust, @@ -944,6 +963,24 @@ async fn find_project_root( Ok(cwd.clone()) } +async fn find_git_checkout_root( + fs: &dyn ExecutorFileSystem, + cwd: &AbsolutePathBuf, +) -> Option { + let base = match fs.get_metadata(cwd, /*sandbox*/ None).await { + Ok(metadata) if metadata.is_directory => cwd.clone(), + _ => cwd.parent()?, + }; + + for dir in base.ancestors() { + let dot_git = dir.join(".git"); + if fs.get_metadata(&dot_git, /*sandbox*/ None).await.is_ok() { + return Some(dir); + } + } + None +} + struct LoadedProjectLayers { layers: Vec, startup_warnings: Vec, @@ -995,6 +1032,7 @@ async fn load_project_layers( let decision = trust_context.decision_for_dir(&dir); let disabled_reason = trust_context.disabled_reason_for_decision(&decision); + let hooks_config_folder_override = trust_context.root_checkout_hooks_folder_for_dir(&dir); let dot_codex_normalized = normalize_path(dot_codex_abs.as_path()).unwrap_or_else(|_| dot_codex_abs.to_path_buf()); if dot_codex_abs == codex_home_abs || dot_codex_normalized == codex_home_normalized { @@ -1019,6 +1057,7 @@ async fn load_project_layers( &dot_codex_abs, TomlValue::Table(toml::map::Map::new()), disabled_reason.clone(), + hooks_config_folder_override.clone(), )); continue; } @@ -1027,13 +1066,25 @@ async fn load_project_layers( let ignored_project_config_keys = sanitize_project_config(&mut config); let config = resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?; + let config = merge_root_checkout_project_hooks( + fs, + config, + hooks_config_folder_override.as_ref(), + decision.is_trusted(), + ) + .await?; if disabled_reason.is_none() && !ignored_project_config_keys.is_empty() { startup_warnings.push(project_ignored_config_keys_warning( &dot_codex_abs, &ignored_project_config_keys, )); } - let entry = project_layer_entry(&dot_codex_abs, config, disabled_reason.clone()); + let entry = project_layer_entry( + &dot_codex_abs, + config, + disabled_reason.clone(), + hooks_config_folder_override.clone(), + ); layers.push(entry); } Err(err) => { @@ -1041,10 +1092,18 @@ async fn load_project_layers( // If there is no config.toml file, record an empty entry // for this project layer, as this may still have subfolders // that are significant in the overall ConfigLayerStack. + let config = merge_root_checkout_project_hooks( + fs, + TomlValue::Table(toml::map::Map::new()), + hooks_config_folder_override.as_ref(), + decision.is_trusted(), + ) + .await?; layers.push(project_layer_entry( &dot_codex_abs, - TomlValue::Table(toml::map::Map::new()), + config, disabled_reason, + hooks_config_folder_override, )); } else { let config_file_display = config_file.as_path().display(); @@ -1062,6 +1121,64 @@ async fn load_project_layers( startup_warnings, }) } + +/// For linked worktrees, preserve ordinary worktree-local project config while +/// replacing only hook declarations with the matching root-checkout layer. +async fn merge_root_checkout_project_hooks( + fs: &dyn ExecutorFileSystem, + mut config: TomlValue, + hooks_config_folder_override: Option<&AbsolutePathBuf>, + is_trusted: bool, +) -> io::Result { + let Some(hooks_config_folder) = hooks_config_folder_override else { + return Ok(config); + }; + let hooks_config_file = hooks_config_folder.join(CONFIG_TOML_FILE); + let root_config = match fs + .read_file_text(&hooks_config_file, /*sandbox*/ None) + .await + { + Ok(contents) => { + let parsed: TomlValue = match toml::from_str(&contents) { + Ok(parsed) => parsed, + Err(err) => { + if is_trusted { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "Error parsing project hooks config file {}: {err}", + hooks_config_file.as_path().display() + ), + )); + } + TomlValue::Table(toml::map::Map::new()) + } + }; + resolve_relative_paths_in_config_toml(parsed, hooks_config_folder.as_path())? + } + Err(err) if err.kind() == io::ErrorKind::NotFound => { + TomlValue::Table(toml::map::Map::new()) + } + Err(err) => { + return Err(io::Error::new( + err.kind(), + format!( + "Failed to read project hooks config file {}: {err}", + hooks_config_file.as_path().display() + ), + )); + } + }; + + let Some(config_table) = config.as_table_mut() else { + return Ok(config); + }; + config_table.remove("hooks"); + if let Some(hooks) = root_config.get("hooks") { + config_table.insert("hooks".to_string(), hooks.clone()); + } + Ok(config) +} /// The legacy mechanism for specifying admin-enforced configuration is to read /// from a file like `/etc/codex/managed_config.toml` that has the same /// structure as `config.toml` where fields like `approval_policy` can specify diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index c409b404d..e718ecae6 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -66,6 +66,7 @@ pub struct ConfigLayerEntry { pub raw_toml: Option, pub version: String, pub disabled_reason: Option, + hooks_config_folder_override: Option, } impl ConfigLayerEntry { @@ -77,6 +78,7 @@ impl ConfigLayerEntry { raw_toml: None, version, disabled_reason: None, + hooks_config_folder_override: None, } } @@ -88,6 +90,7 @@ impl ConfigLayerEntry { raw_toml: Some(raw_toml), version, disabled_reason: None, + hooks_config_folder_override: None, } } @@ -103,6 +106,7 @@ impl ConfigLayerEntry { raw_toml: None, version, disabled_reason: Some(disabled_reason.into()), + hooks_config_folder_override: None, } } @@ -114,6 +118,14 @@ impl ConfigLayerEntry { self.raw_toml.as_deref() } + pub(crate) fn with_hooks_config_folder_override( + mut self, + hooks_config_folder_override: Option, + ) -> Self { + self.hooks_config_folder_override = hooks_config_folder_override; + self + } + pub fn metadata(&self) -> ConfigLayerMetadata { ConfigLayerMetadata { name: self.name.clone(), @@ -142,6 +154,18 @@ impl ConfigLayerEntry { ConfigLayerSource::LegacyManagedConfigTomlFromMdm => None, } } + + /// Returns the `.codex/` folder that should be used for hook declarations. + /// + /// Project layers normally use their own config folder. Linked Git worktrees + /// can instead point hook discovery at the matching folder from the root + /// checkout while the rest of the project config still comes from the + /// worktree. + pub fn hooks_config_folder(&self) -> Option { + self.hooks_config_folder_override + .clone() + .or_else(|| self.config_folder()) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 7d96ef26d..9eec97773 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -23,7 +23,6 @@ use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; use codex_config::loader::load_config_layers_state; use codex_config::loader::load_requirements_toml; -use codex_config::version_for_toml; use codex_exec_server::LOCAL_FS; use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::WebSearchMode; @@ -68,6 +67,45 @@ async fn make_config_for_test( .await } +async fn write_linked_worktree_pointer( + repo_root: &Path, + worktree_root: &Path, +) -> std::io::Result<()> { + let worktree_git_dir = repo_root.join(".git/worktrees/feature-x"); + tokio::fs::create_dir_all(&worktree_git_dir).await?; + tokio::fs::write( + worktree_root.join(".git"), + format!("gitdir: {}\n", worktree_git_dir.display()), + ) + .await +} + +async fn write_project_hook_config( + dot_codex_folder: &Path, + foo: Option<&str>, + command: &str, +) -> std::io::Result<()> { + tokio::fs::create_dir_all(dot_codex_folder).await?; + let foo = foo + .map(|value| format!("foo = \"{value}\"\n\n")) + .unwrap_or_default(); + tokio::fs::write( + dot_codex_folder.join(CONFIG_TOML_FILE), + format!( + r#"{foo}[hooks] + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "{command}" +"# + ), + ) + .await +} + #[tokio::test] async fn cli_overrides_resolve_relative_paths_against_cwd() -> std::io::Result<()> { let codex_home = tempdir().expect("tempdir"); @@ -391,18 +429,13 @@ async fn returns_empty_when_all_layers_missing() { let user_layer = layers .get_user_layer() .expect("expected a user layer even when CODEX_HOME/config.toml does not exist"); - assert_eq!( - &ConfigLayerEntry { - name: ConfigLayerSource::User { - file: AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, tmp.path()) - }, - config: TomlValue::Table(toml::map::Map::new()), - raw_toml: None, - version: version_for_toml(&TomlValue::Table(toml::map::Map::new())), - disabled_reason: None, + let expected_user_layer = ConfigLayerEntry::new( + ConfigLayerSource::User { + file: AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, tmp.path()), }, - user_layer, + TomlValue::Table(toml::map::Map::new()), ); + assert_eq!(&expected_user_layer, user_layer); assert_eq!( user_layer.config, TomlValue::Table(toml::map::Map::new()), @@ -1412,6 +1445,260 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> { Ok(()) } +#[tokio::test] +async fn linked_worktree_project_layers_keep_worktree_config_but_use_root_repo_hooks() +-> std::io::Result<()> { + let tmp = tempdir()?; + let repo_root = tmp.path().join("repo"); + let repo_child = repo_root.join("child"); + let worktree_root = tmp.path().join("worktree"); + let worktree_child = worktree_root.join("child"); + + tokio::fs::create_dir_all(worktree_root.join(".codex")).await?; + tokio::fs::create_dir_all(worktree_child.join(".codex")).await?; + write_linked_worktree_pointer(&repo_root, &worktree_root).await?; + write_project_hook_config( + &repo_root.join(".codex"), + Some("repo-root"), + "echo repo root hook", + ) + .await?; + write_project_hook_config( + &repo_child.join(".codex"), + Some("repo-child"), + "echo repo child hook", + ) + .await?; + write_project_hook_config( + &worktree_root.join(".codex"), + Some("worktree-root"), + "echo worktree root hook", + ) + .await?; + write_project_hook_config( + &worktree_child.join(".codex"), + Some("worktree-child"), + "echo worktree child hook", + ) + .await?; + + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + make_config_for_test( + &codex_home, + &repo_root, + TrustLevel::Trusted, + /*project_root_markers*/ None, + ) + .await?; + + let cwd = AbsolutePathBuf::from_absolute_path(&worktree_child)?; + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + let project_layers: Vec<_> = layers + .layers_high_to_low() + .into_iter() + .filter(|layer| matches!(layer.name, ConfigLayerSource::Project { .. })) + .collect(); + assert_eq!(project_layers.len(), 2); + assert_eq!( + project_layers[0].hooks_config_folder(), + Some(AbsolutePathBuf::from_absolute_path( + repo_child.join(".codex") + )?) + ); + assert_eq!( + project_layers[1].hooks_config_folder(), + Some(AbsolutePathBuf::from_absolute_path( + repo_root.join(".codex") + )?) + ); + assert_eq!( + project_layers[0] + .config + .get("foo") + .and_then(TomlValue::as_str), + Some("worktree-child") + ); + assert_eq!( + project_hook_command(project_layers[0]), + Some("echo repo child hook") + ); + assert_eq!( + project_layers[1] + .config + .get("foo") + .and_then(TomlValue::as_str), + Some("worktree-root") + ); + assert_eq!( + project_hook_command(project_layers[1]), + Some("echo repo root hook") + ); + + Ok(()) +} + +#[tokio::test] +async fn linked_worktree_project_layers_use_root_repo_hooks_without_worktree_config_toml() +-> std::io::Result<()> { + let tmp = tempdir()?; + let repo_root = tmp.path().join("repo"); + let worktree_root = tmp.path().join("worktree"); + + tokio::fs::create_dir_all(worktree_root.join(".codex")).await?; + write_linked_worktree_pointer(&repo_root, &worktree_root).await?; + write_project_hook_config( + &repo_root.join(".codex"), + /*foo*/ None, + "echo repo root hook", + ) + .await?; + + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + make_config_for_test( + &codex_home, + &repo_root, + TrustLevel::Trusted, + /*project_root_markers*/ None, + ) + .await?; + + let cwd = AbsolutePathBuf::from_absolute_path(&worktree_root)?; + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + let project_layers: Vec<_> = layers + .layers_high_to_low() + .into_iter() + .filter(|layer| matches!(layer.name, ConfigLayerSource::Project { .. })) + .collect(); + assert_eq!(project_layers.len(), 1); + assert_eq!( + project_layers[0].hooks_config_folder(), + Some(AbsolutePathBuf::from_absolute_path( + repo_root.join(".codex") + )?) + ); + assert_eq!( + project_hook_command(project_layers[0]), + Some("echo repo root hook") + ); + + Ok(()) +} + +#[tokio::test] +async fn nested_project_root_markers_do_not_redirect_regular_repo_hooks() -> std::io::Result<()> { + let tmp = tempdir()?; + let repo_root = tmp.path().join("repo"); + let project_root = repo_root.join("project"); + let nested = project_root.join("child"); + + tokio::fs::create_dir_all(repo_root.join(".git")).await?; + tokio::fs::create_dir_all(&project_root).await?; + tokio::fs::write(project_root.join(".hg"), "hg").await?; + write_project_hook_config( + &repo_root.join(".codex"), + /*foo*/ None, + "echo repo root hook", + ) + .await?; + write_project_hook_config( + &project_root.join(".codex"), + /*foo*/ None, + "echo project root hook", + ) + .await?; + write_project_hook_config( + &nested.join(".codex"), + /*foo*/ None, + "echo nested hook", + ) + .await?; + + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + make_config_for_test( + &codex_home, + &project_root, + TrustLevel::Trusted, + Some(vec![".hg".to_string()]), + ) + .await?; + + let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + let project_layers: Vec<_> = layers + .layers_high_to_low() + .into_iter() + .filter(|layer| matches!(layer.name, ConfigLayerSource::Project { .. })) + .collect(); + assert_eq!(project_layers.len(), 2); + assert_eq!( + project_layers[0].hooks_config_folder(), + Some(AbsolutePathBuf::from_absolute_path(nested.join(".codex"))?) + ); + assert_eq!( + project_layers[1].hooks_config_folder(), + Some(AbsolutePathBuf::from_absolute_path( + project_root.join(".codex") + )?) + ); + assert_eq!( + project_hook_command(project_layers[0]), + Some("echo nested hook") + ); + assert_eq!( + project_hook_command(project_layers[1]), + Some("echo project root hook") + ); + + Ok(()) +} + +fn project_hook_command(layer: &ConfigLayerEntry) -> Option<&str> { + layer + .config + .get("hooks")? + .get("PreToolUse")? + .as_array()? + .first()? + .get("hooks")? + .as_array()? + .first()? + .get("command")? + .as_str() +} + #[tokio::test] async fn project_paths_resolve_relative_to_dot_codex_and_override_in_order() -> std::io::Result<()> { @@ -1563,18 +1850,13 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s .into_iter() .filter(|layer| matches!(layer.name, ConfigLayerSource::Project { .. })) .collect(); - assert_eq!( - vec![&ConfigLayerEntry { - name: ConfigLayerSource::Project { - dot_codex_folder: AbsolutePathBuf::from_absolute_path(project_root.join(".codex"))?, - }, - config: TomlValue::Table(toml::map::Map::new()), - raw_toml: None, - version: version_for_toml(&TomlValue::Table(toml::map::Map::new())), - disabled_reason: None, - }], - project_layers + let expected_project_layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: AbsolutePathBuf::from_absolute_path(project_root.join(".codex"))?, + }, + TomlValue::Table(toml::map::Map::new()), ); + assert_eq!(vec![&expected_project_layer], project_layers); Ok(()) } @@ -1667,18 +1949,13 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul .collect(); let child_config: TomlValue = toml::from_str("foo = \"child\"\n").expect("parse child config"); - assert_eq!( - vec![&ConfigLayerEntry { - name: ConfigLayerSource::Project { - dot_codex_folder: AbsolutePathBuf::from_absolute_path(&nested_dot_codex)?, - }, - config: child_config.clone(), - raw_toml: None, - version: version_for_toml(&child_config), - disabled_reason: None, - }], - project_layers + let expected_project_layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: AbsolutePathBuf::from_absolute_path(&nested_dot_codex)?, + }, + child_config, ); + assert_eq!(vec![&expected_project_layer], project_layers); assert_eq!( layers.effective_config().get("foo"), Some(&TomlValue::String("child".to_string())) diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index e4bb3c884..8e4402fdf 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -106,7 +106,7 @@ pub(crate) fn discover_handlers( if !policy.allows(&policy_source) { continue; } - let json_hooks = load_hooks_json(layer.config_folder().as_deref(), &mut warnings); + let json_hooks = load_hooks_json(layer.hooks_config_folder().as_deref(), &mut warnings); let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings); if let (Some((json_source_path, json_events)), Some((toml_source_path, toml_events))) = @@ -346,7 +346,10 @@ fn config_toml_source_path(layer: &ConfigLayerEntry) -> AbsolutePathBuf { ConfigLayerSource::System { file } | ConfigLayerSource::User { file } | ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => file.clone(), - ConfigLayerSource::Project { dot_codex_folder } => dot_codex_folder.join(CONFIG_TOML_FILE), + ConfigLayerSource::Project { dot_codex_folder } => layer + .hooks_config_folder() + .unwrap_or_else(|| dot_codex_folder.clone()) + .join(CONFIG_TOML_FILE), ConfigLayerSource::Mdm { domain, key } => { synthetic_layer_path(&format!("/{CONFIG_TOML_FILE}")) }