Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions codex-rs/app-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
105 changes: 105 additions & 0 deletions codex-rs/app-server/tests/suite/v2/hooks_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down Expand Up @@ -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()?;
Expand Down
125 changes: 121 additions & 4 deletions codex-rs/config/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,8 @@ struct ProjectTrustContext {
project_root: AbsolutePathBuf,
project_root_key: String,
project_root_lookup_keys: Vec<String>,
checkout_root: Option<AbsolutePathBuf>,
repo_root: Option<AbsolutePathBuf>,
repo_root_key: Option<String>,
repo_root_lookup_keys: Option<Vec<String>>,
projects_trust: std::collections::HashMap<String, TrustLevel>,
Expand Down Expand Up @@ -712,22 +714,36 @@ impl ProjectTrustContext {
)),
}
}

fn root_checkout_hooks_folder_for_dir(&self, dir: &AbsolutePathBuf) -> Option<AbsolutePathBuf> {
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<String>,
hooks_config_folder_override: Option<AbsolutePathBuf>,
) -> 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<String> {
Expand Down Expand Up @@ -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()
Expand All @@ -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,
Expand Down Expand Up @@ -944,6 +963,24 @@ async fn find_project_root(
Ok(cwd.clone())
}

async fn find_git_checkout_root(
fs: &dyn ExecutorFileSystem,
cwd: &AbsolutePathBuf,
) -> Option<AbsolutePathBuf> {
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<ConfigLayerEntry>,
startup_warnings: Vec<String>,
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
Expand All @@ -1027,24 +1066,44 @@ 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) => {
if err.kind() == io::ErrorKind::NotFound {
// 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();
Expand All @@ -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<TomlValue> {
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
Expand Down
Loading
Loading