mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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.
This commit is contained in:
@@ -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()))
|
||||
|
||||
Reference in New Issue
Block a user