diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a58737477f..66085bccfb 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -9821,12 +9821,6 @@ }, "trustStatus": { "$ref": "#/definitions/v2/HookTrustStatus" - }, - "trustedHash": { - "type": [ - "string", - "null" - ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index e39fa43a16..d53ad20a08 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6430,12 +6430,6 @@ }, "trustStatus": { "$ref": "#/definitions/HookTrustStatus" - }, - "trustedHash": { - "type": [ - "string", - "null" - ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json index b61f1f173a..ae9cd9e63c 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json @@ -100,12 +100,6 @@ }, "trustStatus": { "$ref": "#/definitions/HookTrustStatus" - }, - "trustedHash": { - "type": [ - "string", - "null" - ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts index da4f068a2f..94e3c30c92 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts @@ -7,4 +7,4 @@ import type { HookHandlerType } from "./HookHandlerType"; import type { HookSource } from "./HookSource"; import type { HookTrustStatus } from "./HookTrustStatus"; -export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustedHash: string | null, trustStatus: HookTrustStatus, }; +export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustStatus: HookTrustStatus, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 8ab59906dd..d2ecd4abf1 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -4807,7 +4807,6 @@ pub struct HookMetadata { pub enabled: bool, pub is_managed: bool, pub current_hash: String, - pub trusted_hash: Option, pub trust_status: HookTrustStatus, } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 0184785c62..a885e136c8 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1461,7 +1461,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`, `trustedHash`, 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. ```json { @@ -1494,7 +1494,6 @@ For unmanaged hooks, `currentHash`, `trustedHash`, and `trustStatus` describe wh "displayOrder": 0, "enabled": true, "currentHash": "sha256:...", - "trustedHash": null, "trustStatus": "untrusted" }], "warnings": [], diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index e34263c8c5..501698b12d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -8852,7 +8852,6 @@ fn hooks_to_info(hooks: &[codex_hooks::HookListEntry]) -> Vec { enabled: hook.enabled, is_managed: hook.is_managed, current_hash: hook.current_hash.clone(), - trusted_hash: hook.trusted_hash.clone(), trust_status: hook.trust_status.into(), }) .collect() 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 740b752b23..49430451da 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -164,7 +164,6 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> { /*timeout_sec*/ 5, Some("running listed hook"), ), - trusted_hash: None, trust_status: HookTrustStatus::Untrusted, }], warnings: Vec::new(), @@ -243,7 +242,6 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> { /*timeout_sec*/ 7, Some("running plugin hook"), ), - trusted_hash: None, trust_status: HookTrustStatus::Untrusted, }], warnings: Vec::new(), @@ -369,7 +367,6 @@ timeout = 5 /*timeout_sec*/ 5, /*status_message*/ None, ), - trusted_hash: None, trust_status: HookTrustStatus::Untrusted, }], warnings: Vec::new(), diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index d8a5588c4a..c044869462 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -71,6 +71,7 @@ pub(crate) fn discover_handlers( /*include_disabled*/ false, ) { let hook_source = hook_source_for_config_layer_source(&layer.name); + let is_managed = is_managed_config_layer_source(&layer.name); let json_hooks = load_hooks_json(layer.config_folder().as_deref(), &mut warnings); let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings); @@ -96,7 +97,7 @@ pub(crate) fn discover_handlers( path: &source_path, key_source: source_path.display().to_string(), source: hook_source, - is_managed: hook_source.is_managed(), + is_managed, hook_states: &hook_states, env: HashMap::new(), plugin_id: None, @@ -422,13 +423,10 @@ fn append_matcher_groups( handler_index ); let state = source.hook_states.get(&key); - let enabled = - source.is_managed || state.and_then(|state| state.enabled) != Some(false); - let trusted_hash = (!source.is_managed) - .then(|| state.and_then(|state| state.trusted_hash.clone())) - .flatten(); + let enabled = hook_enabled(source.is_managed, state); + let trusted_hash = hook_trusted_hash(source.is_managed, state); let trust_status = - hook_trust_status(source.is_managed, ¤t_hash, &trusted_hash); + hook_trust_status(source.is_managed, ¤t_hash, trusted_hash); hook_entries.push(HookListEntry { key, event_name, @@ -444,7 +442,6 @@ fn append_matcher_groups( enabled, is_managed: source.is_managed, current_hash, - trusted_hash, trust_status, }); if enabled @@ -537,12 +534,12 @@ fn hook_event_key_label(event_name: codex_protocol::protocol::HookEventName) -> fn hook_trust_status( is_managed: bool, current_hash: &str, - trusted_hash: &Option, + trusted_hash: Option<&str>, ) -> HookTrustStatus { if is_managed { HookTrustStatus::Managed } else { - match trusted_hash.as_deref() { + match trusted_hash { Some(trusted_hash) if trusted_hash == current_hash => HookTrustStatus::Trusted, Some(_) => HookTrustStatus::Modified, None => HookTrustStatus::Untrusted, @@ -550,6 +547,16 @@ fn hook_trust_status( } } +fn hook_enabled(is_managed: bool, state: Option<&HookStateToml>) -> bool { + is_managed || state.and_then(|state| state.enabled) != Some(false) +} + +fn hook_trusted_hash(is_managed: bool, state: Option<&HookStateToml>) -> Option<&str> { + (!is_managed) + .then(|| state.and_then(|state| state.trusted_hash.as_deref())) + .flatten() +} + fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource { match source { ConfigLayerSource::System { .. } => HookSource::System, @@ -564,6 +571,16 @@ fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource } } +fn is_managed_config_layer_source(source: &ConfigLayerSource) -> bool { + matches!( + source, + ConfigLayerSource::System { .. } + | ConfigLayerSource::Mdm { .. } + | ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } + | ConfigLayerSource::LegacyManagedConfigTomlFromMdm + ) +} + fn hook_source_for_requirement_source(source: Option<&RequirementSource>) -> HookSource { match source { Some(RequirementSource::MdmManagedPreferences { .. }) => HookSource::Mdm, @@ -610,12 +627,11 @@ mod tests { path: &'a AbsolutePathBuf, hook_states: &'a std::collections::HashMap, ) -> super::HookHandlerSource<'a> { - let source = hook_source(); super::HookHandlerSource { path, key_source: path.display().to_string(), - source, - is_managed: source.is_managed(), + source: hook_source(), + is_managed: true, hook_states, env: std::collections::HashMap::new(), plugin_id: None, diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 63bf4b371b..dcc260eba2 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -85,7 +85,6 @@ pub struct HookListEntry { pub enabled: bool, pub is_managed: bool, pub current_hash: String, - pub trusted_hash: Option, pub trust_status: HookTrustStatus, } diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 88bcb6ec30..ea6bbe6a22 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -226,7 +226,6 @@ fn unknown_requirement_source_hooks_stay_managed() { assert_eq!(discovered.hook_entries[0].source, HookSource::Unknown); assert_eq!(discovered.hook_entries[0].enabled, true); assert_eq!(discovered.hook_entries[0].is_managed, true); - assert_eq!(discovered.hook_entries[0].trusted_hash, None); assert_eq!( discovered.hook_entries[0].trust_status, HookTrustStatus::Managed @@ -391,7 +390,6 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() { untrusted.hook_entries[0].trust_status, HookTrustStatus::Untrusted ); - assert_eq!(untrusted.hook_entries[0].trusted_hash, None); assert_eq!(untrusted.handlers, Vec::new()); let current_hash = untrusted.hook_entries[0].current_hash.clone(); @@ -403,7 +401,7 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() { &key, HookStateToml { enabled: None, - trusted_hash: Some(current_hash.clone()), + trusted_hash: Some(current_hash), }, ), )], @@ -417,7 +415,6 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() { trusted.hook_entries[0].trust_status, HookTrustStatus::Trusted ); - assert_eq!(trusted.hook_entries[0].trusted_hash, Some(current_hash)); assert_eq!(trusted.handlers.len(), 1); let changed_stack = ConfigLayerStack::new( @@ -444,10 +441,6 @@ fn unmanaged_hook_trust_status_tracks_stored_hash() { changed.hook_entries[0].trust_status, HookTrustStatus::Modified ); - assert_eq!( - changed.hook_entries[0].trusted_hash.as_deref(), - Some("sha256:old") - ); assert_eq!(changed.handlers, Vec::new()); } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index b63c513856..4bb3ddb065 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1559,21 +1559,6 @@ pub enum HookSource { Unknown, } -impl HookSource { - /// Returns whether hooks from this source are managed and therefore not - /// user-configurable. - pub fn is_managed(self) -> bool { - matches!( - self, - Self::System - | Self::Mdm - | Self::CloudRequirements - | Self::LegacyManagedConfigFile - | Self::LegacyManagedConfigMdm - ) - } -} - #[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] pub enum HookTrustStatus { @@ -3965,20 +3950,6 @@ mod tests { use tempfile::NamedTempFile; use tempfile::TempDir; - #[test] - fn hook_source_managedness_is_source_derived() { - assert_eq!(HookSource::System.is_managed(), true); - assert_eq!(HookSource::Mdm.is_managed(), true); - assert_eq!(HookSource::CloudRequirements.is_managed(), true); - assert_eq!(HookSource::LegacyManagedConfigFile.is_managed(), true); - assert_eq!(HookSource::LegacyManagedConfigMdm.is_managed(), true); - assert_eq!(HookSource::User.is_managed(), false); - assert_eq!(HookSource::Project.is_managed(), false); - assert_eq!(HookSource::SessionFlags.is_managed(), false); - assert_eq!(HookSource::Plugin.is_managed(), false); - assert_eq!(HookSource::Unknown.is_managed(), false); - } - fn sorted_writable_roots(roots: Vec) -> Vec<(PathBuf, Vec)> { let mut sorted_roots: Vec<(PathBuf, Vec)> = roots .into_iter() diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index d149f18c9d..78fca8e745 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -725,8 +725,7 @@ mod tests { plugin_id: plugin_id.map(str::to_string), display_order, enabled, - current_hash: current_hash.clone(), - trusted_hash: (!is_managed).then_some(current_hash), + current_hash, trust_status: if is_managed { HookTrustStatus::Managed } else { @@ -821,7 +820,6 @@ mod tests { /*is_managed*/ false, /*display_order*/ 0, ); - untrusted_hook.trusted_hash = None; untrusted_hook.trust_status = HookTrustStatus::Untrusted; let mut view = HooksBrowserView::new( vec![untrusted_hook], @@ -1001,7 +999,6 @@ mod tests { /*is_managed*/ false, /*display_order*/ 0, ); - untrusted_hook.trusted_hash = None; untrusted_hook.trust_status = HookTrustStatus::Untrusted; let view = HooksBrowserView::new( vec![untrusted_hook],