Merge legacy hook state with canonical state

codex-rs/app-server/README.md

@@ -1548,7 +1548,7 @@ Use `hooks/list` to fetch discovered hooks for one or more `cwds`. Each result i
 
 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.
+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. Project-local hooks use the resolved project trust scope plus a project-relative source path, so linked worktrees that share project trust also share hook trust.
 
 ```json
 {

codex-rs/app-server/tests/suite/v2/hooks_list.rs

@@ -1,3 +1,5 @@
+use std::path::Path;
+use std::process::Command;
 use std::time::Duration;
 
 use anyhow::Result;
@@ -22,6 +24,7 @@ use codex_app_server_protocol::ThreadStartParams;
 use codex_app_server_protocol::ThreadStartResponse;
 use codex_app_server_protocol::TurnStartParams;
 use codex_app_server_protocol::UserInput as V2UserInput;
+use codex_config::loader::project_trust_key;
 use codex_core::config::set_project_trust_level;
 use codex_protocol::config_types::TrustLevel;
 use codex_utils_absolute_path::AbsolutePathBuf;
@@ -106,6 +109,19 @@ enabled = true
     Ok(())
 }
 
+fn run_git(cwd: &Path, args: &[&str]) -> Result<()> {
+    let output = Command::new("git").current_dir(cwd).args(args).output()?;
+    if !output.status.success() {
+        anyhow::bail!(
+            "git {} failed in {}: {}",
+            args.join(" "),
+            cwd.display(),
+            String::from_utf8_lossy(&output.stderr)
+        );
+    }
+    Ok(())
+}
+
 #[tokio::test]
 async fn hooks_list_shows_discovered_hook() -> Result<()> {
     let codex_home = TempDir::new()?;
@@ -322,6 +338,7 @@ timeout = 5
     let HooksListResponse { data } = to_response(response)?;
     let project_config_path =
         AbsolutePathBuf::try_from(workspace.path().join(".codex/config.toml"))?;
+    let project_trust_key = project_trust_key(workspace.path());
     assert_eq!(
         data,
         vec![
@@ -334,10 +351,7 @@ timeout = 5
             HooksListEntry {
                 cwd: workspace.path().to_path_buf(),
                 hooks: vec![HookMetadata {
-                    key: format!(
-                        "{}:pre_tool_use:0:0",
-                        project_config_path.as_path().display()
-                    ),
+                    key: format!("project:{project_trust_key}:.codex/config.toml:pre_tool_use:0:0"),
                     event_name: HookEventName::PreToolUse,
                     handler_type: HookHandlerType::Command,
                     matcher: Some("Bash".to_string()),
@@ -367,6 +381,109 @@ timeout = 5
     Ok(())
 }
 
+#[tokio::test]
+async fn hooks_list_shares_project_hook_trust_across_linked_worktrees() -> Result<()> {
+    skip_if_windows!(Ok(()));
+
+    let codex_home = TempDir::new()?;
+    let workspace = TempDir::new()?;
+    let repo = workspace.path().join("repo");
+    let linked_worktree = workspace.path().join("linked-worktree");
+
+    std::fs::create_dir_all(&repo)?;
+    run_git(&repo, &["init"])?;
+    run_git(&repo, &["config", "user.email", "codex@example.com"])?;
+    run_git(&repo, &["config", "user.name", "Codex Tests"])?;
+    std::fs::create_dir_all(repo.join(".codex"))?;
+    std::fs::write(
+        repo.join(".codex/config.toml"),
+        r#"[features]
+hooks = true
+
+[hooks]
+
+[[hooks.PreToolUse]]
+matcher = "Bash"
+
+[[hooks.PreToolUse.hooks]]
+type = "command"
+command = "echo project hook"
+timeout = 5
+"#,
+    )?;
+    run_git(&repo, &["add", ".codex/config.toml"])?;
+    run_git(&repo, &["commit", "-m", "initial hooks config"])?;
+    run_git(
+        &repo,
+        &[
+            "worktree",
+            "add",
+            linked_worktree.to_str().expect("utf-8 path"),
+            "-b",
+            "feature/hooks",
+        ],
+    )?;
+    set_project_trust_level(codex_home.path(), &repo, TrustLevel::Trusted)?;
+
+    let mut mcp = McpProcess::new(codex_home.path()).await?;
+    timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
+
+    let request_id = mcp
+        .send_hooks_list_request(HooksListParams {
+            cwds: vec![repo.clone()],
+        })
+        .await?;
+    let response: JSONRPCResponse = timeout(
+        DEFAULT_TIMEOUT,
+        mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
+    )
+    .await??;
+    let HooksListResponse { data } = to_response(response)?;
+    let hook = data[0].hooks[0].clone();
+    assert_eq!(hook.trust_status, HookTrustStatus::Untrusted);
+
+    let write_id = mcp
+        .send_config_batch_write_request(ConfigBatchWriteParams {
+            edits: vec![ConfigEdit {
+                key_path: "hooks.state".to_string(),
+                value: serde_json::json!({
+                    hook.key.clone(): {
+                        "trusted_hash": 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 request_id = mcp
+        .send_hooks_list_request(HooksListParams {
+            cwds: vec![linked_worktree],
+        })
+        .await?;
+    let response: JSONRPCResponse = timeout(
+        DEFAULT_TIMEOUT,
+        mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
+    )
+    .await??;
+    let HooksListResponse { data } = to_response(response)?;
+    let linked_worktree_hook = &data[0].hooks[0];
+    assert_eq!(linked_worktree_hook.key, hook.key);
+    assert_eq!(linked_worktree_hook.current_hash, hook.current_hash);
+    assert_eq!(linked_worktree_hook.trust_status, HookTrustStatus::Trusted);
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn config_batch_write_toggles_user_hook() -> Result<()> {
     let codex_home = TempDir::new()?;

codex-rs/config/src/loader/mod.rs

@@ -717,16 +717,19 @@ fn project_layer_entry(
     dot_codex_folder: &AbsolutePathBuf,
     config: TomlValue,
     disabled_reason: Option<String>,
+    project_trust_key: String,
+    project_root: 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_project_trust(project_trust_key, project_root)
 }
 
 fn sanitize_project_config(config: &mut TomlValue) -> Vec<String> {
@@ -994,6 +997,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 project_trust_key = decision.trust_key.clone();
         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 {
@@ -1018,6 +1022,8 @@ async fn load_project_layers(
                             &dot_codex_abs,
                             TomlValue::Table(toml::map::Map::new()),
                             disabled_reason.clone(),
+                            project_trust_key.clone(),
+                            project_root.clone(),
                         ));
                         continue;
                     }
@@ -1032,7 +1038,13 @@ async fn load_project_layers(
                         &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(),
+                    project_trust_key.clone(),
+                    project_root.clone(),
+                );
                 layers.push(entry);
             }
             Err(err) => {
@@ -1044,6 +1056,8 @@ async fn load_project_layers(
                         &dot_codex_abs,
                         TomlValue::Table(toml::map::Map::new()),
                         disabled_reason,
+                        project_trust_key,
+                        project_root.clone(),
                     ));
                 } else {
                     let config_file_display = config_file.as_path().display();

codex-rs/config/src/state.rs

@@ -66,6 +66,8 @@ pub struct ConfigLayerEntry {
     pub raw_toml: Option<String>,
     pub version: String,
     pub disabled_reason: Option<String>,
+    pub project_trust_key: Option<String>,
+    pub project_root: Option<AbsolutePathBuf>,
 }
 
 impl ConfigLayerEntry {
@@ -77,6 +79,8 @@ impl ConfigLayerEntry {
             raw_toml: None,
             version,
             disabled_reason: None,
+            project_trust_key: None,
+            project_root: None,
         }
     }
 
@@ -88,6 +92,8 @@ impl ConfigLayerEntry {
             raw_toml: Some(raw_toml),
             version,
             disabled_reason: None,
+            project_trust_key: None,
+            project_root: None,
         }
     }
 
@@ -103,9 +109,21 @@ impl ConfigLayerEntry {
             raw_toml: None,
             version,
             disabled_reason: Some(disabled_reason.into()),
+            project_trust_key: None,
+            project_root: None,
         }
     }
 
+    pub fn with_project_trust(
+        mut self,
+        project_trust_key: String,
+        project_root: AbsolutePathBuf,
+    ) -> Self {
+        self.project_trust_key = Some(project_trust_key);
+        self.project_root = Some(project_root);
+        self
+    }
+
     pub fn is_disabled(&self) -> bool {
         self.disabled_reason.is_some()
     }

codex-rs/core/src/config/config_loader_tests.rs

@@ -330,6 +330,8 @@ async fn returns_empty_when_all_layers_missing() {
             raw_toml: None,
             version: version_for_toml(&TomlValue::Table(toml::map::Map::new())),
             disabled_reason: None,
+            project_trust_key: None,
+            project_root: None,
         },
         user_layer,
     );
@@ -1498,6 +1500,8 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
             raw_toml: None,
             version: version_for_toml(&TomlValue::Table(toml::map::Map::new())),
             disabled_reason: None,
+            project_trust_key: Some(project_root.display().to_string()),
+            project_root: Some(AbsolutePathBuf::from_absolute_path(project_root.clone())?),
         }],
         project_layers
     );
@@ -1602,6 +1606,8 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul
             raw_toml: None,
             version: version_for_toml(&child_config),
             disabled_reason: None,
+            project_trust_key: Some(project_root.display().to_string()),
+            project_root: Some(AbsolutePathBuf::from_absolute_path(project_root.clone())?),
         }],
         project_layers
     );

codex-rs/hooks/src/engine/discovery.rs

@@ -39,6 +39,7 @@ pub(crate) struct DiscoveryResult {
 struct HookHandlerSource<'a> {
     path: &'a AbsolutePathBuf,
     key_source: String,
+    legacy_key_source: Option<String>,
     source: HookSource,
     is_managed: bool,
     hook_states: &'a HashMap<String, HookStateToml>,
@@ -88,6 +89,8 @@ pub(crate) fn discover_handlers(
             }
 
             for (source_path, hook_events) in [json_hooks, toml_hooks].into_iter().flatten() {
+                let (key_source, legacy_key_source) =
+                    hook_key_sources_for_config_layer(layer, &source_path);
                 append_hook_events(
                     &mut handlers,
                     &mut hook_entries,
@@ -95,7 +98,8 @@ pub(crate) fn discover_handlers(
                     &mut display_order,
                     HookHandlerSource {
                         path: &source_path,
-                        key_source: source_path.display().to_string(),
+                        key_source,
+                        legacy_key_source,
                         source: hook_source,
                         is_managed,
                         hook_states: &hook_states,
@@ -148,6 +152,7 @@ fn append_managed_requirement_handlers(
         HookHandlerSource {
             path: &source_path,
             key_source: source_path.display().to_string(),
+            legacy_key_source: None,
             source: hook_source_for_requirement_source(managed_hooks.source.as_ref()),
             is_managed: true,
             hook_states,
@@ -196,6 +201,7 @@ fn append_plugin_hook_sources(
                     plugin_id.as_str(),
                     source_relative_path.as_str(),
                 ),
+                legacy_key_source: None,
                 source: HookSource::Plugin,
                 is_managed: false,
                 hook_states,
@@ -421,9 +427,29 @@ fn append_matcher_groups(
                     // TODO(abhinav): replace this positional suffix with a durable hook id.
                     let key =
                         crate::hook_key(&source.key_source, event_name, group_index, handler_index);
-                    let state = source.hook_states.get(&key);
-                    let enabled = hook_enabled(source.is_managed, state);
-                    let trusted_hash = hook_trusted_hash(source.is_managed, state);
+                    let legacy_key = source.legacy_key_source.as_ref().map(|legacy_key_source| {
+                        crate::hook_key(legacy_key_source, event_name, group_index, handler_index)
+                    });
+                    let canonical_state = source.hook_states.get(&key);
+                    let legacy_state = legacy_key
+                        .as_ref()
+                        .and_then(|key| source.hook_states.get(key));
+                    // Merge alias records field-by-field so a write under the new
+                    // canonical key does not discard state that still only exists
+                    // under the legacy absolute-path key during migration.
+                    let state = match (canonical_state, legacy_state) {
+                        (Some(canonical_state), Some(legacy_state)) => Some(HookStateToml {
+                            enabled: canonical_state.enabled.or(legacy_state.enabled),
+                            trusted_hash: canonical_state
+                                .trusted_hash
+                                .clone()
+                                .or_else(|| legacy_state.trusted_hash.clone()),
+                        }),
+                        (Some(state), None) | (None, Some(state)) => Some(state.clone()),
+                        (None, None) => None,
+                    };
+                    let enabled = hook_enabled(source.is_managed, state.as_ref());
+                    let trusted_hash = hook_trusted_hash(source.is_managed, state.as_ref());
                     let trust_status =
                         hook_trust_status(source.is_managed, &current_hash, trusted_hash);
                     hook_entries.push(HookListEntry {
@@ -476,6 +502,42 @@ fn append_matcher_groups(
     }
 }
 
+/// Returns the canonical hook-key source for this layer plus any legacy source
+/// that should still be read for backwards compatibility.
+///
+/// Project-local hooks used to key trust state from their absolute source path.
+/// That made linked worktrees look like unrelated hook sources even when they
+/// shared one project-trust entry. New project-local keys instead use the
+/// resolved project trust key plus the source path relative to that project
+/// root, so worktrees that share project trust also share hook trust. We still
+/// return the old absolute-path source as a legacy fallback so existing user
+/// approvals remain effective after upgrading.
+///
+/// Non-project layers, and any project source that cannot be expressed relative
+/// to the resolved project root, keep the old path-based source identity.
+fn hook_key_sources_for_config_layer(
+    layer: &ConfigLayerEntry,
+    source_path: &AbsolutePathBuf,
+) -> (String, Option<String>) {
+    let legacy_key_source = source_path.display().to_string();
+    let (Some(project_trust_key), Some(project_root)) = (
+        layer.project_trust_key.as_ref(),
+        layer.project_root.as_ref(),
+    ) else {
+        return (legacy_key_source, None);
+    };
+    let Ok(relative_path) = source_path.as_path().strip_prefix(project_root.as_path()) else {
+        return (legacy_key_source, None);
+    };
+    let relative_path = relative_path
+        .components()
+        .map(|component| component.as_os_str().to_string_lossy())
+        .collect::<Vec<_>>()
+        .join("/");
+    let key_source = format!("project:{project_trust_key}:{relative_path}");
+    (key_source, Some(legacy_key_source))
+}
+
 /// Hash a normalized, config-derived identity instead of source text so equivalent
 /// hooks from config TOML and hooks.json converge on the same trust identity.
 #[derive(Serialize)]
@@ -595,6 +657,7 @@ mod tests {
         super::HookHandlerSource {
             path,
             key_source: path.display().to_string(),
+            legacy_key_source: None,
             source: hook_source(),
             is_managed: true,
             hook_states,

codex-rs/hooks/src/engine/mod_tests.rs

@@ -23,6 +23,8 @@ use codex_protocol::protocol::HookOutputEntryKind;
 use codex_protocol::protocol::HookRunStatus;
 use codex_protocol::protocol::HookSource;
 use codex_protocol::protocol::HookTrustStatus;
+use codex_utils_absolute_path::test_support::PathBufExt;
+use codex_utils_absolute_path::test_support::test_path_buf;
 use pretty_assertions::assert_eq;
 use tempfile::tempdir;
 
@@ -364,6 +366,135 @@ fn user_disablement_does_not_filter_managed_layer_hooks() {
     );
 }
 
+#[test]
+fn project_hook_keys_share_project_trust_across_worktrees() {
+    let trust_key = test_path_buf("/repo").display().to_string();
+    let project_a = test_project_layer("/tmp/worktree-a", &trust_key);
+    let project_b = test_project_layer("/tmp/worktree-b", &trust_key);
+
+    let discovered_a = super::discovery::discover_handlers(
+        Some(&config_layer_stack(vec![project_a])),
+        Vec::new(),
+        Vec::new(),
+    );
+    let discovered_b = super::discovery::discover_handlers(
+        Some(&config_layer_stack(vec![project_b])),
+        Vec::new(),
+        Vec::new(),
+    );
+
+    assert_eq!(discovered_a.hook_entries.len(), 1);
+    assert_eq!(discovered_b.hook_entries.len(), 1);
+    assert_eq!(
+        discovered_a.hook_entries[0].key,
+        expected_project_hook_key(&trust_key)
+    );
+    assert_eq!(
+        discovered_a.hook_entries[0].key,
+        discovered_b.hook_entries[0].key
+    );
+}
+
+#[test]
+fn project_hook_trust_falls_back_to_legacy_path_key() {
+    let project_root = "/tmp/worktree-a";
+    let project_root_abs = test_path_buf(project_root).abs();
+    let trust_key = test_path_buf("/repo").display().to_string();
+    let current_hash = super::discovery::discover_handlers(
+        Some(&config_layer_stack(vec![test_project_layer(
+            project_root,
+            &trust_key,
+        )])),
+        Vec::new(),
+        Vec::new(),
+    )
+    .hook_entries[0]
+        .current_hash
+        .clone();
+    let legacy_key = format!(
+        "{}:pre_tool_use:0:0",
+        project_root_abs.join(".codex/config.toml").display()
+    );
+    let stack = config_layer_stack(vec![
+        ConfigLayerEntry::new(
+            ConfigLayerSource::User {
+                file: AbsolutePathBuf::try_from("/tmp/config.toml").expect("absolute path"),
+            },
+            config_with_trusted_hook_state(&legacy_key, &current_hash),
+        ),
+        test_project_layer(project_root, &trust_key),
+    ]);
+
+    let discovered = super::discovery::discover_handlers(Some(&stack), Vec::new(), Vec::new());
+
+    assert_eq!(discovered.handlers.len(), 1);
+    assert_eq!(discovered.hook_entries.len(), 1);
+    assert_eq!(
+        discovered.hook_entries[0].key,
+        expected_project_hook_key(&trust_key)
+    );
+    assert_eq!(
+        discovered.hook_entries[0].trust_status,
+        HookTrustStatus::Trusted
+    );
+}
+
+#[test]
+fn project_hook_state_merges_canonical_and_legacy_keys() {
+    let project_root = "/tmp/worktree-a";
+    let project_root_abs = test_path_buf(project_root).abs();
+    let trust_key = test_path_buf("/repo").display().to_string();
+    let current_hash = super::discovery::discover_handlers(
+        Some(&config_layer_stack(vec![test_project_layer(
+            project_root,
+            &trust_key,
+        )])),
+        Vec::new(),
+        Vec::new(),
+    )
+    .hook_entries[0]
+        .current_hash
+        .clone();
+    let canonical_key = expected_project_hook_key(&trust_key);
+    let legacy_key = format!(
+        "{}:pre_tool_use:0:0",
+        project_root_abs.join(".codex/config.toml").display()
+    );
+    let config: TomlValue = serde_json::from_value(serde_json::json!({
+        "hooks": {
+            "state": {
+                (legacy_key): {
+                    "enabled": false,
+                },
+                (canonical_key.clone()): {
+                    "trusted_hash": current_hash,
+                },
+            },
+        },
+    }))
+    .expect("config TOML should deserialize");
+    let stack = config_layer_stack(vec![
+        ConfigLayerEntry::new(
+            ConfigLayerSource::User {
+                file: AbsolutePathBuf::try_from("/tmp/config.toml").expect("absolute path"),
+            },
+            config,
+        ),
+        test_project_layer(project_root, &trust_key),
+    ]);
+
+    let discovered = super::discovery::discover_handlers(Some(&stack), Vec::new(), Vec::new());
+
+    assert_eq!(discovered.handlers.len(), 0);
+    assert_eq!(discovered.hook_entries.len(), 1);
+    assert_eq!(discovered.hook_entries[0].key, canonical_key);
+    assert_eq!(discovered.hook_entries[0].enabled, false);
+    assert_eq!(
+        discovered.hook_entries[0].trust_status,
+        HookTrustStatus::Trusted
+    );
+}
+
 fn config_with_hook_state(key: &str, enabled: bool) -> TomlValue {
     serde_json::from_value(serde_json::json!({
         "hooks": {
@@ -377,6 +508,45 @@ fn config_with_hook_state(key: &str, enabled: bool) -> TomlValue {
     .expect("config TOML should deserialize")
 }
 
+fn config_with_trusted_hook_state(key: &str, current_hash: &str) -> TomlValue {
+    serde_json::from_value(serde_json::json!({
+        "hooks": {
+            "state": {
+                (key): {
+                    "trusted_hash": current_hash,
+                },
+            },
+        },
+    }))
+    .expect("config TOML should deserialize")
+}
+
+fn config_layer_stack(layers: Vec<ConfigLayerEntry>) -> ConfigLayerStack {
+    ConfigLayerStack::new(
+        layers,
+        ConfigRequirements::default(),
+        ConfigRequirementsToml::default(),
+    )
+    .expect("config layer stack")
+}
+
+fn test_project_layer(project_root: &str, trust_key: &str) -> ConfigLayerEntry {
+    let project_root = test_path_buf(project_root).abs();
+    let dot_codex_folder = project_root.join(".codex");
+    ConfigLayerEntry::new(
+        ConfigLayerSource::Project { dot_codex_folder },
+        config_with_pre_tool_use_hook("python3 /tmp/project.py"),
+    )
+    .with_project_trust(trust_key.to_string(), project_root)
+}
+
+fn expected_project_hook_key(trust_key: &str) -> String {
+    format!(
+        "project:{trust_key}:{}:pre_tool_use:0:0",
+        Path::new(".codex/config.toml").display()
+    )
+}
+
 fn config_with_pre_tool_use_hook_and_states<const N: usize>(
     command: &str,
     disabled_keys: [&str; N],