diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 0f759fd938..4bb2afa896 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -3328,6 +3328,40 @@ fn hook_run_event_serializes_expected_shape() { ); } +#[test] +fn user_instructions_hook_run_event_serializes_expected_shape() { + let tracking = test_tracking_context("thread-4", "turn-4"); + let event = TrackEventRequest::HookRun(CodexHookRunEventRequest { + event_type: "codex_hook_run", + event_params: codex_hook_run_metadata( + &tracking, + HookRunFact { + event_name: HookEventName::UserInstructions, + hook_source: HookSource::User, + status: HookRunStatus::Completed, + }, + ), + }); + + let payload = serde_json::to_value(&event).expect("serialize UserInstructions hook run event"); + + assert_eq!( + payload, + json!({ + "event_type": "codex_hook_run", + "event_params": { + "thread_id": "thread-4", + "turn_id": null, + "product_client_id": TEST_PRODUCT_CLIENT_ID, + "model_slug": "gpt-5", + "hook_name": "UserInstructions", + "hook_source": "user", + "status": "completed" + } + }) + ); +} + #[test] fn hook_run_metadata_maps_sources_and_statuses() { let tracking = test_tracking_context("thread-1", "turn-1"); diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 37d9dd3bb6..0d489e9a01 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -1215,7 +1215,8 @@ pub(crate) fn codex_hook_run_metadata( ) -> CodexHookRunMetadata { CodexHookRunMetadata { thread_id: Some(tracking.thread_id.clone()), - turn_id: Some(tracking.turn_id.clone()), + turn_id: (hook.event_name != HookEventName::UserInstructions) + .then(|| tracking.turn_id.clone()), product_client_id: Some(tracking.product_client_id.clone()), model_slug: Some(tracking.model_slug.clone()), hook_name: Some(analytics_hook_event_name(hook.event_name).to_owned()), diff --git a/codex-rs/codex-home/src/instructions/mod.rs b/codex-rs/codex-home/src/instructions/mod.rs index 4f4ea76525..f022eb0569 100644 --- a/codex-rs/codex-home/src/instructions/mod.rs +++ b/codex-rs/codex-home/src/instructions/mod.rs @@ -54,7 +54,7 @@ impl CodexHomeUserInstructionsProvider { return LoadedUserInstructions { instructions: Some(UserInstructions { text: trimmed.to_string(), - source: path, + sources: vec![path], }), warnings, }; diff --git a/codex-rs/codex-home/src/instructions/tests.rs b/codex-rs/codex-home/src/instructions/tests.rs index ee2ba9035c..526eb6b66e 100644 --- a/codex-rs/codex-home/src/instructions/tests.rs +++ b/codex-rs/codex-home/src/instructions/tests.rs @@ -27,8 +27,10 @@ fn expected( LoadedUserInstructions { instructions: Some(UserInstructions { text: text.to_string(), - source: AbsolutePathBuf::try_from(home.path().join(filename)) - .expect("absolute source path"), + sources: vec![ + AbsolutePathBuf::try_from(home.path().join(filename)) + .expect("absolute source path"), + ], }), warnings, } diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 1c01c8873f..35b080cd59 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -33,6 +33,9 @@ use std::io; use toml::Value as TomlValue; use tracing::error; +mod hook_loading; +pub(crate) use hook_loading::resolve_user_instructions; + /// Default filename scanned for AGENTS.md instructions. pub const DEFAULT_AGENTS_MD_FILENAME: &str = "AGENTS.md"; /// Preferred local override for AGENTS.md instructions. @@ -255,7 +258,7 @@ impl LoadedAgentsMd { Self { user_instructions: Some(UserInstructions { text: contents, - source: path, + sources: vec![path], }), entries: Vec::new(), } @@ -396,7 +399,8 @@ impl LoadedAgentsMd { pub fn sources(&self) -> impl Iterator + '_ { self.user_instructions .iter() - .map(|instructions| PathUri::from_abs_path(&instructions.source)) + .flat_map(|instructions| instructions.sources.iter()) + .map(PathUri::from_abs_path) .chain( self.entries .iter() diff --git a/codex-rs/core/src/agents_md/hook_loading.rs b/codex-rs/core/src/agents_md/hook_loading.rs new file mode 100644 index 0000000000..0570418c67 --- /dev/null +++ b/codex-rs/core/src/agents_md/hook_loading.rs @@ -0,0 +1,93 @@ +use codex_extension_api::UserInstructions; +use codex_hooks::Hooks; +use codex_hooks::UserInstructionsRequest; +use codex_protocol::protocol::Event; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::HookCompletedEvent; +use codex_protocol::protocol::HookStartedEvent; +use codex_protocol::protocol::WarningEvent; + +use crate::session::INITIAL_SUBMIT_ID; + +pub(crate) struct UserInstructionsResolution { + pub instructions: Option, + pub events: Vec, +} + +/// Resolves a fresh user-instruction snapshot from the configured hook. +pub(crate) async fn resolve_user_instructions( + hooks: &Hooks, + request: UserInstructionsRequest, + mut instructions: Option, + mut on_completed: impl FnMut(&HookCompletedEvent), +) -> UserInstructionsResolution { + let mut events = hooks + .preview_user_instructions(&request) + .into_iter() + .map(|run| { + event(EventMsg::HookStarted(HookStartedEvent { + turn_id: None, + run, + })) + }) + .collect::>(); + + let outcome = hooks.run_user_instructions(request).await; + for completed in outcome.hook_events { + on_completed(&completed); + events.push(event(EventMsg::HookCompleted(completed))); + } + events.extend( + outcome + .warnings + .into_iter() + .map(|message| event(EventMsg::Warning(WarningEvent { message }))), + ); + + let mut texts = Vec::new(); + let mut sources = Vec::new(); + for result in outcome.results { + match result.source_path.to_abs_path() { + Ok(source) => { + texts.push(result.text); + if !sources.contains(&source) { + sources.push(source); + } + } + Err(error) => events.push(event(EventMsg::Warning(WarningEvent { + message: format!( + "UserInstructions hook source `{}` cannot be resolved on this host: {error}; ignoring hook output", + result.source_path + ), + }))), + } + } + if !texts.is_empty() { + if let Some(existing) = &instructions + && let Some(source) = existing.sources.first() + { + events.push(event(EventMsg::Warning(WarningEvent { + message: format!( + "UserInstructions hook output overrides user-level instructions from `{}`.", + source.display() + ), + }))); + } + instructions = Some(UserInstructions { + text: texts.join("\n\n"), + sources, + }); + } + + UserInstructionsResolution { + instructions, + events, + } +} + +fn event(msg: EventMsg) -> Event { + Event { + id: INITIAL_SUBMIT_ID.to_owned(), + msg, + } +} diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index d93e4634db..473b412c37 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -426,7 +426,7 @@ async fn make_config(root: &TempDir, limit: usize, instructions: Option<&str>) - let user_instructions = instructions.map(|text| UserInstructions { text: text.to_owned(), - source: config.codex_home.join(DEFAULT_AGENTS_MD_FILENAME), + sources: vec![config.codex_home.join(DEFAULT_AGENTS_MD_FILENAME)], }); TestConfig { config, @@ -475,7 +475,7 @@ async fn make_config_with_project_root_markers( config.project_doc_max_bytes = limit; let user_instructions = instructions.map(|text| UserInstructions { text: text.to_owned(), - source: config.codex_home.join(DEFAULT_AGENTS_MD_FILENAME), + sources: vec![config.codex_home.join(DEFAULT_AGENTS_MD_FILENAME)], }); TestConfig { config, @@ -944,7 +944,7 @@ secondary doc"#, .user_instructions .as_ref() .expect("global instructions") - .source, + .sources[0], ), PathUri::from_abs_path(&primary.path().join("AGENTS.md").abs()), PathUri::from_abs_path(&primary_nested.join("AGENTS.md").abs()), @@ -1278,7 +1278,7 @@ async fn instruction_sources_include_global_before_agents_md_docs() { let expected = LoadedAgentsMd { user_instructions: Some(UserInstructions { text: "global doc".to_string(), - source: global_agents.clone(), + sources: vec![global_agents.clone()], }), entries: vec![InstructionEntry { contents: "project doc".to_string(), diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 35df4e46c8..c654675f1e 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -2,6 +2,7 @@ use std::future::Future; use std::sync::Arc; use std::time::Duration; +use codex_analytics::AnalyticsEventsClient; use codex_analytics::CompactionTrigger; use codex_analytics::HookRunFact; use codex_analytics::build_track_events_context; @@ -21,6 +22,8 @@ use codex_hooks::UserPromptSubmitOutcome; use codex_hooks::UserPromptSubmitRequest; use codex_otel::HOOK_RUN_DURATION_METRIC; use codex_otel::HOOK_RUN_METRIC; +use codex_otel::SessionTelemetry; +use codex_protocol::ThreadId; use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::models::ResponseItem; @@ -133,7 +136,7 @@ pub(crate) async fn run_pending_session_start_hooks( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy.value()), target, }; let hooks = sess.hooks(); @@ -175,7 +178,7 @@ pub(crate) async fn run_pre_tool_use_hooks( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy.value()), tool_name: tool_name.name().to_string(), matcher_aliases: tool_name.matcher_aliases().to_vec(), tool_use_id, @@ -236,7 +239,7 @@ pub(crate) async fn run_permission_request_hooks( cwd: turn_context.cwd.to_path_buf(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy.value()), tool_name: payload.tool_name.name().to_string(), matcher_aliases: payload.tool_name.matcher_aliases().to_vec(), run_id_suffix: run_id_suffix.to_string(), @@ -278,7 +281,7 @@ pub(crate) async fn run_post_tool_use_hooks( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy.value()), tool_name, matcher_aliases, tool_use_id, @@ -352,7 +355,7 @@ pub(crate) async fn run_turn_stop_hooks( cwd: turn_context.cwd.clone(), transcript_path, model: turn_context.model_info.slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy.value()), stop_hook_active, last_assistant_message, target, @@ -512,7 +515,7 @@ pub(crate) async fn inspect_pending_input( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy.value()), prompt: UserMessageItem::new(content).message(), }; let hooks = sess.hooks(); @@ -637,22 +640,47 @@ pub(crate) async fn emit_hook_completed_events( completed_events: Vec, ) { for completed in completed_events { - emit_hook_completed_metrics(turn_context, &completed); + emit_hook_completed_metrics(&turn_context.session_telemetry, &completed); track_hook_completed_analytics(sess, turn_context, &completed); sess.send_event(turn_context, EventMsg::HookCompleted(completed)) .await; } } -fn emit_hook_completed_metrics(turn_context: &TurnContext, completed: &HookCompletedEvent) { +/// Records a thread-scoped hook completion before a [`TurnContext`] exists. +/// +/// Ordinary hooks flow through [`emit_hook_completed_events`]. Instruction +/// resolution happens earlier, so it needs the same metrics and analytics +/// without routing an unrelated queued event through the startup event loop. +pub(crate) fn record_hook_completed_without_turn_context( + session_telemetry: &SessionTelemetry, + analytics_events_client: &AnalyticsEventsClient, + thread_id: ThreadId, + model_slug: &str, + originator: &str, + completed: &HookCompletedEvent, +) { + emit_hook_completed_metrics(session_telemetry, completed); + let (tracking, hook) = hook_run_analytics_payload_from_parts( + model_slug.to_string(), + thread_id.to_string(), + completed.turn_id.clone().unwrap_or_default(), + originator.to_string(), + completed, + ); + analytics_events_client.track_hook_run(tracking, hook); +} + +fn emit_hook_completed_metrics( + session_telemetry: &SessionTelemetry, + completed: &HookCompletedEvent, +) { let tags = hook_run_metric_tags(&completed.run); - turn_context - .session_telemetry - .counter(HOOK_RUN_METRIC, /*inc*/ 1, &tags); + session_telemetry.counter(HOOK_RUN_METRIC, /*inc*/ 1, &tags); if let Some(duration_ms) = completed.run.duration_ms && let Ok(duration_ms) = u64::try_from(duration_ms) { - turn_context.session_telemetry.record_duration( + session_telemetry.record_duration( HOOK_RUN_DURATION_METRIC, Duration::from_millis(duration_ms), &tags, @@ -676,17 +704,28 @@ fn hook_run_analytics_payload( thread_id: String, turn_context: &TurnContext, completed: &HookCompletedEvent, +) -> (codex_analytics::TrackEventsContext, HookRunFact) { + hook_run_analytics_payload_from_parts( + turn_context.model_info.slug.clone(), + thread_id, + completed + .turn_id + .clone() + .unwrap_or_else(|| turn_context.sub_id.clone()), + turn_context.originator.clone(), + completed, + ) +} + +fn hook_run_analytics_payload_from_parts( + model_slug: String, + thread_id: String, + turn_id: String, + originator: String, + completed: &HookCompletedEvent, ) -> (codex_analytics::TrackEventsContext, HookRunFact) { ( - build_track_events_context( - turn_context.model_info.slug.clone(), - thread_id, - completed - .turn_id - .clone() - .unwrap_or_else(|| turn_context.sub_id.clone()), - turn_context.originator.clone(), - ), + build_track_events_context(model_slug, thread_id, turn_id, originator), HookRunFact { event_name: completed.run.event_name, hook_source: completed.run.source, @@ -737,8 +776,8 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); ] } -fn hook_permission_mode(turn_context: &TurnContext) -> String { - match turn_context.approval_policy.value() { +pub(crate) fn hook_permission_mode(approval_policy: AskForApproval) -> String { + match approval_policy { AskForApproval::Never => "bypassPermissions", AskForApproval::UnlessTrusted | AskForApproval::OnRequest | AskForApproval::Granular(_) => { "default" diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 2261eaa495..80a0146a8c 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1,9 +1,12 @@ use super::input_queue::InputQueue; use super::*; +use crate::agents_md::resolve_user_instructions; use crate::agents_md_manager::AgentsMdManager; use crate::config::ConstraintError; use crate::environment_selection::ThreadEnvironments; use crate::environment_selection::TurnEnvironmentSnapshot; +use crate::hook_runtime::hook_permission_mode; +use crate::hook_runtime::record_hook_completed_without_turn_context; use crate::shell_snapshot::ShellSnapshot; use crate::skills::SkillError; use crate::state::ActiveTurn; @@ -480,7 +483,7 @@ impl Session { pub(crate) async fn new( mut session_configuration: SessionConfiguration, config: Arc, - user_instructions: Option, + mut user_instructions: Option, installation_id: String, auth_manager: Arc, models_manager: SharedModelsManager, @@ -819,6 +822,13 @@ impl Session { if let Some(service_name) = session_configuration.metrics_service_name.as_deref() { session_telemetry = session_telemetry.with_metrics_service_name(service_name); } + let analytics_events_client = analytics_events_client.unwrap_or_else(|| { + AnalyticsEventsClient::new( + Arc::clone(&auth_manager), + config.chatgpt_base_url.trim_end_matches('/').to_string(), + config.analytics_enabled, + ) + }); let network_proxy_audit_metadata = NetworkProxyAuditMetadata { conversation_id: Some(thread_id.to_string()), app_version: Some(env!("CARGO_PKG_VERSION").to_string()), @@ -899,6 +909,54 @@ impl Session { )); turn_environments.update_selections(session_configuration.environment_selections()); let resolved_environments = turn_environments.snapshot().await; + let hooks = build_hooks_for_config( + &config, + plugins_manager.as_ref(), + resolved_environments.single_local_environment(), + ) + .await; + for warning in hooks.startup_warnings() { + post_session_configured_events.push(Event { + id: INITIAL_SUBMIT_ID.to_owned(), + msg: EventMsg::Warning(WarningEvent { + message: warning.clone(), + }), + }); + } + + validate_config_lock_if_configured(&session_configuration).await?; + let model = session_configuration.collaboration_mode.model().to_string(); + let originator = session_configuration.originator.clone(); + let request = codex_hooks::UserInstructionsRequest { + session_id: session_id.into(), + cwd: PathUri::from_abs_path(session_configuration.cwd()), + command_cwd: resolved_environments + .single_local_environment_cwd() + .unwrap_or_else(|| config.codex_home.clone()), + transcript_path: rollout_path.clone(), + model: model.clone(), + permission_mode: hook_permission_mode( + session_configuration.approval_policy.value(), + ), + }; + let resolution = resolve_user_instructions( + &hooks, + request, + user_instructions, + |completed| { + record_hook_completed_without_turn_context( + &session_telemetry, + &analytics_events_client, + thread_id, + &model, + &originator, + completed, + ); + }, + ) + .await; + user_instructions = resolution.instructions; + post_session_configured_events.extend(resolution.events); let agents_md_manager = Arc::new(AgentsMdManager::new(user_instructions)); agents_md_manager .refresh(config.as_ref(), &resolved_environments) @@ -929,7 +987,6 @@ impl Session { )) .await; session_configuration.thread_name = thread_name.clone(); - validate_config_lock_if_configured(&session_configuration).await?; export_config_lock_if_configured(&session_configuration, thread_id).await?; let state = SessionState::new_with_auto_compact_window_ids( session_configuration.clone(), @@ -994,28 +1051,6 @@ impl Session { (None, None) }; - let hooks = build_hooks_for_config( - &config, - plugins_manager.as_ref(), - resolved_environments.single_local_environment(), - ) - .await; - for warning in hooks.startup_warnings() { - post_session_configured_events.push(Event { - id: INITIAL_SUBMIT_ID.to_owned(), - msg: EventMsg::Warning(WarningEvent { - message: warning.clone(), - }), - }); - } - - let analytics_events_client = analytics_events_client.unwrap_or_else(|| { - AnalyticsEventsClient::new( - Arc::clone(&auth_manager), - config.chatgpt_base_url.trim_end_matches('/').to_string(), - config.analytics_enabled, - ) - }); // Keep one stable manager handle for the session so extension resource clients // automatically observe the manager installed at startup and on later refreshes. let mcp_connection_manager = Arc::new(arc_swap::ArcSwap::from_pointee( diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 3e27463370..471b1e52d0 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -130,6 +130,7 @@ mod unified_exec_process_events; #[cfg(unix)] mod unified_exec_zsh_fork_approvals; mod unstable_features_warning; +mod user_instructions_hook; mod user_notification; mod user_shell_cmd; mod view_image; diff --git a/codex-rs/core/tests/suite/user_instructions_hook.rs b/codex-rs/core/tests/suite/user_instructions_hook.rs new file mode 100644 index 0000000000..b4af9c33fa --- /dev/null +++ b/codex-rs/core/tests/suite/user_instructions_hook.rs @@ -0,0 +1,488 @@ +use std::fs; +use std::path::Path; +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Context; +use anyhow::Result; +use codex_features::Feature; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookRunStatus; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +use core_test_support::PathBufExt; +use core_test_support::hooks::trust_discovered_hooks; +use core_test_support::responses; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call_with_namespace; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::mount_sse_once_match; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use serde_json::Value; +use tempfile::TempDir; + +const GLOBAL_INSTRUCTIONS: &str = "global user instructions"; +const HOOK_INSTRUCTIONS: &str = r#"{"source":"user-instructions-hook"}"#; +const PROJECT_INSTRUCTIONS: &str = "project instructions"; + +fn python_hook_command(script_path: &Path) -> String { + let python = if cfg!(windows) { "python" } else { "python3" }; + let script_path = if cfg!(windows) { + format!(r#""{}""#, script_path.display()) + } else { + format!( + "'{}'", + script_path.display().to_string().replace('\'', "'\\''") + ) + }; + format!("{python} {script_path}") +} + +fn write_user_instructions_hook(home: &Path, output: &str, exit_code: Option) -> Result<()> { + write_user_instructions_hooks(home, &[(output, exit_code)]) +} + +fn write_user_instructions_hooks(home: &Path, hooks: &[(&str, Option)]) -> Result<()> { + let log_path = home.join("user_instructions_hook_input.json"); + let mut configured_hooks = Vec::new(); + for (index, (output, exit_code)) in hooks.iter().enumerate() { + let script_path = home.join(format!("user_instructions_hook_{index}.py")); + let output = serde_json::to_string(output).context("serialize hook output")?; + let exit = exit_code + .map(|exit_code| format!("raise SystemExit({exit_code})")) + .unwrap_or_else(|| format!("sys.stdout.write({output})")); + let script = format!( + r#"import json +from pathlib import Path +import sys + +payload = json.load(sys.stdin) +with Path(r"{log_path}").open("a", encoding="utf-8") as log: + log.write(json.dumps(payload) + "\n") +{exit} +"#, + log_path = log_path.display(), + ); + fs::write(&script_path, script).context("write user instructions hook script")?; + configured_hooks.push(serde_json::json!({ + "type": "command", + "command": python_hook_command(&script_path), + "timeout": 10, + "statusMessage": "loading user instructions", + })); + } + let hooks = serde_json::json!({ + "hooks": { + "UserInstructions": configured_hooks + } + }); + + fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?; + Ok(()) +} + +fn write_global_instructions(home: &Path) -> Result { + let path = home.join("AGENTS.md"); + fs::write(&path, GLOBAL_INSTRUCTIONS).context("write global AGENTS.md")?; + Ok(path.abs()) +} + +fn instruction_fragment(request: &responses::ResponsesRequest) -> String { + request + .message_input_texts("user") + .into_iter() + .find(|text| text.starts_with("# AGENTS.md instructions")) + .expect("user instructions fragment") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_instructions_hook_replaces_global_instructions_and_keeps_project_docs() -> Result<()> +{ + // This test uses wiremock's loopback TCP endpoint, which is unavailable + // when the Codex sandbox disables networking. + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let home = Arc::new(TempDir::new()?); + let global_source = write_global_instructions(home.path())?; + write_user_instructions_hook(home.path(), HOOK_INSTRUCTIONS, /*exit_code*/ None)?; + + let mut builder = test_codex() + .with_home(Arc::clone(&home)) + .with_workspace_setup(|cwd, fs| async move { + fs.write_file( + &PathUri::from_host_native_path(cwd.join("AGENTS.md"))?, + PROJECT_INSTRUCTIONS.as_bytes().to_vec(), + /*sandbox*/ None, + ) + .await?; + Ok::<_, anyhow::Error>(()) + }) + .with_config(trust_discovered_hooks); + let test = builder.build_with_auto_env(&server).await?; + + let started = tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let completed = + tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let (started, completed) = match (started.msg, completed.msg) { + (EventMsg::HookStarted(started), EventMsg::HookCompleted(completed)) => { + (started, completed) + } + (started, completed) => panic!( + "expected instruction-resolution hook events immediately after SessionConfigured, got {started:?} then {completed:?}" + ), + }; + assert_eq!(started.turn_id, None); + assert_eq!(completed.turn_id, None); + assert_eq!(started.run.id, completed.run.id); + assert_eq!(started.run.event_name, HookEventName::UserInstructions); + assert_eq!(completed.run.event_name, HookEventName::UserInstructions); + assert_eq!(started.run.source_path, completed.run.source_path); + assert_eq!(started.run.status, HookRunStatus::Running); + assert_eq!(completed.run.status, HookRunStatus::Completed); + + let expected_warning = format!( + "UserInstructions hook output overrides user-level instructions from `{}`.", + global_source.display() + ); + let warning = wait_for_event( + &test.codex, + |event| matches!(event, EventMsg::Warning(warning) if warning.message == expected_warning), + ) + .await; + let EventMsg::Warning(warning) = warning else { + unreachable!("wait_for_event matched a warning") + }; + assert_eq!(warning.message, expected_warning); + + test.submit_turn("hello").await?; + + let contents = format!("{HOOK_INSTRUCTIONS}\n\n--- project-doc ---\n\n{PROJECT_INSTRUCTIONS}"); + let cwd = PathUri::from_abs_path(&test.config.cwd).inferred_native_path_string(); + assert_eq!( + instruction_fragment(&response.single_request()), + format!( + "# AGENTS.md instructions for {cwd}\n\n\n{contents}\n" + ) + ); + + let hook_source = home.path().join("hooks.json").abs(); + let project_source = test.config.cwd.join("AGENTS.md"); + assert_eq!( + test.codex.instruction_sources().await, + vec![ + PathUri::from_abs_path(&hook_source), + PathUri::from_abs_path(&project_source), + ] + ); + let hook_input: Value = serde_json::from_str(&fs::read_to_string( + home.path().join("user_instructions_hook_input.json"), + )?)?; + assert_eq!(hook_input["hook_event_name"], "UserInstructions"); + assert_eq!( + hook_input["session_id"], + test.session_configured.session_id.to_string() + ); + assert_eq!( + hook_input["cwd"], + PathUri::from_abs_path(&test.config.cwd).inferred_native_path_string() + ); + assert_eq!( + hook_input["transcript_path"], + serde_json::json!( + test.session_configured + .rollout_path + .as_ref() + .map(|path| path.display().to_string()) + ) + ); + assert_eq!(hook_input["model"], test.session_configured.model); + assert_eq!(hook_input["permission_mode"], "default"); + assert_eq!(hook_input.get("turn_id"), None); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_instructions_hooks_concatenate_outputs_in_configured_order() -> Result<()> { + // This test uses wiremock's loopback TCP endpoint, which is unavailable + // when the Codex sandbox disables networking. + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let home = Arc::new(TempDir::new()?); + write_user_instructions_hooks( + home.path(), + &[ + ("first hook instructions", None), + ("second hook instructions", None), + ], + )?; + + let mut builder = test_codex() + .with_home(Arc::clone(&home)) + .with_config(trust_discovered_hooks); + let test = builder.build_with_auto_env(&server).await?; + + for _ in 0..2 { + let event = + tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let started = match event.msg { + EventMsg::HookStarted(started) => started, + other => panic!("expected UserInstructions hook start event, got {other:?}"), + }; + assert_eq!(started.run.event_name, HookEventName::UserInstructions); + assert_eq!(started.run.status, HookRunStatus::Running); + } + for _ in 0..2 { + let event = + tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let completed = match event.msg { + EventMsg::HookCompleted(completed) => completed, + other => panic!("expected UserInstructions hook completion event, got {other:?}"), + }; + assert_eq!(completed.run.event_name, HookEventName::UserInstructions); + assert_eq!(completed.run.status, HookRunStatus::Completed); + } + + test.submit_turn("hello").await?; + + assert_eq!( + instruction_fragment(&response.single_request()), + "# AGENTS.md instructions\n\n\nfirst hook instructions\n\nsecond hook instructions\n" + ); + assert_eq!( + test.codex.instruction_sources().await, + vec![PathUri::from_abs_path( + &home.path().join("hooks.json").abs() + )] + ); + let hook_runs = fs::read_to_string(home.path().join("user_instructions_hook_input.json"))?; + assert_eq!(hook_runs.lines().count(), 2); + + Ok(()) +} + +fn body_contains(request: &wiremock::Request, text: &str) -> bool { + serde_json::from_slice::(&request.body).is_ok_and(|body| body.to_string().contains(text)) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn fresh_context_subagent_reruns_user_instructions_hook() -> Result<()> { + // This test uses wiremock's loopback TCP endpoint, which is unavailable + // when the Codex sandbox disables networking. + skip_if_no_network!(Ok(())); + run_subagent_user_instructions_case(/*fork_context*/ false).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn full_context_subagent_reruns_user_instructions_hook() -> Result<()> { + // This test uses wiremock's loopback TCP endpoint, which is unavailable + // when the Codex sandbox disables networking. + skip_if_no_network!(Ok(())); + run_subagent_user_instructions_case(/*fork_context*/ true).await +} + +async fn run_subagent_user_instructions_case(fork_context: bool) -> Result<()> { + const PARENT_PROMPT: &str = "spawn a worker to inspect the instructions"; + const CHILD_PROMPT: &str = "inspect the child instructions"; + const SPAWN_CALL_ID: &str = "spawn-user-instructions-child"; + + let server = start_mock_server().await; + let spawn_args = serde_json::to_string(&serde_json::json!({ + "message": CHILD_PROMPT, + "fork_context": fork_context, + }))?; + mount_sse_once_match( + &server, + |request: &wiremock::Request| body_contains(request, PARENT_PROMPT), + sse(vec![ + ev_response_created("spawn-response"), + ev_function_call_with_namespace( + SPAWN_CALL_ID, + "multi_agent_v1", + "spawn_agent", + &spawn_args, + ), + ev_completed("spawn-response"), + ]), + ) + .await; + let child_response = mount_sse_once_match( + &server, + |request: &wiremock::Request| { + body_contains(request, CHILD_PROMPT) && !body_contains(request, SPAWN_CALL_ID) + }, + sse(vec![ + ev_response_created("child-response"), + ev_assistant_message("child-message", "done"), + ev_completed("child-response"), + ]), + ) + .await; + mount_sse_once_match( + &server, + |request: &wiremock::Request| body_contains(request, SPAWN_CALL_ID), + sse(vec![ + ev_response_created("spawn-follow-up-response"), + ev_assistant_message("spawn-follow-up-message", "child started"), + ev_completed("spawn-follow-up-response"), + ]), + ) + .await; + + let home = Arc::new(TempDir::new()?); + write_user_instructions_hook(home.path(), HOOK_INSTRUCTIONS, /*exit_code*/ None)?; + let mut builder = test_codex() + .with_home(Arc::clone(&home)) + .with_config(trust_discovered_hooks) + .with_config(|config| { + let _ = config.features.enable(Feature::Collab); + let _ = config.features.disable(Feature::EnableRequestCompression); + }); + let test = builder.build_with_auto_env(&server).await?; + + let mut created_threads = test.thread_manager.subscribe_thread_created(); + test.submit_turn(PARENT_PROMPT).await?; + let child_thread_id = + tokio::time::timeout(Duration::from_secs(10), created_threads.recv()).await??; + let child_thread = test.thread_manager.get_thread(child_thread_id).await?; + + let child_request = tokio::time::timeout(Duration::from_secs(10), async { + loop { + if let Some(request) = child_response.last_request() { + break request; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .map_err(|_| anyhow::anyhow!("timed out waiting for the child model request"))?; + assert_eq!( + instruction_fragment(&child_request), + format!("# AGENTS.md instructions\n\n\n{HOOK_INSTRUCTIONS}\n") + ); + assert_eq!( + child_thread.instruction_sources().await, + test.codex.instruction_sources().await + ); + let hook_runs = fs::read_to_string(home.path().join("user_instructions_hook_input.json"))?; + assert_eq!(hook_runs.lines().count(), 2); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn failed_user_instructions_hook_falls_back_to_global_instructions() -> Result<()> { + // This test uses wiremock's loopback TCP endpoint, which is unavailable + // when the Codex sandbox disables networking. + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let home = Arc::new(TempDir::new()?); + let global_source = write_global_instructions(home.path())?; + write_user_instructions_hook(home.path(), "", Some(1))?; + + let mut builder = test_codex() + .with_home(Arc::clone(&home)) + .with_config(trust_discovered_hooks); + let test = builder.build_with_auto_env(&server).await?; + + let started = tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let completed = + tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let warning = tokio::time::timeout(Duration::from_secs(10), test.codex.next_event()).await??; + let (started, completed, warning) = match (started.msg, completed.msg, warning.msg) { + ( + EventMsg::HookStarted(started), + EventMsg::HookCompleted(completed), + EventMsg::Warning(warning), + ) => (started, completed, warning), + (started, completed, warning) => panic!( + "expected failed UserInstructions hook lifecycle events, got {started:?}, {completed:?}, then {warning:?}" + ), + }; + assert_eq!(started.run.id, completed.run.id); + assert_eq!(started.run.status, HookRunStatus::Running); + assert_eq!(completed.run.status, HookRunStatus::Failed); + let expected_warning = format!( + "UserInstructions hook from {} failed: hook exited with code 1", + PathUri::from_abs_path(&completed.run.source_path) + ); + assert_eq!(warning.message, expected_warning); + + test.submit_turn("hello").await?; + + assert_eq!( + instruction_fragment(&response.single_request()), + format!( + "# AGENTS.md instructions\n\n\n{GLOBAL_INSTRUCTIONS}\n" + ) + ); + assert_eq!( + test.codex.instruction_sources().await, + vec![PathUri::from_abs_path(&global_source)] + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_instructions_hook_output_is_not_capped_at_project_doc_limit() -> Result<()> { + // This test uses wiremock's loopback TCP endpoint, which is unavailable + // when the Codex sandbox disables networking. + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let home = Arc::new(TempDir::new()?); + let large_instructions = "uncapped user instruction ".repeat(2_000); + write_user_instructions_hook(home.path(), &large_instructions, /*exit_code*/ None)?; + + let mut builder = test_codex() + .with_home(home) + .with_config(trust_discovered_hooks) + .with_config(|config| config.project_doc_max_bytes = 1); + let test = builder.build_with_auto_env(&server).await?; + test.submit_turn("hello").await?; + + // `core/src/agents_md.rs::load_project_instructions` loads user instructions + // before `read_agents_md` applies `project_doc_max_bytes` only to project docs. + // Hook output replaces that user-level value, so it intentionally remains uncapped. + assert_eq!( + instruction_fragment(&response.single_request()), + format!( + "# AGENTS.md instructions\n\n\n{}\n", + large_instructions.trim() + ) + ); + + Ok(()) +} diff --git a/codex-rs/ext/extension-api/src/user_instructions.rs b/codex-rs/ext/extension-api/src/user_instructions.rs index d5fd63c197..1ff31c02ea 100644 --- a/codex-rs/ext/extension-api/src/user_instructions.rs +++ b/codex-rs/ext/extension-api/src/user_instructions.rs @@ -5,7 +5,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; /// User instructions supplied by the host. /// -/// `source` must be an absolute filesystem path because the app-server +/// `sources` must be absolute filesystem paths because the app-server /// `instructionSources` API currently exposes instruction sources as /// `AbsolutePathBuf` values. // TODO(anp): Replace the absolute path with a more general instruction-source @@ -14,8 +14,8 @@ use codex_utils_absolute_path::AbsolutePathBuf; pub struct UserInstructions { /// Model-visible user instruction text. pub text: String, - /// Absolute filesystem path reported through `instructionSources`. - pub source: AbsolutePathBuf, + /// Absolute filesystem paths reported through `instructionSources`. + pub sources: Vec, } /// Result of loading host-provided user instructions.