From 61234892dd0f70c0d357e18138755fe3c200a864 Mon Sep 17 00:00:00 2001 From: Rahul Raj Date: Thu, 25 Jun 2026 20:56:05 +0200 Subject: [PATCH] fix: emit current Codex hook and skills formats --- adapters/codex/src/hooks.rs | 19 +++-- adapters/codex/src/lib.rs | 112 ++++++++++++++++++++++++++-- crates/agentmesh/tests/cli_flows.rs | 2 +- 3 files changed, 119 insertions(+), 14 deletions(-) diff --git a/adapters/codex/src/hooks.rs b/adapters/codex/src/hooks.rs index e7912a2..2b127f9 100644 --- a/adapters/codex/src/hooks.rs +++ b/adapters/codex/src/hooks.rs @@ -18,13 +18,13 @@ pub(crate) fn install_hooks( request.agentmesh_binary_path.display() ); let mut value = read_json_object(&overlay)?; - let post_tool_use = ensure_hook_array(&mut value, &["PostToolUse"])?; + let post_tool_use = ensure_hook_array(&mut value, &["hooks", "PostToolUse"])?; if let Some(index) = find_hook_group(post_tool_use, &command) { return Ok(InstallHooksResponse { hooks_installed: vec![InstalledHook { overlay_file: workspace_relative(&workspace_root, &overlay)?, - entry_path: format!("$.PostToolUse[{index}]"), + entry_path: format!("$.hooks.PostToolUse[{index}]"), command, matcher, }], @@ -48,7 +48,7 @@ pub(crate) fn install_hooks( Ok(InstallHooksResponse { hooks_installed: vec![InstalledHook { overlay_file: workspace_relative(&workspace_root, &overlay)?, - entry_path: format!("$.PostToolUse[{index}]"), + entry_path: format!("$.hooks.PostToolUse[{index}]"), command, matcher, }], @@ -71,7 +71,7 @@ pub(crate) fn remove_hooks( let mut value = read_json_object(&overlay)?; let removed = { - let Some(post_tool_use) = find_hook_array_mut(&mut value, &["PostToolUse"]) else { + let Some(post_tool_use) = find_hook_array_mut(&mut value, &["hooks", "PostToolUse"]) else { return Ok(RemoveHooksResponse { ok: false, removed_count: 0, @@ -82,7 +82,7 @@ pub(crate) fn remove_hooks( let mut removed = remove_recorded_entries( post_tool_use, &request.entry_paths, - "$.PostToolUse", + "$.hooks.PostToolUse", "codex-hook", ); if removed == 0 { @@ -129,6 +129,11 @@ fn codex_hooks_are_empty(value: &JsonValue) -> bool { return false; }; object - .iter() - .all(|(key, value)| key == "PostToolUse" && value.as_array().is_some_and(Vec::is_empty)) + .get("hooks") + .and_then(JsonValue::as_object) + .is_some_and(|hooks| { + hooks.iter().all(|(key, value)| { + key == "PostToolUse" && value.as_array().is_some_and(Vec::is_empty) + }) + }) } diff --git a/adapters/codex/src/lib.rs b/adapters/codex/src/lib.rs index 6df8e05..5e37748 100644 --- a/adapters/codex/src/lib.rs +++ b/adapters/codex/src/lib.rs @@ -436,6 +436,7 @@ fn import_toml_subagent( .iter() .map(|(key, value)| (key.clone(), toml_to_json(value))) .collect::>(); + normalize_imported_codex_skills(&mut frontmatter); let body = frontmatter .get("instructions") .or_else(|| frontmatter.get("prompt")) @@ -517,6 +518,7 @@ fn render_toml_subagent( .iter() .map(|(key, value)| (key.clone(), value.clone())), ); + normalize_codex_skills_for_emit(&mut merged); let body = toml_instructions_body(&document.body); if !document.body.is_empty() @@ -538,6 +540,58 @@ fn render_toml_subagent( Ok(serialize_toml_table(&table)) } +fn normalize_imported_codex_skills(frontmatter: &mut BTreeMap) { + let Some(skills) = frontmatter.get("skills").cloned() else { + return; + }; + let Some(bundled) = extract_current_codex_bundled_skills(&skills) else { + return; + }; + frontmatter.insert("skills".to_string(), JsonValue::Array(bundled)); +} + +fn normalize_codex_skills_for_emit(frontmatter: &mut BTreeMap) { + let Some(skills) = frontmatter.get("skills").cloned() else { + return; + }; + let Some(bundled) = extract_canonical_skills(&skills) else { + return; + }; + frontmatter.insert( + "skills".to_string(), + JsonValue::Object( + [("bundled".to_string(), JsonValue::Array(bundled))] + .into_iter() + .collect(), + ), + ); +} + +fn extract_current_codex_bundled_skills(value: &JsonValue) -> Option> { + let JsonValue::Object(object) = value else { + return None; + }; + object.get("bundled").and_then(extract_canonical_skills) +} + +fn extract_canonical_skills(value: &JsonValue) -> Option> { + match value { + JsonValue::Array(values) => { + let skills = values + .iter() + .filter_map(JsonValue::as_str) + .map(|skill| JsonValue::String(skill.to_string())) + .collect::>(); + if skills.is_empty() { + None + } else { + Some(skills) + } + } + _ => None, + } +} + fn toml_instructions_body(body: &str) -> String { body.strip_suffix('\n').unwrap_or(body).to_string() } @@ -819,7 +873,7 @@ mod tests { ); write( root.join(".codex/agents/code-reviewer.toml"), - "name = \"code-reviewer\"\nmodel = \"gpt-5\"\ninstructions = \"Review code.\"\n", + "name = \"code-reviewer\"\nmodel = \"gpt-5\"\ninstructions = \"Review code.\"\n\n[skills]\nbundled = [\"security-review\"]\n", ); let adapter = CodexAdapter; @@ -841,6 +895,10 @@ mod tests { }; assert_eq!(subagent.frontmatter.get("model"), Some(&json!("gpt-5"))); assert_eq!(subagent.frontmatter.get("instructions"), None); + assert_eq!( + subagent.frontmatter.get("skills"), + Some(&json!(["security-review"])) + ); assert!( subagent .files @@ -903,6 +961,47 @@ mod tests { assert!(content.contains("instructions = \"Review code.\"")); } + #[test] + fn emits_codex_subagent_skills_as_structured_table() { + let temp = match tempfile::tempdir() { + Ok(temp) => temp, + Err(error) => panic!("tempdir should be available: {error}"), + }; + let root = temp.path(); + let adapter = CodexAdapter; + let files = BTreeMap::from([( + PathBuf::from("code-reviewer.md"), + file( + "---\nname: code-reviewer\nmodel: gpt-5\nskills:\n - add-endpoint\n - explore-architecture\n---\nReview code.\n", + ), + )]); + + let response = match adapter.emit(EmitRequest { + runtime_dir: root.join(".codex"), + mode: RuntimeMode::Managed, + entities: vec![EmitEntity { + id: "subagent:code-reviewer".to_string(), + entity_type: agentmesh_protocol::EntityType::Subagent, + scope: None, + files, + frontmatter: BTreeMap::new(), + overrides: BTreeMap::new(), + }], + }) { + Ok(response) => response, + Err(error) => panic!("emit should succeed: {error}"), + }; + + assert_eq!( + response.files_written, + vec![PathBuf::from(".codex/agents/code-reviewer.toml")] + ); + let content = read(root.join(".codex/agents/code-reviewer.toml")); + assert!(content.contains("[skills]")); + assert!(content.contains("bundled = [\"add-endpoint\", \"explore-architecture\"]")); + assert!(!content.contains("skills = \"")); + } + #[test] fn emits_codex_skill_assets() { let temp = match tempfile::tempdir() { @@ -969,10 +1068,11 @@ mod tests { Err(error) => panic!("install should succeed: {error}"), }; - assert_eq!(installed.hooks_installed[0].entry_path, "$.PostToolUse[0]"); + assert_eq!(installed.hooks_installed[0].entry_path, "$.hooks.PostToolUse[0]"); let overlay = read(root.join(".codex/hooks.json")); assert!(overlay.contains("codex-hook")); assert!(overlay.contains("AgentMesh sync")); + assert!(overlay.contains("\"hooks\"")); let removed = match adapter.remove_hooks(RemoveHooksRequest { runtime_dir: root.join(".codex"), @@ -994,7 +1094,7 @@ mod tests { let root = temp.path(); write( root.join(".codex/hooks.json"), - r#"{"PostToolUse":[{"matcher":"^Bash$","hooks":[{"type":"command","command":"echo user"}]}]}"#, + r#"{"hooks":{"PostToolUse":[{"matcher":"^Bash$","hooks":[{"type":"command","command":"echo user"}]}]}}"#, ); let adapter = CodexAdapter; @@ -1006,7 +1106,7 @@ mod tests { Ok(installed) => installed, Err(error) => panic!("install should succeed: {error}"), }; - assert_eq!(installed.hooks_installed[0].entry_path, "$.PostToolUse[1]"); + assert_eq!(installed.hooks_installed[0].entry_path, "$.hooks.PostToolUse[1]"); let removed = match adapter.remove_hooks(RemoveHooksRequest { runtime_dir: root.join(".codex"), @@ -1164,8 +1264,8 @@ severity = ["high", "medium"] let overlay = read(root.join(".codex/hooks.json")); let hook_count = overlay.matches("codex-hook").count(); - assert_eq!(first.hooks_installed[0].entry_path, "$.PostToolUse[0]"); - assert_eq!(second.hooks_installed[0].entry_path, "$.PostToolUse[0]"); + assert_eq!(first.hooks_installed[0].entry_path, "$.hooks.PostToolUse[0]"); + assert_eq!(second.hooks_installed[0].entry_path, "$.hooks.PostToolUse[0]"); assert_eq!(hook_count, 1); } diff --git a/crates/agentmesh/tests/cli_flows.rs b/crates/agentmesh/tests/cli_flows.rs index 734d382..08eb7b2 100644 --- a/crates/agentmesh/tests/cli_flows.rs +++ b/crates/agentmesh/tests/cli_flows.rs @@ -815,7 +815,7 @@ fn install_stop_and_start_are_machine_local_and_surgical() { ); write( repo.join(".codex/hooks.json"), - r#"{"PostToolUse":[{"matcher":"^Bash$","hooks":[{"type":"command","command":"echo user"}]}]}"#, + r#"{"hooks":{"PostToolUse":[{"matcher":"^Bash$","hooks":[{"type":"command","command":"echo user"}]}]}}"#, ); let original_lockfile = read(repo.join("agentmesh.lock"));