Skip to content

Commit 6632510

Browse files
Tighten Codex plugin hook migration
1 parent c4383e3 commit 6632510

3 files changed

Lines changed: 56 additions & 40 deletions

File tree

src/agents/codex.rs

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,8 @@ impl AgentIntegration for CodexIntegration {
125125
return Ok(UpdatePluginOutcome::Refreshed(refreshed));
126126
}
127127

128-
let target = if codex_plugin_manifest_path(&ctx.home).exists()
129-
|| codex_legacy_config_has_tracedecay(&ctx.home)
130-
{
128+
let legacy_config_install = codex_legacy_config_has_tracedecay(&ctx.home);
129+
let target = if codex_plugin_manifest_path(&ctx.home).exists() || legacy_config_install {
131130
Some(plugin_dir.clone())
132131
} else {
133132
None
@@ -137,6 +136,9 @@ impl AgentIntegration for CodexIntegration {
137136
return Ok(UpdatePluginOutcome::NotInstalled);
138137
};
139138
install_codex_personal_bootstrap(&ctx.home, &ctx.tracedecay_bin)?;
139+
if legacy_config_install {
140+
sweep_legacy_global_codex_config(&ctx.home);
141+
}
140142
Ok(UpdatePluginOutcome::Refreshed(vec![target]))
141143
}
142144

@@ -193,11 +195,7 @@ impl AgentIntegration for CodexIntegration {
193195
eprintln!("\n\x1b[1mCodex CLI integration\x1b[0m");
194196
let local_plugin_dir = codex_repo_plugin_install_dir(&ctx.project_path);
195197
if local_plugin_dir.join(".codex-plugin/plugin.json").exists() {
196-
doctor_check_plugin_dir(
197-
dc,
198-
&local_plugin_dir,
199-
Some(&ctx.home.join(".codex/config.toml")),
200-
);
198+
doctor_check_plugin_dir(dc, &local_plugin_dir, None);
201199
doctor_check_marketplace_entry(
202200
dc,
203201
&codex_repo_marketplace_path(&ctx.project_path),
@@ -483,8 +481,9 @@ fn write_codex_plugin_files(
483481
) -> Result<()> {
484482
for (relative, contents) in codex_embedded_plugin_files() {
485483
let rendered = match relative {
486-
".codex-plugin/plugin.json" => codex_plugin_manifest(contents)?,
484+
".codex-plugin/plugin.json" => codex_plugin_manifest(contents, scope)?,
487485
".mcp.json" => codex_plugin_mcp(contents, tracedecay_bin, scope)?,
486+
"hooks/hooks.json" if scope == InstallScope::ProjectLocal => continue,
488487
"hooks/hooks.json" => codex_plugin_hooks(contents, tracedecay_bin)?,
489488
_ => contents.to_string(),
490489
};
@@ -493,8 +492,17 @@ fn write_codex_plugin_files(
493492
Ok(())
494493
}
495494

496-
fn codex_plugin_manifest(raw: &str) -> Result<String> {
497-
super::plugin_bundle::stamp_manifest_version(raw)
495+
fn codex_plugin_manifest(raw: &str, scope: InstallScope) -> Result<String> {
496+
let stamped = super::plugin_bundle::stamp_manifest_version(raw)?;
497+
if scope != InstallScope::ProjectLocal {
498+
return Ok(stamped);
499+
}
500+
501+
let mut manifest: serde_json::Value = serde_json::from_str(&stamped)?;
502+
if let Some(object) = manifest.as_object_mut() {
503+
object.remove("hooks");
504+
}
505+
Ok(format!("{}\n", serde_json::to_string_pretty(&manifest)?))
498506
}
499507

500508
fn codex_plugin_mcp(raw: &str, tracedecay_bin: &str, scope: InstallScope) -> Result<String> {
@@ -1311,7 +1319,9 @@ fn doctor_check_plugin_dir(dc: &mut DoctorCounters, plugin_dir: &Path, config_pa
13111319
mcp_path.display()
13121320
));
13131321
}
1314-
doctor_check_hooks(dc, &plugin_dir.join("hooks/hooks.json"), config_path);
1322+
if let Some(config_path) = config_path {
1323+
doctor_check_hooks(dc, &plugin_dir.join("hooks/hooks.json"), Some(config_path));
1324+
}
13151325
}
13161326

13171327
/// Check hooks.json registers the tracedecay lifecycle hooks, and report Codex

tests/agent_suite/agent_test.rs

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -389,22 +389,29 @@ fn assert_codex_plugin_bundle(
389389
plugin_dir: &Path,
390390
expected_command: &str,
391391
expected_args: serde_json::Value,
392-
expected_global_env: bool,
392+
expected_global_bundle: bool,
393393
) {
394394
let manifest = read_json(&plugin_dir.join(".codex-plugin/plugin.json"));
395395
assert_eq!(manifest["name"], "tracedecay");
396396
assert_eq!(manifest["version"], env!("CARGO_PKG_VERSION"));
397397
assert_eq!(manifest["license"], "MIT");
398398
assert_eq!(manifest["skills"], "./skills/");
399399
assert_eq!(manifest["mcpServers"], "./.mcp.json");
400-
assert_eq!(manifest["hooks"], "./hooks/hooks.json");
400+
if expected_global_bundle {
401+
assert_eq!(manifest["hooks"], "./hooks/hooks.json");
402+
} else {
403+
assert!(
404+
manifest.get("hooks").is_none(),
405+
"repo-local Codex plugin should not declare lifecycle hooks"
406+
);
407+
}
401408

402409
let mcp = read_json(&plugin_dir.join(".mcp.json"));
403410
let server = &mcp["mcpServers"]["tracedecay"];
404411
assert_eq!(server["type"], "stdio");
405412
assert_eq!(server["command"], expected_command);
406413
assert_eq!(server["args"], expected_args);
407-
if expected_global_env {
414+
if expected_global_bundle {
408415
assert_eq!(server["env"]["TRACEDECAY_ENABLE_GLOBAL_DB"], "1");
409416
} else {
410417
assert!(
@@ -413,14 +420,22 @@ fn assert_codex_plugin_bundle(
413420
);
414421
}
415422

416-
let hooks = read_json(&plugin_dir.join("hooks/hooks.json"));
417-
assert_codex_hooks_registered(&hooks);
418-
assert_command_contains_expected_bin(
419-
&hooks,
420-
"SessionStart",
421-
"hook-codex-session-start",
422-
expected_command,
423-
);
423+
let hooks_path = plugin_dir.join("hooks/hooks.json");
424+
if expected_global_bundle {
425+
let hooks = read_json(&hooks_path);
426+
assert_codex_hooks_registered(&hooks);
427+
assert_command_contains_expected_bin(
428+
&hooks,
429+
"SessionStart",
430+
"hook-codex-session-start",
431+
expected_command,
432+
);
433+
} else {
434+
assert!(
435+
!hooks_path.exists(),
436+
"repo-local Codex plugin should not ship lifecycle hooks"
437+
);
438+
}
424439

425440
let skill = std::fs::read_to_string(plugin_dir.join("skills/exploring-code/SKILL.md"))
426441
.expect("Codex plugin should ship tracedecay steering skills");
@@ -3463,7 +3478,7 @@ fn test_codex_local_install_creates_repo_plugin_bundle_and_marketplace() {
34633478
);
34643479
assert!(
34653480
!project.path().join(".codex/hooks.json").exists(),
3466-
"local Codex install should bundle hooks in the repo plugin"
3481+
"local Codex install should not write project Codex hooks"
34673482
);
34683483
assert!(
34693484
!project.path().join("AGENTS.md").exists(),
@@ -3628,22 +3643,17 @@ fn test_codex_global_install_bundles_hooks_in_plugin() {
36283643
}
36293644

36303645
#[test]
3631-
fn test_codex_local_install_bundles_hooks_in_plugin() {
3646+
fn test_codex_local_install_does_not_bundle_hooks() {
36323647
let home = TempDir::new().unwrap();
36333648
let project = TempDir::new().unwrap();
36343649

36353650
assert_local_install_success("codex", project.path(), home.path());
36363651

36373652
let hooks_path = codex_project_plugin_install_dir(project.path()).join("hooks/hooks.json");
36383653
assert!(
3639-
hooks_path.exists(),
3640-
"local Codex install should bundle hooks in the repo plugin"
3654+
!hooks_path.exists(),
3655+
"local Codex install should not bundle project-local hooks"
36413656
);
3642-
let hooks = read_json(&hooks_path);
3643-
assert_codex_hooks_registered(&hooks);
3644-
// Local install must use the resolved absolute tracedecay binary path.
3645-
assert_command_contains_bin(&hooks, "SessionStart", "hook-codex-session-start");
3646-
36473657
assert!(
36483658
!home.path().join(".codex/hooks.json").exists(),
36493659
"local install must not write the global Codex hooks config"
@@ -3654,11 +3664,6 @@ fn test_codex_local_install_bundles_hooks_in_plugin() {
36543664
);
36553665
}
36563666

3657-
fn assert_command_contains_bin(hooks: &serde_json::Value, event: &str, needle: &str) {
3658-
let expected = expected_tracedecay_bin();
3659-
assert_command_contains_expected_bin(hooks, event, needle, &expected);
3660-
}
3661-
36623667
fn assert_command_contains_expected_bin(
36633668
hooks: &serde_json::Value,
36643669
event: &str,

tests/agent_suite/update_plugin_test.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -599,8 +599,6 @@ fn codex_update_plugin_migrates_legacy_config_only_install_to_plugin() {
599599
"[mcp_servers.tracedecay]\ncommand = \"/old/bin/tracedecay\"\nargs = [\"serve\"]\n",
600600
)
601601
.unwrap();
602-
let before = bytes(&codex_dir.join("config.toml"));
603-
604602
let codex = get_integration("codex").unwrap();
605603
let outcome = codex
606604
.update_plugin(&ctx_with_project(home.path(), NEW_BIN, &project_root))
@@ -609,7 +607,10 @@ fn codex_update_plugin_migrates_legacy_config_only_install_to_plugin() {
609607
panic!("expected codex update_plugin to migrate legacy config to plugin");
610608
};
611609
assert_eq!(paths, vec![home.path().join("plugins/tracedecay")]);
612-
assert_eq!(bytes(&codex_dir.join("config.toml")), before);
610+
assert!(
611+
!codex_dir.join("config.toml").exists(),
612+
"Codex update-plugin should remove the migrated legacy config-managed install"
613+
);
613614
assert_codex_bundle_contains_bin(&home.path().join("plugins/tracedecay"), NEW_BIN);
614615
assert_codex_marketplace_entry(&codex_marketplace_path(home.path()), "./plugins/tracedecay");
615616
}

0 commit comments

Comments
 (0)