Skip to content

Commit a94fe26

Browse files
Finish Codex hook migration cleanup
1 parent 8db1a6c commit a94fe26

2 files changed

Lines changed: 91 additions & 21 deletions

File tree

src/agents/codex.rs

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,12 @@ impl AgentIntegration for CodexIntegration {
5757
for path in [
5858
codex_repo_plugin_install_dir(project_path).join(".codex-plugin/plugin.json"),
5959
codex_repo_plugin_install_dir(project_path).join(".mcp.json"),
60-
codex_repo_plugin_install_dir(project_path).join("hooks/hooks.json"),
6160
codex_repo_marketplace_path(project_path),
6261
] {
6362
super::ensure_project_local_safe_path(project_path, &path)?;
6463
}
6564
install_codex_repo_plugin(&ctx.home, project_path, &ctx.tracedecay_bin)?;
6665
sweep_legacy_project_codex_config(project_path);
67-
print_hook_trust_guidance();
6866
Ok(())
6967
}
7068

@@ -90,6 +88,7 @@ impl AgentIntegration for CodexIntegration {
9088
fn update_plugin(&self, ctx: &InstallContext) -> Result<UpdatePluginOutcome> {
9189
let cached_dirs = codex_plugin_cached_install_dirs(&ctx.home);
9290
let plugin_dir = codex_plugin_install_dir(&ctx.home);
91+
let legacy_config_install = codex_legacy_config_has_tracedecay(&ctx.home);
9392
let mut refreshed = Vec::new();
9493
if !cached_dirs.is_empty() {
9594
let target = install_codex_cached_plugin(&ctx.home, &ctx.tracedecay_bin)?;
@@ -122,10 +121,12 @@ impl AgentIntegration for CodexIntegration {
122121
}
123122

124123
if !refreshed.is_empty() {
124+
if legacy_config_install {
125+
sweep_legacy_global_codex_config(&ctx.home);
126+
}
125127
return Ok(UpdatePluginOutcome::Refreshed(refreshed));
126128
}
127129

128-
let legacy_config_install = codex_legacy_config_has_tracedecay(&ctx.home);
129130
let target = if codex_plugin_manifest_path(&ctx.home).exists() || legacy_config_install {
130131
Some(plugin_dir.clone())
131132
} else {
@@ -573,6 +574,7 @@ const CODEX_MANAGED_HOOKS: &[CodexManagedHook] = &[
573574
/// Subcommands from older bundles that uninstall must also strip even though
574575
/// the current bundle no longer registers them.
575576
const CODEX_LEGACY_HOOK_SUBCOMMANDS: &[&str] = &["hook-codex-pre-tool-use"];
577+
const CODEX_PERSONAL_PLUGIN_HOOK_TRUST_PREFIX: &str = "tracedecay@personal:hooks/hooks.json:";
576578

577579
#[derive(Debug, PartialEq, Eq)]
578580
enum CodexHookTrustState {
@@ -609,14 +611,12 @@ fn codex_plugin_hook_trust_state(config: &toml::Value) -> CodexHookTrustState {
609611
.iter()
610612
.map(|hook| codex_hook_state_event_key(hook.event))
611613
.filter(|event_key| {
612-
let suffix = format!(":hooks/hooks.json:{event_key}:0:0");
613-
!state.iter().any(|(key, entry)| {
614-
key.starts_with("tracedecay@")
615-
&& key.ends_with(&suffix)
616-
&& entry
617-
.get("trusted_hash")
618-
.and_then(|hash| hash.as_str())
619-
.is_some_and(|hash| hash.starts_with("sha256:"))
614+
let trust_key = format!("{CODEX_PERSONAL_PLUGIN_HOOK_TRUST_PREFIX}{event_key}:0:0");
615+
!state.get(&trust_key).is_some_and(|entry| {
616+
entry
617+
.get("trusted_hash")
618+
.and_then(|hash| hash.as_str())
619+
.is_some_and(|hash| hash.starts_with("sha256:"))
620620
})
621621
})
622622
.collect();
@@ -1365,7 +1365,7 @@ fn doctor_check_hooks(dc: &mut DoctorCounters, hooks_path: &Path, config_path: O
13651365
}
13661366
} else {
13671367
dc.warn(&format!(
1368-
"tracedecay hook(s) missing for {} in {} — run `tracedecay install --local --agent codex` or `tracedecay install --agent codex`",
1368+
"tracedecay hook(s) missing for {} in {} — run `tracedecay install --agent codex`",
13691369
missing.join(", "),
13701370
hooks_path.display(),
13711371
));
@@ -1509,6 +1509,42 @@ trusted_hash = "sha256:post"
15091509
);
15101510
}
15111511

1512+
#[test]
1513+
fn codex_hook_trust_state_ignores_repo_local_plugin_entries() {
1514+
let config = toml::from_str::<toml::Value>(
1515+
r#"
1516+
[hooks.state]
1517+
1518+
[hooks.state."tracedecay@local-repo:hooks/hooks.json:post_tool_use:0:0"]
1519+
trusted_hash = "sha256:post"
1520+
1521+
[hooks.state."tracedecay@local-repo:hooks/hooks.json:session_start:0:0"]
1522+
trusted_hash = "sha256:session"
1523+
1524+
[hooks.state."tracedecay@local-repo:hooks/hooks.json:user_prompt_submit:0:0"]
1525+
trusted_hash = "sha256:prompt"
1526+
1527+
[hooks.state."tracedecay@local-repo:hooks/hooks.json:subagent_start:0:0"]
1528+
trusted_hash = "sha256:subagent"
1529+
1530+
[hooks.state."tracedecay@local-repo:hooks/hooks.json:post_compact:0:0"]
1531+
trusted_hash = "sha256:compact"
1532+
"#,
1533+
)
1534+
.unwrap();
1535+
1536+
assert_eq!(
1537+
codex_plugin_hook_trust_state(&config),
1538+
CodexHookTrustState::Missing(vec![
1539+
"session_start".to_string(),
1540+
"user_prompt_submit".to_string(),
1541+
"subagent_start".to_string(),
1542+
"post_tool_use".to_string(),
1543+
"post_compact".to_string(),
1544+
])
1545+
);
1546+
}
1547+
15121548
#[test]
15131549
fn remove_legacy_codex_native_automation_deletes_stale_record() {
15141550
let home = tempfile::tempdir().expect("tempdir should create");

tests/agent_suite/update_plugin_test.rs

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ fn assert_codex_marketplace_entry(marketplace_path: &Path, source_path: &str) {
7272

7373
fn assert_codex_bundle_contains_bin(plugin_dir: &Path, tracedecay_bin: &str) {
7474
assert!(text(&plugin_dir.join(".mcp.json")).contains(tracedecay_bin));
75-
assert!(text(&plugin_dir.join("hooks/hooks.json")).contains(tracedecay_bin));
75+
let hooks_path = plugin_dir.join("hooks/hooks.json");
76+
if hooks_path.exists() {
77+
assert!(text(&hooks_path).contains(tracedecay_bin));
78+
}
7679
}
7780

7881
fn codex_bootstrap_dir(home: &Path) -> PathBuf {
@@ -97,6 +100,18 @@ fn write_codex_plugin_manifest(plugin_dir: &Path, version: &str) {
97100
.unwrap();
98101
}
99102

103+
fn write_codex_legacy_config(home: &Path) -> PathBuf {
104+
let codex_dir = home.join(".codex");
105+
std::fs::create_dir_all(&codex_dir).unwrap();
106+
let config_path = codex_dir.join("config.toml");
107+
std::fs::write(
108+
&config_path,
109+
"[mcp_servers.tracedecay]\ncommand = \"/old/bin/tracedecay\"\nargs = [\"serve\"]\n",
110+
)
111+
.unwrap();
112+
config_path
113+
}
114+
100115
fn write_stale_codex_skill(plugin_dir: &Path) {
101116
std::fs::create_dir_all(plugin_dir.join("skills/stale-skill")).unwrap();
102117
std::fs::write(
@@ -489,6 +504,31 @@ fn codex_update_plugin_recreates_bootstrap_source_from_cache_only_state() {
489504
assert_codex_marketplace_entry(&codex_marketplace_path(home.path()), "./plugins/tracedecay");
490505
}
491506

507+
#[test]
508+
fn codex_update_plugin_sweeps_legacy_config_when_cache_exists() {
509+
let home = TempDir::new().unwrap();
510+
let project_root = home.path().join("workspace");
511+
let cached_plugin_dir = codex_cached_plugin_dir(home.path());
512+
let legacy_config = write_codex_legacy_config(home.path());
513+
write_codex_plugin_manifest(&cached_plugin_dir, "0.0.0");
514+
515+
let codex = get_integration("codex").unwrap();
516+
let outcome = codex
517+
.update_plugin(&ctx_with_project(home.path(), NEW_BIN, &project_root))
518+
.unwrap();
519+
let UpdatePluginOutcome::Refreshed(paths) = outcome else {
520+
panic!("expected codex update_plugin to refresh the installed cache");
521+
};
522+
assert_eq!(
523+
paths,
524+
vec![cached_plugin_dir.clone(), codex_bootstrap_dir(home.path())]
525+
);
526+
assert!(
527+
!legacy_config.exists(),
528+
"Codex update-plugin should remove legacy config even when a plugin cache exists"
529+
);
530+
}
531+
492532
#[test]
493533
fn codex_update_plugin_refreshes_global_cache_and_repo_local_bundle() {
494534
let home = TempDir::new().unwrap();
@@ -592,13 +632,7 @@ fn codex_uninstall_removes_repo_local_bundle_from_project_root() {
592632
fn codex_update_plugin_migrates_legacy_config_only_install_to_plugin() {
593633
let home = TempDir::new().unwrap();
594634
let project_root = home.path().join("workspace");
595-
let codex_dir = home.path().join(".codex");
596-
std::fs::create_dir_all(&codex_dir).unwrap();
597-
std::fs::write(
598-
codex_dir.join("config.toml"),
599-
"[mcp_servers.tracedecay]\ncommand = \"/old/bin/tracedecay\"\nargs = [\"serve\"]\n",
600-
)
601-
.unwrap();
635+
let legacy_config = write_codex_legacy_config(home.path());
602636
let codex = get_integration("codex").unwrap();
603637
let outcome = codex
604638
.update_plugin(&ctx_with_project(home.path(), NEW_BIN, &project_root))
@@ -608,7 +642,7 @@ fn codex_update_plugin_migrates_legacy_config_only_install_to_plugin() {
608642
};
609643
assert_eq!(paths, vec![home.path().join("plugins/tracedecay")]);
610644
assert!(
611-
!codex_dir.join("config.toml").exists(),
645+
!legacy_config.exists(),
612646
"Codex update-plugin should remove the migrated legacy config-managed install"
613647
);
614648
assert_codex_bundle_contains_bin(&home.path().join("plugins/tracedecay"), NEW_BIN);

0 commit comments

Comments
 (0)