mirror of
https://github.com/ultraworkers/claw-code-parity.git
synced 2026-04-23 05:06:12 +02:00
Restore help/config loading when structured hook entries are present
The runtime config loader still treated merged settings.hooks entries as plain strings only. Current user settings can contribute simple wrapper objects that contain command hook definitions, so config loading failed before the CLI could even print help. This change teaches the loader to accept those simple command hook objects, flatten them into the existing string-based runtime hook config, and leaves plugin hook merging in the typed RuntimeHookConfig path. Focused tests cover both the structured hook parse path and the config-plus-plugin merge path. Constraint: Preserve the existing string-based runtime hook runner and plugin hook merge behavior Rejected: Ignore non-string hook entries entirely | would silently drop valid configured command hooks Rejected: Rework the runtime hook pipeline around the full structured hook schema | larger behavioral change than needed for this regression Confidence: high Scope-risk: narrow Reversibility: clean Directive: If hook matchers or non-command hook types are introduced here, extend RuntimeHookConfig deliberately instead of flattening richer semantics implicitly Tested: cargo test -p runtime loads_command_hooks_from_structured_hook_entries Tested: cargo test -p runtime loads_and_merges_claude_code_config_files_by_precedence Tested: cargo test -p rusty-claude-cli build_runtime_plugin_state_ Tested: cargo build -p rusty-claude-cli && CLAW_CONFIG_HOME=$HOME/.claude ./rust/target/debug/claw --help Not-tested: End-to-end hook execution with matcher-bearing structured hook entries
This commit is contained in:
@@ -622,11 +622,11 @@ fn parse_optional_hooks_config(root: &JsonValue) -> Result<RuntimeHookConfig, Co
|
||||
};
|
||||
let hooks = expect_object(hooks_value, "merged settings.hooks")?;
|
||||
Ok(RuntimeHookConfig {
|
||||
pre_tool_use: optional_string_array(hooks, "PreToolUse", "merged settings.hooks")?
|
||||
pre_tool_use: optional_hook_command_array(hooks, "PreToolUse", "merged settings.hooks")?
|
||||
.unwrap_or_default(),
|
||||
post_tool_use: optional_string_array(hooks, "PostToolUse", "merged settings.hooks")?
|
||||
post_tool_use: optional_hook_command_array(hooks, "PostToolUse", "merged settings.hooks")?
|
||||
.unwrap_or_default(),
|
||||
post_tool_use_failure: optional_string_array(
|
||||
post_tool_use_failure: optional_hook_command_array(
|
||||
hooks,
|
||||
"PostToolUseFailure",
|
||||
"merged settings.hooks",
|
||||
@@ -983,6 +983,83 @@ fn optional_string_array(
|
||||
}
|
||||
}
|
||||
|
||||
fn optional_hook_command_array(
|
||||
object: &BTreeMap<String, JsonValue>,
|
||||
key: &str,
|
||||
context: &str,
|
||||
) -> Result<Option<Vec<String>>, ConfigError> {
|
||||
match object.get(key) {
|
||||
Some(value) => {
|
||||
let Some(array) = value.as_array() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} must be an array"
|
||||
)));
|
||||
};
|
||||
let mut commands = Vec::new();
|
||||
for item in array {
|
||||
append_hook_commands(item, &mut commands, key, context)?;
|
||||
}
|
||||
Ok(Some(commands))
|
||||
}
|
||||
None => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
fn append_hook_commands(
|
||||
value: &JsonValue,
|
||||
commands: &mut Vec<String>,
|
||||
key: &str,
|
||||
context: &str,
|
||||
) -> Result<(), ConfigError> {
|
||||
if let Some(command) = value.as_str() {
|
||||
commands.push(command.to_string());
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(object) = value.as_object() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} must contain only strings or simple command hook objects"
|
||||
)));
|
||||
};
|
||||
|
||||
if let Some(command) = object.get("command").and_then(JsonValue::as_str) {
|
||||
commands.push(command.to_string());
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(hooks) = object.get("hooks").and_then(JsonValue::as_array) else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} must contain only strings or simple command hook objects"
|
||||
)));
|
||||
};
|
||||
|
||||
for hook in hooks {
|
||||
let Some(hook_object) = hook.as_object() else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} must contain only command hook objects"
|
||||
)));
|
||||
};
|
||||
let Some(hook_type) = hook_object.get("type").and_then(JsonValue::as_str) else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} command hook objects must include a string type"
|
||||
)));
|
||||
};
|
||||
if hook_type != "command" {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} supports only command hook objects"
|
||||
)));
|
||||
}
|
||||
let Some(command) = hook_object.get("command").and_then(JsonValue::as_str) else {
|
||||
return Err(ConfigError::Parse(format!(
|
||||
"{context}: field {key} command hook objects must include a string command"
|
||||
)));
|
||||
};
|
||||
commands.push(command.to_string());
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn optional_string_map(
|
||||
object: &BTreeMap<String, JsonValue>,
|
||||
key: &str,
|
||||
@@ -1168,6 +1245,52 @@ mod tests {
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loads_command_hooks_from_structured_hook_entries() {
|
||||
let root = temp_dir();
|
||||
let cwd = root.join("project");
|
||||
let home = root.join("home").join(".claw");
|
||||
fs::create_dir_all(cwd.join(".claw")).expect("project config dir");
|
||||
fs::create_dir_all(&home).expect("home config dir");
|
||||
|
||||
fs::write(
|
||||
home.join("settings.json"),
|
||||
r#"{
|
||||
"hooks": {
|
||||
"PreToolUse": [
|
||||
{
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "node ~/.claude/hooks/pre-tool-use.mjs"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"PostToolUse": [
|
||||
"./hooks/post-tool-use.sh"
|
||||
]
|
||||
}
|
||||
}"#,
|
||||
)
|
||||
.expect("write user settings");
|
||||
|
||||
let loaded = ConfigLoader::new(&cwd, &home)
|
||||
.load()
|
||||
.expect("config should load");
|
||||
|
||||
assert_eq!(
|
||||
loaded.hooks().pre_tool_use(),
|
||||
&["node ~/.claude/hooks/pre-tool-use.mjs".to_string()]
|
||||
);
|
||||
assert_eq!(
|
||||
loaded.hooks().post_tool_use(),
|
||||
&["./hooks/post-tool-use.sh".to_string()]
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_sandbox_config() {
|
||||
let root = temp_dir();
|
||||
|
||||
@@ -7295,6 +7295,55 @@ UU conflicted.rs",
|
||||
let _ = fs::remove_dir_all(source_root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_runtime_plugin_state_keeps_structured_config_hooks_and_plugin_hooks() {
|
||||
let config_home = temp_dir();
|
||||
let workspace = temp_dir();
|
||||
let source_root = temp_dir();
|
||||
fs::create_dir_all(&config_home).expect("config home");
|
||||
fs::create_dir_all(workspace.join(".claw")).expect("workspace settings dir");
|
||||
fs::create_dir_all(&source_root).expect("source root");
|
||||
fs::write(
|
||||
workspace.join(".claw").join("settings.json"),
|
||||
r#"{
|
||||
"hooks": {
|
||||
"PreToolUse": [
|
||||
{
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "node ~/.claude/hooks/pre-tool-use.mjs"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
}"#,
|
||||
)
|
||||
.expect("write workspace settings");
|
||||
write_plugin_fixture(&source_root, "hook-runtime-demo", true, false);
|
||||
|
||||
let mut manager = PluginManager::new(PluginManagerConfig::new(&config_home));
|
||||
manager
|
||||
.install(source_root.to_str().expect("utf8 source path"))
|
||||
.expect("plugin install should succeed");
|
||||
let loader = ConfigLoader::new(&workspace, &config_home);
|
||||
let runtime_config = loader.load().expect("runtime config should load");
|
||||
let state = build_runtime_plugin_state_with_loader(&workspace, &loader, &runtime_config)
|
||||
.expect("plugin state should load");
|
||||
let pre_hooks = state.feature_config.hooks().pre_tool_use();
|
||||
assert_eq!(pre_hooks.len(), 2);
|
||||
assert_eq!(pre_hooks[0], "node ~/.claude/hooks/pre-tool-use.mjs");
|
||||
assert!(
|
||||
pre_hooks[1].ends_with("hooks/pre.sh"),
|
||||
"expected installed plugin hook path, got {pre_hooks:?}"
|
||||
);
|
||||
|
||||
let _ = fs::remove_dir_all(config_home);
|
||||
let _ = fs::remove_dir_all(workspace);
|
||||
let _ = fs::remove_dir_all(source_root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_runtime_plugin_state_discovers_mcp_tools_and_surfaces_pending_servers() {
|
||||
let config_home = temp_dir();
|
||||
|
||||
Reference in New Issue
Block a user