mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add Windows hook command overrides (#22159)
# Why
Managed hook configs need a shared cross-platform shape without making
the existing `command` field polymorphic. The common case is still one
command string, with Windows needing a different entrypoint only when
the runtime is actually Windows.
Keeping `command` as the portable/default path and adding an optional
Windows override keeps the config easier to read, preserves the existing
scalar shape for non-Windows users, and avoids forcing every caller into
a `{ unix, windows }` object when only one platform needs special
handling.
# What
- Add optional `command_windows` / `commandWindows` alongside the
existing hook `command` field.
- Resolve `command_windows` only on Windows during hook discovery; other
platforms continue to use `command` unchanged.
- Keep trust hashing aligned to the effective command selected for the
current runtime.
# Docs
The Codex hooks/config reference should document `command_windows` as
the Windows-only override for command hooks.
This commit is contained in:
@@ -388,10 +388,16 @@ fn append_matcher_groups(
|
||||
match handler {
|
||||
HookHandlerConfig::Command {
|
||||
command,
|
||||
command_windows,
|
||||
timeout_sec,
|
||||
r#async,
|
||||
status_message,
|
||||
} => {
|
||||
let command = if cfg!(windows) {
|
||||
command_windows.unwrap_or(command)
|
||||
} else {
|
||||
command
|
||||
};
|
||||
if r#async {
|
||||
warnings.push(format!(
|
||||
"skipping async hook in {}: async hooks are not supported yet",
|
||||
@@ -409,6 +415,7 @@ fn append_matcher_groups(
|
||||
let timeout_sec = timeout_sec.unwrap_or(600).max(1);
|
||||
let normalized_handler = HookHandlerConfig::Command {
|
||||
command: command.clone(),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(timeout_sec),
|
||||
r#async,
|
||||
status_message: status_message.clone(),
|
||||
@@ -608,6 +615,7 @@ mod tests {
|
||||
matcher: matcher.map(str::to_string),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "echo hello".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
@@ -753,6 +761,7 @@ mod tests {
|
||||
matcher: None,
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "echo hello".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
@@ -763,6 +772,45 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pre_tool_use_resolves_windows_command_override_during_discovery() {
|
||||
let mut handlers = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut display_order = 0;
|
||||
let source_path = source_path();
|
||||
let hook_states = std::collections::HashMap::new();
|
||||
|
||||
append_matcher_groups(
|
||||
&mut handlers,
|
||||
&mut Vec::new(),
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
&hook_handler_source(&source_path, &hook_states),
|
||||
HookEventName::PreToolUse,
|
||||
vec![MatcherGroup {
|
||||
matcher: Some("^Bash$".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "echo unix".to_string(),
|
||||
command_windows: Some("echo windows".to_string()),
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
}],
|
||||
}],
|
||||
);
|
||||
|
||||
assert_eq!(warnings, Vec::<String>::new());
|
||||
assert_eq!(handlers.len(), 1);
|
||||
assert_eq!(
|
||||
handlers[0].command,
|
||||
if cfg!(windows) {
|
||||
"echo windows"
|
||||
} else {
|
||||
"echo unix"
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
fn config_with_malformed_state_and_session_start_hook() -> TomlValue {
|
||||
serde_json::from_value(serde_json::json!({
|
||||
"hooks": {
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_config::TomlValue;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::HookOutputEntry;
|
||||
use codex_protocol::protocol::HookOutputEntryKind;
|
||||
use codex_protocol::protocol::HookRunStatus;
|
||||
use codex_protocol::protocol::HookSource;
|
||||
@@ -85,6 +86,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
matcher: Some("^Bash$".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: format!("python3 {}", script_path.display()),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
@@ -169,6 +171,84 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
assert!(log_contents.contains("\"hook_event_name\": \"PreToolUse\""));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn requirements_managed_hooks_execute_windows_command_override() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
let managed_dir =
|
||||
AbsolutePathBuf::try_from(temp.path().join("managed-hooks")).expect("absolute path");
|
||||
fs::create_dir_all(managed_dir.as_path()).expect("create managed hooks dir");
|
||||
|
||||
let managed_hooks = managed_hooks_for_current_platform(
|
||||
managed_dir,
|
||||
HookEventsToml {
|
||||
pre_tool_use: vec![MatcherGroup {
|
||||
matcher: Some("^Bash$".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "exit 17".to_string(),
|
||||
command_windows: Some("exit /B 19".to_string()),
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
ConfigRequirements {
|
||||
managed_hooks: Some(ConstrainedWithSource::new(
|
||||
Constrained::allow_any(managed_hooks.clone()),
|
||||
Some(RequirementSource::CloudRequirements),
|
||||
)),
|
||||
..ConfigRequirements::default()
|
||||
},
|
||||
ConfigRequirementsToml {
|
||||
hooks: Some(managed_hooks),
|
||||
..ConfigRequirementsToml::default()
|
||||
},
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
},
|
||||
);
|
||||
|
||||
let outcome = engine
|
||||
.run_pre_tool_use(PreToolUseRequest {
|
||||
session_id: ThreadId::new(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
cwd: cwd(),
|
||||
transcript_path: None,
|
||||
model: "gpt-test".to_string(),
|
||||
permission_mode: "default".to_string(),
|
||||
tool_name: "Bash".to_string(),
|
||||
matcher_aliases: Vec::new(),
|
||||
tool_use_id: "tool-1".to_string(),
|
||||
tool_input: serde_json::json!({ "command": "echo hello" }),
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(!outcome.should_block);
|
||||
let expected_exit_code = if cfg!(windows) { 19 } else { 17 };
|
||||
assert_eq!(outcome.hook_events.len(), 1);
|
||||
assert_eq!(outcome.hook_events[0].run.status, HookRunStatus::Failed);
|
||||
assert_eq!(
|
||||
outcome.hook_events[0].run.entries,
|
||||
vec![HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Error,
|
||||
text: format!("hook exited with code {expected_exit_code}"),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_requirement_source_hooks_stay_managed() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
@@ -182,6 +262,7 @@ fn unknown_requirement_source_hooks_stay_managed() {
|
||||
matcher: Some("^Bash$".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "python3 /tmp/managed.py".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
@@ -244,6 +325,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
|
||||
matcher: Some("^Bash$".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "python3 /tmp/managed.py".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
@@ -463,6 +545,7 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
|
||||
matcher: Some("^Bash$".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: format!("python3 {}", missing_dir.join("pre.py").display()),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
@@ -674,6 +757,7 @@ print(json.dumps({
|
||||
matcher: Some("Bash".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: format!("python3 {}", script_path.display()),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
@@ -780,8 +864,10 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
pre_tool_use: vec![MatcherGroup {
|
||||
matcher: Some("Bash".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "run ${PLUGIN_ROOT} ${CLAUDE_PLUGIN_ROOT} ${PLUGIN_DATA} ${CLAUDE_PLUGIN_DATA}"
|
||||
.to_string(),
|
||||
command:
|
||||
"run ${PLUGIN_ROOT} ${CLAUDE_PLUGIN_ROOT} ${PLUGIN_DATA} ${CLAUDE_PLUGIN_DATA}"
|
||||
.to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(5),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
|
||||
Reference in New Issue
Block a user