Skip to content

Commit dfe94bd

Browse files
committed
Fix hook run analytics source classification
1 parent a453c15 commit dfe94bd

8 files changed

Lines changed: 104 additions & 75 deletions

File tree

codex-rs/Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codex-rs/analytics/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ codex-git-utils = { workspace = true }
1818
codex-login = { workspace = true }
1919
codex-plugin = { workspace = true }
2020
codex-protocol = { workspace = true }
21-
dirs = { workspace = true }
2221
os_info = { workspace = true }
2322
serde = { workspace = true, features = ["derive"] }
2423
serde_json = { workspace = true }

codex-rs/analytics/src/analytics_client_tests.rs

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use crate::facts::AppInvocation;
2323
use crate::facts::AppMentionedInput;
2424
use crate::facts::AppUsedInput;
2525
use crate::facts::CodexCompactionEvent;
26+
use crate::facts::CodexHookSource;
2627
use crate::facts::CompactionImplementation;
2728
use crate::facts::CompactionPhase;
2829
use crate::facts::CompactionReason;
@@ -1290,8 +1291,6 @@ fn plugin_management_event_serializes_expected_shape() {
12901291

12911292
#[test]
12921293
fn hook_run_event_serializes_expected_shape() {
1293-
let home = dirs::home_dir().expect("home dir should be available for analytics tests");
1294-
12951294
let tracking = TrackEventsContext {
12961295
model_slug: "gpt-5".to_string(),
12971296
thread_id: "thread-3".to_string(),
@@ -1303,10 +1302,8 @@ fn hook_run_event_serializes_expected_shape() {
13031302
&tracking,
13041303
HookRunFact {
13051304
event_name: HookEventName::PreToolUse,
1306-
source_path: PathBuf::from(&home).join(".codex/hooks.json"),
1307-
cwd: PathBuf::from(&home).join("worktree"),
1305+
hook_source: CodexHookSource::User,
13081306
status: HookRunStatus::Completed,
1309-
duration_ms: Some(42),
13101307
},
13111308
),
13121309
});
@@ -1331,7 +1328,6 @@ fn hook_run_event_serializes_expected_shape() {
13311328

13321329
#[test]
13331330
fn hook_run_metadata_maps_sources_and_statuses() {
1334-
let home = dirs::home_dir().expect("home dir should be available for analytics tests");
13351331
let tracking = TrackEventsContext {
13361332
model_slug: "gpt-5".to_string(),
13371333
thread_id: "thread-1".to_string(),
@@ -1342,32 +1338,26 @@ fn hook_run_metadata_maps_sources_and_statuses() {
13421338
&tracking,
13431339
HookRunFact {
13441340
event_name: HookEventName::SessionStart,
1345-
source_path: PathBuf::from("/etc/codex/hooks.json"),
1346-
cwd: PathBuf::from(&home).join("worktree"),
1341+
hook_source: CodexHookSource::System,
13471342
status: HookRunStatus::Completed,
1348-
duration_ms: None,
13491343
},
13501344
))
13511345
.expect("serialize system hook");
13521346
let project = serde_json::to_value(codex_hook_run_metadata(
13531347
&tracking,
13541348
HookRunFact {
13551349
event_name: HookEventName::Stop,
1356-
source_path: PathBuf::from(&home).join("worktree/.codex/hooks.json"),
1357-
cwd: PathBuf::from(&home).join("worktree/src"),
1350+
hook_source: CodexHookSource::Project,
13581351
status: HookRunStatus::Blocked,
1359-
duration_ms: None,
13601352
},
13611353
))
13621354
.expect("serialize project hook");
13631355
let unknown = serde_json::to_value(codex_hook_run_metadata(
13641356
&tracking,
13651357
HookRunFact {
13661358
event_name: HookEventName::UserPromptSubmit,
1367-
source_path: PathBuf::from("/tmp/hooks.json"),
1368-
cwd: PathBuf::from(&home).join("worktree"),
1359+
hook_source: CodexHookSource::Unknown,
13691360
status: HookRunStatus::Failed,
1370-
duration_ms: None,
13711361
},
13721362
))
13731363
.expect("serialize unknown hook");
@@ -1382,7 +1372,6 @@ fn hook_run_metadata_maps_sources_and_statuses() {
13821372

13831373
#[test]
13841374
fn hook_run_metadata_maps_stopped_status() {
1385-
let home = dirs::home_dir().expect("home dir should be available for analytics tests");
13861375
let tracking = TrackEventsContext {
13871376
model_slug: "gpt-5".to_string(),
13881377
thread_id: "thread-1".to_string(),
@@ -1393,10 +1382,8 @@ fn hook_run_metadata_maps_stopped_status() {
13931382
&tracking,
13941383
HookRunFact {
13951384
event_name: HookEventName::Stop,
1396-
source_path: home.join(".codex/hooks.json"),
1397-
cwd: home.join("worktree"),
1385+
hook_source: CodexHookSource::User,
13981386
status: HookRunStatus::Stopped,
1399-
duration_ms: None,
14001387
},
14011388
))
14021389
.expect("serialize stopped hook");
@@ -1497,10 +1484,8 @@ async fn reducer_ingests_hook_run_fact() {
14971484
},
14981485
hook: HookRunFact {
14991486
event_name: HookEventName::PostToolUse,
1500-
source_path: PathBuf::from("/tmp/hooks.json"),
1501-
cwd: PathBuf::from("/tmp/project"),
1487+
hook_source: CodexHookSource::Unknown,
15021488
status: HookRunStatus::Failed,
1503-
duration_ms: Some(7),
15041489
},
15051490
})),
15061491
&mut events,

codex-rs/analytics/src/events.rs

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::facts::AppInvocation;
22
use crate::facts::CodexCompactionEvent;
3+
use crate::facts::CodexHookSource;
34
use crate::facts::HookRunFact;
45
use crate::facts::InvocationType;
56
use crate::facts::PluginState;
@@ -20,7 +21,6 @@ use codex_protocol::protocol::HookEventName;
2021
use codex_protocol::protocol::HookRunStatus;
2122
use codex_protocol::protocol::SubAgentSource;
2223
use serde::Serialize;
23-
use std::path::Path;
2424

2525
#[derive(Clone, Copy, Debug, Serialize)]
2626
#[serde(rename_all = "snake_case")]
@@ -305,15 +305,6 @@ pub(crate) struct CodexAppUsedEventRequest {
305305
pub(crate) event_params: CodexAppMetadata,
306306
}
307307

308-
#[derive(Clone, Copy, Debug, Serialize)]
309-
#[serde(rename_all = "snake_case")]
310-
pub(crate) enum CodexHookSource {
311-
System,
312-
User,
313-
Project,
314-
Unknown,
315-
}
316-
317308
#[derive(Serialize)]
318309
pub(crate) struct CodexHookRunMetadata {
319310
pub(crate) thread_id: Option<String>,
@@ -568,10 +559,7 @@ pub(crate) fn codex_hook_run_metadata(
568559
turn_id: Some(tracking.turn_id.clone()),
569560
model_slug: Some(tracking.model_slug.clone()),
570561
hook_name: Some(hook.event_name),
571-
hook_source: Some(hook_source_for_path(
572-
hook.source_path.as_path(),
573-
hook.cwd.as_path(),
574-
)),
562+
hook_source: Some(hook.hook_source),
575563
status: Some(analytics_hook_status(hook.status)),
576564
}
577565
}
@@ -634,31 +622,6 @@ pub(crate) fn subagent_parent_thread_id(subagent_source: &SubAgentSource) -> Opt
634622
}
635623
}
636624

637-
fn hook_source_for_path(source_path: &Path, cwd: &Path) -> CodexHookSource {
638-
if source_path.starts_with("/etc/codex") {
639-
return CodexHookSource::System;
640-
}
641-
642-
let home = dirs::home_dir();
643-
if let Some(home) = home
644-
&& source_path.starts_with(home.join(".codex"))
645-
{
646-
return CodexHookSource::User;
647-
}
648-
649-
// Project hooks are loaded from a `.codex/hooks.json` rooted at or above the
650-
// current working directory, so classify by walking cwd ancestors.
651-
if source_path.ends_with(".codex/hooks.json")
652-
&& cwd
653-
.ancestors()
654-
.any(|ancestor| source_path.starts_with(ancestor.join(".codex")))
655-
{
656-
return CodexHookSource::Project;
657-
}
658-
659-
CodexHookSource::Unknown
660-
}
661-
662625
fn analytics_hook_status(status: HookRunStatus) -> HookRunStatus {
663626
match status {
664627
// Running is unexpected here and normalized defensively.

codex-rs/analytics/src/facts.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,15 @@ pub enum CompactionStatus {
243243
Interrupted,
244244
}
245245

246+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)]
247+
#[serde(rename_all = "snake_case")]
248+
pub enum CodexHookSource {
249+
System,
250+
User,
251+
Project,
252+
Unknown,
253+
}
254+
246255
#[derive(Clone)]
247256
pub struct CodexCompactionEvent {
248257
pub thread_id: String,
@@ -327,10 +336,8 @@ pub(crate) struct HookRunInput {
327336

328337
pub struct HookRunFact {
329338
pub event_name: HookEventName,
330-
pub source_path: PathBuf,
331-
pub cwd: PathBuf,
339+
pub hook_source: CodexHookSource,
332340
pub status: HookRunStatus,
333-
pub duration_ms: Option<i64>,
334341
}
335342

336343
pub(crate) struct PluginUsedInput {

codex-rs/analytics/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ pub use events::GuardianReviewedAction;
2222
pub use facts::AnalyticsJsonRpcError;
2323
pub use facts::AppInvocation;
2424
pub use facts::CodexCompactionEvent;
25+
pub use facts::CodexHookSource;
2526
pub use facts::CodexTurnSteerEvent;
2627
pub use facts::CompactionImplementation;
2728
pub use facts::CompactionPhase;

codex-rs/core/src/config_loader/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,12 +411,12 @@ fn system_requirements_toml_file() -> io::Result<AbsolutePathBuf> {
411411
}
412412

413413
#[cfg(unix)]
414-
fn system_config_toml_file() -> io::Result<AbsolutePathBuf> {
414+
pub(crate) fn system_config_toml_file() -> io::Result<AbsolutePathBuf> {
415415
AbsolutePathBuf::from_absolute_path(Path::new(SYSTEM_CONFIG_TOML_FILE_UNIX))
416416
}
417417

418418
#[cfg(windows)]
419-
fn system_config_toml_file() -> io::Result<AbsolutePathBuf> {
419+
pub(crate) fn system_config_toml_file() -> io::Result<AbsolutePathBuf> {
420420
windows_system_config_toml_file()
421421
}
422422

codex-rs/core/src/hook_runtime.rs

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::future::Future;
2+
use std::path::Path;
23
use std::sync::Arc;
34

5+
use codex_analytics::CodexHookSource;
46
use codex_analytics::HookRunFact;
57
use codex_analytics::build_track_events_context;
68
use codex_hooks::PostToolUseOutcome;
@@ -24,6 +26,7 @@ use serde_json::Value;
2426

2527
use crate::codex::Session;
2628
use crate::codex::TurnContext;
29+
use crate::config_loader::system_config_toml_file;
2730
use crate::event_mapping::parse_turn_item;
2831

2932
pub(crate) struct HookRuntimeOutcome {
@@ -358,14 +361,41 @@ fn hook_run_analytics_payload(
358361
),
359362
HookRunFact {
360363
event_name: completed.run.event_name,
361-
source_path: completed.run.source_path.to_path_buf(),
362-
cwd: turn_context.cwd.to_path_buf(),
364+
hook_source: hook_source_for_path(
365+
completed.run.source_path.as_path(),
366+
turn_context.config.codex_home.as_path(),
367+
turn_context.cwd.as_path(),
368+
),
363369
status: completed.run.status,
364-
duration_ms: completed.run.duration_ms,
365370
},
366371
)
367372
}
368373

374+
fn hook_source_for_path(source_path: &Path, codex_home: &Path, cwd: &Path) -> CodexHookSource {
375+
if let Ok(system_config_path) = system_config_toml_file()
376+
&& let Some(system_config_dir) = system_config_path.as_path().parent()
377+
&& source_path.starts_with(system_config_dir)
378+
{
379+
return CodexHookSource::System;
380+
}
381+
382+
if source_path.starts_with(codex_home) {
383+
return CodexHookSource::User;
384+
}
385+
386+
// Project hooks are loaded from a `.codex/hooks.json` rooted at or above the
387+
// current working directory, so classify by walking cwd ancestors.
388+
if source_path.ends_with(Path::new(".codex").join("hooks.json"))
389+
&& cwd
390+
.ancestors()
391+
.any(|ancestor| source_path.starts_with(ancestor.join(".codex")))
392+
{
393+
return CodexHookSource::Project;
394+
}
395+
396+
CodexHookSource::Unknown
397+
}
398+
369399
fn hook_permission_mode(turn_context: &TurnContext) -> String {
370400
match turn_context.approval_policy.value() {
371401
AskForApproval::Never => "bypassPermissions",
@@ -379,14 +409,14 @@ fn hook_permission_mode(turn_context: &TurnContext) -> String {
379409

380410
#[cfg(test)]
381411
mod tests {
412+
use codex_analytics::CodexHookSource;
382413
use codex_protocol::models::ContentItem;
383414
use codex_protocol::protocol::HookEventName;
384415
use codex_protocol::protocol::HookExecutionMode;
385416
use codex_protocol::protocol::HookHandlerType;
386417
use codex_protocol::protocol::HookRunStatus;
387418
use codex_protocol::protocol::HookScope;
388419
use pretty_assertions::assert_eq;
389-
use std::path::PathBuf;
390420

391421
use super::additional_context_messages;
392422
use super::hook_run_analytics_payload;
@@ -445,10 +475,8 @@ mod tests {
445475
assert_eq!(tracking.turn_id, "turn-from-hook");
446476
assert_eq!(tracking.model_slug, turn_context.model_info.slug);
447477
assert_eq!(hook.event_name, HookEventName::Stop);
448-
assert_eq!(hook.source_path, PathBuf::from("/tmp/hooks.json"));
449-
assert_eq!(hook.cwd, turn_context.cwd.to_path_buf());
478+
assert_eq!(hook.hook_source, CodexHookSource::Unknown);
450479
assert_eq!(hook.status, HookRunStatus::Blocked);
451-
assert_eq!(hook.duration_ms, Some(27));
452480
}
453481

454482
#[tokio::test]
@@ -466,6 +494,53 @@ mod tests {
466494
assert_eq!(hook.status, HookRunStatus::Failed);
467495
}
468496

497+
#[test]
498+
fn hook_source_for_path_classifies_user_project_and_unknown() {
499+
let codex_home = test_path_buf("/tmp/custom-codex-home").abs();
500+
let cwd = test_path_buf("/tmp/worktree/src").abs();
501+
502+
let system_hooks_path = super::system_config_toml_file()
503+
.expect("system config path")
504+
.parent()
505+
.expect("system config directory")
506+
.join("hooks.json");
507+
508+
assert_eq!(
509+
super::hook_source_for_path(
510+
system_hooks_path.as_path(),
511+
codex_home.as_path(),
512+
cwd.as_path(),
513+
),
514+
CodexHookSource::System,
515+
);
516+
assert_eq!(
517+
super::hook_source_for_path(
518+
codex_home.join("hooks.json").as_path(),
519+
codex_home.as_path(),
520+
cwd.as_path(),
521+
),
522+
CodexHookSource::User,
523+
);
524+
assert_eq!(
525+
super::hook_source_for_path(
526+
test_path_buf("/tmp/worktree/.codex/hooks.json")
527+
.abs()
528+
.as_path(),
529+
codex_home.as_path(),
530+
cwd.as_path(),
531+
),
532+
CodexHookSource::Project,
533+
);
534+
assert_eq!(
535+
super::hook_source_for_path(
536+
test_path_buf("/tmp/hooks.json").abs().as_path(),
537+
codex_home.as_path(),
538+
cwd.as_path(),
539+
),
540+
CodexHookSource::Unknown,
541+
);
542+
}
543+
469544
fn sample_hook_run(status: HookRunStatus) -> HookRunSummary {
470545
HookRunSummary {
471546
id: "stop:0:/tmp/hooks.json".to_string(),

0 commit comments

Comments
 (0)