openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-29 19:08:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refactor hook config selectors

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-04-26 20:53:35 -07:00
parent c2a3b5a289
commit 24c0dcc4b3
15 changed files with 637 additions and 494 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/schema/json/ClientRequest.json
Generated
+80 -34
View File
@@ -1447,44 +1447,90 @@
],
"type": "object"
},
"HookConfigSource": {
"enum": [
"plugin",
"user",
"project"
],
"type": "string"
},
"HooksConfigWriteParams": {
"properties": {
"enabled": {
"type": "boolean"
"oneOf": [
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"pluginId": {
"type": "string"
},
"source": {
"enum": [
"plugin"
],
"type": "string"
}
},
"required": [
"enabled",
"key",
"pluginId",
"source"
],
"type": "object"
},
"key": {
"type": "string"
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"user"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
},
"pluginId": {
"type": [
"string",
"null"
]
},
"source": {
"$ref": "#/definitions/HookConfigSource"
},
"sourcePath": {
"type": [
"string",
"null"
]
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"project"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
}
},
"required": [
"enabled",
"key",
"source"
],
"type": "object"
]
},
"HooksListParams": {
"properties": {
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
Generated
+80 -34
View File
@@ -9615,14 +9615,6 @@
"title": "HookCompletedNotification",
"type": "object"
},
"HookConfigSource": {
"enum": [
"plugin",
"user",
"project"
],
"type": "string"
},
"HookErrorInfo": {
"properties": {
"message": {
@@ -9909,36 +9901,90 @@
},
"HooksConfigWriteParams": {
"$schema": "http://json-schema.org/draft-07/schema#",
"properties": {
"enabled": {
"type": "boolean"
"oneOf": [
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"pluginId": {
"type": "string"
},
"source": {
"enum": [
"plugin"
],
"type": "string"
}
},
"required": [
"enabled",
"key",
"pluginId",
"source"
],
"type": "object"
},
"key": {
"type": "string"
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"user"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
},
"pluginId": {
"type": [
"string",
"null"
]
},
"source": {
"$ref": "#/definitions/v2/HookConfigSource"
},
"sourcePath": {
"type": [
"string",
"null"
]
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"project"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
}
},
"required": [
"enabled",
"key",
"source"
],
"title": "HooksConfigWriteParams",
"type": "object"
"title": "HooksConfigWriteParams"
},
"HooksConfigWriteResponse": {
"$schema": "http://json-schema.org/draft-07/schema#",
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json
Generated
+80 -34
View File
@@ -6245,14 +6245,6 @@
"title": "HookCompletedNotification",
"type": "object"
},
"HookConfigSource": {
"enum": [
"plugin",
"user",
"project"
],
"type": "string"
},
"HookErrorInfo": {
"properties": {
"message": {
@@ -6539,36 +6531,90 @@
},
"HooksConfigWriteParams": {
"$schema": "http://json-schema.org/draft-07/schema#",
"properties": {
"enabled": {
"type": "boolean"
"oneOf": [
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"pluginId": {
"type": "string"
},
"source": {
"enum": [
"plugin"
],
"type": "string"
}
},
"required": [
"enabled",
"key",
"pluginId",
"source"
],
"type": "object"
},
"key": {
"type": "string"
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"user"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
},
"pluginId": {
"type": [
"string",
"null"
]
},
"source": {
"$ref": "#/definitions/HookConfigSource"
},
"sourcePath": {
"type": [
"string",
"null"
]
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"project"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
}
},
"required": [
"enabled",
"key",
"source"
],
"title": "HooksConfigWriteParams",
"type": "object"
"title": "HooksConfigWriteParams"
},
"HooksConfigWriteResponse": {
"$schema": "http://json-schema.org/draft-07/schema#",
codex-rs/app-server-protocol/schema/json/v2/HooksConfigWriteParams.json
Generated
+81 -37
View File
@@ -1,43 +1,87 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"HookConfigSource": {
"enum": [
"plugin",
"user",
"project"
"oneOf": [
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"pluginId": {
"type": "string"
},
"source": {
"enum": [
"plugin"
],
"type": "string"
}
},
"required": [
"enabled",
"key",
"pluginId",
"source"
],
"type": "string"
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"user"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"source": {
"enum": [
"project"
],
"type": "string"
},
"sourcePath": {
"type": "string"
}
},
"required": [
"enabled",
"key",
"source",
"sourcePath"
],
"type": "object"
}
},
"properties": {
"enabled": {
"type": "boolean"
},
"key": {
"type": "string"
},
"pluginId": {
"type": [
"string",
"null"
]
},
"source": {
"$ref": "#/definitions/HookConfigSource"
},
"sourcePath": {
"type": [
"string",
"null"
]
}
},
"required": [
"enabled",
"key",
"source"
],
"title": "HooksConfigWriteParams",
"type": "object"
"title": "HooksConfigWriteParams"
}
codex-rs/app-server-protocol/schema/typescript/v2/HookConfigSource.ts
Generated
-5
View File
@@ -1,5 +0,0 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type HookConfigSource = "plugin" | "user" | "project";
codex-rs/app-server-protocol/schema/typescript/v2/HooksConfigWriteParams.ts
Generated
+1 -2
View File
@@ -1,6 +1,5 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { HookConfigSource } from "./HookConfigSource";
export type HooksConfigWriteParams = { source: HookConfigSource, pluginId?: string | null, sourcePath?: string | null, key: string, enabled: boolean, };
export type HooksConfigWriteParams = { "source": "plugin", pluginId: string, key: string, enabled: boolean, } | { "source": "user", sourcePath: string, key: string, enabled: boolean, } | { "source": "project", sourcePath: string, key: string, enabled: boolean, };
codex-rs/app-server-protocol/schema/typescript/v2/index.ts
Generated
-1
View File
@@ -151,7 +151,6 @@ export type { GuardianRiskLevel } from "./GuardianRiskLevel";
export type { GuardianUserAuthorization } from "./GuardianUserAuthorization";
export type { GuardianWarningNotification } from "./GuardianWarningNotification";
export type { HookCompletedNotification } from "./HookCompletedNotification";
export type { HookConfigSource } from "./HookConfigSource";
export type { HookErrorInfo } from "./HookErrorInfo";
export type { HookEventName } from "./HookEventName";
export type { HookExecutionMode } from "./HookExecutionMode";
codex-rs/app-server-protocol/src/protocol/v2.rs
+60 -19
View File
@@ -4663,27 +4663,32 @@ pub struct HookMetadata {
pub enabled: bool,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
#[ts(rename_all = "snake_case")]
#[ts(export_to = "v2/")]
pub enum HookConfigSource {
Plugin,
User,
Project,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[serde(tag = "source", rename_all = "snake_case", deny_unknown_fields)]
#[ts(tag = "source", rename_all = "snake_case")]
#[ts(export_to = "v2/")]
pub struct HooksConfigWriteParams {
pub source: HookConfigSource,
#[ts(optional = nullable)]
pub plugin_id: Option<String>,
#[ts(optional = nullable)]
pub source_path: Option<PathBuf>,
pub key: String,
pub enabled: bool,
pub enum HooksConfigWriteParams {
Plugin {
#[serde(rename = "pluginId")]
#[ts(rename = "pluginId")]
plugin_id: String,
key: String,
enabled: bool,
},
User {
#[serde(rename = "sourcePath")]
#[ts(rename = "sourcePath")]
source_path: PathBuf,
key: String,
enabled: bool,
},
Project {
#[serde(rename = "sourcePath")]
#[ts(rename = "sourcePath")]
source_path: PathBuf,
key: String,
enabled: bool,
},
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
@@ -9770,6 +9775,42 @@ mod tests {
);
}
#[test]
fn hooks_config_write_params_round_trips_selector_variants() {
let source_path = PathBuf::from(absolute_path_string("repo/.codex/hooks.json"));
let value = json!({
"source": "project",
"sourcePath": source_path,
"key": "PreToolUse:0:0",
"enabled": false,
});
let params: HooksConfigWriteParams =
serde_json::from_value(value.clone()).expect("deserialize project hook selector");
assert_eq!(
params,
HooksConfigWriteParams::Project {
source_path: source_path.clone(),
key: "PreToolUse:0:0".to_string(),
enabled: false,
}
);
assert_eq!(
serde_json::to_value(&params).expect("serialize project hook selector"),
value
);
let err = serde_json::from_value::<HooksConfigWriteParams>(json!({
"source": "project",
"pluginId": "demo-plugin@test-marketplace",
"sourcePath": source_path,
"key": "PreToolUse:0:0",
"enabled": false,
}))
.expect_err("project hook selector should reject pluginId");
assert!(err.to_string().contains("pluginId"));
}
#[test]
fn network_requirements_deserializes_legacy_fields() {
let requirements: NetworkRequirements = serde_json::from_value(json!({
codex-rs/app-server/src/codex_message_processor.rs
+37 -58
View File
@@ -75,7 +75,6 @@ use codex_app_server_protocol::GetConversationSummaryParams;
use codex_app_server_protocol::GetConversationSummaryResponse;
use codex_app_server_protocol::GitDiffToRemoteResponse;
use codex_app_server_protocol::GitInfo as ApiGitInfo;
use codex_app_server_protocol::HookConfigSource as ApiHookConfigSource;
use codex_app_server_protocol::HookErrorInfo;
use codex_app_server_protocol::HookMetadata;
use codex_app_server_protocol::HooksConfigWriteParams;
@@ -238,6 +237,7 @@ use codex_backend_client::AddCreditsNudgeCreditType as BackendAddCreditsNudgeCre
use codex_backend_client::Client as BackendClient;
use codex_chatgpt::connectors;
use codex_chatgpt::workspace_settings;
use codex_config::HookConfigSelector;
use codex_config::types::McpServerTransportConfig;
use codex_core::CodexThread;
use codex_core::CodexThreadTurnContextOverrides;
@@ -256,7 +256,6 @@ use codex_core::config::NetworkProxyAuditMetadata;
use codex_core::config::ThreadStoreConfig;
use codex_core::config::edit::ConfigEdit;
use codex_core::config::edit::ConfigEditsBuilder;
use codex_core::config::edit::HookConfigEditSelector;
use codex_core::config_loader::CloudRequirementsLoadError;
use codex_core::config_loader::CloudRequirementsLoadErrorCode;
use codex_core::config_loader::project_trust_key;
@@ -7174,69 +7173,57 @@ impl CodexMessageProcessor {
request_id: ConnectionRequestId,
params: HooksConfigWriteParams,
) {
let HooksConfigWriteParams {
source,
plugin_id,
source_path,
key,
enabled,
} = params;
let edit = match source {
ApiHookConfigSource::Plugin => {
let Some(plugin_id) = plugin_id.filter(|plugin_id| !plugin_id.trim().is_empty())
else {
let (selector, enabled) = match params {
HooksConfigWriteParams::Plugin {
plugin_id,
key,
enabled,
} => {
if plugin_id.trim().is_empty() || key.trim().is_empty() {
self.send_invalid_request_error(
request_id,
"hooks/config/write requires pluginId when source is plugin".to_string(),
)
.await;
return;
};
if key.trim().is_empty() {
self.send_invalid_request_error(
request_id,
"hooks/config/write requires a non-empty key".to_string(),
"hooks/config/write requires non-empty pluginId and key for plugin selectors"
.to_string(),
)
.await;
return;
}
ConfigEdit::SetHookConfig {
selector: HookConfigEditSelector::Plugin { plugin_id, key },
enabled,
}
(HookConfigSelector::Plugin { plugin_id, key }, enabled)
}
ApiHookConfigSource::User | ApiHookConfigSource::Project => {
let Some(source_path) =
source_path.filter(|source_path| !source_path.as_os_str().is_empty())
else {
HooksConfigWriteParams::User {
source_path,
key,
enabled,
} => {
if source_path.as_os_str().is_empty() || key.trim().is_empty() {
self.send_invalid_request_error(
request_id,
format!(
"hooks/config/write requires sourcePath when source is {}",
hook_config_source_label(source)
),
)
.await;
return;
};
if key.trim().is_empty() {
self.send_invalid_request_error(
request_id,
"hooks/config/write requires a non-empty key".to_string(),
"hooks/config/write requires non-empty sourcePath and key for user selectors"
.to_string(),
)
.await;
return;
}
let selector = match source {
ApiHookConfigSource::User => HookConfigEditSelector::User { source_path, key },
ApiHookConfigSource::Project => {
HookConfigEditSelector::Project { source_path, key }
}
ApiHookConfigSource::Plugin => unreachable!("plugin handled above"),
};
ConfigEdit::SetHookConfig { selector, enabled }
(HookConfigSelector::User { source_path, key }, enabled)
}
HooksConfigWriteParams::Project {
source_path,
key,
enabled,
} => {
if source_path.as_os_str().is_empty() || key.trim().is_empty() {
self.send_invalid_request_error(
request_id,
"hooks/config/write requires non-empty sourcePath and key for project selectors"
.to_string(),
)
.await;
return;
}
(HookConfigSelector::Project { source_path, key }, enabled)
}
};
let edit = ConfigEdit::SetHookConfig { selector, enabled };
let result = ConfigEditsBuilder::new(&self.config.codex_home)
.with_edits([edit])
.apply()
@@ -9594,14 +9581,6 @@ fn hook_to_info(hook: HookInventoryEntry) -> HookMetadata {
}
}
fn hook_config_source_label(source: ApiHookConfigSource) -> &'static str {
match source {
ApiHookConfigSource::Plugin => "plugin",
ApiHookConfigSource::User => "user",
ApiHookConfigSource::Project => "project",
}
}
fn local_plugin_interface_to_info(interface: PluginManifestInterface) -> PluginInterface {
PluginInterface {
display_name: interface.display_name,
codex-rs/app-server/tests/suite/v2/config_rpc.rs
+1 -2
View File
@@ -14,7 +14,6 @@ use codex_app_server_protocol::ConfigReadParams;
use codex_app_server_protocol::ConfigReadResponse;
use codex_app_server_protocol::ConfigValueWriteParams;
use codex_app_server_protocol::ConfigWriteResponse;
use codex_app_server_protocol::HookConfigSource;
use codex_app_server_protocol::HookEventName;
use codex_app_server_protocol::HookSource;
use codex_app_server_protocol::HooksConfigWriteResponse;
@@ -108,7 +107,7 @@ async fn hooks_config_write_persists_project_selector() -> Result<()> {
.send_raw_request(
"hooks/config/write",
Some(json!({
"source": HookConfigSource::Project,
"source": "project",
"sourcePath": project_hook_path.clone(),
"key": "PreToolUse:0:0",
"enabled": false,
codex-rs/config/src/hook_config.rs
+7
View File
@@ -93,6 +93,13 @@ pub enum HookConfigSource {
Project,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum HookConfigSelector {
Plugin { plugin_id: String, key: String },
User { source_path: PathBuf, key: String },
Project { source_path: PathBuf, key: String },
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
pub struct HookConfig {
pub source: HookConfigSource,
codex-rs/config/src/lib.rs
+1
View File
@@ -69,6 +69,7 @@ pub use diagnostics::format_config_error_with_source;
pub use diagnostics::io_error_from_config_error;
pub use fingerprint::version_for_toml;
pub use hook_config::HookConfig;
pub use hook_config::HookConfigSelector;
pub use hook_config::HookConfigSource;
pub use hook_config::HookEventsToml;
pub use hook_config::HookHandlerConfig;
codex-rs/core/src/config/edit.rs
+80 -177
View File
@@ -2,6 +2,7 @@ use crate::path_utils::resolve_symlink_write_paths;
use crate::path_utils::write_atomically;
use anyhow::Context;
use codex_config::CONFIG_TOML_FILE;
use codex_config::HookConfigSelector;
use codex_config::types::McpServerConfig;
use codex_features::FEATURES;
use codex_protocol::config_types::Personality;
@@ -63,7 +64,7 @@ pub enum ConfigEdit {
SetSkillConfigByName { name: String, enabled: bool },
/// Set or clear a hook config entry under `[[hooks.config]]`.
SetHookConfig {
selector: HookConfigEditSelector,
selector: HookConfigSelector,
enabled: bool,
},
/// Set trust_level under `[projects."<path>"]`,
@@ -84,13 +85,6 @@ enum SkillConfigSelector {
Path(PathBuf),
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum HookConfigEditSelector {
Plugin { plugin_id: String, key: String },
User { source_path: PathBuf, key: String },
Project { source_path: PathBuf, key: String },
}
/// Produces a config edit that sets `[tui].theme = "<name>"`.
pub fn syntax_theme_edit(name: &str) -> ConfigEdit {
ConfigEdit::SetPath {
@@ -633,193 +627,87 @@ impl ConfigDocument {
if matches!(&selector, SkillConfigSelector::Name(name) if name.is_empty()) {
return false;
}
let mut remove_skills_table = false;
let mut mutated = false;
{
let root = self.doc.as_table_mut();
let skills_item = match root.get_mut("skills") {
Some(item) => item,
None => {
if enabled {
return false;
}
root.insert(
"skills",
TomlItem::Table(document_helpers::new_implicit_table()),
);
let Some(item) = root.get_mut("skills") else {
return false;
};
item
}
};
if document_helpers::ensure_table_for_write(skills_item).is_none() {
if enabled {
return false;
}
*skills_item = TomlItem::Table(document_helpers::new_implicit_table());
}
let Some(skills_table) = skills_item.as_table_mut() else {
return false;
};
let config_item = match skills_table.get_mut("config") {
Some(item) => item,
None => {
if enabled {
return false;
}
skills_table.insert("config", TomlItem::ArrayOfTables(ArrayOfTables::new()));
let Some(item) = skills_table.get_mut("config") else {
return false;
};
item
}
};
if !matches!(config_item, TomlItem::ArrayOfTables(_)) {
if enabled {
return false;
}
*config_item = TomlItem::ArrayOfTables(ArrayOfTables::new());
}
let TomlItem::ArrayOfTables(overrides) = config_item else {
return false;
};
let existing_index = overrides.iter().enumerate().find_map(|(idx, table)| {
skill_config_selector_from_table(table)
.filter(|value| value == &selector)
.map(|_| idx)
});
if enabled {
if let Some(index) = existing_index {
overrides.remove(index);
mutated = true;
if overrides.is_empty() {
skills_table.remove("config");
if skills_table.is_empty() {
remove_skills_table = true;
}
}
}
} else if let Some(index) = existing_index {
for (idx, table) in overrides.iter_mut().enumerate() {
if idx == index {
write_skill_config_selector(table, &selector);
table["enabled"] = value(false);
mutated = true;
break;
}
}
} else {
let mut entry = TomlTable::new();
entry.set_implicit(false);
write_skill_config_selector(&mut entry, &selector);
entry["enabled"] = value(false);
overrides.push(entry);
mutated = true;
}
}
if remove_skills_table {
let root = self.doc.as_table_mut();
root.remove("skills");
}
mutated
self.set_config_table_entry(
"skills",
&selector,
enabled,
skill_config_selector_from_table,
write_skill_config_selector,
)
}
fn set_hook_config(&mut self, selector: HookConfigEditSelector, enabled: bool) -> bool {
let selector = match selector {
HookConfigEditSelector::Plugin { plugin_id, key } => HookConfigEditSelector::Plugin {
plugin_id: plugin_id.trim().to_string(),
key: key.trim().to_string(),
},
HookConfigEditSelector::User { source_path, key } => HookConfigEditSelector::User {
source_path,
key: key.trim().to_string(),
},
HookConfigEditSelector::Project { source_path, key } => {
HookConfigEditSelector::Project {
source_path,
key: key.trim().to_string(),
}
}
};
fn set_hook_config(&mut self, selector: HookConfigSelector, enabled: bool) -> bool {
let selector = normalize_hook_config_selector(selector);
if match &selector {
HookConfigEditSelector::Plugin { plugin_id, key } => {
plugin_id.is_empty() || key.is_empty()
}
HookConfigEditSelector::User { source_path, key }
| HookConfigEditSelector::Project { source_path, key } => {
HookConfigSelector::Plugin { plugin_id, key } => plugin_id.is_empty() || key.is_empty(),
HookConfigSelector::User { source_path, key }
| HookConfigSelector::Project { source_path, key } => {
source_path.as_os_str().is_empty() || key.is_empty()
}
} {
return false;
}
let selector = match selector {
HookConfigEditSelector::Plugin { plugin_id, key } => HookConfigEditSelector::Plugin {
plugin_id: plugin_id.trim().to_string(),
key: key.trim().to_string(),
},
HookConfigEditSelector::User { source_path, key } => HookConfigEditSelector::User {
source_path: normalize_hook_config_source_path(&source_path),
key,
},
HookConfigEditSelector::Project { source_path, key } => {
HookConfigEditSelector::Project {
source_path: normalize_hook_config_source_path(&source_path),
key,
}
}
};
self.set_config_table_entry(
"hooks",
&selector,
enabled,
hook_config_selector_from_table,
write_hook_config_selector,
)
}
let mut remove_hooks_table = false;
fn set_config_table_entry<S>(
&mut self,
section: &'static str,
selector: &S,
enabled: bool,
selector_from_table: impl Fn(&TomlTable) -> Option<S>,
write_selector: impl Fn(&mut TomlTable, &S),
) -> bool
where
S: Eq,
{
let mut remove_section_table = false;
let mut mutated = false;
{
let root = self.doc.as_table_mut();
let hooks_item = match root.get_mut("hooks") {
let section_item = match root.get_mut(section) {
Some(item) => item,
None => {
if enabled {
return false;
}
root.insert(
"hooks",
section,
TomlItem::Table(document_helpers::new_implicit_table()),
);
let Some(item) = root.get_mut("hooks") else {
let Some(item) = root.get_mut(section) else {
return false;
};
item
}
};
if document_helpers::ensure_table_for_write(hooks_item).is_none() {
if document_helpers::ensure_table_for_write(section_item).is_none() {
if enabled {
return false;
}
*hooks_item = TomlItem::Table(document_helpers::new_implicit_table());
*section_item = TomlItem::Table(document_helpers::new_implicit_table());
}
let Some(hooks_table) = hooks_item.as_table_mut() else {
let Some(section_table) = section_item.as_table_mut() else {
return false;
};
let config_item = match hooks_table.get_mut("config") {
let config_item = match section_table.get_mut("config") {
Some(item) => item,
None => {
if enabled {
return false;
}
hooks_table.insert("config", TomlItem::ArrayOfTables(ArrayOfTables::new()));
let Some(item) = hooks_table.get_mut("config") else {
section_table.insert("config", TomlItem::ArrayOfTables(ArrayOfTables::new()));
let Some(item) = section_table.get_mut("config") else {
return false;
};
item
@@ -838,8 +726,8 @@ impl ConfigDocument {
};
let existing_index = overrides.iter().enumerate().find_map(|(idx, table)| {
hook_config_selector_from_table(table)
.filter(|value| value == &selector)
selector_from_table(table)
.filter(|value| value == selector)
.map(|_| idx)
});
@@ -848,16 +736,16 @@ impl ConfigDocument {
overrides.remove(index);
mutated = true;
if overrides.is_empty() {
hooks_table.remove("config");
if hooks_table.is_empty() {
remove_hooks_table = true;
section_table.remove("config");
if section_table.is_empty() {
remove_section_table = true;
}
}
}
} else if let Some(index) = existing_index {
for (idx, table) in overrides.iter_mut().enumerate() {
if idx == index {
write_hook_config_selector(table, &selector);
write_selector(table, selector);
table["enabled"] = value(false);
mutated = true;
break;
@@ -866,16 +754,16 @@ impl ConfigDocument {
} else {
let mut entry = TomlTable::new();
entry.set_implicit(false);
write_hook_config_selector(&mut entry, &selector);
write_selector(&mut entry, selector);
entry["enabled"] = value(false);
overrides.push(entry);
mutated = true;
}
}
if remove_hooks_table {
if remove_section_table {
let root = self.doc.as_table_mut();
root.remove("hooks");
root.remove(section);
}
mutated
@@ -1027,7 +915,7 @@ fn write_skill_config_selector(table: &mut TomlTable, selector: &SkillConfigSele
}
}
fn hook_config_selector_from_table(table: &TomlTable) -> Option<HookConfigEditSelector> {
fn hook_config_selector_from_table(table: &TomlTable) -> Option<HookConfigSelector> {
let source = table
.get("source")
.and_then(|item| item.as_str())
@@ -1051,18 +939,16 @@ fn hook_config_selector_from_table(table: &TomlTable) -> Option<HookConfigEditSe
.map(|path| normalize_hook_config_source_path(&path));
match (source, plugin_id, source_path, key) {
(Some("plugin"), Some(plugin_id), None, Some(key)) => {
Some(HookConfigEditSelector::Plugin {
plugin_id: plugin_id.to_string(),
key: key.to_string(),
})
}
(Some("user"), None, Some(source_path), Some(key)) => Some(HookConfigEditSelector::User {
(Some("plugin"), Some(plugin_id), None, Some(key)) => Some(HookConfigSelector::Plugin {
plugin_id: plugin_id.to_string(),
key: key.to_string(),
}),
(Some("user"), None, Some(source_path), Some(key)) => Some(HookConfigSelector::User {
source_path,
key: key.to_string(),
}),
(Some("project"), None, Some(source_path), Some(key)) => {
Some(HookConfigEditSelector::Project {
Some(HookConfigSelector::Project {
source_path,
key: key.to_string(),
})
@@ -1071,21 +957,21 @@ fn hook_config_selector_from_table(table: &TomlTable) -> Option<HookConfigEditSe
}
}
fn write_hook_config_selector(table: &mut TomlTable, selector: &HookConfigEditSelector) {
fn write_hook_config_selector(table: &mut TomlTable, selector: &HookConfigSelector) {
match selector {
HookConfigEditSelector::Plugin { plugin_id, key } => {
HookConfigSelector::Plugin { plugin_id, key } => {
table["source"] = value("plugin");
table["plugin_id"] = value(plugin_id.clone());
table.remove("source_path");
table["key"] = value(key.clone());
}
HookConfigEditSelector::User { source_path, key } => {
HookConfigSelector::User { source_path, key } => {
table["source"] = value("user");
table.remove("plugin_id");
table["source_path"] = value(source_path.to_string_lossy().to_string());
table["key"] = value(key.clone());
}
HookConfigEditSelector::Project { source_path, key } => {
HookConfigSelector::Project { source_path, key } => {
table["source"] = value("project");
table.remove("plugin_id");
table["source_path"] = value(source_path.to_string_lossy().to_string());
@@ -1094,6 +980,23 @@ fn write_hook_config_selector(table: &mut TomlTable, selector: &HookConfigEditSe
}
}
fn normalize_hook_config_selector(selector: HookConfigSelector) -> HookConfigSelector {
match selector {
HookConfigSelector::Plugin { plugin_id, key } => HookConfigSelector::Plugin {
plugin_id: plugin_id.trim().to_string(),
key: key.trim().to_string(),
},
HookConfigSelector::User { source_path, key } => HookConfigSelector::User {
source_path: normalize_hook_config_source_path(&source_path),
key: key.trim().to_string(),
},
HookConfigSelector::Project { source_path, key } => HookConfigSelector::Project {
source_path: normalize_hook_config_source_path(&source_path),
key: key.trim().to_string(),
},
}
}
fn normalize_hook_config_source_path(path: &Path) -> PathBuf {
path.canonicalize().unwrap_or_else(|_| path.to_path_buf())
}
codex-rs/core/src/config/edit_tests.rs
+3 -3
View File
@@ -141,7 +141,7 @@ fn set_hook_config_writes_disabled_plugin_entry() {
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::SetHookConfig {
selector: HookConfigEditSelector::Plugin {
selector: HookConfigSelector::Plugin {
plugin_id: "demo-plugin@test-marketplace".to_string(),
key: "hooks/hooks.json:PreToolUse:0:0".to_string(),
},
@@ -177,7 +177,7 @@ enabled = false
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::SetHookConfig {
selector: HookConfigEditSelector::Plugin {
selector: HookConfigSelector::Plugin {
plugin_id: "demo-plugin@test-marketplace".to_string(),
key: "hooks/hooks.json:PreToolUse:0:0".to_string(),
},
@@ -197,7 +197,7 @@ fn set_hook_config_writes_disabled_project_entry() {
ConfigEditsBuilder::new(codex_home)
.with_edits([ConfigEdit::SetHookConfig {
selector: HookConfigEditSelector::Project {
selector: HookConfigSelector::Project {
source_path: PathBuf::from("/repo/.codex/hooks.json"),
key: "PreToolUse:0:0".to_string(),
},
codex-rs/hooks/src/engine/config_rules.rs
+126 -88
View File
@@ -5,6 +5,7 @@ use codex_config::ConfigLayerSource;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::HookConfig;
use codex_config::HookConfigSelector;
use codex_config::HookConfigSource;
use codex_config::HookEventsToml;
use codex_protocol::protocol::HookEventName;
@@ -13,8 +14,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
#[derive(Default)]
pub(crate) struct HookConfigRules {
plugin: HashMap<(String, String), bool>,
path: HashMap<(HookConfigSource, String, String), bool>,
entries: HashMap<HookConfigSelector, bool>,
}
impl HookConfigRules {
@@ -53,13 +53,6 @@ impl HookConfigRules {
rules
}
pub(crate) fn enabled_for_plugin_hook(&self, plugin_id: &str, key: &str) -> bool {
self.plugin
.get(&(plugin_id.to_string(), key.to_string()))
.copied()
.unwrap_or_else(default_hook_enabled)
}
pub(crate) fn enabled_for_hook(
&self,
source: HookSource,
@@ -67,83 +60,25 @@ impl HookConfigRules {
source_path: &AbsolutePathBuf,
key: &str,
) -> bool {
match source {
HookSource::Plugin => plugin_id
.map(|plugin_id| self.enabled_for_plugin_hook(plugin_id, key))
.unwrap_or_else(default_hook_enabled),
HookSource::User | HookSource::Project => {
let Some(source) = hook_config_source_for_hook_source(source) else {
return default_hook_enabled();
};
self.path
.get(&(
source,
normalize_source_path(source_path.as_path()),
key.to_string(),
))
.copied()
.unwrap_or_else(default_hook_enabled)
}
HookSource::System
| HookSource::Mdm
| HookSource::SessionFlags
| HookSource::LegacyManagedConfigFile
| HookSource::LegacyManagedConfigMdm
| HookSource::Unknown => default_hook_enabled(),
}
hook_config_selector_for_hook(source, plugin_id, source_path, key)
.map(|selector| self.enabled_for_selector(selector))
.unwrap_or_else(default_hook_enabled)
}
fn enabled_for_selector(&self, selector: HookConfigSelector) -> bool {
self.entries
.get(&normalize_hook_config_selector(selector))
.copied()
.unwrap_or_else(default_hook_enabled)
}
fn append(&mut self, entry: HookConfig, warnings: &mut Vec<String>) {
match entry.source {
HookConfigSource::Plugin => {
let Some(plugin_id) = entry.plugin_id else {
warnings.push(
"ignoring plugin hooks.config entry without a plugin_id selector"
.to_string(),
);
return;
};
if plugin_id.trim().is_empty() {
warnings.push(
"ignoring plugin hooks.config entry with empty plugin_id".to_string(),
);
return;
}
if entry.key.trim().is_empty() {
warnings.push("ignoring hooks.config entry with empty key".to_string());
return;
}
self.plugin.insert((plugin_id, entry.key), entry.enabled);
}
HookConfigSource::User | HookConfigSource::Project => {
let Some(source_path) = entry.source_path else {
warnings.push(format!(
"ignoring {} hooks.config entry without a source_path selector",
hook_config_source_label(entry.source)
));
return;
};
if source_path.as_os_str().is_empty() {
warnings.push(format!(
"ignoring {} hooks.config entry with empty source_path",
hook_config_source_label(entry.source)
));
return;
}
if entry.key.trim().is_empty() {
warnings.push("ignoring hooks.config entry with empty key".to_string());
return;
}
self.path.insert(
(
entry.source,
normalize_source_path(&source_path),
entry.key.trim().to_string(),
),
entry.enabled,
);
}
}
let enabled = entry.enabled;
let Some(selector) = hook_config_selector_for_config(entry, warnings) else {
return;
};
self.entries
.insert(normalize_hook_config_selector(selector), enabled);
}
}
@@ -166,6 +101,95 @@ fn hook_config_source_for_hook_source(source: HookSource) -> Option<HookConfigSo
}
}
fn hook_config_selector_for_hook(
source: HookSource,
plugin_id: Option<&str>,
source_path: &AbsolutePathBuf,
key: &str,
) -> Option<HookConfigSelector> {
match source {
HookSource::Plugin => plugin_id.map(|plugin_id| HookConfigSelector::Plugin {
plugin_id: plugin_id.to_string(),
key: key.to_string(),
}),
HookSource::User | HookSource::Project => {
hook_config_source_for_hook_source(source).map(|source| match source {
HookConfigSource::User => HookConfigSelector::User {
source_path: source_path.as_path().to_path_buf(),
key: key.to_string(),
},
HookConfigSource::Project => HookConfigSelector::Project {
source_path: source_path.as_path().to_path_buf(),
key: key.to_string(),
},
HookConfigSource::Plugin => unreachable!("plugin hook source handled above"),
})
}
HookSource::System
| HookSource::Mdm
| HookSource::SessionFlags
| HookSource::LegacyManagedConfigFile
| HookSource::LegacyManagedConfigMdm
| HookSource::Unknown => None,
}
}
fn hook_config_selector_for_config(
entry: HookConfig,
warnings: &mut Vec<String>,
) -> Option<HookConfigSelector> {
if entry.key.trim().is_empty() {
warnings.push("ignoring hooks.config entry with empty key".to_string());
return None;
}
match entry.source {
HookConfigSource::Plugin => {
let Some(plugin_id) = entry.plugin_id else {
warnings.push(
"ignoring plugin hooks.config entry without a plugin_id selector".to_string(),
);
return None;
};
if plugin_id.trim().is_empty() {
warnings
.push("ignoring plugin hooks.config entry with empty plugin_id".to_string());
return None;
}
Some(HookConfigSelector::Plugin {
plugin_id,
key: entry.key,
})
}
HookConfigSource::User | HookConfigSource::Project => {
let Some(source_path) = entry.source_path else {
warnings.push(format!(
"ignoring {} hooks.config entry without a source_path selector",
hook_config_source_label(entry.source)
));
return None;
};
if source_path.as_os_str().is_empty() {
warnings.push(format!(
"ignoring {} hooks.config entry with empty source_path",
hook_config_source_label(entry.source)
));
return None;
}
match entry.source {
HookConfigSource::User => Some(HookConfigSelector::User {
source_path,
key: entry.key,
}),
HookConfigSource::Project => Some(HookConfigSelector::Project {
source_path,
key: entry.key,
}),
HookConfigSource::Plugin => unreachable!("plugin config source handled above"),
}
}
}
}
fn hook_config_source_label(source: HookConfigSource) -> &'static str {
match source {
HookConfigSource::Plugin => "plugin",
@@ -174,11 +198,25 @@ fn hook_config_source_label(source: HookConfigSource) -> &'static str {
}
}
fn normalize_source_path(path: &Path) -> String {
path.canonicalize()
.unwrap_or_else(|_| path.to_path_buf())
.to_string_lossy()
.to_string()
fn normalize_hook_config_selector(selector: HookConfigSelector) -> HookConfigSelector {
match selector {
HookConfigSelector::Plugin { plugin_id, key } => HookConfigSelector::Plugin {
plugin_id: plugin_id.trim().to_string(),
key: key.trim().to_string(),
},
HookConfigSelector::User { source_path, key } => HookConfigSelector::User {
source_path: normalize_source_path(&source_path),
key: key.trim().to_string(),
},
HookConfigSelector::Project { source_path, key } => HookConfigSelector::Project {
source_path: normalize_source_path(&source_path),
key: key.trim().to_string(),
},
}
}
fn normalize_source_path(path: &Path) -> std::path::PathBuf {
path.canonicalize().unwrap_or_else(|_| path.to_path_buf())
}
pub(crate) fn hook_config_key(