Fix hook environment cwd routing

This commit is contained in:
starr-openai
2026-05-28 22:54:33 -07:00
parent db42755366
commit 0453652972
16 changed files with 179 additions and 25 deletions
@@ -627,6 +627,8 @@ impl CatalogRequestProcessor {
config_layer_stack: Some(config.config_layer_stack),
plugin_hook_sources: plugin_outcome.effective_plugin_hook_sources(),
plugin_hook_load_warnings: plugin_outcome.effective_plugin_hook_warnings(),
local_cwd: config.cwd.clone(),
environment_manager: self.thread_manager.environment_manager(),
..Default::default()
});
data.push(codex_app_server_protocol::HooksListEntry {
+20
View File
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::future::Future;
use std::sync::Arc;
use std::time::Duration;
@@ -43,6 +44,17 @@ use crate::event_mapping::parse_turn_item;
use crate::session::TurnInput;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
fn hook_environment_cwds(
turn_context: &TurnContext,
) -> HashMap<String, codex_utils_absolute_path::AbsolutePathBuf> {
turn_context
.environments
.turn_environments
.iter()
.map(|environment| (environment.environment_id.clone(), environment.cwd.clone()))
.collect()
}
use crate::tools::hook_names::HookToolName;
use crate::tools::sandboxing::PermissionRequestPayload;
@@ -128,6 +140,7 @@ pub(crate) async fn run_pending_session_start_hooks(
session_id: sess.session_id().into(),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
@@ -170,6 +183,7 @@ pub(crate) async fn run_pre_tool_use_hooks(
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
@@ -231,6 +245,7 @@ pub(crate) async fn run_permission_request_hooks(
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
@@ -273,6 +288,7 @@ pub(crate) async fn run_post_tool_use_hooks(
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
@@ -346,6 +362,7 @@ pub(crate) async fn run_turn_stop_hooks(
turn_id: turn_context.sub_id.clone(),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
@@ -372,6 +389,7 @@ pub(crate) async fn run_pre_compact_hooks(
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
trigger: compaction_trigger_label(trigger).to_string(),
@@ -411,6 +429,7 @@ pub(crate) async fn run_post_compact_hooks(
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
trigger: compaction_trigger_label(trigger).to_string(),
@@ -507,6 +526,7 @@ pub(crate) async fn inspect_pending_input(
subagent: thread_spawn_subagent_hook_context(sess, turn_context),
#[allow(deprecated)]
cwd: turn_context.cwd.clone(),
environment_cwds: hook_environment_cwds(turn_context),
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
+1
View File
@@ -3311,6 +3311,7 @@ async fn build_hooks_for_config(
plugin_hook_load_warnings,
shell_program: Some(hook_shell_program),
shell_args: hook_shell_argv,
local_cwd: config.cwd.clone(),
environment_manager,
})
}
+3
View File
@@ -558,6 +558,7 @@ async fn preview_session_start_hooks(
hooks.preview_session_start(&codex_hooks::SessionStartRequest {
session_id: ThreadId::new(),
cwd: config.cwd.clone(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-5.2".to_string(),
permission_mode: "default".to_string(),
@@ -1329,6 +1330,7 @@ async fn reload_user_config_layer_refreshes_hooks() -> anyhow::Result<()> {
let request = codex_hooks::SessionStartRequest {
session_id: session.conversation_id,
cwd: session.get_config().await.cwd.clone(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-5.2".to_string(),
permission_mode: "default".to_string(),
@@ -1437,6 +1439,7 @@ async fn refresh_runtime_config_refreshes_hooks() -> anyhow::Result<()> {
let request = codex_hooks::SessionStartRequest {
session_id: session.conversation_id,
cwd: session.get_config().await.cwd.clone(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-5.2".to_string(),
permission_mode: "default".to_string(),
+85 -13
View File
@@ -30,21 +30,40 @@ pub(crate) async fn run_command(
shell: &CommandShell,
handler: &ConfiguredHandler,
input_json: &str,
cwd: &Path,
environment_cwds: &std::collections::HashMap<
String,
codex_utils_absolute_path::AbsolutePathBuf,
>,
) -> CommandRunResult {
let started_at = chrono::Utc::now().timestamp();
let started = Instant::now();
if let Some(environment_id) = handler.environment_id.as_deref() {
let Some(environment) = shell.environment_manager.get_environment(environment_id) else {
let environment_id = handler
.environment_id
.as_deref()
.unwrap_or(codex_exec_server::LOCAL_ENVIRONMENT_ID);
let Some(environment) = shell.environment_manager.get_environment(environment_id) else {
return command_error(
started_at,
started,
format!("unknown hook environment id: {environment_id}"),
);
};
let cwd = if environment_id == codex_exec_server::LOCAL_ENVIRONMENT_ID {
environment_cwds
.get(environment_id)
.map_or_else(|| shell.local_cwd.as_path(), |cwd| cwd.as_path())
} else {
let Some(cwd) = environment_cwds.get(environment_id) else {
return command_error(
started_at,
started,
format!("unknown hook environment id: {environment_id}"),
format!("hook environment `{environment_id}` has no selected cwd"),
);
};
if environment.is_remote() {
return run_remote_hook_command(handler, environment, input_json, cwd).await;
}
cwd.as_path()
};
if environment.is_remote() {
return run_remote_hook_command(handler, environment, input_json, cwd).await;
}
let mut command = build_command(shell, handler);
@@ -413,6 +432,7 @@ mod tests {
CommandShell {
program: String::new(),
args: Vec::new(),
local_cwd: test_path_buf("/tmp").abs(),
environment_manager,
}
}
@@ -474,7 +494,7 @@ mod tests {
&shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())),
&handler("printf local-hook", None),
"{}",
test_path_buf("/tmp").as_path(),
&Default::default(),
)
.await;
@@ -483,13 +503,40 @@ mod tests {
assert_eq!(result.error, None);
}
#[tokio::test]
async fn omitted_environment_id_uses_local_hook_cwd() {
let local_cwd = tempfile::tempdir().expect("local cwd");
let local_cwd =
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(local_cwd.path())
.expect("absolute local cwd");
let mut shell = shell(std::sync::Arc::new(EnvironmentManager::default_for_tests()));
shell.local_cwd = local_cwd.clone();
let result = run_command(
&shell,
&handler(if cfg!(windows) { "cd" } else { "pwd -P" }, None),
"{}",
&Default::default(),
)
.await;
assert_eq!(result.exit_code, Some(0));
assert_eq!(
result.stdout.trim(),
std::fs::canonicalize(local_cwd.as_path())
.expect("canonical local cwd")
.display()
.to_string()
);
assert_eq!(result.error, None);
}
#[tokio::test]
async fn explicit_local_environment_id_runs_locally() {
let result = run_command(
&shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())),
&handler("printf explicit-local-hook", Some("local")),
"{}",
test_path_buf("/tmp").as_path(),
&Default::default(),
)
.await;
@@ -504,7 +551,7 @@ mod tests {
&shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())),
&handler("cat; printf stderr >&2; exit 7", None),
"{\"hook\":true}",
test_path_buf("/tmp").as_path(),
&Default::default(),
)
.await;
@@ -522,7 +569,7 @@ mod tests {
&shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())),
&handler,
"{}",
test_path_buf("/tmp").as_path(),
&Default::default(),
)
.await;
@@ -538,7 +585,7 @@ mod tests {
&shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())),
&handler("printf local-hook", Some("missing")),
"{}",
test_path_buf("/tmp").as_path(),
&Default::default(),
)
.await;
@@ -560,7 +607,10 @@ mod tests {
&shell(environment_manager),
&handler("printf local-hook", Some("remote-hook")),
"{}",
test_path_buf("/tmp").as_path(),
&std::collections::HashMap::from([(
"remote-hook".to_string(),
test_path_buf("/tmp").abs(),
)]),
)
.await;
@@ -574,6 +624,28 @@ mod tests {
);
}
#[tokio::test]
async fn explicit_remote_environment_requires_selected_cwd() {
let environment_manager = std::sync::Arc::new(EnvironmentManager::default_for_tests());
environment_manager
.upsert_environment("remote-hook".to_string(), "ws://127.0.0.1:1".to_string())
.expect("remote hook environment");
let result = run_command(
&shell(environment_manager),
&handler("printf remote-hook", Some("remote-hook")),
"{}",
&Default::default(),
)
.await;
assert_eq!(result.exit_code, None);
assert_eq!(result.stdout, "");
assert_eq!(
result.error.as_deref(),
Some("hook environment `remote-hook` has no selected cwd")
);
}
#[tokio::test]
async fn remote_hook_stdin_writes_payload_and_closes_stdin() {
let process = MockExecProcess::new(WriteStatus::Accepted, Vec::new());
+6 -4
View File
@@ -1,5 +1,3 @@
use std::path::Path;
use futures::StreamExt;
use futures::stream::FuturesUnordered;
@@ -90,7 +88,10 @@ pub(crate) async fn execute_handlers<T>(
shell: &CommandShell,
handlers: Vec<ConfiguredHandler>,
input_json: String,
cwd: &Path,
environment_cwds: &std::collections::HashMap<
String,
codex_utils_absolute_path::AbsolutePathBuf,
>,
turn_id: Option<String>,
parse: fn(&ConfiguredHandler, CommandRunResult, Option<String>) -> ParsedHandler<T>,
) -> Vec<ParsedHandler<T>> {
@@ -98,8 +99,9 @@ pub(crate) async fn execute_handlers<T>(
for (configured_order, handler) in handlers.into_iter().enumerate() {
let input_json = input_json.clone();
let turn_id = turn_id.clone();
let environment_cwds = environment_cwds.clone();
pending.push(async move {
let result = run_command(shell, &handler, &input_json, cwd).await;
let result = run_command(shell, &handler, &input_json, &environment_cwds).await;
(configured_order, parse(&handler, result, turn_id))
});
}
+1
View File
@@ -38,6 +38,7 @@ use std::sync::Arc;
pub(crate) struct CommandShell {
pub program: String,
pub args: Vec<String>,
pub local_cwd: AbsolutePathBuf,
pub environment_manager: Arc<EnvironmentManager>,
}
+27
View File
@@ -41,6 +41,7 @@ fn shell() -> CommandShell {
CommandShell {
program: String::new(),
args: Vec::new(),
local_cwd: cwd(),
environment_manager: std::sync::Arc::new(EnvironmentManager::default_for_tests()),
}
}
@@ -226,6 +227,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
plugin_hook_load_warnings: Vec::new(),
shell_program: None,
shell_args: Vec::new(),
local_cwd: cwd(),
environment_manager: std::sync::Arc::new(EnvironmentManager::default_for_tests()),
});
assert!(listed.hooks[0].is_managed);
@@ -235,6 +237,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd.clone(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -252,6 +255,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
turn_id: "turn-1".to_string(),
subagent: None,
cwd,
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -322,6 +326,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -699,6 +704,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd,
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1087,6 +1093,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
turn_id: "turn-1".to_string(),
subagent: None,
cwd,
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1171,6 +1178,7 @@ print(json.dumps({
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1191,6 +1199,7 @@ print(json.dumps({
plugin_hook_load_warnings: Vec::new(),
shell_program: None,
shell_args: Vec::new(),
local_cwd: cwd(),
environment_manager: std::sync::Arc::new(EnvironmentManager::default_for_tests()),
});
assert_eq!(
@@ -1204,6 +1213,7 @@ print(json.dumps({
turn_id: "turn-1".to_string(),
subagent: None,
cwd: cwd(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
@@ -1334,6 +1344,7 @@ fn structured_hooks_require_local_environment() {
CommandShell {
program: String::new(),
args: Vec::new(),
local_cwd: cwd(),
environment_manager: std::sync::Arc::new(EnvironmentManager::without_environments()),
},
);
@@ -1345,6 +1356,22 @@ fn structured_hooks_require_local_environment() {
);
}
#[tokio::test]
async fn list_hooks_requires_local_environment() {
let listed = crate::list_hooks(crate::HooksConfig {
feature_enabled: true,
local_cwd: cwd(),
environment_manager: std::sync::Arc::new(EnvironmentManager::without_environments()),
..Default::default()
});
assert!(listed.hooks.is_empty());
assert_eq!(
listed.warnings,
["structured command hooks require a local execution environment"]
);
}
#[test]
fn explicit_environment_id_is_listed_and_discovered() {
let config = serde_json::from_value(serde_json::json!({
+6 -2
View File
@@ -25,6 +25,7 @@ pub struct PreCompactRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub trigger: String,
@@ -36,6 +37,7 @@ pub struct PostCompactRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub trigger: String,
@@ -106,7 +108,7 @@ pub(crate) async fn run_pre(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id),
parse_pre_completed,
)
@@ -188,7 +190,7 @@ pub(crate) async fn run_post(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id),
parse_post_completed,
)
@@ -574,6 +576,7 @@ mod tests {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: test_path_buf("/tmp").abs(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
trigger: "manual".to_string(),
@@ -587,6 +590,7 @@ mod tests {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: test_path_buf("/tmp").abs(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
trigger: "manual".to_string(),
@@ -38,6 +38,8 @@ pub struct PermissionRequestRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: PathBuf,
pub environment_cwds:
std::collections::HashMap<String, codex_utils_absolute_path::AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -122,7 +124,7 @@ pub(crate) async fn run(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id.clone()),
parse_completed,
)
+3 -1
View File
@@ -25,6 +25,7 @@ pub struct PostToolUseRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -107,7 +108,7 @@ pub(crate) async fn run(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id.clone()),
parse_completed,
)
@@ -579,6 +580,7 @@ mod tests {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: test_path_buf("/tmp").abs(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
+3 -1
View File
@@ -25,6 +25,7 @@ pub struct PreToolUseRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -106,7 +107,7 @@ pub(crate) async fn run(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id.clone()),
parse_completed,
)
@@ -771,6 +772,7 @@ mod tests {
turn_id: "turn-1".to_string(),
subagent: None,
cwd: test_path_buf("/tmp").abs(),
environment_cwds: Default::default(),
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
+2 -1
View File
@@ -42,6 +42,7 @@ impl SessionStartSource {
pub struct SessionStartRequest {
pub session_id: ThreadId,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -184,7 +185,7 @@ pub(crate) async fn run(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
turn_id,
parse_completed,
)
+2 -1
View File
@@ -25,6 +25,7 @@ pub struct StopRequest {
pub session_id: ThreadId,
pub turn_id: String,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -181,7 +182,7 @@ pub(crate) async fn run(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id),
parse_completed,
)
@@ -25,6 +25,7 @@ pub struct UserPromptSubmitRequest {
pub turn_id: String,
pub subagent: Option<common::SubagentHookContext>,
pub cwd: AbsolutePathBuf,
pub environment_cwds: std::collections::HashMap<String, AbsolutePathBuf>,
pub transcript_path: Option<PathBuf>,
pub model: String,
pub permission_mode: String,
@@ -106,7 +107,7 @@ pub(crate) async fn run(
shell,
matched,
input_json,
request.cwd.as_path(),
&request.environment_cwds,
Some(request.turn_id),
parse_completed,
)
+13
View File
@@ -1,6 +1,7 @@
use codex_config::ConfigLayerStack;
use codex_exec_server::EnvironmentManager;
use codex_plugin::PluginHookSource;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::sync::Arc;
use tokio::process::Command;
@@ -38,6 +39,7 @@ pub struct HooksConfig {
pub plugin_hook_load_warnings: Vec<String>,
pub shell_program: Option<String>,
pub shell_args: Vec<String>,
pub local_cwd: AbsolutePathBuf,
pub environment_manager: Arc<EnvironmentManager>,
}
@@ -52,6 +54,8 @@ impl Default for HooksConfig {
plugin_hook_load_warnings: Vec::new(),
shell_program: None,
shell_args: Vec::new(),
local_cwd: AbsolutePathBuf::current_dir()
.expect("test hook config should resolve current directory"),
environment_manager: Arc::new(EnvironmentManager::default_for_tests()),
}
}
@@ -92,6 +96,7 @@ impl Hooks {
CommandShell {
program: config.shell_program.unwrap_or_default(),
args: config.shell_args,
local_cwd: config.local_cwd,
environment_manager: config.environment_manager,
},
);
@@ -229,6 +234,14 @@ pub fn list_hooks(config: HooksConfig) -> HookListOutcome {
if !config.feature_enabled {
return HookListOutcome::default();
}
if config.environment_manager.try_local_environment().is_none() {
return HookListOutcome {
hooks: Vec::new(),
warnings: vec![
"structured command hooks require a local execution environment".to_string(),
],
};
}
let discovered = crate::engine::discovery::discover_handlers(
config.config_layer_stack.as_ref(),