openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 17:38:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify agent hook sessions

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-06-04 12:26:45 -07:00
parent 25394befed
commit bc4b55c09c
12 changed files with 50 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex_delegate.rs
+2 -13
View File
@@ -79,17 +79,6 @@ pub(crate) async fn run_codex_thread_interactive(
let (tx_ops, rx_ops) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
let conversation_history = initial_history.unwrap_or(InitialHistory::New);
let forked_from_thread_id = conversation_history.forked_from_id();
let is_agent_hook = crate::hook_agent::is_agent_hook_subagent_source(&subagent_source);
let extensions = if is_agent_hook {
codex_extension_api::empty_extension_registry()
} else {
Arc::clone(&parent_session.services.extensions)
};
let environment_selections = if is_agent_hook {
Default::default()
} else {
parent_ctx.environments.clone()
};
let CodexSpawnOk { codex, .. } = Box::pin(Codex::spawn(CodexSpawnArgs {
config,
installation_id: parent_session.installation_id.clone(),
@@ -99,7 +88,7 @@ pub(crate) async fn run_codex_thread_interactive(
skills_manager: Arc::clone(&parent_session.services.skills_manager),
plugins_manager: Arc::clone(&parent_session.services.plugins_manager),
mcp_manager: Arc::clone(&parent_session.services.mcp_manager),
extensions,
extensions: Arc::clone(&parent_session.services.extensions),
conversation_history,
session_source: SessionSource::SubAgent(subagent_source.clone()),
forked_from_thread_id,
@@ -113,7 +102,7 @@ pub(crate) async fn run_codex_thread_interactive(
inherited_exec_policy: Some(Arc::clone(&parent_session.services.exec_policy)),
parent_rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(),
parent_trace: None,
environment_selections,
environment_selections: parent_ctx.environments.clone(),
analytics_events_client: Some(parent_session.services.analytics_events_client.clone()),
thread_store: Arc::clone(&parent_session.services.thread_store),
attestation_provider: parent_session.services.attestation_provider.clone(),
codex-rs/core/src/hook_agent.rs
-10
View File
@@ -8,7 +8,6 @@ use codex_hooks::AgentHookRunner;
use codex_protocol::config_types::WebSearchMode;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::user_input::UserInput;
use tokio_util::sync::CancellationToken;
@@ -19,7 +18,6 @@ use crate::config::Constrained;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
pub(crate) const AGENT_HOOK_MAX_MODEL_REQUESTS: usize = 50;
const AGENT_HOOK_SOURCE_NAME: &str = "agent_hook";
const AGENT_HOOK_BASE_INSTRUCTIONS: &str = r#"You evaluate a Codex agent hook.
@@ -32,14 +30,6 @@ or
Use ok:false only when the hook criteria fail. Do not answer the user's task. Do not include Markdown or extra text."#;
pub(crate) fn is_agent_hook_source(session_source: &SessionSource) -> bool {
matches!(session_source, SessionSource::SubAgent(source) if is_agent_hook_subagent_source(source))
}
pub(crate) fn is_agent_hook_subagent_source(source: &SubAgentSource) -> bool {
matches!(source, SubAgentSource::Other(name) if name == AGENT_HOOK_SOURCE_NAME)
}
pub(crate) fn build_agent_hook_runner(
parent_session: Arc<Session>,
parent_turn: Arc<TurnContext>,
codex-rs/core/src/hook_agent_tests.rs
-13
View File
@@ -1,23 +1,10 @@
use codex_features::Feature;
use codex_protocol::config_types::WebSearchMode;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn recognizes_only_agent_hook_subagent_source() {
assert!(is_agent_hook_source(&SessionSource::SubAgent(
SubAgentSource::Other("agent_hook".to_string())
)));
assert!(!is_agent_hook_source(&SessionSource::SubAgent(
SubAgentSource::Other("other".to_string())
)));
assert!(!is_agent_hook_source(&SessionSource::Cli));
}
#[tokio::test]
async fn agent_hook_config_is_isolated_and_inherits_active_permissions() {
let (_, turn) = crate::session::tests::make_session_and_context().await;
codex-rs/core/src/session/mod.rs
+17 -39
View File
@@ -505,20 +505,15 @@ impl Codex {
} = args;
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
let (tx_event, rx_event) = async_channel::unbounded();
let is_agent_hook = crate::hook_agent::is_agent_hook_source(&session_source);
let primary_environment = environment_selections.primary_environment();
let mut user_instruction_warnings = Vec::new();
let user_instructions = if is_agent_hook {
None
} else {
AgentsMdManager::new(&config)
.user_instructions(
primary_environment.as_deref(),
&mut user_instruction_warnings,
)
.await
};
let user_instructions = AgentsMdManager::new(&config)
.user_instructions(
primary_environment.as_deref(),
&mut user_instruction_warnings,
)
.await;
config.startup_warnings.extend(user_instruction_warnings);
let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) {
@@ -596,8 +591,6 @@ impl Codex {
config.features.enabled(Feature::FastMode),
&model_info,
);
let max_model_requests_per_turn =
is_agent_hook.then_some(crate::hook_agent::AGENT_HOOK_MAX_MODEL_REQUESTS);
let session_configuration = SessionConfiguration {
provider: config.model_provider.clone(),
collaboration_mode,
@@ -628,7 +621,6 @@ impl Codex {
dynamic_tools,
inherited_shell_snapshot,
user_shell_override,
max_model_requests_per_turn,
};
// Generate a unique ID for the lifetime of this Codex session.
@@ -1491,7 +1483,7 @@ impl Session {
// layers such as request/session overrides that were present when this session
// was created.
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
let (previous_config, new_config, config, service_tier, session_source) = {
let (previous_config, new_config, config, service_tier) = {
let mut state = self.state.lock().await;
let previous_config = notify_config_contributors
.then(|| Self::build_effective_session_config(&state.session_configuration));
@@ -1506,14 +1498,7 @@ impl Session {
state.session_configuration.original_config_do_not_use = Arc::clone(&config);
let new_config = notify_config_contributors
.then(|| Self::build_effective_session_config(&state.session_configuration));
let session_source = state.session_configuration.session_source.clone();
(
previous_config,
new_config,
config,
service_tier,
session_source,
)
(previous_config, new_config, config, service_tier)
};
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
self.services.skills_manager.clear_cache();
@@ -1530,7 +1515,6 @@ impl Session {
self.services.plugins_manager.as_ref(),
self.services.user_shell.as_ref(),
Some(prompt_hook_runner),
&session_source,
)
.await;
@@ -2879,17 +2863,15 @@ impl Session {
developer_sections.push(skills_instructions.render());
}
}
if !crate::hook_agent::is_agent_hook_source(&turn_context.session_source) {
let loaded_plugins = self
.services
.plugins_manager
.plugins_for_config(&turn_context.config.plugins_config_input())
.await;
if let Some(plugin_instructions) =
AvailablePluginsInstructions::from_plugins(loaded_plugins.capability_summaries())
{
developer_sections.push(plugin_instructions.render());
}
let loaded_plugins = self
.services
.plugins_manager
.plugins_for_config(&turn_context.config.plugins_config_input())
.await;
if let Some(plugin_instructions) =
AvailablePluginsInstructions::from_plugins(loaded_plugins.capability_summaries())
{
developer_sections.push(plugin_instructions.render());
}
let context_contributors = self.services.extensions.context_contributors().to_vec();
for contributor in context_contributors {
@@ -3411,11 +3393,7 @@ async fn build_hooks_for_config(
plugins_manager: &PluginsManager,
user_shell: &crate::shell::Shell,
prompt_hook_runner: Option<PromptHookRunner>,
session_source: &SessionSource,
) -> Hooks {
if crate::hook_agent::is_agent_hook_source(session_source) {
return Hooks::default();
}
let mut hook_shell_argv = user_shell.derive_exec_args("", /*use_login_shell*/ false);
let hook_shell_program = hook_shell_argv.remove(0);
let _ = hook_shell_argv.pop();
codex-rs/core/src/session/review.rs
-1
View File
@@ -138,7 +138,6 @@ pub(super) async fn spawn_review_thread(
#[allow(deprecated)]
cwd: parent_turn_context.cwd.clone(),
final_output_json_schema: None,
max_model_requests: parent_turn_context.max_model_requests,
codex_self_exe: parent_turn_context.codex_self_exe.clone(),
codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(),
dynamic_tools: parent_turn_context.dynamic_tools.clone(),
codex-rs/core/src/session/session.rs
-2
View File
@@ -107,7 +107,6 @@ pub(crate) struct SessionConfiguration {
pub(super) dynamic_tools: Vec<DynamicToolSpec>,
pub(super) inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
pub(super) user_shell_override: Option<shell::Shell>,
pub(super) max_model_requests_per_turn: Option<usize>,
}
impl SessionConfiguration {
@@ -989,7 +988,6 @@ impl Session {
plugins_manager.as_ref(),
&default_shell,
Some(prompt_hook_runner),
&session_configuration.session_source,
)
.await;
for warning in hooks.startup_warnings() {
codex-rs/core/src/session/tests.rs
-8
View File
@@ -3187,7 +3187,6 @@ async fn set_rate_limits_retains_previous_credits() {
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let mut state = SessionState::new(session_configuration);
@@ -3296,7 +3295,6 @@ async fn set_rate_limits_updates_plan_type_when_present() {
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let mut state = SessionState::new(session_configuration);
@@ -3829,7 +3827,6 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
}
}
@@ -4574,7 +4571,6 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let (tx_event, _rx_event) = async_channel::unbounded();
@@ -4686,7 +4682,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let per_turn_config =
Session::build_per_turn_config(&session_configuration, session_configuration.cwd.clone());
@@ -4925,7 +4920,6 @@ async fn make_session_with_config_and_rx(
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let (tx_event, rx_event) = async_channel::unbounded();
@@ -5031,7 +5025,6 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
dynamic_tools: Vec::new(),
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let (tx_event, rx_event) = async_channel::unbounded();
@@ -6779,7 +6772,6 @@ where
dynamic_tools,
inherited_shell_snapshot: None,
user_shell_override: None,
max_model_requests_per_turn: None,
};
let per_turn_config =
Session::build_per_turn_config(&session_configuration, session_configuration.cwd.clone());
codex-rs/core/src/session/turn.rs
-24
View File
@@ -193,7 +193,6 @@ pub(crate) async fn run_turn(
let mut last_agent_message: Option<String> = None;
let mut stop_hook_active = false;
let mut model_request_count = 0usize;
// Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains
// many turns, from the perspective of the user, it is a single turn.
#[allow(deprecated)]
@@ -244,20 +243,6 @@ pub(crate) async fn run_turn(
let turn_metadata_header = turn_context
.turn_metadata_state
.current_header_value_for_model_request(&window_id);
if let Some(message) =
model_request_limit_error(turn_context.max_model_requests, model_request_count)
{
sess.send_event(
&turn_context,
EventMsg::Error(ErrorEvent {
message,
codex_error_info: None,
}),
)
.await;
break;
}
model_request_count = model_request_count.saturating_add(1);
match run_sampling_request(
Arc::clone(&sess),
@@ -2249,15 +2234,6 @@ async fn try_run_sampling_request(
outcome
}
fn model_request_limit_error(
max_model_requests: Option<usize>,
model_request_count: usize,
) -> Option<String> {
let limit = max_model_requests?;
(model_request_count >= limit)
.then(|| format!("agent hook exceeded its {limit} model request limit"))
}
pub(crate) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -> Option<String> {
for item in responses.iter().rev() {
if let Some(message) = last_assistant_message_from_item(item, /*plan_mode*/ false) {
codex-rs/core/src/session/turn_context.rs
-3
View File
@@ -93,7 +93,6 @@ pub struct TurnContext {
pub features: ManagedFeatures,
pub(crate) ghost_snapshot: GhostSnapshotConfig,
pub(crate) final_output_json_schema: Option<Value>,
pub(crate) max_model_requests: Option<usize>,
pub(crate) codex_self_exe: Option<PathBuf>,
pub(crate) codex_linux_sandbox_exe: Option<PathBuf>,
pub(crate) truncation_policy: TruncationPolicy,
@@ -269,7 +268,6 @@ impl TurnContext {
features,
ghost_snapshot: self.ghost_snapshot.clone(),
final_output_json_schema: self.final_output_json_schema.clone(),
max_model_requests: self.max_model_requests,
codex_self_exe: self.codex_self_exe.clone(),
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
truncation_policy,
@@ -570,7 +568,6 @@ impl Session {
features: per_turn_config.features.clone(),
ghost_snapshot: per_turn_config.ghost_snapshot.clone(),
final_output_json_schema: None,
max_model_requests: session_configuration.max_model_requests_per_turn,
codex_self_exe: per_turn_config.codex_self_exe.clone(),
codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(),
truncation_policy: model_info.truncation_policy.into(),
codex-rs/core/src/session/turn_tests.rs
-18
View File
@@ -35,24 +35,6 @@ fn assistant_output_text(text: &str) -> ResponseItem {
}
}
#[test]
fn model_request_limit_allows_exactly_the_configured_number_of_requests() {
let limit = crate::hook_agent::AGENT_HOOK_MAX_MODEL_REQUESTS;
assert_eq!(
model_request_limit_error(
/*max_model_requests*/ None, /*model_request_count*/ 100,
),
None
);
assert_eq!(model_request_limit_error(Some(limit), limit - 1), None);
assert_eq!(
model_request_limit_error(Some(limit), limit),
Some(format!(
"agent hook exceeded its {limit} model request limit"
))
);
}
#[tokio::test]
async fn plan_mode_uses_contributed_turn_item_for_last_agent_message() {
let (mut session, turn_context) = crate::session::tests::make_session_and_context().await;
codex-rs/hooks/src/engine/discovery.rs
+30
View File
@@ -1251,6 +1251,36 @@ mod tests {
);
}
#[test]
fn unsupported_agent_hook_is_skipped_during_discovery() {
let mut handlers = Vec::new();
let mut hook_entries = Vec::new();
let mut warnings = Vec::new();
let mut display_order = 0;
let source_path = source_path();
let hook_states = std::collections::HashMap::new();
append_matcher_groups(
&mut handlers,
&mut hook_entries,
&mut warnings,
&mut display_order,
&hook_handler_source(&source_path, &hook_states),
HookEventName::SessionStart,
vec![agent_group(/*matcher*/ None)],
);
assert_eq!(handlers, Vec::<ConfiguredHandler>::new());
assert_eq!(hook_entries.len(), 0);
assert_eq!(
warnings,
vec![format!(
"skipping agent hook in {}: agent hooks are not supported for SessionStart",
source_path.display()
)]
);
}
#[test]
fn toml_hook_discovery_ignores_malformed_state_entries() {
let layer = ConfigLayerEntry::new(
codex-rs/hooks/src/engine/prompt_runner.rs
+1 -1
View File
@@ -72,7 +72,7 @@ pub(crate) fn model_hook_behavior(event_name: HookEventName) -> ModelHookBehavio
// continueOnBlock feeds the reason back to the model, otherwise it stops
// the current turn.
HookEventName::PostToolUse => ModelHookBehavior::FeedbackOrStop,
// Claude does not support prompt hooks for these lifecycle events.
// Claude does not support model-backed hooks for these lifecycle events.
// Keeping them explicit makes new events choose semantics deliberately.
HookEventName::SessionStart
| HookEventName::SubagentStart