mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
Refresh project hooks when trust changes
This commit is contained in:
@@ -1722,7 +1722,9 @@ For linked Git worktrees, project hook declarations come from the matching `.cod
|
||||
|
||||
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.
|
||||
Project hooks inherit the trust of their project: hooks from active project config layers report `trustStatus: "trusted"` and do not require a matching `hooks.state` entry. If project trust is removed, the project layers and their hooks are removed from running threads when config is reloaded.
|
||||
|
||||
For other 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
|
||||
{
|
||||
|
||||
@@ -274,22 +274,34 @@ impl ConfigRequestProcessor {
|
||||
}
|
||||
|
||||
async fn reload_user_config(&self) {
|
||||
let next_config = match self.load_latest_config(/*fallback_cwd*/ None).await {
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
"failed to rebuild user config for runtime refresh: {}",
|
||||
err.message
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let mut threads_by_cwd = std::collections::HashMap::new();
|
||||
let thread_ids = self.thread_manager.list_thread_ids().await;
|
||||
for thread_id in thread_ids {
|
||||
let Ok(thread) = self.thread_manager.get_thread(thread_id).await else {
|
||||
continue;
|
||||
};
|
||||
thread.refresh_runtime_config(next_config.clone()).await;
|
||||
let cwd = thread.config().await.cwd.clone();
|
||||
threads_by_cwd
|
||||
.entry(cwd)
|
||||
.or_insert_with(Vec::new)
|
||||
.push(thread);
|
||||
}
|
||||
|
||||
for (cwd, threads) in threads_by_cwd {
|
||||
let next_config = match self.load_latest_config(Some(cwd.to_path_buf())).await {
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
cwd = %cwd.display(),
|
||||
"failed to rebuild runtime config for threads: {}",
|
||||
err.message
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
for thread in threads {
|
||||
thread.refresh_runtime_config(next_config.clone()).await;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -434,28 +434,38 @@ impl ConfigLayerStack {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a new stack with the user layer copied from `other`, preserving
|
||||
/// every non-user layer already present in this stack.
|
||||
pub fn with_user_layer_from(&self, other: &Self) -> Self {
|
||||
let user_layers = other
|
||||
/// Returns a new stack with the user and project layers copied from `other`,
|
||||
/// preserving every other layer already present in this stack.
|
||||
pub fn with_user_and_project_layers_from(&self, other: &Self) -> Self {
|
||||
let replacement_layers = other
|
||||
.layers
|
||||
.iter()
|
||||
.filter(|layer| matches!(layer.name, ConfigLayerSource::User { .. }))
|
||||
.filter(|layer| {
|
||||
matches!(
|
||||
layer.name,
|
||||
ConfigLayerSource::User { .. } | ConfigLayerSource::Project { .. }
|
||||
)
|
||||
})
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
let mut layers = self
|
||||
.layers
|
||||
.iter()
|
||||
.filter(|layer| !matches!(layer.name, ConfigLayerSource::User { .. }))
|
||||
.filter(|layer| {
|
||||
!matches!(
|
||||
layer.name,
|
||||
ConfigLayerSource::User { .. } | ConfigLayerSource::Project { .. }
|
||||
)
|
||||
})
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
for user_layer in user_layers {
|
||||
for replacement_layer in replacement_layers {
|
||||
match layers
|
||||
.iter()
|
||||
.position(|layer| layer.name.precedence() > user_layer.name.precedence())
|
||||
.position(|layer| layer.name.precedence() > replacement_layer.name.precedence())
|
||||
{
|
||||
Some(index) => layers.insert(index, user_layer),
|
||||
None => layers.push(user_layer),
|
||||
Some(index) => layers.insert(index, replacement_layer),
|
||||
None => layers.push(replacement_layer),
|
||||
}
|
||||
}
|
||||
let user_layer_index = layers.iter().enumerate().rev().find_map(|(index, layer)| {
|
||||
|
||||
@@ -618,9 +618,9 @@ impl CodexThread {
|
||||
self.codex.session.multi_agent_version()
|
||||
}
|
||||
|
||||
/// Refresh the thread's layer-backed user config state from a caller-supplied
|
||||
/// config snapshot. Thread-scoped layers and session-static settings remain
|
||||
/// unchanged.
|
||||
/// Refresh the thread's layer-backed user and project config state from a
|
||||
/// caller-supplied config snapshot. Thread-scoped layers and session-static
|
||||
/// settings remain unchanged.
|
||||
pub async fn refresh_runtime_config(&self, next_config: crate::config::Config) {
|
||||
self.codex.session.refresh_runtime_config(next_config).await;
|
||||
}
|
||||
|
||||
@@ -1593,9 +1593,9 @@ impl Session {
|
||||
}
|
||||
|
||||
pub(crate) async fn refresh_runtime_config(&self, next_config: Config) {
|
||||
// Refresh only the user layer from the incoming snapshot. Preserve thread-local
|
||||
// layers such as request/session overrides that were present when this session
|
||||
// was created.
|
||||
// Refresh the user and project layers from the incoming snapshot. Project trust
|
||||
// changes can enable or disable project layers, while thread-local layers such as
|
||||
// request/session overrides must remain as they were when this session was created.
|
||||
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
|
||||
let (previous_config, new_config, config) = {
|
||||
let mut state = self.state.lock().await;
|
||||
@@ -1604,7 +1604,7 @@ impl Session {
|
||||
let mut config = (*state.session_configuration.original_config_do_not_use).clone();
|
||||
config.config_layer_stack = config
|
||||
.config_layer_stack
|
||||
.with_user_layer_from(&next_config.config_layer_stack);
|
||||
.with_user_and_project_layers_from(&next_config.config_layer_stack);
|
||||
config.tool_suggest =
|
||||
resolve_tool_suggest_config_from_layer_stack(&config.config_layer_stack);
|
||||
let config = Arc::new(config);
|
||||
|
||||
@@ -1529,6 +1529,70 @@ async fn refresh_runtime_config_refreshes_hooks() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_runtime_config_revokes_hooks_when_project_becomes_untrusted() -> anyhow::Result<()>
|
||||
{
|
||||
let temp = tempfile::tempdir()?;
|
||||
let codex_home = temp.path().join("home");
|
||||
let project_root = temp.path().join("project");
|
||||
let nested = project_root.join("nested");
|
||||
let dot_codex = project_root.join(".codex");
|
||||
std::fs::create_dir_all(&codex_home)?;
|
||||
std::fs::create_dir_all(&nested)?;
|
||||
std::fs::write(project_root.join(".git"), "gitdir: here")?;
|
||||
write_project_hooks(&dot_codex)?;
|
||||
write_project_trust_config(
|
||||
&codex_home,
|
||||
&[(project_root.as_path(), TrustLevel::Trusted)],
|
||||
)
|
||||
.await?;
|
||||
|
||||
let trusted_config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.clone())
|
||||
.fallback_cwd(Some(nested.clone()))
|
||||
.build()
|
||||
.await?;
|
||||
let trusted_cwd = trusted_config.cwd.clone();
|
||||
let trusted_layer_stack = trusted_config.config_layer_stack.clone();
|
||||
let session_codex_home = trusted_config.codex_home.clone();
|
||||
let session = make_session_with_config(move |config| {
|
||||
config.codex_home = session_codex_home;
|
||||
config.cwd = trusted_cwd;
|
||||
config.config_layer_stack = trusted_layer_stack;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodexHooks)
|
||||
.expect("enable Codex hooks");
|
||||
})
|
||||
.await?;
|
||||
let request = codex_hooks::SessionStartRequest {
|
||||
session_id: session.thread_id,
|
||||
cwd: session.get_config().await.cwd.clone(),
|
||||
transcript_path: None,
|
||||
model: "gpt-5.2".to_string(),
|
||||
permission_mode: "default".to_string(),
|
||||
target: codex_hooks::StartHookTarget::SessionStart {
|
||||
source: codex_hooks::SessionStartSource::Startup,
|
||||
},
|
||||
};
|
||||
assert_eq!(session.hooks().preview_session_start(&request).len(), 1);
|
||||
|
||||
write_project_trust_config(
|
||||
&codex_home,
|
||||
&[(project_root.as_path(), TrustLevel::Untrusted)],
|
||||
)
|
||||
.await?;
|
||||
let next_config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home)
|
||||
.fallback_cwd(Some(nested))
|
||||
.build()
|
||||
.await?;
|
||||
session.refresh_runtime_config(next_config).await;
|
||||
|
||||
assert!(session.hooks().preview_session_start(&request).is_empty());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reload_user_config_layer_updates_effective_tool_suggest_config() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -3,13 +3,17 @@ use std::path::Path;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_core::config::set_project_trust_level;
|
||||
use codex_features::Feature;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_model_provider_info::built_in_model_providers;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::items::parse_hook_prompt_fragment;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -763,6 +767,65 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn project_session_start_hook_ran(
|
||||
server: &wiremock::MockServer,
|
||||
trust_level: TrustLevel,
|
||||
) -> Result<bool> {
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
let project = TempDir::new()?;
|
||||
let dot_codex = project.path().join(".codex");
|
||||
let script_path = dot_codex.join("session_start_hook.py");
|
||||
let log_path = project.path().join("project_session_start_hook.log");
|
||||
fs::create_dir_all(project.path().join(".git"))?;
|
||||
fs::create_dir_all(&dot_codex)?;
|
||||
fs::write(
|
||||
&script_path,
|
||||
format!(
|
||||
r#"from pathlib import Path
|
||||
|
||||
Path(r"{log_path}").write_text("ran\n", encoding="utf-8")
|
||||
"#,
|
||||
log_path = log_path.display(),
|
||||
),
|
||||
)?;
|
||||
fs::write(
|
||||
dot_codex.join("config.toml"),
|
||||
format!(
|
||||
r#"[hooks]
|
||||
|
||||
[[hooks.SessionStart]]
|
||||
|
||||
[[hooks.SessionStart.hooks]]
|
||||
type = "command"
|
||||
command = 'python3 {script_path}'
|
||||
"#,
|
||||
script_path = script_path.display(),
|
||||
),
|
||||
)?;
|
||||
set_project_trust_level(home.path(), project.path(), trust_level)?;
|
||||
|
||||
let resolved_config = ConfigBuilder::default()
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.fallback_cwd(Some(project.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
let project_cwd = resolved_config.cwd;
|
||||
let project_layers = resolved_config.config_layer_stack;
|
||||
let mut builder = test_codex().with_home(home).with_config(move |config| {
|
||||
config.cwd = project_cwd;
|
||||
config.config_layer_stack = project_layers;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodexHooks)
|
||||
.expect("enable Codex hooks");
|
||||
});
|
||||
let test = builder.build(server).await?;
|
||||
test.submit_turn("hello").await?;
|
||||
|
||||
Ok(log_path.exists())
|
||||
}
|
||||
|
||||
fn write_session_start_hook_with_context(home: &Path, additional_context: &str) -> Result<()> {
|
||||
let script_path = home.join("session_start_hook.py");
|
||||
let additional_context_json = serde_json::to_string(additional_context)
|
||||
@@ -1173,6 +1236,40 @@ async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn project_session_start_hooks_follow_project_trust() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let _responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-trusted"),
|
||||
ev_assistant_message("msg-trusted", "trusted project"),
|
||||
ev_completed("resp-trusted"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-untrusted"),
|
||||
ev_assistant_message("msg-untrusted", "untrusted project"),
|
||||
ev_completed("resp-untrusted"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
project_session_start_hook_ran(&server, TrustLevel::Trusted).await?,
|
||||
"trusted project hook should run"
|
||||
);
|
||||
assert!(
|
||||
!project_session_start_hook_ran(&server, TrustLevel::Untrusted).await?,
|
||||
"untrusted project hook should not run"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_start_hook_sees_materialized_transcript_path() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user