diff --git a/bt-daemon/src/codex_hooks.rs b/bt-daemon/src/codex_hooks.rs new file mode 100644 index 00000000..20796606 --- /dev/null +++ b/bt-daemon/src/codex_hooks.rs @@ -0,0 +1,498 @@ +//! Read-only hook readiness through Codex's app-server protocol. Trust remains +//! owned by Codex; this module never writes its private trust store. + +use anyhow::{anyhow, Context}; +use serde::Serialize; +use serde_json::{json, Value}; +use std::path::Path; +use std::process::Stdio; +use std::time::Duration; +use tokio::io::{ + AsyncBufRead, AsyncBufReadExt, AsyncReadExt, AsyncWrite, AsyncWriteExt, BufReader, +}; + +const PLUGIN: &str = "trace-codex@braintrust-codex-plugins"; +const TIMEOUT: Duration = Duration::from_secs(5); +const EVENTS: &[&str] = &[ + "sessionStart", + "userPromptSubmit", + "preToolUse", + "permissionRequest", + "postToolUse", + "preCompact", + "postCompact", + "subagentStart", + "subagentStop", + "stop", +]; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum CodexHookState { + Trusted, + ReviewRequired, + Disabled, + PluginMissing, + MissingHooks, + ConfigurationError, + Unknown, +} + +impl std::fmt::Display for CodexHookState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Trusted => "trusted", + Self::ReviewRequired => "review required", + Self::Disabled => "disabled", + Self::PluginMissing => "plugin missing", + Self::MissingHooks => "missing hooks", + Self::ConfigurationError => "configuration error", + Self::Unknown => "unknown", + }) + } +} + +#[derive(Debug, Clone, Serialize)] +pub struct CodexHookReadiness { + pub state: CodexHookState, + pub hook_count: usize, + pub trusted_count: usize, + pub disabled_count: usize, + pub review_required_count: usize, + pub missing_events: Vec, + pub detail: String, +} + +impl CodexHookReadiness { + fn unknown(error: impl std::fmt::Display) -> Self { + Self { + state: CodexHookState::Unknown, + hook_count: 0, trusted_count: 0, disabled_count: 0, review_required_count: 0, + missing_events: Vec::new(), + detail: format!("Could not inspect Codex hooks: {error}. Open /hooks in Codex to inspect Braintrust hook trust; older Codex versions may not expose hooks/list."), + } + } +} + +pub(crate) async fn probe() -> CodexHookReadiness { + let cwd = match std::env::current_dir() { + Ok(cwd) => cwd, + Err(error) => return CodexHookReadiness::unknown(error), + }; + probe_at("codex", &cwd, None).await +} + +async fn probe_at(program: &str, cwd: &Path, home: Option<&Path>) -> CodexHookReadiness { + let mut command: tokio::process::Command = + crate::subprocess::background_command(program).into(); + command + .args(["--no-daemon", "app-server", "--stdio"]) + .current_dir(cwd) + .kill_on_drop(true) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + if let Some(home) = home { + command.env("CODEX_HOME", home); + } + let mut child = match command.spawn() { + Ok(child) => child, + Err(error) => return CodexHookReadiness::unknown(error), + }; + let result = + tokio::time::timeout(TIMEOUT, async { + let mut stdin = child.stdin.take().context("Codex stdin unavailable")?; + let stdout = child.stdout.take().context("Codex stdout unavailable")?; + // Bound both total response bytes and wall time, including notifications. + let mut stdout = BufReader::new(stdout.take(8 * 1024 * 1024)); + request(&mut stdin, &mut stdout, 1, "initialize", json!({ + "clientInfo":{"name":"braintrust_trace_doctor","version":env!("CARGO_PKG_VERSION")}, + "capabilities":{"experimentalApi":true} + })).await?; + stdin.write_all(b"{\"method\":\"initialized\"}\n").await?; + let config = request( + &mut stdin, + &mut stdout, + 2, + "config/read", + json!({"includeLayers":false}), + ) + .await?; + let hooks = request( + &mut stdin, + &mut stdout, + 3, + "hooks/list", + json!({"cwds":[cwd]}), + ) + .await?; + let plugins = request( + &mut stdin, + &mut stdout, + 4, + "plugin/list", + json!({ + "cwds":[cwd], "forceRefetch":false, "marketplaceKinds":["local"] + }), + ) + .await?; + classify(&hooks, &config, &plugins) + }) + .await; + // Always reap the probe, even when the protocol is unsupported or stalls. + let _ = child.kill().await; + match result { + Ok(Ok(readiness)) => readiness, + Ok(Err(error)) => CodexHookReadiness::unknown(error), + Err(_) => CodexHookReadiness::unknown("probe timed out after 5 seconds"), + } +} + +async fn request( + stdin: &mut (impl AsyncWrite + Unpin), + stdout: &mut (impl AsyncBufRead + Unpin), + id: u64, + method: &str, + params: Value, +) -> anyhow::Result { + let mut encoded = serde_json::to_vec(&json!({"id":id,"method":method,"params":params}))?; + encoded.push(b'\n'); + stdin.write_all(&encoded).await?; + stdin.flush().await?; + loop { + let mut line = String::new(); + anyhow::ensure!( + stdout.read_line(&mut line).await? != 0, + "Codex closed the probe stream" + ); + let response: Value = serde_json::from_str(&line).context("invalid Codex response")?; + if response.get("id") != Some(&json!(id)) { + continue; + } + if let Some(error) = response.get("error") { + return Err(anyhow!( + "{method}: {}", + error + .get("message") + .and_then(Value::as_str) + .unwrap_or("request failed") + )); + } + return response + .get("result") + .cloned() + .context("Codex response has no result"); + } +} + +fn classify(hooks: &Value, config: &Value, plugins: &Value) -> anyhow::Result { + let entries = hooks + .get("data") + .and_then(Value::as_array) + .context("hooks/list has no data array")?; + let mut selected = Vec::new(); + let mut has_errors = false; + for entry in entries { + has_errors |= entry + .get("errors") + .and_then(Value::as_array) + .is_none_or(|errors| !errors.is_empty()); + for hook in entry + .get("hooks") + .and_then(Value::as_array) + .context("hooks/list entry has no hooks array")? + { + let is_plugin = hook.get("pluginId").and_then(Value::as_str) == Some(PLUGIN); + let is_managed = hook.get("isManaged").and_then(Value::as_bool) == Some(true) + && hook + .get("command") + .and_then(Value::as_str) + .is_some_and(|command| command.ends_with(" trace hook --source codex")); + if is_plugin || is_managed { + selected.push(hook); + } + } + } + let mut readiness = CodexHookReadiness { + state: CodexHookState::Unknown, + hook_count: selected.len(), + trusted_count: 0, + disabled_count: 0, + review_required_count: 0, + missing_events: EVENTS + .iter() + .filter(|event| { + !selected + .iter() + .any(|hook| hook.get("eventName").and_then(Value::as_str) == Some(**event)) + }) + .map(|event| (*event).to_owned()) + .collect(), + detail: String::new(), + }; + for hook in &selected { + anyhow::ensure!( + hook.get("enabled").and_then(Value::as_bool).is_some(), + "hook has no enabled state" + ); + if hook.get("enabled").and_then(Value::as_bool) == Some(false) { + readiness.disabled_count += 1; + } + match hook.get("trustStatus").and_then(Value::as_str) { + Some("trusted" | "managed") => readiness.trusted_count += 1, + Some("untrusted" | "modified") => readiness.review_required_count += 1, + _ => {} + } + } + let marketplaces = plugins + .get("marketplaces") + .and_then(Value::as_array) + .context("plugin/list has no marketplaces array")?; + let plugin = marketplaces + .iter() + .filter_map(|marketplace| marketplace.get("plugins").and_then(Value::as_array)) + .flatten() + .find(|plugin| plugin.get("id").and_then(Value::as_str) == Some(PLUGIN)); + let config = config + .get("config") + .filter(|value| value.is_object()) + .context("config/read has no config object")?; + let features = &config["features"]; + let plugin_capture = selected.is_empty() + || selected + .iter() + .any(|hook| hook.get("pluginId").and_then(Value::as_str) == Some(PLUGIN)); + let disabled = features + .get("hooks") + .or_else(|| features.get("codex_hooks")) + .and_then(Value::as_bool) + == Some(false) + || plugin_capture + && plugin.is_some_and(|plugin| { + plugin.get("enabled").and_then(Value::as_bool) == Some(false) + }); + let (state, detail) = if disabled || readiness.disabled_count > 0 { + (CodexHookState::Disabled, "Codex hooks or the Braintrust plugin are disabled. Enable them in Codex and inspect /hooks.") + } else if has_errors { + (CodexHookState::ConfigurationError, "Codex reported hook configuration errors. Inspect /hooks and Codex startup diagnostics.") + } else if readiness.review_required_count > 0 { + (CodexHookState::ReviewRequired, "Open /hooks in Codex and review the current Braintrust hook definitions. Untrusted or modified hooks are skipped before capture.") + } else if selected.is_empty() + && plugin + .is_none_or(|plugin| plugin.get("installed").and_then(Value::as_bool) != Some(true)) + { + (CodexHookState::PluginMissing, "Braintrust Codex hooks are not installed. Run bt trace enable codex, then review /hooks in Codex.") + } else if !readiness.missing_events.is_empty() { + (CodexHookState::MissingHooks, "Some Braintrust lifecycle hooks are missing. Update the plugin or complete the managed hook policy, then inspect /hooks.") + } else if readiness.trusted_count == readiness.hook_count { + (CodexHookState::Trusted, "Braintrust hook definitions are enabled and trusted for this working directory. This does not confirm hook execution or Braintrust delivery.") + } else { + ( + CodexHookState::Unknown, + "Codex reported an unknown hook trust state. Inspect /hooks before relying on capture.", + ) + }; + readiness.state = state; + readiness.detail = detail.into(); + Ok(readiness) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn snapshot(trust: &str) -> Value { + json!({"data":[{"errors":[],"hooks":EVENTS.iter().map(|event|json!({ + "eventName":event,"pluginId":PLUGIN,"enabled":true,"trustStatus":trust + })).collect::>()}]}) + } + fn installed() -> Value { + json!({"marketplaces":[{"plugins":[{"id":PLUGIN,"installed":true,"enabled":true}]}]}) + } + + #[test] + fn required_events_match_the_packaged_plugin() { + let hooks: Value = serde_json::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../src/plugins/codex/content/plugins/trace-codex/hooks/hooks.json" + ))) + .unwrap(); + let events: std::collections::BTreeSet<_> = hooks["hooks"] + .as_object() + .unwrap() + .keys() + .map(|key| format!("{}{}", key[..1].to_ascii_lowercase(), &key[1..])) + .collect(); + assert_eq!( + events, + EVENTS.iter().map(|event| (*event).to_owned()).collect() + ); + } + + #[test] + fn classifies_trust_modifications_missing_disabled_and_managed_hooks() { + let config = json!({"config":{"features":{"hooks":true}}}); + for (trust, state) in [ + ("trusted", CodexHookState::Trusted), + ("untrusted", CodexHookState::ReviewRequired), + ("modified", CodexHookState::ReviewRequired), + ("future", CodexHookState::Unknown), + ] { + assert_eq!( + classify(&snapshot(trust), &config, &installed()) + .unwrap() + .state, + state + ); + } + let mut hooks = snapshot("trusted"); + hooks["data"][0]["hooks"][0]["enabled"] = json!(false); + assert_eq!( + classify(&hooks, &config, &installed()).unwrap().state, + CodexHookState::Disabled + ); + let missing = json!({"data":[{"errors":[],"hooks":[]}]}); + assert_eq!( + classify(&missing, &config, &installed()).unwrap().state, + CodexHookState::MissingHooks + ); + assert_eq!( + classify(&missing, &config, &json!({"marketplaces":[]})) + .unwrap() + .state, + CodexHookState::PluginMissing + ); + assert_eq!( + classify( + &snapshot("trusted"), + &json!({"config":{"features":{"hooks":false}}}), + &installed() + ) + .unwrap() + .state, + CodexHookState::Disabled + ); + let mut managed = snapshot("managed"); + for hook in managed["data"][0]["hooks"].as_array_mut().unwrap() { + hook["pluginId"] = Value::Null; + hook["isManaged"] = json!(true); + hook["command"] = json!("/enterprise/bt trace hook --source codex"); + } + assert_eq!( + classify(&managed, &config, &json!({"marketplaces":[]})) + .unwrap() + .state, + CodexHookState::Trusted + ); + } + + #[test] + fn unrelated_plugins_and_invalid_responses_never_establish_readiness() { + let mut hooks = snapshot("trusted"); + for hook in hooks["data"][0]["hooks"].as_array_mut().unwrap() { + hook["pluginId"] = json!("other@marketplace"); + } + assert_ne!( + classify(&hooks, &json!({"config":{}}), &installed()) + .unwrap() + .state, + CodexHookState::Trusted + ); + assert!(classify(&json!({}), &json!({}), &installed()).is_err()); + hooks["data"][0]["errors"] = json!([{"message":"bad hooks"}]); + assert_eq!( + classify(&hooks, &json!({"config":{}}), &installed()) + .unwrap() + .state, + CodexHookState::ConfigurationError + ); + } + + #[tokio::test] + async fn rpc_ignores_notifications_and_reports_unsupported_methods() { + let mut input = Vec::new(); + let mut output = std::io::Cursor::new( + b"{\"method\":\"notice\"}\n{\"id\":3,\"error\":{\"message\":\"Method not found\"}}\n", + ); + let error = request(&mut input, &mut output, 3, "hooks/list", json!({})) + .await + .unwrap_err(); + assert!(error.to_string().contains("Method not found")); + assert_eq!( + serde_json::from_slice::(&input).unwrap()["method"], + "hooks/list" + ); + } + + #[tokio::test] + async fn unavailable_codex_and_closed_protocol_report_unknown() { + let tmp = tempfile::tempdir().unwrap(); + let result = probe_at( + tmp.path().join("missing-codex").to_str().unwrap(), + tmp.path(), + None, + ) + .await; + assert_eq!(result.state, CodexHookState::Unknown); + let mut input = Vec::new(); + let mut output = std::io::Cursor::new(b""); + assert!(request(&mut input, &mut output, 1, "initialize", json!({})) + .await + .unwrap_err() + .to_string() + .contains("closed")); + } + + #[tokio::test] + #[ignore = "Requires an installed Codex CLI; installs only into an isolated temporary home"] + async fn installed_codex_distinguishes_missing_and_unreviewed_hooks() { + let tmp = tempfile::tempdir().unwrap(); + let readiness = probe_at("codex", tmp.path(), Some(tmp.path())).await; + assert_eq!( + readiness.state, + CodexHookState::PluginMissing, + "{readiness:?}" + ); + let marketplace = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../src/plugins/codex/content") + .canonicalize() + .unwrap(); + let marketplace = marketplace.to_string_lossy(); + for args in [ + vec![ + "plugin", + "marketplace", + "add", + marketplace.as_ref(), + "--json", + ], + vec!["plugin", "add", PLUGIN, "--json"], + vec!["plugin", "add", PLUGIN, "--json"], + ] { + let mut command: tokio::process::Command = + crate::subprocess::background_command("codex").into(); + let output = command + .args(["--no-daemon"]) + .args(args) + .env("CODEX_HOME", tmp.path()) + .current_dir(tmp.path()) + .kill_on_drop(true) + .output() + .await + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } + let readiness = probe_at("codex", tmp.path(), Some(tmp.path())).await; + assert_eq!( + readiness.state, + CodexHookState::ReviewRequired, + "{readiness:?}" + ); + assert_eq!(readiness.review_required_count, EVENTS.len()); + assert!(readiness.missing_events.is_empty()); + } +} diff --git a/bt-daemon/src/command_output.rs b/bt-daemon/src/command_output.rs index 704e7f09..3a192062 100644 --- a/bt-daemon/src/command_output.rs +++ b/bt-daemon/src/command_output.rs @@ -59,6 +59,8 @@ pub struct SetupCommandOutput { pub display_name: String, pub settings_path: PathBuf, pub restart_required: bool, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub next_steps: Vec, } #[derive(Debug, Clone, Serialize)] @@ -66,6 +68,8 @@ pub struct UpdateCommandOutput { pub source: String, pub display_name: String, pub restart_required: bool, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub next_steps: Vec, } #[derive(Debug, Clone, Serialize)] @@ -158,6 +162,8 @@ pub struct DoctorCommandOutput { pub daemon: DaemonDiagnostic, pub warnings: Vec, pub plugin_diagnostics: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub codex_hooks: Option, } #[derive(Debug, Clone, Serialize)] @@ -186,8 +192,10 @@ impl TraceCommandOutput { display_name: impl Into, settings_path: impl Into, ) -> Self { + let source = source.into(); Self::Enable(SetupCommandOutput { - source: source.into(), + next_steps: activation_steps(&source), + source, display_name: display_name.into(), settings_path: settings_path.into(), restart_required: true, @@ -208,6 +216,7 @@ impl TraceCommandOutput { settings_path: impl Into, ) -> Self { Self::Disable(SetupCommandOutput { + next_steps: Vec::new(), source: source.into(), display_name: display_name.into(), settings_path: settings_path.into(), @@ -216,8 +225,10 @@ impl TraceCommandOutput { } pub fn update(source: impl Into, display_name: impl Into) -> Self { + let source = source.into(); Self::Update(UpdateCommandOutput { - source: source.into(), + next_steps: activation_steps(&source), + source, display_name: display_name.into(), restart_required: true, }) @@ -274,6 +285,14 @@ impl TraceCommandOutput { for warning in &doctor.warnings { rendered.push_str(&format!("\nWarning: {warning}")); } + if let Some(hooks) = &doctor.codex_hooks { + rendered.push_str(&format!("\nCodex hook trust: {}\n{}\nHooks: {} loaded, {} trusted, {} require review, {} disabled", + hooks.state, hooks.detail, hooks.hook_count, hooks.trusted_count, + hooks.review_required_count, hooks.disabled_count)); + if !hooks.missing_events.is_empty() { + rendered.push_str(&format!("\nMissing hook events: {}", hooks.missing_events.join(", "))); + } + } for diagnostic in &doctor.plugin_diagnostics { rendered.push_str(&format!( "\nPlugin error: {} ({} occurrence{})\nFirst seen: {}\nLast seen: {}\n{}", @@ -288,9 +307,9 @@ impl TraceCommandOutput { Ok(rendered) } Self::Enable(setup) => Ok(format!( - "The Braintrust tracing plugin is installed for {} and configured in {}.\nRestart the coding agent to load the tracing plugin.", + "The Braintrust tracing plugin is installed for {} and configured in {}.\nRestart the coding agent to load the tracing plugin.{}", setup.display_name, - setup.settings_path.display() + setup.settings_path.display(), render_next_steps(&setup.next_steps) )), Self::Disable(disable) => Ok(format!( "The Braintrust tracing plugin and configuration were removed for {} from {}.\nRestart the coding agent to apply the change.", @@ -298,8 +317,8 @@ impl TraceCommandOutput { disable.settings_path.display() )), Self::Update(update) => Ok(format!( - "The Braintrust tracing plugin was updated for {}.\nRestart the coding agent to load the update.", - update.display_name, + "The Braintrust tracing plugin was updated for {}.\nRestart the coding agent to load the update.{}", + update.display_name, render_next_steps(&update.next_steps), )), Self::Stop(stop) if stop.stopped => Ok("Tracing daemon stopped.".into()), Self::Stop(_) => Ok("No tracing daemon is running.".into()), @@ -353,6 +372,18 @@ fn push_auth_lines(rendered: &mut String, prefix: &str, auth: &AuthDiagnostic) { } } +fn activation_steps(source: &str) -> Vec { + if source == "codex" { + vec!["Codex separately requires trust for plugin hooks. Open /hooks in Codex and review the Braintrust definitions, then run bt trace doctor codex. Installation alone does not confirm capture or delivery.".into()] + } else { + Vec::new() + } +} + +fn render_next_steps(steps: &[String]) -> String { + steps.iter().map(|step| format!("\n{step}")).collect() +} + fn render_timestamp(timestamp_ms: i64) -> String { chrono::DateTime::from_timestamp_millis(timestamp_ms) .map(|timestamp| timestamp.to_rfc3339()) @@ -427,6 +458,27 @@ mod tests { .contains("removed for Google Antigravity")); } + #[test] + fn codex_setup_and_updates_explain_the_separate_hook_review_step() { + for output in [ + TraceCommandOutput::setup("codex", "Codex", PathBuf::from("braintrust.json")), + TraceCommandOutput::update("codex", "Codex"), + ] { + let human = output.render(OutputFormat::Human).unwrap(); + assert!(human.contains("Open /hooks")); + assert!(human.contains("Installation alone does not confirm capture or delivery")); + let json: serde_json::Value = + serde_json::from_str(&output.render(OutputFormat::Json).unwrap()).unwrap(); + assert_eq!(json["next_steps"].as_array().unwrap().len(), 1); + } + let disabled = + TraceCommandOutput::disable("codex", "Codex", PathBuf::from("braintrust.json")); + assert!(!disabled + .render(OutputFormat::Human) + .unwrap() + .contains("Open /hooks")); + } + #[test] fn stop_json_reports_idempotent_and_successful_shutdowns() { let absent: serde_json::Value = serde_json::from_str( @@ -535,6 +587,15 @@ mod tests { error: None, }, warnings: Vec::new(), + codex_hooks: Some(crate::CodexHookReadiness { + state: crate::CodexHookState::ReviewRequired, + hook_count: 10, + trusted_count: 0, + disabled_count: 0, + review_required_count: 10, + missing_events: Vec::new(), + detail: "Open /hooks to review Braintrust definitions".into(), + }), plugin_diagnostics: vec![crate::PluginDiagnostic { source: "codex".into(), plugin_path: PathBuf::from("/tmp/redact.mjs"), @@ -551,6 +612,11 @@ mod tests { assert_eq!(value["auth"]["source"], "saved_profile"); assert_eq!(value["daemon"]["status"], "running"); assert_eq!(value["daemon"]["auth"]["status"], "error"); + assert_eq!(value["codex_hooks"]["state"], "review_required"); + assert!(output + .render(OutputFormat::Human) + .unwrap() + .contains("10 require review")); assert_eq!( value["daemon"]["session_errors"][0], "could not resolve Braintrust auth for codex" diff --git a/bt-daemon/src/lib.rs b/bt-daemon/src/lib.rs index 61a3f58f..9e1e5470 100644 --- a/bt-daemon/src/lib.rs +++ b/bt-daemon/src/lib.rs @@ -15,6 +15,7 @@ pub mod paths; mod agents; mod args; mod client; +mod codex_hooks; mod command_output; mod correlation; mod delivery_ledger; @@ -51,6 +52,7 @@ pub use args::{ pub use client::{ flush_managed_run, flush_session, forward_envelope, run_status, shutdown_daemon, HostInfo, }; +pub use codex_hooks::{CodexHookReadiness, CodexHookState}; pub use command_output::{ AuthDiagnostic, DaemonDiagnostic, DaemonStatus, DoctorCommandOutput, ImportSummary, OutputFormat, SetupCommandOutput, StatusCommandOutput, StopCommandOutput, TraceCommandOutput, diff --git a/bt-daemon/src/trace_runtime.rs b/bt-daemon/src/trace_runtime.rs index 684c97b6..5a5fece5 100644 --- a/bt-daemon/src/trace_runtime.rs +++ b/bt-daemon/src/trace_runtime.rs @@ -495,14 +495,28 @@ fn daemon_warnings( async fn doctor_output(host: &TraceHostContext, args: DoctorArgs) -> DoctorCommandOutput { let source = args.agent.identity().id; - doctor_output_at( - host, - args, - &paths::socket_path(None), - paths::agent_settings_path(source, None), - crate::settings::AgentSettings::load(source), - ) - .await + let settings = crate::settings::AgentSettings::load(source); + let socket = paths::socket_path(None); + let inspect_hooks = matches!(args.agent, DoctorAgent::Codex) && settings.tracing_enabled(); + let hooks = async { + if inspect_hooks { + Some(crate::codex_hooks::probe().await) + } else { + None + } + }; + let (mut output, hooks) = tokio::join!( + doctor_output_at( + host, + args, + &socket, + paths::agent_settings_path(source, None), + settings, + ), + hooks + ); + output.codex_hooks = hooks; + output } async fn doctor_output_at( @@ -632,6 +646,7 @@ async fn doctor_output_at( daemon, warnings, plugin_diagnostics, + codex_hooks: None, } } diff --git a/docs/codex-managed-tracing.md b/docs/codex-managed-tracing.md new file mode 100644 index 00000000..c9835761 --- /dev/null +++ b/docs/codex-managed-tracing.md @@ -0,0 +1,131 @@ +# Codex hook activation and enterprise deployment + +Installing `trace-codex` does not approve its hooks. Codex skips untrusted or +modified plugin hooks before Braintrust receives their events. Restart Codex, +open `/hooks`, review the Braintrust definitions, and run `bt trace doctor codex`. +The doctor probes the installed Codex app server for the current working directory. +It distinguishes trusted, review-required, disabled, missing, erroneous, and +unknown configurations. Trust readiness does not prove capture or delivery. + +## Supported approval mechanisms + +[Official OpenAI hook documentation](https://learn.chatgpt.com/docs/hooks#review-and-trust-hooks) +describes user review through `/hooks` and administrator-managed hooks trusted by +policy. Codex CLI 0.163.0 exposes `hooks/list` with each definition's enabled and +trust state, but its generated app-server protocol has no hook approval method. +Braintrust therefore does not modify Codex's private trust files or approve +unrelated hooks. A release that changes hook definitions can require review again; +route changes and `bt` updates do not change the marketplace definitions. + +The invocation-wide `--dangerously-bypass-hook-trust` flag is not scoped to +Braintrust. `bt trace run codex` continues to reject it. For unattended enterprise +activation, use Codex's supported managed policy rather than that flag. + +## Administrator-managed local Codex hooks + +This recipe targets local Codex execution. Administrators own policy deployment; +`bt trace enable` does not write managed policy. Check the installed Codex version +against [OpenAI's managed hook documentation](https://learn.chatgpt.com/docs/hooks#managed-hooks-from-requirementstoml). + +1. Distribute a current `bt` executable to the managed directory on each platform. +2. Have each user authenticate and configure their route with + `bt trace enable codex --project PROJECT`. Credentials stay in the CLI's + authentication store; the route stays in the user's `braintrust.json`. +3. Remove the user plugin with + `codex plugin remove trace-codex@braintrust-codex-plugins` after configuring + the route. This avoids duplicate plugin and managed capture; this Codex + command leaves Braintrust's route settings intact. +4. Merge the following into the organization's Codex `requirements.toml` using + its normal device-management mechanism. Preserve unrelated requirements. + Replace the executable directories with administrator-controlled locations. + +```toml +[features] +hooks = true + +[hooks] +managed_dir = "/opt/company/braintrust" +windows_managed_dir = 'C:\Braintrust' + +[[hooks.SessionStart]] +[[hooks.SessionStart.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.UserPromptSubmit]] +[[hooks.UserPromptSubmit.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.PreToolUse]] +[[hooks.PreToolUse.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.PermissionRequest]] +[[hooks.PermissionRequest.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.PostToolUse]] +[[hooks.PostToolUse.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.PreCompact]] +[[hooks.PreCompact.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.PostCompact]] +[[hooks.PostCompact.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.SubagentStart]] +[[hooks.SubagentStart.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.SubagentStop]] +[[hooks.SubagentStop.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.Stop]] +[[hooks.Stop.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' + +[[hooks.SessionEnd]] +[[hooks.SessionEnd.hooks]] +type = "command" +command = "/opt/company/braintrust/bt trace hook --source codex" +command_windows = 'C:\Braintrust\bt.exe trace hook --source codex' +``` + +Keep all lifecycle events so sessions, turns, tools, and subagents retain their +normal trace structure. Codex does not distribute the executable itself. The +policy above enables hooks without disabling other hook sources; an organization +may separately choose `allow_managed_hooks_only` if that is its intended policy. + +After deployment, restart Codex and inspect `/hooks`: Braintrust definitions +should be managed and enabled. Run `bt trace doctor codex` from the actual project +directory, then run a short session and verify its matching trace in Braintrust. +A running daemon, trusted definitions, or historical traces alone do not confirm +that the newly deployed path delivered the session. + +For ordinary plugin installs, hooks still need review after definition changes. +Events skipped before approval are not in the daemon's journal; replay cannot +recover those events. Historical transcript import may recover supported data, +with the limitations documented for that import path. diff --git a/src/plugins/codex/content/plugins/trace-codex/README.md b/src/plugins/codex/content/plugins/trace-codex/README.md index efc5553e..62441f7a 100644 --- a/src/plugins/codex/content/plugins/trace-codex/README.md +++ b/src/plugins/codex/content/plugins/trace-codex/README.md @@ -23,7 +23,20 @@ bt trace enable codex --project my-coding-agent Use `--profile` or `--org` when needed. Restart Codex after setup so it loads the plugin. Codex will apply its normal hook-review flow; approve the stable -Braintrust hook definition through `/hooks` when prompted. +Braintrust hook definition through `/hooks` when prompted. Installing the plugin +alone does not approve hooks: untrusted or modified definitions are skipped +before Braintrust can capture events. + +`bt trace doctor codex` inspects hook trust through the installed Codex app server +for the current directory. It reports missing or disabled hooks, pending review, +and trusted definitions separately from daemon authentication and delivery. +Older Codex versions without this inspection API report an unknown state and +still require inspection through `/hooks`. + +For unattended enterprise deployment, see the +[administrator-managed hook recipe](https://github.com/braintrustdata/braintrust-coding-agent-plugins/blob/main/docs/codex-managed-tracing.md). +Managed hooks use Codex policy-based trust. Braintrust does not edit Codex's +private trust store or bypass review for other hooks. To inspect tracing configuration and daemon status: